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

Enhance OAuth2TokenExchangeAuthenticationProvider to support additional trusted issuers #1867

Open
joshuawhite929 opened this issue Dec 27, 2024 · 14 comments
Assignees
Labels
type: enhancement A general enhancement

Comments

@joshuawhite929
Copy link

Expected Behavior
OAuth2TokenExchangeAuthenticationProvider should be enhanced to support subject/actor tokens from other trusted issuers

Current Behavior
Today, OAuth2TokenExchangeAuthenticationProvider validates/authorizes the subject/actor token by looking for the JWT in the configured OAuth2AuthorizationService. Additional trusted issuers are not supported in the current implementation.

Context
The current OAuth2TokenExchangeAuthenticationProvider constrains token exchange process to a single IDP. I believe the spirit of RFC 8693 is to also enable token exchange across security domains.

If this is something that the team is willing to support, I have a working example of how OAuth2TokenExchangeAuthenticationProvider could be modified to support this need.

CC: @sjohnr , @jgrandja

@joshuawhite929 joshuawhite929 added the type: enhancement A general enhancement label Dec 27, 2024
@sjohnr
Copy link
Member

sjohnr commented Jan 6, 2025

@joshuawhite929

The current OAuth2TokenExchangeAuthenticationProvider constrains token exchange process to a single IDP. I believe the spirit of RFC 8693 is to also enable token exchange across security domains.

Can you please say more about why you feel the current provider constrains token exchange? Have you explored adding additional parameters to the request? What would be needed that isn't currently possible to support multiple IdPs?

@sjohnr sjohnr self-assigned this Jan 6, 2025
@sjohnr
Copy link
Member

sjohnr commented Jan 6, 2025

Apologies @joshuawhite929 if my above response is confusing, I was thinking about this from the Spring Security (client) side. Please feel free to share any supporting information or an example you are thinking about for supporting multiple IdPs on the server side. However, please consider that the initial implementation was intentionally limited to a single IdP so we could have more conversation prior to targeting more advanced features such as this one.

@jgrandja jgrandja added the status: waiting-for-feedback We need additional information before we can continue label Jan 6, 2025
@joshuawhite929
Copy link
Author

I am definitely interested in the server side piece of this. I am currently on vacation and will be back with more detailed comments on Friday.

@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 Jan 7, 2025
@jgrandja jgrandja added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jan 7, 2025
@joshuawhite929
Copy link
Author

My companies OSS process is taking longer than I anticipated. Would you prefer to wait or close this issue and have me open another when ready?

@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 Jan 12, 2025
@sjohnr
Copy link
Member

sjohnr commented Jan 13, 2025

Hi @joshuawhite929.

Would you prefer to wait or close this issue and have me open another when ready?

That depends I suppose. You wouldn't need to provide anything specific to your internal environment, I am just looking for some details on a use case at a high (e.g. abstract) level. In the opening comment you mention:

Additional trusted issuers are not supported in the current implementation.

If you can add more clarity around the details of what you're requesting here (a specific use case), that would be helpful since I'm not sure I have enough information from that statement to determine what enhancement would be targeted by this issue. If you feel you're unable to provide additional details at the moment, then I think we should close this ticket for now as I feel it is not actionable. Does that make sense?

@sjohnr sjohnr added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jan 13, 2025
@joshuawhite929
Copy link
Author

joshuawhite929 commented Jan 13, 2025

I am looking to use a JWT as a authorization grant to obtain an access token from a different authorization server.

Today, the Spring Authorization Server assumes that the JWT specified by the subject_token/actor_token originates from itself. To facilitate a cross-domain token exchange, I am hoping to validate/trust a JWT from a different issuer. Does this make sense?

I believe this is briefly mentioned in the spec:

Whereas, urn:ietf:params:oauth:token-type:jwt is to indicate specifically that a JWT is being requested or sent (perhaps in a cross-domain use case where the JWT is used as an authorization grant to obtain an access token from a different authorization server as is facilitated by [RFC7523]).

@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 Jan 13, 2025
@sjohnr
Copy link
Member

sjohnr commented Jan 13, 2025

I believe this is briefly mentioned in the spec

It is hinted at in the spec as you mentioned there, but it unfortunately does not spell out any details for how such a use case would work. I think that the spec is both broad and narrow, broad in the sense that many combinations of request parameters are technically valid, but narrow in the sense that very few such combinations have clearly defined behavior. Beyond the basic use cases supported by the initial implementation, we quickly run out of clear usage patterns.

My interpretation of the spec is that these other use cases could be defined by a specific implementation of a purpose-built security token service (STS) that clearly documents the use case(s) it intends to solve, what requests or parameter combinations are valid, what out-of-band setup is required to use them, etc. The use case you mention:

To facilitate a cross-domain token exchange,

...is still very general and while it might make sense, we would need a more specific use case to determine whether it makes sense for the framework to support it. I think there are still a variety of use cases that could be possible based on the broad definitions found in the spec. So focusing on what you have provided for the moment:

I am hoping to validate/trust a JWT from a different issuer

Simply validating an input token is not quite enough. There are certain outputs that we expect from this process. Currently, we can obtain all the information needed from the OAuth2AuthorizationService, which is why we only support internal tokens currently. To support an external token such as a JWT issued by another authorization server, we would need to define a contract that outputs all of the necessary details.

Some details are just used for additional validation, so they could be kept internal to the implementation. Beyond that, we need to know what "principal" (or end user) is represented by the combination of a subject token and an optional actor token. This whole process would end up being fairly generic and would probably look a lot like the existing OAuth2TokenExchangeAuthenticationProvider which provides the current implementation, except that it would use a JwtDecoder (from Spring Security) to perform the validation and extract claims.

If we do all of that, we still wouldn't have enough information to interpret the various allowed combinations of parameters in the request. For example, what if we want to support two issuers? What about a multi-tenant setup? Do we support combinations of internal subjects and external actors or vice versa? And what policies or validation are needed in each case? All of this would need to be accounted for, and would likely result in a fairly complex setup.

For now, I think just referencing the docs for how to implement an extension grant type and plugging in a custom AuthenticationProvider (see OAuth2TokenExchangeAuthenticationProvider) is a good place to start. If there's some learning you can take away from that, you can report back and perhaps we can discuss a more concrete use case.

What do you think?

@joshuawhite929
Copy link
Author

I will look into this and get back to you

@joshuawhite929
Copy link
Author

IMHO - 95% of the existing code/tests can be reused. I'm thinking that the only difference would be the "strategy" used to validate/authenticate the subject and actor tokens.

If we prefer to have this implemented in a different AuthenticationProvider class, can we consider pushing 95% of the code into an perhaps use a template method to validate/authenticate the subject and actor tokens?

Thoughts on this approach?

@sjohnr
Copy link
Member

sjohnr commented Jan 17, 2025

@joshuawhite929 please feel free to create a branch on your fork and give the idea a try. Share a link to the branch here and I'll take a look and provide feedback.

Thoughts on this approach?

I think I would prefer to see code at this point, as I'm curious how you would address some of the issues/thoughts I pointed out in my previous comment.

@jgrandja jgrandja removed the status: feedback-provided Feedback has been provided label Jan 20, 2025
@joshuawhite929
Copy link
Author

I have pushed a new branch here. Here is a couple of quick notes on what you will see:

  • I moved 95% of the code from OAuth2TokenExchangeAuthenticationProvider to a new super class, BaseOAuth2TokenExchangeAuthenticationProvider. This abstract class calls on a "TokenAuthenticator" to authenticate either the subject or actor token. This was done to make unit testing easier.
  • OAuth2TokenExchangeAuthenticationProvider creates a TokenAuthenticator using the authentication logic that was previously these
  • TrustedIssuerOAuth2TokenExchangeAuthenticationProvider is a new class that was inspired from JwtIssuerAuthenticationManagerResolver. This class lets the end user configure a JwtDecoder to "authenticate" a JWT from a specific issuer.
  • The lion share of the unit tests were moved to BaseOAuth2TokenExchangeAuthenticationProviderTests allowing both TrustedIssuerOAuth2TokenExchangeAuthenticationProviderTests and OAuth2TokenExchangeAuthenticationProviderTests to leverage them

Looking forward to your feedback.

CC: @sjohnr , @jgrandja

@sjohnr
Copy link
Member

sjohnr commented Feb 14, 2025

Thanks for providing a branch with your idea, @joshuawhite929! I have reviewed the branch and do understand what you were thinking about in your earlier comment.

While this is definitely helpful for imagining what a more general purpose Token Exchange provider could look like, there are several issues with the approach at present.

One issue is that you have effectively copied the authentication mechanism from spring-security-oauth2-resource-server for JWTs only into a very specific token exchange implementation. What about opaque tokens from another issuer? How would they be supported? Also, in addition to performing the JWT parsing, validation, and authorities conversion that already exist in Spring Security's JwtAuthenticationProvider, you have also copied the multi-tenancy capabilities of JwtIssuerAuthenticationManagerResolver. At a minimum, it would be better if these pieces were able to be reused instead of reproduced in this branch. If you were simply trying to get a concept working, and intend to refactor later to reuse those, then that's fine and this bit of feedback doesn't apply at present.

More to the point though, the fact that authentication logic needs to exist inside of this token exchange implementation means we're coupling the authentication subsystem of Spring Security with the OAuth2-based authorization features provided by SAS. This project has aimed to fully keep these two separate, and so coupling them is not the right direction here. This is also highlighted by the use of a UserDetailsService, which should not be used directly in SAS since it again is intrinsically linked to Authentication, a Spring Security concern.

Another issue is related to the TokenAuthenticator abstraction you introduced, which returns an OAuth2Authorization. This is nice for the existing token exchange implementation, because it doesn't have to change much. But it means that any other implementation would just be a shim on top of the existing one, always needing to adapt to it. This doesn't really drive the abstraction to the place where it is general purpose enough to be reusable. It works, but that's only half the battle. I also see the non-trivial complexity on top of the existing implementation needed to adapt trusted issuer JWT validation to token exchange to be a major downside of this approach. Reusing code could help, but it doesn't make it any less complex in the end.

Lastly, I don't see anywhere in the branch where you are adding your new implementation as another AuthenticationProvider for the OAuth2TokenEndpointFilter to use. This is absolutely key, because it will be difficult (and potentially conflicting) to create logic for handling different combinations of request parameters for token exchange, especially if opaque tokens are supported. The filter (or another layer of code) will need to figure out which AuthenticationProvider implementation to delegate to. As it exists in your branch, I think it's an all-or-nothing choice, either you use the existing implementation, or you swap it out for TrustedIssuer.... What if I want to mix and match subject and actor tokens somehow? This all-or-nothing approach isn't very flexible, so something would need to be done to improve this.

Again, thanks for providing your thoughts as a branch. It is very helpful for constructive dialog. Does this feedback help in any way? As it stands, I don't think we're very close to something that could take Token Exchange support forward, but perhaps with more prototyping something could emerge. What do you think?

@joshuawhite929
Copy link
Author

This example was meant as a prototype to show how this may work. Appreciate the feedback. It sounds like we need to get alignment on a couple of core ideas before moving forward. I'd like to start with coupling authentication logic. From the spec:

A Security Token Service (STS) is a service capable of validating security tokens provided to it and issuing new security tokens in response, which enables clients to obtain appropriate access credentials for resources in heterogeneous environments or across security domains.

The existing OAuth2TokenExchangeAuthenticationProvider only needs to perform basic validation (checking for expiry) because it is the issuer of the token and essentially validating/authenticating the token by finding it in its own implicitly trusted internal token store (this.authorizationService.findByToken). As a result, we can avoid performing traditional token validation/authentication.

As I see it, the challenge is that within the scope of a single request, in addition to authenticating the client, we also need to authenticate the subjectToken and potentially the actorToken. While I understand your desire to avoid coupling the authentication subsystem of Spring Security, if you are processing a token from a different issuer, how else would you perform this validation and avoid this coupling?

@sjohnr
Copy link
Member

sjohnr commented Feb 26, 2025

@joshuawhite929 thanks for following up.

As I see it, the challenge is that within the scope of a single request, in addition to authenticating the client, we also need to authenticate the subjectToken and potentially the actorToken. While I understand your desire to avoid coupling the authentication subsystem of Spring Security, if you are processing a token from a different issuer, how else would you perform this validation and avoid this coupling?

This is a clear example of the challenge that we face by adding generalized Token Exchange support to Spring Authorization Server. I feel that building a general purpose STS is not directly in scope for this framework. However, it could be achieved by specific implementations within a product that uses Spring Authorization Server as a foundation.

I think this is best viewed as a question around whether there's potential for a strategy to introduce pluggable implementations into the framework or not. In other words, can we create an abstraction (interface) for a general purpose STS within Spring Authorization Server? If so, we can then leave users to plug in whatever implementations they want to support.

If not, I think it would still be fairly achievable to build an STS as one or more custom endpoints using Spring MVC (e.g. @Controller etc.) and directly use pieces like OAuth2AuthorizationService to provision tokens and other pieces for anything else that's needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants