Skip to content

Address Incorrect scope map fix in Reactive service #17511

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

Open
wants to merge 1 commit into
base: 5.6.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -17,12 +17,10 @@
package org.springframework.security.oauth2.client.oidc.userinfo;

import java.time.Instant;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.*;
import java.util.function.Function;

import org.springframework.security.oauth2.core.oidc.OidcScopes;
import reactor.core.publisher.Mono;

import org.springframework.core.convert.TypeDescriptor;
Expand Down Expand Up @@ -66,6 +64,9 @@ public class OidcReactiveOAuth2UserService implements ReactiveOAuth2UserService<
private static final Converter<Map<String, Object>, Map<String, Object>> DEFAULT_CLAIM_TYPE_CONVERTER = new ClaimTypeConverter(
createDefaultClaimTypeConverters());

private Set<String> accessibleScopes = new HashSet<>(
Arrays.asList(OidcScopes.PROFILE, OidcScopes.EMAIL, OidcScopes.ADDRESS, OidcScopes.PHONE));

private ReactiveOAuth2UserService<OAuth2UserRequest, OAuth2User> oauth2UserService = new DefaultReactiveOAuth2UserService();

private Function<ClientRegistration, Converter<Map<String, Object>, Map<String, Object>>> claimTypeConverterFactory = (
Expand Down Expand Up @@ -123,7 +124,7 @@ public Mono<OidcUser> loadUser(OidcUserRequest userRequest) throws OAuth2Authent
}

