-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix to return client_secret_expires_at in client registration response #2134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Closes spring-projectsgh-2111 Signed-off-by: wheleph <[email protected]>
@jgrandja please have a look at the change. Looking forward to your feedback :) |
Thanks @wheleph. I have a couple of high priority tasks that I'm currently working on so I just wanted to let you know that I will review this soon. |
I appreciate your transparency. No worries, take your time :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @wheleph. Please see review comments.
|
||
OidcClientRegistration clientRegistrationResponse = registerClient(clientRegistration); | ||
|
||
var expectedSecretExpiryDate = Instant.now().plus(Duration.ofHours(24)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This project does not make use of var
so to keep things consistent please use the type
@@ -643,8 +685,40 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity h | |||
|
|||
@EnableWebSecurity | |||
@Configuration(proxyBeanMethods = false) | |||
static class CustomClientMetadataConfiguration extends AuthorizationServerConfiguration { | |||
static class CustomClientMetadataConfiguration extends ClientRegistrationConvertersConfiguration { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate taking advantage of reuse but there are too many changes applied to other tests and the preference is to isolate the changes to only the issue at hand. Also, I am totally ok with code duplication in tests and keeping things isolated. Please revert all unrelated changes and isolate to only the issue at hand.
Signed-off-by: wheleph <[email protected]>
@jgrandja I've applied your suggestions. Please have a look and let me know if you have any other concerns |
Closes gh-2111