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);
    }

Comment From: jgrandja

@reda-alaoui ROLE_USER is a sensible default role for a "regular" authenticated user. I'd like to understand how this is an issue with your setup?

I am afraid more default roles will be included in the future

No, there won't be any further default roles assigned. User authority mapping is application specific and is typically configured by the application to assign custom roles that are understood by that application.

Refer to the reference: - Using a GrantedAuthoritiesMapper - Delegation-based strategy with OAuth2UserService

Comment From: reda-alaoui

Hi @jgrandja, Thanks for your answer.

There is no issue with my current setup. But there are many spring tutorials using ROLE_USER to protect urls. It felt wrong to see Spring injecting a role coming from nowhere.

Does ROLE_USER has a special meaning in Spring or in the field?

Comment From: jgrandja

ROLE_USER typically represents a "Regular" user with minimum privileges. Unlike ROLE_ADMIN that would have much higher privileges and would never be randomly assigned by the framework. The ROLE_ADMIN authority could only be assigned by the application through customization.

there are many spring tutorials using ROLE_USER to protect urls. It felt wrong to see Spring injecting a role coming from nowhere

I guess I still don't understand why you think it's wrong? Maybe my previous comment cleared things up?

Given that the user has authenticated at the provider after completing the OIDC flow, the user is also OAuth2AuthenticationToken.isAuthenticated() == true with the client application. Hence, it's sensible to assigned the currently authenticated user ROLE_USER as a default authority. Does this make sense?

Comment From: reda-alaoui

In my context (described in my first message), if I had been using ROLE_USER instead of ROLE_ACCESS, I could have ended with a major security breach.

Given that the user has authenticated at the provider after completing the OIDC flow, the user is also OAuth2AuthenticationToken.isAuthenticated() == true with the client application.

When a JWT is verified by Spring OAuth2, I find it normal to have authenticated=true. Because the authentication is sucessful indeed. On the other hand, in my mind at least, everything pertaining to ROLE_ is part of authorization not authentication. I feel like both are mixed here. Does this ROLE_USER is required by something in the framework? The decision to pass it by default seems very opinionated to me.

I have been using Spring Security for many years (without OAuth2), this is the first time I see a role injected by default. Also, I didn't find a single line about "what is ROLE_USER?" in Spring Security documentation. There are 59 occurrences of ROLE_USER. Each time, it is quoted as a meaningless example.

Comment From: jgrandja

@reda-alaoui

In my context (described in my first message), if I had been using ROLE_USER instead of ROLE_ACCESS, I could have ended with a major security breach.

I find this very vague. I do not see how this could have caused a major security breach. I will need much more detail on the scenario to understand.

FYI, the default authorization rules for both Spring Security and Spring Boot auto-configuration is:

http
    .authorizeRequests()
        .anyRequest().authenticated()

So with the default security configuration, the (default) ROLE_USER authority is never applied as an authorization check during request processing. The only check that happens is that Authentication.isAuthenticated() must return true for the default authorization rule to pass.

The only time an authorization check on the ROLE_USER authority can happen is if the application provides a custom WebSecurityConfigurerAdapter with the following rule:

http
    .authorizeRequests()
        .anyRequest().hasRole("USER")

Given that the application has defined this rule, the expectation is that any user with the authority ROLE_USER will be granted access. This also would include OIDC or OAuth2 authenticated users given the default authority assignment ROLE_USER. If the application does not want OIDC or OAuth2 authenticated users to have this default role than it can simply be overridden like this:

http
    .authorizeRequests()
        .anyRequest().authenticated()
        .and()
    .oauth2Login()
        .userInfoEndpoint()
            .userAuthoritiesMapper(authorities -> Collections.emptyList());

This will ensure that OIDC or OAuth2 authenticated users will have no authorities assigned.

As per your previous comment:

There is no issue with my current setup

I'm going to close this issue.

Please see this comment for further details on how to provide your own custom authority mapping.

Comment From: reda-alaoui

@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.

Comment From: koen-serneels

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.

Comment From: arawak

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?

Comment From: jgrandja

@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.0 release.

Comment From: sjohnr

