[GitHub] [pulsar-manager] shiv4289 commented on a change in pull request #434: SAML Integration for Login

[GitBox <gi...@apache.org>]

shiv4289 commented on a change in pull request #434:
URL: https://github.com/apache/pulsar-manager/pull/434#discussion_r765586155



##########
File path: src/main/java/org/apache/pulsar/manager/controller/SSOController.java
##########
@@ -0,0 +1,131 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.pulsar.manager.controller;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+import java.net.URI;
+import java.util.Map;
+import java.util.Optional;
+import javax.annotation.Resource;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.http.client.utils.URIBuilder;
+import org.apache.http.util.Asserts;
+import org.apache.pulsar.manager.entity.RoleBindingEntity;
+import org.apache.pulsar.manager.entity.RoleBindingRepository;
+import org.apache.pulsar.manager.entity.RolesRepository;
+import org.apache.pulsar.manager.entity.UserInfoEntity;
+import org.apache.pulsar.manager.entity.UsersRepository;
+import org.apache.pulsar.manager.service.JwtService;
+import org.apache.pulsar.manager.utils.SAMLParser;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.ResponseBody;
+import org.springframework.web.bind.annotation.RestController;
+
+@Slf4j
+@RestController
+@RequestMapping(value = "/pulsar-manager/saml")
+@Api(description = "Calling the request below this class does not require authentication because " +
+        "the user has not logged in yet.")
+@Validated
+public class SSOController {
+
+
+    @Autowired
+    private UsersRepository usersRepository;
+
+    @Autowired
+    private JwtService jwtService;
+
+    @Resource
+    private RolesRepository rolesRepository;
+
+    @Resource
+    private RoleBindingRepository roleBindingRepository;
+
+    @Value("${sso.redirect.host}")
+    private String redirectHost;
+
+    @Value("${sso.redirect.port}")
+    private int redirectPort;
+
+    @Value("${sso.redirect.scheme}")
+    private String redirectUrlScheme;
+
+    @Value("${sso.certificate}")
+    private String certificate;
+
+    @Value("${sso.whitelisted.domain}")
+    private String whiteListedDomain;
+
+    @ApiOperation(value = "Verify SAML Response")
+    @ApiResponses({
+            @ApiResponse(code = 200, message = "ok"),
+            @ApiResponse(code = 404, message = "Not found"),
+            @ApiResponse(code = 500, message = "Internal server error")
+    })
+    @RequestMapping(value = "/sso", method = RequestMethod.POST)
+    @ResponseBody
+    public void loginByIDP(HttpServletRequest request, HttpServletResponse response) throws Exception {
+        String samlResponse = request.getParameter("SAMLResponse");
+        SAMLParser samlParser = new SAMLParser(certificate);
+        Map<String,String> userDetails = samlParser.getUserDetailsFromSAMLResponse(samlResponse);
+        Asserts.check(StringUtils.contains(userDetails.get("email"),whiteListedDomain),"Login with domain not supported");

Review comment:
       Suggest to check the email format in string(using [InternetAddress](https://stackoverflow.com/questions/624581/what-is-the-best-java-email-address-validation-method)()?) in addition to the domain. 

##########
File path: src/main/java/org/apache/pulsar/manager/controller/SSOController.java
##########
@@ -0,0 +1,131 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.pulsar.manager.controller;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+import java.net.URI;
+import java.util.Map;
+import java.util.Optional;
+import javax.annotation.Resource;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.http.client.utils.URIBuilder;
+import org.apache.http.util.Asserts;
+import org.apache.pulsar.manager.entity.RoleBindingEntity;
+import org.apache.pulsar.manager.entity.RoleBindingRepository;
+import org.apache.pulsar.manager.entity.RolesRepository;
+import org.apache.pulsar.manager.entity.UserInfoEntity;
+import org.apache.pulsar.manager.entity.UsersRepository;
+import org.apache.pulsar.manager.service.JwtService;
+import org.apache.pulsar.manager.utils.SAMLParser;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.ResponseBody;
+import org.springframework.web.bind.annotation.RestController;
+
+@Slf4j
+@RestController
+@RequestMapping(value = "/pulsar-manager/saml")
+@Api(description = "Calling the request below this class does not require authentication because " +
+        "the user has not logged in yet.")
+@Validated
+public class SSOController {
+
+
+    @Autowired
+    private UsersRepository usersRepository;
+
+    @Autowired
+    private JwtService jwtService;
+
+    @Resource
+    private RolesRepository rolesRepository;
+
+    @Resource
+    private RoleBindingRepository roleBindingRepository;
+
+    @Value("${sso.redirect.host}")
+    private String redirectHost;
+
+    @Value("${sso.redirect.port}")
+    private int redirectPort;
+
+    @Value("${sso.redirect.scheme}")
+    private String redirectUrlScheme;
+
+    @Value("${sso.certificate}")
+    private String certificate;
+
+    @Value("${sso.whitelisted.domain}")
+    private String whiteListedDomain;
+
+    @ApiOperation(value = "Verify SAML Response")
+    @ApiResponses({
+            @ApiResponse(code = 200, message = "ok"),
+            @ApiResponse(code = 404, message = "Not found"),
+            @ApiResponse(code = 500, message = "Internal server error")
+    })
+    @RequestMapping(value = "/sso", method = RequestMethod.POST)
+    @ResponseBody
+    public void loginByIDP(HttpServletRequest request, HttpServletResponse response) throws Exception {
+        String samlResponse = request.getParameter("SAMLResponse");

Review comment:
       Suggest to check the call is a SAML response before using it.
   
   Moreover, lets be neutral regarding the sso method here? The config and api endpoint are generic. The implementation assumes SAML. We should check the type of auth and switch to a branch of code (factory?) which gives us (authenticated / not authenticated) as a result.

##########
File path: src/main/java/org/apache/pulsar/manager/interceptor/WebAppConfigurer.java
##########
@@ -51,6 +51,7 @@ public void addInterceptors(InterceptorRegistry registry) {
                 .excludePathPatterns("/doc.html")
                 // BKVM
                 .excludePathPatterns("/bkvm")
+                .excludePathPatterns("/pulsar-manager/saml/**")

Review comment:
       Also why ** ? can we do with just /sso without ** ?

##########
File path: src/main/java/org/apache/pulsar/manager/interceptor/WebAppConfigurer.java
##########
@@ -51,6 +51,7 @@ public void addInterceptors(InterceptorRegistry registry) {
                 .excludePathPatterns("/doc.html")
                 // BKVM
                 .excludePathPatterns("/bkvm")
+                .excludePathPatterns("/pulsar-manager/saml/**")

Review comment:
       our endpoint is /sso right? Should this be /pulsar-manager/sso/**




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org