Skip to content

Commit

Permalink
Fixing that a captcha is required on external login and registration,…
Browse files Browse the repository at this point in the history 
… renaming `IExternalLoginEventHandler` to `IExternalLoginUserHandler` (Lombiq Technologies: GOV-44) (OrchardCMS#17490)

---------

Co-authored-by: Mike Alhayek <mike@crestapps.com>
  • Loading branch information
@Piedone @MikeAlhayek
Piedone and MikeAlhayek authored Feb 17, 2025
1 parent 6b6e0d4 commit 55c5809
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 62 deletions.
21 changes: 18 additions & 3 deletions src/OrchardCore.Modules/OrchardCore.ReCaptcha/Users/Handlers/LoginFormEventEventHandler.cs
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,32 @@
using Microsoft.AspNetCore.Identity;
using OrchardCore.ReCaptcha.Services;
using OrchardCore.Users;
using OrchardCore.Users.Events;

namespace OrchardCore.ReCaptcha.Users.Handlers;

public sealed class LoginFormEventEventHandler : LoginFormEventBase
{
private readonly ReCaptchaService _reCaptchaService;
private readonly SignInManager<IUser> _signInManager;

public LoginFormEventEventHandler(ReCaptchaService reCaptchaService)
public LoginFormEventEventHandler(
ReCaptchaService reCaptchaService,
SignInManager<IUser> signInManager)
{
_reCaptchaService = reCaptchaService;
_signInManager = signInManager;
}

public override Task LoggingInAsync(string userName, Action<string, string> reportError)
=> _reCaptchaService.ValidateCaptchaAsync(reportError);
public override async Task LoggingInAsync(string userName, Action<string, string> reportError)
{
// When logging in via an external provider, authentication security is already handled by the provider.
// Therefore, using a CAPTCHA is unnecessary and impractical, as users wouldn't be able to complete it anyway.
if (await _signInManager.GetExternalLoginInfoAsync() != null)
{
return;
}

await _reCaptchaService.ValidateCaptchaAsync(reportError);
}
}
21 changes: 18 additions & 3 deletions src/OrchardCore.Modules/OrchardCore.ReCaptcha/Users/Handlers/RegistrationFormEventHandler.cs
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,32 @@
using Microsoft.AspNetCore.Identity;
using OrchardCore.ReCaptcha.Services;
using OrchardCore.Users;
using OrchardCore.Users.Events;

namespace OrchardCore.ReCaptcha.Users.Handlers;

public sealed class RegistrationFormEventHandler : RegistrationFormEventsBase
{
private readonly ReCaptchaService _reCaptchaService;
private readonly SignInManager<IUser> _signInManager;

public RegistrationFormEventHandler(ReCaptchaService reCaptchaService)
public RegistrationFormEventHandler(
ReCaptchaService reCaptchaService,
SignInManager<IUser> signInManager)
{
_reCaptchaService = reCaptchaService;
_signInManager = signInManager;
}

public override Task RegistrationValidationAsync(Action<string, string> reportError)
=> _reCaptchaService.ValidateCaptchaAsync(reportError);
public override async Task RegistrationValidationAsync(Action<string, string> reportError)
{
// When logging in via an external provider, authentication security is already handled by the provider.
// Therefore, using a CAPTCHA is unnecessary and impractical, as users wouldn't be able to complete it anyway.
if (await _signInManager.GetExternalLoginInfoAsync() != null)
{
return;
}

await _reCaptchaService.ValidateCaptchaAsync(reportError);
}
}
24 changes: 12 additions & 12 deletions src/OrchardCore.Modules/OrchardCore.Users/Controllers/AccountController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public sealed class AccountController : AccountBaseController
private readonly UserManager<IUser> _userManager;
private readonly ILogger _logger;
private readonly ISiteService _siteService;
private readonly IEnumerable<ILoginFormEvent> _accountEvents;
private readonly IEnumerable<ILoginFormEvent> _loginFormEvents;
private readonly RegistrationOptions _registrationOptions;
private readonly IDisplayManager<LoginForm> _loginFormDisplayManager;
private readonly IUpdateModelAccessor _updateModelAccessor;
Expand All @@ -44,7 +44,7 @@ public AccountController(
ISiteService siteService,
IHtmlLocalizer<AccountController> htmlLocalizer,
IStringLocalizer<AccountController> stringLocalizer,
IEnumerable<ILoginFormEvent> accountEvents,
IEnumerable<ILoginFormEvent> loginFormEvents,
IOptions<RegistrationOptions> registrationOptions,
INotifier notifier,
IDisplayManager<LoginForm> loginFormDisplayManager,
Expand All @@ -55,7 +55,7 @@ public AccountController(
_userService = userService;
_logger = logger;
_siteService = siteService;
_accountEvents = accountEvents;
_loginFormEvents = loginFormEvents;
_registrationOptions = registrationOptions.Value;
_notifier = notifier;
_loginFormDisplayManager = loginFormDisplayManager;
Expand All @@ -76,9 +76,9 @@ public async Task<IActionResult> Login(string returnUrl = null)
// Clear the existing external cookie to ensure a clean login process.
await HttpContext.SignOutAsync(IdentityConstants.ExternalScheme);

foreach (var accountEvent in _accountEvents)
foreach (var loginFormEvent in _loginFormEvents)
{
var result = await accountEvent.LoggingInAsync();
var result = await loginFormEvent.LoggingInAsync();

if (result != null)
{
Expand Down Expand Up @@ -116,7 +116,7 @@ public async Task<IActionResult> LoginPOST(string returnUrl = null)

var formShape = await _loginFormDisplayManager.UpdateEditorAsync(model, _updateModelAccessor.ModelUpdater, false);

await _accountEvents.InvokeAsync((e, model, modelState) => e.LoggingInAsync(model.UserName, (key, message) => modelState.AddModelError(key, message)), model, ModelState, _logger);
await _loginFormEvents.InvokeAsync((e, model, modelState) => e.LoggingInAsync(model.UserName, (key, message) => modelState.AddModelError(key, message)), model, ModelState, _logger);

IUser user = null;

Expand All @@ -130,9 +130,9 @@ public async Task<IActionResult> LoginPOST(string returnUrl = null)

if (result.Succeeded)
{
foreach (var accountEvent in _accountEvents)
foreach (var loginFormEvent in _loginFormEvents)
{
var loginResult = await accountEvent.ValidatingLoginAsync(user);
var loginResult = await loginFormEvent.ValidatingLoginAsync(user);

if (loginResult != null)
{
Expand All @@ -146,7 +146,7 @@ public async Task<IActionResult> LoginPOST(string returnUrl = null)
{
_logger.LogInformation(1, "User logged in.");

await _accountEvents.InvokeAsync((e, user) => e.LoggedInAsync(user), user, _logger);
await _loginFormEvents.InvokeAsync((e, user) => e.LoggedInAsync(user), user, _logger);

return await LoggedInActionResultAsync(user, returnUrl);
}
Expand All @@ -168,7 +168,7 @@ public async Task<IActionResult> LoginPOST(string returnUrl = null)
if (result.IsLockedOut)
{
ModelState.AddModelError(string.Empty, S["The account is locked out"]);
await _accountEvents.InvokeAsync((e, user) => e.IsLockedOutAsync(user), user, _logger);
await _loginFormEvents.InvokeAsync((e, user) => e.IsLockedOutAsync(user), user, _logger);

return View();
}
Expand All @@ -180,12 +180,12 @@ public async Task<IActionResult> LoginPOST(string returnUrl = null)
if (user == null)
{
// Login failed unknown user.
await _accountEvents.InvokeAsync((e, model) => e.LoggingInFailedAsync(model.UserName), model, _logger);
await _loginFormEvents.InvokeAsync((e, model) => e.LoggingInFailedAsync(model.UserName), model, _logger);
}
else
{
// Login failed with a known user.
await _accountEvents.InvokeAsync((e, user) => e.LoggingInFailedAsync(user), user, _logger);
await _loginFormEvents.InvokeAsync((e, user) => e.LoggingInFailedAsync(user), user, _logger);
}

// If we got this far, something failed, redisplay form.
Expand Down
58 changes: 27 additions & 31 deletions src/OrchardCore.Modules/OrchardCore.Users/Controllers/ExternalAuthenticationsController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using Microsoft.AspNetCore.Identity;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Localization;
using Microsoft.Extensions.Caching.Distributed;
using Microsoft.Extensions.Localization;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
Expand All @@ -30,14 +29,13 @@ public sealed class ExternalAuthenticationsController : AccountBaseController
private readonly UserManager<IUser> _userManager;
private readonly ILogger _logger;
private readonly IDataProtectionProvider _dataProtectionProvider;
private readonly IDistributedCache _distributedCache;
private readonly ISiteService _siteService;
private readonly IEnumerable<ILoginFormEvent> _accountEvents;
private readonly IEnumerable<ILoginFormEvent> _loginFormEvents;
private readonly IShellFeaturesManager _shellFeaturesManager;
private readonly IEmailAddressValidator _emailAddressValidator;
private readonly IUserService _userService;
private readonly INotifier _notifier;
private readonly IEnumerable<IExternalLoginEventHandler> _externalLoginHandlers;
private readonly IEnumerable<IExternalLoginEventHandler> _externalLoginEventHandlers;
private readonly ExternalLoginOptions _externalLoginOption;
private readonly IdentityOptions _identityOptions;

Expand All @@ -49,31 +47,29 @@ public ExternalAuthenticationsController(
UserManager<IUser> userManager,
ILogger<ExternalAuthenticationsController> logger,
IDataProtectionProvider dataProtectionProvider,
IDistributedCache distributedCache,
ISiteService siteService,
IHtmlLocalizer<ExternalAuthenticationsController> htmlLocalizer,
IStringLocalizer<ExternalAuthenticationsController> stringLocalizer,
IEnumerable<ILoginFormEvent> accountEvents,
IEnumerable<ILoginFormEvent> loginFormEvents,
IShellFeaturesManager shellFeaturesManager,
IEmailAddressValidator emailAddressValidator,
IUserService userService,
INotifier notifier,
IEnumerable<IExternalLoginEventHandler> externalLoginHandlers,
IEnumerable<IExternalLoginEventHandler> externalLoginEventHandlers,
IOptions<ExternalLoginOptions> externalLoginOption,
IOptions<IdentityOptions> identityOptions)
{
_signInManager = signInManager;
_userManager = userManager;
_logger = logger;
_dataProtectionProvider = dataProtectionProvider;
_distributedCache = distributedCache;
_siteService = siteService;
_accountEvents = accountEvents;
_loginFormEvents = loginFormEvents;
_shellFeaturesManager = shellFeaturesManager;
_emailAddressValidator = emailAddressValidator;
_userService = userService;
_notifier = notifier;
_externalLoginHandlers = externalLoginHandlers;
_externalLoginEventHandlers = externalLoginEventHandlers;
_externalLoginOption = externalLoginOption.Value;
_identityOptions = identityOptions.Value;

Expand Down Expand Up @@ -124,13 +120,13 @@ public async Task<IActionResult> ExternalLoginCallback(string returnUrl = null,
{
_logger.LogInformation("Found user using external provider and provider key.");

await _accountEvents.InvokeAsync((e, user, modelState) => e.LoggingInAsync(user.UserName, (key, message) => modelState.AddModelError(key, message)), iUser, ModelState, _logger);
await _loginFormEvents.InvokeAsync((e, user, modelState) => e.LoggingInAsync(user.UserName, (key, message) => modelState.AddModelError(key, message)), iUser, ModelState, _logger);

if (ModelState.IsValid)
{
foreach (var accountEvent in _accountEvents)
foreach (var loginFormEvent in _loginFormEvents)
{
var loginResult = await accountEvent.ValidatingLoginAsync(iUser);
var loginResult = await loginFormEvent.ValidatingLoginAsync(iUser);

if (loginResult != null)
{
Expand All @@ -142,7 +138,7 @@ public async Task<IActionResult> ExternalLoginCallback(string returnUrl = null,

if (signInResult.Succeeded)
{
await _accountEvents.InvokeAsync((e, user) => e.LoggedInAsync(user), iUser, _logger);
await _loginFormEvents.InvokeAsync((e, user) => e.LoggedInAsync(user), iUser, _logger);

return await LoggedInActionResultAsync(iUser, returnUrl, info);
}
Expand All @@ -167,9 +163,9 @@ public async Task<IActionResult> ExternalLoginCallback(string returnUrl = null,
{
_logger.LogInformation("Found external user using email. Attempt to link them to existing user.");

foreach (var accountEvent in _accountEvents)
foreach (var loginFormEvent in _loginFormEvents)
{
var loginResult = await accountEvent.ValidatingLoginAsync(iUser);
var loginResult = await loginFormEvent.ValidatingLoginAsync(iUser);

if (loginResult != null)
{
Expand Down Expand Up @@ -238,9 +234,9 @@ public async Task<IActionResult> ExternalLoginCallback(string returnUrl = null,
// The login info must be linked before we consider a redirect, or the login info is lost.
if (iUser is User user)
{
foreach (var accountEvent in _accountEvents)
foreach (var loginFormEvent in _loginFormEvents)
{
var loginResult = await accountEvent.ValidatingLoginAsync(user);
var loginResult = await loginFormEvent.ValidatingLoginAsync(user);

if (loginResult != null)
{
Expand All @@ -255,7 +251,7 @@ public async Task<IActionResult> ExternalLoginCallback(string returnUrl = null,

if (signInResult.Succeeded)
{
await _accountEvents.InvokeAsync((e, user) => e.LoggedInAsync(user), iUser, _logger);
await _loginFormEvents.InvokeAsync((e, user) => e.LoggedInAsync(user), iUser, _logger);

return await LoggedInActionResultAsync(iUser, returnUrl, info);
}
Expand Down Expand Up @@ -325,9 +321,9 @@ public async Task<IActionResult> RegisterExternalLogin(RegisterExternalLoginView
{
_logger.LogInformation(3, "User account linked to {LoginProvider} provider.", info.LoginProvider);

foreach (var accountEvent in _accountEvents)
foreach (var loginFormEvent in _loginFormEvents)
{
var loginResult = await accountEvent.ValidatingLoginAsync(iUser);
var loginResult = await loginFormEvent.ValidatingLoginAsync(iUser);

if (loginResult != null)
{
Expand Down Expand Up @@ -378,7 +374,7 @@ public async Task<IActionResult> LinkExternalLogin(LinkExternalLoginViewModel mo

if (ModelState.IsValid)
{
await _accountEvents.InvokeAsync((e, model, modelState) => e.LoggingInAsync(user.UserName, (key, message) => modelState.AddModelError(key, message)), model, ModelState, _logger);
await _loginFormEvents.InvokeAsync((e, model, modelState) => e.LoggingInAsync(user.UserName, (key, message) => modelState.AddModelError(key, message)), model, ModelState, _logger);

var signInResult = await _signInManager.CheckPasswordSignInAsync(user, model.Password, false);

Expand Down Expand Up @@ -604,15 +600,15 @@ private void UpdateAndValidateEmail(RegisterExternalLoginViewModel model, Extern
UserRoles = userRoles,
};

foreach (var externalLoginHandlers in _externalLoginHandlers)
foreach (var externalLoginEventHandler in _externalLoginEventHandlers)
{
try
{
await externalLoginHandlers.UpdateUserAsync(context);
await externalLoginEventHandler.UpdateUserAsync(context);
}
catch (Exception ex)
{
_logger.LogError(ex, "The method {ExternalLoginHandler}.UpdateUserAsync(context) threw an exception", externalLoginHandlers.GetType());
_logger.LogError(ex, "The method {ExternalLoginHandler}.UpdateUserAsync(context) threw an exception", externalLoginEventHandler.GetType());
}
}

Expand All @@ -625,7 +621,7 @@ private void UpdateAndValidateEmail(RegisterExternalLoginViewModel model, Extern

if (result.Succeeded)
{
await _accountEvents.InvokeAsync((e, user) => e.LoggedInAsync(user), user, _logger);
await _loginFormEvents.InvokeAsync((e, user) => e.LoggedInAsync(user), user, _logger);

var identityResult = await _signInManager.UpdateExternalAuthenticationTokensAsync(info);

Expand All @@ -638,7 +634,7 @@ private void UpdateAndValidateEmail(RegisterExternalLoginViewModel model, Extern
}
else
{
await _accountEvents.InvokeAsync((e, user) => e.LoggingInFailedAsync(user), user, _logger);
await _loginFormEvents.InvokeAsync((e, user) => e.LoggingInFailedAsync(user), user, _logger);
}

return result;
Expand Down Expand Up @@ -686,24 +682,24 @@ private async Task<string> GenerateUsernameAsync(ExternalLoginInfo info)
var externalClaims = info?.Principal.GetSerializableClaims();
var userNames = new Dictionary<Type, string>();

foreach (var item in _externalLoginHandlers)
foreach (var externalLoginEventHandler in _externalLoginEventHandlers)
{
try
{
var userName = await item.GenerateUserName(info.LoginProvider, externalClaims.ToArray());
var userName = await externalLoginEventHandler.GenerateUserName(info.LoginProvider, externalClaims.ToArray());
if (!string.IsNullOrWhiteSpace(userName))
{
// Set the return value to the username generated by the first IExternalLoginHandler.
if (userNames.Count == 0)
{
ret = userName;
}
userNames.Add(item.GetType(), userName);
userNames.Add(externalLoginEventHandler.GetType(), userName);
}
}
catch (Exception ex)
{
_logger.LogError(ex, "{ExternalLoginHandler} - IExternalLoginHandler.GenerateUserName threw an exception", item.GetType());
_logger.LogError(ex, "{ExternalLoginHandler} - IExternalLoginHandler.GenerateUserName threw an exception", externalLoginEventHandler.GetType());
}
}

Expand Down
Loading

0 comments on commit 55c5809

Please sign in to comment.