Refactoring authentication validation

src/Orchard.Tests.Modules/Users/Services/MembershipServiceTests.cs

@@ -35,6 +35,7 @@ using Orchard.Services;
 namespace Orchard.Tests.Modules.Users.Services {
     [TestFixture]
     public class MembershipServiceTests {
+        private IMembershipValidationService _membershipValidationService;
         private IMembershipService _membershipService;
         private ISessionFactory _sessionFactory;
         private ISession _session;
@@ -73,6 +74,7 @@ namespace Orchard.Tests.Modules.Users.Services {
         public void Init() {
             var builder = new ContainerBuilder();
             //builder.RegisterModule(new ImplicitCollectionSupportModule());
+            builder.RegisterType<MembershipValidationService>().As<IMembershipValidationService>();
             builder.RegisterType<MembershipService>().As<IMembershipService>();
             builder.RegisterType<DefaultContentQuery>().As<IContentQuery>();
             builder.RegisterType<DefaultContentManager>().As<IContentManager>();
@@ -98,6 +100,7 @@ namespace Orchard.Tests.Modules.Users.Services {
             _session = _sessionFactory.OpenSession();
             builder.RegisterInstance(new TestSessionLocator(_session)).As<ISessionLocator>();
             _container = builder.Build();
+            _membershipValidationService = _container.Resolve<IMembershipValidationService>();
             _membershipService = _container.Resolve<IMembershipService>();
         }
 
@@ -159,7 +162,7 @@ namespace Orchard.Tests.Modules.Users.Services {
         public void UsersWhoHaveNeverLoggedInCanBeAuthenticated() {
             var user = (UserPart)_membershipService.CreateUser(new CreateUserParams("a", "b", "c", null, null, true));
             
-            Assert.That(_membershipService.CanAuthenticateWithCookie(user), Is.True);
+            Assert.That(_membershipValidationService.CanAuthenticateWithCookie(user), Is.True);
         }
 
         [Test]
@@ -169,7 +172,7 @@ namespace Orchard.Tests.Modules.Users.Services {
             user.LastLoginUtc = _clock.UtcNow;
             _clock.Advance(TimeSpan.FromMinutes(1));
 
-            Assert.That(_membershipService.CanAuthenticateWithCookie(user), Is.True);
+            Assert.That(_membershipValidationService.CanAuthenticateWithCookie(user), Is.True);
         }
 
         [Test]
@@ -181,7 +184,7 @@ namespace Orchard.Tests.Modules.Users.Services {
             user.LastLogoutUtc = _clock.UtcNow;
             _clock.Advance(TimeSpan.FromMinutes(1));
 
-            Assert.That(_membershipService.CanAuthenticateWithCookie(user), Is.False);
+            Assert.That(_membershipValidationService.CanAuthenticateWithCookie(user), Is.False);
         }
 
         [Test]
@@ -193,7 +196,7 @@ namespace Orchard.Tests.Modules.Users.Services {
             user.LastLoginUtc = _clock.UtcNow;
             _clock.Advance(TimeSpan.FromMinutes(1));
 
-            Assert.That(_membershipService.CanAuthenticateWithCookie(user), Is.True);
+            Assert.That(_membershipValidationService.CanAuthenticateWithCookie(user), Is.True);
         }
 
         [Test]
@@ -202,7 +205,7 @@ namespace Orchard.Tests.Modules.Users.Services {
 
             user.RegistrationStatus = UserStatus.Pending;
 
-            Assert.That(_membershipService.CanAuthenticateWithCookie(user), Is.False);
+            Assert.That(_membershipValidationService.CanAuthenticateWithCookie(user), Is.False);
         }
 
         [Test]
@@ -211,7 +214,7 @@ namespace Orchard.Tests.Modules.Users.Services {
 
             user.RegistrationStatus = UserStatus.Approved;
 
-            Assert.That(_membershipService.CanAuthenticateWithCookie(user), Is.True);
+            Assert.That(_membershipValidationService.CanAuthenticateWithCookie(user), Is.True);
         }
     }
 }

src/Orchard.Web/Modules/Orchard.Users/Orchard.Users.csproj

@@ -102,6 +102,7 @@
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="Services\AuthenticationRedirectionFilter.cs" />
     <Compile Include="Services\IUserService.cs" />
+    <Compile Include="Services\MembershipValidationService.cs" />
     <Compile Include="Services\UserResolverSelector.cs" />
     <Compile Include="Services\MembershipService.cs" />
     <Compile Include="AdminMenu.cs" />

src/Orchard.Web/Modules/Orchard.Users/Services/MembershipService.cs

@@ -289,29 +289,5 @@ namespace Orchard.Users.Services {
             return String.Equals(password, Encoding.UTF8.GetString(_encryptionService.Decode(Convert.FromBase64String(userPart.Password))), StringComparison.Ordinal);
         }
 
-        public bool CanAuthenticateWithCookie(IUser user) {
-            var userPart = user as UserPart;
-
-            if (userPart == null) {
-                return false;
-            }
-
-            // user has not been approved or is currently disabled
-            if(userPart.RegistrationStatus != UserStatus.Approved) {
-                return false;
-            }
-
-            // if the user has logged out, a cookie should not be accepted
-            if(userPart.LastLogoutUtc.HasValue) {
-
-                if (!userPart.LastLoginUtc.HasValue) {
-                    return true;
-                }
-
-                return userPart.LastLogoutUtc < userPart.LastLoginUtc;
-            }
-
-            return true;
-        }
     }
 }

src/Orchard.Web/Modules/Orchard.Users/Services/MembershipValidationService.cs

@@ -0,0 +1,32 @@
+using Orchard.Security;
+using Orchard.Users.Models;
+
+namespace Orchard.Users.Services {
+    public class MembershipValidationService : IMembershipValidationService {
+
+        public bool CanAuthenticateWithCookie(IUser user) {
+            var userPart = user as UserPart;
+
+            if (userPart == null) {
+                return false;
+            }
+
+            // user has not been approved or is currently disabled
+            if (userPart.RegistrationStatus != UserStatus.Approved) {
+                return false;
+            }
+
+            // if the user has logged out, a cookie should not be accepted
+            if (userPart.LastLogoutUtc.HasValue) {
+
+                if (!userPart.LastLoginUtc.HasValue) {
+                    return true;
+                }
+
+                return userPart.LastLogoutUtc < userPart.LastLoginUtc;
+            }
+
+            return true;
+        }
+    }
+}

src/Orchard/Orchard.Framework.csproj

@@ -149,8 +149,10 @@
     <Reference Include="System.Xml.Linq" />
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="Security\IMembershipValidationService.cs" />
     <Compile Include="Security\ISslSettingsProvider.cs" />
     <Compile Include="Security\Providers\DefaultSslSettingsProvider.cs" />
+    <Compile Include="Security\Providers\DefaultMembershipValidationService.cs" />
     <Compile Include="StaticHttpContextScope.cs" />
     <Compile Include="StaticHttpContextScopeFactory.cs" />
     <Compile Include="Caching\DefaultCacheContextAccessor.cs" />
@@ -1027,4 +1029,4 @@
   <Target Name="AfterBuild">
   </Target>
   -->
-</Project>
+</Project>

src/Orchard/Security/IMembershipService.cs

@@ -6,10 +6,5 @@
         IUser GetUser(string username);
         IUser ValidateUser(string userNameOrEmail, string password);
         void SetPassword(IUser user, string password);
-
-        /// <summary>
-        /// Returns <c>true</c> if the user is allowed to login from an auth cookie, <c>false</c> otherwise.
-        /// </summary>
-        bool CanAuthenticateWithCookie(IUser user);
     }
 }

