#8771 fix get all to return links

add permissionRead and permissionWrite permissions

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<DynamicTest> 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<DynamicTest> 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<DynamicTest> 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<Permission> 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<Permission> 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<Permission> 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<Permission> expectedPermissions) throws URISyntaxException {
+  private void assertGettingExpectedPermissions(ImmutableList<Permission> expectedPermissions, String userPermission) throws URISyntaxException {
     assertExpectedRequest(requestGETAllPermissions
       .expectedResponseStatus(200)
       .responseValidator((response) -> {
         String body = response.getContentAsString();
         ObjectMapper mapper = new ObjectMapper();
         try {
-          List<PermissionDto> actualPermissionDtos = mapper.readValue(body, new TypeReference<List<PermissionDto>>() {
-          });
-          assertThat(actualPermissionDtos)
+          HalRepresentation halRepresentation = mapper.readValue(body, HalRepresentation.class);
+          List<HalRepresentation> actualPermissionDtos = halRepresentation.getEmbedded().getItemsBy("permissions", HalRepresentation.class);
+          List<PermissionDto> 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<Permission> permissions) {
+  private PermissionDto[] getExpectedPermissionDtos(ArrayList<Permission> 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<Permission> permissions) {
-    when(authorizedUserHasARepository().getPermissions()).thenReturn(permissions);
+  private void createUserWithRepositoryAndPermissions(ArrayList<Permission> permissions, String userPermission) {
+    when(createUserWithRepository(userPermission).getPermissions()).thenReturn(permissions);
   }
 
   private Stream<DynamicTest> createDynamicTestsToAssertResponses(ExpectedRequest... expectedRequests) {

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"));
   }
   
   /**