Skip to content

Commit

Permalink
External Authentication Feature is causing an infinite redirect.
Browse files Browse the repository at this point in the history
Fix #17050
  • Loading branch information
MikeAlhayek committed Nov 25, 2024
1 parent b2a5b54 commit 6946b11
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public sealed class ExternalAuthenticationsController : AccountBaseController
private readonly IShellFeaturesManager _shellFeaturesManager;
private readonly INotifier _notifier;
private readonly IEnumerable<IExternalLoginEventHandler> _externalLoginHandlers;
private readonly RegistrationOptions _registrationOptions;
private readonly ExternalLoginOptions _externalLoginOption;
private readonly IdentityOptions _identityOptions;

Expand All @@ -54,6 +55,7 @@ public ExternalAuthenticationsController(
INotifier notifier,
IEnumerable<IExternalLoginEventHandler> externalLoginHandlers,
IOptions<ExternalLoginOptions> externalLoginOption,
IOptions<RegistrationOptions> registrationOptions,
IOptions<IdentityOptions> identityOptions)
{
_signInManager = signInManager;
Expand All @@ -66,6 +68,7 @@ public ExternalAuthenticationsController(
_shellFeaturesManager = shellFeaturesManager;
_notifier = notifier;
_externalLoginHandlers = externalLoginHandlers;
_registrationOptions = registrationOptions.Value;
_externalLoginOption = externalLoginOption.Value;
_identityOptions = identityOptions.Value;

Expand Down Expand Up @@ -138,11 +141,9 @@ public async Task<IActionResult> ExternalLoginCallback(string returnUrl = null,
ViewData["ReturnUrl"] = returnUrl;
ViewData["LoginProvider"] = info.LoginProvider;

var registrationSettings = await _siteService.GetSettingsAsync<RegistrationSettings>();

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
Expand All @@ -161,6 +162,13 @@ public async Task<IActionResult> ExternalLoginCallback(string returnUrl = null,

var settings = await _siteService.GetSettingsAsync<ExternalRegistrationSettings>();

if (settings.DisableNewRegistrations)
{
await _notifier.ErrorAsync(H["New registrations are disabled for this site."]);

return RedirectToLogin();
}

var externalLoginViewModel = new RegisterExternalLoginViewModel
{
NoPassword = settings.NoPassword,
Expand Down Expand Up @@ -197,7 +205,7 @@ 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)
{
if (registrationSettings.UsersMustValidateEmail && !user.EmailConfirmed)
if (_registrationOptions.UsersMustValidateEmail && !user.EmailConfirmed)
{
return RedirectToAction(nameof(EmailConfirmationController.ConfirmEmailSent),
new
Expand All @@ -208,7 +216,7 @@ public async Task<IActionResult> ExternalLoginCallback(string returnUrl = null,
});
}

if (registrationSettings.UsersAreModerated && !user.IsEnabled)
if (_registrationOptions.UsersAreModerated && !user.IsEnabled)
{
return RedirectToAction(nameof(RegistrationController.RegistrationPending),
new
Expand Down Expand Up @@ -237,13 +245,6 @@ public async Task<IActionResult> 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);
}

Expand All @@ -261,7 +262,7 @@ public async Task<IActionResult> RegisterExternalLogin(RegisterExternalLoginView
{
await _notifier.ErrorAsync(H["New registrations are disabled for this site."]);

return RedirectToLogin();
return RedirectToLogin(returnUrl);
}

var info = await _signInManager.GetExternalLoginInfoAsync();
Expand Down Expand Up @@ -322,9 +323,7 @@ public async Task<IActionResult> 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<RegistrationSettings>();

if (registrationSettings.UsersMustValidateEmail && !user.EmailConfirmed)
if (_registrationOptions.UsersMustValidateEmail && !user.EmailConfirmed)
{
return RedirectToAction(nameof(EmailConfirmationController.ConfirmEmailSent),
new
Expand All @@ -335,7 +334,7 @@ public async Task<IActionResult> RegisterExternalLogin(RegisterExternalLoginView
});
}

if (registrationSettings.UsersAreModerated && !user.IsEnabled)
if (_registrationOptions.UsersAreModerated && !user.IsEnabled)
{
return RedirectToAction(nameof(RegistrationController.RegistrationPending),
new
Expand Down Expand Up @@ -609,16 +608,7 @@ private void CopyModelStateErrorsToTempData(string key = "")

private async Task<bool> AddConfirmEmailErrorAsync(IUser user)
{
var registrationFeatureIsAvailable = (await _shellFeaturesManager.GetAvailableFeaturesAsync())
.Any(feature => feature.Id == UserConstants.Features.UserRegistration);

if (!registrationFeatureIsAvailable)
{
return false;
}

var registrationSettings = await _siteService.GetSettingsAsync<RegistrationSettings>();
if (registrationSettings.UsersMustValidateEmail)
if (_registrationOptions.UsersMustValidateEmail)
{
// Require that the users have a confirmed email before they can log on.
if (!await _userManager.IsEmailConfirmedAsync(user))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -31,6 +32,7 @@ public sealed class ResetPasswordController : Controller
private readonly IDisplayManager<ForgotPasswordForm> _forgotPasswordDisplayManager;
private readonly IDisplayManager<ResetPasswordForm> _resetPasswordDisplayManager;
private readonly IShellFeaturesManager _shellFeaturesManager;
private readonly RegistrationOptions _registrationOptions;

internal readonly IStringLocalizer S;

Expand All @@ -43,6 +45,7 @@ public ResetPasswordController(
IDisplayManager<ForgotPasswordForm> forgotPasswordDisplayManager,
IDisplayManager<ResetPasswordForm> resetPasswordDisplayManager,
IShellFeaturesManager shellFeaturesManager,
IOptions<RegistrationOptions> registrationOptions,
IEnumerable<IPasswordRecoveryFormEvents> passwordRecoveryFormEvents,
IStringLocalizer<ResetPasswordController> stringLocalizer)
{
Expand All @@ -54,6 +57,7 @@ public ResetPasswordController(
_forgotPasswordDisplayManager = forgotPasswordDisplayManager;
_resetPasswordDisplayManager = resetPasswordDisplayManager;
_shellFeaturesManager = shellFeaturesManager;
_registrationOptions = registrationOptions.Value;
_passwordRecoveryFormEvents = passwordRecoveryFormEvents;
S = stringLocalizer;
}
Expand Down Expand Up @@ -179,15 +183,7 @@ public IActionResult ResetPasswordConfirmation()

private async Task<bool> MustValidateEmailAsync(User user)
{
var registrationFeatureIsAvailable = (await _shellFeaturesManager.GetAvailableFeaturesAsync())
.Any(feature => feature.Id == UserConstants.Features.UserRegistration);

if (!registrationFeatureIsAvailable)
{
return false;
}

return (await _siteService.GetSettingsAsync<RegistrationSettings>()).UsersMustValidateEmail
&& !await _userManager.IsEmailConfirmedAsync(user);
return _registrationOptions.UsersMustValidateEmail &&
!await _userManager.IsEmailConfirmedAsync(user);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<IUser> _signInManager;
private readonly LinkGenerator _linkGenerator;
private readonly ShellSettings _shellSettings;
private readonly IHttpContextAccessor _httpContextAccessor;

public ExternalLoginFormEvents(
IOptions<ExternalLoginOptions> externalLoginOptions,
SignInManager<IUser> signInManager,
LinkGenerator linkGenerator,
ShellSettings shellSettings,
IHttpContextAccessor httpContextAccessor)
{
_externalLoginOptions = externalLoginOptions.Value;
_signInManager = signInManager;
_linkGenerator = linkGenerator;
_shellSettings = shellSettings;
_httpContextAccessor = httpContextAccessor;
}

Expand All @@ -42,10 +48,30 @@ public async Task<IActionResult> 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();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
@using Microsoft.AspNetCore.Identity
@using Microsoft.Extensions.Options
@using OrchardCore.Users.Models

@model SummaryAdminUserViewModel

@inject UserManager<IUser> UserManager
@inject IAuthorizationService AuthorizationService
@inject IOptions<RegistrationOptions> RegistrationOptions

@if (!Model.User.EmailConfirmed &&
Site.As<RegistrationSettings>().UsersMustValidateEmail &&
RegistrationOptions.Value.UsersMustValidateEmail &&
await AuthorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, Model.User))
{
<li>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
@using OrchardCore.Users.Models
@using Microsoft.Extensions.Options
@using Microsoft.AspNetCore.Identity

@inject IOptions<IdentityOptions> IdentityOptions
@inject IOptions<RegistrationOptions> RegistrationOptions

@if (Site.As<RegistrationSettings>().UsersMustValidateEmail)
@if (RegistrationOptions.Value.UsersMustValidateEmail)
{
<div class="mb-3">
<div class="form-check">
Expand Down

0 comments on commit 6946b11

Please sign in to comment.