Alon Bar-Lev has posted comments on this change. Change subject: core, webadmin: Modify webadmin to use enginesso for authentication ......................................................................
Patch Set 3: (2 comments) http://gerrit.ovirt.org/#/c/36619/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/aaa/LogoutUserCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/aaa/LogoutUserCommand.java: Line 44: } Line 45: SessionDataContainer.getInstance().removeSessionOnLogout(getParameters().getSessionId()); Line 46: } Line 47: Line 48: if (SessionDataContainer.getInstance().getUser(getParameters().getSessionId(), false) != null) { > In the new sso sequence the session in SessionDataContained contains only u hmmm... this is a bug!!!!!!!!!!! no? SessionDataContainer.getInstance().removeSessionOnLogout(getParameters().getSessionId()); should be called unconditionally. Line 49: SessionDataContainer.getInstance().removeSessionOnLogout(getParameters().getSessionId()); Line 50: } Line 51: setSucceeded(true); Line 52: } http://gerrit.ovirt.org/#/c/36619/3/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/SSOLoginFilter.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/SSOLoginFilter.java: Line 48: HttpSession session = ((HttpServletRequest) request).getSession(false); Line 49: if (session == null || Line 50: session.getAttribute(SessionConstants.HTTP_SESSION_ENGINE_SESSION_ID_KEY) == null || Line 51: !SSOUtils.isSessionValid((String) session.getAttribute(SessionConstants.HTTP_SESSION_ENGINE_SESSION_ID_KEY))) { Line 52: request.getRequestDispatcher(loginUrl + "&app_url=" + ((HttpServletResponse) response).encodeURL(requestUrl)).forward(request, response); > If we redirect the URL will change, I am not a big fan of that not sure I understand why url change is bad in this case. Line 53: } else { Line 54: chain.doFilter(request, response); Line 55: } Line 56: } -- To view, visit http://gerrit.ovirt.org/36619 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iff0aee9d0f5ee606ff7f397cab69017ca7d9df08 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@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
