From 116b041f4c1582e9c2a730c162292a7e55f3a201 Mon Sep 17 00:00:00 2001 From: Tran Ngoc Nhan Date: Fri, 20 Dec 2024 00:54:14 +0700 Subject: [PATCH] Encode clientId and clientSecret Closes gh-15988 --- .../SpringOpaqueTokenIntrospector.java | 99 +++++++++++++++++ ...SpringReactiveOpaqueTokenIntrospector.java | 100 ++++++++++++++++++ .../SpringOpaqueTokenIntrospectorTests.java | 45 ++++++++ ...gReactiveOpaqueTokenIntrospectorTests.java | 47 ++++++++ 4 files changed, 291 insertions(+) diff --git a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/SpringOpaqueTokenIntrospector.java b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/SpringOpaqueTokenIntrospector.java index 4674ab78868..4b53fd54884 100644 --- a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/SpringOpaqueTokenIntrospector.java +++ b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/SpringOpaqueTokenIntrospector.java @@ -18,6 +18,8 @@ import java.io.Serial; import java.net.URI; +import java.net.URLEncoder; +import java.nio.charset.Charset; import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; @@ -79,7 +81,9 @@ public class SpringOpaqueTokenIntrospector implements OpaqueTokenIntrospector { * @param introspectionUri The introspection endpoint uri * @param clientId The client id authorized to introspect * @param clientSecret The client's secret + * @deprecated Please use {@link SpringOpaqueTokenIntrospector.Builder} */ + @Deprecated(since = "6.5", forRemoval = true) public SpringOpaqueTokenIntrospector(String introspectionUri, String clientId, String clientSecret) { Assert.notNull(introspectionUri, "introspectionUri cannot be null"); Assert.notNull(clientId, "clientId cannot be null"); @@ -269,6 +273,18 @@ private Collection authorities(List scopes) { return authorities; } + /** + * Creates a {@code SpringOpaqueTokenIntrospector.Builder} with the given + * introspection endpoint uri + * @param introspectionUri The introspection endpoint uri + * @return the {@link SpringOpaqueTokenIntrospector.Builder} + * @since 6.5 + */ + public static Builder withIntrospectionUri(String introspectionUri) { + Assert.notNull(introspectionUri, "introspectionUri cannot be null"); + return new Builder(introspectionUri); + } + // gh-7563 private static final class ArrayListFromString extends ArrayList { @@ -295,4 +311,87 @@ default List getScopes() { } + /** + * Used to build {@link SpringOpaqueTokenIntrospector}. + * + * @author Ngoc Nhan + * @since 6.5 + */ + public static final class Builder { + + private final String introspectionUri; + + private String clientId; + + private String clientSecret; + + private Builder(String introspectionUri) { + this.introspectionUri = introspectionUri; + } + + /** + * Uses the given parameters to build {@code SpringOpaqueTokenIntrospector} + * @param clientId The client id authorized that should be encoded + * @param charset The charset to use + * @return the {@link SpringOpaqueTokenIntrospector.Builder} + * @since 6.5 + */ + public Builder clientId(String clientId, Charset charset) { + Assert.notNull(clientId, "clientId cannot be null"); + Assert.notNull(charset, "charset cannot be null"); + this.clientId = URLEncoder.encode(clientId, charset); + return this; + } + + /** + * Uses the given parameter to build {@code SpringOpaqueTokenIntrospector} + * @param clientId The client id authorized + * @return the {@link SpringOpaqueTokenIntrospector.Builder} + * @since 6.5 + */ + public Builder clientId(String clientId) { + Assert.notNull(clientId, "clientId cannot be null"); + this.clientId = clientId; + return this; + } + + /** + * Uses the given parameters to build {@code SpringOpaqueTokenIntrospector} + * @param clientSecret The client's secret that should be encoded + * @param charset The charset to use + * @return the {@link SpringOpaqueTokenIntrospector.Builder} + * @since 6.5 + */ + public Builder clientSecret(String clientSecret, Charset charset) { + Assert.notNull(clientSecret, "clientSecret cannot be null"); + Assert.notNull(charset, "charset cannot be null"); + this.clientSecret = URLEncoder.encode(clientSecret, charset); + return this; + } + + /** + * Uses the given parameter to build {@code SpringOpaqueTokenIntrospector} + * @param clientSecret The client's secret + * @return the {@link SpringOpaqueTokenIntrospector.Builder} + * @since 6.5 + */ + public Builder clientSecret(String clientSecret) { + Assert.notNull(clientSecret, "clientSecret cannot be null"); + this.clientSecret = clientSecret; + return this; + } + + /** + * Creates a {@code SpringOpaqueTokenIntrospector} + * @return the {@link SpringOpaqueTokenIntrospector} + * @since 6.5 + */ + public SpringOpaqueTokenIntrospector build() { + RestTemplate restTemplate = new RestTemplate(); + restTemplate.getInterceptors().add(new BasicAuthenticationInterceptor(this.clientId, this.clientSecret)); + return new SpringOpaqueTokenIntrospector(this.introspectionUri, restTemplate); + } + + } + } diff --git a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/SpringReactiveOpaqueTokenIntrospector.java b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/SpringReactiveOpaqueTokenIntrospector.java index 7c6bf8ecb05..b37a3e39f0a 100644 --- a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/SpringReactiveOpaqueTokenIntrospector.java +++ b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/SpringReactiveOpaqueTokenIntrospector.java @@ -18,6 +18,8 @@ import java.io.Serial; import java.net.URI; +import java.net.URLEncoder; +import java.nio.charset.Charset; import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; @@ -74,7 +76,9 @@ public class SpringReactiveOpaqueTokenIntrospector implements ReactiveOpaqueToke * @param introspectionUri The introspection endpoint uri * @param clientId The client id authorized to introspect * @param clientSecret The client secret for the authorized client + * @deprecated Please use {@link SpringReactiveOpaqueTokenIntrospector.Builder} */ + @Deprecated(since = "6.5", forRemoval = true) public SpringReactiveOpaqueTokenIntrospector(String introspectionUri, String clientId, String clientSecret) { Assert.hasText(introspectionUri, "introspectionUri cannot be empty"); Assert.hasText(clientId, "clientId cannot be empty"); @@ -223,6 +227,18 @@ private Collection authorities(List scopes) { return authorities; } + /** + * Creates a {@code SpringReactiveOpaqueTokenIntrospector.Builder} with the given + * introspection endpoint uri + * @param introspectionUri The introspection endpoint uri + * @return the {@link SpringReactiveOpaqueTokenIntrospector.Builder} + * @since 6.5 + */ + public static Builder withIntrospectionUri(String introspectionUri) { + + return new Builder(introspectionUri); + } + // gh-7563 private static final class ArrayListFromString extends ArrayList { @@ -249,4 +265,88 @@ default List getScopes() { } + /** + * Used to build {@link SpringReactiveOpaqueTokenIntrospector}. + * + * @author Ngoc Nhan + * @since 6.5 + */ + public static final class Builder { + + private final String introspectionUri; + + private String clientId; + + private String clientSecret; + + private Builder(String introspectionUri) { + this.introspectionUri = introspectionUri; + } + + /** + * Uses the given parameters to build {@code SpringOpaqueTokenIntrospector} + * @param clientId The client id authorized that should be encoded + * @param charset The charset to use + * @return the {@link SpringReactiveOpaqueTokenIntrospector.Builder} + * @since 6.5 + */ + public Builder clientId(String clientId, Charset charset) { + Assert.notNull(clientId, "clientId cannot be null"); + Assert.notNull(charset, "charset cannot be null"); + this.clientId = URLEncoder.encode(clientId, charset); + return this; + } + + /** + * Uses the given parameter to build {@code SpringOpaqueTokenIntrospector} + * @param clientId The client id authorized + * @return the {@link SpringReactiveOpaqueTokenIntrospector.Builder} + * @since 6.5 + */ + public Builder clientId(String clientId) { + Assert.notNull(clientId, "clientId cannot be null"); + this.clientId = clientId; + return this; + } + + /** + * Uses the given parameters to build {@code SpringOpaqueTokenIntrospector} + * @param clientSecret The client's secret that should be encoded + * @param charset The charset to use + * @return the {@link SpringReactiveOpaqueTokenIntrospector.Builder} + * @since 6.5 + */ + public Builder clientSecret(String clientSecret, Charset charset) { + Assert.notNull(clientSecret, "clientSecret cannot be null"); + Assert.notNull(charset, "charset cannot be null"); + this.clientSecret = URLEncoder.encode(clientSecret, charset); + return this; + } + + /** + * Uses the given parameter to build {@code SpringOpaqueTokenIntrospector} + * @param clientSecret The client's secret + * @return the {@link SpringReactiveOpaqueTokenIntrospector.Builder} + * @since 6.5 + */ + public Builder clientSecret(String clientSecret) { + Assert.notNull(clientSecret, "clientSecret cannot be null"); + this.clientSecret = clientSecret; + return this; + } + + /** + * Creates a {@code SpringReactiveOpaqueTokenIntrospector} + * @return the {@link SpringReactiveOpaqueTokenIntrospector} + * @since 6.5 + */ + public SpringReactiveOpaqueTokenIntrospector build() { + WebClient webClient = WebClient.builder() + .defaultHeaders((h) -> h.setBasicAuth(this.clientId, this.clientSecret)) + .build(); + return new SpringReactiveOpaqueTokenIntrospector(this.introspectionUri, webClient); + } + + } + } diff --git a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/SpringOpaqueTokenIntrospectorTests.java b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/SpringOpaqueTokenIntrospectorTests.java index 01555f01fd4..cedf0e321a8 100644 --- a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/SpringOpaqueTokenIntrospectorTests.java +++ b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/SpringOpaqueTokenIntrospectorTests.java @@ -17,6 +17,7 @@ package org.springframework.security.oauth2.server.resource.introspection; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.time.Instant; import java.util.Arrays; import java.util.Base64; @@ -339,6 +340,50 @@ public void setAuthenticationConverterWhenNonNullConverterGivenThenConverterUsed verify(authenticationConverter).convert(any()); } + @Test + public void introspectWithoutEncodeClientCredentialsThenExceptionIsThrown() throws Exception { + try (MockWebServer server = new MockWebServer()) { + String response = """ + { + "active": true, + "username": "client%&1" + } + """; + server.setDispatcher(requiresAuth("client%25%261", "secret%40%242", response)); + String introspectUri = server.url("/introspect").toString(); + OpaqueTokenIntrospector introspectionClient = new SpringOpaqueTokenIntrospector(introspectUri, "client%&1", + "secret@$2"); + assertThatExceptionOfType(OAuth2IntrospectionException.class) + .isThrownBy(() -> introspectionClient.introspect("token")); + } + } + + @Test + public void introspectWithEncodeClientCredentialsThenOk() throws Exception { + try (MockWebServer server = new MockWebServer()) { + String response = """ + { + "active": true, + "username": "client%&1" + } + """; + server.setDispatcher(requiresAuth("client%25%261", "secret%40%242", response)); + String introspectUri = server.url("/introspect").toString(); + OpaqueTokenIntrospector introspectionClient = SpringOpaqueTokenIntrospector + .withIntrospectionUri(introspectUri) + .clientId("client%&1", StandardCharsets.UTF_8) + .clientSecret("secret@$2", StandardCharsets.UTF_8) + .build(); + OAuth2AuthenticatedPrincipal authority = introspectionClient.introspect("token"); + // @formatter:off + assertThat(authority.getAttributes()) + .isNotNull() + .containsEntry(OAuth2TokenIntrospectionClaimNames.ACTIVE, true) + .containsEntry(OAuth2TokenIntrospectionClaimNames.USERNAME, "client%&1"); + // @formatter:on + } + } + private static ResponseEntity> response(String content) { HttpHeaders headers = new HttpHeaders(); headers.setContentType(MediaType.APPLICATION_JSON); diff --git a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/SpringReactiveOpaqueTokenIntrospectorTests.java b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/SpringReactiveOpaqueTokenIntrospectorTests.java index ae0f01afd7f..e6c80b4d6f5 100644 --- a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/SpringReactiveOpaqueTokenIntrospectorTests.java +++ b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/SpringReactiveOpaqueTokenIntrospectorTests.java @@ -17,6 +17,7 @@ package org.springframework.security.oauth2.server.resource.introspection; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.time.Instant; import java.util.Arrays; import java.util.Base64; @@ -261,6 +262,52 @@ public void constructorWhenRestOperationsIsNullThenIllegalArgumentException() { .isThrownBy(() -> new SpringReactiveOpaqueTokenIntrospector(INTROSPECTION_URL, null)); } + @Test + public void introspectWithoutEncodeClientCredentialsThenExceptionIsThrown() throws Exception { + try (MockWebServer server = new MockWebServer()) { + String response = """ + { + "active": true, + "username": "client%&1" + } + """; + server.setDispatcher(requiresAuth("client%25%261", "secret%40%242", response)); + String introspectUri = server.url("/introspect").toString(); + ReactiveOpaqueTokenIntrospector introspectionClient = new SpringReactiveOpaqueTokenIntrospector( + introspectUri, "client%&1", "secret@$2"); + // @formatter:off + assertThatExceptionOfType(OAuth2IntrospectionException.class) + .isThrownBy(() -> introspectionClient.introspect("token").block()); + // @formatter:on + } + } + + @Test + public void introspectWithEncodeClientCredentialsThenOk() throws Exception { + try (MockWebServer server = new MockWebServer()) { + String response = """ + { + "active": true, + "username": "client%&1" + } + """; + server.setDispatcher(requiresAuth("client%25%261", "secret%40%242", response)); + String introspectUri = server.url("/introspect").toString(); + ReactiveOpaqueTokenIntrospector introspectionClient = SpringReactiveOpaqueTokenIntrospector + .withIntrospectionUri(introspectUri) + .clientId("client%&1", StandardCharsets.UTF_8) + .clientSecret("secret@$2", StandardCharsets.UTF_8) + .build(); + OAuth2AuthenticatedPrincipal authority = introspectionClient.introspect("token").block(); + // @formatter:off + assertThat(authority.getAttributes()) + .isNotNull() + .containsEntry(OAuth2TokenIntrospectionClaimNames.ACTIVE, true) + .containsEntry(OAuth2TokenIntrospectionClaimNames.USERNAME, "client%&1"); + // @formatter:on + } + } + private WebClient mockResponse(String response) { return mockResponse(toMap(response)); }