From 3836091fe29aa92db05db0fc82b7b4d950a4402f Mon Sep 17 00:00:00 2001 From: Joe Grandja <10884212+jgrandja@users.noreply.github.com> Date: Wed, 8 Jul 2026 08:07:45 -0400 Subject: [PATCH] Apply html encoding in DefaultConsentPage --- .../authorization/web/DefaultConsentPage.java | 30 ++++++++++------ ...Auth2AuthorizationEndpointFilterTests.java | 34 +++++++++++++++++++ 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/DefaultConsentPage.java b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/DefaultConsentPage.java index 20ecdd9be1..d953c01fbb 100644 --- a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/DefaultConsentPage.java +++ b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/DefaultConsentPage.java @@ -29,6 +29,7 @@ import org.springframework.http.MediaType; import org.springframework.security.core.Authentication; import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; import org.springframework.security.oauth2.core.oidc.OidcScopes; +import org.springframework.web.util.HtmlUtils; /** * For internal use only. @@ -56,6 +57,12 @@ final class DefaultConsentPage { private static String generateConsentPage(HttpServletRequest request, String clientId, Authentication principal, Set requestedScopes, Set authorizedScopes, String state, Map additionalParameters) { + + String encodedClientId = HtmlUtils.htmlEscape(clientId); + String encodedState = HtmlUtils.htmlEscape(state); + String encodedPrincipalName = HtmlUtils.htmlEscape(principal.getName()); + String encodedRequestUri = HtmlUtils.htmlEscape(request.getRequestURI()); + Set scopesToAuthorize = new HashSet<>(); Set scopesPreviouslyAuthorized = new HashSet<>(); for (String scope : requestedScopes) { @@ -74,6 +81,7 @@ final class DefaultConsentPage { // the "user_code" being displayed on the device to confirm they are // authorizing the correct device. String userCode = additionalParameters.get(OAuth2ParameterNames.USER_CODE); + String encodedUserCode = (userCode != null) ? HtmlUtils.htmlEscape(userCode) : null; // @formatter:off StringBuilder builder = new StringBuilder(); @@ -98,13 +106,13 @@ final class DefaultConsentPage { builder.append(" "); builder.append("
"); builder.append("
"); - builder.append("

" + clientId + " wants to access your account " + principal.getName() + "

"); + builder.append("

" + encodedClientId + " wants to access your account " + encodedPrincipalName + "

"); builder.append("
"); builder.append("
"); if (userCode != null) { builder.append("
"); builder.append("
"); - builder.append("

You have provided the code " + userCode + ". Verify that this code matches what is shown on your device.

"); + builder.append("

You have provided the code " + encodedUserCode + ". Verify that this code matches what is shown on your device.

"); builder.append("
"); builder.append("
"); } @@ -115,26 +123,28 @@ final class DefaultConsentPage { builder.append(" "); builder.append("
"); builder.append("
"); - builder.append("
"); - builder.append(" "); - builder.append(" "); + builder.append(" "); + builder.append(" "); + builder.append(" "); if (userCode != null) { - builder.append(" "); + builder.append(" "); } for (String scope : scopesToAuthorize) { + String encodedScope = HtmlUtils.htmlEscape(scope); builder.append("
"); - builder.append(" "); - builder.append(" "); + builder.append(" "); + builder.append(" "); builder.append("
"); } if (!scopesPreviouslyAuthorized.isEmpty()) { builder.append("

You have already granted the following permissions to the above app:

"); for (String scope : scopesPreviouslyAuthorized) { + String encodedScope = HtmlUtils.htmlEscape(scope); builder.append("
"); - builder.append(" "); - builder.append(" "); + builder.append(" "); + builder.append(" "); builder.append("
"); } } diff --git a/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java b/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java index e37cf233ae..9b107c835e 100644 --- a/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java +++ b/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java @@ -64,6 +64,7 @@ import org.springframework.security.web.authentication.AuthenticationSuccessHand import org.springframework.security.web.authentication.WebAuthenticationDetails; import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy; import org.springframework.util.StringUtils; +import org.springframework.web.util.HtmlUtils; import org.springframework.web.util.UriComponentsBuilder; import static org.assertj.core.api.Assertions.assertThat; @@ -581,6 +582,39 @@ public class OAuth2AuthorizationEndpointFilterTests { } } + @Test + public void doFilterWhenAuthorizationRequestConsentRequiredThenConsentResponseHtmlEscaped() throws Exception { + String unencodedScope = ""; + String unencodedState = ""; + Set requestedScopes = new HashSet<>(Arrays.asList("scope1", unencodedScope)); + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().scopes((scopes) -> { + scopes.clear(); + scopes.addAll(requestedScopes); + }).build(); + // No scopes previously approved + OAuth2AuthorizationConsentAuthenticationToken authorizationConsentAuthenticationResult = new OAuth2AuthorizationConsentAuthenticationToken( + AUTHORIZATION_URI, registeredClient.getClientId(), this.principal, unencodedState, new HashSet<>(), + null); + authorizationConsentAuthenticationResult.setAuthenticated(true); + given(this.authenticationManager.authenticate(any())).willReturn(authorizationConsentAuthenticationResult); + + MockHttpServletRequest request = createAuthorizationRequest(registeredClient); + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + + this.filter.doFilter(request, response, filterChain); + + verify(this.authenticationManager).authenticate(any()); + verifyNoInteractions(filterChain); + + assertThat(response.getStatus()).isEqualTo(HttpStatus.OK.value()); + String html = response.getContentAsString(); + assertThat(html).doesNotContain(unencodedScope); + assertThat(html).doesNotContain(unencodedState); + assertThat(html).contains(HtmlUtils.htmlEscape(unencodedScope)); + assertThat(html).contains("name=\"state\" value=\"" + HtmlUtils.htmlEscape(unencodedState) + "\""); + } + @Test public void doFilterWhenAuthorizationRequestAuthenticatedThenAuthorizationResponse() throws Exception { RegisteredClient registeredClient = TestRegisteredClients.registeredClient().redirectUris((redirectUris) -> {