Merged in bugfix/repo_permissions_without_admin (pull request #258)

Fix bug for repository owners without global role permission
This commit is contained in:
Sebastian Sdorra
2019-06-03 13:56:11 +00:00
7 changed files with 31 additions and 26 deletions

View File

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

View File

@@ -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()));
}
} else {
builder.single(link("login", resourceLinks.authentication().jsonLogin()));
}

View File

@@ -25,7 +25,7 @@ public class RepositoryRoleCollectionToDtoMapper extends BasicCollectionToDtoMap
}
Optional<String> createCreateLink() {
return RepositoryRolePermissions.modify().isPermitted() ? of(resourceLinks.repositoryRoleCollection().create()): empty();
return RepositoryRolePermissions.write().isPermitted() ? of(resourceLinks.repositoryRoleCollection().create()): empty();
}
String createSelfLink() {

View File

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

View File

@@ -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<RepositoryRole> getAll() {
List<RepositoryRole> repositoryRoles = new ArrayList<>();
if (!RepositoryRolePermissions.read().isPermitted()) {
return Collections.emptyList();
}
for (RepositoryRole repositoryRole : repositoryPermissionProvider.availableRoles()) {
repositoryRoles.add(repositoryRole.clone());
}

View File

@@ -67,7 +67,7 @@
<value>configuration:read,write:*</value>
</permission>
<permission>
<value>repositoryRole:read,write</value>
<value>repositoryRole:write</value>
</permission>
</permissions>

View File

@@ -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<RepositoryRole> allRoles = manager.getAll();
assertThat(allRoles).containsExactly(CUSTOM_ROLE, SYSTEM_ROLE);
}
@Test
void shouldReturnEmptyFilteredList() {
assertThat(manager.getAll(x -> true, null)).isEmpty();
void shouldReturnFilteredList() {
Collection<RepositoryRole> 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<RepositoryRole> allRoles =
manager.getAll(
Comparator.comparing(RepositoryRole::getType),
1, 1
);
assertThat(allRoles).containsExactly(CUSTOM_ROLE);
}
}
}