Skip to content

Fix: add length validation to prevent 500 error on invalid usercode #1309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
martinr0x wants to merge 2 commits into from
Closed

Fix: add length validation to prevent 500 error on invalid usercode #1309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9e19fdd
Fix userCode validation
martinr0x Jul 20, 2023
12625ee
fix formatting and tests
martinr0x Jul 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...orization/web/authentication/OAuth2DeviceAuthorizationConsentAuthenticationConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public Authentication convert(HttpServletRequest request) {

// user_code (REQUIRED)
String userCode = parameters.getFirst(OAuth2ParameterNames.USER_CODE);
if (!StringUtils.hasText(userCode) ||
if (!OAuth2EndpointUtils.validateUserCode(userCode) ||
parameters.get(OAuth2ParameterNames.USER_CODE).size() != 1) {
OAuth2EndpointUtils.throwError(
OAuth2ErrorCodes.INVALID_REQUEST,
Expand Down
4 changes: 1 addition & 3 deletions ...ver/authorization/web/authentication/OAuth2DeviceVerificationAuthenticationConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.springframework.security.oauth2.server.authorization.web.OAuth2DeviceVerificationEndpointFilter;
import org.springframework.security.web.authentication.AuthenticationConverter;
import org.springframework.util.MultiValueMap;
import org.springframework.util.StringUtils;

/**
* Attempts to extract a user code from {@link HttpServletRequest} for the
Expand All @@ -49,7 +48,6 @@ public final class OAuth2DeviceVerificationAuthenticationConverter implements Au
private static final String ERROR_URI = "https://datatracker.ietf.org/doc/html/rfc6749#section-5.2";
private static final Authentication ANONYMOUS_AUTHENTICATION = new AnonymousAuthenticationToken(
"anonymous", "anonymousUser", AuthorityUtils.createAuthorityList("ROLE_ANONYMOUS"));

@Override
public Authentication convert(HttpServletRequest request) {
if (!("GET".equals(request.getMethod()) || "POST".equals(request.getMethod()))) {
Expand All @@ -64,7 +62,7 @@ public Authentication convert(HttpServletRequest request) {

// user_code (REQUIRED)
String userCode = parameters.getFirst(OAuth2ParameterNames.USER_CODE);
if (!StringUtils.hasText(userCode) ||
if (!OAuth2EndpointUtils.validateUserCode(userCode) ||
parameters.get(OAuth2ParameterNames.USER_CODE).size() != 1) {
OAuth2EndpointUtils.throwError(
OAuth2ErrorCodes.INVALID_REQUEST,
Expand Down
3 changes: 3 additions & 0 deletions ...ramework/security/oauth2/server/authorization/web/authentication/OAuth2EndpointUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,7 @@ static String normalizeUserCode(String userCode) {
return sb.toString();
}

static boolean validateUserCode(String userCode) {
return userCode != null && userCode.toUpperCase().replaceAll("[^A-Z\\d]+", "").length() == 8;
}
}
16 changes: 16 additions & 0 deletions ...tion/web/authentication/OAuth2DeviceAuthorizationConsentAuthenticationConverterTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,22 @@ public void convertWhenEmptyUserCodeThenInvalidRequestError() {
// @formatter:on
}

@Test
public void convertWhenInvalidUserCodeThenInvalidRequestError() {
MockHttpServletRequest request = createRequest();
request.addParameter(OAuth2ParameterNames.STATE, STATE);
request.addParameter(OAuth2ParameterNames.CLIENT_ID, CLIENT_ID);
request.addParameter(OAuth2ParameterNames.USER_CODE, "LONG-USER-CODE");
// @formatter:off
assertThatExceptionOfType(OAuth2AuthenticationException.class)
.isThrownBy(() -> this.converter.convert(request))
.withMessageContaining(OAuth2ParameterNames.USER_CODE)
.extracting(OAuth2AuthenticationException::getError)
.extracting(OAuth2Error::getErrorCode)
.isEqualTo(OAuth2ErrorCodes.INVALID_REQUEST);
// @formatter:on
}

@Test
public void convertWhenMultipleUserCodeParametersThenInvalidRequestError() {
MockHttpServletRequest request = createRequest();
Expand Down
14 changes: 14 additions & 0 deletions ...uthorization/web/authentication/OAuth2DeviceVerificationAuthenticationConverterTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,20 @@ public void convertWhenEmptyUserCodeParameterThenInvalidRequestError() {
// @formatter:on
}

@Test
public void convertWhenInvalidUserCodeParameterThenInvalidRequestError() {
MockHttpServletRequest request = createRequest();
request.addParameter(OAuth2ParameterNames.USER_CODE, "LONG-USER-CODE");
// @formatter:off
assertThatExceptionOfType(OAuth2AuthenticationException.class)
.isThrownBy(() -> this.converter.convert(request))
.withMessageContaining(OAuth2ParameterNames.USER_CODE)
.extracting(OAuth2AuthenticationException::getError)
.extracting(OAuth2Error::getErrorCode)
.isEqualTo(OAuth2ErrorCodes.INVALID_REQUEST);
// @formatter:on
}

@Test
public void convertWhenMultipleUserCodeParameterThenInvalidRequestError() {
MockHttpServletRequest request = createRequest();
Expand Down