Skip to content

Commit 5d88a0e

Browse files
authored
Make 2FA requirement check extensible (#13889)
1 parent 4c89183 commit 5d88a0e

File tree

8 files changed

+116
-26
lines changed

8 files changed

+116
-26
lines changed

src/OrchardCore.Modules/OrchardCore.Users/Controllers/TwoFactorAuthenticationController.cs

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ public class TwoFactorAuthenticationController : AccountBaseController
4040
private readonly IDistributedCache _distributedCache;
4141
private readonly UrlEncoder _urlEncoder;
4242
private readonly ShellSettings _shellSettings;
43+
private readonly ITwoFactorAuthenticationHandlerCoordinator _twoFactorHandlerCoordinator;
4344
private readonly IHtmlLocalizer H;
4445
private readonly IStringLocalizer S;
4546

@@ -54,7 +55,8 @@ public TwoFactorAuthenticationController(
5455
INotifier notifier,
5556
IDistributedCache distributedCache,
5657
UrlEncoder urlEncoder,
57-
ShellSettings shellSettings)
58+
ShellSettings shellSettings,
59+
ITwoFactorAuthenticationHandlerCoordinator twoFactorHandlerCoordinator)
5860
: base(userManager)
5961
{
6062
_signInManager = signInManager;
@@ -65,6 +67,7 @@ public TwoFactorAuthenticationController(
6567
_distributedCache = distributedCache;
6668
_urlEncoder = urlEncoder;
6769
_shellSettings = shellSettings;
70+
_twoFactorHandlerCoordinator = twoFactorHandlerCoordinator;
6871
H = htmlLocalizer;
6972
S = stringLocalizer;
7073
}
@@ -139,7 +142,7 @@ public async Task<IActionResult> LoginWithTwoFactorAuthentication(LoginWithTwoFa
139142
if (result.IsLockedOut)
140143
{
141144
_logger.LogWarning("User account locked out.");
142-
ModelState.AddModelError(String.Empty, S["The account is locked out"]);
145+
ModelState.AddModelError(String.Empty, S["The account is locked out."]);
143146
await _accountEvents.InvokeAsync((e, user) => e.IsLockedOutAsync(user), user, _logger);
144147

145148
return RedirectToAction(nameof(AccountController.Login), typeof(AccountController).ControllerName());
@@ -202,8 +205,6 @@ public async Task<IActionResult> LoginWithRecoveryCode(LoginWithRecoveryCodeView
202205

203206
var result = await _signInManager.TwoFactorRecoveryCodeSignInAsync(recoveryCode);
204207

205-
var userId = await _userManager.GetUserIdAsync(user);
206-
207208
if (result.Succeeded)
208209
{
209210
await _accountEvents.InvokeAsync((e, user) => e.LoggedInAsync(user), user, _logger);
@@ -221,7 +222,7 @@ public async Task<IActionResult> LoginWithRecoveryCode(LoginWithRecoveryCodeView
221222
return RedirectToAction(nameof(AccountController.Login), typeof(AccountController).ControllerName());
222223
}
223224

224-
_logger.LogWarning("Invalid recovery code entered for user with ID '{UserId}' ", userId);
225+
_logger.LogWarning("Invalid recovery code entered for user.");
225226
ModelState.AddModelError(String.Empty, S["Invalid recovery code entered."]);
226227
}
227228

@@ -241,7 +242,7 @@ public async Task<IActionResult> EnableAuthenticator(string returnUrl)
241242
var user = await _userManager.GetUserAsync(User);
242243
if (user == null)
243244
{
244-
return NotFound($"Unable to load user with ID '{_userManager.GetUserId(User)}'.");
245+
return NotFound("Unable to load user.");
245246
}
246247

247248
var model = await LoadSharedKeyAndQrCodeUriAsync(user, loginSettings);
@@ -266,7 +267,7 @@ public async Task<IActionResult> EnableAuthenticator(EnableAuthenticatorViewMode
266267
var user = await _userManager.GetUserAsync(User);
267268
if (user == null)
268269
{
269-
return NotFound($"Unable to load user with ID '{_userManager.GetUserId(User)}'.");
270+
return NotFound("Unable to load user.");
270271
}
271272

272273
if (!ModelState.IsValid)
@@ -322,7 +323,7 @@ public async Task<IActionResult> Index()
322323
var user = await _userManager.GetUserAsync(User);
323324
if (user == null)
324325
{
325-
return NotFound($"Unable to load user with ID '{_userManager.GetUserId(User)}'.");
326+
return NotFound("Unable to load user.");
326327
}
327328

328329
var model = new TwoFactorAuthenticationViewModel()
@@ -331,7 +332,7 @@ public async Task<IActionResult> Index()
331332
IsTwoFaEnabled = await _userManager.GetTwoFactorEnabledAsync(user),
332333
IsMachineRemembered = await _signInManager.IsTwoFactorClientRememberedAsync(user),
333334
RecoveryCodesLeft = await _userManager.CountRecoveryCodesAsync(user),
334-
CanDisableTwoFa = !loginSettings.RequireTwoFactorAuthentication
335+
CanDisableTwoFa = !await _twoFactorHandlerCoordinator.IsRequiredAsync()
335336
|| !await loginSettings.CanEnableTwoFactorAuthenticationAsync(role => _userManager.IsInRoleAsync(user, role)),
336337
};
337338

@@ -353,7 +354,7 @@ public async Task<IActionResult> ForgetTwoFactorClient()
353354
var user = await _userManager.GetUserAsync(User);
354355
if (user == null)
355356
{
356-
return NotFound($"Unable to load user with ID '{_userManager.GetUserId(User)}'.");
357+
return NotFound("Unable to load user.");
357358
}
358359

359360
await _signInManager.ForgetTwoFactorClientAsync();
@@ -375,7 +376,7 @@ public async Task<IActionResult> GenerateRecoveryCodes()
375376
var user = await _userManager.GetUserAsync(User);
376377
if (user == null)
377378
{
378-
return NotFound($"Unable to load user with ID '{_userManager.GetUserId(User)}'.");
379+
return NotFound("Unable to load user.");
379380
}
380381

381382
var isTwoFactorEnabled = await _userManager.GetTwoFactorEnabledAsync(user);
@@ -405,11 +406,10 @@ public async Task<IActionResult> GenerateRecoveryCodesPost()
405406
var user = await _userManager.GetUserAsync(User);
406407
if (user == null)
407408
{
408-
return NotFound($"Unable to load user with ID '{_userManager.GetUserId(User)}'.");
409+
return NotFound("Unable to load user.");
409410
}
410411

411-
var isTwoFactorEnabled = await _userManager.GetTwoFactorEnabledAsync(user);
412-
if (!isTwoFactorEnabled)
412+
if (!await _userManager.GetTwoFactorEnabledAsync(user))
413413
{
414414
await _notifier.ErrorAsync(H["Cannot generate recovery codes for user because they do not have 2FA enabled."]);
415415

@@ -437,7 +437,7 @@ public async Task<IActionResult> ShowRecoveryCodes()
437437
var user = await _userManager.GetUserAsync(User);
438438
if (user == null)
439439
{
440-
return NotFound($"Unable to load user with ID '{_userManager.GetUserId(User)}'.");
440+
return NotFound("Unable to load user.");
441441
}
442442

443443
var userId = await _userManager.GetUserIdAsync(user);
@@ -468,7 +468,7 @@ public async Task<IActionResult> ResetAuthenticator()
468468
var user = await _userManager.GetUserAsync(User);
469469
if (user == null)
470470
{
471-
return NotFound($"Unable to load user with ID '{_userManager.GetUserId(User)}'.");
471+
return NotFound("Unable to load user.");
472472
}
473473

474474
return View();
@@ -490,7 +490,7 @@ public async Task<IActionResult> ResetAuthenticatorPost()
490490
var user = await _userManager.GetUserAsync(User);
491491
if (user == null)
492492
{
493-
return NotFound($"Unable to load user with ID '{_userManager.GetUserId(User)}'.");
493+
return NotFound("Unable to load user.");
494494
}
495495

496496
await _userManager.SetTwoFactorEnabledAsync(user, false);
@@ -514,10 +514,10 @@ public async Task<IActionResult> DisableTwoFactorAuthentication()
514514
var user = await _userManager.GetUserAsync(User);
515515
if (user == null)
516516
{
517-
return NotFound($"Unable to load user with ID '{_userManager.GetUserId(User)}'.");
517+
return NotFound("Unable to load user.");
518518
}
519519

520-
if (loginSettings.RequireTwoFactorAuthentication
520+
if (await _twoFactorHandlerCoordinator.IsRequiredAsync()
521521
&& await loginSettings.CanEnableTwoFactorAuthenticationAsync(role => _userManager.IsInRoleAsync(user, role)))
522522
{
523523
await _notifier.WarningAsync(H["Two-factor authentication cannot be disabled for the current user."]);
@@ -544,10 +544,10 @@ public async Task<IActionResult> DisableTwoFactorAuthenticationPost()
544544
var user = await _userManager.GetUserAsync(User);
545545
if (user == null)
546546
{
547-
return NotFound($"Unable to load user with ID '{_userManager.GetUserId(User)}'.");
547+
return NotFound("Unable to load user.");
548548
}
549549

550-
if (loginSettings.RequireTwoFactorAuthentication
550+
if (await _twoFactorHandlerCoordinator.IsRequiredAsync()
551551
&& await loginSettings.CanEnableTwoFactorAuthenticationAsync(role => _userManager.IsInRoleAsync(user, role)))
552552
{
553553
await _notifier.WarningAsync(H["Two-factor authentication cannot be disabled for the current user."]);

src/OrchardCore.Modules/OrchardCore.Users/Filters/TwoFactorAuthenticationAuthorizationFilter.cs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using Microsoft.Extensions.Options;
77
using OrchardCore.Entities;
88
using OrchardCore.Settings;
9+
using OrchardCore.Users.Events;
910
using OrchardCore.Users.Models;
1011
using OrchardCore.Users.Services;
1112

@@ -14,10 +15,14 @@ namespace OrchardCore.Users.Filters;
1415
public class TwoFactorAuthenticationAuthorizationFilter : IAsyncAuthorizationFilter
1516
{
1617
private readonly UserOptions _userOptions;
18+
private readonly ITwoFactorAuthenticationHandlerCoordinator _twoFactorHandlerCoordinator;
1719

18-
public TwoFactorAuthenticationAuthorizationFilter(IOptions<UserOptions> userOptions)
20+
public TwoFactorAuthenticationAuthorizationFilter(
21+
IOptions<UserOptions> userOptions,
22+
ITwoFactorAuthenticationHandlerCoordinator twoFactorHandlerCoordinator)
1923
{
2024
_userOptions = userOptions.Value;
25+
_twoFactorHandlerCoordinator = twoFactorHandlerCoordinator;
2126
}
2227

2328
public async Task OnAuthorizationAsync(AuthorizationFilterContext context)
@@ -43,8 +48,8 @@ public async Task OnAuthorizationAsync(AuthorizationFilterContext context)
4348

4449
var loginSettings = (await siteService.GetSiteSettingsAsync()).As<LoginSettings>();
4550

46-
if (loginSettings.RequireTwoFactorAuthentication
47-
&& loginSettings.IsTwoFactorAuthenticationEnabled()
51+
if (loginSettings.IsTwoFactorAuthenticationEnabled()
52+
&& await _twoFactorHandlerCoordinator.IsRequiredAsync()
4853
&& context.HttpContext.User.HasClaim(claim => claim.Type == TwoFactorAuthenticationClaimsProvider.TwoFactorAuthenticationClaimType))
4954
{
5055
context.Result = new RedirectResult("~/" + _userOptions.EnableAuthenticatorPath);

src/OrchardCore.Modules/OrchardCore.Users/Services/TwoFactorAuthenticationClaimsProvider.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using Microsoft.AspNetCore.Identity;
55
using OrchardCore.Entities;
66
using OrchardCore.Settings;
7+
using OrchardCore.Users.Events;
78
using OrchardCore.Users.Models;
89

910
namespace OrchardCore.Users.Services;
@@ -14,13 +15,16 @@ public class TwoFactorAuthenticationClaimsProvider : IUserClaimsProvider
1415

1516
private readonly UserManager<IUser> _userManager;
1617
private readonly ISiteService _siteService;
18+
private readonly ITwoFactorAuthenticationHandlerCoordinator _twoFactorHandlerCoordinator;
1719

1820
public TwoFactorAuthenticationClaimsProvider(
1921
UserManager<IUser> userManager,
20-
ISiteService siteService)
22+
ISiteService siteService,
23+
ITwoFactorAuthenticationHandlerCoordinator twoFactorHandlerCoordinator)
2124
{
2225
_userManager = userManager;
2326
_siteService = siteService;
27+
_twoFactorHandlerCoordinator = twoFactorHandlerCoordinator;
2428
}
2529

2630
public async Task GenerateAsync(IUser user, ClaimsIdentity claims)
@@ -37,7 +41,7 @@ public async Task GenerateAsync(IUser user, ClaimsIdentity claims)
3741

3842
var loginSettings = (await _siteService.GetSiteSettingsAsync()).As<LoginSettings>();
3943

40-
if (loginSettings.RequireTwoFactorAuthentication
44+
if (await _twoFactorHandlerCoordinator.IsRequiredAsync()
4145
&& !await _userManager.GetTwoFactorEnabledAsync(user)
4246
&& await loginSettings.CanEnableTwoFactorAuthenticationAsync(role => _userManager.IsInRoleAsync(user, role)))
4347
{
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
using System.Threading.Tasks;
2+
3+
namespace OrchardCore.Users.Events;
4+
5+
public interface ITwoFactorAuthenticationHandler
6+
{
7+
/// <summary>
8+
/// Checks if the two-factor authentication should be required or not.
9+
/// </summary>
10+
/// <returns>true if the two-factor authentication is required otherwise false.</returns>
11+
Task<bool> IsRequiredAsync();
12+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
using System.Threading.Tasks;
2+
3+
namespace OrchardCore.Users.Events;
4+
5+
public interface ITwoFactorAuthenticationHandlerCoordinator
6+
{
7+
/// <summary>
8+
/// Checks if the two-factor authentication should be required or not.
9+
/// </summary>
10+
/// <returns>true if any of the two-factor authentication providers require 2FA otherwise false.</returns>
11+
Task<bool> IsRequiredAsync();
12+
}

src/OrchardCore/OrchardCore.Users.Core/Extensions/UsersServiceCollectionExtensions.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using OrchardCore.Security;
66
using OrchardCore.Security.Services;
77
using OrchardCore.Users;
8+
using OrchardCore.Users.Events;
89
using OrchardCore.Users.Handlers;
910
using OrchardCore.Users.Indexes;
1011
using OrchardCore.Users.Services;
@@ -50,6 +51,9 @@ public static IServiceCollection AddUsers(this IServiceCollection services)
5051
services.AddScoped<IUserClaimsPrincipalFactory<IUser>, DefaultUserClaimsPrincipalProviderFactory>();
5152
services.AddScoped<IUserEventHandler, UserDisabledEventHandler>();
5253

54+
services.AddScoped<ITwoFactorAuthenticationHandler, DefaultTwoFactorAuthenticationHandler>();
55+
services.AddScoped<ITwoFactorAuthenticationHandlerCoordinator, DefaultTwoFactorAuthenticationHandlerCoordinator>();
56+
5357
return services;
5458
}
5559
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
using System.Threading.Tasks;
2+
using OrchardCore.Entities;
3+
using OrchardCore.Settings;
4+
using OrchardCore.Users.Events;
5+
using OrchardCore.Users.Models;
6+
7+
namespace OrchardCore.Users.Services;
8+
9+
public class DefaultTwoFactorAuthenticationHandler : ITwoFactorAuthenticationHandler
10+
{
11+
private readonly ISiteService _siteService;
12+
13+
public DefaultTwoFactorAuthenticationHandler(ISiteService siteService)
14+
{
15+
_siteService = siteService;
16+
}
17+
18+
public async Task<bool> IsRequiredAsync()
19+
{
20+
var loginSettings = (await _siteService.GetSiteSettingsAsync()).As<LoginSettings>();
21+
22+
return loginSettings.RequireTwoFactorAuthentication;
23+
}
24+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
using System.Collections.Generic;
2+
using System.Threading.Tasks;
3+
using OrchardCore.Users.Events;
4+
5+
namespace OrchardCore.Users.Services;
6+
7+
public class DefaultTwoFactorAuthenticationHandlerCoordinator : ITwoFactorAuthenticationHandlerCoordinator
8+
{
9+
private readonly IEnumerable<ITwoFactorAuthenticationHandler> _handlers;
10+
11+
public DefaultTwoFactorAuthenticationHandlerCoordinator(
12+
IEnumerable<ITwoFactorAuthenticationHandler> handlers)
13+
{
14+
_handlers = handlers;
15+
}
16+
17+
public async Task<bool> IsRequiredAsync()
18+
{
19+
foreach (var handler in _handlers)
20+
{
21+
if (await handler.IsRequiredAsync())
22+
{
23+
return true;
24+
}
25+
}
26+
27+
return false;
28+
}
29+
}

0 commit comments

Comments
 (0)