Alon Bar-Lev has posted comments on this change. Change subject: aaa: Add Bearer and Negotiate auth filters ......................................................................
Patch Set 1: (5 comments) https://gerrit.ovirt.org/#/c/42292/1/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/SSOOAuthServiceUtils.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/SSOOAuthServiceUtils.java: Line 66: resp.encodeURL("password"), Line 67: resp.encodeURL((String) payload.get("username")), Line 68: resp.encodeURL(""), Line 69: resp.encodeURL(scope), Line 70: resp.encodeURL(jsonPayload))); what is payload? Line 71: return getData(connection); Line 72: } finally { Line 73: if (connection != null) { Line 74: connection.disconnect(); https://gerrit.ovirt.org/#/c/42292/1/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/SSORestApiBasicAuthFilter.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/SSORestApiBasicAuthFilter.java: Line 14: private final Logger log = LoggerFactory.getLogger(getClass()); Line 15: private static final String scope = "ovirt-app-api"; Line 16: Line 17: protected void authenticateWithSSO(HttpServletRequest req, HttpServletResponse res) throws ServletException { Line 18: try { don't you need to check here if this is "Basic"? same as you have done with Bearer? Line 19: Map<String, Object> response = SSOOAuthServiceUtils.authenticate(req, res, scope); Line 20: FiltersHelper.isStatusOk(response); Line 21: createUserSession(req, FiltersHelper.getPayloadForToken(req, res, (String) response.get("access_token"))); Line 22: } catch (Exception e) { https://gerrit.ovirt.org/#/c/42292/1/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/SSORestApiBearerAuthFilter.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/SSORestApiBearerAuthFilter.java: Line 17: String headerValue = req.getHeader(FiltersHelper.Constants.HEADER_AUTHORIZATION); Line 18: if (headerValue != null && headerValue.startsWith("Bearer ")) { Line 19: try { Line 20: createUserSession(req, FiltersHelper.getPayloadForToken(req, res, Line 21: new String(Base64.decodeBase64(headerValue.substring("Bearer".length())), Charset.forName("UTF-8")) why do you decode the Bearer? Line 22: )); Line 23: } catch (Exception e) { Line 24: log.error(e.getMessage()); Line 25: log.debug("Bearer Authentication with SSO failed", e); https://gerrit.ovirt.org/#/c/42292/1/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/SSORestApiNegotiationFilter.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/SSORestApiNegotiationFilter.java: Line 99: } Line 100: Map<String, Object> payload = doAuth(req, resp); Line 101: Line 102: if (!payload.isEmpty()) { Line 103: Map<String, Object> response = SSOOAuthServiceUtils.loginOnBehalf(req, resp, payload, scope); ok, I understand what payload now... all we need to have is user@profile and we can construct it out of the auth record. not sure we need the others, but if we do, we need to provide some more post parameters the missing bit is expiration. please note that authz access should be done at the sso side not here... all you need to do is to perform authentication. Line 104: FiltersHelper.isStatusOk(response); Line 105: createUserSession(req, FiltersHelper.getPayloadForToken(req, resp, (String) response.get("access_token"))); Line 106: } Line 107: } catch (Exception e) { Line 154: Authn.InvokeKeys.AUTH_RECORD, Line 155: authRecord Line 156: ); Line 157: } Line 158: ExtMap outputMap = profile.getAuthz().invoke(new ExtMap().mput( this ^ is not required, access to authz should be done at sso side, also the above mapper. Line 159: Base.InvokeKeys.COMMAND, Line 160: Authz.InvokeCommands.FETCH_PRINCIPAL_RECORD Line 161: ).mput( Line 162: Authn.InvokeKeys.AUTH_RECORD, -- To view, visit https://gerrit.ovirt.org/42292 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idee5137430cefa7ca99c047cfd2d550222e5809a Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
