From 6946b11865befb0eb0016448b480ab56548508e9 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Mon, 25 Nov 2024 09:03:53 -0800 Subject: [PATCH] External Authentication Feature is causing an infinite redirect. Fix #17050 --- .../ExternalAuthenticationsController.cs | 44 +++++++------------ .../Controllers/ResetPasswordController.cs | 16 +++---- .../Services/ExternalLoginFormEvents.cs | 26 +++++++++++ .../UserSendConfirmationActionsMenu.cshtml | 4 +- .../Views/UserFields.Edit.cshtml | 4 +- 5 files changed, 55 insertions(+), 39 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Controllers/ExternalAuthenticationsController.cs b/src/OrchardCore.Modules/OrchardCore.Users/Controllers/ExternalAuthenticationsController.cs index b6d19808f32..f02961b9dee 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Controllers/ExternalAuthenticationsController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Controllers/ExternalAuthenticationsController.cs @@ -34,6 +34,7 @@ public sealed class ExternalAuthenticationsController : AccountBaseController private readonly IShellFeaturesManager _shellFeaturesManager; private readonly INotifier _notifier; private readonly IEnumerable _externalLoginHandlers; + private readonly RegistrationOptions _registrationOptions; private readonly ExternalLoginOptions _externalLoginOption; private readonly IdentityOptions _identityOptions; @@ -54,6 +55,7 @@ public ExternalAuthenticationsController( INotifier notifier, IEnumerable externalLoginHandlers, IOptions externalLoginOption, + IOptions registrationOptions, IOptions identityOptions) { _signInManager = signInManager; @@ -66,6 +68,7 @@ public ExternalAuthenticationsController( _shellFeaturesManager = shellFeaturesManager; _notifier = notifier; _externalLoginHandlers = externalLoginHandlers; + _registrationOptions = registrationOptions.Value; _externalLoginOption = externalLoginOption.Value; _identityOptions = identityOptions.Value; @@ -138,11 +141,9 @@ public async Task ExternalLoginCallback(string returnUrl = null, ViewData["ReturnUrl"] = returnUrl; ViewData["LoginProvider"] = info.LoginProvider; - var registrationSettings = await _siteService.GetSettingsAsync(); - if (iUser != null) { - if (iUser is User userToLink && registrationSettings.UsersMustValidateEmail && !userToLink.EmailConfirmed) + if (iUser is User userToLink && _registrationOptions.UsersMustValidateEmail && !userToLink.EmailConfirmed) { return RedirectToAction(nameof(EmailConfirmationController.ConfirmEmailSent), new @@ -161,6 +162,13 @@ public async Task ExternalLoginCallback(string returnUrl = null, var settings = await _siteService.GetSettingsAsync(); + if (settings.DisableNewRegistrations) + { + await _notifier.ErrorAsync(H["New registrations are disabled for this site."]); + + return RedirectToLogin(); + } + var externalLoginViewModel = new RegisterExternalLoginViewModel { NoPassword = settings.NoPassword, @@ -197,7 +205,7 @@ public async Task 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) { - if (registrationSettings.UsersMustValidateEmail && !user.EmailConfirmed) + if (_registrationOptions.UsersMustValidateEmail && !user.EmailConfirmed) { return RedirectToAction(nameof(EmailConfirmationController.ConfirmEmailSent), new @@ -208,7 +216,7 @@ public async Task ExternalLoginCallback(string returnUrl = null, }); } - if (registrationSettings.UsersAreModerated && !user.IsEnabled) + if (_registrationOptions.UsersAreModerated && !user.IsEnabled) { return RedirectToAction(nameof(RegistrationController.RegistrationPending), new @@ -237,13 +245,6 @@ public async Task ExternalLoginCallback(string returnUrl = null, } } - if (settings.DisableNewRegistrations) - { - await _notifier.ErrorAsync(H["New registrations are disabled for this site."]); - - return RedirectToLogin(); - } - return View(nameof(RegisterExternalLogin), externalLoginViewModel); } @@ -261,7 +262,7 @@ public async Task RegisterExternalLogin(RegisterExternalLoginView { await _notifier.ErrorAsync(H["New registrations are disabled for this site."]); - return RedirectToLogin(); + return RedirectToLogin(returnUrl); } var info = await _signInManager.GetExternalLoginInfoAsync(); @@ -322,9 +323,7 @@ public async Task RegisterExternalLogin(RegisterExternalLoginView // The login info must be linked before we consider a redirect, or the login info is lost. if (iUser is User user) { - var registrationSettings = await _siteService.GetSettingsAsync(); - - if (registrationSettings.UsersMustValidateEmail && !user.EmailConfirmed) + if (_registrationOptions.UsersMustValidateEmail && !user.EmailConfirmed) { return RedirectToAction(nameof(EmailConfirmationController.ConfirmEmailSent), new @@ -335,7 +334,7 @@ public async Task RegisterExternalLogin(RegisterExternalLoginView }); } - if (registrationSettings.UsersAreModerated && !user.IsEnabled) + if (_registrationOptions.UsersAreModerated && !user.IsEnabled) { return RedirectToAction(nameof(RegistrationController.RegistrationPending), new @@ -609,16 +608,7 @@ private void CopyModelStateErrorsToTempData(string key = "") private async Task AddConfirmEmailErrorAsync(IUser user) { - var registrationFeatureIsAvailable = (await _shellFeaturesManager.GetAvailableFeaturesAsync()) - .Any(feature => feature.Id == UserConstants.Features.UserRegistration); - - if (!registrationFeatureIsAvailable) - { - return false; - } - - var registrationSettings = await _siteService.GetSettingsAsync(); - if (registrationSettings.UsersMustValidateEmail) + if (_registrationOptions.UsersMustValidateEmail) { // Require that the users have a confirmed email before they can log on. if (!await _userManager.IsEmailConfirmedAsync(user)) diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Controllers/ResetPasswordController.cs b/src/OrchardCore.Modules/OrchardCore.Users/Controllers/ResetPasswordController.cs index f58d9900b86..66fe42c2c47 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Controllers/ResetPasswordController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Controllers/ResetPasswordController.cs @@ -4,6 +4,7 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Localization; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using OrchardCore.DisplayManagement; using OrchardCore.DisplayManagement.ModelBinding; using OrchardCore.Environment.Shell; @@ -31,6 +32,7 @@ public sealed class ResetPasswordController : Controller private readonly IDisplayManager _forgotPasswordDisplayManager; private readonly IDisplayManager _resetPasswordDisplayManager; private readonly IShellFeaturesManager _shellFeaturesManager; + private readonly RegistrationOptions _registrationOptions; internal readonly IStringLocalizer S; @@ -43,6 +45,7 @@ public ResetPasswordController( IDisplayManager forgotPasswordDisplayManager, IDisplayManager resetPasswordDisplayManager, IShellFeaturesManager shellFeaturesManager, + IOptions registrationOptions, IEnumerable passwordRecoveryFormEvents, IStringLocalizer stringLocalizer) { @@ -54,6 +57,7 @@ public ResetPasswordController( _forgotPasswordDisplayManager = forgotPasswordDisplayManager; _resetPasswordDisplayManager = resetPasswordDisplayManager; _shellFeaturesManager = shellFeaturesManager; + _registrationOptions = registrationOptions.Value; _passwordRecoveryFormEvents = passwordRecoveryFormEvents; S = stringLocalizer; } @@ -179,15 +183,7 @@ public IActionResult ResetPasswordConfirmation() private async Task MustValidateEmailAsync(User user) { - var registrationFeatureIsAvailable = (await _shellFeaturesManager.GetAvailableFeaturesAsync()) - .Any(feature => feature.Id == UserConstants.Features.UserRegistration); - - if (!registrationFeatureIsAvailable) - { - return false; - } - - return (await _siteService.GetSettingsAsync()).UsersMustValidateEmail - && !await _userManager.IsEmailConfirmedAsync(user); + return _registrationOptions.UsersMustValidateEmail && + !await _userManager.IsEmailConfirmedAsync(user); } } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Services/ExternalLoginFormEvents.cs b/src/OrchardCore.Modules/OrchardCore.Users/Services/ExternalLoginFormEvents.cs index 6a9fa06d650..8185490f983 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Services/ExternalLoginFormEvents.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Services/ExternalLoginFormEvents.cs @@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.Options; +using OrchardCore.Environment.Shell; using OrchardCore.Mvc.Core.Utilities; using OrchardCore.Users.Controllers; using OrchardCore.Users.Events; @@ -12,20 +13,25 @@ namespace OrchardCore.Users.Services; public sealed class ExternalLoginFormEvents : ILoginFormEvent { + private const string ExternalLoginAutoRedirectCookieName = "ELAR"; + private readonly ExternalLoginOptions _externalLoginOptions; private readonly SignInManager _signInManager; private readonly LinkGenerator _linkGenerator; + private readonly ShellSettings _shellSettings; private readonly IHttpContextAccessor _httpContextAccessor; public ExternalLoginFormEvents( IOptions externalLoginOptions, SignInManager signInManager, LinkGenerator linkGenerator, + ShellSettings shellSettings, IHttpContextAccessor httpContextAccessor) { _externalLoginOptions = externalLoginOptions.Value; _signInManager = signInManager; _linkGenerator = linkGenerator; + _shellSettings = shellSettings; _httpContextAccessor = httpContextAccessor; } @@ -42,10 +48,30 @@ public async Task LoggingInAsync() return null; } + // To prevent infinite redirects, we store an ELAR cookie, which stands for ExternalLoginAutoRedirect. + // This cookie helps us avoid executing the same redirect multiple times. + // The acronym ELAR has no special meaning, and any other value could be used in its place; it is chosen simply for brevity. + if (_httpContextAccessor.HttpContext.Request.Cookies.TryGetValue(ExternalLoginAutoRedirectCookieName, out var _)) + { + _httpContextAccessor.HttpContext.Response.Cookies.Delete(ExternalLoginAutoRedirectCookieName); + + return null; + } + var schemes = await _signInManager.GetExternalAuthenticationSchemesAsync(); if (schemes.Count() == 1) { + _httpContextAccessor.HttpContext.Response.Cookies.Append(ExternalLoginAutoRedirectCookieName, "1", new CookieOptions + { + HttpOnly = true, + Secure = true, + SameSite = SameSiteMode.Strict, + Path = !string.IsNullOrEmpty(_shellSettings.RequestUrlPrefix) + ? _shellSettings.RequestUrlPrefix + : "/", + }); + var provider = schemes.First().Name; var model = new RouteValueDictionary(); diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/Items/UserSendConfirmationActionsMenu.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/Items/UserSendConfirmationActionsMenu.cshtml index 9f13dec6c58..6e86308fde4 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/Items/UserSendConfirmationActionsMenu.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/Items/UserSendConfirmationActionsMenu.cshtml @@ -1,13 +1,15 @@ @using Microsoft.AspNetCore.Identity +@using Microsoft.Extensions.Options @using OrchardCore.Users.Models @model SummaryAdminUserViewModel @inject UserManager UserManager @inject IAuthorizationService AuthorizationService +@inject IOptions RegistrationOptions @if (!Model.User.EmailConfirmed && -Site.As().UsersMustValidateEmail && +RegistrationOptions.Value.UsersMustValidateEmail && await AuthorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, Model.User)) {
  • diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserFields.Edit.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserFields.Edit.cshtml index 98d3203d770..fae9f8695ab 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserFields.Edit.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserFields.Edit.cshtml @@ -4,9 +4,11 @@ @using OrchardCore.Users.Models @using Microsoft.Extensions.Options @using Microsoft.AspNetCore.Identity + @inject IOptions IdentityOptions +@inject IOptions RegistrationOptions -@if (Site.As().UsersMustValidateEmail) +@if (RegistrationOptions.Value.UsersMustValidateEmail) {