From 298430a90fa706d19eae329348547a0ac0b46857 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 21 Aug 2018 13:38:37 +0200 Subject: [PATCH 01/15] Satisfy sonar --- .../api/v2/resources/PermissionDtoToPermissionMapper.java | 2 +- .../scm/api/v2/resources/PermissionRootResource.java | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionDtoToPermissionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionDtoToPermissionMapper.java index 9128479836..29cb7e496a 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionDtoToPermissionMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionDtoToPermissionMapper.java @@ -16,6 +16,6 @@ public abstract class PermissionDtoToPermissionMapper { * @param permissionDto the source dto * @return the mapped target permission object */ - public abstract Permission map(@MappingTarget Permission target, PermissionDto permissionDto); + public abstract Permission modify(@MappingTarget Permission target, PermissionDto permissionDto); } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java index 716e240bee..fdce46b727 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java @@ -7,6 +7,7 @@ import com.webcohesion.enunciate.metadata.rs.TypeHint; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang.StringUtils; import sonia.scm.repository.NamespaceAndName; +import sonia.scm.repository.Permission; import sonia.scm.repository.PermissionAlreadyExistsException; import sonia.scm.repository.PermissionNotFoundException; import sonia.scm.repository.Repository; @@ -155,13 +156,12 @@ public class PermissionRootResource { PermissionDto permission) throws RepositoryException { log.info("try to update the permission with name: {}. the modified permission is: {}", permissionName, permission); Repository repository = checkPermission(namespace, name); - repository.getPermissions() + Permission existingPermission = repository.getPermissions() .stream() .filter(perm -> StringUtils.isNotBlank(perm.getName()) && perm.getName().equals(permissionName)) .findFirst() - .map(p -> dtoToModelMapper.map(p, permission)) - .orElseThrow(() -> new PermissionNotFoundException(repository, permissionName)) - ; + .orElseThrow(() -> new PermissionNotFoundException(repository, permissionName)); + dtoToModelMapper.modify(existingPermission, permission); manager.modify(repository); log.info("the permission with name: {} is updated.", permissionName); return Response.noContent().build(); From cd54086e7da755f29ff6ebd4405b782b1fceba7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 21 Aug 2018 14:18:03 +0200 Subject: [PATCH 02/15] Remove redundant permission check --- .../PermissionDtoToPermissionMapper.java | 2 +- .../v2/resources/PermissionRootResource.java | 28 ++++--------------- .../resources/PermissionRootResourceTest.java | 15 +++------- 3 files changed, 11 insertions(+), 34 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionDtoToPermissionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionDtoToPermissionMapper.java index 29cb7e496a..1e90c23aa7 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionDtoToPermissionMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionDtoToPermissionMapper.java @@ -16,6 +16,6 @@ public abstract class PermissionDtoToPermissionMapper { * @param permissionDto the source dto * @return the mapped target permission object */ - public abstract Permission modify(@MappingTarget Permission target, PermissionDto permissionDto); + public abstract void modify(@MappingTarget Permission target, PermissionDto permissionDto); } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java index fdce46b727..d08f2e7057 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java @@ -14,7 +14,6 @@ import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryException; import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.RepositoryNotFoundException; -import sonia.scm.repository.RepositoryPermissions; import sonia.scm.web.VndMediaType; import javax.inject.Inject; @@ -69,7 +68,7 @@ public class PermissionRootResource { @Consumes(VndMediaType.PERMISSION) public Response create(@PathParam("namespace") String namespace, @PathParam("name") String name, PermissionDto permission) throws RepositoryException { log.info("try to add new permission: {}", permission); - Repository repository = checkPermission(namespace, name); + Repository repository = load(namespace, name); checkPermissionAlreadyExists(permission, repository); repository.getPermissions().add(dtoToModelMapper.map(permission)); manager.modify(repository); @@ -95,7 +94,7 @@ public class PermissionRootResource { @TypeHint(PermissionDto.class) @Path("{permission-name}") public Response get(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("permission-name") String permissionName) throws RepositoryException { - Repository repository = checkPermission(namespace, name); + Repository repository = load(namespace, name); return Response.ok( repository.getPermissions() .stream() @@ -125,7 +124,7 @@ public class PermissionRootResource { @TypeHint(PermissionDto.class) @Path("") public Response getAll(@PathParam("namespace") String namespace, @PathParam("name") String name) throws RepositoryNotFoundException { - Repository repository = checkPermission(namespace, name); + Repository repository = load(namespace, name); List permissionDtoList = repository.getPermissions() .stream() .map(per -> modelToDtoMapper.map(per, new NamespaceAndName(repository.getNamespace(), repository.getName()))) @@ -155,7 +154,7 @@ public class PermissionRootResource { @PathParam("permission-name") String permissionName, PermissionDto permission) throws RepositoryException { log.info("try to update the permission with name: {}. the modified permission is: {}", permissionName, permission); - Repository repository = checkPermission(namespace, name); + Repository repository = load(namespace, name); Permission existingPermission = repository.getPermissions() .stream() .filter(perm -> StringUtils.isNotBlank(perm.getName()) && perm.getName().equals(permissionName)) @@ -186,7 +185,7 @@ public class PermissionRootResource { @PathParam("name") String name, @PathParam("permission-name") String permissionName) throws RepositoryException { log.info("try to delete the permission with name: {}.", permissionName); - Repository repository = checkPermission(namespace, name); + Repository repository = load(namespace, name); repository.getPermissions() .stream() .filter(perm -> StringUtils.isNotBlank(perm.getName()) && perm.getName().equals(permissionName)) @@ -208,26 +207,11 @@ public class PermissionRootResource { * @return the repository if the user is permitted * @throws RepositoryNotFoundException if the repository does not exists */ - private Repository checkPermission(@PathParam("namespace") String namespace, @PathParam("name") String name) throws RepositoryNotFoundException { + private Repository load(String namespace, String name) throws RepositoryNotFoundException { return Optional.ofNullable(manager.get(new NamespaceAndName(namespace, name))) - .filter(repository -> { - checkUserPermitted(repository); - return true; - }) .orElseThrow(() -> new RepositoryNotFoundException(name)); } - - /** - * throw exception if the user is not permitted - * - * @param repository - */ - protected void checkUserPermitted(Repository repository) { - RepositoryPermissions.modify(repository).check(); - } - - /** * check if the permission already exists in the repository * diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java index 547e2dbcb1..39da0ce42e 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java @@ -18,10 +18,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.TestFactory; -import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; import sonia.scm.api.rest.AuthorizationExceptionMapper; import sonia.scm.repository.NamespaceAndName; import sonia.scm.repository.Permission; @@ -44,14 +42,11 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.fail; import static org.junit.jupiter.api.DynamicTest.dynamicTest; import static org.mockito.Matchers.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; -@RunWith(MockitoJUnitRunner.Silent.class) @Slf4j public class PermissionRootResourceTest { private static final String REPOSITORY_NAMESPACE = "repo_namespace"; @@ -112,7 +107,7 @@ public class PermissionRootResourceTest { @Before public void prepareEnvironment() { initMocks(this); - permissionRootResource = spy(new PermissionRootResource(permissionDtoToPermissionMapper, permissionToPermissionDtoMapper, resourceLinks, repositoryManager)); + permissionRootResource = new PermissionRootResource(permissionDtoToPermissionMapper, permissionToPermissionDtoMapper, resourceLinks, repositoryManager); RepositoryRootResource repositoryRootResource = new RepositoryRootResource(MockProvider .of(new RepositoryResource(null, null, null, null, null, null, null,null, MockProvider.of(permissionRootResource))), null); dispatcher.getRegistry().addSingletonResource(repositoryRootResource); @@ -150,8 +145,7 @@ public class PermissionRootResourceTest { Stream missedPermissionUserForbiddenTestFactory() { Repository mockRepository = mock(Repository.class); when(mockRepository.getId()).thenReturn(REPOSITORY_NAME); - doThrow(AuthorizationException.class).when(permissionRootResource).checkUserPermitted(mockRepository); - when(repositoryManager.get(any(NamespaceAndName.class))).thenReturn(mockRepository); + doThrow(AuthorizationException.class).when(repositoryManager).get(any(NamespaceAndName.class)); return createDynamicTestsToAssertResponses( requestGETPermission.expectedResponseStatus(403), requestPOSTPermission.expectedResponseStatus(403), @@ -321,7 +315,6 @@ public class PermissionRootResourceTest { when(mockRepository.getId()).thenReturn(REPOSITORY_NAME); when(mockRepository.getNamespace()).thenReturn(REPOSITORY_NAMESPACE); when(mockRepository.getName()).thenReturn(REPOSITORY_NAME); - doNothing().when(permissionRootResource).checkUserPermitted(mockRepository); when(repositoryManager.get(any(NamespaceAndName.class))).thenReturn(mockRepository); return mockRepository; } @@ -344,9 +337,9 @@ public class PermissionRootResourceTest { .contentType(VndMediaType.PERMISSION); dispatcher.invoke(request, response); log.info("Test the Request :{}", entry); - assertThat(entry.expectedResponseStatus) + assertThat(response.getStatus()) .as("assert status code") - .isEqualTo(response.getStatus()); + .isEqualTo(entry.expectedResponseStatus); if (entry.responseValidator != null) { entry.responseValidator.accept(response); } From 8c128127deef3b505eb621a8912773ec18eaae2f Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Wed, 22 Aug 2018 09:18:17 +0200 Subject: [PATCH 03/15] #8771 implement integration tests --- .../java/sonia/scm/it/PermissionsITCase.java | 139 ++++++++++++++++++ .../java/sonia/scm/it/RepositoriesITCase.java | 60 +------- .../sonia/scm/it/RepositoryAccessITCase.java | 11 +- .../java/sonia/scm/it/RepositoryUtil.java | 72 +++++++-- .../src/test/java/sonia/scm/it/RestUtil.java | 18 ++- .../src/test/java/sonia/scm/it/TestData.java | 56 ++++++- .../DefaultAuthorizationCollector.java | 4 +- 7 files changed, 275 insertions(+), 85 deletions(-) create mode 100644 scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java diff --git a/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java b/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java new file mode 100644 index 0000000000..c372efaa2f --- /dev/null +++ b/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java @@ -0,0 +1,139 @@ +/* + * Copyright (c) 2010, Sebastian Sdorra + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * 3. Neither the name of SCM-Manager; nor the names of its + * contributors may be used to endorse or promote products derived from this + * software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * http://bitbucket.org/sdorra/scm-manager + */ + + +package sonia.scm.it; + +import org.apache.http.HttpStatus; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; +import sonia.scm.repository.PermissionType; +import sonia.scm.repository.client.api.RepositoryClient; + +import java.io.IOException; +import java.util.Collection; +import java.util.List; +import java.util.Objects; + +import static org.junit.Assert.*; +import static sonia.scm.it.ScmTypes.availableScmTypes; + +@RunWith(Parameterized.class) +public class PermissionsITCase { + + public static final String USER_READ = "user_read"; + public static final String USER_PASS = "pass"; + private static final String USER_WRITE = "user_write"; + private static final String USER_OWNER = "user_owner"; + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + private final String repositoryType; + private int createdPermissions; + + + public PermissionsITCase(String repositoryType) { + this.repositoryType = repositoryType; + } + + @Parameters(name = "{0}") + public static Collection createParameters() { + return availableScmTypes(); + } + + @Before + public void prepareEnvironment() { + TestData.createDefault(); + TestData.createUser(USER_READ, USER_PASS); + TestData.createUserPermission(USER_READ, PermissionType.READ, repositoryType); + TestData.createUser(USER_WRITE, USER_PASS); + TestData.createUserPermission(USER_WRITE, PermissionType.WRITE, repositoryType); + TestData.createUser(USER_OWNER, USER_PASS); + TestData.createUserPermission(USER_OWNER, PermissionType.OWNER, repositoryType); + createdPermissions = 3; + } + + @Test + public void everyUserShouldSeePermissions() { + List userPermissions = TestData.getUserPermissions(USER_READ, USER_PASS, repositoryType); + assertEquals(userPermissions.size(), createdPermissions); + userPermissions = TestData.getUserPermissions(USER_WRITE, USER_PASS, repositoryType); + assertEquals(userPermissions.size(), createdPermissions); + userPermissions = TestData.getUserPermissions(USER_OWNER, USER_PASS, repositoryType); + assertEquals(userPermissions.size(), createdPermissions); + } + + @Test + public void everyUserShouldCloneRepository() throws IOException { + RepositoryClient client = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.newFolder(), USER_READ, USER_PASS); + assertEquals(1, Objects.requireNonNull(client.getWorkingCopy().list()).length); + client = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.newFolder(), USER_WRITE, USER_PASS); + assertEquals(1, Objects.requireNonNull(client.getWorkingCopy().list()).length); + client = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.newFolder(), USER_OWNER, USER_PASS); + assertEquals(1, Objects.requireNonNull(client.getWorkingCopy().list()).length); + } + + @Test + public void userWithREADPermissionShouldBeNotAuthorizedToCommit() throws IOException { + assertFalse(RepositoryUtil.canUserCommit(USER_READ, USER_PASS, repositoryType, temporaryFolder)); + } + + @Test + public void userWithOwnerPermissionShouldBeAuthorizedToCommit() throws IOException { + assertTrue(RepositoryUtil.canUserCommit(USER_OWNER, USER_PASS, repositoryType, temporaryFolder)); + } + + @Test + public void userWithWritePermissionShouldBeAuthorizedToCommit() throws IOException { + assertTrue(RepositoryUtil.canUserCommit(USER_WRITE, USER_PASS, repositoryType, temporaryFolder)); + } + + @Test + public void userWithWOwnerPermissionShouldBeAuthorizedToDeleteRepository(){ + RepositoryUtil.assertDeleteRepositoryOperation(USER_OWNER, HttpStatus.SC_NO_CONTENT, HttpStatus.SC_NOT_FOUND, USER_PASS, repositoryType); + } + + @Test + public void userWithWReadPermissionShouldNotBeAuthorizedToDeleteRepository(){ + RepositoryUtil.assertDeleteRepositoryOperation(USER_READ, HttpStatus.SC_FORBIDDEN, HttpStatus.SC_OK, USER_PASS, repositoryType); + } + + @Test + public void userWithWWritePermissionShouldNotBeAuthorizedToDeleteRepository(){ + RepositoryUtil.assertDeleteRepositoryOperation(USER_WRITE, HttpStatus.SC_FORBIDDEN, HttpStatus.SC_OK, USER_PASS, repositoryType); + } + +} diff --git a/scm-it/src/test/java/sonia/scm/it/RepositoriesITCase.java b/scm-it/src/test/java/sonia/scm/it/RepositoriesITCase.java index cd791cb013..e609b0075e 100644 --- a/scm-it/src/test/java/sonia/scm/it/RepositoriesITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/RepositoriesITCase.java @@ -41,22 +41,18 @@ import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; -import sonia.scm.repository.Person; -import sonia.scm.repository.client.api.ClientCommand; import sonia.scm.repository.client.api.RepositoryClient; -import sonia.scm.repository.client.api.RepositoryClientFactory; import sonia.scm.web.VndMediaType; -import java.io.File; -import java.io.FileOutputStream; import java.io.IOException; import java.util.Collection; -import java.util.UUID; +import java.util.Objects; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static sonia.scm.it.RegExMatcher.matchesPattern; import static sonia.scm.it.RestUtil.createResourceUrl; import static sonia.scm.it.RestUtil.given; @@ -66,8 +62,6 @@ import static sonia.scm.it.TestData.repositoryJson; @RunWith(Parameterized.class) public class RepositoriesITCase { - public static final Person AUTHOR = new Person("SCM Administrator", "scmadmin@scm-manager.org"); - @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); @@ -142,57 +136,13 @@ public class RepositoriesITCase { @Test public void shouldCloneRepository() throws IOException { - RepositoryClient client = createRepositoryClient(); - assertEquals("expected metadata dir", 1, client.getWorkingCopy().list().length); + RepositoryClient client = RepositoryUtil.createRepositoryClient(repositoryType,temporaryFolder.getRoot()); + assertEquals("expected metadata dir", 1, Objects.requireNonNull(client.getWorkingCopy().list()).length); } @Test public void shouldCommitFiles() throws IOException { - RepositoryClient client = createRepositoryClient(); - - for (int i = 0; i < 5; i++) { - createRandomFile(client); - } - - commit(client); - - RepositoryClient checkClient = createRepositoryClient(); - assertEquals("expected 5 files and metadata dir", 6, checkClient.getWorkingCopy().list().length); + assertTrue(RepositoryUtil.canScmAdminCommit(repositoryType,temporaryFolder)); } - private static void createRandomFile(RepositoryClient client) throws IOException { - String uuid = UUID.randomUUID().toString(); - String name = "file-" + uuid + ".uuid"; - - File file = new File(client.getWorkingCopy(), name); - try (FileOutputStream out = new FileOutputStream(file)) { - out.write(uuid.getBytes()); - } - - client.getAddCommand().add(name); - } - - private static void commit(RepositoryClient repositoryClient) throws IOException { - repositoryClient.getCommitCommand().commit(AUTHOR, "commit"); - if ( repositoryClient.isCommandSupported(ClientCommand.PUSH) ) { - repositoryClient.getPushCommand().push(); - } - } - - private RepositoryClient createRepositoryClient() throws IOException { - RepositoryClientFactory clientFactory = new RepositoryClientFactory(); - String cloneUrl = readCloneUrl(); - return clientFactory.create(repositoryType, cloneUrl, "scmadmin", "scmadmin", temporaryFolder.newFolder()); - } - - private String readCloneUrl() { - return given(VndMediaType.REPOSITORY) - - .when() - .get(repositoryUrl) - - .then() - .extract() - .path("_links.httpProtocol.href"); - } } diff --git a/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java b/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java index a461e40dea..ff8a3092a7 100644 --- a/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java @@ -7,7 +7,9 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; +import sonia.scm.repository.client.api.RepositoryClient; +import java.io.File; import java.io.IOException; import java.util.Collection; @@ -23,7 +25,7 @@ public class RepositoryAccessITCase { public TemporaryFolder tempFolder = new TemporaryFolder(); private final String repositoryType; - private RepositoryUtil repositoryUtil; + private File folder; public RepositoryAccessITCase(String repositoryType) { this.repositoryType = repositoryType; @@ -35,16 +37,17 @@ public class RepositoryAccessITCase { } @Before - public void initClient() throws IOException { + public void initClient() { TestData.createDefault(); - repositoryUtil = new RepositoryUtil(repositoryType, tempFolder.getRoot()); + folder = tempFolder.getRoot(); } @Test public void shouldFindBranches() throws IOException { assumeFalse("There are no branches for SVN", repositoryType.equals("svn")); - repositoryUtil.createAndCommitFile("a.txt", "a"); + RepositoryClient repositoryClient = RepositoryUtil.createRepositoryClient(repositoryType, folder ); + RepositoryUtil.createAndCommitFile(folder, repositoryClient, "scmadmin", "a.txt", "a"); String branchesUrl = given() .when() diff --git a/scm-it/src/test/java/sonia/scm/it/RepositoryUtil.java b/scm-it/src/test/java/sonia/scm/it/RepositoryUtil.java index 98d4c8cdab..0c59bdc5ad 100644 --- a/scm-it/src/test/java/sonia/scm/it/RepositoryUtil.java +++ b/scm-it/src/test/java/sonia/scm/it/RepositoryUtil.java @@ -3,34 +3,61 @@ package sonia.scm.it; import com.google.common.base.Charsets; import com.google.common.io.Files; import org.apache.http.HttpStatus; +import org.junit.rules.TemporaryFolder; import sonia.scm.repository.Changeset; import sonia.scm.repository.Person; import sonia.scm.repository.client.api.ClientCommand; import sonia.scm.repository.client.api.RepositoryClient; +import sonia.scm.repository.client.api.RepositoryClientException; import sonia.scm.repository.client.api.RepositoryClientFactory; import sonia.scm.web.VndMediaType; import java.io.File; +import java.io.FileOutputStream; import java.io.IOException; +import java.util.UUID; -import static sonia.scm.it.RestUtil.ADMIN_PASSWORD; -import static sonia.scm.it.RestUtil.ADMIN_USERNAME; import static sonia.scm.it.RestUtil.given; public class RepositoryUtil { private static final RepositoryClientFactory REPOSITORY_CLIENT_FACTORY = new RepositoryClientFactory(); - private final RepositoryClient repositoryClient; - private final File folder; + static void addRandomFileToRepository(RepositoryClient client) throws IOException { + String uuid = UUID.randomUUID().toString(); + String name = "file-" + uuid + ".uuid"; - RepositoryUtil(String repositoryType, File folder) throws IOException { - this.repositoryClient = createRepositoryClient(repositoryType, folder); - this.folder = folder; + File file = new File(client.getWorkingCopy(), name); + try (FileOutputStream out = new FileOutputStream(file)) { + out.write(uuid.getBytes()); + } + client.getAddCommand().add(name); + } + + static boolean canScmAdminCommit(String repositoryType, TemporaryFolder temporaryFolder) throws IOException { + return canUserCommit("scmadmin", "scmadmin", repositoryType, temporaryFolder); + } + + static boolean canUserCommit(String username, String password, String repositoryType, TemporaryFolder temporaryFolder) throws IOException { + RepositoryClient client = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.newFolder(), username, password); + for (int i = 0; i < 5; i++) { + addRandomFileToRepository(client); + } + try{ + commit(client, username, "commit"); + }catch (RepositoryClientException e){ + return false; + } + RepositoryClient checkClient = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.newFolder(), username, password); + return checkClient.getWorkingCopy().list().length == 6; } static RepositoryClient createRepositoryClient(String repositoryType, File folder) throws IOException { - String httpProtocolUrl = given(VndMediaType.REPOSITORY) + return createRepositoryClient(repositoryType, folder, "scmadmin", "scmadmin"); + } + + static RepositoryClient createRepositoryClient(String repositoryType, File folder, String username, String password) throws IOException { + String httpProtocolUrl = given(VndMediaType.REPOSITORY, username, password) .when() .get(TestData.getDefaultRepositoryUrl(repositoryType)) @@ -40,20 +67,33 @@ public class RepositoryUtil { .extract() .path("_links.httpProtocol.href"); - - return REPOSITORY_CLIENT_FACTORY.create(repositoryType, httpProtocolUrl, ADMIN_USERNAME, ADMIN_PASSWORD, folder); + return REPOSITORY_CLIENT_FACTORY.create(repositoryType, httpProtocolUrl, username, password, folder); } + static void assertDeleteRepositoryOperation(String user, int deleteStatus, int getStatus, String password, String repositoryType) { + given(VndMediaType.REPOSITORY, user, password) - void createAndCommitFile(String fileName, String content) throws IOException { + .when() + .delete(TestData.getDefaultRepositoryUrl(repositoryType)) + + .then() + .statusCode(deleteStatus); + + given(VndMediaType.REPOSITORY, user, password) + + .when() + .get(TestData.getDefaultRepositoryUrl(repositoryType)) + + .then() + .statusCode(getStatus); + } + static void createAndCommitFile(File folder, RepositoryClient repositoryClient, String username, String fileName, String content) throws IOException { Files.write(content, new File(folder, fileName), Charsets.UTF_8); repositoryClient.getAddCommand().add(fileName); - commit("added " + fileName); + commit(repositoryClient, username, "added " + fileName); } - Changeset commit(String message) throws IOException { - Changeset changeset = repositoryClient.getCommitCommand().commit( - new Person("scmadmin", "scmadmin@scm-manager.org"), message - ); + static Changeset commit(RepositoryClient repositoryClient, String username, String message) throws IOException { + Changeset changeset = repositoryClient.getCommitCommand().commit(new Person(username, username + "@scm-manager.org"), message); if (repositoryClient.isCommandSupported(ClientCommand.PUSH)) { repositoryClient.getPushCommand().push(); } diff --git a/scm-it/src/test/java/sonia/scm/it/RestUtil.java b/scm-it/src/test/java/sonia/scm/it/RestUtil.java index 76d2461637..a7409e1995 100644 --- a/scm-it/src/test/java/sonia/scm/it/RestUtil.java +++ b/scm-it/src/test/java/sonia/scm/it/RestUtil.java @@ -19,15 +19,19 @@ public class RestUtil { public static final String ADMIN_USERNAME = "scmadmin"; public static final String ADMIN_PASSWORD = "scmadmin"; - public static RequestSpecification given(String mediaType) { - return RestAssured.given() - .contentType(mediaType) - .accept(mediaType) - .auth().preemptive().basic(ADMIN_USERNAME, ADMIN_PASSWORD); - } - public static RequestSpecification given() { return RestAssured.given() .auth().preemptive().basic(ADMIN_USERNAME, ADMIN_PASSWORD); } + + public static RequestSpecification given(String mediaType) { + return given(mediaType, ADMIN_USERNAME, ADMIN_PASSWORD); + } + + public static RequestSpecification given(String mediaType, String username, String password) { + return RestAssured.given() + .contentType(mediaType) + .accept(mediaType) + .auth().preemptive().basic(username, password); + } } diff --git a/scm-it/src/test/java/sonia/scm/it/TestData.java b/scm-it/src/test/java/sonia/scm/it/TestData.java index b2785b2051..e9177807eb 100644 --- a/scm-it/src/test/java/sonia/scm/it/TestData.java +++ b/scm-it/src/test/java/sonia/scm/it/TestData.java @@ -3,6 +3,7 @@ package sonia.scm.it; import org.apache.http.HttpStatus; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import sonia.scm.repository.PermissionType; import sonia.scm.web.VndMediaType; import javax.json.Json; @@ -19,7 +20,9 @@ public class TestData { private static final Logger LOG = LoggerFactory.getLogger(TestData.class); - private static final List PROTECTED_USERS = asList("scmadmin", "anonymous"); + public static final String USER_SCM_ADMIN = "scmadmin"; + public static final String USER_ANONYMOUS = "anonymous"; +private static final List PROTECTED_USERS = asList(USER_SCM_ADMIN, USER_ANONYMOUS); private static Map DEFAULT_REPOSITORIES = new HashMap<>(); @@ -38,6 +41,57 @@ public class TestData { return DEFAULT_REPOSITORIES.get(repositoryType); } + public static void createUser(String username, String password) { + given(VndMediaType.USER) + .when() + .content(" {\n" + + " \"active\": true,\n" + + " \"admin\": false,\n" + + " \"creationDate\": \"2018-08-21T12:26:46.084Z\",\n" + + " \"displayName\": \""+username+"\",\n" + + " \"mail\": \"user1@scm-manager.org\",\n" + + " \"name\": \"" + username + "\",\n" + + " \"password\": \"" + password + "\",\n" + + " \"type\": \"xml\"\n" + + " \n" + + " }") + .post(createResourceUrl("users")) + .then() + .statusCode(HttpStatus.SC_CREATED) + ; + } + + + public static void createUserPermission(String name, PermissionType permissionType, String repositoryType) { + given(VndMediaType.PERMISSION) + .when() + .content("{\n" + + "\t\"type\": \""+permissionType.name()+"\",\n" + + "\t\"name\": \""+name+"\",\n" + + "\t\"groupPermission\": false\n" + + "\t\n" + + "}") + .post(TestData.getDefaultPermissionUrl(repositoryType)) + .then() + .statusCode(HttpStatus.SC_CREATED) + ; + } + + public static List getUserPermissions(String username, String password, String repositoryType) { + return given(VndMediaType.PERMISSION, username, password) + .when() + .get(TestData.getDefaultPermissionUrl(repositoryType)) + .then() + .statusCode(HttpStatus.SC_OK) + .extract() + .body().jsonPath().getList(""); + } + + private static String getDefaultPermissionUrl(String repositoryType) { + return getDefaultRepositoryUrl(repositoryType)+"/permissions/"; + } + + private static void cleanupRepositories() { List repositories = given(VndMediaType.REPOSITORY_COLLECTION) .when() diff --git a/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java b/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java index d868b81499..e6a2502997 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java +++ b/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java @@ -207,9 +207,9 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector boolean hasPermission = false; for (sonia.scm.repository.Permission permission : repositoryPermissions) { - if (isUserPermitted(user, groups, permission)) + hasPermission = isUserPermitted(user, groups, permission); + if (hasPermission) { - String perm = permission.getType().getPermissionPrefix().concat(repository.getId()); if (logger.isTraceEnabled()) { From b68c8400a7b15178ac8088481b35347f86bfbd6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 22 Aug 2018 10:59:46 +0200 Subject: [PATCH 04/15] Move tests to test classes --- .../java/sonia/scm/it/PermissionsITCase.java | 50 ++++++++++++---- .../java/sonia/scm/it/RepositoriesITCase.java | 9 ++- .../sonia/scm/it/RepositoryAccessITCase.java | 4 +- .../java/sonia/scm/it/RepositoryUtil.java | 57 +++---------------- 4 files changed, 54 insertions(+), 66 deletions(-) diff --git a/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java b/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java index c372efaa2f..07e2a36534 100644 --- a/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java @@ -41,13 +41,17 @@ import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; import sonia.scm.repository.PermissionType; import sonia.scm.repository.client.api.RepositoryClient; +import sonia.scm.repository.client.api.RepositoryClientException; +import sonia.scm.web.VndMediaType; import java.io.IOException; import java.util.Collection; import java.util.List; import java.util.Objects; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static sonia.scm.it.RepositoryUtil.addAndCommitRandomFile; +import static sonia.scm.it.RestUtil.given; import static sonia.scm.it.ScmTypes.availableScmTypes; @RunWith(Parameterized.class) @@ -106,34 +110,56 @@ public class PermissionsITCase { assertEquals(1, Objects.requireNonNull(client.getWorkingCopy().list()).length); } - @Test - public void userWithREADPermissionShouldBeNotAuthorizedToCommit() throws IOException { - assertFalse(RepositoryUtil.canUserCommit(USER_READ, USER_PASS, repositoryType, temporaryFolder)); + @Test(expected = RepositoryClientException.class) + public void userWithReadPermissionShouldBeNotAuthorizedToCommit() throws IOException { + createAndCommit(USER_READ); } @Test public void userWithOwnerPermissionShouldBeAuthorizedToCommit() throws IOException { - assertTrue(RepositoryUtil.canUserCommit(USER_OWNER, USER_PASS, repositoryType, temporaryFolder)); + createAndCommit(USER_OWNER); } @Test public void userWithWritePermissionShouldBeAuthorizedToCommit() throws IOException { - assertTrue(RepositoryUtil.canUserCommit(USER_WRITE, USER_PASS, repositoryType, temporaryFolder)); + createAndCommit(USER_WRITE); + } + + private void createAndCommit(String username) throws IOException { + RepositoryClient client = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.newFolder(), username, PermissionsITCase.USER_PASS); + addAndCommitRandomFile(client, username); } @Test - public void userWithWOwnerPermissionShouldBeAuthorizedToDeleteRepository(){ - RepositoryUtil.assertDeleteRepositoryOperation(USER_OWNER, HttpStatus.SC_NO_CONTENT, HttpStatus.SC_NOT_FOUND, USER_PASS, repositoryType); + public void userWithOwnerPermissionShouldBeAuthorizedToDeleteRepository(){ + assertDeleteRepositoryOperation(HttpStatus.SC_NO_CONTENT, HttpStatus.SC_NOT_FOUND, USER_OWNER, repositoryType); } @Test - public void userWithWReadPermissionShouldNotBeAuthorizedToDeleteRepository(){ - RepositoryUtil.assertDeleteRepositoryOperation(USER_READ, HttpStatus.SC_FORBIDDEN, HttpStatus.SC_OK, USER_PASS, repositoryType); + public void userWithReadPermissionShouldNotBeAuthorizedToDeleteRepository(){ + assertDeleteRepositoryOperation(HttpStatus.SC_FORBIDDEN, HttpStatus.SC_OK, USER_READ, repositoryType); } @Test - public void userWithWWritePermissionShouldNotBeAuthorizedToDeleteRepository(){ - RepositoryUtil.assertDeleteRepositoryOperation(USER_WRITE, HttpStatus.SC_FORBIDDEN, HttpStatus.SC_OK, USER_PASS, repositoryType); + public void userWithWritePermissionShouldNotBeAuthorizedToDeleteRepository(){ + assertDeleteRepositoryOperation(HttpStatus.SC_FORBIDDEN, HttpStatus.SC_OK, USER_WRITE, repositoryType); } + private void assertDeleteRepositoryOperation(int expectedDeleteStatus, int expectedGetStatus, String user, String repositoryType) { + given(VndMediaType.REPOSITORY, user, PermissionsITCase.USER_PASS) + + .when() + .delete(TestData.getDefaultRepositoryUrl(repositoryType)) + + .then() + .statusCode(expectedDeleteStatus); + + given(VndMediaType.REPOSITORY, user, PermissionsITCase.USER_PASS) + + .when() + .get(TestData.getDefaultRepositoryUrl(repositoryType)) + + .then() + .statusCode(expectedGetStatus); + } } diff --git a/scm-it/src/test/java/sonia/scm/it/RepositoriesITCase.java b/scm-it/src/test/java/sonia/scm/it/RepositoriesITCase.java index e609b0075e..21d4d97b1b 100644 --- a/scm-it/src/test/java/sonia/scm/it/RepositoriesITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/RepositoriesITCase.java @@ -34,6 +34,7 @@ package sonia.scm.it; //~--- non-JDK imports -------------------------------------------------------- import org.apache.http.HttpStatus; +import org.assertj.core.api.Assertions; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -52,7 +53,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; import static sonia.scm.it.RegExMatcher.matchesPattern; import static sonia.scm.it.RestUtil.createResourceUrl; import static sonia.scm.it.RestUtil.given; @@ -136,13 +136,16 @@ public class RepositoriesITCase { @Test public void shouldCloneRepository() throws IOException { - RepositoryClient client = RepositoryUtil.createRepositoryClient(repositoryType,temporaryFolder.getRoot()); + RepositoryClient client = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.getRoot()); assertEquals("expected metadata dir", 1, Objects.requireNonNull(client.getWorkingCopy().list()).length); } @Test public void shouldCommitFiles() throws IOException { - assertTrue(RepositoryUtil.canScmAdminCommit(repositoryType,temporaryFolder)); + RepositoryClient client = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.newFolder(), "scmadmin", "scmadmin"); + String name = RepositoryUtil.addAndCommitRandomFile(client, "scmadmin"); + RepositoryClient checkClient = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.newFolder(), "scmadmin", "scmadmin"); + Assertions.assertThat(checkClient.getWorkingCopy().list()).contains(name); } } diff --git a/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java b/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java index ff8a3092a7..7b5f1ae2e8 100644 --- a/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java @@ -46,8 +46,8 @@ public class RepositoryAccessITCase { public void shouldFindBranches() throws IOException { assumeFalse("There are no branches for SVN", repositoryType.equals("svn")); - RepositoryClient repositoryClient = RepositoryUtil.createRepositoryClient(repositoryType, folder ); - RepositoryUtil.createAndCommitFile(folder, repositoryClient, "scmadmin", "a.txt", "a"); + RepositoryClient repositoryClient = RepositoryUtil.createRepositoryClient(repositoryType, folder); + RepositoryUtil.createAndCommitFile(repositoryClient, "scmadmin", "a.txt", "a"); String branchesUrl = given() .when() diff --git a/scm-it/src/test/java/sonia/scm/it/RepositoryUtil.java b/scm-it/src/test/java/sonia/scm/it/RepositoryUtil.java index 0c59bdc5ad..6706fba21d 100644 --- a/scm-it/src/test/java/sonia/scm/it/RepositoryUtil.java +++ b/scm-it/src/test/java/sonia/scm/it/RepositoryUtil.java @@ -3,17 +3,14 @@ package sonia.scm.it; import com.google.common.base.Charsets; import com.google.common.io.Files; import org.apache.http.HttpStatus; -import org.junit.rules.TemporaryFolder; import sonia.scm.repository.Changeset; import sonia.scm.repository.Person; import sonia.scm.repository.client.api.ClientCommand; import sonia.scm.repository.client.api.RepositoryClient; -import sonia.scm.repository.client.api.RepositoryClientException; import sonia.scm.repository.client.api.RepositoryClientFactory; import sonia.scm.web.VndMediaType; import java.io.File; -import java.io.FileOutputStream; import java.io.IOException; import java.util.UUID; @@ -23,35 +20,6 @@ public class RepositoryUtil { private static final RepositoryClientFactory REPOSITORY_CLIENT_FACTORY = new RepositoryClientFactory(); - static void addRandomFileToRepository(RepositoryClient client) throws IOException { - String uuid = UUID.randomUUID().toString(); - String name = "file-" + uuid + ".uuid"; - - File file = new File(client.getWorkingCopy(), name); - try (FileOutputStream out = new FileOutputStream(file)) { - out.write(uuid.getBytes()); - } - client.getAddCommand().add(name); - } - - static boolean canScmAdminCommit(String repositoryType, TemporaryFolder temporaryFolder) throws IOException { - return canUserCommit("scmadmin", "scmadmin", repositoryType, temporaryFolder); - } - - static boolean canUserCommit(String username, String password, String repositoryType, TemporaryFolder temporaryFolder) throws IOException { - RepositoryClient client = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.newFolder(), username, password); - for (int i = 0; i < 5; i++) { - addRandomFileToRepository(client); - } - try{ - commit(client, username, "commit"); - }catch (RepositoryClientException e){ - return false; - } - RepositoryClient checkClient = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.newFolder(), username, password); - return checkClient.getWorkingCopy().list().length == 6; - } - static RepositoryClient createRepositoryClient(String repositoryType, File folder) throws IOException { return createRepositoryClient(repositoryType, folder, "scmadmin", "scmadmin"); } @@ -69,25 +37,16 @@ public class RepositoryUtil { return REPOSITORY_CLIENT_FACTORY.create(repositoryType, httpProtocolUrl, username, password, folder); } - static void assertDeleteRepositoryOperation(String user, int deleteStatus, int getStatus, String password, String repositoryType) { - given(VndMediaType.REPOSITORY, user, password) - .when() - .delete(TestData.getDefaultRepositoryUrl(repositoryType)) - - .then() - .statusCode(deleteStatus); - - given(VndMediaType.REPOSITORY, user, password) - - .when() - .get(TestData.getDefaultRepositoryUrl(repositoryType)) - - .then() - .statusCode(getStatus); + static String addAndCommitRandomFile(RepositoryClient client, String username) throws IOException { + String uuid = UUID.randomUUID().toString(); + String name = "file-" + uuid + ".uuid"; + createAndCommitFile(client, username, name, uuid); + return name; } - static void createAndCommitFile(File folder, RepositoryClient repositoryClient, String username, String fileName, String content) throws IOException { - Files.write(content, new File(folder, fileName), Charsets.UTF_8); + + static void createAndCommitFile(RepositoryClient repositoryClient, String username, String fileName, String content) throws IOException { + Files.write(content, new File(repositoryClient.getWorkingCopy(), fileName), Charsets.UTF_8); repositoryClient.getAddCommand().add(fileName); commit(repositoryClient, username, "added " + fileName); } From 585d37feedb3f380dec3469b77ee36b1dd283a9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 22 Aug 2018 11:19:19 +0200 Subject: [PATCH 05/15] Split up test methods for each user --- .../java/sonia/scm/it/PermissionsITCase.java | 45 +++++++++++++++---- .../java/sonia/scm/it/RepositoryUtil.java | 11 +---- .../src/test/java/sonia/scm/it/TestData.java | 45 ++++++++++++------- 3 files changed, 68 insertions(+), 33 deletions(-) diff --git a/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java b/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java index 07e2a36534..6335907e76 100644 --- a/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java @@ -53,6 +53,7 @@ import static org.junit.Assert.assertEquals; import static sonia.scm.it.RepositoryUtil.addAndCommitRandomFile; import static sonia.scm.it.RestUtil.given; import static sonia.scm.it.ScmTypes.availableScmTypes; +import static sonia.scm.it.TestData.callUserPermissions; @RunWith(Parameterized.class) public class PermissionsITCase { @@ -61,6 +62,7 @@ public class PermissionsITCase { public static final String USER_PASS = "pass"; private static final String USER_WRITE = "user_write"; private static final String USER_OWNER = "user_owner"; + private static final String USER_OTHER = "user_other"; @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); @@ -87,29 +89,56 @@ public class PermissionsITCase { TestData.createUserPermission(USER_WRITE, PermissionType.WRITE, repositoryType); TestData.createUser(USER_OWNER, USER_PASS); TestData.createUserPermission(USER_OWNER, PermissionType.OWNER, repositoryType); + TestData.createUser(USER_OTHER, USER_PASS); createdPermissions = 3; } @Test - public void everyUserShouldSeePermissions() { + public void readUserShouldSeePermissions() { List userPermissions = TestData.getUserPermissions(USER_READ, USER_PASS, repositoryType); assertEquals(userPermissions.size(), createdPermissions); - userPermissions = TestData.getUserPermissions(USER_WRITE, USER_PASS, repositoryType); - assertEquals(userPermissions.size(), createdPermissions); - userPermissions = TestData.getUserPermissions(USER_OWNER, USER_PASS, repositoryType); - assertEquals(userPermissions.size(), createdPermissions); } @Test - public void everyUserShouldCloneRepository() throws IOException { + public void writeUserShouldSeePermissions() { + List userPermissions = TestData.getUserPermissions(USER_WRITE, USER_PASS, repositoryType); + assertEquals(userPermissions.size(), createdPermissions); + } + + @Test + public void ownerShouldSeePermissions() { + List userPermissions = TestData.getUserPermissions(USER_OWNER, USER_PASS, repositoryType); + assertEquals(userPermissions.size(), createdPermissions); + } + + @Test + public void otherUserShouldNotSeePermissions() { + callUserPermissions(USER_OTHER, USER_PASS, repositoryType, HttpStatus.SC_FORBIDDEN); + } + + @Test + public void readUserShouldCloneRepository() throws IOException { RepositoryClient client = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.newFolder(), USER_READ, USER_PASS); assertEquals(1, Objects.requireNonNull(client.getWorkingCopy().list()).length); - client = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.newFolder(), USER_WRITE, USER_PASS); + } + + @Test + public void writeUserShouldCloneRepository() throws IOException { + RepositoryClient client = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.newFolder(), USER_WRITE, USER_PASS); assertEquals(1, Objects.requireNonNull(client.getWorkingCopy().list()).length); - client = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.newFolder(), USER_OWNER, USER_PASS); + } + + @Test + public void ownerShouldCloneRepository() throws IOException { + RepositoryClient client = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.newFolder(), USER_OWNER, USER_PASS); assertEquals(1, Objects.requireNonNull(client.getWorkingCopy().list()).length); } + @Test + public void otherUserShouldNotCloneRepository() { + TestData.callRepository(USER_OTHER, USER_PASS, repositoryType, HttpStatus.SC_FORBIDDEN); + } + @Test(expected = RepositoryClientException.class) public void userWithReadPermissionShouldBeNotAuthorizedToCommit() throws IOException { createAndCommit(USER_READ); diff --git a/scm-it/src/test/java/sonia/scm/it/RepositoryUtil.java b/scm-it/src/test/java/sonia/scm/it/RepositoryUtil.java index 6706fba21d..e755f3c3c1 100644 --- a/scm-it/src/test/java/sonia/scm/it/RepositoryUtil.java +++ b/scm-it/src/test/java/sonia/scm/it/RepositoryUtil.java @@ -8,14 +8,11 @@ import sonia.scm.repository.Person; import sonia.scm.repository.client.api.ClientCommand; import sonia.scm.repository.client.api.RepositoryClient; import sonia.scm.repository.client.api.RepositoryClientFactory; -import sonia.scm.web.VndMediaType; import java.io.File; import java.io.IOException; import java.util.UUID; -import static sonia.scm.it.RestUtil.given; - public class RepositoryUtil { private static final RepositoryClientFactory REPOSITORY_CLIENT_FACTORY = new RepositoryClientFactory(); @@ -25,13 +22,7 @@ public class RepositoryUtil { } static RepositoryClient createRepositoryClient(String repositoryType, File folder, String username, String password) throws IOException { - String httpProtocolUrl = given(VndMediaType.REPOSITORY, username, password) - - .when() - .get(TestData.getDefaultRepositoryUrl(repositoryType)) - - .then() - .statusCode(HttpStatus.SC_OK) + String httpProtocolUrl = TestData.callRepository(username, password, repositoryType, HttpStatus.SC_OK) .extract() .path("_links.httpProtocol.href"); diff --git a/scm-it/src/test/java/sonia/scm/it/TestData.java b/scm-it/src/test/java/sonia/scm/it/TestData.java index e9177807eb..b758c81c0e 100644 --- a/scm-it/src/test/java/sonia/scm/it/TestData.java +++ b/scm-it/src/test/java/sonia/scm/it/TestData.java @@ -1,5 +1,6 @@ package sonia.scm.it; +import io.restassured.response.ValidatableResponse; import org.apache.http.HttpStatus; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -22,7 +23,7 @@ public class TestData { public static final String USER_SCM_ADMIN = "scmadmin"; public static final String USER_ANONYMOUS = "anonymous"; -private static final List PROTECTED_USERS = asList(USER_SCM_ADMIN, USER_ANONYMOUS); + private static final List PROTECTED_USERS = asList(USER_SCM_ADMIN, USER_ANONYMOUS); private static Map DEFAULT_REPOSITORIES = new HashMap<>(); @@ -48,7 +49,7 @@ private static final List PROTECTED_USERS = asList(USER_SCM_ADMIN, USER_ " \"active\": true,\n" + " \"admin\": false,\n" + " \"creationDate\": \"2018-08-21T12:26:46.084Z\",\n" + - " \"displayName\": \""+username+"\",\n" + + " \"displayName\": \"" + username + "\",\n" + " \"mail\": \"user1@scm-manager.org\",\n" + " \"name\": \"" + username + "\",\n" + " \"password\": \"" + password + "\",\n" + @@ -66,8 +67,8 @@ private static final List PROTECTED_USERS = asList(USER_SCM_ADMIN, USER_ given(VndMediaType.PERMISSION) .when() .content("{\n" + - "\t\"type\": \""+permissionType.name()+"\",\n" + - "\t\"name\": \""+name+"\",\n" + + "\t\"type\": \"" + permissionType.name() + "\",\n" + + "\t\"name\": \"" + name + "\",\n" + "\t\"groupPermission\": false\n" + "\t\n" + "}") @@ -77,18 +78,32 @@ private static final List PROTECTED_USERS = asList(USER_SCM_ADMIN, USER_ ; } - public static List getUserPermissions(String username, String password, String repositoryType) { - return given(VndMediaType.PERMISSION, username, password) - .when() - .get(TestData.getDefaultPermissionUrl(repositoryType)) - .then() - .statusCode(HttpStatus.SC_OK) - .extract() - .body().jsonPath().getList(""); - } + public static List getUserPermissions(String username, String password, String repositoryType) { + return callUserPermissions(username, password, repositoryType, HttpStatus.SC_OK) + .extract() + .body().jsonPath().getList(""); + } - private static String getDefaultPermissionUrl(String repositoryType) { - return getDefaultRepositoryUrl(repositoryType)+"/permissions/"; + public static ValidatableResponse callUserPermissions(String username, String password, String repositoryType, int expectedStatusCode) { + return given(VndMediaType.PERMISSION, username, password) + .when() + .get(TestData.getDefaultPermissionUrl(repositoryType)) + .then() + .statusCode(expectedStatusCode); + } + + public static ValidatableResponse callRepository(String username, String password, String repositoryType, int expectedStatusCode) { + return given(VndMediaType.REPOSITORY, username, password) + + .when() + .get(getDefaultRepositoryUrl(repositoryType)) + + .then() + .statusCode(expectedStatusCode); + } + + public static String getDefaultPermissionUrl(String repositoryType) { + return getDefaultRepositoryUrl(repositoryType) + "/permissions/"; } From 02f4801b58e6db0569c5db5fb9212d278276c589 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 22 Aug 2018 12:20:09 +0200 Subject: [PATCH 06/15] Let integration tests use links from HAL and test brute force links --- .../java/sonia/scm/it/PermissionsITCase.java | 49 +++++++++++++++---- .../src/test/java/sonia/scm/it/TestData.java | 14 ++++-- 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java b/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java index 6335907e76..f015d9e73d 100644 --- a/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java @@ -50,10 +50,12 @@ import java.util.List; import java.util.Objects; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static sonia.scm.it.RepositoryUtil.addAndCommitRandomFile; import static sonia.scm.it.RestUtil.given; import static sonia.scm.it.ScmTypes.availableScmTypes; -import static sonia.scm.it.TestData.callUserPermissions; +import static sonia.scm.it.TestData.USER_SCM_ADMIN; +import static sonia.scm.it.TestData.callRepository; @RunWith(Parameterized.class) public class PermissionsITCase { @@ -94,15 +96,35 @@ public class PermissionsITCase { } @Test - public void readUserShouldSeePermissions() { - List userPermissions = TestData.getUserPermissions(USER_READ, USER_PASS, repositoryType); - assertEquals(userPermissions.size(), createdPermissions); + public void readUserShouldNotSeePermissions() { + assertNull(callRepository(USER_WRITE, USER_PASS, repositoryType, HttpStatus.SC_OK) + .extract() + .body().jsonPath().getString("_links.permissions.href")); } @Test - public void writeUserShouldSeePermissions() { - List userPermissions = TestData.getUserPermissions(USER_WRITE, USER_PASS, repositoryType); - assertEquals(userPermissions.size(), createdPermissions); + public void readUserShouldNotSeeBruteForcePermissions() { + given(VndMediaType.PERMISSION, USER_READ, USER_PASS) + .when() + .get(TestData.getDefaultPermissionUrl(USER_SCM_ADMIN, USER_SCM_ADMIN, repositoryType)) + .then() + .statusCode(HttpStatus.SC_FORBIDDEN); + } + + @Test + public void writeUserShouldNotSeePermissions() { + assertNull(callRepository(USER_WRITE, USER_PASS, repositoryType, HttpStatus.SC_OK) + .extract() + .body().jsonPath().getString("_links.permissions.href")); + } + + @Test + public void writeUserShouldNotSeeBruteForcePermissions() { + given(VndMediaType.PERMISSION, USER_WRITE, USER_PASS) + .when() + .get(TestData.getDefaultPermissionUrl(USER_SCM_ADMIN, USER_SCM_ADMIN, repositoryType)) + .then() + .statusCode(HttpStatus.SC_FORBIDDEN); } @Test @@ -112,8 +134,17 @@ public class PermissionsITCase { } @Test - public void otherUserShouldNotSeePermissions() { - callUserPermissions(USER_OTHER, USER_PASS, repositoryType, HttpStatus.SC_FORBIDDEN); + public void otherUserShouldNotSeeRepository() { + callRepository(USER_OTHER, USER_PASS, repositoryType, HttpStatus.SC_FORBIDDEN); + } + + @Test + public void otherUserShouldNotSeeBruteForcePermissions() { + given(VndMediaType.PERMISSION, USER_OTHER, USER_PASS) + .when() + .get(TestData.getDefaultPermissionUrl(USER_SCM_ADMIN, USER_SCM_ADMIN, repositoryType)) + .then() + .statusCode(HttpStatus.SC_FORBIDDEN); } @Test diff --git a/scm-it/src/test/java/sonia/scm/it/TestData.java b/scm-it/src/test/java/sonia/scm/it/TestData.java index b758c81c0e..999d5117c8 100644 --- a/scm-it/src/test/java/sonia/scm/it/TestData.java +++ b/scm-it/src/test/java/sonia/scm/it/TestData.java @@ -72,7 +72,7 @@ public class TestData { "\t\"groupPermission\": false\n" + "\t\n" + "}") - .post(TestData.getDefaultPermissionUrl(repositoryType)) + .post(TestData.getDefaultPermissionUrl(USER_SCM_ADMIN, USER_SCM_ADMIN, repositoryType)) .then() .statusCode(HttpStatus.SC_CREATED) ; @@ -87,7 +87,7 @@ public class TestData { public static ValidatableResponse callUserPermissions(String username, String password, String repositoryType, int expectedStatusCode) { return given(VndMediaType.PERMISSION, username, password) .when() - .get(TestData.getDefaultPermissionUrl(repositoryType)) + .get(TestData.getDefaultPermissionUrl(username, password, repositoryType)) .then() .statusCode(expectedStatusCode); } @@ -102,8 +102,14 @@ public class TestData { .statusCode(expectedStatusCode); } - public static String getDefaultPermissionUrl(String repositoryType) { - return getDefaultRepositoryUrl(repositoryType) + "/permissions/"; + public static String getDefaultPermissionUrl(String username, String password, String repositoryType) { + return given(VndMediaType.REPOSITORY, username, password) + .when() + .get(getDefaultRepositoryUrl(repositoryType)) + .then() + .statusCode(HttpStatus.SC_OK) + .extract() + .body().jsonPath().getString("_links.permissions.href"); } From e77c633d64c1bd52d3427f6e4fb449b60a3a34c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 22 Aug 2018 12:29:45 +0200 Subject: [PATCH 07/15] Fix permission check for read permissions --- .../sonia/scm/api/v2/resources/PermissionRootResource.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java index d08f2e7057..d8e1c56e29 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java @@ -14,6 +14,7 @@ import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryException; import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.RepositoryNotFoundException; +import sonia.scm.repository.RepositoryPermissions; import sonia.scm.web.VndMediaType; import javax.inject.Inject; @@ -95,6 +96,7 @@ public class PermissionRootResource { @Path("{permission-name}") public Response get(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("permission-name") String permissionName) throws RepositoryException { Repository repository = load(namespace, name); + RepositoryPermissions.modify(repository).check(); return Response.ok( repository.getPermissions() .stream() @@ -125,6 +127,7 @@ public class PermissionRootResource { @Path("") public Response getAll(@PathParam("namespace") String namespace, @PathParam("name") String name) throws RepositoryNotFoundException { Repository repository = load(namespace, name); + RepositoryPermissions.modify(repository).check(); List permissionDtoList = repository.getPermissions() .stream() .map(per -> modelToDtoMapper.map(per, new NamespaceAndName(repository.getNamespace(), repository.getName()))) From 87e32087ac6d1f712812b22ad07afc7c96514dc0 Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Wed, 22 Aug 2018 15:55:29 +0200 Subject: [PATCH 08/15] #8771 run JUnit 5 with maven and fix tests --- pom.xml | 78 ++++++++++++++----- scm-test/pom.xml | 6 -- scm-webapp/pom.xml | 18 ----- .../v2/resources/PermissionRootResource.java | 27 ++----- .../resources/PermissionRootResourceTest.java | 24 +++--- 5 files changed, 82 insertions(+), 71 deletions(-) diff --git a/pom.xml b/pom.xml index 3295e732db..1c08c334f5 100644 --- a/pom.xml +++ b/pom.xml @@ -1,5 +1,6 @@ - + 4.0.0 @@ -83,7 +84,7 @@ scm-manager release repository http://maven.scm-manager.org/nexus/content/groups/public - + ossrh https://oss.sonatype.org/content/repositories/snapshots @@ -118,10 +119,26 @@ + + - junit - junit - test + org.junit.jupiter + junit-jupiter-api + + + + org.junit.jupiter + junit-jupiter-params + + + + org.junit.jupiter + junit-jupiter-engine + + + + org.junit.vintage + junit-vintage-engine @@ -139,15 +156,11 @@ org.mockito mockito-core - ${mockito.version} - test org.assertj assertj-core - 3.10.0 - test @@ -161,7 +174,7 @@ provided - + @@ -282,9 +295,32 @@ ${jackson.version} + + - junit - junit + org.junit.jupiter + junit-jupiter-api + ${junit.version} + test + + + + org.junit.jupiter + junit-jupiter-params + ${junit.version} + test + + + + org.junit.jupiter + junit-jupiter-engine + ${junit.version} + test + + + + org.junit.vintage + junit-vintage-engine ${junit.version} test @@ -348,7 +384,11 @@ - + + org.apache.maven.plugins + maven-surefire-plugin + 2.22.0 + org.apache.maven.plugins maven-enforcer-plugin @@ -536,9 +576,9 @@ maven-eclipse-plugin 2.6 - + - + org.jacoco jacoco-maven-plugin @@ -558,7 +598,7 @@ - + @@ -695,13 +735,13 @@ 2.10.0 1.3 - 4.12 + 5.2.0 1.7.22 1.1.10 3.0.1 - + 2.0.1 3.1.3.Final 1.19.4 @@ -711,7 +751,7 @@ 1.3.0 - + 9.2.10.v20150310 9.2.10.v20150310 diff --git a/scm-test/pom.xml b/scm-test/pom.xml index 0da030652b..98441ec898 100644 --- a/scm-test/pom.xml +++ b/scm-test/pom.xml @@ -28,12 +28,6 @@ 2.0.0-SNAPSHOT - - junit - junit - compile - - com.github.sdorra shiro-unit diff --git a/scm-webapp/pom.xml b/scm-webapp/pom.xml index 0b0ad145c6..b34da3454e 100644 --- a/scm-webapp/pom.xml +++ b/scm-webapp/pom.xml @@ -288,24 +288,6 @@ test - - org.junit.jupiter - junit-jupiter-api - 5.2.0 - test - - - org.junit.jupiter - junit-jupiter-params - 5.2.0 - test - - - org.junit.jupiter - junit-jupiter-engine - 5.2.0 - test - diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java index d8e1c56e29..61c7cc8f1e 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java @@ -6,26 +6,11 @@ import com.webcohesion.enunciate.metadata.rs.StatusCodes; import com.webcohesion.enunciate.metadata.rs.TypeHint; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang.StringUtils; -import sonia.scm.repository.NamespaceAndName; -import sonia.scm.repository.Permission; -import sonia.scm.repository.PermissionAlreadyExistsException; -import sonia.scm.repository.PermissionNotFoundException; -import sonia.scm.repository.Repository; -import sonia.scm.repository.RepositoryException; -import sonia.scm.repository.RepositoryManager; -import sonia.scm.repository.RepositoryNotFoundException; -import sonia.scm.repository.RepositoryPermissions; +import sonia.scm.repository.*; import sonia.scm.web.VndMediaType; import javax.inject.Inject; -import javax.ws.rs.Consumes; -import javax.ws.rs.DELETE; -import javax.ws.rs.GET; -import javax.ws.rs.POST; -import javax.ws.rs.PUT; -import javax.ws.rs.Path; -import javax.ws.rs.PathParam; -import javax.ws.rs.Produces; +import javax.ws.rs.*; import javax.ws.rs.core.Response; import java.net.URI; import java.util.List; @@ -96,7 +81,7 @@ public class PermissionRootResource { @Path("{permission-name}") public Response get(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("permission-name") String permissionName) throws RepositoryException { Repository repository = load(namespace, name); - RepositoryPermissions.modify(repository).check(); + checkUserPermission(repository); return Response.ok( repository.getPermissions() .stream() @@ -127,7 +112,7 @@ public class PermissionRootResource { @Path("") public Response getAll(@PathParam("namespace") String namespace, @PathParam("name") String name) throws RepositoryNotFoundException { Repository repository = load(namespace, name); - RepositoryPermissions.modify(repository).check(); + checkUserPermission(repository); List permissionDtoList = repository.getPermissions() .stream() .map(per -> modelToDtoMapper.map(per, new NamespaceAndName(repository.getNamespace(), repository.getName()))) @@ -135,6 +120,10 @@ public class PermissionRootResource { return Response.ok(permissionDtoList).build(); } + protected void checkUserPermission(Repository repository) { + RepositoryPermissions.modify(repository).check(); + } + /** * Update a permission to the user or group managed by the repository diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java index 39da0ce42e..16f8357850 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java @@ -2,6 +2,8 @@ package sonia.scm.api.v2.resources; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; +import com.github.sdorra.shiro.ShiroRule; +import com.github.sdorra.shiro.SubjectAware; import com.google.common.collect.ImmutableList; import lombok.ToString; import lombok.extern.slf4j.Slf4j; @@ -13,6 +15,7 @@ import org.jboss.resteasy.mock.MockHttpRequest; import org.jboss.resteasy.mock.MockHttpResponse; import org.jboss.resteasy.spi.HttpRequest; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -21,11 +24,7 @@ import org.junit.jupiter.api.TestFactory; import org.mockito.InjectMocks; import org.mockito.Mock; import sonia.scm.api.rest.AuthorizationExceptionMapper; -import sonia.scm.repository.NamespaceAndName; -import sonia.scm.repository.Permission; -import sonia.scm.repository.PermissionType; -import sonia.scm.repository.Repository; -import sonia.scm.repository.RepositoryManager; +import sonia.scm.repository.*; import sonia.scm.web.VndMediaType; import java.io.IOException; @@ -42,12 +41,15 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.fail; import static org.junit.jupiter.api.DynamicTest.dynamicTest; import static org.mockito.Matchers.any; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; import static org.mockito.MockitoAnnotations.initMocks; @Slf4j +@SubjectAware( + username = "trillian", + password = "secret", + configuration = "classpath:sonia/scm/repository/shiro.ini" +) public class PermissionRootResourceTest { private static final String REPOSITORY_NAMESPACE = "repo_namespace"; private static final String REPOSITORY_NAME = "repo"; @@ -89,6 +91,9 @@ public class PermissionRootResourceTest { private final Dispatcher dispatcher = MockDispatcherFactory.createDispatcher(); + @Rule + public ShiroRule shiro = new ShiroRule(); + @Mock private RepositoryManager repositoryManager; @@ -107,7 +112,7 @@ public class PermissionRootResourceTest { @Before public void prepareEnvironment() { initMocks(this); - permissionRootResource = new PermissionRootResource(permissionDtoToPermissionMapper, permissionToPermissionDtoMapper, resourceLinks, repositoryManager); + permissionRootResource = spy(new PermissionRootResource(permissionDtoToPermissionMapper, permissionToPermissionDtoMapper, resourceLinks, repositoryManager)); RepositoryRootResource repositoryRootResource = new RepositoryRootResource(MockProvider .of(new RepositoryResource(null, null, null, null, null, null, null,null, MockProvider.of(permissionRootResource))), null); dispatcher.getRegistry().addSingletonResource(repositoryRootResource); @@ -316,6 +321,7 @@ public class PermissionRootResourceTest { when(mockRepository.getNamespace()).thenReturn(REPOSITORY_NAMESPACE); when(mockRepository.getName()).thenReturn(REPOSITORY_NAME); when(repositoryManager.get(any(NamespaceAndName.class))).thenReturn(mockRepository); + doNothing().when(permissionRootResource).checkUserPermission(any()); return mockRepository; } From bc009c02ac14f01bfb7da52eb1dfc62850315ac3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 23 Aug 2018 17:24:00 +0200 Subject: [PATCH 09/15] Fix parameterized tests for junit 5 --- .../test/java/sonia/scm/it/IntegrationTestUtil.java | 10 +++++----- .../java/sonia/scm/it/RepositoryArchiveITCase.java | 2 +- .../test/java/sonia/scm/it/RepositoryHookITCase.java | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/scm-webapp/src/test/java/sonia/scm/it/IntegrationTestUtil.java b/scm-webapp/src/test/java/sonia/scm/it/IntegrationTestUtil.java index 741702ad81..3f17aff7a7 100644 --- a/scm-webapp/src/test/java/sonia/scm/it/IntegrationTestUtil.java +++ b/scm-webapp/src/test/java/sonia/scm/it/IntegrationTestUtil.java @@ -120,15 +120,15 @@ public final class IntegrationTestUtil } } - public static Collection createRepositoryTypeParameters() { - Collection params = new ArrayList<>(); + public static Collection createRepositoryTypeParameters() { + Collection params = new ArrayList<>(); - params.add("git"); - params.add("svn" ); + params.add(new String[] {"git"}); + params.add(new String[] {"svn"}); if (IOUtil.search("hg") != null) { - params.add("hg"); + params.add(new String[] {"hg"}); } return params; diff --git a/scm-webapp/src/test/java/sonia/scm/it/RepositoryArchiveITCase.java b/scm-webapp/src/test/java/sonia/scm/it/RepositoryArchiveITCase.java index 4cf7cd6571..bc79b9fa20 100644 --- a/scm-webapp/src/test/java/sonia/scm/it/RepositoryArchiveITCase.java +++ b/scm-webapp/src/test/java/sonia/scm/it/RepositoryArchiveITCase.java @@ -81,7 +81,7 @@ public class RepositoryArchiveITCase //~--- methods -------------------------------------------------------------- @Parameterized.Parameters(name = "{0}") - public static Collection createParameters() { + public static Collection createParameters() { return IntegrationTestUtil.createRepositoryTypeParameters(); } diff --git a/scm-webapp/src/test/java/sonia/scm/it/RepositoryHookITCase.java b/scm-webapp/src/test/java/sonia/scm/it/RepositoryHookITCase.java index 2be3bfd81b..dae80cd00d 100644 --- a/scm-webapp/src/test/java/sonia/scm/it/RepositoryHookITCase.java +++ b/scm-webapp/src/test/java/sonia/scm/it/RepositoryHookITCase.java @@ -208,7 +208,7 @@ public class RepositoryHookITCase extends AbstractAdminITCaseBase * @return repository types test parameter */ @Parameters(name = "{0}") - public static Collection createParameters() + public static Collection createParameters() { return IntegrationTestUtil.createRepositoryTypeParameters(); } From b3fd051321d29417b5592fe18072bbeb30e10095 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 24 Aug 2018 07:57:13 +0200 Subject: [PATCH 10/15] Check against feature instead of repository type --- .../test/java/sonia/scm/it/RepositoryAccessITCase.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java b/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java index 7b5f1ae2e8..861c9e7aee 100644 --- a/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java @@ -1,12 +1,14 @@ package sonia.scm.it; import org.apache.http.HttpStatus; +import org.junit.Assume; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; +import sonia.scm.repository.client.api.ClientCommand; import sonia.scm.repository.client.api.RepositoryClient; import java.io.File; @@ -14,7 +16,6 @@ import java.io.IOException; import java.util.Collection; import static org.junit.Assert.assertNotNull; -import static org.junit.Assume.assumeFalse; import static sonia.scm.it.RestUtil.given; import static sonia.scm.it.ScmTypes.availableScmTypes; @@ -44,9 +45,10 @@ public class RepositoryAccessITCase { @Test public void shouldFindBranches() throws IOException { - assumeFalse("There are no branches for SVN", repositoryType.equals("svn")); - RepositoryClient repositoryClient = RepositoryUtil.createRepositoryClient(repositoryType, folder); + + Assume.assumeTrue("There are no branches for " + repositoryType, repositoryClient.isCommandSupported(ClientCommand.BRANCH)); + RepositoryUtil.createAndCommitFile(repositoryClient, "scmadmin", "a.txt", "a"); String branchesUrl = given() From abe3dec8dfbe96b16121ee08c448d67a55a28372 Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Fri, 24 Aug 2018 10:57:50 +0200 Subject: [PATCH 11/15] #8771 fix get all to return links add permissionRead and permissionWrite permissions --- .../sonia/scm/repository/PermissionType.java | 4 +- .../java/sonia/scm/repository/Repository.java | 9 +- .../scm/web/filter/PermissionFilter.java | 17 +-- .../src/test/java/sonia/scm/it/TestData.java | 2 +- .../PermissionCollectionToDtoMapper.java | 51 +++++++ .../v2/resources/PermissionRootResource.java | 36 +++-- .../PermissionToPermissionDtoMapper.java | 25 ++-- .../scm/api/v2/resources/ResourceLinks.java | 4 + .../resources/PermissionRootResourceTest.java | 126 ++++++++++++------ .../DefaultAuthorizationCollectorTest.java | 15 ++- 10 files changed, 196 insertions(+), 93 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionCollectionToDtoMapper.java diff --git a/scm-core/src/main/java/sonia/scm/repository/PermissionType.java b/scm-core/src/main/java/sonia/scm/repository/PermissionType.java index bd4d773877..bba0d44f3d 100644 --- a/scm-core/src/main/java/sonia/scm/repository/PermissionType.java +++ b/scm-core/src/main/java/sonia/scm/repository/PermissionType.java @@ -42,10 +42,10 @@ public enum PermissionType { /** read permision */ - READ(0, "repository:read:"), + READ(0, "repository:read,pull:"), /** read and write permissionPrefix */ - WRITE(10, "repository:read,write:"), + WRITE(10, "repository:read,pull,push:"), /** * read, write and diff --git a/scm-core/src/main/java/sonia/scm/repository/Repository.java b/scm-core/src/main/java/sonia/scm/repository/Repository.java index 44841893e7..aabd022ecb 100644 --- a/scm-core/src/main/java/sonia/scm/repository/Repository.java +++ b/scm-core/src/main/java/sonia/scm/repository/Repository.java @@ -43,12 +43,7 @@ import sonia.scm.util.HttpUtil; import sonia.scm.util.Util; import sonia.scm.util.ValidationUtil; -import javax.xml.bind.annotation.XmlAccessType; -import javax.xml.bind.annotation.XmlAccessorType; -import javax.xml.bind.annotation.XmlElement; -import javax.xml.bind.annotation.XmlElementWrapper; -import javax.xml.bind.annotation.XmlRootElement; -import javax.xml.bind.annotation.XmlTransient; +import javax.xml.bind.annotation.*; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -60,7 +55,7 @@ import java.util.List; */ @StaticPermissions( value = "repository", - permissions = {"read", "write", "modify", "delete", "healthCheck"} + permissions = {"read", "modify", "delete", "healthCheck", "pull", "push", "permissionRead", "permissionWrite"} ) @XmlAccessorType(XmlAccessType.FIELD) @XmlRootElement(name = "repositories") diff --git a/scm-core/src/main/java/sonia/scm/web/filter/PermissionFilter.java b/scm-core/src/main/java/sonia/scm/web/filter/PermissionFilter.java index 7517f4848c..dd3c82e800 100644 --- a/scm-core/src/main/java/sonia/scm/web/filter/PermissionFilter.java +++ b/scm-core/src/main/java/sonia/scm/web/filter/PermissionFilter.java @@ -36,13 +36,11 @@ package sonia.scm.web.filter; //~--- non-JDK imports -------------------------------------------------------- import com.google.common.base.Splitter; - import org.apache.shiro.SecurityUtils; +import org.apache.shiro.authz.AuthorizationException; import org.apache.shiro.subject.Subject; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; - import sonia.scm.ArgumentIsInvalidException; import sonia.scm.SCMContext; import sonia.scm.config.ScmConfiguration; @@ -53,17 +51,14 @@ import sonia.scm.security.ScmSecurityException; import sonia.scm.util.HttpUtil; import sonia.scm.util.Util; -//~--- JDK imports ------------------------------------------------------------ - -import java.io.IOException; - -import java.util.Iterator; - import javax.servlet.FilterChain; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.apache.shiro.authz.AuthorizationException; +import java.io.IOException; +import java.util.Iterator; + +//~--- JDK imports ------------------------------------------------------------ /** * Abstract http filter to check repository permissions. @@ -339,7 +334,7 @@ public abstract class PermissionFilter extends HttpFilter if (writeRequest) { - permitted = RepositoryPermissions.write(repository).isPermitted(); + permitted = RepositoryPermissions.push(repository).isPermitted(); } else { diff --git a/scm-it/src/test/java/sonia/scm/it/TestData.java b/scm-it/src/test/java/sonia/scm/it/TestData.java index 999d5117c8..ada67d588c 100644 --- a/scm-it/src/test/java/sonia/scm/it/TestData.java +++ b/scm-it/src/test/java/sonia/scm/it/TestData.java @@ -81,7 +81,7 @@ public class TestData { public static List getUserPermissions(String username, String password, String repositoryType) { return callUserPermissions(username, password, repositoryType, HttpStatus.SC_OK) .extract() - .body().jsonPath().getList(""); + .body().jsonPath().getList("_embedded.permissions"); } public static ValidatableResponse callUserPermissions(String username, String password, String repositoryType, int expectedStatusCode) { diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionCollectionToDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionCollectionToDtoMapper.java new file mode 100644 index 0000000000..4789915f3d --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionCollectionToDtoMapper.java @@ -0,0 +1,51 @@ +package sonia.scm.api.v2.resources; + +import com.google.inject.Inject; +import de.otto.edison.hal.Embedded; +import de.otto.edison.hal.HalRepresentation; +import de.otto.edison.hal.Links; +import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryPermissions; + +import java.util.List; + +import static de.otto.edison.hal.Embedded.embeddedBuilder; +import static de.otto.edison.hal.Link.link; +import static de.otto.edison.hal.Links.linkingTo; +import static java.util.stream.Collectors.toList; + +public class PermissionCollectionToDtoMapper { + + private final ResourceLinks resourceLinks; + private final PermissionToPermissionDtoMapper permissionToPermissionDtoMapper; + + @Inject + public PermissionCollectionToDtoMapper(PermissionToPermissionDtoMapper permissionToPermissionDtoMapper, ResourceLinks resourceLinks) { + this.resourceLinks = resourceLinks; + this.permissionToPermissionDtoMapper = permissionToPermissionDtoMapper; + } + + public HalRepresentation map(Repository repository) { + List permissionDtoList = repository.getPermissions() + .stream() + .map(permission -> permissionToPermissionDtoMapper.map(permission, repository)) + .collect(toList()); + return new HalRepresentation(createLinks(repository), embedDtos(permissionDtoList)); + } + + private Links createLinks(Repository repository) { + RepositoryPermissions.permissionRead(repository).check(); + Links.Builder linksBuilder = linkingTo() + .with(Links.linkingTo().self(resourceLinks.permission().all(repository.getNamespace(), repository.getName())).build()); + if (RepositoryPermissions.permissionWrite(repository).isPermitted()) { + linksBuilder.single(link("create", resourceLinks.permission().create(repository.getNamespace(), repository.getName()))); + } + return linksBuilder.build(); + } + + private Embedded embedDtos(List permissionDtoList) { + return embeddedBuilder() + .with("permissions", permissionDtoList) + .build(); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java index 61c7cc8f1e..801c1b104c 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java @@ -13,23 +13,23 @@ import javax.inject.Inject; import javax.ws.rs.*; import javax.ws.rs.core.Response; import java.net.URI; -import java.util.List; import java.util.Optional; -import java.util.stream.Collectors; @Slf4j public class PermissionRootResource { private PermissionDtoToPermissionMapper dtoToModelMapper; private PermissionToPermissionDtoMapper modelToDtoMapper; + private PermissionCollectionToDtoMapper permissionCollectionToDtoMapper; private ResourceLinks resourceLinks; private final RepositoryManager manager; @Inject - public PermissionRootResource(PermissionDtoToPermissionMapper dtoToModelMapper, PermissionToPermissionDtoMapper modelToDtoMapper, ResourceLinks resourceLinks, RepositoryManager manager) { + public PermissionRootResource(PermissionDtoToPermissionMapper dtoToModelMapper, PermissionToPermissionDtoMapper modelToDtoMapper, PermissionCollectionToDtoMapper permissionCollectionToDtoMapper, ResourceLinks resourceLinks, RepositoryManager manager) { this.dtoToModelMapper = dtoToModelMapper; this.modelToDtoMapper = modelToDtoMapper; + this.permissionCollectionToDtoMapper = permissionCollectionToDtoMapper; this.resourceLinks = resourceLinks; this.manager = manager; } @@ -52,9 +52,11 @@ public class PermissionRootResource { }) @TypeHint(TypeHint.NO_CONTENT.class) @Consumes(VndMediaType.PERMISSION) + @Path("") public Response create(@PathParam("namespace") String namespace, @PathParam("name") String name, PermissionDto permission) throws RepositoryException { log.info("try to add new permission: {}", permission); Repository repository = load(namespace, name); + RepositoryPermissions.permissionWrite(repository).check(); checkPermissionAlreadyExists(permission, repository); repository.getPermissions().add(dtoToModelMapper.map(permission)); manager.modify(repository); @@ -81,12 +83,12 @@ public class PermissionRootResource { @Path("{permission-name}") public Response get(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("permission-name") String permissionName) throws RepositoryException { Repository repository = load(namespace, name); - checkUserPermission(repository); + RepositoryPermissions.permissionRead(repository).check(); return Response.ok( repository.getPermissions() .stream() .filter(permission -> permissionName.equals(permission.getName())) - .map(permission -> modelToDtoMapper.map(permission, new NamespaceAndName(repository.getNamespace(), repository.getName()))) + .map(permission -> modelToDtoMapper.map(permission, repository)) .findFirst() .orElseThrow(() -> new PermissionNotFoundException(repository, permissionName)) ).build(); @@ -112,16 +114,8 @@ public class PermissionRootResource { @Path("") public Response getAll(@PathParam("namespace") String namespace, @PathParam("name") String name) throws RepositoryNotFoundException { Repository repository = load(namespace, name); - checkUserPermission(repository); - List permissionDtoList = repository.getPermissions() - .stream() - .map(per -> modelToDtoMapper.map(per, new NamespaceAndName(repository.getNamespace(), repository.getName()))) - .collect(Collectors.toList()); - return Response.ok(permissionDtoList).build(); - } - - protected void checkUserPermission(Repository repository) { - RepositoryPermissions.modify(repository).check(); + RepositoryPermissions.permissionRead(repository).check(); + return Response.ok(permissionCollectionToDtoMapper.map(repository)).build(); } @@ -142,11 +136,12 @@ public class PermissionRootResource { @Consumes(VndMediaType.PERMISSION) @Path("{permission-name}") public Response update(@PathParam("namespace") String namespace, - @PathParam("name") String name, - @PathParam("permission-name") String permissionName, - PermissionDto permission) throws RepositoryException { + @PathParam("name") String name, + @PathParam("permission-name") String permissionName, + PermissionDto permission) throws RepositoryException { log.info("try to update the permission with name: {}. the modified permission is: {}", permissionName, permission); Repository repository = load(namespace, name); + RepositoryPermissions.permissionWrite(repository).check(); Permission existingPermission = repository.getPermissions() .stream() .filter(perm -> StringUtils.isNotBlank(perm.getName()) && perm.getName().equals(permissionName)) @@ -174,10 +169,11 @@ public class PermissionRootResource { @TypeHint(TypeHint.NO_CONTENT.class) @Path("{permission-name}") public Response delete(@PathParam("namespace") String namespace, - @PathParam("name") String name, - @PathParam("permission-name") String permissionName) throws RepositoryException { + @PathParam("name") String name, + @PathParam("permission-name") String permissionName) throws RepositoryException { log.info("try to delete the permission with name: {}.", permissionName); Repository repository = load(namespace, name); + RepositoryPermissions.modify(repository).check(); repository.getPermissions() .stream() .filter(perm -> StringUtils.isNotBlank(perm.getName()) && perm.getName().equals(permissionName)) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionToPermissionDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionToPermissionDtoMapper.java index 49adb0e1c0..e7b2da1487 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionToPermissionDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionToPermissionDtoMapper.java @@ -2,8 +2,9 @@ package sonia.scm.api.v2.resources; import de.otto.edison.hal.Links; import org.mapstruct.*; -import sonia.scm.repository.NamespaceAndName; import sonia.scm.repository.Permission; +import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryPermissions; import javax.inject.Inject; @@ -17,20 +18,28 @@ public abstract class PermissionToPermissionDtoMapper { private ResourceLinks resourceLinks; @Mapping(target = "attributes", ignore = true) // We do not map HAL attributes - public abstract PermissionDto map(Permission permission, @Context NamespaceAndName namespaceAndName); + public abstract PermissionDto map(Permission permission, @Context Repository repository); + + + @BeforeMapping + void validatePermissions(@Context Repository repository) { + RepositoryPermissions.permissionRead(repository).check(); + } /** * Add the self, update and delete links. * - * @param target the mapped dto - * @param namespaceAndName the repository namespace and name + * @param target the mapped dto + * @param repository the repository */ @AfterMapping - void appendLinks(@MappingTarget PermissionDto target, @Context NamespaceAndName namespaceAndName) { + void appendLinks(@MappingTarget PermissionDto target, @Context Repository repository) { Links.Builder linksBuilder = linkingTo() - .self(resourceLinks.permission().self(namespaceAndName.getNamespace(), namespaceAndName.getName(), target.getName())); - linksBuilder.single(link("update", resourceLinks.permission().update(namespaceAndName.getNamespace(), namespaceAndName.getName(), target.getName()))); - linksBuilder.single(link("delete", resourceLinks.permission().delete(namespaceAndName.getNamespace(), namespaceAndName.getName(), target.getName()))); + .self(resourceLinks.permission().self(repository.getNamespace(), repository.getName(), target.getName())); + if (RepositoryPermissions.permissionWrite(repository).isPermitted()) { + linksBuilder.single(link("update", resourceLinks.permission().update(repository.getNamespace(), repository.getName(), target.getName()))); + linksBuilder.single(link("delete", resourceLinks.permission().delete(repository.getNamespace(), repository.getName(), target.getName()))); + } target.add(linksBuilder.build()); } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java index 47e6e93777..3361bc43b1 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java @@ -313,6 +313,10 @@ class ResourceLinks { return permissionLinkBuilder.method("getRepositoryResource").parameters(namespace, name).method("permissions").parameters().method("getAll").parameters().href(); } + String create(String repositoryNamespace, String repositoryName) { + return permissionLinkBuilder.method("getRepositoryResource").parameters(repositoryNamespace, repositoryName).method("permissions").parameters().method("create").parameters().href(); + } + String self(String repositoryNamespace, String repositoryName, String permissionName) { return getLink(repositoryNamespace, repositoryName, permissionName, "get"); } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java index 16f8357850..3736f99301 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java @@ -1,13 +1,17 @@ package sonia.scm.api.v2.resources; -import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import com.github.sdorra.shiro.ShiroRule; import com.github.sdorra.shiro.SubjectAware; import com.google.common.collect.ImmutableList; +import de.otto.edison.hal.HalRepresentation; import lombok.ToString; import lombok.extern.slf4j.Slf4j; import org.apache.shiro.authz.AuthorizationException; +import org.apache.shiro.subject.Subject; +import org.apache.shiro.subject.support.SubjectThreadState; +import org.apache.shiro.util.ThreadContext; +import org.apache.shiro.util.ThreadState; import org.assertj.core.util.Lists; import org.jboss.resteasy.core.Dispatcher; import org.jboss.resteasy.mock.MockDispatcherFactory; @@ -33,6 +37,7 @@ import java.net.URISyntaxException; import java.util.ArrayList; import java.util.List; import java.util.function.Consumer; +import java.util.stream.Collectors; import java.util.stream.Stream; import static de.otto.edison.hal.Link.link; @@ -53,6 +58,10 @@ import static org.mockito.MockitoAnnotations.initMocks; public class PermissionRootResourceTest { private static final String REPOSITORY_NAMESPACE = "repo_namespace"; private static final String REPOSITORY_NAME = "repo"; + private static final String PERMISSION_WRITE = "repository:permissionWrite:" + REPOSITORY_NAME; + private static final String PERMISSION_READ = "repository:permissionRead:" + REPOSITORY_NAME; + private static final String PERMISSION_OWNER = "repository:modify:" + REPOSITORY_NAME; + private static final String PERMISSION_NAME = "perm"; private static final String PATH_OF_ALL_PERMISSIONS = REPOSITORY_NAMESPACE + "/" + REPOSITORY_NAME + "/permissions/"; private static final String PATH_OF_ONE_PERMISSION = PATH_OF_ALL_PERMISSIONS + PERMISSION_NAME; @@ -106,15 +115,23 @@ public class PermissionRootResourceTest { @InjectMocks private PermissionDtoToPermissionMapperImpl permissionDtoToPermissionMapper; + private PermissionCollectionToDtoMapper permissionCollectionToDtoMapper; + private PermissionRootResource permissionRootResource; + private final Subject subject = mock(Subject.class); + private final ThreadState subjectThreadState = new SubjectThreadState(subject); + @BeforeEach @Before public void prepareEnvironment() { initMocks(this); - permissionRootResource = spy(new PermissionRootResource(permissionDtoToPermissionMapper, permissionToPermissionDtoMapper, resourceLinks, repositoryManager)); + permissionCollectionToDtoMapper = new PermissionCollectionToDtoMapper(permissionToPermissionDtoMapper, resourceLinks); + permissionRootResource = new PermissionRootResource(permissionDtoToPermissionMapper, permissionToPermissionDtoMapper, permissionCollectionToDtoMapper, resourceLinks, repositoryManager); RepositoryRootResource repositoryRootResource = new RepositoryRootResource(MockProvider - .of(new RepositoryResource(null, null, null, null, null, null, null,null, MockProvider.of(permissionRootResource))), null); + .of(new RepositoryResource(null, null, null, null, null, null, null, null, MockProvider.of(permissionRootResource))), null); + subjectThreadState.bind(); + ThreadContext.bind(subject); dispatcher.getRegistry().addSingletonResource(repositoryRootResource); dispatcher.getProviderFactory().registerProvider(RepositoryNotFoundExceptionMapper.class); dispatcher.getProviderFactory().registerProvider(PermissionNotFoundExceptionMapper.class); @@ -123,7 +140,7 @@ public class PermissionRootResourceTest { } @TestFactory - @DisplayName("test endpoints on missing repository and user is Admin") + @DisplayName("test endpoints on missing repository") Stream missedRepositoryTestFactory() { return createDynamicTestsToAssertResponses( requestGETAllPermissions.expectedResponseStatus(404), @@ -134,9 +151,13 @@ public class PermissionRootResourceTest { } @TestFactory - @DisplayName("test endpoints on missing permission and user is Admin") + @DisplayName("test endpoints on missing permissions and user is Admin") Stream missedPermissionTestFactory() { - authorizedUserHasARepository(); + Repository mockRepository = mock(Repository.class); + when(mockRepository.getId()).thenReturn(REPOSITORY_NAME); + when(mockRepository.getNamespace()).thenReturn(REPOSITORY_NAMESPACE); + when(mockRepository.getName()).thenReturn(REPOSITORY_NAME); + when(repositoryManager.get(any(NamespaceAndName.class))).thenReturn(mockRepository); return createDynamicTestsToAssertResponses( requestGETPermission.expectedResponseStatus(404), requestPOSTPermission.expectedResponseStatus(201), @@ -146,10 +167,12 @@ public class PermissionRootResourceTest { } @TestFactory - @DisplayName("test endpoints on missing permission and user is not Admin") + @DisplayName("test endpoints on missing permissions and user is not Admin") Stream missedPermissionUserForbiddenTestFactory() { Repository mockRepository = mock(Repository.class); when(mockRepository.getId()).thenReturn(REPOSITORY_NAME); + when(mockRepository.getNamespace()).thenReturn(REPOSITORY_NAMESPACE); + when(mockRepository.getName()).thenReturn(REPOSITORY_NAME); doThrow(AuthorizationException.class).when(repositoryManager).get(any(NamespaceAndName.class)); return createDynamicTestsToAssertResponses( requestGETPermission.expectedResponseStatus(403), @@ -159,15 +182,27 @@ public class PermissionRootResourceTest { requestPUTPermission.expectedResponseStatus(403)); } + @Test + public void userWithPermissionWritePermissionShouldGetAllPermissionsWithCreateAndUpdateLinks() throws URISyntaxException { + createUserWithRepositoryAndPermissions(TEST_PERMISSIONS, PERMISSION_WRITE); + assertGettingExpectedPermissions(ImmutableList.copyOf(TEST_PERMISSIONS), PERMISSION_WRITE); + } + + @Test + public void userWithPermissionReadPermissionShouldGetAllPermissionsWithoutCreateAndUpdateLinks() throws URISyntaxException { + createUserWithRepositoryAndPermissions(TEST_PERMISSIONS, PERMISSION_READ); + assertGettingExpectedPermissions(ImmutableList.copyOf(TEST_PERMISSIONS), PERMISSION_READ); + } + @Test public void shouldGetAllPermissions() throws URISyntaxException { - authorizedUserHasARepositoryWithPermissions(TEST_PERMISSIONS); - assertGettingExpectedPermissions(ImmutableList.copyOf(TEST_PERMISSIONS)); + createUserWithRepositoryAndPermissions(TEST_PERMISSIONS, PERMISSION_READ); + assertGettingExpectedPermissions(ImmutableList.copyOf(TEST_PERMISSIONS), PERMISSION_READ); } @Test public void shouldGetPermissionByName() throws URISyntaxException { - authorizedUserHasARepositoryWithPermissions(TEST_PERMISSIONS); + createUserWithRepositoryAndPermissions(TEST_PERMISSIONS, PERMISSION_READ); Permission expectedPermission = TEST_PERMISSIONS.get(0); assertExpectedRequest(requestGETPermission .expectedResponseStatus(200) @@ -179,7 +214,7 @@ public class PermissionRootResourceTest { PermissionDto actualPermissionDto = mapper.readValue(body, PermissionDto.class); assertThat(actualPermissionDto) .as("response payload match permission object model") - .isEqualToComparingFieldByFieldRecursively(getExpectedPermissionDto(expectedPermission)) + .isEqualToComparingFieldByFieldRecursively(getExpectedPermissionDto(expectedPermission, PERMISSION_READ)) ; } catch (IOException e) { fail(); @@ -190,7 +225,7 @@ public class PermissionRootResourceTest { @Test public void shouldGetCreatedPermissions() throws URISyntaxException { - authorizedUserHasARepositoryWithPermissions(TEST_PERMISSIONS); + createUserWithRepositoryAndPermissions(TEST_PERMISSIONS, PERMISSION_WRITE); Permission newPermission = new Permission("new_group_perm", PermissionType.WRITE, true); ArrayList permissions = Lists.newArrayList(TEST_PERMISSIONS); permissions.add(newPermission); @@ -202,12 +237,12 @@ public class PermissionRootResourceTest { .as("POST response has no body") .isBlank()) ); - assertGettingExpectedPermissions(expectedPermissions); + assertGettingExpectedPermissions(expectedPermissions, PERMISSION_WRITE); } @Test public void shouldNotAddExistingPermission() throws URISyntaxException { - authorizedUserHasARepositoryWithPermissions(TEST_PERMISSIONS); + createUserWithRepositoryAndPermissions(TEST_PERMISSIONS, PERMISSION_WRITE); Permission newPermission = TEST_PERMISSIONS.get(0); assertExpectedRequest(requestPOSTPermission .content("{\"name\" : \"" + newPermission.getName() + "\" , \"type\" : \"WRITE\" , \"groupPermission\" : true}") @@ -217,7 +252,7 @@ public class PermissionRootResourceTest { @Test public void shouldGetUpdatedPermissions() throws URISyntaxException { - authorizedUserHasARepositoryWithPermissions(TEST_PERMISSIONS); + createUserWithRepositoryAndPermissions(TEST_PERMISSIONS, PERMISSION_WRITE); Permission modifiedPermission = TEST_PERMISSIONS.get(0); // modify the type to owner modifiedPermission.setType(PermissionType.OWNER); @@ -230,13 +265,13 @@ public class PermissionRootResourceTest { .as("PUT response has no body") .isBlank()) ); - assertGettingExpectedPermissions(expectedPermissions); + assertGettingExpectedPermissions(expectedPermissions, PERMISSION_WRITE); } @Test public void shouldDeletePermissions() throws URISyntaxException { - authorizedUserHasARepositoryWithPermissions(TEST_PERMISSIONS); + createUserWithRepositoryAndPermissions(TEST_PERMISSIONS, PERMISSION_OWNER); Permission deletedPermission = TEST_PERMISSIONS.get(0); ImmutableList expectedPermissions = ImmutableList.copyOf(TEST_PERMISSIONS.subList(1, TEST_PERMISSIONS.size())); assertExpectedRequest(requestDELETEPermission @@ -246,12 +281,12 @@ public class PermissionRootResourceTest { .as("DELETE response has no body") .isBlank()) ); - assertGettingExpectedPermissions(expectedPermissions); + assertGettingExpectedPermissions(expectedPermissions, PERMISSION_READ); } @Test public void deletingNotExistingPermissionShouldProcess() throws URISyntaxException { - authorizedUserHasARepositoryWithPermissions(TEST_PERMISSIONS); + createUserWithRepositoryAndPermissions(TEST_PERMISSIONS, PERMISSION_OWNER); Permission deletedPermission = TEST_PERMISSIONS.get(0); ImmutableList expectedPermissions = ImmutableList.copyOf(TEST_PERMISSIONS.subList(1, TEST_PERMISSIONS.size())); assertExpectedRequest(requestDELETEPermission @@ -261,7 +296,7 @@ public class PermissionRootResourceTest { .as("DELETE response has no body") .isBlank()) ); - assertGettingExpectedPermissions(expectedPermissions); + assertGettingExpectedPermissions(expectedPermissions, PERMISSION_READ); assertExpectedRequest(requestDELETEPermission .path(PATH_OF_ALL_PERMISSIONS + deletedPermission.getName()) .expectedResponseStatus(204) @@ -269,23 +304,32 @@ public class PermissionRootResourceTest { .as("DELETE response has no body") .isBlank()) ); - assertGettingExpectedPermissions(expectedPermissions); + assertGettingExpectedPermissions(expectedPermissions, PERMISSION_READ); } - private void assertGettingExpectedPermissions(ImmutableList expectedPermissions) throws URISyntaxException { + private void assertGettingExpectedPermissions(ImmutableList expectedPermissions, String userPermission) throws URISyntaxException { assertExpectedRequest(requestGETAllPermissions .expectedResponseStatus(200) .responseValidator((response) -> { String body = response.getContentAsString(); ObjectMapper mapper = new ObjectMapper(); try { - List actualPermissionDtos = mapper.readValue(body, new TypeReference>() { - }); - assertThat(actualPermissionDtos) + HalRepresentation halRepresentation = mapper.readValue(body, HalRepresentation.class); + List actualPermissionDtos = halRepresentation.getEmbedded().getItemsBy("permissions", HalRepresentation.class); + List permissionDtoStream = actualPermissionDtos.stream() + .map(hal -> { + PermissionDto result = new PermissionDto(); + result.setName(hal.getAttribute("name").asText()); + result.setType(hal.getAttribute("type").asText()); + result.setGroupPermission(hal.getAttribute("groupPermission").asBoolean()); + result.add(hal.getLinks()); + return result; + }).collect(Collectors.toList()); + assertThat(permissionDtoStream) .as("response payload match permission object models") .hasSize(expectedPermissions.size()) .usingRecursiveFieldByFieldElementComparator() - .containsExactlyInAnyOrder(getExpectedPermissionDtos(Lists.newArrayList(expectedPermissions))) + .containsExactlyInAnyOrder(getExpectedPermissionDtos(Lists.newArrayList(expectedPermissions), userPermission)) ; } catch (IOException e) { fail(); @@ -294,39 +338,45 @@ public class PermissionRootResourceTest { ); } - private PermissionDto[] getExpectedPermissionDtos(ArrayList permissions) { + private PermissionDto[] getExpectedPermissionDtos(ArrayList permissions, String userPermission) { return permissions .stream() - .map(this::getExpectedPermissionDto) + .map(p -> getExpectedPermissionDto(p, userPermission)) .toArray(PermissionDto[]::new); } - private PermissionDto getExpectedPermissionDto(Permission permission) { + private PermissionDto getExpectedPermissionDto(Permission permission, String userPermission) { PermissionDto result = new PermissionDto(); result.setName(permission.getName()); result.setGroupPermission(permission.isGroupPermission()); result.setType(permission.getType().name()); String permissionHref = "/" + RepositoryRootResource.REPOSITORIES_PATH_V2 + PATH_OF_ALL_PERMISSIONS + permission.getName(); - result.add(linkingTo() - .self(permissionHref) - .single(link("update", permissionHref)) - .single(link("delete", permissionHref)) - .build()); + if (PERMISSION_READ.equals(userPermission)) { + result.add(linkingTo() + .self(permissionHref) + .build()); + } else { + result.add(linkingTo() + .self(permissionHref) + .single(link("update", permissionHref)) + .single(link("delete", permissionHref)) + .build()); + } return result; } - private Repository authorizedUserHasARepository() { + private Repository createUserWithRepository(String userPermission) { Repository mockRepository = mock(Repository.class); when(mockRepository.getId()).thenReturn(REPOSITORY_NAME); when(mockRepository.getNamespace()).thenReturn(REPOSITORY_NAMESPACE); when(mockRepository.getName()).thenReturn(REPOSITORY_NAME); when(repositoryManager.get(any(NamespaceAndName.class))).thenReturn(mockRepository); - doNothing().when(permissionRootResource).checkUserPermission(any()); + when(subject.isPermitted(userPermission != null ? eq(userPermission) : any(String.class))).thenReturn(true); return mockRepository; } - private void authorizedUserHasARepositoryWithPermissions(ArrayList permissions) { - when(authorizedUserHasARepository().getPermissions()).thenReturn(permissions); + private void createUserWithRepositoryAndPermissions(ArrayList permissions, String userPermission) { + when(createUserWithRepository(userPermission).getPermissions()).thenReturn(permissions); } private Stream createDynamicTestsToAssertResponses(ExpectedRequest... expectedRequests) { diff --git a/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java b/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java index 2b157c4dab..2c0ef898bc 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java @@ -40,15 +40,12 @@ import org.apache.shiro.authz.SimpleAuthorizationInfo; import org.apache.shiro.subject.SimplePrincipalCollection; import org.apache.shiro.subject.Subject; import org.hamcrest.Matchers; -import org.junit.Test; import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.Mockito; -import static org.mockito.Mockito.*; -import static org.hamcrest.Matchers.*; -import static org.junit.Assert.*; -import org.junit.Rule; import org.mockito.junit.MockitoJUnitRunner; import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; @@ -60,6 +57,12 @@ import sonia.scm.repository.RepositoryTestData; import sonia.scm.user.User; import sonia.scm.user.UserTestData; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.*; + /** * Unit tests for {@link AuthorizationCollector}. * @@ -200,7 +203,7 @@ public class DefaultAuthorizationCollectorTest { AuthorizationInfo authInfo = collector.collect(); assertThat(authInfo.getRoles(), Matchers.containsInAnyOrder(Role.USER)); assertThat(authInfo.getObjectPermissions(), nullValue()); - assertThat(authInfo.getStringPermissions(), containsInAnyOrder("repository:read:one", "repository:read,write:two")); + assertThat(authInfo.getStringPermissions(), containsInAnyOrder("repository:read,pull:one", "repository:read,pull,push:two")); } /** From 6acded2eed473d643be9c8844284fe7c6f334621 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 27 Aug 2018 10:31:07 +0200 Subject: [PATCH 12/15] Flip equals check so that missing properties will not result in NPE --- .../java/sonia/scm/api/v2/resources/RepositoryResource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java index 817eb29f11..c21b7727cb 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java @@ -176,6 +176,6 @@ public class RepositoryResource { } private Predicate nameAndNamespaceStaysTheSame(String namespace, String name) { - return changed -> changed.getName().equals(name) && changed.getNamespace().equals(namespace); + return changed -> name.equals(changed.getName()) && namespace.equals(changed.getNamespace()); } } From 7766a99154ec51518f64039ada3eac59d01e76de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 27 Aug 2018 10:45:20 +0200 Subject: [PATCH 13/15] Do not delete permissions on repository update --- .../api/v2/resources/RepositoryResource.java | 8 +++++- .../resources/RepositoryRootResourceTest.java | 28 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java index c21b7727cb..3ee27f5f84 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java @@ -128,11 +128,17 @@ public class RepositoryResource { public Response update(@PathParam("namespace") String namespace, @PathParam("name") String name, RepositoryDto repositoryDto) { return adapter.update( loadBy(namespace, name), - existing -> dtoToRepositoryMapper.map(repositoryDto, existing.getId()), + existing -> processUpdate(repositoryDto, existing), nameAndNamespaceStaysTheSame(namespace, name) ); } + private Repository processUpdate(RepositoryDto repositoryDto, Repository existing) { + Repository changedRepository = dtoToRepositoryMapper.map(repositoryDto, existing.getId()); + changedRepository.setPermissions(existing.getPermissions()); + return changedRepository; + } + @Path("tags/") public TagRootResource tags() { return tagRootResource.get(); diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryRootResourceTest.java index d47b26e5eb..31cacf2d72 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryRootResourceTest.java @@ -11,10 +11,13 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.mockito.Answers; +import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import sonia.scm.PageResult; import sonia.scm.repository.NamespaceAndName; +import sonia.scm.repository.Permission; +import sonia.scm.repository.PermissionType; import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryException; import sonia.scm.repository.RepositoryIsNotArchivedException; @@ -35,10 +38,13 @@ import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT; import static javax.servlet.http.HttpServletResponse.SC_OK; import static javax.servlet.http.HttpServletResponse.SC_PRECONDITION_FAILED; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentCaptor.forClass; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyObject; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -240,6 +246,28 @@ public class RepositoryRootResourceTest { verify(repositoryManager).create(any(Repository.class)); } + @Test + public void shouldNotOverwriteExistingPermissionsOnUpdate() throws Exception { + Repository existingRepository = mockRepository("space", "repo"); + existingRepository.setPermissions(singletonList(new Permission("user", PermissionType.READ))); + + URL url = Resources.getResource("sonia/scm/api/v2/repository-test-update.json"); + byte[] repository = Resources.toByteArray(url); + + ArgumentCaptor modifiedRepositoryCaptor = forClass(Repository.class); + doNothing().when(repositoryManager).modify(modifiedRepositoryCaptor.capture()); + + MockHttpRequest request = MockHttpRequest + .put("/" + RepositoryRootResource.REPOSITORIES_PATH_V2 + "space/repo") + .contentType(VndMediaType.REPOSITORY) + .content(repository); + MockHttpResponse response = new MockHttpResponse(); + + dispatcher.invoke(request, response); + + assertFalse(modifiedRepositoryCaptor.getValue().getPermissions().isEmpty()); + } + private PageResult createSingletonPageResult(Repository repository) { return new PageResult<>(singletonList(repository), 0); } From f66267e1070b1e0467328cb1139b69cdabc6b671 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 27 Aug 2018 13:04:05 +0200 Subject: [PATCH 14/15] Correct test case --- scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java b/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java index f015d9e73d..63d8b46d47 100644 --- a/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java @@ -97,7 +97,7 @@ public class PermissionsITCase { @Test public void readUserShouldNotSeePermissions() { - assertNull(callRepository(USER_WRITE, USER_PASS, repositoryType, HttpStatus.SC_OK) + assertNull(callRepository(USER_READ, USER_PASS, repositoryType, HttpStatus.SC_OK) .extract() .body().jsonPath().getString("_links.permissions.href")); } From 9a630c5677bf42c6a013576919079e579d688526 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 27 Aug 2018 11:07:18 +0000 Subject: [PATCH 15/15] Close branch feature/permissions_fix_it_and_iunit5_tests