merged with upstream

This commit is contained in:
Florian Scholdei
2019-05-08 13:17:48 +02:00
8 changed files with 182 additions and 26 deletions

View File

@@ -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);

View File

@@ -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<PREV extends ModelResponse> extends ModelResponse<RepositoryResponse<PREV>, PREV> {

View File

@@ -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> repositoryRoleCollectionResource;
private final Provider<RepositoryRoleResource> repositoryRoleResource;

View File

@@ -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<RepositoryRole> 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<RepositoryRole> getAll() {
return getAll(repositoryRole -> true, null);
}
@Override
public Collection<RepositoryRole> getAll(Predicate<RepositoryRole> filter, Comparator<RepositoryRole> comparator) {
public List<RepositoryRole> getAll() {
List<RepositoryRole> 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<RepositoryRole> getAll(Comparator<RepositoryRole> comaparator, int start, int limit) {
if (!RepositoryRolePermissions.read().isPermitted()) {
return Collections.emptySet();
public Collection<RepositoryRole> getAll(Predicate<RepositoryRole> filter, Comparator<RepositoryRole> comparator) {
List<RepositoryRole> repositoryRoles = getAll();
List<RepositoryRole> 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<RepositoryRole> getAll(Comparator<RepositoryRole> comaparator, int start, int limit) {
return Util.createSubCollection(getAll(), comaparator,
(collection, item) -> {
collection.add(item.clone());
}, start, limit);

View File

@@ -66,5 +66,8 @@
<permission>
<value>configuration:read,write:*</value>
</permission>
<permission>
<value>repositoryRole:read,write</value>
</permission>
</permissions>

View File

@@ -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": {

View File

@@ -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": {

View File

@@ -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<RepositoryRole> allRoles = manager.getAll();
assertThat(allRoles).containsExactly(CUSTOM_ROLE, SYSTEM_ROLE);
}
@Test
void shouldReturnFilteredRoles() {
Collection<RepositoryRole> allRoles = manager.getAll(role -> CUSTOM_ROLE_NAME.equals(role.getName()), null);
assertThat(allRoles).containsExactly(CUSTOM_ROLE);
}
@Test
void shouldReturnOrderedFilteredRoles() {
Collection<RepositoryRole> allRoles =
manager.getAll(
role -> true,
Comparator.comparing(RepositoryRole::getType));
assertThat(allRoles).containsExactly(SYSTEM_ROLE, CUSTOM_ROLE);
}
@Test
void shouldReturnPaginatedRoles() {
Collection<RepositoryRole> 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();
}
}
}