Nemcio/SCM-Manager
1
0
Fork 0
mirror of https://github.com/scm-manager/scm-manager.git synced 2025-11-12 00:15:44 +01:00
Code Issues Packages Projects Releases Activity

do not use IllegalArgumentException for parameter validation

Browse Source
This commit is contained in:
Sebastian Sdorra
2018-07-30 16:13:17 +02:00
parent cba3fc38e6
commit b8897b273a
3 changed files with 65 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
scm-webapp/src/main/java/sonia/scm/api/v2/resources/AuthenticationRequestDto.java
View File

@@ -1,7 +1,6 @@
package sonia.scm.api.v2.resources; package sonia.scm.api.v2.resources;
import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import javax.ws.rs.FormParam; import javax.ws.rs.FormParam;
@@ -45,9 +44,8 @@ public class AuthenticationRequestDto {
return scope; return scope;
} }
public void validate() { public boolean isValid() {
Preconditions.checkArgument(!Strings.isNullOrEmpty(grantType), "grant_type parameter is required"); // password is currently the only valid grant_type
Preconditions.checkArgument(!Strings.isNullOrEmpty(username), "username parameter is required"); return "password".equals(grantType) && !Strings.isNullOrEmpty(username) && !Strings.isNullOrEmpty(password);
Preconditions.checkArgument(!Strings.isNullOrEmpty(password), "password parameter is required");
} }
} }

6
scm-webapp/src/main/java/sonia/scm/api/v2/resources/AuthenticationResource.java
View File

@@ -29,7 +29,7 @@ public class AuthenticationResource {
private static final Logger LOG = LoggerFactory.getLogger(AuthenticationResource.class); private static final Logger LOG = LoggerFactory.getLogger(AuthenticationResource.class);
public static final String PATH = "v2/auth"; static final String PATH = "v2/auth";
private final AccessTokenBuilderFactory tokenBuilderFactory; private final AccessTokenBuilderFactory tokenBuilderFactory;
private final AccessTokenCookieIssuer cookieIssuer; private final AccessTokenCookieIssuer cookieIssuer;
@@ -81,7 +81,9 @@ public class AuthenticationResource {
HttpServletResponse response, HttpServletResponse response,
AuthenticationRequestDto authentication AuthenticationRequestDto authentication
) { ) {
authentication.validate(); if (!authentication.isValid()) {
return Response.status(Response.Status.BAD_REQUEST).build();
}
Response res; Response res;
Subject subject = SecurityUtils.getSubject(); Subject subject = SecurityUtils.getSubject();

59
scm-webapp/src/test/java/sonia/scm/api/v2/resources/AuthenticationResourceTest.java
View File

@@ -80,6 +80,35 @@ public class AuthenticationResourceTest {
"\t\"password\": \"doesNotMatter\"\n" + "\t\"password\": \"doesNotMatter\"\n" +
"}"; "}";
private static final String AUTH_JSON_WITHOUT_USERNAME = String.join("\n",
"{",
"\"grant_type\": \"password\",",
"\"password\": \"tricia123\"",
"}"
);
private static final String AUTH_JSON_WITHOUT_PASSWORD = String.join("\n",
"{",
"\"grant_type\": \"password\",",
"\"username\": \"trillian\"",
"}"
);
private static final String AUTH_JSON_WITHOUT_GRANT_TYPE = String.join("\n",
"{",
"\"username\": \"trillian\",",
"\"password\": \"tricia123\"",
"}"
);
private static final String AUTH_JSON_WITH_INVALID_GRANT_TYPE = String.join("\n",
"{",
"\"grant_type\": \"el speciale\",",
"\"username\": \"trillian\",",
"\"password\": \"tricia123\"",
"}"
);
@Before @Before
public void prepareEnvironment() { public void prepareEnvironment() {
AuthenticationResource authenticationResource = new AuthenticationResource(accessTokenBuilderFactory, cookieIssuer); AuthenticationResource authenticationResource = new AuthenticationResource(accessTokenBuilderFactory, cookieIssuer);
@@ -122,7 +151,6 @@ public class AuthenticationResourceTest {
@Test @Test
public void shouldNotAuthNonexistingUser() throws URISyntaxException { public void shouldNotAuthNonexistingUser() throws URISyntaxException {
MockHttpRequest request = getMockHttpRequest(AUTH_JSON_NOT_EXISTING_USER); MockHttpRequest request = getMockHttpRequest(AUTH_JSON_NOT_EXISTING_USER);
MockHttpResponse response = new MockHttpResponse(); MockHttpResponse response = new MockHttpResponse();
@@ -131,6 +159,35 @@ public class AuthenticationResourceTest {
assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.getStatus()); assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.getStatus());
} }
@Test
public void shouldReturnBadStatusIfPasswordParameterIsMissing() throws URISyntaxException {
shouldReturnBadRequest(AUTH_JSON_WITHOUT_USERNAME);
}
@Test
public void shouldReturnBadStatusIfUsernameParameterIsMissing() throws URISyntaxException {
shouldReturnBadRequest(AUTH_JSON_WITHOUT_PASSWORD);
}
@Test
public void shouldReturnBadStatusIfGrantTypeParameterIsMissing() throws URISyntaxException {
shouldReturnBadRequest(AUTH_JSON_WITHOUT_GRANT_TYPE);
}
@Test
public void shouldReturnBadStatusIfGrantTypeParameterIsInvalid() throws URISyntaxException {
shouldReturnBadRequest(AUTH_JSON_WITH_INVALID_GRANT_TYPE);
}
private void shouldReturnBadRequest(String requestBody) throws URISyntaxException {
MockHttpRequest request = getMockHttpRequest(requestBody);
MockHttpResponse response = new MockHttpResponse();
dispatcher.invoke(request, response);
assertEquals(HttpServletResponse.SC_BAD_REQUEST, response.getStatus());
}
private MockHttpRequest getMockHttpRequest(String jsonPayload) throws URISyntaxException { private MockHttpRequest getMockHttpRequest(String jsonPayload) throws URISyntaxException {
MockHttpRequest request = MockHttpRequest.post("/" + AuthenticationResource.PATH + "/access_token"); MockHttpRequest request = MockHttpRequest.post("/" + AuthenticationResource.PATH + "/access_token");