Add SpEL support for nested username extraction in OAuth2 user info
This PR adds support for extracting usernames from nested properties in OAuth2 user info responses using SpEL expressions, addressing the limitation where providers wrap user data in nested objects.
Fixes #16390
Problem
OAuth2 providers (like X/Twitter) wrap user data in nested objects, requiring complex workarounds to extract usernames.
Solution
Commit 1: Allow injecting principal name into DefaultOAuth2User
- Add username field to DefaultOAuth2User
- Add static factory method withUsername() for direct username injection
- Deprecate constructor that uses nameAttributeKey lookup
- Maintain backward compatibility and serialization format
Commit 2: Add SpEL support for nested username extraction
- Add usernameExpression property to ClientRegistration
- Add username field to OAuth2UserAuthority
- Support SpEL expressions for nested property access (e.g., "data.username")
- Use SimpleEvaluationContext for secure expression evaluation
- Update both DefaultOAuth2UserService and DefaultReactiveOAuth2UserService
- Pass evaluated username directly to OAuth2UserAuthority for gh-15012 compatibility
Backward Compatibility
- No breaking changes - all existing APIs continue to work
- Deprecated APIs remain functional with clear migration path
Thanks for the PR @yybmion! I wonder if this might be better implemented using SpEL to provide more powerful options for resolving the username. What are your thoughts?
Hi @rwinch , Thank you for your guidance on this.
I initially chose the dot notation approach because it offers a simple and intuitive solution specifically for the nested user-name-attribute issue.
However, I can see the value in using SpEL as you suggested. While I think it may be slightly more complex, SpEL provides much greater extensibility for future use cases beyond simple nested structures. The consistency with other parts of the Spring Security framework is also a advantage.
If you confirm that SpEL is the preferred direction, I'd be happy to update the PR accordingly.
Yes. Please provide an implementation that uses SpEL.
Hello @rwinch, I'd like to clarify your feedback on my PR about supporting nested properties in the user-name-attribute.
Did you mean that I should implement support for expressions like #{data.username} in the properties and yml configuration files to handle nested structures? (Instead using data.username)
Thank you for your guidance!
I think an outline would be:
Allow Injecting the Principal Name into DefaultOAuth2User
Right now OAuth2UserService requires defining the name property as an attribute name. This limits the flexibility of resolving the name.
Update DefaultOAuth2User to allow injecting the name (rather than a property for obtaining the name). You would add a new member variable of username. This would likely require using a static factory method to distinguish from the existing constructor since the types of the nameAttributeKey and username are the same.
You can remove the nameAttributeKey member variable and instead populate the username using the attributes[nameAttributeKey] in the constructor if nameAttributeKey was specified.
Deprecate the old DefaultOAuth2User constructor in favor of injecting the name directly and update the usage of DefaultOAuth2User within Spring Security to no longer use the deprecated constructor.
Add SpEL Support
Update ClientRegistration to have a property named usernameExpression and remove the userNameAttributeName property but preserving and deprecating the methods which instead populate the usernameExpression member variable. This should be passive since the usernameAttributeName is a valid SpEL expression.
Update OAuth2UserService to extract the username using the usernameExpression property from the ClientRegistration as a SpEL expression with the attributes as the root object. Create the DefaultOAuth2User with the username injected into it rather than using the nameAttributeKey.
I think creating these as two separate commits is valuable since they are useful on their own. Let me know your thoughts.
Hi! @rwinch I've implemented the first part of your suggestion - adding the username field to DefaultOAuth2User and the withUsername() static factory method.
For backward compatibility, I kept the nameAttributeKey field alongside the new username field. The implementation works as follows
public class DefaultOAuth2User {
private final String nameAttributeKey; // kept for backward compatibility
private final String username; // newly added
@Deprecated
public DefaultOAuth2User(..., String nameAttributeKey) {
this.nameAttributeKey = nameAttributeKey;
// Extract username from attributes[nameAttributeKey]
this.username = attributes.get(nameAttributeKey).toString();
}
public static DefaultOAuth2User withUsername(..., String username) {
// Direct username injection
}
@Override
public String getName() {
return (this.username != null) ? this.username : getAttribute(nameAttributeKey).toString();
}
}
Issue: The Jackson serialization tests (OAuth2AuthenticationTokenMixinTests)
serializeWhenMixinRegisteredThenSerializes()serializeWhenRequiredAttributesOnlyThenSerializes()
are now failing because Jackson serializes all fields, including both nameAttributeKey and username. The test expects the old JSON structure but gets the new one with the additional username field.
Question: What's your preferred approach for handling this?
Thanks!
Hi @rwinch thank you for the detailed guide.
I've implemented the SpEL-based approach as outlined in the original issue. The solution provides
- SpEL expressions for nested property access (e.g.,
"data.username") - Full backward compatibility with existing
userNameAttributeName - Support for both sync and reactive implementations
Please see the PR description for detailed changes.
Thanks for the feedback @rwinch ! I've addressed all the review comments and updated the code accordingly.
But the build failure appears to be related to Kotlin version. What would you recommend I do?
Hi @rwinch. I've addressed all the review feedback and updated the documentation as requested. Please let me know if there are any areas in the documentation that need further improvement or clarification.
Thank you for the review @rwinch ! I'm planning to create a separate commit to address the deprecated constructor/method usage you mentioned. However, I have a few questions that came up during the modifications.
I'm wondering about your thoughts on :
- Deprecating the nameAttributeKey field in DefaultOAuth2User constructors for consistency with the new usernameExpression approach
- Adding builder pattern to DefaultOidcUser since it extends DefaultOAuth2User and currently calls the deprecated parent constructor via super()
Should I include these changes in this PR?
Thanks for reaching out and your patience as we put the final touches on this @yybmion
Deprecating the nameAttributeKey field in DefaultOAuth2User constructors for consistency with the new usernameExpression approach
Yes. I think that this is good. Generally, we want people to move to the builder instead.
Adding builder pattern to DefaultOidcUser since it extends DefaultOAuth2User and currently calls the deprecated parent constructor via super().
Yes I think that is good too. You will likely need to ensure the non-deprecated constructor in DefaultOAuth2User is protected so that it can be used by DefaultOidcUser.
Should I include these changes in this PR?
Yes please. It is important that we push people towards the new, more powerful, approach.
Thank you for the guidance, @rwinch !
I've updated the deprecated constructor/method usage in main based on your review feedback. After searching through the codebase, I found that most of the deprecated usage was in OidcUserRequestUtils using deprecated constructors/methods. The remaining usage was in test code and documentation, which I left unchanged as requested.
Additionally, since the username SpEL evaluation logic was being used in multiple places, I created a utility class OAuth2UsernameExpressionUtils to centralize this functionality and eliminate code duplication.
Hi @rwinch, I noticed that the "What's New" documentation for Spring Security 7 has recently been updated, which caused a conflict with the changes I previously added. I've resolved the conflict accordingly.
Thanks again for all of your work on this PR!
I'd like to encapsulate the logic (keep private) in OAuth2UsernameExpressionUtils so that we can refactor without breaking anything later. One example of such a refactoring is that at some point the ExpressionParser might need to be injected to allow resolving Bean references in the expressions. Even if we allow injecting the ExpressionParser, I'd still like to keep this logic encapsulated in case of other refactorings.
To work toward that end I created and resolved gh-17626. In the case where we invoke the OAuth2UserService (or the reactive equivalent), we could use the result to obtain the username. The problem is that OidcUserService (and the reactive equivalent) leverage userNameAttributeName even when the OAuth2Service is not invoked and it does not use the result from the delegate service even when it does invoke the user info endpoint.
To address those concerns I created gh-17627 gh-17628
I believe we could extract the logic for injecting the username rather than the attribute into a separate PR and merge that without being blocked since that behavior would be passive and not be impacted by the decisions of the issues above. Feel free to do this if you like. However, as of now we are blocked on merging resolving the username using an expression until those two issues are resolved.
cc @jgrandja
FYI I rebased and pushed the commits
Thanks for the rebase and I understood about the blocking issues.