Alon Bar-Lev has posted comments on this change.

Change subject: core, webadmin: Modify webadmin to use enginesso for 
authentication
......................................................................


Patch Set 8:

(4 comments)

http://gerrit.ovirt.org/#/c/36619/8/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/SSOLoginFilter.java
File 
backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/SSOLoginFilter.java:

Line 33:         String queryString = req.getQueryString();
Line 34:         String requestUrl = req.getRequestURI() + 
(StringUtils.isEmpty(queryString) ? "" : "?" + queryString);
Line 35: 
Line 36:         if (!FiltersHelper.isAuthenticated(req)) {
Line 37:             ((HttpServletResponse) 
response).sendRedirect(String.format("%s%s&app_url=%s", 
filterConfig.getServletContext().getContextPath(), loginUrl, 
((HttpServletResponse) response).encodeURL(requestUrl)));
get the servlet context out of request? req.getServletContext() ?
Line 38:         } else {
Line 39:             chain.doFilter(request, response);
Line 40:         }
Line 41:     }


http://gerrit.ovirt.org/#/c/36619/8/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/servlet/SSOPostLoginServlet.java
File 
backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/servlet/SSOPostLoginServlet.java:

Line 51:         }
Line 52: 
Line 53:         try {
Line 54:             InitialContext ctx = new InitialContext();
Line 55:             try {
if the NamingException creates complex structure then:

 ctx = null;
 try {
     ctx = new InitialContext();
 } catch (Exception xxx) {
 } finally {
     if (ctx != null) {
        ctx.close();
     }
 }
Line 56:                 JsonObjectDeserializer deserializer = new 
JsonObjectDeserializer();
Line 57:                 ExtMap principalRecord = 
deserializer.deserialize(request.getParameter("principalRecord"), ExtMap.class);
Line 58:                 String principal = 
principalRecord.get(Authz.PrincipalRecord.PRINCIPAL);
Line 59:                 String username = principal != null ? principal : 
principalRecord.<String>get(Authz.PrincipalRecord.NAME);


Line 63:                                 
deserializer.deserialize(request.getParameter("authRecord"), ExtMap.class),
Line 64:                                 principalRecord,
Line 65:                                 loginAsAdmin));
Line 66:                 if (!queryRetVal.getSucceeded()) {
Line 67:                     log.error(String.format("The user %s is not 
authorized to perform login", username));
why not just catch generic error at try block and log whatever we got?
Line 68:                     throw new RuntimeException(String.format("The user 
%s is not authorized to perform login", username));
Line 69:                 }
Line 70:                 HttpSession httpSession = request.getSession(true);
Line 71:                 httpSession.setAttribute(


http://gerrit.ovirt.org/#/c/36619/8/backend/manager/modules/extensions-api-root/extensions-api/src/main/java/org/ovirt/engine/api/extensions/ExtKey.java
File 
backend/manager/modules/extensions-api-root/extensions-api/src/main/java/org/ovirt/engine/api/extensions/ExtKey.java:

Line 36:     }
Line 37: 
Line 38:     public ExtKey() {
Line 39:     }
Line 40: 
I will post this as separate.
Line 41:     /**
Line 42:      * Constructor.
Line 43:      * @param type value type.
Line 44:      * @param uuid unique identifier for key.


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

Reply via email to