Alon Bar-Lev has posted comments on this change. Change subject: core : Add engine sso ......................................................................
Patch Set 4: (5 comments) ok, good! at least it seems I will be able to understand now what it does :) quick comments, I will review it later. http://gerrit.ovirt.org/#/c/36119/4/backend/manager/modules/enginesso/pom.xml File backend/manager/modules/enginesso/pom.xml: Line 20: <groupId>${engine.groupId}</groupId> Line 21: <artifactId>aaa</artifactId> Line 22: <version>${engine.version}</version> Line 23: <scope>provided</scope> Line 24: </dependency> so we can drop this^? Line 25: Line 26: <dependency> Line 27: <groupId>${engine.groupId}</groupId> Line 28: <artifactId>extensions-manager</artifactId> http://gerrit.ovirt.org/#/c/36119/4/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/AuthenticationUtils.java File backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/AuthenticationUtils.java: Line 5: import org.ovirt.engine.api.extensions.aaa.Authn; Line 6: import org.ovirt.engine.api.extensions.aaa.Authz; Line 7: import org.ovirt.engine.api.extensions.aaa.Mapping; Line 8: import org.ovirt.engine.core.extensions.mgr.ExtensionProxy; Line 9: import org.ovirt.engine.core.utils.extensionsmgr.EngineExtensionsManager; we should not use the engine one, use the extensions manager directly. Line 10: import org.slf4j.Logger; Line 11: import org.slf4j.LoggerFactory; Line 12: Line 13: import javax.servlet.ServletException; http://gerrit.ovirt.org/#/c/36119/4/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/SSOUtils.java File backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/SSOUtils.java: Line 22: redirectUrl.append("/login?"); Line 23: Line 24: StringBuilder params = new StringBuilder(""); Line 25: appendParameter(params, "id", principalRecord.<String> get(Authz.PrincipalRecord.ID)); Line 26: appendParameter(params, "authzname", profile); profile is human visible string, authz is the extension name, two different strings :) when we convert this to passing "internal" id, we won't need ths anyway.... so we are good. Line 27: appendParameter(params, "namespace", principalRecord.<String>get(Authz.PrincipalRecord.NAMESPACE)); Line 28: appendParameter(params, "username", getLoginName(principalRecord)); Line 29: appendParameter(params, "firstname", principalRecord.<String>get(Authz.PrincipalRecord.FIRST_NAME)); Line 30: appendParameter(params, "lastname", principalRecord.<String>get(Authz.PrincipalRecord.LAST_NAME)); Line 23: Line 24: StringBuilder params = new StringBuilder(""); Line 25: appendParameter(params, "id", principalRecord.<String> get(Authz.PrincipalRecord.ID)); Line 26: appendParameter(params, "authzname", profile); Line 27: appendParameter(params, "namespace", principalRecord.<String>get(Authz.PrincipalRecord.NAMESPACE)); I am unsure we actually need the namespace. Line 28: appendParameter(params, "username", getLoginName(principalRecord)); Line 29: appendParameter(params, "firstname", principalRecord.<String>get(Authz.PrincipalRecord.FIRST_NAME)); Line 30: appendParameter(params, "lastname", principalRecord.<String>get(Authz.PrincipalRecord.LAST_NAME)); Line 31: appendParameter(params, "department", principalRecord.<String>get(Authz.PrincipalRecord.DEPARTMENT)); Line 30: appendParameter(params, "lastname", principalRecord.<String>get(Authz.PrincipalRecord.LAST_NAME)); Line 31: appendParameter(params, "department", principalRecord.<String>get(Authz.PrincipalRecord.DEPARTMENT)); Line 32: appendParameter(params, "email", principalRecord.<String> get(Authz.PrincipalRecord.EMAIL)); Line 33: appendParameter(params, "groupIds", StringUtils.join(getGroupIds(principalRecord), ",")); Line 34: redirectUrl.append(params.toString()); in future this will be the "internal" group ids, if not we need to encode it as we cannot guess what will it will contain. Line 35: response.sendRedirect(redirectUrl.toString()); Line 36: } Line 37: Line 38: private static String getLoginName(ExtMap principalRecord) { -- 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: 4 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
