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 <jus...@dynam.ac>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ew...@kohlvanwijngaarden.nl>
Gerrit-Reviewer: Itamar Heim <ih...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Justin Hammond <jus...@dynam.ac>
Gerrit-Reviewer: Laszlo Hornyak <lhorn...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Sharad Mishra <snmis...@linux.vnet.ibm.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to