Justin Hammond has posted comments on this change.
Change subject: engine: Integrate Atlassian Crowd Client as a new
Authentication Domain
......................................................................
Patch Set 1: (7 inline comments)
Replied to a few comments.
Yair - Got it. Thanks!
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/CrowdBrokerUtils.java
Line 35:
Line 36: public class CrowdBrokerUtils {
Line 37:
Line 38: /* have we previously init'ed Crowd? */
Line 39: private static boolean crowdok = false;
m_CrowdClient can be a valid object but it doesn't mean the Crowdclient is
functional.
As long as the crowd.properties file can be loaded and passed in the
constructor of the RestCrowdClientFactory, then m_CrowdClient will be valid.
The problem is that the library doesn't actually test the connection to the
server to validate you got your settings correct.
The actual validation is done via the testConnection method. The reason I used
a boolean, was to stop oVirt trying to talk to crowd server everytime a crowd
function was called.
(But there is a flaw in my logic - if we can't talk to crowd for whatever
reason, it will keep trying... let me think about it for a while, because I
also see it as a bit of a feature - automatic recovery if the crowd server goes
offline for a while and then comes back)
Line 40:
Line 41: private static Log log = LogFactory.getLog(CrowdBrokerUtils.class);
Line 42: /* The Couwd Client */
Line 43: static CrowdClient m_CrowdClient;
Line 51: public static boolean initCrowd() {
Line 52: /* if we have already init'd Crowd, then don't do it again */
Line 53: if (crowdok == false) {
Line 54: Properties m_CrowdConfig = new Properties();
Line 55: File f = new File("/etc/ovirt-engine/",
"crowd.properties");
Thanks for the pointer. I did search for something like that, but found a few
other classes using fixed paths. I'll update it.
Line 56: try {
Line 57: m_CrowdConfig.load(new FileReader(f));
Line 58: } catch (Exception e) {
Line 59: log.warnFormat("Failed to load Crowd Configuration
from file {0}{1}: {2}",
Line 85: retVal.setName(user.getDisplayName());
Line 86: retVal.setUserName(user.getName());
Line 87: retVal.setEmail(user.getEmailAddress());
Line 88: /* All Crowd Based Users live in the "Crowd" domain */
Line 89: retVal.setDomainControler("Crowd");
Almost. All the Crowd*.java files are just copies of the Internal*.java files.
Iĺl update with a static as we use it a few places.
Line 90: /* Crowd doesn't have a something like a Guid, so the
following code
Line 91: * checks if we have stored a previous Guid, and if not,
create a new one
Line 92: * and update a Crowd User Attribute called oVirtGuid with it
so
Line 93: * next time we retrive this user, the Guid is stable
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/CrowdGetAdUserByUserNameCommand.java
Line 19: setSucceeded(true);
Line 20: setReturnValue(user);
Line 21: } else {
Line 22: setSucceeded(false);
Line 23: }
These were copies of the Internal*.java equivilents. Let me check out the Ldap
classes and I'll match to the majority...
Line 24:
Line 25: }
Line 26:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/CrowdSearchGroupsByQueryCommand.java
Line 9: }
Line 10:
Line 11: @Override
Line 12: protected void ExecuteQuery() {
Line 13: java.util.List<ad_groups> groupList =
CrowdBrokerUtils.getAllGroups();
Noted. let me implement the filtering if there is some consensus on this making
it into oVirt one way or another.
Line 14: setReturnValue(groupList);
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 11: }
Line 12:
Line 13: @Override
Line 14: protected void ExecuteQuery() {
Line 15: List<AdUser> userList = CrowdBrokerUtils.getAllUsers();
Yes and no!
I'm undecided on if its better to perform the filtering at the crowd server, or
oVirt level. Regardless, this is just a copy of the existing
InternalSearchUserByQueryCommand which doesn't do any filtering.
I'll look into the best options here.
Line 16: setReturnValue(userList);
Line 17: setSucceeded(true);
Line 18: }
Line 19:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerUtils.java
Line 55: if (!filterInternalDomain) {
Line 56: results.add(Config.<String>
GetValue(ConfigValues.AdminDomain).trim());
Line 57: /* Only add the Crowd Domain if it can initilize correctly
*/
Line 58: if (CrowdBrokerUtils.initCrowd())
Line 59: results.add("Crowd");
Noted.
Line 60: }
Line 61: return results;
Line 62: }
Line 63:
--
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: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
