Encode clientId and clientSecret

Closes gh-15988
spring-projects · Dec 21, 2024 · 116b041 · 116b041
1 parent fa58ebb
commit 116b041
Show file tree

Hide file tree

Showing 4 changed files with 291 additions and 0 deletions.
diff --git a/...ramework/security/oauth2/server/resource/introspection/SpringOpaqueTokenIntrospector.java b/...ramework/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<GrantedAuthority> authorities(List<String> 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<String> {
 
@@ -295,4 +311,87 @@ default List<String> 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/.../security/oauth2/server/resource/introspection/SpringReactiveOpaqueTokenIntrospector.java b/.../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<GrantedAuthority> authorities(List<String> 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<String> {
 
@@ -249,4 +265,88 @@ default List<String> 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/...ork/security/oauth2/server/resource/introspection/SpringOpaqueTokenIntrospectorTests.java b/...ork/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<Map<String, Object>> response(String content) {
 		HttpHeaders headers = new HttpHeaders();
 		headers.setContentType(MediaType.APPLICATION_JSON);

diff --git a/...rity/oauth2/server/resource/introspection/SpringReactiveOpaqueTokenIntrospectorTests.java b/...rity/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));
 	}