Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V14 External login linking + Proposed error handling #16052

Merged
merged 5 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
private readonly ILogger<BackOfficeController> _logger;
private readonly IBackOfficeTwoFactorOptions _backOfficeTwoFactorOptions;
private readonly IUserTwoFactorLoginService _userTwoFactorLoginService;
private readonly IBackOfficeExternalLoginProviders _backOfficeExternalLoginProviders;

public BackOfficeController(
IHttpContextAccessor httpContextAccessor,
Expand All @@ -47,15 +48,17 @@
IOptions<SecuritySettings> securitySettings,
ILogger<BackOfficeController> logger,
IBackOfficeTwoFactorOptions backOfficeTwoFactorOptions,
IUserTwoFactorLoginService userTwoFactorLoginService)
IUserTwoFactorLoginService userTwoFactorLoginService,
IBackOfficeExternalLoginProviders backOfficeExternalLoginProviders)
{
_httpContextAccessor = httpContextAccessor;
_backOfficeSignInManager = backOfficeSignInManager;
_backOfficeUserManager = backOfficeUserManager;
_securitySettings = securitySettings;
_logger = logger;
_backOfficeTwoFactorOptions = backOfficeTwoFactorOptions;
_userTwoFactorLoginService = userTwoFactorLoginService;
_backOfficeExternalLoginProviders = backOfficeExternalLoginProviders;

Check notice on line 61 in src/Umbraco.Cms.Api.Management/Controllers/Security/BackOfficeController.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v14/dev)

ℹ Getting worse: Constructor Over-Injection

BackOfficeController increases from 7 to 8 arguments, threshold = 5. This constructor has too many arguments, indicating an object with low cohesion or missing function argument abstraction. Avoid adding more arguments.
}

[HttpPost("login")]
Expand Down Expand Up @@ -184,6 +187,145 @@
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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)$" });
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
using System.ComponentModel.DataAnnotations;
using System.Runtime.Serialization;

namespace Umbraco.Cms.Api.Management.ViewModels.Security;

public class UnLinkLoginRequestModel
{
[Required]
[DataMember(Name = "loginProvider", IsRequired = true)]
Migaroez marked this conversation as resolved.
Show resolved Hide resolved
public required string LoginProvider { get; set; }

[Required]
[DataMember(Name = "providerKey", IsRequired = true)]
public required string ProviderKey { get; set; }
}
4 changes: 4 additions & 0 deletions src/Umbraco.Core/Configuration/Models/SecuritySettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -116,4 +117,7 @@ public class SecuritySettings
/// </summary>
[DefaultValue(StaticAuthorizeCallbackPathName)]
public string AuthorizeCallbackPathName { get; set; } = StaticAuthorizeCallbackPathName;

[DefaultValue(StaticAuthorizeCallbackErrorPathName)]
public string AuthorizeCallbackErrorPathName { get; set; } = StaticAuthorizeCallbackErrorPathName;
Migaroez marked this conversation as resolved.
Show resolved Hide resolved
}
80 changes: 80 additions & 0 deletions src/Umbraco.Web.Common/Extensions/OAuthOptionsExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Authentication.OAuth;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Extensions;

namespace Umbraco.Cms.Web.Common.Extensions;

public static class OAuthOptionsExtensions
{
// 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 IReadOnlyCollection<string> oathCallbackErrorParams = new string[] { "error", "error_description" };

/// <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 static T SetDefaultErrorEventHandling<T>(this 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 static Task HandleResponseWithDefaultUmbracoRedirect(HandleRequestContext<RemoteAuthenticationOptions> context, string providerFriendlyName, string eventName)
{
context.SetUmbracoRedirectWithFilteredParams(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 static T SetUmbracoRedirectWithFilteredParams<T>(this T context, string providerFriendlyName, string eventName)
where T : HandleRequestContext<RemoteAuthenticationOptions>
{
var callbackPath = StaticServiceProvider.Instance.GetRequiredService<IOptions<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);
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Show resolved Hide resolved
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 static RemoteAuthenticationOptions SetUmbracoBasedCallbackPath(this RemoteAuthenticationOptions options, string path)
{
var umbracoCallbackPath = StaticServiceProvider.Instance.GetRequiredService<IOptions<SecuritySettings>>().Value
bergmania marked this conversation as resolved.
Show resolved Hide resolved
.AuthorizeCallbackPathName;

options.CallbackPath = umbracoCallbackPath + path.EnsureStartsWith("/");
return options;
}
}
Loading