Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

When using numeric only id, registration fails but after checking captcha, mail, ... #7576

Closed
BillCarsonFr opened this issue May 27, 2020 · 6 comments · Fixed by #7625
Closed
Assignees
Labels
z-p2 (Deprecated Label)

Comments

@BillCarsonFr
Copy link
Member

BillCarsonFr commented May 27, 2020

This error

400, "Numeric user IDs are reserved for guest users."

You get this error very late in the register flow, a bit annoying to know it only after having completed all the register steps

Can it be checked like for 'User ID already taken.'?

image

@babolivier
Copy link
Contributor

I'm wondering if this should be caught by the call to /_matrix/client/r0/register/available. My first thought was no because we want to check that the username is available, not valid. But then Synapse seems to ignore any username provided in a guest registration request, and determines the numeric username of the guest user itself, so it could be correct to interpret that as Synapse reserving numeric usernames and making those unavailable for registration.

@babolivier
Copy link
Contributor

Another solution would be to move the check on whether the username is numeric much sooner in the registration flow, e.g. as soon as we can see a username:

if "username" in body:
if (
not isinstance(body["username"], string_types)
or len(body["username"]) > 512
):
raise SynapseError(400, "Invalid username")
desired_username = body["username"]

@bmarty
Copy link

bmarty commented May 27, 2020

FTR RiotX does not use /_matrix/client/r0/register/available, so yes if the error comes from the registration flow, when we send the username, we (developers of RiotX) will be more than happy! And other clients will also get benefit from this fix

@babolivier
Copy link
Contributor

Personally I'd advocate that clients should always call /available before attempting registration, so that they don't have a hard dependency of one hs implem doing the right thing and they don't provide a poorer UX when using a hs implem that doesn't. Regardless of whether we do this in Synapse there's no guarantee that Dendrite, Conduit, Ruma or any other hs implem will do the same, or will even be able to.

For this reason I think we should do both, having /available do this for clients that support calling this endpoint, and move that check higher in the flow as a fallback if for some reason the client doesn't implement this call.

FTR it looks like simply moving that check to check_username would kill both birds with one stone.

@bmarty
Copy link

bmarty commented May 27, 2020

Ok, I understand. But /available is not reliable, as per comment in this issue. Also when a user is creating an account, once he has sent their desired userId, which is available, /available still returns true for the same userId. So I think the userId should be reserved for the duration of the OAuth session, but this is another bug.

To my point of view, /available could be eventually use by the client app to propose alternatives when the desired userId is in use

And as per https://matrix.org/docs/spec/client_server/latest#get-matrix-client-r0-register-available:

Matrix clients may wish to use this API prior to attempting registration, however the clients must also be aware that using this API does not normally reserve the username. This can mean that the username becomes unavailable between checking its availability and attempting to register it.

Which confirm my point of view that it's better not to use this API during registration :)

@anoadragon453 anoadragon453 added enhancement z-p2 (Deprecated Label) labels May 27, 2020
@babolivier babolivier changed the title When you using numeric only id, registration fails but after checking captcha, mail, ... When using numeric only id, registration fails but after checking captcha, mail, ... May 28, 2020
@babolivier babolivier self-assigned this Jun 3, 2020
@babolivier
Copy link
Contributor

Fixed by #7625

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
z-p2 (Deprecated Label)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants