diff --git a/scm-core/src/main/java/sonia/scm/AlreadyExistsException.java b/scm-core/src/main/java/sonia/scm/AlreadyExistsException.java index 36052d99e9..4c6a52b878 100644 --- a/scm-core/src/main/java/sonia/scm/AlreadyExistsException.java +++ b/scm-core/src/main/java/sonia/scm/AlreadyExistsException.java @@ -1,4 +1,11 @@ package sonia.scm; public class AlreadyExistsException extends Exception { + + public AlreadyExistsException(String message) { + super(message); + } + + public AlreadyExistsException() { + } } diff --git a/scm-core/src/main/java/sonia/scm/NotFoundException.java b/scm-core/src/main/java/sonia/scm/NotFoundException.java index 8a7ae642bd..0d8c14c61b 100644 --- a/scm-core/src/main/java/sonia/scm/NotFoundException.java +++ b/scm-core/src/main/java/sonia/scm/NotFoundException.java @@ -7,4 +7,9 @@ public class NotFoundException extends Exception { public NotFoundException() { } + + + public NotFoundException(String message) { + super(message); + } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/rest/AlreadyExistsExceptionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/rest/AlreadyExistsExceptionMapper.java index 28240482e5..13d341e1e6 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/rest/AlreadyExistsExceptionMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/rest/AlreadyExistsExceptionMapper.java @@ -11,6 +11,8 @@ import javax.ws.rs.ext.Provider; public class AlreadyExistsExceptionMapper implements ExceptionMapper { @Override public Response toResponse(AlreadyExistsException exception) { - return Response.status(Status.CONFLICT).build(); + return Response.status(Status.CONFLICT) + .entity(exception.getMessage()) + .build(); } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/rest/StatusExceptionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/rest/StatusExceptionMapper.java index fef7fc96c4..6f4978637e 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/rest/StatusExceptionMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/rest/StatusExceptionMapper.java @@ -90,6 +90,8 @@ public class StatusExceptionMapper logger.debug(msg.toString()); } - return Response.status(status).build(); + return Response.status(status) + .entity(exception.getMessage()) + .build(); } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionDto.java index b184bc3934..7fa74e9f87 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionDto.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionDto.java @@ -26,6 +26,14 @@ public class PermissionDto extends HalRepresentation { private boolean groupPermission = false; + public PermissionDto() { + } + + public PermissionDto(String permissionName, boolean groupPermission) { + name = permissionName; + this.groupPermission = groupPermission; + } + @Override @SuppressWarnings("squid:S1185") // We want to have this method available in this package diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java index b3a65852a0..16cdf17915 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java @@ -136,6 +136,7 @@ public class PermissionRootResource { /** * Update a permission to the user or group managed by the repository + * ignore the user input for groupPermission and take it from the path parameter (if the group prefix (@) exists it is a group permission) * * @param permission permission to modify * @param permissionName permission to modify @@ -153,10 +154,18 @@ public class PermissionRootResource { public Response update(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("permission-name") String permissionName, - PermissionDto permission) throws NotFoundException { + PermissionDto permission) throws NotFoundException, AlreadyExistsException { log.info("try to update the permission with name: {}. the modified permission is: {}", permissionName, permission); Repository repository = load(namespace, name); RepositoryPermissions.permissionWrite(repository).check(); + String extractedPermissionName = getPermissionName(permissionName); + if (!isPermissionExist(new PermissionDto(extractedPermissionName, isGroupPermission(permissionName)), repository)) { + throw new NotFoundException("the permission " + extractedPermissionName + " does not exist"); + } + permission.setGroupPermission(isGroupPermission(permissionName)); + if (!extractedPermissionName.equals(permission.getName())) { + checkPermissionAlreadyExists(permission, repository, "target permission " + permission.getName() + " already exists"); + } Permission existingPermission = repository.getPermissions() .stream() .filter(filterPermission(permissionName)) @@ -201,13 +210,19 @@ public class PermissionRootResource { } Predicate filterPermission(String permissionName) { - boolean isGroupPermission = permissionName.startsWith(GROUP_PREFIX); - return permission -> Optional.of(permissionName) - .filter(p -> !isGroupPermission) - .orElse(permissionName.substring(1)) - .equals(permission.getName()) + return permission -> getPermissionName(permissionName).equals(permission.getName()) && - permission.isGroupPermission() == isGroupPermission; + permission.isGroupPermission() == isGroupPermission(permissionName); + } + + private String getPermissionName(String permissionName) { + return Optional.of(permissionName) + .filter(p -> !isGroupPermission(permissionName)) + .orElse(permissionName.substring(1)); + } + + private boolean isGroupPermission(String permissionName) { + return permissionName.startsWith(GROUP_PREFIX); } @@ -230,15 +245,23 @@ public class PermissionRootResource { * * @param permission the searched permission * @param repository the repository to be inspected + * @param errorMessage error message * @throws AlreadyExistsException if the permission already exists in the repository */ - private void checkPermissionAlreadyExists(PermissionDto permission, Repository repository) throws AlreadyExistsException { - boolean isPermissionAlreadyExist = repository.getPermissions() + private void checkPermissionAlreadyExists(PermissionDto permission, Repository repository, String errorMessage) throws AlreadyExistsException { + if (isPermissionExist(permission, repository)) { + throw new AlreadyExistsException(errorMessage); + } + } + + private boolean isPermissionExist(PermissionDto permission, Repository repository) { + return repository.getPermissions() .stream() .anyMatch(p -> p.getName().equals(permission.getName()) && p.isGroupPermission() == permission.isGroupPermission()); - if (isPermissionAlreadyExist) { - throw new AlreadyExistsException(); - } + } + + private void checkPermissionAlreadyExists(PermissionDto permission, Repository repository) throws AlreadyExistsException { + checkPermissionAlreadyExists(permission, repository, "the permission " + permission.getName() + " already exist."); } }