Expected Behavior

I was trying to configure my app to use the pagerduty oauth2 provider, I was using the following configuration:

spring:
  security:
    oauth2:
      client:
        registration:
          pagerduty:
            provider: pagerduty
            clientId: <redacted>
            clientSecret: <redacted>
            authorizationGrantType: authorization_code
            redirectUri: "{baseUrl}/login/oauth2/code/{registrationId}"
            clientAuthenticationMethod: client_secret_post

        provider:
          pagerduty:
            authorizationUri: "https://identity.pagerduty.com/oauth/authorize"
            tokenUri: "https://identity.pagerduty.com/oauth/token"
            userInfoUri: "https://api.pagerduty.com/users/me"
            userNameAttribute:  user.email

The pagerduty /me api returns users with all the interesting properties nested under the user field, like so:

{
  "user": {
    "id": ...,
    "email": "fancy@pants.com",

I tried setting userNameAttribute: user.email expecting the name field to be extracted as the name property on the user object. But I get an error from DefaultOAuth2User: "Missing attribute 'user.email' in attributes.

Current Behavior

Ideally a user would successfully be extracted, and login would be successful.

Context

I wound up exposing a custom OAuth2UserService class, but it's 90% of a copy paste of DefaultOAuth2UserService, and this seems like something that another user info API might reasonably do.

Comment From: jzheaux

Hi, @leeavital, thanks for the suggestion. I think this sounds reasonable.

I believe three steps are needed to make this work.

First, DefaultOAuth2User should have a new constructor and deprecate the existing one. The new one should look like this:

public DefaultOAuth2User(Map<String, Object> attributes, Collection<? extends GrantedAuthority> authorities, String name)

Second, Spring Security should update existing production code (not tests) to use the new constructor.

Third, DefaultOAuth2UserService should change to process the userNameAttribute as a simple SpEL expression like so:

SimpleEvaluationContext context = SimpleEvaluationContext
        .forPropertyAccessors(new MapAccessor())
        .withRootObject(userAttributes).build();
SpelExpressionParser parser = new SpelExpressionParser();
Expression expression = parser.parseExpression(userNameAttributeName);
String name = (String) expression.getValue(context);
return new DefaultOAuth2User(userAttributes, authorities, name);

Would you be interested in submitting a PR to make these changes? If not, I can make the changes instead and invite you to review the PR.

Comment From: spring-projects-issues

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

Comment From: ahmd-nabil

@jzheaux I think I can do that. is it ok if I work on it ?

Comment From: jzheaux

Sounds good, @ahmd-nabil

Comment From: jzheaux

Closing in favor of https://github.com/spring-projects/spring-security/pull/14265

Comment From: Julius-Babies

Hello, when setting my user-name-attribute to data.email, which is the correct key for my case[^1], I still get the exception Missing attribute 'data.email' in attributes. The error occurred in ...core.user.DefaultOAuth2User. I took a look at your PR but I discovered, that you didn't change this file. My spring version is at 3.2.4. Did I misconfigure something or can I force it to use your class which should be inside org...oauth2.client.userinfo....?

When adding a breakpoint in IntelliJ, I discovered, that the returned object for my userinfo is a LinkedHashMap containing more LinkedHashMaps, so the returned data by the server is correct.

I don't know if this issue covered DefaultOAuth2User.java. I suggest defining some separator character (maybe /, I don't which chars are allowed in JSON keys) and searching for them recursively.


1: This is what the data looks like: ```json { "data": { "email": "users email address", "more": "data" }, "meta": { "other": "data" } }

Comment From: Julius-Babies

Ok, sorry for this, I didn't realise that this wasn't included in the Spring Boot Starter release. Here is a working example with this PR.