From abe3dec8dfbe96b16121ee08c448d67a55a28372 Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Fri, 24 Aug 2018 10:57:50 +0200 Subject: [PATCH] #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")); } /**