@reda-alaoui @koen-serneels @arawak I've reviewed the conversation so far on this issue and am looking to come up with an approach for resolving it. Some options I'm thinking of are as follows:

  • Document the special authority (OAuth2UserAuthority or OidcUserAuthority) in detail in the reference
  • Use a blank authority string instead of ROLE_USER
  • Return the toString() of the special authority class instead of ROLE_USER
  • A combination of the above

Any thoughts on which of these options might solve it most effectively?

Comment From: reda-alaoui

Hello @jgrandja , @sjohnr ,

First, thank you for reopening this issue.

Use a blank authority string instead of ROLE_USER

Without ROLE_USER, it seems to me that OidcUserAuthority and OAuth2UserAuthority act only as carriers for OidcUserInfo and OidcIdToken. But these attributes are already directly carried by OidcUser .

So IMO, there is no need to have a GrantedAuthority added at all.

Comment From: sjohnr

Hi @reda-alaoui!

there is no need to have a GrantedAuthority added at all.

The OidcUserAuthority and OAuth2UserAuthority exist for the GrantedAuthoritiesMapper @Bean (see docs). So we need to find a way to continue to support it.

Comment From: arawak

I'm struggling to remember where, I but seem to recall that somewhere there is implementation code that checks to see that a Principal (maybe?) has at least one GrantedAuthority. Maybe an isAuthenticated()? I'll dig around to see if I can find it, but maybe someone more familiar with the codebase might recall?

I'd suggest that if in fact an authority string is needed, and it must be hardcoded, it could provide more value if it was representative of the authentication's implementation, something like ROLE_OAUTH2 maybe. I could see my self having flows that are different if the user came from a social login rather than a username/password or some other authentication source.

Comment From: jgrandja

We should also consider OAUTH2_USER and OIDC_USER for the authority string. And we might want to avoid the ROLE_ prefix as I believe this is causing issues in certain scenarios.

Comment From: arawak

I think these names make sense. It doesn't look like the names are used for anything, we just need to stick the claims into the GrantedAuthority as attributes in case someone wants to look at them.

I'd suggest these names would be better as package-private or even public constants in their respective classes for tidier tests and better self-documentation.

Further, as this is an implementation detail and not exposed anywhere as a contract, I don't see that it would be a breaking change, so perhaps such a change could go in before 6.0?

Comment From: sjohnr

Further, as this is an implementation detail and not exposed anywhere as a contract, I don't see that it would be a breaking change, so perhaps such a change could go in before 6.0?

Typically, we would consider something like this part of the contract, in the sense that it breaks passivity for an unknown (hopefully small) number of users. If anyone is using hasRole("USER"), their authorization scheme could be broken by this change.

If we wanted to introduce a change in 5.8, we would need to make it opt-in, meaning we would need to introduce a configuration option for it. Then we could switch that option to be the default in 6.0. I don't think it makes sense to pursue that here, so it would probably just go into 6.0. We will just need to ensure it's clear from release notes that this is changing. That is my take on it. If anyone has another thought about this, I'm open to it.

Comment From: sjohnr

Based on feedback so far, it sounds like trying OAUTH2_USER and OIDC_USER as authority strings might be the way to go. I will proceed with this option for now and add some documentation to the section on GrantedAuthoritiesMapper.

Comment From: sven-tsi

With the New default "OIDC_USER" the userinfo is always null. With the Old implementation "ROLE_USER" evertyhing works fine. Is there a Bug?

Comment From: sjohnr

Hi @sven-tsi, may I ask what version of Spring Security you're using? Perhaps you're facing gh-12144, which has already been fixed?

Comment From: sven-tsi

Hi @sjohnr, we use the current Version. i found the root cause of the missing/null userinfo. -> https://spring.io/security/cve-2022-31690 The Security version before this Cve-fix works fine. The problem is, the authorization server of our customer responds always with empty scopes. So we Can no longer do authorization with the Spring Cloud Gateway.

Comment From: sjohnr

@sven-tsi, you may find this blog post helpful in regards to impacts of that CVE fix. https://spring.io/blog/2022/10/31/cve-2022-31690-privilege-escalation-in-spring-security-oauth2-client

The example given there is for a Servlet application, but I believe the code can be adapted to Reactive as well.