From 641b201b17fda33f603d9563c7ca6c620e8a74b2 Mon Sep 17 00:00:00 2001 From: doctormacky Date: Thu, 18 Aug 2022 22:14:33 +0800 Subject: [PATCH 1/4] validate client secret expired or not --- .../authentication/ClientSecretAuthenticationProvider.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java index 61eadb7bb..d25f94589 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java @@ -15,6 +15,8 @@ */ package org.springframework.security.oauth2.server.authorization.authentication; +import java.time.Instant; + import org.springframework.security.authentication.AuthenticationProvider; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; @@ -93,6 +95,11 @@ public Authentication authenticate(Authentication authentication) throws Authent throwInvalidClient(OAuth2ParameterNames.CLIENT_ID); } + Instant expiredAt = registeredClient.getClientSecretExpiresAt(); + if (expiredAt!=null && Instant.now().isAfter(expiredAt)) { + throwInvalidClient("client_secret_expires_at"); + } + if (!registeredClient.getClientAuthenticationMethods().contains( clientAuthentication.getClientAuthenticationMethod())) { throwInvalidClient("authentication_method"); From 15437db2c3165edb8ef83c4bae1ae312b1dca913 Mon Sep 17 00:00:00 2001 From: doctormacky Date: Fri, 19 Aug 2022 21:21:09 +0800 Subject: [PATCH 2/4] add negative unit test case on ClientSecretAuthenticationProviderTests --- .../ClientSecretAuthenticationProvider.java | 10 +++++----- ...lientSecretAuthenticationProviderTests.java | 18 ++++++++++++++++++ .../client/TestRegisteredClients.java | 16 ++++++++++++++++ 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java index d25f94589..974952dcf 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java @@ -95,11 +95,6 @@ public Authentication authenticate(Authentication authentication) throws Authent throwInvalidClient(OAuth2ParameterNames.CLIENT_ID); } - Instant expiredAt = registeredClient.getClientSecretExpiresAt(); - if (expiredAt!=null && Instant.now().isAfter(expiredAt)) { - throwInvalidClient("client_secret_expires_at"); - } - if (!registeredClient.getClientAuthenticationMethods().contains( clientAuthentication.getClientAuthenticationMethod())) { throwInvalidClient("authentication_method"); @@ -114,6 +109,11 @@ public Authentication authenticate(Authentication authentication) throws Authent throwInvalidClient(OAuth2ParameterNames.CLIENT_SECRET); } + if (registeredClient.getClientSecretExpiresAt() != null && + Instant.now().isAfter(registeredClient.getClientSecretExpiresAt())) { + throwInvalidClient("client_secret_expires_at"); + } + // Validate the "code_verifier" parameter for the confidential client, if available this.codeVerifierAuthenticator.authenticateIfAvailable(clientAuthentication, registeredClient); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProviderTests.java index d20d29237..421126424 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProviderTests.java @@ -182,6 +182,24 @@ public void authenticateWhenInvalidClientSecretThenThrowOAuth2AuthenticationExce verify(this.passwordEncoder).matches(any(), any()); } + @Test + public void authenticateWhenExpiredClientSecretThenThrowOAuth2AuthenticationException() { + RegisteredClient registeredClient = TestRegisteredClients.registeredClient4().build(); + when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId()))) + .thenReturn(registeredClient); + + OAuth2ClientAuthenticationToken authentication = new OAuth2ClientAuthenticationToken( + registeredClient.getClientId(), ClientAuthenticationMethod.CLIENT_SECRET_BASIC, registeredClient.getClientSecret(), null); + assertThatThrownBy(() -> this.authenticationProvider.authenticate(authentication)) + .isInstanceOf(OAuth2AuthenticationException.class) + .extracting(ex -> ((OAuth2AuthenticationException) ex).getError()) + .satisfies(error -> { + assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_CLIENT); + assertThat(error.getDescription()).contains("client_secret_expires_at"); + }); + verify(this.passwordEncoder).matches(any(), any()); + } + @Test public void authenticateWhenValidCredentialsThenAuthenticated() { RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java index aafd01d32..b68c460ea 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java @@ -64,4 +64,20 @@ public static RegisteredClient.Builder registeredPublicClient() { .scope("scope1") .clientSettings(ClientSettings.builder().requireProofKey(true).build()); } + + public static RegisteredClient.Builder registeredClient4() { + return RegisteredClient.withId("registration-4") + .clientId("client-4") + .clientIdIssuedAt(Instant.now().minus(1, ChronoUnit.DAYS).truncatedTo(ChronoUnit.SECONDS)) + .clientSecret("secret") + .clientSecretExpiresAt(Instant.now().truncatedTo(ChronoUnit.SECONDS)) + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .authorizationGrantType(AuthorizationGrantType.REFRESH_TOKEN) + .authorizationGrantType(AuthorizationGrantType.CLIENT_CREDENTIALS) + .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC) + .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_POST) + .redirectUri("https://example.com") + .scope("scope1") + .scope("scope2"); + } } From 4e6b1d11a2d9362e6e1c656d437f5adfcf7bc8b1 Mon Sep 17 00:00:00 2001 From: doctormacky Date: Fri, 19 Aug 2022 21:32:43 +0800 Subject: [PATCH 3/4] set the client secret expired 1 hour ago --- .../server/authorization/client/TestRegisteredClients.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java index b68c460ea..016751ef2 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java @@ -70,7 +70,7 @@ public static RegisteredClient.Builder registeredClient4() { .clientId("client-4") .clientIdIssuedAt(Instant.now().minus(1, ChronoUnit.DAYS).truncatedTo(ChronoUnit.SECONDS)) .clientSecret("secret") - .clientSecretExpiresAt(Instant.now().truncatedTo(ChronoUnit.SECONDS)) + .clientSecretExpiresAt(Instant.now().minus(1, ChronoUnit.HOURS).truncatedTo(ChronoUnit.SECONDS)) .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) .authorizationGrantType(AuthorizationGrantType.REFRESH_TOKEN) .authorizationGrantType(AuthorizationGrantType.CLIENT_CREDENTIALS) From 74e16290c94d005af838e2ec2da9a483a101684f Mon Sep 17 00:00:00 2001 From: doctormacky Date: Thu, 25 Aug 2022 09:20:54 +0800 Subject: [PATCH 4/4] rebase 0.4.x and also removed some duplicate method --- ...ClientSecretAuthenticationProviderTests.java | 6 +++++- .../client/TestRegisteredClients.java | 17 +---------------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProviderTests.java index 421126424..800beec0d 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProviderTests.java @@ -15,6 +15,8 @@ */ package org.springframework.security.oauth2.server.authorization.authentication; +import java.time.Instant; +import java.time.temporal.ChronoUnit; import java.util.HashMap; import java.util.Map; @@ -184,7 +186,9 @@ public void authenticateWhenInvalidClientSecretThenThrowOAuth2AuthenticationExce @Test public void authenticateWhenExpiredClientSecretThenThrowOAuth2AuthenticationException() { - RegisteredClient registeredClient = TestRegisteredClients.registeredClient4().build(); + RegisteredClient registeredClient = TestRegisteredClients.registeredClient() + .clientSecretExpiresAt(Instant.now().minus(1, ChronoUnit.HOURS).truncatedTo(ChronoUnit.SECONDS)) + .build(); when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId()))) .thenReturn(registeredClient); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java index 016751ef2..257076060 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java @@ -64,20 +64,5 @@ public static RegisteredClient.Builder registeredPublicClient() { .scope("scope1") .clientSettings(ClientSettings.builder().requireProofKey(true).build()); } - - public static RegisteredClient.Builder registeredClient4() { - return RegisteredClient.withId("registration-4") - .clientId("client-4") - .clientIdIssuedAt(Instant.now().minus(1, ChronoUnit.DAYS).truncatedTo(ChronoUnit.SECONDS)) - .clientSecret("secret") - .clientSecretExpiresAt(Instant.now().minus(1, ChronoUnit.HOURS).truncatedTo(ChronoUnit.SECONDS)) - .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) - .authorizationGrantType(AuthorizationGrantType.REFRESH_TOKEN) - .authorizationGrantType(AuthorizationGrantType.CLIENT_CREDENTIALS) - .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC) - .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_POST) - .redirectUri("https://example.com") - .scope("scope1") - .scope("scope2"); - } + }