From 828f0856bb9cab7fcbe300a0099c0e8b5d517136 Mon Sep 17 00:00:00 2001 From: Rex Lorenzo Date: Wed, 13 May 2026 00:09:03 -0700 Subject: [PATCH 1/3] chore: style sweep for remaining CodeQL alerts --- .../EmailNotificationTest.cs | 4 +- test/ClinicalScheduler/TestDataBuilder.cs | 2 +- web/Areas/CMS/Controllers/CMSController.cs | 11 ++---- .../Curriculum/Services/TermCodeService.cs | 2 +- web/Areas/Directory/Models/LdapUserContact.cs | 2 +- .../Effort/Services/VerificationService.cs | 12 ++---- web/Areas/RAPS/Controllers/RAPSController.cs | 24 ++++-------- web/Areas/RAPS/Services/UinformService.cs | 2 +- .../Students/Services/GradYearClassLevel.cs | 6 +-- web/Classes/HttpHelper.cs | 23 +++-------- web/Classes/UserHelper.cs | 39 ++----------------- web/Controllers/HomeController.cs | 2 +- 12 files changed, 32 insertions(+), 97 deletions(-) diff --git a/test/ClinicalScheduler/EmailNotificationTest.cs b/test/ClinicalScheduler/EmailNotificationTest.cs index be00573e6..764af287b 100644 --- a/test/ClinicalScheduler/EmailNotificationTest.cs +++ b/test/ClinicalScheduler/EmailNotificationTest.cs @@ -119,8 +119,8 @@ private async Task AddTestWeekGradYearAsync(int weekId, int gradYear = 2025, int await _context.Weeks.AddAsync(new Week { WeekId = weekId, - DateStart = DateTime.UtcNow.AddDays(-7 * (10 - weekId)), - DateEnd = DateTime.UtcNow.AddDays(-7 * (10 - weekId) + 6), + DateStart = DateTime.UtcNow.AddDays((double)(-7 * (10 - weekId))), + DateEnd = DateTime.UtcNow.AddDays((double)(-7 * (10 - weekId) + 6)), TermCode = 202501 }); } diff --git a/test/ClinicalScheduler/TestDataBuilder.cs b/test/ClinicalScheduler/TestDataBuilder.cs index 828d3a9d6..fe2e0e8f8 100644 --- a/test/ClinicalScheduler/TestDataBuilder.cs +++ b/test/ClinicalScheduler/TestDataBuilder.cs @@ -176,7 +176,7 @@ public static Viper.Models.ClinicalScheduler.Person CreatePerson(string mothraId /// public static Week CreateWeek(int weekId, DateTime? startDate = null) { - var start = startDate ?? DateTime.UtcNow.AddDays(-7 * (10 - weekId)); + var start = startDate ?? DateTime.UtcNow.AddDays((double)(-7 * (10 - weekId))); return new Week { WeekId = weekId, diff --git a/web/Areas/CMS/Controllers/CMSController.cs b/web/Areas/CMS/Controllers/CMSController.cs index ec726c484..1bd7888d5 100644 --- a/web/Areas/CMS/Controllers/CMSController.cs +++ b/web/Areas/CMS/Controllers/CMSController.cs @@ -25,14 +25,9 @@ public IActionResult Files(string id = "", string fn = "", string oldURL = "", s { Data.CMS cms = new(_viperContext, _rapsContext, _sanitizerService, _cmsLogger); - if (ids.Length > 0) - { - return cms.DownloadZip(this, ids.Split(','), fileName); - } - else - { - return cms.ProvideFile(this, id, fn, oldURL); - } + return ids.Length > 0 + ? cms.DownloadZip(this, ids.Split(','), fileName) + : cms.ProvideFile(this, id, fn, oldURL); } } diff --git a/web/Areas/Curriculum/Services/TermCodeService.cs b/web/Areas/Curriculum/Services/TermCodeService.cs index 4b1695f23..a49d8066e 100644 --- a/web/Areas/Curriculum/Services/TermCodeService.cs +++ b/web/Areas/Curriculum/Services/TermCodeService.cs @@ -36,7 +36,7 @@ static public string GetTermCodeDescription(int termCode) default: desc = "Unknown Term"; break; } - return string.Format("{0} {1}", desc, year.ToString()); + return string.Format("{0} {1}", desc, year); } public async Task> GetTerms(string? TermType = null, bool? current = null, bool? currentMulti = null) diff --git a/web/Areas/Directory/Models/LdapUserContact.cs b/web/Areas/Directory/Models/LdapUserContact.cs index 763678ed9..53be63189 100644 --- a/web/Areas/Directory/Models/LdapUserContact.cs +++ b/web/Areas/Directory/Models/LdapUserContact.cs @@ -36,7 +36,7 @@ public LdapUserContact(SearchResultEntry entry) foreach (DirectoryAttribute attr in entry.Attributes.Values) { var v = attr[0]; - or.Add(attr.Name + "=" + v.ToString()); + or.Add(attr.Name + "=" + v); switch (attr.Name) { case "uid": Uid = v.ToString(); break; diff --git a/web/Areas/Effort/Services/VerificationService.cs b/web/Areas/Effort/Services/VerificationService.cs index a84eac6b5..77afc55ea 100644 --- a/web/Areas/Effort/Services/VerificationService.cs +++ b/web/Areas/Effort/Services/VerificationService.cs @@ -769,15 +769,9 @@ private VerificationReminderViewModel BuildVerificationEmailViewModel( var useWeeksForClinical = termCode >= EffortConstants.ClinicalAsWeeksStartTermCode; // Due date: ExpectedCloseDate - 7 days if set, otherwise fallback to Now + 7 - DateTime dueDate; - if (expectedCloseDate.HasValue) - { - dueDate = expectedCloseDate.Value.AddDays(-_settings.VerificationReplyDays); - } - else - { - dueDate = DateTime.Now.AddDays(_settings.VerificationReplyDays); - } + DateTime dueDate = expectedCloseDate.HasValue + ? expectedCloseDate.Value.AddDays(-_settings.VerificationReplyDays) + : DateTime.Now.AddDays(_settings.VerificationReplyDays); var isPastDue = DateTime.Now.Date > dueDate.Date; var replyByDate = dueDate.ToString("M/d/yy"); diff --git a/web/Areas/RAPS/Controllers/RAPSController.cs b/web/Areas/RAPS/Controllers/RAPSController.cs index cf544e83e..5499feab1 100644 --- a/web/Areas/RAPS/Controllers/RAPSController.cs +++ b/web/Areas/RAPS/Controllers/RAPSController.cs @@ -312,15 +312,10 @@ public async Task RoleMembers(string instance, int RoleId) { return NotFound(); } - if (_securityService.IsAllowedTo("EditRoleMembership", instance, Role)) - { - return View("~/Areas/RAPS/Views/Roles/Members.cshtml"); - } - else - { - //TODO: Should probably have a deny access helper function that writes logs and sets view - return View("~/Views/Home/403.cshtml"); - } + //TODO: Should probably have a deny access helper function that writes logs and sets view + return _securityService.IsAllowedTo("EditRoleMembership", instance, Role) + ? View("~/Areas/RAPS/Views/Roles/Members.cshtml") + : View("~/Views/Home/403.cshtml"); } /// @@ -356,14 +351,9 @@ public async Task RolePermissions(int roleId) [Route("/[area]/{Instance}/[action]")] public async Task RolePermissionsComparison(string instance) { - if (_securityService.IsAllowedTo("EditRoleMembership", instance)) - { - return await Task.Run(() => View("~/Areas/RAPS/Views/Roles/PermissionComparison.cshtml")); - } - else - { - return await Task.Run(() => View("~/Views/Home/403.cshtml")); - } + return _securityService.IsAllowedTo("EditRoleMembership", instance) + ? await Task.Run(() => View("~/Areas/RAPS/Views/Roles/PermissionComparison.cshtml")) + : await Task.Run(() => View("~/Views/Home/403.cshtml")); } /// diff --git a/web/Areas/RAPS/Services/UinformService.cs b/web/Areas/RAPS/Services/UinformService.cs index 359e1b785..1e52359c1 100644 --- a/web/Areas/RAPS/Services/UinformService.cs +++ b/web/Areas/RAPS/Services/UinformService.cs @@ -271,7 +271,7 @@ private static string GetAuthSignature(HttpMethod method, string publicKey, int //take "{METHOD}:{epochTime}:{publicKey}" and sign it with the privateKey, then convert to base64 if (!string.IsNullOrEmpty(publicKey) && !string.IsNullOrEmpty(privateKey)) { - string toSign = method.Method.ToUpper() + ":" + epochTime.ToString() + ":" + publicKey; + string toSign = method.Method.ToUpper() + ":" + epochTime + ":" + publicKey; // Legacy API requires HMACSHA1 - third-party system constraint #pragma warning disable CA5350 // Do Not Use Weak Cryptographic Algorithms using var sha1 = new HMACSHA1(Encoding.ASCII.GetBytes(privateKey)); diff --git a/web/Areas/Students/Services/GradYearClassLevel.cs b/web/Areas/Students/Services/GradYearClassLevel.cs index 5a1e35e41..4312c9acc 100644 --- a/web/Areas/Students/Services/GradYearClassLevel.cs +++ b/web/Areas/Students/Services/GradYearClassLevel.cs @@ -54,10 +54,10 @@ static public Tuple GetTermCodeAndClassLevelForGradYear(int gradYea switch (termPart) { case 2: - termAndClassLevel = Tuple.Create(currentTerm, "V" + (4 - (gradYear - termYear)).ToString()); + termAndClassLevel = Tuple.Create(currentTerm, "V" + (4 - (gradYear - termYear))); break; case 9: - termAndClassLevel = Tuple.Create(currentTerm, "V" + (5 - (gradYear - termYear)).ToString()); + termAndClassLevel = Tuple.Create(currentTerm, "V" + (5 - (gradYear - termYear))); break; case 4: if (gradYear - termYear == 1) @@ -66,7 +66,7 @@ static public Tuple GetTermCodeAndClassLevelForGradYear(int gradYea } else { - termAndClassLevel = Tuple.Create((termYear * 100) + 9, "V" + (5 - (gradYear - termYear)).ToString()); + termAndClassLevel = Tuple.Create((termYear * 100) + 9, "V" + (5 - (gradYear - termYear))); } break; } diff --git a/web/Classes/HttpHelper.cs b/web/Classes/HttpHelper.cs index 64fc0485f..64fc4d7e7 100644 --- a/web/Classes/HttpHelper.cs +++ b/web/Classes/HttpHelper.cs @@ -21,14 +21,14 @@ public static class HttpHelper /// Helper functions constructor (gets injected with the memeory cache object) /// /// - public static void Configure(IMemoryCache? memoryCache, IConfiguration? configurationSettings, IWebHostEnvironment env, IHttpContextAccessor? httpContextAccessor, IAuthorizationService? authorizationService, IDataProtectionProvider? dataProtectionProvider) + public static void Configure(IMemoryCache? memoryCache, IConfiguration? configurationSettings, IWebHostEnvironment env, IHttpContextAccessor? contextAccessor, IAuthorizationService? authzService, IDataProtectionProvider? dataProtection) { Cache = memoryCache; Settings = configurationSettings; Environment = env; - HttpHelper.httpContextAccessor = httpContextAccessor; - HttpHelper.authorizationService = authorizationService; - HttpHelper.dataProtectionProvider = dataProtectionProvider; + httpContextAccessor = contextAccessor; + authorizationService = authzService; + dataProtectionProvider = dataProtection; } /// @@ -44,20 +44,7 @@ public static void Configure(IMemoryCache? memoryCache, IConfiguration? configur /// /// Get the current HttpContext /// - public static HttpContext? HttpContext - { - get - { - if (httpContextAccessor != null) - { - return httpContextAccessor.HttpContext; - } - else - { - return null; - } - } - } + public static HttpContext? HttpContext => httpContextAccessor?.HttpContext; /// /// Get the current memory cache diff --git a/web/Classes/UserHelper.cs b/web/Classes/UserHelper.cs index 88853d526..ad02bd48d 100644 --- a/web/Classes/UserHelper.cs +++ b/web/Classes/UserHelper.cs @@ -65,15 +65,7 @@ on role.RoleId equals memberRoles.RoleId select role).ToList(); }); - if (result != null) - { - return result; - } - else - { - return new List(); - } - + return result ?? new List(); } else { @@ -149,15 +141,7 @@ on permission.PermissionId equals memberPermissions.PermissionId select permission).ToList(); }); - if (result != null) - { - return result; - } - else - { - return new List(); - } - + return result ?? new List(); } else { @@ -277,15 +261,7 @@ public bool HasPermission(RAPSContext? rapsContext, AaudUser? user, string permi user = HttpHelper.Cache.GetOrCreate("AaudUser-" + userLoginId, entry => { AaudUser? aaudUser = aaudContext.AaudUsers.FirstOrDefault(m => m.LoginId == loginId); - if (aaudUser != null) - { - return aaudUser; - } - else - { - return user; - } - + return aaudUser ?? user; }); return user; @@ -342,14 +318,7 @@ public bool HasPermission(RAPSContext? rapsContext, AaudUser? user, string permi ?? (AAUDContext?)HttpHelper.HttpContext?.RequestServices.GetService(typeof(AAUDContext)); AaudUser? trueUser = GetByLoginId(aaudContext, claims?.FirstOrDefault(claim => claim.Type == ClaimTypes.NameIdentifier)?.Value); - if (trueUser != null) - { - return trueUser; - } - else - { - return GetCurrentUser(); - } + return trueUser ?? GetCurrentUser(); } diff --git a/web/Controllers/HomeController.cs b/web/Controllers/HomeController.cs index d854ff6ed..01f977f51 100644 --- a/web/Controllers/HomeController.cs +++ b/web/Controllers/HomeController.cs @@ -312,7 +312,7 @@ private async Task AuthenticateCasLogin(string? ticket, string? r // uncommenting this line will log what CAS is sending. When the user in question logs in while trying to access our site if (string.IsNullOrEmpty(validatedUserName)) { - HttpHelper.Logger.Log(NLog.LogLevel.Warn, "No username. CAS response: " + doc.ToString()); + HttpHelper.Logger.Log(NLog.LogLevel.Warn, "No username. CAS response: " + doc); } if (!string.IsNullOrEmpty(validatedUserName)) From eea4ff3caa31480711edaf63f255a91a8f3bb8d0 Mon Sep 17 00:00:00 2001 From: Rex Lorenzo Date: Wed, 13 May 2026 02:10:47 -0700 Subject: [PATCH 2/3] chore(resharper): clean up warnings from codeql/12 style sweep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - UserHelper.cs: capture user into 'fallbackUser' local before GetOrCreate lambda so ReSharper AccessToModifiedClosure is silenced - HttpHelper.cs: drop stale '' XML tag now that the other params don't have matching tags (was creating 5 InvalidXmlDocComment warnings since the rename) - test/ClinicalScheduler/{EmailNotificationTest,TestDataBuilder}.cs: replace '(double)(-7 * x)' with '-7.0 * x' — same intent (force the multiplication into double per cs/loss-of-precision) but no RedundantCast warning from ReSharper --- test/ClinicalScheduler/EmailNotificationTest.cs | 4 ++-- test/ClinicalScheduler/TestDataBuilder.cs | 2 +- web/Classes/HttpHelper.cs | 3 +-- web/Classes/UserHelper.cs | 3 ++- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/ClinicalScheduler/EmailNotificationTest.cs b/test/ClinicalScheduler/EmailNotificationTest.cs index 764af287b..a91ec1dcd 100644 --- a/test/ClinicalScheduler/EmailNotificationTest.cs +++ b/test/ClinicalScheduler/EmailNotificationTest.cs @@ -119,8 +119,8 @@ private async Task AddTestWeekGradYearAsync(int weekId, int gradYear = 2025, int await _context.Weeks.AddAsync(new Week { WeekId = weekId, - DateStart = DateTime.UtcNow.AddDays((double)(-7 * (10 - weekId))), - DateEnd = DateTime.UtcNow.AddDays((double)(-7 * (10 - weekId) + 6)), + DateStart = DateTime.UtcNow.AddDays(-7.0 * (10 - weekId)), + DateEnd = DateTime.UtcNow.AddDays(-7.0 * (10 - weekId) + 6), TermCode = 202501 }); } diff --git a/test/ClinicalScheduler/TestDataBuilder.cs b/test/ClinicalScheduler/TestDataBuilder.cs index fe2e0e8f8..de032972d 100644 --- a/test/ClinicalScheduler/TestDataBuilder.cs +++ b/test/ClinicalScheduler/TestDataBuilder.cs @@ -176,7 +176,7 @@ public static Viper.Models.ClinicalScheduler.Person CreatePerson(string mothraId /// public static Week CreateWeek(int weekId, DateTime? startDate = null) { - var start = startDate ?? DateTime.UtcNow.AddDays((double)(-7 * (10 - weekId))); + var start = startDate ?? DateTime.UtcNow.AddDays(-7.0 * (10 - weekId)); return new Week { WeekId = weekId, diff --git a/web/Classes/HttpHelper.cs b/web/Classes/HttpHelper.cs index 64fc4d7e7..37b68bd3a 100644 --- a/web/Classes/HttpHelper.cs +++ b/web/Classes/HttpHelper.cs @@ -18,9 +18,8 @@ public static class HttpHelper private static IDataProtectionProvider? dataProtectionProvider; /// - /// Helper functions constructor (gets injected with the memeory cache object) + /// Helper functions constructor (gets injected with the memory cache object) /// - /// public static void Configure(IMemoryCache? memoryCache, IConfiguration? configurationSettings, IWebHostEnvironment env, IHttpContextAccessor? contextAccessor, IAuthorizationService? authzService, IDataProtectionProvider? dataProtection) { Cache = memoryCache; diff --git a/web/Classes/UserHelper.cs b/web/Classes/UserHelper.cs index ad02bd48d..50aeda3af 100644 --- a/web/Classes/UserHelper.cs +++ b/web/Classes/UserHelper.cs @@ -258,10 +258,11 @@ public bool HasPermission(RAPSContext? rapsContext, AaudUser? user, string permi } else if (HttpHelper.Cache != null && aaudContext != null) { + var fallbackUser = user; user = HttpHelper.Cache.GetOrCreate("AaudUser-" + userLoginId, entry => { AaudUser? aaudUser = aaudContext.AaudUsers.FirstOrDefault(m => m.LoginId == loginId); - return aaudUser ?? user; + return aaudUser ?? fallbackUser; }); return user; From 974f2885648992fc2d1202f4db7e34c657e17a59 Mon Sep 17 00:00:00 2001 From: Rex Lorenzo Date: Fri, 15 May 2026 11:17:37 -0700 Subject: [PATCH 3/3] fix(security): sanitize CAS XML before logging on auth failure The fallback log path for a CAS validation response with no username embedded the raw XML in the message. CAS responses come from an external source and can contain user identifiers and claim attributes, so per CLAUDE.md the value must pass through LogSanitizer before hitting the log. Wraps the doc.ToString() in LogSanitizer.SanitizeString. --- web/Controllers/HomeController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/Controllers/HomeController.cs b/web/Controllers/HomeController.cs index 01f977f51..b5fe8d6b1 100644 --- a/web/Controllers/HomeController.cs +++ b/web/Controllers/HomeController.cs @@ -312,7 +312,7 @@ private async Task AuthenticateCasLogin(string? ticket, string? r // uncommenting this line will log what CAS is sending. When the user in question logs in while trying to access our site if (string.IsNullOrEmpty(validatedUserName)) { - HttpHelper.Logger.Log(NLog.LogLevel.Warn, "No username. CAS response: " + doc); + HttpHelper.Logger.Log(NLog.LogLevel.Warn, "No username. CAS response: " + LogSanitizer.SanitizeString(doc.ToString())); } if (!string.IsNullOrEmpty(validatedUserName))