Create fine-grained configuration permissions.

No more hard-coded isAdmin() checks.

pom.xml

@@ -91,7 +91,6 @@
       </snapshots>
     </repository>
 
-    <!-- TODO remove jitpack after edison hal bugfix -->
     <repository>
       <id>jitpack</id>
       <url>https://jitpack.io</url>
@@ -167,6 +166,20 @@
         <scope>test</scope>
       </dependency>
 
+      <!-- TODO replace by proper version from maven central (group: com.github.sdorra) once its there. -->
+      <dependency>
+        <groupId>com.github.schnatterer.shiro-static-permissions</groupId>
+        <artifactId>ssp-lib</artifactId>
+        <version>${ssp.version}</version>
+      </dependency>
+
+      <dependency>
+        <groupId>com.github.schnatterer.shiro-static-permissions</groupId>
+        <artifactId>ssp-processor</artifactId>
+        <version>${ssp.version}</version>
+        <optional>true</optional>
+      </dependency>
+
     </dependencies>
   </dependencyManagement>
 
@@ -551,7 +564,7 @@
     <jetty.maven.version>9.2.10.v20150310</jetty.maven.version>
 
     <!-- security libraries -->
-    <ssp.version>1.0.0-SNAPSHOT</ssp.version>
+    <ssp.version>-SNAPSHOT</ssp.version>
     <shiro.version>1.4.0</shiro.version>
 
     <!-- repostitory libraries -->

scm-core/pom.xml

@@ -129,16 +129,15 @@
       <scope>provided</scope>
     </dependency>
 
+    <!-- TODO replace by proper version from maven central (group: com.github.sdorra) once its there. -->
     <dependency>
-      <groupId>com.github.sdorra</groupId>
+      <groupId>com.github.schnatterer.shiro-static-permissions</groupId>
       <artifactId>ssp-lib</artifactId>
-      <version>${ssp.version}</version>
     </dependency>
 
     <dependency>
-      <groupId>com.github.sdorra</groupId>
+      <groupId>com.github.schnatterer.shiro-static-permissions</groupId>
       <artifactId>ssp-processor</artifactId>
-      <version>${ssp.version}</version>
       <optional>true</optional>
     </dependency>
 

scm-core/src/main/java/sonia/scm/config/Configuration.java

@@ -0,0 +1,28 @@
+package sonia.scm.config;
+
+import com.github.sdorra.ssp.PermissionObject;
+import com.github.sdorra.ssp.StaticPermissions;
+
+/**
+ * Base for all kinds of configurations.
+ *
+ * Allows for permission like
+ *
+ * <ul>
+ *   <li>"configuration:read:global",</li>
+ *   <li>"configuration:write:svn",</li>
+ *   <li>"configuration:*:git",</li>
+ *   <li>"configuration:*"</li>
+ * </ul>
+ *
+ * <br/>
+ *
+ * And for permission checks like {@code ConfigurationPermissions.read(configurationObject).check();}
+ */
+@StaticPermissions(
+  value = "configuration",
+  permissions = {"read", "write"},
+  globalPermissions = {}
+)
+public interface Configuration extends PermissionObject {
+}

scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java

@@ -44,6 +44,7 @@ import javax.xml.bind.annotation.XmlAccessType;
 import javax.xml.bind.annotation.XmlAccessorType;
 import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlRootElement;
+import javax.xml.bind.annotation.XmlTransient;
 import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter;
 import java.io.File;
 import java.util.Set;
@@ -57,10 +58,11 @@ import java.util.concurrent.TimeUnit;
  *
  * @author Sebastian Sdorra
  */
+
 @Singleton
 @XmlRootElement(name = "scm-config")
 @XmlAccessorType(XmlAccessType.FIELD)
-public class ScmConfiguration {
+public class ScmConfiguration implements Configuration {
 
   /**
    * Default JavaScript date format
@@ -501,4 +503,12 @@ public class ScmConfiguration {
   public void setDefaultNamespaceStrategy(String defaultNamespaceStrategy) {
     this.defaultNamespaceStrategy = defaultNamespaceStrategy;
   }
+
+  @Override
+  // Only for permission checks, don't serialize to XML
+  @XmlTransient
+  public String getId() {
+    // Don't change this without migrating SCM permission configuration!
+    return "global";
+  }
 }

scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java

@@ -56,7 +56,7 @@ import sonia.scm.store.ConfigurationStoreFactory;
  *
  * @param <C>
  */
-public abstract class AbstractRepositoryHandler<C extends SimpleRepositoryConfig>
+public abstract class AbstractRepositoryHandler<C extends RepositoryConfig>
   implements RepositoryHandler
 {
 

scm-core/src/main/java/sonia/scm/repository/AbstractSimpleRepositoryHandler.java

@@ -55,7 +55,7 @@ import java.net.URL;
  * @param <C>
  * @author Sebastian Sdorra
  */
-public abstract class AbstractSimpleRepositoryHandler<C extends SimpleRepositoryConfig>
+public abstract class AbstractSimpleRepositoryHandler<C extends RepositoryConfig>
   extends AbstractRepositoryHandler<C> implements RepositoryDirectoryHandler {
 
   public static final String DEFAULT_VERSION_INFORMATION = "unknown";

scm-core/src/main/java/sonia/scm/repository/RepositoryConfig.java

@@ -33,15 +33,12 @@
 
 package sonia.scm.repository;
 
-//~--- non-JDK imports --------------------------------------------------------
-
 import sonia.scm.Validateable;
-
-//~--- JDK imports ------------------------------------------------------------
-
-import java.io.File;
+import sonia.scm.config.Configuration;
 
 import javax.xml.bind.annotation.XmlRootElement;
+import javax.xml.bind.annotation.XmlTransient;
+import java.io.File;
 
 /**
  * Basic {@link Repository} configuration class.
@@ -49,7 +46,7 @@ import javax.xml.bind.annotation.XmlRootElement;
  * @author Sebastian Sdorra
  */
 @XmlRootElement
-public class SimpleRepositoryConfig implements Validateable
+public abstract class RepositoryConfig implements Validateable, Configuration
 {
 
   /**
@@ -119,4 +116,19 @@ public class SimpleRepositoryConfig implements Validateable
 
   /** directory for repositories */
   private File repositoryDirectory;
+
+  /**
+   * Specifies the identifier of the concrete {@link RepositoryConfig} when checking permissions of an object.
+   * The permission Strings will have the following format: "configuration:*:ID", where the ID part is defined by this
+   * method.
+   *
+   * For example: "configuration:read:git".
+   *
+   * No need to serialize this.
+   *
+   * @return identifier of this RepositoryConfig in permission strings
+   */
+  @Override
+  @XmlTransient // Only for permission checks, don't serialize to XML
+  public abstract String getId();
 }

scm-core/src/main/java/sonia/scm/repository/RepositoryHandlerConfigChangedEvent.java

@@ -37,7 +37,7 @@ import sonia.scm.event.Event;
  * @since 2.0.0
  */
 @Event
-public class RepositoryHandlerConfigChangedEvent<C extends SimpleRepositoryConfig>
+public class RepositoryHandlerConfigChangedEvent<C extends RepositoryConfig>
 {
   
   private final C configuration;

scm-core/src/main/java/sonia/scm/repository/RepositoryUtil.java

@@ -74,7 +74,7 @@ public final class RepositoryUtil {
     return getRepositoryId(handler.getConfig(), directory);
   }
 
-  public static String getRepositoryId(SimpleRepositoryConfig config, File directory) throws IOException {
+  public static String getRepositoryId(RepositoryConfig config, File directory) throws IOException {
     return getRepositoryId(config.getRepositoryDirectory(), directory);
   }
 

scm-core/src/test/java/sonia/scm/repository/RepositoryUtilTest.java

@@ -21,9 +21,14 @@ public class RepositoryUtilTest {
   public TemporaryFolder temporaryFolder = new TemporaryFolder();
 
   @Mock
-  private AbstractRepositoryHandler<SimpleRepositoryConfig> repositoryHandler;
+  private AbstractRepositoryHandler<RepositoryConfig> repositoryHandler;
 
-  private SimpleRepositoryConfig repositoryConfig = new SimpleRepositoryConfig();
+  private RepositoryConfig repositoryConfig = new RepositoryConfig() {
+    @Override
+    public String getId() {
+      return "repository";
+    }
+  };
 
   @Before
   public void setUpMocks() {

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitConfig.java

@@ -39,6 +39,7 @@ import javax.xml.bind.annotation.XmlAccessType;
 import javax.xml.bind.annotation.XmlAccessorType;
 import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlRootElement;
+import javax.xml.bind.annotation.XmlTransient;
 
 /**
  *
@@ -46,7 +47,7 @@ import javax.xml.bind.annotation.XmlRootElement;
  */
 @XmlRootElement(name = "config")
 @XmlAccessorType(XmlAccessType.FIELD)
-public class GitConfig extends SimpleRepositoryConfig {
+public class GitConfig extends RepositoryConfig {
   
   @XmlElement(name = "gc-expression")
   private String gcExpression;
@@ -56,4 +57,10 @@ public class GitConfig extends SimpleRepositoryConfig {
     return gcExpression;
   }
 
+  @Override
+  @XmlTransient // Only for permission checks, don't serialize to XML
+  public String getId() {
+    // Don't change this without migrating SCM permission configuration!
+    return "git";
+  }
 }

scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/HgConfig.java

@@ -33,20 +33,19 @@
 
 package sonia.scm.repository;
 
-//~--- non-JDK imports --------------------------------------------------------
 
 import sonia.scm.util.Util;
 
-//~--- JDK imports ------------------------------------------------------------
-
 import javax.xml.bind.annotation.XmlRootElement;
+import javax.xml.bind.annotation.XmlTransient;
+
 
 /**
  *
  * @author Sebastian Sdorra
  */
 @XmlRootElement(name = "config")
-public class HgConfig extends SimpleRepositoryConfig
+public class HgConfig extends RepositoryConfig
 {
 
   /**
@@ -223,4 +222,11 @@ public class HgConfig extends SimpleRepositoryConfig
 
   /** Field description */
   private boolean showRevisionInId = false;
+
+  @Override
+  @XmlTransient // Only for permission checks, don't serialize to XML
+  public String getId() {
+    // Don't change this without migrating SCM permission configuration!
+    return "hg";
+  }
 }

scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/SvnConfig.java

@@ -33,12 +33,11 @@
 
 package sonia.scm.repository;
 
-//~--- JDK imports ------------------------------------------------------------
-
 import javax.xml.bind.annotation.XmlAccessType;
 import javax.xml.bind.annotation.XmlAccessorType;
 import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlRootElement;
+import javax.xml.bind.annotation.XmlTransient;
 
 /**
  *
@@ -46,7 +45,7 @@ import javax.xml.bind.annotation.XmlRootElement;
  */
 @XmlRootElement(name = "config")
 @XmlAccessorType(XmlAccessType.FIELD)
-public class SvnConfig extends SimpleRepositoryConfig
+public class SvnConfig extends RepositoryConfig
 {
 
   /**
@@ -108,4 +107,11 @@ public class SvnConfig extends SimpleRepositoryConfig
 
   /** Field description */
   private Compatibility compatibility = Compatibility.NONE;
+
+  @Override
+  @XmlTransient // Only for permission checks, don't serialize to XML
+  public String getId() {
+    // Don't change this without migrating SCM permission configuration!
+    return "svn";
+  }
 }

scm-test/src/main/java/sonia/scm/repository/DummyRepositoryHandler.java

@@ -37,6 +37,7 @@ import sonia.scm.Type;
 import sonia.scm.io.DefaultFileSystem;
 import sonia.scm.store.ConfigurationStoreFactory;
 
+import javax.xml.bind.annotation.XmlRootElement;
 import java.io.File;
 import java.util.HashSet;
 import java.util.Set;
@@ -48,7 +49,7 @@ import java.util.Set;
  * @author Sebastian Sdorra
  */
 public class DummyRepositoryHandler
-  extends AbstractSimpleRepositoryHandler<SimpleRepositoryConfig> {
+  extends AbstractSimpleRepositoryHandler<DummyRepositoryHandler.DummyRepositoryConfig> {
 
   public static final String TYPE_DISPLAYNAME = "Dummy";
 
@@ -79,12 +80,20 @@ public class DummyRepositoryHandler
   }
 
   @Override
-  protected SimpleRepositoryConfig createInitialConfig() {
-    return new SimpleRepositoryConfig();
+  protected DummyRepositoryConfig createInitialConfig() {
+    return new DummyRepositoryConfig();
   }
 
   @Override
-  protected Class<SimpleRepositoryConfig> getConfigClass() {
-    return SimpleRepositoryConfig.class;
+  protected Class<DummyRepositoryConfig> getConfigClass() {
+    return DummyRepositoryConfig.class;
+  }
+
+  @XmlRootElement(name = "config")
+  public static class DummyRepositoryConfig extends RepositoryConfig {
+    @Override
+    public String getId() {
+      return TYPE_NAME;
+    }
   }
 }

scm-webapp/src/main/java/sonia/scm/api/v2/resources/GlobalConfigResource.java

@@ -3,14 +3,17 @@ 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.config.ScmConfiguration;
-import sonia.scm.security.Role;
 import sonia.scm.util.ScmConfigurationUtil;
 import sonia.scm.web.VndMediaType;
 
 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;
@@ -44,16 +47,12 @@ public class GlobalConfigResource {
     @ResponseCode(code = 500, condition = "internal server error")
   })
   public Response get() {
-    Response response;
 
-    // TODO ConfigPermisions?
-    if (SecurityUtils.getSubject().hasRole(Role.ADMIN)) {
-      response = Response.ok(configToDtoMapper.map(configuration)).build();
-    } else {
-      response = Response.status(Response.Status.FORBIDDEN).build();
-    }
+    // We do this permission check in Resource and not in ScmConfiguration, because it must be available for reading
+    // from within the code (plugins, etc.), but not for the whole anonymous world outside.
+    ConfigurationPermissions.read(configuration).check();
 
-    return response;
+    return Response.ok(configToDtoMapper.map(configuration)).build();
   }
 
   /**
@@ -72,20 +71,17 @@ public class GlobalConfigResource {
   })
   @TypeHint(TypeHint.NO_CONTENT.class)
   public Response update(GlobalConfigDto configDto, @Context UriInfo uriInfo) {
-    Response response;
 
-    // TODO ConfigPermisions?
-    if (SecurityUtils.getSubject().hasRole(Role.ADMIN)) {
+    // This *could* be moved to ScmConfiguration or ScmConfigurationUtil classes.
+    // But to where to check? load() or store()? Leave it for now, SCMv1 legacy that can be cleaned up later.
+    ConfigurationPermissions.write(configuration).check();
+
     ScmConfiguration config = dtoToConfigMapper.map(configDto);
-      configuration.load(config);
     synchronized (ScmConfiguration.class) {
+      configuration.load(config);
       ScmConfigurationUtil.getInstance().store(configuration);
     }
-      response = Response.created(uriInfo.getRequestUri()).build();
-    } else {
-      response = Response.status(Response.Status.FORBIDDEN).build();
-    }
 
-    return response;
+    return Response.created(uriInfo.getRequestUri()).build();
   }
 }

scm-webapp/src/main/java/sonia/scm/api/v2/resources/ScmConfigurationToGlobalConfigDtoMapper.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.config.ScmConfiguration;
-import sonia.scm.security.Role;
 
 import javax.inject.Inject;
 
@@ -26,8 +25,7 @@ public abstract class ScmConfigurationToGlobalConfigDtoMapper {
   @AfterMapping
   void appendLinks(ScmConfiguration config, @MappingTarget GlobalConfigDto target) {
     Links.Builder linksBuilder = linkingTo().self(resourceLinks.globalConfig().self());
-    // TODO: ConfigPermissions?
-    if (SecurityUtils.getSubject().hasRole(Role.ADMIN)) {
+    if (ConfigurationPermissions.write(config).isPermitted()) {
       linksBuilder.single(link("update", resourceLinks.globalConfig().update()));
     }
     target.add(linksBuilder.build());

scm-webapp/src/main/java/sonia/scm/filter/AdminSecurityFilter.java

@@ -49,6 +49,7 @@ import sonia.scm.security.Role;
  * 
  * @author Sebastian Sdorra
  */
+// TODO before releasing v2, delete this filter (we use Permission objects now)
 @WebElement(
   value = Filters.PATTERN_CONFIG,
   morePatterns = { 

scm-webapp/src/test/java/sonia/scm/api/v2/resources/GlobalConfigResourceTest.java

@@ -10,6 +10,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.mockito.InjectMocks;
 import sonia.scm.config.ScmConfiguration;
 import sonia.scm.web.VndMediaType;
@@ -23,19 +24,22 @@ import java.util.Arrays;
 import java.util.HashSet;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.MockitoAnnotations.initMocks;
 
 @SubjectAware(
-  username = "trillian",
-  password = "secret",
-  configuration = "classpath:sonia/scm/repository/shiro.ini"
+  configuration = "classpath:sonia/scm/configuration/shiro.ini",
+  password = "secret"
 )
 public class GlobalConfigResourceTest {
 
   @Rule
   public ShiroRule shiro = new ShiroRule();
 
+  @Rule
+  public ExpectedException thrown = ExpectedException.none();
+
   private Dispatcher dispatcher = MockDispatcherFactory.createDispatcher();
 
   private final URI baseUri = URI.create("/");
@@ -58,6 +62,7 @@ public class GlobalConfigResourceTest {
   }
 
   @Test
+  @SubjectAware(username = "readOnly")
   public void shouldGetGlobalConfig() throws URISyntaxException {
     MockHttpRequest request = MockHttpRequest.get("/" + GlobalConfigResource.GLOBAL_CONFIG_PATH_V2);
     MockHttpResponse response = new MockHttpResponse();
@@ -65,22 +70,22 @@ public class GlobalConfigResourceTest {
     assertEquals(HttpServletResponse.SC_OK, response.getStatus());
     assertTrue(response.getContentAsString().contains("\"proxyPassword\":\"heartOfGold\""));
     assertTrue(response.getContentAsString().contains("\"self\":{\"href\":\"/v2/config/global"));
-    assertTrue("link not found", response.getContentAsString().contains("\"update\":{\"href\":\"/v2/config/global"));
+    assertFalse("Update link present", response.getContentAsString().contains("\"update\":{\"href\":\"/v2/config/global"));
   }
 
-  @SubjectAware(
-    username = "dent"
-  )
   @Test
-  public void shouldGetForbiddenGlobalConfig() throws URISyntaxException {
+  @SubjectAware(username = "writeOnly")
+  public void shouldGetGlobalConfigOnlyWhenAuthorized() throws URISyntaxException {
     MockHttpRequest request = MockHttpRequest.get("/" + GlobalConfigResource.GLOBAL_CONFIG_PATH_V2);
     MockHttpResponse response = new MockHttpResponse();
 
+    thrown.expectMessage("Subject does not have permission [configuration:read:global]");
+
     dispatcher.invoke(request, response);
-    assertEquals(HttpServletResponse.SC_FORBIDDEN, response.getStatus());
   }
 
   @Test
+  @SubjectAware(username = "readWrite")
   public void shouldUpdateGlobalConfig() throws URISyntaxException, IOException {
     URL url = Resources.getResource("sonia/scm/api/v2/globalConfig-test-update.json");
     byte[] configJson = Resources.toByteArray(url);
@@ -102,11 +107,9 @@ public class GlobalConfigResourceTest {
 
   }
 
-  @SubjectAware(
-    username = "dent"
-  )
   @Test
-  public void shouldUpdateForbiddenGlobalConfig() throws URISyntaxException, IOException {
+  @SubjectAware(username = "readOnly")
+  public void shouldUpdateGlobalConfigOnlyWhenAuthorized() throws URISyntaxException, IOException {
     URL url = Resources.getResource("sonia/scm/api/v2/globalConfig-test-update.json");
     byte[] configJson = Resources.toByteArray(url);
     MockHttpRequest request = MockHttpRequest.put("/" + GlobalConfigResource.GLOBAL_CONFIG_PATH_V2)
@@ -114,8 +117,10 @@ public class GlobalConfigResourceTest {
       .content(configJson);
     MockHttpResponse response = new MockHttpResponse();
 
+    thrown.expectMessage("Subject does not have permission [configuration:write:global]");
+
+
     dispatcher.invoke(request, response);
-    assertEquals(HttpServletResponse.SC_FORBIDDEN, response.getStatus());
   }
 
   public static ScmConfiguration createConfiguration() {

scm-webapp/src/test/java/sonia/scm/api/v2/resources/ScmConfigurationToGlobalConfigDtoMapperTest.java

@@ -51,7 +51,7 @@ public class ScmConfigurationToGlobalConfigDtoMapperTest {
   public void shouldMapFields() {
     ScmConfiguration config = createConfiguration();
 
-    when(subject.hasRole(Role.ADMIN)).thenReturn(true);
+    when(subject.isPermitted("configuration:write:global")).thenReturn(true);
     GlobalConfigDto dto = mapper.map(config);
 
     assertEquals("baseurl", dto.getBaseUrl());
@@ -63,7 +63,7 @@ public class ScmConfigurationToGlobalConfigDtoMapperTest {
   public void shouldMapFieldsWithoutUpdate() {
     ScmConfiguration config = createConfiguration();
 
-    when(subject.hasRole(Role.ADMIN)).thenReturn(false);
+    when(subject.hasRole("configuration:write:global")).thenReturn(false);
     GlobalConfigDto dto = mapper.map(config);
 
     assertEquals("baseurl", dto.getBaseUrl());

scm-webapp/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
+writer = configuration:write
+readerWriter = configuration:*