From 6dd08b23777ff5867436d438bd9d49c5c641fb07 Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Tue, 16 Apr 2024 17:08:55 +0200 Subject: [PATCH] V14 External login linking + Proposed error handling (#16052) * Added mostly working linking methods to the backoffice controller Cleanup still required * Added proposed default error handling extionsion methods * Cleanup, clarification and PR feedback * More cleanup * Transformed the OAuthOptionsExtensions into a helper class this allows for proper DI for the dependencies --------- Co-authored-by: Sven Geusens --- .../Security/BackOfficeController.cs | 144 +++++++++++++++++- .../UmbracoBuilder.BackOffice.cs | 1 + .../Routing/BackOfficeAreaRoutes.cs | 2 +- .../BackOfficeAuthenticationBuilder.cs | 2 +- .../Security/UnLinkLoginRequestModel.cs | 13 ++ .../Configuration/Models/SecuritySettings.cs | 4 + .../UmbracoBuilderExtensions.cs | 8 + .../Helpers/OAuthOptionsHelper.cs | 83 ++++++++++ 8 files changed, 254 insertions(+), 3 deletions(-) create mode 100644 src/Umbraco.Cms.Api.Management/ViewModels/Security/UnLinkLoginRequestModel.cs create mode 100644 src/Umbraco.Web.Common/Helpers/OAuthOptionsHelper.cs diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Security/BackOfficeController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Security/BackOfficeController.cs index e65eafb457..a4f466e58c 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Security/BackOfficeController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Security/BackOfficeController.cs @@ -39,6 +39,7 @@ public class BackOfficeController : SecurityControllerBase private readonly ILogger _logger; private readonly IBackOfficeTwoFactorOptions _backOfficeTwoFactorOptions; private readonly IUserTwoFactorLoginService _userTwoFactorLoginService; + private readonly IBackOfficeExternalLoginProviders _backOfficeExternalLoginProviders; public BackOfficeController( IHttpContextAccessor httpContextAccessor, @@ -47,7 +48,8 @@ public class BackOfficeController : SecurityControllerBase IOptions securitySettings, ILogger logger, IBackOfficeTwoFactorOptions backOfficeTwoFactorOptions, - IUserTwoFactorLoginService userTwoFactorLoginService) + IUserTwoFactorLoginService userTwoFactorLoginService, + IBackOfficeExternalLoginProviders backOfficeExternalLoginProviders) { _httpContextAccessor = httpContextAccessor; _backOfficeSignInManager = backOfficeSignInManager; @@ -56,6 +58,7 @@ public class BackOfficeController : SecurityControllerBase _logger = logger; _backOfficeTwoFactorOptions = backOfficeTwoFactorOptions; _userTwoFactorLoginService = userTwoFactorLoginService; + _backOfficeExternalLoginProviders = backOfficeExternalLoginProviders; } [HttpPost("login")] @@ -184,6 +187,145 @@ public class BackOfficeController : SecurityControllerBase return SignOut(Constants.Security.BackOfficeAuthenticationType, OpenIddictServerAspNetCoreDefaults.AuthenticationScheme); } + /// + /// Called when a user links an external login provider in the back office + /// + /// + /// + [HttpPost("link-login")] + [MapToApiVersion("1.0")] + public IActionResult LinkLogin(string provider) + { + // Request a redirect to the external login provider to link a login for the current user + var redirectUrl = Url.Action(nameof(ExternalLinkLoginCallback), this.GetControllerName()); + + // Configures the redirect URL and user identifier for the specified external login including xsrf data + AuthenticationProperties properties = + _backOfficeSignInManager.ConfigureExternalAuthenticationProperties(provider, redirectUrl, _backOfficeUserManager.GetUserId(User)); + + return Challenge(properties, provider); + } + + /// + /// Callback path when the user initiates a link login request from the back office to the external provider from the + /// action + /// + /// + /// An example of this is here + /// https://github.com/dotnet/aspnetcore/blob/main/src/Identity/samples/IdentitySample.Mvc/Controllers/AccountController.cs#L155 + /// which this is based on + /// + [HttpGet("ExternalLinkLoginCallback")] + [AllowAnonymous] + [MapToApiVersion("1.0")] + public async Task ExternalLinkLoginCallback() + { + var cookieAuthenticatedUserAttempt = + await HttpContext.AuthenticateAsync(Constants.Security.BackOfficeAuthenticationType); + + if (cookieAuthenticatedUserAttempt.Succeeded == false) + { + return Redirect(_securitySettings.Value.AuthorizeCallbackErrorPathName.AppendQueryStringToUrl( + "flow=external-login-callback", + "status=unauthorized")); + } + + BackOfficeIdentityUser? user = await _backOfficeUserManager.GetUserAsync(cookieAuthenticatedUserAttempt.Principal); + if (user == null) + { + return Redirect(_securitySettings.Value.AuthorizeCallbackErrorPathName.AppendQueryStringToUrl( + "flow=external-login-callback", + "status=user-not-found")); + } + + ExternalLoginInfo? info = + await _backOfficeSignInManager.GetExternalLoginInfoAsync(); + + if (info == null) + { + return Redirect(_securitySettings.Value.AuthorizeCallbackErrorPathName.AppendQueryStringToUrl( + "flow=external-login-callback", + "status=external-info-not-found")); + } + + IdentityResult addLoginResult = await _backOfficeUserManager.AddLoginAsync(user, info); + if (addLoginResult.Succeeded) + { + // Update any authentication tokens if succeeded + await _backOfficeSignInManager.UpdateExternalAuthenticationTokensAsync(info); + return Redirect("/umbraco"); // todo shouldn't this come from configuration + } + + // Add errors and redirect for it to be displayed + // TempData[ViewDataExtensions.TokenExternalSignInError] = addLoginResult.Errors; + // return RedirectToLogin(new { flow = "external-login", status = "failed", logout = "true" }); + // todo + return Redirect(_securitySettings.Value.AuthorizeCallbackErrorPathName.AppendQueryStringToUrl( + "flow=external-login-callback", + "status=failed")); + } + + // todo cleanup unhappy responses + [HttpPost("unlink-login")] + [MapToApiVersion("1.0")] + public async Task PostUnLinkLogin(UnLinkLoginRequestModel unlinkLoginRequestModel) + { + var userId = User.Identity?.GetUserId(); + if (userId is null) + { + throw new InvalidOperationException("Could not find userId"); + } + + BackOfficeIdentityUser? user = await _backOfficeUserManager.FindByIdAsync(userId); + if (user == null) + { + throw new InvalidOperationException("Could not find user"); + } + + AuthenticationScheme? authType = (await _backOfficeSignInManager.GetExternalAuthenticationSchemesAsync()) + .FirstOrDefault(x => x.Name == unlinkLoginRequestModel.LoginProvider); + + if (authType == null) + { + _logger.LogWarning("Could not find the supplied external authentication provider"); + } + else + { + BackOfficeExternaLoginProviderScheme? opt = await _backOfficeExternalLoginProviders.GetAsync(authType.Name); + if (opt == null) + { + return StatusCode(StatusCodes.Status400BadRequest, new ProblemDetailsBuilder() + .WithTitle("Missing Authentication options") + .WithDetail($"Could not find external authentication options registered for provider {authType.Name}") + .Build()); + } + + if (!opt.ExternalLoginProvider.Options.AutoLinkOptions.AllowManualLinking) + { + // If AllowManualLinking is disabled for this provider we cannot unlink + return StatusCode(StatusCodes.Status400BadRequest, new ProblemDetailsBuilder() + .WithTitle("Unlinking disabled") + .WithDetail($"Manual linking is disabled for provider {authType.Name}") + .Build()); + } + } + + IdentityResult result = await _backOfficeUserManager.RemoveLoginAsync( + user, + unlinkLoginRequestModel.LoginProvider, + unlinkLoginRequestModel.ProviderKey); + + if (result.Succeeded) + { + await _backOfficeSignInManager.SignInAsync(user, true); + return Ok(); + } + + return StatusCode(StatusCodes.Status400BadRequest, new ProblemDetailsBuilder() + .WithTitle("Unlinking failed") + .Build()); + } + /// /// Retrieve the user principal stored in the authentication cookie. /// diff --git a/src/Umbraco.Cms.Api.Management/DependencyInjection/UmbracoBuilder.BackOffice.cs b/src/Umbraco.Cms.Api.Management/DependencyInjection/UmbracoBuilder.BackOffice.cs index 4cc22890cd..907e3057af 100644 --- a/src/Umbraco.Cms.Api.Management/DependencyInjection/UmbracoBuilder.BackOffice.cs +++ b/src/Umbraco.Cms.Api.Management/DependencyInjection/UmbracoBuilder.BackOffice.cs @@ -24,6 +24,7 @@ public static partial class UmbracoBuilderExtensions .AddConfiguration() .AddUmbracoCore() .AddWebComponents() + .AddHelpers() .AddBackOfficeCore() .AddBackOfficeIdentity() .AddBackOfficeAuthentication() diff --git a/src/Umbraco.Cms.Api.Management/Routing/BackOfficeAreaRoutes.cs b/src/Umbraco.Cms.Api.Management/Routing/BackOfficeAreaRoutes.cs index a5e93c6f67..e51da4246a 100644 --- a/src/Umbraco.Cms.Api.Management/Routing/BackOfficeAreaRoutes.cs +++ b/src/Umbraco.Cms.Api.Management/Routing/BackOfficeAreaRoutes.cs @@ -82,6 +82,6 @@ public sealed class BackOfficeAreaRoutes : IAreaRoutes Controller = ControllerExtensions.GetControllerName(), Action = nameof(BackOfficeDefaultController.Index), }, - constraints: new { slug = @"^(section.*|upgrade|install|logout)$" }); + constraints: new { slug = @"^(section.*|upgrade|install|logout|error)$" }); } } diff --git a/src/Umbraco.Cms.Api.Management/Security/BackOfficeAuthenticationBuilder.cs b/src/Umbraco.Cms.Api.Management/Security/BackOfficeAuthenticationBuilder.cs index f688d138bd..98489a849c 100644 --- a/src/Umbraco.Cms.Api.Management/Security/BackOfficeAuthenticationBuilder.cs +++ b/src/Umbraco.Cms.Api.Management/Security/BackOfficeAuthenticationBuilder.cs @@ -20,7 +20,7 @@ public class BackOfficeAuthenticationBuilder : AuthenticationBuilder : base(services) => _loginProviderOptions = loginProviderOptions ?? (x => { }); - public string? SchemeForBackOffice(string scheme) + public static string? SchemeForBackOffice(string scheme) => scheme?.EnsureStartsWith(Constants.Security.BackOfficeExternalAuthenticationTypePrefix); /// diff --git a/src/Umbraco.Cms.Api.Management/ViewModels/Security/UnLinkLoginRequestModel.cs b/src/Umbraco.Cms.Api.Management/ViewModels/Security/UnLinkLoginRequestModel.cs new file mode 100644 index 0000000000..50d34810b4 --- /dev/null +++ b/src/Umbraco.Cms.Api.Management/ViewModels/Security/UnLinkLoginRequestModel.cs @@ -0,0 +1,13 @@ +using System.ComponentModel.DataAnnotations; +using System.Runtime.Serialization; + +namespace Umbraco.Cms.Api.Management.ViewModels.Security; + +public class UnLinkLoginRequestModel +{ + [Required] + public required string LoginProvider { get; set; } + + [Required] + public required string ProviderKey { get; set; } +} diff --git a/src/Umbraco.Core/Configuration/Models/SecuritySettings.cs b/src/Umbraco.Core/Configuration/Models/SecuritySettings.cs index f7bb1fe2f4..80a9b38d4f 100644 --- a/src/Umbraco.Core/Configuration/Models/SecuritySettings.cs +++ b/src/Umbraco.Core/Configuration/Models/SecuritySettings.cs @@ -26,6 +26,7 @@ public class SecuritySettings internal const int StaticMemberDefaultLockoutTimeInMinutes = 30 * 24 * 60; internal const int StaticUserDefaultLockoutTimeInMinutes = 30 * 24 * 60; internal const string StaticAuthorizeCallbackPathName = "/umbraco"; + internal const string StaticAuthorizeCallbackErrorPathName = "/umbraco/error"; /// /// Gets or sets a value indicating whether to keep the user logged in. @@ -116,4 +117,7 @@ public class SecuritySettings /// [DefaultValue(StaticAuthorizeCallbackPathName)] public string AuthorizeCallbackPathName { get; set; } = StaticAuthorizeCallbackPathName; + + [DefaultValue(StaticAuthorizeCallbackErrorPathName)] + public string AuthorizeCallbackErrorPathName { get; set; } = StaticAuthorizeCallbackErrorPathName; } diff --git a/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs index 480ae90397..685adf2fbd 100644 --- a/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs @@ -46,6 +46,7 @@ using Umbraco.Cms.Web.Common.Blocks; using Umbraco.Cms.Web.Common.Configuration; using Umbraco.Cms.Web.Common.DependencyInjection; using Umbraco.Cms.Web.Common.FileProviders; +using Umbraco.Cms.Web.Common.Helpers; using Umbraco.Cms.Web.Common.Localization; using Umbraco.Cms.Web.Common.Middleware; using Umbraco.Cms.Web.Common.ModelBinders; @@ -306,6 +307,13 @@ public static partial class UmbracoBuilderExtensions return builder; } + public static IUmbracoBuilder AddHelpers(this IUmbracoBuilder builder) + { + builder.Services.AddSingleton(); + + return builder; + } + // TODO: Does this need to exist and/or be public? public static IUmbracoBuilder AddWebServer(this IUmbracoBuilder builder) { diff --git a/src/Umbraco.Web.Common/Helpers/OAuthOptionsHelper.cs b/src/Umbraco.Web.Common/Helpers/OAuthOptionsHelper.cs new file mode 100644 index 0000000000..4179819f73 --- /dev/null +++ b/src/Umbraco.Web.Common/Helpers/OAuthOptionsHelper.cs @@ -0,0 +1,83 @@ +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Authentication.OAuth; +using Microsoft.Extensions.Options; +using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Web.Common.Helpers; + +public class OAuthOptionsHelper +{ + // https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1 + // we omit "state" and "error_uri" here as it hold no value in determining the message to display to the user + private static readonly IReadOnlyCollection _oathCallbackErrorParams = new string[] { "error", "error_description" }; + + private readonly IOptions _securitySettings; + + public OAuthOptionsHelper(IOptions securitySettings) + { + _securitySettings = securitySettings; + } + + /// + /// Applies SetUmbracoRedirectWithFilteredParams to both OnAccessDenied and OnRemoteFailure + /// on the OAuthOptions so Umbraco can do its best to nicely display the error messages + /// that are passed back from the external login provider on failure. + /// + public T SetDefaultErrorEventHandling(T oAuthOptions, string providerFriendlyName) where T : OAuthOptions + { + oAuthOptions.Events.OnAccessDenied = + context => HandleResponseWithDefaultUmbracoRedirect(context, providerFriendlyName, "OnAccessDenied"); + oAuthOptions.Events.OnRemoteFailure = + context => HandleResponseWithDefaultUmbracoRedirect(context, providerFriendlyName, "OnRemoteFailure"); + + return oAuthOptions; + } + + private Task HandleResponseWithDefaultUmbracoRedirect(HandleRequestContext context, string providerFriendlyName, string eventName) + { + SetUmbracoRedirectWithFilteredParams(context, providerFriendlyName, eventName) + .HandleResponse(); + + return Task.FromResult(0); + } + + /// + /// Sets the context to redirect to the path with all parameters, except state, that are passed to the initial server callback configured for the configured external login provider + /// + public T SetUmbracoRedirectWithFilteredParams(T context, string providerFriendlyName, string eventName) + where T : HandleRequestContext + { + var callbackPath = _securitySettings.Value.AuthorizeCallbackErrorPathName; + + callbackPath = callbackPath.AppendQueryStringToUrl("flow=external-login") + .AppendQueryStringToUrl($"provider={providerFriendlyName}") + .AppendQueryStringToUrl($"callback-event={eventName}"); + + foreach (var oathCallbackErrorParam in _oathCallbackErrorParams) + { + if (context.Request.Query.ContainsKey(oathCallbackErrorParam)) + { + callbackPath = callbackPath.AppendQueryStringToUrl($"{oathCallbackErrorParam}={context.Request.Query[oathCallbackErrorParam]}"); + } + } + + context.Response.Redirect(callbackPath); + return context; + } + + /// + /// Sets the callbackPath for the RemoteAuthenticationOptions based on the configured Umbraco path and the path supplied. + /// By default this will result in "/umbraco/your-supplied-path". + /// + /// The options object to set the path on. + /// The path that should go after the umbraco path, will add a leading slash if it's missing. + /// + public RemoteAuthenticationOptions SetUmbracoBasedCallbackPath(RemoteAuthenticationOptions options, string path) + { + var umbracoCallbackPath = _securitySettings.Value.AuthorizeCallbackPathName; + + options.CallbackPath = umbracoCallbackPath + path.EnsureStartsWith("/"); + return options; + } +}