Remove permissions from repository

This commit is contained in:
René Pfeuffer
2019-01-22 13:00:02 +01:00
parent decc6d29d8
commit 4dcbcb80e7
15 changed files with 103 additions and 131 deletions

View File

@@ -68,7 +68,6 @@ import java.util.Set;
@XmlRootElement(name = "repositories")
public class Repository extends BasicPropertiesAware implements ModelObject, PermissionObject{
private static final long serialVersionUID = 3486560714961909711L;
private String contact;
@@ -81,7 +80,6 @@ public class Repository extends BasicPropertiesAware implements ModelObject, Per
private Long lastModified;
private String namespace;
private String name;
private final Set<RepositoryPermission> permissions = new HashSet<>();
@XmlElement(name = "public")
private boolean publicReadable = false;
private boolean archived = false;
@@ -119,20 +117,14 @@ public class Repository extends BasicPropertiesAware implements ModelObject, Per
* @param contact email address of a person who is responsible for
* this repository.
* @param description a short description of the repository
* @param permissions permissions for specific users and groups.
*/
public Repository(String id, String type, String namespace, String name, String contact,
String description, RepositoryPermission... permissions) {
public Repository(String id, String type, String namespace, String name, String contact, String description) {
this.id = id;
this.type = type;
this.namespace = namespace;
this.name = name;
this.contact = contact;
this.description = description;
if (Util.isNotEmpty(permissions)) {
this.permissions.addAll(Arrays.asList(permissions));
}
}
/**
@@ -201,10 +193,6 @@ public class Repository extends BasicPropertiesAware implements ModelObject, Per
return new NamespaceAndName(getNamespace(), getName());
}
public Collection<RepositoryPermission> getPermissions() {
return Collections.unmodifiableCollection(permissions);
}
/**
* Returns the type (hg, git, svn ...) of the {@link Repository}.
*
@@ -297,19 +285,6 @@ public class Repository extends BasicPropertiesAware implements ModelObject, Per
this.name = name;
}
public void setPermissions(Collection<RepositoryPermission> permissions) {
this.permissions.clear();
this.permissions.addAll(permissions);
}
public void addPermission(RepositoryPermission newPermission) {
this.permissions.add(newPermission);
}
public void removePermission(RepositoryPermission permission) {
this.permissions.remove(permission);
}
public void setPublicReadable(boolean publicReadable) {
this.publicReadable = publicReadable;
}
@@ -347,7 +322,6 @@ public class Repository extends BasicPropertiesAware implements ModelObject, Per
repository.setCreationDate(creationDate);
repository.setLastModified(lastModified);
repository.setDescription(description);
repository.setPermissions(permissions);
repository.setPublicReadable(publicReadable);
repository.setArchived(archived);
@@ -379,7 +353,6 @@ public class Repository extends BasicPropertiesAware implements ModelObject, Per
&& Objects.equal(description, other.description)
&& Objects.equal(publicReadable, other.publicReadable)
&& Objects.equal(archived, other.archived)
&& Objects.equal(permissions, other.permissions)
&& Objects.equal(type, other.type)
&& Objects.equal(creationDate, other.creationDate)
&& Objects.equal(lastModified, other.lastModified)
@@ -390,7 +363,7 @@ public class Repository extends BasicPropertiesAware implements ModelObject, Per
@Override
public int hashCode() {
return Objects.hashCode(id, namespace, name, contact, description, publicReadable,
archived, permissions, type, creationDate, lastModified, properties,
archived, type, creationDate, lastModified, properties,
healthCheckFailures);
}
@@ -404,7 +377,6 @@ public class Repository extends BasicPropertiesAware implements ModelObject, Per
.add("description", description)
.add("publicReadable", publicReadable)
.add("archived", archived)
.add("permissions", permissions)
.add("type", type)
.add("lastModified", lastModified)
.add("creationDate", creationDate)

View File

@@ -100,7 +100,8 @@ public class RepositoryCollectionResource {
private Repository createModelObjectFromDto(@Valid RepositoryDto repositoryDto) {
Repository repository = dtoToRepositoryMapper.map(repositoryDto, null);
repository.setPermissions(singletonList(new RepositoryPermission(currentUser(), PermissionType.OWNER)));
// TODO RP
// repository.setPermissions(singletonList(new RepositoryPermission(currentUser(), PermissionType.OWNER)));
return repository;
}

View File

@@ -10,7 +10,6 @@ public abstract class RepositoryDtoToRepositoryMapper extends BaseDtoMapper {
@Mapping(target = "id", ignore = true)
@Mapping(target = "publicReadable", ignore = true)
@Mapping(target = "healthCheckFailures", ignore = true)
@Mapping(target = "permissions", ignore = true)
public abstract Repository map(RepositoryDto repositoryDto, @Context String id);
@AfterMapping

View File

@@ -26,11 +26,12 @@ public class RepositoryPermissionCollectionToDtoMapper {
}
public HalRepresentation map(Repository repository) {
List<RepositoryPermissionDto> repositoryPermissionDtoList = repository.getPermissions()
.stream()
.map(permission -> repositoryPermissionToRepositoryPermissionDtoMapper.map(permission, repository))
.collect(toList());
return new HalRepresentation(createLinks(repository), embedDtos(repositoryPermissionDtoList));
// List<RepositoryPermissionDto> repositoryPermissionDtoList = repository.getPermissions()
// .stream()
// .map(permission -> repositoryPermissionToRepositoryPermissionDtoMapper.map(permission, repository))
// .collect(toList());
// return new HalRepresentation(createLinks(repository), embedDtos(repositoryPermissionDtoList));
return new HalRepresentation(createLinks(repository));
}
private Links createLinks(Repository repository) {

View File

@@ -78,7 +78,8 @@ public class RepositoryPermissionRootResource {
Repository repository = load(namespace, name);
RepositoryPermissions.permissionWrite(repository).check();
checkPermissionAlreadyExists(permission, repository);
repository.addPermission(dtoToModelMapper.map(permission));
// TODO RP
// repository.addPermission(dtoToModelMapper.map(permission));
manager.modify(repository);
String urlPermissionName = modelToDtoMapper.getUrlPermissionName(permission);
return Response.created(URI.create(resourceLinks.repositoryPermission().self(namespace, name, urlPermissionName))).build();
@@ -106,12 +107,13 @@ public class RepositoryPermissionRootResource {
Repository repository = load(namespace, name);
RepositoryPermissions.permissionRead(repository).check();
return Response.ok(
repository.getPermissions()
.stream()
.filter(filterPermission(permissionName))
.map(permission -> modelToDtoMapper.map(permission, repository))
.findFirst()
.orElseThrow(() -> notFound(entity(RepositoryPermission.class, namespace).in(Repository.class, namespace + "/" + name)))
// TODO RP
// repository.getPermissions()
// .stream()
// .filter(filterPermission(permissionName))
// .map(permission -> modelToDtoMapper.map(permission, repository))
// .findFirst()
// .orElseThrow(() -> notFound(entity(RepositoryPermission.class, namespace).in(Repository.class, namespace + "/" + name)))
).build();
}
@@ -172,12 +174,14 @@ public class RepositoryPermissionRootResource {
if (!extractedPermissionName.equals(permission.getName())) {
checkPermissionAlreadyExists(permission, repository);
}
RepositoryPermission existingPermission = repository.getPermissions()
.stream()
.filter(filterPermission(permissionName))
.findFirst()
.orElseThrow(() -> notFound(entity(RepositoryPermission.class, namespace).in(Repository.class, namespace + "/" + name)));
dtoToModelMapper.modify(existingPermission, permission);
// TODO RP
// RepositoryPermission existingPermission = repository.getPermissions()
// .stream()
// .filter(filterPermission(permissionName))
// .findFirst()
// .orElseThrow(() -> notFound(entity(RepositoryPermission.class, namespace).in(Repository.class, namespace + "/" + name)));
// dtoToModelMapper.modify(existingPermission, permission);
manager.modify(repository);
log.info("the permission with name: {} is updated.", permissionName);
return Response.noContent().build();
@@ -204,12 +208,13 @@ public class RepositoryPermissionRootResource {
log.info("try to delete the permission with name: {}.", permissionName);
Repository repository = load(namespace, name);
RepositoryPermissions.modify(repository).check();
repository.getPermissions()
.stream()
.filter(filterPermission(permissionName))
.findFirst()
.ifPresent(repository::removePermission)
;
// TODO RP
// repository.getPermissions()
// .stream()
// .filter(filterPermission(permissionName))
// .findFirst()
// .ifPresent(repository::removePermission)
// ;
manager.modify(repository);
log.info("the permission with name: {} is updated.", permissionName);
return Response.noContent().build();
@@ -261,9 +266,10 @@ public class RepositoryPermissionRootResource {
}
private boolean isPermissionExist(RepositoryPermissionDto permission, Repository repository) {
return repository.getPermissions()
.stream()
.anyMatch(p -> p.getName().equals(permission.getName()) && p.isGroupPermission() == permission.isGroupPermission());
return true;
// return repository.getPermissions()
// .stream()
// .anyMatch(p -> p.getName().equals(permission.getName()) && p.isGroupPermission() == permission.isGroupPermission());
}
}

View File

@@ -154,7 +154,6 @@ public class RepositoryResource {
private Repository processUpdate(RepositoryDto repositoryDto, Repository existing) {
Repository changedRepository = dtoToRepositoryMapper.map(repositoryDto, existing.getId());
changedRepository.setPermissions(existing.getPermissions());
return changedRepository;
}

View File

@@ -167,7 +167,9 @@ public class AuthorizationChangedEventProducer {
private boolean isAuthorizationDataModified(Repository repository, Repository beforeModification) {
return repository.isArchived() != beforeModification.isArchived()
|| repository.isPublicReadable() != beforeModification.isPublicReadable()
|| !(repository.getPermissions().containsAll(beforeModification.getPermissions()) && beforeModification.getPermissions().containsAll(repository.getPermissions()));
// TODO RP
// || !(repository.getPermissions().containsAll(beforeModification.getPermissions()) && beforeModification.getPermissions().containsAll(repository.getPermissions()))
;
}
private void fireEventForEveryUser() {

View File

@@ -63,6 +63,7 @@ import sonia.scm.user.UserPermissions;
import sonia.scm.util.Util;
import java.util.Collection;
import java.util.Collections;
import java.util.Set;
//~--- JDK imports ------------------------------------------------------------
@@ -198,8 +199,12 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector
private void collectRepositoryPermissions(Builder<String> builder,
Repository repository, User user, GroupNames groups)
{
// TODO RP
Collection<RepositoryPermission> repositoryPermissions
= repository.getPermissions();
= Collections.emptyList();
// = repository.getPermissions();
if (Util.isNotEmpty(repositoryPermissions))
{

View File

@@ -0,0 +1,6 @@
package sonia.scm.security;
public class RepositoryPermissionAssigner {
}

View File

@@ -412,7 +412,8 @@ public class RepositoryPermissionRootResourceTest extends RepositoryTestBase {
}
private void createUserWithRepositoryAndPermissions(ArrayList<RepositoryPermission> permissions, String userPermission) {
createUserWithRepository(userPermission).setPermissions(permissions);
// TODO RP
// createUserWithRepository(userPermission).setPermissions(permissions);
}
private Stream<DynamicTest> createDynamicTestsToAssertResponses(ExpectedRequest... expectedRequests) {

View File

@@ -291,34 +291,13 @@ public class RepositoryRootResourceTest extends RepositoryTestBase {
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");
existingRepository.setPermissions(singletonList(new RepositoryPermission("user", PermissionType.READ)));
URL url = Resources.getResource("sonia/scm/api/v2/repository-test-update.json");
byte[] repository = Resources.toByteArray(url);
ArgumentCaptor<Repository> 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());
// TODO RP
// Assertions.assertThat(createCaptor.getValue().getPermissions())
// .hasSize(1)
// .allSatisfy(p -> {
// assertThat(p.getName()).isEqualTo("trillian");
// assertThat(p.getType()).isEqualTo(PermissionType.OWNER);
// });
}
@Test

View File

@@ -238,7 +238,6 @@ public class RepositoryToRepositoryDtoMapperTest {
repository.setId("1");
repository.setCreationDate(System.currentTimeMillis());
repository.setHealthCheckFailures(singletonList(new HealthCheckFailure("1", "summary", "url", "failure")));
repository.setPermissions(singletonList(new RepositoryPermission("permission", PermissionType.READ)));
return repository;
}

View File

@@ -184,7 +184,7 @@ private long calculateAverage(List<Long> times) {
private Repository createTestRepository(int number) {
Repository repository = new Repository(keyGenerator.createKey(), REPOSITORY_TYPE, "namespace", "repo-" + number);
repository.addPermission(new RepositoryPermission("trillian", PermissionType.READ));
// repository.addPermission(new RepositoryPermission("trillian", PermissionType.READ));
return repository;
}

View File

@@ -172,42 +172,43 @@ public class AuthorizationChangedEventProducerTest {
@Test
public void testOnRepositoryModificationEvent()
{
Repository repositoryModified = RepositoryTestData.createHeartOfGold();
repositoryModified.setName("test123");
repositoryModified.setPermissions(Lists.newArrayList(new RepositoryPermission("test")));
Repository repository = RepositoryTestData.createHeartOfGold();
repository.setPermissions(Lists.newArrayList(new RepositoryPermission("test")));
producer.onEvent(new RepositoryModificationEvent(HandlerEventType.BEFORE_CREATE, repositoryModified, repository));
assertEventIsNotFired();
producer.onEvent(new RepositoryModificationEvent(HandlerEventType.CREATE, repositoryModified, repository));
assertEventIsNotFired();
repositoryModified.setPermissions(Lists.newArrayList(new RepositoryPermission("test")));
producer.onEvent(new RepositoryModificationEvent(HandlerEventType.CREATE, repositoryModified, repository));
assertEventIsNotFired();
repositoryModified.setPermissions(Lists.newArrayList(new RepositoryPermission("test123")));
producer.onEvent(new RepositoryModificationEvent(HandlerEventType.CREATE, repositoryModified, repository));
assertGlobalEventIsFired();
resetStoredEvent();
repositoryModified.setPermissions(
Lists.newArrayList(new RepositoryPermission("test", PermissionType.READ, true))
);
producer.onEvent(new RepositoryModificationEvent(HandlerEventType.CREATE, repositoryModified, repository));
assertGlobalEventIsFired();
resetStoredEvent();
repositoryModified.setPermissions(
Lists.newArrayList(new RepositoryPermission("test", PermissionType.WRITE))
);
producer.onEvent(new RepositoryModificationEvent(HandlerEventType.CREATE, repositoryModified, repository));
assertGlobalEventIsFired();
// TODO RP
// Repository repositoryModified = RepositoryTestData.createHeartOfGold();
// repositoryModified.setName("test123");
// repositoryModified.setPermissions(Lists.newArrayList(new RepositoryPermission("test")));
//
// Repository repository = RepositoryTestData.createHeartOfGold();
// repository.setPermissions(Lists.newArrayList(new RepositoryPermission("test")));
//
// producer.onEvent(new RepositoryModificationEvent(HandlerEventType.BEFORE_CREATE, repositoryModified, repository));
// assertEventIsNotFired();
//
// producer.onEvent(new RepositoryModificationEvent(HandlerEventType.CREATE, repositoryModified, repository));
// assertEventIsNotFired();
//
// repositoryModified.setPermissions(Lists.newArrayList(new RepositoryPermission("test")));
// producer.onEvent(new RepositoryModificationEvent(HandlerEventType.CREATE, repositoryModified, repository));
// assertEventIsNotFired();
//
// repositoryModified.setPermissions(Lists.newArrayList(new RepositoryPermission("test123")));
// producer.onEvent(new RepositoryModificationEvent(HandlerEventType.CREATE, repositoryModified, repository));
// assertGlobalEventIsFired();
//
// resetStoredEvent();
//
// repositoryModified.setPermissions(
// Lists.newArrayList(new RepositoryPermission("test", PermissionType.READ, true))
// );
// producer.onEvent(new RepositoryModificationEvent(HandlerEventType.CREATE, repositoryModified, repository));
// assertGlobalEventIsFired();
//
// resetStoredEvent();
//
// repositoryModified.setPermissions(
// Lists.newArrayList(new RepositoryPermission("test", PermissionType.WRITE))
// );
// producer.onEvent(new RepositoryModificationEvent(HandlerEventType.CREATE, repositoryModified, repository));
// assertGlobalEventIsFired();
}
private void resetStoredEvent(){

View File

@@ -225,11 +225,12 @@ public class DefaultAuthorizationCollectorTest {
authenticate(UserTestData.createTrillian(), group);
Repository heartOfGold = RepositoryTestData.createHeartOfGold();
heartOfGold.setId("one");
heartOfGold.setPermissions(Lists.newArrayList(new RepositoryPermission("trillian")));
// TODO RP
// heartOfGold.setPermissions(Lists.newArrayList(new RepositoryPermission("trillian")));
Repository puzzle42 = RepositoryTestData.create42Puzzle();
puzzle42.setId("two");
RepositoryPermission permission = new RepositoryPermission(group, PermissionType.WRITE, true);
puzzle42.setPermissions(Lists.newArrayList(permission));
// puzzle42.setPermissions(Lists.newArrayList(permission));
when(repositoryDAO.getAll()).thenReturn(Lists.newArrayList(heartOfGold, puzzle42));
// execute and assert