Fix external authentication failure

Change-Id: I0f415941a4bfd5e63d85c60613cea0c7d10cbb49

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<UserModel>
 	}

 

 	public boolean isLocalAccount() {

-		return accountType.isLocal();

+		return !Constants.EXTERNAL_ACCOUNT.equals(password) || accountType.isLocal();

 	}

 

 	/**