Browse Source

Polish gh-16551

Steve Riesenberg 6 months ago
parent
commit
7fc5d50adf

+ 1 - 1
docs/modules/ROOT/pages/reactive/oauth2/login/logout.adoc

@@ -126,7 +126,7 @@ If used, the application's base URL, such as `https://app.example.org`, replaces
 [NOTE]
 ====
 By default, `OidcClientInitiatedServerLogoutSuccessHandler` redirects to the logout URL using a standard HTTP redirect with the `GET` method.
-To perform the logout using a `POST` request, set the redirect strategy to `ServerFormPostRedirectStrategy`, for example with `OidcClientInitiatedServerLogoutSuccessHandler.setRedirectStrategy(new ServerFormPostRedirectStrategy())`.
+To perform the logout using a `POST` request, set the redirect strategy to `FormPostServerRedirectStrategy`, for example with `OidcClientInitiatedServerLogoutSuccessHandler.setRedirectStrategy(new ServerFormPostRedirectStrategy())`.
 ====
 
 [[configure-provider-initiated-oidc-logout]]

+ 1 - 1
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/web/server/logout/OidcClientInitiatedServerLogoutSuccessHandlerTests.java

@@ -229,7 +229,7 @@ public class OidcClientInitiatedServerLogoutSuccessHandlerTests {
 	}
 
 	@Test
