Spring-security: Simplify access to OAuth 2.0 token attributes

[jzheaux]

Related to #5200 and https://github.com/spring-projects/spring-security/pull/6352#discussion_r253137232

We should introduce OAuth2TokenAttributes, a class that holds an OAuth 2.0 Token's attributes. To this point, each authentication token has exposed attributes simply as a Map.

It works out best, though, when an new class's usage can be demonstrated, ensuring that the API is correct. So, to complete this ticket, a few things need to be done:

• [ ] Introduce OAuth2TokenAttributes

This is a simple domain object that contains a Map:

public class OAuth2TokenAttributes {
    <T> T getAttribute(String name);
    Map<String, Object> getAttributes();
}

It should probably go in oauth2-core in org.springframework.security.oauth2.core.

• [ ] Change OAuth2IntrospectionAuthenticationToken's constructor

It should take an OAuth2TokenAttributes for its principal instead of a Map. While this is a breaking change, it's allowable since that API has not GA'd yet

• [ ] Update the oauth2resourceserver-opaque sample

This sample should be adjusted to use the token's principal (OAuth2TokenAttributes)

Extra Background

Spring Security offers different ways to access OAuth 2.0 token attributes, depending on how they were obtained.

For example, a Resource Server can authenticate via JWT tokens and obtain their contents in the following way:

@GetMapping("/protected")
public String method(@AuthenticationPrincipal Jwt jwt) {
    String subject = jwt.getSubject();
    // ...
}

// and

@GetMapping("/protected")
public String method(@AuthenticationPrincipal(expression="subject") String subject) {
    // ...
}

When using introspection, though, the principal is simply a map, so the pattern is:

@GetMapping("/protected")
public String method(@AuthenticationPrincipal Map<String, Object> attributes) {
    String subject = (String) attributes.get("sub");
    // ...
}

// and 

@GetMapping("/protected")
public String method(@AuthenticationPrincipal(expression="['sub']") String subject) {
    // ...
}

It would be nice if application code didn't have to know the token verification strategy to extract the attributes from the principal.

Further, there is potentially some clean up here with clarifying the meaning around attributes and claims. Claims are unverified attributes. Or, in other words, a JWT has a claim set, but the ensuing Authentication object has attributes since the JWT at that point is verified.

There is already one way to get attributes in an agnostic way:

@GetMapping("/protected")
public String method(AbstractOAuth2TokenAuthenticationToken token) {
    String subject = (String) token.getTokenAttributes().get("sub");
    // ...
}

But it would be nicer if this were a bit simpler and users could take advantage of more Spring Security features agnostically.

By thinking more about the principal and how to expose it, it may be possible to achieve something like:

@GetMapping("/protected")
public String method(@AuthenticationPrincipal OAuth2TokenAttributes attributes) {
    String subject = attributes.getAttribute("sub");
    // ...
}

// and

@GetMapping("/protected")
public String method(@AuthenticationPrincipal(expression="attribute['sub']") String subject) {
    // ...
}

that works for any OAuth 2.0 Authentication type.

[kwolfp]

Hello,
this issue is available for first timer?

[jzheaux]

Hi, @kwolfp, thanks for your interest! There is still a bit of discussion among the team as to what this might look like. It's not finalized that what's been described will actually be the way it is implemented, so I'd recommend waiting.

[rwinch]

I'm wondering if we can do something like this:

GetMapping("/protected")
public String method(@CurrentSecurityContext(expression = "authentication.tokenAttributes") Map<String, Object> attrs) {
    String subject = (String) attrs.get("sub");
    // ...
}

I like this because this also solves other attributes we might need. @AuthenticationPrincipal could be changed to work with @CurrentSecurityContext(expression = "authentication?.principal")

[jzheaux]

I'm of the mind that @CurrentAuthentication would be more common. Or, at least, I think that I would almost never do:

@GetMapping("/protected")
public String method(@CurrentSecurityContext SecurityContext context) {
}

And so I'd always be at least typing @CurrentSecurityContext(expression="authentication followed by whatever I needed from it.

What about:

@GetMapping("/protected")
public String method(@CurrentAuthentication(expression="tokenAttributes") Map<String, Object> attributes) {
    String subject = (String) attrs.get("sub");
    // ...
}

Also, what I like about both is that it's possible for a user to do something like:

