merge 2.0.0-m3

scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeDtoFactoryTest.java

@@ -1,9 +1,9 @@
 package sonia.scm.api.v2.resources;
 
+import com.google.common.collect.ImmutableSet;
 import org.apache.shiro.subject.PrincipalCollection;
 import org.apache.shiro.subject.Subject;
 import org.apache.shiro.util.ThreadContext;
-import org.assertj.core.util.Lists;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -12,7 +12,7 @@ import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.mockito.junit.jupiter.MockitoSettings;
 import org.mockito.quality.Strictness;
-import sonia.scm.group.GroupNames;
+import sonia.scm.group.GroupCollector;
 import sonia.scm.user.User;
 import sonia.scm.user.UserManager;
 import sonia.scm.user.UserTestData;
@@ -20,7 +20,6 @@ import sonia.scm.user.UserTestData;
 import java.net.URI;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -33,6 +32,9 @@ class MeDtoFactoryTest {
   @Mock
   private UserManager userManager;
 
+  @Mock
+  private GroupCollector groupCollector;
+
   @Mock
   private Subject subject;
 
@@ -42,7 +44,7 @@ class MeDtoFactoryTest {
   void setUpContext() {
     ThreadContext.bind(subject);
     ResourceLinks resourceLinks = ResourceLinksMock.createMock(baseUri);
-    meDtoFactory = new MeDtoFactory(resourceLinks, userManager);
+    meDtoFactory = new MeDtoFactory(resourceLinks, userManager, groupCollector);
   }
 
   @AfterEach
@@ -69,24 +71,16 @@ class MeDtoFactoryTest {
 
   @Test
   void shouldCreateMeDtoWithGroups() {
-    prepareSubject(UserTestData.createTrillian(), "HeartOfGold", "Puzzle42");
+    when(groupCollector.collect("trillian")).thenReturn(ImmutableSet.of("HeartOfGold", "Puzzle42"));
+    prepareSubject(UserTestData.createTrillian());
     MeDto dto = meDtoFactory.create();
     assertThat(dto.getGroups()).containsOnly("HeartOfGold", "Puzzle42");
   }
 
-  private void prepareSubject(User user, String... groups) {
+  private void prepareSubject(User user) {
     PrincipalCollection collection = mock(PrincipalCollection.class);
     when(subject.getPrincipals()).thenReturn(collection);
-    when(collection.oneByType(any(Class.class))).then(ic -> {
-      Class<?> type = ic.getArgument(0);
-      if (type.isAssignableFrom(User.class)) {
-        return user;
-      } else if (type.isAssignableFrom(GroupNames.class)) {
-        return new GroupNames(Lists.newArrayList(groups));
-      } else {
-        return null;
-      }
-    });
+    when(collection.oneByType(User.class)).thenReturn(user);
   }
 
   @Test

scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java

@@ -2,6 +2,7 @@ package sonia.scm.api.v2.resources;
 
 import com.github.sdorra.shiro.ShiroRule;
 import com.github.sdorra.shiro.SubjectAware;
+import com.google.common.collect.ImmutableSet;
 import org.apache.shiro.SecurityUtils;
 import org.apache.shiro.authc.credential.PasswordService;
 import org.apache.shiro.subject.PrincipalCollection;
@@ -16,6 +17,7 @@ import org.mockito.ArgumentCaptor;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import sonia.scm.ContextEntry;
+import sonia.scm.group.GroupCollector;
 import sonia.scm.user.InvalidPasswordException;
 import sonia.scm.user.User;
 import sonia.scm.user.UserManager;
@@ -28,7 +30,12 @@ import java.net.URISyntaxException;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
 import static org.mockito.MockitoAnnotations.initMocks;
 import static sonia.scm.api.v2.resources.DispatcherMock.createDispatcher;
 
@@ -50,6 +57,9 @@ public class MeResourceTest {
   @Mock
   private ScmPathInfoStore scmPathInfoStore;
 
+  @Mock
+  private GroupCollector groupCollector;
+
   @Mock
   private UserManager userManager;
 
@@ -69,6 +79,7 @@ public class MeResourceTest {
     when(userManager.create(userCaptor.capture())).thenAnswer(invocation -> invocation.getArguments()[0]);
     doNothing().when(userManager).modify(userCaptor.capture());
     doNothing().when(userManager).delete(userCaptor.capture());
+    when(groupCollector.collect("trillian")).thenReturn(ImmutableSet.of("group1", "group2"));
     when(userManager.isTypeDefault(userCaptor.capture())).thenCallRealMethod();
     when(userManager.getDefaultType()).thenReturn("xml");
     MeResource meResource = new MeResource(meDtoFactory, userManager, passwordService);

scm-webapp/src/test/java/sonia/scm/api/v2/resources/MergeResourceTest.java

@@ -1,10 +1,14 @@
 package sonia.scm.api.v2.resources;
 
+import com.github.sdorra.shiro.SubjectAware;
 import com.google.common.io.Resources;
 import com.google.inject.util.Providers;
+import org.apache.shiro.subject.Subject;
+import org.apache.shiro.util.ThreadContext;
 import org.jboss.resteasy.core.Dispatcher;
 import org.jboss.resteasy.mock.MockHttpRequest;
 import org.jboss.resteasy.mock.MockHttpResponse;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
@@ -13,6 +17,7 @@ import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import sonia.scm.repository.NamespaceAndName;
+import sonia.scm.repository.Repository;
 import sonia.scm.repository.api.MergeCommandBuilder;
 import sonia.scm.repository.api.MergeCommandResult;
 import sonia.scm.repository.api.MergeDryRunCommandResult;
@@ -26,12 +31,20 @@ import java.net.URL;
 import static java.util.Arrays.asList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.when;
+import static sonia.scm.repository.RepositoryTestData.createHeartOfGold;
 
 @ExtendWith(MockitoExtension.class)
+@SubjectAware(
+  configuration = "classpath:sonia/scm/shiro-001.ini",
+  username = "trillian",
+  password = "secret"
+)
 public class MergeResourceTest extends RepositoryTestBase {
 
   public static final String MERGE_URL = "/" + RepositoryRootResource.REPOSITORIES_PATH_V2 + "space/repo/merge/";
+  private Repository repository = createHeartOfGold();
 
   private Dispatcher dispatcher;
   @Mock
@@ -72,10 +85,21 @@ public class MergeResourceTest extends RepositoryTestBase {
   @Nested
   class ExecutingMergeCommand {
 
+    @Mock
+    private Subject subject;
+
     @BeforeEach
     void initRepository() {
       when(serviceFactory.create(new NamespaceAndName("space", "repo"))).thenReturn(repositoryService);
-      when(repositoryService.getMergeCommand()).thenReturn(mergeCommandBuilder);
+      lenient().when(repositoryService.getMergeCommand()).thenReturn(mergeCommandBuilder);
+      when(repositoryService.getRepository()).thenReturn(repository);
+
+      ThreadContext.bind(subject);
+    }
+
+    @AfterEach
+    void tearDownShiro() {
+      ThreadContext.unbindSubject();
     }
 
     @Test
@@ -115,6 +139,7 @@ public class MergeResourceTest extends RepositoryTestBase {
 
     @Test
     void shouldHandleSuccessfulDryRun() throws Exception {
+      when(subject.isPermitted("repository:push:" + repositoryService.getRepository().getId())).thenReturn(true);
       when(mergeCommand.dryRun(any())).thenReturn(new MergeDryRunCommandResult(true));
 
       URL url = Resources.getResource("sonia/scm/api/v2/mergeCommand.json");
@@ -132,6 +157,7 @@ public class MergeResourceTest extends RepositoryTestBase {
 
     @Test
     void shouldHandleFailedDryRun() throws Exception {
+      when(subject.isPermitted("repository:push:" + repositoryService.getRepository().getId())).thenReturn(true);
       when(mergeCommand.dryRun(any())).thenReturn(new MergeDryRunCommandResult(false));
 
       URL url = Resources.getResource("sonia/scm/api/v2/mergeCommand.json");
@@ -146,5 +172,22 @@ public class MergeResourceTest extends RepositoryTestBase {
 
       assertThat(response.getStatus()).isEqualTo(409);
     }
+
+    @Test
+    void shouldSkipDryRunIfSubjectHasNoPushPermission() throws Exception {
+      when(subject.isPermitted("repository:push:" + repositoryService.getRepository().getId())).thenReturn(false);
+
+      URL url = Resources.getResource("sonia/scm/api/v2/mergeCommand.json");
+      byte[] mergeCommandJson = Resources.toByteArray(url);
+
+      MockHttpRequest request = MockHttpRequest
+        .post(MERGE_URL + "dry-run/")
+        .content(mergeCommandJson)
+        .contentType(VndMediaType.MERGE_COMMAND);
+      MockHttpResponse response = new MockHttpResponse();
+      dispatcher.invoke(request, response);
+
+      assertThat(response.getStatus()).isEqualTo(204);
+    }
   }
 }

scm-webapp/src/test/java/sonia/scm/api/v2/resources/ResourceLinksMock.java

@@ -50,6 +50,7 @@ public class ResourceLinksMock {
     when(resourceLinks.repositoryRole()).thenReturn(new ResourceLinks.RepositoryRoleLinks(uriInfo));
     when(resourceLinks.repositoryRoleCollection()).thenReturn(new ResourceLinks.RepositoryRoleCollectionLinks(uriInfo));
     when(resourceLinks.namespaceStrategies()).thenReturn(new ResourceLinks.NamespaceStrategiesLinks(uriInfo));
+    when(resourceLinks.pluginCollection()).thenReturn(new ResourceLinks.PluginCollectionLinks(uriInfo));
 
     return resourceLinks;
   }

scm-webapp/src/test/java/sonia/scm/group/DefaultGroupCollectorTest.java

@@ -0,0 +1,100 @@
+package sonia.scm.group;
+
+import com.google.common.collect.ImmutableSet;
+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.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import sonia.scm.cache.MapCache;
+import sonia.scm.cache.MapCacheManager;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+class DefaultGroupCollectorTest {
+
+  @Mock
+  private GroupDAO groupDAO;
+
+  @Mock
+  private GroupResolver groupResolver;
+
+  private MapCacheManager mapCacheManager;
+
+  private Set<GroupResolver> groupResolvers;
+
+  private DefaultGroupCollector collector;
+
+  @BeforeEach
+  void initCollector() {
+    groupResolvers = new HashSet<>();
+    mapCacheManager = new MapCacheManager();
+    collector = new DefaultGroupCollector(groupDAO, mapCacheManager, groupResolvers);
+  }
+
+  @Test
+  void shouldAlwaysReturnAuthenticatedGroup() {
+    Iterable<String> groupNames = collector.collect("trillian");
+    assertThat(groupNames).containsOnly("_authenticated");
+  }
+
+  @Test
+  void shouldReturnGroupsFromCache() {
+    MapCache<String, Set<String>> cache = mapCacheManager.getCache(DefaultGroupCollector.CACHE_NAME);
+    cache.put("trillian", ImmutableSet.of("awesome", "incredible"));
+
+    Set<String> groups = collector.collect("trillian");
+    assertThat(groups).containsOnly("_authenticated", "awesome", "incredible");
+  }
+
+  @Test
+  void shouldNotCallResolverIfExternalGroupsAreCached() {
+    groupResolvers.add(groupResolver);
+
+    MapCache<String, Set<String>> cache = mapCacheManager.getCache(DefaultGroupCollector.CACHE_NAME);
+    cache.put("trillian", ImmutableSet.of("awesome", "incredible"));
+
+    Set<String> groups = collector.collect("trillian");
+    assertThat(groups).containsOnly("_authenticated", "awesome", "incredible");
+
+    verify(groupResolver, never()).resolve("trillian");
+  }
+
+  @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() {
+      Iterable<String> groupNames = collector.collect("trillian");
+      assertThat(groupNames).containsOnly("_authenticated", "heartOfGold", "fjordsOfAfrican");
+    }
+
+    @Test
+    void shouldCombineWithResolvers() {
+      when(groupResolver.resolve("trillian")).thenReturn(ImmutableSet.of("awesome", "incredible"));
+      groupResolvers.add(groupResolver);
+      Iterable<String> groupNames = collector.collect("trillian");
+      assertThat(groupNames).containsOnly("_authenticated", "heartOfGold", "fjordsOfAfrican", "awesome", "incredible");
+    }
+  }
+}

scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java

@@ -90,12 +90,10 @@ class BearerRealmTest {
     Set<String> groups = ImmutableSet.of("HeartOfGold", "Puzzle42");
 
     when(accessToken.getSubject()).thenReturn("trillian");
-    when(accessToken.getGroups()).thenReturn(groups);
     when(accessToken.getClaims()).thenReturn(new HashMap<>());
     when(accessTokenResolver.resolve(bearerToken)).thenReturn(accessToken);
 
     when(realmHelper.authenticationInfoBuilder("trillian")).thenReturn(builder);
-    when(builder.withGroups(groups)).thenReturn(builder);
     when(builder.withCredentials("__bearer__")).thenReturn(builder);
     when(builder.withScope(any(Scope.class))).thenReturn(builder);
     when(builder.build()).thenReturn(authenticationInfo);

scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java

@@ -33,6 +33,7 @@ package sonia.scm.security;
 
 import com.github.sdorra.shiro.ShiroRule;
 import com.github.sdorra.shiro.SubjectAware;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import org.apache.shiro.authz.AuthorizationInfo;
 import org.apache.shiro.authz.SimpleAuthorizationInfo;
@@ -49,7 +50,7 @@ import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
 import sonia.scm.cache.Cache;
 import sonia.scm.cache.CacheManager;
-import sonia.scm.group.GroupNames;
+import sonia.scm.group.GroupCollector;
 import sonia.scm.repository.Repository;
 import sonia.scm.repository.RepositoryDAO;
 import sonia.scm.repository.RepositoryPermission;
@@ -58,8 +59,6 @@ import sonia.scm.repository.RepositoryTestData;
 import sonia.scm.user.User;
 import sonia.scm.user.UserTestData;
 
-import java.util.Collections;
-
 import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
 import static org.hamcrest.Matchers.containsInAnyOrder;
@@ -96,6 +95,9 @@ public class DefaultAuthorizationCollectorTest {
   @Mock
   private RepositoryPermissionProvider repositoryPermissionProvider;
 
+  @Mock
+  private GroupCollector groupCollector;
+
   private DefaultAuthorizationCollector collector;
 
   @Rule
@@ -107,7 +109,7 @@ public class DefaultAuthorizationCollectorTest {
   @Before
   public void setUp(){
     when(cacheManager.getCache(Mockito.any(String.class))).thenReturn(cache);
-    collector = new DefaultAuthorizationCollector(cacheManager, repositoryDAO, securitySystem, repositoryPermissionProvider);
+    collector = new DefaultAuthorizationCollector(cacheManager, repositoryDAO, securitySystem, repositoryPermissionProvider, groupCollector);
   }
 
   /**
@@ -290,9 +292,13 @@ public class DefaultAuthorizationCollectorTest {
     SimplePrincipalCollection spc = new SimplePrincipalCollection();
     spc.add(user.getName(), "unit");
     spc.add(user, "unit");
-    spc.add(new GroupNames(group, groups), "unit");
     Subject subject = new Subject.Builder().authenticated(true).principals(spc).buildSubject();
     shiro.setSubject(subject);
+
+    ImmutableSet.Builder<String> builder = ImmutableSet.builder();
+    builder.add(group);
+    builder.add(groups);
+    when(groupCollector.collect(user.getName())).thenReturn(builder.build());
   }
 
   /**

scm-webapp/src/test/java/sonia/scm/security/DefaultRealmTest.java

@@ -36,8 +36,6 @@ package sonia.scm.security;
 //~--- non-JDK imports --------------------------------------------------------
 
 import com.google.common.collect.Collections2;
-import com.google.common.collect.Lists;
-
 import org.apache.shiro.authc.AuthenticationInfo;
 import org.apache.shiro.authc.AuthenticationToken;
 import org.apache.shiro.authc.DisabledAccountException;
@@ -45,43 +43,44 @@ import org.apache.shiro.authc.IncorrectCredentialsException;
 import org.apache.shiro.authc.UnknownAccountException;
 import org.apache.shiro.authc.UsernamePasswordToken;
 import org.apache.shiro.authc.credential.DefaultPasswordService;
-import org.apache.shiro.crypto.hash.DefaultHashService;
-import org.apache.shiro.subject.PrincipalCollection;
-import org.apache.shiro.subject.SimplePrincipalCollection;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-
-import org.mockito.Mock;
-import org.mockito.junit.MockitoJUnitRunner;
-
-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;
-import sonia.scm.user.UserTestData;
-
-import static org.hamcrest.Matchers.*;
-
-import static org.junit.Assert.*;
-
-import static org.mockito.Mockito.*;
-
-//~--- JDK imports ------------------------------------------------------------
-
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
 import org.apache.shiro.authz.AuthorizationInfo;
 import org.apache.shiro.authz.Permission;
 import org.apache.shiro.authz.SimpleAuthorizationInfo;
 import org.apache.shiro.authz.permission.WildcardPermissionResolver;
+import org.apache.shiro.crypto.hash.DefaultHashService;
+import org.apache.shiro.subject.PrincipalCollection;
+import org.apache.shiro.subject.SimplePrincipalCollection;
 import org.hamcrest.Matchers;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+import sonia.scm.group.GroupDAO;
+import sonia.scm.user.User;
+import sonia.scm.user.UserDAO;
+import sonia.scm.user.UserTestData;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import static org.hamcrest.Matchers.allOf;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.hasItem;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.nullValue;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+//~--- JDK imports ------------------------------------------------------------
 
 /**
  *
@@ -206,32 +205,6 @@ public class DefaultRealmTest
     );
   }
 
-  /**
-   * Method description
-   *
-   */
-  @Test
-  public void testGroupCollection()
-  {
-    User user = UserTestData.createTrillian();
-    //J-
-    List<Group> groups = Lists.newArrayList(
-      new Group(DefaultRealm.REALM, "scm", user.getName()),
-      new Group(DefaultRealm.REALM, "developers", "perfect")
-    );
-    //J+
-
-    when(groupDAO.getAll()).thenReturn(groups);
-
-    UsernamePasswordToken token = daoUser(user, "secret");
-    AuthenticationInfo info = realm.getAuthenticationInfo(token);
-    GroupNames groupNames = info.getPrincipals().oneByType(GroupNames.class);
-
-    assertNotNull(groupNames);
-    assertThat(groupNames.getCollection(), hasSize(2));
-    assertThat(groupNames, hasItems("scm", GroupNames.AUTHENTICATED));
-  }
-
   /**
    * Method description
    *
@@ -251,12 +224,6 @@ public class DefaultRealmTest
     assertThat(collection.getRealmNames(), hasSize(1));
     assertThat(collection.getRealmNames(), hasItem(DefaultRealm.REALM));
     assertEquals(user, collection.oneByType(User.class));
-
-    GroupNames groups = collection.oneByType(GroupNames.class);
-
-    assertNotNull(groups);
-    assertThat(groups.getCollection(), hasSize(1));
-    assertThat(groups.getCollection(), hasItem(GroupNames.AUTHENTICATED));
   }
 
   /**

scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java

@@ -36,27 +36,25 @@ import com.github.sdorra.shiro.SubjectAware;
 import com.google.common.collect.Sets;
 import io.jsonwebtoken.Claims;
 import io.jsonwebtoken.Jwts;
-import org.apache.shiro.SecurityUtils;
-import org.apache.shiro.subject.PrincipalCollection;
-import org.apache.shiro.subject.Subject;
 import org.apache.shiro.util.ThreadContext;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
-import org.mockito.invocation.InvocationOnMock;
 import org.mockito.junit.MockitoJUnitRunner;
-import sonia.scm.group.ExternalGroupNames;
 
-import java.util.Arrays;
-import java.util.List;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
-import static org.hamcrest.Matchers.*;
-import static org.junit.Assert.*;
-import static org.mockito.Mockito.*;
+import static org.hamcrest.Matchers.isEmptyOrNullString;
+import static org.hamcrest.Matchers.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.anyString;
+import static org.mockito.Mockito.when;
 import static sonia.scm.security.SecureKeyTestUtil.createSecureKey;
 
 /**
@@ -137,7 +135,6 @@ public class JwtAccessTokenBuilderTest {
       .issuer("https://www.scm-manager.org")
       .expiresIn(5, TimeUnit.SECONDS)
       .custom("a", "b")
-      .groups("one", "two", "three")
       .scope(Scope.valueOf("repo:*"))
       .build();
     
@@ -154,36 +151,6 @@ public class JwtAccessTokenBuilderTest {
     assertClaims(new JwtAccessToken(claims, compact));
   }
 
-  @Test
-  public void testWithExternalGroups() {
-    applyExternalGroupsToSubject(true, "external");
-    JwtAccessToken token = factory.create().subject("dent").build();
-    assertArrayEquals(new String[]{"external"}, token.getCustom(JwtAccessToken.GROUPS_CLAIM_KEY).map(x -> (String[]) x).get());
-  }
-
-  @Test
-  public void testWithInternalGroups() {
-    applyExternalGroupsToSubject(false, "external");
-    JwtAccessToken token = factory.create().subject("dent").build();
-    assertFalse(token.getCustom(JwtAccessToken.GROUPS_CLAIM_KEY).isPresent());
-  }
-
-  private void applyExternalGroupsToSubject(boolean external, String... groups) {
-    Subject subject = spy(SecurityUtils.getSubject());
-    when(subject.getPrincipals()).thenAnswer(invocation -> enrichWithGroups(invocation, groups, external));
-    shiro.setSubject(subject);
-  }
-
-  private Object enrichWithGroups(InvocationOnMock invocation, String[] groups, boolean external) throws Throwable {
-    PrincipalCollection principals = (PrincipalCollection) spy(invocation.callRealMethod());
-
-    List<String> groupCollection = Arrays.asList(groups);
-    if (external) {
-      when(principals.oneByType(ExternalGroupNames.class)).thenReturn(new ExternalGroupNames(groupCollection));
-    }
-    return principals;
-  }
-
   private void assertClaims(JwtAccessToken token){
     assertThat(token.getId(), not(isEmptyOrNullString()));
     assertNotNull( token.getIssuedAt() );
@@ -194,6 +161,5 @@ public class JwtAccessTokenBuilderTest {
     assertEquals(token.getIssuer().get(), "https://www.scm-manager.org");
     assertEquals("b", token.getCustom("a").get());
     assertEquals("[\"repo:*\"]", token.getScope().toString());
-    assertThat(token.getGroups(), containsInAnyOrder("one", "two", "three"));
   }
 }

scm-webapp/src/test/java/sonia/scm/update/MigrationWizardServletTest.java

@@ -5,8 +5,8 @@ import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
+import sonia.scm.update.repository.DefaultMigrationStrategyDAO;
 import sonia.scm.update.repository.MigrationStrategy;
-import sonia.scm.update.repository.MigrationStrategyDao;
 import sonia.scm.update.repository.V1Repository;
 import sonia.scm.update.repository.XmlRepositoryV1UpdateStep;
 
@@ -26,7 +26,7 @@ class MigrationWizardServletTest {
   @Mock
   XmlRepositoryV1UpdateStep updateStep;
   @Mock
-  MigrationStrategyDao migrationStrategyDao;
+  DefaultMigrationStrategyDAO migrationStrategyDao;
 
   @Mock
   HttpServletRequest request;
@@ -233,6 +233,6 @@ class MigrationWizardServletTest {
 
     servlet.doPost(request, response);
 
-    verify(migrationStrategyDao).set("id", "name", MigrationStrategy.COPY, "namespace", "name");
+    verify(migrationStrategyDao).set("id", "git", "name", MigrationStrategy.COPY, "namespace", "name");
   }
 }

scm-webapp/src/test/java/sonia/scm/update/repository/DefaultMigrationStrategyDAOTest.java

@@ -12,7 +12,6 @@ import sonia.scm.SCMContextProvider;
 import sonia.scm.store.ConfigurationStoreFactory;
 import sonia.scm.store.JAXBConfigurationStoreFactory;
 
-import javax.xml.bind.JAXBException;
 import java.nio.file.Path;
 import java.util.Optional;
 
@@ -21,7 +20,7 @@ import static sonia.scm.update.repository.MigrationStrategy.INLINE;
 
 @ExtendWith(MockitoExtension.class)
 @ExtendWith(TempDirectory.class)
-class MigrationStrategyDaoTest {
+class DefaultMigrationStrategyDAOTest {
 
   @Mock
   SCMContextProvider contextProvider;
@@ -36,7 +35,7 @@ class MigrationStrategyDaoTest {
 
   @Test
   void shouldReturnEmptyOptionalWhenStoreIsEmpty() {
-    MigrationStrategyDao dao = new MigrationStrategyDao(storeFactory);
+    DefaultMigrationStrategyDAO dao = new DefaultMigrationStrategyDAO(storeFactory);
 
     Optional<RepositoryMigrationPlan.RepositoryMigrationEntry> entry = dao.get("any");
 
@@ -45,9 +44,9 @@ class MigrationStrategyDaoTest {
 
   @Test
   void shouldReturnNewValue() {
-    MigrationStrategyDao dao = new MigrationStrategyDao(storeFactory);
+    DefaultMigrationStrategyDAO dao = new DefaultMigrationStrategyDAO(storeFactory);
 
-    dao.set("id", "originalName", INLINE, "space", "name");
+    dao.set("id", "git", "originalName", INLINE, "space", "name");
 
     Optional<RepositoryMigrationPlan.RepositoryMigrationEntry> entry = dao.get("id");
 
@@ -66,14 +65,14 @@ class MigrationStrategyDaoTest {
   class WithExistingDatabase {
     @BeforeEach
     void initExistingDatabase() {
-      MigrationStrategyDao dao = new MigrationStrategyDao(storeFactory);
+      DefaultMigrationStrategyDAO dao = new DefaultMigrationStrategyDAO(storeFactory);
 
-      dao.set("id", "originalName", INLINE, "space", "name");
+      dao.set("id", "git", "originalName", INLINE, "space", "name");
     }
 
     @Test
     void shouldFindExistingValue() {
-      MigrationStrategyDao dao = new MigrationStrategyDao(storeFactory);
+      DefaultMigrationStrategyDAO dao = new DefaultMigrationStrategyDAO(storeFactory);
 
       Optional<RepositoryMigrationPlan.RepositoryMigrationEntry> entry = dao.get("id");
 

scm-webapp/src/test/java/sonia/scm/update/repository/XmlRepositoryV1UpdateStepTest.java

@@ -16,8 +16,6 @@ import sonia.scm.repository.Repository;
 import sonia.scm.repository.RepositoryPermission;
 import sonia.scm.repository.xml.XmlRepositoryDAO;
 import sonia.scm.store.ConfigurationEntryStore;
-import sonia.scm.store.ConfigurationEntryStoreFactory;
-import sonia.scm.store.InMemoryConfigurationEntryStore;
 import sonia.scm.store.InMemoryConfigurationEntryStoreFactory;
 import sonia.scm.update.UpdateStepTestUtil;
 
@@ -49,7 +47,7 @@ class XmlRepositoryV1UpdateStepTest {
   @Mock
   XmlRepositoryDAO repositoryDAO;
   @Mock
-  MigrationStrategyDao migrationStrategyDao;
+  DefaultMigrationStrategyDAO migrationStrategyDao;
 
   InMemoryConfigurationEntryStoreFactory configurationEntryStoreFactory = new InMemoryConfigurationEntryStoreFactory();
 
@@ -91,7 +89,7 @@ class XmlRepositoryV1UpdateStepTest {
     void createMigrationPlan() {
       Answer<Object> planAnswer = invocation -> {
         String id = invocation.getArgument(0).toString();
-        return of(new RepositoryMigrationPlan.RepositoryMigrationEntry(id, "originalName", MOVE, "namespace-" + id, "name-" + id));
+        return of(new RepositoryMigrationPlan.RepositoryMigrationEntry(id, "git", "originalName", MOVE, "namespace-" + id, "name-" + id));
       };
 
       lenient().when(migrationStrategyDao.get("3b91caa5-59c3-448f-920b-769aaa56b761")).thenAnswer(planAnswer);

scm-webapp/src/test/java/sonia/scm/update/security/XmlSecurityV1UpdateStepTest.java

@@ -11,10 +11,7 @@ import org.mockito.junit.jupiter.MockitoExtension;
 import sonia.scm.SCMContextProvider;
 import sonia.scm.security.AssignedPermission;
 import sonia.scm.store.ConfigurationEntryStore;
-import sonia.scm.store.ConfigurationEntryStoreFactory;
-import sonia.scm.store.InMemoryConfigurationEntryStore;
 import sonia.scm.store.InMemoryConfigurationEntryStoreFactory;
-import sonia.scm.update.security.XmlSecurityV1UpdateStep;
 
 import javax.xml.bind.JAXBException;
 import java.io.IOException;
@@ -81,6 +78,32 @@ class XmlSecurityV1UpdateStepTest {
           .collect(toList());
       assertThat(assignedPermission).contains("admins", "vogons");
     }
+
+  }
+
+  @Nested
+  class WithExistingSecurityXml {
+
+    @BeforeEach
+    void createSecurityV1XML(@TempDirectory.TempDir Path tempDir) throws IOException {
+      Path configDir = tempDir.resolve("config");
+      Files.createDirectories(configDir);
+      copyTestDatabaseFile(configDir, "securityV1.xml");
+    }
+
+    @Test
+    void shouldMapV1PermissionsFromSecurityV1XML() throws JAXBException {
+      updateStep.doUpdate();
+      List<String> assignedPermission =
+        assignedPermissionStore.getAll().values()
+          .stream()
+          .filter(a -> a.getPermission().getValue().contains("repository:"))
+          .map(AssignedPermission::getName)
+          .collect(toList());
+      assertThat(assignedPermission).contains("scmadmin");
+      assertThat(assignedPermission).contains("test");
+    }
+
   }
 
   private void copyTestDatabaseFile(Path configDir, String fileName) throws IOException {

scm-webapp/src/test/java/sonia/scm/web/filter/HttpProtocolServletAuthenticationFilterTest.java

@@ -1,72 +0,0 @@
-package sonia.scm.web.filter;
-
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.ExtendWith;
-import org.mockito.Mock;
-import org.mockito.junit.jupiter.MockitoExtension;
-import sonia.scm.config.ScmConfiguration;
-import sonia.scm.util.HttpUtil;
-import sonia.scm.web.UserAgent;
-import sonia.scm.web.UserAgentParser;
-import sonia.scm.web.WebTokenGenerator;
-
-import javax.servlet.FilterChain;
-import javax.servlet.ServletException;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import java.io.IOException;
-import java.util.Collections;
-import java.util.Set;
-
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
-@ExtendWith(MockitoExtension.class)
-class HttpProtocolServletAuthenticationFilterTest {
-
-  private ScmConfiguration configuration = new ScmConfiguration();
-
-  private Set<WebTokenGenerator> tokenGenerators = Collections.emptySet();
-
-  @Mock
-  private UserAgentParser userAgentParser;
-
-  private HttpProtocolServletAuthenticationFilter authenticationFilter;
-
-  @Mock
-  private HttpServletRequest request;
-
-  @Mock
-  private HttpServletResponse response;
-
-  @Mock
-  private FilterChain filterChain;
-
-  private UserAgent nonBrowser = UserAgent.builder("i'm not a browser").browser(false).build();
-  private UserAgent browser = UserAgent.builder("i am a browser").browser(true).build();
-
-  @BeforeEach
-  void setUpObjectUnderTest() {
-    authenticationFilter = new HttpProtocolServletAuthenticationFilter(configuration, tokenGenerators, userAgentParser);
-  }
-
-  @Test
-  void shouldSendUnauthorized() throws IOException, ServletException {
-    when(userAgentParser.parse(request)).thenReturn(nonBrowser);
-
-    authenticationFilter.handleUnauthorized(request, response, filterChain);
-
-    verify(response).sendError(HttpServletResponse.SC_UNAUTHORIZED, HttpUtil.STATUS_UNAUTHORIZED_MESSAGE);
-  }
-
-  @Test
-  void shouldCallFilterChain() throws IOException, ServletException {
-    when(userAgentParser.parse(request)).thenReturn(browser);
-
-    authenticationFilter.handleUnauthorized(request, response, filterChain);
-
-    verify(filterChain).doFilter(request, response);
-  }
-
-}

scm-webapp/src/test/java/sonia/scm/web/protocol/HttpProtocolServletTest.java

@@ -1,114 +1,136 @@
 package sonia.scm.web.protocol;
 
-import org.junit.Before;
-import org.junit.Test;
+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.NotFoundException;
 import sonia.scm.PushStateDispatcher;
 import sonia.scm.repository.DefaultRepositoryProvider;
 import sonia.scm.repository.NamespaceAndName;
 import sonia.scm.repository.Repository;
+import sonia.scm.repository.RepositoryTestData;
 import sonia.scm.repository.api.RepositoryService;
 import sonia.scm.repository.api.RepositoryServiceFactory;
 import sonia.scm.repository.spi.HttpScmProtocol;
 import sonia.scm.web.UserAgent;
 import sonia.scm.web.UserAgentParser;
 
-import javax.inject.Provider;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
+import java.util.Optional;
 
-import static org.mockito.AdditionalMatchers.not;
-import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
-import static org.mockito.MockitoAnnotations.initMocks;
-
-public class HttpProtocolServletTest {
 
+@ExtendWith(MockitoExtension.class)
+class HttpProtocolServletTest {
 
   @Mock
   private RepositoryServiceFactory serviceFactory;
+
   @Mock
-  private HttpServletRequest httpServletRequest;
+  private NamespaceAndNameFromPathExtractor extractor;
+
   @Mock
   private PushStateDispatcher dispatcher;
+
   @Mock
   private UserAgentParser userAgentParser;
-  @Mock
-  private Provider<HttpServletRequest> requestProvider;
 
   @InjectMocks
   private HttpProtocolServlet servlet;
 
   @Mock
   private RepositoryService repositoryService;
+
   @Mock
   private UserAgent userAgent;
 
   @Mock
   private HttpServletRequest request;
+
   @Mock
   private HttpServletResponse response;
+
   @Mock
   private HttpScmProtocol protocol;
 
-  @Before
-  public void init() {
-    initMocks(this);
-    when(userAgentParser.parse(request)).thenReturn(userAgent);
-    when(userAgent.isBrowser()).thenReturn(false);
-    NamespaceAndName existingRepo = new NamespaceAndName("space", "repo");
-    when(serviceFactory.create(not(eq(existingRepo)))).thenThrow(new NotFoundException("Test", "a"));
-    when(serviceFactory.create(existingRepo)).thenReturn(repositoryService);
-    when(requestProvider.get()).thenReturn(httpServletRequest);
+  @Nested
+  class Browser {
+
+    @BeforeEach
+    void prepareMocks() {
+      when(userAgentParser.parse(request)).thenReturn(userAgent);
+      when(userAgent.isBrowser()).thenReturn(true);
+      when(request.getRequestURI()).thenReturn("uri");
+    }
+
+    @Test
+    void shouldDispatchBrowserRequests() throws ServletException, IOException {
+      when(userAgent.isBrowser()).thenReturn(true);
+      when(request.getRequestURI()).thenReturn("uri");
+
+      servlet.service(request, response);
+
+      verify(dispatcher).dispatch(request, response, "uri");
+    }
+
   }
 
-  @Test
-  public void shouldDispatchBrowserRequests() throws ServletException, IOException {
-    when(userAgent.isBrowser()).thenReturn(true);
-    when(request.getRequestURI()).thenReturn("uri");
+  @Nested
+  class ScmClient {
 
-    servlet.service(request, response);
+    @BeforeEach
+    void prepareMocks() {
+      when(userAgentParser.parse(request)).thenReturn(userAgent);
+      when(userAgent.isBrowser()).thenReturn(false);
+    }
 
-    verify(dispatcher).dispatch(request, response, "uri");
-  }
+    @Test
+    void shouldHandleBadPaths() throws IOException, ServletException {
+      when(request.getPathInfo()).thenReturn("/illegal");
 
-  @Test
-  public void shouldHandleBadPaths() throws IOException, ServletException {
-    when(request.getPathInfo()).thenReturn("/illegal");
+      servlet.service(request, response);
 
-    servlet.service(request, response);
+      verify(response).setStatus(400);
+    }
 
-    verify(response).setStatus(400);
-  }
+    @Test
+    void shouldHandleNotExistingRepository() throws IOException, ServletException {
+      when(request.getPathInfo()).thenReturn("/not/exists");
 
-  @Test
-  public void shouldHandleNotExistingRepository() throws IOException, ServletException {
-    when(request.getPathInfo()).thenReturn("/not/exists");
+      NamespaceAndName repo = new NamespaceAndName("not", "exists");
+      when(extractor.fromUri("/not/exists")).thenReturn(Optional.of(repo));
+      when(serviceFactory.create(repo)).thenThrow(new NotFoundException("Test", "a"));
 
-    servlet.service(request, response);
+      servlet.service(request, response);
 
-    verify(response).setStatus(404);
-  }
+      verify(response).setStatus(404);
+    }
 
-  @Test
-  public void shouldDelegateToProvider() throws IOException, ServletException {
-    when(request.getPathInfo()).thenReturn("/space/name");
-    NamespaceAndName namespaceAndName = new NamespaceAndName("space", "name");
-    doReturn(repositoryService).when(serviceFactory).create(namespaceAndName);
-    Repository repository = new Repository();
-    when(repositoryService.getRepository()).thenReturn(repository);
-    when(repositoryService.getProtocol(HttpScmProtocol.class)).thenReturn(protocol);
+    @Test
+    void shouldDelegateToProvider() throws IOException, ServletException {
+      NamespaceAndName repo = new NamespaceAndName("space", "name");
+      when(extractor.fromUri("/space/name")).thenReturn(Optional.of(repo));
+      when(serviceFactory.create(repo)).thenReturn(repositoryService);
 
-    servlet.service(request, response);
+      when(request.getPathInfo()).thenReturn("/space/name");
+      Repository repository = RepositoryTestData.createHeartOfGold();
+      when(repositoryService.getRepository()).thenReturn(repository);
+      when(repositoryService.getProtocol(HttpScmProtocol.class)).thenReturn(protocol);
+
+      servlet.service(request, response);
+
+      verify(request).setAttribute(DefaultRepositoryProvider.ATTRIBUTE_NAME, repository);
+      verify(protocol).serve(request, response, null);
+      verify(repositoryService).close();
+    }
 
-    verify(httpServletRequest).setAttribute(DefaultRepositoryProvider.ATTRIBUTE_NAME, repository);
-    verify(protocol).serve(request, response, null);
-    verify(repositoryService).close();
   }
 }

scm-webapp/src/test/java/sonia/scm/web/protocol/NamespaceAndNameFromPathExtractorTest.java

@@ -1,17 +1,46 @@
 package sonia.scm.web.protocol;
 
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.DynamicNode;
 import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
 import sonia.scm.repository.NamespaceAndName;
+import sonia.scm.repository.RepositoryManager;
+import sonia.scm.repository.RepositoryType;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
 import java.util.Optional;
 import java.util.stream.Stream;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.DynamicTest.dynamicTest;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+class NamespaceAndNameFromPathExtractorTest {
+
+  @Mock
+  private RepositoryManager repositoryManager;
+
+  private NamespaceAndNameFromPathExtractor extractor;
+
+  @BeforeEach
+  void setUpObjectUnderTest() {
+    List<RepositoryType> types = Arrays.asList(
+      new RepositoryType("git", "Git", Collections.emptySet()),
+      new RepositoryType("hg", "Mercurial", Collections.emptySet()),
+      new RepositoryType("svn", "Subversion", Collections.emptySet())
+    );
+    when(repositoryManager.getConfiguredTypes()).thenReturn(types);
+    extractor = new NamespaceAndNameFromPathExtractor(repositoryManager);
+  }
 
-public class NamespaceAndNameFromPathExtractorTest {
   @TestFactory
   Stream<DynamicNode> shouldExtractCorrectNamespaceAndName() {
     return Stream.of(
@@ -26,21 +55,26 @@ public class NamespaceAndNameFromPathExtractorTest {
   }
 
   @TestFactory
-  Stream<DynamicNode> shouldHandleTrailingDotSomethings() {
+  Stream<DynamicNode> shouldHandleTypeSuffix() {
     return Stream.of(
       "/space/repo.git",
-      "/space/repo.and.more",
-      "/space/repo."
+      "/space/repo.hg",
+      "/space/repo.svn",
+      "/space/repo"
     ).map(this::createCorrectTest);
   }
 
   private DynamicTest createCorrectTest(String path) {
+    return createCorrectTest(path, new NamespaceAndName("space", "repo"));
+  }
+
+  private DynamicTest createCorrectTest(String path, NamespaceAndName expected) {
     return dynamicTest(
       "should extract correct namespace and name for path " + path,
       () -> {
-        Optional<NamespaceAndName> namespaceAndName = NamespaceAndNameFromPathExtractor.fromUri(path);
+        Optional<NamespaceAndName> namespaceAndName = extractor.fromUri(path);
 
-        assertThat(namespaceAndName.get()).isEqualTo(new NamespaceAndName("space", "repo"));
+        assertThat(namespaceAndName.get()).isEqualTo(expected);
       }
     );
   }
@@ -59,10 +93,26 @@ public class NamespaceAndNameFromPathExtractorTest {
     return dynamicTest(
       "should not fail for wrong path " + path,
       () -> {
-        Optional<NamespaceAndName> namespaceAndName = NamespaceAndNameFromPathExtractor.fromUri(path);
+        Optional<NamespaceAndName> namespaceAndName = extractor.fromUri(path);
 
         assertThat(namespaceAndName.isPresent()).isFalse();
       }
     );
   }
+
+  @TestFactory
+  Stream<DynamicNode> shouldHandleDots() {
+    return Stream.of(
+      "/space/repo.with.dots.git",
+      "/space/repo.with.dots.hg",
+      "/space/repo.with.dots.svn",
+      "/space/repo.with.dots"
+    ).map(path -> createCorrectTest(path, new NamespaceAndName("space", "repo.with.dots")));
+  }
+
+  @Test
+  void shouldNotFailOnEndingDot() {
+    Optional<NamespaceAndName> namespaceAndName = extractor.fromUri("/space/repo.");
+    assertThat(namespaceAndName).contains(new NamespaceAndName("space", "repo."));
+  }
 }