Ravi Nori has posted comments on this change.

Change subject: aaa : Add engine sso
......................................................................


Patch Set 49:

(2 comments)

https://gerrit.ovirt.org/#/c/36119/49/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/SSOQueryServlet.java
File 
backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/SSOQueryServlet.java:

Line 49:                 if (!SSOUtils.versionSupported(version)) {
Line 50:                     throw new Exception("Unsupported version found in 
request.");
Line 51:                 }
Line 52:             }
Line 53:             switch (command) {
> can't we have some kind of map between command an reference to method? no n
I changed to something like this

private enum Command {
        AuthnCredentials(
                new Logic() {
                    public void execute(HttpServletRequest request, 
HttpServletResponse response) throws Exception {
                        authCredentials(request, response);
                    }
                }
        ),
       ..........;

        private interface Logic {
            void execute(HttpServletRequest request, HttpServletResponse 
response) throws Exception;
        }

        private Logic logic;

        private Command(Logic logic) {
            this.logic = logic;
        }

        public void execute(HttpServletRequest request, HttpServletResponse 
response) throws Exception {
            logic.execute(request, response);
        }
    }
Line 54:                 case AUTHN_CREDENTIALS:
Line 55:                     authCredentials(request, response);
Line 56:                     break;
Line 57:                 case GET_TOKEN_PAYLOAD:


Line 57:                 case GET_TOKEN_PAYLOAD:
Line 58:                     getTokenPayload(request, response);
Line 59:                     break;
Line 60:                 case INVALIDATE_SESSION:
Line 61:                     HttpSession existingSession = 
request.getSession(false);
> not sure how invalidate does not take sso token.
We do not have a map token to session, only token to session data exists.

So session.invalidate needs to be invoked only by the client. 

Do we want to have token to session mapping?
Line 62:                     if (existingSession != null) {
Line 63:                         existingSession.invalidate();
Line 64:                     }
Line 65:                     break;


-- 
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

Reply via email to