diff --git a/scm-core/src/main/java/sonia/scm/repository/RepositoryRole.java b/scm-core/src/main/java/sonia/scm/repository/RepositoryRole.java index 1f8f638cb8..e5ca046aac 100644 --- a/scm-core/src/main/java/sonia/scm/repository/RepositoryRole.java +++ b/scm-core/src/main/java/sonia/scm/repository/RepositoryRole.java @@ -57,7 +57,7 @@ import static java.util.Collections.unmodifiableSet; * Custom role with specific permissions related to {@link Repository}. * This object should be immutable, but could not be due to mapstruct. */ -@StaticPermissions(value = "repositoryRole", permissions = {}, globalPermissions = {"read", "modify"}) +@StaticPermissions(value = "repositoryRole", permissions = {}, globalPermissions = {"write"}) @XmlRootElement(name = "roles") @XmlAccessorType(XmlAccessType.FIELD) public class RepositoryRole implements ModelObject, PermissionObject { diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/IndexDtoGenerator.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/IndexDtoGenerator.java index 634de9381c..00982a1609 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/IndexDtoGenerator.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/IndexDtoGenerator.java @@ -63,9 +63,7 @@ public class IndexDtoGenerator extends HalAppenderMapper { builder.single(link("repositoryTypes", resourceLinks.repositoryTypeCollection().self())); builder.single(link("namespaceStrategies", resourceLinks.namespaceStrategies().self())); - if (RepositoryRolePermissions.read().isPermitted()) { - builder.single(link("repositoryRoles", resourceLinks.repositoryRoleCollection().self())); - } + builder.single(link("repositoryRoles", resourceLinks.repositoryRoleCollection().self())); } else { builder.single(link("login", resourceLinks.authentication().jsonLogin())); } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryRoleCollectionToDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryRoleCollectionToDtoMapper.java index ab34efd7fe..82af9a5cda 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryRoleCollectionToDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryRoleCollectionToDtoMapper.java @@ -25,7 +25,7 @@ public class RepositoryRoleCollectionToDtoMapper extends BasicCollectionToDtoMap } Optional createCreateLink() { - return RepositoryRolePermissions.modify().isPermitted() ? of(resourceLinks.repositoryRoleCollection().create()): empty(); + return RepositoryRolePermissions.write().isPermitted() ? of(resourceLinks.repositoryRoleCollection().create()): empty(); } String createSelfLink() { diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryRoleToRepositoryRoleDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryRoleToRepositoryRoleDtoMapper.java index 43d364c1b3..a0cc8bae69 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryRoleToRepositoryRoleDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryRoleToRepositoryRoleDtoMapper.java @@ -27,7 +27,7 @@ public abstract class RepositoryRoleToRepositoryRoleDtoMapper extends BaseMapper @ObjectFactory RepositoryRoleDto createDto(RepositoryRole repositoryRole) { Links.Builder linksBuilder = linkingTo().self(resourceLinks.repositoryRole().self(repositoryRole.getName())); - if (!"system".equals(repositoryRole.getType()) && RepositoryRolePermissions.modify().isPermitted()) { + if (!"system".equals(repositoryRole.getType()) && RepositoryRolePermissions.write().isPermitted()) { linksBuilder.single(link("delete", resourceLinks.repositoryRole().delete(repositoryRole.getName()))); linksBuilder.single(link("update", resourceLinks.repositoryRole().update(repositoryRole.getName()))); } diff --git a/scm-webapp/src/main/java/sonia/scm/repository/DefaultRepositoryRoleManager.java b/scm-webapp/src/main/java/sonia/scm/repository/DefaultRepositoryRoleManager.java index ab0ad16d0e..b725fb614e 100644 --- a/scm-webapp/src/main/java/sonia/scm/repository/DefaultRepositoryRoleManager.java +++ b/scm-webapp/src/main/java/sonia/scm/repository/DefaultRepositoryRoleManager.java @@ -88,7 +88,7 @@ public class DefaultRepositoryRoleManager extends AbstractRepositoryRoleManager return managerDaoAdapter.create( repositoryRole, - RepositoryRolePermissions::modify, + RepositoryRolePermissions::write, newRepositoryRole -> fireEvent(HandlerEventType.BEFORE_CREATE, newRepositoryRole), newRepositoryRole -> fireEvent(HandlerEventType.CREATE, newRepositoryRole) ); @@ -100,7 +100,7 @@ public class DefaultRepositoryRoleManager extends AbstractRepositoryRoleManager logger.info("delete repositoryRole {} of type {}", repositoryRole.getName(), repositoryRole.getType()); managerDaoAdapter.delete( repositoryRole, - RepositoryRolePermissions::modify, + RepositoryRolePermissions::write, toDelete -> fireEvent(HandlerEventType.BEFORE_DELETE, toDelete), toDelete -> fireEvent(HandlerEventType.DELETE, toDelete) ); @@ -116,7 +116,7 @@ public class DefaultRepositoryRoleManager extends AbstractRepositoryRoleManager logger.info("modify repositoryRole {} of type {}", repositoryRole.getName(), repositoryRole.getType()); managerDaoAdapter.modify( repositoryRole, - x -> RepositoryRolePermissions.modify(), + x -> RepositoryRolePermissions.write(), notModified -> fireEvent(HandlerEventType.BEFORE_MODIFY, repositoryRole, notModified), notModified -> fireEvent(HandlerEventType.MODIFY, repositoryRole, notModified)); } @@ -125,7 +125,6 @@ public class DefaultRepositoryRoleManager extends AbstractRepositoryRoleManager public void refresh(RepositoryRole repositoryRole) { logger.info("refresh repositoryRole {} of type {}", repositoryRole.getName(), repositoryRole.getType()); - RepositoryRolePermissions.read().check(); RepositoryRole fresh = repositoryRoleDAO.get(repositoryRole.getName()); if (fresh == null) { @@ -135,8 +134,6 @@ public class DefaultRepositoryRoleManager extends AbstractRepositoryRoleManager @Override public RepositoryRole get(String id) { - RepositoryRolePermissions.read().check(); - return findSystemRole(id).orElse(findCustomRole(id)); } @@ -168,9 +165,6 @@ public class DefaultRepositoryRoleManager extends AbstractRepositoryRoleManager public List getAll() { List repositoryRoles = new ArrayList<>(); - if (!RepositoryRolePermissions.read().isPermitted()) { - return Collections.emptyList(); - } for (RepositoryRole repositoryRole : repositoryPermissionProvider.availableRoles()) { repositoryRoles.add(repositoryRole.clone()); } diff --git a/scm-webapp/src/main/resources/META-INF/scm/permissions.xml b/scm-webapp/src/main/resources/META-INF/scm/permissions.xml index 620fc484b1..3598b178bf 100644 --- a/scm-webapp/src/main/resources/META-INF/scm/permissions.xml +++ b/scm-webapp/src/main/resources/META-INF/scm/permissions.xml @@ -67,7 +67,7 @@ configuration:read,write:* - repositoryRole:read,write + repositoryRole:write diff --git a/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryRoleManagerTest.java b/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryRoleManagerTest.java index cd0ba963b9..9c6ab10041 100644 --- a/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryRoleManagerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryRoleManagerTest.java @@ -89,8 +89,7 @@ class DefaultRepositoryRoleManagerTest { @BeforeEach void authorizeUser() { - when(subject.isPermitted("repositoryRole:read")).thenReturn(true); - when(subject.isPermitted("repositoryRole:modify")).thenReturn(true); + when(subject.isPermitted("repositoryRole:write")).thenReturn(true); } @Test @@ -184,8 +183,15 @@ class DefaultRepositoryRoleManagerTest { } @Test - void shouldThrowException_forGet() { - assertThrows(UnauthorizedException.class, () -> manager.get("any")); + void shouldReturnNull_forNotExistingRole() { + RepositoryRole role = manager.get("noSuchRole"); + assertThat(role).isNull(); + } + + @Test + void shouldReturnRole_forExistingRole() { + RepositoryRole role = manager.get(CUSTOM_ROLE_NAME); + assertThat(role).isNotNull(); } @Test @@ -201,18 +207,25 @@ class DefaultRepositoryRoleManagerTest { } @Test - void shouldReturnEmptyList() { - assertThat(manager.getAll()).isEmpty(); + void shouldReturnAllRoles() { + List allRoles = manager.getAll(); + assertThat(allRoles).containsExactly(CUSTOM_ROLE, SYSTEM_ROLE); } @Test - void shouldReturnEmptyFilteredList() { - assertThat(manager.getAll(x -> true, null)).isEmpty(); + void shouldReturnFilteredList() { + Collection allRoles = manager.getAll(role -> CUSTOM_ROLE_NAME.equals(role.getName()), null); + assertThat(allRoles).containsExactly(CUSTOM_ROLE); } @Test - void shouldReturnEmptyPaginatedList() { - assertThat(manager.getAll(1, 1)).isEmpty(); + void shouldReturnPaginatedRoles() { + Collection allRoles = + manager.getAll( + Comparator.comparing(RepositoryRole::getType), + 1, 1 + ); + assertThat(allRoles).containsExactly(CUSTOM_ROLE); } } }