make usernames case insensitive #3575 #5783
Conversation
There was a problem hiding this comment.
We need to give someone who tries to make a new account with a taken identifier (case-insensitive) an indication/ error message that it's taken (Right now it fails with no feedback). Maybe update the named query countOfIdentifier on AuthenticatedUser to compare on lowercase. We are saving the identifier as whatever the user enters on account creation, but maybe we should allow the user to log in with another case version of his/her identifier.
Ah, like you did in 95a8086. Thanks! I'll take a look. |
Here's the stack trace as of 0a0dc25: stacktrace-0a0dc25.txt |
|
When I said no feedback I meant that if you try to create an account that has an identifier that is already taken, but with a different case from the existing account, the save fails at the database level and the user doesn't get an error message. It just fails. and they can hit Save again and wonder why nothing is happening. If you add "LOWER(" to the named query I mentioned above then the user sees an error message of "This username is already taken". |
|
@sekmiller yes. I get you. Thanks! I just pushed 392d26e. Can you please take a look and move it QA if you feel like we're ready? Thanks! |
|
The test for existing usernames on account create looks good. Also, if we're saying that username is case insensitive we should probably allow you to log in whatever case you like, so if my username is sekmiller I should also be able to log in as SekMiller or SEKMILLER, etc. |
|
@sekmiller you're saying I should be able to log in with DATAVERSEADMIN. Obviously, this only applies to built-in accounts, not Shib or OAuth. @djbrooke thoughts? |
|
Not Danny, but yes, we should allow this. If we're saying usernames are case insensitive then we should treat them that way functionally. (it's ok to preserve a user's chosen format for presentation). We do the same for dataverse alias, e.g. both dataverse.harvard.edu/dataverse/PSI and dataverse.harvard.edu/dataverse/psi will get you to PIS's dataverse. |
|
@pdurbin thanks for tagging me. I think it's fine if people log in with DaTaVeRsEADMIN vs dataverseadmin vs. whatever other capitalization. Regarding the title of this PR - is there any internationalization/localization concern forcing lower case? |
|
@pdurbin @sekmiller one more note - we should basically find all places in the code that look for username and make sure they're case insensitive - create user and user login are the two obvious ones, but another one that comes to mind is when you are assigning roles and looking for the user. (a good approach here would be to look for all the queries that involve username) |
pdurbin
changed the title
|
I suppose a corollary of all this is that I should be able to use the changeIdentifier API to rename dataverseAdmin to DATAVERSEADMIN. Same case but stored differently in the database. I'm beginning to feel like this issue needs more definition of done. |
|
@pdurbin sounds good! Happy to discuss tomorrow. |
|
Sure. I think this is lower priority, but at some point someone may want to change it and it should be an easy add. (ChangeUserIdentifierCommand first checks if the newusername already exists, so we could add a 2nd check there to see that if it does, it still allows the change if it's just a case variant of the old username). Should be a one line change. |
allow changeIdentifier to be called on same user (in order to change case of username)
|
I went ahead and made the change, though used slightly different logic: |
Should be all set now.
|
@pdurbin Please update from /develop -version was changed. Thanks! |
connects to #3575