From b1e0b656fb00724e44bbdebe863ce95808dbdb6b Mon Sep 17 00:00:00 2001 From: Selene Feigl Date: Wed, 25 Sep 2024 12:56:21 +0200 Subject: [PATCH 1/4] #1714: Invalid verification_uri in device authorization response when context path exists --- .../web/OAuth2DeviceAuthorizationEndpointFilter.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceAuthorizationEndpointFilter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceAuthorizationEndpointFilter.java index ae2190702..cf7cd653f 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceAuthorizationEndpointFilter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceAuthorizationEndpointFilter.java @@ -52,6 +52,7 @@ import org.springframework.util.Assert; import org.springframework.web.filter.OncePerRequestFilter; import org.springframework.web.util.UriComponentsBuilder; +import org.springframework.web.util.UrlPathHelper; /** * A {@code Filter} for the OAuth 2.0 Device Authorization endpoint, which handles the @@ -219,10 +220,15 @@ private void sendDeviceAuthorizationResponse(HttpServletRequest request, HttpSer OAuth2DeviceCode deviceCode = deviceAuthorizationRequestAuthentication.getDeviceCode(); OAuth2UserCode userCode = deviceAuthorizationRequestAuthentication.getUserCode(); + String relativeVerificationPath = this.verificationUri.startsWith("/") + ? this.verificationUri.substring(1) + : this.verificationUri; + // Generate the fully-qualified verification URI UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder .fromHttpUrl(UrlUtils.buildFullRequestUrl(request)) - .replacePath(this.verificationUri); + .replacePath(UrlPathHelper.defaultInstance.getContextPath(request)) + .pathSegment(relativeVerificationPath); String verificationUri = uriComponentsBuilder.build().toUriString(); // @formatter:off String verificationUriComplete = uriComponentsBuilder From 21c58a4d01093bd04c9290a84a0ba52e84b20525 Mon Sep 17 00:00:00 2001 From: Selene Feigl Date: Wed, 25 Sep 2024 14:28:43 +0200 Subject: [PATCH 2/4] Strip of the query part when building the verification URI The query part of the request URI was copied to the verification URI - which is - in my opinion - not the expected behaviour. According to https://datatracker.ietf.org/doc/html/rfc8628#section-3.1 "Parameters sent without a value MUST be treated as if they were omitted from the request. The authorization server MUST ignore unrecognized request parameters. Request and response parameters MUST NOT be included more than once." Passing through the request parameters is - IMHO - a different behaviour than ignoring them. --- .../web/OAuth2DeviceAuthorizationEndpointFilter.java | 1 + 1 file changed, 1 insertion(+) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceAuthorizationEndpointFilter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceAuthorizationEndpointFilter.java index cf7cd653f..a55787b5c 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceAuthorizationEndpointFilter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceAuthorizationEndpointFilter.java @@ -228,6 +228,7 @@ private void sendDeviceAuthorizationResponse(HttpServletRequest request, HttpSer UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder .fromHttpUrl(UrlUtils.buildFullRequestUrl(request)) .replacePath(UrlPathHelper.defaultInstance.getContextPath(request)) + .replaceQuery(null) .pathSegment(relativeVerificationPath); String verificationUri = uriComponentsBuilder.build().toUriString(); // @formatter:off From ff3bc965b456282e2d7a3fe1f1426e45e55b2bee Mon Sep 17 00:00:00 2001 From: Selene Feigl Date: Wed, 2 Oct 2024 11:35:11 +0200 Subject: [PATCH 3/4] Move building of verification URI to internal method using RedirectUrlBuilder --- ...uth2DeviceAuthorizationEndpointFilter.java | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceAuthorizationEndpointFilter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceAuthorizationEndpointFilter.java index a55787b5c..27dd4f6e7 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceAuthorizationEndpointFilter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceAuthorizationEndpointFilter.java @@ -46,13 +46,13 @@ import org.springframework.security.web.authentication.AuthenticationFailureHandler; import org.springframework.security.web.authentication.AuthenticationSuccessHandler; import org.springframework.security.web.authentication.WebAuthenticationDetailsSource; +import org.springframework.security.web.util.RedirectUrlBuilder; import org.springframework.security.web.util.UrlUtils; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; import org.springframework.web.filter.OncePerRequestFilter; import org.springframework.web.util.UriComponentsBuilder; -import org.springframework.web.util.UrlPathHelper; /** * A {@code Filter} for the OAuth 2.0 Device Authorization endpoint, which handles the @@ -220,16 +220,8 @@ private void sendDeviceAuthorizationResponse(HttpServletRequest request, HttpSer OAuth2DeviceCode deviceCode = deviceAuthorizationRequestAuthentication.getDeviceCode(); OAuth2UserCode userCode = deviceAuthorizationRequestAuthentication.getUserCode(); - String relativeVerificationPath = this.verificationUri.startsWith("/") - ? this.verificationUri.substring(1) - : this.verificationUri; - // Generate the fully-qualified verification URI - UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder - .fromHttpUrl(UrlUtils.buildFullRequestUrl(request)) - .replacePath(UrlPathHelper.defaultInstance.getContextPath(request)) - .replaceQuery(null) - .pathSegment(relativeVerificationPath); + UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUriString(resolveVerificationUri(request)); String verificationUri = uriComponentsBuilder.build().toUriString(); // @formatter:off String verificationUriComplete = uriComponentsBuilder @@ -249,4 +241,17 @@ private void sendDeviceAuthorizationResponse(HttpServletRequest request, HttpSer this.deviceAuthorizationHttpResponseConverter.write(deviceAuthorizationResponse, null, httpResponse); } + private String resolveVerificationUri(HttpServletRequest request) { + if (UrlUtils.isAbsoluteUrl(this.verificationUri)) { + return this.verificationUri; + } + RedirectUrlBuilder urlBuilder = new RedirectUrlBuilder(); + urlBuilder.setScheme(request.getScheme()); + urlBuilder.setServerName(request.getServerName()); + urlBuilder.setPort(request.getServerPort()); + urlBuilder.setContextPath(request.getContextPath()); + urlBuilder.setPathInfo(this.verificationUri); + return urlBuilder.getUrl(); + } + } From 893b6d25f48321ff1b6c99b6281238ae9acb3a8b Mon Sep 17 00:00:00 2001 From: Selene Feigl Date: Thu, 3 Oct 2024 09:35:47 +0200 Subject: [PATCH 4/4] Add test for device authentication request with context path set --- ...eviceAuthorizationEndpointFilterTests.java | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceAuthorizationEndpointFilterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceAuthorizationEndpointFilterTests.java index 9b9e18ed4..8b09ec169 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceAuthorizationEndpointFilterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceAuthorizationEndpointFilterTests.java @@ -241,6 +241,39 @@ public void doFilterWhenDeviceAuthorizationRequestThenDeviceAuthorizationRespons assertThat(deviceCode.getExpiresAt()).isAfter(deviceCode.getIssuedAt()); } + @Test + public void doFilterWhenDeviceAuthorizationRequestWithContextPathThenDeviceAuthorizationResponse() throws Exception { + Authentication authenticationResult = createAuthentication(); + given(this.authenticationManager.authenticate(any(Authentication.class))).willReturn(authenticationResult); + + Authentication clientPrincipal = (Authentication) authenticationResult.getPrincipal(); + mockSecurityContext(clientPrincipal); + + MockHttpServletRequest request = createRequest(); + request.setContextPath("/contextPath"); + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + this.filter.doFilter(request, response, filterChain); + assertThat(response.getStatus()).isEqualTo(HttpStatus.OK.value()); + + ArgumentCaptor deviceAuthorizationRequestAuthenticationCaptor = ArgumentCaptor + .forClass(OAuth2DeviceAuthorizationRequestAuthenticationToken.class); + verify(this.authenticationManager).authenticate(deviceAuthorizationRequestAuthenticationCaptor.capture()); + verifyNoInteractions(filterChain); + + OAuth2DeviceAuthorizationResponse deviceAuthorizationResponse = readDeviceAuthorizationResponse(response); + String verificationUri = ISSUER_URI + "/contextPath" + VERIFICATION_URI; + assertThat(deviceAuthorizationResponse.getVerificationUri()).isEqualTo(verificationUri); + assertThat(deviceAuthorizationResponse.getVerificationUriComplete()) + .isEqualTo("%s?%s=%s".formatted(verificationUri, OAuth2ParameterNames.USER_CODE, USER_CODE)); + OAuth2DeviceCode deviceCode = deviceAuthorizationResponse.getDeviceCode(); + assertThat(deviceCode.getTokenValue()).isEqualTo(DEVICE_CODE); + assertThat(deviceCode.getExpiresAt()).isAfter(deviceCode.getIssuedAt()); + OAuth2UserCode userCode = deviceAuthorizationResponse.getUserCode(); + assertThat(userCode.getTokenValue()).isEqualTo(USER_CODE); + assertThat(deviceCode.getExpiresAt()).isAfter(deviceCode.getIssuedAt()); + } + @Test public void doFilterWhenInvalidRequestErrorThenBadRequest() throws Exception { AuthenticationConverter authenticationConverter = mock(AuthenticationConverter.class);