diff --git a/scm-core/src/main/java/sonia/scm/api/v2/resources/HalAppender.java b/scm-core/src/main/java/sonia/scm/api/v2/resources/HalAppender.java index 6afb542646..b313f68af8 100644 --- a/scm-core/src/main/java/sonia/scm/api/v2/resources/HalAppender.java +++ b/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 embeddedItems); + /** * Builder for link arrays. */ diff --git a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java index e11afa4be9..90978d75ea 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java +++ b/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()); diff --git a/scm-core/src/main/java/sonia/scm/security/AssignedPermission.java b/scm-core/src/main/java/sonia/scm/security/AssignedPermission.java index c98d81f8ba..cc7ff87534 100644 --- a/scm-core/src/main/java/sonia/scm/security/AssignedPermission.java +++ b/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+ diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/EdisonHalAppender.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/EdisonHalAppender.java index 769de2b705..bb89c90556 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/EdisonHalAppender.java +++ b/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 embedded) { + embeddedBuilder.with(rel, embedded); + } + private static class EdisonLinkArrayBuilder implements LinkArrayBuilder { private final Links.Builder builder; diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MergeResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MergeResource.java index 2214b258c6..df59ba2abc 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MergeResource.java +++ b/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(); } } } diff --git a/scm-webapp/src/main/java/sonia/scm/update/security/XmlSecurityV1UpdateStep.java b/scm-webapp/src/main/java/sonia/scm/update/security/XmlSecurityV1UpdateStep.java index f62b81b5df..8dac2b5073 100644 --- a/scm-webapp/src/main/java/sonia/scm/update/security/XmlSecurityV1UpdateStep.java +++ b/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 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 userConsumer, Consumer 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 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; + } } diff --git a/scm-webapp/src/main/java/sonia/scm/update/user/XmlUserV1UpdateStep.java b/scm-webapp/src/main/java/sonia/scm/update/user/XmlUserV1UpdateStep.java index b2da69fd9b..f2561ac53e 100644 --- a/scm-webapp/src/main/java/sonia/scm/update/user/XmlUserV1UpdateStep.java +++ b/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 v1UsersFile = determineV1File(); + Optional 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 determineV1File() { - Path existingUsersFile = resolveConfigFile("users"); - Path usersV1File = resolveConfigFile("usersV1"); - if (existingUsersFile.toFile().exists()) { + private Optional 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(); } diff --git a/scm-webapp/src/main/java/sonia/scm/web/protocol/HttpProtocolServlet.java b/scm-webapp/src/main/java/sonia/scm/web/protocol/HttpProtocolServlet.java index 250846a8cc..623be728c1 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/protocol/HttpProtocolServlet.java +++ b/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 requestProvider; - + private final NamespaceAndNameFromPathExtractor pathExtractor; private final PushStateDispatcher dispatcher; private final UserAgentParser userAgentParser; @Inject - public HttpProtocolServlet(RepositoryServiceFactory serviceFactory, Provider 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 = NamespaceAndNameFromPathExtractor.fromUri(pathInfo); + Optional 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) { diff --git a/scm-webapp/src/main/java/sonia/scm/web/protocol/NamespaceAndNameFromPathExtractor.java b/scm-webapp/src/main/java/sonia/scm/web/protocol/NamespaceAndNameFromPathExtractor.java index 22e2433561..2c1eeb1b90 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/protocol/NamespaceAndNameFromPathExtractor.java +++ b/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 types; - static Optional fromUri(String uri) { + @Inject + public NamespaceAndNameFromPathExtractor(RepositoryManager repositoryManager) { + this.types = repositoryManager.getConfiguredTypes() + .stream() + .map(Type::getName) + .collect(Collectors.toSet()); + } + + Optional 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)); } } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MergeResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MergeResourceTest.java index d47ff35e5f..b9e720fc71 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MergeResourceTest.java +++ b/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); + } } } diff --git a/scm-webapp/src/test/java/sonia/scm/update/security/XmlSecurityV1UpdateStepTest.java b/scm-webapp/src/test/java/sonia/scm/update/security/XmlSecurityV1UpdateStepTest.java index d0115e3426..73c7fe6aca 100644 --- a/scm-webapp/src/test/java/sonia/scm/update/security/XmlSecurityV1UpdateStepTest.java +++ b/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 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 { diff --git a/scm-webapp/src/test/java/sonia/scm/web/protocol/HttpProtocolServletTest.java b/scm-webapp/src/test/java/sonia/scm/web/protocol/HttpProtocolServletTest.java index 1bd6358c95..59b29c8eda 100644 --- a/scm-webapp/src/test/java/sonia/scm/web/protocol/HttpProtocolServletTest.java +++ b/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 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(); } } diff --git a/scm-webapp/src/test/java/sonia/scm/web/protocol/NamespaceAndNameFromPathExtractorTest.java b/scm-webapp/src/test/java/sonia/scm/web/protocol/NamespaceAndNameFromPathExtractorTest.java index 0998010069..5ac8f37c01 100644 --- a/scm-webapp/src/test/java/sonia/scm/web/protocol/NamespaceAndNameFromPathExtractorTest.java +++ b/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 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 shouldExtractCorrectNamespaceAndName() { return Stream.of( @@ -26,21 +57,26 @@ public class NamespaceAndNameFromPathExtractorTest { } @TestFactory - Stream shouldHandleTrailingDotSomethings() { + Stream 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 = NamespaceAndNameFromPathExtractor.fromUri(path); + Optional 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 = NamespaceAndNameFromPathExtractor.fromUri(path); + Optional namespaceAndName = extractor.fromUri(path); assertThat(namespaceAndName.isPresent()).isFalse(); } ); } + + @TestFactory + Stream 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 = extractor.fromUri("/space/repo."); + assertThat(namespaceAndName).contains(new NamespaceAndName("space", "repo.")); + } } diff --git a/scm-webapp/src/test/resources/sonia/scm/update/security/securityV1.xml b/scm-webapp/src/test/resources/sonia/scm/update/security/securityV1.xml new file mode 100644 index 0000000000..8de82f88d9 --- /dev/null +++ b/scm-webapp/src/test/resources/sonia/scm/update/security/securityV1.xml @@ -0,0 +1,35 @@ + + + + 4lRWOA7DH1 + + false + scmadmin + repository:*:READ + + + + CfRWOAANM2 + + true + test + repository:*:OWNER + + + + CfRWOAANM2 + + true + test + invalid:permission + + + + CfRWOAANM2 + + true + test + repository:*:STRANGE + + +