From 670d62a2dcecc22535ee04694a3514f731826273 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 2 Jun 2020 17:48:08 +1000 Subject: [PATCH] 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 @@ -