merge with 2.0.0-m3

scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java

@@ -174,8 +174,8 @@ public class ScmConfiguration implements Configuration {
   @XmlElement(name = "xsrf-protection")
   private boolean enabledXsrfProtection = true;
 
-  @XmlElement(name = "default-namespace-strategy")
-  private String defaultNamespaceStrategy = "sonia.scm.repository.DefaultNamespaceStrategy";
+  @XmlElement(name = "namespace-strategy")
+  private String namespaceStrategy = "UsernameNamespaceStrategy";
 
 
   /**
@@ -215,7 +215,7 @@ public class ScmConfiguration implements Configuration {
     this.loginAttemptLimit = other.loginAttemptLimit;
     this.loginAttemptLimitTimeout = other.loginAttemptLimitTimeout;
     this.enabledXsrfProtection = other.enabledXsrfProtection;
-    this.defaultNamespaceStrategy = other.defaultNamespaceStrategy;
+    this.namespaceStrategy = other.namespaceStrategy;
   }
 
   /**
@@ -346,8 +346,8 @@ public class ScmConfiguration implements Configuration {
     return loginAttemptLimit > 0;
   }
 
-  public String getDefaultNamespaceStrategy() {
-    return defaultNamespaceStrategy;
+  public String getNamespaceStrategy() {
+    return namespaceStrategy;
   }
 
 
@@ -473,8 +473,8 @@ public class ScmConfiguration implements Configuration {
     this.enabledXsrfProtection = enabledXsrfProtection;
   }
 
-  public void setDefaultNamespaceStrategy(String defaultNamespaceStrategy) {
-    this.defaultNamespaceStrategy = defaultNamespaceStrategy;
+  public void setNamespaceStrategy(String namespaceStrategy) {
+    this.namespaceStrategy = namespaceStrategy;
   }
 
   @Override

scm-core/src/main/java/sonia/scm/group/ExternalGroupNames.java

@@ -0,0 +1,22 @@
+package sonia.scm.group;
+
+import java.util.Collection;
+
+/**
+ * This class represents all associated groups which are provided by external systems for a certain user.
+ *
+ * @author Sebastian Sdorra
+ * @since 2.0.0
+ */
+public class ExternalGroupNames extends GroupNames {
+  public ExternalGroupNames() {
+  }
+
+  public ExternalGroupNames(String groupName, String... groupNames) {
+    super(groupName, groupNames);
+  }
+
+  public ExternalGroupNames(Collection<String> collection) {
+    super(collection);
+  }
+}

scm-core/src/main/java/sonia/scm/group/GroupNames.java

@@ -52,7 +52,7 @@ import java.util.Iterator;
  * @author Sebastian Sdorra
  * @since 1.21
  */
-public final class GroupNames implements Serializable, Iterable<String>
+public class GroupNames implements Serializable, Iterable<String>
 {
 
   /**
@@ -94,20 +94,8 @@ public final class GroupNames implements Serializable, Iterable<String>
    * @param collection
    */
   public GroupNames(Collection<String> collection)
-  {
-    this(collection, false);
-  }
-
-  /**
-   * Constructs ...
-   *
-   *
-   * @param collection
-   */
-  public GroupNames(Collection<String> collection, boolean external)
   {
     this.collection = Collections.unmodifiableCollection(collection);
-    this.external = external;
   }
 
   //~--- methods --------------------------------------------------------------
@@ -176,7 +164,7 @@ public final class GroupNames implements Serializable, Iterable<String>
   @Override
   public String toString()
   {
-    return Joiner.on(", ").join(collection) + "(" + (external? "external": "internal") + ")";
+    return Joiner.on(", ").join(collection);
   }
 
   //~--- get methods ----------------------------------------------------------
@@ -192,13 +180,8 @@ public final class GroupNames implements Serializable, Iterable<String>
     return collection;
   }
 
-  public boolean isExternal() {
-    return external;
-  }
 
   //~--- fields ---------------------------------------------------------------
   /** Field description */
   private final Collection<String> collection;
-
-  private final boolean external;
 }

