From 1293c279d1ab41ba7f4721009f87e89d4d48bf3d Mon Sep 17 00:00:00 2001 From: James Moger Date: Wed, 11 Dec 2013 08:08:37 -0500 Subject: [PATCH] Fix external authentication failure Change-Id: I0f415941a4bfd5e63d85c60613cea0c7d10cbb49 --- .../com/gitblit/auth/LdapAuthProvider.java | 35 +++++++++++-------- .../manager/AuthenticationManager.java | 5 +-- .../java/com/gitblit/models/UserModel.java | 3 +- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/gitblit/auth/LdapAuthProvider.java b/src/main/java/com/gitblit/auth/LdapAuthProvider.java index 67d98c7f..6a2dd437 100644 --- a/src/main/java/com/gitblit/auth/LdapAuthProvider.java +++ b/src/main/java/com/gitblit/auth/LdapAuthProvider.java @@ -289,16 +289,19 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider { UserModel user = null; synchronized (this) { user = userManager.getUserModel(simpleUsername); - if (user == null) // create user object for new authenticated user + if (user == null) { + // create user object for new authenticated user user = new UserModel(simpleUsername); + } // create a user cookie if (StringUtils.isEmpty(user.cookie) && !ArrayUtils.isEmpty(password)) { user.cookie = StringUtils.getSHA1(user.username + new String(password)); } - if (!supportsTeamMembershipChanges()) + if (!supportsTeamMembershipChanges()) { getTeamsFromLdap(ldapConnection, simpleUsername, loggingInUser, user); + } // Get User Attributes setUserAttributes(user, loggingInUser); @@ -307,8 +310,9 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider { updateUser(user); if (!supportsTeamMembershipChanges()) { - for (TeamModel userTeam : user.teams) + for (TeamModel userTeam : user.teams) { updateTeam(userTeam); + } } } @@ -337,12 +341,13 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider { if (!ArrayUtils.isEmpty(admins)) { user.canAdmin = false; for (String admin : admins) { - if (admin.startsWith("@")) { // Team - if (user.getTeam(admin.substring(1)) != null) - user.canAdmin = true; - } else - if (user.getName().equalsIgnoreCase(admin)) - user.canAdmin = true; + if (admin.startsWith("@") && user.isTeamMember(admin.substring(1))) { + // admin team + user.canAdmin = true; + } else if (user.getName().equalsIgnoreCase(admin)) { + // admin user + user.canAdmin = true; + } } } } @@ -361,9 +366,9 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider { if (!StringUtils.isEmpty(displayName)) { // Replace embedded ${} with attributes if (displayName.contains("${")) { - for (Attribute userAttribute : userEntry.getAttributes()) + for (Attribute userAttribute : userEntry.getAttributes()) { displayName = StringUtils.replace(displayName, "${" + userAttribute.getName() + "}", userAttribute.getValue()); - + } user.displayName = displayName; } else { Attribute attribute = userEntry.getAttribute(displayName); @@ -377,9 +382,9 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider { String email = settings.getString(Keys.realm.ldap.email, ""); if (!StringUtils.isEmpty(email)) { if (email.contains("${")) { - for (Attribute userAttribute : userEntry.getAttributes()) + for (Attribute userAttribute : userEntry.getAttributes()) { email = StringUtils.replace(email, "${" + userAttribute.getName() + "}", userAttribute.getValue()); - + } user.emailAddress = email; } else { Attribute attribute = userEntry.getAttribute(email); @@ -393,7 +398,9 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider { private void getTeamsFromLdap(LDAPConnection ldapConnection, String simpleUsername, SearchResultEntry loggingInUser, UserModel user) { String loggingInUserDN = loggingInUser.getDN(); - user.teams.clear(); // Clear the users team memberships - we're going to get them from LDAP + // Clear the users team memberships - we're going to get them from LDAP + user.teams.clear(); + String groupBase = settings.getString(Keys.realm.ldap.groupBase, ""); String groupMemberPattern = settings.getString(Keys.realm.ldap.groupMemberPattern, "(&(objectClass=group)(member=${dn}))"); diff --git a/src/main/java/com/gitblit/manager/AuthenticationManager.java b/src/main/java/com/gitblit/manager/AuthenticationManager.java index 60687d44..eef675b2 100644 --- a/src/main/java/com/gitblit/manager/AuthenticationManager.java +++ b/src/main/java/com/gitblit/manager/AuthenticationManager.java @@ -323,9 +323,10 @@ public class AuthenticationManager implements IAuthenticationManager { return null; } - // try local authentication UserModel user = userManager.getUserModel(usernameDecoded); - if (user != null) { + + // try local authentication + if (user != null && user.isLocalAccount()) { UserModel returnedUser = null; if (user.password.startsWith(StringUtils.MD5_TYPE)) { // password digest diff --git a/src/main/java/com/gitblit/models/UserModel.java b/src/main/java/com/gitblit/models/UserModel.java index 0b59927f..6e8de402 100644 --- a/src/main/java/com/gitblit/models/UserModel.java +++ b/src/main/java/com/gitblit/models/UserModel.java @@ -27,6 +27,7 @@ import java.util.Map; import java.util.Set; import java.util.TreeSet; +import com.gitblit.Constants; import com.gitblit.Constants.AccessPermission; import com.gitblit.Constants.AccessRestrictionType; import com.gitblit.Constants.AccountType; @@ -94,7 +95,7 @@ public class UserModel implements Principal, Serializable, Comparable } public boolean isLocalAccount() { - return accountType.isLocal(); + return !Constants.EXTERNAL_ACCOUNT.equals(password) || accountType.isLocal(); } /** -- 2.39.5