Merge branch 'v8/8.8' into v8/8.9

Author: nul800sebastiaan

src/Umbraco.Tests/Membership/MembersMembershipProviderTests.cs

@@ -0,0 +1,114 @@
+using System.Collections.Specialized;
+using System.Web.Security;
+using Moq;
+using NUnit.Framework;
+using Umbraco.Core;
+using Umbraco.Core.Cache;
+using Umbraco.Core.Composing;
+using Umbraco.Core.Logging;
+using Umbraco.Core.Models;
+using Umbraco.Core.Services;
+using Umbraco.Core.Sync;
+using Umbraco.Tests.Integration;
+using Umbraco.Tests.TestHelpers;
+using Umbraco.Tests.TestHelpers.Entities;
+using Umbraco.Tests.Testing;
+using Umbraco.Web;
+using Umbraco.Web.Cache;
+using Umbraco.Web.Security.Providers;
+
+namespace Umbraco.Tests.Membership
+{
+    [TestFixture]
+    [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerFixture)]
+    public class MembersMembershipProviderTests : TestWithDatabaseBase
+    {
+        private MembersMembershipProvider MembersMembershipProvider { get; set; }
+        private IDistributedCacheBinder DistributedCacheBinder { get; set; }
+
+        public IMemberService MemberService => Current.Factory.GetInstance<IMemberService>();
+        public IMemberTypeService MemberTypeService => Current.Factory.GetInstance<IMemberTypeService>();
+        public ILogger Logger => Current.Factory.GetInstance<ILogger>();
+
+        public override void SetUp()
+        {
+            base.SetUp();
+
+            MembersMembershipProvider = new MembersMembershipProvider(MemberService, MemberTypeService);
+
+            MembersMembershipProvider.Initialize("test", new NameValueCollection { { "passwordFormat", MembershipPasswordFormat.Clear.ToString() } });
+
+            DistributedCacheBinder = new DistributedCacheBinder(new DistributedCache(), Mock.Of<IUmbracoContextFactory>(), Logger);
+            DistributedCacheBinder.BindEvents(true);
+        }
+
+        [TearDown]
+        public void Teardown()
+        {
+            DistributedCacheBinder?.UnbindEvents();
+            DistributedCacheBinder = null;
+        }
+
+        protected override void Compose()
+        {
+            base.Compose();
+
+            // the cache refresher component needs to trigger to refresh caches
+            // but then, it requires a lot of plumbing ;(
+            // FIXME: and we cannot inject a DistributedCache yet
+            // so doing all this mess
+            Composition.RegisterUnique<IServerMessenger, ContentEventsTests.LocalServerMessenger>();
+            Composition.RegisterUnique(f => Mock.Of<IServerRegistrar>());
+            Composition.WithCollectionBuilder<CacheRefresherCollectionBuilder>()
+                .Add(() => Composition.TypeLoader.GetCacheRefreshers());
+        }
+
+        protected override AppCaches GetAppCaches()
+        {
+            // this is what's created core web runtime
+            return new AppCaches(
+                new DeepCloneAppCache(new ObjectCacheAppCache()),
+                NoAppCache.Instance,
+                new IsolatedCaches(type => new DeepCloneAppCache(new ObjectCacheAppCache())));
+        }
+
+        /// <summary>
+        /// MembersMembershipProvider.ValidateUser is expected to increase the number of failed attempts and also read that same number.
+        /// </summary>
+        /// <remarks>
+        /// This test requires the caching to be enabled, as it already is correct in the database.
+        /// Shows the error described here: https://github.com/umbraco/Umbraco-CMS/issues/9861
+        /// </remarks>
+        [Test]
+        public void ValidateUser__must_lock_out_users_after_max_attempts_of_wrong_password()
+        {
+            // Arrange
+            IMemberType memberType = MockedContentTypes.CreateSimpleMemberType();
+            ServiceContext.MemberTypeService.Save(memberType);
+            var member = MockedMember.CreateSimpleMember(memberType, "test", "[email protected]",  "password","test");
+            ServiceContext.MemberService.Save(member);
+
+            var wrongPassword = "wrongPassword";
+            var numberOfFailedAttempts = MembersMembershipProvider.MaxInvalidPasswordAttempts+2;
+
+            // Act
+            var memberBefore = ServiceContext.MemberService.GetById(member.Id);
+            for (int i = 0; i < numberOfFailedAttempts; i++)
+            {
+                MembersMembershipProvider.ValidateUser(member.Username, wrongPassword);
+            }
+            var memberAfter = ServiceContext.MemberService.GetById(member.Id);
+
+            // Assert
+            Assert.Multiple(() =>
+            {
+                Assert.AreEqual(0 , memberBefore.FailedPasswordAttempts, "Expected 0 failed password attempts before");
+                Assert.IsFalse(memberBefore.IsLockedOut, "Expected the member NOT to be locked out before");
+
+                Assert.AreEqual(MembersMembershipProvider.MaxInvalidPasswordAttempts, memberAfter.FailedPasswordAttempts, "Expected exactly the max possible failed password attempts after");
+                Assert.IsTrue(memberAfter.IsLockedOut, "Expected the member to be locked out after");
+            });
+
+        }
+    }
+}

src/Umbraco.Tests/Umbraco.Tests.csproj

@@ -132,6 +132,7 @@
     <Compile Include="Logging\LogviewerTests.cs" />
     <Compile Include="Manifest\ManifestContentAppTests.cs" />
     <Compile Include="Mapping\MappingTests.cs" />
+    <Compile Include="Membership\MembersMembershipProviderTests.cs" />
     <Compile Include="Migrations\MigrationPlanTests.cs" />
     <Compile Include="Migrations\MigrationTests.cs" />
     <Compile Include="ModelsBuilder\BuilderTests.cs" />

src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs

@@ -3,7 +3,6 @@
 using System.Configuration.Provider;
 using System.Linq;
 using System.Text;
-using System.Web;
 using System.Web.Configuration;
 using System.Web.Security;
 using Umbraco.Core;
@@ -14,7 +13,6 @@
 using Umbraco.Core.Security;
 using Umbraco.Core.Services;
 using Umbraco.Web.Composing;
-using Umbraco.Core.Models.Identity;
 
 namespace Umbraco.Web.Security.Providers
 {
@@ -357,7 +355,7 @@ public override MembershipUser GetUser(string username, bool userIsOnline)
                     member.LastLoginDate = now;
                     member.UpdateDate = now;
                 }
-                    
+
             }
 
             return ConvertToMembershipUser(member);
@@ -604,11 +602,21 @@ internal virtual ValidateUserResult PerformValidateUser(string username, string
             {
                 // when upgrading from 7.2 to 7.3 trying to save will throw
                 if (UmbracoVersion.Current >= new Version(7, 3, 0, 0))
-                    MemberService.Save(member, false);
+                {
+                    // We need to raise event to ensure caches are updated. (e.g. the cache that uses username as key).
+                    // Even that this is a heavy operation, because indexes are updates, we consider that okay, as it
+                    // is still cheap to do a successful login.
+                    MemberService.Save(member, true);
+                }
+
             }
             else
             {
-                // set the last login date without full save (fast, no locks)
+                // set the last login date without full save (fast, no locks).
+                // We do not update caches. This is to the best of our knowledge okay, as this info are only stored
+                // because it is required by the membership provider.
+                // If we one day have to revisit this, we will most likely need to spilt the events in membership info
+                // saved and umbraco info saved.  We don't want to update indexes etc when it is just membership info that is saved
                 MemberService.SetLastLogin(member.Username, member.LastLoginDate);
             }