From 37a9c506d9248374337df99f109442de984724c8 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Sat, 8 Jan 2011 13:04:04 +0100 Subject: [PATCH] improve authentication api for issue '#3 Add extension point for plugins to define groups and their members' --- .../scm/pam/PAMAuthenticationHandler.java | 2 +- .../web/security/AuthenticationManager.java | 6 +-- .../web/security/AuthenticationResult.java | 49 +++++++++++++++++++ .../web/security/BasicSecurityContext.java | 49 +++++++++++++++++-- .../security/ChainAuthenticatonManager.java | 13 ++--- .../ChainAuthenticationManagerTest.java | 22 ++++----- 6 files changed, 114 insertions(+), 27 deletions(-) diff --git a/plugins/scm-pam-plugin/src/main/java/sonia/scm/pam/PAMAuthenticationHandler.java b/plugins/scm-pam-plugin/src/main/java/sonia/scm/pam/PAMAuthenticationHandler.java index 1cf1c87311..207961eda6 100644 --- a/plugins/scm-pam-plugin/src/main/java/sonia/scm/pam/PAMAuthenticationHandler.java +++ b/plugins/scm-pam-plugin/src/main/java/sonia/scm/pam/PAMAuthenticationHandler.java @@ -138,7 +138,7 @@ public class PAMAuthenticationHandler implements AuthenticationHandler User user = new User(username); user.setAdmin(isAdmin(unixUser)); - result = new AuthenticationResult(user); + result = new AuthenticationResult(user, unixUser.getGroups()); } } catch (PAMException ex) diff --git a/scm-core/src/main/java/sonia/scm/web/security/AuthenticationManager.java b/scm-core/src/main/java/sonia/scm/web/security/AuthenticationManager.java index d0abd7b69a..1951a9d35d 100644 --- a/scm-core/src/main/java/sonia/scm/web/security/AuthenticationManager.java +++ b/scm-core/src/main/java/sonia/scm/web/security/AuthenticationManager.java @@ -37,7 +37,6 @@ package sonia.scm.web.security; import sonia.scm.Initable; import sonia.scm.ListenerSupport; -import sonia.scm.user.User; //~--- JDK imports ------------------------------------------------------------ @@ -65,7 +64,6 @@ public interface AuthenticationManager * * @return */ - public User authenticate(HttpServletRequest request, - HttpServletResponse response, String username, - String password); + public AuthenticationResult authenticate(HttpServletRequest request, + HttpServletResponse response, String username, String password); } diff --git a/scm-core/src/main/java/sonia/scm/web/security/AuthenticationResult.java b/scm-core/src/main/java/sonia/scm/web/security/AuthenticationResult.java index 5a581d3212..73bdf3a698 100644 --- a/scm-core/src/main/java/sonia/scm/web/security/AuthenticationResult.java +++ b/scm-core/src/main/java/sonia/scm/web/security/AuthenticationResult.java @@ -37,6 +37,10 @@ package sonia.scm.web.security; import sonia.scm.user.User; +//~--- JDK imports ------------------------------------------------------------ + +import java.util.Collection; + /** * * @author Sebastian Sdorra @@ -91,6 +95,37 @@ public class AuthenticationResult this.state = state; } + /** + * Constructs ... + * + * + * + * @param user + * @param groups + */ + public AuthenticationResult(User user, Collection groups) + { + this.user = user; + this.groups = groups; + this.state = AuthenticationState.SUCCESS; + } + + /** + * Constructs ... + * + * + * @param user + * @param groups + * @param state + */ + public AuthenticationResult(User user, Collection groups, + AuthenticationState state) + { + this.user = user; + this.groups = groups; + this.state = state; + } + //~--- methods -------------------------------------------------------------- /** @@ -118,6 +153,17 @@ public class AuthenticationResult //~--- get methods ---------------------------------------------------------- + /** + * Method description + * + * + * @return + */ + public Collection getGroups() + { + return groups; + } + /** * Method description * @@ -142,6 +188,9 @@ public class AuthenticationResult //~--- fields --------------------------------------------------------------- + /** Field description */ + private Collection groups; + /** Field description */ private AuthenticationState state; diff --git a/scm-webapp/src/main/java/sonia/scm/web/security/BasicSecurityContext.java b/scm-webapp/src/main/java/sonia/scm/web/security/BasicSecurityContext.java index 91f4cf4743..86643ff23a 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/security/BasicSecurityContext.java +++ b/scm-webapp/src/main/java/sonia/scm/web/security/BasicSecurityContext.java @@ -51,6 +51,7 @@ import sonia.scm.user.UserManager; import java.util.Collection; import java.util.HashSet; +import java.util.Iterator; import java.util.Set; import javax.servlet.http.HttpServletRequest; @@ -113,10 +114,13 @@ public class BasicSecurityContext implements WebSecurityContext HttpServletResponse response, String username, String password) { - user = authenticator.authenticate(request, response, username, password); + AuthenticationResult ar = authenticator.authenticate(request, response, + username, password); - if (user != null) + if (ar != null) { + user = ar.getUser(); + try { user.setLastLogin(System.currentTimeMillis()); @@ -138,7 +142,19 @@ public class BasicSecurityContext implements WebSecurityContext userManager.create(user); } + Collection groupCollection = ar.getGroups(); + + if (groupCollection != null) + { + groups.addAll(groupCollection); + } + loadGroups(); + + if (logger.isDebugEnabled()) + { + logGroups(); + } } catch (Exception ex) { @@ -161,7 +177,7 @@ public class BasicSecurityContext implements WebSecurityContext public void logout(HttpServletRequest request, HttpServletResponse response) { user = null; - groups = null; + groups = new HashSet(); } //~--- get methods ---------------------------------------------------------- @@ -220,8 +236,6 @@ public class BasicSecurityContext implements WebSecurityContext */ private void loadGroups() { - groups = new HashSet(); - Collection groupCollection = groupManager.getGroupsForMember(user.getName()); @@ -234,6 +248,31 @@ public class BasicSecurityContext implements WebSecurityContext } } + /** + * Method description + * + */ + private void logGroups() + { + StringBuilder msg = new StringBuilder("user "); + + msg.append(user.getName()).append(" is member of "); + + Iterator groupIt = groups.iterator(); + + while (groupIt.hasNext()) + { + msg.append(groupIt.next()); + + if (groupIt.hasNext()) + { + msg.append(", "); + } + } + + logger.debug(msg.toString()); + } + //~--- fields --------------------------------------------------------------- /** Field description */ diff --git a/scm-webapp/src/main/java/sonia/scm/web/security/ChainAuthenticatonManager.java b/scm-webapp/src/main/java/sonia/scm/web/security/ChainAuthenticatonManager.java index df08016332..d814b1f5d3 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/security/ChainAuthenticatonManager.java +++ b/scm-webapp/src/main/java/sonia/scm/web/security/ChainAuthenticatonManager.java @@ -97,11 +97,10 @@ public class ChainAuthenticatonManager extends AbstractAuthenticationManager * @return */ @Override - public User authenticate(HttpServletRequest request, - HttpServletResponse response, String username, - String password) + public AuthenticationResult authenticate(HttpServletRequest request, + HttpServletResponse response, String username, String password) { - User user = null; + AuthenticationResult ar = null; for (AuthenticationHandler authenticator : authenticationHandlerSet) { @@ -122,8 +121,10 @@ public class ChainAuthenticatonManager extends AbstractAuthenticationManager { if (result.getState().isSuccessfully() && (result.getUser() != null)) { - user = result.getUser(); + User user = result.getUser(); + user.setType(authenticator.getType()); + ar = result; // notify authentication listeners fireAuthenticationEvent(request, response, user); @@ -138,7 +139,7 @@ public class ChainAuthenticatonManager extends AbstractAuthenticationManager } } - return user; + return ar; } /** diff --git a/scm-webapp/src/test/java/sonia/scm/web/security/ChainAuthenticationManagerTest.java b/scm-webapp/src/test/java/sonia/scm/web/security/ChainAuthenticationManagerTest.java index 05b1b7e014..27810c5ad1 100644 --- a/scm-webapp/src/test/java/sonia/scm/web/security/ChainAuthenticationManagerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/web/security/ChainAuthenticationManagerTest.java @@ -69,10 +69,10 @@ public class ChainAuthenticationManagerTest extends AbstractTestBase @Test public void testAuthenticateFailed() { - User user = manager.authenticate(request, response, trillian.getName(), + AuthenticationResult result = manager.authenticate(request, response, trillian.getName(), "trillian"); - assertNull(user); + assertNull(result); } /** @@ -82,9 +82,9 @@ public class ChainAuthenticationManagerTest extends AbstractTestBase @Test public void testAuthenticateNotFound() { - User user = manager.authenticate(request, response, "dent", "trillian"); + AuthenticationResult result = manager.authenticate(request, response, "dent", "trillian"); - assertNull(user); + assertNull(result); } /** @@ -94,17 +94,17 @@ public class ChainAuthenticationManagerTest extends AbstractTestBase @Test public void testAuthenticateSuccess() { - User user = manager.authenticate(request, response, trillian.getName(), + AuthenticationResult result = manager.authenticate(request, response, trillian.getName(), "trillian123"); - assertNotNull(user); - assertUserEquals(trillian, user); - assertEquals("trilliansType", user.getType()); - user = manager.authenticate(request, response, perfect.getName(), + assertNotNull(result); + assertUserEquals(trillian, result.getUser()); + assertEquals("trilliansType", result.getUser().getType()); + result = manager.authenticate(request, response, perfect.getName(), "perfect123"); assertNotNull(perfect); - assertUserEquals(perfect, user); - assertEquals("perfectsType", user.getType()); + assertUserEquals(perfect, result.getUser()); + assertEquals("perfectsType", result.getUser().getType()); } /**