Alon Bar-Lev has posted comments on this change.

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


Patch Set 31:

(3 comments)

http://gerrit.ovirt.org/#/c/36119/31/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/LogoutServlet.java
File 
backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/LogoutServlet.java:

Line 26:             
response.sendRedirect(request.getParameter(SSOUtils.POST_LOGIN_URL));
Line 27:         } else {
Line 28:             SSOConfig config = (SSOConfig) 
request.getServletContext().getAttribute(SSOUtils.SSO_CONFIG);
Line 29:             String url = config.getWelcomeUrl();
Line 30:             if (!url.startsWith("http")) {
I think you can check if this is absolute file name, which is better than 
trying to parse.
Line 31:                 url = request.getContextPath() + url;
Line 32:             }
Line 33:             response.sendRedirect(url);
Line 34:         }


http://gerrit.ovirt.org/#/c/36119/31/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/utils/SSOConfig.java
File 
backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/utils/SSOConfig.java:

Line 49:     public void setWelcomeUrl(String welcomeUrl) {
Line 50:         this.welcomeUrl = welcomeUrl;
Line 51:     }
Line 52: 
Line 53:     public String getSsoVersion() {
version is not part of configuration but implementation.
Line 54:         return ssoVersion;
Line 55:     }
Line 56: 
Line 57:     public void setSsoVersion(String ssoVersion) {


http://gerrit.ovirt.org/#/c/36119/31/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 54:             }
Line 55: 
Line 56:             String principal = 
principalRecord.get(Authz.PrincipalRecord.PRINCIPAL);
Line 57:             Map<String, Object> payload = new HashMap<>();
Line 58:             payload.put("userId", userId);
please add version field as well to ease parsing at remote.
Line 59:             payload.put("userName", principal != null ? principal : 
principalRecord.<String>get(Authz.PrincipalRecord.NAME));
Line 60:             payload.put("email", 
principalRecord.<String>get(Authz.PrincipalRecord.EMAIL));
Line 61:             payload.put("groupIds", 
getGroupIds(config.getSsoUserGroupManager(), authzName, principalRecord));
Line 62:             payload.put("profile", 
session.getAttribute(SSOUtils.SSO_PROFILE_ATTR_NAME));


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4894fc12653027271b6abd4dd5313b10593703fa
Gerrit-PatchSet: 31
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[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