-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Return registration_endpoint in OidcProviderConfigurationEndpointFilter #371
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
Conversation
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 enhancement @bibibiu2017. Please see review comments.
Also, can you please rebase
on top of main
. I noticed you performed a merge
instead. Please rebase
your changes on top of current main
.
...r/src/main/java/org/springframework/security/oauth2/core/oidc/OidcProviderConfiguration.java
Outdated
Show resolved
Hide resolved
.../main/java/org/springframework/security/oauth2/core/oidc/OidcProviderMetadataClaimNames.java
Outdated
Show resolved
Hide resolved
96b9600
to
f0cb790
Compare
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 updates @bibibiu2017 . Please see review comments.
After you apply the remaining updates please squash commits and format commit message.
* @see <a target="_blank" href="https://tools.ietf.org/html/rfc8414#section-3.2">3.2. Authorization Server Metadata Response</a> | ||
* @see <a target="_blank" href="https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationResponse">4.2. OpenID Provider Configuration Response</a> | ||
* @since 0.1.1 |
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.
Move @since
below @author
- the same order as in all classes
@@ -105,6 +107,16 @@ public B tokenEndpoint(String tokenEndpoint) { | |||
return claim(OAuth2AuthorizationServerMetadataClaimNames.TOKEN_ENDPOINT, tokenEndpoint); | |||
} | |||
|
|||
/** | |||
* Add this registration endpoint in the resulting {@link OidcProviderConfiguration} OPTIONAL |
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.
OidcProviderConfiguration
should not be referenced
* @param registrationEndpoint the supported registration client url | ||
* @return the {@link OidcProviderConfiguration.Builder} for further configuration | ||
*/ | ||
public B clientRegistrationEndpoint(String registrationEndpoint) { |
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.
Rename argument -> clientRegistrationEndpoint
@@ -299,7 +311,7 @@ public B codeChallengeMethods(Consumer<List<String>> codeChallengeMethodsConsume | |||
/** | |||
* Use this claim in the resulting {@link AbstractOAuth2AuthorizationServerMetadata}. | |||
* | |||
* @param name the claim name | |||
* @param name the claim name |
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.
whitespace added?
@@ -41,6 +42,11 @@ | |||
*/ | |||
String TOKEN_ENDPOINT = "token_endpoint"; | |||
|
|||
/** | |||
* {@code registration_endpoint} - the {@code URL} of the OpenID Connect Discovery 1.0 Client Registration Endpoint |
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.
javadoc is incorrect -> "OpenID Connect Discovery 1.0"
21c468c
to
252894b
Compare
Hi @jgrandja thanks for the feedback, I have made the recommended changes and squashed all the commits |
...otation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationServerConfigurer.java
Outdated
Show resolved
Hide resolved
…ration but the client registration url was not included in open id Configuration metadata. This has been added by this commit. closes spring-projectsgh-370
@bibibiu2017 Based on the updates in gh-398, more changes are required in this PR.
I'm going to push this out for the |
@bibibiu2017 I'm going to close this PR as the requested changes have not been applied. If you have time please submit a new PR with the requested changes and rebase on latest main. Thanks. |
Fixes gh-370