From 8a70c440f26ddd4c8b29412191bafb336e2d8d6c Mon Sep 17 00:00:00 2001 From: Shannon Date: Sat, 28 Dec 2013 18:55:43 +1100 Subject: [PATCH] Added many more unit tests for the membership provider base class and fixed some logic inconsistencies. --- .../NameValueCollectionExtensions.cs | 36 +- .../Security/MembershipProviderBase.cs | 105 ++++-- .../MembersMembershipProviderTests.cs | 129 +++++++ .../Membership/MembershipProviderBaseTests.cs | 318 +++++++++++------- src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + .../Providers/MembersMembershipProvider.cs | 7 +- src/umbraco.businesslogic/User.cs | 3 + .../businesslogic/member/Member.cs | 4 +- .../UsersMembershipProvider.cs | 29 +- .../members/UmbracoMembershipProvider.cs | 35 +- 10 files changed, 422 insertions(+), 245 deletions(-) create mode 100644 src/Umbraco.Tests/Membership/MembersMembershipProviderTests.cs diff --git a/src/Umbraco.Core/NameValueCollectionExtensions.cs b/src/Umbraco.Core/NameValueCollectionExtensions.cs index 0bfa485438..d47add61ed 100644 --- a/src/Umbraco.Core/NameValueCollectionExtensions.cs +++ b/src/Umbraco.Core/NameValueCollectionExtensions.cs @@ -13,23 +13,23 @@ namespace Umbraco.Core { return collection.Keys.Cast().Any(k => (string) k == key); } - - public static T GetValue(this NameValueCollection collection, string key, T defaultIfNotFound) - { - if (collection.ContainsKey(key) == false) - { - return defaultIfNotFound; - } - - var val = collection[key]; - if (val == null) - { - return defaultIfNotFound; - } - - var result = val.TryConvertTo(); - - return result.Success ? result.Result : defaultIfNotFound; - } + + public static T GetValue(this NameValueCollection collection, string key, T defaultIfNotFound) + { + if (collection.ContainsKey(key) == false) + { + return defaultIfNotFound; + } + + var val = collection[key]; + if (val == null) + { + return defaultIfNotFound; + } + + var result = val.TryConvertTo(); + + return result.Success ? result.Result : defaultIfNotFound; + } } } diff --git a/src/Umbraco.Core/Security/MembershipProviderBase.cs b/src/Umbraco.Core/Security/MembershipProviderBase.cs index 3794327781..5480b8220c 100644 --- a/src/Umbraco.Core/Security/MembershipProviderBase.cs +++ b/src/Umbraco.Core/Security/MembershipProviderBase.cs @@ -61,7 +61,7 @@ namespace Umbraco.Core.Security private string _passwordStrengthRegularExpression; private bool _requiresQuestionAndAnswer; private bool _requiresUniqueEmail; - + private string _customHashAlgorithmType ; internal bool UseLegacyEncoding; #region Properties @@ -251,7 +251,8 @@ namespace Umbraco.Core.Security LogHelper.Error("Cannot specify a Hashed password format with the enabledPasswordRetrieval option set to true", ex); throw ex; } - + + _customHashAlgorithmType = config.GetValue("hashAlgorithmType", string.Empty); } /// @@ -582,15 +583,26 @@ namespace Umbraco.Core.Security return num; } - protected string FormatPasswordForStorage(string pass, string salt) + /// + /// If the password format is a hashed keyed algorithm then we will pre-pend the salt used to hash the password + /// to the hashed password itself. + /// + /// + /// + /// + protected internal string FormatPasswordForStorage(string pass, string salt) { if (UseLegacyEncoding) { return pass; } - - //the better way, we use salt per member - return salt + pass; + + if (PasswordFormat == MembershipPasswordFormat.Hashed) + { + //the better way, we use salt per member + return salt + pass; + } + return pass; } protected bool IsEmailValid(string email) @@ -602,7 +614,7 @@ namespace Umbraco.Core.Security return Regex.IsMatch(email, pattern, RegexOptions.IgnoreCase | RegexOptions.Compiled); } - protected string EncryptOrHashPassword(string pass, string salt) + protected internal string EncryptOrHashPassword(string pass, string salt) { //if we are doing it the old way @@ -613,12 +625,13 @@ namespace Umbraco.Core.Security //This is the correct way to implement this (as per the sql membership provider) - if ((int)PasswordFormat == 0) + if (PasswordFormat == MembershipPasswordFormat.Clear) return pass; var bytes = Encoding.Unicode.GetBytes(pass); var numArray1 = Convert.FromBase64String(salt); byte[] inArray; - if ((int)PasswordFormat == 1) + + if (PasswordFormat == MembershipPasswordFormat.Hashed) { var hashAlgorithm = GetHashAlgorithm(pass); var algorithm = hashAlgorithm as KeyedHashAlgorithm; @@ -657,6 +670,8 @@ namespace Umbraco.Core.Security } else { + //this code is copied from the sql membership provider - pretty sure this could be nicely re-written to completely + // ignore the salt stuff since we are not salting the password when encrypting. var password = new byte[numArray1.Length + bytes.Length]; Buffer.BlockCopy(numArray1, 0, password, 0, numArray1.Length); Buffer.BlockCopy(bytes, 0, password, numArray1.Length, bytes.Length); @@ -665,6 +680,31 @@ namespace Umbraco.Core.Security return Convert.ToBase64String(inArray); } + /// + /// Checks the password. + /// + /// The password. + /// The dbPassword. + /// + protected internal bool CheckPassword(string password, string dbPassword) + { + switch (PasswordFormat) + { + case MembershipPasswordFormat.Encrypted: + var decrypted = DecryptPassword(dbPassword); + return decrypted == password; + case MembershipPasswordFormat.Hashed: + string salt; + var storedHashedPass = StoredPassword(dbPassword, out salt); + var hashed = EncryptOrHashPassword(password, salt); + return storedHashedPass == hashed; + case MembershipPasswordFormat.Clear: + return password == dbPassword; + default: + throw new ArgumentOutOfRangeException(); + } + } + /// /// Encrypt/hash a new password with a new salt /// @@ -677,26 +717,7 @@ namespace Umbraco.Core.Security return EncryptOrHashPassword(newPassword, salt); } - /// - /// Gets the encrypted or hashed string of an existing password for an existing user - /// - /// The stored string for the password - /// - protected string EncryptOrHashExistingPassword(string storedPassword) - { - if (UseLegacyEncoding) - { - return EncryptOrHashPassword(storedPassword, storedPassword); - } - else - { - string salt; - var pass = StoredPassword(storedPassword, PasswordFormat, out salt); - return EncryptOrHashPassword(pass, salt); - } - } - - protected string DecodePassword(string pass) + protected internal string DecryptPassword(string pass) { //if we are doing it the old way @@ -712,7 +733,7 @@ namespace Umbraco.Core.Security case 0: return pass; case 1: - throw new ProviderException("Provider can not decode hashed password"); + throw new ProviderException("Provider can not decrypt hashed password"); default: var bytes = DecryptPassword(Convert.FromBase64String(pass)); return bytes == null ? null : Encoding.Unicode.GetString(bytes, 16, bytes.Length - 16); @@ -723,17 +744,16 @@ namespace Umbraco.Core.Security /// Returns the hashed password without the salt if it is hashed /// /// - /// /// returns the salt /// - internal static string StoredPassword(string storedString, MembershipPasswordFormat format, out string salt) + internal string StoredPassword(string storedString, out string salt) { - switch (format) + switch (PasswordFormat) { case MembershipPasswordFormat.Hashed: var saltLen = GenerateSalt(); salt = storedString.Substring(0, saltLen.Length); - return storedString.Substring(saltLen.Length); + return storedString.Substring(saltLen.Length); case MembershipPasswordFormat.Clear: case MembershipPasswordFormat.Encrypted: default: @@ -750,7 +770,7 @@ namespace Umbraco.Core.Security return Convert.ToBase64String(numArray); } - protected HashAlgorithm GetHashAlgorithm(string password) + protected internal HashAlgorithm GetHashAlgorithm(string password) { if (UseLegacyEncoding) { @@ -765,10 +785,21 @@ namespace Umbraco.Core.Security }; } } - + //get the algorithm by name - return HashAlgorithm.Create(Membership.HashAlgorithmType); + if (_customHashAlgorithmType.IsNullOrWhiteSpace()) + { + _customHashAlgorithmType = Membership.HashAlgorithmType; + } + + var alg = HashAlgorithm.Create(_customHashAlgorithmType); + if (alg == null) + { + throw new InvalidOperationException("The hash algorithm specified " + Membership.HashAlgorithmType + " cannot be resolved"); + } + + return alg; } /// diff --git a/src/Umbraco.Tests/Membership/MembersMembershipProviderTests.cs b/src/Umbraco.Tests/Membership/MembersMembershipProviderTests.cs new file mode 100644 index 0000000000..411c22f750 --- /dev/null +++ b/src/Umbraco.Tests/Membership/MembersMembershipProviderTests.cs @@ -0,0 +1,129 @@ +using System.Collections.Specialized; +using System.Web.Security; +using Moq; +using NUnit.Framework; +using Umbraco.Core; +using Umbraco.Core.Models; +using Umbraco.Core.Services; +using Umbraco.Tests.TestHelpers.Entities; +using Umbraco.Web.Security.Providers; + +namespace Umbraco.Tests.Membership +{ + [TestFixture] + public class MembersMembershipProviderTests + { + //[Test] + //public void Set_Default_Member_Type_On_Init() + + //[Test] + //public void Create_User_Already_Exists() + //{ + + //} + + //[Test] + //public void Create_User_Requires_Unique_Email() + //{ + + //} + + [Test] + public void Answer_Is_Encrypted() + { + IMember createdMember = null; + var memberType = MockedContentTypes.CreateSimpleMemberType(); + foreach (var p in Constants.Conventions.Member.GetStandardPropertyTypeStubs()) + { + memberType.AddPropertyType(p.Value); + } + var mServiceMock = new Mock(); + mServiceMock.Setup(service => service.Exists("test")).Returns(false); + mServiceMock.Setup(service => service.GetByEmail("test@test.com")).Returns(() => null); + mServiceMock.Setup( + service => service.CreateMember(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((string u, string e, string p, string m) => + { + createdMember = new Member("test", e, u, p, memberType); + }) + .Returns(() => createdMember); + var provider = new MembersMembershipProvider(mServiceMock.Object); + + MembershipCreateStatus status; + provider.CreateUser("test", "test", "test", "test@test.com", "test", "test", true, "test", out status); + + Assert.AreNotEqual("test", createdMember.PasswordAnswer); + Assert.AreEqual(provider.EncryptString("test"), createdMember.PasswordAnswer); + } + + [Test] + public void Password_Encrypted() + { + IMember createdMember = null; + var memberType = MockedContentTypes.CreateSimpleMemberType(); + foreach (var p in Constants.Conventions.Member.GetStandardPropertyTypeStubs()) + { + memberType.AddPropertyType(p.Value); + } + var mServiceMock = new Mock(); + mServiceMock.Setup(service => service.Exists("test")).Returns(false); + mServiceMock.Setup(service => service.GetByEmail("test@test.com")).Returns(() => null); + mServiceMock.Setup( + service => service.CreateMember(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((string u, string e, string p, string m) => + { + createdMember = new Member("test", e, u, p, memberType); + }) + .Returns(() => createdMember); + + var provider = new MembersMembershipProvider(mServiceMock.Object); + provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Encrypted" } }); + MembershipCreateStatus status; + provider.CreateUser("test", "test", "test", "test@test.com", "test", "test", true, "test", out status); + + Assert.AreNotEqual("test", createdMember.Password); + var decrypted = provider.DecryptPassword(createdMember.Password); + Assert.AreEqual("test", decrypted); + } + + [Test] + public void Password_Hashed_With_Salt() + { + IMember createdMember = null; + var memberType = MockedContentTypes.CreateSimpleMemberType(); + foreach (var p in Constants.Conventions.Member.GetStandardPropertyTypeStubs()) + { + memberType.AddPropertyType(p.Value); + } + var mServiceMock = new Mock(); + mServiceMock.Setup(service => service.Exists("test")).Returns(false); + mServiceMock.Setup(service => service.GetByEmail("test@test.com")).Returns(() => null); + mServiceMock.Setup( + service => service.CreateMember(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((string u, string e, string p, string m) => + { + createdMember = new Member("test", e, u, p, memberType); + }) + .Returns(() => createdMember); + + var provider = new MembersMembershipProvider(mServiceMock.Object); + provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Hashed" }, { "hashAlgorithmType", "HMACSHA256" } }); + MembershipCreateStatus status; + provider.CreateUser("test", "test", "test", "test@test.com", "test", "test", true, "test", out status); + + Assert.AreNotEqual("test", createdMember.Password); + + string salt; + var storedPassword = provider.StoredPassword(createdMember.Password, out salt); + var hashedPassword = provider.EncryptOrHashPassword("test", salt); + Assert.AreEqual(hashedPassword, storedPassword); + } + + //[Test] + //public void Password_Encrypted_Validated_With_Salt() + + //[Test] + //public void Password_Encrypted_Validated_With_Salt() + + } +} \ No newline at end of file diff --git a/src/Umbraco.Tests/Membership/MembershipProviderBaseTests.cs b/src/Umbraco.Tests/Membership/MembershipProviderBaseTests.cs index 2af97b49e2..defa6fb4cd 100644 --- a/src/Umbraco.Tests/Membership/MembershipProviderBaseTests.cs +++ b/src/Umbraco.Tests/Membership/MembershipProviderBaseTests.cs @@ -4,138 +4,16 @@ using System.Collections.Specialized; using System.Configuration.Provider; using System.Diagnostics; using System.Linq; +using System.Security.Cryptography; using System.Text; using System.Threading.Tasks; using System.Web.Security; using Moq; using NUnit.Framework; -using Umbraco.Core; -using Umbraco.Core.Models; using Umbraco.Core.Security; -using Umbraco.Core.Services; -using Umbraco.Tests.TestHelpers.Entities; -using Umbraco.Web.Security.Providers; namespace Umbraco.Tests.Membership { - [TestFixture] - public class MembersMembershipProviderTests - { - //[Test] - //public void Set_Default_Member_Type_On_Init() - - //[Test] - //public void Create_User_Already_Exists() - //{ - - //} - - //[Test] - //public void Create_User_Requires_Unique_Email() - //{ - - //} - - [Test] - public void Answer_Is_Encrypted() - { - IMember createdMember = null; - var memberType = MockedContentTypes.CreateSimpleMemberType(); - foreach (var p in Constants.Conventions.Member.GetStandardPropertyTypeStubs()) - { - memberType.AddPropertyType(p.Value); - } - var mServiceMock = new Mock(); - mServiceMock.Setup(service => service.Exists("test")).Returns(false); - mServiceMock.Setup(service => service.GetByEmail("test@test.com")).Returns(() => null); - mServiceMock.Setup( - service => service.CreateMember(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - .Callback((string u, string e, string p, string m) => - { - createdMember = new Member("test", e, u, p, memberType); - }) - .Returns(() => createdMember); - var provider = new MembersMembershipProvider(mServiceMock.Object); - - MembershipCreateStatus status; - provider.CreateUser("test", "test", "test", "test@test.com", "test", "test", true, "test", out status); - - Assert.AreNotEqual("test", createdMember.PasswordAnswer); - Assert.AreEqual(provider.EncryptString("test"), createdMember.PasswordAnswer); - } - - [Test] - public void Password_Encrypted_With_Salt() - { - IMember createdMember = null; - var memberType = MockedContentTypes.CreateSimpleMemberType(); - foreach (var p in Constants.Conventions.Member.GetStandardPropertyTypeStubs()) - { - memberType.AddPropertyType(p.Value); - } - var mServiceMock = new Mock(); - mServiceMock.Setup(service => service.Exists("test")).Returns(false); - mServiceMock.Setup(service => service.GetByEmail("test@test.com")).Returns(() => null); - mServiceMock.Setup( - service => service.CreateMember(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - .Callback((string u, string e, string p, string m) => - { - createdMember = new Member("test", e, u, p, memberType); - }) - .Returns(() => createdMember); - - var provider = new MembersMembershipProvider(mServiceMock.Object); - provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Encrypted" } }); - MembershipCreateStatus status; - provider.CreateUser("test", "test", "test", "test@test.com", "test", "test", true, "test", out status); - - Assert.AreNotEqual("test", createdMember.Password); - //Assert.AreNotEqual(provider.EncryptString("test"), createdMember.PasswordAnswer); - string salt; - var encodedPassword = provider.EncryptOrHashNewPassword("test", out salt); - Assert.AreEqual(encodedPassword, createdMember.Password); - } - - //[Test] - //public void Password_Hashed_With_Salt() - //{ - // IMember createdMember = null; - // var memberType = MockedContentTypes.CreateSimpleMemberType(); - // foreach (var p in Constants.Conventions.Member.GetStandardPropertyTypeStubs()) - // { - // memberType.AddPropertyType(p.Value); - // } - // var mServiceMock = new Mock(); - // mServiceMock.Setup(service => service.Exists("test")).Returns(false); - // mServiceMock.Setup(service => service.GetByEmail("test@test.com")).Returns(() => null); - // mServiceMock.Setup( - // service => service.CreateMember(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - // .Callback((string u, string e, string p, string m) => - // { - // createdMember = new Member("test", e, u, p, memberType); - // }) - // .Returns(() => createdMember); - - // var provider = new MembersMembershipProvider(mServiceMock.Object); - // provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Hashed" } }); - // MembershipCreateStatus status; - // provider.CreateUser("test", "test", "test", "test@test.com", "test", "test", true, "test", out status); - - // Assert.AreNotEqual("test", createdMember.Password); - // Assert.AreNotEqual(provider.EncryptString("test"), createdMember.PasswordAnswer); - // string salt; - // var encodedPassword = provider.EncryptOrHashNewPassword("test", out salt); - // Assert.AreEqual(encodedPassword, createdMember.Password); - //} - - //[Test] - //public void Password_Encrypted_Validated_With_Salt() - - //[Test] - //public void Password_Encrypted_Validated_With_Salt() - - } - [TestFixture] public class MembershipProviderBaseTests { @@ -330,18 +208,204 @@ namespace Umbraco.Tests.Membership lastLength = result.Length; } } - + [Test] - public void Get_StoredPassword() + public void Get_Stored_Password_Hashed() { + var providerMock = new Mock() { CallBase = true }; + var provider = providerMock.Object; + provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Hashed" }, { "hashAlgorithmType", "HMACSHA256" } }); + var salt = MembershipProviderBase.GenerateSalt(); var stored = salt + "ThisIsAHashedPassword"; string initSalt; - var result = MembershipProviderBase.StoredPassword(stored, MembershipPasswordFormat.Hashed, out initSalt); + var result = provider.StoredPassword(stored, out initSalt); - Assert.AreEqual(salt, initSalt); + Assert.AreEqual("ThisIsAHashedPassword", result); } + [Test] + public void Get_Stored_Password_Encrypted() + { + var providerMock = new Mock() { CallBase = true }; + var provider = providerMock.Object; + provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Encrypted" } }); + + var stored = "ThisIsAnEncryptedPassword"; + + string initSalt; + var result = provider.StoredPassword(stored, out initSalt); + + Assert.AreEqual("ThisIsAnEncryptedPassword", result); + } + + [Test] + public void Get_Stored_Password_Clear() + { + var providerMock = new Mock() { CallBase = true }; + var provider = providerMock.Object; + provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Clear" } }); + + var salt = MembershipProviderBase.GenerateSalt(); + var stored = "ThisIsAClearPassword"; + + string initSalt; + var result = provider.StoredPassword(stored, out initSalt); + + Assert.AreEqual("ThisIsAClearPassword", result); + } + + [Test] + public void Format_Pass_For_Storage_Hashed() + { + var providerMock = new Mock() { CallBase = true }; + var provider = providerMock.Object; + provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Hashed" }, { "hashAlgorithmType", "HMACSHA256" } }); + + var salt = MembershipProviderBase.GenerateSalt(); + var stored = "ThisIsAHashedPassword"; + + var result = provider.FormatPasswordForStorage(stored, salt); + + Assert.AreEqual(salt + "ThisIsAHashedPassword", result); + } + + [Test] + public void Format_Pass_For_Storage_Encrypted() + { + var providerMock = new Mock() { CallBase = true }; + var provider = providerMock.Object; + provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Encrypted" } }); + + var salt = MembershipProviderBase.GenerateSalt(); + var stored = "ThisIsAnEncryptedPassword"; + + var result = provider.FormatPasswordForStorage(stored, salt); + + Assert.AreEqual("ThisIsAnEncryptedPassword", result); + } + + [Test] + public void Format_Pass_For_Storage_Clear() + { + var providerMock = new Mock() { CallBase = true }; + var provider = providerMock.Object; + provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Clear" } }); + + var salt = MembershipProviderBase.GenerateSalt(); + var stored = "ThisIsAClearPassword"; + + var result = provider.FormatPasswordForStorage(stored, salt); + + Assert.AreEqual("ThisIsAClearPassword", result); + } + + [Test] + public void Check_Password_Hashed_KeyedHashAlgorithm() + { + var providerMock = new Mock() { CallBase = true }; + var provider = providerMock.Object; + provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Hashed" }, { "hashAlgorithmType", "HMACSHA256" } }); + + string salt; + var pass = "ThisIsAHashedPassword"; + var hashed = provider.EncryptOrHashNewPassword(pass, out salt); + var storedPassword = provider.FormatPasswordForStorage(hashed, salt); + + var result = provider.CheckPassword("ThisIsAHashedPassword", storedPassword); + + Assert.IsTrue(result); + } + + [Test] + public void Check_Password_Hashed_Non_KeyedHashAlgorithm() + { + var providerMock = new Mock() { CallBase = true }; + var provider = providerMock.Object; + provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Hashed" } }); + + string salt; + var pass = "ThisIsAHashedPassword"; + var hashed = provider.EncryptOrHashNewPassword(pass, out salt); + var storedPassword = provider.FormatPasswordForStorage(hashed, salt); + + var result = provider.CheckPassword("ThisIsAHashedPassword", storedPassword); + + Assert.IsTrue(result); + } + + [Test] + public void Check_Password_Encrypted() + { + var providerMock = new Mock() { CallBase = true }; + var provider = providerMock.Object; + provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Encrypted" } }); + + string salt; + var pass = "ThisIsAnEncryptedPassword"; + var encrypted = provider.EncryptOrHashNewPassword(pass, out salt); + + var result = provider.CheckPassword("ThisIsAnEncryptedPassword", encrypted); + + Assert.IsTrue(result); + } + + [Test] + public void Check_Password_Clear() + { + var providerMock = new Mock() { CallBase = true }; + var provider = providerMock.Object; + provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Clear" } }); + + var pass = "ThisIsAClearPassword"; + + var result = provider.CheckPassword("ThisIsAClearPassword", pass); + + Assert.IsTrue(result); + } + + [Test] + public void Can_Decrypt_Password() + { + var providerMock = new Mock() { CallBase = true }; + var provider = providerMock.Object; + provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Encrypted" } }); + + string salt; + var pass = "ThisIsAnEncryptedPassword"; + var encrypted = provider.EncryptOrHashNewPassword(pass, out salt); + + var result = provider.DecryptPassword(encrypted); + + Assert.AreEqual(pass, result); + + } + + [Test] + public void Get_Hash_Algorithm_Legacy() + { + var providerMock = new Mock() { CallBase = true }; + var provider = providerMock.Object; + provider.Initialize("test", new NameValueCollection { {"useLegacyEncoding", "true"}, { "passwordFormat", "Hashed" }, { "hashAlgorithmType", "HMACSHA256" } }); + + var alg = provider.GetHashAlgorithm("blah"); + + Assert.IsTrue(alg is HMACSHA1); + } + + [Test] + public void Get_Hash_Algorithm() + { + var providerMock = new Mock() { CallBase = true }; + var provider = providerMock.Object; + provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Hashed" }, { "hashAlgorithmType", "HMACSHA256" } }); + + var alg = provider.GetHashAlgorithm("blah"); + + Assert.IsTrue(alg is HMACSHA256); + } + + } } diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index b3dc588345..6a3ded0fde 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -155,6 +155,7 @@ + diff --git a/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs b/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs index e4fbda1d08..a735e3a9a2 100644 --- a/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs +++ b/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs @@ -310,7 +310,7 @@ namespace Umbraco.Web.Security.Providers throw new ProviderException("Incorrect password answer"); } - var decodedPassword = DecodePassword(m.Password); + var decodedPassword = DecryptPassword(m.Password); return decodedPassword; } @@ -511,10 +511,7 @@ namespace Umbraco.Web.Security.Providers return false; } - string salt; - var encodedPassword = EncryptOrHashNewPassword(password, out salt); - - var authenticated = (encodedPassword == member.Password); + var authenticated = CheckPassword(password, member.Password); if (authenticated == false) { diff --git a/src/umbraco.businesslogic/User.cs b/src/umbraco.businesslogic/User.cs index 5da83d2a6d..9584b2f1cd 100644 --- a/src/umbraco.businesslogic/User.cs +++ b/src/umbraco.businesslogic/User.cs @@ -212,6 +212,7 @@ namespace umbraco.BusinessLogic return UserType.Alias == "admin"; } + [Obsolete("Do not use this method to validate credentials, use the user's membership provider to do authentication. This method will not work if the password format is 'Encrypted'")] public bool ValidatePassword(string password) { string userLogin = @@ -310,6 +311,7 @@ namespace umbraco.BusinessLogic /// The login name. /// The password. /// + [Obsolete("Do not use this method to validate credentials, use the user's membership provider to do authentication. This method will not work if the password format is 'Encrypted'")] public static bool validateCredentials(string lname, string passw) { return validateCredentials(lname, passw, true); @@ -322,6 +324,7 @@ namespace umbraco.BusinessLogic /// The password. /// if set to true [check for umbraco console access]. /// + [Obsolete("Do not use this method to validate credentials, use the user's membership provider to do authentication. This method will not work if the password format is 'Encrypted'")] public static bool validateCredentials(string lname, string passw, bool checkForUmbracoConsoleAccess) { string consoleCheckSql = ""; diff --git a/src/umbraco.cms/businesslogic/member/Member.cs b/src/umbraco.cms/businesslogic/member/Member.cs index 9b051aea7b..40275672d1 100644 --- a/src/umbraco.cms/businesslogic/member/Member.cs +++ b/src/umbraco.cms/businesslogic/member/Member.cs @@ -353,7 +353,6 @@ namespace umbraco.cms.businesslogic.member /// Member login /// Member password /// The member with the credentials - null if none exists - public static Member GetMemberFromLoginNameAndPassword(string loginName, string password) { if (IsMember(loginName)) @@ -375,7 +374,8 @@ namespace umbraco.cms.businesslogic.member return null; } } - + + [Obsolete("This method will not work if the password format is encrypted since the encryption that is performed is not static and a new value will be created each time the same string is encrypted")] public static Member GetMemberFromLoginAndEncodedPassword(string loginName, string password) { var o = SqlHelper.ExecuteScalar( diff --git a/src/umbraco.providers/UsersMembershipProvider.cs b/src/umbraco.providers/UsersMembershipProvider.cs index 64534abf3a..2f625095ad 100644 --- a/src/umbraco.providers/UsersMembershipProvider.cs +++ b/src/umbraco.providers/UsersMembershipProvider.cs @@ -460,7 +460,7 @@ namespace umbraco.providers return true; } - return user.ValidatePassword(EncryptOrHashExistingPassword(password)); + return CheckPassword(password, user.Password); } } return false; @@ -468,32 +468,7 @@ namespace umbraco.providers #endregion #region Helper Methods - /// - /// Checks the password. - /// - /// The password. - /// The dbPassword. - /// - internal bool CheckPassword(string password, string dbPassword) - { - string pass1 = password; - string pass2 = dbPassword; - - switch (PasswordFormat) - { - case MembershipPasswordFormat.Encrypted: - pass2 = DecodePassword(dbPassword); - break; - case MembershipPasswordFormat.Hashed: - pass1 = EncryptOrHashExistingPassword(password); - break; - default: - break; - } - return (pass1 == pass2) ? true : false; - } - - + /// /// Encodes the password. /// diff --git a/src/umbraco.providers/members/UmbracoMembershipProvider.cs b/src/umbraco.providers/members/UmbracoMembershipProvider.cs index fa3a70804e..cf4714496d 100644 --- a/src/umbraco.providers/members/UmbracoMembershipProvider.cs +++ b/src/umbraco.providers/members/UmbracoMembershipProvider.cs @@ -452,7 +452,7 @@ namespace umbraco.providers.members } } - var decodedPassword = DecodePassword(m.GetPassword()); + var decodedPassword = DecryptPassword(m.GetPassword()); return decodedPassword; } @@ -667,8 +667,11 @@ namespace umbraco.providers.members /// public override bool ValidateUser(string username, string password) { - var m = Member.GetMemberFromLoginAndEncodedPassword(username, EncryptOrHashExistingPassword(password)); - if (m != null) + var m = Member.GetMemberFromLoginName(username); + if (m == null) return false; + var authenticated = CheckPassword(password, m.GetPassword()); + + if (authenticated) { // check for lock status. If locked, then set the member property to null if (string.IsNullOrEmpty(LockPropertyTypeAlias) == false) @@ -824,32 +827,6 @@ namespace umbraco.providers.members #region Helper Methods - /// - /// Checks the password. - /// - /// The password. - /// The dbPassword. - /// - internal bool CheckPassword(string password, string dbPassword) - { - string pass1 = password; - string pass2 = dbPassword; - - switch (PasswordFormat) - { - case MembershipPasswordFormat.Encrypted: - pass2 = DecodePassword(dbPassword); - break; - case MembershipPasswordFormat.Hashed: - pass1 = EncryptOrHashExistingPassword(password); - break; - default: - break; - } - return (pass1 == pass2) ? true : false; - } - - /// /// Encodes the password. ///