Alon Bar-Lev has posted comments on this change. Change subject: core : Add engine sso ......................................................................
Patch Set 22: (4 comments) > Handled all comments except your "opaque" app url comment why? it should be the simplest... just rename the parameter name? or I am missing something. http://gerrit.ovirt.org/#/c/36119/22/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 19: public static final String SSO_PRINCIPAL_RECORD_ATTR_NAME = "PRINCIPAL_RECORD"; Line 20: public static final String SSO_AUTH_RECORD_ATTR_NAME = "AUTH_RECORD"; Line 21: public static final String APP_URL = "app_url"; Line 22: public static final String POST_LOGIN_URL = "post_login_url"; Line 23: public static final String REAUTHENTICATE = "reauthenticate"; do we still need this? Line 24: public static final String EXTENSION_MANAGER = "ext_manager"; Line 25: public static final String SSO_LOCAL_CONFIG = "localConfig"; Line 26: Line 27: public static boolean isUserAuthenticated(HttpSession session) { Line 43: .enableDefaultTyping(ObjectMapper.DefaultTyping.OBJECT_AND_NON_CONCRETE); Line 44: mapper.getSerializationConfig().addMixInAnnotations(ExtMap.class, JsonExtMapMixIn.class); Line 45: appendParameter(response, params, "principalRecord", mapper.writeValueAsString(principalRecord)); Line 46: appendParameter(response, params, "authRecord", mapper.writeValueAsString(session.getAttribute(SSOUtils.SSO_AUTH_RECORD_ATTR_NAME))); Line 47: appendParameter(response, params, "principalRecord", new ObjectMapper().writeValueAsString(principalRecord)); twice? Line 48: appendParameter(response, params, "authRecord", new ObjectMapper().writeValueAsString(session.getAttribute(SSOUtils.SSO_AUTH_RECORD_ATTR_NAME))); Line 49: redirectUrl.append(params.toString()); Line 50: response.sendRedirect(redirectUrl.toString()); Line 51: } Line 44: mapper.getSerializationConfig().addMixInAnnotations(ExtMap.class, JsonExtMapMixIn.class); Line 45: appendParameter(response, params, "principalRecord", mapper.writeValueAsString(principalRecord)); Line 46: appendParameter(response, params, "authRecord", mapper.writeValueAsString(session.getAttribute(SSOUtils.SSO_AUTH_RECORD_ATTR_NAME))); Line 47: appendParameter(response, params, "principalRecord", new ObjectMapper().writeValueAsString(principalRecord)); Line 48: appendParameter(response, params, "authRecord", new ObjectMapper().writeValueAsString(session.getAttribute(SSOUtils.SSO_AUTH_RECORD_ATTR_NAME))); twice? Line 49: redirectUrl.append(params.toString()); Line 50: response.sendRedirect(redirectUrl.toString()); Line 51: } Line 52: Line 57: } Line 58: return accumulator; Line 59: } Line 60: Line 61: private static void appendParameter(HttpServletResponse response, StringBuilder params, String name, String value) { ok, this should go into single parameter in json format and not as query string. so best to use a map k, v and transform into json, I've done this at[1] and it looks simple enough. [1] http://gerrit.ovirt.org/#/c/36401/5/backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/crypto/ticket/TicketEncoder.java,cm Line 62: params.append("&"); Line 63: params.append(name); Line 64: params.append("="); Line 65: params.append(response.encodeURL(value)); -- 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: 22 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
