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

Synapse doesn't require new_password on every call to /account/password, contrary to the spec. (But maybe this is a spec oversight?) #13187

Open
DMRobertson opened this issue Jul 5, 2022 · 2 comments
Labels
A-Spec-Compliance places where synapse does not conform to the spec S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@DMRobertson
Copy link
Contributor

DMRobertson commented Jul 5, 2022

Link to problem area:

The spec says that new_password is a required field.

new_password string Required: The new password for the account.

However, this endpoint is part of the user-interactive auth stuff, and Synapse only requires you to have provided new_password at least once during a sequence of calls to /account/password. If you've provided a new_password as part of the user-interactive auth, you don't have to provide it with later requests.

This is against the letter of the law as mandated by the spec (but perhaps not the spirit). This might also fall under the category of "user-interactive auth is weird; give us matrix-org/matrix-spec#636 please).

Noticed in #13183.

@DMRobertson DMRobertson added A-Spec-Compliance places where synapse does not conform to the spec S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Jul 5, 2022
@richvdh
Copy link
Member

richvdh commented Jul 5, 2022

This might also fall under the category of "user-interactive auth is weird; give us matrix-org/matrix-spec#636 please.

This, I think. I might be lacking context, but given password management in particular is going to be replaced by OIDC, it's not obviously a thing particularly worth worrying about right now.

@DMRobertson
Copy link
Contributor Author

it's not obviously a thing particularly worth worrying about right now.

Agreed; just getting it written down somewhere for completeness (though the right place might be a spec issue).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Spec-Compliance places where synapse does not conform to the spec S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

2 participants