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 21 commits into
base: main
Choose a base branch
from
Open
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions proposals/4183-submitToken-error-codes.md
dbkr marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# MSC4183: Additional Error Codes for submitToken endpoint

The [`POST
/_matrix/identity/v2/validate/email/submitToken`](https://spec.matrix.org/v1.11/identity-service-api/#post_matrixidentityv2validateemailsubmittoken)
and [`POST
/_matrix/identity/v2/validate/msisdn/submitToken`](https://spec.matrix.org/v1.11/identity-service-api/#post_matrixidentityv2validatemsisdnsubmittoken)
endpoints do not specify any specific error codes, instead relying on the common error codes defined in the identity
service API.

However, these common error codes don't have any codes to signal many errors that can occur in these APIs: most
obviously, that the token the user entered was incorrect.

This MSC can be considered similar to [MSC4178](https://github.com/matrix-org/matrix-spec-proposals/pull/4178) 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. Everything this MSC specifies applies to this endpoint in the same way.

Note that the email `submitToken` endpoint ([`POST
Copy link
Member

Choose a reason for hiding this comment

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

it's a bit confusing that we jump from talking about an endpoint in the C-S API to one in the IS API without mentioning that.

Also, if you're talking specifically about the POST variant, you better say that, rather than saying that the entire endpoint is unused.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes - sort of, I don't think there is a POST variant for email on the C/S API. Hopefully clearer in af1dc86.

Copy link
Member

Choose a reason for hiding this comment

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

This is still confusing: we still jump from talking about an endpoint in the C-S API to one in the IS API without mentioning that.

/_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 and therefore use the
`GET` version. This proposal updates both for consistency.

## Proposal

Add the following specific error code as a code that can be returned by the two endpoints given above:
Copy link
Member

Choose a reason for hiding this comment

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

not least because there are now at least three different endpoints given above, I think you should restate which endpoints are changing 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.

fixed

* `M_TOKEN_INCORRECT`: Indicates that the token that the user entered to validate the session is incorrect.
richvdh marked this conversation as resolved.
Show resolved Hide resolved

Note that we deliberately chose not to re-use `M_UNKNOWN_TOKEN` since that refers to an access token, whereas this
refers to a token that the user enters.

HTTP status code 400 should be used for this error.

Additionally specify that the following common error codes can be returned:
* `M_INVALID_PARAM`: One of the supplied parameters in not valid.
* `M_SESSION_EXPIRED`: The validation session is question has expired.

HTTP status code 400 should also be used for both of these errors.

Also change the `submit_url` field in the response to [`POST
/_matrix/client/v3/account/3pid/email/requestToken`](https://spec.matrix.org/v1.11/client-server-api/#post_matrixclientv3account3pidemailrequesttoken),
to specify that response parameters and error codes are the same as the I/S API version, as well as request parameters.
Copy link
Member

Choose a reason for hiding this comment

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

why does this only apply to POST /_matrix/client/v3/account/3pid/email/requestToken, and not also:

  • POST /_matrix/client/v3/account/password/email/requestToken
  • POST /_matrix/client/v3/account/password/msisdn/requestToken
  • POST /_matrix/client/v3/register/email/requestToken
  • POST /_matrix/client/v3/register/msisdn/requestToken
  • POST /_matrix/client/v3/account/3pid/msisdn/requestToken

?

Copy link
Member Author

Choose a reason for hiding this comment

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

Mmm, yeah - I'd done it as only editing what I needed to, but you're right, they should be consistent. Changed in 0f43a51.


## Potential issues

None foreseen.

## Alternatives

None considered.

## Security considerations

None foreseen.

## Unstable prefix

No unstable prefix is deemed necessary. Sydent already sends the common error codes and also sends
`M_NO_VALID_SESSION` if the code is incorrect. Once an identity server (or homeserver) switches to
use the new error code, clients (including homeservers proxying the IS API) 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 codes with API versions. Since the identity
server is generally used via the homeserver now, most users of this API will not currently receive
a sensible error code in this situation anyway.

## Dependencies

None