fix: Modernized JWT stateless introspection

[mitar]

Related issue

Fixes: #445

Proposed changes

This makes it so that you can:

compose.OAuth2TokenRevocationFactory,
compose.OAuth2TokenIntrospectionFactory,

replace with:

compose.OAuth2StatelessJWTIntrospectionFactory,

and it just works. From looking at the code it looked to me like this was done long time ago where some other current interfaces were not yet around. So I modernized the code to use existing interfaces and removed a specialized interface used just by previous implementation.

All tests still pass. But I am not 100% sure if everything is correct (I have some doubts about original code). I also have few TODOs around the code so input there would be appreciated.

Checklist

• I have read the contributing guidelines and signed the CLA.
• I have read the security policy.
• I confirm that this pull request does not address a security
  vulnerability. If this pull request addresses a security vulnerability, I
  confirm that I got green light (please contact
  security@ory.sh) from the maintainers to push
  the changes.
• I have added tests that prove my fix is effective or that my feature works.
• I have added necessary documentation within the code base (if appropriate).

[aeneasr]

Hm yeah good question. I think we generally assume that ID and Access Tokens use different private keys but that check is obviously implicit. I mean we could add the token_type to the JWT and set it to Bearer (ID Tokens are not HTTP Authorization Bearer Tokens) to check this?

[mitar]

I think we generally assume that ID and Access Tokens use different private keys but that check is obviously implicit.

Ohh. Is this documented anywhere? Because this is the first time I hear this. I was passing same private key to both NewOAuth2JWTECDSAStrategy and NewOpenIDConnectECDSAStrategy. It does not help that compose does it as well.

I mean we could add the token_type to the JWT and set it to Bearer

Is this claim standardized anywhere? I can see it some implementations do it, but I could not find a spec?

I see that currently ID tokens have auth_time and access tokens do not have it. And access tokens have scope and ID tokens do not have it (not sure if any spec controls this). I also have client_id in access tokens in my implementation. In my implementation scope is always required but I think fosite does not enforce that.

Maybe we could just make it that scope claim is always present in access tokens (even if empty string) and never in ID tokens and use that?

[mitar]

Personally, I would suggest that:

• Fosite introspection endpoint supports both ID tokens and access tokens (Support introspection of ID tokens #410).
  • Then here we do not have to care which one it is, we just return all claims.
• Caller of introspect has to check aud claim to verify that the token contains expected audience (which is in fact the main way to differentiate between ID tokens and access tokens, because they have different audience).
  • To make this easier, we add audience parameter to introspection endpoint, for caller to be able to specify expected audiences. Similar to how we currently have scope parameter already (both would then be a fosite extension, I have not found any spec for scope parameter).

[lazeratops]

I actually did not know ID and access tokens are assumed to use different private keys. I believe we have been using the same key for ID and access tokens on our end. I think I need to do some reading up on this. Does anyone know if this is this mentioned somewhere in the OIDC RFC?

[aeneasr]

They are fundamentally different tokens (proof of authentication, proof of authorization). They should get different keys to prevent mixing them together.

[aeneasr]

The access token keys should not exposed for external consumption IMO.

[mitar]

For JWTs of course they are? That is the whole point that you can validate JWT based access tokens without having to contact introspection endpoint.

[aeneasr]

Introspection is internal facing, not external.

[aeneasr]

This specification defines a method for a protected resource to query
an OAuth 2.0 authorization server to determine the active state of an
OAuth 2.0 token and to determine meta-information about this token.
OAuth 2.0 deployments can use this method to convey information about
the authorization context of the token from the authorization server
to the protected resource.

[mitar]

I mean, it is a protected resource. But that does not mean it has to be internal facing? It just has to be protected to prevent scanning attacks.

[mitar]

I made a new implementation of this. I think it addressed all of my TODOs. I think it reconstructs request as much as possible.
I think we should do #410 which addresses the issue of ID token vs. access token as well. Also, when we do #441 it will also address the question of username, because that will simply be mapped through the claims map, however it was encoded originally.

[aeneasr]

This looks good from my side! Anything left that need discussion/documentation or can I merge this?

[mitar]

I think it is good for merging from my side. I think you should tell me if I can implement #410 which I would also like, even if it is not by the spec.