Alon Bar-Lev has posted comments on this change. Change subject: aaa : Add engine sso ......................................................................
Patch Set 56: (4 comments) https://gerrit.ovirt.org/#/c/36119/56/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/SSOServiceInteractiveServlet.java File backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/SSOServiceInteractiveServlet.java: Line 178: } Line 179: existingSession.invalidate(); Line 180: } Line 181: if (StringUtils.isNotEmpty(request.getParameter(SSOUtils.POST_LOGOUT_URL))) { Line 182: response.sendRedirect(request.getParameter(SSOUtils.POST_LOGOUT_URL)); post command url instead of post logout url to be consistent. also, missing opaque all over in redirects. Line 183: } else { Line 184: response.sendRedirect(((SSOConfig) request.getServletContext().getAttribute(SSOUtils.SSO_CONFIG)).getSsoPostLoginDefaultUrl()); Line 185: } Line 186: } Line 180: } Line 181: if (StringUtils.isNotEmpty(request.getParameter(SSOUtils.POST_LOGOUT_URL))) { Line 182: response.sendRedirect(request.getParameter(SSOUtils.POST_LOGOUT_URL)); Line 183: } else { Line 184: response.sendRedirect(((SSOConfig) request.getServletContext().getAttribute(SSOUtils.SSO_CONFIG)).getSsoPostLoginDefaultUrl()); this should be a function that takes url and returns default if empty. so: response.sendRedirect(getRedirectURL(request.getParameter(SSOUtils.POST_COMMAND_URL)); Line 185: } Line 186: } Line 187: Line 188: public static void getSsoVersion(HttpServletResponse response) throws IOException, ServletException { https://gerrit.ovirt.org/#/c/36119/56/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 33: public static final String OPAQUE = "opaque"; Line 34: public static final String VERSION = "version"; Line 35: public static final String SSO_VERSION = "0"; Line 36: public static final String POST_LOGOUT_URL = "post_logout_url"; Line 37: public static final String POST_COMMAND_URL = "post_command_url"; can we merge these two? do we need these at SSOUtils? they are not actually to be shared. Line 38: public static final String SSO_CONFIG = "config"; Line 39: public static final String SSO_SESSION_DATA = "session_data"; Line 40: public static final String SSO_SESSIONS = "sessions"; Line 41: public static final String AUTH_PROFILE_REPOSITORY = "auth_profile_repository"; Line 61: return sessionData.get(SSO_PRINCIPAL_RECORD_ATTR_NAME) != null && Line 62: "true".equals(sessionData.get(SSO_IS_EXTERNAL_AUTHN)); Line 63: } Line 64: Line 65: public static void redirectToModule(HttpSession session, probably this should go into the interactive interface. Line 66: HttpServletRequest request, Line 67: HttpServletResponse response) Line 68: throws IOException { Line 69: try { -- To view, visit https://gerrit.ovirt.org/36119 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4894fc12653027271b6abd4dd5313b10593703fa Gerrit-PatchSet: 56 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: Oved Ourfali <oourf...@redhat.com> 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
