From 11f5c98206311c13368d4b95e845fc76bff827bf Mon Sep 17 00:00:00 2001 From: Scott Brady Date: Mon, 20 Apr 2020 19:45:08 +0100 Subject: [PATCH] Finished security stamp validator --- .../UmbracoSecurityStampValidatorTests.cs | 277 ++++++++++++++++++ src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + .../Security/AppBuilderExtensions.cs | 2 +- .../Security/UmbracoSecurityStampValidator.cs | 14 +- 4 files changed, 289 insertions(+), 5 deletions(-) create mode 100644 src/Umbraco.Tests/Security/UmbracoSecurityStampValidatorTests.cs diff --git a/src/Umbraco.Tests/Security/UmbracoSecurityStampValidatorTests.cs b/src/Umbraco.Tests/Security/UmbracoSecurityStampValidatorTests.cs new file mode 100644 index 0000000000..92fa032271 --- /dev/null +++ b/src/Umbraco.Tests/Security/UmbracoSecurityStampValidatorTests.cs @@ -0,0 +1,277 @@ +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.Configuration; +using Umbraco.Core.Models.Membership; +using Umbraco.Net; +using Umbraco.Web.Models.Identity; +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, BackOfficeIdentityUser>( + TimeSpan.MaxValue, null, null)); + } + + [Test] + public async Task OnValidateIdentity_When_Validation_Interval_Not_Met_Expect_No_Op() + { + var func = UmbracoSecurityStampValidator + .OnValidateIdentity, BackOfficeIdentityUser>( + 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, BackOfficeIdentityUser>( + TimeSpan.MinValue, null, identity => throw new Exception()); + + _mockOwinContext.Setup(x => x.Get>(It.IsAny())) + .Returns((BackOfficeUserManager) 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, BackOfficeIdentityUser>( + 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, BackOfficeIdentityUser>( + 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, BackOfficeIdentityUser>( + 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, BackOfficeIdentityUser>( + 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, BackOfficeIdentityUser>( + 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, BackOfficeIdentityUser, Task> regenFunc = + (signInManager, userManager, user) => + { + regenFuncCalled = true; + return Task.FromResult(expectedIdentity); + }; + + var func = UmbracoSecurityStampValidator + .OnValidateIdentity, BackOfficeIdentityUser>( + 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 3a7b33e0b5..1f54e1e629 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -148,6 +148,7 @@ + diff --git a/src/Umbraco.Web/Security/AppBuilderExtensions.cs b/src/Umbraco.Web/Security/AppBuilderExtensions.cs index fa39bcc42b..b7218a3723 100644 --- a/src/Umbraco.Web/Security/AppBuilderExtensions.cs +++ b/src/Umbraco.Web/Security/AppBuilderExtensions.cs @@ -155,7 +155,7 @@ namespace Umbraco.Web.Security // 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( + .OnValidateIdentity( TimeSpan.FromMinutes(3), (signInManager, manager, user) => signInManager.CreateUserIdentityAsync(user), identity => identity.GetUserId()), diff --git a/src/Umbraco.Web/Security/UmbracoSecurityStampValidator.cs b/src/Umbraco.Web/Security/UmbracoSecurityStampValidator.cs index f506a64fba..a5f9c17765 100644 --- a/src/Umbraco.Web/Security/UmbracoSecurityStampValidator.cs +++ b/src/Umbraco.Web/Security/UmbracoSecurityStampValidator.cs @@ -12,10 +12,11 @@ namespace Umbraco.Web.Security /// public class UmbracoSecurityStampValidator { - public static Func OnValidateIdentity( + public static Func OnValidateIdentity( TimeSpan validateInterval, Func> regenerateIdentityCallback, Func getUserIdCallback) + where TSignInManager : BackOfficeSignInManager where TManager : BackOfficeUserManager where TUser : BackOfficeIdentityUser { @@ -42,11 +43,14 @@ namespace Umbraco.Web.Security if (validate) { var manager = context.OwinContext.Get(); - var signInManager = context.OwinContext.GetBackOfficeSignInManager(); + 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 (manager != null && userId != null) + if (userId != null) { var user = await manager.FindByIdAsync(userId); var reject = true; @@ -55,7 +59,9 @@ namespace Umbraco.Web.Security if (user != null && manager.SupportsUserSecurityStamp) { var securityStamp = context.Identity.FindFirst(Constants.Web.SecurityStampClaimType)?.Value; - if (securityStamp == await manager.GetSecurityStampAsync(user)) + var newSecurityStamp = await manager.GetSecurityStampAsync(user); + + if (securityStamp == newSecurityStamp) { reject = false; // Regenerate fresh claims if possible and resign in