From 82b70595ad336dd26d1c27f4a4cb6147f65ae0c3 Mon Sep 17 00:00:00 2001 From: Martin Bogusz Date: Thu, 20 Jul 2023 16:06:43 +0200 Subject: [PATCH 1/2] Fix userCode validation Issue gh-44 --- ...horizationConsentAuthenticationConverter.java | 2 +- ...eviceVerificationAuthenticationConverter.java | 4 +--- .../web/authentication/OAuth2EndpointUtils.java | 3 +++ ...ationConsentAuthenticationConverterTests.java | 16 ++++++++++++++++ ...VerificationAuthenticationConverterTests.java | 14 ++++++++++++++ 5 files changed, 35 insertions(+), 4 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceAuthorizationConsentAuthenticationConverter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceAuthorizationConsentAuthenticationConverter.java index 54cd5c8f..17625281 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceAuthorizationConsentAuthenticationConverter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceAuthorizationConsentAuthenticationConverter.java @@ -80,7 +80,7 @@ public final class OAuth2DeviceAuthorizationConsentAuthenticationConverter imple // 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, diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceVerificationAuthenticationConverter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceVerificationAuthenticationConverter.java index 98c09056..ac1ebf55 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceVerificationAuthenticationConverter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceVerificationAuthenticationConverter.java @@ -30,7 +30,6 @@ import org.springframework.security.oauth2.server.authorization.authentication.O 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 @@ -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()))) { @@ -64,7 +62,7 @@ public final class OAuth2DeviceVerificationAuthenticationConverter implements Au // 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, diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2EndpointUtils.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2EndpointUtils.java index 9e2473d3..192489e3 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2EndpointUtils.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2EndpointUtils.java @@ -95,4 +95,7 @@ final class OAuth2EndpointUtils { return sb.toString(); } + static boolean validateUserCode(String userCode) { + return userCode != null && userCode.toUpperCase().replaceAll("[^A-Z\\d]+", "").length() == 8; + } } diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceAuthorizationConsentAuthenticationConverterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceAuthorizationConsentAuthenticationConverterTests.java index fc87a444..dfa5ed68 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceAuthorizationConsentAuthenticationConverterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceAuthorizationConsentAuthenticationConverterTests.java @@ -147,6 +147,22 @@ public class OAuth2DeviceAuthorizationConsentAuthenticationConverterTests { // @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(); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceVerificationAuthenticationConverterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceVerificationAuthenticationConverterTests.java index 843d0ee7..d7d17980 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceVerificationAuthenticationConverterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceVerificationAuthenticationConverterTests.java @@ -94,6 +94,20 @@ public class OAuth2DeviceVerificationAuthenticationConverterTests { // @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(); From 215c101c3d3a805cd4da89931a9bf3bbb7c72ca8 Mon Sep 17 00:00:00 2001 From: Steve Riesenberg Date: Thu, 27 Jul 2023 13:58:14 -0500 Subject: [PATCH 2/2] Polish gh-1309 --- .../OAuth2DeviceVerificationAuthenticationConverter.java | 1 + .../authorization/web/authentication/OAuth2EndpointUtils.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceVerificationAuthenticationConverter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceVerificationAuthenticationConverter.java index ac1ebf55..ee90d815 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceVerificationAuthenticationConverter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceVerificationAuthenticationConverter.java @@ -48,6 +48,7 @@ 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()))) { diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2EndpointUtils.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2EndpointUtils.java index 192489e3..9469e23f 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2EndpointUtils.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2EndpointUtils.java @@ -96,6 +96,6 @@ final class OAuth2EndpointUtils { } static boolean validateUserCode(String userCode) { - return userCode != null && userCode.toUpperCase().replaceAll("[^A-Z\\d]+", "").length() == 8; + return (userCode != null && userCode.toUpperCase().replaceAll("[^A-Z\\d]+", "").length() == 8); } }