V14 External login linking + Proposed error handling (#16052)

Migaroez · web-flow · commit 6dd08b23777f · 2024-04-16T17:08:55.000+02:00
* 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 &lt;sge@umbraco.dk&gt;
diff --git 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<BackOfficeController> _logger;
     private readonly IBackOfficeTwoFactorOptions _backOfficeTwoFactorOptions;
     private readonly IUserTwoFactorLoginService _userTwoFactorLoginService;
+    private readonly IBackOfficeExternalLoginProviders _backOfficeExternalLoginProviders;
 
     public BackOfficeController(
         IHttpContextAccessor httpContextAccessor,
@@ -47,7 +48,8 @@ public BackOfficeController(
         IOptions<SecuritySettings> securitySettings,
         ILogger<BackOfficeController> logger,
         IBackOfficeTwoFactorOptions backOfficeTwoFactorOptions,
-        IUserTwoFactorLoginService userTwoFactorLoginService)
+        IUserTwoFactorLoginService userTwoFactorLoginService,
+        IBackOfficeExternalLoginProviders backOfficeExternalLoginProviders)
     {
         _httpContextAccessor = httpContextAccessor;
         _backOfficeSignInManager = backOfficeSignInManager;
@@ -56,6 +58,7 @@ public BackOfficeController(
         _logger = logger;
         _backOfficeTwoFactorOptions = backOfficeTwoFactorOptions;
         _userTwoFactorLoginService = userTwoFactorLoginService;
+        _backOfficeExternalLoginProviders = backOfficeExternalLoginProviders;
     }
 
     [HttpPost("login")]
@@ -184,6 +187,145 @@ public async Task<IActionResult> Signout(CancellationToken cancellationToken)
         return SignOut(Constants.Security.BackOfficeAuthenticationType, OpenIddictServerAspNetCoreDefaults.AuthenticationScheme);
     }
 
+    /// <summary>
+    ///     Called when a user links an external login provider in the back office
+    /// </summary>
+    /// <param name="provider"></param>
+    /// <returns></returns>
+    [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);
+    }
+
+    /// <summary>
+    ///     Callback path when the user initiates a link login request from the back office to the external provider from the
+    ///     <see cref="LinkLogin(string)" /> action
+    /// </summary>
+    /// <remarks>
+    ///     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
+    /// </remarks>
+    [HttpGet("ExternalLinkLoginCallback")]
+    [AllowAnonymous]
+    [MapToApiVersion("1.0")]
+    public async Task<IActionResult> 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<IActionResult> 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());
+    }
+
     /// <summary>
     ///     Retrieve the user principal stored in the authentication cookie.
     /// </summary>
diff --git 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 IUmbracoBuilder
         .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
@@ -82,6 +82,6 @@ private void MapMinimalBackOffice(IEndpointRouteBuilder endpoints)
                 Controller = ControllerExtensions.GetControllerName<BackOfficeDefaultController>(),
                 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
@@ -20,7 +20,7 @@ public BackOfficeAuthenticationBuilder(
         : base(services)
         => _loginProviderOptions = loginProviderOptions ?? (x => { });
 
-    public string? SchemeForBackOffice(string scheme)
+    public static string? SchemeForBackOffice(string scheme)
         => scheme?.EnsureStartsWith(Constants.Security.BackOfficeExternalAuthenticationTypePrefix);
 
     /// <summary>
diff --git a/src/Umbraco.Cms.Api.Management/ViewModels/Security/UnLinkLoginRequestModel.cs 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
@@ -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";
 
     /// <summary>
     ///     Gets or sets a value indicating whether to keep the user logged in.
@@ -116,4 +117,7 @@ public class SecuritySettings
     /// </summary>
     [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
@@ -46,6 +46,7 @@
 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 IUmbracoBuilder AddWebComponents(this IUmbracoBuilder builder)
         return builder;
     }
 
+    public static IUmbracoBuilder AddHelpers(this IUmbracoBuilder builder)
+    {
+        builder.Services.AddSingleton<OAuthOptionsHelper>();
+
+        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
@@ -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<string> _oathCallbackErrorParams = new string[] { "error", "error_description" };
+
+    private readonly IOptions<SecuritySettings> _securitySettings;
+
+    public OAuthOptionsHelper(IOptions<SecuritySettings> securitySettings)
+    {
+        _securitySettings = securitySettings;
+    }
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    public T SetDefaultErrorEventHandling<T>(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<RemoteAuthenticationOptions> context, string providerFriendlyName, string eventName)
+    {
+        SetUmbracoRedirectWithFilteredParams(context, providerFriendlyName, eventName)
+            .HandleResponse();
+
+        return Task.FromResult(0);
+    }
+
+    /// <summary>
+    /// Sets the context to redirect to the <see cref="SecuritySettings.AuthorizeCallbackErrorPathName"/> path with all parameters, except state, that are passed to the initial server callback configured for the configured external login provider
+    /// </summary>
+    public T SetUmbracoRedirectWithFilteredParams<T>(T context, string providerFriendlyName, string eventName)
+        where T : HandleRequestContext<RemoteAuthenticationOptions>
+    {
+        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;
+    }
+
+    /// <summary>
+    /// 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".
+    /// </summary>
+    /// <param name="options">The options object to set the path on.</param>
+    /// <param name="path">The path that should go after the umbraco path, will add a leading slash if it's missing.</param>
+    /// <returns></returns>
+    public RemoteAuthenticationOptions SetUmbracoBasedCallbackPath(RemoteAuthenticationOptions options, string path)
+    {
+        var umbracoCallbackPath = _securitySettings.Value.AuthorizeCallbackPathName;
+
+        options.CallbackPath = umbracoCallbackPath + path.EnsureStartsWith("/");
+        return options;
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -82,6 +82,6 @@ private void MapMinimalBackOffice(IEndpointRouteBuilder endpoints)`
`82`	`82`	`Controller = ControllerExtensions.GetControllerName<BackOfficeDefaultController>(),`
`83`	`83`	`Action = nameof(BackOfficeDefaultController.Index),`
`84`	`84`	`},`
`85`		`- constraints: new { slug = @"^(section.*\|upgrade\|install\|logout)$" });`
	`85`	`+ constraints: new { slug = @"^(section.*\|upgrade\|install\|logout\|error)$" });`
`86`	`86`	`}`
`87`	`87`	`}`