From 4763c28c4aa4c6e53b194a4061abc0bff69567f2 Mon Sep 17 00:00:00 2001 From: Scott Brady Date: Wed, 22 Apr 2020 20:05:06 +0100 Subject: [PATCH] Completed No-op lookup normalizer and OWIN daa protection token provider --- .../Security/NopLookupNormalizerTests.cs | 52 ++++ .../OwinDataProtectorTokenProviderTests.cs | 246 ++++++++++++++++++ src/Umbraco.Tests/Umbraco.Tests.csproj | 2 + .../Security/NopLookupNormalizer.cs | 16 +- .../OwinDataProtectorTokenProvider.cs | 39 +-- 5 files changed, 323 insertions(+), 32 deletions(-) create mode 100644 src/Umbraco.Tests/Security/NopLookupNormalizerTests.cs create mode 100644 src/Umbraco.Tests/Security/OwinDataProtectorTokenProviderTests.cs diff --git a/src/Umbraco.Tests/Security/NopLookupNormalizerTests.cs b/src/Umbraco.Tests/Security/NopLookupNormalizerTests.cs new file mode 100644 index 0000000000..1e035035dc --- /dev/null +++ b/src/Umbraco.Tests/Security/NopLookupNormalizerTests.cs @@ -0,0 +1,52 @@ +using System; +using NUnit.Framework; +using Umbraco.Web.Security; + +namespace Umbraco.Tests.Security +{ + public class NopLookupNormalizerTests + { + [Test] + [TestCase(null)] + [TestCase("")] + [TestCase(" ")] + public void NormalizeName_When_Name_Null_Or_Whitespace_Expect_ArgumentNullException(string name) + { + var sut = new NopLookupNormalizer(); + + Assert.Throws(() => sut.NormalizeName(name)); + } + + [Test] + public void NormalizeName_Expect_Input_Returned() + { + var name = Guid.NewGuid().ToString(); + var sut = new NopLookupNormalizer(); + + var normalizedName = sut.NormalizeName(name); + + Assert.AreEqual(name, normalizedName); + } + [Test] + [TestCase(null)] + [TestCase("")] + [TestCase(" ")] + public void NormalizeEmail_When_Name_Null_Or_Whitespace_Expect_ArgumentNullException(string email) + { + var sut = new NopLookupNormalizer(); + + Assert.Throws(() => sut.NormalizeEmail(email)); + } + + [Test] + public void NormalizeEmail_Expect_Input_Returned() + { + var name = $"{Guid.NewGuid()}@umbraco"; + var sut = new NopLookupNormalizer(); + + var normalizedName = sut.NormalizeEmail(name); + + Assert.AreEqual(name, normalizedName); + } + } +} diff --git a/src/Umbraco.Tests/Security/OwinDataProtectorTokenProviderTests.cs b/src/Umbraco.Tests/Security/OwinDataProtectorTokenProviderTests.cs new file mode 100644 index 0000000000..5e9b731aa9 --- /dev/null +++ b/src/Umbraco.Tests/Security/OwinDataProtectorTokenProviderTests.cs @@ -0,0 +1,246 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Identity; +using Microsoft.Owin.Security.DataProtection; +using Moq; +using NUnit.Framework; +using Umbraco.Core.Configuration; +using Umbraco.Core.Models.Membership; +using Umbraco.Web.Models.Identity; +using Umbraco.Web.Security; + +namespace Umbraco.Tests.Security +{ + public class OwinDataProtectorTokenProviderTests + { + private Mock _mockDataProtector; + private Mock> _mockUserManager; + private BackOfficeIdentityUser _testUser; + private const string _testPurpose = "test"; + + [Test] + public void Ctor_When_Protector_Is_Null_Expect_ArgumentNullException() + { + Assert.Throws(() => new OwinDataProtectorTokenProvider(null)); + } + + [Test] + public async Task CanGenerateTwoFactorTokenAsync_Expect_False() + { + var sut = CreateSut(); + + var canGenerate = await sut.CanGenerateTwoFactorTokenAsync(_mockUserManager.Object, _testUser); + + Assert.False(canGenerate); + } + + [Test] + public void GenerateAsync_When_UserManager_Is_Null_Expect_ArgumentNullException() + { + var sut = CreateSut(); + Assert.ThrowsAsync(async () => await sut.GenerateAsync(null, null, _testUser)); + } + + [Test] + public void GenerateAsync_When_User_Is_Null_Expect_ArgumentNullException() + { + var sut = CreateSut(); + Assert.ThrowsAsync(async () => await sut.GenerateAsync(null, _mockUserManager.Object, null)); + } + + [Test] + public async Task GenerateAsync_When_Token_Generated_Expect_Ticks_And_User_ID() + { + var sut = CreateSut(); + + var token = await sut.GenerateAsync(null, _mockUserManager.Object, _testUser); + + using (var reader = new BinaryReader(new MemoryStream(Convert.FromBase64String(token)))) + { + var creationTime = new DateTimeOffset(reader.ReadInt64(), TimeSpan.Zero); + var foundUserId = reader.ReadString(); + + Assert.That(creationTime.DateTime, Is.EqualTo(DateTime.UtcNow).Within(1).Minutes); + Assert.AreEqual(_testUser.Id.ToString(), foundUserId); + } + } + + [Test] + public async Task GenerateAsync_When_Token_Generated_With_Purpose_Expect_Purpose_In_Token() + { + var expectedPurpose = Guid.NewGuid().ToString(); + + var sut = CreateSut(); + + var token = await sut.GenerateAsync(expectedPurpose, _mockUserManager.Object, _testUser); + + using (var reader = new BinaryReader(new MemoryStream(Convert.FromBase64String(token)))) + { + reader.ReadInt64(); // creation time + reader.ReadString(); // user ID + var purpose = reader.ReadString(); + + Assert.AreEqual(expectedPurpose, purpose); + } + } + + [Test] + public async Task GenerateAsync_When_Token_Generated_And_SecurityStamp_Supported_Expect_SecurityStamp_In_Token() + { + var expectedSecurityStamp = Guid.NewGuid().ToString(); + + var sut = CreateSut(); + _mockUserManager.Setup(x => x.SupportsUserSecurityStamp).Returns(true); + _mockUserManager.Setup(x => x.GetSecurityStampAsync(_testUser)).ReturnsAsync(expectedSecurityStamp); + + var token = await sut.GenerateAsync(null, _mockUserManager.Object, _testUser); + + using (var reader = new BinaryReader(new MemoryStream(Convert.FromBase64String(token)))) + { + reader.ReadInt64(); // creation time + reader.ReadString(); // user ID + reader.ReadString(); // purpose + var securityStamp = reader.ReadString(); + + Assert.AreEqual(expectedSecurityStamp, securityStamp); + } + } + + [Test] + [TestCase(null)] + [TestCase("")] + [TestCase(" ")] + public void ValidateAsync_When_Token_Is_Null_Or_Whitespace_Expect_ArgumentNullException(string token) + { + var sut = CreateSut(); + + Assert.ThrowsAsync(() => sut.ValidateAsync(null, token, _mockUserManager.Object, _testUser)); + } + + [Test] + public void ValidateAsync_When_UserManager_Is_Null_Expect_ArgumentNullException() + { + var sut = CreateSut(); + + Assert.ThrowsAsync(() => sut.ValidateAsync(null, Guid.NewGuid().ToString(), null, _testUser)); + } + + [Test] + public void ValidateAsync_When_User_Is_Null_Expect_ArgumentNullException() + { + var sut = CreateSut(); + + Assert.ThrowsAsync(() => sut.ValidateAsync(null, Guid.NewGuid().ToString(), _mockUserManager.Object, null)); + } + + [Test] + public async Task ValidateAsync_When_Token_Has_Expired_Expect_False() + { + var sut = CreateSut(); + var testToken = CreateTestToken(creationDate: DateTime.UtcNow.AddYears(-10)); + + var isValid = await sut.ValidateAsync(null, testToken, _mockUserManager.Object, _testUser); + + Assert.False(isValid); + } + + [Test] + public async Task ValidateAsync_When_Token_Was_Issued_To_Wrong_User_Expect_False() + { + var sut = CreateSut(); + var testToken = CreateTestToken(userId: Guid.NewGuid().ToString()); + + var isValid = await sut.ValidateAsync(_testPurpose, testToken, _mockUserManager.Object, _testUser); + + Assert.False(isValid); + } + + [Test] + public async Task ValidateAsync_When_Token_Was_Has_Wrong_Purpose_Expect_False() + { + var sut = CreateSut(); + var testToken = CreateTestToken(purpose: "invalid"); + + var isValid = await sut.ValidateAsync("valid", testToken, _mockUserManager.Object, _testUser); + + Assert.False(isValid); + } + + [Test] + public async Task ValidateAsync_When_Token_Was_Has_Wrong_SecurityStamp_Expect_False() + { + var sut = CreateSut(); + _mockUserManager.Setup(x => x.SupportsUserSecurityStamp).Returns(true); + _mockUserManager.Setup(x => x.GetSecurityStampAsync(_testUser)).ReturnsAsync(Guid.NewGuid().ToString); + + var testToken = CreateTestToken(securityStamp: "invalid"); + + var isValid = await sut.ValidateAsync(_testPurpose, testToken, _mockUserManager.Object, _testUser); + + Assert.False(isValid); + } + + [Test] + public async Task ValidateAsync_When_Valid_Token_Expect_True() + { + const string validPurpose = "test"; + var validSecurityStamp = Guid.NewGuid().ToString(); + + var sut = CreateSut(); + _mockUserManager.Setup(x => x.SupportsUserSecurityStamp).Returns(true); + _mockUserManager.Setup(x => x.GetSecurityStampAsync(_testUser)).ReturnsAsync(validSecurityStamp); + + var testToken = CreateTestToken( + creationDate: DateTime.UtcNow, + userId: _testUser.Id.ToString(), + purpose: validPurpose, + securityStamp: validSecurityStamp); + + var isValid = await sut.ValidateAsync(validPurpose, testToken, _mockUserManager.Object, _testUser); + + Assert.True(isValid); + } + + private OwinDataProtectorTokenProvider CreateSut() + => new OwinDataProtectorTokenProvider(_mockDataProtector.Object); + + private string CreateTestToken(DateTime? creationDate = null, string userId = null, string purpose = null, string securityStamp = null) + { + var ms = new MemoryStream(); + using (var writer = new BinaryWriter(ms)) + { + writer.Write(creationDate?.Ticks ?? DateTimeOffset.UtcNow.UtcTicks); + writer.Write(userId ?? _testUser.Id.ToString()); + writer.Write(purpose ?? _testPurpose); + writer.Write(securityStamp ?? ""); + } + + return Convert.ToBase64String(ms.ToArray()); + } + + [SetUp] + public void Setup() + { + _mockDataProtector = new Mock(); + _mockDataProtector.Setup(x => x.Protect(It.IsAny())).Returns((byte[] originalBytes) => originalBytes); + _mockDataProtector.Setup(x => x.Unprotect(It.IsAny())).Returns((byte[] originalBytes) => originalBytes); + + var mockGlobalSettings = new Mock(); + mockGlobalSettings.Setup(x => x.DefaultUILanguage).Returns("test"); + + _mockUserManager = new Mock>(new Mock>().Object, + null, null, null, null, null, null, null, null); + _mockUserManager.Setup(x => x.SupportsUserSecurityStamp).Returns(false); + + _testUser = new BackOfficeIdentityUser(mockGlobalSettings.Object, 2, new List()) + { + UserName = "alice", + Name = "Alice", + Email = "alice@umbraco.test", + SecurityStamp = Guid.NewGuid().ToString() + }; + } + } +} diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index c36d35a56d..b0c0321925 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -149,6 +149,8 @@ + + diff --git a/src/Umbraco.Web/Security/NopLookupNormalizer.cs b/src/Umbraco.Web/Security/NopLookupNormalizer.cs index 211cea076e..48e42c2d5e 100644 --- a/src/Umbraco.Web/Security/NopLookupNormalizer.cs +++ b/src/Umbraco.Web/Security/NopLookupNormalizer.cs @@ -1,4 +1,5 @@ -using Microsoft.AspNetCore.Identity; +using System; +using Microsoft.AspNetCore.Identity; namespace Umbraco.Web.Security { @@ -7,7 +8,16 @@ namespace Umbraco.Web.Security /// public class NopLookupNormalizer : ILookupNormalizer { - public string NormalizeName(string name) => name; - public string NormalizeEmail(string email) => email; + public string NormalizeName(string name) + { + if (string.IsNullOrWhiteSpace(name)) throw new ArgumentNullException(nameof(name)); + return name; + } + + public string NormalizeEmail(string email) + { + if (string.IsNullOrWhiteSpace(email)) throw new ArgumentNullException(nameof(email)); + return email; + } } } diff --git a/src/Umbraco.Web/Security/OwinDataProtectorTokenProvider.cs b/src/Umbraco.Web/Security/OwinDataProtectorTokenProvider.cs index 51a42aae60..15bd4dfd75 100644 --- a/src/Umbraco.Web/Security/OwinDataProtectorTokenProvider.cs +++ b/src/Umbraco.Web/Security/OwinDataProtectorTokenProvider.cs @@ -15,6 +15,7 @@ namespace Umbraco.Web.Security public class OwinDataProtectorTokenProvider : IUserTwoFactorTokenProvider where TUser : BackOfficeIdentityUser { public TimeSpan TokenLifespan { get; set; } + private static readonly Encoding _defaultEncoding = new UTF8Encoding(false, true); private readonly IDataProtector _protector; public OwinDataProtectorTokenProvider(IDataProtector protector) @@ -25,12 +26,13 @@ namespace Umbraco.Web.Security public async Task GenerateAsync(string purpose, UserManager manager, TUser user) { + if (manager == null) throw new ArgumentNullException(nameof(manager)); if (user == null) throw new ArgumentNullException(nameof(user)); var ms = new MemoryStream(); - using (var writer = ms.CreateWriter()) + using (var writer = new BinaryWriter(ms, _defaultEncoding, true)) { - writer.Write(DateTimeOffset.UtcNow); + writer.Write(DateTimeOffset.UtcNow.UtcTicks); writer.Write(Convert.ToString(user.Id, CultureInfo.InvariantCulture)); writer.Write(purpose ?? ""); @@ -48,13 +50,17 @@ namespace Umbraco.Web.Security public async Task ValidateAsync(string purpose, string token, UserManager manager, TUser user) { + if (string.IsNullOrWhiteSpace(token)) throw new ArgumentNullException(nameof(token)); + if (manager == null) throw new ArgumentNullException(nameof(manager)); + if (user == null) throw new ArgumentNullException(nameof(user)); + try { var unprotectedData = _protector.Unprotect(Convert.FromBase64String(token)); var ms = new MemoryStream(unprotectedData); - using (var reader = ms.CreateReader()) + using (var reader = new BinaryReader(ms, _defaultEncoding, true)) { - var creationTime = reader.ReadDateTimeOffset(); + var creationTime = new DateTimeOffset(reader.ReadInt64(), TimeSpan.Zero); var expirationTime = creationTime + TokenLifespan; if (expirationTime < DateTimeOffset.UtcNow) { @@ -103,29 +109,4 @@ namespace Umbraco.Web.Security return Task.FromResult(false); } } - - internal static class StreamExtensions - { - private static readonly Encoding DefaultEncoding = new UTF8Encoding(false, true); - - public static BinaryReader CreateReader(this Stream stream) - { - return new BinaryReader(stream, DefaultEncoding, true); - } - - public static BinaryWriter CreateWriter(this Stream stream) - { - return new BinaryWriter(stream, DefaultEncoding, true); - } - - public static DateTimeOffset ReadDateTimeOffset(this BinaryReader reader) - { - return new DateTimeOffset(reader.ReadInt64(), TimeSpan.Zero); - } - - public static void Write(this BinaryWriter writer, DateTimeOffset value) - { - writer.Write(value.UtcTicks); - } - } }