Allow custom settings in ClientRegistration.ClientSettings#18933
Allow custom settings in ClientRegistration.ClientSettings#18933pranavmanglik wants to merge 1 commit intospring-projects:mainfrom
Conversation
pranavmanglik
force-pushed
the
feature/extensible-client-settings
branch
from
3caa493 to
8a68648
Compare
spring-projects-issues
added
the
status: waiting-for-triage
jgrandja
added
type: enhancement
jgrandja
left a comment
jgrandja
left a comment
There was a problem hiding this comment.
Thanks for the PR @pranavmanglik. Please see review comments.
Also, please rebase to latest on main. Thank you.
| } | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| public <T> T getSetting(String name, T defaultValue) { |
There was a problem hiding this comment.
Please remove this as I don't think providing a default is necessary.
|
|
||
| public boolean isRequireProofKey() { | ||
| return this.requireProofKey; | ||
| return getSetting("settings.client.require-proof-key", true); |
There was a problem hiding this comment.
Please extract settings.client.require-proof-key to a private constant
| * @param settings the configuration settings | ||
| * @return the {@link Builder} for further configuration | ||
| */ | ||
| public Builder settings(Map<String, Object> settings) { |
There was a problem hiding this comment.
Please change to settings(Consumer<Map<String, Object>> settingsConsumer)
| } | ||
|
|
||
| @Test | ||
| void buildWhenScopesHaveInvalidCharactersThenThrowException() { |
There was a problem hiding this comment.
All 3x tests are not related to the changes made to ClientSettings. Please remove and add applicable tests.
jgrandja
changed the title
pranavmanglik
force-pushed
the
feature/extensible-client-settings
branch
from
8a68648 to
fc3e093
Compare
Signed-off-by: Pranav Manglik <pranav@undreamt.in>
pranavmanglik
force-pushed
the
feature/extensible-client-settings
branch
from
fc3e093 to
c57ea55
Compare
|
@jgrandja I think i have made the changes suggested by you, though i am not sure about these test cases, is there anything you suggest for that? |
3 participants
Description
This pull request introduces an extensible
ClientSettingsconfiguration withinClientRegistration. Currently,ClientRegistrationhandles core OAuth2 properties, but adding provider-specific or advanced settings (such as custom PKCE requirements or extended client metadata) often requires modifying the core class or maintaining complex external maps.By encapsulating these configurations in a dedicated, immutable
ClientSettingsobject, we provide a cleaner API for future extensions without bloating theClientRegistrationroot. This aligns with the framework's goals of modularity and extensibility for modern OAuth2/OIDC flows.Key Changes
ClientSettings: A new configuration class held byClientRegistrationto store specialized client behavior settings.ClientRegistration.Builder: Added the.clientSettings(ClientSettings)method to the builder pattern to allow for fluent configuration.ClientRegistration.Builderto ensure that providedClientSettings(e.g.,requireProofKey) are compatible with the selectedAuthorizationGrantType.ClientRegistrationTestscovering valid configurations, edge cases, and validation failures.Related Issues
Ref: #18863 Checklist
./gradlew checkstyleMainClientRegistrationTests./gradlew :spring-security-oauth2-client:test