remove GroupNames and ExternalGroupNames in favor of GroupCollector

2025-11-13 00:45:44 +01:00 · 2019-08-02 09:32:44 +02:00
parent 8550baaea9
commit 442aacbcdb
16 changed files with 100 additions and 566 deletions
--- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDto.java
+++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDto.java
@@ -7,7 +7,7 @@ import lombok.Getter;
 import lombok.NoArgsConstructor;
 import lombok.Setter;

-import java.util.List;
+import java.util.Set;

@Getter
@Setter
@@ -17,7 +17,7 @@ public class MeDto extends HalRepresentation {
  private String name;
  private String displayName;
  private String mail;
-  private List<String> groups;
+  private Set<String> groups;

  MeDto(Links links, Embedded embedded) {
    super(links, embedded);
--- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java
+++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java
@@ -1,18 +1,16 @@
 package sonia.scm.api.v2.resources;

-import com.google.common.collect.ImmutableList;
 import de.otto.edison.hal.Embedded;
 import de.otto.edison.hal.Links;
 import org.apache.shiro.SecurityUtils;
 import org.apache.shiro.subject.PrincipalCollection;
 import org.apache.shiro.subject.Subject;
-import sonia.scm.group.GroupNames;
+import sonia.scm.group.GroupCollector;
 import sonia.scm.user.User;
 import sonia.scm.user.UserManager;
 import sonia.scm.user.UserPermissions;

 import javax.inject.Inject;
-import java.util.Collections;

 import static de.otto.edison.hal.Embedded.embeddedBuilder;
 import static de.otto.edison.hal.Link.link;
@@ -22,11 +20,13 @@ public class MeDtoFactory extends HalAppenderMapper {

  private final ResourceLinks resourceLinks;
  private final UserManager userManager;
+  private final GroupCollector groupCollector;

  @Inject
-  public MeDtoFactory(ResourceLinks resourceLinks, UserManager userManager) {
+  public MeDtoFactory(ResourceLinks resourceLinks, UserManager userManager, GroupCollector groupCollector) {
    this.resourceLinks = resourceLinks;
    this.userManager = userManager;
+    this.groupCollector = groupCollector;
  }

  public MeDto create() {
@@ -35,16 +35,12 @@ public class MeDtoFactory extends HalAppenderMapper {

    MeDto dto = createDto(user);
    mapUserProperties(user, dto);
-    mapGroups(principals, dto);
+    mapGroups(user, dto);
    return dto;
  }

-  private void mapGroups(PrincipalCollection principals, MeDto dto) {
-    Iterable<String> groups = principals.oneByType(GroupNames.class);
-    if (groups == null) {
-      groups = Collections.emptySet();
-    }
-    dto.setGroups(ImmutableList.copyOf(groups));
+  private void mapGroups(User user, MeDto dto) {
+    dto.setGroups(groupCollector.collect(user.getName()));
  }

  private void mapUserProperties(User user, MeDto dto) {
--- a/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java
+++ b/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java
@@ -52,17 +52,18 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import sonia.scm.cache.Cache;
 import sonia.scm.cache.CacheManager;
-import sonia.scm.group.GroupNames;
+import sonia.scm.group.GroupCollector;
 import sonia.scm.group.GroupPermissions;
 import sonia.scm.plugin.Extension;
-import sonia.scm.repository.RepositoryPermission;
 import sonia.scm.repository.Repository;
 import sonia.scm.repository.RepositoryDAO;
+import sonia.scm.repository.RepositoryPermission;
 import sonia.scm.user.User;
 import sonia.scm.user.UserPermissions;
 import sonia.scm.util.Util;

 import java.util.Collection;
+import java.util.Set;

 //~--- JDK imports ------------------------------------------------------------

@@ -88,19 +89,21 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector

  /**
   * Constructs ...
-   *  @param cacheManager
+   * @param cacheManager
   * @param repositoryDAO
   * @param securitySystem
   * @param repositoryPermissionProvider
+   * @param groupCollector
   */
  @Inject
  public DefaultAuthorizationCollector(CacheManager cacheManager,
-                                       RepositoryDAO repositoryDAO, SecuritySystem securitySystem, RepositoryPermissionProvider repositoryPermissionProvider)
+                                       RepositoryDAO repositoryDAO, SecuritySystem securitySystem, RepositoryPermissionProvider repositoryPermissionProvider, GroupCollector groupCollector)
  {
    this.cache = cacheManager.getCache(CACHE_NAME);
    this.repositoryDAO = repositoryDAO;
    this.securitySystem = securitySystem;
    this.repositoryPermissionProvider = repositoryPermissionProvider;
+    this.groupCollector = groupCollector;
  }

  //~--- methods --------------------------------------------------------------
@@ -145,16 +148,16 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector

    Preconditions.checkNotNull(user, "no user found in principal collection");

-    GroupNames groupNames = principals.oneByType(GroupNames.class);
+    Set<String> groups = groupCollector.collect(user.getName());

-    CacheKey cacheKey = new CacheKey(user.getId(), groupNames);
+    CacheKey cacheKey = new CacheKey(user.getId(), groups);

    AuthorizationInfo info = cache.get(cacheKey);

    if (info == null)
    {
      logger.trace("collect AuthorizationInfo for user {}", user.getName());
-      info = createAuthorizationInfo(user, groupNames);
+      info = createAuthorizationInfo(user, groups);
      cache.put(cacheKey, info);
    }
    else if (logger.isTraceEnabled())
@@ -166,7 +169,7 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector
  }

  private void collectGlobalPermissions(Builder<String> builder,
-    final User user, final GroupNames groups)
+    final User user, final Set<String> groups)
  {
    Collection<AssignedPermission> globalPermissions =
      securitySystem.getPermissions((AssignedPermission input) -> isUserPermitted(user, groups, input));
@@ -181,7 +184,7 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector
  }

  private void collectRepositoryPermissions(Builder<String> builder, User user,
-    GroupNames groups)
+    Set<String> groups)
  {
    for (Repository repository : repositoryDAO.getAll())
    {
@@ -190,7 +193,7 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector
  }

  private void collectRepositoryPermissions(Builder<String> builder,
-    Repository repository, User user, GroupNames groups)
+    Repository repository, User user, Set<String> groups)
  {
    Collection<RepositoryPermission> repositoryPermissions = repository.getPermissions();

@@ -245,7 +248,7 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector
      .getVerbs();
  }

-  private AuthorizationInfo createAuthorizationInfo(User user, GroupNames groups) {
+  private AuthorizationInfo createAuthorizationInfo(User user, Set<String> groups) {
    Builder<String> builder = ImmutableSet.builder();

    collectGlobalPermissions(builder, user, groups);
@@ -279,7 +282,7 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector

  //~--- get methods ----------------------------------------------------------

-  private boolean isUserPermitted(User user, GroupNames groups,
+  private boolean isUserPermitted(User user, Set<String> groups,
    PermissionObject perm)
  {
    //J-
@@ -314,7 +317,7 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector
   */
  private static class CacheKey
  {
-    private CacheKey(String username, GroupNames groupnames)
+    private CacheKey(String username, Set<String> groupnames)
    {
      this.username = username;
      this.groupnames = groupnames;
@@ -356,7 +359,7 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector
    //~--- fields -------------------------------------------------------------

    /** group names */
-    private final GroupNames groupnames;
+    private final Set<String> groupnames;

    /** username */
    private final String username;
@@ -374,4 +377,5 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector
  private final SecuritySystem securitySystem;

  private final RepositoryPermissionProvider repositoryPermissionProvider;
+  private final GroupCollector groupCollector;
 }
--- a/scm-webapp/src/main/java/sonia/scm/security/DefaultRealm.java
+++ b/scm-webapp/src/main/java/sonia/scm/security/DefaultRealm.java
@@ -34,7 +34,6 @@ package sonia.scm.security;
 //~--- non-JDK imports --------------------------------------------------------

 import com.google.common.annotations.VisibleForTesting;
-
 import org.apache.shiro.authc.AuthenticationException;
 import org.apache.shiro.authc.AuthenticationInfo;
 import org.apache.shiro.authc.AuthenticationToken;
@@ -45,21 +44,16 @@ 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;
-import sonia.scm.group.GroupNames;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import sonia.scm.plugin.Extension;

-//~--- JDK imports ------------------------------------------------------------
-
 import javax.inject.Inject;
 import javax.inject.Singleton;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 import java.util.Set;

+//~--- JDK imports ------------------------------------------------------------
+
 /**
 * Default authorizing realm.
 *
@@ -149,7 +143,7 @@ public class DefaultRealm extends AuthorizingRealm
      LOG.trace("principal does not contain scope information, returning all permissions");
      log(principals, info, null);
    }
-    
+
    return info;
  }

@@ -180,8 +174,6 @@ public class DefaultRealm extends AuthorizingRealm
    StringBuilder buffer = new StringBuilder("authorization summary: ");
    
    buffer.append(SEPARATOR).append("username   : ").append(collection.getPrimaryPrincipal());
-    buffer.append(SEPARATOR).append("groups     : ");
-    append(buffer, collection.oneByType(GroupNames.class));
    buffer.append(SEPARATOR).append("roles      : ");
    append(buffer, original.getRoles()); 
    buffer.append(SEPARATOR).append("scope      : ");
--- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java
+++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java
@@ -40,11 +40,9 @@ import org.apache.shiro.SecurityUtils;
 import org.apache.shiro.subject.Subject;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import sonia.scm.group.ExternalGroupNames;

 import java.time.Clock;
 import java.time.Instant;
-import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -139,12 +137,6 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder {
    return this;
  }

-  @Override
-  public JwtAccessTokenBuilder groups(String... groups) {
-    Collections.addAll(this.groups, groups);
-    return this;
-  }
-
  JwtAccessTokenBuilder refreshExpiration(Instant refreshExpiration) {
    this.refreshExpiration = refreshExpiration;
    this.refreshableFor = 0;
@@ -206,16 +198,6 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder {
      claims.setIssuer(issuer);
    }

-    if (!groups.isEmpty()) {
-      claims.put(JwtAccessToken.GROUPS_CLAIM_KEY, groups);
-    } else {
-      Subject currentSubject = SecurityUtils.getSubject();
-      ExternalGroupNames externalGroupNames = currentSubject.getPrincipals().oneByType(ExternalGroupNames.class);
-      if (externalGroupNames != null) {
-        claims.put(JwtAccessToken.GROUPS_CLAIM_KEY, externalGroupNames.getCollection().toArray(new String[]{}));
-      }
-    }
-
    // sign token and create compact version
    String compact = Jwts.builder()
        .setClaims(claims)
--- a/scm-webapp/src/main/java/sonia/scm/web/security/DefaultAdministrationContext.java
+++ b/scm-webapp/src/main/java/sonia/scm/web/security/DefaultAdministrationContext.java
@@ -38,7 +38,6 @@ package sonia.scm.web.security;
 import com.google.inject.Inject;
 import com.google.inject.Injector;
 import com.google.inject.Singleton;
-
 import org.apache.shiro.SecurityUtils;
 import org.apache.shiro.subject.PrincipalCollection;
 import org.apache.shiro.subject.SimplePrincipalCollection;
@@ -46,21 +45,17 @@ import org.apache.shiro.subject.Subject;
 import org.apache.shiro.subject.support.SubjectThreadState;
 import org.apache.shiro.util.ThreadContext;
 import org.apache.shiro.util.ThreadState;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-
 import sonia.scm.SCMContext;
-import sonia.scm.group.GroupNames;
 import sonia.scm.security.Role;
 import sonia.scm.user.User;
 import sonia.scm.util.AssertUtil;

-//~--- JDK imports ------------------------------------------------------------
-
+import javax.xml.bind.JAXB;
 import java.net.URL;

-import javax.xml.bind.JAXB;
+//~--- JDK imports ------------------------------------------------------------

 /**
 *
@@ -161,7 +156,6 @@ public class DefaultAdministrationContext implements AdministrationContext

    collection.add(adminUser.getId(), REALM);
    collection.add(adminUser, REALM);
-    collection.add(new GroupNames(), REALM);
    collection.add(AdministrationContextMarker.MARKER, REALM);

    return collection;