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.

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

Umbraco allows usernames with spaces but doesnt allow change password for this users #14808

Closed
bielu opened this issue Sep 15, 2023 · 12 comments · Fixed by #14810
Closed

Umbraco allows usernames with spaces but doesnt allow change password for this users #14808

bielu opened this issue Sep 15, 2023 · 12 comments · Fixed by #14810
Labels

Comments

@bielu
Copy link
Contributor

bielu commented Sep 15, 2023

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

10.6.1

Bug summary

So I had little digging, when UsernameIsEmail is set to false, umbraco allows to set username to anything, including namespaces!
so there are 2 different issues around it:

  1. Umbraco is not using UserValidator when saving users, which allows to create user with spaces even when UserOptions.AllowedUserNameCharacters does not allows spaces!
  2. When using AllowedUserNameCharacters in settings for umbraco security it applies only to members, not backoffice users!

Specifics

No response

Steps to reproduce

subbug 1:

  1. set UsernameIsEmail to false
  2. create user with name containing a space in login
  3. try to change password / unlock / disable user
  4. you will get notication about username is incorrect

subbug 2:

  1. set UsernameIsEmail to false
  2. set AllowedUserNameCharacters to include space
  3. create user name containing a space in login
  4. try to change password / unlock / disable user
  5. you will get notication about username is incorrect

Expected result / actual result

User will be validated with same user validator on save.
It will be possible to override allowed characters for users in backoffice.

@bielu bielu added the type/bug label Sep 15, 2023
@github-actions
Copy link

Hi there @bielu!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

  • We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
  • If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
  • We'll replicate the issue to ensure that the problem is as described.
  • We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

@Zeegaan
Copy link
Member

Zeegaan commented Sep 18, 2023

@bielu How are you allowing space in usernames, I have my AllowedUserNameCharacters set like:

        "AllowedUserNameCharacters": "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-._@+\\ "

But that doesn´t work 🤔

@Zeegaan
Copy link
Member

Zeegaan commented Sep 18, 2023

Also, could I ask that you create an entire seperate issue for subbug 1, this way we can track both seperately, and close this when your PR gets merged 😁

@Zeegaan Zeegaan added the state/needs-more-info We don't have enough information to give a good reply label Sep 18, 2023
@bielu
Copy link
Contributor Author

bielu commented Sep 18, 2023

@bielu How are you allowing space in usernames, I have my AllowedUserNameCharacters set like:

        "AllowedUserNameCharacters": "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-._@+\\ "

But that doesn´t work 🤔

i did exactly that and it did work 😕

@Zeegaan
Copy link
Member

Zeegaan commented Sep 18, 2023

I cannot create a User with a space in the Username, but I can change it and then save it 🤔
(I can however still change it to with a space afterwards)
So I'm not sure why you're allowed to 😅
CantCreateUserWithSpaceInName

@Zeegaan
Copy link
Member

Zeegaan commented Sep 18, 2023

So just to conclude: I cannot reproduce sub-bug 1, this should be its own issue (Not sure it is an issue, as it works as intended on my end)
But I can reproduce sub-bug 2 and will take a look at the PR (looks like it fixes the issue), how does that sound for you @bielu 😁

@bielu
Copy link
Contributor Author

bielu commented Sep 18, 2023

Hmmm, I created users by migrating them from v8 🤔 so API allowed to do it but backoffice doesnt, sounds like we just found 3 issues 😢
and yes sub bug 2 is resolved by pr

@Zeegaan
Copy link
Member

Zeegaan commented Sep 18, 2023

Fixed in #14810

@Zeegaan Zeegaan removed the state/needs-more-info We don't have enough information to give a good reply label Sep 18, 2023
@Zeegaan
Copy link
Member

Zeegaan commented Sep 18, 2023

And yes some funky behavior will occur if you migrate users with spaces, but doesn't have them allowed. Will talk to the team on how to go about handling such a case 😁

@bielu
Copy link
Contributor Author

bielu commented Sep 18, 2023

@Zeegaan can you confirm in which v10 patch version it will be release + approx date? :)

@Zeegaan
Copy link
Member

Zeegaan commented Sep 18, 2023

I will cherry pick this for the 10.7.1 release, no approximate release date as of yet though 🙏

@jerpenol
Copy link

@Zeegaan any idea when this will be included in a release? I am still encountering #14823 in 13.2.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants