ory / fosite

Extensible security first OAuth 2.0 and OpenID Connect SDK for Go.
https://www.ory.sh/?utm_source=github&utm_medium=banner&utm_campaign=fosite
Apache License 2.0
2.28k stars 356 forks source link

fix: rfc9068 must condition ignored in introspection #767

Open james-d-elliott opened 10 months ago

james-d-elliott commented 10 months ago

This fixes an outstanding TODO where the requirement for a correctly formed RFC9068 access token MUST have the media type of 'application/at+jwt', and that this media type MUST be appropriately reflected in the typ header as either 'at+jwt' (SHOULD), or as the media type itself. See https://datatracker.ietf.org/doc/html/rfc9068#section-2.1 for more information.

Related Issue or Design Document

Checklist

Further comments

mitar commented 4 months ago

Tests are missing.

Also, I could not find where in fosite we set type for access tokens to at+jwt? It seems to me we only set it to JWT.

james-d-elliott commented 4 months ago

I must have missed the tests and the change which makes the JWT Profile default to this typ when picking this out of local changes. Let me see what I have there.

mitar commented 4 months ago

So there is no typ for ID tokens? Have you made a test which makes sure we are not setting now this new typ for ID tokens as well?

james-d-elliott commented 4 months ago

So there is no typ for ID tokens? Have you made a test which makes sure we are not setting now this new typ for ID tokens as well?

This change doesn't affect that code path, just GetJWTHeader() *jwt.Headers not IDTokenHeaders() *jwt.Headers.

mitar commented 4 months ago

Any thoughts on how to transition this on a live system? There might be a period when old valid access tokens are returned as invalid because they have old typ?

(This is not a problem for me, but it might be for other users of fosite.)

james-d-elliott commented 4 months ago

I have made sure the function is exported for this reason. I think from a practical standpoint they should be considered invalid or the implementer should implement their own validator which allows both based on iat claim. i.e. tokens issued before x should be safe if they have the JWT typ. However I think it would be irresponsible for security to make this a default option, and implementers should consider all tokens absent this typ to be invalid.