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..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 map(@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 716e240bee..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 @@ -7,13 +7,13 @@ 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.web.VndMediaType; import javax.inject.Inject; @@ -68,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); @@ -94,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() @@ -124,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()))) @@ -154,14 +154,13 @@ 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.getPermissions() + Repository repository = load(namespace, name); + 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(); @@ -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); }