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 19c5b31349..fd8f07df8d 100644 --- a/scm-core/src/main/java/sonia/scm/repository/Repository.java +++ b/scm-core/src/main/java/sonia/scm/repository/Repository.java @@ -37,7 +37,6 @@ import com.github.sdorra.ssp.PermissionObject; import com.github.sdorra.ssp.StaticPermissions; import com.google.common.base.MoreObjects; import com.google.common.base.Objects; -import com.google.common.collect.Lists; import sonia.scm.BasicPropertiesAware; import sonia.scm.ModelObject; import sonia.scm.util.Util; @@ -50,8 +49,11 @@ import javax.xml.bind.annotation.XmlElementWrapper; import javax.xml.bind.annotation.XmlRootElement; import javax.xml.bind.annotation.XmlTransient; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; /** * Source code repository. @@ -79,7 +81,7 @@ public class Repository extends BasicPropertiesAware implements ModelObject, Per private Long lastModified; private String namespace; private String name; - private List permissions; + private final Set permissions = new HashSet<>(); @XmlElement(name = "public") private boolean publicReadable = false; private boolean archived = false; @@ -127,7 +129,6 @@ public class Repository extends BasicPropertiesAware implements ModelObject, Per this.name = name; this.contact = contact; this.description = description; - this.permissions = Lists.newArrayList(); if (Util.isNotEmpty(permissions)) { this.permissions.addAll(Arrays.asList(permissions)); @@ -200,12 +201,8 @@ public class Repository extends BasicPropertiesAware implements ModelObject, Per return new NamespaceAndName(getNamespace(), getName()); } - public List getPermissions() { - if (permissions == null) { - permissions = Lists.newArrayList(); - } - - return permissions; + public Collection getPermissions() { + return Collections.unmodifiableCollection(permissions); } /** @@ -300,8 +297,17 @@ public class Repository extends BasicPropertiesAware implements ModelObject, Per this.name = name; } - public void setPermissions(List permissions) { - this.permissions = permissions; + public void setPermissions(Collection permissions) { + this.permissions.clear(); + this.permissions.addAll(permissions); + } + + public void addPermission(Permission newPermission) { + this.permissions.add(newPermission); + } + + public void removePermission(Permission permission) { + this.permissions.remove(permission); } public void setPublicReadable(boolean publicReadable) { 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 8785f1d8ce..aa91e67022 100644 --- a/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java @@ -32,6 +32,7 @@ package sonia.scm.it; import org.apache.http.HttpStatus; +import org.assertj.core.api.Assertions; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -49,8 +50,10 @@ import sonia.scm.web.VndMediaType; import java.io.IOException; import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.Objects; +import static java.util.Arrays.asList; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static sonia.scm.it.utils.RepositoryUtil.addAndCommitRandomFile; @@ -72,7 +75,7 @@ public class PermissionsITCase { public TemporaryFolder temporaryFolder = new TemporaryFolder(); private final String repositoryType; - private int createdPermissions; + private Collection createdPermissions; public PermissionsITCase(String repositoryType) { @@ -94,7 +97,7 @@ public class PermissionsITCase { TestData.createNotAdminUser(USER_OWNER, USER_PASS); TestData.createUserPermission(USER_OWNER, PermissionType.OWNER, repositoryType); TestData.createNotAdminUser(USER_OTHER, USER_PASS); - createdPermissions = 3; + createdPermissions = asList(USER_READ, USER_WRITE, USER_OWNER); } @Test @@ -131,8 +134,8 @@ public class PermissionsITCase { @Test public void ownerShouldSeePermissions() { - List userPermissions = TestData.getUserPermissions(USER_OWNER, USER_PASS, repositoryType); - assertEquals(userPermissions.size(), createdPermissions); + List userPermissions = TestData.getUserPermissions(USER_OWNER, USER_PASS, repositoryType); + Assertions.assertThat(userPermissions).extracting(e -> e.get("name")).containsAll(createdPermissions); } @Test diff --git a/scm-it/src/test/java/sonia/scm/it/utils/TestData.java b/scm-it/src/test/java/sonia/scm/it/utils/TestData.java index a164fc6649..48605437c6 100644 --- a/scm-it/src/test/java/sonia/scm/it/utils/TestData.java +++ b/scm-it/src/test/java/sonia/scm/it/utils/TestData.java @@ -99,10 +99,10 @@ public class TestData { ; } - public static List getUserPermissions(String username, String password, String repositoryType) { + public static List getUserPermissions(String username, String password, String repositoryType) { return callUserPermissions(username, password, repositoryType, HttpStatus.SC_OK) .extract() - .body().jsonPath().getList("_embedded.permissions"); + .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/PermissionRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java index 9335e2b2df..127a3f450e 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 @@ -79,7 +79,7 @@ public class PermissionRootResource { Repository repository = load(namespace, name); RepositoryPermissions.permissionWrite(repository).check(); checkPermissionAlreadyExists(permission, repository); - repository.getPermissions().add(dtoToModelMapper.map(permission)); + repository.addPermission(dtoToModelMapper.map(permission)); manager.modify(repository); String urlPermissionName = modelToDtoMapper.getUrlPermissionName(permission); return Response.created(URI.create(resourceLinks.permission().self(namespace, name, urlPermissionName))).build(); @@ -209,7 +209,7 @@ public class PermissionRootResource { .stream() .filter(filterPermission(permissionName)) .findFirst() - .ifPresent(p -> repository.getPermissions().remove(p)) + .ifPresent(repository::removePermission) ; manager.modify(repository); log.info("the permission with name: {} is updated.", permissionName); diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryCollectionResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryCollectionResource.java index ad9c29f14e..d8d5280456 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryCollectionResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryCollectionResource.java @@ -5,12 +5,15 @@ import com.webcohesion.enunciate.metadata.rs.ResponseHeader; import com.webcohesion.enunciate.metadata.rs.ResponseHeaders; import com.webcohesion.enunciate.metadata.rs.StatusCodes; import com.webcohesion.enunciate.metadata.rs.TypeHint; +import org.apache.shiro.SecurityUtils; +import sonia.scm.repository.Permission; +import sonia.scm.repository.PermissionType; import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryManager; +import sonia.scm.user.User; import sonia.scm.web.VndMediaType; import javax.inject.Inject; -import javax.inject.Named; import javax.validation.Valid; import javax.ws.rs.Consumes; import javax.ws.rs.DefaultValue; @@ -21,6 +24,8 @@ import javax.ws.rs.Produces; import javax.ws.rs.QueryParam; import javax.ws.rs.core.Response; +import static java.util.Collections.singletonList; + public class RepositoryCollectionResource { private static final int DEFAULT_PAGE_SIZE = 10; @@ -89,7 +94,17 @@ public class RepositoryCollectionResource { @ResponseHeaders(@ResponseHeader(name = "Location", description = "uri to the created repository")) public Response create(@Valid RepositoryDto repository) { return adapter.create(repository, - () -> dtoToRepositoryMapper.map(repository, null), + () -> createModelObjectFromDto(repository), r -> resourceLinks.repository().self(r.getNamespace(), r.getName())); } + + private Repository createModelObjectFromDto(@Valid RepositoryDto repositoryDto) { + Repository repository = dtoToRepositoryMapper.map(repositoryDto, null); + repository.setPermissions(singletonList(new Permission(currentUser(), PermissionType.OWNER))); + return repository; + } + + private String currentUser() { + return SecurityUtils.getSubject().getPrincipals().oneByType(User.class).getName(); + } } diff --git a/scm-webapp/src/main/java/sonia/scm/security/AuthorizationChangedEventProducer.java b/scm-webapp/src/main/java/sonia/scm/security/AuthorizationChangedEventProducer.java index 5a954bb1e7..cf4c980625 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/AuthorizationChangedEventProducer.java +++ b/scm-webapp/src/main/java/sonia/scm/security/AuthorizationChangedEventProducer.java @@ -167,7 +167,7 @@ public class AuthorizationChangedEventProducer { private boolean isAuthorizationDataModified(Repository repository, Repository beforeModification) { return repository.isArchived() != beforeModification.isArchived() || repository.isPublicReadable() != beforeModification.isPublicReadable() - || ! repository.getPermissions().equals(beforeModification.getPermissions()); + || !(repository.getPermissions().containsAll(beforeModification.getPermissions()) && beforeModification.getPermissions().containsAll(repository.getPermissions())); } private void fireEventForEveryUser() { 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 faa7208c66..8a79293642 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java +++ b/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java @@ -54,13 +54,14 @@ import sonia.scm.cache.CacheManager; import sonia.scm.group.GroupNames; import sonia.scm.group.GroupPermissions; import sonia.scm.plugin.Extension; +import sonia.scm.repository.Permission; import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryDAO; -import sonia.scm.repository.RepositoryPermissions; import sonia.scm.user.User; import sonia.scm.user.UserPermissions; import sonia.scm.util.Util; +import java.util.Collection; import java.util.List; import java.util.Set; @@ -198,7 +199,7 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector private void collectRepositoryPermissions(Builder builder, Repository repository, User user, GroupNames groups) { - List repositoryPermissions + Collection repositoryPermissions = repository.getPermissions(); if (Util.isNotEmpty(repositoryPermissions)) 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 0487e445c1..c008e0b8db 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 @@ -402,18 +402,17 @@ public class PermissionRootResourceTest extends RepositoryTestBase { } 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(mockRepository.getNamespaceAndName()).thenReturn(new NamespaceAndName(REPOSITORY_NAMESPACE, REPOSITORY_NAME)); + Repository mockRepository = new Repository(); + mockRepository.setId(REPOSITORY_NAME); + mockRepository.setNamespace(REPOSITORY_NAMESPACE); + mockRepository.setName(REPOSITORY_NAME); when(repositoryManager.get(any(NamespaceAndName.class))).thenReturn(mockRepository); when(subject.isPermitted(userPermission != null ? eq(userPermission) : any(String.class))).thenReturn(true); return mockRepository; } private void createUserWithRepositoryAndPermissions(ArrayList permissions, String userPermission) { - when(createUserWithRepository(userPermission).getPermissions()).thenReturn(permissions); + createUserWithRepository(userPermission).setPermissions(permissions); } private Stream createDynamicTestsToAssertResponses(ExpectedRequest... expectedRequests) { @@ -421,10 +420,9 @@ public class PermissionRootResourceTest extends RepositoryTestBase { .map(entry -> dynamicTest("the endpoint " + entry.description + " should return the status code " + entry.expectedResponseStatus, () -> assertExpectedRequest(entry))); } - private MockHttpResponse assertExpectedRequest(ExpectedRequest entry) throws URISyntaxException { + private void assertExpectedRequest(ExpectedRequest entry) throws URISyntaxException { MockHttpResponse response = new MockHttpResponse(); - HttpRequest request = null; - request = MockHttpRequest + HttpRequest request = MockHttpRequest .create(entry.method, "/" + RepositoryRootResource.REPOSITORIES_PATH_V2 + entry.path) .content(entry.content) .contentType(VndMediaType.PERMISSION); @@ -436,7 +434,6 @@ public class PermissionRootResourceTest extends RepositoryTestBase { if (entry.responseValidator != null) { entry.responseValidator.accept(response); } - return response; } @ToString @@ -470,12 +467,12 @@ public class PermissionRootResourceTest extends RepositoryTestBase { return this; } - public ExpectedRequest expectedResponseStatus(int expectedResponseStatus) { + ExpectedRequest expectedResponseStatus(int expectedResponseStatus) { this.expectedResponseStatus = expectedResponseStatus; return this; } - public ExpectedRequest responseValidator(Consumer responseValidator) { + ExpectedRequest responseValidator(Consumer responseValidator) { this.responseValidator = responseValidator; return this; } 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 0b2e50b6d9..fe403088f2 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 @@ -4,6 +4,9 @@ import com.github.sdorra.shiro.ShiroRule; import com.github.sdorra.shiro.SubjectAware; import com.google.common.io.Resources; import com.google.inject.util.Providers; +import org.apache.shiro.subject.SimplePrincipalCollection; +import org.apache.shiro.subject.Subject; +import org.assertj.core.api.Assertions; import org.jboss.resteasy.core.Dispatcher; import org.jboss.resteasy.mock.MockHttpRequest; import org.jboss.resteasy.mock.MockHttpResponse; @@ -22,6 +25,7 @@ import sonia.scm.repository.RepositoryIsNotArchivedException; import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.api.RepositoryService; import sonia.scm.repository.api.RepositoryServiceFactory; +import sonia.scm.user.User; import sonia.scm.web.VndMediaType; import javax.servlet.http.HttpServletResponse; @@ -37,6 +41,7 @@ import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; 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.assertj.core.api.AssertionsForClassTypes.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -59,6 +64,8 @@ import static sonia.scm.api.v2.resources.DispatcherMock.createDispatcher; ) public class RepositoryRootResourceTest extends RepositoryTestBase { + private static final String REALM = "AdminRealm"; + private Dispatcher dispatcher; @Rule @@ -96,6 +103,13 @@ public class RepositoryRootResourceTest extends RepositoryTestBase { when(serviceFactory.create(any(Repository.class))).thenReturn(service); when(scmPathInfoStore.get()).thenReturn(uriInfo); when(uriInfo.getApiRestUri()).thenReturn(URI.create("/x/y")); + SimplePrincipalCollection trillian = new SimplePrincipalCollection("trillian", REALM); + trillian.add(new User("trillian"), REALM); + shiro.setSubject( + new Subject.Builder() + .principals(trillian) + .authenticated(true) + .buildSubject()); } @Test @@ -257,6 +271,34 @@ public class RepositoryRootResourceTest extends RepositoryTestBase { verify(repositoryManager).create(any(Repository.class)); } + @Test + public void shouldSetCurrentUserAsOwner() throws Exception { + ArgumentCaptor createCaptor = ArgumentCaptor.forClass(Repository.class); + when(repositoryManager.create(createCaptor.capture())).thenAnswer(invocation -> { + Repository repository = (Repository) invocation.getArguments()[0]; + repository.setNamespace("otherspace"); + return repository; + }); + + URL url = Resources.getResource("sonia/scm/api/v2/repository-test-update.json"); + byte[] repositoryJson = Resources.toByteArray(url); + + MockHttpRequest request = MockHttpRequest + .post("/" + RepositoryRootResource.REPOSITORIES_PATH_V2) + .contentType(VndMediaType.REPOSITORY) + .content(repositoryJson); + MockHttpResponse response = new MockHttpResponse(); + + dispatcher.invoke(request, response); + + Assertions.assertThat(createCaptor.getValue().getPermissions()) + .hasSize(1) + .allSatisfy(p -> { + assertThat(p.getName()).isEqualTo("trillian"); + assertThat(p.getType()).isEqualTo(PermissionType.OWNER); + }); + } + @Test public void shouldNotOverwriteExistingPermissionsOnUpdate() throws Exception { Repository existingRepository = mockRepository("space", "repo"); diff --git a/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerPerfTest.java b/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerPerfTest.java index 448c2561f3..9d03fa02ca 100644 --- a/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerPerfTest.java +++ b/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerPerfTest.java @@ -184,7 +184,7 @@ private long calculateAverage(List times) { private Repository createTestRepository(int number) { Repository repository = new Repository(keyGenerator.createKey(), REPOSITORY_TYPE, "namespace", "repo-" + number); - repository.getPermissions().add(new Permission("trillian", PermissionType.READ)); + repository.addPermission(new Permission("trillian", PermissionType.READ)); return repository; }