Alon Bar-Lev has posted comments on this change. Change subject: aaa : Add engine sso ......................................................................
Patch Set 49: (4 comments) in principal, the debug should be more like checkpoints than a story, in regard to format, not that important, see example. https://gerrit.ovirt.org/#/c/36119/49/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/GetSsoTokenServlet.java File backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/GetSsoTokenServlet.java: Line 35: HttpSession session = request.getSession(true); Line 36: if (SSOUtils.isUserAuthenticated(session)) { Line 37: redirectUrl.append(response.encodeURL(SSOUtils.getSsoToken(session))); Line 38: } else { Line 39: redirectUrl.append(""); strange... what is the effect of append ""? Line 40: } Line 41: Line 42: response.sendRedirect(redirectUrl.toString()); Line 43: } https://gerrit.ovirt.org/#/c/36119/49/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 21: Line 22: @Override Line 23: protected void service(HttpServletRequest request, HttpServletResponse response) Line 24: throws ServletException, IOException { Line 25: log.debug("Entered LoginPhase1Servlet query string = %s", request.getQueryString()); please try to avoid adding spaces between debug fields... please remember we are log4j.. :) I suggest: LoginPhase1Servlet entry queryString='{}' Line 26: if (StringUtils.isEmpty(request.getParameter(SSOUtils.POST_ACTION_URL))) { Line 27: throw new RuntimeException("No post action url found in request."); Line 28: } Line 29: Line 49: SSOConfig config = (SSOConfig) request.getServletContext().getAttribute(SSOUtils.SSO_CONFIG); Line 50: String externalParamValue = request.getParameter("external"); Line 51: boolean disableExternal = config.isAllowDisableExternalAuth() && Line 52: StringUtils.isEmpty(externalParamValue) ? false : "0".equals(externalParamValue); Line 53: if (config.isEnableExternalAuth() && !disableExternal) { . String redirect; if () { } else { } log.debug("LoginPhase1Servlet redirect url='{}'", redirect); Line 54: log.debug("Redirected to ExternalAuthServlet"); Line 55: response.sendRedirect(request.getContextPath() + SSOUtils.LOGIN_EXTERNAL_URI); Line 56: } else { Line 57: log.debug("Redirected to LoginPhase2Servlet"); https://gerrit.ovirt.org/#/c/36119/49/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 88: String ssoTokenId; Line 89: try { Line 90: byte s[] = new byte[64]; Line 91: SecureRandom.getInstance("SHA1PRNG").nextBytes(s); Line 92: ssoTokenId = new Base64(0, new byte[0], true).encodeToString(s);; double ;; Line 93: } catch (NoSuchAlgorithmException e) { Line 94: throw new RuntimeException(e); Line 95: } Line 96: return ssoTokenId; -- 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: 49 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
