Alon Bar-Lev has posted comments on this change. Change subject: aaa : Add engine sso ......................................................................
Patch Set 30: (2 comments) http://gerrit.ovirt.org/#/c/36119/30/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/LoginPhase1Servlet.java File backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/LoginPhase1Servlet.java: Line 18: protected void service(HttpServletRequest request, HttpServletResponse response) Line 19: throws ServletException, IOException { Line 20: if (StringUtils.isEmpty(request.getParameter(SSOUtils.POST_ACTION_URL))) { Line 21: throw new RuntimeException("No post action url found in request."); Line 22: } I suggest to pass version= into the query parameters to enable protocol version. if unsupported display an error. newer sso should support older versions, for now we will support version 0 only. Line 23: Line 24: request.getSession(true).setAttribute(SSOUtils.PARAMS_MAP, new HashMap<>(request.getParameterMap())); Line 25: response.sendRedirect(request.getContextPath() + SSOUtils.LOGIN_EXTERNAL_URI); Line 26: http://gerrit.ovirt.org/#/c/36119/30/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/utils/SSOUtils.java File backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/utils/SSOUtils.java: Line 71: payload.put("authRecord", session.getAttribute(SSOUtils.SSO_AUTH_RECORD_ATTR_NAME)); Line 72: Line 73: ObjectMapper mapper = new ObjectMapper().configure(Feature.FAIL_ON_UNKNOWN_PROPERTIES, false) Line 74: .enableDefaultTyping(ObjectMapper.DefaultTyping.OBJECT_AND_NON_CONCRETE); Line 75: mapper.getSerializationConfig().addMixInAnnotations(ExtMap.class, JsonExtMapMixIn.class); I suggest we add version field into the payload, so that we will be able to check compatibility at remote. I suggest to pass version= into the query parameters as well to enable protocol version. Line 76: if (StringUtils.isNotEmpty(SSOUtils.getParameter(request, OPAQUE))) { Line 77: redirectUrl.append("&opaque="); Line 78: redirectUrl.append(response.encodeURL(SSOUtils.getParameter(request, OPAQUE))); Line 79: } -- To view, visit http://gerrit.ovirt.org/36119 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4894fc12653027271b6abd4dd5313b10593703fa Gerrit-PatchSet: 30 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Ravi Nori <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
