Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Caching of token introspection response when using MSC3861 auth #16087

Closed
hughns opened this issue Aug 8, 2023 · 7 comments
Closed

Caching of token introspection response when using MSC3861 auth #16087

hughns opened this issue Aug 8, 2023 · 7 comments
Assignees
Labels
A-Performance Performance, both client-facing and admin-facing O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience roadmap S-Minor Blocks non-critical functionality, workarounds exist.

Comments

@hughns
Copy link
Member

hughns commented Aug 8, 2023

Description:

When experimental support for MSC3861 delegated auth by OIDC is enabled the Synapse will call the token introspection endpoint on the OpenID Provider for each invocation of get_user_by_access_token().

This is adding latency on all authenticated requests and load on the OpenID Provider.

Instead the Synapse could cache the result for some period of time.

Things to consider:

  • what happens when the token is proactively revoked by the OpenID Provider

CC @sandhose @erikjohnston

@clokep
Copy link
Member

clokep commented Aug 8, 2023

FWIW We cache queries from the database and do cache invalidation, so we should be able to do something similar here with a backchannel logout or a custom API.

We probably also have to worry about if the user's claims change?

@pmaier1 pmaier1 added the roadmap label Aug 9, 2023
@erikjohnston erikjohnston added A-Performance Performance, both client-facing and admin-facing S-Minor Blocks non-critical functionality, workarounds exist. O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience labels Aug 9, 2023
@hughns
Copy link
Member Author

hughns commented Aug 14, 2023

In terms of timings on this: ideally we would have this in the RC that is cut on 2023-08-22 so that it can make it into the release on 2023-08-29.

@H-Shay
Copy link
Contributor

H-Shay commented Aug 14, 2023

In terms of timings on this

Got it, will prioritize this.

@H-Shay
Copy link
Contributor

H-Shay commented Aug 14, 2023

A question about implementing this: reading through the RFC for token revocation I saw information about how a client could revoke a token with the authorizing server, but not how the authorizing server should interact with the resource server after revoking a token (I am assuming Synapse is the resource server and Matrix Authentication Server is the authorizing server?) - although the RFC did contain this comment here:

"In the former case, some (currently non-standardized) backend
interaction between the authorization server and the resource server
may be used when immediate access token revocation is desired."

which I think is saying that it's essentially an implementation detail whether the authorizing server alerts the resource server if a token is revoked? Since presumably we are in control of the behavior of both the authorizing server and the resource server, it seems we could implement an endpoint in Synapse to listen for when an access token is revoked (I imagine this is what @clokep was talking about when mentioning a custom API?), which would be mirrored in the Matrix Authentication Service to send alerts about the revocation.

If all of this is correct (I am not familiar with OIDC or the Matrix Authentication Service, this has just been gleaned from the MSCs and RFCs and code so please correct me if I am wrong), then it seems like it the plan for this issue would be to:

  • add an expiring cache to get_user_by_access_token (sub-question: is there a way to set the cache expiration other than setting the cache factors in the config?)
  • add an endpoint that can be hit when the auth service revokes a token which invalidates the cache
  • add logic to the backchannel logout code to invalidate the cache when a backchannel logout request is received

Am I missing anything/does this make sense?

@sandhose
Copy link
Member

You are right that there is no standard way to signal token expiry. The OIDC CAEP events might help in the future, but right now I'd rather add a custom admin API on the Synapse side for this.

In terms of implementing this, I think it makes more sense to cache the _introspect_token method, probably with a small TTL (~5min ?), and make sure to validate the exp field after the introspection (which is not the case right now). If we properly evaluate the exp field it means that MAS won't have to send revocation notification on every token expiry, but rather when the user revokes the token. Note that this field might not be set for token that don't have an expiration set.

For designing the MAS -> Synapse API, what would be helpful is to use the jti as key for revoking the cache entries. The jti is supposed to be a unique token ID, so it makes sense that MAS uses that to revoke specific token.

@H-Shay
Copy link
Contributor

H-Shay commented Aug 15, 2023

Here is a preliminary PR for this: #16117
Erik suggested separating out adding the cache and adding the revocation endpoint, so I will do that in a follow-up/separate PR.

@H-Shay
Copy link
Contributor

H-Shay commented Aug 16, 2023

The follow-up PR the one mentioned above is this: #16125.
I wasn't sure what the endpoint should look like (beyond @sandhose's comment that it should be an admin api) so the PR represents my best guess about that.

@hughns hughns closed this as completed Aug 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Performance Performance, both client-facing and admin-facing O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience roadmap S-Minor Blocks non-critical functionality, workarounds exist.
Projects
None yet
Development

No branches or pull requests

6 participants