@GetMapping("/protected")
public String method(@OAuth2TokenAttributes Map<String, Object> attributes) {
    String subject = (String) attrs.get("sub");
    // ...
}

with minimal effort by doing a meta-annotation.

[kwolfp]

@jzheaux when I tried to implement your idea with @OAuth2TokenAttributes Map<String, Object> ... (just for fun), I cannot implement resolver for @OAuth2TokenAttributes because MapMethodProcessor resolvs all parameters with Map type.

[jzheaux]

Thanks for looking into that, @kwolfp.

If MapMethodProcessor takes precedence regardless, then a solution based off of resolving to a Map won't be ideal.

This points me in the direction of a first-class object to represent attributes, e.g. OAuth2TokenAttributes yielding @CurrentAuthentication(expression="some expression") OAuth2TokenAttributes attributes

[rwinch]

@jzheaux I'd prefer to make it resolve the SecurityContext since it can do anything then. We can always provide our own @CurrentAuthentication that is annotated with @CurrentSecurityContext(expression = "authentication")

[jzheaux]

K, makes sense, @rwinch.

I think we're ready to split this out a bit, then. I've created a ticket to introduce @CurrentSecurityContext so that can easily be identified as a separate and new feature. Here we can continue to discuss how to get token attributes in a unified way, likely based on that new feature.

[jzheaux]

I propose we introduce the OAuth2TokenAttributes class. It would contain the original token and its associated attributes.

I'd imagine that we'd introduce AbstractOAuth2TokenAuthenticationToken#getAttributes:

public OAuth2TokenAttributes getAttributes() {
    return new OAuth2TokenAttributes(getTokenValue(), getTokenAttributes());
}

This would then enable an application to do:

public String method(@CurrentSecurityContext(expression="authentication.attributes") 
        OAuth2TokenAttributes attributes)

Or:

@CurrentSecurityContext(expression="authentication.attributes")
public @interface CurrentOAuth2TokenAttributes

// ...

public String method(@CurrentOAuth2TokenAttributes OAuth2TokenAttributes attributes)

[rwinch]

@jzheaux Can you outline what OAuth2TokenAttributes looks like (what methods are on it)?

[jzheaux]

Good question, @rwinch.

OAuth2TokenAttributes would have at least two methods in it:

public <A> A getAttribute(String name);
public Map<String, Object> getAttributes();

Originally, I was thinking it would also contain String getTokenValue(); however, not all OAuth Authentication implementations keep track of the raw token, so I think that would limit getTokenValue's utility.

[jzheaux]

@kwolfp I believe we can start work on this ticket now. Are you still interested in submitting a PR?

[jzheaux]

I've just summarized the work to be done for this ticket up in the description.

[clementkng]

Hi @jzheaux, I'm interested in trying out this ticket.

[jzheaux]

Sounds great, @clementkng, glad to have you working on it.

Note that the discussion is probably good for background, but the consensus can be found in the description itself.

[clementkng]

Hi @jzheaux, I have a couple questions.

On the high level, my understanding is that the OAuth2TokenAttributes class is going to wrap both the token and the attributes map, so that the application does not need to handle the token verification step (as mentioned in the first code sample), which will instead be handled in one place by this new class. Does this mean that we are going to overhaul everything in the oauth2resourceserver-opaque to use the new class, or should there be places where we convert between attributes map and OAuth2TokenAttributes representation? For example, in OAuth2IntrospectionAuthenticationToken, do we change the atributes instance variable in addition to the constructor to be of type OAuth2TokenAttributes?

I was also wondering if it was necessary to introduce a new test class for OAuth2TokenAttributes or simply modify the existing tests for OAuth2TokenAttributes.

[jzheaux]

@clementkng Thanks for bringing up these points.

wrap both the token and the attributes

I realize that there was a bit of discussion about this in the thread, but let's just do the attributes for now. You can find the proposed contract at the top of the description.

overhaul everything in the oauth2resourceserver-opaque

I'm not sure what you mean by this. As far as I can tell, updating the sample should be a simple matter of updating its controller methods. Am I missing something, or is this what you were asking?

change the attributes instance

No, just the constructor is fine. OAuth2IntrospectionAuthenticationToken will still have an attributes member, just now derived from the OAuth2TokenAttributes sent in the constructor.

[clementkng]

@jzheaux I've created a PR for the issue here.