Alon Bar-Lev has posted comments on this change.

Change subject: aaa: add support for basic athentication
......................................................................


Patch Set 12:

(5 comments)

http://gerrit.ovirt.org/#/c/37299/12/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/ExternalAuthServlet.java
File 
backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/ExternalAuthServlet.java:

Line 83:         SSOConfig config = (SSOConfig) 
request.getServletContext().getAttribute(SSOUtils.SSO_CONFIG);
Line 84:         Credentials credentials = null;
Line 85:         if (config.isAcceptBasicAuthHeaders()) {
Line 86:             credentials = 
SSOUtils.getUserCredentialsFromHeader(request);
Line 87:         }
not sure I would have gotten credentials before I must
Line 88:         if (doAuth(request, response)) {
Line 89:             response.sendRedirect(request.getContextPath() + 
SSOUtils.LOGIN_PHASE4_URI);
Line 90:         } else if (credentials != null) {
Line 91:             
request.getSession(true).setAttribute(SSOUtils.USER_CREDENTIALS, credentials);


Line 89:             response.sendRedirect(request.getContextPath() + 
SSOUtils.LOGIN_PHASE4_URI);
Line 90:         } else if (credentials != null) {
Line 91:             
request.getSession(true).setAttribute(SSOUtils.USER_CREDENTIALS, credentials);
Line 92:             response.sendRedirect(request.getContextPath() + 
SSOUtils.LOGIN_PHASE3_URI);
Line 93:         } else if (config.isEnforceNego()) {
we still need to figure out how to process user escape (For example 3 times and 
we should proceed anyway to login), store in session? not sure how this is done 
in java.
Line 94:             enforceNego(response);
Line 95:         } else {
Line 96:             response.sendRedirect(request.getContextPath() + 
SSOUtils.LOGIN_PHASE2_URI);
Line 97:         }


Line 97:         }
Line 98:     }
Line 99: 
Line 100:     private void enforceNego(HttpServletResponse response) throws 
IOException {
Line 101:         for (String scheme:  new HashSet<>(schemes)) {
why can't the schemes be hash from starters?
Line 102:             response.setHeader("WWW-Authenticate", scheme);
Line 103:         }
Line 104:         response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
Line 105:     }


http://gerrit.ovirt.org/#/c/37299/12/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/UnauthorizedServlet.java
File 
backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/UnauthorizedServlet.java:

Line 17:     protected void service(HttpServletRequest request, 
HttpServletResponse response)
Line 18:             throws ServletException, IOException {
Line 19:         SSOConfig config = (SSOConfig) 
request.getServletContext().getAttribute(SSOUtils.SSO_CONFIG);
Line 20:         if (config.isEnforceNego()) {
Line 21:             
request.getRequestDispatcher(SSOUtils.LOGIN_EXTERNAL_URI).forward(request, 
response);
not sure how you can forward, we need the apache to know that we are on 
external uri
Line 22:         } else {
Line 23:             
request.getRequestDispatcher("/WEB-INF/login.jsp").forward(request, response);
Line 24:         }
Line 25:     }


http://gerrit.ovirt.org/#/c/37299/12/packaging/services/ovirt-engine/ovirt-engine.conf.in
File packaging/services/ovirt-engine/ovirt-engine.conf.in:

Line 253: 
Line 254: SSO_WELCOME_URL=/ovirt-engine/
Line 255: SSO_VERSION=0
Line 256: SSO_ENABLE_EXTERNAL_AUTH=true
Line 257: SSO_ENFORCE_NEGO=true
default should be false
Line 258: SSO_ACCEPT_BASIC_AUTH_HEADERS=true


-- 
To view, visit http://gerrit.ovirt.org/37299
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If09285f0e6cd8909f21aa7e88ae1a3c1a30763c2
Gerrit-PatchSet: 12
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Ravi Nori <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to