private Mono<OidcUserInfo> getUserInfo(OidcUserRequest userRequest) {
if (!OidcUserRequestUtils.shouldRetrieveUserInfo(userRequest)) {
if (!OidcUserRequestUtils.shouldRetrieveUserInfo(userRequest, accessibleScopes)) {
return Mono.empty();
}
// @formatter:off
Expand Down Expand Up @@ -169,4 +170,20 @@ public final void setClaimTypeConverterFactory(
this.claimTypeConverterFactory = claimTypeConverterFactory;
}

/**
* Sets the scope(s) that allow access to the user info resource. The default is
* {@link OidcScopes#PROFILE profile}, {@link OidcScopes#EMAIL email},
* {@link OidcScopes#ADDRESS address} and {@link OidcScopes#PHONE phone}. The scope(s)
* are checked against the "granted" scope(s) associated to the
* {@link OidcUserRequest#getAccessToken() access token} to determine if the user info
* resource is accessible or not. If there is at least one match, the user info
* resource will be requested, otherwise it will not.
* @param accessibleScopes the scope(s) that allow access to the user info resource
* @since 5.6
*/
public final void setAccessibleScopes(Set<String> accessibleScopes) {
Assert.notNull(accessibleScopes, "accessibleScopes cannot be null");
this.accessibleScopes = accessibleScopes;
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -21,6 +21,8 @@
import org.springframework.util.CollectionUtils;
import org.springframework.util.StringUtils;

import java.util.Set;

/**
* Utilities for working with the {@link OidcUserRequest}
*
Expand All @@ -42,10 +44,11 @@ final class OidcUserRequestUtils {
* @param userRequest
* @return
*/
static boolean shouldRetrieveUserInfo(OidcUserRequest userRequest) {
static boolean shouldRetrieveUserInfo(OidcUserRequest userRequest, Set<String> accessibleScopes) {
// Auto-disabled if UserInfo Endpoint URI is not provided
ClientRegistration clientRegistration = userRequest.getClientRegistration();
if (StringUtils.isEmpty(clientRegistration.getProviderDetails().getUserInfoEndpoint().getUri())) {
ClientRegistration.ProviderDetails providerDetails = clientRegistration.getProviderDetails();
if (StringUtils.isEmpty(providerDetails.getUserInfoEndpoint().getUri())) {
return false;
}
// The Claims requested by the profile, email, address, and phone scope values
Expand All @@ -59,9 +62,15 @@ static boolean shouldRetrieveUserInfo(OidcUserRequest userRequest) {
// Access Token being issued.
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());
// 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 accessibleScopes.isEmpty()
|| CollectionUtils.isEmpty(userRequest.getAccessToken().getScopes())
|| CollectionUtils.containsAny(userRequest.getAccessToken().getScopes(), accessibleScopes);
// @formatter:on
}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -34,7 +34,6 @@
import org.springframework.security.oauth2.client.userinfo.DefaultOAuth2UserService;
import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest;
import org.springframework.security.oauth2.client.userinfo.OAuth2UserService;
import org.springframework.security.oauth2.core.AuthorizationGrantType;
import org.springframework.security.oauth2.core.OAuth2AccessToken;
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
import org.springframework.security.oauth2.core.OAuth2Error;
Expand All @@ -48,7 +47,6 @@
import org.springframework.security.oauth2.core.oidc.user.OidcUserAuthority;
import org.springframework.security.oauth2.core.user.OAuth2User;
import org.springframework.util.Assert;
import org.springframework.util.CollectionUtils;
import org.springframework.util.StringUtils;

/**
Expand Down Expand Up @@ -105,7 +103,7 @@ public class OidcUserService implements OAuth2UserService<OidcUserRequest, OidcU
public OidcUser loadUser(OidcUserRequest userRequest) throws OAuth2AuthenticationException {
Assert.notNull(userRequest, "userRequest cannot be null");
OidcUserInfo userInfo = null;
if (this.shouldRetrieveUserInfo(userRequest)) {
if (OidcUserRequestUtils.shouldRetrieveUserInfo(userRequest, accessibleScopes)) {
OAuth2User oauth2User = this.oauth2UserService.loadUser(userRequest);
Map<String, Object> claims = getClaims(userRequest, oauth2User);
userInfo = new OidcUserInfo(claims);
Expand Down Expand Up @@ -154,37 +152,6 @@ private OidcUser getUser(OidcUserRequest userRequest, OidcUserInfo userInfo, Set
return new DefaultOidcUser(authorities, userRequest.getIdToken(), userInfo);
}

private boolean shouldRetrieveUserInfo(OidcUserRequest userRequest) {
// Auto-disabled if UserInfo Endpoint URI is not provided
ProviderDetails providerDetails = userRequest.getClientRegistration().getProviderDetails();
if (StringUtils.isEmpty(providerDetails.getUserInfoEndpoint().getUri())) {
return false;
}
// The Claims requested by the profile, email, address, and phone scope values
// are returned from the UserInfo Endpoint (as described in Section 5.3.2),
// when a response_type value is used that results in an Access Token being
// issued.
// However, when no Access Token is issued, which is the case for the
// response_type=id_token,
// the resulting Claims are returned in the ID Token.
// The Authorization Code Grant Flow, which is response_type=code, results in an
// Access Token being issued.
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
}
return false;
}

/**
* Sets the {@link OAuth2UserService} used when requesting the user info resource.
* @param oauth2UserService the {@link OAuth2UserService} used when requesting the
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -18,7 +18,10 @@

import java.time.Duration;
import java.time.Instant;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

import org.junit.jupiter.api.Test;

Expand All @@ -27,6 +30,7 @@
import org.springframework.security.oauth2.core.AuthorizationGrantType;
import org.springframework.security.oauth2.core.OAuth2AccessToken;
import org.springframework.security.oauth2.core.oidc.OidcIdToken;
import org.springframework.security.oauth2.core.oidc.OidcScopes;
import org.springframework.security.oauth2.core.oidc.TestOidcIdTokens;

import static org.assertj.core.api.Assertions.assertThat;
Expand All @@ -39,32 +43,36 @@ public class OidcUserRequestUtilsTests {

private ClientRegistration.Builder registration = TestClientRegistrations.clientRegistration();

private Set<String> accessibleScopes = new HashSet<>(
Arrays.asList(OidcScopes.PROFILE, OidcScopes.EMAIL, OidcScopes.ADDRESS, OidcScopes.PHONE));

OidcIdToken idToken = TestOidcIdTokens.idToken().build();

OAuth2AccessToken accessToken = new OAuth2AccessToken(OAuth2AccessToken.TokenType.BEARER, "token", Instant.now(),
Instant.now().plus(Duration.ofDays(1)), Collections.singleton("read:user"));

@Test
public void shouldRetrieveUserInfoWhenEndpointDefinedAndScopesOverlapThenTrue() {
assertThat(OidcUserRequestUtils.shouldRetrieveUserInfo(userRequest())).isTrue();
accessibleScopes.add("read:user");
assertThat(OidcUserRequestUtils.shouldRetrieveUserInfo(userRequest(), accessibleScopes)).isTrue();
}

@Test
public void shouldRetrieveUserInfoWhenNoUserInfoUriThenFalse() {
this.registration.userInfoUri(null);
assertThat(OidcUserRequestUtils.shouldRetrieveUserInfo(userRequest())).isFalse();
assertThat(OidcUserRequestUtils.shouldRetrieveUserInfo(userRequest(), accessibleScopes)).isFalse();
}

@Test
public void shouldRetrieveUserInfoWhenDifferentScopesThenFalse() {
this.registration.scope("notintoken");
assertThat(OidcUserRequestUtils.shouldRetrieveUserInfo(userRequest())).isFalse();
assertThat(OidcUserRequestUtils.shouldRetrieveUserInfo(userRequest(), accessibleScopes)).isFalse();
}

@Test
public void shouldRetrieveUserInfoWhenNotAuthorizationCodeThenFalse() {
this.registration.authorizationGrantType(AuthorizationGrantType.IMPLICIT);
assertThat(OidcUserRequestUtils.shouldRetrieveUserInfo(userRequest())).isFalse();
assertThat(OidcUserRequestUtils.shouldRetrieveUserInfo(userRequest(), accessibleScopes)).isFalse();
}

private OidcUserRequest userRequest() {
Expand Down