Skip to content

Get userInfo, if scopes in token is empty #12513

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

Closed
wants to merge 1 commit into from

Conversation

sharat87
Copy link

In a WebFlux app, with OIDC authentication, the userInfo request is handled here:

private Mono<OidcUserInfo> getUserInfo(OidcUserRequest userRequest) {
if (!OidcUserRequestUtils.shouldRetrieveUserInfo(userRequest)) {
return Mono.empty();
}

Here, we call the OidcUserRequestUtils.shouldRetrieveUserInfo static function, to determine if we should do userInfo. In this function, we check if the scopes in the token, and the configured scopes match:

if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(clientRegistration.getAuthorizationGrantType())) {
// Return true if there is at least one match between the authorized scope(s)
// and UserInfo scope(s)
return CollectionUtils.containsAny(userRequest.getAccessToken().getScopes(),
userRequest.getClientRegistration().getScopes());
}

Now consider this check for a non-Webflux (non-reactive) OIDC authentication:

if (AuthorizationGrantType.AUTHORIZATION_CODE
.equals(userRequest.getClientRegistration().getAuthorizationGrantType())) {
// Return true if there is at least one match between the authorized scope(s)
// and accessible scope(s)
//
// Also return true if authorized scope(s) is empty, because the provider has
// not indicated which scopes are accessible via the access token
// @formatter:off
return this.accessibleScopes.isEmpty()
|| CollectionUtils.isEmpty(userRequest.getAccessToken().getScopes())
|| CollectionUtils.containsAny(userRequest.getAccessToken().getScopes(), this.accessibleScopes);
// @formatter:on
}

We can see two major differences:

  1. The non-Webflux flow, when it encounters missing scope field, or empty scope field, it will call the userInfo endpoint. Whereas the Webflux flow will not.

  2. The non-Webflux flow, also checks this.accessibleScopes.isEmpty(). Overriding this accessibleScopes has been recommended, to handle cases like missing scope in token. But this affordance is missing in the Webflux flow.

So, when using Webflux, I don't see a way out. Can we add these (at least the first) one, to the Webflux's flow as well? That's what this PR does. If the scopes are missing/empty in the token, do call the userInfo endpoint.

Thank you for your time.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 10, 2023
@pivotal-cla
Copy link

@sharat87 Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla
Copy link

@sharat87 Thank you for signing the Contributor License Agreement!

@sjohnr sjohnr self-assigned this Mar 1, 2023
@sjohnr sjohnr added type: bug A general bug in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) labels Mar 1, 2023
@sjohnr
Copy link
Member

sjohnr commented Mar 1, 2023

@sharat87, apologies as I seem to have missed this PR. Thanks for contributing!

Please note that it's usually best to open an issue first, and discuss possible solutions prior to submitting a PR. I noticed you submitted the PR using the GitHub editor. Are you able to download the project and run tests to verify that everything is working? There should be at least one test that needs to change, and if not, would you please add a new test that covers this change?

Also please review the contributing guidelines if needed and make sure to update the copyright year of any files that have been modified in this PR.

@sjohnr sjohnr removed the status: waiting-for-triage An issue we've not yet triaged label Mar 1, 2023
@sjohnr
Copy link
Member

sjohnr commented Oct 12, 2023

@sharat87 have you seen my above comment?

@sjohnr sjohnr added the status: waiting-for-feedback We need additional information before we can continue label Oct 12, 2023
@sjohnr
Copy link
Member

sjohnr commented Feb 13, 2024

Closing in favor of gh-13259.

@sjohnr sjohnr closed this Feb 13, 2024
@sjohnr sjohnr added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: declined A suggestion or change that we don't feel we should currently apply type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants