MSC4183: Additional Error Codes for submitToken endpoint #4183
Conversation
turt2live
added
client-server
|
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. |
turt2live
removed
the
needs-implementation
mscbot
added
proposed-final-comment-period
turt2live
removed
the
needs-implementation
Co-authored-by: Johannes Marbach <[email protected]>
dbkr
changed the title
| although that MSC is for `requestToken` on the C/S API only. | ||
|
|
||
| The [`POST /_matrix/client/v3/account/3pid/email/requestToken`](https://spec.matrix.org/v1.11/client-server-api/#post_matrixclientv3account3pidemailrequesttoken) endpoint in the C/S API also specifies a `submit_url` response parameter, defining its parameters to | ||
| be the same as the Identity API's `submitToken` endpoints. This MSC also affects this. |
There was a problem hiding this comment.
This MSC also affects this.
Could you briefly expand upon how it does so? The proposal section doesn't mention this.
| Also change the C/S API's definition of [`POST /_matrix/client/v3/account/3pid/email/requestToken`](https://spec.matrix.org/v1.11/client-server-api/#post_matrixclientv3account3pidemailrequesttoken) | ||
| to specify that the entire API is the same, including response / error codes, rather than just parameters. |
There was a problem hiding this comment.
I don't understand what you mean by "specify that the entire API is the same".
| `M_NO_VALID_SESSION` if the code is incorrect. Once an identity server (or homeserver) switches to | ||
| use the new error code, they may not recognise the error condition correctly until updated to support | ||
| the new code. We say that this is acceptable in favour of avoiding the complexity of negotiating error |
There was a problem hiding this comment.
Once an identity server (or homeserver) switches to use the new error code, they may not recognise the error condition correctly until updated to support the new code.
I'm failing to understand what this means. Is "they" the homeserver?
There was a problem hiding this comment.
Yeah, that wasn't super clear. Is that better?
| ## Proposal | ||
|
|
||
| Add the following specific error code as a code that can be returned by the two endpoints given above: | ||
| * `M_TOKEN_INCORRECT`: Indicates that the token that the user entered to validate the session is incorrect. |
There was a problem hiding this comment.
Would it be worth re-using/redefining M_UNKNOWN_TOKEN from the C-S API?
There was a problem hiding this comment.
On second thoughts... that's talking about an access token whereas this is the user entering a code that's been sent to them, so they're not really the same thing. I think maybe it's clearer if it's a separate code? I can update the MSC to call this out if you agree.
There was a problem hiding this comment.
I think maybe it's clearer if it's a separate code?
(I agree)
Spelling / grammar fixes Co-authored-by: Andrew Morgan <[email protected]>
| Also change the C/S API's definition of [`POST | ||
| /_matrix/client/v3/account/3pid/email/requestToken`](https://spec.matrix.org/v1.11/client-server-api/#post_matrixclientv3account3pidemailrequesttoken) | ||
| to specify that the endpoint is the same as the I/S API version in all ways, including response / error codes, rather than just parameters. |
There was a problem hiding this comment.
wait, what? this came out of left field. Why are we suddenly changing requestToken in an MSC which purports to be about submitToken?
There doesn't seem to be any justification for this in the proposal, or discussion of it in general.
What exactly would need to change to bring the current definition into line with the I/S API?
There was a problem hiding this comment.
ohhh by "the endpoint" do you mean "the endpoint referenced by the submit_url response field"? Could you say so if so?
Still, presumably you don't mean "in all ways" -- for example, it doesn't need an IS access token. Would be good to tighten this up a bit.
|
@mscbot concern lack of clarity about changes to requestToken |
mscbot
added
the
unresolved-concerns
Rendered
Impls:
Sydent: matrix-org/sydent#592
Synapse: element-hq/synapse#17625
Element Web/Desktop: matrix-org/matrix-react-sdk#12936
FCP tickyboxes