scm-core/src/main/java/sonia/scm/repository/NamespaceStrategyProvider.java

@@ -1,32 +0,0 @@
-package sonia.scm.repository;
-
-import sonia.scm.config.ScmConfiguration;
-
-import javax.inject.Inject;
-import javax.inject.Provider;
-import java.util.Set;
-
-public class NamespaceStrategyProvider implements Provider<NamespaceStrategy> {
-
-  private final Set<NamespaceStrategy> strategies;
-  private final ScmConfiguration scmConfiguration;
-
-  @Inject
-  public NamespaceStrategyProvider(Set<NamespaceStrategy> strategies, ScmConfiguration scmConfiguration) {
-    this.strategies = strategies;
-    this.scmConfiguration = scmConfiguration;
-  }
-
-  @Override
-  public NamespaceStrategy get() {
-    String namespaceStrategy = scmConfiguration.getDefaultNamespaceStrategy();
-
-    for (NamespaceStrategy s : this.strategies) {
-      if (s.getClass().getCanonicalName().equals(namespaceStrategy)) {
-          return s;
-      }
-    }
-    return null;
-  }
-
-}

scm-core/src/main/java/sonia/scm/repository/Repository.java

@@ -248,7 +248,8 @@ public class Repository extends BasicPropertiesAware implements ModelObject, Per
   /**
    * Returns true if the {@link Repository} is valid.
    * <ul>
-   * <li>The name is not empty and contains only A-z, 0-9, _, -, /</li>
+   * <li>The namespace is valid</li>
+   * <li>The name is valid</li>
    * <li>The type is not empty</li>
    * <li>The contact is empty or contains a valid email address</li>
    * </ul>
@@ -257,9 +258,10 @@ public class Repository extends BasicPropertiesAware implements ModelObject, Per
    */
   @Override
   public boolean isValid() {
-    return ValidationUtil.isRepositoryNameValid(name) && Util.isNotEmpty(type)
-      && ((Util.isEmpty(contact))
-      || ValidationUtil.isMailAddressValid(contact));
+    return ValidationUtil.isRepositoryNameValid(namespace)
+      && ValidationUtil.isRepositoryNameValid(name)
+      && Util.isNotEmpty(type)
+      && ((Util.isEmpty(contact)) || ValidationUtil.isMailAddressValid(contact));
   }
 
   /**

scm-core/src/main/java/sonia/scm/security/DAORealmHelper.java

@@ -35,8 +35,6 @@ package sonia.scm.security;
 
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.ImmutableSet.Builder;
 import org.apache.shiro.authc.AuthenticationInfo;
 import org.apache.shiro.authc.AuthenticationToken;
 import org.apache.shiro.authc.DisabledAccountException;
@@ -47,9 +45,7 @@ import org.apache.shiro.authc.credential.CredentialsMatcher;
 import org.apache.shiro.subject.SimplePrincipalCollection;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import sonia.scm.group.Group;
 import sonia.scm.group.GroupDAO;
-import sonia.scm.group.GroupNames;
 import sonia.scm.user.User;
 import sonia.scm.user.UserDAO;
 
@@ -75,7 +71,7 @@ public final class DAORealmHelper {
   
   private final UserDAO userDAO;
   
-  private final GroupDAO groupDAO;
+  private final GroupCollector groupCollector;
 
   private final String realm;
   
@@ -87,14 +83,14 @@ public final class DAORealmHelper {
    *
    * @param loginAttemptHandler login attempt handler for wrapping credentials matcher
    * @param userDAO user dao
-   * @param groupDAO group dao
+   * @param groupCollector collect groups for a principal
    * @param realm name of realm
    */
