Clarify that an access token is optional on /account/password and /account/deactivate

[zecakeh]

The descriptions of the endpoints clearly state that the access token is not required, but it also says:

Requires Authentication: Yes

This changes it to show:

Requires Authentication: Optional

Fixes #1807.

Pull Request Checklist

• Pull request includes a changelog file
• Pull request includes a sign off
• Pull request is classified as 'other changes'

Preview: https://pr1843--matrix-spec-previews.netlify.app

[uhoreg]

Looking at the synapse code, /account/password indeed doesn't seem to require authentication, but /account/deactivate does, AFAICT. I haven't checked what other servers do yet. I think we may need to untangle whether the discrepancy in /account/deactivate is a spec bug or a synapse bug.

[zecakeh]

From what I can tell both Dendrite and Conduit seem to require authentication for both endpoints.

[richvdh]

It took me quite a while to grok what was going on here. Clearly, both endpoints require authentication of some form -- anything else would be anarchy where people go around changing other peoples' passwords -- so the phrasing of the PR is a bit confusing.

So, for the record: both endpoints require authentication via UIA. The question at stake is whether they also need an Authorization header with an access token. The spec is inconsistent. As @uhoreg says, Synapse's implementation of /account/deactivate requires an access token, but that of /account/password does not.

[richvdh]

Doing some archaeology:

/password was implemented in https://github.com/matrix-org/synapse/pull/126/files#diff-20445d423fa1657115c589560de173027dbba8b9a866b21fb3592cd55e30dc50R31
and specced in matrix-org/matrix-spec-proposals#190. We can see from the start that the access token was not required if you did UIA with m.login.email.identity, which remains the case today; however the swagger implied that an access token was required. @zecakeh's change seems correct here and corrects a 9-year-old spec bug!

/deactivate was implemented in matrix-org/synapse#921 and specced in matrix-org/matrix-spec-proposals#361. The spec implies that an access token may not be required, but in practice, an access token was required (https://github.com/matrix-org/synapse/pull/921/files#diff-20445d423fa1657115c589560de173027dbba8b9a866b21fb3592cd55e30dc50R150).

My impression is that a server could implement /deactivate without the access token, for example by allowing UIA with m.login.token, and be spec-compliant. The fact that no implementation has yet done so isn't important.

In short: LGTM

[zecakeh]

It took me quite a while to grok what was going on here. Clearly, both endpoints require authentication of some form -- anything else would be anarchy where people go around changing other peoples' passwords -- so the phrasing of the PR is a bit confusing.

In that case, should we consider that the phrasing of the spec is also confusing? Do we need to change "Requires Authentication" to "Requires Access Token"?

[richvdh]

In that case, should we consider that the phrasing of the spec is also confusing? Do we need to change "Requires Authentication" to "Requires Access Token"?

I think something like that would be good, yes. Not sure exactly what words to use though.