diff --git a/test/Classes/DuoAuthenticationRequirementTests.cs b/test/Classes/DuoAuthenticationRequirementTests.cs new file mode 100644 index 000000000..89caf518f --- /dev/null +++ b/test/Classes/DuoAuthenticationRequirementTests.cs @@ -0,0 +1,75 @@ +using System.Security.Claims; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using NSubstitute; +using Web.Authorization; + +namespace Viper.test.Classes +{ + /// + /// Pins the deliberate Development bypass, since no Duo credential can be issued for a localhost + /// callback, and pins that the failure message is set only when the requirement fails. + /// + public class DuoAuthenticationRequirementTests + { + private const string ErrorKey = "ErrorMessage"; + + private static HttpContext ContextFor(string environmentName) + { + var env = Substitute.For(); + env.EnvironmentName = environmentName; + + var services = new ServiceCollection(); + services.AddSingleton(env); + + return new DefaultHttpContext { RequestServices = services.BuildServiceProvider() }; + } + + private static ClaimsPrincipal UserWithDuo() => + new(new ClaimsIdentity(new[] { new Claim("credentialType", "DuoCredential") }, "test")); + + private static async Task<(bool Succeeded, object? Error)> EvaluateAsync(ClaimsPrincipal user, string environmentName) + { + var httpContext = ContextFor(environmentName); + var requirement = new DuoAuthenticationRequirement(); + var context = new AuthorizationHandlerContext(new[] { requirement }, user, httpContext); + + await requirement.HandleAsync(context); + + httpContext.Items.TryGetValue(ErrorKey, out object? error); + return (context.HasSucceeded, error); + } + + [Fact] + public async Task DuoCredential_Succeeds() + { + var (succeeded, error) = await EvaluateAsync(UserWithDuo(), "Production"); + + Assert.True(succeeded); + Assert.Null(error); + } + + [Fact] + public async Task Development_SucceedsWithoutDuo() + { + var (succeeded, error) = await EvaluateAsync(new ClaimsPrincipal(new ClaimsIdentity()), "Development"); + + Assert.True(succeeded); + // The bypass is not a failure, so it must not leave a failure message behind. + Assert.Null(error); + } + + [Theory] + [InlineData("Production")] + [InlineData("Test")] + public async Task OutsideDevelopment_FailsWithoutDuoAndExplainsWhy(string environmentName) + { + var (succeeded, error) = await EvaluateAsync(new ClaimsPrincipal(new ClaimsIdentity()), environmentName); + + Assert.False(succeeded); + Assert.Equal("DUO two-factor authentication is required", error); + } + } +} diff --git a/test/Classes/UserHelperCacheTests.cs b/test/Classes/UserHelperCacheTests.cs new file mode 100644 index 000000000..94e034f2b --- /dev/null +++ b/test/Classes/UserHelperCacheTests.cs @@ -0,0 +1,96 @@ +using Microsoft.Extensions.Caching.Memory; +using Viper.Models.AAUD; + +namespace Viper.test.Classes +{ + [Collection(HttpHelperCacheCollection.Name)] + public sealed class UserHelperCacheTests : IDisposable + { + private MemoryCache? _installed; + + // Empty, not dispose: HttpHelper.Cache is a process-wide static, so a disposed instance would + // make every later test throw. + public void Dispose() + { + _installed?.Clear(); + } + + private const string MothraId = "00012345"; + + // Spelled out rather than shared with UserHelper so a rename there has to be deliberate. + private static readonly string[] CacheKeys = + { + "Roles-" + MothraId, + "PermissionsAssigned-" + MothraId + "-True", + "PermissionsAssigned-" + MothraId + "-False", + "PermissionsInherited-" + MothraId + "-True", + "PermissionsInherited-" + MothraId + "-False", + }; + + private IMemoryCache ConfigureCache() + { + var memoryCache = new MemoryCache(new MemoryCacheOptions()); + HttpHelper.Configure(memoryCache, null!, null!, null!, null!, null!); + _installed = memoryCache; + return memoryCache; + } + + [Fact] + public void ClearCachedRolesAndPermissions_RemovesEveryRoleAndPermissionKey() + { + var memoryCache = ConfigureCache(); + foreach (string key in CacheKeys) + { + memoryCache.Set(key, "cached"); + } + + new UserHelper().ClearCachedRolesAndPermissions(new AaudUser { MothraId = MothraId }); + + foreach (string key in CacheKeys) + { + Assert.False(memoryCache.TryGetValue(key, out _), $"{key} was left in the cache"); + } + } + + [Fact] + public void ClearCachedRolesAndPermissions_LeavesOtherUsersEntriesAlone() + { + var memoryCache = ConfigureCache(); + memoryCache.Set("Roles-99999999", "cached"); + + new UserHelper().ClearCachedRolesAndPermissions(new AaudUser { MothraId = MothraId }); + + Assert.True(memoryCache.TryGetValue("Roles-99999999", out _)); + } + + [Fact] + public void ClearCachedRolesAndPermissions_ByMothraId_NeedsNoAaudUser() + { + var memoryCache = ConfigureCache(); + foreach (string key in CacheKeys) + { + memoryCache.Set(key, "cached"); + } + + // The interceptor only has the RAPS-side id, so this overload is the one it calls. + UserHelper.ClearCachedRolesAndPermissions(MothraId); + + foreach (string key in CacheKeys) + { + Assert.False(memoryCache.TryGetValue(key, out _), $"{key} was left in the cache"); + } + } + + [Fact] + public void ClearCachedRolesAndPermissions_IgnoresAnEmptyMothraId() + { + var memoryCache = ConfigureCache(); + memoryCache.Set("Roles-", "cached"); + + UserHelper.ClearCachedRolesAndPermissions(string.Empty); + + // Guards the old LoginId behaviour, where users without one shared the "Roles-" key. + Assert.True(memoryCache.TryGetValue("Roles-", out _)); + } + } +} diff --git a/test/ClinicalScheduler/CliniciansControllerTest.cs b/test/ClinicalScheduler/CliniciansControllerTest.cs index f4730e9ce..9ef8fc2e0 100644 --- a/test/ClinicalScheduler/CliniciansControllerTest.cs +++ b/test/ClinicalScheduler/CliniciansControllerTest.cs @@ -12,6 +12,7 @@ namespace Viper.test.ClinicalScheduler { + [Collection(HttpHelperCacheCollection.Name)] public class CliniciansControllerTest : ClinicalSchedulerTestBase { private readonly AAUDContext _aaudContext; diff --git a/test/ClinicalScheduler/EmailNotificationTest.cs b/test/ClinicalScheduler/EmailNotificationTest.cs index 7dd97c420..0df55e64c 100644 --- a/test/ClinicalScheduler/EmailNotificationTest.cs +++ b/test/ClinicalScheduler/EmailNotificationTest.cs @@ -17,6 +17,7 @@ namespace Viper.test.ClinicalScheduler /// Tests for email notification functionality when primary evaluators are changed. /// Covers email sending, content validation, and error handling scenarios. /// + [Collection(HttpHelperCacheCollection.Name)] public class EmailNotificationTest : IDisposable { private readonly IScheduleAuditService _mockAuditService; diff --git a/test/ClinicalScheduler/Integration/ControllerServiceIntegrationTest.cs b/test/ClinicalScheduler/Integration/ControllerServiceIntegrationTest.cs index 9bb83e89c..feb65371a 100644 --- a/test/ClinicalScheduler/Integration/ControllerServiceIntegrationTest.cs +++ b/test/ClinicalScheduler/Integration/ControllerServiceIntegrationTest.cs @@ -24,6 +24,7 @@ namespace Viper.test.ClinicalScheduler.Integration /// because the PermissionAttribute creates its own UserHelper instance, preventing proper mocking. /// These tests are kept for documentation purposes and can be converted to unit tests. /// + [Collection(HttpHelperCacheCollection.Name)] public class ControllerServiceIntegrationTest : IntegrationTestBase { private readonly IPersonService _personService; diff --git a/test/ClinicalScheduler/Integration/PermissionServiceIntegrationTest.cs b/test/ClinicalScheduler/Integration/PermissionServiceIntegrationTest.cs index 8cd7365b0..bdf32143f 100644 --- a/test/ClinicalScheduler/Integration/PermissionServiceIntegrationTest.cs +++ b/test/ClinicalScheduler/Integration/PermissionServiceIntegrationTest.cs @@ -10,6 +10,7 @@ namespace Viper.test.ClinicalScheduler.Integration /// Tests the complete flow of permission checks after consolidating /// ClinicalScheduleSecurityService into SchedulePermissionService. /// + [Collection(HttpHelperCacheCollection.Name)] public class PermissionServiceIntegrationTest : IntegrationTestBase { private readonly ILogger _mockLogger; diff --git a/test/ClinicalScheduler/Integration/ServiceLayerIntegrationTest.cs b/test/ClinicalScheduler/Integration/ServiceLayerIntegrationTest.cs index 6c92581e7..f3bb6260e 100644 --- a/test/ClinicalScheduler/Integration/ServiceLayerIntegrationTest.cs +++ b/test/ClinicalScheduler/Integration/ServiceLayerIntegrationTest.cs @@ -16,6 +16,7 @@ namespace Viper.test.ClinicalScheduler.Integration /// Tests the new service architecture with StudentScheduleService, InstructorScheduleService, /// and how ClinicalScheduleService delegates to them. /// + [Collection(HttpHelperCacheCollection.Name)] public class ServiceLayerIntegrationTest : IntegrationTestBase { private static readonly DateTime ScheduleStart = new(2024, 1, 1, 0, 0, 0, DateTimeKind.Local); diff --git a/test/ClinicalScheduler/PermissionsControllerTest.cs b/test/ClinicalScheduler/PermissionsControllerTest.cs index 7280cbff3..34ae6f87b 100644 --- a/test/ClinicalScheduler/PermissionsControllerTest.cs +++ b/test/ClinicalScheduler/PermissionsControllerTest.cs @@ -8,6 +8,7 @@ namespace Viper.test.ClinicalScheduler { + [Collection(HttpHelperCacheCollection.Name)] public class PermissionsControllerTest : ClinicalSchedulerTestBase { private readonly ISchedulePermissionService _mockPermissionService; diff --git a/test/ClinicalScheduler/RotationsControllerTest.cs b/test/ClinicalScheduler/RotationsControllerTest.cs index d9a1b8090..91afaf46d 100644 --- a/test/ClinicalScheduler/RotationsControllerTest.cs +++ b/test/ClinicalScheduler/RotationsControllerTest.cs @@ -9,6 +9,7 @@ namespace Viper.test.ClinicalScheduler { + [Collection(HttpHelperCacheCollection.Name)] public class RotationsControllerTest : ClinicalSchedulerTestBase { private readonly ILogger _mockLogger; diff --git a/test/ClinicalScheduler/ScheduleEditServiceRollbackTest.cs b/test/ClinicalScheduler/ScheduleEditServiceRollbackTest.cs index 7d08884f6..94543d878 100644 --- a/test/ClinicalScheduler/ScheduleEditServiceRollbackTest.cs +++ b/test/ClinicalScheduler/ScheduleEditServiceRollbackTest.cs @@ -18,6 +18,7 @@ namespace Viper.test.ClinicalScheduler /// prove a failed audit write rolls the schedule change back, not just that the /// exception surfaces. /// + [Collection(HttpHelperCacheCollection.Name)] public class ScheduleEditServiceRollbackTest : IDisposable { private readonly SqliteConnection _connection; diff --git a/test/ClinicalScheduler/ScheduleEditServiceTest.cs b/test/ClinicalScheduler/ScheduleEditServiceTest.cs index a05887886..4cf982080 100644 --- a/test/ClinicalScheduler/ScheduleEditServiceTest.cs +++ b/test/ClinicalScheduler/ScheduleEditServiceTest.cs @@ -13,6 +13,7 @@ namespace Viper.test.ClinicalScheduler { + [Collection(HttpHelperCacheCollection.Name)] public class ScheduleEditServiceTest : IDisposable { private readonly ISchedulePermissionService _mockPermissionService; diff --git a/test/ClinicalScheduler/SchedulePermissionServiceTest.cs b/test/ClinicalScheduler/SchedulePermissionServiceTest.cs index 9f0cc4616..cc7672c65 100644 --- a/test/ClinicalScheduler/SchedulePermissionServiceTest.cs +++ b/test/ClinicalScheduler/SchedulePermissionServiceTest.cs @@ -4,6 +4,7 @@ namespace Viper.test.ClinicalScheduler { + [Collection(HttpHelperCacheCollection.Name)] public class SchedulePermissionServiceTest : ClinicalSchedulerTestBase { private readonly ILogger _mockLogger; diff --git a/test/Effort/EffortTypesControllerIntegrationTests.cs b/test/Effort/EffortTypesControllerIntegrationTests.cs index 7cd3d28bf..7d2866cf3 100644 --- a/test/Effort/EffortTypesControllerIntegrationTests.cs +++ b/test/Effort/EffortTypesControllerIntegrationTests.cs @@ -13,6 +13,7 @@ namespace Viper.test.Effort; /// Integration tests for EffortTypesController. /// Tests the full stack: Controller -> Service -> DbContext. /// +[Collection(HttpHelperCacheCollection.Name)] public class EffortTypesControllerIntegrationTests : EffortIntegrationTestBase { private readonly EffortTypesController _controller; diff --git a/test/Effort/Integration/EffortPermissionIntegrationTests.cs b/test/Effort/Integration/EffortPermissionIntegrationTests.cs index b1bde6d61..1bb00070e 100644 --- a/test/Effort/Integration/EffortPermissionIntegrationTests.cs +++ b/test/Effort/Integration/EffortPermissionIntegrationTests.cs @@ -8,6 +8,7 @@ namespace Viper.test.Effort.Integration; /// Tests the complete flow of permission checks for department-level, /// full access, and self-service permission models. /// +[Collection(HttpHelperCacheCollection.Name)] public class EffortPermissionIntegrationTests : EffortIntegrationTestBase { private readonly EffortPermissionService _permissionService; diff --git a/test/Effort/PercentAssignTypesControllerIntegrationTests.cs b/test/Effort/PercentAssignTypesControllerIntegrationTests.cs index 43f291048..07b95d868 100644 --- a/test/Effort/PercentAssignTypesControllerIntegrationTests.cs +++ b/test/Effort/PercentAssignTypesControllerIntegrationTests.cs @@ -12,6 +12,7 @@ namespace Viper.test.Effort; /// Integration tests for PercentAssignTypesController. /// Tests the full stack: Controller -> Service -> DbContext. /// +[Collection(HttpHelperCacheCollection.Name)] public class PercentAssignTypesControllerIntegrationTests : EffortIntegrationTestBase { private readonly PercentAssignTypesController _controller; diff --git a/test/HttpHelperCacheCollection.cs b/test/HttpHelperCacheCollection.cs new file mode 100644 index 000000000..909d5f305 --- /dev/null +++ b/test/HttpHelperCacheCollection.cs @@ -0,0 +1,11 @@ +namespace Viper.test; + +/// +/// HttpHelper.Configure swaps a process-wide static cache, and xUnit runs test classes in parallel by +/// default. Every class that configures the cache must join this collection. +/// +[CollectionDefinition(HttpHelperCacheCollection.Name)] +public static class HttpHelperCacheCollection +{ + public const string Name = "HttpHelperCache"; +} diff --git a/test/RAPS/RapsCacheInvalidationInterceptorTests.cs b/test/RAPS/RapsCacheInvalidationInterceptorTests.cs new file mode 100644 index 000000000..a440b5ddf --- /dev/null +++ b/test/RAPS/RapsCacheInvalidationInterceptorTests.cs @@ -0,0 +1,177 @@ +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Caching.Memory; +using Viper.Areas.RAPS.Services; +using Viper.Classes.SQLContext; +using Viper.Models.RAPS; + +namespace Viper.test.RAPS +{ + /// + /// Pins the interceptor that replaced the per-call-site invalidation, which several write paths + /// (the nightly role refresh, the OU group sync) never called. + /// + [Collection(HttpHelperCacheCollection.Name)] + public class RapsCacheInvalidationInterceptorTests : IAsyncLifetime + { + private const string MothraId = "00012345"; + + private static readonly string[] CacheKeys = + { + "Roles-" + MothraId, + "PermissionsAssigned-" + MothraId + "-True", + "PermissionsAssigned-" + MothraId + "-False", + "PermissionsInherited-" + MothraId + "-True", + "PermissionsInherited-" + MothraId + "-False", + }; + + private SqliteConnection _connection = null!; + private RAPSContext _context = null!; + + public async ValueTask InitializeAsync() + { + _connection = new SqliteConnection("Filename=:memory:"); + await _connection.OpenAsync(TestContext.Current.CancellationToken); + _context = await NewContextAsync(_connection); + } + + public async ValueTask DisposeAsync() + { + await _context.DisposeAsync(); + await _connection.DisposeAsync(); + } + + private static async Task NewContextAsync(SqliteConnection connection) + { + var options = new DbContextOptionsBuilder() + .UseSqlite(connection) + .AddInterceptors(new RapsCacheInvalidationInterceptor()) + .Options; + var context = new RAPSContext(options); + await context.Database.EnsureCreatedAsync(TestContext.Current.CancellationToken); + + // Parent rows for the foreign keys the writes below depend on. + context.TblRoles.AddRange( + new TblRole { RoleId = 1, Role = "VIPER.Test", Application = 0, UpdateFreq = 0, AllowAllUsers = false }, + new TblRole { RoleId = 7, Role = "VIPER.Shared", Application = 0, UpdateFreq = 0, AllowAllUsers = false }); + context.TblPermissions.Add(new TblPermission { PermissionId = 1, Permission = "SVMSecure.Test" }); + // TblRoleMember.MemberId and TblMemberPermission.MemberId both FK to VwAaudUser. + context.VwAaudUser.AddRange( + new VwAaudUser { MothraId = MothraId, DisplayFirstName = "Test", DisplayLastName = "User", DisplayFullName = "User, Test" }, + new VwAaudUser { MothraId = "99999999", DisplayFirstName = "Other", DisplayLastName = "User", DisplayFullName = "User, Other" }); + await context.SaveChangesAsync(TestContext.Current.CancellationToken); + return context; + } + + private static IMemoryCache SeedCache() + { + var memoryCache = new MemoryCache(new MemoryCacheOptions()); + HttpHelper.Configure(memoryCache, null!, null!, null!, null!, null!); + foreach (string key in CacheKeys) + { + memoryCache.Set(key, "cached"); + } + return memoryCache; + } + + private static void AssertCleared(IMemoryCache cache) + { + foreach (string key in CacheKeys) + { + Assert.False(cache.TryGetValue(key, out _), $"{key} was left in the cache"); + } + } + + [Fact] + public async Task AddingARoleMember_EvictsThatMember() + { + var cache = SeedCache(); + + _context.TblRoleMembers.Add(new TblRoleMember { RoleId = 1, MemberId = MothraId }); + await _context.SaveChangesAsync(TestContext.Current.CancellationToken); + + AssertCleared(cache); + } + + [Fact] + public async Task RemovingARoleMember_EvictsThatMember() + { + var member = new TblRoleMember { RoleId = 1, MemberId = MothraId }; + _context.TblRoleMembers.Add(member); + await _context.SaveChangesAsync(TestContext.Current.CancellationToken); + + // Revocation is the direction that matters: a stale allow keeps access alive. + var cache = SeedCache(); + _context.TblRoleMembers.Remove(member); + await _context.SaveChangesAsync(TestContext.Current.CancellationToken); + + AssertCleared(cache); + } + + [Fact] + public async Task ChangingAnIndividualPermission_EvictsThatMember() + { + var cache = SeedCache(); + + _context.TblMemberPermissions.Add(new TblMemberPermission { PermissionId = 1, MemberId = MothraId, Access = 1 }); + await _context.SaveChangesAsync(TestContext.Current.CancellationToken); + + AssertCleared(cache); + } + + [Fact] + public async Task ChangingARolesPermissions_EvictsEveryMemberOfThatRole() + { + _context.TblRoleMembers.Add(new TblRoleMember { RoleId = 7, MemberId = MothraId }); + await _context.SaveChangesAsync(TestContext.Current.CancellationToken); + + // The write names the role, not the people, so the interceptor has to expand it. + var cache = SeedCache(); + _context.TblRolePermissions.Add(new TblRolePermission { RoleId = 7, PermissionId = 1, Access = 1 }); + await _context.SaveChangesAsync(TestContext.Current.CancellationToken); + + AssertCleared(cache); + } + + [Fact] + public async Task ChangingTheRoleItself_EvictsEveryMemberOfThatRole() + { + _context.TblRoleMembers.Add(new TblRoleMember { RoleId = 7, MemberId = MothraId }); + await _context.SaveChangesAsync(TestContext.Current.CancellationToken); + + // Renaming a role repoints the permissions its members inherit, so it has to expand too. + var cache = SeedCache(); + TblRole role = await _context.TblRoles.SingleAsync(r => r.RoleId == 7, TestContext.Current.CancellationToken); + role.Description = "Renamed"; + await _context.SaveChangesAsync(TestContext.Current.CancellationToken); + + AssertCleared(cache); + } + + [Fact] + public void SynchronousSave_EvictsToo() + { + var cache = SeedCache(); + + // Not every RAPS write path is async, and the sync overrides stash and evict separately. + _context.TblRoleMembers.Add(new TblRoleMember { RoleId = 1, MemberId = MothraId }); + _context.SaveChanges(); + + AssertCleared(cache); + } + + [Fact] + public async Task AnUnrelatedWrite_LeavesTheCacheAlone() + { + var cache = SeedCache(); + + _context.TblRoleMembers.Add(new TblRoleMember { RoleId = 1, MemberId = "99999999" }); + await _context.SaveChangesAsync(TestContext.Current.CancellationToken); + + foreach (string key in CacheKeys) + { + Assert.True(cache.TryGetValue(key, out _), $"{key} should not have been evicted"); + } + } + } +} diff --git a/test/RAPS/RapsControllerAuthorizationTests.cs b/test/RAPS/RapsControllerAuthorizationTests.cs new file mode 100644 index 000000000..5fc92de44 --- /dev/null +++ b/test/RAPS/RapsControllerAuthorizationTests.cs @@ -0,0 +1,48 @@ +using System.Reflection; +using Microsoft.AspNetCore.Authorization; +using Viper.Areas.RAPS.Controllers; + +namespace Viper.test.RAPS +{ + /// + /// Every controller in the area has to sit behind both the RAPS roles and Duo. A gap is invisible in + /// Development, where Duo auto-succeeds, and shows up on Test/Prod as a page whose API 403s. + /// + public class RapsControllerAuthorizationTests + { + public static TheoryData RapsControllers() + { + return new TheoryData(typeof(RAPSController).Assembly.GetTypes() + .Where(t => t.Namespace == typeof(RAPSController).Namespace && t.Name.EndsWith("Controller")) + .OrderBy(t => t.Name)); + } + + [Theory] + [MemberData(nameof(RapsControllers))] + public void EveryRapsController_RequiresDuoTwoFactor(Type controller) + { + var authorize = controller.GetCustomAttributes(inherit: true) + .FirstOrDefault(a => a.Policy == "2faAuthentication"); + + Assert.True(authorize is not null, $"{controller.Name} is missing [Authorize(Policy = \"2faAuthentication\")]"); + } + + [Theory] + [MemberData(nameof(RapsControllers))] + public void EveryRapsController_RequiresARapsRole(Type controller) + { + var roles = controller.GetCustomAttributes(inherit: true) + .Select(a => a.Roles) + .FirstOrDefault(r => !string.IsNullOrEmpty(r)); + + Assert.False(string.IsNullOrEmpty(roles), $"{controller.Name} is missing an [Authorize(Roles = ...)] restriction"); + } + + [Fact] + public void RapsControllersAreDiscovered() + { + // Guards the two theories above against silently passing on an empty set. + Assert.NotEmpty(RapsControllers()); + } + } +} diff --git a/test/RAPS/RapsSecurityServiceTests.cs b/test/RAPS/RapsSecurityServiceTests.cs new file mode 100644 index 000000000..0aec1b4a4 --- /dev/null +++ b/test/RAPS/RapsSecurityServiceTests.cs @@ -0,0 +1,100 @@ +using MockQueryable.NSubstitute; +using NSubstitute; +using Viper.Areas.RAPS.Services; +using Viper.Classes.SQLContext; +using Viper.Models.AAUD; +using Viper.Models.RAPS; + +namespace Viper.test.RAPS +{ + /// + /// Covers CanViewRoleList, which gates both the Role List nav item and the RoleList action. The nav + /// item used to be unconditional, so a user without access saw a link that landed on a 403. + /// + public class RapsSecurityServiceTests + { + private const string MemberId = "10000001"; + + private static RAPSContext ContextWithRoles(List roles) + { + // BuildMockDbSet() makes its own NSubstitute calls, so build it before opening the Returns() call + var mockSet = roles.BuildMockDbSet(); + var context = Substitute.For(); + context.TblRoles.Returns(mockSet); + return context; + } + + private static IUserHelper UserWith(params string[] permissions) + { + var userHelper = Substitute.For(); + userHelper.GetCurrentUser().Returns(new AaudUser { MothraId = MemberId }); + userHelper.HasPermission(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(call => permissions.Contains(call.ArgAt(2))); + return userHelper; + } + + /// + /// A delegate role (Application = 1) the user belongs to, which puts the named role under their + /// control. The controlled role's name is what decides the instance it counts for. + /// + private static List RolesWithDelegatedRole(string controlledRole) + { + var controlled = new TblRole { RoleId = 2, Role = controlledRole }; + var delegateRole = new TblRole + { + RoleId = 1, + Role = "VIPER.DelegateRole", + Application = 1, + TblRoleMembers = { new TblRoleMember { RoleId = 1, MemberId = MemberId } }, + ChildRoles = { new TblAppRole { AppRoleId = 1, RoleId = 2, Role = controlled } } + }; + return new List { delegateRole, controlled }; + } + + [Fact] + public void AdminCanViewRoleList() + { + var service = new RAPSSecurityService(ContextWithRoles(new List()), UserWith("RAPS.Admin")); + + Assert.True(service.CanViewRoleList("VIPER")); + } + + [Theory] + [InlineData("VMACS.VMTH", true)] + [InlineData("VIPER", false)] + public void HelpDeskCanViewRoleList_OnlyInVMACSInstances(string instance, bool expected) + { + var service = new RAPSSecurityService(ContextWithRoles(new List()), UserWith("RAPS.ViewRoles")); + + Assert.Equal(expected, service.CanViewRoleList(instance)); + } + + [Theory] + [InlineData("VMACS.VMTH", true)] + [InlineData("VIPER", false)] + public void DelegateCanViewRoleList_OnlyInTheInstanceHoldingTheControlledRole(string instance, bool expected) + { + var service = new RAPSSecurityService(ContextWithRoles(RolesWithDelegatedRole("VMACS.VMTH.Controlled")), UserWith()); + + // The role list filters to the instance, so a delegate whose controlled role is in + // VMACS must not be offered the VIPER list that would come back empty. + Assert.Equal(expected, service.CanViewRoleList(instance)); + } + + [Fact] + public void DelegateCanViewRoleList_WithoutAnyRapsPermission() + { + var service = new RAPSSecurityService(ContextWithRoles(RolesWithDelegatedRole("VIPER.Controlled")), UserWith()); + + Assert.True(service.CanViewRoleList("VIPER")); + } + + [Fact] + public void CannotViewRoleList_WithoutPermissionsOrDelegatedRoles() + { + var service = new RAPSSecurityService(ContextWithRoles(new List()), UserWith()); + + Assert.False(service.CanViewRoleList("VMACS.VMTH")); + } + } +} diff --git a/test/RAPS/RoleMembersControllerTests.cs b/test/RAPS/RoleMembersControllerTests.cs index 5ee37333a..84d3050f4 100644 --- a/test/RAPS/RoleMembersControllerTests.cs +++ b/test/RAPS/RoleMembersControllerTests.cs @@ -14,7 +14,6 @@ public class RoleMembersControllerTests : IAsyncLifetime { private SqliteConnection _connection = null!; private RAPSContext _context = null!; - private AAUDContext _aaudContext = null!; public async ValueTask InitializeAsync() { @@ -24,19 +23,15 @@ public async ValueTask InitializeAsync() .UseSqlite(_connection) .Options); await _context.Database.EnsureCreatedAsync(TestContext.Current.CancellationToken); - _aaudContext = new AAUDContext(new DbContextOptionsBuilder() - .UseSqlite(_connection) - .Options); } public async ValueTask DisposeAsync() { await _context.DisposeAsync(); - await _aaudContext.DisposeAsync(); await _connection.DisposeAsync(); } - private RoleMembersController CreateController() => new(_context, _aaudContext); + private RoleMembersController CreateController() => new(_context); [Fact] public async Task PushRolesToVMACS_EmptyRoleIds_ReturnsBadRequest() diff --git a/test/RAPS/RoleTemplateCrudTests.cs b/test/RAPS/RoleTemplateCrudTests.cs index 31cc8760b..85421cc33 100644 --- a/test/RAPS/RoleTemplateCrudTests.cs +++ b/test/RAPS/RoleTemplateCrudTests.cs @@ -91,12 +91,9 @@ private static async Task CreateContextAsync(SqliteConnection conne return context; } - // The AAUD context only reaches the cache service, which neither create nor update touches. private static RoleTemplatesController CreateController(RAPSContext context) { - var aaudContext = new AAUDContext(new DbContextOptionsBuilder() - .UseInMemoryDatabase("AAUD_" + Guid.NewGuid()).Options); - return new RoleTemplatesController(context, aaudContext); + return new RoleTemplatesController(context); } private static async Task SeedTemplateAsync(RAPSContext context) diff --git a/test/RAPS/RoleTemplatesControllerTests.cs b/test/RAPS/RoleTemplatesControllerTests.cs index 66cbcf6d2..4c467a363 100644 --- a/test/RAPS/RoleTemplatesControllerTests.cs +++ b/test/RAPS/RoleTemplatesControllerTests.cs @@ -24,7 +24,6 @@ public class RoleTemplatesControllerTests : IAsyncLifetime private SqliteConnection _connection = null!; private RAPSContext _context = null!; - private AAUDContext _aaudContext = null!; public async ValueTask InitializeAsync() { @@ -34,9 +33,6 @@ public async ValueTask InitializeAsync() .UseSqlite(_connection) .Options); await _context.Database.EnsureCreatedAsync(TestContext.Current.CancellationToken); - _aaudContext = new AAUDContext(new DbContextOptionsBuilder() - .UseSqlite(_connection) - .Options); var alreadyHeld = new TblRole { RoleId = RoleAlreadyHeldId, Role = "VIPER.AlreadyHeld", Description = "Held" }; var toAdd = new TblRole { RoleId = RoleToAddId, Role = "VIPER.ToAdd", Description = "Not held" }; @@ -66,11 +62,10 @@ public async ValueTask InitializeAsync() public async ValueTask DisposeAsync() { await _context.DisposeAsync(); - await _aaudContext.DisposeAsync(); await _connection.DisposeAsync(); } - private RoleTemplatesController CreateController() => new(_context, _aaudContext); + private RoleTemplatesController CreateController() => new(_context); [Fact] public async Task PreviewRoleTemplateApply_UnknownMember_ReturnsOkWithNullResult() diff --git a/web/Areas/Directory/Views/Card.cshtml b/web/Areas/Directory/Views/Card.cshtml index 53158cec2..5ef4c0687 100644 --- a/web/Areas/Directory/Views/Card.cshtml +++ b/web/Areas/Directory/Views/Card.cshtml @@ -38,10 +38,13 @@ Email {{user.mailId}}@@ucdavis.edu - - - Emulate {{user.name}} - + @if (UserHelper.HasPermission(rapsContext, UserHelper.GetCurrentUser(), "SVMSecure.SU")) + { + + + Emulate {{user.name}} + + } AAUD Check diff --git a/web/Areas/Directory/Views/Table.cshtml b/web/Areas/Directory/Views/Table.cshtml index 6427c31a1..41fb5bcd8 100644 --- a/web/Areas/Directory/Views/Table.cshtml +++ b/web/Areas/Directory/Views/Table.cshtml @@ -114,7 +114,7 @@ } @if (UserHelper.HasPermission(rapsContext, UserHelper.GetCurrentUser(), "SVMSecure.SU")) { - @: + @: } diff --git a/web/Areas/RAPS/Controllers/AuditController.cs b/web/Areas/RAPS/Controllers/AuditController.cs index 4e6b4e9c9..5ca712687 100644 --- a/web/Areas/RAPS/Controllers/AuditController.cs +++ b/web/Areas/RAPS/Controllers/AuditController.cs @@ -1,3 +1,4 @@ +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; using Viper.Areas.RAPS.Models; @@ -10,6 +11,7 @@ namespace Viper.Areas.RAPS.Controllers { [Route("raps/{instance}/[controller]")] + [Authorize(Roles = "VMDO SVM-IT,RAPS Users", Policy = "2faAuthentication")] [Permission(Allow = "RAPS.Admin,RAPS.ViewAuditTrail")] public class AuditController : ApiController { diff --git a/web/Areas/RAPS/Controllers/MemberPermissionsController.cs b/web/Areas/RAPS/Controllers/MemberPermissionsController.cs index ef3fb54ad..9f68dfa2f 100644 --- a/web/Areas/RAPS/Controllers/MemberPermissionsController.cs +++ b/web/Areas/RAPS/Controllers/MemberPermissionsController.cs @@ -18,16 +18,14 @@ public class MemberPermissionsController : ApiController private readonly RAPSContext _context; private readonly RAPSSecurityService _securityService; private readonly RAPSAuditService _auditService; - private readonly RAPSCacheService _rapsCacheService; public IUserHelper UserHelper { get; private set; } - public MemberPermissionsController(RAPSContext context, AAUDContext aaudContext) + public MemberPermissionsController(RAPSContext context) { _context = context; _securityService = new RAPSSecurityService(_context); _auditService = new RAPSAuditService(_context); UserHelper = new UserHelper(); - _rapsCacheService = new RAPSCacheService(context, aaudContext, UserHelper); } // GET: Members/12345678/Permissions @@ -204,8 +202,6 @@ public async Task PutTblMemberPermission(string instance, string throw; } - _rapsCacheService.ClearCachedRolesAndPermissionsForUser(memberId); - return NoContent(); } @@ -262,8 +258,6 @@ public async Task> PostTblMemberPermission(str throw; } - _rapsCacheService.ClearCachedRolesAndPermissionsForUser(memberId); - return CreatedAtAction("GetTblMemberPermission", new { memberId, permissionId }, tblMemberPermission); } @@ -292,8 +286,6 @@ public async Task DeleteTblMemberPermission(string instance, stri _auditService.AuditPermissionMemberChange(tblMemberPermission, RAPSAuditService.AuditActionType.Delete); await _context.SaveChangesAsync(); - _rapsCacheService.ClearCachedRolesAndPermissionsForUser(memberId); - return NoContent(); } diff --git a/web/Areas/RAPS/Controllers/MembersController.cs b/web/Areas/RAPS/Controllers/MembersController.cs index 1825847a6..b97610e2d 100644 --- a/web/Areas/RAPS/Controllers/MembersController.cs +++ b/web/Areas/RAPS/Controllers/MembersController.cs @@ -1,3 +1,4 @@ +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; using Viper.Areas.RAPS.Models; @@ -11,21 +12,23 @@ namespace Viper.Areas.RAPS.Controllers { [Route("raps/{Instance=VIPER}/[controller]")] [ApiController] + [Authorize(Roles = "VMDO SVM-IT,RAPS Users", Policy = "2faAuthentication")] public class MembersController : ControllerBase { private readonly RAPSContext _context; private readonly RAPSSecurityService _securityService; private readonly RAPSAuditService _auditService; - private readonly RAPSCacheService _rapsCacheService; - public MembersController(RAPSContext context, AAUDContext aaudContext) + public MembersController(RAPSContext context) { _context = context; _securityService = new RAPSSecurityService(_context); _auditService = new RAPSAuditService(_context); - _rapsCacheService = new RAPSCacheService(_context, aaudContext); } // GET: + // The union of what the three pages using this typeahead require. Without it, any RAPS Users + // member could enumerate identities here. + [Permission(Allow = "RAPS.Admin,RAPS.UserLookup,RAPS.EditRoleMembership,RAPS.EditMemberPermissions")] [HttpGet] public async Task>> Search(string search, string active = "active") { @@ -68,6 +71,7 @@ public async Task>> Search(string s } // GET /12345678 + [Permission(Allow = "RAPS.Admin,RAPS.UserLookup,RAPS.EditRoleMembership,RAPS.EditMemberPermissions")] [HttpGet("{memberId}")] public async Task> Get(string memberId) { @@ -217,7 +221,6 @@ public async Task Clone(string instance, string sourceMemberId, st } await new CloneService(_context).Clone(instance, sourceMemberId, targetMemberId, objectsToClone); - _rapsCacheService.ClearCachedRolesAndPermissionsForUser(targetMemberId); return NoContent(); } diff --git a/web/Areas/RAPS/Controllers/RAPSController.cs b/web/Areas/RAPS/Controllers/RAPSController.cs index aaaf3c877..4f960d03c 100644 --- a/web/Areas/RAPS/Controllers/RAPSController.cs +++ b/web/Areas/RAPS/Controllers/RAPSController.cs @@ -6,7 +6,6 @@ using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.Data.SqlClient; using Microsoft.EntityFrameworkCore; -using Microsoft.IdentityModel.Tokens; using NLog; using Viper.Areas.RAPS.Services; using Viper.Classes; @@ -18,7 +17,7 @@ namespace Viper.Areas.RAPS.Controllers { [Area("RAPS")] [Route("[area]/[action]")] - [Authorize(Roles = "VMDO SVM-IT,RAPS Users")]//, Policy = "2faAuthentication" + [Authorize(Roles = "VMDO SVM-IT,RAPS Users", Policy = "2faAuthentication")] public class RAPSController : AreaController { private readonly RAPSContext _RAPSContext; @@ -121,7 +120,10 @@ public async Task Nav(int? roleId, int? permissionId, string? memberId, nav.Add(new NavMenuItem { MenuItemText = inst, MenuItemURL = "~/raps/" + inst + "/" + (usePage ? page : "RoleList") }); } nav.Add(new NavMenuItem { MenuItemText = "Roles", IsHeader = true }); - nav.Add(new NavMenuItem { MenuItemText = "Role List", MenuItemURL = "Rolelist" }); + if (_securityService.CanViewRoleList(instance)) + { + nav.Add(new NavMenuItem { MenuItemText = "Role List", MenuItemURL = "Rolelist" }); + } if (_securityService.IsAllowedTo("EditRoleMembership", instance)) { nav.Add(new NavMenuItem { MenuItemText = "Role Comparison", MenuItemURL = "RolePermissionsComparison" }); @@ -236,8 +238,7 @@ public IActionResult RoleList(string instance) return View("~/Areas/RAPS/Views/Roles/ListAdmin.cshtml"); } - if (_securityService.IsAllowedTo("ViewAllRoles", instance) || - !_securityService.GetControlledRoleIds(UserHelper.GetCurrentUser()?.MothraId).IsNullOrEmpty()) + if (_securityService.CanViewRoleList(instance)) { return View("~/Areas/RAPS/Views/Roles/List.cshtml"); } diff --git a/web/Areas/RAPS/Controllers/RoleMembersController.cs b/web/Areas/RAPS/Controllers/RoleMembersController.cs index 21d3bf3e3..e46f6f395 100644 --- a/web/Areas/RAPS/Controllers/RoleMembersController.cs +++ b/web/Areas/RAPS/Controllers/RoleMembersController.cs @@ -17,14 +17,12 @@ public class RoleMembersController : ApiController private readonly RAPSContext _context; private readonly RAPSSecurityService _securityService; private readonly RAPSAuditService _auditService; - private readonly RAPSCacheService _rapsCacheService; - public RoleMembersController(RAPSContext context, AAUDContext aaudContext) + public RoleMembersController(RAPSContext context) { _context = context; _securityService = new RAPSSecurityService(_context); _auditService = new RAPSAuditService(_context); - _rapsCacheService = new RAPSCacheService(_context, aaudContext); } //GET: Roles/5/Members @@ -102,8 +100,6 @@ public async Task>> PostTblRoleMembers(s return BadRequest(result); } - _rapsCacheService.ClearCachedRolesAndPermissionsForUser(memberId); - TblRoleMember? tblRoleMember = await _context.TblRoleMembers.FindAsync(roleId, memberId); return CreatedAtAction("GetTblRole", new { roleId, memberId }, tblRoleMember); } @@ -139,8 +135,6 @@ public async Task>> PutTblRoleMembers(st _auditService.AuditRoleMemberChange(tblRoleMember, RAPSAuditService.AuditActionType.Update, roleMemberCreateUpdate.Comment); await _context.SaveChangesAsync(); - _rapsCacheService.ClearCachedRolesAndPermissionsForUser(memberId); - return NoContent(); } @@ -169,8 +163,6 @@ public async Task DeleteTblRoleMembers(string instance, int roleI _auditService.AuditRoleMemberChange(tblRoleMember, RAPSAuditService.AuditActionType.Delete, comment); await _context.SaveChangesAsync(); - _rapsCacheService.ClearCachedRolesAndPermissionsForUser(memberId); - return NoContent(); } diff --git a/web/Areas/RAPS/Controllers/RolePermissionsController.cs b/web/Areas/RAPS/Controllers/RolePermissionsController.cs index 82f8b3b65..3c218e429 100644 --- a/web/Areas/RAPS/Controllers/RolePermissionsController.cs +++ b/web/Areas/RAPS/Controllers/RolePermissionsController.cs @@ -18,14 +18,12 @@ public class RolePermissionsController : ApiController private readonly RAPSContext _context; private readonly RAPSSecurityService _securityService; private readonly RAPSAuditService _auditService; - private readonly RAPSCacheService _rapsCacheService; - public RolePermissionsController(RAPSContext context, AAUDContext aaudContext) + public RolePermissionsController(RAPSContext context) { _context = context; _securityService = new RAPSSecurityService(_context); _auditService = new RAPSAuditService(_context); - _rapsCacheService = new RAPSCacheService(_context, aaudContext); } private ActionResult? CheckRoleAndPermissionParams(string instance, int? roleId, int? permissionId) @@ -174,8 +172,6 @@ public async Task> PostTblRolePermission(string await _context.SaveChangesAsync(); await transaction.CommitAsync(); - await ClearCacheForAllRoleMembers(rolePermission.RoleId); - return CreatedAtAction("GetTblRole", new { roleId, permissionId, tblRolePermission.Access }, tblRolePermission); } @@ -206,8 +202,6 @@ public async Task> DeleteTblRolePermission(string in _auditService.AuditRolePermissionChange(tblRolePermission, RAPSAuditService.AuditActionType.Delete); await _context.SaveChangesAsync(); - await ClearCacheForAllRoleMembers(roleId); - return NoContent(); } @@ -219,14 +213,5 @@ private static void UpdateTblRolePermissionsWithDto(TblRolePermission tblRolePer tblRolePermission.ModTime = DateTime.Now; tblRolePermission.ModBy = new UserHelper().GetCurrentUser()?.LoginId; } - - private async Task ClearCacheForAllRoleMembers(int roleId) - { - var roleMembers = await _context.TblRoleMembers.Where(rm => rm.RoleId == roleId).ToListAsync(); - foreach (var member in roleMembers) - { - _rapsCacheService.ClearCachedRolesAndPermissionsForUser(member.MemberId); - } - } } } diff --git a/web/Areas/RAPS/Controllers/RoleTemplatesController.cs b/web/Areas/RAPS/Controllers/RoleTemplatesController.cs index 2903fc97e..a65fbbc9d 100644 --- a/web/Areas/RAPS/Controllers/RoleTemplatesController.cs +++ b/web/Areas/RAPS/Controllers/RoleTemplatesController.cs @@ -16,14 +16,12 @@ namespace Viper.Areas.RAPS.Controllers public class RoleTemplatesController : ApiController { private readonly RAPSContext _context; - private readonly RAPSCacheService _rapsCacheService; public IUserHelper UserHelper { get; private set; } - public RoleTemplatesController(RAPSContext context, AAUDContext aaudContext) + public RoleTemplatesController(RAPSContext context) { _context = context; UserHelper = new UserHelper(); - _rapsCacheService = new RAPSCacheService(context, aaudContext, UserHelper); } // GET: RoleTemplates @@ -125,8 +123,6 @@ public async Task> RoleTemplateApply(stri await roleMemberService.AddMemberToRole(role.RoleId, memberId, null, null, string.Format("Added via role template {0}", roleTemplate.TemplateName)); } - _rapsCacheService.ClearCachedRolesAndPermissionsForUser(memberId); - return NoContent(); } diff --git a/web/Areas/RAPS/Services/RAPSCacheService.cs b/web/Areas/RAPS/Services/RAPSCacheService.cs deleted file mode 100644 index 1590c1fb6..000000000 --- a/web/Areas/RAPS/Services/RAPSCacheService.cs +++ /dev/null @@ -1,28 +0,0 @@ -using Microsoft.EntityFrameworkCore; -using Viper.Classes.SQLContext; -using Viper.Models.AAUD; - -namespace Viper.Areas.RAPS.Services -{ - public class RAPSCacheService - { - private readonly AAUDContext aaudContext; - private readonly IUserHelper userHelper; - public RAPSCacheService(RAPSContext rapsContext, AAUDContext aaudContext, IUserHelper? userHelper = null) - { - this.aaudContext = aaudContext; - this.userHelper = userHelper ?? new UserHelper(); - } - - - public void ClearCachedRolesAndPermissionsForUser(string mothraId) - { - AaudUser? user = aaudContext.AaudUsers.AsNoTracking().FirstOrDefault(u => u.MothraId == mothraId); - if (user != null) - { - userHelper.ClearCachedRolesAndPermissions(user); - } - } - } - -} diff --git a/web/Areas/RAPS/Services/RAPSSecurityService.cs b/web/Areas/RAPS/Services/RAPSSecurityService.cs index 76cc25ea2..90d27d114 100644 --- a/web/Areas/RAPS/Services/RAPSSecurityService.cs +++ b/web/Areas/RAPS/Services/RAPSSecurityService.cs @@ -10,6 +10,7 @@ public class RAPSSecurityService { private readonly IUserHelper _userHelper; private readonly RAPSContext _context; + private readonly Dictionary> _appRolesForUser = new(); public RAPSSecurityService(RAPSContext context, IUserHelper? userHelper = null) { @@ -212,13 +213,23 @@ public bool IsAllowedTo(string action, string instance, TblRole Role) /// A list of delegate roles the user is assigned to public List GetAppRolesForUser(string? userId) { + // Nav() asks once per instance on every page load, so memoize for the life of the + // service, which is one request. + string key = userId ?? string.Empty; + if (_appRolesForUser.TryGetValue(key, out List? cached)) + { + return cached; + } + List roles = _context.TblRoles - .Include(r => r.TblRoleMembers) + .AsNoTracking() .Include(r => r.ChildRoles) .ThenInclude(cr => cr.Role) .Where(r => r.Application == 1) .Where(r => r.TblRoleMembers.Any(rm => rm.MemberId == userId)) .ToList(); + + _appRolesForUser[key] = roles; return roles; } @@ -229,16 +240,41 @@ public List GetAppRolesForUser(string? userId) /// List of roleIds the user controls public List GetControlledRoleIds(string? userId) { - List controlledRoles = GetAppRolesForUser(userId); - List controlledRoleIds = new(); - foreach (TblRole controlledRole in controlledRoles) - { - foreach (TblAppRole childRole in controlledRole.ChildRoles) - { - controlledRoleIds.Add(childRole.Role.RoleId); - } - } - return controlledRoleIds; + return ControlledRoles(userId).Select(r => r.RoleId).ToList(); + } + + /// + /// The role ids a user controls through delegate roles, limited to one instance. + /// + /// The instance + /// User mothra id + /// List of roleIds the user controls in that instance + public List GetControlledRoleIdsInInstance(string instance, string? userId) + { + return ControlledRoles(userId) + .Where(r => RoleBelongsToInstance(instance, r)) + .Select(r => r.RoleId) + .Distinct() + .ToList(); + } + + private IEnumerable ControlledRoles(string? userId) + { + return GetAppRolesForUser(userId).SelectMany(r => r.ChildRoles).Select(cr => cr.Role); + } + + /// + /// Check if the user can see the role list for an instance. Admins see every role, others see + /// all roles only in VMACS instances, or just the roles delegated to them. + /// + /// The instance + /// true if the user can view the role list, false otherwise + public bool CanViewRoleList(string instance) + { + // Instance-scoped because the role list is: an unscoped count would offer a delegate a + // nav link to a list filtered down to nothing. + return IsAllowedTo("ViewAllRoles", instance) + || GetControlledRoleIdsInInstance(instance, _userHelper.GetCurrentUser()?.MothraId).Count > 0; } /// diff --git a/web/Areas/RAPS/Services/RapsCacheInvalidationInterceptor.cs b/web/Areas/RAPS/Services/RapsCacheInvalidationInterceptor.cs new file mode 100644 index 000000000..d822be79e --- /dev/null +++ b/web/Areas/RAPS/Services/RapsCacheInvalidationInterceptor.cs @@ -0,0 +1,140 @@ +using System.Runtime.CompilerServices; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Viper.Classes.SQLContext; +using Viper.Models.RAPS; + +namespace Viper.Areas.RAPS.Services +{ + /// + /// Evicts cached roles and permissions for whoever a RAPS write affected. Lives on the context + /// rather than at each call site so every path reaching the database is covered, including ones + /// added later. + /// + public class RapsCacheInvalidationInterceptor : SaveChangesInterceptor + { + private sealed record Affected(HashSet MemberIds, HashSet RoleIds); + + // Keyed on the context instance so a scoped context's pending set cannot leak into another + // request's. ConditionalWeakTable drops entries when the context is collected. + private static readonly ConditionalWeakTable Pending = new(); + + public override InterceptionResult SavingChanges(DbContextEventData eventData, InterceptionResult result) + { + StashAffected(eventData); + return base.SavingChanges(eventData, result); + } + + public override ValueTask> SavingChangesAsync(DbContextEventData eventData, + InterceptionResult result, CancellationToken cancellationToken = default) + { + StashAffected(eventData); + return base.SavingChangesAsync(eventData, result, cancellationToken); + } + + public override int SavedChanges(SaveChangesCompletedEventData eventData, int result) + { + Evict(eventData); + return base.SavedChanges(eventData, result); + } + + public override ValueTask SavedChangesAsync(SaveChangesCompletedEventData eventData, int result, + CancellationToken cancellationToken = default) + { + Evict(eventData); + return base.SavedChangesAsync(eventData, result, cancellationToken); + } + + // Read the ChangeTracker before SaveChanges: afterwards deleted entries are detached and their + // ids are gone. + private static void StashAffected(DbContextEventData eventData) + { + var context = eventData.Context; + if (context == null) + { + return; + } + + var affected = new Affected(new HashSet(StringComparer.OrdinalIgnoreCase), new HashSet()); + + foreach (var entry in context.ChangeTracker.Entries()) + { + if (entry.State is not (EntityState.Added or EntityState.Modified or EntityState.Deleted)) + { + continue; + } + + switch (entry.Entity) + { + // A membership or an individual grant changes exactly one person. + case TblRoleMember roleMember: + Add(affected.MemberIds, roleMember.MemberId); + break; + case TblMemberPermission memberPermission: + Add(affected.MemberIds, memberPermission.MemberId); + break; + // A role's permissions change everyone currently in that role. + case TblRolePermission rolePermission: + affected.RoleIds.Add(rolePermission.RoleId); + break; + case TblRole role: + affected.RoleIds.Add(role.RoleId); + break; + } + } + + Pending.Remove(context); + if (affected.MemberIds.Count > 0 || affected.RoleIds.Count > 0) + { + Pending.Add(context, affected); + } + } + + private static void Evict(DbContextEventData eventData) + { + var context = eventData.Context; + if (context == null) + { + return; + } + + if (!Pending.TryGetValue(context, out var affected)) + { + return; + } + // Dropped from the table, so the sets below are ours to mutate. + Pending.Remove(context); + + // Expand role-level changes to that role's current members. Runs after the save, so the + // membership read reflects what was just committed. + if (affected.RoleIds.Count > 0 && context is RAPSContext rapsContext) + { + // EF.Parameter so a bulk change (role template apply, OU sync) translates through + // OPENJSON rather than inlining every id. + List changedRoleIds = affected.RoleIds.ToList(); + foreach (string memberId in rapsContext.TblRoleMembers + .AsNoTracking() + .Where(rm => EF.Parameter(changedRoleIds).Contains(rm.RoleId)) + .Select(rm => rm.MemberId) + .Distinct() + .ToList()) + { + Add(affected.MemberIds, memberId); + } + } + + foreach (string mothraId in affected.MemberIds) + { + UserHelper.ClearCachedRolesAndPermissions(mothraId); + } + } + + private static void Add(HashSet set, string? memberId) + { + if (!string.IsNullOrEmpty(memberId)) + { + set.Add(memberId); + } + } + } +} diff --git a/web/Areas/Students/Services/EmergencyContactService.cs b/web/Areas/Students/Services/EmergencyContactService.cs index 38d81e1e1..cffbed1da 100644 --- a/web/Areas/Students/Services/EmergencyContactService.cs +++ b/web/Areas/Students/Services/EmergencyContactService.cs @@ -17,7 +17,6 @@ public class EmergencyContactService : IEmergencyContactService private readonly AAUDContext _aaudContext; private readonly IUserHelper _userHelper; private readonly ILogger _logger; - private readonly RAPSCacheService _rapsCacheService; private readonly RAPSAuditService _rapsAuditService; public EmergencyContactService( @@ -32,7 +31,6 @@ public EmergencyContactService( _aaudContext = aaudContext; _userHelper = userHelper; _logger = logger; - _rapsCacheService = new RAPSCacheService(rapsContext, aaudContext, userHelper); _rapsAuditService = new RAPSAuditService(rapsContext, userHelper); } @@ -311,7 +309,6 @@ public async Task ToggleAppAccessAsync() _rapsAuditService.AuditRolePermissionChange(rolePermission!, RAPSAuditService.AuditActionType.Delete); _rapsContext.TblRolePermissions.Remove(rolePermission!); await _rapsContext.SaveChangesAsync(); - ClearCacheForRoleMembers(roleId); return false; } @@ -337,7 +334,6 @@ public async Task ToggleAppAccessAsync() _rapsAuditService.AuditRolePermissionChange(rolePermission, RAPSAuditService.AuditActionType.Update); } await _rapsContext.SaveChangesAsync(); - ClearCacheForRoleMembers(roleId); return true; } @@ -374,7 +370,6 @@ public async Task ToggleIndividualAccessAsync(int personId) } _rapsContext.TblMemberPermissions.RemoveRange(existing); await _rapsContext.SaveChangesAsync(); - _rapsCacheService.ClearCachedRolesAndPermissionsForUser(user.MothraId); return false; } @@ -391,7 +386,6 @@ public async Task ToggleIndividualAccessAsync(int personId) _rapsContext.TblMemberPermissions.Add(memberPermission); _rapsAuditService.AuditPermissionMemberChange(memberPermission, RAPSAuditService.AuditActionType.Create); await _rapsContext.SaveChangesAsync(); - _rapsCacheService.ClearCachedRolesAndPermissionsForUser(user.MothraId); return true; } @@ -598,25 +592,6 @@ private async Task IsCurrentDvmStudentAsync(int personId) return null; } - /// - /// Clears the cached roles and permissions for all members of a given role, - /// so that permission changes take effect immediately. - /// - private void ClearCacheForRoleMembers(int roleId) - { - var memberIds = _rapsContext.TblRoleMembers - .Where(rm => rm.RoleId == roleId - && (rm.StartDate == null || rm.StartDate <= DateTime.Now) - && (rm.EndDate == null || rm.EndDate >= DateTime.Now)) - .Select(rm => rm.MemberId) - .ToList(); - - foreach (var memberId in memberIds) - { - _rapsCacheService.ClearCachedRolesAndPermissionsForUser(memberId); - } - } - private static void ValidatePhone(string? value, string fieldName, List invalidFields) { if (!PhoneHelper.IsValidPhone(value)) diff --git a/web/Classes/DuoAuthenticationRequirement.cs b/web/Classes/DuoAuthenticationRequirement.cs index dc927775d..25f85c6d4 100644 --- a/web/Classes/DuoAuthenticationRequirement.cs +++ b/web/Classes/DuoAuthenticationRequirement.cs @@ -32,12 +32,17 @@ protected override Task HandleRequirementAsync(AuthorizationHandlerContext conte { if (httpContext is not null) { + // No Duo credential can be issued for a localhost callback, so Development bypasses + // the check. RapsControllerAuthorizationTests is what guards the policy itself. var env = httpContext.RequestServices.GetRequiredService(); if (env != null && env.EnvironmentName == "Development") { context.Succeed(requirement); } - httpContext.Items["ErrorMessage"] = "DUO two-factor authentication is required"; + else + { + httpContext.Items["ErrorMessage"] = "DUO two-factor authentication is required"; + } } else { diff --git a/web/Classes/UserHelper.cs b/web/Classes/UserHelper.cs index 849af0d1e..de762aa8f 100644 --- a/web/Classes/UserHelper.cs +++ b/web/Classes/UserHelper.cs @@ -17,6 +17,33 @@ public class UserHelper : IUserHelper { private readonly AAUDContext? _aaudContext; + // Keyed by MothraId, not the nullable LoginId, which collided across every user without one. + // Entries do not expire; RapsCacheInvalidationInterceptor evicts them. + private static string RolesCacheKey(string mothraId) => "Roles-" + mothraId; + + private static string AssignedPermissionsCacheKey(string mothraId, bool deny) => "PermissionsAssigned-" + mothraId + "-" + deny; + + private static string InheritedPermissionsCacheKey(string mothraId, bool deny) => "PermissionsInherited-" + mothraId + "-" + deny; + + /// + /// Evict one user's cached roles and permissions. Takes a MothraId so callers holding only the + /// RAPS-side identifier need no AaudUser lookup. + /// + public static void ClearCachedRolesAndPermissions(string mothraId) + { + if (HttpHelper.Cache == null || string.IsNullOrEmpty(mothraId)) + { + return; + } + + HttpHelper.Cache.Remove(RolesCacheKey(mothraId)); + foreach (bool deny in new[] { true, false }) + { + HttpHelper.Cache.Remove(AssignedPermissionsCacheKey(mothraId, deny)); + HttpHelper.Cache.Remove(InheritedPermissionsCacheKey(mothraId, deny)); + } + } + public UserHelper() { } public UserHelper(AAUDContext aaudContext) @@ -56,7 +83,7 @@ public IEnumerable GetRoles(RAPSContext rapsContext, AaudUser user) if (HttpHelper.Cache != null && rapsContext != null) { - result = HttpHelper.Cache.GetOrCreate("Roles-" + user.LoginId, entry => + result = HttpHelper.Cache.GetOrCreate(RolesCacheKey(user.MothraId), _ => { return (from role in rapsContext.TblRoles join memberRoles in rapsContext.TblRoleMembers @@ -117,7 +144,7 @@ public IEnumerable GetAssignedPermissions(RAPSContext rapsContext if (HttpHelper.Cache != null && rapsContext != null) { - result = HttpHelper.Cache.GetOrCreate("PermissionsAssigned-" + user.LoginId + "-" + deny, entry => + result = HttpHelper.Cache.GetOrCreate(AssignedPermissionsCacheKey(user.MothraId, deny), _ => { return (from permission in rapsContext.TblPermissions join memberPermissions in rapsContext.TblMemberPermissions @@ -157,7 +184,7 @@ public static IEnumerable GetInheritedPermissions(RAPSContext rap if (HttpHelper.Cache != null && rapsContext != null) { - result = HttpHelper.Cache.GetOrCreate("PermissionsInherited-" + user.LoginId + "-" + deny, entry => + result = HttpHelper.Cache.GetOrCreate(InheritedPermissionsCacheKey(user.MothraId, deny), _ => { return (from permission in rapsContext.TblPermissions join rolePermissions in rapsContext.TblRolePermissions @@ -346,13 +373,9 @@ public bool IsEmulating() public void ClearCachedRolesAndPermissions(AaudUser? user) { - if (user != null && HttpHelper.Cache != null) + if (user != null) { - HttpHelper.Cache.Remove("Roles-" + user.LoginId); - HttpHelper.Cache.Remove("PermissionsAssigned-" + user.LoginId + "-" + true); - HttpHelper.Cache.Remove("PermissionsAssigned-" + user.LoginId + "-" + false); - HttpHelper.Cache.Remove("PermissionsInherited-" + user.LoginId + "-" + false); - HttpHelper.Cache.Remove("PermissionsInherited-" + user.LoginId + "-" + false); + ClearCachedRolesAndPermissions(user.MothraId); } } diff --git a/web/Program.cs b/web/Program.cs index 126ab15c8..d51acaf3d 100644 --- a/web/Program.cs +++ b/web/Program.cs @@ -31,6 +31,7 @@ using Viper.Areas.Effort; using Viper.Areas.Effort.Data; using Viper.Areas.Effort.Services.Harvest; +using Viper.Areas.RAPS.Services; using Viper.Classes; using Viper.Classes.HealthChecks; using Viper.Classes.Scheduler; @@ -200,7 +201,7 @@ // Configure DbContext options with connection strings via DI var enableDetailedErrors = builder.Environment.EnvironmentName != "Production"; - void RegisterDbContext(string connectionStringKey) where TContext : DbContext + void RegisterDbContext(string connectionStringKey, Action? configure = null) where TContext : DbContext { var connStr = builder.Configuration.GetConnectionString(connectionStringKey) ?? throw new InvalidOperationException($"Connection string '{connectionStringKey}' not configured"); @@ -209,6 +210,7 @@ void RegisterDbContext(string connectionStringKey) where TContext : Db // Match our SQL Server 2016 compat level (130) so EF Core 10 generates optimal SQL for our DB version options.UseSqlServer(connStr, o => o.UseCompatibilityLevel(130)); if (enableDetailedErrors) options.EnableDetailedErrors(); + configure?.Invoke(options); }); } @@ -216,7 +218,8 @@ void RegisterDbContext(string connectionStringKey) where TContext : Db RegisterDbContext("Courses"); RegisterDbContext("CREST"); RegisterDbContext("Dictionary"); - RegisterDbContext("RAPS"); + // The interceptor evicts affected users' cached permissions on any RAPS write, whatever made it. + RegisterDbContext("RAPS", o => o.AddInterceptors(new RapsCacheInvalidationInterceptor())); RegisterDbContext("VIPER"); RegisterDbContext("ClinicalScheduler"); RegisterDbContext("SIS");