This repository has been archived by the owner on Apr 12, 2024. It is now read-only.
Prevent M_USER_IN_USE from being raised by registration methods until after email has been verified #48
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@anoadragon453 I had a look on your PR. I'm sorry but I think you took a wrong way. We should not abort the register request because the provided username is already used. I'm available to sync with you in order to expose my point of view |
|
@giomfo OK, let's sync tomorrow |
|
About |
|
@anoadragon453 I don't know if I'm right, but don't you think you should pass Perhaps this change is useless in case of Tchap. I don't know |
anoadragon453
commented
Jun 19, 2020
|
|
||
| self.assertEquals(channel.result["code"], b"400", channel.result) | ||
| self.assertEquals(channel.json_body["error"], "Invalid username") | ||
|
|
There was a problem hiding this comment.
This test is no longer relevant.
|
@giomfo Have updated to:
|
giomfo
reviewed
Jun 19, 2020
| @@ -357,15 +357,9 @@ async def on_GET(self, request): | |||
| 403, "Registration has been disabled", errcode=Codes.FORBIDDEN | |||
| ) | |||
|
|
|||
| ip = self.hs.get_ip_from_request(request) | |||
| with self.ratelimiter.ratelimit(ip) as wait_deferred: | |||
There was a problem hiding this comment.
why do you remove the ratelimiter check here?
There was a problem hiding this comment.
Now that this endpoint just always returns {"available": True} I don't think there's much point in ratelimiting it.
There was a problem hiding this comment.
you're right, I agree
I forgot it was change about /register/available
There was a problem hiding this comment.
Yeah, hard to notice in the diff :)
babolivier
added a commit
to matrix-org/synapse
that referenced
this pull request
Jan 26, 2022
) This is mostly motivated by the tchap use case, where usernames are automatically generated from the user's email address (in a way that allows figuring out the email address from the username). Therefore, it's an issue if we respond to requests on /register and /register/available with M_USER_IN_USE, because it can potentially leak email addresses (which include the user's real name and place of work). This commit adds a flag to inhibit the M_USER_IN_USE errors that are raised both by /register/available, and when providing a username early into the registration process. This error will still be raised if the user completes the registration process but the username conflicts. This is particularly useful when using modules (#11790 adds a module callback to set the username of users at registration) or SSO, since they can ensure the username is unique. More context is available in the PR that introduced this behaviour to synapse-dinsic: matrix-org/synapse-dinsic#48 - as well as the issue in the matrix-dinsic repo: matrix-org/matrix-dinsic#476
babolivier
added a commit
that referenced
this pull request
Jan 26, 2022
…743) This is mostly motivated by the tchap use case, where usernames are automatically generated from the user's email address (in a way that allows figuring out the email address from the username). Therefore, it's an issue if we respond to requests on /register and /register/available with M_USER_IN_USE, because it can potentially leak email addresses (which include the user's real name and place of work). This commit adds a flag to inhibit the M_USER_IN_USE errors that are raised both by /register/available, and when providing a username early into the registration process. This error will still be raised if the user completes the registration process but the username conflicts. This is particularly useful when using modules (matrix-org/synapse#11790 adds a module callback to set the username of users at registration) or SSO, since they can ensure the username is unique. More context is available in the PR that introduced this behaviour to synapse-dinsic: #48 - as well as the issue in the matrix-dinsic repo: matrix-org/matrix-dinsic#476
babolivier
added a commit
to matrix-org/synapse
that referenced
this pull request
Jan 26, 2022
) This is mostly motivated by the tchap use case, where usernames are automatically generated from the user's email address (in a way that allows figuring out the email address from the username). Therefore, it's an issue if we respond to requests on /register and /register/available with M_USER_IN_USE, because it can potentially leak email addresses (which include the user's real name and place of work). This commit adds a flag to inhibit the M_USER_IN_USE errors that are raised both by /register/available, and when providing a username early into the registration process. This error will still be raised if the user completes the registration process but the username conflicts. This is particularly useful when using modules (#11790 adds a module callback to set the username of users at registration) or SSO, since they can ensure the username is unique. More context is available in the PR that introduced this behaviour to synapse-dinsic: matrix-org/synapse-dinsic#48 - as well as the issue in the matrix-dinsic repo: matrix-org/matrix-dinsic#476
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prevents
M_USER_IN_USEfrom being returned to the client before they're verified an associated email.