From b4a63aad7f56486c164a15ae2477bcd251b0bb1b Mon Sep 17 00:00:00 2001 From: James Moger <james.moger@gitblit.com> Date: Tue, 18 Mar 2014 21:10:48 -0400 Subject: [PATCH] Fix authentication security hole with external providers --- src/main/java/com/gitblit/manager/AuthenticationManager.java | 73 ++++++++++++++++++++++-------------- 1 files changed, 44 insertions(+), 29 deletions(-) diff --git a/src/main/java/com/gitblit/manager/AuthenticationManager.java b/src/main/java/com/gitblit/manager/AuthenticationManager.java index ad4a985..4f3e652 100644 --- a/src/main/java/com/gitblit/manager/AuthenticationManager.java +++ b/src/main/java/com/gitblit/manager/AuthenticationManager.java @@ -159,6 +159,10 @@ } return this; } + + public void addAuthenticationProvider(AuthenticationProvider prov) { + authenticationProviders.add(prov); + } /** * Authenticate a user based on HTTP request parameters. @@ -341,41 +345,52 @@ // try local authentication if (user != null && user.isLocalAccount()) { - UserModel returnedUser = null; - if (user.password.startsWith(StringUtils.MD5_TYPE)) { - // password digest - String md5 = StringUtils.MD5_TYPE + StringUtils.getMD5(new String(password)); - if (user.password.equalsIgnoreCase(md5)) { - returnedUser = user; - } - } else if (user.password.startsWith(StringUtils.COMBINED_MD5_TYPE)) { - // username+password digest - String md5 = StringUtils.COMBINED_MD5_TYPE - + StringUtils.getMD5(username.toLowerCase() + new String(password)); - if (user.password.equalsIgnoreCase(md5)) { - returnedUser = user; - } - } else if (user.password.equals(new String(password))) { - // plain-text password - returnedUser = user; - } - return validateAuthentication(returnedUser, AuthenticationType.CREDENTIALS); + return authenticateLocal(user, password); } // try registered external authentication providers - if (user == null) { - for (AuthenticationProvider provider : authenticationProviders) { - if (provider instanceof UsernamePasswordAuthenticationProvider) { - user = provider.authenticate(usernameDecoded, password); - if (user != null) { - // user authenticated - user.accountType = provider.getAccountType(); - return validateAuthentication(user, AuthenticationType.CREDENTIALS); - } + for (AuthenticationProvider provider : authenticationProviders) { + if (provider instanceof UsernamePasswordAuthenticationProvider) { + UserModel returnedUser = provider.authenticate(usernameDecoded, password); + if (returnedUser != null) { + // user authenticated + returnedUser.accountType = provider.getAccountType(); + return validateAuthentication(returnedUser, AuthenticationType.CREDENTIALS); } } } - return validateAuthentication(user, AuthenticationType.CREDENTIALS); + + // could not authenticate locally or with a provider + return null; + } + + /** + * Returns a UserModel if local authentication succeeds. + * + * @param user + * @param password + * @return a UserModel if local authentication succeeds, null otherwise + */ + protected UserModel authenticateLocal(UserModel user, char [] password) { + UserModel returnedUser = null; + if (user.password.startsWith(StringUtils.MD5_TYPE)) { + // password digest + String md5 = StringUtils.MD5_TYPE + StringUtils.getMD5(new String(password)); + if (user.password.equalsIgnoreCase(md5)) { + returnedUser = user; + } + } else if (user.password.startsWith(StringUtils.COMBINED_MD5_TYPE)) { + // username+password digest + String md5 = StringUtils.COMBINED_MD5_TYPE + + StringUtils.getMD5(user.username.toLowerCase() + new String(password)); + if (user.password.equalsIgnoreCase(md5)) { + returnedUser = user; + } + } else if (user.password.equals(new String(password))) { + // plain-text password + returnedUser = user; + } + return validateAuthentication(returnedUser, AuthenticationType.CREDENTIALS); } /** -- Gitblit v1.9.1