Git Plugin Config: Create fine-grained configuration permissions.

No more hard-coded isAdmin() checks.
Also adds more tests.

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) {
+    if (config == null) {
-        config = new GitConfig();
+      config = new GitConfig();
-        repositoryHandler.setConfig(config);
+      repositoryHandler.setConfig(config);
-      }
-
-      response = Response.ok(configToDtoMapper.map(config)).build();
-    } else {
-      response = Response.status(Response.Status.FORBIDDEN).build();
     }
 
-    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)) {
+    GitConfig config = dtoToConfigMapper.map(configDto);
-      repositoryHandler.setConfig(dtoToConfigMapper.map(configDto));
-      repositoryHandler.storeConfig();
-      response = Response.created(uriInfo.getRequestUri()).build();
-    } else {
-      response = Response.status(Response.Status.FORBIDDEN).build();
-    }
 
-    return response;
+    ConfigurationPermissions.write(config).check();
+
+    repositoryHandler.setConfig(config);
+    repositoryHandler.storeConfig();
+
+    return Response.noContent().build();
   }
 }

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 (ConfigurationPermissions.write(config).isPermitted()) {
-    if (SecurityUtils.getSubject().hasRole(Role.ADMIN)) {
       linksBuilder.single(link("update", update()));
     }
     target.add(linksBuilder.build());

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() {

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",
+  configuration = "classpath:sonia/scm/configuration/shiro.ini",
-  password = "secret",
+  password = "secret"
-  configuration = "classpath:sonia/scm/repository/shiro.ini"
 )
 @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 = get();
-    MockHttpResponse response = new MockHttpResponse();
+
-    dispatcher.invoke(request, response);
     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();

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

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

scm-plugins/scm-git-plugin/src/test/resources/sonia/scm/repository/shiro.ini

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

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")

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);
+    dispatcher.invoke(request, response);    assertEquals(HttpServletResponse.SC_OK, response.getStatus());
-    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"));

scm-webapp/src/test/resources/sonia/scm/configuration/shiro.ini

@@ -4,6 +4,6 @@ writeOnly = secret, writer
 readWrite = secret, readerWriter
 
 [roles]
-reader = configuration:read
+reader = configuration:read:global
-writer = configuration:write
+writer = configuration:write:global
-readerWriter = configuration:*
+readerWriter = configuration:*:global