matrix-org / synapse

Synapse: Matrix homeserver written in Python/Twisted.
https://matrix-org.github.io/synapse
Apache License 2.0
11.82k stars 2.13k forks source link

Appservice login works for sender_localpart even if the user isn't registered, but returned access token doesn't work #16269

Open tulir opened 1 year ago

tulir commented 1 year ago

Description

The appservice login mechanism seems to allow logging in as the sender_localpart user, even if the user doesn't exist. The result is extremely confusing, as the login will return a success with an access token, but trying to use the access token will just throw M_UNKNOWN_TOKEN. Doing the same with a namespace user rather than the sender_localpart will throw "Application service has not registered this user" in the login call.

Steps to reproduce

  1. Register an appservice
  2. Use appservice login to log in as the sender_localpart user and receive a token back
  3. Try to use the returned token and get M_UNKNOWN_TOKEN
  4. Be confused

Synapse Version

1.92.0rc1

Database

PostgreSQL

Workers

Single process

Relevant log output

2023-09-06 22:33:47,767 - synapse.rest.client.login - 277 - INFO - POST-70 - Got appservice login request with identifier: {'type': 'm.id.user', 'user': 'sabot'}
2023-09-06 22:33:47,784 - synapse.handlers.auth - 997 - INFO - POST-70 - Logging in user @sabot:localhost on device SVMYUSNZRL
2023-09-06 22:33:47,790 - synapse.access.http.8008 - 465 - INFO - POST-70 - 127.0.0.1 - 8008 - {@sabot:localhost} Processed request: 0.023sec/0.000sec (0.002sec, 0.000sec) (0.001sec/0.018sec/4) 140B 200 "POST /_matrix/client/v3/login HTTP/1.1" "curl/7.88.1" [0 dbevts]
2023-09-06 22:33:55,561 - synapse.api.auth.internal - 288 - WARNING - GET-72 - Invalid access token in auth: <class 'pymacaroons.exceptions.MacaroonDeserializationException'> cannot determine data format of binary-encoded macaroon.
2023-09-06 22:33:55,562 - synapse.http.server - 124 - INFO - GET-72 - <XForwardedForRequest at 0x7efd2c672450 method='GET' uri='/_matrix/client/v3/account/whoami' clientproto='HTTP/1.1' site='8008'> SynapseError: 401 - Invalid access token passed.
2023-09-06 22:33:55,562 - synapse.access.http.8008 - 465 - INFO - GET-72 - 127.0.0.1 - 8008 - {None} Processed request: 0.002sec/0.000sec (0.000sec, 0.000sec) (0.000sec/0.001sec/1) 88B 401 "GET /_matrix/client/v3/account/whoami HTTP/1.1" "curl/7.88.1" [0 dbevts]
clokep commented 1 year ago

That does sound quite confusing!

It looks like the clause around here is incorrect:

https://github.com/matrix-org/synapse/blob/4d0231b3648d5d70a8e0f4d99a0c040f12f15669/synapse/api/auth/base.py#L160-L162

My hunch is that we just should remove that special clause since the is_interested_in_user checks the same thing internally. And you do still need the user to be registered to use it?

tulir commented 1 year ago

Using the as_token directly for other endpoints does mostly work without /registering, so theoretically some appservices may be relying on that if they just don't use appservice /login.

I don't see anything in the spec saying whether calling /register for the sender_localpart should be required or not. If it turns out that it isn't required, then Synapse should probably auto-register the sender_localpart on startup, so all endpoints work properly. Either way the special case can be removed.