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

Downcase userid on registration #2662

Merged
merged 4 commits into from
Nov 10, 2017
Merged

Downcase userid on registration #2662

merged 4 commits into from
Nov 10, 2017

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Nov 9, 2017

Force username to lowercase before attempting to register

#2660

@richvdh
Revert "Allow upper-case characters in mxids"
9b803cc 
This reverts commit b70b646.
@richvdh
Downcase userid on registration
9b599bc 
Force username to lowercase before attempting to register

#2660
@ara4n
Copy link
Member

ara4n commented Nov 9, 2017

Unless we also do this in the login API (first trying with the non-squashed mxid, and then retrying with the squashed one if the non-squashed one didn't exist), we risk further UX problems, where the user thinks they've registered @MrFlibble:matrix.org and then tries to log in as MrFlibble only to be rejected. (This is what we did on Riot/Web and promptly forgot to do on Riot/iOS & Android - in retrospect it should probably have been all done serverside to make life easier for client implementers and avoid precisely the failure mode which screwed us here)

@richvdh
Copy link
Member Author

richvdh commented Nov 10, 2017

We do already do it on login.

@richvdh
Copy link
Member Author

richvdh commented Nov 10, 2017

(Serverside)

@richvdh
Copy link
Member Author

richvdh commented Nov 10, 2017

retest this please

@richvdh richvdh mentioned this pull request Nov 10, 2017
Merged
dbkr
dbkr reviewed Nov 10, 2017
View reviewed changes
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

Discussed this an matrix, but summary is we probably ought to make sure usernames registered via shared secret registration are valid too, and maybe elaborate on why we don't validate appservice userids, otherwise lgtm

@dbkr dbkr assigned richvdh and unassigned dbkr Nov 10, 2017
@richvdh
Copy link
Member Author

richvdh commented Nov 10, 2017

ok, ptal.

@richvdh richvdh removed their assignment Nov 10, 2017
@richvdh
Copy link
Member Author

richvdh commented Nov 10, 2017

retest this please

@richvdh richvdh merged commit 4d0414c into develop Nov 10, 2017
@richvdh richvdh deleted the rav/fix_mxids_again branch November 10, 2017 13:57
@richvdh richvdh mentioned this pull request Nov 10, 2017
Closed
@ara4n ara4n mentioned this pull request Nov 13, 2017
Closed
richvdh added a commit that referenced this pull request Nov 27, 2017
@richvdh
Revert "Allow upper-case characters in mxids"
39b9c83 
This has already been reverted on develop and replaced with a better
implementation (see PR #2662), but because the 'allow upper-case characters'
commit got separately applied to both develop and matrix-org-hotfixes, it
hasn't been correctly reverted on matrix-org-hotfixes.
@richvdh richvdh mentioned this pull request Nov 27, 2017
Merged
@enannos
Copy link

enannos commented Oct 1, 2019

(Serverside)

How?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dbkr dbkr dbkr approved these changes

Assignees

@dbkr dbkr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@richvdh @ara4n @enannos @dbkr