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

Port V13 backoffice cookie validation to V14 #15886

Merged
merged 2 commits into from
Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
@@ -0,0 +1,254 @@
using System.Security.Claims;
using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Authentication.Cookies;
using Microsoft.AspNetCore.DataProtection;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;
using Umbraco.Cms.Api.Management.Security;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.Net;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Web.Common.Security;
using Umbraco.Extensions;

namespace Umbraco.Cms.Api.Management.Configuration;

/// <summary>
/// Used to configure <see cref="CookieAuthenticationOptions" /> for the back office authentication type
/// </summary>
public class ConfigureBackOfficeCookieOptions : IConfigureNamedOptions<CookieAuthenticationOptions>
{
private readonly IDataProtectionProvider _dataProtection;
private readonly GlobalSettings _globalSettings;
private readonly IIpResolver _ipResolver;
private readonly IRuntimeState _runtimeState;
private readonly SecuritySettings _securitySettings;
private readonly IUserService _userService;
private readonly TimeProvider _timeProvider;

/// <summary>
/// Initializes a new instance of the <see cref="ConfigureBackOfficeCookieOptions" /> class.
/// </summary>
/// <param name="securitySettings">The <see cref="SecuritySettings" /> options</param>
/// <param name="globalSettings">The <see cref="GlobalSettings" /> options</param>
/// <param name="runtimeState">The <see cref="IRuntimeState" /></param>
/// <param name="dataProtection">The <see cref="IDataProtectionProvider" /></param>
/// <param name="userService">The <see cref="IUserService" /></param>
/// <param name="ipResolver">The <see cref="IIpResolver" /></param>
/// <param name="timeProvider">The <see cref="TimeProvider" /></param>
public ConfigureBackOfficeCookieOptions(
IOptions<SecuritySettings> securitySettings,
IOptions<GlobalSettings> globalSettings,
IRuntimeState runtimeState,
IDataProtectionProvider dataProtection,
IUserService userService,
IIpResolver ipResolver,
TimeProvider timeProvider)
{
_securitySettings = securitySettings.Value;
_globalSettings = globalSettings.Value;
_runtimeState = runtimeState;
_dataProtection = dataProtection;
_userService = userService;
_ipResolver = ipResolver;
_timeProvider = timeProvider;
}

Check warning on line 57 in src/Umbraco.Cms.Api.Management/Configuration/ConfigureBackOfficeCookieOptions.cs

View check run for this annotation

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

❌ New issue: Constructor Over-Injection

ConfigureBackOfficeCookieOptions has 7 arguments, threshold = 5. This constructor has too many arguments, indicating an object with low cohesion or missing function argument abstraction. Avoid adding more arguments.

/// <inheritdoc />
public void Configure(string? name, CookieAuthenticationOptions options)
{
if (name != Constants.Security.BackOfficeAuthenticationType)
{
return;
}

Configure(options);
}

/// <inheritdoc />
public void Configure(CookieAuthenticationOptions options)
{
options.SlidingExpiration = false;
options.ExpireTimeSpan = _globalSettings.TimeOut;
options.Cookie.Domain = _securitySettings.AuthCookieDomain;
options.Cookie.Name = _securitySettings.AuthCookieName;
options.Cookie.HttpOnly = true;
options.Cookie.SecurePolicy =
_globalSettings.UseHttps ? CookieSecurePolicy.Always : CookieSecurePolicy.SameAsRequest;
options.Cookie.Path = "/";

// NOTE: matches route in BackOfficeLoginController
const string backOfficeLoginPath = "/umbraco/login";
options.LoginPath = backOfficeLoginPath;
options.LogoutPath = backOfficeLoginPath;
options.AccessDeniedPath = backOfficeLoginPath;

options.DataProtectionProvider = _dataProtection;

// NOTE: This is borrowed directly from aspnetcore source
// Note: the purpose for the data protector must remain fixed for interop to work.
IDataProtector dataProtector = options.DataProtectionProvider.CreateProtector(
"Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationMiddleware",
Constants.Security.BackOfficeAuthenticationType,
"v2");
var ticketDataFormat = new TicketDataFormat(dataProtector);

options.TicketDataFormat = new BackOfficeSecureDataFormat(_globalSettings.TimeOut, ticketDataFormat);

options.Events = new CookieAuthenticationEvents
{
// IMPORTANT! If you set any of OnRedirectToLogin, OnRedirectToAccessDenied, OnRedirectToLogout, OnRedirectToReturnUrl
// you need to be aware that this will bypass the default behavior of returning the correct status codes for ajax requests and
// not redirecting for non-ajax requests. This is because the default behavior is baked into this class here:
// https://github.com/dotnet/aspnetcore/blob/master/src/Security/Authentication/Cookies/src/CookieAuthenticationEvents.cs#L58
// It would be possible to re-use the default behavior if any of these need to be set but that must be taken into account else
// our back office requests will not function correctly. For now we don't need to set/configure any of these callbacks because
// the defaults work fine with our setup.
OnValidatePrincipal = async ctx =>
{
// We need to resolve the BackOfficeSecurityStampValidator per request as a requirement (even in aspnetcore they do this)
BackOfficeSecurityStampValidator securityStampValidator =
ctx.HttpContext.RequestServices.GetRequiredService<BackOfficeSecurityStampValidator>();

// Same goes for the signinmanager
IBackOfficeSignInManager signInManager =
ctx.HttpContext.RequestServices.GetRequiredService<IBackOfficeSignInManager>();

ClaimsIdentity? backOfficeIdentity = ctx.Principal?.GetUmbracoIdentity();
if (backOfficeIdentity == null)
{
ctx.RejectPrincipal();
await signInManager.SignOutAsync();
}

// ensure the thread culture is set
backOfficeIdentity?.EnsureCulture();

EnsureTicketRenewalIfKeepUserLoggedIn(ctx);

// add or update a claim to track when the cookie expires, we use this to track time remaining
backOfficeIdentity?.AddOrUpdateClaim(new Claim(
Constants.Security.TicketExpiresClaimType,
ctx.Properties.ExpiresUtc!.Value.ToString("o"),
ClaimValueTypes.DateTime,
Constants.Security.BackOfficeAuthenticationType,
Constants.Security.BackOfficeAuthenticationType,
backOfficeIdentity));

await securityStampValidator.ValidateAsync(ctx);

// We have to manually specify Issued and Expires,
// because the SecurityStampValidator refreshes the principal every 30 minutes,
// When the principal is refreshed the Issued is update to time of refresh, however, the Expires remains unchanged
// When we then try and renew, the difference of issued and expires effectively becomes the new ExpireTimeSpan
// meaning we effectively lose 30 minutes of our ExpireTimeSpan for EVERY principal refresh if we don't
// https://github.com/dotnet/aspnetcore/blob/main/src/Security/Authentication/Cookies/src/CookieAuthenticationHandler.cs#L115
ctx.Properties.IssuedUtc = _timeProvider.GetUtcNow();
ctx.Properties.ExpiresUtc = _timeProvider.GetUtcNow().Add(_globalSettings.TimeOut);
ctx.ShouldRenew = true;
},
OnSigningIn = ctx =>
{
// occurs when sign in is successful but before the ticket is written to the outbound cookie
ClaimsIdentity? backOfficeIdentity = ctx.Principal?.GetUmbracoIdentity();
if (backOfficeIdentity != null)
{
// generate a session id and assign it
// create a session token - if we are configured and not in an upgrade state then use the db, otherwise just generate one
Guid session = _runtimeState.Level == RuntimeLevel.Run
? _userService.CreateLoginSession(
backOfficeIdentity.GetId()!.Value,
_ipResolver.GetCurrentRequestIpAddress())
: Guid.NewGuid();

// add our session claim
backOfficeIdentity.AddClaim(new Claim(
Constants.Security.SessionIdClaimType,
session.ToString(),
ClaimValueTypes.String,
Constants.Security.BackOfficeAuthenticationType,
Constants.Security.BackOfficeAuthenticationType,
backOfficeIdentity));

// since it is a cookie-based authentication add that claim
backOfficeIdentity.AddClaim(new Claim(
ClaimTypes.CookiePath,
"/",
ClaimValueTypes.String,
Constants.Security.BackOfficeAuthenticationType,
Constants.Security.BackOfficeAuthenticationType,
backOfficeIdentity));
}

return Task.CompletedTask;
},
OnSignedIn = ctx =>
{
// occurs when sign in is successful and after the ticket is written to the outbound cookie

// When we are signed in with the cookie, assign the principal to the current HttpContext
ctx.HttpContext.SetPrincipalForRequest(ctx.Principal);

return Task.CompletedTask;
},
OnSigningOut = ctx =>
{
// Clear the user's session on sign out
if (ctx.HttpContext?.User?.Identity != null)
{
var claimsIdentity = ctx.HttpContext.User.Identity as ClaimsIdentity;
var sessionId = claimsIdentity?.FindFirstValue(Constants.Security.SessionIdClaimType);
if (sessionId.IsNullOrWhiteSpace() == false && Guid.TryParse(sessionId, out Guid guidSession))
{
_userService.ClearLoginSession(guidSession);
}
}

// Remove all of our cookies
var cookies = new[]
{
_securitySettings.AuthCookieName,
Constants.Web.PreviewCookieName, Constants.Security.BackOfficeExternalCookieName,
Constants.Web.AngularCookieName, Constants.Web.CsrfValidationCookieName
};
foreach (var cookie in cookies)
{
ctx.Options.CookieManager.DeleteCookie(ctx.HttpContext!, cookie, new CookieOptions { Path = "/" });
}

return Task.CompletedTask;
}
};
}

Check warning on line 224 in src/Umbraco.Cms.Api.Management/Configuration/ConfigureBackOfficeCookieOptions.cs

View check run for this annotation

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

❌ New issue: Complex Method

Configure has a cyclomatic complexity of 9, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

/// <summary>
/// Ensures the ticket is renewed if the <see cref="SecuritySettings.KeepUserLoggedIn" /> is set to true
/// and the current request is for the get user seconds endpoint
/// </summary>
/// <param name="context">The <see cref="CookieValidatePrincipalContext" /></param>
private void EnsureTicketRenewalIfKeepUserLoggedIn(CookieValidatePrincipalContext context)
{
if (!_securitySettings.KeepUserLoggedIn)
{
return;
}

DateTimeOffset currentUtc = _timeProvider.GetUtcNow();
DateTimeOffset? issuedUtc = context.Properties.IssuedUtc;
DateTimeOffset? expiresUtc = context.Properties.ExpiresUtc;

if (expiresUtc.HasValue && issuedUtc.HasValue)
{
TimeSpan timeElapsed = currentUtc.Subtract(issuedUtc.Value);
TimeSpan timeRemaining = expiresUtc.Value.Subtract(currentUtc);

// if it's time to renew, then do it
if (timeRemaining < timeElapsed)
{
context.ShouldRenew = true;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
using Microsoft.Extensions.Options;
using Umbraco.Cms.Api.Management.Security;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Web.Common.Security;

namespace Umbraco.Cms.Api.Management.Configuration;

/// <summary>
/// Configures the back office security stamp options.
/// </summary>
public class ConfigureBackOfficeSecurityStampValidatorOptions : IConfigureOptions<BackOfficeSecurityStampValidatorOptions>
{
private readonly SecuritySettings _securitySettings;
private readonly TimeProvider _timeProvider;

public ConfigureBackOfficeSecurityStampValidatorOptions(IOptions<SecuritySettings> securitySettings, TimeProvider timeProvider)
{
_timeProvider = timeProvider;
_securitySettings = securitySettings.Value;
}

/// <inheritdoc />
public void Configure(BackOfficeSecurityStampValidatorOptions options)
{
options.TimeProvider = _timeProvider;
ConfigureSecurityStampOptions.ConfigureOptions(options, _securitySettings);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Microsoft.AspNetCore.Builder;
using Microsoft.Extensions.DependencyInjection;
using Umbraco.Cms.Api.Common.DependencyInjection;
using Umbraco.Cms.Api.Management.Configuration;
using Umbraco.Cms.Api.Management.Handlers;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.DependencyInjection;
Expand Down Expand Up @@ -52,11 +53,7 @@ private static IUmbracoBuilder AddBackOfficeLogin(this IUmbracoBuilder builder)
builder.Services
.AddAuthentication()
// Add our custom schemes which are cookie handlers
.AddCookie(Constants.Security.BackOfficeAuthenticationType, options =>
{
options.LoginPath = "/umbraco/login";
options.Cookie.Name = Constants.Security.BackOfficeAuthenticationType;
})
.AddCookie(Constants.Security.BackOfficeAuthenticationType)
.AddCookie(Constants.Security.BackOfficeExternalAuthenticationType, o =>
{
o.Cookie.Name = Constants.Security.BackOfficeExternalAuthenticationType;
Expand All @@ -76,6 +73,10 @@ private static IUmbracoBuilder AddBackOfficeLogin(this IUmbracoBuilder builder)
o.ExpireTimeSpan = TimeSpan.FromMinutes(5);
});

builder.Services.AddScoped<BackOfficeSecurityStampValidator>();
builder.Services.ConfigureOptions<ConfigureBackOfficeCookieOptions>();
builder.Services.ConfigureOptions<ConfigureBackOfficeSecurityStampValidatorOptions>();

return builder;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
using Microsoft.AspNetCore.Identity;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core.Security;

namespace Umbraco.Cms.Api.Management.Security;

/// <summary>
/// A security stamp validator for the back office
/// </summary>
public class BackOfficeSecurityStampValidator : SecurityStampValidator<BackOfficeIdentityUser>
{
public BackOfficeSecurityStampValidator(
IOptions<BackOfficeSecurityStampValidatorOptions> options,
BackOfficeSignInManager signInManager,
ILoggerFactory logger)
: base(options, signInManager, logger)
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
using Microsoft.AspNetCore.Identity;

namespace Umbraco.Cms.Api.Management.Security;

/// <summary>
/// Custom <see cref="SecurityStampValidatorOptions" /> for the back office
/// </summary>
public class BackOfficeSecurityStampValidatorOptions : SecurityStampValidatorOptions
{
}
Loading