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

Incorrect HTTP Status returned on unknown/invalid access token #2602

Closed
Tracked by #3506
maxidorius opened this issue Oct 29, 2017 · 6 comments
Closed
Tracked by #3506

Incorrect HTTP Status returned on unknown/invalid access token #2602

maxidorius opened this issue Oct 29, 2017 · 6 comments

Comments

@maxidorius
Copy link
Contributor

maxidorius commented Oct 29, 2017

As per latest stable and unstable spec, 401 must be returned but 403 is returned instead.

This leads to further spec breakage down the line for endpoints that are supposed to return 403 for another error, like (not complete list):

  • /_matrix/client/r0/rooms/{roomId}/state/{eventType}/{stateKey}
  • /_matrix/client/r0/rooms/{roomId}/state/{eventType}
  • /_matrix/client/r0/rooms/{roomId}/state
  • /_matrix/client/r0/rooms/{roomId}/members
  • /_matrix/client/r0/rooms/{roomId}/joined_members
  • /_matrix/client/r0/rooms/{roomId}/messages
  • /_matrix/client/r0/rooms/{roomId}/invite
  • etc.

On some of those endpoints, the client would be expected to behave differently, depending if the token is invalid or if authorization to perform the call was lacking (invalidate session & re-login VS give up specific action).

@her001
Copy link
Contributor

her001 commented Nov 14, 2017

I believe I've found a comment in the code which refers to this:

Matrix spec makes no reference to what HTTP status code is returned,
but the V1 API uses 403 where it means 401, and the webclient
relies on this behaviour, so V1 gets its own copy of the auth
with backwards compat behaviour.

I'm not sure what should be done, but I assume this is the reason for the bug.

@NotAFile
Copy link
Contributor

I was curious, and as far as I can tell, this has been fixed:

$ curl -v 'https://matrix.org/_matrix/client/r0/account/whoami?access_token=192837918'
[...]
< HTTP/2 401

Same if the token is missing.

@maxidorius
Copy link
Contributor Author

maxidorius commented Apr 30, 2018

It's still not.

Without access token:

$ curl -vd '{}' 'https://matrix.org/_matrix/client/r0/rooms/!dummy:matrix.org/invite'
[...]
< HTTP/1.1 403 Forbidden
[...]
{
    "errcode": "M_MISSING_TOKEN",
    "error": "Missing access token."
}
$ curl -v 'https://matrix.org/_matrix/client/r0/rooms/!dummy:matrix.org/members'
[...]
< HTTP/1.1 403 Forbidden
[...]
{
    "errcode": "M_MISSING_TOKEN",
    "error": "Missing access token."
}

With invalid access token:

$ curl -vd '{}' 'https://matrix.org/_matrix/client/r0/rooms/!dummy:matrix.org/invite?access_token=1'
[...]
< HTTP/1.1 403 Forbidden
[...]
{
    "errcode": "M_UNKNOWN_TOKEN",
    "error": "Unrecognised access token."
}
$ curl -v 'https://matrix.org/_matrix/client/r0/rooms/!dummy:matrix.org/members?access_token=1'
[...]
< HTTP/1.1 403 Forbidden
[...]
{
    "errcode": "M_UNKNOWN_TOKEN",
    "error": "Unrecognised access token."
}

@NotAFile
Copy link
Contributor

NotAFile commented Apr 30, 2018

So here's what I've been able to discover about this issue and why it only affects some endpoints:

There used to be two versions: v1 and v2_alpha, which were later merged into r0, however they are still separate in the synapse code. Any endpoints that are identical to the original v1 spec are served by the v1 code path (which returns 403), while anything else is served by the v2_alpha code path (which returns 401). This causes the mix between the two behaviours.

The solution would be to simply move the v1 endpoints to the new behaviour. This might possibly break the old angular client, which is deprecated anyway.

@ara4n
Copy link
Member

ara4n commented Apr 30, 2018

i suspect when we fixed this on the v2 servlets we forgot to fix the v1 servlets, and it'd be fine to apply the same fix there (and hope that no clients are relying on the incorrect behaviour....)

@NotAFile
Copy link
Contributor

@ara4n hope is good, but client authors should be notified ahead of time in case anyone is relying on it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants