Merged in bugfix/permission_restrictions_v2 (pull request #80)

Bugfix/permission restrictions v2

scm-webapp/src/main/java/sonia/scm/api/v2/ValidationConstraints.java

@@ -0,0 +1,14 @@
+package sonia.scm.api.v2;
+
+public final class ValidationConstraints {
+
+  private ValidationConstraints() {}
+
+  /**
+   * A user or group name should not start with <code>@</code> or a whitespace
+   * and it not contains whitespaces
+   * and the characters: . - _ @ are allowed
+   */
+  public static final String USER_GROUP_PATTERN = "^[A-Za-z0-9\\.\\-_][A-Za-z0-9\\.\\-_@]*$";
+
+}

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

@@ -13,6 +13,8 @@ import java.time.Instant;
 import java.util.List;
 import java.util.Map;
 
+import static sonia.scm.api.v2.ValidationConstraints.USER_GROUP_PATTERN;
+
 @Getter @Setter @NoArgsConstructor
 public class GroupDto extends HalRepresentation {
 
@@ -20,7 +22,7 @@ public class GroupDto extends HalRepresentation {
   private String description;
   @JsonInclude(JsonInclude.Include.NON_NULL)
   private Instant lastModified;
-  @Pattern(regexp = "^[A-z0-9\\.\\-_@]|[^ ]([A-z0-9\\.\\-_@ ]*[A-z0-9\\.\\-_@]|[^ ])?$")
+  @Pattern(regexp = USER_GROUP_PATTERN)
   private String name;
   @NotEmpty
   private String type;

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

@@ -4,15 +4,20 @@ import com.fasterxml.jackson.annotation.JsonInclude;
 import de.otto.edison.hal.HalRepresentation;
 import de.otto.edison.hal.Links;
 import lombok.Getter;
+import lombok.NoArgsConstructor;
 import lombok.Setter;
 import lombok.ToString;
 
-@Getter @Setter @ToString
+import javax.validation.constraints.Pattern;
+
+import static sonia.scm.api.v2.ValidationConstraints.USER_GROUP_PATTERN;
+
+@Getter @Setter @ToString @NoArgsConstructor
 public class PermissionDto extends HalRepresentation {
 
   public static final String GROUP_PREFIX = "@";
 
-  @JsonInclude(JsonInclude.Include.NON_NULL)
+  @Pattern(regexp = USER_GROUP_PATTERN)
   private String name;
 
   /**
@@ -28,9 +33,6 @@ public class PermissionDto extends HalRepresentation {
 
   private boolean groupPermission = false;
 
-  public PermissionDto() {
-  }
-
   public PermissionDto(String permissionName, boolean groupPermission) {
     name = permissionName;
     this.groupPermission = groupPermission;

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

@@ -16,6 +16,7 @@ import sonia.scm.repository.RepositoryPermissions;
 import sonia.scm.web.VndMediaType;
 
 import javax.inject.Inject;
+import javax.validation.Valid;
 import javax.ws.rs.Consumes;
 import javax.ws.rs.DELETE;
 import javax.ws.rs.GET;
@@ -70,7 +71,7 @@ public class PermissionRootResource {
   @TypeHint(TypeHint.NO_CONTENT.class)
   @Consumes(VndMediaType.PERMISSION)
   @Path("")
-  public Response create(@PathParam("namespace") String namespace, @PathParam("name") String name, PermissionDto permission) throws Exception {
+  public Response create(@PathParam("namespace") String namespace, @PathParam("name") String name,@Valid PermissionDto permission) throws AlreadyExistsException, NotFoundException {
     log.info("try to add new permission: {}", permission);
     Repository repository = load(namespace, name);
     RepositoryPermissions.permissionWrite(repository).check();
@@ -156,7 +157,7 @@ public class PermissionRootResource {
   public Response update(@PathParam("namespace") String namespace,
                          @PathParam("name") String name,
                          @PathParam("permission-name") String permissionName,
-                         PermissionDto permission) throws NotFoundException, AlreadyExistsException {
+                         @Valid 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();

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

@@ -13,6 +13,8 @@ import javax.validation.constraints.Pattern;
 import java.time.Instant;
 import java.util.Map;
 
+import static sonia.scm.api.v2.ValidationConstraints.USER_GROUP_PATTERN;
+
 @NoArgsConstructor @Getter @Setter
 public class UserDto extends HalRepresentation {
   private boolean active;
@@ -24,7 +26,7 @@ public class UserDto extends HalRepresentation {
   private Instant lastModified;
   @NotEmpty @Email
   private String mail;
-  @Pattern(regexp = "^[A-z0-9\\.\\-_@]|[^ ]([A-z0-9\\.\\-_@ ]*[A-z0-9\\.\\-_@]|[^ ])?$")
+  @Pattern(regexp = USER_GROUP_PATTERN)
   private String name;
   @JsonInclude(JsonInclude.Include.NON_NULL)
   private String password;

scm-webapp/src/test/java/sonia/scm/api/v2/ValidationConstraints_IllegalCharactersTest.java

@@ -0,0 +1,47 @@
+package sonia.scm.api.v2;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
+
+import static java.util.Arrays.asList;
+import static org.junit.Assert.assertFalse;
+import static sonia.scm.api.v2.ValidationConstraints.USER_GROUP_PATTERN;
+
+@RunWith(Parameterized.class)
+public class ValidationConstraints_IllegalCharactersTest {
+
+  private static final List<Character> ACCEPTED_CHARS = asList('@', '_', '-', '.');
+
+  private final Pattern userGroupPattern=Pattern.compile(USER_GROUP_PATTERN);
+
+  private final String expression;
+
+  public ValidationConstraints_IllegalCharactersTest(String expression) {
+    this.expression = expression;
+  }
+
+  @Parameterized.Parameters(name = "{0}")
+  public static Collection<String[]> createParameters() {
+    return Stream.concat(IntStream.range(0x20, 0x2f).mapToObj(i -> (char) i),    // chars before '0'
+           Stream.concat(IntStream.range(0x3a, 0x40).mapToObj(i -> (char) i),    // chars between '9' and 'A'
+           Stream.concat(IntStream.range(0x5b, 0x60).mapToObj(i -> (char) i),    // chars between 'Z' and 'a'
+                         IntStream.range(0x7b, 0xff).mapToObj(i -> (char) i))))  // chars after 'z'
+      .filter(c -> !ACCEPTED_CHARS.contains(c))
+      .flatMap(c -> Stream.of("abc" + c + "xyz", "@" + c, c + "tail"))
+      .map(c -> new String[] {c})
+      .collect(Collectors.toList());
+  }
+
+  @Test
+  public void shouldNotAcceptSpecialCharacters() {
+    assertFalse(userGroupPattern.matcher(expression).matches());
+  }
+}

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

@@ -224,6 +224,64 @@ public class GroupRootResourceTest {
     assertEquals("user1", createdGroup.getMembers().get(0));
   }
 
+  @Test
+  public void shouldGet400OnCreatingNewGroupWithNotAllowedCharacters() throws URISyntaxException {
+    // the @ character at the begin of the name is not allowed
+    String groupJson = "{ \"name\": \"@grpname\", \"type\": \"admin\" }";
+    MockHttpRequest request = MockHttpRequest
+      .post("/" + GroupRootResource.GROUPS_PATH_V2)
+      .contentType(VndMediaType.GROUP)
+      .content(groupJson.getBytes());
+    MockHttpResponse response = new MockHttpResponse();
+
+    dispatcher.invoke(request, response);
+
+    assertEquals(400, response.getStatus());
+
+    // the whitespace at the begin of the name is not allowed
+    groupJson = "{ \"name\": \" grpname\", \"type\": \"admin\" }";
+    request = MockHttpRequest
+      .post("/" + GroupRootResource.GROUPS_PATH_V2)
+      .contentType(VndMediaType.GROUP)
+      .content(groupJson.getBytes());
+
+    dispatcher.invoke(request, response);
+
+    assertEquals(400, response.getStatus());
+
+    // the characters {[ are not allowed
+    groupJson = "{ \"name\": \"grp{name}\", \"type\": \"admin\" }";
+    request = MockHttpRequest
+      .post("/" + GroupRootResource.GROUPS_PATH_V2)
+      .contentType(VndMediaType.GROUP)
+      .content(groupJson.getBytes());
+
+    dispatcher.invoke(request, response);
+
+    assertEquals(400, response.getStatus());
+
+    groupJson = "{ \"name\": \"grp[name]\", \"type\": \"admin\" }";
+    request = MockHttpRequest
+      .post("/" + GroupRootResource.GROUPS_PATH_V2)
+      .contentType(VndMediaType.GROUP)
+      .content(groupJson.getBytes());
+
+    dispatcher.invoke(request, response);
+
+    assertEquals(400, response.getStatus());
+
+    groupJson = "{ \"name\": \"grp/name\", \"type\": \"admin\" }";
+    request = MockHttpRequest
+      .post("/" + GroupRootResource.GROUPS_PATH_V2)
+      .contentType(VndMediaType.GROUP)
+      .content(groupJson.getBytes());
+
+    dispatcher.invoke(request, response);
+
+    assertEquals(400, response.getStatus());
+
+  }
+
   @Test
   public void shouldFailForMissingContent() throws URISyntaxException {
     MockHttpRequest request = MockHttpRequest

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

@@ -48,6 +48,7 @@ import java.util.stream.Stream;
 import static de.otto.edison.hal.Link.link;
 import static de.otto.edison.hal.Links.linkingTo;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 import static org.junit.jupiter.api.DynamicTest.dynamicTest;
 import static org.mockito.Matchers.any;
@@ -233,6 +234,35 @@ public class PermissionRootResourceTest extends RepositoryTestBase {
     );
   }
 
+
+  @Test
+  public void shouldGet400OnCreatingNewPermissionWithNotAllowedCharacters() throws URISyntaxException {
+    // the @ character at the begin of the name is not allowed
+    createUserWithRepository("user");
+    String permissionJson = "{ \"name\": \"@permission\", \"type\": \"OWNER\" }";
+    MockHttpRequest request = MockHttpRequest
+      .post("/" + RepositoryRootResource.REPOSITORIES_PATH_V2 + PATH_OF_ALL_PERMISSIONS)
+      .content(permissionJson.getBytes())
+      .contentType(VndMediaType.PERMISSION);
+    MockHttpResponse response = new MockHttpResponse();
+
+    dispatcher.invoke(request, response);
+
+    assertEquals(400, response.getStatus());
+
+    // the whitespace at the begin opf the name is not allowed
+    permissionJson = "{ \"name\": \" permission\", \"type\": \"OWNER\" }";
+    request = MockHttpRequest
+      .post("/" + RepositoryRootResource.REPOSITORIES_PATH_V2 + PATH_OF_ALL_PERMISSIONS)
+      .content(permissionJson.getBytes())
+      .contentType(VndMediaType.PERMISSION);
+    response = new MockHttpResponse();
+
+    dispatcher.invoke(request, response);
+
+    assertEquals(400, response.getStatus());
+  }
+
   @Test
   public void shouldGetCreatedPermissions() throws URISyntaxException {
     createUserWithRepositoryAndPermissions(TEST_PERMISSIONS, PERMISSION_WRITE);

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

@@ -98,6 +98,32 @@ public class UserRootResourceTest {
     assertTrue(response.getContentAsString().contains("\"delete\":{\"href\":\"/v2/users/Neo\"}"));
   }
 
+  @Test
+  public void shouldGet400OnCreatingNewUserWithNotAllowedCharacters() throws URISyntaxException {
+    // the @ character at the begin of the name is not allowed
+    String userJson = "{ \"name\": \"@user\", \"type\": \"db\" }";
+    MockHttpRequest request = MockHttpRequest
+      .post("/" + UserRootResource.USERS_PATH_V2)
+      .contentType(VndMediaType.USER)
+      .content(userJson.getBytes());
+    MockHttpResponse response = new MockHttpResponse();
+
+    dispatcher.invoke(request, response);
+
+    assertEquals(400, response.getStatus());
+
+    // the whitespace at the begin opf the name is not allowed
+    userJson = "{ \"name\": \" user\", \"type\": \"db\" }";
+    request = MockHttpRequest
+      .post("/" + UserRootResource.USERS_PATH_V2)
+      .contentType(VndMediaType.USER)
+      .content(userJson.getBytes());
+
+    dispatcher.invoke(request, response);
+
+    assertEquals(400, response.getStatus());
+  }
+
   @Test
   @SubjectAware(username = "unpriv")
   public void shouldCreateLimitedResponseForSimpleUser() throws URISyntaxException {