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

Synapse can return 401 when accessing profile information #8520

Closed
LEdoian opened this issue Oct 12, 2020 · 2 comments · Fixed by #8580
Closed

Synapse can return 401 when accessing profile information #8520

LEdoian opened this issue Oct 12, 2020 · 2 comments · Fixed by #8580
Labels
z-bug (Deprecated Label) z-p3 (Deprecated Label)

Comments

@LEdoian
Copy link
Contributor

LEdoian commented Oct 12, 2020

Description

When accessing /_matrix/client/r0/profile/@user:domain:tld API endpoint, Synapse sometimes returns HTTP 401, which shouldn't be possible according to the Spec.

According to the log, this might be caused by forwarding responses from federation:

2020-10-11 14:17:15,841 - synapse.http.matrixfederationclient - 581 - WARNING - GET-394941 - {GET-O-111777} [conduit.rs] Request failed: GET matrix://conduit.rs/_matrix/federation/v1/query/profile?user_id=%40timo%3Aconduit.rs&field=displayname: HttpResponseException('401: Unauthorized')
2020-10-11 14:17:15,841 - synapse.http.server - 76 - INFO - GET-394941 - <XForwardedForRequest at 0x7f7134cee7b8 method='GET' uri='/_matrix/client/r0/profile/@timo:conduit.rs' clientproto='HTTP/1.0' site=8008> SynapseError: 401 - Unauthorized
2020-10-11 14:17:15,842 - synapse.access.http.8008 - 311 - INFO - GET-394941 - Red.act.ed.IP4 - 8008 - {None} Processed request: 0.038sec/-0.000sec (0.010sec, 0.000sec) (0.000sec/0.000sec/0) 46B 401 "GET /_matrix/client/r0/profile/@timo:conduit.rs HTTP/1.0" "Python/3.8 aiohttp/3.6.2" [0 dbevts]

Some clients misunderstand this as an authentication token rejection, logging the user out.

Steps to reproduce

  • curl -D - 'https://matrix.org/_matrix/client/r0/profile/@timo:conduit.rs'

(This endpoint is not authenticated, so this should work.)

Expected result: Either returning HTTP 200 with the response, or falling back to HTTP 404, not to confuse clients.

Version information

  • Homeserver: Log above is from from dolujeme.eu, but seems to work even on matrix.org.

If not matrix.org:

  • Version: 1.20.1 (according to pip freeze)
  • Install method: pip
  • Platform: Debian

Additional information

There are some database errors regarding the request as well, snippet is here. But that is probably unrelated / worth separate issue.

@LEdoian
Copy link
Contributor Author

LEdoian commented Oct 12, 2020

For completeness: the response of Synapse to curl -D - 'https://matrix.org/_matrix/client/r0/profile/@timo:conduit.rs':

HTTP/2 401 
date: Mon, 12 Oct 2020 00:27:29 GMT
content-type: application/json
set-cookie: __cfduid=db7bf1c3334800fa2233c2c36537709971602462449; expires=Wed, 11-Nov-20 00:27:29 GMT; path=/; domain=.matrix.org; HttpOnly; SameSite=Lax
cache-control: no-cache, no-store, must-revalidate
access-control-allow-origin: *
access-control-allow-methods: GET, HEAD, POST, PUT, DELETE, OPTIONS
access-control-allow-headers: Origin, X-Requested-With, Content-Type, Accept, Authorization
cf-cache-status: DYNAMIC
cf-request-id: 05bbcd961b0000277c341d8200000001
expect-ct: max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct"
server: cloudflare
cf-ray: 5e0cb2035ef3277c-PRG

{"errcode":"M_UNKNOWN","error":"Unauthorized"}

@anoadragon453
Copy link
Member

It's worth noting that the option require_auth_for_profile_requests can be enabled in Synapse's config in order to reject /profile/* requests without an access token. However, this defaults to false, and is false on matrix.org.

I can reproduce the 401 with your provided cURL command, but not with, say curl -D - 'https://matrix.org/_matrix/client/r0/profile/@andrewm:amorgan.xyz', which suggests the problem has something to do with which profile you query.

When Synapse attempts to query the profile from another homeserver, it makes a request, and if it receives an http code other than 200, it'll blindly return that code to the client along with an M_UNKNOWN error code:

try:
result = await self.federation.make_query(
destination=target_user.domain,
query_type="profile",
args={"user_id": target_user.to_string(), "field": "displayname"},
ignore_backoff=True,
)
except RequestSendFailed as e:
raise SynapseError(502, "Failed to fetch profile") from e
except HttpResponseException as e:
raise e.to_synapse_error()

Synapse will not return a 401 over federation in response to a profile query:

async def on_profile_query(self, args):
user = UserID.from_string(args["user_id"])
if not self.hs.is_mine(user):
raise SynapseError(400, "User is not hosted on this homeserver")
just_field = args.get("field", None)
response = {}
try:
if just_field is None or just_field == "displayname":
response["displayname"] = await self.store.get_profile_displayname(
user.localpart
)
if just_field is None or just_field == "avatar_url":
response["avatar_url"] = await self.store.get_profile_avatar_url(
user.localpart
)
except StoreError as e:
if e.code == 404:
raise SynapseError(404, "Profile was not found", Codes.NOT_FOUND)
raise
return response

Thus, what I think is happening is Conduit is returning a 401, and Synapse is relaying that. The spec doesn't mention that a 401 can be returned over federation here, so Conduit may want to change this behaviour.

Additionally, Synapse needs to fix its error handling here so that regardless of what it receives from other servers, it will not return error codes to the client that it's not expected.

@anoadragon453 anoadragon453 added z-bug (Deprecated Label) z-p3 (Deprecated Label) labels Oct 12, 2020
LEdoian pushed a commit to LEdoian/synapse that referenced this issue Oct 17, 2020
fixes matrix-org#8520

Signed-off-by: Pavel Turinsky <pavel.turinsky@matfyz.cz>
erikjohnston added a commit that referenced this issue Oct 26, 2020
Fixes #8520

Signed-off-by: Pavel Turinsky <pavel.turinsky@matfyz.cz>

Co-authored-by: Erik Johnston <erikj@jki.re>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
z-bug (Deprecated Label) z-p3 (Deprecated Label)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants