diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/api/v2/resources/GitConfigResource.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/api/v2/resources/GitConfigResource.java index 1db28dfb5d..ddd6bf3823 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/api/v2/resources/GitConfigResource.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/api/v2/resources/GitConfigResource.java @@ -3,18 +3,24 @@ 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 org.apache.shiro.SecurityUtils; +import sonia.scm.config.ConfigurationPermissions; import sonia.scm.repository.GitConfig; import sonia.scm.repository.GitRepositoryHandler; -import sonia.scm.security.Role; import sonia.scm.web.GitVndMediaType; import javax.inject.Inject; -import javax.ws.rs.*; +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.PUT; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; import javax.ws.rs.core.Context; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; +/** + * RESTful Web Service Resource to manage the configuration of the git plugin. + */ @Path(GitConfigResource.GIT_CONFIG_PATH_V2) public class GitConfigResource { @@ -44,22 +50,17 @@ public class GitConfigResource { @ResponseCode(code = 500, condition = "internal server error") }) public Response get() { - Response response; - if (SecurityUtils.getSubject().hasRole(Role.ADMIN)) { - GitConfig config = repositoryHandler.getConfig(); + GitConfig config = repositoryHandler.getConfig(); - if (config == null) { - config = new GitConfig(); - repositoryHandler.setConfig(config); - } - - response = Response.ok(configToDtoMapper.map(config)).build(); - } else { - response = Response.status(Response.Status.FORBIDDEN).build(); + if (config == null) { + config = new GitConfig(); + repositoryHandler.setConfig(config); } - return response; + ConfigurationPermissions.read(config).check(); + + return Response.ok(configToDtoMapper.map(config)).build(); } /** @@ -71,23 +72,21 @@ public class GitConfigResource { @Path("") @Consumes(GitVndMediaType.GIT_CONFIG) @StatusCodes({ - @ResponseCode(code = 201, condition = "update success"), + @ResponseCode(code = 204, condition = "update success"), @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"), @ResponseCode(code = 403, condition = "not authorized, the current user has no privileges to update the git config"), @ResponseCode(code = 500, condition = "internal server error") }) @TypeHint(TypeHint.NO_CONTENT.class) public Response update(@Context UriInfo uriInfo, GitConfigDto configDto) { - Response response; - if (SecurityUtils.getSubject().hasRole(Role.ADMIN)) { - repositoryHandler.setConfig(dtoToConfigMapper.map(configDto)); - repositoryHandler.storeConfig(); - response = Response.created(uriInfo.getRequestUri()).build(); - } else { - response = Response.status(Response.Status.FORBIDDEN).build(); - } + GitConfig config = dtoToConfigMapper.map(configDto); - return response; + ConfigurationPermissions.write(config).check(); + + repositoryHandler.setConfig(config); + repositoryHandler.storeConfig(); + + return Response.noContent().build(); } } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/api/v2/resources/GitConfigToGitConfigDtoMapper.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/api/v2/resources/GitConfigToGitConfigDtoMapper.java index ceab24d6f7..b929d6522b 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/api/v2/resources/GitConfigToGitConfigDtoMapper.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/api/v2/resources/GitConfigToGitConfigDtoMapper.java @@ -1,12 +1,11 @@ package sonia.scm.api.v2.resources; import de.otto.edison.hal.Links; -import org.apache.shiro.SecurityUtils; import org.mapstruct.AfterMapping; import org.mapstruct.Mapper; import org.mapstruct.MappingTarget; +import sonia.scm.config.ConfigurationPermissions; import sonia.scm.repository.GitConfig; -import sonia.scm.security.Role; import javax.inject.Inject; @@ -26,8 +25,7 @@ public abstract class GitConfigToGitConfigDtoMapper { @AfterMapping void appendLinks(GitConfig config, @MappingTarget GitConfigDto target) { Links.Builder linksBuilder = linkingTo().self(self()); - // TODO: ConfigPermissions? - if (SecurityUtils.getSubject().hasRole(Role.ADMIN)) { + if (ConfigurationPermissions.write(config).isPermitted()) { linksBuilder.single(link("update", update())); } target.add(linksBuilder.build()); diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/api/v2/resources/GitConfigDtoToGitConfigMapperTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/api/v2/resources/GitConfigDtoToGitConfigMapperTest.java index 3b09dff51f..968b6b7f6f 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/api/v2/resources/GitConfigDtoToGitConfigMapperTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/api/v2/resources/GitConfigDtoToGitConfigMapperTest.java @@ -9,6 +9,7 @@ import sonia.scm.repository.GitConfig; import java.io.File; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; @RunWith(MockitoJUnitRunner.class) public class GitConfigDtoToGitConfigMapperTest { @@ -22,7 +23,7 @@ public class GitConfigDtoToGitConfigMapperTest { GitConfig config = mapper.map(dto); assertEquals("express", config.getGcExpression()); assertEquals("repository/directory", config.getRepositoryDirectory().getPath()); - assertEquals(false, config.isDisabled()); + assertFalse(config.isDisabled()); } private GitConfigDto createDefaultDto() { diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/api/v2/resources/GitConfigResourceTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/api/v2/resources/GitConfigResourceTest.java index 7535e43db5..2389f4abd7 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/api/v2/resources/GitConfigResourceTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/api/v2/resources/GitConfigResourceTest.java @@ -11,6 +11,7 @@ import org.jboss.resteasy.mock.MockHttpResponse; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.mockito.Answers; import org.mockito.InjectMocks; @@ -18,6 +19,7 @@ import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; import sonia.scm.repository.GitConfig; import sonia.scm.repository.GitRepositoryHandler; +import sonia.scm.web.GitVndMediaType; import javax.servlet.http.HttpServletResponse; import java.io.File; @@ -27,12 +29,12 @@ import java.net.URISyntaxException; import static junit.framework.TestCase.assertTrue; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.mockito.Mockito.when; @SubjectAware( - username = "trillian", - password = "secret", - configuration = "classpath:sonia/scm/repository/shiro.ini" + configuration = "classpath:sonia/scm/configuration/shiro.ini", + password = "secret" ) @RunWith(MockitoJUnitRunner.class) public class GitConfigResourceTest { @@ -40,6 +42,9 @@ public class GitConfigResourceTest { @Rule public ShiroRule shiro = new ShiroRule(); + @Rule + public ExpectedException thrown = ExpectedException.none(); + private Dispatcher dispatcher = MockDispatcherFactory.createDispatcher(); private final URI baseUri = URI.create("/"); @@ -66,10 +71,10 @@ public class GitConfigResourceTest { } @Test + @SubjectAware(username = "readWrite") public void shouldGetGitConfig() throws URISyntaxException, IOException { - MockHttpRequest request = MockHttpRequest.get("/" + GitConfigResource.GIT_CONFIG_PATH_V2); - MockHttpResponse response = new MockHttpResponse(); - dispatcher.invoke(request, response); + MockHttpResponse response = get(); + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); String responseString = response.getContentAsString(); @@ -82,7 +87,55 @@ public class GitConfigResourceTest { assertTrue(responseString.contains("\"update\":{\"href\":\"/v2/config/git")); } - // TODO update & negative tests + @Test + @SubjectAware(username = "readOnly") + public void shouldGetGitConfigWithoutUpdateLink() throws URISyntaxException { + MockHttpResponse response = get(); + + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + + assertFalse(response.getContentAsString().contains("\"update\":{\"href\":\"/v2/config/git")); + } + + @Test + @SubjectAware(username = "writeOnly") + public void shouldGetConfigOnlyWhenAuthorized() throws URISyntaxException { + thrown.expectMessage("Subject does not have permission [configuration:read:git]"); + + get(); + } + + @Test + @SubjectAware(username = "writeOnly") + public void shouldUpdateConfig() throws URISyntaxException { + MockHttpResponse response = put(); + assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatus()); + } + + @Test + @SubjectAware(username = "readOnly") + public void shouldUpdateConfigOnlyWhenAuthorized() throws URISyntaxException, IOException { + thrown.expectMessage("Subject does not have permission [configuration:write:git]"); + + put(); + } + + private MockHttpResponse get() throws URISyntaxException { + MockHttpRequest request = MockHttpRequest.get("/" + GitConfigResource.GIT_CONFIG_PATH_V2); + MockHttpResponse response = new MockHttpResponse(); + dispatcher.invoke(request, response); + return response; + } + + private MockHttpResponse put() throws URISyntaxException { + MockHttpRequest request = MockHttpRequest.put("/" + GitConfigResource.GIT_CONFIG_PATH_V2) + .contentType(GitVndMediaType.GIT_CONFIG) + .content("{\"disabled\":true}".getBytes()); + + MockHttpResponse response = new MockHttpResponse(); + dispatcher.invoke(request, response); + return response; + } private GitConfig createConfiguration() { GitConfig config = new GitConfig(); diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/api/v2/resources/GitConfigToGitConfigDtoMapperTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/api/v2/resources/GitConfigToGitConfigDtoMapperTest.java index 59f7cc4799..51fded4839 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/api/v2/resources/GitConfigToGitConfigDtoMapperTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/api/v2/resources/GitConfigToGitConfigDtoMapperTest.java @@ -13,7 +13,6 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; import sonia.scm.repository.GitConfig; -import sonia.scm.security.Role; import java.io.File; import java.net.URI; @@ -56,7 +55,7 @@ public class GitConfigToGitConfigDtoMapperTest { public void shouldMapFields() { GitConfig config = createConfiguration(); - when(subject.hasRole(Role.ADMIN)).thenReturn(true); + when(subject.isPermitted("configuration:write:git")).thenReturn(true); GitConfigDto dto = mapper.map(config); assertEquals("express", dto.getGcExpression()); @@ -66,6 +65,17 @@ public class GitConfigToGitConfigDtoMapperTest { assertEquals(expectedBaseUri.toString(), dto.getLinks().getLinkBy("update").get().getHref()); } + @Test + public void shouldMapFieldsWithoutUpdate() { + GitConfig config = createConfiguration(); + + when(subject.isPermitted("configuration:write:git")).thenReturn(false); + GitConfigDto dto = mapper.map(config); + + assertEquals(expectedBaseUri.toString(), dto.getLinks().getLinkBy("self").get().getHref()); + assertFalse(dto.getLinks().hasLink("update")); + } + private GitConfig createConfiguration() { GitConfig config = new GitConfig(); config.setDisabled(false); diff --git a/scm-plugins/scm-git-plugin/src/test/resources/sonia/scm/configuration/shiro.ini b/scm-plugins/scm-git-plugin/src/test/resources/sonia/scm/configuration/shiro.ini new file mode 100644 index 0000000000..36226edd7d --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/test/resources/sonia/scm/configuration/shiro.ini @@ -0,0 +1,9 @@ +[users] +readOnly = secret, reader +writeOnly = secret, writer +readWrite = secret, readerWriter + +[roles] +reader = configuration:read:git +writer = configuration:write:git +readerWriter = configuration:*:git diff --git a/scm-plugins/scm-git-plugin/src/test/resources/sonia/scm/repository/shiro.ini b/scm-plugins/scm-git-plugin/src/test/resources/sonia/scm/repository/shiro.ini deleted file mode 100644 index 5073bf398d..0000000000 --- a/scm-plugins/scm-git-plugin/src/test/resources/sonia/scm/repository/shiro.ini +++ /dev/null @@ -1,11 +0,0 @@ -[users] -trillian = secret, admin -dent = secret, creator, heartOfGold, puzzle42 -unpriv = secret -crato = secret, creator - -[roles] -admin = * -creator = repository:create -heartOfGold = "repository:read,modify,delete:hof" -puzzle42 = "repository:read,write:p42" diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ConfigResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ConfigResource.java index bf3da19416..1415605770 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ConfigResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ConfigResource.java @@ -64,7 +64,7 @@ public class ConfigResource { @Path("") @Consumes(VndMediaType.CONFIG) @StatusCodes({ - @ResponseCode(code = 201, condition = "update success"), + @ResponseCode(code = 204, condition = "update success"), @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"), @ResponseCode(code = 403, condition = "not authorized, the current user has no privileges to update the global config"), @ResponseCode(code = 500, condition = "internal server error") diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ConfigResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ConfigResourceTest.java index 5365e9fbde..2a8a44b4d1 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ConfigResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ConfigResourceTest.java @@ -96,8 +96,7 @@ public class ConfigResourceTest { request = MockHttpRequest.get("/" + ConfigResource.CONFIG_PATH_V2); response = new MockHttpResponse(); - dispatcher.invoke(request, response); - assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + dispatcher.invoke(request, response); assertEquals(HttpServletResponse.SC_OK, response.getStatus()); assertTrue(response.getContentAsString().contains("\"proxyPassword\":\"newPassword\"")); assertTrue(response.getContentAsString().contains("\"self\":{\"href\":\"/v2/config")); assertTrue("link not found", response.getContentAsString().contains("\"update\":{\"href\":\"/v2/config")); diff --git a/scm-webapp/src/test/resources/sonia/scm/configuration/shiro.ini b/scm-webapp/src/test/resources/sonia/scm/configuration/shiro.ini index 8647142b19..fc2eac14f1 100644 --- a/scm-webapp/src/test/resources/sonia/scm/configuration/shiro.ini +++ b/scm-webapp/src/test/resources/sonia/scm/configuration/shiro.ini @@ -4,6 +4,6 @@ writeOnly = secret, writer readWrite = secret, readerWriter [roles] -reader = configuration:read -writer = configuration:write -readerWriter = configuration:* +reader = configuration:read:global +writer = configuration:write:global +readerWriter = configuration:*:global