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