diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/ValidationConstraints.java b/scm-webapp/src/main/java/sonia/scm/api/v2/ValidationConstraints.java new file mode 100644 index 0000000000..b136ee1bf7 --- /dev/null +++ b/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 @ 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\\.\\-_@]*$"; + +} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/GroupDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/GroupDto.java index b847412a33..9589af122e 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/GroupDto.java +++ b/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; diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionDto.java index 581b2c24cd..82405a6ac2 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionDto.java +++ b/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; diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java index b7f6df8c2d..cbb3fe0e46 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PermissionRootResource.java +++ b/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(); diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDto.java index 4e4345445a..9dc5b850bd 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDto.java +++ b/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; diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/ValidationConstraints_IllegalCharactersTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/ValidationConstraints_IllegalCharactersTest.java new file mode 100644 index 0000000000..c56f6195fe --- /dev/null +++ b/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 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 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()); + } +} diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/GroupRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/GroupRootResourceTest.java index f662542ff7..036579e9dd 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/GroupRootResourceTest.java +++ b/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 diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java index 563e9bdbc0..635d994763 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java +++ b/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); 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 065a33313f..6ab1dc6aeb 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 @@ -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 {