This repository was archived by the owner on Apr 26, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
This repository was archived by the owner on Apr 26, 2024. It is now read-only.
Clean up synapse.api.auth.Auth #13019
Copy link
Copy link
Closed
Labels
T-TaskRefactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Description
I'm trying to make sense to what is in synapse.api.auth.Auth
.
The goal is to extract a clear interface for verifying access tokens, so we can override that more easily for the OIDC work.
I already cleaned up things related to access token verification & macaroons in #12986
I'm finding a few method types:
- things related to checking if a user can do something:
check_user_in_room_or_world_readable
check_can_change_room_list
check_user_in_room
is_server_admin
- static methods to get the access token in a request
has_access_token
get_access_token_from_request
- methods to get the requester of a request
get_user_by_access_token
get_user_by_req
get_appservice_by_req
There is also check_auth_blocking
, which only acts as a proxy to AuthBlocking.check_auth_blocking
.
Note that is_server_admin
is also a simple proxy to RegistrationWorkerStore.is_server_admin
.
The other interesting thing I found out is that check_user_in_room
is called from three places:
check_user_in_room_or_world_readable
(which makes sense)- in handlers related to typing notifications (which also makes sense, since you can't send typing notifications in rooms you're not in)
- in
rest.client.room.TimestampLookupRestServlet
, which looks like an oversight, and should probably callcheck_user_in_room_or_world_readable
instead
Other interesting things about a method: get_appservice_by_req
is called from three places:
/register
, which seems not necessary, since a few lines bellow it would raise anyway if the appservice does not existDELETE /directory/room/<room_alias>
, for some reason?/login
, which looks like the only legit place where it is called
So, what I would like to do is:
- remove the
check_auth_blocking
method, and use directlyAuthBlocking.check_auth_blocking
where it makes sense Decouplesynapse.api.auth_blocking.AuthBlocking
fromsynapse.api.auth.Auth
. #13021 - move the two static methods as regular functions outside of the class
- make the
TimestampLookupRestServlet
usecheck_user_in_room_or_world_readable
Allow MSC3030 'timestamp_to_event' calls from anyone on world-readable rooms. #13062 - group and move the permissions checks to their own class
- remove the usage of
get_appservice_by_req
:
Metadata
Metadata
Assignees
Labels
T-TaskRefactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.