From aa26a9c0e3e13b4a8818a90ec33dd0a687e574e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 17 Jan 2019 15:29:21 +0100 Subject: [PATCH] Fix REST endpoint for user permissions --- .../java/sonia/scm/security/Permission.java | 2 +- .../main/java/sonia/scm/web/VndMediaType.java | 1 + .../PermissionCollectionToDtoMapper.java | 40 ++++++++++ .../api/v2/resources/PermissionListDto.java | 10 ++- .../scm/api/v2/resources/ResourceLinks.java | 20 +++++ .../v2/resources/UserPermissionResource.java | 79 +++++++++++++++++++ .../scm/api/v2/resources/UserResource.java | 60 +++----------- .../api/v2/resources/UserToUserDtoMapper.java | 4 + .../api/v2/resources/ResourceLinksMock.java | 1 + .../v2/resources/UserRootResourceTest.java | 58 ++++++++++++-- 10 files changed, 215 insertions(+), 60 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionCollectionToDtoMapper.java create mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserPermissionResource.java diff --git a/scm-core/src/main/java/sonia/scm/security/Permission.java b/scm-core/src/main/java/sonia/scm/security/Permission.java index ef6b350c09..1b7c34f740 100644 --- a/scm-core/src/main/java/sonia/scm/security/Permission.java +++ b/scm-core/src/main/java/sonia/scm/security/Permission.java @@ -6,7 +6,7 @@ import com.github.sdorra.ssp.StaticPermissions; @StaticPermissions( value = "permission", permissions = {}, - globalPermissions = {"list", "assign"} + globalPermissions = {"list", "read", "assign"} ) public interface Permission extends PermissionObject { } diff --git a/scm-core/src/main/java/sonia/scm/web/VndMediaType.java b/scm-core/src/main/java/sonia/scm/web/VndMediaType.java index 2a409482c8..8596bab754 100644 --- a/scm-core/src/main/java/sonia/scm/web/VndMediaType.java +++ b/scm-core/src/main/java/sonia/scm/web/VndMediaType.java @@ -41,6 +41,7 @@ public class VndMediaType { public static final String PASSWORD_CHANGE = PREFIX + "passwordChange" + SUFFIX; @SuppressWarnings("squid:S2068") public static final String PASSWORD_OVERWRITE = PREFIX + "passwordOverwrite" + SUFFIX; + public static final String PERMISSION_COLLECTION = PREFIX + "permissionCollection" + SUFFIX; public static final String MERGE_RESULT = PREFIX + "mergeResult" + SUFFIX; public static final String MERGE_COMMAND = PREFIX + "mergeCommand" + SUFFIX; diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionCollectionToDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionCollectionToDtoMapper.java new file mode 100644 index 0000000000..093b2930e9 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionCollectionToDtoMapper.java @@ -0,0 +1,40 @@ +package sonia.scm.api.v2.resources; + +import de.otto.edison.hal.Links; +import org.mapstruct.Context; +import sonia.scm.security.PermissionDescriptor; +import sonia.scm.security.PermissionPermissions; + +import javax.inject.Inject; +import java.util.Collection; + +import static de.otto.edison.hal.Link.link; +import static de.otto.edison.hal.Links.linkingTo; + +public class PermissionCollectionToDtoMapper { + + private final ResourceLinks resourceLinks; + + @Inject + public PermissionCollectionToDtoMapper(ResourceLinks resourceLinks) { + this.resourceLinks = resourceLinks; + } + + public PermissionListDto map(Collection permissions, @Context String userId) { + String[] permissionStrings = permissions + .stream() + .map(PermissionDescriptor::getValue) + .toArray(String[]::new); + PermissionListDto target = new PermissionListDto(permissionStrings); + + Links.Builder linksBuilder = linkingTo().self(resourceLinks.userPermissions().permissions(userId)); + + if (PermissionPermissions.assign().isPermitted()) { + linksBuilder.single(link("overwrite", resourceLinks.userPermissions().overwritePermissions(userId))); + } + + target.add(linksBuilder.build()); + + return target; + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionListDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionListDto.java index 807330dd97..23d57f4d8e 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionListDto.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionListDto.java @@ -1,5 +1,7 @@ package sonia.scm.api.v2.resources; +import de.otto.edison.hal.HalRepresentation; +import de.otto.edison.hal.Links; import lombok.AllArgsConstructor; import lombok.Getter; import lombok.NoArgsConstructor; @@ -9,7 +11,13 @@ import lombok.Setter; @Setter @AllArgsConstructor @NoArgsConstructor -public class PermissionListDto { +public class PermissionListDto extends HalRepresentation { private String[] permissions; + + @Override + @SuppressWarnings("squid:S1185") // We want to have this method available in this package + protected HalRepresentation add(Links links) { + return super.add(links); + } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java index 0b09f0c5d7..3f7536e2e8 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java @@ -96,6 +96,26 @@ class ResourceLinks { } } + UserPermissionLinks userPermissions() { + return new UserPermissionLinks(scmPathInfoStore.get()); + } + + static class UserPermissionLinks { + private final LinkBuilder userPermissionLinkBuilder; + + UserPermissionLinks(ScmPathInfo pathInfo) { + this.userPermissionLinkBuilder = new LinkBuilder(pathInfo, UserRootResource.class, UserResource.class, UserPermissionResource.class); + } + + public String permissions(String name) { + return userPermissionLinkBuilder.method("getUserResource").parameters(name).method("permissions").parameters().method("getPermissions").parameters().href(); + } + + public String overwritePermissions(String name) { + return userPermissionLinkBuilder.method("getUserResource").parameters(name).method("permissions").parameters().method("overwritePermissions").parameters().href(); + } + } + MeLinks me() { return new MeLinks(scmPathInfoStore.get(), this.user()); } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserPermissionResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserPermissionResource.java new file mode 100644 index 0000000000..9bd5396aa6 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserPermissionResource.java @@ -0,0 +1,79 @@ +package sonia.scm.api.v2.resources; + +import com.webcohesion.enunciate.metadata.rs.ResponseCode; +import com.webcohesion.enunciate.metadata.rs.StatusCodes; +import com.webcohesion.enunciate.metadata.rs.TypeHint; +import sonia.scm.security.PermissionAssigner; +import sonia.scm.security.PermissionDescriptor; +import sonia.scm.web.VndMediaType; + +import javax.inject.Inject; +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.PUT; +import javax.ws.rs.Path; +import javax.ws.rs.PathParam; +import javax.ws.rs.Produces; +import javax.ws.rs.core.Response; +import java.util.Arrays; +import java.util.Collection; +import java.util.stream.Collectors; + +public class UserPermissionResource { + + private final PermissionAssigner permissionAssigner; + private final PermissionCollectionToDtoMapper permissionCollectionToDtoMapper; + + @Inject + public UserPermissionResource(PermissionAssigner permissionAssigner, PermissionCollectionToDtoMapper permissionCollectionToDtoMapper) { + this.permissionAssigner = permissionAssigner; + this.permissionCollectionToDtoMapper = permissionCollectionToDtoMapper; + } + + /** + * Returns permissions for a user. + * + * @param id the id/name of the user + */ + @GET + @Path("") + @Produces(VndMediaType.USER) + @TypeHint(UserDto.class) + @StatusCodes({ + @ResponseCode(code = 200, condition = "success"), + @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"), + @ResponseCode(code = 403, condition = "not authorized, the current user has no privileges to read the user"), + @ResponseCode(code = 404, condition = "not found, no user with the specified id/name available"), + @ResponseCode(code = 500, condition = "internal server error") + }) + public Response getPermissions(@PathParam("id") String id) { + Collection permissions = permissionAssigner.readPermissionsForUser(id); + return Response.ok(permissionCollectionToDtoMapper.map(permissions, id)).build(); + } + + /** + * Sets permissions for a user. Overwrites all existing permissions. + * + * @param id id of the user to be modified + * @param newPermissions New list of permissions for the user + */ + @PUT + @Path("") + @Consumes(VndMediaType.PERMISSION_COLLECTION) + @StatusCodes({ + @ResponseCode(code = 204, condition = "update success"), + @ResponseCode(code = 400, condition = "Invalid body"), + @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"), + @ResponseCode(code = 403, condition = "not authorized, the current user does not have the correct privilege"), + @ResponseCode(code = 404, condition = "not found, no user with the specified id/name available"), + @ResponseCode(code = 500, condition = "internal server error") + }) + @TypeHint(TypeHint.NO_CONTENT.class) + public Response overwritePermissions(@PathParam("id") String id, PermissionListDto newPermissions) { + Collection permissionDescriptors = Arrays.stream(newPermissions.getPermissions()) + .map(PermissionDescriptor::new) + .collect(Collectors.toList()); + permissionAssigner.setPermissionsForUser(id, permissionDescriptors); + return Response.noContent().build(); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserResource.java index 479e4094fb..6e90b4e6ec 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserResource.java @@ -4,9 +4,6 @@ import com.webcohesion.enunciate.metadata.rs.ResponseCode; import com.webcohesion.enunciate.metadata.rs.StatusCodes; import com.webcohesion.enunciate.metadata.rs.TypeHint; import org.apache.shiro.authc.credential.PasswordService; -import sonia.scm.security.AssignedPermission; -import sonia.scm.security.PermissionDescriptor; -import sonia.scm.security.SecuritySystem; import sonia.scm.user.User; import sonia.scm.user.UserManager; import sonia.scm.web.VndMediaType; @@ -30,16 +27,20 @@ public class UserResource { private final IdResourceManagerAdapter adapter; private final UserManager userManager; private final PasswordService passwordService; - private final SecuritySystem securitySystem; + private final UserPermissionResource userPermissionResource; @Inject - public UserResource(UserDtoToUserMapper dtoToUserMapper, UserToUserDtoMapper userToDtoMapper, UserManager manager, PasswordService passwordService, SecuritySystem securitySystem) { + public UserResource( + UserDtoToUserMapper dtoToUserMapper, + UserToUserDtoMapper userToDtoMapper, + UserManager manager, + PasswordService passwordService, UserPermissionResource userPermissionResource) { this.dtoToUserMapper = dtoToUserMapper; this.userToDtoMapper = userToDtoMapper; this.adapter = new IdResourceManagerAdapter<>(manager, User.class); this.userManager = manager; this.passwordService = passwordService; - this.securitySystem = securitySystem; + this.userPermissionResource = userPermissionResource; } /** @@ -137,51 +138,8 @@ public class UserResource { return Response.noContent().build(); } - /** - * Returns permissions for a user. - * - * @param id the id/name of the user - */ - @GET @Path("permissions") - @Produces(VndMediaType.USER) - @TypeHint(UserDto.class) - @StatusCodes({ - @ResponseCode(code = 200, condition = "success"), - @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"), - @ResponseCode(code = 403, condition = "not authorized, the current user has no privileges to read the user"), - @ResponseCode(code = 404, condition = "not found, no user with the specified id/name available"), - @ResponseCode(code = 500, condition = "internal server error") - }) - public Response getPermissions(@PathParam("id") String id) { - String[] permissions = - securitySystem.getPermissions(p -> !p.isGroupPermission() && p.getName().equals(id)) - .stream() - .map(AssignedPermission::getPermission) - .map(PermissionDescriptor::getValue) - .toArray(String[]::new); - return Response.ok(new PermissionListDto(permissions)).build(); - } - - /** - * Sets permissions for a user. Overwrites all existing permissions. - * - * @param id id of the user to be modified - * @param newPermissions New list of permissions for the user - */ - @PUT - @Path("permissions") - @Consumes(VndMediaType.PASSWORD_OVERWRITE) - @StatusCodes({ - @ResponseCode(code = 204, condition = "update success"), - @ResponseCode(code = 400, condition = "Invalid body"), - @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"), - @ResponseCode(code = 403, condition = "not authorized, the current user does not have the correct privilege"), - @ResponseCode(code = 404, condition = "not found, no user with the specified id/name available"), - @ResponseCode(code = 500, condition = "internal server error") - }) - @TypeHint(TypeHint.NO_CONTENT.class) - public Response overwritePermissions(@PathParam("id") String id, PermissionListDto newPermissions) { - return Response.noContent().build(); + public UserPermissionResource permissions() { + return userPermissionResource; } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserToUserDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserToUserDtoMapper.java index 5874e5767a..3c7e9fd7f1 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserToUserDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserToUserDtoMapper.java @@ -5,6 +5,7 @@ import org.mapstruct.AfterMapping; import org.mapstruct.Mapper; import org.mapstruct.Mapping; import org.mapstruct.MappingTarget; +import sonia.scm.security.PermissionPermissions; import sonia.scm.user.User; import sonia.scm.user.UserManager; import sonia.scm.user.UserPermissions; @@ -42,6 +43,9 @@ public abstract class UserToUserDtoMapper extends BaseMapper { linksBuilder.single(link("password", resourceLinks.user().passwordChange(target.getName()))); } } + if (PermissionPermissions.read().isPermitted()) { + linksBuilder.single(link("permissions", resourceLinks.userPermissions().permissions(target.getName()))); + } appendLinks(new EdisonLinkAppender(linksBuilder), user); diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ResourceLinksMock.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ResourceLinksMock.java index 435d8b4673..96587271e3 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ResourceLinksMock.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ResourceLinksMock.java @@ -16,6 +16,7 @@ public class ResourceLinksMock { when(resourceLinks.user()).thenReturn(userLinks); when(resourceLinks.me()).thenReturn(new ResourceLinks.MeLinks(uriInfo,userLinks)); when(resourceLinks.userCollection()).thenReturn(new ResourceLinks.UserCollectionLinks(uriInfo)); + when(resourceLinks.userPermissions()).thenReturn(new ResourceLinks.UserPermissionLinks(uriInfo)); when(resourceLinks.autoComplete()).thenReturn(new ResourceLinks.AutoCompleteLinks(uriInfo)); when(resourceLinks.group()).thenReturn(new ResourceLinks.GroupLinks(uriInfo)); when(resourceLinks.groupCollection()).thenReturn(new ResourceLinks.GroupCollectionLinks(uriInfo)); diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java index 06376675df..88142e4d50 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java @@ -14,10 +14,12 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.Spy; import sonia.scm.ContextEntry; import sonia.scm.NotFoundException; import sonia.scm.PageResult; -import sonia.scm.security.SecuritySystem; +import sonia.scm.security.PermissionAssigner; +import sonia.scm.security.PermissionDescriptor; import sonia.scm.user.ChangePasswordNotAllowedException; import sonia.scm.user.User; import sonia.scm.user.UserManager; @@ -27,6 +29,7 @@ import javax.servlet.http.HttpServletResponse; import java.net.URI; import java.net.URISyntaxException; import java.net.URL; +import java.util.Collection; import static java.util.Collections.singletonList; import static org.junit.Assert.assertEquals; @@ -61,11 +64,13 @@ public class UserRootResourceTest { @Mock private UserManager userManager; @Mock - private SecuritySystem securitySystem; + private PermissionAssigner permissionAssigner; @InjectMocks private UserDtoToUserMapperImpl dtoToUserMapper; @InjectMocks private UserToUserDtoMapperImpl userToDtoMapper; + @InjectMocks + private PermissionCollectionToDtoMapper permissionCollectionToDtoMapper; private ArgumentCaptor userCaptor = ArgumentCaptor.forClass(User.class); private User originalUser; @@ -83,7 +88,8 @@ public class UserRootResourceTest { UserCollectionToDtoMapper userCollectionToDtoMapper = new UserCollectionToDtoMapper(userToDtoMapper, resourceLinks); UserCollectionResource userCollectionResource = new UserCollectionResource(userManager, dtoToUserMapper, userCollectionToDtoMapper, resourceLinks, passwordService); - UserResource userResource = new UserResource(dtoToUserMapper, userToDtoMapper, userManager, passwordService, securitySystem); + UserPermissionResource userPermissionResource = new UserPermissionResource(permissionAssigner, permissionCollectionToDtoMapper); + UserResource userResource = new UserResource(dtoToUserMapper, userToDtoMapper, userManager, passwordService, userPermissionResource); UserRootResource userRootResource = new UserRootResource(Providers.of(userCollectionResource), Providers.of(userResource)); @@ -333,8 +339,6 @@ public class UserRootResourceTest { dispatcher.invoke(request, response); - System.out.println(response.getContentAsString()); - assertEquals(HttpServletResponse.SC_OK, response.getStatus()); assertTrue(response.getContentAsString().contains("\"name\":\"Neo\"")); assertTrue(response.getContentAsString().contains("\"self\":{\"href\":\"/v2/users/?page=0")); @@ -351,8 +355,6 @@ public class UserRootResourceTest { dispatcher.invoke(request, response); - System.out.println(response.getContentAsString()); - assertEquals(HttpServletResponse.SC_OK, response.getStatus()); assertTrue(response.getContentAsString().contains("\"name\":\"Neo\"")); assertTrue(response.getContentAsString().contains("\"self\":{\"href\":\"/v2/users/?page=1")); @@ -362,6 +364,48 @@ public class UserRootResourceTest { assertTrue(response.getContentAsString().contains("\"last\":{\"href\":\"/v2/users/?page=2")); } + @Test + public void shouldGetPermissionLink() throws URISyntaxException { + MockHttpRequest request = MockHttpRequest.get("/" + UserRootResource.USERS_PATH_V2 + "Neo"); + MockHttpResponse response = new MockHttpResponse(); + + dispatcher.invoke(request, response); + + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + + assertTrue(response.getContentAsString().contains("\"permissions\":{")); + } + + @Test + public void shouldGetPermissions() throws URISyntaxException { + when(permissionAssigner.readPermissionsForUser("Neo")).thenReturn(singletonList(new PermissionDescriptor("something:*"))); + MockHttpRequest request = MockHttpRequest.get("/" + UserRootResource.USERS_PATH_V2 + "Neo/permissions"); + MockHttpResponse response = new MockHttpResponse(); + + dispatcher.invoke(request, response); + + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + + assertTrue(response.getContentAsString().contains("\"permissions\":[\"something:*\"]")); + } + + @Test + public void shouldSetPermissions() throws URISyntaxException { + MockHttpRequest request = MockHttpRequest + .put("/" + UserRootResource.USERS_PATH_V2 + "Neo/permissions") + .contentType(VndMediaType.PERMISSION_COLLECTION) + .content("{\"permissions\":[\"other:*\"]}".getBytes()); + MockHttpResponse response = new MockHttpResponse(); + ArgumentCaptor> captor = ArgumentCaptor.forClass(Collection.class); + doNothing().when(permissionAssigner).setPermissionsForUser(eq("Neo"), captor.capture()); + + dispatcher.invoke(request, response); + + assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatus()); + + assertEquals("other:*", captor.getValue().iterator().next().getValue()); + } + private PageResult createSingletonPageResult(int overallCount) { return new PageResult<>(singletonList(createDummyUser("Neo")), overallCount); }