-  public DAORealmHelper(LoginAttemptHandler loginAttemptHandler, UserDAO userDAO, GroupDAO groupDAO, String realm) {
+  public DAORealmHelper(LoginAttemptHandler loginAttemptHandler, UserDAO userDAO, GroupCollector groupCollector, String realm) {
     this.loginAttemptHandler = loginAttemptHandler;
     this.realm = realm;
     this.userDAO = userDAO;
-    this.groupDAO = groupDAO;
+    this.groupCollector = groupCollector;
   }
 
   //~--- get methods ----------------------------------------------------------
@@ -157,7 +153,7 @@ public final class DAORealmHelper {
 
     collection.add(principal, realm);
     collection.add(user, realm);
-    collection.add(collectGroups(principal, groups), realm);
+    collection.add(groupCollector.collect(principal, groups), realm);
     collection.add(MoreObjects.firstNonNull(scope, Scope.empty()), realm);
 
     String creds = credentials;
@@ -171,26 +167,6 @@ public final class DAORealmHelper {
 
   //~--- methods --------------------------------------------------------------
 
-  private GroupNames collectGroups(String principal, Iterable<String> groupNames) {
-    Builder<String> builder = ImmutableSet.builder();
-
-    builder.add(GroupNames.AUTHENTICATED);
-
-    for (String group : groupNames) {
-      builder.add(group);
-    }
-
-    for (Group group : groupDAO.getAll()) {
-      if (group.isMember(principal)) {
-        builder.add(group.getName());
-      }
-    }
-
-    GroupNames groups = new GroupNames(builder.build());
-    LOG.debug("collected following groups for principal {}: {}", principal, groups);
-    return groups;
-  }
-
   /**
    * Builder class for {@link AuthenticationInfo}.
    */

scm-core/src/main/java/sonia/scm/security/DAORealmHelperFactory.java

@@ -30,10 +30,11 @@
  */
 package sonia.scm.security;
 
-import javax.inject.Inject;
 import sonia.scm.group.GroupDAO;
 import sonia.scm.user.UserDAO;
 
+import javax.inject.Inject;
+
 /**
  * Factory to create {@link DAORealmHelper} instances.
  * 
@@ -44,7 +45,7 @@ public final class DAORealmHelperFactory {
   
   private final LoginAttemptHandler loginAttemptHandler;
   private final UserDAO userDAO; 
-  private final GroupDAO groupDAO;
+  private final GroupCollector groupCollector;
 
   /**
    * Constructs a new instance.
@@ -57,7 +58,7 @@ public final class DAORealmHelperFactory {
   public DAORealmHelperFactory(LoginAttemptHandler loginAttemptHandler, UserDAO userDAO, GroupDAO groupDAO) {
     this.loginAttemptHandler = loginAttemptHandler;
     this.userDAO = userDAO;
-    this.groupDAO = groupDAO;
+    this.groupCollector = new GroupCollector(groupDAO);
   }
   
   /**
@@ -68,7 +69,7 @@ public final class DAORealmHelperFactory {
    * @return new {@link DAORealmHelper} instance.
    */
   public DAORealmHelper create(String realm) {
-    return new DAORealmHelper(loginAttemptHandler, userDAO, groupDAO, realm);
+    return new DAORealmHelper(loginAttemptHandler, userDAO, groupCollector, realm);
   }
   
 }

scm-core/src/main/java/sonia/scm/security/GroupCollector.java

@@ -0,0 +1,43 @@
+package sonia.scm.security;
+
+import com.google.common.collect.ImmutableSet;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import sonia.scm.group.Group;
+import sonia.scm.group.GroupDAO;
+import sonia.scm.group.GroupNames;
+
+/**
+ * Collect groups for a certain principal.
+ * <strong>Warning</strong>: The class is only for internal use and should never used directly.
+ */
+class GroupCollector {
+
+  private static final Logger LOG = LoggerFactory.getLogger(GroupCollector.class);
+
+  private final GroupDAO groupDAO;
+
+  GroupCollector(GroupDAO groupDAO) {
+    this.groupDAO = groupDAO;
+  }
+
+  GroupNames collect(String principal, Iterable<String> groupNames) {
+    ImmutableSet.Builder<String> builder = ImmutableSet.builder();
+
+    builder.add(GroupNames.AUTHENTICATED);
+
+    for (String group : groupNames) {
+      builder.add(group);
+    }
+
+    for (Group group : groupDAO.getAll()) {
+      if (group.isMember(principal)) {
+        builder.add(group.getName());
+      }
+    }
+
+    GroupNames groups = new GroupNames(builder.build());
+    LOG.debug("collected following groups for principal {}: {}", principal, groups);
+    return groups;
+  }
+}

scm-core/src/main/java/sonia/scm/security/SyncingRealmHelper.java

@@ -28,7 +28,6 @@
  */
 package sonia.scm.security;
 
-import com.google.common.collect.ImmutableList;
 import com.google.inject.Inject;
 import org.apache.shiro.authc.AuthenticationInfo;
 import org.apache.shiro.authc.SimpleAuthenticationInfo;
@@ -37,20 +36,19 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import sonia.scm.AlreadyExistsException;
 import sonia.scm.NotFoundException;
+import sonia.scm.group.ExternalGroupNames;
 import sonia.scm.group.Group;
+import sonia.scm.group.GroupDAO;
 import sonia.scm.group.GroupManager;
-import sonia.scm.group.GroupNames;
 import sonia.scm.plugin.Extension;
 import sonia.scm.user.User;
 import sonia.scm.user.UserManager;
 import sonia.scm.web.security.AdministrationContext;
 
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 
 import static java.util.Arrays.asList;
-import static java.util.Collections.emptyList;
 
 /**
  * Helper class for syncing realms. The class should simplify the creation of realms, which are syncing authenticated
@@ -65,24 +63,24 @@ public final class SyncingRealmHelper {
   private static final Logger LOG = LoggerFactory.getLogger(SyncingRealmHelper.class);
 
   private final AdministrationContext ctx;
-  
-  private final GroupManager groupManager;
-
   private final UserManager userManager;
-  
+  private final GroupManager groupManager;
+  private final GroupCollector groupCollector;
+
   /**
    * Constructs a new SyncingRealmHelper.
    *
-   *
    * @param ctx administration context
    * @param userManager user manager
    * @param groupManager group manager
+   * @param groupDAO group dao
    */
   @Inject
-  public SyncingRealmHelper(AdministrationContext ctx, UserManager userManager, GroupManager groupManager) {
+  public SyncingRealmHelper(AdministrationContext ctx, UserManager userManager, GroupManager groupManager, GroupDAO groupDAO) {
     this.ctx = ctx;
     this.userManager = userManager;
     this.groupManager = groupManager;
+    this.groupCollector = new GroupCollector(groupDAO);
   }
 
   /**
@@ -95,11 +93,11 @@ public final class SyncingRealmHelper {
   public class AuthenticationInfoBuilder {
     private String realm;
     private User user;
-    private Collection<String> groups;
-    private boolean external;
+    private Collection<String> groups = Collections.emptySet();
+    private Collection<String> externalGroups = Collections.emptySet();
 
     private AuthenticationInfo build() {
-      return SyncingRealmHelper.this.createAuthenticationInfo(realm, user, groups, external);
+      return SyncingRealmHelper.this.createAuthenticationInfo(realm, user, groups, externalGroups);
     }
 
     public class ForRealm {
@@ -134,52 +132,51 @@ public final class SyncingRealmHelper {
       private WithGroups() {
       }
 
-      /**
-       * Build the authentication info without groups.
-       * @return The complete {@link AuthenticationInfo}
-       */
-      public AuthenticationInfo withoutGroups() {
-        return withGroups(emptyList());
-      }
-
       /**
        * Set the internal groups for the user.
        * @param groups groups of the authenticated user
-       * @return The complete {@link AuthenticationInfo}
+       * @return builder step for groups
        */
-      public AuthenticationInfo withGroups(String... groups) {
+      public WithGroups withGroups(String... groups) {
         return withGroups(asList(groups));
       }
 
       /**
        * Set the internal groups for the user.
        * @param groups groups of the authenticated user
-       * @return The complete {@link AuthenticationInfo}
+       * @return builder step for groups
        */
-      public AuthenticationInfo withGroups(Collection<String> groups) {
+      public WithGroups withGroups(Collection<String> groups) {
         AuthenticationInfoBuilder.this.groups = groups;
-        AuthenticationInfoBuilder.this.external = false;
-        return build();
+        return this;
       }
 
       /**
        * Set the external groups for the user.
-       * @param groups external groups of the authenticated user
-       * @return The complete {@link AuthenticationInfo}
+       * @param externalGroups external groups of the authenticated user
+       * @return builder step for groups
        */
-      public AuthenticationInfo withExternalGroups(String... groups) {
-        return withExternalGroups(asList(groups));
+      public WithGroups withExternalGroups(String... externalGroups) {
+        return withExternalGroups(asList(externalGroups));
       }
 
       /**
        * Set the external groups for the user.
-       * @param groups external groups of the authenticated user
-       * @return The complete {@link AuthenticationInfo}
+       * @param externalGroups external groups of the authenticated user
+       * @return builder step for groups
        */
-      public AuthenticationInfo withExternalGroups(Collection<String> groups) {
-        AuthenticationInfoBuilder.this.groups = groups;
-        AuthenticationInfoBuilder.this.external = true;
-        return build();
+      public WithGroups withExternalGroups(Collection<String> externalGroups) {
+        AuthenticationInfoBuilder.this.externalGroups = externalGroups;
+        return this;
+      }
+
+      /**
+       * Builds the {@link AuthenticationInfo} from the given options.
+       *
+       * @return complete autentication info
+       */
+      public AuthenticationInfo build() {
+        return AuthenticationInfoBuilder.this.build();
       }
     }
   }
@@ -197,12 +194,13 @@ public final class SyncingRealmHelper {
    * @return authentication info
    */
   private AuthenticationInfo createAuthenticationInfo(String realm, User user,
-    Collection<String> groups, boolean externalGroups) {
+    Collection<String> groups, Collection<String> externalGroups) {
     SimplePrincipalCollection collection = new SimplePrincipalCollection();
 
     collection.add(user.getId(), realm);
     collection.add(user, realm);
-    collection.add(new GroupNames(groups, externalGroups), realm);
+    collection.add(groupCollector.collect(user.getId(), groups), realm);
+    collection.add(new ExternalGroupNames(externalGroups), realm);
 
     return new SimpleAuthenticationInfo(collection, user.getPassword());
   }

scm-core/src/main/java/sonia/scm/util/ValidationUtil.java

@@ -35,14 +35,12 @@ package sonia.scm.util;
 
 //~--- non-JDK imports --------------------------------------------------------
 
-import com.google.common.base.Splitter;
-
 import sonia.scm.Validateable;
 
-//~--- JDK imports ------------------------------------------------------------
-
 import java.util.regex.Pattern;
 
+//~--- JDK imports ------------------------------------------------------------
+
 /**
  *
  * @author Sebastian Sdorra
@@ -52,15 +50,16 @@ public final class ValidationUtil
 
   /** Field description */
   private static final String REGEX_MAIL =
-    "^[A-z0-9][\\w.-]*@[A-z0-9][\\w\\-\\.]*\\.[A-z0-9][A-z0-9-]+$";
+    "^[A-Za-z0-9][\\w.-]*@[A-Za-z0-9][\\w\\-\\.]*\\.[A-Za-z0-9][A-Za-z0-9-]+$";
 
   /** Field description */
   private static final String REGEX_NAME =
-    "^[A-z0-9\\.\\-_@]|[^ ]([A-z0-9\\.\\-_@ ]*[A-z0-9\\.\\-_@]|[^ ])?$";
+    "^[A-Za-z0-9\\.\\-_@]|[^ ]([A-Za-z0-9\\.\\-_@ ]*[A-Za-z0-9\\.\\-_@]|[^ ])?$";
+
+  public static final String REGEX_REPOSITORYNAME = "(?!^\\.\\.$)(?!^\\.$)(?!.*[\\\\\\[\\]])^[A-Za-z0-9\\.][A-Za-z0-9\\.\\-_]*$";
 
   /** Field description */
-  private static final String REGEX_REPOSITORYNAME =
-    "(?!^\\.\\.$)(?!^\\.$)(?!.*[\\\\\\[\\]])^[A-z0-9\\.][A-z0-9\\.\\-_/]*$";
+  private static final Pattern PATTERN_REPOSITORYNAME = Pattern.compile(REGEX_REPOSITORYNAME);
 
   //~--- constructors ---------------------------------------------------------
 
@@ -142,37 +141,15 @@ public final class ValidationUtil
   }
 
   /**
-   * Method description
+   * Returns {@code true} if the repository name is valid.
    *
-   *
-   * @param name
+   * @param name repository name
    * @since 1.9
    *
-   * @return
+   * @return {@code true} if repository name is valid
    */
-  public static boolean isRepositoryNameValid(String name)
-  {
-    Pattern pattern = Pattern.compile(REGEX_REPOSITORYNAME);
-    boolean result = true;
-
-    if (Util.isNotEmpty(name))
-    {
-      for (String p : Splitter.on('/').split(name))
-      {
-        if (!pattern.matcher(p).matches())
-        {
-          result = false;
-
-          break;
-        }
-      }
-    }
-    else
-    {
-      result = false;
-    }
-
-    return result;
+  public static boolean isRepositoryNameValid(String name) {
+    return PATTERN_REPOSITORYNAME.matcher(name).matches();
   }
 
   /**

scm-core/src/main/java/sonia/scm/web/VndMediaType.java

@@ -46,6 +46,8 @@ public class VndMediaType {
   public static final String MERGE_RESULT = PREFIX + "mergeResult" + SUFFIX;
   public static final String MERGE_COMMAND = PREFIX + "mergeCommand" + SUFFIX;
 
+  public static final String NAMESPACE_STRATEGIES = PREFIX + "namespaceStrategies" + SUFFIX;
+
   public static final String ME = PREFIX + "me" + SUFFIX;
   public static final String SOURCE = PREFIX + "source" + SUFFIX;
   public static final String ERROR_TYPE = PREFIX + "error" + SUFFIX;

scm-core/src/test/java/sonia/scm/security/DAORealmHelperTest.java

@@ -6,6 +6,7 @@ import org.apache.shiro.authc.DisabledAccountException;
 import org.apache.shiro.authc.UnknownAccountException;
 import org.apache.shiro.authc.UsernamePasswordToken;
 import org.apache.shiro.subject.PrincipalCollection;
+import org.junit.Ignore;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -37,7 +38,7 @@ class DAORealmHelperTest {
 
   @BeforeEach
   void setUpObjectUnderTest() {
-    helper = new DAORealmHelper(loginAttemptHandler, userDAO, groupDAO, "hitchhiker");
+    helper = new DAORealmHelper(loginAttemptHandler, userDAO, new GroupCollector(groupDAO), "hitchhiker");
   }
 
   @Test
@@ -77,6 +78,7 @@ class DAORealmHelperTest {
   }
 
   @Test
+  @Ignore
   void shouldReturnAuthenticationInfoWithGroups() {
     User user = new User("trillian");
     when(userDAO.get("trillian")).thenReturn(user);

scm-core/src/test/java/sonia/scm/security/GroupCollectorTest.java

@@ -0,0 +1,64 @@
+package sonia.scm.security;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import sonia.scm.group.Group;
+import sonia.scm.group.GroupDAO;
+import sonia.scm.group.GroupNames;
+
+import java.util.Collections;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+class GroupCollectorTest {
+
+  @Mock
+  private GroupDAO groupDAO;
+
+  @InjectMocks
+  private GroupCollector collector;
+
+  @Test
+  void shouldAlwaysReturnAuthenticatedGroup() {
+    GroupNames groupNames = collector.collect("trillian", Collections.emptySet());
+    assertThat(groupNames).containsOnly("_authenticated");
+  }
+
+  @Nested
+  class WithGroupsFromDao {
+
+    @BeforeEach
+    void setUpGroupsDao() {
+      List<Group> groups = Lists.newArrayList(
+        new Group("xml", "heartOfGold", "trillian"),
+        new Group("xml", "g42", "dent", "prefect"),
+        new Group("xml", "fjordsOfAfrican", "dent", "trillian")
+      );
+      when(groupDAO.getAll()).thenReturn(groups);
+    }
+
+    @Test
+    void shouldReturnGroupsFromDao() {
+      GroupNames groupNames = collector.collect("trillian", Collections.emptySet());
+      assertThat(groupNames).contains("_authenticated", "heartOfGold", "fjordsOfAfrican");
+    }
+
+    @Test
+    void shouldCombineGivenWithDao() {
+      GroupNames groupNames = collector.collect("trillian", ImmutableList.of("awesome", "incredible"));
+      assertThat(groupNames).contains("_authenticated", "heartOfGold", "fjordsOfAfrican", "awesome", "incredible");
+    }
+
+  }
+
+}

scm-core/src/test/java/sonia/scm/security/SyncingRealmHelperTest.java

@@ -36,6 +36,7 @@ package sonia.scm.security;
 //~--- non-JDK imports --------------------------------------------------------
 
 import com.google.common.base.Throwables;
+import com.google.common.collect.Lists;
 import org.apache.shiro.authc.AuthenticationInfo;
 import org.assertj.core.api.Assertions;
 import org.junit.Before;
@@ -44,7 +45,9 @@ import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 import sonia.scm.AlreadyExistsException;
+import sonia.scm.group.ExternalGroupNames;
 import sonia.scm.group.Group;
+import sonia.scm.group.GroupDAO;
 import sonia.scm.group.GroupManager;
 import sonia.scm.group.GroupNames;
 import sonia.scm.user.User;
@@ -53,19 +56,11 @@ import sonia.scm.web.security.AdministrationContext;
 import sonia.scm.web.security.PrivilegedAction;
 
 import java.io.IOException;
+import java.util.List;
 
-import static java.util.Collections.singletonList;
-import static org.assertj.core.util.Arrays.asList;
 import static org.hamcrest.Matchers.hasItem;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.doThrow;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
 
 //~--- JDK imports ------------------------------------------------------------
 
@@ -83,6 +78,9 @@ public class SyncingRealmHelperTest {
   @Mock
   private UserManager userManager;
 
+  @Mock
+  private GroupDAO groupDAO;
+
   private SyncingRealmHelper helper;
 
   /**
@@ -108,7 +106,7 @@ public class SyncingRealmHelperTest {
       }
     };
 
-    helper = new SyncingRealmHelper(ctx, userManager, groupManager);
+    helper = new SyncingRealmHelper(ctx, userManager, groupManager, groupDAO);
   }
 
   /**
@@ -191,11 +189,11 @@ public class SyncingRealmHelperTest {
       .authenticationInfo()
       .forRealm("unit-test")
       .andUser(new User("ziltoid"))
-      .withGroups("internal");
+      .withGroups("internal")
+      .build();
 
     GroupNames groupNames = authenticationInfo.getPrincipals().oneByType(GroupNames.class);
-    Assertions.assertThat(groupNames.getCollection()).containsOnly("internal");
-    Assertions.assertThat(groupNames.isExternal()).isFalse();
+    Assertions.assertThat(groupNames.getCollection()).contains("_authenticated", "internal");
   }
 
   @Test
@@ -204,11 +202,11 @@ public class SyncingRealmHelperTest {
       .authenticationInfo()
       .forRealm("unit-test")
       .andUser(new User("ziltoid"))
-      .withExternalGroups("external");
+      .withExternalGroups("external")
+      .build();
 
-    GroupNames groupNames = authenticationInfo.getPrincipals().oneByType(GroupNames.class);
+    ExternalGroupNames groupNames = authenticationInfo.getPrincipals().oneByType(ExternalGroupNames.class);
     Assertions.assertThat(groupNames.getCollection()).containsOnly("external");
-    Assertions.assertThat(groupNames.isExternal()).isTrue();
   }
 
   @Test
@@ -218,11 +216,34 @@ public class SyncingRealmHelperTest {
       .authenticationInfo()
       .forRealm("unit-test")
       .andUser(user)
-      .withoutGroups();
+      .build();
 
     assertNotNull(authInfo);
     assertEquals("ziltoid", authInfo.getPrincipals().getPrimaryPrincipal());
     assertThat(authInfo.getPrincipals().getRealmNames(), hasItem("unit-test"));
     assertEquals(user, authInfo.getPrincipals().oneByType(User.class));
   }
+
+  @Test
+  public void shouldReturnCombinedGroupNames() {
+    User user = new User("tricia");
+
+    List<Group> groups = Lists.newArrayList(new Group("xml", "heartOfGold", "tricia"));
+    when(groupDAO.getAll()).thenReturn(groups);
+
+    AuthenticationInfo authInfo = helper
+      .authenticationInfo()
+      .forRealm("unit-test")
+      .andUser(user)
+      .withGroups("fjordsOfAfrican")
+      .withExternalGroups("g42")
+      .build();
+
+
+    GroupNames groupNames = authInfo.getPrincipals().oneByType(GroupNames.class);
+    Assertions.assertThat(groupNames).contains("_authenticated", "heartOfGold", "fjordsOfAfrican");
+
+    ExternalGroupNames externalGroupNames = authInfo.getPrincipals().oneByType(ExternalGroupNames.class);
+    Assertions.assertThat(externalGroupNames).contains("g42");
+  }
 }

scm-core/src/test/java/sonia/scm/util/ValidationUtilTest.java

@@ -143,51 +143,21 @@ public class ValidationUtilTest
     assertFalse(ValidationUtil.isNotContaining("test", "t"));
   }
 
-  /**
-   * Method description
-   *
-   */
   @Test
-  public void testIsRepositoryNameValid()
-  {
-    assertTrue(ValidationUtil.isRepositoryNameValid("scm"));
-    assertTrue(ValidationUtil.isRepositoryNameValid("scm/main"));
-    assertTrue(ValidationUtil.isRepositoryNameValid("scm/plugins/git-plugin"));
-    assertTrue(ValidationUtil.isRepositoryNameValid("s"));
-    assertTrue(ValidationUtil.isRepositoryNameValid("sc"));
-    assertTrue(ValidationUtil.isRepositoryNameValid(".scm/plugins"));
-
-    // issue 142
-    assertFalse(ValidationUtil.isRepositoryNameValid("."));
-    assertFalse(ValidationUtil.isRepositoryNameValid("/"));
-    assertFalse(ValidationUtil.isRepositoryNameValid("scm/plugins/."));
-    assertFalse(ValidationUtil.isRepositoryNameValid("scm/../plugins"));
-    assertFalse(ValidationUtil.isRepositoryNameValid("scm/main/"));
-    assertFalse(ValidationUtil.isRepositoryNameValid("/scm/main/"));
-
-    // issue 144
-    assertFalse(ValidationUtil.isRepositoryNameValid("scm/./main"));
-    assertFalse(ValidationUtil.isRepositoryNameValid("scm//main"));
-
-    // issue 148
-    //J-
+  public void testIsRepositoryNameValid() {
     String[] validPaths = {
       "scm",
-      "scm/main",
-      "scm/plugins/git-plugin",
       "s",
       "sc",
-      ".scm/plugins",
       ".hiddenrepo",
       "b.",
       "...",
       "..c",
       "d..",
-      "a/b..",
-      "a/..b",
-      "a..c",
+      "a..c"
     };
-    
+
+    // issue 142, 144 and 148
     String[] invalidPaths = {
       ".",
       "/",
@@ -228,17 +198,22 @@ public class ValidationUtilTest
       "abc)abc",
       "abc[abc",
       "abc]abc",
-      "abc|abc"
+      "abc|abc",
+      "scm/main",
+      "scm/plugins/git-plugin",
+      ".scm/plugins",
+      "a/b..",
+      "a/..b",
+      "scm/main",
+      "scm/plugins/git-plugin",
+      "scm/plugins/git-plugin"
     };
-    //J+
 
-    for (String path : validPaths)
-    {
+    for (String path : validPaths) {
       assertTrue(ValidationUtil.isRepositoryNameValid(path));
     }
 
-    for (String path : invalidPaths)
-    {
+    for (String path : invalidPaths) {
       assertFalse(ValidationUtil.isRepositoryNameValid(path));
     }
   }