diff --git a/.github/renovate.json5 b/.github/renovate.json5 index 36f793e8c1..5c01832c06 100644 --- a/.github/renovate.json5 +++ b/.github/renovate.json5 @@ -84,7 +84,6 @@ "Serilog.AspNetCore", "Serilog.Extensions.Logging", "Serilog.Extensions.Logging.File", - "Serilog.Sinks.AzureCosmosDB", "Serilog.Sinks.SyslogMessages", "Stripe.net", "Swashbuckle.AspNetCore", diff --git a/src/Core/Utilities/LoggerFactoryExtensions.cs b/src/Core/Utilities/LoggerFactoryExtensions.cs index 5809da9c7a..54bd84df6f 100644 --- a/src/Core/Utilities/LoggerFactoryExtensions.cs +++ b/src/Core/Utilities/LoggerFactoryExtensions.cs @@ -1,7 +1,4 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using System.Security.Cryptography.X509Certificates; +using System.Security.Cryptography.X509Certificates; using Bit.Core.Settings; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; @@ -33,7 +30,7 @@ public static class LoggerFactoryExtensions public static ILoggingBuilder AddSerilog( this ILoggingBuilder builder, WebHostBuilderContext context, - Func filter = null) + Func? filter = null) { var globalSettings = new GlobalSettings(); ConfigurationBinder.Bind(context.Configuration.GetSection("GlobalSettings"), globalSettings); @@ -57,19 +54,27 @@ public static class LoggerFactoryExtensions return filter(e, globalSettings); } + var logSentryWarning = false; + var logSyslogWarning = false; + + // Path format is the only required option for file logging, we will use that as + // the keystone for if they have configured the new location. + var newPathFormat = context.Configuration["Logging:PathFormat"]; + var config = new LoggerConfiguration() .MinimumLevel.Verbose() .Enrich.FromLogContext() .Filter.ByIncludingOnly(inclusionPredicate); - if (CoreHelpers.SettingHasValue(globalSettings?.Sentry.Dsn)) + if (CoreHelpers.SettingHasValue(globalSettings.Sentry.Dsn)) { config.WriteTo.Sentry(globalSettings.Sentry.Dsn) .Enrich.FromLogContext() .Enrich.WithProperty("Project", globalSettings.ProjectName); } - else if (CoreHelpers.SettingHasValue(globalSettings?.Syslog.Destination)) + else if (CoreHelpers.SettingHasValue(globalSettings.Syslog.Destination)) { + logSyslogWarning = true; // appending sitename to project name to allow easier identification in syslog. var appName = $"{globalSettings.SiteName}-{globalSettings.ProjectName}"; if (globalSettings.Syslog.Destination.Equals("local", StringComparison.OrdinalIgnoreCase)) @@ -107,10 +112,14 @@ public static class LoggerFactoryExtensions certProvider: new CertificateFileProvider(globalSettings.Syslog.CertificatePath, globalSettings.Syslog?.CertificatePassword ?? string.Empty)); } - } } } + else if (!string.IsNullOrEmpty(newPathFormat)) + { + // Use new location + builder.AddFile(context.Configuration.GetSection("Logging")); + } else if (CoreHelpers.SettingHasValue(globalSettings.LogDirectory)) { if (globalSettings.LogRollBySizeLimit.HasValue) @@ -138,6 +147,17 @@ public static class LoggerFactoryExtensions } var serilog = config.CreateLogger(); + + if (logSentryWarning) + { + serilog.Warning("Sentry for logging has been deprecated. Read more: https://btwrdn.com/log-deprecation"); + } + + if (logSyslogWarning) + { + serilog.Warning("Syslog for logging has been deprecated. Read more: https://btwrdn.com/log-deprecation"); + } + builder.AddSerilog(serilog); return builder; diff --git a/test/Core.Test/Utilities/LoggerFactoryExtensionsTests.cs b/test/Core.Test/Utilities/LoggerFactoryExtensionsTests.cs new file mode 100644 index 0000000000..06bb362336 --- /dev/null +++ b/test/Core.Test/Utilities/LoggerFactoryExtensionsTests.cs @@ -0,0 +1,195 @@ +using System.Net; +using System.Net.Sockets; +using System.Text; +using Bit.Core.Utilities; +using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using NSubstitute; +using Serilog; +using Serilog.Extensions.Logging; +using Xunit; + +namespace Bit.Core.Test.Utilities; + +public class LoggerFactoryExtensionsTests +{ + [Fact] + public void AddSerilog_IsDevelopment_AddsNoProviders() + { + var providers = GetProviders([], "Development"); + + Assert.Empty(providers); + } + + [Fact] + public void AddSerilog_IsDevelopment_DevLoggingEnabled_AddsSerilog() + { + var providers = GetProviders(new Dictionary + { + { "GlobalSettings:EnableDevLogging", "true" }, + }, "Development"); + + var provider = Assert.Single(providers); + Assert.IsAssignableFrom(provider); + } + + [Fact] + public void AddSerilog_IsProduction_AddsSerilog() + { + var providers = GetProviders([]); + + var provider = Assert.Single(providers); + Assert.IsAssignableFrom(provider); + } + + [Fact] + public async Task AddSerilog_FileLogging_Old_Works() + { + var tempDir = Directory.CreateTempSubdirectory(); + + var providers = GetProviders(new Dictionary + { + { "GlobalSettings:ProjectName", "Test" }, + { "GlobalSetting:LogDirectoryByProject", "true" }, + { "GlobalSettings:LogDirectory", tempDir.FullName }, + }); + + var provider = Assert.Single(providers); + Assert.IsAssignableFrom(provider); + + var logger = provider.CreateLogger("Test"); + logger.LogWarning("This is a test"); + + var logFile = Assert.Single(tempDir.EnumerateFiles("Test/*.txt")); + + var logFileContents = await File.ReadAllTextAsync(logFile.FullName); + + Assert.Contains( + "This is a test", + logFileContents + ); + } + + [Fact] + public async Task AddSerilog_FileLogging_New_Works() + { + var tempDir = Directory.CreateTempSubdirectory(); + + var provider = GetServiceProvider(new Dictionary + { + { "Logging:PathFormat", $"{tempDir}/Logs/log-{{Date}}.log" }, + }, "Production"); + + var logger = provider + .GetRequiredService() + .CreateLogger("Test"); + + logger.LogWarning("This is a test"); + + // Writing to the file is buffered, give it a little time to flush + await Task.Delay(5); + + var logFile = Assert.Single(tempDir.EnumerateFiles("Logs/*.log")); + + var logFileContents = await File.ReadAllTextAsync(logFile.FullName); + + Assert.DoesNotContain( + "This configuration location for file logging has been deprecated.", + logFileContents + ); + Assert.Contains( + "This is a test", + logFileContents + ); + } + + [Fact(Skip = "Only for local development.")] + public async Task AddSerilog_SyslogConfigured_Warns() + { + // Setup a fake syslog server + var cts = new CancellationTokenSource(TimeSpan.FromSeconds(20)); + using var listener = new TcpListener(IPAddress.Parse("127.0.0.1"), 25000); + listener.Start(); + + var provider = GetServiceProvider(new Dictionary + { + { "GlobalSettings:SysLog:Destination", "tcp://127.0.0.1:25000" }, + { "GlobalSettings:SiteName", "TestSite" }, + { "GlobalSettings:ProjectName", "TestProject" }, + }, "Production"); + + var loggerFactory = provider.GetRequiredService(); + var logger = loggerFactory.CreateLogger("Test"); + + logger.LogWarning("This is a test"); + + // Look in syslog for data + using var socket = await listener.AcceptSocketAsync(cts.Token); + + // This is rather lazy as opposed to implementing smarter syslog message + // reading but thats not what this test about, so instead just give + // the sink time to finish its work in the background + + List messages = []; + + while (true) + { + var buffer = new byte[1024]; + var received = await socket.ReceiveAsync(buffer, SocketFlags.None, cts.Token); + + if (received == 0) + { + break; + } + + var response = Encoding.ASCII.GetString(buffer, 0, received); + messages.Add(response); + + if (messages.Count == 2) + { + break; + } + } + + Assert.Collection( + messages, + (firstMessage) => Assert.Contains("Syslog for logging has been deprecated", firstMessage), + (secondMessage) => Assert.Contains("This is a test", secondMessage) + ); + } + + private static IEnumerable GetProviders(Dictionary initialData, string environment = "Production") + { + var provider = GetServiceProvider(initialData, environment); + return provider.GetServices(); + } + + private static IServiceProvider GetServiceProvider(Dictionary initialData, string environment) + { + var config = new ConfigurationBuilder() + .AddInMemoryCollection(initialData) + .Build(); + + var hostingEnvironment = Substitute.For(); + + hostingEnvironment + .EnvironmentName + .Returns(environment); + + var context = new WebHostBuilderContext + { + HostingEnvironment = hostingEnvironment, + Configuration = config, + }; + + var services = new ServiceCollection(); + services.AddLogging(builder => + { + builder.AddSerilog(context); + }); + + return services.BuildServiceProvider(); + } +}