Skip to content
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

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Aug 28, 2024

@dbkr dbkr added the proposal A matrix spec change proposal label Aug 28, 2024
@turt2live turt2live added client-server Client-Server API kind:maintenance MSC which clarifies/updates existing spec needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. identity service and removed client-server Client-Server API labels Aug 28, 2024
@turt2live
Copy link
Member

I don't feel as though implementation is required for this MSC.

@mscbot fcp merge

@mscbot
Copy link
Collaborator

mscbot commented Aug 28, 2024

Team member @mscbot has proposed to merge this. The next step is review by the rest of the tagged people:

Concerns:

  • lack of clarity about changes to requestToken

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.

@turt2live turt2live removed the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Aug 28, 2024
@mscbot mscbot added proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. disposition-merge needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels Aug 28, 2024
@turt2live turt2live removed the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Aug 28, 2024
@dbkr dbkr changed the title MSC4180: Additional Error Codes for submitToken endpoint MSC4183: Additional Error Codes for submitToken endpoint Aug 29, 2024
@richvdh
Copy link
Member

richvdh commented Nov 5, 2024

@mscbot concern lack of clarity about changes to requestToken

@mscbot mscbot added the unresolved-concerns This proposal has at least one outstanding concern label Nov 5, 2024
@turt2live turt2live requested a review from richvdh November 26, 2024 01:15
dbkr and others added 2 commits November 26, 2024 17:59
Co-authored-by: Richard van der Hoff <[email protected]>
and how it isn't really a thing in practice
dbkr added 2 commits November 29, 2024 09:34
...from the other requestToken enpoints

Also try to further clarify the note about the largely unused POST email submitToken
Copy link
Member

@turt2live turt2live left a 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.
Copy link
Member

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully clearer

Copy link
Member

@richvdh richvdh left a 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

@richvdh
Copy link
Member

richvdh commented Feb 19, 2025

@mscbot resolve lack of clarity about changes to requestToken

@mscbot mscbot removed the unresolved-concerns This proposal has at least one outstanding concern label Feb 19, 2025
@richvdh richvdh added the client-server Client-Server API label Feb 19, 2025
@mscbot
Copy link
Collaborator

mscbot commented Feb 19, 2025

🔔 This is now entering its final comment period, as per the review above. 🔔

@mscbot mscbot added final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. and removed proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. labels Feb 19, 2025
dbkr and others added 2 commits February 19, 2025 10:13
Co-authored-by: Richard van der Hoff <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client-server Client-Server API disposition-merge final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. identity service kind:maintenance MSC which clarifies/updates existing spec proposal A matrix spec change proposal
Projects
Status: Ready for FCP ticks
Development

Successfully merging this pull request may close these issues.

6 participants