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

Allowing admin users to change users email #3118

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

DieselTech
Copy link
Collaborator

Changed

Copy link
Member

@majora2007 majora2007 left a comment

Choose a reason for hiding this comment

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

Just did a preliminary review. I don't see any of the email code to trigger a link so the user can confirm the email change.

// Validate email change
var emailValidationErrors = await _accountService.ValidateEmail(dto.Email);
if (emailValidationErrors.Any()) return BadRequest(await _localizationService.Translate(User.GetUserId(), "email-taken"));
user.Email = dto.Email;
Copy link
Member

Choose a reason for hiding this comment

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

You need a new line after the if statement to help the code be easier to digest.

user.Email = dto.Email;
user.EmailConfirmed = false;
user.NormalizedEmail = _userManager.NormalizeEmail(dto.Email);
user.ConfirmationToken = await _userManager.GenerateEmailConfirmationTokenAsync(user);
Copy link
Member

Choose a reason for hiding this comment

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

You generate a confirmation token, but I don't see you triggering an email for the end user to confirm and get rid of that pending status.

@majora2007
Copy link
Member

Not sure if you still want to work on this to get it in?

Added new line as requested
Removed the confirmation token since if the admin is changing the users email, it should be a force change.
Set the `EmailConfirmed` status to `True`, again since the admin is the one pushing it through.
@DieselTech
Copy link
Collaborator Author

Changes made. Got rid of the email confirmation step because it is an admin making the change. That should force change it and not rely on the confirmation flow. Not sure what I was thinking before.

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

Successfully merging this pull request may close these issues.

Admin can't change users email address
2 participants