validate client secret expired or not #862
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
|
@jgrandja Please help to review this PR. thanks! |
jgrandja
added
status: duplicate
There was a problem hiding this comment.
Thanks for the PR @doctormacky. Please see review comments.
Also, please add a test in ClientSecretAuthenticationProviderTests.
| @@ -93,6 +95,11 @@ public Authentication authenticate(Authentication authentication) throws Authent | |||
| throwInvalidClient(OAuth2ParameterNames.CLIENT_ID); | |||
| } | |||
|
|
|||
| Instant expiredAt = registeredClient.getClientSecretExpiresAt(); | |||
There was a problem hiding this comment.
Please move this validation below if (!this.passwordEncoder.matches(clientSecret, registeredClient.getClientSecret()))
| @@ -93,6 +95,11 @@ public Authentication authenticate(Authentication authentication) throws Authent | |||
| throwInvalidClient(OAuth2ParameterNames.CLIENT_ID); | |||
| } | |||
|
|
|||
| Instant expiredAt = registeredClient.getClientSecretExpiresAt(); | |||
There was a problem hiding this comment.
Remove the variable expiredAt and just use registeredClient.getClientSecretExpiresAt()
| @@ -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)) { | |||
There was a problem hiding this comment.
Add spaces between the comparison expiredAt!=null so it's consistent with other code
|
@jgrandja Please help to review it again. thanks. |
|
@jgrandja Any progress on this PR ? |
There was a problem hiding this comment.
Thanks for the updates @doctormacky.
One minor change left and we'll be ready to merge.
Can you also rebase off latest 0.4.x and squash the commits.
Thanks!
| @@ -64,4 +64,20 @@ public static RegisteredClient.Builder registeredPublicClient() { | |||
| .scope("scope1") | |||
| .clientSettings(ClientSettings.builder().requireProofKey(true).build()); | |||
| } | |||
|
|
|||
| public static RegisteredClient.Builder registeredClient4() { | |||
There was a problem hiding this comment.
Please remove this and reuse TestRegisteredClients.registeredClient(), which returns the Builder and allows you to update clientSecretExpiresAt()
|
Thanks for the updates @doctormacky. This is now merged. |
Thanks @jgrandja It's my pleasure. |
validate the client secret is expired or not.
see the details here