diff --git a/scm-it/src/test/java/sonia/scm/it/RoleITCase.java b/scm-it/src/test/java/sonia/scm/it/RoleITCase.java index 998c4e315e..331a251d23 100644 --- a/scm-it/src/test/java/sonia/scm/it/RoleITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/RoleITCase.java @@ -3,12 +3,14 @@ package sonia.scm.it; import org.apache.http.HttpStatus; import org.junit.Before; import org.junit.Test; -import sonia.scm.it.utils.RestUtil; +import sonia.scm.it.utils.ScmRequests; import sonia.scm.it.utils.TestData; import sonia.scm.web.VndMediaType; import static org.junit.Assert.assertNotNull; import static sonia.scm.it.PermissionsITCase.USER_PASS; +import static sonia.scm.it.utils.RestUtil.ADMIN_PASSWORD; +import static sonia.scm.it.utils.RestUtil.ADMIN_USERNAME; import static sonia.scm.it.utils.RestUtil.given; import static sonia.scm.it.utils.TestData.USER_SCM_ADMIN; import static sonia.scm.it.utils.TestData.callRepository; @@ -28,9 +30,13 @@ public class RoleITCase { public void userShouldSeePermissionsAfterAddingRoleToUser() { callRepository(USER, USER_PASS, "git", HttpStatus.SC_FORBIDDEN); + String repositoryRolesUrl = new ScmRequests() + .requestIndexResource(ADMIN_USERNAME, ADMIN_PASSWORD) + .getUrl("repositoryRoles"); + given() .when() - .delete(RestUtil.createResourceUrl("repository-roles/" + ROLE_NAME)) + .delete(repositoryRolesUrl + ROLE_NAME) .then() .statusCode(HttpStatus.SC_NO_CONTENT); @@ -40,7 +46,7 @@ public class RoleITCase { "\"name\": \"" + ROLE_NAME + "\"," + "\"verbs\": [\"read\",\"permissionRead\"]" + "}") - .post(RestUtil.createResourceUrl("repository-roles/")) + .post(repositoryRolesUrl) .then() .statusCode(HttpStatus.SC_CREATED); diff --git a/scm-it/src/test/java/sonia/scm/it/utils/ScmRequests.java b/scm-it/src/test/java/sonia/scm/it/utils/ScmRequests.java index 2164617772..69b9940f70 100644 --- a/scm-it/src/test/java/sonia/scm/it/utils/ScmRequests.java +++ b/scm-it/src/test/java/sonia/scm/it/utils/ScmRequests.java @@ -201,7 +201,12 @@ public class ScmRequests { return super.assertPropertyPathDoesNotExists(LINK_USERS); } - + public String getUrl(String linkName) { + return response + .then() + .extract() + .path("_links." + linkName + ".href"); + } } public class RepositoryResponse extends ModelResponse, PREV> { diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryRoleRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryRoleRootResource.java index f4adb02438..44e3a10fba 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryRoleRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryRoleRootResource.java @@ -10,7 +10,7 @@ import javax.ws.rs.Path; @Path(RepositoryRoleRootResource.REPOSITORY_ROLES_PATH_V2) public class RepositoryRoleRootResource { - static final String REPOSITORY_ROLES_PATH_V2 = "v2/repository-roles/"; + static final String REPOSITORY_ROLES_PATH_V2 = "v2/repositoryRoles/"; private final Provider repositoryRoleCollectionResource; private final Provider repositoryRoleResource; 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 82b3f8bd58..ab0ad16d0e 100644 --- a/scm-webapp/src/main/java/sonia/scm/repository/DefaultRepositoryRoleManager.java +++ b/scm-webapp/src/main/java/sonia/scm/repository/DefaultRepositoryRoleManager.java @@ -53,6 +53,7 @@ import java.util.Comparator; import java.util.List; import java.util.Optional; import java.util.function.Predicate; +import java.util.stream.Collectors; @Singleton @EagerSingleton public class DefaultRepositoryRoleManager extends AbstractRepositoryRoleManager @@ -156,38 +157,43 @@ public class DefaultRepositoryRoleManager extends AbstractRepositoryRoleManager } private Optional findSystemRole(String id) { - return repositoryPermissionProvider.availableRoles().stream().filter(role -> role.getName().equals(id)).findFirst(); + return repositoryPermissionProvider + .availableRoles() + .stream() + .filter(role -> !repositoryRoleDAO.getType().equals(role.getType())) + .filter(role -> role.getName().equals(id)).findFirst(); } @Override - public Collection getAll() { - return getAll(repositoryRole -> true, null); - } - - @Override - public Collection getAll(Predicate filter, Comparator comparator) { + public List getAll() { List repositoryRoles = new ArrayList<>(); if (!RepositoryRolePermissions.read().isPermitted()) { - return Collections.emptySet(); + return Collections.emptyList(); } for (RepositoryRole repositoryRole : repositoryPermissionProvider.availableRoles()) { repositoryRoles.add(repositoryRole.clone()); } - if (comparator != null) { - Collections.sort(repositoryRoles, comparator); - } - return repositoryRoles; } @Override - public Collection getAll(Comparator comaparator, int start, int limit) { - if (!RepositoryRolePermissions.read().isPermitted()) { - return Collections.emptySet(); + public Collection getAll(Predicate filter, Comparator comparator) { + List repositoryRoles = getAll(); + + List filteredRoles = repositoryRoles.stream().filter(filter::test).collect(Collectors.toList()); + + if (comparator != null) { + filteredRoles.sort(comparator); } - return Util.createSubCollection(repositoryRoleDAO.getAll(), comaparator, + + return filteredRoles; + } + + @Override + public Collection getAll(Comparator comaparator, int start, int limit) { + return Util.createSubCollection(getAll(), comaparator, (collection, item) -> { collection.add(item.clone()); }, start, limit); 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 27f343bc30..620fc484b1 100644 --- a/scm-webapp/src/main/resources/META-INF/scm/permissions.xml +++ b/scm-webapp/src/main/resources/META-INF/scm/permissions.xml @@ -66,5 +66,8 @@ configuration:read,write:* + + repositoryRole:read,write + diff --git a/scm-webapp/src/main/resources/locales/de/plugins.json b/scm-webapp/src/main/resources/locales/de/plugins.json index 41bb53de1e..0bdfa39b3e 100644 --- a/scm-webapp/src/main/resources/locales/de/plugins.json +++ b/scm-webapp/src/main/resources/locales/de/plugins.json @@ -60,6 +60,12 @@ } } }, + "repositoryRole": { + "read,write": { + "displayName": "Benutzerdefinierte Repository-Rollen-Berechtigungen verwalten", + "description": "Kann benutzerdefinierte Rollen und deren Berechtigungen erstellen, ändern und löschen" + } + }, "unknown": "Unbekannte Berechtigung" }, "verbs": { diff --git a/scm-webapp/src/main/resources/locales/en/plugins.json b/scm-webapp/src/main/resources/locales/en/plugins.json index 0ad62ddf5c..4255f519ca 100644 --- a/scm-webapp/src/main/resources/locales/en/plugins.json +++ b/scm-webapp/src/main/resources/locales/en/plugins.json @@ -60,6 +60,12 @@ } } }, + "repositoryRole": { + "read,write": { + "displayName": "Administer custom repository role permissions", + "description": "May create, modify and delete custom repository roles and their permissions" + } + }, "unknown": "Unknown permission" }, "verbs": { 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 3f2d62c600..0542626dd5 100644 --- a/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryRoleManagerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryRoleManagerTest.java @@ -5,7 +5,6 @@ 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.api.Assertions; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; @@ -14,19 +13,36 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; +import sonia.scm.NotFoundException; import sonia.scm.security.RepositoryPermissionProvider; +import java.util.Collection; +import java.util.Comparator; +import java.util.List; + +import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) class DefaultRepositoryRoleManagerTest { + private static final String CUSTOM_ROLE_NAME = "customRole"; + private static final String SYSTEM_ROLE_NAME = "systemRole"; + private static final RepositoryRole CUSTOM_ROLE = new RepositoryRole(CUSTOM_ROLE_NAME, singletonList("custom"), "xml"); + private static final RepositoryRole SYSTEM_ROLE = new RepositoryRole(SYSTEM_ROLE_NAME, singletonList("system"), "system"); + private final Subject subject = mock(Subject.class); private final ThreadState subjectThreadState = new SubjectThreadState(subject); @@ -51,6 +67,17 @@ class DefaultRepositoryRoleManagerTest { ThreadContext.bind(subject); } + @BeforeEach + void initDao() { + when(dao.getType()).thenReturn("xml"); + } + + @BeforeEach + void mockExistingRole() { + when(dao.get(CUSTOM_ROLE_NAME)).thenReturn(CUSTOM_ROLE); + when(permissionProvider.availableRoles()).thenReturn(asList(CUSTOM_ROLE, SYSTEM_ROLE)); + } + @AfterEach void cleanupContext() { ThreadContext.unbindSubject(); @@ -61,7 +88,8 @@ class DefaultRepositoryRoleManagerTest { @BeforeEach void authorizeUser() { - when(subject.isPermitted(any(String.class))).thenReturn(true); + when(subject.isPermitted("repositoryRole:read")).thenReturn(true); + when(subject.isPermitted("repositoryRole:modify")).thenReturn(true); } @Test @@ -69,6 +97,75 @@ class DefaultRepositoryRoleManagerTest { RepositoryRole role = manager.get("noSuchRole"); assertThat(role).isNull(); } + + @Test + void shouldReturnRole_forExistingRole() { + RepositoryRole role = manager.get(CUSTOM_ROLE_NAME); + assertThat(role).isNotNull(); + } + + @Test + void shouldCreateRole() { + RepositoryRole role = manager.create(new RepositoryRole("new", singletonList("custom"), null)); + assertThat(role.getType()).isEqualTo("xml"); + verify(dao).add(role); + } + + @Test + void shouldNotCreateRole_whenSystemRoleExists() { + assertThrows(UnauthorizedException.class, () -> manager.create(new RepositoryRole(SYSTEM_ROLE_NAME, singletonList("custom"), null))); + verify(dao, never()).add(any()); + } + + @Test + void shouldModifyRole() { + RepositoryRole role = new RepositoryRole(CUSTOM_ROLE_NAME, singletonList("changed"), null); + manager.modify(role); + verify(dao).modify(role); + } + + @Test + void shouldNotModifyRole_whenRoleDoesNotExists() { + assertThrows(NotFoundException.class, () -> manager.modify(new RepositoryRole("noSuchRole", singletonList("changed"), null))); + verify(dao, never()).modify(any()); + } + + @Test + void shouldNotModifyRole_whenSystemRoleExists() { + assertThrows(UnauthorizedException.class, () -> manager.modify(new RepositoryRole(SYSTEM_ROLE_NAME, singletonList("changed"), null))); + verify(dao, never()).modify(any()); + } + + @Test + void shouldReturnAllRoles() { + List allRoles = manager.getAll(); + assertThat(allRoles).containsExactly(CUSTOM_ROLE, SYSTEM_ROLE); + } + + @Test + void shouldReturnFilteredRoles() { + Collection allRoles = manager.getAll(role -> CUSTOM_ROLE_NAME.equals(role.getName()), null); + assertThat(allRoles).containsExactly(CUSTOM_ROLE); + } + + @Test + void shouldReturnOrderedFilteredRoles() { + Collection allRoles = + manager.getAll( + role -> true, + Comparator.comparing(RepositoryRole::getType)); + assertThat(allRoles).containsExactly(SYSTEM_ROLE, CUSTOM_ROLE); + } + + @Test + void shouldReturnPaginatedRoles() { + Collection allRoles = + manager.getAll( + Comparator.comparing(RepositoryRole::getType), + 1, 1 + ); + assertThat(allRoles).containsExactly(CUSTOM_ROLE); + } } @Nested @@ -80,8 +177,35 @@ class DefaultRepositoryRoleManagerTest { } @Test - void x() { - assertThrows(UnauthorizedException.class, () -> manager.get("noSuchRole")); + void shouldThrowException_forGet() { + assertThrows(UnauthorizedException.class, () -> manager.get("any")); + } + + @Test + void shouldThrowException_forCreate() { + assertThrows(UnauthorizedException.class, () -> manager.create(new RepositoryRole("new", singletonList("custom"), null))); + verify(dao, never()).add(any()); + } + + @Test + void shouldThrowException_forModify() { + assertThrows(UnauthorizedException.class, () -> manager.modify(new RepositoryRole(CUSTOM_ROLE_NAME, singletonList("custom"), null))); + verify(dao, never()).modify(any()); + } + + @Test + void shouldReturnEmptyList() { + assertThat(manager.getAll()).isEmpty(); + } + + @Test + void shouldReturnEmptyFilteredList() { + assertThat(manager.getAll(x -> true, null)).isEmpty(); + } + + @Test + void shouldReturnEmptyPaginatedList() { + assertThat(manager.getAll(1, 1)).isEmpty(); } } }