-
Notifications
You must be signed in to change notification settings - Fork 389
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MSC4183: Additional Error Codes for submitToken endpoint #4183
base: main
Are you sure you want to change the base?
Conversation
I don't feel as though implementation is required for this MSC. @mscbot fcp merge |
Team member @mscbot has proposed to merge this. The next step is review by the rest of the tagged people: Concerns:
Once at least 75% of reviewers approve (and there are no outstanding concerns), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for information about what commands tagged team members can give me. |
Co-authored-by: Johannes Marbach <[email protected]>
Spelling / grammar fixes Co-authored-by: Andrew Morgan <[email protected]>
@mscbot concern lack of clarity about changes to requestToken |
Co-authored-by: Richard van der Hoff <[email protected]>
and how it isn't really a thing in practice
...from the other requestToken enpoints Also try to further clarify the note about the largely unused POST email submitToken
Co-authored-by: Richard van der Hoff <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-asserting approval as a read marker
/_matrix/identity/v2/validate/email/submitToken`](https://spec.matrix.org/v1.11/identity-service-api/#post_matrixidentityv2validateemailsubmittoken)) | ||
is not generally used in practice: Sydent's emails includes a link to click instead of the `submit_url` response field and | ||
therefore use the `GET` version. Synapse does not implement the `POST` API for email validation for this reason. This | ||
proposal updates both for consistency. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what exactly does "both" refer to here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully clearer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am now finally up to speed, and other than a few minor editorial nits, this lgtm
@mscbot resolve lack of clarity about changes to requestToken |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
Co-authored-by: Richard van der Hoff <[email protected]>
Rendered
Impls:
Sydent: matrix-org/sydent#592
Synapse: element-hq/synapse#17625
Element Web/Desktop: matrix-org/matrix-react-sdk#12936
FCP tickyboxes