Skip to content

Commit

Permalink
Increate Logging in External Login and return correct result when fai…
Browse files Browse the repository at this point in the history 
…l to create user
  • Loading branch information
@MikeAlhayek
MikeAlhayek committed Nov 25, 2024
1 parent 1cc5fdd commit 05ff0e1
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 76 deletions.
111 changes: 89 additions & 22 deletions src/OrchardCore.Modules/OrchardCore.Users/Controllers/ExternalAuthenticationsController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using OrchardCore.DisplayManagement.Notify;
using OrchardCore.Email;
using OrchardCore.Environment.Shell;
using OrchardCore.Modules;
using OrchardCore.Mvc.Core.Utilities;
Expand All @@ -32,6 +33,7 @@ public sealed class ExternalAuthenticationsController : AccountBaseController
private readonly ISiteService _siteService;
private readonly IEnumerable<ILoginFormEvent> _accountEvents;
private readonly IShellFeaturesManager _shellFeaturesManager;
private readonly IEmailAddressValidator _emailAddressValidator;
private readonly IUserService _userService;
private readonly INotifier _notifier;
private readonly IEnumerable<IExternalLoginEventHandler> _externalLoginHandlers;
Expand All @@ -52,6 +54,7 @@ public ExternalAuthenticationsController(
IStringLocalizer<ExternalAuthenticationsController> stringLocalizer,
IEnumerable<ILoginFormEvent> accountEvents,
IShellFeaturesManager shellFeaturesManager,
IEmailAddressValidator emailAddressValidator,
IUserService userService,
INotifier notifier,
IEnumerable<IExternalLoginEventHandler> externalLoginHandlers,
Expand All @@ -66,6 +69,7 @@ public ExternalAuthenticationsController(
_siteService = siteService;
_accountEvents = accountEvents;
_shellFeaturesManager = shellFeaturesManager;
_emailAddressValidator = emailAddressValidator;
_userService = userService;
_notifier = notifier;
_externalLoginHandlers = externalLoginHandlers;
Expand Down Expand Up @@ -215,14 +219,16 @@ public async Task<IActionResult> ExternalLoginCallback(string returnUrl = null,
}, ModelState.AddModelError);

// If the registration was successful we can link the external provider and redirect the user.
if (iUser == null)
if (iUser == null || !ModelState.IsValid)
{
_logger.LogError("Unable to create internal account and link it to the external user.");

await _notifier.ErrorAsync(H["Unable to create internal account and link it to the external user."]);

return RedirectToLogin(returnUrl);
return View(nameof(RegisterExternalLogin), externalLoginViewModel);
}

var identityResult = await _signInManager.UserManager.AddLoginAsync(iUser, new UserLoginInfo(info.LoginProvider, info.ProviderKey, info.ProviderDisplayName));
var identityResult = await _userManager.AddLoginAsync(iUser, new UserLoginInfo(info.LoginProvider, info.ProviderKey, info.ProviderDisplayName));

if (identityResult.Succeeded)
{
Expand Down Expand Up @@ -258,6 +264,8 @@ public async Task<IActionResult> ExternalLoginCallback(string returnUrl = null,
return RedirectToLogin(returnUrl);
}

_logger.LogError("Unable to add external provider to a user: {LoginProvider} provider.", info.LoginProvider);

AddErrorsToModelState(identityResult.Errors);
}

Expand Down Expand Up @@ -294,25 +302,11 @@ public async Task<IActionResult> RegisterExternalLogin(RegisterExternalLoginView
model.NoEmail = settings.NoEmail;
model.NoUsername = settings.NoUsername;

ModelState.Clear();
UpdateAndValidateEmail(model, info, settings.NoEmail);
await UpdateAndValidateUserNameAsync(model, info, settings.NoUsername);
await UpdateAndValidatePasswordAsync(model, settings.NoPassword);

if (settings.NoEmail && string.IsNullOrWhiteSpace(model.Email))
{
model.Email = info.GetEmail();
}

if (settings.NoUsername && string.IsNullOrWhiteSpace(model.UserName))
{
model.UserName = await GenerateUsernameAsync(info);
}

if (settings.NoPassword)
{
model.Password = null;
model.ConfirmPassword = null;
}

if (TryValidateModel(model) && ModelState.IsValid)
if (ModelState.IsValid)
{
var iUser = await _userService.RegisterAsync(
new RegisterUserForm()
Expand All @@ -322,7 +316,7 @@ public async Task<IActionResult> RegisterExternalLogin(RegisterExternalLoginView
Password = model.Password,
}, ModelState.AddModelError);

if (iUser is not null)
if (iUser is not null && ModelState.IsValid)
{
var identityResult = await _signInManager.UserManager.AddLoginAsync(iUser, new UserLoginInfo(info.LoginProvider, info.ProviderKey, info.ProviderDisplayName));

Expand Down Expand Up @@ -455,6 +449,7 @@ public async Task<IActionResult> LinkLogin(string provider)
public async Task<IActionResult> LinkLoginCallback()
{
var user = await _userManager.GetUserAsync(User);

if (user == null)
{
_logger.LogError("Unable to load user with ID '{UserId}'.", _userManager.GetUserId(User));
Expand All @@ -463,6 +458,7 @@ public async Task<IActionResult> LinkLoginCallback()
}

var info = await _signInManager.GetExternalLoginInfoAsync();

if (info == null)
{
_logger.LogError("Unexpected error occurred loading external login info for user '{UserName}'.", user.UserName);
Expand All @@ -471,6 +467,7 @@ public async Task<IActionResult> LinkLoginCallback()
}

var result = await _userManager.AddLoginAsync(user, new UserLoginInfo(info.LoginProvider, info.ProviderKey, info.ProviderDisplayName));

if (!result.Succeeded)
{
_logger.LogError("Unexpected error occurred adding external login info for user '{UserName}'.", user.UserName);
Expand All @@ -480,6 +477,7 @@ public async Task<IActionResult> LinkLoginCallback()

// Clear the existing external cookie to ensure a clean login process.
await HttpContext.SignOutAsync(IdentityConstants.ExternalScheme);

// Perform External Login SignIn.
await ExternalSignInAsync(user, info);

Expand All @@ -491,6 +489,7 @@ public async Task<IActionResult> LinkLoginCallback()
public async Task<IActionResult> RemoveLogin(RemoveLoginViewModel model)
{
var user = await _userManager.GetUserAsync(User);

if (user == null || user is not User u)
{
_logger.LogError("Unable to load user with ID '{UserId}'.", _userManager.GetUserId(User));
Expand All @@ -499,6 +498,7 @@ public async Task<IActionResult> RemoveLogin(RemoveLoginViewModel model)
}

var result = await _userManager.RemoveLoginAsync(user, model.LoginProvider, model.ProviderKey);

if (!result.Succeeded)
{
_logger.LogError("Unexpected error occurred removing external login info for user '{UserName}'.", user.UserName);
Expand All @@ -520,6 +520,73 @@ public async Task<IActionResult> RemoveLogin(RemoveLoginViewModel model)
return RedirectToAction(nameof(ExternalLogins));
}

private async Task UpdateAndValidatePasswordAsync(RegisterExternalLoginViewModel model, bool noPassword)
{
if (noPassword)
{
model.Password = null;
model.ConfirmPassword = null;

return;
}

if (string.IsNullOrWhiteSpace(model.Password))
{
ModelState.AddModelError(nameof(model.Password), S["Password is required!"]);
}
else if (model.Password != model.ConfirmPassword)
{
ModelState.AddModelError(nameof(model.ConfirmPassword), S["Confirm Password do not match"]);
}
else if (_userManager.PasswordValidators != null)
{
var user = new User();

foreach (var passwordValidator in _userManager.PasswordValidators)
{
var validationResult = await passwordValidator.ValidateAsync(_userManager, user, model.Password);

if (!validationResult.Succeeded)
{
foreach (var error in validationResult.Errors)
{
ModelState.AddModelError(nameof(model.Password), error.Description);
}
}
}
}
}

private async Task UpdateAndValidateUserNameAsync(RegisterExternalLoginViewModel model, ExternalLoginInfo info, bool noUsername)
{
if (noUsername && string.IsNullOrWhiteSpace(model.UserName))
{
model.UserName = await GenerateUsernameAsync(info);
}

if (string.IsNullOrWhiteSpace(model.UserName))
{
ModelState.AddModelError(nameof(model.UserName), S["Username is required!"]);
}
}

private void UpdateAndValidateEmail(RegisterExternalLoginViewModel model, ExternalLoginInfo info, bool noEmail)
{
if (noEmail && string.IsNullOrWhiteSpace(model.Email))
{
model.Email = info.GetEmail();
}

if (string.IsNullOrWhiteSpace(model.Email))
{
ModelState.AddModelError(nameof(model.Email), S["Email is required!"]);
}
else if (!_emailAddressValidator.Validate(model.Email))
{
ModelState.AddModelError(nameof(model.Email), S["Invalid Email."]);
}
}

private async Task<Microsoft.AspNetCore.Identity.SignInResult> ExternalSignInAsync(IUser user, ExternalLoginInfo info)
{
_logger.LogInformation("Attempting to do an external sign in.");
Expand Down
2 changes: 1 addition & 1 deletion src/OrchardCore.Modules/OrchardCore.Users/Handlers/UserModerationRegistrationFormEvents.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public override Task RegisteringAsync(UserRegisteringContext context)
if (context.User is User user &&
!(_registrationOptions.UsersAreModerated && !user.IsEnabled))
{
context.Cancel = true;
context.CancelSignIn = true;
}

return Task.CompletedTask;
Expand Down
47 changes: 5 additions & 42 deletions src/OrchardCore.Modules/OrchardCore.Users/ViewModels/RegisterExternalLoginViewModel.cs
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
using System.ComponentModel.DataAnnotations;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Localization;
using OrchardCore.Email;
using Microsoft.AspNetCore.Mvc.ModelBinding;

namespace OrchardCore.Users.ViewModels;

public class RegisterExternalLoginViewModel : IValidatableObject
public class RegisterExternalLoginViewModel
{
[BindNever]
public bool NoUsername { get; set; }

[BindNever]
public bool NoEmail { get; set; }

[BindNever]
public bool NoPassword { get; set; }

public string UserName { get; set; }
Expand All @@ -22,42 +23,4 @@ public class RegisterExternalLoginViewModel : IValidatableObject

[DataType(DataType.Password)]
public string ConfirmPassword { get; set; }

public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
{
var emailAddressValidator = validationContext.GetService<IEmailAddressValidator>();
var S = validationContext.GetService<IStringLocalizer<RegisterExternalLoginViewModel>>();

if (string.IsNullOrWhiteSpace(Email))
{
if (!NoEmail)
{
yield return new ValidationResult(S["Email is required!"], ["Email"]);
}
}
else if (!emailAddressValidator.Validate(Email))
{
yield return new ValidationResult(S["Invalid Email."], ["Email"]);
}

if (string.IsNullOrWhiteSpace(UserName) && !NoUsername)
{
yield return new ValidationResult(S["Username is required!"], ["UserName"]);
}

if (string.IsNullOrWhiteSpace(Password) && !NoPassword)
{
yield return new ValidationResult(S["Password is required!"], ["Password"]);
}

if (Password != ConfirmPassword)
{
yield return new ValidationResult(S["Confirm Password do not match"], ["ConfirmPassword"]);
}

if (Password != null && (Password.Length < 6 || Password.Length > 100))
{
yield return new ValidationResult(string.Format(S["Password must be between {0} and {1} characters"], 6, 100), ["Password"]);
}
}
}
4 changes: 2 additions & 2 deletions src/OrchardCore/OrchardCore.Users.Abstractions/Events/UserRegisteringContext.cs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
namespace OrchardCore.Users.Events;
namespace OrchardCore.Users.Events;

public class UserRegisteringContext
{
public bool Cancel { get; set; }
public bool CancelSignIn { get; set; }

public IUser User { get; }

Expand Down
39 changes: 30 additions & 9 deletions src/OrchardCore/OrchardCore.Users.Core/Services/UserService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,16 +117,38 @@ public async Task<IUser> CreateUserAsync(IUser user, string password, Action<str
throw new ArgumentException("Expected a User instance.", nameof(user));
}

var hasPassword = !string.IsNullOrWhiteSpace(password);

// Accounts can be created with no password.
var identityResult = string.IsNullOrWhiteSpace(password)
? await _userManager.CreateAsync(user)
: await _userManager.CreateAsync(user, password);
var identityResult = hasPassword
? await _userManager.CreateAsync(user, password)
: await _userManager.CreateAsync(user);

if (!identityResult.Succeeded)
{
if (hasPassword)
{
_logger.LogInformation(3, "Unable to create a new account with password.");
}
else
{
_logger.LogInformation(3, "Unable to create a new account with no password.");
}

ProcessValidationErrors(identityResult.Errors, newUser, reportError);

return null;
}

if (hasPassword)
{
_logger.LogInformation(3, "User created a new account with password.");
}
else
{
_logger.LogInformation(3, "User created a new account with no password.");
}

return user;
}

Expand Down Expand Up @@ -169,14 +191,14 @@ public async Task<IUser> GetForgotPasswordUserAsync(string userId)
{
if (string.IsNullOrWhiteSpace(userId))
{
return await Task.FromResult<IUser>(null);
return null;
}

var user = await GetUserAsync(userId);

if (user == null)
{
return await Task.FromResult<IUser>(null);
return null;
}

if (user is User u)
Expand Down Expand Up @@ -328,23 +350,22 @@ public async Task<IUser> RegisterAsync(RegisterUserForm model, Action<string, st
Email = model.Email,
EmailConfirmed = !_registrationOptions.UsersMustValidateEmail,
IsEnabled = !_registrationOptions.UsersAreModerated,
}, model.Password, reportError) as User;
}, model.Password, reportError);

if (user == null)
{
return null;
}

var context = new UserRegisteringContext(user);

await _registrationFormEvents.InvokeAsync((e, ctx) => e.RegisteringAsync(ctx), context, _logger);

if (!context.Cancel)
if (!context.CancelSignIn)
{
await _signInManager.SignInAsync(user, isPersistent: false);
}

_logger.LogInformation(3, "User created a new account with password.");

await _registrationFormEvents.InvokeAsync((e, user) => e.RegisteredAsync(user), user, _logger);

return user;
Expand Down

0 comments on commit 05ff0e1

Please sign in to comment.