spring-security: OidcUserAuthority should not automatically include ROLE_USER authority
Summary
I am building a Spring based application that delegates authentication to an OIDC provider like Keycloak. The OIDC could be under my control or not. e.g. Today we use Keycloak, tomorrow we may be using Linkedin.
The users must be granted access to the application by an admin. Since the OIDC may not be under my control, I don’t want a random Linkedin user be able to authenticate AND start using the application.
To prevent that, all protected uris require ROLE_ACCESS in addition to a successful authentication. ROLE_ACCESS has to be provided manually by an application admin.
Actual Behavior
I noticed that, by default, the OIDC authentication has always the ROLE_USER assigned. I am lucky since it does not clash with my expected ROLE_ACCESS, but I find this very disturbing and I am afraid more default roles will be included in the future.
Expected Behavior
org.springframework.security.oauth2.core.oidc.user.OidcUserAuthority should not include a default role, ever.
Version
5.2.1
Sample
public class OidcUserAuthority extends OAuth2UserAuthority {
private final OidcIdToken idToken;
private final OidcUserInfo userInfo;
/**
* Constructs a {@code OidcUserAuthority} using the provided parameters.
*
* @param idToken the {@link OidcIdToken ID Token} containing claims about the user
*/
public OidcUserAuthority(OidcIdToken idToken) {
this(idToken, null);
}
/**
* Constructs a {@code OidcUserAuthority} using the provided parameters
* and defaults {@link #getAuthority()} to {@code ROLE_USER}.
*
* @param idToken the {@link OidcIdToken ID Token} containing claims about the user
* @param userInfo the {@link OidcUserInfo UserInfo} containing claims about the user, may be {@code null}
*/
public OidcUserAuthority(OidcIdToken idToken, OidcUserInfo userInfo) {
this("ROLE_USER", idToken, userInfo);
}
About this issue
- Original URL
- State: closed
- Created 4 years ago
- Comments: 21 (11 by maintainers)
Commits related to this issue
- Change default authority for oauth2Login() Previously, the default authority was ROLE_USER when using oauth2Login() for both OAuth2 and OIDC providers. * Default authority for OAuth2UserAuthority is... — committed to sjohnr/spring-security by sjohnr 2 years ago
- Change default authority for oauth2Login() Previously, the default authority was ROLE_USER when using oauth2Login() for both OAuth2 and OIDC providers. * Default authority for OAuth2UserAuthority is... — committed to sjohnr/spring-security by sjohnr 2 years ago
- Change default authority for oauth2Login() Previously, the default authority was ROLE_USER when using oauth2Login() for both OAuth2 and OIDC providers. * Default authority for OAuth2UserAuthority is... — committed to sjohnr/spring-security by sjohnr 2 years ago
- Change default authority for oauth2Login() Previously, the default authority was ROLE_USER when using oauth2Login() for both OAuth2 and OIDC providers. * Default authority for OAuth2UserAuthority is... — committed to sjohnr/spring-security by sjohnr 2 years ago
Although this has been closed, I do agree that it is rather unexpected that a default role is being added by the framework. In my case it took me some time to figure out where this ‘rogue’ role was coming from stumbling on this ticket. Why does Spring Security need to make the assumption that ‘ROLE_USER’ is a role that each authenticated user should receive by default? What if I have a normal user role and read only user role? In this case all authenticated users would get the normal user role even users that are only granted the read only role.
@jgrandja , the issue is about a bad default. I don’t see how to be more clear.
And this is unexpected as OidcUserAuthority is the only spring security component automatically injecting a role other than Anonymous.
Even if this default is kept, it should be at least documented. It is currently not.
Based on feedback so far, it sounds like trying
OAUTH2_USERandOIDC_USERas authority strings might be the way to go. I will proceed with this option for now and add some documentation to the section onGrantedAuthoritiesMapper.@reda-alaoui @koen-serneels @arawak I’m reopening this ticket and we’ll look at removing the default authority
ROLE_USER.This could potentially break existing applications so we’ll consider this for the
6.0release.FWIW I agree with the @reda-alaoui and @koen-serneels … this is very non-obvious and took a bit of debugging to figure out why a user who was merely authenticated also had a role, and worse, the hardcoded USER role.
The case in hand is an application where a user must register with the system either by email/username/password or by social account, and provide some specific information before they are a user of the system. I’m adopting the OP’s approach of using another name for the role to specify a user with access to the system, but it still seems kludgy.
If nothing else, perhaps a means to override the default role could be a future feature?