src/Orchard/Security/IMembershipValidationService.cs

@@ -0,0 +1,8 @@
+namespace Orchard.Security {
+    public interface IMembershipValidationService : IDependency {
+        /// <summary>
+        /// Returns <c>true</c> if the user is allowed to login from an auth cookie, <c>false</c> otherwise.
+        /// </summary>
+        bool CanAuthenticateWithCookie(IUser user);
+    }
+}

src/Orchard/Security/Providers/DefaultMembershipValidationService.cs

@@ -0,0 +1,7 @@
+namespace Orchard.Security {
+    public class DefaultMembershipValidationService : IMembershipValidationService {
+        public bool CanAuthenticateWithCookie(IUser user) {
+            return true;
+        }
+    }
+}

src/Orchard/Security/Providers/FormsAuthenticationService.cs

@@ -15,6 +15,8 @@ namespace Orchard.Security.Providers {
         private readonly IContentManager _contentManager;
         private readonly IHttpContextAccessor _httpContextAccessor;
         private readonly ISslSettingsProvider _sslSettingsProvider;
+        private readonly IMembershipValidationService _membershipValidationService;
+
         private IUser _signedInUser;
         private bool _isAuthenticated;
 
@@ -23,12 +25,14 @@ namespace Orchard.Security.Providers {
             IClock clock, 
             IContentManager contentManager, 
             IHttpContextAccessor httpContextAccessor,
-            ISslSettingsProvider sslSettingsProvider) {
+            ISslSettingsProvider sslSettingsProvider,
+            IMembershipValidationService membershipValidationService) {
             _settings = settings;
             _clock = clock;
             _contentManager = contentManager;
             _httpContextAccessor = httpContextAccessor;
             _sslSettingsProvider = sslSettingsProvider;
+            _membershipValidationService = membershipValidationService;
 
             Logger = NullLogger.Instance;
             
@@ -137,8 +141,15 @@ namespace Orchard.Security.Providers {
                 return null;
             }
 
+            // todo: this issues a sql query for each authenticated request
+            _signedInUser = _contentManager.Get(userId).As<IUser>();
+
+            if (_signedInUser == null || !_membershipValidationService.CanAuthenticateWithCookie(_signedInUser)) {
+                return null;
+            }
+
             _isAuthenticated = true;
-            return _signedInUser = _contentManager.Get(userId).As<IUser>();
+            return _signedInUser;
         }
 
         private string GetCookiePath(HttpContextBase httpContext) {