From d10789ca1b10882e2c68abe61447704157ab4e1f Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Fri, 6 Dec 2024 10:29:19 -0600 Subject: [PATCH] Feedback from review - Cut warning about license expiration - Simplify usage summary --- hosts/main/Program.cs | 22 +++++- .../BuilderExtensions/Core.cs | 3 +- ...ntityServerApplicationBuilderExtensions.cs | 4 - src/IdentityServer/Hosting/EndpointRouter.cs | 4 - .../Licensing/v2/ILicenseExpirationChecker.cs | 11 --- .../{ILicenseSummary.cs => IUsageSummary.cs} | 21 +----- .../Licensing/v2/LicenseExpirationChecker.cs | 44 ----------- .../Licensing/v2/LicenseSummary.cs | 51 ------------- .../Licensing/v2/ProtocolRequestCounter.cs | 2 +- .../Licensing/v2/UsageSummary.cs | 22 ++++++ .../Common/TestLicenseExpirationChecker.cs | 13 ---- .../Hosting/EndpointRouterTests.cs | 2 +- .../v2/LicenseExpirationCheckerTests.cs | 73 ------------------- 13 files changed, 47 insertions(+), 225 deletions(-) delete mode 100644 src/IdentityServer/Licensing/v2/ILicenseExpirationChecker.cs rename src/IdentityServer/Licensing/v2/{ILicenseSummary.cs => IUsageSummary.cs} (52%) delete mode 100644 src/IdentityServer/Licensing/v2/LicenseExpirationChecker.cs delete mode 100644 src/IdentityServer/Licensing/v2/LicenseSummary.cs create mode 100644 src/IdentityServer/Licensing/v2/UsageSummary.cs delete mode 100644 test/IdentityServer.UnitTests/Common/TestLicenseExpirationChecker.cs delete mode 100644 test/IdentityServer.UnitTests/Licensing/v2/LicenseExpirationCheckerTests.cs diff --git a/hosts/main/Program.cs b/hosts/main/Program.cs index 0823abede..61d4dc068 100644 --- a/hosts/main/Program.cs +++ b/hosts/main/Program.cs @@ -7,6 +7,7 @@ using Serilog.Events; using Serilog.Sinks.SystemConsole.Themes; using System.Globalization; +using System.Text; Console.Title = "IdentityServer (Main)"; @@ -37,11 +38,12 @@ .ConfigureServices() .ConfigurePipeline(); - var license = app.Services.GetRequiredService(); + var usage = app.Services.GetRequiredService(); app.Run(); - Console.Write(license.Summary); + Console.Write(Summary(usage)); + Console.ReadKey(); } catch (Exception ex) { @@ -51,4 +53,18 @@ { Log.Information("Shut down complete"); Log.CloseAndFlush(); -} \ No newline at end of file +} + +string Summary(IUsageSummary usage) +{ + var sb = new StringBuilder(); + sb.AppendLine("IdentityServer Usage Summary:"); + sb.AppendLine(CultureInfo.InvariantCulture, $" License: {usage.LicenseEdition}"); + var features = usage.FeaturesUsed.Any() ? string.Join(", ", usage.FeaturesUsed) : "None"; + sb.AppendLine(CultureInfo.InvariantCulture, $" Business and Enterprise Edition Features Used: {features}"); + sb.AppendLine(CultureInfo.InvariantCulture, $" {usage.UsedClients.Count()} Client Id(s) Used"); + sb.AppendLine(CultureInfo.InvariantCulture, $" {usage.UsedIssuers.Count()} Issuer(s) Used"); + + return sb.ToString(); +} + \ No newline at end of file diff --git a/src/IdentityServer/Configuration/DependencyInjection/BuilderExtensions/Core.cs b/src/IdentityServer/Configuration/DependencyInjection/BuilderExtensions/Core.cs index 1888ed4e8..0627693d8 100644 --- a/src/IdentityServer/Configuration/DependencyInjection/BuilderExtensions/Core.cs +++ b/src/IdentityServer/Configuration/DependencyInjection/BuilderExtensions/Core.cs @@ -208,7 +208,6 @@ public static IIdentityServerBuilder AddCoreServices(this IIdentityServerBuilder builder.Services.AddTransient(services => IdentityServerLicenseValidator.Instance.GetLicense()); builder.Services.AddSingleton(); - builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); @@ -403,7 +402,7 @@ public static IIdentityServerBuilder AddDefaultSecretValidators(this IIdentitySe /// public static IIdentityServerBuilder AddLicenseSummary(this IIdentityServerBuilder builder) { - builder.Services.AddSingleton(); + builder.Services.AddSingleton(); return builder; } diff --git a/src/IdentityServer/Configuration/IdentityServerApplicationBuilderExtensions.cs b/src/IdentityServer/Configuration/IdentityServerApplicationBuilderExtensions.cs index 7d83433ff..d26505d15 100644 --- a/src/IdentityServer/Configuration/IdentityServerApplicationBuilderExtensions.cs +++ b/src/IdentityServer/Configuration/IdentityServerApplicationBuilderExtensions.cs @@ -79,9 +79,6 @@ internal static void Validate(this IApplicationBuilder app) var env = serviceProvider.GetRequiredService(); IdentityServerLicenseValidator.Instance.Initialize(loggerFactory, options, env.IsDevelopment()); - var licenseExpiration = serviceProvider.GetRequiredService(); - licenseExpiration.CheckExpiration(); - if (options.KeyManagement.Enabled) { var licenseUsage = serviceProvider.GetRequiredService(); @@ -109,7 +106,6 @@ internal static void Validate(this IApplicationBuilder app) private static void ValidateLicenseServices(IServiceProvider serviceProvider) { VerifyDefaultServiceRegistration(serviceProvider); - VerifyDefaultServiceRegistration(serviceProvider); VerifyDefaultServiceRegistration(serviceProvider); VerifyDefaultServiceRegistration(serviceProvider); } diff --git a/src/IdentityServer/Hosting/EndpointRouter.cs b/src/IdentityServer/Hosting/EndpointRouter.cs index 7fd2e7e58..35fabbb22 100644 --- a/src/IdentityServer/Hosting/EndpointRouter.cs +++ b/src/IdentityServer/Hosting/EndpointRouter.cs @@ -16,20 +16,17 @@ internal class EndpointRouter : IEndpointRouter { private readonly IEnumerable _endpoints; private readonly IProtocolRequestCounter _requestCounter; - private readonly ILicenseExpirationChecker _expirationChecker; private readonly IdentityServerOptions _options; private readonly ILogger _logger; public EndpointRouter( IEnumerable endpoints, IProtocolRequestCounter requestCounter, - ILicenseExpirationChecker expirationChecker, IdentityServerOptions options, ILogger logger) { _endpoints = endpoints; _requestCounter = requestCounter; - _expirationChecker = expirationChecker; _options = options; _logger = logger; } @@ -47,7 +44,6 @@ public IEndpointHandler Find(HttpContext context) _logger.LogDebug("Request path {path} matched to endpoint type {endpoint}", context.Request.Path, endpointName); _requestCounter.Increment(); - _expirationChecker.CheckExpiration(); return GetEndpointHandler(endpoint, context); } diff --git a/src/IdentityServer/Licensing/v2/ILicenseExpirationChecker.cs b/src/IdentityServer/Licensing/v2/ILicenseExpirationChecker.cs deleted file mode 100644 index 04a1fde60..000000000 --- a/src/IdentityServer/Licensing/v2/ILicenseExpirationChecker.cs +++ /dev/null @@ -1,11 +0,0 @@ -// Copyright (c) Duende Software. All rights reserved. -// See LICENSE in the project root for license information. - -#nullable enable - -namespace Duende.IdentityServer.Licensing.v2; - -internal interface ILicenseExpirationChecker -{ - void CheckExpiration(); -} diff --git a/src/IdentityServer/Licensing/v2/ILicenseSummary.cs b/src/IdentityServer/Licensing/v2/IUsageSummary.cs similarity index 52% rename from src/IdentityServer/Licensing/v2/ILicenseSummary.cs rename to src/IdentityServer/Licensing/v2/IUsageSummary.cs index 227675ada..5738cae70 100644 --- a/src/IdentityServer/Licensing/v2/ILicenseSummary.cs +++ b/src/IdentityServer/Licensing/v2/IUsageSummary.cs @@ -10,32 +10,17 @@ namespace Duende.IdentityServer.Licensing.v2; /// /// Summarizes the usage of IdentityServer /// -public interface ILicenseSummary +public interface IUsageSummary { - /// - /// Summarizes the usage of IdentityServer, including licensed features, clients, and issuers. - /// - public string Summary { get; } - /// /// Gets the license edition. /// public string LicenseEdition { get; } /// - /// Gets the licensed enterprise edition features that have been used. - /// - IEnumerable EnterpriseFeaturesUsed { get; } - - /// - /// Gets the licensed business edition features that have been used. - /// - IEnumerable BusinessFeaturesUsed { get; } - - /// - /// Gets other licensed features that have been used. + /// Gets the licensed features that have been used. /// - IEnumerable OtherFeaturesUsed { get; } + IEnumerable FeaturesUsed { get; } /// /// Gets the client ids that have been used. diff --git a/src/IdentityServer/Licensing/v2/LicenseExpirationChecker.cs b/src/IdentityServer/Licensing/v2/LicenseExpirationChecker.cs deleted file mode 100644 index 0f9d72c97..000000000 --- a/src/IdentityServer/Licensing/v2/LicenseExpirationChecker.cs +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright (c) Duende Software. All rights reserved. -// See LICENSE in the project root for license information. - -#nullable enable - -using System; -using Microsoft.Extensions.Logging; - -namespace Duende.IdentityServer.Licensing.v2; - -internal class LicenseExpirationChecker : ILicenseExpirationChecker -{ - private readonly ILicenseAccessor _license; - private readonly TimeProvider _timeProvider; - private readonly ILogger _logger; - - public LicenseExpirationChecker( - ILicenseAccessor license, - TimeProvider timeProvider, - ILoggerFactory loggerFactory) - { - _license = license; - _timeProvider = timeProvider; - _logger = loggerFactory.CreateLogger("Duende.IdentityServer.License"); - } - - /// - /// The amount of time an expired license may be used for before we shut down the server. - /// - private readonly TimeSpan _gracePeriod = TimeSpan.FromDays(90); - - private bool _expiredLicenseWarned; - - public void CheckExpiration() - { - if (!_expiredLicenseWarned && !_license.Current.Redistribution && IsExpired) - { - _expiredLicenseWarned = true; - _logger.LogError("The IdentityServer license is expired. In a future version of IdentityServer, expired licenses will stop the server after {gracePeriod} days.", _gracePeriod.Days); - } - } - - private bool IsExpired => _timeProvider.GetUtcNow() > _license.Current.Expiration; -} diff --git a/src/IdentityServer/Licensing/v2/LicenseSummary.cs b/src/IdentityServer/Licensing/v2/LicenseSummary.cs deleted file mode 100644 index 6cfd376c6..000000000 --- a/src/IdentityServer/Licensing/v2/LicenseSummary.cs +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright (c) Duende Software. All rights reserved. -// See LICENSE in the project root for license information. - -#nullable enable - -using System.Collections.Generic; -using System.Linq; -using System.Text; - -namespace Duende.IdentityServer.Licensing.v2; - -internal class LicenseSummary(ILicenseAccessor license, ILicenseUsageService usage) : ILicenseSummary -{ - public string Summary - { - get - { - var sb = new StringBuilder(); - sb.AppendLine("IdentityServer Usage Summary:"); - sb.AppendLine($"\tLicense: {LicenseEdition}"); - - AppendSummary(sb, "Client Id", usage.UsedClients); - AppendSummary(sb, "Business Edition Feature", usage.BusinessFeaturesUsed); - AppendSummary(sb, "Enterprise Edition Feature", usage.EnterpriseFeaturesUsed); - AppendSummary(sb, "Other Feature", usage.OtherFeaturesUsed); - AppendSummary(sb, "Issuer", usage.UsedIssuers); - - return sb.ToString(); - } - } - - private void AppendSummary(StringBuilder sb, string label, IReadOnlyCollection items) - { - if (items.Count == 1) - { - sb.AppendLine($"\t{items.Count} {label} Used: {items.Single()}"); - } - else if (items.Count > 1) - { - sb.AppendLine($"\t{items.Count} {label}s Used: {string.Join(", ", items)}"); - } - } - - public string LicenseEdition => license.Current.Edition?.ToString() ?? "None"; - public IEnumerable UsedClients => usage.UsedClients; - public IEnumerable UsedIssuers => usage.UsedIssuers; - - public IEnumerable EnterpriseFeaturesUsed => usage.EnterpriseFeaturesUsed.Select(f => f.ToString()); - public IEnumerable BusinessFeaturesUsed => usage.BusinessFeaturesUsed.Select(f => f.ToString()); - public IEnumerable OtherFeaturesUsed => usage.OtherFeaturesUsed.Select(f => f.ToString()); -} diff --git a/src/IdentityServer/Licensing/v2/ProtocolRequestCounter.cs b/src/IdentityServer/Licensing/v2/ProtocolRequestCounter.cs index 8bf7e6bf8..3031d425e 100644 --- a/src/IdentityServer/Licensing/v2/ProtocolRequestCounter.cs +++ b/src/IdentityServer/Licensing/v2/ProtocolRequestCounter.cs @@ -21,7 +21,7 @@ public ProtocolRequestCounter( /// /// The number of protocol requests allowed for unlicensed use. This should only be changed in tests. /// - internal uint Threshold = 1000; + internal uint Threshold = 500; private readonly ILicenseAccessor _license; private readonly ILogger _logger; diff --git a/src/IdentityServer/Licensing/v2/UsageSummary.cs b/src/IdentityServer/Licensing/v2/UsageSummary.cs new file mode 100644 index 000000000..0d32bb9f1 --- /dev/null +++ b/src/IdentityServer/Licensing/v2/UsageSummary.cs @@ -0,0 +1,22 @@ +// Copyright (c) Duende Software. All rights reserved. +// See LICENSE in the project root for license information. + +#nullable enable + +using System.Collections.Generic; +using System.Linq; + +namespace Duende.IdentityServer.Licensing.v2; + +internal class UsageSummary(ILicenseAccessor license, ILicenseUsageService usage) : IUsageSummary +{ + public string LicenseEdition => license.Current.Edition?.ToString() ?? "None"; + public IEnumerable UsedClients => usage.UsedClients; + public IEnumerable UsedIssuers => usage.UsedIssuers; + + public IEnumerable FeaturesUsed => + usage.EnterpriseFeaturesUsed + .Concat(usage.BusinessFeaturesUsed) + .Concat(usage.OtherFeaturesUsed) + .Select(f => f.ToString()); +} diff --git a/test/IdentityServer.UnitTests/Common/TestLicenseExpirationChecker.cs b/test/IdentityServer.UnitTests/Common/TestLicenseExpirationChecker.cs deleted file mode 100644 index b5178e585..000000000 --- a/test/IdentityServer.UnitTests/Common/TestLicenseExpirationChecker.cs +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright (c) Duende Software. All rights reserved. -// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. - -using Duende.IdentityServer.Licensing.v2; - -namespace UnitTests.Common; - -public class TestLicenseExpirationChecker : ILicenseExpirationChecker -{ - public void CheckExpiration() - { - } -} \ No newline at end of file diff --git a/test/IdentityServer.UnitTests/Hosting/EndpointRouterTests.cs b/test/IdentityServer.UnitTests/Hosting/EndpointRouterTests.cs index 645b6c5de..a99f1d0d2 100644 --- a/test/IdentityServer.UnitTests/Hosting/EndpointRouterTests.cs +++ b/test/IdentityServer.UnitTests/Hosting/EndpointRouterTests.cs @@ -27,7 +27,7 @@ public EndpointRouterTests() _pathMap = new Dictionary(); _endpoints = new List(); _options = new IdentityServerOptions(); - _subject = new EndpointRouter(_endpoints, new TestProtocolRequestCounter(), new TestLicenseExpirationChecker(), _options, TestLogger.Create()); + _subject = new EndpointRouter(_endpoints, new TestProtocolRequestCounter(), _options, TestLogger.Create()); } [Fact] diff --git a/test/IdentityServer.UnitTests/Licensing/v2/LicenseExpirationCheckerTests.cs b/test/IdentityServer.UnitTests/Licensing/v2/LicenseExpirationCheckerTests.cs deleted file mode 100644 index 0130fc521..000000000 --- a/test/IdentityServer.UnitTests/Licensing/v2/LicenseExpirationCheckerTests.cs +++ /dev/null @@ -1,73 +0,0 @@ -using System; -using Duende.IdentityServer.Licensing.v2; -using FluentAssertions; -using Microsoft.Extensions.Logging.Testing; -using Microsoft.Extensions.Time.Testing; -using Xunit; - -namespace IdentityServer.UnitTests.Licensing.v2; - -public class LicenseExpirationCheckerTests -{ - - private readonly LicenseExpirationChecker _expirationCheck; - private readonly TestLicenseAccessor _license; - private readonly FakeLogger _logger; - private readonly FakeTimeProvider _timeProvider; - - // There is no particular significance to 2024-01-01, this is just an - // arbitrary date that we will use as a base for expiration tests - private readonly DateTimeOffset _januaryFirst = new DateTimeOffset( - new DateOnly(2024, 1, 1), - new TimeOnly(12, 00), - TimeSpan.Zero); - - public LicenseExpirationCheckerTests() - { - _license = new TestLicenseAccessor(); - _timeProvider = new FakeTimeProvider(); - _logger = new FakeLogger(); - _expirationCheck = new LicenseExpirationChecker(_license, _timeProvider, new StubLoggerFactory(_logger)); - } - - [Fact] - public void warning_is_logged_for_expired_license() - { - _timeProvider.SetUtcNow(_januaryFirst); - _license.Current = LicenseFactory.Create(LicenseEdition.Enterprise, _januaryFirst.Subtract(TimeSpan.FromDays(1))); - - _expirationCheck.CheckExpiration(); - var expiration = _license.Current.Expiration?.ToString("yyyy-MM-dd"); - // REMINDER - If this test needs to change because the log message was updated, so should no_warning_is_logged_for_unexpired_license - _logger.Collector.GetSnapshot().Should() - .ContainSingle(r => - r.Message == $"The IdentityServer license is expired. In a future version of IdentityServer, expired licenses will stop the server after 90 days."); - } - - [Fact] - public void no_warning_is_logged_for_unexpired_license() - { - _timeProvider.SetUtcNow(_januaryFirst); - _license.Current = LicenseFactory.Create(LicenseEdition.Enterprise, _januaryFirst); - - _expirationCheck.CheckExpiration(); - var expiration = _license.Current.Expiration?.ToString("yyyy-MM-dd"); - _logger.Collector.GetSnapshot().Should() - .NotContain(r => - r.Message == $"The IdentityServer license is expired. In a future version of IdentityServer, expired licenses will stop the server after 90 days."); - } - - [Fact] - public void no_expired_license_warning_for_redistribution_license() - { - _timeProvider.SetUtcNow(_januaryFirst); - _license.Current = LicenseFactory.Create(LicenseEdition.Enterprise, _januaryFirst.Subtract(TimeSpan.FromDays(1)), redistribution: true); - - _license.Current.IsEnabled(LicenseFeature.Redistribution).Should().BeTrue(); - - - _expirationCheck.CheckExpiration(); - _logger.Collector.GetSnapshot().Should() - .BeEmpty(); - } -} \ No newline at end of file