Ravi Nori has posted comments on this change. Change subject: aaa : Add engine sso ......................................................................
Patch Set 59: (9 comments) https://gerrit.ovirt.org/#/c/36119/59/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/SSOServiceBatchServlet.java File backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/SSOServiceBatchServlet.java: Line 75: public void init() throws ServletException { Line 76: commandsMap = new HashMap<>(); Line 77: for (Command cmd : Command.values()) { Line 78: commandsMap.put(cmd.getName(), cmd); Line 79: } > stupid question... as you get the request/response as parameters, won't it Done Line 80: } Line 81: Line 82: @Override Line 83: protected void service(HttpServletRequest request, HttpServletResponse response) Line 214: Line 215: private static String getSsoToken(HttpServletRequest request) { Line 216: String ssoToken = request.getParameter("sso_token"); Line 217: if (StringUtils.isEmpty(ssoToken)) { Line 218: ssoToken = request.getHeader("sso_token"); > OVIRT-SSO-TOKEN Done Line 219: } Line 220: if (StringUtils.isEmpty(ssoToken)) { Line 221: ssoToken = SSOUtils.getSsoTokenForRequestId(request.getServletContext(), request.getParameter("request_id")); Line 222: } Line 218: ssoToken = request.getHeader("sso_token"); Line 219: } Line 220: if (StringUtils.isEmpty(ssoToken)) { Line 221: ssoToken = SSOUtils.getSsoTokenForRequestId(request.getServletContext(), request.getParameter("request_id")); Line 222: } > I do not understand how getSsoToken() relates to the request id... probably Done Line 223: if (StringUtils.isEmpty(ssoToken)) { Line 224: ssoToken = SSOUtils.getSsoTokenForRequestId(request.getServletContext(), request.getHeader("request_id")); Line 225: } Line 226: return ssoToken; https://gerrit.ovirt.org/#/c/36119/59/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 49: setRequestId(request, response); Line 50: } Line 51: public String getName() { Line 52: return "request-id"; Line 53: } > what is this command? why do we need it? lets engine set request-id in session Line 54: }, Line 55: Switch() { Line 56: public void execute(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { Line 57: switchUser(request, response); Line 116: } Line 117: Line 118: private static void login(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { Line 119: log.debug("Entered login queryString: {}", request.getQueryString()); Line 120: if (StringUtils.isEmpty(request.getParameter(SSOUtils.POST_COMMAND_URL))) { > what about the error? The exception thrown from the methods are captured by the service method and error and debug of the exception is logged. Line 121: throw new RuntimeException("No post command url found in request."); Line 122: } Line 123: Line 124: if (!request.getParameterMap().isEmpty()) { Line 146: } Line 147: Line 148: private static void logout(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { Line 149: log.debug("Entered logout queryString: {}", request.getQueryString()); Line 150: HttpSession existingSession = request.getSession(false); > post command, error should apply here as well Done Line 151: if (existingSession != null) { Line 152: Map<String, Object> sessionData = SSOUtils.getSessionData(existingSession); Line 153: AuthenticationProfileRepository repo = (AuthenticationProfileRepository) existingSession.getServletContext().getAttribute(SSOUtils.AUTH_PROFILE_REPOSITORY); Line 154: if (sessionData != null && repo != null && sessionData.containsKey(SSOUtils.SSO_PROFILE_ATTR_NAME)) { Line 171: } Line 172: SSOUtils.redirectToModule(request, response); Line 173: } Line 174: Line 175: private static void setRequestId(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { > not sure why we need this >From what I understand, engine generates a request id and sets it in session. This command is invoked to set the request id in session before query entity is invoked Line 176: log.debug("Performing set request id queryString: {}", request.getQueryString()); Line 177: if (StringUtils.isEmpty(request.getParameter(SSOUtils.POST_COMMAND_URL))) { Line 178: throw new RuntimeException("No post command url found in request."); Line 179: } https://gerrit.ovirt.org/#/c/36119/59/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 279: put(requestId, (String) request.getSession(true).getAttribute(SSOUtils.SSO_TOKEN)); Line 280: } Line 281: } Line 282: Line 283: public static void removeRequestIdFromContext(ServletContext ctx, String requestId) { > I did not follow... but we should probably perform house keeping these, for The request id is valid only for 1 request so I remove it after the request is processed Line 284: if (StringUtils.isNotEmpty(requestId)) { Line 285: ((Map<String, String>) ctx.getAttribute(SSOUtils.REQUEST_ID_TO_TOKEN_MAP)).remove(requestId); Line 286: } Line 287: } https://gerrit.ovirt.org/#/c/36119/59/ovirt-engine.spec.in File ovirt-engine.spec.in: Line 641: # Line 642: # Force TLS/SSL for selected applications. Line 643: # Line 644: for war in \ Line 645: "%{buildroot}%{engine_ear}"/{userportal,webadmin,enginesso,welcome}.war \ > welcome does not belong to this patch Done Line 646: "%{buildroot}%{engine_restapi_war}" \ Line 647: "%{buildroot}%{engine_legacy_restapi_war}"; do Line 648: sed -i \ Line 649: -e 's#<transport-guarantee>NONE</transport-guarantee>#<transport-guarantee>CONFIDENTIAL</transport-guarantee>#' \ -- 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: 59 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
