Sharad Mishra has posted comments on this change.
Change subject: engine: Integrate Atlassian Crowd Client as a new
Authentication Domain
......................................................................
Patch Set 1: (6 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/CrowdAuthenticateUserCommand.java
Line 7: public CrowdAuthenticateUserCommand(LdapUserPasswordBaseParameters
parameters) {
Line 8: super(parameters);
Line 9: }
Line 10:
Line 11: public String getUPNForUser(String userName, String domain) {
It is recommended to use camelCase, so getUpnForUser works better.
Line 12: String UPN = userName;
Line 13: if (!userName.contains("@")) {
Line 14: UPN = userName + '@' + domain;
Line 15: }
Line 15: }
Line 16: return UPN;
Line 17: }
Line 18:
Line 19: public String getUserNameForUPN(String UPN) {
camelCase here too.
Line 20: String userName = UPN;
Line 21: if (userName.contains("@")) {
Line 22: userName = userName.split("@")[0];
Line 23: }
Line 28: protected void ExecuteQuery() {
Line 29: String userName = getParameters().getLoginName();
Line 30: String password = getParameters().getPassword();
Line 31: String domain = BrokerUtils.getLoginDomain(userName,
getDomain());
Line 32: String userUPN = getUPNForUser(userName, domain);
camelCase
Line 33: userName = getUserNameForUPN(userUPN);
Line 34: UserAuthenticationResult result =
CrowdBrokerUtils.authenticate(userName, password, domain);
Line 35:
Line 36: setSucceeded(result.isSuccessful());
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/CrowdGetAdUserByUserIdListCommand.java
Line 24: AdUser user = CrowdBrokerUtils.getUserByUserGuid(guid);
Line 25: if (user != null) {
Line 26: results.add(user);
Line 27: }
Line 28: }
should there be a check to verify that results in not empty?
Line 29: setReturnValue(results);
Line 30: setSucceeded(true);
Line 31: }
Line 32:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/CrowdSearchGroupsByQueryCommand.java
Line 10:
Line 11: @Override
Line 12: protected void ExecuteQuery() {
Line 13: java.util.List<ad_groups> groupList =
CrowdBrokerUtils.getAllGroups();
Line 14: setReturnValue(groupList);
do you want to check if groupList is non-null and not empty before
setSucceeded(true)?
Line 15: setSucceeded(true);
Line 16: }
Line 17:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/CrowdSearchUserByQueryCommand.java
Line 12:
Line 13: @Override
Line 14: protected void ExecuteQuery() {
Line 15: List<AdUser> userList = CrowdBrokerUtils.getAllUsers();
Line 16: setReturnValue(userList);
do you want to check if userList is non-null and not empty before
setSucceeded(true)?
Line 17: setSucceeded(true);
Line 18: }
Line 19:
--
To view, visit http://gerrit.ovirt.org/9324
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide867f16d092eb329c0ce2fccf4ebd02f3aae0df
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Justin Hammond <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Justin Hammond <[email protected]>
Gerrit-Reviewer: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Sharad Mishra <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches