diff --git a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AuthenticationResource.java b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AuthenticationResource.java index 60e01e4e1f..c374eca42e 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AuthenticationResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AuthenticationResource.java @@ -59,7 +59,7 @@ import sonia.scm.ScmState; import sonia.scm.config.ScmConfiguration; import sonia.scm.group.GroupNames; import sonia.scm.repository.RepositoryManager; -import sonia.scm.security.PermissionCollector; +import sonia.scm.security.AuthorizationCollector; import sonia.scm.security.PermissionDescriptor; import sonia.scm.security.Role; import sonia.scm.security.SecuritySystem; @@ -121,7 +121,7 @@ public class AuthenticationResource public AuthenticationResource(SCMContextProvider contextProvider, ScmConfiguration configuration, RepositoryManager repositoryManger, UserManager userManager, SecuritySystem securitySystem, - PermissionCollector collector) + AuthorizationCollector collector) { this.contextProvider = contextProvider; this.configuration = configuration; @@ -338,7 +338,7 @@ public class AuthenticationResource Builder builder = ImmutableList.builder(); - for (Permission p : permissionCollector.collect()) + for (Permission p : permissionCollector.collect().getObjectPermissions()) { if (p instanceof StringablePermission) { @@ -380,7 +380,7 @@ public class AuthenticationResource private SCMContextProvider contextProvider; /** Field description */ - private PermissionCollector permissionCollector; + private AuthorizationCollector permissionCollector; /** Field description */ private RepositoryManager repositoryManger; diff --git a/scm-webapp/src/main/java/sonia/scm/security/PermissionCollector.java b/scm-webapp/src/main/java/sonia/scm/security/AuthorizationCollector.java similarity index 74% rename from scm-webapp/src/main/java/sonia/scm/security/PermissionCollector.java rename to scm-webapp/src/main/java/sonia/scm/security/AuthorizationCollector.java index 0d7d231b99..badc8ca932 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/PermissionCollector.java +++ b/scm-webapp/src/main/java/sonia/scm/security/AuthorizationCollector.java @@ -33,15 +33,19 @@ package sonia.scm.security; //~--- non-JDK imports -------------------------------------------------------- +import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList.Builder; +import com.google.common.collect.ImmutableSet; import com.google.common.eventbus.Subscribe; import com.google.inject.Inject; import com.google.inject.Singleton; import org.apache.shiro.SecurityUtils; +import org.apache.shiro.authz.AuthorizationInfo; import org.apache.shiro.authz.Permission; +import org.apache.shiro.authz.SimpleAuthorizationInfo; import org.apache.shiro.authz.permission.PermissionResolver; import org.apache.shiro.subject.PrincipalCollection; import org.apache.shiro.subject.Subject; @@ -52,36 +56,34 @@ import org.slf4j.LoggerFactory; import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; import sonia.scm.group.GroupNames; -import sonia.scm.repository.PermissionType; import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryDAO; import sonia.scm.repository.RepositoryEvent; import sonia.scm.user.User; +import sonia.scm.user.UserEvent; import sonia.scm.util.Util; //~--- JDK imports ------------------------------------------------------------ -import java.util.Collections; import java.util.List; +import java.util.Set; /** * * @author Sebastian Sdorra */ @Singleton -public class PermissionCollector +public class AuthorizationCollector { - // CACHE Authorization info ??? - /** Field description */ - private static final String NAME = "sonia.cache.permissions"; + private static final String CACHE_NAME = "sonia.cache.authorizing"; /** - * the logger for PermissionCollector + * the logger for AuthorizationCollector */ private static final Logger logger = - LoggerFactory.getLogger(PermissionCollector.class); + LoggerFactory.getLogger(AuthorizationCollector.class); //~--- constructors --------------------------------------------------------- @@ -96,11 +98,12 @@ public class PermissionCollector * @param resolver */ @Inject - public PermissionCollector(CacheManager cacheManager, + public AuthorizationCollector(CacheManager cacheManager, RepositoryDAO repositoryDAO, SecuritySystem securitySystem, PermissionResolver resolver) { - this.cache = cacheManager.getCache(String.class, List.class, NAME); + this.cache = cacheManager.getCache(String.class, AuthorizationInfo.class, + CACHE_NAME); this.repositoryDAO = repositoryDAO; this.securitySystem = securitySystem; this.resolver = resolver; @@ -114,24 +117,45 @@ public class PermissionCollector * * @return */ - public List collect() + public AuthorizationInfo collect() { - List permissions; + AuthorizationInfo authorizationInfo; Subject subject = SecurityUtils.getSubject(); if (subject.hasRole(Role.USER)) { - PrincipalCollection pc = subject.getPrincipals(); - - permissions = collect(pc.oneByType(User.class), - pc.oneByType(GroupNames.class)); + authorizationInfo = collect(subject.getPrincipals()); } else { - permissions = Collections.EMPTY_LIST; + authorizationInfo = new SimpleAuthorizationInfo(); } - return permissions; + return authorizationInfo; + } + + /** + * Method description + * + * + * @param event + */ + @Subscribe + public void onEvent(UserEvent event) + { + if (event.getEventType().isPost()) + { + User user = event.getItem(); + + if (logger.isDebugEnabled()) + { + logger.debug( + "clear cache of user {}, because user properties have changed", + user.getName()); + } + + cache.remove(user.getId()); + } } /** @@ -183,19 +207,39 @@ public class PermissionCollector * @param user * @param groups * + * @param principals + * * @return */ - List collect(User user, GroupNames groups) + AuthorizationInfo collect(PrincipalCollection principals) { - List permissions = cache.get(user.getName()); + Preconditions.checkNotNull(principals, "principals parameter is required"); - if (permissions == null) + User user = principals.oneByType(User.class); + + Preconditions.checkNotNull(user, "no user found in principal collection"); + + AuthorizationInfo info = cache.get(user.getId()); + + if (info == null) { - permissions = doCollect(user, groups); - cache.put(user.getName(), permissions); + if (logger.isTraceEnabled()) + { + logger.trace("collect AuthorizationInfo for user {}", user.getName()); + } + + GroupNames groupNames = principals.oneByType(GroupNames.class); + + info = createAuthorizationInfo(user, groupNames); + cache.put(user.getId(), info); + } + else if (logger.isTraceEnabled()) + { + logger.trace("retrieve AuthorizationInfo for user {} from cache", + user.getName()); } - return permissions; + return info; } /** @@ -324,31 +368,34 @@ public class PermissionCollector * * @return */ - private List doCollect(User user, GroupNames groups) + private AuthorizationInfo createAuthorizationInfo(User user, + GroupNames groups) { - Builder builder = ImmutableList.builder(); + Set roles; - if (user.isActive()) + if (user.isAdmin()) { - if (user.isAdmin()) + if (logger.isDebugEnabled()) { - //J- - builder.add( - new RepositoryPermission( - RepositoryPermission.WILDCARD, - PermissionType.OWNER - ) - ); - //J+ - } - else - { - collectRepositoryPermissions(builder, user, groups); - collectGlobalPermissions(builder, user, groups); + logger.debug("grant admin role for user {}", user.getName()); } + + roles = ImmutableSet.of(Role.USER, Role.ADMIN); + } + else + { + roles = ImmutableSet.of(Role.USER); } - return builder.build(); + SimpleAuthorizationInfo info = new SimpleAuthorizationInfo(roles); + + Builder permissions = ImmutableList.builder(); + + collectGlobalPermissions(permissions, user, groups); + collectRepositoryPermissions(permissions, user, groups); + info.addObjectPermissions(permissions.build()); + + return info; } //~--- get methods ---------------------------------------------------------- @@ -375,7 +422,7 @@ public class PermissionCollector //~--- fields --------------------------------------------------------------- /** Field description */ - private Cache cache; + private Cache cache; /** Field description */ private RepositoryDAO repositoryDAO; diff --git a/scm-webapp/src/main/java/sonia/scm/security/ScmRealm.java b/scm-webapp/src/main/java/sonia/scm/security/ScmRealm.java index ad9b12bd62..33fc84727d 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/ScmRealm.java +++ b/scm-webapp/src/main/java/sonia/scm/security/ScmRealm.java @@ -37,7 +37,6 @@ package sonia.scm.security; import com.google.common.base.Joiner; import com.google.common.collect.Sets; -import com.google.common.eventbus.Subscribe; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; @@ -52,7 +51,6 @@ import org.apache.shiro.authc.UnknownAccountException; import org.apache.shiro.authc.UsernamePasswordToken; import org.apache.shiro.authc.pam.UnsupportedTokenException; import org.apache.shiro.authz.AuthorizationInfo; -import org.apache.shiro.authz.SimpleAuthorizationInfo; import org.apache.shiro.realm.AuthorizingRealm; import org.apache.shiro.subject.PrincipalCollection; import org.apache.shiro.subject.SimplePrincipalCollection; @@ -61,8 +59,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.HandlerEvent; -import sonia.scm.cache.Cache; -import sonia.scm.cache.CacheManager; import sonia.scm.config.ScmConfiguration; import sonia.scm.event.Subscriber; import sonia.scm.group.Group; @@ -71,7 +67,6 @@ import sonia.scm.group.GroupNames; import sonia.scm.repository.RepositoryManager; import sonia.scm.user.User; import sonia.scm.user.UserDAO; -import sonia.scm.user.UserEvent; import sonia.scm.user.UserEventHack; import sonia.scm.user.UserException; import sonia.scm.user.UserManager; @@ -102,9 +97,6 @@ public class ScmRealm extends AuthorizingRealm /** Field description */ public static final String NAME = "scm"; - /** Field description */ - private static final String CACHE_NAME = "sonia.cache.authorizing"; - /** Field description */ private static final String SCM_CREDENTIALS = "SCM_CREDENTIALS"; @@ -135,8 +127,8 @@ public class ScmRealm extends AuthorizingRealm */ @Inject public ScmRealm(ScmConfiguration configuration, - PermissionCollector collector, CacheManager cacheManager, - UserManager userManager, GroupManager groupManager, UserDAO userDAO, + AuthorizationCollector collector,UserManager userManager, + GroupManager groupManager, UserDAO userDAO, AuthenticationManager authenticator, RepositoryManager manager, Provider requestProvider, Provider responseProvider) @@ -150,10 +142,6 @@ public class ScmRealm extends AuthorizingRealm this.requestProvider = requestProvider; this.responseProvider = responseProvider; - // init cache - this.cache = cacheManager.getCache(String.class, AuthorizationInfo.class, - CACHE_NAME); - // set token class setAuthenticationTokenClass(UsernamePasswordToken.class); @@ -168,30 +156,6 @@ public class ScmRealm extends AuthorizingRealm //~--- methods -------------------------------------------------------------- - /** - * Method description - * - * - * @param event - */ - @Subscribe - public void onEvent(UserEvent event) - { - if (event.getEventType().isPost()) - { - User user = event.getItem(); - - if (logger.isDebugEnabled()) - { - logger.debug( - "clear cache of user {}, because user properties have changed", - user.getName()); - } - - cache.remove(user.getId()); - } - } - /** * Method description * @@ -250,29 +214,7 @@ public class ScmRealm extends AuthorizingRealm protected AuthorizationInfo doGetAuthorizationInfo( PrincipalCollection principals) { - User user = principals.oneByType(User.class); - - AuthorizationInfo info = cache.get(user.getId()); - - if (info == null) - { - if (logger.isTraceEnabled()) - { - logger.trace("collect AuthorizationInfo for user {}", user.getName()); - } - - GroupNames groups = principals.oneByType(GroupNames.class); - - info = createAuthorizationInfo(user, groups); - cache.put(user.getId(), info); - } - else if (logger.isTraceEnabled()) - { - logger.trace("retrieve AuthorizationInfo for user {} from cache", - user.getName()); - } - - return info; + return collector.collect(principals); } /** @@ -476,39 +418,6 @@ public class ScmRealm extends AuthorizingRealm return new SimpleAuthenticationInfo(collection, token.getPassword()); } - /** - * Method description - * - * - * @param user - * @param groups - * - * @return - */ - private AuthorizationInfo createAuthorizationInfo(User user, - GroupNames groups) - { - Set roles = Sets.newHashSet(); - - roles.add(Role.USER); - - if (user.isAdmin()) - { - if (logger.isDebugEnabled()) - { - logger.debug("grant admin role for user {}", user.getName()); - } - - roles.add(Role.ADMIN); - } - - SimpleAuthorizationInfo info = new SimpleAuthorizationInfo(roles); - - info.addObjectPermissions(collector.collect(user, groups)); - - return info; - } - /** * Method description * @@ -628,10 +537,7 @@ public class ScmRealm extends AuthorizingRealm private AuthenticationManager authenticator; /** Field description */ - private Cache cache; - - /** Field description */ - private PermissionCollector collector; + private AuthorizationCollector collector; /** Field description */ private ScmConfiguration configuration; diff --git a/scm-webapp/src/main/resources/config/ehcache.xml b/scm-webapp/src/main/resources/config/ehcache.xml index 446e35b505..19abbde727 100644 --- a/scm-webapp/src/main/resources/config/ehcache.xml +++ b/scm-webapp/src/main/resources/config/ehcache.xml @@ -126,6 +126,7 @@ timeToIdleSeconds="1200" timeToLiveSeconds="2400" diskPersistent="false" + copyOnRead="true" />