[BEEEP] Fix all CA2254 occurrences (#6357)

* Fix all CA2254 occurrences

* Fix tests

.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

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);
     }
 }

bitwarden_license/test/Scim.Test/Groups/PatchGroupCommandTests.cs

@@ -436,7 +436,7 @@ public class PatchGroupCommandTests
         await sutProvider.GetDependency<IGroupService>().DidNotReceiveWithAnyArgs().DeleteUserAsync(default, default);
 
         // Assert: logging
-        sutProvider.GetDependency<ILogger<PatchGroupCommand>>().ReceivedWithAnyArgs().LogWarning(default);
+        sutProvider.GetDependency<ILogger<PatchGroupCommand>>().ReceivedWithAnyArgs().LogWarning("");
     }
 
     [Theory]

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 };
         }
 

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);
     }
 }

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);
     }
 }

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;
                     }
                 }

src/Api/Utilities/ExceptionHandlerFilterAttribute.cs

@@ -152,7 +152,7 @@ public class ExceptionHandlerFilterAttribute : ExceptionFilterAttribute
         else
         {
             var logger = context.HttpContext.RequestServices.GetRequiredService<ILogger<ExceptionHandlerFilterAttribute>>();
-            logger.LogError(0, exception, exception.Message);
+            logger.LogError(0, exception, "Unhandled exception");
             errorMessage = "An unhandled server error has occurred.";
             context.HttpContext.Response.StatusCode = 500;
         }

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;
                     }
                 }

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<OrganizationAdminAuthRequest> 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;
         }
 

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);
         }
     }

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<OrganizationSponsorshipData>();

src/Core/Utilities/LoggingExceptionHandlerFilterAttribute.cs

@@ -17,6 +17,6 @@ public class LoggingExceptionHandlerFilterAttribute : ExceptionFilterAttribute
 
         var logger = context.HttpContext.RequestServices
             .GetRequiredService<ILogger<LoggingExceptionHandlerFilterAttribute>>();
-        logger.LogError(0, exception, exception.Message);
+        logger.LogError(0, exception, "Unhandled exception");
     }
 }

src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs

@@ -268,8 +268,7 @@ public abstract class BaseRequestValidator<T> 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<T> 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.
     }

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);
     }
 }

src/SharedWeb/Utilities/ExceptionHandlerFilterAttribute.cs

@@ -75,7 +75,7 @@ public class ExceptionHandlerFilterAttribute : ExceptionFilterAttribute
         else
         {
             var logger = context.HttpContext.RequestServices.GetRequiredService<ILogger<ExceptionHandlerFilterAttribute>>();
-            logger.LogError(0, exception, exception.Message);
+            logger.LogError(0, exception, "Unhandled exception");
             errorMessage = "An unhandled server error has occurred.";
             context.HttpContext.Response.StatusCode = 500;
         }

test/Core.Test/Auth/Services/AuthRequestServiceTests.cs

@@ -467,10 +467,9 @@ public class AuthRequestServiceTests
                 Arg.Any<string>(),
                 Arg.Any<string>());
 
-        var expectedLogMessage = "There are no admin emails to send to.";
         sutProvider.GetDependency<ILogger<AuthRequestService>>()
             .Received(1)
-            .LogWarning(expectedLogMessage);
+            .LogWarning("There are no admin emails to send to.");
     }
 
     /// <summary>

test/Identity.Test/Identity.Test.csproj

@@ -5,6 +5,7 @@
   </PropertyGroup>
 
   <ItemGroup>
+    <PackageReference Include="Microsoft.Extensions.Diagnostics.Testing" Version="9.3.0" />
     <PackageReference Include="coverlet.collector" Version="$(CoverletCollectorVersion)">
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
       <PrivateAssets>all</PrivateAssets>

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<BaseRequestValidatorTests> _logger;
+    private readonly FakeLogger<BaseRequestValidatorTests> _logger;
     private readonly ICurrentContext _currentContext;
     private readonly GlobalSettings _globalSettings;
     private readonly IUserRepository _userRepository;
@@ -65,7 +66,7 @@ public class BaseRequestValidatorTests
         _deviceValidator = Substitute.For<IDeviceValidator>();
         _twoFactorAuthenticationValidator = Substitute.For<ITwoFactorAuthenticationValidator>();
         _organizationUserRepository = Substitute.For<IOrganizationUserRepository>();
-        _logger = Substitute.For<ILogger<BaseRequestValidatorTests>>();
+        _logger = new FakeLogger<BaseRequestValidatorTests>();
         _currentContext = Substitute.For<ICurrentContext>();
         _globalSettings = Substitute.For<GlobalSettings>();
         _userRepository = Substitute.For<IUserRepository>();
@@ -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

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;
         }
 

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));
     }
 }