Skip to content
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

[Azure Oauth2] IllegalArgumentException: Attribute value for "xxx" is null #16340

Open
jloisel opened this issue Dec 24, 2024 · 7 comments
Open
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: feedback-provided Feedback has been provided type: bug A general bug

Comments

@jloisel
Copy link

jloisel commented Dec 24, 2024

Hi,

We are having the following issue when trying to configure Azure SSO Authentication using Spring Security Oauth2:

{ json: { log: "2024-12-20 12:42:53,285 [http-nio-8090-exec-40] ERROR o.a.c.c.C.[.[.[.[dispatcherServlet] - Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception java.lang.IllegalArgumentException: Attribute value for 'preferred_username' cannot be null at org.springframework.util.Assert.notNull(Assert.java:181) at org.springframework.security.oauth2.core.user.DefaultOAuth2User.(DefaultOAuth2User.java:72) at org.springframework.security.oauth2.client.userinfo.DefaultOAuth2UserService.loadUser(DefaultOAuth2UserService.java:99) at org.springframework.security.oauth2.client.oidc.userinfo.OidcUserService.loadUser(OidcUserService.java:115) at org.springframework.security.oauth2.client.oidc.userinfo.OidcUserService.loadUser(OidcUserService.java:68) at org.springframework.security.oauth2.client.oidc.authentication.OidcAuthorizationCodeAuthenticationProvider.authenticate(OidcAuthorizationCodeAuthenticationProvider.java:158) at org.springframework.security.authentication.ProviderManager.authenticate(ProviderManager.java:182) at org.springframework.security.authentication.ObservationAuthenticationManager.lambda$authenticate$1(ObservationAuthenticationManager.java:54) at io.micrometer.observation.Observation.observe(Observation.java:565) at org.springframework.security.authentication.ObservationAuthenticationManager.authenticate(ObservationAuthenticationManager.java:53) at org.springframework.security.oauth2.client.web.OAuth2LoginAuthenticationFilter.attemptAuthentication(OAuth2LoginAuthenticationFilter.java:196) at org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter.doFilter(AbstractAuthenticationProcessingFilter.java:231) at

Seems linked to #15338.

If we use "sub" username attribute, there is no longer the issue (but sub cannot be mapped to an email on our system), but we can dump the user in an Oauth2AuthenticationSuccessHandler:

2024-12-20 08:19:46,820 [http-nio-8090-exec-7] ERROR - attrs={sub=xxxxxxx, ver=2.0, iss=https://login.microsoftonline.com/xxxxxx/v2.0, oid=xxxxx, preferred_username=[email protected], uti=xxxxxx, given_name=John, nonce=xxxx, picture=https://graph.microsoft.com/v1.0/me/photo/$value, tid=xxxxxx, aud=[xxxx], nbf=Fri Dec 20 08:14:45 UTC 2024, rh=xxxx, name=John Smith, exp=2024-12-20T09:19:45Z, family_name=Smith, iat=2024-12-20T08:14:45Z, email=[email protected]}

The claims have "preferred_username" in Oauth2AuthenticationSuccessHandler but if we use any as user-name-attribute it fails with a null pointer exception (despite being in the user claims afterwards).

I believe the null check is done too early and claims seem to be filled afterwards.

We are using Spring Security 6.4.2, Spring Boot 3.4.1.

@jloisel jloisel added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Dec 24, 2024
@jloisel jloisel changed the title IllegalArgumentException: Attribute value for "xxx" is null [Azure Oauth2] IllegalArgumentException: Attribute value for "xxx" is null Dec 24, 2024
@jloisel
Copy link
Author

jloisel commented Jan 9, 2025

Code failing on Spring Security side:

	public DefaultOAuth2User(Collection<? extends GrantedAuthority> authorities, Map<String, Object> attributes,
			String nameAttributeKey) {
		Assert.notEmpty(attributes, "attributes cannot be empty");
		Assert.hasText(nameAttributeKey, "nameAttributeKey cannot be empty");
		Assert.notNull(attributes.get(nameAttributeKey),
				"Attribute value for '" + nameAttributeKey + "' cannot be null");

		this.authorities = (authorities != null)
				? Collections.unmodifiableSet(new LinkedHashSet<>(this.sortAuthorities(authorities)))
				: Collections.unmodifiableSet(new LinkedHashSet<>(AuthorityUtils.NO_AUTHORITIES));
		this.attributes = Collections.unmodifiableMap(new LinkedHashMap<>(attributes));
		this.nameAttributeKey = nameAttributeKey;
	}

If we use "preferred_username" as attribute:

java.lang.IllegalArgumentException: Attribute value for 'preferred_username' cannot be null
	at org.springframework.util.Assert.notNull(Assert.java:181)
	at org.springframework.security.oauth2.core.user.DefaultOAuth2User.<init>(DefaultOAuth2User.java:72)
	at org.springframework.security.oauth2.client.userinfo.DefaultOAuth2UserService.loadUser(DefaultOAuth2UserService.java:99)
	at org.springframework.security.oauth2.client.oidc.userinfo.OidcUserService.loadUser(OidcUserService.java:115)
	at org.springframework.security.oauth2.client.oidc.userinfo.OidcUserService.loadUser(OidcUserService.java:68)
	at org.springframework.security.oauth2.client.oidc.authentication.OidcAuthorizationCodeAuthenticationProvider.authenticate(OidcAuthorizationCodeAuthenticationProvider.java:158)

If we use "sub" as name attribute key, we are able to proceed further into our Oauth2AuthenticationSuccessHandler and log all user attributes:

  @Override
  public void onAuthenticationSuccess(
    final HttpServletRequest request,
    final HttpServletResponse response,
    final Authentication authentication) throws IOException {

    final OAuth2AuthenticationToken token = (OAuth2AuthenticationToken) authentication;
    final String registrationId = token.getAuthorizedClientRegistrationId();
    final OAuth2User p = token.getPrincipal();

    final Pair<String, String> pair = repository.getUsernameSuffixAndAllowedDomains(registrationId);
    final String username = lowerCase(getUsername(p, pair.getKey()));

    log.error(
      "[SSO] id='{}', username='{}', attrs='{}', domains='{}'",
      registrationId, username, p.getAttributes(), allowedDomains
    );
....
}

Where we can see that all attributes are there:

ERROR com.octoperf.tc - [SSO] Domain not allowed - id='azure', username='xxxxx', attrs='{sub=xxxxx, ver=2.0, iss=https://login.microsoftonline.com/xxxx/v2.0, oid=xxxx, preferred_username=[email protected], uti=sssss, given_name=John, nonce=xxxxx, picture=https://graph.microsoft.com/v1.0/me/photo/$value, tid=ssss, aud=[xxxx], nbf=Thu Jan 09 08:37:02 UTC 2025, rh=xxxx, name=User XX, exp=2025-01-09T09:42:02Z, family_name=XX, iat=2025-01-09T08:37:02Z, email=[email protected]}', domains='mail.com'

That means, "preferred_username" is an attribute but it's not working, despite being there.

@jloisel
Copy link
Author

jloisel commented Feb 6, 2025

I have more info about the issue after debugging.

The issue is in OidcUserService:

@Override
	public OidcUser loadUser(OidcUserRequest userRequest) throws OAuth2AuthenticationException {
		Assert.notNull(userRequest, "userRequest cannot be null");
		OidcUserInfo userInfo = null;
		if (this.retrieveUserInfo.test(userRequest)) {
			OAuth2User oauth2User = this.oauth2UserService.loadUser(userRequest);
			Map<String, Object> claims = getClaims(userRequest, oauth2User);
			userInfo = new OidcUserInfo(claims);
			// https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse
			// 1) The sub (subject) Claim MUST always be returned in the UserInfo Response
			if (userInfo.getSubject() == null) {
				OAuth2Error oauth2Error = new OAuth2Error(INVALID_USER_INFO_RESPONSE_ERROR_CODE);
				throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString());
			}
			// 2) Due to the possibility of token substitution attacks (see Section
			// 16.11),
			// the UserInfo Response is not guaranteed to be about the End-User
			// identified by the sub (subject) element of the ID Token.
			// The sub Claim in the UserInfo Response MUST be verified to exactly match
			// the sub Claim in the ID Token; if they do not match,
			// the UserInfo Response values MUST NOT be used.
			if (!userInfo.getSubject().equals(userRequest.getIdToken().getSubject())) {
				OAuth2Error oauth2Error = new OAuth2Error(INVALID_USER_INFO_RESPONSE_ERROR_CODE);
				throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString());
			}
		}
		return this.oidcUserMapper.apply(userRequest, userInfo);
	}

First, the Oauth2 user is created using oauth2UserService:

OAuth2User oauth2User = this.oauth2UserService.loadUser(userRequest);

In this case, the user has only limited attributes (4 in our case, like "sub"). Later on:

return this.oidcUserMapper.apply(userRequest, userInfo);

Which delegates to:

org.springframework.security.oauth2.client.oidc.userinfo.OidcUserRequestUtils#getUser

This is where the DefaultOidcUser instance is created with all claims being available (coming from OidcIdToken.getClaims()).

The bug is in

		Assert.notNull(attributes.get(nameAttributeKey),
				"Attribute value for '" + nameAttributeKey + "' cannot be null");

Checking for nameAttributeKey here is too early. The first DefaultOauth2User being created has not the nameAttributeKey claim populated at this point. The claims are available later when querying the userinfo endpoint. Later on, Spring Security re-creates an instance of DefaultOidcUser with all the claims populated.

The bug has been introduced in this commit:
e69e0eb

This check cannot be performed this early.

jloisel added a commit to OctoPerf/spring-security that referenced this issue Feb 6, 2025
jloisel added a commit to OctoPerf/spring-security that referenced this issue Feb 6, 2025
@jloisel
Copy link
Author

jloisel commented Feb 13, 2025

@jzheaux

Hi,

This bug is preventing us from using Oauth2 authentication. I suspect it's breaking any oauth2 integration which has the username-attribute claim provided by the userinfo endpoint. Can you please take a look?

Best Regards,

@jloisel
Copy link
Author

jloisel commented Feb 24, 2025

Associated pull request: #16546

@sjohnr sjohnr self-assigned this Feb 26, 2025
@sjohnr
Copy link
Member

sjohnr commented Feb 26, 2025

@jloisel thanks for following up on this, and apologies that we didn't see this earlier.

This bug is preventing us from using Oauth2 authentication. I suspect it's breaking any oauth2 integration which has the username-attribute claim provided by the userinfo endpoint. Can you please take a look?

I have personally used the user-name-attribute with (for example) Auth0 since this commit was introduced. It does work in that case. It sounds like the difference with Azure is that the claim is not coming in the id_token.

Can you please put together a minimal reproducer that includes your Spring Security @Configuration and any application.yml or properties for Spring Boot (with any credentials removed)? This will help me ensure I have your exact client-side setup. I can then set up a test with the SSO provider to reproduce.

@sjohnr sjohnr added status: waiting-for-feedback We need additional information before we can continue in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 26, 2025
@jloisel
Copy link
Author

jloisel commented Feb 27, 2025

Hi,

I understand you are probably quite busy! You will have to find a provider which doesn't provide the user-name-attribute in in the first claims. I don't know which other SSO provider works that way. We also have a Google SSO integration and this one works well (but all claims are provided by the first call).

You don't even need any specific Spring Security configuration. You can use a sample SSO, as long as you use Azure SSO (maybe other SSO providers working same way as Azure are broken too).

application.yml: (use your own clientId / clientSecret and TENANT_ID)

    spring.security.oauth2.client:
      registration:
        azure:
          client-id: XXX
          client-secret: XXX
          name: "Azure"
          scope: openid,profile,email
          client-authentication-method: client_secret_basic
          authorization-grant-type: authorization_code
          redirect-uri: "{baseUrl}/{action}/oauth2/code/{registrationId}"
          client-name: azure
      provider:
        azure:
          authorization-uri: "https://login.microsoftonline.com/TENANT_ID/oauth2/v2.0/authorize"
          token-uri: "https://login.microsoftonline.com/TENANT_ID/oauth2/v2.0/token"
          user-info-uri: "https://graph.microsoft.com/oidc/userinfo"
          jwk-set-uri: "https://login.microsoftonline.com/TENANT_ID/discovery/v2.0/keys"
          issuer-uri: "https://login.microsoftonline.com/TENANT_ID/v2.0"
          user-name-attribute: preferred_username

You can use the default login page and try to login. It will fail with a null pointer if the user-name-attribute is not provided in the first claims.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 27, 2025
@sjohnr
Copy link
Member

sjohnr commented Feb 27, 2025

You will have to find a provider which doesn't provide the user-name-attribute in in the first claims.

If I can't get a developer account, I often just write integrations tests or customize Spring Authorization Server to emulate the misbehaving or problematic provider. I can handle that no problem. Thanks for the config.

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: feedback-provided Feedback has been provided type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants