diff --git a/.editorconfig b/.editorconfig index 2d30f9d38..407159296 100644 --- a/.editorconfig +++ b/.editorconfig @@ -163,6 +163,13 @@ dotnet_style_qualification_for_property = false:silent dotnet_style_qualification_for_method = false:silent dotnet_style_qualification_for_event = false:silent -# EF Core scaffold-generated DbContext files — partial methods are intentional extension points +# EF Core scaffold-generated DbContext files — partial methods are intentional extension points, +# and the maintainability index is dominated by the generated OnModelCreating (not hand-maintained code) [**/SQLContext/*Context.cs] dotnet_diagnostic.S3251.severity = none +dotnet_diagnostic.CA1505.severity = none + +# S6964: flags value-type props as under-postable, but ApiPagination is never bound from a request: +# ApiPaginationAttribute.OnActionExecuting always constructs the object and overwrites the action argument +[web/Models/ApiPagination.cs] +dotnet_diagnostic.S6964.severity = none diff --git a/test/ClinicalScheduler/Integration/ServiceLayerIntegrationTest.cs b/test/ClinicalScheduler/Integration/ServiceLayerIntegrationTest.cs index c55c21156..93c813bb6 100644 --- a/test/ClinicalScheduler/Integration/ServiceLayerIntegrationTest.cs +++ b/test/ClinicalScheduler/Integration/ServiceLayerIntegrationTest.cs @@ -24,9 +24,25 @@ public class ServiceLayerIntegrationTest : IntegrationTestBase public ServiceLayerIntegrationTest() { - // Create mock services since actual services require VIPERContext - // Note: studentLogger not used since we're mocking the service + _studentScheduleService = CreateMockStudentScheduleService(); + _instructorScheduleService = CreateMockInstructorScheduleService(); + + _clinicalScheduleService = new ClinicalScheduleService( + _studentScheduleService, + _instructorScheduleService + ); + + var personLogger = Substitute.For>(); + _personService = new PersonService(personLogger, Context, AaudContext); + + // EvaluationPolicyService is now static-like with no constructor parameters needed + _evaluationPolicyService = new EvaluationPolicyService(); + } + + // Note: studentLogger not used since we're mocking the service + private static IStudentScheduleService CreateMockStudentScheduleService() + { var mockStudentService = Substitute.For(); mockStudentService.GetStudentScheduleAsync( Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), @@ -84,9 +100,12 @@ public ServiceLayerIntegrationTest() return Task.FromResult(mockData); }); - _studentScheduleService = mockStudentService; + return mockStudentService; + } - // Note: instructorLogger not used since we're mocking the service + // Note: instructorLogger not used since we're mocking the service + private static IInstructorScheduleService CreateMockInstructorScheduleService() + { var mockInstructorService = Substitute.For(); mockInstructorService.GetInstructorScheduleAsync( Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), @@ -159,18 +178,7 @@ public ServiceLayerIntegrationTest() return Task.FromResult(mockData); }); - _instructorScheduleService = mockInstructorService; - - _clinicalScheduleService = new ClinicalScheduleService( - _studentScheduleService, - _instructorScheduleService - ); - - var personLogger = Substitute.For>(); - _personService = new PersonService(personLogger, Context, AaudContext); - - // EvaluationPolicyService is now static-like with no constructor parameters needed - _evaluationPolicyService = new EvaluationPolicyService(); + return mockInstructorService; } [Fact] diff --git a/test/Directory/DirectoryControllerTests.cs b/test/Directory/DirectoryControllerTests.cs new file mode 100644 index 000000000..e73855ab6 --- /dev/null +++ b/test/Directory/DirectoryControllerTests.cs @@ -0,0 +1,141 @@ +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Viper.Areas.Directory.Controllers; +using Viper.Classes.SQLContext; +using Viper.Models.AAUD; + +// Not Viper.test.Directory: a namespace segment named "Directory" shadows System.IO.Directory +// for every file under Viper.test.* +// ReSharper disable once CheckNamespace +namespace Viper.test.DirectoryArea; + +/// +/// Tests for the shared directory search query, run against the relational SQLite +/// provider so the predicate goes through EF's SQL translator like it does on SQL +/// Server (the InMemory provider executes LINQ in memory and skips translation). +/// SQLite string matching is case-sensitive where SQL Server's default collation is +/// not, so seed data matches the search term's exact case. +/// +public sealed class DirectoryControllerTests : IDisposable +{ + /// + /// A trimmed AAUDContext mapping only AaudUser: the full AAUD warehouse model has + /// computed columns and views SQLite EnsureCreated cannot build (same pattern as + /// StudentGroupServiceQueryTests). Current is a plain column here, so tests set it + /// directly instead of via the production computed column. + /// + private sealed class TestAaudContext(DbContextOptions options) : AAUDContext(options) + { + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + foreach (var clrType in modelBuilder.Model.GetEntityTypes() + .Select(e => e.ClrType) + .Where(t => t != typeof(AaudUser)) + .Distinct() + .ToList()) + { + modelBuilder.Ignore(clrType); + } + modelBuilder.Entity().HasKey(e => e.AaudUserId); + } + } + + private readonly SqliteConnection _connection; + private readonly AAUDContext _context; + + public DirectoryControllerTests() + { + _connection = new SqliteConnection("DataSource=:memory:"); + _connection.Open(); + _context = new TestAaudContext( + new DbContextOptionsBuilder().UseSqlite(_connection).Options); + _context.Database.EnsureCreated(); + } + + public void Dispose() + { + _context.Dispose(); + _connection.Dispose(); + } + + [Fact] + public async Task SearchCurrentAaudUsers_MatchesNameAndEveryIdentifierField() + { + SeedUser(1, lastName: "Delfigo", firstName: "Ann"); + SeedUser(2, lastName: "Berry", firstName: "Bo", mailId: "fig@ucdavis.edu"); + SeedUser(3, lastName: "Cherry", firstName: "Cy", loginId: "figuser"); + SeedUser(4, lastName: "Damson", firstName: "Di", spridenId: "fig123"); + SeedUser(5, lastName: "Elder", firstName: "Ed", pidm: "00fig"); + SeedUser(6, lastName: "Fennel", firstName: "Flo", mothraId: "fig999"); + SeedUser(7, lastName: "Guava", firstName: "Gil", employeeId: "emp-fig"); + SeedUser(8, lastName: "Haw", firstName: "Hal", iamId: "iam-fig"); + SeedUser(9, lastName: "Ivy", firstName: "Ira"); + await _context.SaveChangesAsync(TestContext.Current.CancellationToken); + + var results = await DirectoryController.SearchCurrentAaudUsers(_context, "fig"); + + // One match per field, none for user 9, ordered by last name + Assert.Equal([2, 3, 4, 1, 5, 6, 7, 8], results.Select(u => u.AaudUserId)); + } + + [Fact] + public async Task SearchCurrentAaudUsers_MatchesTermSpanningFirstAndLastName() + { + SeedUser(1, lastName: "Graham", firstName: "Anna"); + SeedUser(2, lastName: "Graham", firstName: "Steve"); + await _context.SaveChangesAsync(TestContext.Current.CancellationToken); + + var results = await DirectoryController.SearchCurrentAaudUsers(_context, "na Gra"); + + Assert.Equal([1], results.Select(u => u.AaudUserId)); + } + + [Fact] + public async Task SearchCurrentAaudUsers_ExcludesUsersNoLongerCurrent() + { + SeedUser(1, lastName: "Figworth", firstName: "Cur"); + SeedUser(2, lastName: "Figworth", firstName: "Old", current: 0); + await _context.SaveChangesAsync(TestContext.Current.CancellationToken); + + var results = await DirectoryController.SearchCurrentAaudUsers(_context, "Figworth"); + + Assert.Equal([1], results.Select(u => u.AaudUserId)); + } + + [Fact] + public async Task SearchCurrentAaudUsers_OrdersByLastNameThenFirstName() + { + SeedUser(1, lastName: "Fig", firstName: "Zoe"); + SeedUser(2, lastName: "Fig", firstName: "Al"); + SeedUser(3, lastName: "Elm", firstName: "Bea", mailId: "Fig@ucdavis.edu"); + await _context.SaveChangesAsync(TestContext.Current.CancellationToken); + + var results = await DirectoryController.SearchCurrentAaudUsers(_context, "Fig"); + + Assert.Equal([3, 2, 1], results.Select(u => u.AaudUserId)); + } + + private void SeedUser(int id, string lastName, string firstName, string? mailId = null, + string? loginId = null, string? spridenId = null, string? pidm = null, + string? mothraId = null, string? employeeId = null, string? iamId = null, int current = 1) + { + _context.AaudUsers.Add(new AaudUser + { + AaudUserId = id, + ClientId = "test", + MothraId = mothraId ?? $"m-{id}", + LoginId = loginId, + MailId = mailId, + SpridenId = spridenId, + Pidm = pidm, + EmployeeId = employeeId, + IamId = iamId, + LastName = lastName, + FirstName = firstName, + DisplayLastName = lastName, + DisplayFirstName = firstName, + DisplayFullName = firstName + " " + lastName, + Current = current + }); + } +} diff --git a/test/RAPS/AdGroupsControllerTests.cs b/test/RAPS/AdGroupsControllerTests.cs new file mode 100644 index 000000000..14491df5e --- /dev/null +++ b/test/RAPS/AdGroupsControllerTests.cs @@ -0,0 +1,50 @@ +using System.Runtime.Versioning; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Viper.Areas.RAPS.Controllers; +using Viper.Areas.RAPS.Models; +using Viper.Classes.SQLContext; +using Viper.Models.RAPS; + +namespace Viper.test.RAPS +{ + [SupportedOSPlatform("windows")] + public class AdGroupsControllerTests + { + [Fact] + public async Task UpdateGroup_RejectsMismatchedGroupId_AndLeavesGroupUnchanged() + { + using var sqlLiteConnection = new SqliteConnection("Filename=:memory:"); + await sqlLiteConnection.OpenAsync(TestContext.Current.CancellationToken); + var sqlLiteContextOptions = new DbContextOptionsBuilder() + .UseSqlite(sqlLiteConnection) + .Options; + using var context = new RAPSContext(sqlLiteContextOptions); + await context.Database.EnsureCreatedAsync(TestContext.Current.CancellationToken); + + // arrange + var ouGroup = new OuGroup { Name = "OriginalName", Description = "Original description" }; + context.OuGroups.Add(ouGroup); + await context.SaveChangesAsync(TestContext.Current.CancellationToken); + + var adGroupsController = new AdGroupsController(context); + var mismatchedEdit = new GroupAddEdit + { + GroupId = ouGroup.OugroupId + 1, + Name = "ChangedName", + Description = "Changed description" + }; + + // act + var result = await adGroupsController.UpdateGroup(ouGroup.OugroupId, mismatchedEdit); + + // assert + Assert.IsType(result); + var unchangedGroup = await context.OuGroups.FindAsync(new object?[] { ouGroup.OugroupId }, TestContext.Current.CancellationToken); + Assert.NotNull(unchangedGroup); + Assert.Equal("OriginalName", unchangedGroup.Name); + Assert.Equal("Original description", unchangedGroup.Description); + } + } +} diff --git a/test/RAPS/RAPSControllerTests.cs b/test/RAPS/RAPSControllerTests.cs new file mode 100644 index 000000000..e9a6d0594 --- /dev/null +++ b/test/RAPS/RAPSControllerTests.cs @@ -0,0 +1,239 @@ +using System.Runtime.Versioning; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using NSubstitute; +using Viper.Areas.RAPS.Controllers; +using Viper.Classes.SQLContext; +using Viper.Models.RAPS; + +namespace Viper.test.RAPS +{ + [SupportedOSPlatform("windows")] + public class RAPSControllerTests + { + [Fact] + public async Task RoleMembers_ReturnsBadRequest_WhenModelBindingFailed() + { + await AssertBadRequestForInvalidModelStateAsync(c => c.RoleMembers("VIPER", 1)); + } + + [Fact] + public async Task RolePermissions_ReturnsBadRequest_WhenModelBindingFailed() + { + await AssertBadRequestForInvalidModelStateAsync(c => c.RolePermissions(1)); + } + + [Fact] + public async Task PermissionMembers_ReturnsBadRequest_WhenModelBindingFailed() + { + await AssertBadRequestForInvalidModelStateAsync(c => c.PermissionMembers(1)); + } + + [Fact] + public async Task PermissionRoles_ReturnsBadRequest_WhenModelBindingFailed() + { + await AssertBadRequestForInvalidModelStateAsync(c => c.PermissionRoles(1)); + } + + [Fact] + public async Task PermissionRolesRO_ReturnsBadRequest_WhenModelBindingFailed() + { + await AssertBadRequestForInvalidModelStateAsync(c => c.PermissionRolesRO(1)); + } + + [Fact] + public async Task AllMembersWithPermission_ReturnsBadRequest_WhenModelBindingFailed() + { + await AssertBadRequestForInvalidModelStateAsync(c => c.AllMembersWithPermission(1)); + } + + [Fact] + public async Task ExportToVMACS_ReturnsBadRequest_WhenModelBindingFailed() + { + await AssertBadRequestForInvalidModelStateAsync(c => c.ExportToVMACS("VMTH-test")); + } + + [Fact] + public async Task GroupSync_ReturnsBadRequest_WhenModelBindingFailed() + { + await AssertBadRequestForInvalidModelStateAsync(c => c.GroupSync(1)); + } + + [Fact] + public async Task RolePermissions_ReturnsView_ForValidRoleId() + { + using var connection = await OpenConnectionAsync(); + using var context = await CreateContextAsync(connection); + var controller = CreateController(context); + + var result = controller.RolePermissions(5); + + var view = Assert.IsType(result); + Assert.Equal(5, view.ViewData["roleId"]); + } + + [Fact] + public async Task PermissionMembers_ReturnsView_WhenPermissionExists() + { + using var connection = await OpenConnectionAsync(); + using var context = await CreateContextAsync(connection); + var permission = await SeedPermissionAsync(context); + var controller = CreateController(context); + + var result = await controller.PermissionMembers(permission.PermissionId); + + Assert.IsType(result); + } + + [Fact] + public async Task PermissionMembers_ReturnsNotFound_WhenPermissionMissing() + { + using var connection = await OpenConnectionAsync(); + using var context = await CreateContextAsync(connection); + var controller = CreateController(context); + + var result = await controller.PermissionMembers(9999); + + Assert.IsType(result); + } + + [Fact] + public async Task PermissionRoles_ReturnsView_WhenPermissionExists() + { + using var connection = await OpenConnectionAsync(); + using var context = await CreateContextAsync(connection); + var permission = await SeedPermissionAsync(context); + var controller = CreateController(context); + + var result = await controller.PermissionRoles(permission.PermissionId); + + Assert.IsType(result); + } + + [Fact] + public async Task PermissionRoles_ReturnsNotFound_WhenPermissionMissing() + { + using var connection = await OpenConnectionAsync(); + using var context = await CreateContextAsync(connection); + var controller = CreateController(context); + + var result = await controller.PermissionRoles(9999); + + Assert.IsType(result); + } + + [Fact] + public async Task PermissionRolesRO_ReturnsView_WhenPermissionExists() + { + using var connection = await OpenConnectionAsync(); + using var context = await CreateContextAsync(connection); + var permission = await SeedPermissionAsync(context); + var controller = CreateController(context); + + var result = await controller.PermissionRolesRO(permission.PermissionId); + + Assert.IsType(result); + } + + [Fact] + public async Task PermissionRolesRO_ReturnsNotFound_WhenPermissionMissing() + { + using var connection = await OpenConnectionAsync(); + using var context = await CreateContextAsync(connection); + var controller = CreateController(context); + + var result = await controller.PermissionRolesRO(9999); + + Assert.IsType(result); + } + + [Fact] + public async Task AllMembersWithPermission_ReturnsView_WhenPermissionExists() + { + using var connection = await OpenConnectionAsync(); + using var context = await CreateContextAsync(connection); + var permission = await SeedPermissionAsync(context); + var controller = CreateController(context); + + var result = await controller.AllMembersWithPermission(permission.PermissionId); + + Assert.IsType(result); + } + + [Fact] + public async Task AllMembersWithPermission_ReturnsNotFound_WhenPermissionMissing() + { + using var connection = await OpenConnectionAsync(); + using var context = await CreateContextAsync(connection); + var controller = CreateController(context); + + var result = await controller.AllMembersWithPermission(9999); + + Assert.IsType(result); + } + + [Fact] + public async Task GroupSync_RendersWithoutSyncing_WhenGroupMissing() + { + using var connection = await OpenConnectionAsync(); + using var context = await CreateContextAsync(connection); + var scopeFactory = Substitute.For(); + var controller = CreateController(context, scopeFactory); + + var result = await controller.GroupSync(9999); + + var view = Assert.IsType(result); + Assert.Null(view.ViewData["Group"]); + scopeFactory.DidNotReceive().CreateScope(); + } + + private static Task AssertBadRequestForInvalidModelStateAsync(Func action) + { + return AssertBadRequestForInvalidModelStateAsync(c => Task.FromResult(action(c))); + } + + private static async Task AssertBadRequestForInvalidModelStateAsync(Func> action) + { + using var connection = await OpenConnectionAsync(); + using var context = await CreateContextAsync(connection); + var controller = CreateController(context); + controller.ModelState.AddModelError("param", "The value is not valid."); + + var result = await action(controller); + + Assert.IsType(result); + } + + private static async Task OpenConnectionAsync() + { + var connection = new SqliteConnection("Filename=:memory:"); + await connection.OpenAsync(TestContext.Current.CancellationToken); + return connection; + } + + private static async Task CreateContextAsync(SqliteConnection connection) + { + var options = new DbContextOptionsBuilder() + .UseSqlite(connection) + .Options; + var context = new RAPSContext(options); + await context.Database.EnsureCreatedAsync(TestContext.Current.CancellationToken); + return context; + } + + private static RAPSController CreateController(RAPSContext context, IServiceScopeFactory? scopeFactory = null) + { + return new RAPSController(context, scopeFactory ?? Substitute.For()); + } + + private static async Task SeedPermissionAsync(RAPSContext context) + { + var permission = new TblPermission { Permission = "SVMSecure.Test" }; + context.TblPermissions.Add(permission); + await context.SaveChangesAsync(TestContext.Current.CancellationToken); + return permission; + } + } +} diff --git a/web/Areas/CMS/Controllers/CMSController.cs b/web/Areas/CMS/Controllers/CMSController.cs index e7703e074..030c532bb 100644 --- a/web/Areas/CMS/Controllers/CMSController.cs +++ b/web/Areas/CMS/Controllers/CMSController.cs @@ -12,12 +12,13 @@ public class CMSController : Controller private readonly IHtmlSanitizerService _sanitizerService; private readonly ILogger _cmsLogger; - public CMSController(RAPSContext rapsContext, VIPERContext viperContext, IHtmlSanitizerService sanitizerService, ILogger cmsLogger) + public CMSController(RAPSContext rapsContext, VIPERContext viperContext, IHtmlSanitizerService sanitizerService, ILoggerFactory loggerFactory) { _rapsContext = rapsContext; _viperContext = viperContext; _sanitizerService = sanitizerService; - _cmsLogger = cmsLogger; + //the logger belongs to Data.CMS (which does the logging), not this controller + _cmsLogger = loggerFactory.CreateLogger(); } [HttpGet] diff --git a/web/Areas/CTS/Controllers/CTSController.cs b/web/Areas/CTS/Controllers/CTSController.cs index 38410306a..ff77d9891 100644 --- a/web/Areas/CTS/Controllers/CTSController.cs +++ b/web/Areas/CTS/Controllers/CTSController.cs @@ -28,12 +28,14 @@ public CTSController(VIPERContext context, RAPSContext rapsContext, IWebHostEnvi /// /// /// +#pragma warning disable S6967 // filter override, not an action: returning BadRequest is impossible and checking ModelState here would blanket-validate every CTS action public override async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) { await base.OnActionExecutionAsync(context, next); ViewData["ViperLeftNav"] = Nav(); } +#pragma warning restore S6967 public NavMenu Nav() { diff --git a/web/Areas/Directory/Controllers/DirectoryController.cs b/web/Areas/Directory/Controllers/DirectoryController.cs index 12e587267..42bcda161 100644 --- a/web/Areas/Directory/Controllers/DirectoryController.cs +++ b/web/Areas/Directory/Controllers/DirectoryController.cs @@ -13,6 +13,7 @@ namespace Viper.Areas.Directory.Controllers { [Area("Directory")] + [Route("/[area]")] [Permission(Allow = "SVMSecure")] [Authorize(Roles = "VMDO SVM-IT")] //locking directory for now until it's complete public class DirectoryController : AreaController @@ -31,20 +32,20 @@ public DirectoryController(AAUDContext aaud, RAPSContext rapsContext) /// /// Directory home page /// - [Route("/[area]/")] - public async Task Index(string? useExample) + [Route("")] + public ActionResult Index(string? useExample) { - return await Task.Run(() => View("~/Areas/Directory/Views/Card.cshtml")); + return View("~/Areas/Directory/Views/Card.cshtml"); } /// /// Directory home page /// - [Route("/[area]/nav")] - public async Task>> Nav() + [Route("nav")] + public ActionResult> Nav() { var nav = new List(); - return await Task.Run(() => nav); + return nav; } @@ -53,27 +54,14 @@ public async Task>> Nav() /// /// search string [SupportedOSPlatform("windows")] - [Route("/[area]/search/{search}")] + [Route("search/{search}")] public async Task>> Get(string search) { - var individuals = await _aaud.AaudUsers - .Where(u => (u.DisplayFirstName + " " + u.DisplayLastName).Contains(search) - || (u.MailId != null && u.MailId.Contains(search)) - || (u.LoginId != null && u.LoginId.Contains(search)) - || (u.SpridenId != null && u.SpridenId.Contains(search)) - || (u.Pidm != null && u.Pidm.Contains(search)) - || (u.MothraId != null && u.MothraId.Contains(search)) - || (u.EmployeeId != null && u.EmployeeId.Contains(search)) - || (u.IamId != null && u.IamId.Contains(search)) - ) - .Where(u => u.Current != 0) - .OrderBy(u => u.DisplayLastName) - .ThenBy(u => u.DisplayFirstName) - .ToListAsync(); + var individuals = await SearchCurrentAaudUsers(_aaud, search); List results = new(); AaudUser? currentUser = UserHelper.GetCurrentUser(); bool hasDetailPermission = UserHelper.HasPermission(_rapsContext, currentUser, "SVMSecure.DirectoryDetail"); - individuals.ForEach(m => + foreach (var m in individuals) { LdapUserContact? l = LdapService.GetUserByID(m.IamId); var result = hasDetailPermission @@ -81,14 +69,8 @@ public async Task>> Get(string : new IndividualSearchResult(m, l); result.LookupEmailHost(_aaud); results.Add(result); - - var vmsearch = VMACSService.Search(result.LoginId); - var vm = vmsearch.Result; - if (vm != null && vm.item != null && vm.item.Nextel != null) result.Nextel = vm.item.Nextel[0]; - if (vm != null && vm.item != null && vm.item.LDPager != null) result.LDPager = vm.item.LDPager[0]; - if (vm != null && vm.item != null && vm.item.Unit != null) result.Department = vm.item.Unit[0]; - - }); + await AddVmacsContactInfoAsync(result); + } return results; } @@ -97,41 +79,24 @@ public async Task>> Get(string /// /// search string [SupportedOSPlatform("windows")] - [Route("/[area]/search/{search}/ucd")] + [Route("search/{search}/ucd")] public async Task>> GetUCD(string search) { List results = new(); List ldap = LdapService.GetUsersContact(search); - var individuals = await _aaud.AaudUsers - .Where(u => (u.DisplayFirstName + " " + u.DisplayLastName).Contains(search) - || (u.MailId != null && u.MailId.Contains(search)) - || (u.LoginId != null && u.LoginId.Contains(search)) - || (u.SpridenId != null && u.SpridenId.Contains(search)) - || (u.Pidm != null && u.Pidm.Contains(search)) - || (u.MothraId != null && u.MothraId.Contains(search)) - || (u.EmployeeId != null && u.EmployeeId.Contains(search)) - || (u.IamId != null && u.IamId.Contains(search)) - ) - .Where(u => u.Current != 0) - .OrderBy(u => u.DisplayLastName) - .ThenBy(u => u.DisplayFirstName) - .ToListAsync(); + var individuals = await SearchCurrentAaudUsers(_aaud, search); + var individualsByIamId = individuals.ToLookup(m => m.IamId); AaudUser? currentUser = UserHelper.GetCurrentUser(); bool hasDetailPermission = UserHelper.HasPermission(_rapsContext, currentUser, "SVMSecure.DirectoryDetail"); foreach (var l in ldap) { - AaudUser? userInfo = individuals.Find(m => m.IamId == l.IamId); + AaudUser? userInfo = individualsByIamId[l.IamId].FirstOrDefault(); var result = hasDetailPermission ? new IndividualSearchResultWithIDs(userInfo, l) : new IndividualSearchResult(userInfo, l); result.LookupEmailHost(_aaud); results.Add(result); - - var vmsearch = VMACSService.Search(result.LoginId); - var vm = vmsearch.Result; - if (vm != null && vm.item != null && vm.item.Nextel != null) result.Nextel = vm.item.Nextel[0]; - if (vm != null && vm.item != null && vm.item.LDPager != null) result.LDPager = vm.item.LDPager[0]; - if (vm != null && vm.item != null && vm.item.Unit != null) result.Department = vm.item.Unit[0]; + await AddVmacsContactInfoAsync(result); } return results; @@ -141,11 +106,50 @@ public async Task>> GetUCD(stri /// Directory results /// /// User ID - [Route("/[area]/userInfo/{mothraID}")] - public async Task DirectoryResult(string mothraID) + [Route("userInfo/{mothraID}")] + public IActionResult DirectoryResult(string mothraID) { // pull in the user based on uid - return await Task.Run(() => View("~/Areas/Directory/Views/UserInfo.cshtml")); + return View("~/Areas/Directory/Views/UserInfo.cshtml"); + } + + /// + /// Current AAUD users matching the search term on name or any directory identifier, + /// ordered for display. Shared by Get and GetUCD. + /// + internal static Task> SearchCurrentAaudUsers(AAUDContext aaud, string search) + { + return aaud.AaudUsers + .AsNoTracking() + .Where(u => (u.DisplayFirstName + " " + u.DisplayLastName).Contains(search) + || new[] { u.MailId, u.LoginId, u.SpridenId, u.Pidm, u.MothraId, u.EmployeeId, u.IamId } + .Any(id => id != null && id.Contains(search))) + .Where(u => u.Current != 0) + .OrderBy(u => u.DisplayLastName) + .ThenBy(u => u.DisplayFirstName) + .ToListAsync(); + } + + /// + /// Add VMACS phone/pager/department info to a search result when the lookup finds a match. + /// + private static async Task AddVmacsContactInfoAsync(IndividualSearchResult result) + { + // Without a login ID the VMACS query would run with an empty find value; + // skip the pointless lookup. Empty element lists deserialize as empty + // arrays (not null), so guard on length before indexing. + if (string.IsNullOrWhiteSpace(result.LoginId)) + { + return; + } + var item = (await VMACSService.Search(result.LoginId))?.item; + if (item == null) + { + return; + } + if (item.Nextel is { Length: > 0 }) result.Nextel = item.Nextel[0]; + if (item.LDPager is { Length: > 0 }) result.LDPager = item.LDPager[0]; + if (item.Unit is { Length: > 0 }) result.Department = item.Unit[0]; } } } diff --git a/web/Areas/RAPS/Controllers/AdGroupsController.cs b/web/Areas/RAPS/Controllers/AdGroupsController.cs index 106d9dd03..5b32cf43c 100644 --- a/web/Areas/RAPS/Controllers/AdGroupsController.cs +++ b/web/Areas/RAPS/Controllers/AdGroupsController.cs @@ -81,7 +81,7 @@ public async Task UpdateGroup(int groupId, GroupAddEdit group) { if (groupId != group.GroupId) { - BadRequest(); + return BadRequest(); } OuGroup? ouGroup = await _context.OuGroups.FindAsync(groupId); diff --git a/web/Areas/RAPS/Controllers/RAPSController.cs b/web/Areas/RAPS/Controllers/RAPSController.cs index 8deda2fe0..0b5912ba1 100644 --- a/web/Areas/RAPS/Controllers/RAPSController.cs +++ b/web/Areas/RAPS/Controllers/RAPSController.cs @@ -4,6 +4,7 @@ using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.EntityFrameworkCore; using Microsoft.IdentityModel.Tokens; +using NLog; using Viper.Areas.RAPS.Services; using Viper.Classes; using Viper.Classes.SQLContext; @@ -19,27 +20,31 @@ public class RAPSController : AreaController { private readonly RAPSContext _RAPSContext; private readonly RAPSSecurityService _securityService; + private readonly IServiceScopeFactory _scopeFactory; public IUserHelper UserHelper { get; private set; } public int Count { get; set; } public string? UserName { get; set; } - public RAPSController(RAPSContext context) + public RAPSController(RAPSContext context, IServiceScopeFactory scopeFactory) { _RAPSContext = context; _securityService = new RAPSSecurityService(context); UserHelper = new UserHelper(); + _scopeFactory = scopeFactory; } /// /// Getting left nav for each page. This is a little complicated - alternatively, ViewData["ViperLeftNav"] = await Nav() /// could be added to each action. /// +#pragma warning disable S6967, S6932 // filter override, not an action: returning BadRequest is impossible and checking ModelState here would + // blanket-validate every RAPS action; the raw query-string values read here only feed left-nav context + // (never authorization), are TryParse-guarded, and model binding is not available in a filter public override async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) { await base.OnActionExecutionAsync(context, next); - await next(); bool roleIdValid = int.TryParse(HttpContext?.Request?.Query["roleId"].FirstOrDefault(), out int roleId); bool permIdValid = int.TryParse(HttpContext?.Request?.Query["permissionId"].FirstOrDefault(), out int permissionId); string? memberId = HttpContext?.Request?.Query["memberId"].FirstOrDefault(); @@ -64,32 +69,34 @@ public override async Task OnActionExecutionAsync(ActionExecutingContext context instance, page); } +#pragma warning restore S6967, S6932 /// /// RAPS home page /// [Route("/[area]/{instance?}")] - public async Task Index(string? instance) + public ActionResult Index(string? instance) { ViewData["KeyColumnName"] = "RoleId"; instance ??= _securityService.GetDefaultInstanceForUser(); return instance.ToUpper() switch { - "VIPER" => await Task.Run(() => Redirect("~/raps/VIPER/rolelist")), - "VMACS.VMTH" => await Task.Run(() => Redirect("~/raps/VMACS.VMTH/rolelist")), - "VMACS.VMLF" => await Task.Run(() => Redirect("~/raps/VMACS.VMLF/rolelist")), - "VMACS.UCVMCSD" => await Task.Run(() => Redirect("~/raps/VMACS.UCVMCSD/rolelist")), - "VIPERFORMS" => await Task.Run(() => Redirect("~/raps/ViperForms/rolelist")), - _ => await Task.Run(() => View("~/Views/Home/403.cshtml")), + "VIPER" => Redirect("~/raps/VIPER/rolelist"), + "VMACS.VMTH" => Redirect("~/raps/VMACS.VMTH/rolelist"), + "VMACS.VMLF" => Redirect("~/raps/VMACS.VMLF/rolelist"), + "VMACS.UCVMCSD" => Redirect("~/raps/VMACS.UCVMCSD/rolelist"), + "VIPERFORMS" => Redirect("~/raps/ViperForms/rolelist"), + _ => View("~/Views/Home/403.cshtml"), }; } + [NonAction] public async Task Nav(int? roleId, int? permissionId, string? memberId, string instance = "VIPER", string page = "") { TblRole? selectedRole = (roleId != null) ? await _RAPSContext.TblRoles.FindAsync(roleId) : null; TblPermission? selectedPermission = (permissionId != null) ? await _RAPSContext.TblPermissions.FindAsync(permissionId) : null; - VwAaudUser? selecteduser = (memberId != null) ? await _RAPSContext.VwAaudUser.SingleAsync(r => r.MothraId == memberId) : null; + VwAaudUser? selecteduser = (memberId != null) ? await _RAPSContext.VwAaudUser.AsNoTracking().SingleOrDefaultAsync(r => r.MothraId == memberId) : null; var nav = new List { @@ -213,21 +220,21 @@ public async Task Nav(int? roleId, int? permissionId, string? memberId, /// /// RAPS Instance [Route("/[area]/{instance}/[action]")] - public async Task RoleList(string instance) + public IActionResult RoleList(string instance) { if (UserHelper.HasPermission(_RAPSContext, UserHelper.GetCurrentUser(), "RAPS.Admin")) { - return await Task.Run(() => View("~/Areas/RAPS/Views/Roles/ListAdmin.cshtml")); + return View("~/Areas/RAPS/Views/Roles/ListAdmin.cshtml"); } if (_securityService.IsAllowedTo("ViewAllRoles", instance) || !_securityService.GetControlledRoleIds(UserHelper.GetCurrentUser()?.MothraId).IsNullOrEmpty()) { - return await Task.Run(() => View("~/Areas/RAPS/Views/Roles/List.cshtml")); + return View("~/Areas/RAPS/Views/Roles/List.cshtml"); } //TODO: Should probably have a deny access helper function that writes logs and sets view - return await Task.Run(() => View("~/Views/Home/403.cshtml")); + return View("~/Views/Home/403.cshtml"); } /// @@ -235,15 +242,15 @@ public async Task RoleList(string instance) /// [Route("/[area]/{instance}/[action]")] [Permission(Allow = "RAPS.Admin,RAPS.ViewRoles")] - public async Task RoleTemplateList(string instance) + public IActionResult RoleTemplateList(string instance) { if (!_securityService.IsAllowedTo("ViewRoles", instance)) { - return await Task.Run(() => View("~/Views/Home/403.cshtml")); + return View("~/Views/Home/403.cshtml"); } ViewData["canEditRoleTemplates"] = _securityService.IsAllowedTo("EditRoleTemplates", instance); ViewData["canApplyTemplates"] = _securityService.IsAllowedTo("EditRoleMembership", instance); - return await Task.Run(() => View("~/Areas/RAPS/Views/Roles/Templates.cshtml")); + return View("~/Areas/RAPS/Views/Roles/Templates.cshtml"); } /// @@ -251,13 +258,13 @@ public async Task RoleTemplateList(string instance) /// [Route("/[area]/{instance}/[action]")] [Permission(Allow = "RAPS.Admin,RAPS.EditRoleMembership")] - public async Task RoleTemplateApply(string instance) + public IActionResult RoleTemplateApply(string instance) { if (!_securityService.IsAllowedTo("EditRoleMembership", instance)) { - return await Task.Run(() => View("~/Views/Home/403.cshtml")); + return View("~/Views/Home/403.cshtml"); } - return await Task.Run(() => View("~/Areas/RAPS/Views/Roles/ApplyTemplate.cshtml")); + return View("~/Areas/RAPS/Views/Roles/ApplyTemplate.cshtml"); } /// @@ -265,13 +272,13 @@ public async Task RoleTemplateApply(string instance) /// [Route("/[area]/{instance}/[action]")] [Permission(Allow = "RAPS.Admin,RAPS.EditRoles")] - public async Task RoleTemplateRoles(string instance) + public IActionResult RoleTemplateRoles(string instance) { if (!_securityService.IsAllowedTo("EditRoleTemplates", instance)) { - return await Task.Run(() => View("~/Views/Home/403.cshtml")); + return View("~/Views/Home/403.cshtml"); } - return await Task.Run(() => View("~/Areas/RAPS/Views/Roles/TemplateRoles.cshtml")); + return View("~/Areas/RAPS/Views/Roles/TemplateRoles.cshtml"); } /// @@ -279,10 +286,10 @@ public async Task RoleTemplateRoles(string instance) /// [Permission(Allow = "RAPS.Admin")] [Route("/[area]/{instance}/DelegateRoles")] - public async Task DelegateRoles() + public IActionResult DelegateRoles() { - return await Task.Run(() => View("~/Areas/RAPS/Views/Roles/DelegateRoles.cshtml")); + return View("~/Areas/RAPS/Views/Roles/DelegateRoles.cshtml"); } /// @@ -292,6 +299,10 @@ public async Task DelegateRoles() [Route("/[area]/{instance}/[action]")] public async Task RoleMembers(string instance, int RoleId) { + if (!ModelState.IsValid) + { + return BadRequest(); + } ViewData["RoleId"] = RoleId; ViewData["canEditPermissions"] = _securityService.IsAllowedTo("EditMemberPermissions", instance); @@ -315,12 +326,11 @@ public async Task RoleMembers(string instance, int RoleId) /// [Permission(Allow = "RAPS.Admin,RAPS.ViewPermissions")] [Route("/[area]/{Instance}/[action]")] - public async Task PermissionList() + public IActionResult PermissionList() { - return await Task.Run(() => - UserHelper.HasPermission(_RAPSContext, UserHelper.GetCurrentUser(), "RAPS.Admin") - ? View("~/Areas/RAPS/Views/Permissions/ListAdmin.cshtml") - : View("~/Areas/RAPS/Views/Permissions/List.cshtml")); + return UserHelper.HasPermission(_RAPSContext, UserHelper.GetCurrentUser(), "RAPS.Admin") + ? View("~/Areas/RAPS/Views/Permissions/ListAdmin.cshtml") + : View("~/Areas/RAPS/Views/Permissions/List.cshtml"); } /// @@ -328,24 +338,28 @@ public async Task PermissionList() /// [Permission(Allow = "RAPS.Admin,RAPS.ManageAllPermissions")] [Route("/[area]/{Instance}/[action]")] - public async Task RolePermissions(int roleId) + public IActionResult RolePermissions(int roleId) { + if (!ModelState.IsValid) + { + return BadRequest(); + } ViewData["roleId"] = roleId; - return await Task.Run(() => View("~/Areas/RAPS/Views/Roles/Permissions.cshtml")); + return View("~/Areas/RAPS/Views/Roles/Permissions.cshtml"); } /// /// Compare permissions for two roles /// [Route("/[area]/{Instance}/[action]")] - public async Task RolePermissionsComparison(string instance) + public IActionResult RolePermissionsComparison(string instance) { if (_securityService.IsAllowedTo("EditRoleMembership", instance)) { - return await Task.Run(() => View("~/Areas/RAPS/Views/Roles/PermissionComparison.cshtml")); + return View("~/Areas/RAPS/Views/Roles/PermissionComparison.cshtml"); } - return await Task.Run(() => View("~/Views/Home/403.cshtml")); + return View("~/Views/Home/403.cshtml"); } /// @@ -355,6 +369,10 @@ public async Task RolePermissionsComparison(string instance) [Route("/[area]/{Instance}/[action]")] public async Task PermissionMembers(int? permissionId) { + if (!ModelState.IsValid) + { + return BadRequest(); + } ViewData["permissionId"] = permissionId; TblPermission? permission = await _RAPSContext.TblPermissions.FindAsync(permissionId); @@ -363,7 +381,7 @@ public async Task PermissionMembers(int? permissionId) { return NotFound(); } - return await Task.Run(() => View("~/Areas/RAPS/Views/Permissions/Members.cshtml")); + return View("~/Areas/RAPS/Views/Permissions/Members.cshtml"); } /// @@ -373,6 +391,10 @@ public async Task PermissionMembers(int? permissionId) [Route("/[area]/{Instance}/[action]")] public async Task PermissionRoles(int? permissionId) { + if (!ModelState.IsValid) + { + return BadRequest(); + } ViewData["permissionId"] = permissionId; TblPermission? permission = await _RAPSContext.TblPermissions.FindAsync(permissionId); @@ -381,13 +403,17 @@ public async Task PermissionRoles(int? permissionId) { return NotFound(); } - return await Task.Run(() => View("~/Areas/RAPS/Views/Permissions/Roles.cshtml")); + return View("~/Areas/RAPS/Views/Permissions/Roles.cshtml"); } [Permission(Allow = "RAPS.Admin,RAPS.ViewPermissions")] [Route("/[area]/{Instance}/[action]")] public async Task PermissionRolesRO(int? permissionId) { + if (!ModelState.IsValid) + { + return BadRequest(); + } ViewData["permissionId"] = permissionId; TblPermission? permission = await _RAPSContext.TblPermissions.FindAsync(permissionId); @@ -396,13 +422,17 @@ public async Task PermissionRolesRO(int? permissionId) { return NotFound(); } - return await Task.Run(() => View("~/Areas/RAPS/Views/Permissions/RolesRO.cshtml")); + return View("~/Areas/RAPS/Views/Permissions/RolesRO.cshtml"); } [Permission(Allow = "RAPS.Admin,RAPS.ViewPermissions")] [Route("/[area]/{Instance}/[action]")] public async Task AllMembersWithPermission(int? permissionId) { + if (!ModelState.IsValid) + { + return BadRequest(); + } ViewData["permissionId"] = permissionId; TblPermission? permission = await _RAPSContext.TblPermissions.FindAsync(permissionId); @@ -411,7 +441,7 @@ public async Task AllMembersWithPermission(int? permissionId) { return NotFound(); } - return await Task.Run(() => View("~/Areas/RAPS/Views/Permissions/AllMembers.cshtml")); + return View("~/Areas/RAPS/Views/Permissions/AllMembers.cshtml"); } /** @@ -423,13 +453,13 @@ public async Task AllMembersWithPermission(int? permissionId) /// [Permission(Allow = "RAPS.Admin,RAPS.UserLookup")] [Route("/[area]/{Instance}/[action]")] - public async Task UserSearch(string instance) + public IActionResult UserSearch(string instance) { ViewData["canRSOP"] = _securityService.IsAllowedTo("RSOP", instance); ViewData["canEditRoleMembership"] = _securityService.IsAllowedTo("EditRoleMembership", instance); ViewData["canEditMemberPermissions"] = _securityService.IsAllowedTo("EditMemberPermissions", instance); ViewData["canViewHistory"] = _securityService.IsAllowedTo("ViewHistory", instance); - return await Task.Run(() => View("~/Areas/RAPS/Views/Members/List.cshtml")); + return View("~/Areas/RAPS/Views/Members/List.cshtml"); } /// @@ -437,7 +467,7 @@ public async Task UserSearch(string instance) /// [Permission(Allow = "RAPS.Admin,RAPS.EditRoleMembership")] [Route("/[area]/{Instance}/[action]")] - public async Task MemberRoles(string instance) + public IActionResult MemberRoles(string instance) { ViewData["canEditPermissions"] = _securityService.IsAllowedTo("ManageAllPermissions", instance); //EditRoleMembership grants access only to the VMACS instance @@ -446,7 +476,7 @@ public async Task MemberRoles(string instance) //TODO: Should probably have a deny access helper function that writes logs and sets view return View("~/Views/Home/403.cshtml"); } - return await Task.Run(() => View("~/Areas/RAPS/Views/Members/Roles.cshtml")); + return View("~/Areas/RAPS/Views/Members/Roles.cshtml"); } /// @@ -454,9 +484,9 @@ public async Task MemberRoles(string instance) /// [Permission(Allow = "RAPS.Admin,RAPS.EditMemberPermissions")] [Route("/[area]/{Instance}/[action]")] - public async Task MemberPermissions() + public IActionResult MemberPermissions() { - return await Task.Run(() => View("~/Areas/RAPS/Views/Members/Permissions.cshtml")); + return View("~/Areas/RAPS/Views/Members/Permissions.cshtml"); } /// @@ -464,7 +494,7 @@ public async Task MemberPermissions() /// [Permission(Allow = "RAPS.Admin,RAPS.RSOP")] [Route("/[area]/{Instance}/[action]")] - public async Task RSOP(string instance) + public IActionResult RSOP(string instance) { //RSOP grants access only to the VMACS instance if (!_securityService.IsAllowedTo("RSOP", instance)) @@ -472,7 +502,7 @@ public async Task RSOP(string instance) //TODO: Should probably have a deny access helper function that writes logs and sets view return View("~/Views/Home/403.cshtml"); } - return await Task.Run(() => View("~/Areas/RAPS/Views/Members/RSOP.cshtml")); + return View("~/Areas/RAPS/Views/Members/RSOP.cshtml"); } /// @@ -480,7 +510,7 @@ public async Task RSOP(string instance) /// [Permission(Allow = "RAPS.Admin,RAPS.EditRoleMembership")] [Route("/[area]/{Instance}/[action]")] - public async Task MemberHistory(string instance) + public IActionResult MemberHistory(string instance) { //EditRoleMembership grants access only to the VMACS instance if (!_securityService.IsAllowedTo("ViewHistory", instance)) @@ -488,24 +518,28 @@ public async Task MemberHistory(string instance) //TODO: Should probably have a deny access helper function that writes logs and sets view return View("~/Views/Home/403.cshtml"); } - return await Task.Run(() => View("~/Areas/RAPS/Views/Members/History.cshtml")); + return View("~/Areas/RAPS/Views/Members/History.cshtml"); } [Permission(Allow = "RAPS.Admin,RAPS.Clone")] [Route("/[area]/{Instance}/[action]")] - public async Task UserClone(string instance) + public IActionResult UserClone(string instance) { if (!_securityService.IsAllowedTo("Clone", instance)) { return View("~/Views/Home/403.cshtml"); } - return await Task.Run(() => View("~/Areas/RAPS/Views/Members/Clone.cshtml")); + return View("~/Areas/RAPS/Views/Members/Clone.cshtml"); } [Permission(Allow = "RAPS.Admin")] [Route("/[area]/{Instance}/[action]")] public async Task ExportToVMACS(string? server = null, string? loginId = null, bool? debugOnly = false) { + if (!ModelState.IsValid) + { + return BadRequest(); + } var vmacsExport = new VMACSExport(_RAPSContext); var servers = vmacsExport.GetServers(); if (server != null && servers.Contains(server)) @@ -519,7 +553,7 @@ public async Task ExportToVMACS(string? server = null, string? lo ViewData["Servers"] = servers; } - return await Task.Run(() => View("~/Areas/RAPS/Views/Export.cshtml")); + return View("~/Areas/RAPS/Views/Export.cshtml"); } [Permission(Allow = "RAPS.Admin")] @@ -528,28 +562,28 @@ public async Task RoleViewUpdate() { ViewData["Messages"] = await new RoleViews(_RAPSContext) .UpdateRoles(debugOnly: true); - return await Task.Run(() => View("~/Areas/RAPS/Views/RoleViewUpdate.cshtml")); + return View("~/Areas/RAPS/Views/RoleViewUpdate.cshtml"); } [Permission(Allow = "RAPS.Admin,RAPS.OUGroupsView")] [Route("/[area]/{Instance}/[action]")] - public async Task GroupList() + public IActionResult GroupList() { - return await Task.Run(() => View("~/Areas/RAPS/Views/Groups/List.cshtml")); + return View("~/Areas/RAPS/Views/Groups/List.cshtml"); } [Permission(Allow = "RAPS.Admin,RAPS.OUGroupsView")] [Route("/[area]/{Instance}/[action]")] - public async Task GroupRoles() + public IActionResult GroupRoles() { - return await Task.Run(() => View("~/Areas/RAPS/Views/Groups/Roles.cshtml")); + return View("~/Areas/RAPS/Views/Groups/Roles.cshtml"); } [Permission(Allow = "RAPS.Admin,RAPS.OUGroupsView")] [Route("/[area]/{Instance}/[action]")] - public async Task GroupMembers() + public IActionResult GroupMembers() { - return await Task.Run(() => View("~/Areas/RAPS/Views/Groups/Members.cshtml")); + return View("~/Areas/RAPS/Views/Groups/Members.cshtml"); } [Permission(Allow = "RAPS.Admin,RAPS.OUGroupsView")] @@ -557,28 +591,55 @@ public async Task GroupMembers() [SupportedOSPlatform("windows")] public async Task GroupSync(int groupId) { + if (!ModelState.IsValid) + { + return BadRequest(); + } OuGroup? group = await _RAPSContext.OuGroups.FindAsync(groupId); if (group != null) { - _ = new OuGroupService(_RAPSContext).Sync(groupId, group.Name); + _ = SyncGroupInBackground(groupId, group.Name); } ViewData["Group"] = group; - return await Task.Run(() => View("~/Areas/RAPS/Views/Groups/Sync.cshtml")); + return View("~/Areas/RAPS/Views/Groups/Sync.cshtml"); + } + + /// + /// Run the AD/OU group sync outside the request scope so it can keep running after the response + /// is returned (the sync page tells users it may take a few minutes). Resolves its own RAPSContext + /// from a fresh DI scope, since the request-scoped _RAPSContext is disposed once the request ends. + /// + [SupportedOSPlatform("windows")] + [NonAction] + public async Task SyncGroupInBackground(int groupId, string groupName) + { + try + { + using var scope = _scopeFactory.CreateScope(); + var context = scope.ServiceProvider.GetRequiredService(); + await new OuGroupService(context).Sync(groupId, groupName); + } + catch (Exception ex) + { + // Background-job entry point: the task is discarded, so anything not caught + // here becomes an unobserved exception and the sync fails with no log entry. + LogManager.GetCurrentClassLogger().Error(ex, "Group sync failed for group {GroupId}", groupId); + } } [Permission(Allow = "RAPS.Admin,RAPS.OUGroupsView")] [Route("/[area]/{Instance}/[action]")] - public async Task CreateADGroup() + public IActionResult CreateADGroup() { - return await Task.Run(() => View("~/Areas/RAPS/Views/Groups/CreateADGroup.cshtml")); + return View("~/Areas/RAPS/Views/Groups/CreateADGroup.cshtml"); } [Permission(Allow = "RAPS.ViewAuditTrail")] [Route("/[area]/{Instance}/[action]")] - public async Task AuditTrail() + public IActionResult AuditTrail() { - return await Task.Run(() => View("~/Areas/RAPS/Views/AuditLog.cshtml")); + return View("~/Areas/RAPS/Views/AuditLog.cshtml"); } } } diff --git a/web/Areas/RAPS/Models/GroupAddEdit.cs b/web/Areas/RAPS/Models/GroupAddEdit.cs index c6506c2d1..73be4ccfd 100644 --- a/web/Areas/RAPS/Models/GroupAddEdit.cs +++ b/web/Areas/RAPS/Models/GroupAddEdit.cs @@ -1,12 +1,12 @@ namespace Viper.Areas.RAPS.Models { /// - /// DTO for creating/editing groups. GroupId is 0 for new groups. + /// DTO for creating/editing groups. GroupId is null for new groups. /// This class is also a base class for Group which sets GroupId in constructor. /// public class GroupAddEdit { - public int GroupId { get; set; } + public int? GroupId { get; set; } public string Name { get; set; } = null!; diff --git a/web/Areas/Students/Services/PhotoService.cs b/web/Areas/Students/Services/PhotoService.cs index 93ef0788e..3b1db7149 100644 --- a/web/Areas/Students/Services/PhotoService.cs +++ b/web/Areas/Students/Services/PhotoService.cs @@ -118,6 +118,8 @@ public async Task StudentPhotoExistsAsync(string mailId) return false; } + // Legitimate Task.Run: File.Exists is blocking I/O (photo store may be a + // network share) and callers fan these checks out with Task.WhenAll. return await Task.Run(() => { var photoPath = GetPhotoPath(mailId); diff --git a/web/Classes/HealthChecks/LdapHealthCheck.cs b/web/Classes/HealthChecks/LdapHealthCheck.cs index 71dd5dd9d..82be4d52d 100644 --- a/web/Classes/HealthChecks/LdapHealthCheck.cs +++ b/web/Classes/HealthChecks/LdapHealthCheck.cs @@ -32,6 +32,8 @@ public async Task CheckHealthAsync( try { + // Legitimate Task.Run: S.DS.Protocols has no async API, so the + // blocking LDAP bind is offloaded to keep the caller responsive. await Task.Run(() => { var ldapIdentifier = new LdapDirectoryIdentifier(_ldapServer, _ldapSSLPort); diff --git a/web/Views/Shared/Components/CMSBlocks/CMSBlocks.cs b/web/Views/Shared/Components/CMSBlocks/CMSBlocks.cs index aa2c10d97..6ec9c3a64 100644 --- a/web/Views/Shared/Components/CMSBlocks/CMSBlocks.cs +++ b/web/Views/Shared/Components/CMSBlocks/CMSBlocks.cs @@ -16,11 +16,11 @@ public CMSBlocksViewComponent(VIPERContext viperContext, RAPSContext rapsContext CMS = new CMS(viperContext, rapsContext, sanitizerService); } - public async Task InvokeAsync(int? contentBlockID, string? friendlyName, string? system, string? viperSectionPath, string? page, int? blockOrder, bool? allowPublicAccess, int? status) + public IViewComponentResult Invoke(int? contentBlockID, string? friendlyName, string? system, string? viperSectionPath, string? page, int? blockOrder, bool? allowPublicAccess, int? status) { List? blocks = CMS.GetContentBlocksAllowed(contentBlockID, friendlyName, system, viperSectionPath, page, blockOrder, allowPublicAccess, status)?.ToList(); - return await Task.Run(() => View("Default", blocks)); + return View("Default", blocks); } } diff --git a/web/Views/Shared/Components/EmulationBanner/EmulationBanner.cs b/web/Views/Shared/Components/EmulationBanner/EmulationBanner.cs index 4f4c7e18a..aa328c275 100644 --- a/web/Views/Shared/Components/EmulationBanner/EmulationBanner.cs +++ b/web/Views/Shared/Components/EmulationBanner/EmulationBanner.cs @@ -5,16 +5,16 @@ namespace Viper.Views.Shared.Components.EmulationBanner [ViewComponent(Name = "EmulationBanner")] public class EmulationBannerViewComponent : ViewComponent { - public async Task InvokeAsync() + public IViewComponentResult Invoke() { IUserHelper UserHelper = new UserHelper(); if (!UserHelper.IsEmulating()) { - return await Task.Run(() => (IViewComponentResult)Content(string.Empty)); + return Content(string.Empty); } string? displayFullName = UserHelper.GetCurrentUser()?.DisplayFullName; - return await Task.Run(() => View("Default", displayFullName)); + return View("Default", displayFullName); } } } diff --git a/web/Views/Shared/Components/LeftNav/LeftNav.cs b/web/Views/Shared/Components/LeftNav/LeftNav.cs index bf06ee092..af1b1567a 100644 --- a/web/Views/Shared/Components/LeftNav/LeftNav.cs +++ b/web/Views/Shared/Components/LeftNav/LeftNav.cs @@ -6,12 +6,12 @@ namespace Viper.Views.Shared.Components.LeftNav [ViewComponent(Name = "LeftNav")] public class LeftNavViewComponent : ViewComponent { - public async Task InvokeAsync(AaudUser user, int nav) + public IViewComponentResult Invoke(AaudUser user, int nav) { - return await Task.Run(() => View("Default", user)); + return View("Default", user); } } diff --git a/web/Views/Shared/Components/MainNav/MainNav.cs b/web/Views/Shared/Components/MainNav/MainNav.cs index 09bbafd15..b9060889d 100644 --- a/web/Views/Shared/Components/MainNav/MainNav.cs +++ b/web/Views/Shared/Components/MainNav/MainNav.cs @@ -40,7 +40,7 @@ public MainNavViewComponent(RAPSContext context) _context = context; } - public async Task InvokeAsync(AaudUser user) + public IViewComponentResult Invoke(AaudUser user) { ViewData["OldViperURL"] = oldViperURL; var userHelper = new UserHelper(); @@ -67,7 +67,7 @@ public async Task InvokeAsync(AaudUser user) "scheduler" => "Computing", _ => "VIPER Home", }; - return await Task.Run(() => View("Default", user)); + return View("Default", user); } } diff --git a/web/Views/Shared/Components/MiniNav/MiniNav.cs b/web/Views/Shared/Components/MiniNav/MiniNav.cs index 881e52696..e18c216a7 100644 --- a/web/Views/Shared/Components/MiniNav/MiniNav.cs +++ b/web/Views/Shared/Components/MiniNav/MiniNav.cs @@ -8,10 +8,10 @@ public class MiniNavViewComponent : ViewComponent { private readonly string oldViperURL = HttpHelper.GetOldViperRootURL(); - public async Task InvokeAsync(AaudUser user) + public IViewComponentResult Invoke(AaudUser user) { ViewData["OldViperURL"] = oldViperURL; - return await Task.Run(() => View("Default", user)); + return View("Default", user); } } diff --git a/web/Views/Shared/Components/ProfilePic/ProfilePic.cs b/web/Views/Shared/Components/ProfilePic/ProfilePic.cs index 806a0c51e..782f77c93 100644 --- a/web/Views/Shared/Components/ProfilePic/ProfilePic.cs +++ b/web/Views/Shared/Components/ProfilePic/ProfilePic.cs @@ -14,12 +14,12 @@ public ProfilePicViewComponent(AAUDContext context) _AAUDContext = context; } - public async Task InvokeAsync(string? userName) + public IViewComponentResult Invoke(string? userName) { IUserHelper UserHelper = new UserHelper(); AaudUser? user = UserHelper.GetByLoginId(_AAUDContext, userName); - return await Task.Run(() => View("Default", user)); + return View("Default", user); } } diff --git a/web/Views/Shared/Components/SessionTimeout/SessionTimeout.cs b/web/Views/Shared/Components/SessionTimeout/SessionTimeout.cs index 5f14f08c7..2bf3579ba 100644 --- a/web/Views/Shared/Components/SessionTimeout/SessionTimeout.cs +++ b/web/Views/Shared/Components/SessionTimeout/SessionTimeout.cs @@ -5,7 +5,7 @@ namespace Viper.Views.Shared.Components.SessionTimeout [ViewComponent(Name = "SessionTimeout")] public class SessionTimeout : ViewComponent { - public async Task InvokeAsync() + public IViewComponentResult Invoke() { UserHelper userHelper = new UserHelper(); string? loginId = userHelper.GetCurrentUser()?.LoginId; @@ -14,7 +14,7 @@ public async Task InvokeAsync() + "/public/timeout/seconds_until_timeout_v2.cfm?id=" + (loginId ?? "") + "&service=" + (onDev ? "Viper2-dev" : "Viper2"); - return await Task.Run(() => View("Default")); + return View("Default"); } } diff --git a/web/Views/Shared/Components/VueCdn/VueCdnCreate.cs b/web/Views/Shared/Components/VueCdn/VueCdnCreate.cs index fec18122b..88e644759 100644 --- a/web/Views/Shared/Components/VueCdn/VueCdnCreate.cs +++ b/web/Views/Shared/Components/VueCdn/VueCdnCreate.cs @@ -5,9 +5,9 @@ namespace Viper.Views.Shared.Components.VueCdn [ViewComponent(Name = "VueCdnCreate")] public class VueCdnCreate : ViewComponent { - public async Task InvokeAsync() + public IViewComponentResult Invoke() { - return await Task.Run(() => View("~/Views/Shared/Components/VueCdn/VueCdnCreate.cshtml")); + return View("~/Views/Shared/Components/VueCdn/VueCdnCreate.cshtml"); } } } diff --git a/web/Views/Shared/Components/VueCdn/VueCdnInit.cs b/web/Views/Shared/Components/VueCdn/VueCdnInit.cs index 74573ac4b..724f145ef 100644 --- a/web/Views/Shared/Components/VueCdn/VueCdnInit.cs +++ b/web/Views/Shared/Components/VueCdn/VueCdnInit.cs @@ -5,9 +5,9 @@ namespace Viper.Views.Shared.Components.VueCdn [ViewComponent(Name = "VueCdnInit")] public class VueCdnInit : ViewComponent { - public async Task InvokeAsync() + public IViewComponentResult Invoke() { - return await Task.Run(() => View("~/Views/Shared/Components/VueCdn/VueCdnInit.cshtml")); + return View("~/Views/Shared/Components/VueCdn/VueCdnInit.cshtml"); } } diff --git a/web/Views/Shared/Components/VueTableDefault/VueTableDefault.cs b/web/Views/Shared/Components/VueTableDefault/VueTableDefault.cs index 600a39ccd..b0143d924 100644 --- a/web/Views/Shared/Components/VueTableDefault/VueTableDefault.cs +++ b/web/Views/Shared/Components/VueTableDefault/VueTableDefault.cs @@ -8,7 +8,7 @@ namespace Viper.Views.Shared.Components.VueTableDefault public class VueTableDefaultViewComponent : ViewComponent { - public async Task InvokeAsync(IEnumerable? data, string keyColumnName, + public IViewComponentResult Invoke(IEnumerable? data, string keyColumnName, IEnumerable? skipColumns = null, IEnumerable>? altColumnNames = null, IEnumerable? skipColumnsVisible = null ) @@ -25,7 +25,7 @@ public async Task InvokeAsync(IEnumerable? data, s ViewData["Rows"] = GetDefaultRows(dataList, skipList); ViewData["VisibleColumns"] = GetDefaultVisibleColumns(dataList, skipVisibleList); - return await Task.Run(() => View("Default")); + return View("Default"); } #region public static string GetDefaultColumnNames(IEnumerable? data, IEnumerable? skipColumns = null, IEnumerable>? altColumnNames = null)