U4-10111 - Changing email on a user doesn't show the username field

Created by Claus Jensen 05 Jul 2017, 12:37:57 Updated by Brian Powell 29 Aug 2017, 18:34:47

Relates to: U4-10368

Subtask of: U4-8632

A user has both an email and a login/username. We set the username == email when creating users and that is all good.

If you however edit a user and update his email address, this will now be different from his username. Because of this, editing a user that has a username != email, will now show edit fields for both the email address and the username/login.

But ... when you initially edit a user with username==email and then change the email so it is different from the username, the UI will however not display the username input field until you refresh/reload the edit user page. It should actually show instantly when clicking save, if the email no longer corresponds to his username. It should be possible as we are actually returning these values on the post request.

Comments

Shannon Deminick 10 Jul 2017, 05:42:21

This is by design, by default the email IS the username and the username field is not ever shown. The only time it will be shown is if the username is already different from the email (legacy). When changing the email address, this should also change the username and the validation should deal with this accordingly too. Perhaps there's a bug in this logic currently but changing the email address shouldn't actually show the username field.


Claus Jensen 10 Jul 2017, 06:14:19

Hmm .. yep then it might be a bug because when I tested changing the email, it certainly didn't change the username (which then resulted in having them not equal and showing both the fields on a browser refresh)


Claus Jensen 10 Jul 2017, 10:28:41

fixed in commit: https://github.com/umbraco/Umbraco-CMS/commit/f2b52585e156b397a95b38f8f2e039a3ba260808

Checking if username == email on the found/existing user and then also updating the username to match if the email has changed on the object being saved (so this is not doing anything if they were already different - only if they were also aligned before saving).

I have also added in some cross-check validations to make sure you can't actually use someone else's email address for your username in case they have a legacy username not matching their email address (meaning they dont actually ''use'' their own email address for login... - we still shouldn't allow another account to use it for login)


Shannon Deminick 11 Jul 2017, 02:23:08

I've added a question to the PR


Stephan 11 Jul 2017, 11:24:54

fresh install, changed email of admin user, get a nullref exception in UserExtensions:177 in IsAdmin?


Stephan 11 Jul 2017, 11:26:21

ah no, it's argnull because 'user' is null


Stephan 11 Jul 2017, 11:27:58

log out, log in again and it works


Stephan 11 Jul 2017, 11:33:14

so... IIC we now decide that username == email, always (unless legacy) and it's not possible to have username != email anymore - fine - but then the UI should be explicit 'cos at the moment it's misleading (ie I'd change my email but still try to use the old username)

now, one cannot change the email of the currently logged-in user - causes the argnull exception I mention above.


Shannon Deminick 18 Jul 2017, 03:14:23

@zpqrtbnk I can't replicate this problem. I do a fresh install, go to the admin user (the only one there), i can change the email without issue

Also it "Is" possible to have username != email. If the username is already different than it will show you this field.

Let's chat today so i can understand what the problem is.


Stephan 18 Jul 2017, 08:45:51

About the username vs email: in new installs, the email ''is'' the username, there is no username really, so it's expected that changing the email changes what you use to log in. Got it, ok.

About the exception: can repro. Log in as admin, go to users section, edit admin user, change admin email (username) and save. Now try to view any user: throws an ArgumentNullException in UserExtensions.IsAdmin, because the current user is now null.


Shannon Deminick 19 Jul 2017, 06:46:33

Ah of course, this is a bit interesting. This would actually probably happen today if you change your username in the back office. The reason is because the current Identity assigned to the thread/request contains the user data ticket which contains the username and it doesn't get updated when you change the username of yourself.

...looking into this now


Shannon Deminick 19 Jul 2017, 07:13:45

Actually, this goes deeper than this. If two users log in and one user changes the other ones username, the other one will get this same problem (there might be other problems too).

We'll have to create a custom auth filter for all controllers to check if the current Identity/Principal assigned (based on the auth ticket) is out of sync and either sync it up or log them out


Shannon Deminick 19 Jul 2017, 09:25:15

This is fixed in rev: 8df00d55253a43c42fc9eb8dbd49f073276894cd

  • Ensures that the IsAdmin looks up the user by ID which is a cached call, looking up by username is NOT
  • Creates new VerifyIfUserTicketDataIsStaleAttribute filter which will check if the current Principal/Identity assigned to the Request/Thread is out of sync with the persisted user data. It will check before and after the request executes and re-syncs the Principal/Identity/Auth ticket with the correct data

TODO:

  • --Need to fix up the BackOfficeIdentityUser Roles--
  • Should probably deal with what happens if a user's culture changes, there's a TODO in the code and we could handle that so that the back office refreshes or something but that will need to be a separate task.


Shannon Deminick 19 Jul 2017, 09:43:16

@zpqrtbnk I've just pushed the fixed and it's all merged. The TODO's above still stand but i'll work on those separately, if you can just test that you no longer get a ysod and have a peek at the code, then you can close this one.


Stephan 27 Jul 2017, 11:34:36

Tested: as admin, changed my email and username, save, try to view any user = works. Also, changing another user does not YSOD and the other user identity is updated. Closing.


Priority: Normal

Type: Bug

State: Fixed

Assignee:

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 7.7.0

Due in version: 7.7.0

Sprint: Sprint 64

Story Points: 1

Cycle: