Merge with 2.0.0-m3

scm-core/src/main/java/sonia/scm/api/v2/resources/HalAppender.java

@@ -2,6 +2,8 @@ package sonia.scm.api.v2.resources;
 
 import de.otto.edison.hal.HalRepresentation;
 
+import java.util.List;
+
 /**
  * The {@link HalAppender} can be used within an {@link HalEnricher} to append hateoas links to a json response.
  *
@@ -34,6 +36,14 @@ public interface HalAppender {
    */
   void appendEmbedded(String rel, HalRepresentation embeddedItem);
 
+  /**
+   * Appends a list of embedded objects to the json response.
+   *
+   * @param rel name of relation
+   * @param embeddedItems embedded objects
+   */
+  void appendEmbedded(String rel, List<HalRepresentation> embeddedItems);
+
   /**
    * Builder for link arrays.
    */

scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java

@@ -379,7 +379,6 @@ public final class RepositoryService implements Closeable {
    * @since 2.0.0
    */
   public MergeCommandBuilder getMergeCommand() {
-    RepositoryPermissions.push(getRepository()).check();
     LOG.debug("create merge command for repository {}",
       repository.getNamespaceAndName());
 

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

@@ -162,7 +162,7 @@ public class AssignedPermission implements PermissionObject, Serializable
     //J-
     return MoreObjects.toStringHelper(this)
                   .add("name", name)
-                  .add("groupPermisison", groupPermission)
+                  .add("groupPermission", groupPermission)
                   .add("permission", permission)
                   .toString();
     //J+

scm-webapp/src/main/java/sonia/scm/api/v2/resources/EdisonHalAppender.java

@@ -33,6 +33,11 @@ class EdisonHalAppender implements HalAppender {
     embeddedBuilder.with(rel, embedded);
   }
 
+  @Override
+  public void appendEmbedded(String rel, List<HalRepresentation> embedded) {
+    embeddedBuilder.with(rel, embedded);
+  }
+
   private static class EdisonLinkArrayBuilder implements LinkArrayBuilder {
 
     private final Links.Builder builder;

scm-webapp/src/main/java/sonia/scm/api/v2/resources/MergeResource.java

@@ -6,6 +6,7 @@ import lombok.extern.slf4j.Slf4j;
 import org.apache.http.HttpStatus;
 import sonia.scm.ConcurrentModificationException;
 import sonia.scm.repository.NamespaceAndName;
+import sonia.scm.repository.RepositoryPermissions;
 import sonia.scm.repository.api.MergeCommandBuilder;
 import sonia.scm.repository.api.MergeCommandResult;
 import sonia.scm.repository.api.MergeDryRunCommandResult;
@@ -49,6 +50,7 @@ public class MergeResource {
     NamespaceAndName namespaceAndName = new NamespaceAndName(namespace, name);
     log.info("Merge in Repository {}/{} from {} to {}", namespace, name, mergeCommand.getSourceRevision(), mergeCommand.getTargetRevision());
     try (RepositoryService repositoryService = serviceFactory.create(namespaceAndName)) {
+      RepositoryPermissions.push(repositoryService.getRepository()).check();
       MergeCommandResult mergeCommandResult = createMergeCommand(mergeCommand, repositoryService).executeMerge();
       if (mergeCommandResult.isSuccess()) {
         return Response.noContent().build();
@@ -67,14 +69,19 @@ public class MergeResource {
     @ResponseCode(code = 500, condition = "internal server error")
   })
   public Response dryRun(@PathParam("namespace") String namespace, @PathParam("name") String name, @Valid MergeCommandDto mergeCommand) {
+
     NamespaceAndName namespaceAndName = new NamespaceAndName(namespace, name);
     log.info("Merge in Repository {}/{} from {} to {}", namespace, name, mergeCommand.getSourceRevision(), mergeCommand.getTargetRevision());
     try (RepositoryService repositoryService = serviceFactory.create(namespaceAndName)) {
-      MergeDryRunCommandResult mergeCommandResult = createMergeCommand(mergeCommand, repositoryService).dryRun();
-      if (mergeCommandResult.isMergeable()) {
-        return Response.noContent().build();
+      if (RepositoryPermissions.push(repositoryService.getRepository()).isPermitted()) {
+        MergeDryRunCommandResult mergeCommandResult = createMergeCommand(mergeCommand, repositoryService).dryRun();
+        if (mergeCommandResult.isMergeable()) {
+          return Response.noContent().build();
+        } else {
+          throw new ConcurrentModificationException("revision", mergeCommand.getTargetRevision());
+        }
       } else {
-        throw new ConcurrentModificationException("revision", mergeCommand.getTargetRevision());
+        return Response.noContent().build();
       }
     }
   }

scm-webapp/src/main/java/sonia/scm/update/security/XmlSecurityV1UpdateStep.java

@@ -21,7 +21,10 @@ import javax.xml.bind.annotation.XmlRootElement;
 import java.io.File;
 import java.nio.file.Path;
 import java.util.Arrays;
+import java.util.List;
 import java.util.function.Consumer;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import static java.util.Optional.ofNullable;
 import static sonia.scm.version.Version.parse;
@@ -29,6 +32,8 @@ import static sonia.scm.version.Version.parse;
 @Extension
 public class XmlSecurityV1UpdateStep implements UpdateStep {
 
+  private static final Pattern v1PermissionPattern = Pattern.compile("^repository:\\*:(READ|WRITE|OWNER)$");
+
   private static final Logger LOG = LoggerFactory.getLogger(XmlSecurityV1UpdateStep.class);
 
   private final SCMContextProvider contextProvider;
@@ -46,6 +51,50 @@ public class XmlSecurityV1UpdateStep implements UpdateStep {
 
     forAllAdmins(user -> createSecurityEntry(user, false, securityStore),
         group -> createSecurityEntry(group, true, securityStore));
+
+    mapV1Permissions(securityStore);
+  }
+
+  private void mapV1Permissions(ConfigurationEntryStore<AssignedPermission> securityStore) throws JAXBException {
+    Path v1SecurityFile = determineConfigDirectory().resolve("securityV1" + StoreConstants.FILE_EXTENSION);
+
+    if (!v1SecurityFile.toFile().exists()) {
+      LOG.info("no v1 file for security found");
+      return;
+    }
+
+    JAXBContext jaxbContext = JAXBContext.newInstance(XmlSecurityV1UpdateStep.V1Security.class);
+    V1Security v1Security = (V1Security) jaxbContext.createUnmarshaller().unmarshal(v1SecurityFile.toFile());
+
+    v1Security.entries.forEach(assignedPermission -> {
+      Matcher matcher = v1PermissionPattern.matcher(assignedPermission.value.permission);
+      if (matcher.matches()) {
+        String newPermission = convertRole(matcher.group(1));
+        securityStore.put(new AssignedPermission(
+          assignedPermission.value.name,
+          Boolean.parseBoolean(assignedPermission.value.groupPermission),
+          newPermission
+        ));
+      }
+    });
+  }
+
+  private String convertRole(String role) {
+    String newPermission;
+    switch (role) {
+      case "OWNER":
+        newPermission = "repository:*";
+        break;
+      case "WRITE":
+        newPermission = "repository:read,pull,push:*";
+        break;
+      case "READ":
+        newPermission = "repository:read,pull:*";
+        break;
+      default:
+        newPermission = "";
+    }
+    return newPermission;
   }
 
   private void forAllAdmins(Consumer<String> userConsumer, Consumer<String> groupConsumer) throws JAXBException {
@@ -70,10 +119,9 @@ public class XmlSecurityV1UpdateStep implements UpdateStep {
     Arrays.stream(entries.split(",")).forEach(consumer);
   }
 
-
   @Override
   public Version getTargetVersion() {
-    return parse("2.0.0");
+    return parse("2.0.1");
   }
 
   @Override
@@ -102,4 +150,29 @@ public class XmlSecurityV1UpdateStep implements UpdateStep {
     @XmlElement(name = "admin-groups")
     private String adminGroups;
   }
+
+  @XmlAccessorType(XmlAccessType.FIELD)
+  @XmlRootElement(name = "configuration")
+  private static class V1Security {
+    @XmlElement(name = "entry")
+    private List<Entry> entries;
+  }
+
+  @XmlAccessorType(XmlAccessType.FIELD)
+  private static class Entry {
+    @XmlElement(name = "key")
+    private String key;
+    @XmlElement(name = "value")
+    private Value value;
+  }
+
+  @XmlAccessorType(XmlAccessType.FIELD)
+  private static class Value {
+    @XmlElement(name = "permission")
+    String permission;
+    @XmlElement(name = "name")
+    String name;
+    @XmlElement(name = "group-permission")
+    String groupPermission;
+  }
 }

scm-webapp/src/main/java/sonia/scm/update/user/XmlUserV1UpdateStep.java

@@ -57,7 +57,8 @@ public class XmlUserV1UpdateStep implements UpdateStep {
 
   @Override
   public void doUpdate() throws JAXBException {
-    Optional<Path> v1UsersFile = determineV1File();
+    Optional<Path> v1UsersFile = determineV1File("users");
+    determineV1File("security");
     if (!v1UsersFile.isPresent()) {
       LOG.info("no v1 file for users found");
       return;
@@ -107,17 +108,17 @@ public class XmlUserV1UpdateStep implements UpdateStep {
     return configurationEntryStoreFactory.withType(AssignedPermission.class).withName("security").build();
   }
 
-  private Optional<Path> determineV1File() {
-    Path existingUsersFile = resolveConfigFile("users");
-    Path usersV1File = resolveConfigFile("usersV1");
-    if (existingUsersFile.toFile().exists()) {
+  private Optional<Path> determineV1File(String filename) {
+    Path existingFile = resolveConfigFile(filename);
+    Path v1File = resolveConfigFile(filename + "V1");
+    if (existingFile.toFile().exists()) {
       try {
-        Files.move(existingUsersFile, usersV1File);
+        Files.move(existingFile, v1File);
       } catch (IOException e) {
-        throw new UpdateException("could not move old users file to " + usersV1File.toAbsolutePath());
+        throw new UpdateException("could not move old " + filename + " file to " + v1File.toAbsolutePath());
       }
-      LOG.info("moved old users file to {}", usersV1File.toAbsolutePath());
-      return of(usersV1File);
+      LOG.info("moved old " + filename + " file to {}", v1File.toAbsolutePath());
+      return of(v1File);
     }
     return empty();
   }

scm-webapp/src/main/java/sonia/scm/web/protocol/HttpProtocolServlet.java

@@ -16,7 +16,6 @@ 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.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
@@ -33,17 +32,15 @@ public class HttpProtocolServlet extends HttpServlet {
   public static final String PATTERN = PATH + "/*";
 
   private final RepositoryServiceFactory serviceFactory;
-
-  private final Provider<HttpServletRequest> requestProvider;
-
+  private final NamespaceAndNameFromPathExtractor pathExtractor;
   private final PushStateDispatcher dispatcher;
   private final UserAgentParser userAgentParser;
 
 
   @Inject
-  public HttpProtocolServlet(RepositoryServiceFactory serviceFactory, Provider<HttpServletRequest> requestProvider, PushStateDispatcher dispatcher, UserAgentParser userAgentParser) {
+  public HttpProtocolServlet(RepositoryServiceFactory serviceFactory, NamespaceAndNameFromPathExtractor pathExtractor, PushStateDispatcher dispatcher, UserAgentParser userAgentParser) {
     this.serviceFactory = serviceFactory;
-    this.requestProvider = requestProvider;
+    this.pathExtractor = pathExtractor;
     this.dispatcher = dispatcher;
     this.userAgentParser = userAgentParser;
   }
@@ -55,9 +52,8 @@ public class HttpProtocolServlet extends HttpServlet {
       log.trace("dispatch browser request for user agent {}", userAgent);
       dispatcher.dispatch(request, response, request.getRequestURI());
     } else {
-
       String pathInfo = request.getPathInfo();
-      Optional<NamespaceAndName> namespaceAndName = NamespaceAndNameFromPathExtractor.fromUri(pathInfo);
+      Optional<NamespaceAndName> namespaceAndName = pathExtractor.fromUri(pathInfo);
       if (namespaceAndName.isPresent()) {
         service(request, response, namespaceAndName.get());
       } else {
@@ -69,7 +65,7 @@ public class HttpProtocolServlet extends HttpServlet {
 
   private void service(HttpServletRequest req, HttpServletResponse resp, NamespaceAndName namespaceAndName) throws IOException, ServletException {
     try (RepositoryService repositoryService = serviceFactory.create(namespaceAndName)) {
-      requestProvider.get().setAttribute(DefaultRepositoryProvider.ATTRIBUTE_NAME, repositoryService.getRepository());
+      req.setAttribute(DefaultRepositoryProvider.ATTRIBUTE_NAME, repositoryService.getRepository());
       HttpScmProtocol protocol = repositoryService.getProtocol(HttpScmProtocol.class);
       protocol.serve(req, resp, getServletConfig());
     } catch (NotFoundException e) {

scm-webapp/src/main/java/sonia/scm/web/protocol/NamespaceAndNameFromPathExtractor.java

@@ -1,18 +1,31 @@
 package sonia.scm.web.protocol;
 
+import sonia.scm.Type;
 import sonia.scm.repository.NamespaceAndName;
+import sonia.scm.repository.RepositoryManager;
 import sonia.scm.util.HttpUtil;
 
+import javax.inject.Inject;
 import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 import static java.util.Optional.empty;
 import static java.util.Optional.of;
 
 final class NamespaceAndNameFromPathExtractor {
 
-  private NamespaceAndNameFromPathExtractor() {}
+  private final Set<String> types;
 
-  static Optional<NamespaceAndName> fromUri(String uri) {
+  @Inject
+  public NamespaceAndNameFromPathExtractor(RepositoryManager repositoryManager) {
+    this.types = repositoryManager.getConfiguredTypes()
+      .stream()
+      .map(Type::getName)
+      .collect(Collectors.toSet());
+  }
+
+  Optional<NamespaceAndName> fromUri(String uri) {
     if (uri.startsWith(HttpUtil.SEPARATOR_PATH)) {
       uri = uri.substring(1);
     }
@@ -30,12 +43,13 @@ final class NamespaceAndNameFromPathExtractor {
     }
 
     String name = uri.substring(endOfNamespace + 1, nameIndex);
-
-    int nameDotIndex = name.indexOf('.');
+    int nameDotIndex = name.lastIndexOf('.');
     if (nameDotIndex >= 0) {
-      return of(new NamespaceAndName(namespace, name.substring(0, nameDotIndex)));
-    } else {
-      return of(new NamespaceAndName(namespace, name));
+      String suffix = name.substring(nameDotIndex + 1);
+      if (types.contains(suffix)) {
+        name = name.substring(0, nameDotIndex);
+      }
     }
+    return of(new NamespaceAndName(namespace, name));
   }
 }

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/update/security/XmlSecurityV1UpdateStepTest.java

@@ -81,6 +81,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/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,48 @@
 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.InjectMocks;
+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 javax.inject.Inject;
+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 +57,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 +95,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."));
+  }
 }

scm-webapp/src/test/resources/sonia/scm/update/security/securityV1.xml

@@ -0,0 +1,35 @@
+<?xml version="1.0" ?>
+<configuration>
+  <entry>
+    <key>4lRWOA7DH1</key>
+    <value>
+      <group-permission>false</group-permission>
+      <name>scmadmin</name>
+      <permission>repository:*:READ</permission>
+    </value>
+  </entry>
+  <entry>
+    <key>CfRWOAANM2</key>
+    <value>
+      <group-permission>true</group-permission>
+      <name>test</name>
+      <permission>repository:*:OWNER</permission>
+    </value>
+  </entry>
+  <entry>
+    <key>CfRWOAANM2</key>
+    <value>
+      <group-permission>true</group-permission>
+      <name>test</name>
+      <permission>invalid:permission</permission>
+    </value>
+  </entry>
+  <entry>
+    <key>CfRWOAANM2</key>
+    <value>
+      <group-permission>true</group-permission>
+      <name>test</name>
+      <permission>repository:*:STRANGE</permission>
+    </value>
+  </entry>
+</configuration>