Merge pull request #1152 from fzs/fixAdminRoleLDAP
Set "can admin" permission on LDAP users and teams correctly
diff --git a/src/main/java/com/gitblit/auth/LdapAuthProvider.java b/src/main/java/com/gitblit/auth/LdapAuthProvider.java
index e1dec48..19fd463 100644
--- a/src/main/java/com/gitblit/auth/LdapAuthProvider.java
+++ b/src/main/java/com/gitblit/auth/LdapAuthProvider.java
@@ -171,6 +171,8 @@
final Map<String, TeamModel> userTeams = new HashMap<String, TeamModel>();
for (UserModel user : ldapUsers.values()) {
for (TeamModel userTeam : user.teams) {
+ // Is this an administrative team?
+ setAdminAttribute(userTeam);
userTeams.put(userTeam.name, userTeam);
}
}
@@ -238,10 +240,7 @@
public boolean supportsRoleChanges(UserModel user, Role role) {
if (Role.ADMIN == role) {
if (!supportsTeamMembershipChanges()) {
- List<String> admins = settings.getStrings(Keys.realm.ldap.admins);
- if (admins.contains(user.username)) {
- return false;
- }
+ return false;
}
}
return true;
@@ -251,10 +250,7 @@
public boolean supportsRoleChanges(TeamModel team, Role role) {
if (Role.ADMIN == role) {
if (!supportsTeamMembershipChanges()) {
- List<String> admins = settings.getStrings(Keys.realm.ldap.admins);
- if (admins.contains("@" + team.name)) {
- return false;
- }
+ return false;
}
}
return true;
@@ -325,6 +321,8 @@
if (!supportsTeamMembershipChanges()) {
for (TeamModel userTeam : user.teams) {
+ // Is this an administrative team?
+ setAdminAttribute(userTeam);
updateTeam(userTeam);
}
}
@@ -355,10 +353,7 @@
if (!ArrayUtils.isEmpty(admins)) {
user.canAdmin = false;
for (String admin : admins) {
- if (admin.startsWith("@") && user.isTeamMember(admin.substring(1))) {
- // admin team
- user.canAdmin = true;
- } else if (user.getName().equalsIgnoreCase(admin)) {
+ if (user.getName().equalsIgnoreCase(admin)) {
// admin user
user.canAdmin = true;
}
@@ -367,6 +362,30 @@
}
}
+ /**
+ * Set the canAdmin attribute for team retrieved from LDAP.
+ * If we are not storing teams in LDAP and/or we have not defined any
+ * administrator teams, then do not change the admin flag.
+ *
+ * @param team
+ */
+ private void setAdminAttribute(TeamModel team) {
+ if (!supportsTeamMembershipChanges()) {
+ List<String> admins = settings.getStrings(Keys.realm.ldap.admins);
+ // if we have defined administrative teams, then set admin flag
+ // otherwise leave admin flag unchanged
+ if (!ArrayUtils.isEmpty(admins)) {
+ team.canAdmin = false;
+ for (String admin : admins) {
+ if (admin.startsWith("@") && team.name.equalsIgnoreCase(admin.substring(1))) {
+ // admin team
+ team.canAdmin = true;
+ }
+ }
+ }
+ }
+ }
+
private void setUserAttributes(UserModel user, SearchResultEntry userEntry) {
// Is this user an admin?
setAdminAttribute(user);
@@ -462,6 +481,7 @@
TeamModel teamModel = userManager.getTeamModel(teamName);
if (teamModel == null) {
teamModel = createTeamFromLdap(teamEntry);
+ setAdminAttribute(teamModel);
userManager.updateTeamModel(teamModel);
}
}
diff --git a/src/test/java/com/gitblit/tests/LdapAuthenticationTest.java b/src/test/java/com/gitblit/tests/LdapAuthenticationTest.java
index 2ade681..b7a77fc 100644
--- a/src/test/java/com/gitblit/tests/LdapAuthenticationTest.java
+++ b/src/test/java/com/gitblit/tests/LdapAuthenticationTest.java
@@ -296,7 +296,6 @@
assertNotNull(userOneModel);
assertNotNull(userOneModel.getTeam("git_admins"));
assertNotNull(userOneModel.getTeam("git_users"));
- assertTrue(userOneModel.canAdmin);
UserModel userOneModelFailedAuth = ldap.authenticate("UserOne", "userTwoPassword".toCharArray());
assertNull(userOneModelFailedAuth);
@@ -306,13 +305,49 @@
assertNotNull(userTwoModel.getTeam("git_users"));
assertNull(userTwoModel.getTeam("git_admins"));
assertNotNull(userTwoModel.getTeam("git admins"));
- assertTrue(userTwoModel.canAdmin);
UserModel userThreeModel = ldap.authenticate("UserThree", "userThreePassword".toCharArray());
assertNotNull(userThreeModel);
assertNotNull(userThreeModel.getTeam("git_users"));
assertNull(userThreeModel.getTeam("git_admins"));
+
+ UserModel userFourModel = ldap.authenticate("UserFour", "userFourPassword".toCharArray());
+ assertNotNull(userFourModel);
+ assertNotNull(userFourModel.getTeam("git_users"));
+ assertNull(userFourModel.getTeam("git_admins"));
+ assertNull(userFourModel.getTeam("git admins"));
+ }
+
+ @Test
+ public void testAdminPropertyTeamsInLdap() {
+ UserModel userOneModel = ldap.authenticate("UserOne", "userOnePassword".toCharArray());
+ assertNotNull(userOneModel);
+ assertNotNull(userOneModel.getTeam("git_admins"));
+ assertNull(userOneModel.getTeam("git admins"));
+ assertNotNull(userOneModel.getTeam("git_users"));
+ assertFalse(userOneModel.canAdmin);
+ assertTrue(userOneModel.canAdmin());
+ assertTrue(userOneModel.getTeam("git_admins").canAdmin);
+ assertFalse(userOneModel.getTeam("git_users").canAdmin);
+
+ UserModel userTwoModel = ldap.authenticate("UserTwo", "userTwoPassword".toCharArray());
+ assertNotNull(userTwoModel);
+ assertNotNull(userTwoModel.getTeam("git_users"));
+ assertNull(userTwoModel.getTeam("git_admins"));
+ assertNotNull(userTwoModel.getTeam("git admins"));
+ assertFalse(userTwoModel.canAdmin);
+ assertTrue(userTwoModel.canAdmin());
+ assertTrue(userTwoModel.getTeam("git admins").canAdmin);
+ assertFalse(userTwoModel.getTeam("git_users").canAdmin);
+
+ UserModel userThreeModel = ldap.authenticate("UserThree", "userThreePassword".toCharArray());
+ assertNotNull(userThreeModel);
+ assertNotNull(userThreeModel.getTeam("git_users"));
+ assertNull(userThreeModel.getTeam("git_admins"));
+ assertNull(userThreeModel.getTeam("git admins"));
assertTrue(userThreeModel.canAdmin);
+ assertTrue(userThreeModel.canAdmin());
+ assertFalse(userThreeModel.getTeam("git_users").canAdmin);
UserModel userFourModel = ldap.authenticate("UserFour", "userFourPassword".toCharArray());
assertNotNull(userFourModel);
@@ -320,6 +355,51 @@
assertNull(userFourModel.getTeam("git_admins"));
assertNull(userFourModel.getTeam("git admins"));
assertFalse(userFourModel.canAdmin);
+ assertFalse(userFourModel.canAdmin());
+ assertFalse(userFourModel.getTeam("git_users").canAdmin);
+ }
+
+ @Test
+ public void testAdminPropertyTeamsNotInLdap() {
+ settings.put(Keys.realm.ldap.maintainTeams, "false");
+
+ UserModel userOneModel = ldap.authenticate("UserOne", "userOnePassword".toCharArray());
+ assertNotNull(userOneModel);
+ assertNotNull(userOneModel.getTeam("git_admins"));
+ assertNull(userOneModel.getTeam("git admins"));
+ assertNotNull(userOneModel.getTeam("git_users"));
+ assertTrue(userOneModel.canAdmin);
+ assertTrue(userOneModel.canAdmin());
+ assertFalse(userOneModel.getTeam("git_admins").canAdmin);
+ assertFalse(userOneModel.getTeam("git_users").canAdmin);
+
+ UserModel userTwoModel = ldap.authenticate("UserTwo", "userTwoPassword".toCharArray());
+ assertNotNull(userTwoModel);
+ assertNotNull(userTwoModel.getTeam("git_users"));
+ assertNull(userTwoModel.getTeam("git_admins"));
+ assertNotNull(userTwoModel.getTeam("git admins"));
+ assertFalse(userTwoModel.canAdmin);
+ assertTrue(userTwoModel.canAdmin());
+ assertTrue(userTwoModel.getTeam("git admins").canAdmin);
+ assertFalse(userTwoModel.getTeam("git_users").canAdmin);
+
+ UserModel userThreeModel = ldap.authenticate("UserThree", "userThreePassword".toCharArray());
+ assertNotNull(userThreeModel);
+ assertNotNull(userThreeModel.getTeam("git_users"));
+ assertNull(userThreeModel.getTeam("git_admins"));
+ assertNull(userThreeModel.getTeam("git admins"));
+ assertFalse(userThreeModel.canAdmin);
+ assertFalse(userThreeModel.canAdmin());
+ assertFalse(userThreeModel.getTeam("git_users").canAdmin);
+
+ UserModel userFourModel = ldap.authenticate("UserFour", "userFourPassword".toCharArray());
+ assertNotNull(userFourModel);
+ assertNotNull(userFourModel.getTeam("git_users"));
+ assertNull(userFourModel.getTeam("git_admins"));
+ assertNull(userFourModel.getTeam("git admins"));
+ assertFalse(userFourModel.canAdmin);
+ assertFalse(userFourModel.canAdmin());
+ assertFalse(userFourModel.getTeam("git_users").canAdmin);
}
@Test
@@ -392,6 +472,17 @@
}
@Test
+ public void addingGroupsInLdapShouldUpdateGitBlitUsersNotGroups2() throws Exception {
+ settings.put(Keys.realm.ldap.synchronize, "true");
+ settings.put(Keys.realm.ldap.maintainTeams, "false");
+ getDS().addEntries(LDIFReader.readEntries(RESOURCE_DIR + "adduser.ldif"));
+ getDS().addEntries(LDIFReader.readEntries(RESOURCE_DIR + "addgroup.ldif"));
+ ldap.sync();
+ assertEquals("Number of ldap users in gitblit user model", 6, countLdapUsersInUserManager());
+ assertEquals("Number of ldap groups in gitblit team model", 0, countLdapTeamsInUserManager());
+ }
+
+ @Test
public void addingGroupsInLdapShouldUpdateGitBlitUsersAndGroups() throws Exception {
// This test only makes sense if the authentication mode allows for synchronization.
assumeTrue(authMode == AuthMode.ANONYMOUS || authMode == AuthMode.DS_MANAGER);
@@ -403,12 +494,91 @@
}
@Test
+ public void syncUpdateUsersAndGroupsAdminProperty() throws Exception {
+ // This test only makes sense if the authentication mode allows for synchronization.
+ assumeTrue(authMode == AuthMode.ANONYMOUS || authMode == AuthMode.DS_MANAGER);
+
+ settings.put(Keys.realm.ldap.synchronize, "true");
+ ldap.sync();
+
+ UserModel user = userManager.getUserModel("UserOne");
+ assertNotNull(user);
+ assertFalse(user.canAdmin);
+ assertTrue(user.canAdmin());
+
+ user = userManager.getUserModel("UserTwo");
+ assertNotNull(user);
+ assertFalse(user.canAdmin);
+ assertTrue(user.canAdmin());
+
+ user = userManager.getUserModel("UserThree");
+ assertNotNull(user);
+ assertTrue(user.canAdmin);
+ assertTrue(user.canAdmin());
+
+ user = userManager.getUserModel("UserFour");
+ assertNotNull(user);
+ assertFalse(user.canAdmin);
+ assertFalse(user.canAdmin());
+
+ TeamModel team = userManager.getTeamModel("Git_Admins");
+ assertNotNull(team);
+ assertTrue(team.canAdmin);
+
+ team = userManager.getTeamModel("Git Admins");
+ assertNotNull(team);
+ assertTrue(team.canAdmin);
+
+ team = userManager.getTeamModel("Git_Users");
+ assertNotNull(team);
+ assertFalse(team.canAdmin);
+ }
+
+ @Test
+ public void syncNotUpdateUsersAndGroupsAdminProperty() throws Exception {
+ settings.put(Keys.realm.ldap.synchronize, "true");
+ settings.put(Keys.realm.ldap.maintainTeams, "false");
+ ldap.sync();
+
+ UserModel user = userManager.getUserModel("UserOne");
+ assertNotNull(user);
+ assertTrue(user.canAdmin);
+ assertTrue(user.canAdmin());
+
+ user = userManager.getUserModel("UserTwo");
+ assertNotNull(user);
+ assertFalse(user.canAdmin);
+ assertTrue(user.canAdmin());
+
+ user = userManager.getUserModel("UserThree");
+ assertNotNull(user);
+ assertFalse(user.canAdmin);
+ assertFalse(user.canAdmin());
+
+ user = userManager.getUserModel("UserFour");
+ assertNotNull(user);
+ assertFalse(user.canAdmin);
+ assertFalse(user.canAdmin());
+
+ TeamModel team = userManager.getTeamModel("Git_Admins");
+ assertNotNull(team);
+ assertFalse(team.canAdmin);
+
+ team = userManager.getTeamModel("Git Admins");
+ assertNotNull(team);
+ assertTrue(team.canAdmin);
+
+ team = userManager.getTeamModel("Git_Users");
+ assertNotNull(team);
+ assertFalse(team.canAdmin);
+ }
+
+ @Test
public void testAuthenticationManager() {
UserModel userOneModel = auth.authenticate("UserOne", "userOnePassword".toCharArray(), null);
assertNotNull(userOneModel);
assertNotNull(userOneModel.getTeam("git_admins"));
assertNotNull(userOneModel.getTeam("git_users"));
- assertTrue(userOneModel.canAdmin);
UserModel userOneModelFailedAuth = auth.authenticate("UserOne", "userTwoPassword".toCharArray(), null);
assertNull(userOneModelFailedAuth);
@@ -418,13 +588,52 @@
assertNotNull(userTwoModel.getTeam("git_users"));
assertNull(userTwoModel.getTeam("git_admins"));
assertNotNull(userTwoModel.getTeam("git admins"));
- assertTrue(userTwoModel.canAdmin);
UserModel userThreeModel = auth.authenticate("UserThree", "userThreePassword".toCharArray(), null);
assertNotNull(userThreeModel);
assertNotNull(userThreeModel.getTeam("git_users"));
assertNull(userThreeModel.getTeam("git_admins"));
+
+ UserModel userFourModel = auth.authenticate("UserFour", "userFourPassword".toCharArray(), null);
+ assertNotNull(userFourModel);
+ assertNotNull(userFourModel.getTeam("git_users"));
+ assertNull(userFourModel.getTeam("git_admins"));
+ assertNull(userFourModel.getTeam("git admins"));
+ }
+
+ @Test
+ public void testAuthenticationManagerAdminPropertyTeamsInLdap() {
+ UserModel userOneModel = auth.authenticate("UserOne", "userOnePassword".toCharArray(), null);
+ assertNotNull(userOneModel);
+ assertNotNull(userOneModel.getTeam("git_admins"));
+ assertNull(userOneModel.getTeam("git admins"));
+ assertNotNull(userOneModel.getTeam("git_users"));
+ assertFalse(userOneModel.canAdmin);
+ assertTrue(userOneModel.canAdmin());
+ assertTrue(userOneModel.getTeam("git_admins").canAdmin);
+ assertFalse(userOneModel.getTeam("git_users").canAdmin);
+
+ UserModel userOneModelFailedAuth = auth.authenticate("UserOne", "userTwoPassword".toCharArray(), null);
+ assertNull(userOneModelFailedAuth);
+
+ UserModel userTwoModel = auth.authenticate("UserTwo", "userTwoPassword".toCharArray(), null);
+ assertNotNull(userTwoModel);
+ assertNotNull(userTwoModel.getTeam("git_users"));
+ assertNull(userTwoModel.getTeam("git_admins"));
+ assertNotNull(userTwoModel.getTeam("git admins"));
+ assertFalse(userTwoModel.canAdmin);
+ assertTrue(userTwoModel.canAdmin());
+ assertTrue(userTwoModel.getTeam("git admins").canAdmin);
+ assertFalse(userTwoModel.getTeam("git_users").canAdmin);
+
+ UserModel userThreeModel = auth.authenticate("UserThree", "userThreePassword".toCharArray(), null);
+ assertNotNull(userThreeModel);
+ assertNotNull(userThreeModel.getTeam("git_users"));
+ assertNull(userThreeModel.getTeam("git_admins"));
+ assertNull(userThreeModel.getTeam("git admins"));
assertTrue(userThreeModel.canAdmin);
+ assertTrue(userThreeModel.canAdmin());
+ assertFalse(userThreeModel.getTeam("git_users").canAdmin);
UserModel userFourModel = auth.authenticate("UserFour", "userFourPassword".toCharArray(), null);
assertNotNull(userFourModel);
@@ -432,6 +641,54 @@
assertNull(userFourModel.getTeam("git_admins"));
assertNull(userFourModel.getTeam("git admins"));
assertFalse(userFourModel.canAdmin);
+ assertFalse(userFourModel.canAdmin());
+ assertFalse(userFourModel.getTeam("git_users").canAdmin);
+ }
+
+ @Test
+ public void testAuthenticationManagerAdminPropertyTeamsNotInLdap() {
+ settings.put(Keys.realm.ldap.maintainTeams, "false");
+
+ UserModel userOneModel = auth.authenticate("UserOne", "userOnePassword".toCharArray(), null);
+ assertNotNull(userOneModel);
+ assertNotNull(userOneModel.getTeam("git_admins"));
+ assertNull(userOneModel.getTeam("git admins"));
+ assertNotNull(userOneModel.getTeam("git_users"));
+ assertTrue(userOneModel.canAdmin);
+ assertTrue(userOneModel.canAdmin());
+ assertFalse(userOneModel.getTeam("git_admins").canAdmin);
+ assertFalse(userOneModel.getTeam("git_users").canAdmin);
+
+ UserModel userOneModelFailedAuth = auth.authenticate("UserOne", "userTwoPassword".toCharArray(), null);
+ assertNull(userOneModelFailedAuth);
+
+ UserModel userTwoModel = auth.authenticate("UserTwo", "userTwoPassword".toCharArray(), null);
+ assertNotNull(userTwoModel);
+ assertNotNull(userTwoModel.getTeam("git_users"));
+ assertNull(userTwoModel.getTeam("git_admins"));
+ assertNotNull(userTwoModel.getTeam("git admins"));
+ assertFalse(userTwoModel.canAdmin);
+ assertTrue(userTwoModel.canAdmin());
+ assertTrue(userTwoModel.getTeam("git admins").canAdmin);
+ assertFalse(userTwoModel.getTeam("git_users").canAdmin);
+
+ UserModel userThreeModel = auth.authenticate("UserThree", "userThreePassword".toCharArray(), null);
+ assertNotNull(userThreeModel);
+ assertNotNull(userThreeModel.getTeam("git_users"));
+ assertNull(userThreeModel.getTeam("git_admins"));
+ assertNull(userThreeModel.getTeam("git admins"));
+ assertFalse(userThreeModel.canAdmin);
+ assertFalse(userThreeModel.canAdmin());
+ assertFalse(userThreeModel.getTeam("git_users").canAdmin);
+
+ UserModel userFourModel = auth.authenticate("UserFour", "userFourPassword".toCharArray(), null);
+ assertNotNull(userFourModel);
+ assertNotNull(userFourModel.getTeam("git_users"));
+ assertNull(userFourModel.getTeam("git_admins"));
+ assertNull(userFourModel.getTeam("git admins"));
+ assertFalse(userFourModel.canAdmin);
+ assertFalse(userFourModel.canAdmin());
+ assertFalse(userFourModel.getTeam("git_users").canAdmin);
}
@Test
diff --git a/src/test/resources/ldap/users.conf b/src/test/resources/ldap/users.conf
index 7d1e319..a2390fa 100644
--- a/src/test/resources/ldap/users.conf
+++ b/src/test/resources/ldap/users.conf
@@ -10,7 +10,7 @@
displayName = Mrs. User Three
emailAddress = userthree@gitblit.com
accountType = LDAP
- role = "#admin"
+ role = "#none"
[user "userfive"]
password = "#externalAccount"
cookie = 220bafef069b8b399b2597644015b6b0f4667982
@@ -31,7 +31,7 @@
displayName = Mr. User Two
emailAddress = usertwo@gitblit.com
accountType = LDAP
- role = "#admin"
+ role = "#none"
[user "basic"]
password = MD5:f17aaabc20bfe045075927934fed52d2
cookie = dd94709528bb1c83d08f3088d4043f4742891f4f
@@ -63,6 +63,6 @@
user = userthree
user = userfour
[team "Git Admins"]
- role = "#none"
+ role = "#admin"
accountType = LOCAL
user = usertwo
