From b75fba71f5877092757ed0ff135e7fc9b3e87c84 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 2 Jun 2020 13:28:30 +1000 Subject: [PATCH 01/14] Cleaning up websecurity and implementing it, migrates security stamp and session id validation for cookie auth --- .../Security/AuthenticationExtensions.cs | 33 +++ src/Umbraco.Core/Security/IWebSecurity.cs | 6 - .../UmbracoSecurityStampValidatorTests.cs | 278 ------------------ src/Umbraco.Tests/Umbraco.Tests.csproj | 1 - .../Controllers/AuthenticationController.cs | 2 +- .../Controllers/BackOfficeController.cs | 5 +- .../HtmlHelperBackOfficeExtensions.cs | 2 +- ...coBackOfficeServiceCollectionExtensions.cs | 8 +- .../Filters/OnlyLocalRequestsAttribute.cs | 3 +- .../Runtime/BackOfficeComposer.cs | 2 + ...ormat.cs => BackOfficeSecureDataFormat.cs} | 4 +- .../BackOfficeSecurityStampValidator.cs | 27 ++ ...BackOfficeSecurityStampValidatorOptions.cs | 13 + .../Security/BackOfficeSessionIdValidator.cs | 134 +++++++++ .../ConfigureBackOfficeCookieOptions.cs | 185 ++++++++++++ ... => ConfigureBackOfficeIdentityOptions.cs} | 4 +- ...BackOfficeSecurityStampValidatorOptions.cs | 18 ++ ...ConfigureUmbracoBackOfficeCookieOptions.cs | 94 ------ .../Extensions/HttpRequestExtensions.cs | 19 +- .../UmbracoRequestLoggingMiddleware.cs | 3 +- .../HttpQueryStringModelBinder.cs | 2 +- .../Profiler/WebProfiler.cs | 3 +- .../Security/BackOfficeSignInManager.cs | 25 +- .../Security/WebSecurity.cs | 16 +- .../UmbracoContext/UmbracoContextFactory.cs | 10 +- .../Views/BackOffice/Default.cshtml | 2 +- .../Security/AppBuilderExtensions.cs | 13 - .../Security/AuthenticationExtensions.cs | 121 +------- .../BackOfficeCookieAuthenticationProvider.cs | 90 +----- .../Security/UmbracoSecurityStampValidator.cs | 88 ------ src/Umbraco.Web/Security/WebSecurity.cs | 32 +- src/Umbraco.Web/Umbraco.Web.csproj | 1 - src/Umbraco.Web/UmbracoInjectedModule.cs | 1 + 33 files changed, 489 insertions(+), 756 deletions(-) create mode 100644 src/Umbraco.Core/Security/AuthenticationExtensions.cs delete mode 100644 src/Umbraco.Tests/Security/UmbracoSecurityStampValidatorTests.cs rename src/Umbraco.Web.BackOffice/Security/{UmbracoSecureDataFormat.cs => BackOfficeSecureDataFormat.cs} (93%) create mode 100644 src/Umbraco.Web.BackOffice/Security/BackOfficeSecurityStampValidator.cs create mode 100644 src/Umbraco.Web.BackOffice/Security/BackOfficeSecurityStampValidatorOptions.cs create mode 100644 src/Umbraco.Web.BackOffice/Security/BackOfficeSessionIdValidator.cs create mode 100644 src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs rename src/Umbraco.Web.BackOffice/Security/{ConfigureUmbracoBackOfficeIdentityOptions.cs => ConfigureBackOfficeIdentityOptions.cs} (89%) create mode 100644 src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeSecurityStampValidatorOptions.cs delete mode 100644 src/Umbraco.Web.BackOffice/Security/ConfigureUmbracoBackOfficeCookieOptions.cs rename src/{Umbraco.Web.BackOffice => Umbraco.Web.Common}/Security/BackOfficeSignInManager.cs (90%) delete mode 100644 src/Umbraco.Web/Security/UmbracoSecurityStampValidator.cs diff --git a/src/Umbraco.Core/Security/AuthenticationExtensions.cs b/src/Umbraco.Core/Security/AuthenticationExtensions.cs new file mode 100644 index 0000000000..d0b4416eed --- /dev/null +++ b/src/Umbraco.Core/Security/AuthenticationExtensions.cs @@ -0,0 +1,33 @@ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Globalization; +using System.Security.Principal; +using System.Text; +using System.Threading; +using Umbraco.Core.BackOffice; + +namespace Umbraco.Core.Security +{ + public static class AuthenticationExtensions + { + /// + /// Ensures that the thread culture is set based on the back office user's culture + /// + /// + public static void EnsureCulture(this IIdentity identity) + { + if (identity is UmbracoBackOfficeIdentity umbIdentity && umbIdentity.IsAuthenticated) + { + Thread.CurrentThread.CurrentUICulture = + Thread.CurrentThread.CurrentCulture = UserCultures.GetOrAdd(umbIdentity.Culture, s => new CultureInfo(s)); + } + } + + + /// + /// Used so that we aren't creating a new CultureInfo object for every single request + /// + private static readonly ConcurrentDictionary UserCultures = new ConcurrentDictionary(); + } +} diff --git a/src/Umbraco.Core/Security/IWebSecurity.cs b/src/Umbraco.Core/Security/IWebSecurity.cs index 0822b5cb69..594f6d96ea 100644 --- a/src/Umbraco.Core/Security/IWebSecurity.cs +++ b/src/Umbraco.Core/Security/IWebSecurity.cs @@ -12,12 +12,6 @@ namespace Umbraco.Web.Security /// The current user. IUser CurrentUser { get; } - [Obsolete("This needs to be removed, ASP.NET Identity should always be used for this operation, this is currently only used in the installer which needs to be updated")] - double PerformLogin(int userId); - - [Obsolete("This needs to be removed, ASP.NET Identity should always be used for this operation, this is currently only used in the installer which needs to be updated")] - void ClearCurrentLogin(); - /// /// Gets the current user's id. /// diff --git a/src/Umbraco.Tests/Security/UmbracoSecurityStampValidatorTests.cs b/src/Umbraco.Tests/Security/UmbracoSecurityStampValidatorTests.cs deleted file mode 100644 index b80e526037..0000000000 --- a/src/Umbraco.Tests/Security/UmbracoSecurityStampValidatorTests.cs +++ /dev/null @@ -1,278 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Security.Claims; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Identity; -using Microsoft.Owin; -using Microsoft.Owin.Logging; -using Microsoft.Owin.Security; -using Microsoft.Owin.Security.Cookies; -using Moq; -using NUnit.Framework; -using Umbraco.Core; -using Umbraco.Core.BackOffice; -using Umbraco.Core.Configuration; -using Umbraco.Core.Models.Membership; -using Umbraco.Net; -using Umbraco.Web.Security; - - -namespace Umbraco.Tests.Security -{ - public class UmbracoSecurityStampValidatorTests - { - private Mock _mockOwinContext; - private Mock _mockUserManager; - private Mock _mockSignInManager; - - private AuthenticationTicket _testAuthTicket; - private CookieAuthenticationOptions _testOptions; - private BackOfficeIdentityUser _testUser; - private const string _testAuthType = "cookie"; - - [Test] - public void OnValidateIdentity_When_GetUserIdCallback_Is_Null_Expect_ArgumentNullException() - { - Assert.Throws(() => UmbracoSecurityStampValidator - .OnValidateIdentity( - TimeSpan.MaxValue, null, null)); - } - - [Test] - public async Task OnValidateIdentity_When_Validation_Interval_Not_Met_Expect_No_Op() - { - var func = UmbracoSecurityStampValidator - .OnValidateIdentity( - TimeSpan.MaxValue, null, identity => throw new Exception()); - - _testAuthTicket.Properties.IssuedUtc = DateTimeOffset.UtcNow; - - var context = new CookieValidateIdentityContext( - _mockOwinContext.Object, - _testAuthTicket, - _testOptions); - - await func(context); - - Assert.AreEqual(_testAuthTicket.Identity, context.Identity); - } - - [Test] - public void OnValidateIdentity_When_Time_To_Validate_But_No_UserManager_Expect_InvalidOperationException() - { - var func = UmbracoSecurityStampValidator - .OnValidateIdentity( - TimeSpan.MinValue, null, identity => throw new Exception()); - - _mockOwinContext.Setup(x => x.Get(It.IsAny())) - .Returns((BackOfficeOwinUserManager) null); - - var context = new CookieValidateIdentityContext( - _mockOwinContext.Object, - _testAuthTicket, - _testOptions); - - Assert.ThrowsAsync(async () => await func(context)); - } - - [Test] - public void OnValidateIdentity_When_Time_To_Validate_But_No_SignInManager_Expect_InvalidOperationException() - { - var func = UmbracoSecurityStampValidator - .OnValidateIdentity( - TimeSpan.MinValue, null, identity => throw new Exception()); - - _mockOwinContext.Setup(x => x.Get(It.IsAny())) - .Returns((BackOfficeSignInManager) null); - - var context = new CookieValidateIdentityContext( - _mockOwinContext.Object, - _testAuthTicket, - _testOptions); - - Assert.ThrowsAsync(async () => await func(context)); - } - - [Test] - public async Task OnValidateIdentity_When_Time_To_Validate_And_User_No_Longer_Found_Expect_Rejected() - { - var userId = Guid.NewGuid().ToString(); - - var func = UmbracoSecurityStampValidator - .OnValidateIdentity( - TimeSpan.MinValue, null, identity => userId); - - _mockUserManager.Setup(x => x.FindByIdAsync(userId)) - .ReturnsAsync((BackOfficeIdentityUser) null); - - var context = new CookieValidateIdentityContext( - _mockOwinContext.Object, - _testAuthTicket, - _testOptions); - - await func(context); - - Assert.IsNull(context.Identity); - _mockOwinContext.Verify(x => x.Authentication.SignOut(_testAuthType), Times.Once); - } - - [Test] - public async Task OnValidateIdentity_When_Time_To_Validate_And_User_Exists_And_Does_Not_Support_SecurityStamps_Expect_Rejected() - { - var userId = Guid.NewGuid().ToString(); - - var func = UmbracoSecurityStampValidator - .OnValidateIdentity( - TimeSpan.MinValue, null, identity => userId); - - _mockUserManager.Setup(x => x.FindByIdAsync(userId)).ReturnsAsync(_testUser); - _mockUserManager.Setup(x => x.SupportsUserSecurityStamp).Returns(false); - - var context = new CookieValidateIdentityContext( - _mockOwinContext.Object, - _testAuthTicket, - _testOptions); - - await func(context); - - Assert.IsNull(context.Identity); - _mockOwinContext.Verify(x => x.Authentication.SignOut(_testAuthType), Times.Once); - } - - [Test] - public async Task OnValidateIdentity_When_Time_To_Validate_And_SecurityStamp_Has_Changed_Expect_Rejected() - { - var userId = Guid.NewGuid().ToString(); - - var func = UmbracoSecurityStampValidator - .OnValidateIdentity( - TimeSpan.MinValue, null, identity => userId); - - _mockUserManager.Setup(x => x.FindByIdAsync(userId)).ReturnsAsync(_testUser); - _mockUserManager.Setup(x => x.SupportsUserSecurityStamp).Returns(true); - _mockUserManager.Setup(x => x.GetSecurityStampAsync(_testUser)).ReturnsAsync(Guid.NewGuid().ToString); - - var context = new CookieValidateIdentityContext( - _mockOwinContext.Object, - _testAuthTicket, - _testOptions); - - await func(context); - - Assert.IsNull(context.Identity); - _mockOwinContext.Verify(x => x.Authentication.SignOut(_testAuthType), Times.Once); - } - - [Test] - public async Task OnValidateIdentity_When_Time_To_Validate_And_SecurityStamp_Has_Not_Changed_Expect_No_Change() - { - var userId = Guid.NewGuid().ToString(); - - var func = UmbracoSecurityStampValidator - .OnValidateIdentity( - TimeSpan.MinValue, null, identity => userId); - - _mockUserManager.Setup(x => x.FindByIdAsync(userId)).ReturnsAsync(_testUser); - _mockUserManager.Setup(x => x.SupportsUserSecurityStamp).Returns(true); - _mockUserManager.Setup(x => x.GetSecurityStampAsync(_testUser)).ReturnsAsync(_testUser.SecurityStamp); - - var context = new CookieValidateIdentityContext( - _mockOwinContext.Object, - _testAuthTicket, - _testOptions); - - await func(context); - - Assert.AreEqual(_testAuthTicket.Identity, context.Identity); - } - - [Test] - public async Task OnValidateIdentity_When_User_Validated_And_RegenerateIdentityCallback_Present_Expect_User_Refreshed() - { - var userId = Guid.NewGuid().ToString(); - var expectedIdentity = new ClaimsIdentity(new List {new Claim("sub", "bob")}); - - var regenFuncCalled = false; - Func> regenFunc = - (signInManager, userManager, user) => - { - regenFuncCalled = true; - return Task.FromResult(expectedIdentity); - }; - - var func = UmbracoSecurityStampValidator - .OnValidateIdentity( - TimeSpan.MinValue, regenFunc, identity => userId); - - _mockUserManager.Setup(x => x.FindByIdAsync(userId)).ReturnsAsync(_testUser); - _mockUserManager.Setup(x => x.SupportsUserSecurityStamp).Returns(true); - _mockUserManager.Setup(x => x.GetSecurityStampAsync(_testUser)).ReturnsAsync(_testUser.SecurityStamp); - - var context = new CookieValidateIdentityContext( - _mockOwinContext.Object, - _testAuthTicket, - _testOptions); - - ClaimsIdentity callbackIdentity = null; - _mockOwinContext.Setup(x => x.Authentication.SignIn(context.Properties, It.IsAny())) - .Callback((AuthenticationProperties props, ClaimsIdentity[] identities) => callbackIdentity = identities.FirstOrDefault()) - .Verifiable(); - - await func(context); - - Assert.True(regenFuncCalled); - Assert.AreEqual(expectedIdentity, callbackIdentity); - Assert.IsNull(context.Properties.IssuedUtc); - Assert.IsNull(context.Properties.ExpiresUtc); - - _mockOwinContext.Verify(); - } - - [SetUp] - public void Setup() - { - var mockGlobalSettings = new Mock(); - mockGlobalSettings.Setup(x => x.DefaultUILanguage).Returns("test"); - - _testUser = new BackOfficeIdentityUser(mockGlobalSettings.Object, 2, new List()) - { - UserName = "alice", - Name = "Alice", - Email = "alice@umbraco.test", - SecurityStamp = Guid.NewGuid().ToString() - }; - - _testAuthTicket = new AuthenticationTicket( - new ClaimsIdentity( - new List {new Claim("sub", "alice"), new Claim(Constants.Web.SecurityStampClaimType, _testUser.SecurityStamp)}, - _testAuthType), - new AuthenticationProperties()); - _testOptions = new CookieAuthenticationOptions { AuthenticationType = _testAuthType }; - - _mockUserManager = new Mock( - new Mock().Object, - new Mock().Object, - new Mock>().Object, - null, null, null, null, null, null, null); - _mockUserManager.Setup(x => x.FindByIdAsync(It.IsAny())).ReturnsAsync((BackOfficeIdentityUser) null); - _mockUserManager.Setup(x => x.SupportsUserSecurityStamp).Returns(false); - - _mockSignInManager = new Mock( - _mockUserManager.Object, - new Mock>().Object, - new Mock().Object, - new Mock().Object, - new Mock().Object, - new Mock().Object); - - _mockOwinContext = new Mock(); - _mockOwinContext.Setup(x => x.Get(It.IsAny())) - .Returns(_mockUserManager.Object); - _mockOwinContext.Setup(x => x.Get(It.IsAny())) - .Returns(_mockSignInManager.Object); - - _mockOwinContext.Setup(x => x.Authentication.SignOut(It.IsAny())); - } - } -} diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 0a3787aa8a..56212d3169 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -149,7 +149,6 @@ - diff --git a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs index 3a3c936cbe..f501fb1579 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs @@ -11,11 +11,11 @@ using Umbraco.Core.Mapping; using Umbraco.Core.Models.Membership; using Umbraco.Core.Services; using Umbraco.Extensions; -using Umbraco.Web.BackOffice.Security; using Umbraco.Web.Common.Attributes; using Umbraco.Web.Common.Controllers; using Umbraco.Web.Common.Exceptions; using Umbraco.Web.Common.Filters; +using Umbraco.Web.Common.Security; using Umbraco.Web.Models; using Umbraco.Web.Models.ContentEditing; using Umbraco.Web.Security; diff --git a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs index 3a22da3a04..4f7a15be4f 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs @@ -16,9 +16,9 @@ using Umbraco.Core.Services; using Umbraco.Core.WebAssets; using Umbraco.Extensions; using Umbraco.Web.BackOffice.Filters; -using Umbraco.Web.BackOffice.Security; using Umbraco.Web.Common.ActionResults; using Umbraco.Web.Common.Attributes; +using Umbraco.Web.Common.Security; using Umbraco.Web.Models; using Umbraco.Web.WebAssets; using Constants = Umbraco.Core.Constants; @@ -50,8 +50,7 @@ namespace Umbraco.Web.BackOffice.Controllers IGridConfig gridConfig, BackOfficeServerVariables backOfficeServerVariables, AppCaches appCaches, - BackOfficeSignInManager signInManager // TODO: Review this, do we want it/need it or create our own? - ) + BackOfficeSignInManager signInManager) { _userManager = userManager; _runtimeMinifier = runtimeMinifier; diff --git a/src/Umbraco.Web.BackOffice/Extensions/HtmlHelperBackOfficeExtensions.cs b/src/Umbraco.Web.BackOffice/Extensions/HtmlHelperBackOfficeExtensions.cs index 4be24ee4a4..ad51b1b543 100644 --- a/src/Umbraco.Web.BackOffice/Extensions/HtmlHelperBackOfficeExtensions.cs +++ b/src/Umbraco.Web.BackOffice/Extensions/HtmlHelperBackOfficeExtensions.cs @@ -13,7 +13,7 @@ using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.Hosting; using Umbraco.Core.WebAssets; using Umbraco.Web.BackOffice.Controllers; -using Umbraco.Web.BackOffice.Security; +using Umbraco.Web.Common.Security; using Umbraco.Web.Features; using Umbraco.Web.Models; using Umbraco.Web.WebApi; diff --git a/src/Umbraco.Web.BackOffice/Extensions/UmbracoBackOfficeServiceCollectionExtensions.cs b/src/Umbraco.Web.BackOffice/Extensions/UmbracoBackOfficeServiceCollectionExtensions.cs index a0704da14c..ef5a4bfa5f 100644 --- a/src/Umbraco.Web.BackOffice/Extensions/UmbracoBackOfficeServiceCollectionExtensions.cs +++ b/src/Umbraco.Web.BackOffice/Extensions/UmbracoBackOfficeServiceCollectionExtensions.cs @@ -9,6 +9,7 @@ using Umbraco.Core.Serialization; using Umbraco.Net; using Umbraco.Web.BackOffice.Security; using Umbraco.Web.Common.AspNetCore; +using Umbraco.Web.Common.Security; namespace Umbraco.Extensions { @@ -25,8 +26,9 @@ namespace Umbraco.Extensions services .AddAuthentication(Constants.Security.BackOfficeAuthenticationType) .AddCookie(Constants.Security.BackOfficeAuthenticationType); + // TODO: Need to add more cookie options, see https://github.com/dotnet/aspnetcore/blob/3.0/src/Identity/Core/src/IdentityServiceCollectionExtensions.cs#L45 - services.ConfigureOptions(); + services.ConfigureOptions(); } /// @@ -47,8 +49,8 @@ namespace Umbraco.Extensions .AddClaimsPrincipalFactory>(); // Configure the options specifically for the UmbracoBackOfficeIdentityOptions instance - services.ConfigureOptions(); - //services.TryAddScoped>(); + services.ConfigureOptions(); + services.ConfigureOptions(); } private static IdentityBuilder BuildUmbracoBackOfficeIdentity(this IServiceCollection services) diff --git a/src/Umbraco.Web.BackOffice/Filters/OnlyLocalRequestsAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/OnlyLocalRequestsAttribute.cs index 9ee289a39c..32d8e9d543 100644 --- a/src/Umbraco.Web.BackOffice/Filters/OnlyLocalRequestsAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/OnlyLocalRequestsAttribute.cs @@ -1,7 +1,6 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; -using Microsoft.Extensions.DependencyInjection; -using Umbraco.Web.Common.Extensions; +using Umbraco.Extensions; namespace Umbraco.Web.BackOffice.Filters { diff --git a/src/Umbraco.Web.BackOffice/Runtime/BackOfficeComposer.cs b/src/Umbraco.Web.BackOffice/Runtime/BackOfficeComposer.cs index b13cb4a192..2d6a46d903 100644 --- a/src/Umbraco.Web.BackOffice/Runtime/BackOfficeComposer.cs +++ b/src/Umbraco.Web.BackOffice/Runtime/BackOfficeComposer.cs @@ -16,6 +16,8 @@ namespace Umbraco.Web.BackOffice.Runtime { composition.RegisterUnique(); composition.RegisterUnique(); + composition.RegisterUnique(); + composition.RegisterUnique(); composition.RegisterUnique(); } diff --git a/src/Umbraco.Web.BackOffice/Security/UmbracoSecureDataFormat.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficeSecureDataFormat.cs similarity index 93% rename from src/Umbraco.Web.BackOffice/Security/UmbracoSecureDataFormat.cs rename to src/Umbraco.Web.BackOffice/Security/BackOfficeSecureDataFormat.cs index 04fd3e59f1..88017f509a 100644 --- a/src/Umbraco.Web.BackOffice/Security/UmbracoSecureDataFormat.cs +++ b/src/Umbraco.Web.BackOffice/Security/BackOfficeSecureDataFormat.cs @@ -10,12 +10,12 @@ namespace Umbraco.Web.BackOffice.Security /// Custom secure format that ensures the Identity in the ticket is and not just a ClaimsIdentity /// // TODO: Unsure if we really need this, there's no real reason why we have a custom Identity instead of just a ClaimsIdentity - internal class UmbracoSecureDataFormat : ISecureDataFormat + internal class BackOfficeSecureDataFormat : ISecureDataFormat { private readonly int _loginTimeoutMinutes; private readonly ISecureDataFormat _ticketDataFormat; - public UmbracoSecureDataFormat(int loginTimeoutMinutes, ISecureDataFormat ticketDataFormat) + public BackOfficeSecureDataFormat(int loginTimeoutMinutes, ISecureDataFormat ticketDataFormat) { _loginTimeoutMinutes = loginTimeoutMinutes; _ticketDataFormat = ticketDataFormat ?? throw new ArgumentNullException(nameof(ticketDataFormat)); diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficeSecurityStampValidator.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficeSecurityStampValidator.cs new file mode 100644 index 0000000000..f12b6279bb --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Security/BackOfficeSecurityStampValidator.cs @@ -0,0 +1,27 @@ +using System; +using System.Security.Claims; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Umbraco.Core.BackOffice; +using Umbraco.Web.Common.Security; + +namespace Umbraco.Web.BackOffice.Security +{ + + /// + /// A security stamp validator for the back office + /// + public class BackOfficeSecurityStampValidator : SecurityStampValidator + { + public BackOfficeSecurityStampValidator( + IOptions options, + BackOfficeSignInManager signInManager, ISystemClock clock, ILoggerFactory logger) + : base(options, signInManager, clock, logger) + { + } + + } +} diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficeSecurityStampValidatorOptions.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficeSecurityStampValidatorOptions.cs new file mode 100644 index 0000000000..55d4a9cb94 --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Security/BackOfficeSecurityStampValidatorOptions.cs @@ -0,0 +1,13 @@ +using Microsoft.AspNetCore.Identity; + +namespace Umbraco.Web.BackOffice.Security +{ + /// + /// Custom for the back office + /// + public class BackOfficeSecurityStampValidatorOptions : SecurityStampValidatorOptions + { + } + + +} diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficeSessionIdValidator.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficeSessionIdValidator.cs new file mode 100644 index 0000000000..70bcf57954 --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Security/BackOfficeSessionIdValidator.cs @@ -0,0 +1,134 @@ + +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Authentication.Cookies; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Extensions; +using System; +using System.Collections.Generic; +using System.Security.Claims; +using System.Text; +using System.Threading.Tasks; +using Umbraco.Core; +using Umbraco.Core.BackOffice; +using Umbraco.Core.Configuration; +using Umbraco.Core.Hosting; +using Umbraco.Extensions; + +namespace Umbraco.Web.BackOffice.Security +{ + using ICookieManager = Microsoft.AspNetCore.Authentication.Cookies.ICookieManager; + + /// + /// Static helper class used to configure a CookieAuthenticationProvider to validate a cookie against a user's session id + /// + /// + /// This uses another cookie to track the last checked time which is done for a few reasons: + /// * We can't use the user's auth ticket to do this because we'd be re-issuing the auth ticket all of the time and it would never expire + /// plus the auth ticket size is much larger than this small value + /// * This will execute quite often (every minute per user) and in some cases there might be several requests that end up re-issuing the cookie so the cookie value should be small + /// * We want to avoid the user lookup if it's not required so that will only happen when the time diff is great enough in the cookie + /// + public class BackOfficeSessionIdValidator + { + public const string CookieName = "UMB_UCONTEXT_C"; + private readonly ISystemClock _systemClock; + private readonly IGlobalSettings _globalSettings; + private readonly IHostingEnvironment _hostingEnvironment; + private readonly BackOfficeUserManager _userManager; + + public BackOfficeSessionIdValidator(ISystemClock systemClock, IGlobalSettings globalSettings, IHostingEnvironment hostingEnvironment, BackOfficeUserManager userManager) + { + _systemClock = systemClock; + _globalSettings = globalSettings; + _hostingEnvironment = hostingEnvironment; + _userManager = userManager; + } + + public async Task ValidateSessionAsync(TimeSpan validateInterval, CookieValidatePrincipalContext context) + { + if (!context.Request.IsBackOfficeRequest(_globalSettings, _hostingEnvironment)) + return; + + var valid = await ValidateSessionAsync(validateInterval, context.HttpContext, context.Options.CookieManager, _systemClock, context.Properties.IssuedUtc, context.Principal.Identity as ClaimsIdentity); + + if (valid == false) + { + context.RejectPrincipal(); + await context.HttpContext.SignOutAsync(Constants.Security.BackOfficeAuthenticationType); + } + } + + private async Task ValidateSessionAsync( + TimeSpan validateInterval, + HttpContext httpContext, + ICookieManager cookieManager, + ISystemClock systemClock, + DateTimeOffset? authTicketIssueDate, + ClaimsIdentity currentIdentity) + { + if (httpContext == null) throw new ArgumentNullException(nameof(httpContext)); + if (cookieManager == null) throw new ArgumentNullException(nameof(cookieManager)); + if (systemClock == null) throw new ArgumentNullException(nameof(systemClock)); + + if (currentIdentity == null) + { + return false; + } + + DateTimeOffset? issuedUtc = null; + var currentUtc = systemClock.UtcNow; + + //read the last checked time from a custom cookie + var lastCheckedCookie = cookieManager.GetRequestCookie(httpContext, CookieName); + + if (lastCheckedCookie.IsNullOrWhiteSpace() == false) + { + if (DateTimeOffset.TryParse(lastCheckedCookie, out var parsed)) + { + issuedUtc = parsed; + } + } + + //no cookie, use the issue time of the auth ticket + if (issuedUtc.HasValue == false) + { + issuedUtc = authTicketIssueDate; + } + + // Only validate if enough time has elapsed + var validate = issuedUtc.HasValue == false; + if (issuedUtc.HasValue) + { + var timeElapsed = currentUtc.Subtract(issuedUtc.Value); + validate = timeElapsed > validateInterval; + } + + if (validate == false) + return true; + + var userId = currentIdentity.GetUserId(); + var user = await _userManager.FindByIdAsync(userId); + if (user == null) + return false; + + var sessionId = currentIdentity.FindFirstValue(Constants.Security.SessionIdClaimType); + if (await _userManager.ValidateSessionIdAsync(userId, sessionId) == false) + return false; + + //we will re-issue the cookie last checked cookie + cookieManager.AppendResponseCookie( + httpContext, + CookieName, + DateTimeOffset.UtcNow.ToString("yyyy-MM-ddTHH:mm:ss.fffffffzzz"), + new CookieOptions + { + HttpOnly = true, + Secure = _globalSettings.UseHttps || httpContext.Request.IsHttps, + Path = "/" + }); + + return true; + } + + } +} diff --git a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs new file mode 100644 index 0000000000..db4269dbb0 --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs @@ -0,0 +1,185 @@ +using System; +using System.Security.Claims; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Authentication.Cookies; +using Microsoft.AspNetCore.DataProtection; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Options; +using Umbraco.Core; +using Umbraco.Core.BackOffice; +using Umbraco.Core.Cache; +using Umbraco.Core.Configuration; +using Umbraco.Core.Configuration.UmbracoSettings; +using Umbraco.Core.Hosting; +using Umbraco.Core.Services; +using Umbraco.Net; +using Umbraco.Core.Security; +using Umbraco.Web; + +namespace Umbraco.Web.BackOffice.Security +{ + /// + /// Used to configure for the back office authentication type + /// + public class ConfigureBackOfficeCookieOptions : IConfigureNamedOptions + { + private readonly IUmbracoContextAccessor _umbracoContextAccessor; + private readonly ISecuritySettings _securitySettings; + private readonly IGlobalSettings _globalSettings; + private readonly IHostingEnvironment _hostingEnvironment; + private readonly IRuntimeState _runtimeState; + private readonly IDataProtectionProvider _dataProtection; + private readonly IRequestCache _requestCache; + private readonly IUserService _userService; + private readonly IIpResolver _ipResolver; + private readonly BackOfficeSessionIdValidator _sessionIdValidator; + private readonly BackOfficeSecurityStampValidator _securityStampValidator; + + public ConfigureBackOfficeCookieOptions( + IUmbracoContextAccessor umbracoContextAccessor, + ISecuritySettings securitySettings, + IGlobalSettings globalSettings, + IHostingEnvironment hostingEnvironment, + IRuntimeState runtimeState, + IDataProtectionProvider dataProtection, + IRequestCache requestCache, + IUserService userService, + IIpResolver ipResolver, + BackOfficeSessionIdValidator sessionIdValidator, + BackOfficeSecurityStampValidator securityStampValidator) + { + _umbracoContextAccessor = umbracoContextAccessor; + _securitySettings = securitySettings; + _globalSettings = globalSettings; + _hostingEnvironment = hostingEnvironment; + _runtimeState = runtimeState; + _dataProtection = dataProtection; + _requestCache = requestCache; + _userService = userService; + _ipResolver = ipResolver; + _sessionIdValidator = sessionIdValidator; + _securityStampValidator = securityStampValidator; + } + + public void Configure(string name, CookieAuthenticationOptions options) + { + if (name != Constants.Security.BackOfficeAuthenticationType) return; + Configure(options); + } + + public void Configure(CookieAuthenticationOptions options) + { + options.SlidingExpiration = true; + options.ExpireTimeSpan = TimeSpan.FromMinutes(_globalSettings.TimeOutInMinutes); + 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 = "/"; + + 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. + var dataProtector = options.DataProtectionProvider.CreateProtector("Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationMiddleware", Constants.Security.BackOfficeAuthenticationType, "v2"); + var ticketDataFormat = new TicketDataFormat(dataProtector); + + options.TicketDataFormat = new BackOfficeSecureDataFormat(_globalSettings.TimeOutInMinutes, ticketDataFormat); + + //Custom cookie manager so we can filter requests + options.CookieManager = new BackOfficeCookieManager( + _umbracoContextAccessor, + _runtimeState, + _hostingEnvironment, + _globalSettings, + _requestCache); + // _explicitPaths); TODO: Implement this once we do OAuth somehow + + + options.Events = new CookieAuthenticationEvents + { + OnValidatePrincipal = async ctx => + { + //ensure the thread culture is set + ctx.Principal?.Identity?.EnsureCulture(); + + await EnsureValidSessionId(ctx); + + if (ctx.Principal?.Identity == null) + { + await ctx.HttpContext.SignOutAsync(Constants.Security.BackOfficeAuthenticationType); + return; + } + + await _securityStampValidator.ValidateAsync(ctx); + }, + OnSignedIn = ctx => + { + // When we are signed in with the cookie, assign the principal to the current HttpContext + ctx.HttpContext.User = ctx.Principal; + + if (ctx.Principal.Identity is UmbracoBackOfficeIdentity backOfficeIdentity) + { + //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 + + var session = _runtimeState.Level == RuntimeLevel.Run + ? _userService.CreateLoginSession(backOfficeIdentity.Id, _ipResolver.GetCurrentRequestIpAddress()) + : Guid.NewGuid(); + + backOfficeIdentity.SessionId = session.ToString(); + + //since it is a cookie-based authentication add that claim + backOfficeIdentity.AddClaim(new Claim(ClaimTypes.CookiePath, "/", ClaimValueTypes.String, UmbracoBackOfficeIdentity.Issuer, UmbracoBackOfficeIdentity.Issuer, backOfficeIdentity)); + } + + 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 var guidSession)) + { + _userService.ClearLoginSession(guidSession); + } + } + + // Remove all of our cookies + var cookies = new[] + { + BackOfficeSessionIdValidator.CookieName, + _securitySettings.AuthCookieName, + Constants.Web.PreviewCookieName, + Constants.Security.BackOfficeExternalCookieName + }; + foreach (var cookie in cookies) + { + ctx.Options.CookieManager.DeleteCookie(ctx.HttpContext, cookie, new CookieOptions + { + Path = "/" + }); + } + + return Task.CompletedTask; + } + }; + } + + /// + /// Ensures that the user has a valid session id + /// + /// + /// So that we are not overloading the database this throttles it's check to every minute + /// + private async Task EnsureValidSessionId(CookieValidatePrincipalContext context) + { + if (_runtimeState.Level == RuntimeLevel.Run) + await _sessionIdValidator.ValidateSessionAsync(TimeSpan.FromMinutes(1), context); + } + } +} diff --git a/src/Umbraco.Web.BackOffice/Security/ConfigureUmbracoBackOfficeIdentityOptions.cs b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeIdentityOptions.cs similarity index 89% rename from src/Umbraco.Web.BackOffice/Security/ConfigureUmbracoBackOfficeIdentityOptions.cs rename to src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeIdentityOptions.cs index 245c989f88..3a3f9b1759 100644 --- a/src/Umbraco.Web.BackOffice/Security/ConfigureUmbracoBackOfficeIdentityOptions.cs +++ b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeIdentityOptions.cs @@ -11,11 +11,11 @@ namespace Umbraco.Web.BackOffice.Security /// /// Used to configure for the Umbraco Back office /// - public class ConfigureUmbracoBackOfficeIdentityOptions : IConfigureOptions + public class ConfigureBackOfficeIdentityOptions : IConfigureOptions { private readonly IUserPasswordConfiguration _userPasswordConfiguration; - public ConfigureUmbracoBackOfficeIdentityOptions(IUserPasswordConfiguration userPasswordConfiguration) + public ConfigureBackOfficeIdentityOptions(IUserPasswordConfiguration userPasswordConfiguration) { _userPasswordConfiguration = userPasswordConfiguration; } diff --git a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeSecurityStampValidatorOptions.cs b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeSecurityStampValidatorOptions.cs new file mode 100644 index 0000000000..1facf094d1 --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeSecurityStampValidatorOptions.cs @@ -0,0 +1,18 @@ +using Microsoft.Extensions.Options; +using System; + +namespace Umbraco.Web.BackOffice.Security +{ + /// + /// Configures the back office security stamp options + /// + public class ConfigureBackOfficeSecurityStampValidatorOptions : IConfigureOptions + { + public void Configure(BackOfficeSecurityStampValidatorOptions options) + { + options.ValidationInterval = TimeSpan.FromMinutes(30); + } + } + + +} diff --git a/src/Umbraco.Web.BackOffice/Security/ConfigureUmbracoBackOfficeCookieOptions.cs b/src/Umbraco.Web.BackOffice/Security/ConfigureUmbracoBackOfficeCookieOptions.cs deleted file mode 100644 index 70172bed37..0000000000 --- a/src/Umbraco.Web.BackOffice/Security/ConfigureUmbracoBackOfficeCookieOptions.cs +++ /dev/null @@ -1,94 +0,0 @@ -using System; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Authentication; -using Microsoft.AspNetCore.Authentication.Cookies; -using Microsoft.AspNetCore.DataProtection; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.Options; -using Umbraco.Core; -using Umbraco.Core.Cache; -using Umbraco.Core.Configuration; -using Umbraco.Core.Configuration.UmbracoSettings; -using Umbraco.Core.Hosting; -using Umbraco.Web; - -namespace Umbraco.Web.BackOffice.Security -{ - /// - /// Used to configure for the back office authentication type - /// - public class ConfigureUmbracoBackOfficeCookieOptions : IConfigureNamedOptions - { - private readonly IUmbracoContextAccessor _umbracoContextAccessor; - private readonly ISecuritySettings _securitySettings; - private readonly IGlobalSettings _globalSettings; - private readonly IHostingEnvironment _hostingEnvironment; - private readonly IRuntimeState _runtimeState; - private readonly IDataProtectionProvider _dataProtection; - private readonly IRequestCache _requestCache; - - public ConfigureUmbracoBackOfficeCookieOptions( - IUmbracoContextAccessor umbracoContextAccessor, - ISecuritySettings securitySettings, - IGlobalSettings globalSettings, - IHostingEnvironment hostingEnvironment, - IRuntimeState runtimeState, - IDataProtectionProvider dataProtection, - IRequestCache requestCache) - { - _umbracoContextAccessor = umbracoContextAccessor; - _securitySettings = securitySettings; - _globalSettings = globalSettings; - _hostingEnvironment = hostingEnvironment; - _runtimeState = runtimeState; - _dataProtection = dataProtection; - _requestCache = requestCache; - } - - public void Configure(string name, CookieAuthenticationOptions options) - { - if (name != Constants.Security.BackOfficeAuthenticationType) return; - - options.SlidingExpiration = true; - options.ExpireTimeSpan = TimeSpan.FromMinutes(_globalSettings.TimeOutInMinutes); - 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 = "/"; - - 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. - var dataProtector = options.DataProtectionProvider.CreateProtector("Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationMiddleware", name, "v2"); - var ticketDataFormat = new TicketDataFormat(dataProtector); - - options.TicketDataFormat = new UmbracoSecureDataFormat(_globalSettings.TimeOutInMinutes, ticketDataFormat); - - //Custom cookie manager so we can filter requests - options.CookieManager = new BackOfficeCookieManager( - _umbracoContextAccessor, - _runtimeState, - _hostingEnvironment, - _globalSettings, - _requestCache); - // _explicitPaths); TODO: Implement this once we do OAuth somehow - - - options.Events = new CookieAuthenticationEvents - { - OnSignedIn = ctx => - { - // When we are signed in with the cookie, assign the principal to the current HttpContext - ctx.HttpContext.User = ctx.Principal; - return Task.CompletedTask; - } - }; - } - - public void Configure(CookieAuthenticationOptions options) - { - } - } -} diff --git a/src/Umbraco.Web.Common/Extensions/HttpRequestExtensions.cs b/src/Umbraco.Web.Common/Extensions/HttpRequestExtensions.cs index c346f0ddc9..b6652de6cf 100644 --- a/src/Umbraco.Web.Common/Extensions/HttpRequestExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/HttpRequestExtensions.cs @@ -1,10 +1,25 @@ -using System.Net; +using System; +using System.Net; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Extensions; +using Umbraco.Core; +using Umbraco.Core.Configuration; +using Umbraco.Core.Hosting; -namespace Umbraco.Web.Common.Extensions +namespace Umbraco.Extensions { public static class HttpRequestExtensions { + public static bool IsBackOfficeRequest(this HttpRequest request, IGlobalSettings globalSettings, IHostingEnvironment hostingEnvironment) + { + return new Uri(request.GetEncodedUrl(), UriKind.RelativeOrAbsolute).IsBackOfficeRequest(globalSettings, hostingEnvironment); + } + + public static bool IsClientSideRequest(this HttpRequest request) + { + return new Uri(request.GetEncodedUrl(), UriKind.RelativeOrAbsolute).IsClientSideRequest(); + } + internal static string ClientCulture(this HttpRequest request) { return request.Headers.TryGetValue("X-UMB-CULTURE", out var values) ? values[0] : null; diff --git a/src/Umbraco.Web.Common/Middleware/UmbracoRequestLoggingMiddleware.cs b/src/Umbraco.Web.Common/Middleware/UmbracoRequestLoggingMiddleware.cs index febf939d03..1bda56bd37 100644 --- a/src/Umbraco.Web.Common/Middleware/UmbracoRequestLoggingMiddleware.cs +++ b/src/Umbraco.Web.Common/Middleware/UmbracoRequestLoggingMiddleware.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Http.Extensions; using Serilog.Context; using Umbraco.Core; using Umbraco.Core.Logging.Serilog.Enrichers; +using Umbraco.Extensions; namespace Umbraco.Web.Common.Middleware { @@ -30,7 +31,7 @@ namespace Umbraco.Web.Common.Middleware public async Task InvokeAsync(HttpContext context, RequestDelegate next) { // do not process if client-side request - if (new Uri(context.Request.GetEncodedUrl(), UriKind.RelativeOrAbsolute).IsClientSideRequest()) + if (context.Request.IsClientSideRequest()) { await next(context); return; diff --git a/src/Umbraco.Web.Common/ModelBinders/HttpQueryStringModelBinder.cs b/src/Umbraco.Web.Common/ModelBinders/HttpQueryStringModelBinder.cs index eb6a1ab7fb..824da4fcd0 100644 --- a/src/Umbraco.Web.Common/ModelBinders/HttpQueryStringModelBinder.cs +++ b/src/Umbraco.Web.Common/ModelBinders/HttpQueryStringModelBinder.cs @@ -5,7 +5,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.Extensions.Primitives; using Umbraco.Core; -using Umbraco.Web.Common.Extensions; +using Umbraco.Extensions; namespace Umbraco.Web.Common.ModelBinders { diff --git a/src/Umbraco.Web.Common/Profiler/WebProfiler.cs b/src/Umbraco.Web.Common/Profiler/WebProfiler.cs index 30777d07a5..2569179c06 100644 --- a/src/Umbraco.Web.Common/Profiler/WebProfiler.cs +++ b/src/Umbraco.Web.Common/Profiler/WebProfiler.cs @@ -6,6 +6,7 @@ using Microsoft.AspNetCore.Http.Extensions; using StackExchange.Profiling; using Umbraco.Core; using Umbraco.Core.Logging; +using Umbraco.Extensions; namespace Umbraco.Web.Common.Profiler { @@ -70,7 +71,7 @@ namespace Umbraco.Web.Common.Profiler private static bool ShouldProfile(HttpRequest request) { - if (new Uri(request.GetEncodedUrl(), UriKind.RelativeOrAbsolute).IsClientSideRequest()) return false; + if (request.IsClientSideRequest()) return false; if (bool.TryParse(request.Query["umbDebug"], out var umbDebug)) return umbDebug; if (bool.TryParse(request.Headers["X-UMB-DEBUG"], out var xUmbDebug)) return xUmbDebug; if (bool.TryParse(request.Cookies["UMB-DEBUG"], out var cUmbDebug)) return cUmbDebug; diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficeSignInManager.cs b/src/Umbraco.Web.Common/Security/BackOfficeSignInManager.cs similarity index 90% rename from src/Umbraco.Web.BackOffice/Security/BackOfficeSignInManager.cs rename to src/Umbraco.Web.Common/Security/BackOfficeSignInManager.cs index 404e38c396..dd0afe5a80 100644 --- a/src/Umbraco.Web.BackOffice/Security/BackOfficeSignInManager.cs +++ b/src/Umbraco.Web.Common/Security/BackOfficeSignInManager.cs @@ -11,8 +11,12 @@ using System.Threading.Tasks; using Umbraco.Core; using Umbraco.Core.BackOffice; -namespace Umbraco.Web.BackOffice.Security +namespace Umbraco.Web.Common.Security { + using Constants = Umbraco.Core.Constants; + + // TODO: There's potential to extract an interface for this for only what we use and put that in Core without aspnetcore refs, but we need to wait till were done with it since there's a bit to implement + public class BackOfficeSignInManager : SignInManager { private readonly BackOfficeUserManager _userManager; @@ -30,6 +34,17 @@ namespace Umbraco.Web.BackOffice.Security _userManager = userManager; } + // TODO: Implement these, this is what the security stamp thingy calls + public override Task ValidateSecurityStampAsync(BackOfficeIdentityUser user, string securityStamp) + { + return base.ValidateSecurityStampAsync(user, securityStamp); + } + // TODO: Implement these, this is what the security stamp thingy calls + public override Task ValidateSecurityStampAsync(ClaimsPrincipal principal) + { + return base.ValidateSecurityStampAsync(principal); + } + // TODO: Need to migrate more from Umbraco.Web.Security.BackOfficeSignInManager // Things like dealing with auto-linking, cookie options, and a ton of other stuff. Some might not need to be ported but it // will be a case by case basis. @@ -132,21 +147,13 @@ namespace Umbraco.Web.BackOffice.Security _userManager.RaiseLoginSuccessEvent(user, user.Id); } else if (result.IsLockedOut) - { Logger.LogInformation("Login attempt failed for username {UserName} from IP address {IpAddress}, the user is locked", username, Context.Connection.RemoteIpAddress); - } else if (result.RequiresTwoFactor) - { Logger.LogInformation("Login attempt requires verification for username {UserName} from IP address {IpAddress}", username, Context.Connection.RemoteIpAddress); - } else if (!result.Succeeded || result.IsNotAllowed) - { Logger.LogInformation("Login attempt failed for username {UserName} from IP address {IpAddress}", username, Context.Connection.RemoteIpAddress); - } else - { throw new ArgumentOutOfRangeException(); - } return result; } diff --git a/src/Umbraco.Web.Common/Security/WebSecurity.cs b/src/Umbraco.Web.Common/Security/WebSecurity.cs index 21968e8b98..5f4780a387 100644 --- a/src/Umbraco.Web.Common/Security/WebSecurity.cs +++ b/src/Umbraco.Web.Common/Security/WebSecurity.cs @@ -23,7 +23,11 @@ namespace Umbraco.Web.Common.Security private readonly IHostingEnvironment _hostingEnvironment; private readonly IHttpContextAccessor _httpContextAccessor; - public WebSecurity(IUserService userService, IGlobalSettings globalSettings, IHostingEnvironment hostingEnvironment, IHttpContextAccessor httpContextAccessor) + public WebSecurity( + IUserService userService, + IGlobalSettings globalSettings, + IHostingEnvironment hostingEnvironment, + IHttpContextAccessor httpContextAccessor) { _userService = userService; _globalSettings = globalSettings; @@ -63,11 +67,6 @@ namespace Umbraco.Web.Common.Security return ValidateCurrentUser(throwExceptions); } - public void ClearCurrentLogin() - { - //throw new NotImplementedException(); - } - public Attempt GetUserId() { return Attempt.Succeed(-1); @@ -79,11 +78,6 @@ namespace Umbraco.Web.Common.Security return httpContext?.User != null && httpContext.User.Identity.IsAuthenticated && httpContext.GetCurrentIdentity() != null; } - public double PerformLogin(int userId) - { - return 100; - } - public bool UserHasSectionAccess(string section, IUser user) { return true; diff --git a/src/Umbraco.Web.Common/UmbracoContext/UmbracoContextFactory.cs b/src/Umbraco.Web.Common/UmbracoContext/UmbracoContextFactory.cs index 498b86a575..7fde583523 100644 --- a/src/Umbraco.Web.Common/UmbracoContext/UmbracoContextFactory.cs +++ b/src/Umbraco.Web.Common/UmbracoContext/UmbracoContextFactory.cs @@ -52,11 +52,11 @@ namespace Umbraco.Web _defaultCultureAccessor = defaultCultureAccessor ?? throw new ArgumentNullException(nameof(defaultCultureAccessor)); _globalSettings = globalSettings ?? throw new ArgumentNullException(nameof(globalSettings)); _userService = userService ?? throw new ArgumentNullException(nameof(userService)); - _hostingEnvironment = hostingEnvironment; - _uriUtility = uriUtility; - _httpContextAccessor = httpContextAccessor; - _cookieManager = cookieManager; - _requestAccessor = requestAccessor; + _hostingEnvironment = hostingEnvironment ?? throw new ArgumentNullException(nameof(hostingEnvironment)); + _uriUtility = uriUtility ?? throw new ArgumentNullException(nameof(uriUtility)); + _httpContextAccessor = httpContextAccessor ?? throw new ArgumentNullException(nameof(httpContextAccessor)); + _cookieManager = cookieManager ?? throw new ArgumentNullException(nameof(cookieManager)); + _requestAccessor = requestAccessor ?? throw new ArgumentNullException(nameof(requestAccessor)); } private IUmbracoContext CreateUmbracoContext() diff --git a/src/Umbraco.Web.UI.NetCore/Areas/UmbracoBackOffice/Views/BackOffice/Default.cshtml b/src/Umbraco.Web.UI.NetCore/Areas/UmbracoBackOffice/Views/BackOffice/Default.cshtml index cfd01a2534..5bf833dedd 100644 --- a/src/Umbraco.Web.UI.NetCore/Areas/UmbracoBackOffice/Views/BackOffice/Default.cshtml +++ b/src/Umbraco.Web.UI.NetCore/Areas/UmbracoBackOffice/Views/BackOffice/Default.cshtml @@ -2,7 +2,7 @@ @using Umbraco.Web.Composing @using Umbraco.Web @using Umbraco.Web.WebAssets -@using Umbraco.Web.BackOffice.Security +@using Umbraco.Web.Common.Security @using Umbraco.Core.WebAssets @using Umbraco.Core.Configuration @using Umbraco.Core.Hosting diff --git a/src/Umbraco.Web/Security/AppBuilderExtensions.cs b/src/Umbraco.Web/Security/AppBuilderExtensions.cs index 13d068a5f0..f766a142d9 100644 --- a/src/Umbraco.Web/Security/AppBuilderExtensions.cs +++ b/src/Umbraco.Web/Security/AppBuilderExtensions.cs @@ -147,19 +147,6 @@ namespace Umbraco.Web.Security //Create the default options and provider var authOptions = app.CreateUmbracoCookieAuthOptions(umbracoContextAccessor, globalSettings, runtimeState, securitySettings, hostingEnvironment, requestCache); - authOptions.Provider = new BackOfficeCookieAuthenticationProvider(userService, runtimeState, globalSettings, hostingEnvironment, securitySettings) - { - // Enables the application to validate the security stamp when the user - // logs in. This is a security feature which is used when you - // change a password or add an external login to your account. - OnValidateIdentity = UmbracoSecurityStampValidator - .OnValidateIdentity( - TimeSpan.FromMinutes(30), - (signInManager, manager, user) => signInManager.CreateUserIdentityAsync(user), - identity => identity.GetUserId()), - - }; - return app.UseUmbracoBackOfficeCookieAuthentication(umbracoContextAccessor, runtimeState, globalSettings, securitySettings, hostingEnvironment, requestCache, authOptions, stage); } diff --git a/src/Umbraco.Web/Security/AuthenticationExtensions.cs b/src/Umbraco.Web/Security/AuthenticationExtensions.cs index deb735ca56..8e9f2abc01 100644 --- a/src/Umbraco.Web/Security/AuthenticationExtensions.cs +++ b/src/Umbraco.Web/Security/AuthenticationExtensions.cs @@ -68,7 +68,7 @@ namespace Umbraco.Web.Security if (ex is FormatException || ex is JsonReaderException) { // this will occur if the cookie data is invalid - http.UmbracoLogout(); + } else { @@ -86,15 +86,10 @@ namespace Umbraco.Web.Security /// This will return the current back office identity. /// /// - /// - /// If set to true and a back office identity is not found and not authenticated, this will attempt to authenticate the - /// request just as is done in the Umbraco module and then set the current identity if it is valid. - /// Just like in the UmbracoModule, if this is true then the user's culture will be assigned to the request. - /// /// /// Returns the current back office identity if an admin is authenticated otherwise null /// - public static UmbracoBackOfficeIdentity GetCurrentIdentity(this HttpContextBase http, bool authenticateRequestIfNotFound) + public static UmbracoBackOfficeIdentity GetCurrentIdentity(this HttpContextBase http) { if (http == null) throw new ArgumentNullException(nameof(http)); if (http.User == null) return null; //there's no user at all so no identity @@ -103,59 +98,20 @@ namespace Umbraco.Web.Security var backOfficeIdentity = http.User.GetUmbracoIdentity(); if (backOfficeIdentity != null) return backOfficeIdentity; - if (authenticateRequestIfNotFound == false) return null; - - // even if authenticateRequestIfNotFound is true we cannot continue if the request is actually authenticated - // which would mean something strange is going on that it is not an umbraco identity. - if (http.User.Identity.IsAuthenticated) return null; - - // So the user is not authed but we've been asked to do the auth if authenticateRequestIfNotFound = true, - // which might occur in old webforms style things or for routes that aren't included as a back office request. - // in this case, we are just reverting to authing using the cookie. - - // TODO: Even though this is in theory legacy, we have legacy bits laying around and we'd need to do the auth based on - // how the Module will eventually do it (by calling in to any registered authenticators). - - var ticket = http.GetUmbracoAuthTicket(); - if (http.AuthenticateCurrentRequest(ticket, true)) - { - //now we 'should have an umbraco identity - return http.User.Identity as UmbracoBackOfficeIdentity; - } return null; } /// /// This will return the current back office identity. /// - /// - /// - /// If set to true and a back office identity is not found and not authenticated, this will attempt to authenticate the - /// request just as is done in the Umbraco module and then set the current identity if it is valid - /// + /// /// /// Returns the current back office identity if an admin is authenticated otherwise null /// - internal static UmbracoBackOfficeIdentity GetCurrentIdentity(this HttpContext http, bool authenticateRequestIfNotFound) + internal static UmbracoBackOfficeIdentity GetCurrentIdentity(this HttpContext http) { if (http == null) throw new ArgumentNullException("http"); - return new HttpContextWrapper(http).GetCurrentIdentity(authenticateRequestIfNotFound); - } - - public static void UmbracoLogout(this HttpContextBase http) - { - if (http == null) throw new ArgumentNullException("http"); - Logout(http, Current.Configs.Security().AuthCookieName); - } - - /// - /// This clears the forms authentication cookie - /// - /// - internal static void UmbracoLogout(this HttpContext http) - { - if (http == null) throw new ArgumentNullException("http"); - new HttpContextWrapper(http).UmbracoLogout(); + return new HttpContextWrapper(http).GetCurrentIdentity(); } /// @@ -218,52 +174,6 @@ namespace Umbraco.Web.Security return GetAuthTicket(ctx, Current.Configs.Security().AuthCookieName); } - /// - /// This clears the forms authentication cookie - /// - /// - /// - private static void Logout(this HttpContextBase http, string cookieName) - { - // We need to clear the sessionId from the database. This is legacy code to do any logging out and shouldn't really be used at all but in any case - // we need to make sure the session is cleared. Due to the legacy nature of this it means we need to use singletons - if (http.User != null) - { - var claimsIdentity = http.User.Identity as ClaimsIdentity; - if (claimsIdentity != null) - { - var sessionId = claimsIdentity.FindFirstValue(Constants.Security.SessionIdClaimType); - Guid guidSession; - if (sessionId.IsNullOrWhiteSpace() == false && Guid.TryParse(sessionId, out guidSession)) - { - Current.Services.UserService.ClearLoginSession(guidSession); - } - } - } - - if (http == null) throw new ArgumentNullException("http"); - // clear the preview cookie and external login - var cookies = new[] { cookieName, Constants.Web.PreviewCookieName, Constants.Security.BackOfficeExternalCookieName }; - foreach (var c in cookies) - { - // remove from the request - http.Request.Cookies.Remove(c); - - // expire from the response - var formsCookie = http.Response.Cookies[c]; - if (formsCookie != null) - { - // this will expire immediately and be removed from the browser - formsCookie.Expires = DateTime.Now.AddYears(-1); - } - else - { - // ensure there's def an expired cookie - http.Response.Cookies.Add(new HttpCookie(c) { Expires = DateTime.Now.AddYears(-1) }); - } - } - } - private static AuthenticationTicket GetAuthTicket(this IOwinContext owinCtx, string cookieName) { var asDictionary = new Dictionary(); @@ -313,7 +223,7 @@ namespace Umbraco.Web.Security catch (Exception) { // occurs when decryption fails - http.Logout(cookieName); + return null; } } @@ -334,23 +244,6 @@ namespace Umbraco.Web.Security return secureDataFormat.Unprotect(formsCookie); } - /// - /// Ensures that the thread culture is set based on the back office user's culture - /// - /// - public static void EnsureCulture(this IIdentity identity) - { - if (identity is UmbracoBackOfficeIdentity umbIdentity && umbIdentity.IsAuthenticated) - { - Thread.CurrentThread.CurrentUICulture = - Thread.CurrentThread.CurrentCulture = UserCultures.GetOrAdd(umbIdentity.Culture, s => new CultureInfo(s)); - } - } - - - /// - /// Used so that we aren't creating a new CultureInfo object for every single request - /// - private static readonly ConcurrentDictionary UserCultures = new ConcurrentDictionary(); + } } diff --git a/src/Umbraco.Web/Security/BackOfficeCookieAuthenticationProvider.cs b/src/Umbraco.Web/Security/BackOfficeCookieAuthenticationProvider.cs index 35edb251f9..26b85d6c39 100644 --- a/src/Umbraco.Web/Security/BackOfficeCookieAuthenticationProvider.cs +++ b/src/Umbraco.Web/Security/BackOfficeCookieAuthenticationProvider.cs @@ -9,9 +9,12 @@ using Umbraco.Core.Configuration; using Umbraco.Core.Services; using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.Hosting; +using Umbraco.Core.Security; namespace Umbraco.Web.Security { + // TODO: Migrate this logic to cookie events in ConfigureUmbracoBackOfficeCookieOptions + public class BackOfficeCookieAuthenticationProvider : CookieAuthenticationProvider { private readonly IUserService _userService; @@ -29,97 +32,12 @@ namespace Umbraco.Web.Security _securitySettings = securitySettings; } - public override void ResponseSignIn(CookieResponseSignInContext context) - { - if (context.Identity is UmbracoBackOfficeIdentity backOfficeIdentity) - { - //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 - - var session = _runtimeState.Level == RuntimeLevel.Run - ? _userService.CreateLoginSession(backOfficeIdentity.Id, context.OwinContext.GetCurrentRequestIpAddress()) - : Guid.NewGuid(); - - backOfficeIdentity.SessionId = session.ToString(); - - //since it is a cookie-based authentication add that claim - backOfficeIdentity.AddClaim(new Claim(ClaimTypes.CookiePath, "/", ClaimValueTypes.String, UmbracoBackOfficeIdentity.Issuer, UmbracoBackOfficeIdentity.Issuer, backOfficeIdentity)); - } - - base.ResponseSignIn(context); - } public override void ResponseSignOut(CookieResponseSignOutContext context) { - //Clear the user's session on sign out - if (context?.OwinContext?.Authentication?.User?.Identity != null) - { - var claimsIdentity = context.OwinContext.Authentication.User.Identity as ClaimsIdentity; - var sessionId = claimsIdentity.FindFirstValue(Constants.Security.SessionIdClaimType); - if (sessionId.IsNullOrWhiteSpace() == false && Guid.TryParse(sessionId, out var guidSession)) - { - _userService.ClearLoginSession(guidSession); - } - } - - base.ResponseSignOut(context); - - //Make sure the definitely all of these cookies are cleared when signing out with cookies - context.Response.Cookies.Append(SessionIdValidator.CookieName, "", new CookieOptions - { - Expires = DateTime.Now.AddYears(-1), - Path = "/" - }); - context.Response.Cookies.Append(_securitySettings.AuthCookieName, "", new CookieOptions - { - Expires = DateTime.Now.AddYears(-1), - Path = "/" - }); - context.Response.Cookies.Append(Constants.Web.PreviewCookieName, "", new CookieOptions - { - Expires = DateTime.Now.AddYears(-1), - Path = "/" - }); - context.Response.Cookies.Append(Constants.Security.BackOfficeExternalCookieName, "", new CookieOptions - { - Expires = DateTime.Now.AddYears(-1), - Path = "/" - }); + } - /// - /// Ensures that the culture is set correctly for the current back office user and that the user's session token is valid - /// - /// - /// - public override async Task ValidateIdentity(CookieValidateIdentityContext context) - { - //ensure the thread culture is set - context?.Identity?.EnsureCulture(); - - await EnsureValidSessionId(context); - - if (context?.Identity == null) - { - context?.OwinContext.Authentication.SignOut(context.Options.AuthenticationType); - return; - } - await base.ValidateIdentity(context); - } - - /// - /// Ensures that the user has a valid session id - /// - /// - /// So that we are not overloading the database this throttles it's check to every minute - /// - protected virtual async Task EnsureValidSessionId(CookieValidateIdentityContext context) - { - if (_runtimeState.Level == RuntimeLevel.Run) - await SessionIdValidator.ValidateSessionAsync(TimeSpan.FromMinutes(1), context, _globalSettings, _hostingEnvironment); - } - - } diff --git a/src/Umbraco.Web/Security/UmbracoSecurityStampValidator.cs b/src/Umbraco.Web/Security/UmbracoSecurityStampValidator.cs deleted file mode 100644 index 18539d8fab..0000000000 --- a/src/Umbraco.Web/Security/UmbracoSecurityStampValidator.cs +++ /dev/null @@ -1,88 +0,0 @@ -using System; -using System.Security.Claims; -using System.Threading.Tasks; -using Microsoft.Owin.Security.Cookies; -using Umbraco.Core; -using Umbraco.Core.BackOffice; - -namespace Umbraco.Web.Security -{ - /// - /// Adapted from Microsoft.AspNet.Identity.Owin.SecurityStampValidator - /// - public class UmbracoSecurityStampValidator - { - public static Func OnValidateIdentity( - TimeSpan validateInterval, - Func> regenerateIdentityCallback, - Func getUserIdCallback) - where TSignInManager : BackOfficeSignInManager - where TManager : BackOfficeUserManager - where TUser : BackOfficeIdentityUser - { - if (getUserIdCallback == null) throw new ArgumentNullException(nameof(getUserIdCallback)); - - return async context => - { - var currentUtc = context.Options?.SystemClock?.UtcNow ?? DateTimeOffset.UtcNow; - - var issuedUtc = context.Properties.IssuedUtc; - - // Only validate if enough time has elapsed - var validate = issuedUtc == null; - if (issuedUtc != null) - { - var timeElapsed = currentUtc.Subtract(issuedUtc.Value); - validate = timeElapsed > validateInterval; - } - - if (validate) - { - var manager = context.OwinContext.Get(); - if (manager == null) throw new InvalidOperationException("Unable to load BackOfficeUserManager"); - - var signInManager = context.OwinContext.Get(); - if (signInManager == null) throw new InvalidOperationException("Unable to load BackOfficeSignInManager"); - - var userId = getUserIdCallback(context.Identity); - - if (userId != null) - { - var user = await manager.FindByIdAsync(userId); - var reject = true; - - // Refresh the identity if the stamp matches, otherwise reject - if (user != null && manager.SupportsUserSecurityStamp) - { - var securityStamp = context.Identity.FindFirstValue(Constants.Web.SecurityStampClaimType); - var newSecurityStamp = await manager.GetSecurityStampAsync(user); - - if (securityStamp == newSecurityStamp) - { - reject = false; - // Regenerate fresh claims if possible and resign in - if (regenerateIdentityCallback != null) - { - var identity = await regenerateIdentityCallback.Invoke(signInManager, manager, user); - if (identity != null) - { - // Fix for regression where this value is not updated - // Setting it to null so that it is refreshed by the cookie middleware - context.Properties.IssuedUtc = null; - context.Properties.ExpiresUtc = null; - context.OwinContext.Authentication.SignIn(context.Properties, identity); - } - } - } - } - if (reject) - { - context.RejectIdentity(); - context.OwinContext.Authentication.SignOut(context.Options.AuthenticationType); - } - } - } - }; - } - } -} diff --git a/src/Umbraco.Web/Security/WebSecurity.cs b/src/Umbraco.Web/Security/WebSecurity.cs index 6d3de9be1d..143f195de0 100644 --- a/src/Umbraco.Web/Security/WebSecurity.cs +++ b/src/Umbraco.Web/Security/WebSecurity.cs @@ -74,41 +74,13 @@ namespace Umbraco.Web.Security protected BackOfficeOwinUserManager UserManager => _userManager ?? (_userManager = _httpContextAccessor.GetRequiredHttpContext().GetOwinContext().GetBackOfficeUserManager()); - [Obsolete("This needs to be removed, ASP.NET Identity should always be used for this operation, this is currently only used in the installer which needs to be updated")] - public double PerformLogin(int userId) - { - var httpContext = _httpContextAccessor.GetRequiredHttpContext(); - var owinCtx = httpContext.GetOwinContext(); - //ensure it's done for owin too - owinCtx.Authentication.SignOut(Constants.Security.BackOfficeExternalAuthenticationType); - - var user = UserManager.FindByIdAsync(userId.ToString()).Result; - - SignInManager.SignInAsync(user, isPersistent: true, rememberBrowser: false).Wait(); - - httpContext.SetPrincipalForRequest(owinCtx.Request.User); - - return TimeSpan.FromMinutes(_globalSettings.TimeOutInMinutes).TotalSeconds; - } - - [Obsolete("This needs to be removed, ASP.NET Identity should always be used for this operation, this is currently only used in the installer which needs to be updated")] - public void ClearCurrentLogin() - { - var httpContext = _httpContextAccessor.GetRequiredHttpContext(); - httpContext.UmbracoLogout(); - httpContext.GetOwinContext().Authentication.SignOut( - Core.Constants.Security.BackOfficeAuthenticationType, - Core.Constants.Security.BackOfficeExternalAuthenticationType); - } - - /// /// Gets the current user's id. /// /// public Attempt GetUserId() { - var identity = _httpContextAccessor.GetRequiredHttpContext().GetCurrentIdentity(false); + var identity = _httpContextAccessor.GetRequiredHttpContext().GetCurrentIdentity(); return identity == null ? Attempt.Fail() : Attempt.Succeed(Convert.ToInt32(identity.Id)); } @@ -191,7 +163,7 @@ namespace Umbraco.Web.Security public bool IsAuthenticated() { var httpContext = _httpContextAccessor.HttpContext; - return httpContext?.User != null && httpContext.User.Identity.IsAuthenticated && httpContext.GetCurrentIdentity(false) != null; + return httpContext?.User != null && httpContext.User.Identity.IsAuthenticated && httpContext.GetCurrentIdentity() != null; } } diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 9e885eaca9..cae3aa077f 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -201,7 +201,6 @@ - diff --git a/src/Umbraco.Web/UmbracoInjectedModule.cs b/src/Umbraco.Web/UmbracoInjectedModule.cs index a63e305ce2..ecb67c997d 100644 --- a/src/Umbraco.Web/UmbracoInjectedModule.cs +++ b/src/Umbraco.Web/UmbracoInjectedModule.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Web; using System.Web.Routing; using Umbraco.Core; +using Umbraco.Core.Security; using Umbraco.Core.Cache; using Umbraco.Core.Configuration; using Umbraco.Core.Exceptions; From ea58dce644bd6bc6136704ff510719320ff17afd Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 2 Jun 2020 13:47:58 +1000 Subject: [PATCH 02/14] implements logic to log the user in on install --- .../CompositionExtensions/Installer.cs | 2 +- .../Install/InstallHelper.cs | 5 +- .../Install/InstallStepCollection.cs | 2 +- .../InstallSteps/CompleteInstallStep.cs | 31 +++++++++ .../InstallSteps/SetUmbracoVersionStep.cs | 67 ------------------- .../Install/InstallApiController.cs | 17 ++++- .../Install/InstallController.cs | 2 +- 7 files changed, 49 insertions(+), 77 deletions(-) create mode 100644 src/Umbraco.Infrastructure/Install/InstallSteps/CompleteInstallStep.cs delete mode 100644 src/Umbraco.Infrastructure/Install/InstallSteps/SetUmbracoVersionStep.cs diff --git a/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Installer.cs b/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Installer.cs index 0b9fb65606..585d6badfb 100644 --- a/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Installer.cs +++ b/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Installer.cs @@ -24,7 +24,7 @@ namespace Umbraco.Web.Composing.CompositionExtensions // composition.Register(Lifetime.Scope); // composition.Register(Lifetime.Scope); - composition.Register(Lifetime.Scope); + composition.Register(Lifetime.Scope); composition.Register(); composition.Register(); diff --git a/src/Umbraco.Infrastructure/Install/InstallHelper.cs b/src/Umbraco.Infrastructure/Install/InstallHelper.cs index b178a1e710..9c3eddbf2a 100644 --- a/src/Umbraco.Infrastructure/Install/InstallHelper.cs +++ b/src/Umbraco.Infrastructure/Install/InstallHelper.cs @@ -21,7 +21,6 @@ namespace Umbraco.Web.Install private static HttpClient _httpClient; private readonly DatabaseBuilder _databaseBuilder; private readonly ILogger _logger; - private readonly IGlobalSettings _globalSettings; private readonly IUmbracoVersion _umbracoVersion; private readonly IConnectionStrings _connectionStrings; private readonly IInstallationService _installationService; @@ -33,7 +32,6 @@ namespace Umbraco.Web.Install public InstallHelper(DatabaseBuilder databaseBuilder, ILogger logger, - IGlobalSettings globalSettings, IUmbracoVersion umbracoVersion, IConnectionStrings connectionStrings, IInstallationService installationService, @@ -43,7 +41,6 @@ namespace Umbraco.Web.Install IJsonSerializer jsonSerializer) { _logger = logger; - _globalSettings = globalSettings; _umbracoVersion = umbracoVersion; _databaseBuilder = databaseBuilder; _connectionStrings = connectionStrings ?? throw new ArgumentNullException(nameof(connectionStrings)); @@ -59,7 +56,7 @@ namespace Umbraco.Web.Install return _installationType ?? (_installationType = IsBrandNewInstall ? InstallationType.NewInstall : InstallationType.Upgrade).Value; } - public async Task InstallStatus(bool isCompleted, string errorMsg) + public async Task SetInstallStatusAsync(bool isCompleted, string errorMsg) { try { diff --git a/src/Umbraco.Infrastructure/Install/InstallStepCollection.cs b/src/Umbraco.Infrastructure/Install/InstallStepCollection.cs index f31f5f7dbb..523cd35a27 100644 --- a/src/Umbraco.Infrastructure/Install/InstallStepCollection.cs +++ b/src/Umbraco.Infrastructure/Install/InstallStepCollection.cs @@ -30,7 +30,7 @@ namespace Umbraco.Web.Install // a.OfType().First(), // a.OfType().First(), - a.OfType().First(), + a.OfType().First(), }; } diff --git a/src/Umbraco.Infrastructure/Install/InstallSteps/CompleteInstallStep.cs b/src/Umbraco.Infrastructure/Install/InstallSteps/CompleteInstallStep.cs new file mode 100644 index 0000000000..c95defe51a --- /dev/null +++ b/src/Umbraco.Infrastructure/Install/InstallSteps/CompleteInstallStep.cs @@ -0,0 +1,31 @@ +using System.Threading.Tasks; +using Umbraco.Web.Install.Models; + +namespace Umbraco.Web.Install.InstallSteps +{ + [InstallSetupStep(InstallationType.NewInstall | InstallationType.Upgrade, + "UmbracoVersion", 50, "Installation is complete! Get ready to be redirected to your new CMS.", + PerformsAppRestart = true)] + public class CompleteInstallStep : InstallSetupStep + { + private readonly InstallHelper _installHelper; + + public CompleteInstallStep(InstallHelper installHelper) + { + _installHelper = installHelper; + } + + public override async Task ExecuteAsync(object model) + { + //reports the ended install + await _installHelper.SetInstallStatusAsync(true, ""); + + return null; + } + + public override bool RequiresExecution(object model) + { + return true; + } + } +} diff --git a/src/Umbraco.Infrastructure/Install/InstallSteps/SetUmbracoVersionStep.cs b/src/Umbraco.Infrastructure/Install/InstallSteps/SetUmbracoVersionStep.cs deleted file mode 100644 index 190e24d64b..0000000000 --- a/src/Umbraco.Infrastructure/Install/InstallSteps/SetUmbracoVersionStep.cs +++ /dev/null @@ -1,67 +0,0 @@ -using System.Threading.Tasks; -using Umbraco.Core; -using Umbraco.Core.Configuration; -using Umbraco.Net; -using Umbraco.Web.Install.Models; - -namespace Umbraco.Web.Install.InstallSteps -{ - [InstallSetupStep(InstallationType.NewInstall | InstallationType.Upgrade, - "UmbracoVersion", 50, "Installation is complete! Get ready to be redirected to your new CMS.", - PerformsAppRestart = true)] - public class SetUmbracoVersionStep : InstallSetupStep - { - private readonly IUmbracoContextAccessor _umbracoContextAccessor; - private readonly InstallHelper _installHelper; - private readonly IGlobalSettings _globalSettings; - private readonly IUmbracoVersion _umbracoVersion; - - public SetUmbracoVersionStep(IUmbracoContextAccessor umbracoContextAccessor, InstallHelper installHelper, - IGlobalSettings globalSettings, IUmbracoVersion umbracoVersion) - { - _umbracoContextAccessor = umbracoContextAccessor; - _installHelper = installHelper; - _globalSettings = globalSettings; - _umbracoVersion = umbracoVersion; - } - - public override Task ExecuteAsync(object model) - { - //TODO: This needs to be reintroduced, when users are compatible with ASP.NET Core Identity. - // var security = _umbracoContextAccessor.GetRequiredUmbracoContext().Security; - // if (security.IsAuthenticated() == false && _globalSettings.ConfigurationStatus.IsNullOrWhiteSpace()) - // { - // security.PerformLogin(-1); - // } - // - // if (security.IsAuthenticated()) - // { - // // when a user is already logged in, we need to check whether it's user 'zero' - // // which is the legacy super user from v7 - and then we need to actually log the - // // true super user in - but before that we need to log off, else audit events - // // will try to reference user zero and fail - // var userIdAttempt = security.GetUserId(); - // if (userIdAttempt && userIdAttempt.Result == 0) - // { - // security.ClearCurrentLogin(); - // security.PerformLogin(Constants.Security.SuperUserId); - // } - // } - // else if (_globalSettings.ConfigurationStatus.IsNullOrWhiteSpace()) - // { - // // for installs, we need to log the super user in - // security.PerformLogin(Constants.Security.SuperUserId); - // } - - //reports the ended install - _installHelper.InstallStatus(true, ""); - - return Task.FromResult(null); - } - - public override bool RequiresExecution(object model) - { - return true; - } - } -} diff --git a/src/Umbraco.Web.Common/Install/InstallApiController.cs b/src/Umbraco.Web.Common/Install/InstallApiController.cs index c171b09fa8..c8f42e0d04 100644 --- a/src/Umbraco.Web.Common/Install/InstallApiController.cs +++ b/src/Umbraco.Web.Common/Install/InstallApiController.cs @@ -14,11 +14,13 @@ using Umbraco.Web.Common.Attributes; using Umbraco.Web.Common.Exceptions; using Umbraco.Web.Common.Filters; using Umbraco.Web.Common.ModelBinding; +using Umbraco.Web.Common.Security; using Umbraco.Web.Install; using Umbraco.Web.Install.Models; namespace Umbraco.Web.Common.Install { + using Constants = Umbraco.Core.Constants; [UmbracoApiController] [TypeFilter(typeof(HttpResponseExceptionFilter))] @@ -30,19 +32,21 @@ namespace Umbraco.Web.Common.Install private readonly DatabaseBuilder _databaseBuilder; private readonly InstallStatusTracker _installStatusTracker; private readonly IUmbracoApplicationLifetime _umbracoApplicationLifetime; + private readonly BackOfficeSignInManager _backOfficeSignInManager; private readonly InstallStepCollection _installSteps; private readonly ILogger _logger; private readonly IProfilingLogger _proflog; public InstallApiController(DatabaseBuilder databaseBuilder, IProfilingLogger proflog, InstallHelper installHelper, InstallStepCollection installSteps, InstallStatusTracker installStatusTracker, - IUmbracoApplicationLifetime umbracoApplicationLifetime) + IUmbracoApplicationLifetime umbracoApplicationLifetime, BackOfficeSignInManager backOfficeSignInManager) { _databaseBuilder = databaseBuilder ?? throw new ArgumentNullException(nameof(databaseBuilder)); _proflog = proflog ?? throw new ArgumentNullException(nameof(proflog)); _installSteps = installSteps; _installStatusTracker = installStatusTracker; _umbracoApplicationLifetime = umbracoApplicationLifetime; + _backOfficeSignInManager = backOfficeSignInManager; InstallHelper = installHelper; _logger = _proflog; } @@ -85,10 +89,17 @@ namespace Umbraco.Web.Common.Install return starterKits; } - [HttpPost] - public ActionResult CompleteInstall() + public async Task CompleteInstall() { + // log the super user in if it's a new install + var installType = InstallHelper.GetInstallationType(); + if (installType == InstallationType.NewInstall) + { + var user = await _backOfficeSignInManager.UserManager.FindByIdAsync(Constants.Security.SuperUserId.ToString()); + await _backOfficeSignInManager.SignInAsync(user, false); + } + _umbracoApplicationLifetime.Restart(); return NoContent(); } diff --git a/src/Umbraco.Web.Common/Install/InstallController.cs b/src/Umbraco.Web.Common/Install/InstallController.cs index a4f659379f..4ea3452d92 100644 --- a/src/Umbraco.Web.Common/Install/InstallController.cs +++ b/src/Umbraco.Web.Common/Install/InstallController.cs @@ -87,7 +87,7 @@ namespace Umbraco.Web.Common.Install ViewData.SetUmbracoVersion(_umbracoVersion.SemanticVersion); - await _installHelper.InstallStatus(false, ""); + await _installHelper.SetInstallStatusAsync(false, ""); return View(); } From 4afaade436d74887ce3d98abc21e27a9108d4807 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 2 Jun 2020 14:46:58 +1000 Subject: [PATCH 03/14] Gets WebSecurity implemented --- .../PublishedContentCacheTests.cs | 2 +- .../PublishedContentSnapshotTestBase.cs | 2 +- .../Scoping/ScopedNuCacheTests.cs | 2 +- .../Security/BackOfficeCookieManagerTests.cs | 4 +- .../TestHelpers/TestWithDatabaseBase.cs | 3 +- .../Web/Mvc/UmbracoViewPageTests.cs | 2 +- .../Web/WebExtensionMethodTests.cs | 6 +- .../Security/WebSecurity.cs | 23 +-- src/Umbraco.Web/Security/WebSecurity.cs | 159 +++--------------- src/Umbraco.Web/Umbraco.Web.csproj | 2 +- src/Umbraco.Web/UmbracoContextFactory.cs | 4 +- 11 files changed, 44 insertions(+), 165 deletions(-) diff --git a/src/Umbraco.Tests/Cache/PublishedCache/PublishedContentCacheTests.cs b/src/Umbraco.Tests/Cache/PublishedCache/PublishedContentCacheTests.cs index 524a939574..932a974b75 100644 --- a/src/Umbraco.Tests/Cache/PublishedCache/PublishedContentCacheTests.cs +++ b/src/Umbraco.Tests/Cache/PublishedCache/PublishedContentCacheTests.cs @@ -79,7 +79,7 @@ namespace Umbraco.Tests.Cache.PublishedCache _umbracoContext = new UmbracoContext( httpContextAccessor, publishedSnapshotService.Object, - new WebSecurity(httpContextAccessor, Mock.Of(), globalSettings, HostingEnvironment), + Mock.Of(), globalSettings, HostingEnvironment, new TestVariationContextAccessor(), diff --git a/src/Umbraco.Tests/PublishedContent/PublishedContentSnapshotTestBase.cs b/src/Umbraco.Tests/PublishedContent/PublishedContentSnapshotTestBase.cs index 02d380d843..5af889bcc0 100644 --- a/src/Umbraco.Tests/PublishedContent/PublishedContentSnapshotTestBase.cs +++ b/src/Umbraco.Tests/PublishedContent/PublishedContentSnapshotTestBase.cs @@ -74,7 +74,7 @@ namespace Umbraco.Tests.PublishedContent var umbracoContext = new UmbracoContext( httpContextAccessor, publishedSnapshotService.Object, - new WebSecurity(httpContextAccessor, ServiceContext.UserService, globalSettings, HostingEnvironment), + Mock.Of(), globalSettings, HostingEnvironment, new TestVariationContextAccessor(), diff --git a/src/Umbraco.Tests/Scoping/ScopedNuCacheTests.cs b/src/Umbraco.Tests/Scoping/ScopedNuCacheTests.cs index 0113237486..49bca378c7 100644 --- a/src/Umbraco.Tests/Scoping/ScopedNuCacheTests.cs +++ b/src/Umbraco.Tests/Scoping/ScopedNuCacheTests.cs @@ -123,7 +123,7 @@ namespace Umbraco.Tests.Scoping var umbracoContext = new UmbracoContext( httpContextAccessor, service, - new WebSecurity(httpContextAccessor, ServiceContext.UserService, globalSettings, HostingEnvironment), + Mock.Of(), globalSettings, HostingEnvironment, new TestVariationContextAccessor(), diff --git a/src/Umbraco.Tests/Security/BackOfficeCookieManagerTests.cs b/src/Umbraco.Tests/Security/BackOfficeCookieManagerTests.cs index 988dfb178d..1e451cf57d 100644 --- a/src/Umbraco.Tests/Security/BackOfficeCookieManagerTests.cs +++ b/src/Umbraco.Tests/Security/BackOfficeCookieManagerTests.cs @@ -34,7 +34,7 @@ namespace Umbraco.Tests.Security var umbracoContext = new UmbracoContext( httpContextAccessor, Mock.Of(), - new WebSecurity(httpContextAccessor, ServiceContext.UserService, globalSettings, HostingEnvironment), + Mock.Of(), globalSettings, HostingEnvironment, new TestVariationContextAccessor(), @@ -58,7 +58,7 @@ namespace Umbraco.Tests.Security var umbCtx = new UmbracoContext( httpContextAccessor, Mock.Of(), - new WebSecurity(httpContextAccessor, ServiceContext.UserService, globalSettings, HostingEnvironment), + Mock.Of(), globalSettings, HostingEnvironment, new TestVariationContextAccessor(), diff --git a/src/Umbraco.Tests/TestHelpers/TestWithDatabaseBase.cs b/src/Umbraco.Tests/TestHelpers/TestWithDatabaseBase.cs index 369f5db86e..cfcdacdadf 100644 --- a/src/Umbraco.Tests/TestHelpers/TestWithDatabaseBase.cs +++ b/src/Umbraco.Tests/TestHelpers/TestWithDatabaseBase.cs @@ -374,8 +374,7 @@ namespace Umbraco.Tests.TestHelpers var umbracoContext = new UmbracoContext( httpContextAccessor, service, - new WebSecurity(httpContextAccessor, Factory.GetInstance(), - Factory.GetInstance(), HostingEnvironment), + Mock.Of(), globalSettings ?? Factory.GetInstance(), HostingEnvironment, new TestVariationContextAccessor(), diff --git a/src/Umbraco.Tests/Web/Mvc/UmbracoViewPageTests.cs b/src/Umbraco.Tests/Web/Mvc/UmbracoViewPageTests.cs index dbe45ccd29..6f1a073eca 100644 --- a/src/Umbraco.Tests/Web/Mvc/UmbracoViewPageTests.cs +++ b/src/Umbraco.Tests/Web/Mvc/UmbracoViewPageTests.cs @@ -438,7 +438,7 @@ namespace Umbraco.Tests.Web.Mvc var ctx = new UmbracoContext( httpContextAccessor, _service, - new WebSecurity(httpContextAccessor, ServiceContext.UserService, globalSettings, HostingEnvironment), + Mock.Of(), globalSettings, HostingEnvironment, new TestVariationContextAccessor(), diff --git a/src/Umbraco.Tests/Web/WebExtensionMethodTests.cs b/src/Umbraco.Tests/Web/WebExtensionMethodTests.cs index a9610c6de2..32d8e0917b 100644 --- a/src/Umbraco.Tests/Web/WebExtensionMethodTests.cs +++ b/src/Umbraco.Tests/Web/WebExtensionMethodTests.cs @@ -32,7 +32,7 @@ namespace Umbraco.Tests.Web var umbCtx = new UmbracoContext( httpContextAccessor, Mock.Of(), - new WebSecurity(httpContextAccessor, ServiceContext.UserService, TestObjects.GetGlobalSettings(), HostingEnvironment), + Mock.Of(), TestObjects.GetGlobalSettings(), HostingEnvironment, new TestVariationContextAccessor(), @@ -53,7 +53,7 @@ namespace Umbraco.Tests.Web var umbCtx = new UmbracoContext( httpContextAccessor, Mock.Of(), - new WebSecurity(httpContextAccessor, ServiceContext.UserService, TestObjects.GetGlobalSettings(), HostingEnvironment), + Mock.Of(), TestObjects.GetGlobalSettings(), HostingEnvironment, new TestVariationContextAccessor(), @@ -84,7 +84,7 @@ namespace Umbraco.Tests.Web var umbCtx = new UmbracoContext( httpContextAccessor, Mock.Of(), - new WebSecurity(httpContextAccessor, ServiceContext.UserService, TestObjects.GetGlobalSettings(), HostingEnvironment), + Mock.Of(), TestObjects.GetGlobalSettings(), HostingEnvironment, new TestVariationContextAccessor(), diff --git a/src/Umbraco.Web.Common/Security/WebSecurity.cs b/src/Umbraco.Web.Common/Security/WebSecurity.cs index 5f4780a387..c67d5a9b4f 100644 --- a/src/Umbraco.Web.Common/Security/WebSecurity.cs +++ b/src/Umbraco.Web.Common/Security/WebSecurity.cs @@ -1,9 +1,6 @@ using System; -using System.Collections.Generic; using System.Security; -using System.Text; using Microsoft.AspNetCore.Http; -using Umbraco.Composing; using Umbraco.Core; using Umbraco.Core.Configuration; using Umbraco.Core.Hosting; @@ -11,10 +8,10 @@ using Umbraco.Core.Models.Membership; using Umbraco.Core.Services; using Umbraco.Extensions; using Umbraco.Web.Security; +using Umbraco.Core.Models; namespace Umbraco.Web.Common.Security { - // TODO: need to implement this public class WebSecurity : IWebSecurity { @@ -37,10 +34,7 @@ namespace Umbraco.Web.Common.Security private IUser _currentUser; - /// - /// Gets the current user. - /// - /// The current user. + /// public IUser CurrentUser { get @@ -56,6 +50,7 @@ namespace Umbraco.Web.Common.Security } } + /// public ValidateRequestAttempt AuthorizeRequest(bool throwExceptions = false) { // check for secure connection @@ -67,27 +62,33 @@ namespace Umbraco.Web.Common.Security return ValidateCurrentUser(throwExceptions); } + /// public Attempt GetUserId() { - return Attempt.Succeed(-1); + var identity = _httpContextAccessor.GetRequiredHttpContext().GetCurrentIdentity(); + return identity == null ? Attempt.Fail() : Attempt.Succeed(identity.Id); } + /// public bool IsAuthenticated() { var httpContext = _httpContextAccessor.HttpContext; return httpContext?.User != null && httpContext.User.Identity.IsAuthenticated && httpContext.GetCurrentIdentity() != null; } + /// public bool UserHasSectionAccess(string section, IUser user) { - return true; + return user.HasSectionAccess(section); } + /// public bool ValidateCurrentUser() { - return true; + return ValidateCurrentUser(false, true) == ValidateRequestAttempt.Success; } + /// public ValidateRequestAttempt ValidateCurrentUser(bool throwExceptions, bool requiresApproval = true) { //This will first check if the current user is already authenticated - which should be the case in nearly all circumstances diff --git a/src/Umbraco.Web/Security/WebSecurity.cs b/src/Umbraco.Web/Security/WebSecurity.cs index 143f195de0..5776ec876a 100644 --- a/src/Umbraco.Web/Security/WebSecurity.cs +++ b/src/Umbraco.Web/Security/WebSecurity.cs @@ -13,158 +13,39 @@ using Umbraco.Core.Models; namespace Umbraco.Web.Security { - /// - /// A utility class used for dealing with USER security in Umbraco - /// + // NOTE: Moved to netcore public class WebSecurity : IWebSecurity { - private readonly IHttpContextAccessor _httpContextAccessor; - private readonly IUserService _userService; - private readonly IGlobalSettings _globalSettings; - private readonly IHostingEnvironment _hostingEnvironment; + public IUser CurrentUser => throw new NotImplementedException(); - public WebSecurity(IHttpContextAccessor httpContextAccessor, IUserService userService, IGlobalSettings globalSettings, IHostingEnvironment hostingEnvironment) - { - _httpContextAccessor = httpContextAccessor; - _userService = userService; - _globalSettings = globalSettings; - _hostingEnvironment = hostingEnvironment; - } - - private IUser _currentUser; - - /// - /// Gets the current user. - /// - /// The current user. - public IUser CurrentUser - { - get - { - //only load it once per instance! (but make sure groups are loaded) - if (_currentUser == null) - { - var id = GetUserId(); - _currentUser = id ? _userService.GetUserById(id.Result) : null; - } - - return _currentUser; - } - } - - private BackOfficeSignInManager _signInManager; - private BackOfficeSignInManager SignInManager - { - get - { - if (_signInManager == null) - { - var mgr = _httpContextAccessor.GetRequiredHttpContext().GetOwinContext().Get(); - if (mgr == null) - { - throw new NullReferenceException("Could not resolve an instance of " + typeof(BackOfficeSignInManager) + " from the " + typeof(IOwinContext)); - } - _signInManager = mgr; - } - return _signInManager; - } - } - - private BackOfficeOwinUserManager _userManager; - protected BackOfficeOwinUserManager UserManager - => _userManager ?? (_userManager = _httpContextAccessor.GetRequiredHttpContext().GetOwinContext().GetBackOfficeUserManager()); - - /// - /// Gets the current user's id. - /// - /// - public Attempt GetUserId() - { - var identity = _httpContextAccessor.GetRequiredHttpContext().GetCurrentIdentity(); - return identity == null ? Attempt.Fail() : Attempt.Succeed(Convert.ToInt32(identity.Id)); - } - - /// - /// Validates the currently logged in user and ensures they are not timed out - /// - /// - public bool ValidateCurrentUser() - { - return ValidateCurrentUser(false, true) == ValidateRequestAttempt.Success; - } - - /// - /// Validates the current user assigned to the request and ensures the stored user data is valid - /// - /// set to true if you want exceptions to be thrown if failed - /// If true requires that the user is approved to be validated - /// - public ValidateRequestAttempt ValidateCurrentUser(bool throwExceptions, bool requiresApproval = true) - { - //This will first check if the current user is already authenticated - which should be the case in nearly all circumstances - // since the authentication happens in the Module, that authentication also checks the ticket expiry. We don't - // need to check it a second time because that requires another decryption phase and nothing can tamper with it during the request. - - if (IsAuthenticated() == false) - { - //There is no user - if (throwExceptions) throw new InvalidOperationException("The user has no umbraco contextid - try logging in"); - return ValidateRequestAttempt.FailedNoContextId; - } - - var user = CurrentUser; - - // Check for console access - if (user == null || (requiresApproval && user.IsApproved == false) || (user.IsLockedOut && RequestIsInUmbracoApplication(_httpContextAccessor, _globalSettings, _hostingEnvironment))) - { - if (throwExceptions) throw new ArgumentException("You have no privileges to the umbraco console. Please contact your administrator"); - return ValidateRequestAttempt.FailedNoPrivileges; - } - return ValidateRequestAttempt.Success; - - } - - private static bool RequestIsInUmbracoApplication(IHttpContextAccessor httpContextAccessor, IGlobalSettings globalSettings, IHostingEnvironment hostingEnvironment) - { - return httpContextAccessor.GetRequiredHttpContext().Request.Path.ToLower().IndexOf(hostingEnvironment.ToAbsolute(globalSettings.UmbracoPath).ToLower(), StringComparison.Ordinal) > -1; - } - - /// - /// Authorizes the full request, checks for SSL and validates the current user - /// - /// set to true if you want exceptions to be thrown if failed - /// public ValidateRequestAttempt AuthorizeRequest(bool throwExceptions = false) { - // check for secure connection - if (_globalSettings.UseHttps && _httpContextAccessor.GetRequiredHttpContext().Request.IsSecureConnection == false) - { - if (throwExceptions) throw new SecurityException("This installation requires a secure connection (via SSL). Please update the URL to include https://"); - return ValidateRequestAttempt.FailedNoSsl; - } - return ValidateCurrentUser(throwExceptions); + throw new NotImplementedException(); } - /// - /// Checks if the specified user as access to the app - /// - /// - /// - /// - public bool UserHasSectionAccess(string section, IUser user) + public Attempt GetUserId() { - return user.HasSectionAccess(section); + throw new NotImplementedException(); } - /// - /// Ensures that a back office user is logged in - /// - /// public bool IsAuthenticated() { - var httpContext = _httpContextAccessor.HttpContext; - return httpContext?.User != null && httpContext.User.Identity.IsAuthenticated && httpContext.GetCurrentIdentity() != null; + throw new NotImplementedException(); } + public bool UserHasSectionAccess(string section, IUser user) + { + throw new NotImplementedException(); + } + + public bool ValidateCurrentUser() + { + throw new NotImplementedException(); + } + + public ValidateRequestAttempt ValidateCurrentUser(bool throwExceptions, bool requiresApproval = true) + { + throw new NotImplementedException(); + } } } diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index cae3aa077f..d9e821a4fd 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -151,6 +151,7 @@ + @@ -418,7 +419,6 @@ - diff --git a/src/Umbraco.Web/UmbracoContextFactory.cs b/src/Umbraco.Web/UmbracoContextFactory.cs index 719785c1fb..bfb4dd627d 100644 --- a/src/Umbraco.Web/UmbracoContextFactory.cs +++ b/src/Umbraco.Web/UmbracoContextFactory.cs @@ -67,9 +67,7 @@ namespace Umbraco.Web _variationContextAccessor.VariationContext = new VariationContext(_defaultCultureAccessor.DefaultCulture); } - var webSecurity = new WebSecurity(_httpContextAccessor, _userService, _globalSettings, _hostingEnvironment); - - return new UmbracoContext(_httpContextAccessor, _publishedSnapshotService, webSecurity, _globalSettings, _hostingEnvironment, _variationContextAccessor, _uriUtility, _cookieManager); + return new UmbracoContext(_httpContextAccessor, _publishedSnapshotService, new WebSecurity(), _globalSettings, _hostingEnvironment, _variationContextAccessor, _uriUtility, _cookieManager); } /// From 22c842f2e5b28641cc082151044e7b9ffdab7bfd Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 2 Jun 2020 14:51:27 +1000 Subject: [PATCH 04/14] fixes service registration --- src/Umbraco.Web.BackOffice/Runtime/BackOfficeComposer.cs | 4 ++-- .../Security/BackOfficeSessionIdValidator.cs | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Web.BackOffice/Runtime/BackOfficeComposer.cs b/src/Umbraco.Web.BackOffice/Runtime/BackOfficeComposer.cs index 2d6a46d903..ccbec26f4a 100644 --- a/src/Umbraco.Web.BackOffice/Runtime/BackOfficeComposer.cs +++ b/src/Umbraco.Web.BackOffice/Runtime/BackOfficeComposer.cs @@ -16,8 +16,8 @@ namespace Umbraco.Web.BackOffice.Runtime { composition.RegisterUnique(); composition.RegisterUnique(); - composition.RegisterUnique(); - composition.RegisterUnique(); + composition.Register(Lifetime.Request); + composition.Register(Lifetime.Request); composition.RegisterUnique(); } diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficeSessionIdValidator.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficeSessionIdValidator.cs index 70bcf57954..fdf630e01c 100644 --- a/src/Umbraco.Web.BackOffice/Security/BackOfficeSessionIdValidator.cs +++ b/src/Umbraco.Web.BackOffice/Security/BackOfficeSessionIdValidator.cs @@ -19,14 +19,19 @@ namespace Umbraco.Web.BackOffice.Security using ICookieManager = Microsoft.AspNetCore.Authentication.Cookies.ICookieManager; /// - /// Static helper class used to configure a CookieAuthenticationProvider to validate a cookie against a user's session id + /// Used to validate a cookie against a user's session id /// /// + /// /// This uses another cookie to track the last checked time which is done for a few reasons: /// * We can't use the user's auth ticket to do this because we'd be re-issuing the auth ticket all of the time and it would never expire /// plus the auth ticket size is much larger than this small value /// * This will execute quite often (every minute per user) and in some cases there might be several requests that end up re-issuing the cookie so the cookie value should be small /// * We want to avoid the user lookup if it's not required so that will only happen when the time diff is great enough in the cookie + /// + /// + /// This is a scoped/request based object. + /// /// public class BackOfficeSessionIdValidator { From 670d62a2dcecc22535ee04694a3514f731826273 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 2 Jun 2020 17:48:08 +1000 Subject: [PATCH 05/14] Gets cookie authentication actually working along with session validation --- .../BackOffice/ClaimsPrincipalExtensions.cs | 5 +- .../BackOffice/UmbracoBackOfficeIdentity.cs | 45 ++----- src/Umbraco.Core/Constants-Security.cs | 5 + src/Umbraco.Core/Constants-Web.cs | 5 - .../BackOfficeClaimsPrincipalFactory.cs | 9 -- .../BackOfficeClaimsPrincipalFactoryTests.cs | 2 +- .../UmbracoBackOfficeIdentityTests.cs | 12 +- .../Security/BackOfficeAntiforgeryTests.cs | 2 +- .../AuthenticateEverythingMiddleware.cs | 4 +- .../Controllers/BackOfficeController.cs | 6 +- ...oBackOfficeApplicationBuilderExtensions.cs | 2 - .../ConfigureBackOfficeCookieOptions.cs | 19 ++- .../ConfigureBackOfficeIdentityOptions.cs | 2 +- .../ApplicationBuilderExtensions.cs | 5 + .../Middleware/UmbracoRequestMiddleware.cs | 3 + .../Editors/AuthenticationController.cs | 15 +-- .../Editors/BackOfficeController.cs | 36 ------ .../Security/BackOfficeOwinUserManager.cs | 2 +- .../Security/GetUserSecondsMiddleWare.cs | 5 +- .../Security/SessionIdValidator.cs | 118 ------------------ src/Umbraco.Web/Umbraco.Web.csproj | 1 - 21 files changed, 58 insertions(+), 245 deletions(-) delete mode 100644 src/Umbraco.Web/Security/SessionIdValidator.cs diff --git a/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs b/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs index 3b09b935a5..fa89ee6975 100644 --- a/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs +++ b/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs @@ -34,8 +34,9 @@ namespace Umbraco.Extensions return UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity); } catch (InvalidOperationException) - { - // TODO: Look into this? Why did we do this, see git history and add some notes + { + // We catch this error because it's what we throw when the required claims are not in the identity. + // when that happens something strange is going on, we'll swallow this exception and return null. } } diff --git a/src/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentity.cs b/src/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentity.cs index c1ec692339..18684e072a 100644 --- a/src/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentity.cs +++ b/src/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentity.cs @@ -9,10 +9,6 @@ namespace Umbraco.Core.BackOffice /// /// A custom user identity for the Umbraco backoffice /// - /// - /// This inherits from FormsIdentity for backwards compatibility reasons since we still support the forms auth cookie, in v8 we can - /// change over to 'pure' asp.net identity and just inherit from ClaimsIdentity. - /// [Serializable] public class UmbracoBackOfficeIdentity : ClaimsIdentity { @@ -30,22 +26,20 @@ namespace Umbraco.Core.BackOffice /// /// /// - /// /// /// /// public UmbracoBackOfficeIdentity(int userId, string username, string realName, IEnumerable startContentNodes, IEnumerable startMediaNodes, string culture, - string sessionId, string securityStamp, IEnumerable allowedApps, IEnumerable roles) + string securityStamp, IEnumerable allowedApps, IEnumerable roles) : base(Enumerable.Empty(), Constants.Security.BackOfficeAuthenticationType) //this ctor is used to ensure the IsAuthenticated property is true { if (allowedApps == null) throw new ArgumentNullException(nameof(allowedApps)); if (string.IsNullOrWhiteSpace(username)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(username)); if (string.IsNullOrWhiteSpace(realName)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(realName)); if (string.IsNullOrWhiteSpace(culture)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(culture)); - if (string.IsNullOrWhiteSpace(sessionId)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(sessionId)); if (string.IsNullOrWhiteSpace(securityStamp)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(securityStamp)); - AddRequiredClaims(userId, username, realName, startContentNodes, startMediaNodes, culture, sessionId, securityStamp, allowedApps, roles); + AddRequiredClaims(userId, username, realName, startContentNodes, startMediaNodes, culture, securityStamp, allowedApps, roles); } /// @@ -60,23 +54,21 @@ namespace Umbraco.Core.BackOffice /// /// /// - /// /// /// /// public UmbracoBackOfficeIdentity(ClaimsIdentity childIdentity, int userId, string username, string realName, IEnumerable startContentNodes, IEnumerable startMediaNodes, string culture, - string sessionId, string securityStamp, IEnumerable allowedApps, IEnumerable roles) + string securityStamp, IEnumerable allowedApps, IEnumerable roles) : base(childIdentity.Claims, Constants.Security.BackOfficeAuthenticationType) { if (string.IsNullOrWhiteSpace(username)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(username)); if (string.IsNullOrWhiteSpace(realName)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(realName)); if (string.IsNullOrWhiteSpace(culture)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(culture)); - if (string.IsNullOrWhiteSpace(sessionId)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(sessionId)); if (string.IsNullOrWhiteSpace(securityStamp)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(securityStamp)); Actor = childIdentity; - AddRequiredClaims(userId, username, realName, startContentNodes, startMediaNodes, culture, sessionId, securityStamp, allowedApps, roles); + AddRequiredClaims(userId, username, realName, startContentNodes, startMediaNodes, culture, securityStamp, allowedApps, roles); } /// @@ -115,8 +107,7 @@ namespace Umbraco.Core.BackOffice Constants.Security.StartContentNodeIdClaimType, Constants.Security.StartMediaNodeIdClaimType, ClaimTypes.Locality, - Constants.Security.SessionIdClaimType, - Constants.Web.SecurityStampClaimType + Constants.Security.SecurityStampClaimType }; /// @@ -124,7 +115,7 @@ namespace Umbraco.Core.BackOffice /// private void AddRequiredClaims(int userId, string username, string realName, IEnumerable startContentNodes, IEnumerable startMediaNodes, string culture, - string sessionId, string securityStamp, IEnumerable allowedApps, IEnumerable roles) + string securityStamp, IEnumerable allowedApps, IEnumerable roles) { //This is the id that 'identity' uses to check for the user id if (HasClaim(x => x.Type == ClaimTypes.NameIdentifier) == false) @@ -155,13 +146,9 @@ namespace Umbraco.Core.BackOffice if (HasClaim(x => x.Type == ClaimTypes.Locality) == false) AddClaim(new Claim(ClaimTypes.Locality, culture, ClaimValueTypes.String, Issuer, Issuer, this)); - if (HasClaim(x => x.Type == Constants.Security.SessionIdClaimType) == false && SessionId.IsNullOrWhiteSpace() == false) - AddClaim(new Claim(Constants.Security.SessionIdClaimType, sessionId, ClaimValueTypes.String, Issuer, Issuer, this)); - - //The security stamp claim is also required... this is because this claim type is hard coded - // by the SecurityStampValidator, see: https://katanaproject.codeplex.com/workitem/444 - if (HasClaim(x => x.Type == Constants.Web.SecurityStampClaimType) == false) - AddClaim(new Claim(Constants.Web.SecurityStampClaimType, securityStamp, ClaimValueTypes.String, Issuer, Issuer, this)); + //The security stamp claim is also required + if (HasClaim(x => x.Type == Constants.Security.SecurityStampClaimType) == false) + AddClaim(new Claim(Constants.Security.SecurityStampClaimType, securityStamp, ClaimValueTypes.String, Issuer, Issuer, this)); //Add each app as a separate claim if (HasClaim(x => x.Type == Constants.Security.AllowedApplicationsClaimType) == false && allowedApps != null) @@ -211,19 +198,7 @@ namespace Umbraco.Core.BackOffice public string Culture => this.FindFirstValue(ClaimTypes.Locality); - public string SessionId - { - get => this.FindFirstValue(Constants.Security.SessionIdClaimType); - set - { - var existing = FindFirst(Constants.Security.SessionIdClaimType); - if (existing != null) - TryRemoveClaim(existing); - AddClaim(new Claim(Constants.Security.SessionIdClaimType, value, ClaimValueTypes.String, Issuer, Issuer, this)); - } - } - - public string SecurityStamp => this.FindFirstValue(Constants.Web.SecurityStampClaimType); + public string SecurityStamp => this.FindFirstValue(Constants.Security.SecurityStampClaimType); public string[] Roles => this.FindAll(x => x.Type == DefaultRoleClaimType).Select(role => role.Value).ToArray(); diff --git a/src/Umbraco.Core/Constants-Security.cs b/src/Umbraco.Core/Constants-Security.cs index 491f257f5c..534070be6e 100644 --- a/src/Umbraco.Core/Constants-Security.cs +++ b/src/Umbraco.Core/Constants-Security.cs @@ -53,6 +53,11 @@ public const string AllowedApplicationsClaimType = "http://umbraco.org/2015/02/identity/claims/backoffice/allowedapp"; public const string SessionIdClaimType = "http://umbraco.org/2015/02/identity/claims/backoffice/sessionid"; + /// + /// The claim type for the ASP.NET Identity security stamp + /// + public const string SecurityStampClaimType = "AspNet.Identity.SecurityStamp"; + public const string AspNetCoreV3PasswordHashAlgorithmName = "PBKDF2.ASPNETCORE.V3"; public const string AspNetCoreV2PasswordHashAlgorithmName = "PBKDF2.ASPNETCORE.V2"; public const string AspNetUmbraco8PasswordHashAlgorithmName = "HMACSHA256"; diff --git a/src/Umbraco.Core/Constants-Web.cs b/src/Umbraco.Core/Constants-Web.cs index b263de816b..7e96c6a912 100644 --- a/src/Umbraco.Core/Constants-Web.cs +++ b/src/Umbraco.Core/Constants-Web.cs @@ -40,11 +40,6 @@ /// public const string NoContentRouteName = "umbraco-no-content"; - /// - /// The claim type for the ASP.NET Identity security stamp - /// - public const string SecurityStampClaimType = "AspNet.Identity.SecurityStamp"; - /// /// The default authentication type used for remembering that 2FA is not needed on next login /// diff --git a/src/Umbraco.Infrastructure/BackOffice/BackOfficeClaimsPrincipalFactory.cs b/src/Umbraco.Infrastructure/BackOffice/BackOfficeClaimsPrincipalFactory.cs index 8caf8f837a..31e9a7775b 100644 --- a/src/Umbraco.Infrastructure/BackOffice/BackOfficeClaimsPrincipalFactory.cs +++ b/src/Umbraco.Infrastructure/BackOffice/BackOfficeClaimsPrincipalFactory.cs @@ -4,16 +4,12 @@ using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Options; -using Umbraco.Core.BackOffice; namespace Umbraco.Core.BackOffice { public class BackOfficeClaimsPrincipalFactory : UserClaimsPrincipalFactory where TUser : BackOfficeIdentityUser { - private const string _identityProviderClaimType = "http://schemas.microsoft.com/accesscontrolservice/2010/07/claims/identityprovider"; - private const string _identityProviderClaimValue = "ASP.NET Identity"; - public BackOfficeClaimsPrincipalFactory(UserManager userManager, IOptions optionsAccessor) : base(userManager, optionsAccessor) { @@ -25,9 +21,6 @@ namespace Umbraco.Core.BackOffice var baseIdentity = await base.GenerateClaimsAsync(user); - // Required by ASP.NET 4.x anti-forgery implementation - baseIdentity.AddClaim(new Claim(_identityProviderClaimType, _identityProviderClaimValue)); - var umbracoIdentity = new UmbracoBackOfficeIdentity( baseIdentity, user.Id, @@ -36,8 +29,6 @@ namespace Umbraco.Core.BackOffice user.CalculatedContentStartNodeIds, user.CalculatedMediaStartNodeIds, user.Culture, - //NOTE - there is no session id assigned here, this is just creating the identity, a session id will be generated when the cookie is written - Guid.NewGuid().ToString(), user.SecurityStamp, user.AllowedSections, user.Roles.Select(x => x.RoleId).ToArray()); diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/BackOfficeClaimsPrincipalFactoryTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/BackOfficeClaimsPrincipalFactoryTests.cs index 34aa5047bc..db7e7379aa 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/BackOfficeClaimsPrincipalFactoryTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/BackOfficeClaimsPrincipalFactoryTests.cs @@ -110,7 +110,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice [Test] public async Task CreateAsync_When_SecurityStamp_Supported_Expect_SecurityStamp_Claim() { - const string expectedClaimType = Constants.Web.SecurityStampClaimType; + const string expectedClaimType = Constants.Security.SecurityStampClaimType; var expectedClaimValue = _testUser.SecurityStamp; _mockUserManager.Setup(x => x.SupportsUserSecurityStamp).Returns(true); diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs index 5d0cec0e6e..d3380c06e4 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs @@ -33,13 +33,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice new Claim(ClaimTypes.Locality, "en-us", ClaimValueTypes.String, TestIssuer, TestIssuer), new Claim(Constants.Security.SessionIdClaimType, sessionId, Constants.Security.SessionIdClaimType, TestIssuer, TestIssuer), new Claim(ClaimsIdentity.DefaultRoleClaimType, "admin", ClaimValueTypes.String, TestIssuer, TestIssuer), - new Claim(Constants.Web.SecurityStampClaimType, securityStamp, ClaimValueTypes.String, TestIssuer, TestIssuer), + new Claim(Constants.Security.SecurityStampClaimType, securityStamp, ClaimValueTypes.String, TestIssuer, TestIssuer), }); var backofficeIdentity = UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity); Assert.AreEqual(1234, backofficeIdentity.Id); - Assert.AreEqual(sessionId, backofficeIdentity.SessionId); + //Assert.AreEqual(sessionId, backofficeIdentity.SessionId); Assert.AreEqual(securityStamp, backofficeIdentity.SecurityStamp); Assert.AreEqual("testing", backofficeIdentity.Username); Assert.AreEqual("hello world", backofficeIdentity.RealName); @@ -90,7 +90,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice [Test] public void Create_With_Claims_And_User_Data() { - var sessionId = Guid.NewGuid().ToString(); + var securityStamp = Guid.NewGuid().ToString(); var claimsIdentity = new ClaimsIdentity(new[] { @@ -99,7 +99,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice }); var identity = new UmbracoBackOfficeIdentity(claimsIdentity, - 1234, "testing", "hello world", new[] { 654 }, new[] { 654 }, "en-us", sessionId, sessionId, new[] { "content", "media" }, new[] { "admin" }); + 1234, "testing", "hello world", new[] { 654 }, new[] { 654 }, "en-us", securityStamp, new[] { "content", "media" }, new[] { "admin" }); Assert.AreEqual(12, identity.Claims.Count()); } @@ -108,10 +108,10 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice [Test] public void Clone() { - var sessionId = Guid.NewGuid().ToString(); + var securityStamp = Guid.NewGuid().ToString(); var identity = new UmbracoBackOfficeIdentity( - 1234, "testing", "hello world", new[] { 654 }, new[] { 654 }, "en-us", sessionId, sessionId, new[] { "content", "media" }, new[] { "admin" }); + 1234, "testing", "hello world", new[] { 654 }, new[] { 654 }, "en-us", securityStamp, new[] { "content", "media" }, new[] { "admin" }); var cloned = identity.Clone(); diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgeryTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgeryTests.cs index 0ae8ff9610..d93bc01b4e 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgeryTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgeryTests.cs @@ -26,7 +26,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Security var httpContext = new DefaultHttpContext() { User = new ClaimsPrincipal(new UmbracoBackOfficeIdentity(-1, "test", "test", Enumerable.Empty(), Enumerable.Empty(), "en-US", - Guid.NewGuid().ToString(), Guid.NewGuid().ToString(), Enumerable.Empty(), Enumerable.Empty())) + Guid.NewGuid().ToString(), Enumerable.Empty(), Enumerable.Empty())) }; httpContext.Request.IsHttps = true; return httpContext; diff --git a/src/Umbraco.Tests/TestHelpers/ControllerTesting/AuthenticateEverythingMiddleware.cs b/src/Umbraco.Tests/TestHelpers/ControllerTesting/AuthenticateEverythingMiddleware.cs index 72941633e7..48ffdbcdec 100644 --- a/src/Umbraco.Tests/TestHelpers/ControllerTesting/AuthenticateEverythingMiddleware.cs +++ b/src/Umbraco.Tests/TestHelpers/ControllerTesting/AuthenticateEverythingMiddleware.cs @@ -27,9 +27,9 @@ namespace Umbraco.Tests.TestHelpers.ControllerTesting { protected override Task AuthenticateCoreAsync() { - var sessionId = Guid.NewGuid().ToString(); + var securityStamp = Guid.NewGuid().ToString(); var identity = new UmbracoBackOfficeIdentity( - -1, "admin", "Admin", new []{-1}, new[] { -1 }, "en-US", sessionId, sessionId, new[] { "content", "media", "members" }, new[] { "admin" }); + -1, "admin", "Admin", new []{-1}, new[] { -1 }, "en-US", securityStamp, new[] { "content", "media", "members" }, new[] { "admin" }); return Task.FromResult(new AuthenticationTicket(identity, new AuthenticationProperties() diff --git a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs index 4f7a15be4f..081f05fbce 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs @@ -178,7 +178,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// otherwise process the external login info. /// /// - private async Task RenderDefaultOrProcessExternalLoginAsync( + private Task RenderDefaultOrProcessExternalLoginAsync( Func defaultResponse, Func externalSignInResponse) { @@ -190,9 +190,9 @@ namespace Umbraco.Web.BackOffice.Controllers //check if there is the TempData with the any token name specified, if so, assign to view bag and render the view if (ViewData.FromTempData(TempData, ViewDataExtensions.TokenExternalSignInError) || ViewData.FromTempData(TempData, ViewDataExtensions.TokenPasswordResetCode)) - return defaultResponse(); + return Task.FromResult(defaultResponse()); - return defaultResponse(); + return Task.FromResult(defaultResponse()); //First check if there's external login info, if there's not proceed as normal // TODO: Review this, not sure if this will work as expected until we integrate OAuth diff --git a/src/Umbraco.Web.BackOffice/Extensions/UmbracoBackOfficeApplicationBuilderExtensions.cs b/src/Umbraco.Web.BackOffice/Extensions/UmbracoBackOfficeApplicationBuilderExtensions.cs index d0ed409610..4f0ba8097d 100644 --- a/src/Umbraco.Web.BackOffice/Extensions/UmbracoBackOfficeApplicationBuilderExtensions.cs +++ b/src/Umbraco.Web.BackOffice/Extensions/UmbracoBackOfficeApplicationBuilderExtensions.cs @@ -20,8 +20,6 @@ namespace Umbraco.Extensions backOfficeRoutes.CreateRoutes(endpoints); }); - app.UseAuthentication(); - app.UseUmbracoRuntimeMinification(); // Important we handle image manipulations before the static files, otherwise the querystring is just ignored. diff --git a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs index db4269dbb0..dff941242c 100644 --- a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs +++ b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs @@ -114,28 +114,35 @@ namespace Umbraco.Web.BackOffice.Security await _securityStampValidator.ValidateAsync(ctx); }, - OnSignedIn = ctx => + OnSigningIn = ctx => { - // When we are signed in with the cookie, assign the principal to the current HttpContext - ctx.HttpContext.User = ctx.Principal; + // occurs when sign in is successful but before the ticket is written to the outbound cookie if (ctx.Principal.Identity is UmbracoBackOfficeIdentity backOfficeIdentity) { //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 - var session = _runtimeState.Level == RuntimeLevel.Run ? _userService.CreateLoginSession(backOfficeIdentity.Id, _ipResolver.GetCurrentRequestIpAddress()) : Guid.NewGuid(); - backOfficeIdentity.SessionId = session.ToString(); - + //add our session claim + backOfficeIdentity.AddClaim(new Claim(Constants.Security.SessionIdClaimType, session.ToString(), ClaimValueTypes.String, UmbracoBackOfficeIdentity.Issuer, UmbracoBackOfficeIdentity.Issuer, backOfficeIdentity)); //since it is a cookie-based authentication add that claim backOfficeIdentity.AddClaim(new Claim(ClaimTypes.CookiePath, "/", ClaimValueTypes.String, UmbracoBackOfficeIdentity.Issuer, UmbracoBackOfficeIdentity.Issuer, 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.User = ctx.Principal; + + return Task.CompletedTask; + }, OnSigningOut = ctx => { //Clear the user's session on sign out diff --git a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeIdentityOptions.cs b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeIdentityOptions.cs index 3a3f9b1759..13d608bd9b 100644 --- a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeIdentityOptions.cs +++ b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeIdentityOptions.cs @@ -26,7 +26,7 @@ namespace Umbraco.Web.BackOffice.Security options.ClaimsIdentity.UserIdClaimType = ClaimTypes.NameIdentifier; options.ClaimsIdentity.UserNameClaimType = ClaimTypes.Name; options.ClaimsIdentity.RoleClaimType = ClaimTypes.Role; - options.ClaimsIdentity.SecurityStampClaimType = Constants.Web.SecurityStampClaimType; + options.ClaimsIdentity.SecurityStampClaimType = Constants.Security.SecurityStampClaimType; options.Lockout.AllowedForNewUsers = true; options.Lockout.DefaultLockoutTimeSpan = TimeSpan.FromDays(30); diff --git a/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs b/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs index 5a68fddb73..dcc9bbca70 100644 --- a/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs @@ -72,6 +72,11 @@ namespace Umbraco.Extensions { app.UseMiddleware(); app.UseMiddleware(); + + // TODO: Both of these need to be done before any endpoints but after UmbracoRequestMiddleware + // because they rely on an UmbracoContext. But should they be here? + app.UseAuthentication(); + app.UseAuthorization(); } return app; diff --git a/src/Umbraco.Web.Common/Middleware/UmbracoRequestMiddleware.cs b/src/Umbraco.Web.Common/Middleware/UmbracoRequestMiddleware.cs index 6cf3929e06..587a60caa9 100644 --- a/src/Umbraco.Web.Common/Middleware/UmbracoRequestMiddleware.cs +++ b/src/Umbraco.Web.Common/Middleware/UmbracoRequestMiddleware.cs @@ -15,6 +15,9 @@ namespace Umbraco.Web.Common.Middleware /// /// Manages Umbraco request objects and their lifetime /// + /// + /// This is responsible for creating and assigning an + /// public class UmbracoRequestMiddleware : IMiddleware { private readonly ILogger _logger; diff --git a/src/Umbraco.Web/Editors/AuthenticationController.cs b/src/Umbraco.Web/Editors/AuthenticationController.cs index cd2876647c..f070badc5e 100644 --- a/src/Umbraco.Web/Editors/AuthenticationController.cs +++ b/src/Umbraco.Web/Editors/AuthenticationController.cs @@ -440,20 +440,7 @@ namespace Umbraco.Web.Editors // NOTE: This has been migrated to netcore, but in netcore we don't explicitly set the principal in this method, that's done in ConfigureUmbracoBackOfficeCookieOptions so don't worry about that private HttpResponseMessage SetPrincipalAndReturnUserDetail(IUser user, IPrincipal principal) { - if (user == null) throw new ArgumentNullException("user"); - if (principal == null) throw new ArgumentNullException(nameof(principal)); - - var userDetail = Mapper.Map(user); - // update the userDetail and set their remaining seconds - userDetail.SecondsUntilTimeout = TimeSpan.FromMinutes(GlobalSettings.TimeOutInMinutes).TotalSeconds; - - // create a response with the userDetail object - var response = Request.CreateResponse(HttpStatusCode.OK, userDetail); - - // ensure the user is set for the current request - Request.SetPrincipalForRequest(principal); - - return response; + throw new NotImplementedException(); } private string ConstructCallbackUrl(int userId, string code) diff --git a/src/Umbraco.Web/Editors/BackOfficeController.cs b/src/Umbraco.Web/Editors/BackOfficeController.cs index 6dff99b7ba..328ce92052 100644 --- a/src/Umbraco.Web/Editors/BackOfficeController.cs +++ b/src/Umbraco.Web/Editors/BackOfficeController.cs @@ -90,20 +90,6 @@ namespace Umbraco.Web.Editors protected IAuthenticationManager AuthenticationManager => OwinContext.Authentication; - /// - /// Render the default view - /// - /// - public async Task Default() - { - return await RenderDefaultOrProcessExternalLoginAsync( - () => - View(GlobalSettings.GetBackOfficePath(_hostingEnvironment).EnsureEndsWith('/') + "Views/Default.cshtml", new BackOfficeModel(_features, GlobalSettings, _umbracoVersion, _contentSettings, _hostingEnvironment, _runtimeSettings, _securitySettings)), - () => - View(GlobalSettings.GetBackOfficePath(_hostingEnvironment).EnsureEndsWith('/') + "Views/Default.cshtml", new BackOfficeModel(_features, GlobalSettings, _umbracoVersion, _contentSettings, _hostingEnvironment, _runtimeSettings, _securitySettings)) - ); - } - [HttpGet] public async Task VerifyInvite(string invite) { @@ -184,28 +170,6 @@ namespace Umbraco.Web.Editors } - /// - /// Returns the JavaScript object representing the static server variables javascript object - /// - /// - [UmbracoAuthorize(Order = 0)] - [MinifyJavaScriptResult(Order = 1)] - public JavaScriptResult ServerVariables() - { - var serverVars = new BackOfficeServerVariables(Url, _runtimeState, _features, GlobalSettings, _umbracoVersion, _contentSettings, _treeCollection, _hostingEnvironment, _runtimeSettings, _securitySettings, _runtimeMinifier); - - //cache the result if debugging is disabled - var result = _hostingEnvironment.IsDebugMode - ? ServerVariablesParser.Parse(serverVars.GetServerVariables()) - : AppCaches.RuntimeCache.GetCacheItem( - typeof(BackOfficeController) + "ServerVariables", - () => ServerVariablesParser.Parse(serverVars.GetServerVariables()), - new TimeSpan(0, 10, 0)); - - return JavaScript(result); - } - - // TODO: for converting to netcore, some examples: // * https://github.com/dotnet/aspnetcore/blob/master/src/Identity/samples/IdentitySample.Mvc/Controllers/AccountController.cs // * https://github.com/dotnet/aspnetcore/blob/master/src/MusicStore/samples/MusicStore/Controllers/AccountController.cs diff --git a/src/Umbraco.Web/Security/BackOfficeOwinUserManager.cs b/src/Umbraco.Web/Security/BackOfficeOwinUserManager.cs index c38c2a0ff9..2ef28cf22a 100644 --- a/src/Umbraco.Web/Security/BackOfficeOwinUserManager.cs +++ b/src/Umbraco.Web/Security/BackOfficeOwinUserManager.cs @@ -93,7 +93,7 @@ namespace Umbraco.Web.Security options.ClaimsIdentity.UserIdClaimType = ClaimTypes.NameIdentifier; options.ClaimsIdentity.UserNameClaimType = ClaimTypes.Name; options.ClaimsIdentity.RoleClaimType = ClaimTypes.Role; - options.ClaimsIdentity.SecurityStampClaimType = Constants.Web.SecurityStampClaimType; + options.ClaimsIdentity.SecurityStampClaimType = Constants.Security.SecurityStampClaimType; options.Lockout.AllowedForNewUsers = true; options.Lockout.MaxFailedAccessAttempts = passwordConfiguration.MaxFailedAccessAttemptsBeforeLockout; diff --git a/src/Umbraco.Web/Security/GetUserSecondsMiddleWare.cs b/src/Umbraco.Web/Security/GetUserSecondsMiddleWare.cs index f6b8fa765d..3ab37f0f70 100644 --- a/src/Umbraco.Web/Security/GetUserSecondsMiddleWare.cs +++ b/src/Umbraco.Web/Security/GetUserSecondsMiddleWare.cs @@ -110,8 +110,9 @@ namespace Umbraco.Web.Security } } - //We also need to re-validate the user's session if we are relying on this ping to keep their session alive - await SessionIdValidator.ValidateSessionAsync(TimeSpan.FromMinutes(1), context, _authOptions.CookieManager, _authOptions.SystemClock, issuedUtc, ticket.Identity, _globalSettings); + // NOTE: SessionIdValidator has been moved to netcore + ////We also need to re-validate the user's session if we are relying on this ping to keep their session alive + //await SessionIdValidator.ValidateSessionAsync(TimeSpan.FromMinutes(1), context, _authOptions.CookieManager, _authOptions.SystemClock, issuedUtc, ticket.Identity, _globalSettings); } else if (remainingSeconds <= 30) { diff --git a/src/Umbraco.Web/Security/SessionIdValidator.cs b/src/Umbraco.Web/Security/SessionIdValidator.cs deleted file mode 100644 index 090b6c6dac..0000000000 --- a/src/Umbraco.Web/Security/SessionIdValidator.cs +++ /dev/null @@ -1,118 +0,0 @@ -using System; -using System.Security.Claims; -using System.Threading.Tasks; -using System.Web; -using Microsoft.Owin; -using Microsoft.Owin.Infrastructure; -using Microsoft.Owin.Security.Cookies; -using Umbraco.Core; -using Umbraco.Core.Configuration; -using Umbraco.Core.Hosting; -using Umbraco.Core.IO; -using Constants = Umbraco.Core.Constants; - -namespace Umbraco.Web.Security -{ - /// - /// Static helper class used to configure a CookieAuthenticationProvider to validate a cookie against a user's session id - /// - /// - /// This uses another cookie to track the last checked time which is done for a few reasons: - /// * We can't use the user's auth ticket to do this because we'd be re-issuing the auth ticket all of the time and it would never expire - /// plus the auth ticket size is much larger than this small value - /// * This will execute quite often (every minute per user) and in some cases there might be several requests that end up re-issuing the cookie so the cookie value should be small - /// * We want to avoid the user lookup if it's not required so that will only happen when the time diff is great enough in the cookie - /// - internal static class SessionIdValidator - { - public const string CookieName = "UMB_UCONTEXT_C"; - - public static async Task ValidateSessionAsync(TimeSpan validateInterval, CookieValidateIdentityContext context, IGlobalSettings globalSettings, IHostingEnvironment hostingEnvironment) - { - if (context.Request.Uri.IsBackOfficeRequest(globalSettings, hostingEnvironment) == false) - return; - - var valid = await ValidateSessionAsync(validateInterval, context.OwinContext, context.Options.CookieManager, context.Options.SystemClock, context.Properties.IssuedUtc, context.Identity, globalSettings); - - if (valid == false) - { - context.RejectIdentity(); - context.OwinContext.Authentication.SignOut(context.Options.AuthenticationType); - } - } - - public static async Task ValidateSessionAsync( - TimeSpan validateInterval, - IOwinContext owinCtx, - Microsoft.Owin.Infrastructure.ICookieManager cookieManager, - ISystemClock systemClock, - DateTimeOffset? authTicketIssueDate, - ClaimsIdentity currentIdentity, - IGlobalSettings globalSettings) - { - if (owinCtx == null) throw new ArgumentNullException("owinCtx"); - if (cookieManager == null) throw new ArgumentNullException("cookieManager"); - if (systemClock == null) throw new ArgumentNullException("systemClock"); - - DateTimeOffset? issuedUtc = null; - var currentUtc = systemClock.UtcNow; - - //read the last checked time from a custom cookie - var lastCheckedCookie = cookieManager.GetRequestCookie(owinCtx, CookieName); - - if (lastCheckedCookie.IsNullOrWhiteSpace() == false) - { - DateTimeOffset parsed; - if (DateTimeOffset.TryParse(lastCheckedCookie, out parsed)) - { - issuedUtc = parsed; - } - } - - //no cookie, use the issue time of the auth ticket - if (issuedUtc.HasValue == false) - { - issuedUtc = authTicketIssueDate; - } - - // Only validate if enough time has elapsed - var validate = issuedUtc.HasValue == false; - if (issuedUtc.HasValue) - { - var timeElapsed = currentUtc.Subtract(issuedUtc.Value); - validate = timeElapsed > validateInterval; - } - - if (validate == false) - return true; - - var manager = owinCtx.Get(); - if (manager == null) - return false; - - var userId = currentIdentity.GetUserId(); - var user = await manager.FindByIdAsync(userId); - if (user == null) - return false; - - var sessionId = currentIdentity.FindFirstValue(Constants.Security.SessionIdClaimType); - if (await manager.ValidateSessionIdAsync(userId, sessionId) == false) - return false; - - //we will re-issue the cookie last checked cookie - cookieManager.AppendResponseCookie( - owinCtx, - CookieName, - DateTimeOffset.UtcNow.ToString("yyyy-MM-ddTHH:mm:ss.fffffffzzz"), - new CookieOptions - { - HttpOnly = true, - Secure = globalSettings.UseHttps || owinCtx.Request.IsSecure, - Path = "/" - }); - - return true; - } - - } -} diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index d9e821a4fd..77e0210606 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -227,7 +227,6 @@ - From d1a95b2498c1b565e91b747d37e7d8746c64530c Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 3 Jun 2020 10:29:10 +1000 Subject: [PATCH 06/14] adds notes --- .../ConfigureBackOfficeCookieOptions.cs | 34 +++++++++++++++++++ .../Editors/AuthenticationController.cs | 2 ++ 2 files changed, 36 insertions(+) diff --git a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs index dff941242c..16f57a4d74 100644 --- a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs +++ b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs @@ -78,6 +78,20 @@ namespace Umbraco.Web.BackOffice.Security options.Cookie.SecurePolicy = _globalSettings.UseHttps ? CookieSecurePolicy.Always : CookieSecurePolicy.SameAsRequest; options.Cookie.Path = "/"; + // TODO: Review these, we shouldn't really be redirecting at all, need to check the source to see if we can prevent any redirects. + // I think we can do that by setting these to null in the events below, we cannot set them null here else they'll be replaced with defaults. + // OK ... so figured it out, we need to have certain headers in the request to ensure that aspnetcore knows it's an ajax request, + // see: https://github.com/dotnet/aspnetcore/blob/master/src/Security/Authentication/Cookies/src/CookieAuthenticationEvents.cs#L43 + // and https://github.com/dotnet/aspnetcore/blob/master/src/Security/Authentication/Cookies/src/CookieAuthenticationEvents.cs#L104 + // when those headers are set then it will respond with the correct status codes. + // OR we override `CookieAuthenticationEvents` with our own and do + // options.Events = new BackOfficeCookieAuthenticationEvents(); ... maybe that will give us more control anyways instead of using callbacks below? + // Those methods like OnRedirectToLogin are get/set so we can replace their logic, though actually looking at the code, if we replace these callbacks like + // we are doing below then no redirections should occur but we may need to deal with the status code, we'll need to see + options.AccessDeniedPath = _globalSettings.GetBackOfficePath(_hostingEnvironment); + options.LoginPath = _globalSettings.GetBackOfficePath(_hostingEnvironment); + options.LogoutPath = _globalSettings.GetBackOfficePath(_hostingEnvironment); + options.DataProtectionProvider = _dataProtection; // NOTE: This is borrowed directly from aspnetcore source @@ -99,6 +113,26 @@ namespace Umbraco.Web.BackOffice.Security options.Events = new CookieAuthenticationEvents { + OnRedirectToReturnUrl = ctx => + { + return Task.CompletedTask; + }, + OnRedirectToLogout = ctx => + { + return Task.CompletedTask; + }, + OnRedirectToLogin = ctx => + { + return Task.CompletedTask; + }, + OnRedirectToAccessDenied = ctx => + { + // TODO: Need to deal with this so the right response is returned since we are overriding this and it's not doing its normal check + // we can copy how they check for an ajax request and basically just do what they are doing so we don't have to add yet another header + + + return Task.CompletedTask; + }, OnValidatePrincipal = async ctx => { //ensure the thread culture is set diff --git a/src/Umbraco.Web/Editors/AuthenticationController.cs b/src/Umbraco.Web/Editors/AuthenticationController.cs index f070badc5e..7e27a0dcde 100644 --- a/src/Umbraco.Web/Editors/AuthenticationController.cs +++ b/src/Umbraco.Web/Editors/AuthenticationController.cs @@ -171,6 +171,8 @@ namespace Umbraco.Web.Editors [CheckIfUserTicketDataIsStale] public UserDetail GetCurrentUser() { + // TODO: We need to migrate this next + var user = UmbracoContext.Security.CurrentUser; var result = Mapper.Map(user); var httpContextAttempt = TryGetHttpContext(); From 6c59f26c8319fc299fbbcfd000296f0de39fd805 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 3 Jun 2020 11:20:53 +1000 Subject: [PATCH 07/14] Ensures the required ajax headers are sent up in the back office, adds notes. --- .../ConfigureBackOfficeCookieOptions.cs | 26 +++++----------- .../src/common/interceptors/_module.js | 4 +-- .../interceptors/debugrequest.interceptor.js | 25 --------------- .../requiredheaders.interceptor.js | 31 +++++++++++++++++++ src/Umbraco.Web.UI.Client/src/init.js | 3 ++ 5 files changed, 43 insertions(+), 46 deletions(-) delete mode 100644 src/Umbraco.Web.UI.Client/src/common/interceptors/debugrequest.interceptor.js create mode 100644 src/Umbraco.Web.UI.Client/src/common/interceptors/requiredheaders.interceptor.js diff --git a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs index 16f57a4d74..f2c3bedb48 100644 --- a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs +++ b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs @@ -113,26 +113,14 @@ namespace Umbraco.Web.BackOffice.Security options.Events = new CookieAuthenticationEvents { - OnRedirectToReturnUrl = ctx => - { - return Task.CompletedTask; - }, - OnRedirectToLogout = ctx => - { - return Task.CompletedTask; - }, - OnRedirectToLogin = ctx => - { - return Task.CompletedTask; - }, - OnRedirectToAccessDenied = ctx => - { - // TODO: Need to deal with this so the right response is returned since we are overriding this and it's not doing its normal check - // we can copy how they check for an ajax request and basically just do what they are doing so we don't have to add yet another header + // 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. - - return Task.CompletedTask; - }, OnValidatePrincipal = async ctx => { //ensure the thread culture is set diff --git a/src/Umbraco.Web.UI.Client/src/common/interceptors/_module.js b/src/Umbraco.Web.UI.Client/src/common/interceptors/_module.js index 69a4fe35c9..7e1971352a 100644 --- a/src/Umbraco.Web.UI.Client/src/common/interceptors/_module.js +++ b/src/Umbraco.Web.UI.Client/src/common/interceptors/_module.js @@ -1,11 +1,11 @@ angular.module('umbraco.interceptors', []) // We have to add the interceptor to the queue as a string because the interceptor depends upon service instances that are not available in the config block. - .config(['$httpProvider', function($httpProvider) { + .config(['$httpProvider', function ($httpProvider) { $httpProvider.defaults.xsrfHeaderName = 'X-UMB-XSRF-TOKEN'; $httpProvider.defaults.xsrfCookieName = 'UMB-XSRF-TOKEN'; $httpProvider.interceptors.push('securityInterceptor'); - $httpProvider.interceptors.push('debugRequestInterceptor'); + $httpProvider.interceptors.push('requiredHeadersInterceptor'); $httpProvider.interceptors.push('doNotPostDollarVariablesOnPostRequestInterceptor'); $httpProvider.interceptors.push('cultureRequestInterceptor'); diff --git a/src/Umbraco.Web.UI.Client/src/common/interceptors/debugrequest.interceptor.js b/src/Umbraco.Web.UI.Client/src/common/interceptors/debugrequest.interceptor.js deleted file mode 100644 index 9fe0afd481..0000000000 --- a/src/Umbraco.Web.UI.Client/src/common/interceptors/debugrequest.interceptor.js +++ /dev/null @@ -1,25 +0,0 @@ -(function() { - 'use strict'; - - /** - * Used to set debug headers on all requests where necessary - * @param {any} $q - * @param {any} urlHelper - */ - function debugRequestInterceptor($q, urlHelper) { - return { - //dealing with requests: - 'request': function(config) { - var queryStrings = urlHelper.getQueryStringParams(); - if (queryStrings.umbDebug === "true" || queryStrings.umbdebug === "true") { - config.headers["X-UMB-DEBUG"] = "true"; - } - return config; - } - }; - } - - angular.module('umbraco.interceptors').factory('debugRequestInterceptor', debugRequestInterceptor); - - -})(); diff --git a/src/Umbraco.Web.UI.Client/src/common/interceptors/requiredheaders.interceptor.js b/src/Umbraco.Web.UI.Client/src/common/interceptors/requiredheaders.interceptor.js new file mode 100644 index 0000000000..2ded2533c8 --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/common/interceptors/requiredheaders.interceptor.js @@ -0,0 +1,31 @@ +(function() { + 'use strict'; + + /** + * Used to set required headers on all requests where necessary + * @param {any} $q + * @param {any} urlHelper + */ + function requiredHeadersInterceptor($q, urlHelper) { + return { + //dealing with requests: + 'request': function (config) { + + // This is a standard header that should be sent for all ajax requests and is required for + // how the server handles auth rejections, etc... see https://github.com/dotnet/aspnetcore/blob/a2568cbe1e8dd92d8a7976469100e564362f778e/src/Security/Authentication/Cookies/src/CookieAuthenticationEvents.cs#L106-L107 + config.headers["X-Requested-With"] = "XMLHttpRequest"; + + // Set the debug header if in debug mode + var queryStrings = urlHelper.getQueryStringParams(); + if (queryStrings.umbDebug === "true" || queryStrings.umbdebug === "true") { + config.headers["X-UMB-DEBUG"] = "true"; + } + return config; + } + }; + } + + angular.module('umbraco.interceptors').factory('requiredHeadersInterceptor', requiredHeadersInterceptor); + + +})(); diff --git a/src/Umbraco.Web.UI.Client/src/init.js b/src/Umbraco.Web.UI.Client/src/init.js index ce824f63c0..b31ea0e699 100644 --- a/src/Umbraco.Web.UI.Client/src/init.js +++ b/src/Umbraco.Web.UI.Client/src/init.js @@ -8,6 +8,9 @@ app.run(['$rootScope', '$route', '$location', 'urlHelper', 'navigationService', $.ajaxSetup({ beforeSend: function (xhr) { xhr.setRequestHeader("X-UMB-XSRF-TOKEN", $cookies["UMB-XSRF-TOKEN"]); + // This is a standard header that should be sent for all ajax requests and is required for + // how the server handles auth rejections, etc... see https://github.com/dotnet/aspnetcore/blob/a2568cbe1e8dd92d8a7976469100e564362f778e/src/Security/Authentication/Cookies/src/CookieAuthenticationEvents.cs#L106-L107 + xhr.setRequestHeader("X-Requested-With", "XMLHttpRequest"); var queryStrings = urlHelper.getQueryStringParams(); if (queryStrings.umbDebug === "true" || queryStrings.umbdebug === "true") { xhr.setRequestHeader("X-UMB-DEBUG", "true"); From c6481bdabb749f6bb4229222b5d421229d5e459f Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 3 Jun 2020 12:47:40 +1000 Subject: [PATCH 08/14] Migrates another methods of authentication controller over along with calculating the ticket's remaining seconds --- .../BackOffice/ClaimsPrincipalExtensions.cs | 28 ++++++++++++ src/Umbraco.Core/Constants-Security.cs | 1 + .../BackOffice/BackOfficeUserManager.cs | 8 ++-- .../ClaimsPrincipalExtensionsTests.cs | 43 ++++++++++++++++++ .../Controllers/AuthenticationController.cs | 26 +++++++++++ .../ConfigureBackOfficeCookieOptions.cs | 44 +++++++++++++------ .../Security/BackOfficeSignInManager.cs | 12 +---- .../Editors/AuthenticationController.cs | 28 ------------ .../Security/AuthenticationExtensions.cs | 12 +---- 9 files changed, 136 insertions(+), 66 deletions(-) create mode 100644 src/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ClaimsPrincipalExtensionsTests.cs diff --git a/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs b/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs index fa89ee6975..62b827dc6d 100644 --- a/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs +++ b/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs @@ -1,4 +1,5 @@ using System; +using System.Globalization; using System.Linq; using System.Security.Claims; using System.Security.Principal; @@ -42,5 +43,32 @@ namespace Umbraco.Extensions return null; } + + /// + /// Returns the remaining seconds on an auth ticket for the user based on the claim applied to the user durnig authentication + /// + /// + /// + public static double GetRemainingAuthSeconds(this IPrincipal user) => user.GetRemainingAuthSeconds(DateTimeOffset.UtcNow); + + /// + /// Returns the remaining seconds on an auth ticket for the user based on the claim applied to the user durnig authentication + /// + /// + /// + /// + public static double GetRemainingAuthSeconds(this IPrincipal user, DateTimeOffset now) + { + var umbIdentity = user.GetUmbracoIdentity(); + if (umbIdentity == null) return 0; + + var ticketExpires = umbIdentity.FindFirstValue(Constants.Security.TicketExpiresClaimType); + if (ticketExpires.IsNullOrWhiteSpace()) return 0; + + var utcExpired = DateTimeOffset.Parse(ticketExpires, null, DateTimeStyles.RoundtripKind); + + var secondsRemaining = utcExpired.Subtract(now).TotalSeconds; + return secondsRemaining; + } } } diff --git a/src/Umbraco.Core/Constants-Security.cs b/src/Umbraco.Core/Constants-Security.cs index 534070be6e..fda4c23dc0 100644 --- a/src/Umbraco.Core/Constants-Security.cs +++ b/src/Umbraco.Core/Constants-Security.cs @@ -52,6 +52,7 @@ public const string StartMediaNodeIdClaimType = "http://umbraco.org/2015/02/identity/claims/backoffice/startmedianode"; public const string AllowedApplicationsClaimType = "http://umbraco.org/2015/02/identity/claims/backoffice/allowedapp"; public const string SessionIdClaimType = "http://umbraco.org/2015/02/identity/claims/backoffice/sessionid"; + public const string TicketExpiresClaimType = "http://umbraco.org/2020/06/identity/claims/backoffice/ticketexpires"; /// /// The claim type for the ASP.NET Identity security stamp diff --git a/src/Umbraco.Infrastructure/BackOffice/BackOfficeUserManager.cs b/src/Umbraco.Infrastructure/BackOffice/BackOfficeUserManager.cs index 0ef3ff47ff..1ac7a7f1ec 100644 --- a/src/Umbraco.Infrastructure/BackOffice/BackOfficeUserManager.cs +++ b/src/Umbraco.Infrastructure/BackOffice/BackOfficeUserManager.cs @@ -53,10 +53,11 @@ namespace Umbraco.Core.BackOffice } #region What we do not currently support - // TODO: We could support this - but a user claims will mostly just be what is in the auth cookie + + // We don't support an IUserClaimStore and don't need to (at least currently) public override bool SupportsUserClaim => false; - // TODO: Support this + // It would be nice to support this but we don't need to currently and that would require IQueryable support for our user service/repository public override bool SupportsQueryableUsers => false; /// @@ -64,8 +65,9 @@ namespace Umbraco.Core.BackOffice /// public override bool SupportsUserTwoFactor => false; - // TODO: Support this + // We haven't needed to support this yet, though might be necessary for 2FA public override bool SupportsUserPhoneNumber => false; + #endregion /// diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ClaimsPrincipalExtensionsTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ClaimsPrincipalExtensionsTests.cs new file mode 100644 index 0000000000..a078456f8f --- /dev/null +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ClaimsPrincipalExtensionsTests.cs @@ -0,0 +1,43 @@ +using NUnit.Framework; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Security.Claims; +using Umbraco.Extensions; +using Umbraco.Core; +using Umbraco.Core.BackOffice; + +namespace Umbraco.Tests.UnitTests.Umbraco.Core.Extensions +{ + [TestFixture] + public class ClaimsPrincipalExtensionsTests + { + [Test] + public void Get_Remaining_Ticket_Seconds() + { + var backOfficeIdentity = new UmbracoBackOfficeIdentity(-1, "test", "test", + Enumerable.Empty(), Enumerable.Empty(), "en-US", Guid.NewGuid().ToString(), + Enumerable.Empty(), Enumerable.Empty()); + var principal = new ClaimsPrincipal(backOfficeIdentity); + + var expireSeconds = 99; + var elapsedSeconds = 3; + var remainingSeconds = expireSeconds - elapsedSeconds; + var now = DateTimeOffset.Now; + var then = now.AddSeconds(elapsedSeconds); + var expires = now.AddSeconds(expireSeconds).ToString("o"); + + backOfficeIdentity.AddClaim(new Claim( + Constants.Security.TicketExpiresClaimType, + expires, + ClaimValueTypes.DateTime, + UmbracoBackOfficeIdentity.Issuer, + UmbracoBackOfficeIdentity.Issuer, + backOfficeIdentity)); + + var ticketRemainingSeconds = principal.GetRemainingAuthSeconds(then); + + Assert.AreEqual(remainingSeconds, ticketRemainingSeconds); + } + } +} diff --git a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs index f501fb1579..03718cb1c4 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs @@ -11,6 +11,7 @@ using Umbraco.Core.Mapping; using Umbraco.Core.Models.Membership; using Umbraco.Core.Services; using Umbraco.Extensions; +using Umbraco.Web.BackOffice.Filters; using Umbraco.Web.Common.Attributes; using Umbraco.Web.Common.Controllers; using Umbraco.Web.Common.Exceptions; @@ -71,6 +72,31 @@ namespace Umbraco.Web.BackOffice.Controllers return false; } + /// + /// Returns the currently logged in Umbraco user + /// + /// + /// + /// We have the attribute [SetAngularAntiForgeryTokens] applied because this method is called initially to determine if the user + /// is valid before the login screen is displayed. The Auth cookie can be persisted for up to a day but the csrf cookies are only session + /// cookies which means that the auth cookie could be valid but the csrf cookies are no longer there, in that case we need to re-set the csrf cookies. + /// + [UmbracoAuthorize] + [TypeFilter(typeof(SetAngularAntiForgeryTokens))] + //[CheckIfUserTicketDataIsStale] // TODO: Migrate this, though it will need to be done differently at the cookie auth level + public UserDetail GetCurrentUser() + { + var umbracoContext = _umbracoContextAccessor.GetRequiredUmbracoContext(); + + var user = umbracoContext.Security.CurrentUser; + var result = _umbracoMapper.Map(user); + + //set their remaining seconds + result.SecondsUntilTimeout = HttpContext.User.GetRemainingAuthSeconds(); + + return result; + } + /// /// Logs a user in /// diff --git a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs index f2c3bedb48..f3db54093c 100644 --- a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs +++ b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs @@ -15,7 +15,9 @@ using Umbraco.Core.Hosting; using Umbraco.Core.Services; using Umbraco.Net; using Umbraco.Core.Security; -using Umbraco.Web; +using Umbraco.Extensions; +using Microsoft.Extensions.DependencyInjection; +using Umbraco.Web.Common.Security; namespace Umbraco.Web.BackOffice.Security { @@ -34,7 +36,6 @@ namespace Umbraco.Web.BackOffice.Security private readonly IUserService _userService; private readonly IIpResolver _ipResolver; private readonly BackOfficeSessionIdValidator _sessionIdValidator; - private readonly BackOfficeSecurityStampValidator _securityStampValidator; public ConfigureBackOfficeCookieOptions( IUmbracoContextAccessor umbracoContextAccessor, @@ -46,8 +47,7 @@ namespace Umbraco.Web.BackOffice.Security IRequestCache requestCache, IUserService userService, IIpResolver ipResolver, - BackOfficeSessionIdValidator sessionIdValidator, - BackOfficeSecurityStampValidator securityStampValidator) + BackOfficeSessionIdValidator sessionIdValidator) { _umbracoContextAccessor = umbracoContextAccessor; _securitySettings = securitySettings; @@ -59,7 +59,6 @@ namespace Umbraco.Web.BackOffice.Security _userService = userService; _ipResolver = ipResolver; _sessionIdValidator = sessionIdValidator; - _securityStampValidator = securityStampValidator; } public void Configure(string name, CookieAuthenticationOptions options) @@ -123,24 +122,40 @@ namespace Umbraco.Web.BackOffice.Security OnValidatePrincipal = async ctx => { - //ensure the thread culture is set - ctx.Principal?.Identity?.EnsureCulture(); + // We need to resolve the BackOfficeSecurityStampValidator per request as a requirement (even in aspnetcore they do this) + var securityStampValidator = ctx.HttpContext.RequestServices.GetRequiredService(); + // Same goes for the signinmanager + var signInManager = ctx.HttpContext.RequestServices.GetRequiredService(); - await EnsureValidSessionId(ctx); - - if (ctx.Principal?.Identity == null) + var backOfficeIdentity = ctx.Principal.GetUmbracoIdentity(); + if (backOfficeIdentity == null) { - await ctx.HttpContext.SignOutAsync(Constants.Security.BackOfficeAuthenticationType); - return; + ctx.RejectPrincipal(); + await signInManager.SignOutAsync(); } - await _securityStampValidator.ValidateAsync(ctx); + //ensure the thread culture is set + backOfficeIdentity.EnsureCulture(); + + await EnsureValidSessionId(ctx); + await securityStampValidator.ValidateAsync(ctx); + + // add a claim to track when the cookie expires, we use this to track time remaining + backOfficeIdentity.AddClaim(new Claim( + Constants.Security.TicketExpiresClaimType, + ctx.Properties.ExpiresUtc.Value.ToString("o"), + ClaimValueTypes.DateTime, + UmbracoBackOfficeIdentity.Issuer, + UmbracoBackOfficeIdentity.Issuer, + backOfficeIdentity)); + }, OnSigningIn = ctx => { // occurs when sign in is successful but before the ticket is written to the outbound cookie - if (ctx.Principal.Identity is UmbracoBackOfficeIdentity backOfficeIdentity) + var 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 @@ -168,6 +183,7 @@ namespace Umbraco.Web.BackOffice.Security OnSigningOut = ctx => { //Clear the user's session on sign out + // TODO: We need to test this once we have signout functionality, not sure if the httpcontext.user.identity will still be set here if (ctx.HttpContext?.User?.Identity != null) { var claimsIdentity = ctx.HttpContext.User.Identity as ClaimsIdentity; diff --git a/src/Umbraco.Web.Common/Security/BackOfficeSignInManager.cs b/src/Umbraco.Web.Common/Security/BackOfficeSignInManager.cs index dd0afe5a80..b39e54935d 100644 --- a/src/Umbraco.Web.Common/Security/BackOfficeSignInManager.cs +++ b/src/Umbraco.Web.Common/Security/BackOfficeSignInManager.cs @@ -10,6 +10,7 @@ using System.Security.Claims; using System.Threading.Tasks; using Umbraco.Core; using Umbraco.Core.BackOffice; +using Umbraco.Extensions; namespace Umbraco.Web.Common.Security { @@ -34,17 +35,6 @@ namespace Umbraco.Web.Common.Security _userManager = userManager; } - // TODO: Implement these, this is what the security stamp thingy calls - public override Task ValidateSecurityStampAsync(BackOfficeIdentityUser user, string securityStamp) - { - return base.ValidateSecurityStampAsync(user, securityStamp); - } - // TODO: Implement these, this is what the security stamp thingy calls - public override Task ValidateSecurityStampAsync(ClaimsPrincipal principal) - { - return base.ValidateSecurityStampAsync(principal); - } - // TODO: Need to migrate more from Umbraco.Web.Security.BackOfficeSignInManager // Things like dealing with auto-linking, cookie options, and a ton of other stuff. Some might not need to be ported but it // will be a case by case basis. diff --git a/src/Umbraco.Web/Editors/AuthenticationController.cs b/src/Umbraco.Web/Editors/AuthenticationController.cs index 7e27a0dcde..1cfd2f7da3 100644 --- a/src/Umbraco.Web/Editors/AuthenticationController.cs +++ b/src/Umbraco.Web/Editors/AuthenticationController.cs @@ -157,34 +157,6 @@ namespace Umbraco.Web.Editors } - /// - /// Returns the currently logged in Umbraco user - /// - /// - /// - /// We have the attribute [SetAngularAntiForgeryTokens] applied because this method is called initially to determine if the user - /// is valid before the login screen is displayed. The Auth cookie can be persisted for up to a day but the csrf cookies are only session - /// cookies which means that the auth cookie could be valid but the csrf cookies are no longer there, in that case we need to re-set the csrf cookies. - /// - [WebApi.UmbracoAuthorize] - [SetAngularAntiForgeryTokens] - [CheckIfUserTicketDataIsStale] - public UserDetail GetCurrentUser() - { - // TODO: We need to migrate this next - - var user = UmbracoContext.Security.CurrentUser; - var result = Mapper.Map(user); - var httpContextAttempt = TryGetHttpContext(); - if (httpContextAttempt.Success) - { - //set their remaining seconds - result.SecondsUntilTimeout = httpContextAttempt.Result.GetRemainingAuthSeconds(); - } - - return result; - } - /// /// When a user is invited they are not approved but we need to resolve the partially logged on (non approved) /// user. diff --git a/src/Umbraco.Web/Security/AuthenticationExtensions.cs b/src/Umbraco.Web/Security/AuthenticationExtensions.cs index 8e9f2abc01..2b8ba2ddfb 100644 --- a/src/Umbraco.Web/Security/AuthenticationExtensions.cs +++ b/src/Umbraco.Web/Security/AuthenticationExtensions.cs @@ -126,11 +126,7 @@ namespace Umbraco.Web.Security return true; } - /// - /// returns the number of seconds the user has until their auth session times out - /// - /// - /// + // NOTE: Migrated to netcore (though in a different way) public static double GetRemainingAuthSeconds(this HttpContextBase http) { if (http == null) throw new ArgumentNullException(nameof(http)); @@ -138,11 +134,7 @@ namespace Umbraco.Web.Security return ticket.GetRemainingAuthSeconds(); } - /// - /// returns the number of seconds the user has until their auth session times out - /// - /// - /// + // NOTE: Migrated to netcore (though in a different way) public static double GetRemainingAuthSeconds(this AuthenticationTicket ticket) { var utcExpired = ticket?.Properties.ExpiresUtc; From bba0eceac05da1c04767bb1d197c741dd73a4882 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 3 Jun 2020 14:30:55 +1000 Subject: [PATCH 09/14] removes notes --- .../ConfigureBackOfficeCookieOptions.cs | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs index f3db54093c..fd009eab23 100644 --- a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs +++ b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs @@ -77,19 +77,11 @@ namespace Umbraco.Web.BackOffice.Security options.Cookie.SecurePolicy = _globalSettings.UseHttps ? CookieSecurePolicy.Always : CookieSecurePolicy.SameAsRequest; options.Cookie.Path = "/"; - // TODO: Review these, we shouldn't really be redirecting at all, need to check the source to see if we can prevent any redirects. - // I think we can do that by setting these to null in the events below, we cannot set them null here else they'll be replaced with defaults. - // OK ... so figured it out, we need to have certain headers in the request to ensure that aspnetcore knows it's an ajax request, - // see: https://github.com/dotnet/aspnetcore/blob/master/src/Security/Authentication/Cookies/src/CookieAuthenticationEvents.cs#L43 - // and https://github.com/dotnet/aspnetcore/blob/master/src/Security/Authentication/Cookies/src/CookieAuthenticationEvents.cs#L104 - // when those headers are set then it will respond with the correct status codes. - // OR we override `CookieAuthenticationEvents` with our own and do - // options.Events = new BackOfficeCookieAuthenticationEvents(); ... maybe that will give us more control anyways instead of using callbacks below? - // Those methods like OnRedirectToLogin are get/set so we can replace their logic, though actually looking at the code, if we replace these callbacks like - // we are doing below then no redirections should occur but we may need to deal with the status code, we'll need to see - options.AccessDeniedPath = _globalSettings.GetBackOfficePath(_hostingEnvironment); - options.LoginPath = _globalSettings.GetBackOfficePath(_hostingEnvironment); - options.LogoutPath = _globalSettings.GetBackOfficePath(_hostingEnvironment); + // For any redirections that may occur for the back office, they all go to the same path + var backOfficePath = _globalSettings.GetBackOfficePath(_hostingEnvironment); + options.AccessDeniedPath = backOfficePath; + options.LoginPath = backOfficePath; + options.LogoutPath = backOfficePath; options.DataProtectionProvider = _dataProtection; From d024443b230b4155f042f7e07dcd116b9b2f2df0 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 3 Jun 2020 15:00:39 +1000 Subject: [PATCH 10/14] Converts FromClaimsIdentity to a bool method --- .../BackOffice/ClaimsPrincipalExtensions.cs | 16 ++----- .../BackOffice/UmbracoBackOfficeIdentity.cs | 46 ++++++++++--------- .../UmbracoBackOfficeIdentityTests.cs | 17 ++++--- .../Security/BackOfficeSecureDataFormat.cs | 12 +---- .../PreviewAuthenticationMiddleware.cs | 8 +++- .../Security/UmbracoSecureDataFormat.cs | 12 +---- src/Umbraco.Web/Umbraco.Web.csproj | 1 - 7 files changed, 47 insertions(+), 65 deletions(-) diff --git a/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs b/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs index 62b827dc6d..6a792ce69b 100644 --- a/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs +++ b/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs @@ -27,18 +27,12 @@ namespace Umbraco.Extensions if (backOfficeIdentity != null) return backOfficeIdentity; } - //Otherwise convert to a UmbracoBackOfficeIdentity if it's auth'd and has the back office session - if (user.Identity is ClaimsIdentity claimsIdentity && claimsIdentity.IsAuthenticated && claimsIdentity.HasClaim(x => x.Type == Constants.Security.SessionIdClaimType)) + //Otherwise convert to a UmbracoBackOfficeIdentity if it's auth'd + if (user.Identity is ClaimsIdentity claimsIdentity + && claimsIdentity.IsAuthenticated + && UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity, out var umbracoIdentity)) { - try - { - return UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity); - } - catch (InvalidOperationException) - { - // We catch this error because it's what we throw when the required claims are not in the identity. - // when that happens something strange is going on, we'll swallow this exception and return null. - } + return umbracoIdentity; } return null; diff --git a/src/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentity.cs b/src/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentity.cs index 18684e072a..15db0e0dc7 100644 --- a/src/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentity.cs +++ b/src/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentity.cs @@ -12,9 +12,31 @@ namespace Umbraco.Core.BackOffice [Serializable] public class UmbracoBackOfficeIdentity : ClaimsIdentity { - public static UmbracoBackOfficeIdentity FromClaimsIdentity(ClaimsIdentity identity) + public static bool FromClaimsIdentity(ClaimsIdentity identity, out UmbracoBackOfficeIdentity backOfficeIdentity) { - return new UmbracoBackOfficeIdentity(identity); + //validate that all claims exist + foreach (var t in RequiredBackOfficeIdentityClaimTypes) + { + //if the identity doesn't have the claim, or the claim value is null + if (identity.HasClaim(x => x.Type == t) == false || identity.HasClaim(x => x.Type == t && x.Value.IsNullOrWhiteSpace())) + { + backOfficeIdentity = null; + return false; + } + } + + backOfficeIdentity = new UmbracoBackOfficeIdentity(identity); + return true; + } + + /// + /// Create a back office identity based on an existing claims identity + /// + /// + private UmbracoBackOfficeIdentity(ClaimsIdentity identity) + : base(identity.Claims, Constants.Security.BackOfficeAuthenticationType) + { + Actor = identity; } /// @@ -71,26 +93,6 @@ namespace Umbraco.Core.BackOffice AddRequiredClaims(userId, username, realName, startContentNodes, startMediaNodes, culture, securityStamp, allowedApps, roles); } - /// - /// Create a back office identity based on an existing claims identity - /// - /// - private UmbracoBackOfficeIdentity(ClaimsIdentity identity) - : base(identity.Claims, Constants.Security.BackOfficeAuthenticationType) - { - Actor = identity; - - //validate that all claims exist - foreach (var t in RequiredBackOfficeIdentityClaimTypes) - { - //if the identity doesn't have the claim, or the claim value is null - if (identity.HasClaim(x => x.Type == t) == false || identity.HasClaim(x => x.Type == t && x.Value.IsNullOrWhiteSpace())) - { - throw new InvalidOperationException("Cannot create a " + typeof(UmbracoBackOfficeIdentity) + " from " + typeof(ClaimsIdentity) + " since the required claim " + t + " is missing"); - } - } - } - public const string Issuer = Constants.Security.BackOfficeAuthenticationType; /// diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs index d3380c06e4..ed1d681a77 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs @@ -16,7 +16,6 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice [Test] public void Create_From_Claims_Identity() { - var sessionId = Guid.NewGuid().ToString(); var securityStamp = Guid.NewGuid().ToString(); var claimsIdentity = new ClaimsIdentity(new[] { @@ -31,12 +30,12 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice new Claim(Constants.Security.AllowedApplicationsClaimType, "content", ClaimValueTypes.String, TestIssuer, TestIssuer), new Claim(Constants.Security.AllowedApplicationsClaimType, "media", ClaimValueTypes.String, TestIssuer, TestIssuer), new Claim(ClaimTypes.Locality, "en-us", ClaimValueTypes.String, TestIssuer, TestIssuer), - new Claim(Constants.Security.SessionIdClaimType, sessionId, Constants.Security.SessionIdClaimType, TestIssuer, TestIssuer), new Claim(ClaimsIdentity.DefaultRoleClaimType, "admin", ClaimValueTypes.String, TestIssuer, TestIssuer), new Claim(Constants.Security.SecurityStampClaimType, securityStamp, ClaimValueTypes.String, TestIssuer, TestIssuer), }); - var backofficeIdentity = UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity); + if (!UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity, out var backofficeIdentity)) + Assert.Fail(); Assert.AreEqual(1234, backofficeIdentity.Id); //Assert.AreEqual(sessionId, backofficeIdentity.SessionId); @@ -61,13 +60,15 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice new Claim(ClaimTypes.Name, "testing", ClaimValueTypes.String, TestIssuer, TestIssuer), }); - Assert.Throws(() => UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity)); + if (UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity, out var backofficeIdentity)) + Assert.Fail(); + + Assert.Pass(); } [Test] public void Create_From_Claims_Identity_Required_Claim_Null() { - var sessionId = Guid.NewGuid().ToString(); var claimsIdentity = new ClaimsIdentity(new[] { //null or empty @@ -79,11 +80,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice new Claim(Constants.Security.AllowedApplicationsClaimType, "content", ClaimValueTypes.String, TestIssuer, TestIssuer), new Claim(Constants.Security.AllowedApplicationsClaimType, "media", ClaimValueTypes.String, TestIssuer, TestIssuer), new Claim(ClaimTypes.Locality, "en-us", ClaimValueTypes.String, TestIssuer, TestIssuer), - new Claim(Constants.Security.SessionIdClaimType, sessionId, Constants.Security.SessionIdClaimType, TestIssuer, TestIssuer), new Claim(ClaimsIdentity.DefaultRoleClaimType, "admin", ClaimValueTypes.String, TestIssuer, TestIssuer), }); - Assert.Throws(() => UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity)); + if (UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity, out var backofficeIdentity)) + Assert.Fail(); + + Assert.Pass(); } diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficeSecureDataFormat.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficeSecureDataFormat.cs index 88017f509a..3a23ae6c88 100644 --- a/src/Umbraco.Web.BackOffice/Security/BackOfficeSecureDataFormat.cs +++ b/src/Umbraco.Web.BackOffice/Security/BackOfficeSecureDataFormat.cs @@ -60,18 +60,8 @@ namespace Umbraco.Web.BackOffice.Security return null; } - UmbracoBackOfficeIdentity identity; - - try - { - identity = UmbracoBackOfficeIdentity.FromClaimsIdentity((ClaimsIdentity)decrypt.Principal.Identity); - } - catch (Exception) - { - //if it cannot be created return null, will be due to serialization errors in user data most likely due to corrupt cookies or cookies - //for previous versions of Umbraco + if (!UmbracoBackOfficeIdentity.FromClaimsIdentity((ClaimsIdentity)decrypt.Principal.Identity, out var identity)) return null; - } //return the ticket with a UmbracoBackOfficeIdentity var ticket = new AuthenticationTicket(new ClaimsPrincipal(identity), decrypt.Properties, decrypt.AuthenticationScheme); diff --git a/src/Umbraco.Web/Security/PreviewAuthenticationMiddleware.cs b/src/Umbraco.Web/Security/PreviewAuthenticationMiddleware.cs index 799edb5f60..feecd85364 100644 --- a/src/Umbraco.Web/Security/PreviewAuthenticationMiddleware.cs +++ b/src/Umbraco.Web/Security/PreviewAuthenticationMiddleware.cs @@ -1,4 +1,5 @@ -using System.Security.Claims; +using System; +using System.Security.Claims; using System.Threading.Tasks; using Microsoft.Owin; using Umbraco.Core; @@ -59,7 +60,10 @@ namespace Umbraco.Web.Security //Ok, we've got a real ticket, now we can add this ticket's identity to the current // Principal, this means we'll have 2 identities assigned to the principal which we can // use to authorize the preview and allow for a back office User. - claimsPrincipal.AddIdentity(UmbracoBackOfficeIdentity.FromClaimsIdentity(unprotected.Identity)); + if (!UmbracoBackOfficeIdentity.FromClaimsIdentity(unprotected.Identity, out var umbracoIdentity)) + throw new InvalidOperationException("Cannot convert identity"); + + claimsPrincipal.AddIdentity(umbracoIdentity); } } } diff --git a/src/Umbraco.Web/Security/UmbracoSecureDataFormat.cs b/src/Umbraco.Web/Security/UmbracoSecureDataFormat.cs index 41a2ad3bba..73c1c3fd55 100644 --- a/src/Umbraco.Web/Security/UmbracoSecureDataFormat.cs +++ b/src/Umbraco.Web/Security/UmbracoSecureDataFormat.cs @@ -55,18 +55,8 @@ namespace Umbraco.Web.Security return null; } - UmbracoBackOfficeIdentity identity; - - try - { - identity = UmbracoBackOfficeIdentity.FromClaimsIdentity(decrypt.Identity); - } - catch (Exception) - { - //if it cannot be created return null, will be due to serialization errors in user data most likely due to corrupt cookies or cookies - //for previous versions of Umbraco + if (!UmbracoBackOfficeIdentity.FromClaimsIdentity(decrypt.Identity, out var identity)) return null; - } //return the ticket with a UmbracoBackOfficeIdentity var ticket = new AuthenticationTicket(identity, decrypt.Properties); diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 959aab671c..51249d6348 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -152,7 +152,6 @@ - From 396d7e9b31779e572278d363e4a0d27c7a8c655a Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Wed, 3 Jun 2020 09:09:25 +0200 Subject: [PATCH 11/14] Updateded the ValidateAngularAntiForgeryTokenAttribute to use the IBackOfficeAntiforgery instead of default IAntiforgery --- .../Filters/ValidateAngularAntiForgeryTokenAttribute.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs index b8f035077b..2fd37e2875 100644 --- a/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs @@ -10,6 +10,7 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; using Umbraco.Core; using Umbraco.Core.Logging; +using Umbraco.Web.BackOffice.Security; namespace Umbraco.Web.BackOffice.Filters { @@ -24,10 +25,10 @@ namespace Umbraco.Web.BackOffice.Filters public sealed class ValidateAngularAntiForgeryTokenAttribute : ActionFilterAttribute { private readonly ILogger _logger; - private readonly IAntiforgery _antiforgery; + private readonly IBackOfficeAntiforgery _antiforgery; private readonly ICookieManager _cookieManager; - public ValidateAngularAntiForgeryTokenAttribute(ILogger logger, IAntiforgery antiforgery, ICookieManager cookieManager) + public ValidateAngularAntiForgeryTokenAttribute(ILogger logger, IBackOfficeAntiforgery antiforgery, ICookieManager cookieManager) { _logger = logger; _antiforgery = antiforgery; From 846432111d87e14a8a57a3ee0f0a2623a242cb44 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 3 Jun 2020 18:10:35 +1000 Subject: [PATCH 12/14] fixes tests --- .../BackOfficeClaimsPrincipalFactory.cs | 13 +++++ .../BackOfficeClaimsPrincipalFactoryTests.cs | 54 ++++++------------- .../UmbracoBackOfficeIdentityTests.cs | 2 +- 3 files changed, 29 insertions(+), 40 deletions(-) diff --git a/src/Umbraco.Infrastructure/BackOffice/BackOfficeClaimsPrincipalFactory.cs b/src/Umbraco.Infrastructure/BackOffice/BackOfficeClaimsPrincipalFactory.cs index 31e9a7775b..32f0fbccf6 100644 --- a/src/Umbraco.Infrastructure/BackOffice/BackOfficeClaimsPrincipalFactory.cs +++ b/src/Umbraco.Infrastructure/BackOffice/BackOfficeClaimsPrincipalFactory.cs @@ -35,5 +35,18 @@ namespace Umbraco.Core.BackOffice return new ClaimsPrincipal(umbracoIdentity); } + + protected override async Task GenerateClaimsAsync(TUser user) + { + // TODO: Have a look at the base implementation https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Extensions.Core/src/UserClaimsPrincipalFactory.cs#L79 + // since it's setting an authentication type that is probably not what we want. + // also, this is the method that we should be returning our UmbracoBackOfficeIdentity from , not the method above, + // the method above just returns a principal that wraps the identity and we dont use a custom principal, + // see https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Extensions.Core/src/UserClaimsPrincipalFactory.cs#L66 + + var identity = await base.GenerateClaimsAsync(user); + + return identity; + } } } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/BackOfficeClaimsPrincipalFactoryTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/BackOfficeClaimsPrincipalFactoryTests.cs index db7e7379aa..1aa1010458 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/BackOfficeClaimsPrincipalFactoryTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/BackOfficeClaimsPrincipalFactoryTests.cs @@ -17,6 +17,11 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice [TestFixture] public class BackOfficeClaimsPrincipalFactoryTests { + private const int _testUserId = 2; + private const string _testUserName = "bob"; + private const string _testUserGivenName = "Bob"; + private const string _testUserCulture = "en-US"; + private const string _testUserSecurityStamp = "B6937738-9C17-4C7D-A25A-628A875F5177"; private BackOfficeIdentityUser _testUser; private Mock> _mockUserManager; @@ -65,46 +70,16 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice Assert.IsNotNull(umbracoBackOfficeIdentity); } - [Test] - public async Task CreateAsync_Should_Create_NameId() + [TestCase(ClaimTypes.NameIdentifier, _testUserId)] + [TestCase(ClaimTypes.Name, _testUserName)] + public async Task CreateAsync_Should_Include_Claim(string expectedClaimType, object expectedClaimValue) { - const string expectedClaimType = ClaimTypes.NameIdentifier; - var expectedClaimValue = _testUser.Id.ToString(); - var sut = CreateSut(); var claimsPrincipal = await sut.CreateAsync(_testUser); - Assert.True(claimsPrincipal.HasClaim(expectedClaimType, expectedClaimValue)); - Assert.True(claimsPrincipal.GetUmbracoIdentity().Actor.HasClaim(expectedClaimType, expectedClaimValue)); - } - - [Test] - public async Task CreateAsync_Should_Create_Name() - { - const string expectedClaimType = ClaimTypes.Name; - var expectedClaimValue = _testUser.UserName; - - var sut = CreateSut(); - - var claimsPrincipal = await sut.CreateAsync(_testUser); - - Assert.True(claimsPrincipal.HasClaim(expectedClaimType, expectedClaimValue)); - Assert.True(claimsPrincipal.GetUmbracoIdentity().Actor.HasClaim(expectedClaimType, expectedClaimValue)); - } - - [Test] - public async Task CreateAsync_Should_Create_IdentityProvider() - { - const string expectedClaimType = "http://schemas.microsoft.com/accesscontrolservice/2010/07/claims/identityprovider"; - const string expectedClaimValue = "ASP.NET Identity"; - - var sut = CreateSut(); - - var claimsPrincipal = await sut.CreateAsync(_testUser); - - Assert.True(claimsPrincipal.HasClaim(expectedClaimType, expectedClaimValue)); - Assert.True(claimsPrincipal.GetUmbracoIdentity().Actor.HasClaim(expectedClaimType, expectedClaimValue)); + Assert.True(claimsPrincipal.HasClaim(expectedClaimType, expectedClaimValue.ToString())); + Assert.True(claimsPrincipal.GetUmbracoIdentity().Actor.HasClaim(expectedClaimType, expectedClaimValue.ToString())); } [Test] @@ -165,12 +140,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice var mockGlobalSettings = new Mock(); mockGlobalSettings.Setup(x => x.DefaultUILanguage).Returns("test"); - _testUser = new BackOfficeIdentityUser(mockGlobalSettings.Object, 2, new List()) + _testUser = new BackOfficeIdentityUser(mockGlobalSettings.Object, _testUserId, new List()) { - UserName = "bob", - Name = "Bob", + UserName = _testUserName, + Name = _testUserGivenName, Email = "bob@umbraco.test", - SecurityStamp = "B6937738-9C17-4C7D-A25A-628A875F5177" + SecurityStamp = _testUserSecurityStamp, + Culture = _testUserCulture }; _mockUserManager = new Mock>(new Mock>().Object, diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs index ed1d681a77..47e1261c09 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs @@ -48,7 +48,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice Assert.AreEqual("en-us", backofficeIdentity.Culture); Assert.IsTrue(new[] { "admin" }.SequenceEqual(backofficeIdentity.Roles)); - Assert.AreEqual(12, backofficeIdentity.Claims.Count()); + Assert.AreEqual(11, backofficeIdentity.Claims.Count()); } [Test] From d682bc9903d17c3edfd23bafac0d128c1b127ec5 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Wed, 3 Jun 2020 13:33:05 +0200 Subject: [PATCH 13/14] Uncommented test of code moved to .NET Core --- .../AuthenticationControllerTests.cs | 177 +++++++++--------- 1 file changed, 89 insertions(+), 88 deletions(-) diff --git a/src/Umbraco.Tests/Web/Controllers/AuthenticationControllerTests.cs b/src/Umbraco.Tests/Web/Controllers/AuthenticationControllerTests.cs index a35e891c1c..aa329d2b48 100644 --- a/src/Umbraco.Tests/Web/Controllers/AuthenticationControllerTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/AuthenticationControllerTests.cs @@ -60,93 +60,94 @@ namespace Umbraco.Tests.Web.Controllers } - [Test] - public async System.Threading.Tasks.Task GetCurrentUser_Fips() - { - ApiController CtrlFactory(HttpRequestMessage message, IUmbracoContextAccessor umbracoContextAccessor) - { - //setup some mocks - var userServiceMock = Mock.Get(ServiceContext.UserService); - userServiceMock.Setup(service => service.GetUserById(It.IsAny())) - .Returns(() => null); - - if (Thread.GetDomain().GetData(".appPath") != null) - { - HttpContext.Current = new HttpContext(new SimpleWorkerRequest("", "", new StringWriter())); - } - else - { - var baseDir = IOHelper.MapPath("").TrimEnd(Path.DirectorySeparatorChar); - HttpContext.Current = new HttpContext(new SimpleWorkerRequest("/", baseDir, "", "", new StringWriter())); - } - - var usersController = new AuthenticationController( - new TestUserPasswordConfig(), - Factory.GetInstance(), - Factory.GetInstance(), - umbracoContextAccessor, - Factory.GetInstance(), - Factory.GetInstance(), - Factory.GetInstance(), - Factory.GetInstance(), - Factory.GetInstance(), - Factory.GetInstance(), - Factory.GetInstance(), - Factory.GetInstance(), - Factory.GetInstance(), - Factory.GetInstance() - ); - return usersController; - } - - Mock.Get(Current.SqlContext) - .Setup(x => x.Query()) - .Returns(new Query(Current.SqlContext)); - - var syntax = new SqlCeSyntaxProvider(); - - Mock.Get(Current.SqlContext) - .Setup(x => x.SqlSyntax) - .Returns(syntax); - - var mappers = new MapperCollection(new[] - { - new UserMapper(new Lazy(() => Current.SqlContext), new ConcurrentDictionary>()) - }); - - Mock.Get(Current.SqlContext) - .Setup(x => x.Mappers) - .Returns(mappers); - - // Testing what happens if the system were configured to only use FIPS-compliant algorithms - var typ = typeof(CryptoConfig); - var flds = typ.GetFields(BindingFlags.Static | BindingFlags.NonPublic); - var haveFld = flds.FirstOrDefault(f => f.Name == "s_haveFipsAlgorithmPolicy"); - var isFld = flds.FirstOrDefault(f => f.Name == "s_fipsAlgorithmPolicy"); - var originalFipsValue = CryptoConfig.AllowOnlyFipsAlgorithms; - - try - { - if (!originalFipsValue) - { - haveFld.SetValue(null, true); - isFld.SetValue(null, true); - } - - var runner = new TestRunner(CtrlFactory); - var response = await runner.Execute("Authentication", "GetCurrentUser", HttpMethod.Get); - - var obj = JsonConvert.DeserializeObject(response.Item2); - Assert.AreEqual(-1, obj.UserId); - } - finally - { - if (!originalFipsValue) - { - haveFld.SetValue(null, false); - isFld.SetValue(null, false); - } - } - } + // TODO Reintroduce when moved to .NET Core + // [Test] + // public async System.Threading.Tasks.Task GetCurrentUser_Fips() + // { + // ApiController CtrlFactory(HttpRequestMessage message, IUmbracoContextAccessor umbracoContextAccessor) + // { + // //setup some mocks + // var userServiceMock = Mock.Get(ServiceContext.UserService); + // userServiceMock.Setup(service => service.GetUserById(It.IsAny())) + // .Returns(() => null); + // + // if (Thread.GetDomain().GetData(".appPath") != null) + // { + // HttpContext.Current = new HttpContext(new SimpleWorkerRequest("", "", new StringWriter())); + // } + // else + // { + // var baseDir = IOHelper.MapPath("").TrimEnd(Path.DirectorySeparatorChar); + // HttpContext.Current = new HttpContext(new SimpleWorkerRequest("/", baseDir, "", "", new StringWriter())); + // } + // + // var usersController = new AuthenticationController( + // new TestUserPasswordConfig(), + // Factory.GetInstance(), + // Factory.GetInstance(), + // umbracoContextAccessor, + // Factory.GetInstance(), + // Factory.GetInstance(), + // Factory.GetInstance(), + // Factory.GetInstance(), + // Factory.GetInstance(), + // Factory.GetInstance(), + // Factory.GetInstance(), + // Factory.GetInstance(), + // Factory.GetInstance(), + // Factory.GetInstance() + // ); + // return usersController; + // } + // + // Mock.Get(Current.SqlContext) + // .Setup(x => x.Query()) + // .Returns(new Query(Current.SqlContext)); + // + // var syntax = new SqlCeSyntaxProvider(); + // + // Mock.Get(Current.SqlContext) + // .Setup(x => x.SqlSyntax) + // .Returns(syntax); + // + // var mappers = new MapperCollection(new[] + // { + // new UserMapper(new Lazy(() => Current.SqlContext), new ConcurrentDictionary>()) + // }); + // + // Mock.Get(Current.SqlContext) + // .Setup(x => x.Mappers) + // .Returns(mappers); + // + // // Testing what happens if the system were configured to only use FIPS-compliant algorithms + // var typ = typeof(CryptoConfig); + // var flds = typ.GetFields(BindingFlags.Static | BindingFlags.NonPublic); + // var haveFld = flds.FirstOrDefault(f => f.Name == "s_haveFipsAlgorithmPolicy"); + // var isFld = flds.FirstOrDefault(f => f.Name == "s_fipsAlgorithmPolicy"); + // var originalFipsValue = CryptoConfig.AllowOnlyFipsAlgorithms; + // + // try + // { + // if (!originalFipsValue) + // { + // haveFld.SetValue(null, true); + // isFld.SetValue(null, true); + // } + // + // var runner = new TestRunner(CtrlFactory); + // var response = await runner.Execute("Authentication", "GetCurrentUser", HttpMethod.Get); + // + // var obj = JsonConvert.DeserializeObject(response.Item2); + // Assert.AreEqual(-1, obj.UserId); + // } + // finally + // { + // if (!originalFipsValue) + // { + // haveFld.SetValue(null, false); + // isFld.SetValue(null, false); + // } + // } + // } } } From 1973dd371a5a38c592ac5062d13fc56a224c249f Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Wed, 10 Jun 2020 11:17:55 +0200 Subject: [PATCH 14/14] Fix for login doing install Signed-off-by: Bjarke Berg --- .../Composing/CompositionExtensions/Installer.cs | 2 +- src/Umbraco.Infrastructure/Install/InstallHelper.cs | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Installer.cs b/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Installer.cs index 585d6badfb..707fcdbdc1 100644 --- a/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Installer.cs +++ b/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Installer.cs @@ -27,7 +27,7 @@ namespace Umbraco.Web.Composing.CompositionExtensions composition.Register(Lifetime.Scope); composition.Register(); - composition.Register(); + composition.RegisterUnique(); return composition; } diff --git a/src/Umbraco.Infrastructure/Install/InstallHelper.cs b/src/Umbraco.Infrastructure/Install/InstallHelper.cs index 9c3eddbf2a..1333363355 100644 --- a/src/Umbraco.Infrastructure/Install/InstallHelper.cs +++ b/src/Umbraco.Infrastructure/Install/InstallHelper.cs @@ -49,6 +49,9 @@ namespace Umbraco.Web.Install _userAgentProvider = userAgentProvider; _umbracoDatabaseFactory = umbracoDatabaseFactory; _jsonSerializer = jsonSerializer; + + //We need to initialize the type already, as we can't detect later, if the connection string is added on the fly. + GetInstallationType(); } public InstallationType GetInstallationType()