Add storage for Personal Access Tokens #5106
Conversation
crates/storage-pg/migrations/20250924132713_personal_access_tokens.sql
Outdated
Show resolved
Hide resolved
crates/storage-pg/migrations/20250924132713_personal_access_tokens.sql
Outdated
Show resolved
Hide resolved
|
|
||
| TokenType::PersonalAccessToken => { | ||
| // TODO | ||
| return Err(RouteError::UnknownToken(TokenType::PersonalAccessToken)); |
There was a problem hiding this comment.
A few things I have in mind: this needs to check that both the actor and the owner are still active.
Deactivating (not locking) users should revoke all their tokens
Validating those tokens should also be done for requests to the GraphQL endpoint (optional maybe, we want to deprecate 'admin' access to the GraphQL API) and for the admin API access
There was a problem hiding this comment.
Noted down. Given our plans with GraphQL I'm tempted to say that we just don't accept PATs there; cuts off one more opportunity for people to get locked into a deprecated API.
Deploying matrix-authentication-service-docs with
|
| Latest commit: |
277e8e8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://374be286.matrix-authentication-service-docs.pages.dev |
| Branch Preview URL: | https://rei-pat-1.matrix-authentication-service-docs.pages.dev |
reivilibre
force-pushed
the
rei/pat_1
branch
2 times, most recently
from
0783870 to
9dffe4f
Compare
| pub state: SessionState, | ||
| pub owner: PersonalSessionOwner, | ||
| pub actor_user_id: Ulid, | ||
| pub human_name: String, |
There was a problem hiding this comment.
I'm wondering if this should be optional… but I guess most tools force you to have a name
There was a problem hiding this comment.
That was my thought really; other tools make you put in a name and it seems self-sabotaging to not have a label for your PATs, so it seems like a good idea to just have one for simplicity.
| pub id: Ulid, | ||
| pub state: SessionState, | ||
| pub owner: PersonalSessionOwner, | ||
| pub actor_user_id: Ulid, |
There was a problem hiding this comment.
I'm now thinking that potentially you could have no actor (like you don't need one when talking to the MAS admin API) but then the main reason for you to use PATs (and not just the client credentials grant) is if you need to access Synapse and MAS, so probably not worth supporting
There was a problem hiding this comment.
Also thought the same really. Seems like extra kerfuffle for no apparent benefit; could be relaxed later if it turns out to be useful though
2 participants
This PR defines a token format for PATs and implements the base storage functionality for PATs and 'Personal Sessions' which are conceptually the parent containers of PATs.
Personal Sessions survive across regenerations of PATs and are the entities associated with the device ID. In virtually every way they are functionally the same as Compat or OAuth2 Sessions.
Future work that will follow soon after this PR:
Part of: #4492