Fix/user login does not check active flags

test/Classes/UserHelperGetByLoginIdTests.cs

@@ -0,0 +1,162 @@
+using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.Caching.Memory;
+using Microsoft.Extensions.Internal;
+using Viper.Classes.SQLContext;
+using Viper.Models.AAUD;
+using Viper.test.Students;
+
+namespace Viper.test.Classes
+{
+    [Collection(HttpHelperCacheCollection.Name)]
+    public sealed class UserHelperGetByLoginIdTests : IDisposable
+    {
+        private const string LoginId = "jdoe";
+
+        private readonly FakeClock _clock = new();
+        private readonly MemoryCache _memoryCache;
+        private readonly TestableAAUDContext _context;
+
+        public UserHelperGetByLoginIdTests()
+        {
+            _memoryCache = new MemoryCache(new MemoryCacheOptions { Clock = _clock });
+            HttpHelper.Configure(_memoryCache, null!, null!, null!, null!, null!, null!);
+
+            var options = new DbContextOptionsBuilder<AAUDContext>()
+                .UseInMemoryDatabase(databaseName: Guid.NewGuid().ToString())
+                .Options;
+            _context = new TestableAAUDContext(options);
+        }
+
+        // Clear, not dispose: HttpHelper.Cache is a process-wide static, so a disposed instance would
+        // make every later test throw.
+        public void Dispose()
+        {
+            _memoryCache.Clear();
+            _context.Dispose();
+        }
+
+        private const int ActiveUserId = 1;
+        private const string ActiveMothraId = "00012345";
+
+        private void AddUser(int current = 1, int future = 0, int aaudUserId = ActiveUserId, string mothraId = ActiveMothraId)
+        {
+            _context.AaudUsers.Add(new AaudUser
+            {
+                AaudUserId = aaudUserId,
+                ClientId = "C" + aaudUserId,
+                MothraId = mothraId,
+                LoginId = LoginId,
+                FirstName = "Jane",
+                LastName = "Doe",
+                DisplayFirstName = "Jane",
+                DisplayLastName = "Doe",
+                DisplayFullName = "Jane Doe",
+                Current = current,
+                Future = future,
+            });
+            _context.SaveChanges();
+            _context.ChangeTracker.Clear();
+        }
+
+        private void RemoveUsers()
+        {
+            _context.AaudUsers.RemoveRange(_context.AaudUsers);
+            _context.SaveChanges();
+        }
+
+        [Theory]
+        [InlineData(1, 0)]
+        [InlineData(0, 1)]
+        public void GetByLoginId_ReturnsCurrentOrFutureUser(int current, int future)
+        {
+            AddUser(current, future);
+
+            var result = new UserHelper().GetByLoginId(_context, LoginId);
+
+            Assert.NotNull(result);
+            Assert.Equal(ActiveUserId, result.AaudUserId);
+        }
+
+        [Fact]
+        public void GetByLoginId_ReturnsNullForInactiveUser()
+        {
+            AddUser(current: 0, future: 0);
+
+            Assert.Null(new UserHelper().GetByLoginId(_context, LoginId));
+        }
+
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void GetByLoginId_PrefersActiveUserOverFormerUserWithSameLoginId(bool formerInsertedFirst)
+        {
+            // Both orders, so the active-flag filter is proven rather than the row order.
+            if (formerInsertedFirst)
+            {
+                AddUser(current: 0, future: 0, aaudUserId: 2, mothraId: "00099999");
+                AddUser();
+            }
+            else
+            {
+                AddUser();
+                AddUser(current: 0, future: 0, aaudUserId: 2, mothraId: "00099999");
+            }
+
+            var result = new UserHelper().GetByLoginId(_context, LoginId);
+
+            Assert.NotNull(result);
+            Assert.Equal(ActiveUserId, result.AaudUserId);
+            Assert.Equal(ActiveMothraId, result.MothraId);
+        }
+
+        [Fact]
+        public void GetByLoginId_ServesFoundUserFromCache()
+        {
+            AddUser();
+            var helper = new UserHelper();
+            helper.GetByLoginId(_context, LoginId);
+
+            RemoveUsers();
+
+            Assert.Equal(ActiveUserId, helper.GetByLoginId(_context, LoginId)?.AaudUserId);
+        }
+
+        [Fact]
+        public void GetByLoginId_DoesNotCacheMiss()
+        {
+            var helper = new UserHelper();
+            Assert.Null(helper.GetByLoginId(_context, LoginId));
+
+            // A user who becomes active after a failed lookup must not be locked out by a cached null.
+            AddUser();
+
+            Assert.Equal(ActiveUserId, helper.GetByLoginId(_context, LoginId)?.AaudUserId);
+        }
+
+        [Fact]
+        public void GetByLoginId_CachedUserExpires()
+        {
+            AddUser();
+            var helper = new UserHelper();
+            helper.GetByLoginId(_context, LoginId);
+
+            // A deactivated user must drop out once the cache entry expires.
+            RemoveUsers();
+            AddUser(current: 0, future: 0);
+            _clock.UtcNow = _clock.UtcNow.AddMinutes(11);
+
+            Assert.Null(helper.GetByLoginId(_context, LoginId));
+        }
+
+        [Fact]
+        public void GetByLoginId_ReturnsNullForNullLoginId()
+        {
+            Assert.Null(new UserHelper().GetByLoginId(_context, null));
+        }
+
+        private sealed class FakeClock : ISystemClock
+        {
+            public DateTimeOffset UtcNow { get; set; } = DateTimeOffset.UtcNow;
+        }
+    }
+}

web/Classes/UserHelper.cs

@@ -262,24 +262,24 @@ public bool HasPermission(RAPSContext? rapsContext, AaudUser? user, string permi
             {
                 try
                 {
-                    string userLoginId = loginId.ToLower();
-                    AaudUser? user = null;
+                    string cacheKey = "AaudUser-" + loginId.ToLower();
 
                     // if we have already cached this user as an UCD user then create a Viper user from that object
-                    if (HttpHelper.Cache != null)
+                    if (HttpHelper.Cache?.TryGetValue(cacheKey, out AaudUser? cachedUser) == true && cachedUser != null)
                     {
-                        user = HttpHelper.Cache.Get<AaudUser>("AaudUser-" + userLoginId);
-                    }
-
-                    if (user != null)
-                    {
-                        return user;
+                        return cachedUser;
                     }
 
                     if (HttpHelper.Cache != null && aaudContext != null)
                     {
-                        user = HttpHelper.Cache.GetOrCreate("AaudUser-" + userLoginId,
-                            _ => aaudContext.AaudUsers.FirstOrDefault(m => m.LoginId == loginId));
+                        AaudUser? user = aaudContext.AaudUsers.AsNoTracking()
+                            .FirstOrDefault(m => m.LoginId == loginId && (m.Current == 1 || m.Future == 1));
+
+                        // Only cache found users, and expire them so changes to active flags are picked up
+                        if (user != null)
+                        {
+                            HttpHelper.Cache.Set(cacheKey, user, TimeSpan.FromMinutes(10));
+                        }
 
                         return user;
                     }