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.

Sign up for GitHub

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

Jump to bottom

MSC4183: Additional Error Codes for submitToken endpoint #4183

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

MSC4183: Additional Error Codes for submitToken endpoint #4183

Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
e2cc52f
MSC4180: Additional Error Codes for submitToken endpoint
dbkr Aug 28, 2024
daa2132
Bah, markdown
dbkr Aug 28, 2024
f4fdd0f
Line wrapping
dbkr Aug 28, 2024
0cdf8c6
Typos
dbkr Aug 28, 2024
7fb14aa
Right MSC number
dbkr Aug 29, 2024
725f2ec
Rename
dbkr Aug 29, 2024
d383285
Add http status code
dbkr Aug 29, 2024
7809ced
Apply suggestions from code review
dbkr Oct 2, 2024
be435a8
Word wrap
dbkr Oct 2, 2024
007d364
Clearer wording
dbkr Oct 2, 2024
43638f1
clarify
dbkr Oct 23, 2024
5b8b163
Clarify
dbkr Oct 23, 2024
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
62 changes: 62 additions & 0 deletions proposals/4183-submitToken-error-codes.md
View file
Open in desktop
dbkr marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# 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.

## 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.
Copy link
Member

Choose a reason for hiding this comment

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

Would it be worth re-using/redefining M_UNKNOWN_TOKEN from the C-S API?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, yes, probably

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

I think maybe it's clearer if it's a separate code?

(I agree)


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 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.
Comment on lines +34 to +36
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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.


## 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