-	public void logoutWhenCustomRedirectStrategySetThenCustomRedirectStrategyUse() {
+	public void logoutWhenCustomRedirectStrategySetThenCustomRedirectStrategyUsed() {
 		ServerRedirectStrategy redirectStrategy = mock(ServerRedirectStrategy.class);
 		given(redirectStrategy.sendRedirect(any(), any())).willReturn(Mono.empty());
 		OAuth2AuthenticationToken token = new OAuth2AuthenticationToken(TestOidcUsers.create(),

+ 29 - 30
web/src/main/java/org/springframework/security/web/server/ServerFormPostRedirectStrategy.java → web/src/main/java/org/springframework/security/web/server/FormPostServerRedirectStrategy.java

@@ -26,6 +26,7 @@ import reactor.core.publisher.Mono;
 
 import org.springframework.core.io.buffer.DataBuffer;
 import org.springframework.core.io.buffer.DataBufferFactory;
+import org.springframework.core.io.buffer.DataBufferUtils;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.http.server.reactive.ServerHttpResponse;
@@ -41,15 +42,13 @@ import org.springframework.web.util.UriComponentsBuilder;
  * data instead of query string data.
  *
  * @author Max Batischev
+ * @author Steve Riesenberg
  * @since 6.5
  */
-public final class ServerFormPostRedirectStrategy implements ServerRedirectStrategy {
+public final class FormPostServerRedirectStrategy implements ServerRedirectStrategy {
 
 	private static final String CONTENT_SECURITY_POLICY_HEADER = "Content-Security-Policy";
 
-	private static final StringKeyGenerator DEFAULT_NONCE_GENERATOR = new Base64StringKeyGenerator(
-			Base64.getUrlEncoder().withoutPadding(), 96);
-
 	private static final String REDIRECT_PAGE_TEMPLATE = """
 			<!DOCTYPE html>
 			<html lang="en">
@@ -79,46 +78,46 @@ public final class ServerFormPostRedirectStrategy implements ServerRedirectStrat
 			<input name="{{name}}" type="hidden" value="{{value}}" />
 			""";
 
+	private static final StringKeyGenerator DEFAULT_NONCE_GENERATOR = new Base64StringKeyGenerator(
+			Base64.getUrlEncoder().withoutPadding(), 96);
+
 	@Override
 	public Mono<Void> sendRedirect(ServerWebExchange exchange, URI location) {
-		String nonce = DEFAULT_NONCE_GENERATOR.generateKey();
-		String policyDirective = "script-src 'nonce-%s'".formatted(nonce);
-
-		ServerHttpResponse response = exchange.getResponse();
-		response.setStatusCode(HttpStatus.OK);
-		response.getHeaders().setContentType(MediaType.TEXT_HTML);
-		response.getHeaders().add(CONTENT_SECURITY_POLICY_HEADER, policyDirective);
-		return response.writeWith(createBuffer(exchange, location, nonce));
-	}
+		final UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUri(location);
 
-	private Mono<DataBuffer> createBuffer(ServerWebExchange exchange, URI location, String nonce) {
-		byte[] bytes = createPage(location, nonce);
-		DataBufferFactory bufferFactory = exchange.getResponse().bufferFactory();
-		return Mono.just(bufferFactory.wrap(bytes));
-	}
-
-	private byte[] createPage(URI location, String nonce) {
-		UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUri(location);
-
-		StringBuilder hiddenInputsHtmlBuilder = new StringBuilder();
+		final StringBuilder hiddenInputsHtmlBuilder = new StringBuilder();
 		for (final Map.Entry<String, List<String>> entry : uriComponentsBuilder.build().getQueryParams().entrySet()) {
 			final String name = entry.getKey();
 			for (final String value : entry.getValue()) {
 				// @formatter:off
 				final String hiddenInput = HIDDEN_INPUT_TEMPLATE
-						.replace("{{name}}", HtmlUtils.htmlEscape(name))
-						.replace("{{value}}", HtmlUtils.htmlEscape(value));
+					.replace("{{name}}", HtmlUtils.htmlEscape(name))
+					.replace("{{value}}", HtmlUtils.htmlEscape(value));
 				// @formatter:on
 				hiddenInputsHtmlBuilder.append(hiddenInput.trim());
 			}
 		}
+
+		// Create the script-src policy directive for the Content-Security-Policy header
+		final String nonce = DEFAULT_NONCE_GENERATOR.generateKey();
+		final String policyDirective = "script-src 'nonce-%s'".formatted(nonce);
+
 		// @formatter:off
-		return REDIRECT_PAGE_TEMPLATE
-				.replace("{{action}}", HtmlUtils.htmlEscape(uriComponentsBuilder.query(null).build().toUriString()))
-				.replace("{{params}}", hiddenInputsHtmlBuilder.toString())
-				.replace("{{nonce}}", HtmlUtils.htmlEscape(nonce))
-				.getBytes(StandardCharsets.UTF_8);
+		final String html = REDIRECT_PAGE_TEMPLATE
+			// Clear the query string as we don't want that to be part of the form action URL
+			.replace("{{action}}", HtmlUtils.htmlEscape(uriComponentsBuilder.query(null).build().toUriString()))
+			.replace("{{params}}", hiddenInputsHtmlBuilder.toString())
+			.replace("{{nonce}}", HtmlUtils.htmlEscape(nonce));
 		// @formatter:on
+
+		final ServerHttpResponse response = exchange.getResponse();
+		response.setStatusCode(HttpStatus.OK);
+		response.getHeaders().setContentType(MediaType.TEXT_HTML);
+		response.getHeaders().set(CONTENT_SECURITY_POLICY_HEADER, policyDirective);
+
+		final DataBufferFactory bufferFactory = response.bufferFactory();
+		final DataBuffer buffer = bufferFactory.wrap(html.getBytes(StandardCharsets.UTF_8));
+		return response.writeWith(Mono.just(buffer)).doOnError((error) -> DataBufferUtils.release(buffer));
 	}
 
 }

+ 5 - 5
web/src/test/java/org/springframework/security/web/server/ServerFormPostRedirectStrategyTests.java → web/src/test/java/org/springframework/security/web/server/FormPostServerRedirectStrategyTests.java

@@ -30,15 +30,15 @@ import org.springframework.mock.web.server.MockServerWebExchange;
 import static org.assertj.core.api.Assertions.assertThat;
 
 /**
- * Tests for {@link ServerFormPostRedirectStrategy}.
+ * Tests for {@link FormPostServerRedirectStrategy}.
  *
  * @author Max Batischev
  */
-public class ServerFormPostRedirectStrategyTests {
+public class FormPostServerRedirectStrategyTests {
 
 	private static final String POLICY_DIRECTIVE_PATTERN = "script-src 'nonce-(.+)'";
 
-	private final ServerRedirectStrategy redirectStrategy = new ServerFormPostRedirectStrategy();
+	private final ServerRedirectStrategy redirectStrategy = new FormPostServerRedirectStrategy();
 
 	private final MockServerHttpRequest request = MockServerHttpRequest.get("https://localhost").build();
 
@@ -89,7 +89,7 @@ public class ServerFormPostRedirectStrategyTests {
 	}
 
 	@Test
-	public void redirectWhenLocationAbsoluteUilWithQueryParamsIsPresentThenRedirect() {
+	public void redirectWhenLocationAbsoluteUriWithQueryParamsIsPresentThenRedirect() {
 		this.redirectStrategy
 			.sendRedirect(this.webExchange, URI.create("https://example.com/path?param1=one&param2=two#fragment"))
 			.block();
@@ -105,7 +105,7 @@ public class ServerFormPostRedirectStrategyTests {
 
 	private ThrowingConsumer<MockServerHttpResponse> hasScriptSrcNonce() {
 		return (response) -> {
-			final String policyDirective = response.getHeaders().get("Content-Security-Policy").get(0);
+			final String policyDirective = response.getHeaders().getFirst("Content-Security-Policy");
 			assertThat(policyDirective).isNotEmpty();
 			assertThat(policyDirective).matches(POLICY_DIRECTIVE_PATTERN);