From c6f1acede9a64c434a662dc0e54204859bada390 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Mon, 20 Oct 2025 11:34:31 -0400 Subject: [PATCH] [BEEEP] Fix all CA2254 occurrences (#6357) * Fix all CA2254 occurrences * Fix tests --- .editorconfig | 9 +++++++++ bitwarden_license/src/Sso/Startup.cs | 2 +- .../test/Scim.Test/Groups/PatchGroupCommandTests.cs | 2 +- src/Admin/Controllers/HomeController.cs | 4 ++-- src/Admin/Jobs/AliveJob.cs | 2 +- src/Api/Startup.cs | 2 +- src/Api/Tools/Controllers/SendsController.cs | 2 +- src/Api/Utilities/ExceptionHandlerFilterAttribute.cs | 2 +- src/Api/Vault/Controllers/CiphersController.cs | 2 +- .../UpdateOrganizationAuthRequestCommand.cs | 4 ++-- src/Core/Jobs/BaseJobsHostedService.cs | 6 +++--- .../SelfHosted/SelfHostedSyncSponsorshipsCommand.cs | 2 +- .../LoggingExceptionHandlerFilterAttribute.cs | 2 +- .../RequestValidators/BaseRequestValidator.cs | 5 ++--- src/Identity/Startup.cs | 2 +- .../Utilities/ExceptionHandlerFilterAttribute.cs | 2 +- .../Auth/Services/AuthRequestServiceTests.cs | 3 +-- test/Identity.Test/Identity.Test.csproj | 1 + .../IdentityServer/BaseRequestValidatorTests.cs | 10 ++++++---- util/Migrator/DbMigrator.cs | 4 ++-- util/Migrator/DbUpLogger.cs | 12 ++++++------ 21 files changed, 45 insertions(+), 35 deletions(-) diff --git a/.editorconfig b/.editorconfig index 21d7ac4a3a..fd68808456 100644 --- a/.editorconfig +++ b/.editorconfig @@ -123,3 +123,12 @@ csharp_style_namespace_declarations = file_scoped:warning # Switch expression dotnet_diagnostic.CS8509.severity = error # missing switch case for named enum value dotnet_diagnostic.CS8524.severity = none # missing switch case for unnamed enum value + +# CA2253: Named placeholders should nto be numeric values +dotnet_diagnostic.CA2253.severity = suggestion + +# CA2254: Template should be a static expression +dotnet_diagnostic.CA2254.severity = warning + +# CA1727: Use PascalCase for named placeholders +dotnet_diagnostic.CA1727.severity = suggestion diff --git a/bitwarden_license/src/Sso/Startup.cs b/bitwarden_license/src/Sso/Startup.cs index 3aeb9c6beb..3ae8883ac4 100644 --- a/bitwarden_license/src/Sso/Startup.cs +++ b/bitwarden_license/src/Sso/Startup.cs @@ -157,6 +157,6 @@ public class Startup app.UseEndpoints(endpoints => endpoints.MapDefaultControllerRoute()); // Log startup - logger.LogInformation(Constants.BypassFiltersEventId, globalSettings.ProjectName + " started."); + logger.LogInformation(Constants.BypassFiltersEventId, "{Project} started.", globalSettings.ProjectName); } } diff --git a/bitwarden_license/test/Scim.Test/Groups/PatchGroupCommandTests.cs b/bitwarden_license/test/Scim.Test/Groups/PatchGroupCommandTests.cs index 1b02e62970..8816885ea7 100644 --- a/bitwarden_license/test/Scim.Test/Groups/PatchGroupCommandTests.cs +++ b/bitwarden_license/test/Scim.Test/Groups/PatchGroupCommandTests.cs @@ -436,7 +436,7 @@ public class PatchGroupCommandTests await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().DeleteUserAsync(default, default); // Assert: logging - sutProvider.GetDependency>().ReceivedWithAnyArgs().LogWarning(default); + sutProvider.GetDependency>().ReceivedWithAnyArgs().LogWarning(""); } [Theory] diff --git a/src/Admin/Controllers/HomeController.cs b/src/Admin/Controllers/HomeController.cs index debe5979f5..5b36032ec9 100644 --- a/src/Admin/Controllers/HomeController.cs +++ b/src/Admin/Controllers/HomeController.cs @@ -61,7 +61,7 @@ public class HomeController : Controller } catch (HttpRequestException e) { - _logger.LogError(e, $"Error encountered while sending GET request to {requestUri}"); + _logger.LogError(e, "Error encountered while sending GET request to {RequestUri}", requestUri); return new JsonResult("Unable to fetch latest version") { StatusCode = StatusCodes.Status500InternalServerError }; } @@ -83,7 +83,7 @@ public class HomeController : Controller } catch (HttpRequestException e) { - _logger.LogError(e, $"Error encountered while sending GET request to {requestUri}"); + _logger.LogError(e, "Error encountered while sending GET request to {RequestUri}", requestUri); return new JsonResult("Unable to fetch installed version") { StatusCode = StatusCodes.Status500InternalServerError }; } diff --git a/src/Admin/Jobs/AliveJob.cs b/src/Admin/Jobs/AliveJob.cs index b97d597e58..d62f4cc2cc 100644 --- a/src/Admin/Jobs/AliveJob.cs +++ b/src/Admin/Jobs/AliveJob.cs @@ -22,7 +22,7 @@ public class AliveJob : BaseJob { _logger.LogInformation(Constants.BypassFiltersEventId, "Execute job task: Keep alive"); var response = await _httpClient.GetAsync(_globalSettings.BaseServiceUri.Admin); - _logger.LogInformation(Constants.BypassFiltersEventId, "Finished job task: Keep alive, " + + _logger.LogInformation(Constants.BypassFiltersEventId, "Finished job task: Keep alive, {StatusCode}", response.StatusCode); } } diff --git a/src/Api/Startup.cs b/src/Api/Startup.cs index 5d9918d1d4..1519bb25c8 100644 --- a/src/Api/Startup.cs +++ b/src/Api/Startup.cs @@ -326,6 +326,6 @@ public class Startup } // Log startup - logger.LogInformation(Constants.BypassFiltersEventId, globalSettings.ProjectName + " started."); + logger.LogInformation(Constants.BypassFiltersEventId, "{Project} started.", globalSettings.ProjectName); } } diff --git a/src/Api/Tools/Controllers/SendsController.cs b/src/Api/Tools/Controllers/SendsController.cs index c02e9b0c20..c54a9b90c9 100644 --- a/src/Api/Tools/Controllers/SendsController.cs +++ b/src/Api/Tools/Controllers/SendsController.cs @@ -166,7 +166,7 @@ public class SendsController : Controller } catch (Exception e) { - _logger.LogError(e, $"Uncaught exception occurred while handling event grid event: {JsonSerializer.Serialize(eventGridEvent)}"); + _logger.LogError(e, "Uncaught exception occurred while handling event grid event: {Event}", JsonSerializer.Serialize(eventGridEvent)); return; } } diff --git a/src/Api/Utilities/ExceptionHandlerFilterAttribute.cs b/src/Api/Utilities/ExceptionHandlerFilterAttribute.cs index 91079d5040..1caa7cf841 100644 --- a/src/Api/Utilities/ExceptionHandlerFilterAttribute.cs +++ b/src/Api/Utilities/ExceptionHandlerFilterAttribute.cs @@ -152,7 +152,7 @@ public class ExceptionHandlerFilterAttribute : ExceptionFilterAttribute else { var logger = context.HttpContext.RequestServices.GetRequiredService>(); - logger.LogError(0, exception, exception.Message); + logger.LogError(0, exception, "Unhandled exception"); errorMessage = "An unhandled server error has occurred."; context.HttpContext.Response.StatusCode = 500; } diff --git a/src/Api/Vault/Controllers/CiphersController.cs b/src/Api/Vault/Controllers/CiphersController.cs index eb04ac1210..fe3069d8c7 100644 --- a/src/Api/Vault/Controllers/CiphersController.cs +++ b/src/Api/Vault/Controllers/CiphersController.cs @@ -1593,7 +1593,7 @@ public class CiphersController : Controller } catch (Exception e) { - _logger.LogError(e, $"Uncaught exception occurred while handling event grid event: {JsonSerializer.Serialize(eventGridEvent)}"); + _logger.LogError(e, "Uncaught exception occurred while handling event grid event: {Event}", JsonSerializer.Serialize(eventGridEvent)); return; } } diff --git a/src/Core/AdminConsole/OrganizationAuth/UpdateOrganizationAuthRequestCommand.cs b/src/Core/AdminConsole/OrganizationAuth/UpdateOrganizationAuthRequestCommand.cs index af966a6e16..9c699a61cb 100644 --- a/src/Core/AdminConsole/OrganizationAuth/UpdateOrganizationAuthRequestCommand.cs +++ b/src/Core/AdminConsole/OrganizationAuth/UpdateOrganizationAuthRequestCommand.cs @@ -89,7 +89,7 @@ public class UpdateOrganizationAuthRequestCommand : IUpdateOrganizationAuthReque AuthRequestExpiresAfter = _globalSettings.PasswordlessAuth.AdminRequestExpiration } ); - processor.Process((Exception e) => _logger.LogError(e.Message)); + processor.Process((Exception e) => _logger.LogError("Error processing organization auth request: {Message}", e.Message)); await processor.Save((IEnumerable authRequests) => _authRequestRepository.UpdateManyAsync(authRequests)); await processor.SendPushNotifications((ar) => _pushNotificationService.PushAuthRequestResponseAsync(ar)); await processor.SendApprovalEmailsForProcessedRequests(SendApprovalEmail); @@ -114,7 +114,7 @@ public class UpdateOrganizationAuthRequestCommand : IUpdateOrganizationAuthReque // This should be impossible if (user == null) { - _logger.LogError($"User {authRequest.UserId} not found. Trusted device admin approval email not sent."); + _logger.LogError("User {UserId} not found. Trusted device admin approval email not sent.", authRequest.UserId); return; } diff --git a/src/Core/Jobs/BaseJobsHostedService.cs b/src/Core/Jobs/BaseJobsHostedService.cs index 3e7bce7e0f..8b74052f8f 100644 --- a/src/Core/Jobs/BaseJobsHostedService.cs +++ b/src/Core/Jobs/BaseJobsHostedService.cs @@ -107,7 +107,7 @@ public abstract class BaseJobsHostedService : IHostedService, IDisposable throw new Exception("Job failed to start after 10 retries."); } - _logger.LogWarning($"Exception while trying to schedule job: {job.FullName}, {e}"); + _logger.LogWarning(e, "Exception while trying to schedule job: {JobName}", job.FullName); var random = new Random(); await Task.Delay(random.Next(50, 250)); } @@ -125,7 +125,7 @@ public abstract class BaseJobsHostedService : IHostedService, IDisposable continue; } - _logger.LogInformation($"Deleting old job with key {key}"); + _logger.LogInformation("Deleting old job with key {Key}", key); await _scheduler.DeleteJob(key); } @@ -138,7 +138,7 @@ public abstract class BaseJobsHostedService : IHostedService, IDisposable continue; } - _logger.LogInformation($"Unscheduling old trigger with key {key}"); + _logger.LogInformation("Unscheduling old trigger with key {Key}", key); await _scheduler.UnscheduleJob(key); } } diff --git a/src/Core/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/SelfHosted/SelfHostedSyncSponsorshipsCommand.cs b/src/Core/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/SelfHosted/SelfHostedSyncSponsorshipsCommand.cs index 9a995a9cf0..965e0cf2a9 100644 --- a/src/Core/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/SelfHosted/SelfHostedSyncSponsorshipsCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/SelfHosted/SelfHostedSyncSponsorshipsCommand.cs @@ -62,7 +62,7 @@ public class SelfHostedSyncSponsorshipsCommand : BaseIdentityClientService, ISel .ToDictionary(i => i.SponsoringOrganizationUserId); if (!organizationSponsorshipsDict.Any()) { - _logger.LogInformation($"No existing sponsorships to sync for organization {organizationId}"); + _logger.LogInformation("No existing sponsorships to sync for organization {organizationId}", organizationId); return; } var syncedSponsorships = new List(); diff --git a/src/Core/Utilities/LoggingExceptionHandlerFilterAttribute.cs b/src/Core/Utilities/LoggingExceptionHandlerFilterAttribute.cs index 6709bbb271..300c30641e 100644 --- a/src/Core/Utilities/LoggingExceptionHandlerFilterAttribute.cs +++ b/src/Core/Utilities/LoggingExceptionHandlerFilterAttribute.cs @@ -17,6 +17,6 @@ public class LoggingExceptionHandlerFilterAttribute : ExceptionFilterAttribute var logger = context.HttpContext.RequestServices .GetRequiredService>(); - logger.LogError(0, exception, exception.Message); + logger.LogError(0, exception, "Unhandled exception"); } } diff --git a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index 28d4630993..b976775aca 100644 --- a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -268,8 +268,7 @@ public abstract class BaseRequestValidator where T : class if (_globalSettings.SelfHosted) { _logger.LogWarning(Constants.BypassFiltersEventId, - string.Format("Failed login attempt{0}{1}", twoFactorRequest ? ", 2FA invalid." : ".", - $" {CurrentContext.IpAddress}")); + "Failed login attempt. Is2FARequest: {Is2FARequest} IpAddress: {IpAddress}", twoFactorRequest, CurrentContext.IpAddress); } await Task.Delay(2000); // Delay for brute force. @@ -299,7 +298,7 @@ public abstract class BaseRequestValidator where T : class formattedMessage = "Failed login attempt."; break; } - _logger.LogWarning(Constants.BypassFiltersEventId, formattedMessage); + _logger.LogWarning(Constants.BypassFiltersEventId, "{FailedLoginMessage}", formattedMessage); } await Task.Delay(2000); // Delay for brute force. } diff --git a/src/Identity/Startup.cs b/src/Identity/Startup.cs index 8da31d87d6..74344977a0 100644 --- a/src/Identity/Startup.cs +++ b/src/Identity/Startup.cs @@ -240,6 +240,6 @@ public class Startup app.UseEndpoints(endpoints => endpoints.MapDefaultControllerRoute()); // Log startup - logger.LogInformation(Constants.BypassFiltersEventId, globalSettings.ProjectName + " started."); + logger.LogInformation(Constants.BypassFiltersEventId, "{Project} started.", globalSettings.ProjectName); } } diff --git a/src/SharedWeb/Utilities/ExceptionHandlerFilterAttribute.cs b/src/SharedWeb/Utilities/ExceptionHandlerFilterAttribute.cs index 332aa6838c..aba1a6a8dc 100644 --- a/src/SharedWeb/Utilities/ExceptionHandlerFilterAttribute.cs +++ b/src/SharedWeb/Utilities/ExceptionHandlerFilterAttribute.cs @@ -75,7 +75,7 @@ public class ExceptionHandlerFilterAttribute : ExceptionFilterAttribute else { var logger = context.HttpContext.RequestServices.GetRequiredService>(); - logger.LogError(0, exception, exception.Message); + logger.LogError(0, exception, "Unhandled exception"); errorMessage = "An unhandled server error has occurred."; context.HttpContext.Response.StatusCode = 500; } diff --git a/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs b/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs index 5da0e78422..9c95930c18 100644 --- a/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs +++ b/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs @@ -467,10 +467,9 @@ public class AuthRequestServiceTests Arg.Any(), Arg.Any()); - var expectedLogMessage = "There are no admin emails to send to."; sutProvider.GetDependency>() .Received(1) - .LogWarning(expectedLogMessage); + .LogWarning("There are no admin emails to send to."); } /// diff --git a/test/Identity.Test/Identity.Test.csproj b/test/Identity.Test/Identity.Test.csproj index fc0cf07b63..496d652b30 100644 --- a/test/Identity.Test/Identity.Test.csproj +++ b/test/Identity.Test/Identity.Test.csproj @@ -5,6 +5,7 @@ + runtime; build; native; contentfiles; analyzers; buildtransitive all diff --git a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs index 6e7c327621..53615cd1d1 100644 --- a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs @@ -24,6 +24,7 @@ using Bit.Test.Common.AutoFixture.Attributes; using Duende.IdentityServer.Validation; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Testing; using Microsoft.Extensions.Options; using NSubstitute; using Xunit; @@ -42,7 +43,7 @@ public class BaseRequestValidatorTests private readonly IDeviceValidator _deviceValidator; private readonly ITwoFactorAuthenticationValidator _twoFactorAuthenticationValidator; private readonly IOrganizationUserRepository _organizationUserRepository; - private readonly ILogger _logger; + private readonly FakeLogger _logger; private readonly ICurrentContext _currentContext; private readonly GlobalSettings _globalSettings; private readonly IUserRepository _userRepository; @@ -65,7 +66,7 @@ public class BaseRequestValidatorTests _deviceValidator = Substitute.For(); _twoFactorAuthenticationValidator = Substitute.For(); _organizationUserRepository = Substitute.For(); - _logger = Substitute.For>(); + _logger = new FakeLogger(); _currentContext = Substitute.For(); _globalSettings = Substitute.For(); _userRepository = Substitute.For(); @@ -120,7 +121,8 @@ public class BaseRequestValidatorTests await _sut.ValidateAsync(context); // Assert - _logger.Received(1).LogWarning(Constants.BypassFiltersEventId, "Failed login attempt. "); + var logs = _logger.Collector.GetSnapshot(true); + Assert.Contains(logs, l => l.Level == LogLevel.Warning && l.Message == "Failed login attempt. Is2FARequest: False IpAddress: "); var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; Assert.Equal("Username or password is incorrect. Try again.", errorResponse.Message); } @@ -356,7 +358,7 @@ public class BaseRequestValidatorTests // 1 -> initial validation passes _sut.isValid = true; - // 2 -> enable the FailedTwoFactorEmail feature flag + // 2 -> enable the FailedTwoFactorEmail feature flag _featureService.IsEnabled(FeatureFlagKeys.FailedTwoFactorEmail).Returns(true); // 3 -> set up 2FA as required diff --git a/util/Migrator/DbMigrator.cs b/util/Migrator/DbMigrator.cs index e5e7a569b2..9345a72fc2 100644 --- a/util/Migrator/DbMigrator.cs +++ b/util/Migrator/DbMigrator.cs @@ -53,7 +53,7 @@ public class DbMigrator if (ex.Message.Contains("Server is in script upgrade mode.")) { attempt++; - _logger.LogInformation($"Database is in script upgrade mode, trying again (attempt #{attempt})."); + _logger.LogInformation("Database is in script upgrade mode, trying again (attempt #{Attempt}).", attempt); Thread.Sleep(20000); } else @@ -165,7 +165,7 @@ public class DbMigrator { stringBuilder.AppendLine(script.Name); } - _logger.LogInformation(Constants.BypassFiltersEventId, stringBuilder.ToString()); + _logger.LogInformation(Constants.BypassFiltersEventId, "{Scripts}", stringBuilder.ToString()); return true; } diff --git a/util/Migrator/DbUpLogger.cs b/util/Migrator/DbUpLogger.cs index 2587ce4d80..3f94990ba9 100644 --- a/util/Migrator/DbUpLogger.cs +++ b/util/Migrator/DbUpLogger.cs @@ -15,31 +15,31 @@ public class DbUpLogger : IUpgradeLog public void LogTrace(string format, params object[] args) { - _logger.LogTrace(Constants.BypassFiltersEventId, format, args); + _logger.LogTrace(Constants.BypassFiltersEventId, "{TraceMessage}", string.Format(format, args)); } public void LogDebug(string format, params object[] args) { - _logger.LogDebug(Constants.BypassFiltersEventId, format, args); + _logger.LogDebug(Constants.BypassFiltersEventId, "{DebugMessage}", string.Format(format, args)); } public void LogInformation(string format, params object[] args) { - _logger.LogInformation(Constants.BypassFiltersEventId, format, args); + _logger.LogInformation(Constants.BypassFiltersEventId, "{InfoMessage}", string.Format(format, args)); } public void LogWarning(string format, params object[] args) { - _logger.LogWarning(Constants.BypassFiltersEventId, format, args); + _logger.LogWarning(Constants.BypassFiltersEventId, "{WarningMessage}", string.Format(format, args)); } public void LogError(string format, params object[] args) { - _logger.LogError(Constants.BypassFiltersEventId, format, args); + _logger.LogError(Constants.BypassFiltersEventId, "{ErrorMessage}", string.Format(format, args)); } public void LogError(Exception ex, string format, params object[] args) { - _logger.LogError(Constants.BypassFiltersEventId, ex, format, args); + _logger.LogError(Constants.BypassFiltersEventId, ex, "{ErrorMessage}", string.Format(format, args)); } }