Skip to content

Commit

Permalink
Rework license validator to not use ConcurrentDictionary
Browse files Browse the repository at this point in the history 
- ConcurrentDictionary has excellent concurrency performance for reading/writing
  invidivual key/values. But the locking pattern for reading the count and
  getting all keys is quite bad.
- Add tests
- Fixes #1526
  • Loading branch information
@AndersAbel
AndersAbel committed Mar 11, 2024
1 parent d7e9a55 commit f2930e2
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 21 deletions.
77 changes: 57 additions & 20 deletions src/IdentityServer/Licensing/IdentityServerLicenseValidator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using Duende.IdentityServer.Configuration;
using Microsoft.Extensions.Logging;
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;

Expand All @@ -19,8 +18,6 @@ internal class IdentityServerLicenseValidator : LicenseValidator<IdentityServerL
internal readonly static IdentityServerLicenseValidator Instance = new IdentityServerLicenseValidator();

IdentityServerOptions _options;
ConcurrentDictionary<string, byte> _clientIds = new ConcurrentDictionary<string, byte>();
ConcurrentDictionary<string, byte> _issuers = new ConcurrentDictionary<string, byte>();

public void Initialize(ILoggerFactory loggerFactory, IdentityServerOptions options, bool isDevelopment = false)
{
Expand Down Expand Up @@ -56,7 +53,7 @@ protected override void ValidateProductFeatures(List<string> errors)
{
throw new Exception("Invalid License: The BFF edition license is not valid for IdentityServer.");
}

if (_options.KeyManagement.Enabled && !License.KeyManagementFeature)
{
errors.Add("You have automatic key management enabled, but your license does not include that feature of Duende IdentityServer. This feature requires the Business or Enterprise Edition tier of license. Either upgrade your license or disable automatic key management by setting the KeyManagement.Enabled property to false on the IdentityServerOptions.");
Expand All @@ -70,54 +67,94 @@ protected override void WarnForProductFeaturesWhenMissingLicense()
}
}

bool ValidateClientWarned = false;
private void EnsureAdded(ref HashSet<string> hashSet, object lockObject, string key)
{
// Lock free test first.
if (!hashSet.Contains(key))
{
lock (lockObject)
{
// Check again after lock, to quite early if another thread
// already did the job.
if (!hashSet.Contains(key))
{
// The HashSet is not thread safe. And we don't want to lock for every single
// time we use it. Our access pattern should be a lot of reads and a few writes
// so better to create a new copy every time we need to add a value.
var newSet = new HashSet<string>(hashSet)
{
key
};

// Reference assignment is atomic so non-locked readers will handle this.
hashSet = newSet;
}
}
}
}

HashSet<string> _clientIds = new();
object _clientIdLock = new();
bool _validateClientWarned = false;
public void ValidateClient(string clientId)
{
_clientIds.TryAdd(clientId, 1);
if (License != null && !License.ClientLimit.HasValue)
{
return;
}

EnsureAdded(ref _clientIds, _clientIdLock, clientId);

if (License != null)
{
if (License.ClientLimit.HasValue && _clientIds.Count > License.ClientLimit)
if (_clientIds.Count > License.ClientLimit)
{
ErrorLog.Invoke(
"Your license for Duende IdentityServer only permits {clientLimit} number of clients. You have processed requests for {clientCount}. The clients used were: {clients}.",
new object[] { License.ClientLimit, _clientIds.Count, _clientIds.Keys.ToArray() });
[License.ClientLimit, _clientIds.Count, _clientIds.ToArray()]);
}
}
else
{
if (_clientIds.Count > 5 && !ValidateClientWarned)
if (!_validateClientWarned && _clientIds.Count > 5)
{
ValidateClientWarned = true;
_validateClientWarned = true;
WarningLog?.Invoke(
"You do not have a license, and you have processed requests for {clientCount} clients. This number requires a tier of license higher than Starter Edition. The clients used were: {clients}.",
new object[] { _clientIds.Count, _clientIds.Keys.ToArray() });
[_clientIds.Count, _clientIds.ToArray()]);
}
}
}

bool ValidateIssuerWarned = false;
HashSet<string> _issuers = new();
object _issuerLock = new();
bool _validateIssuerWarned = false;
public void ValidateIssuer(string iss)
{
_issuers.TryAdd(iss, 1);
if (License != null && !License.IssuerLimit.HasValue)
{
return;
}

EnsureAdded(ref _issuers, _issuerLock, iss);

if (License != null)
{
if (License.IssuerLimit.HasValue && _issuers.Count > License.IssuerLimit)
if (_issuers.Count > License.IssuerLimit)
{
ErrorLog.Invoke(
"Your license for Duende IdentityServer only permits {issuerLimit} number of issuers. You have processed requests for {issuerCount}. The issuers used were: {issuers}. This might be due to your server being accessed via different URLs or a direct IP and/or you have reverse proxy or a gateway involved. This suggests a network infrastructure configuration problem, or you are deliberately hosting multiple URLs and require an upgraded license.",
new object[] { License.IssuerLimit, _issuers.Count, _issuers.Keys.ToArray() });
[License.IssuerLimit, _issuers.Count, _issuers.ToArray()]);
}
}
else
{
if (_issuers.Count > 1 && !ValidateIssuerWarned)
if (!_validateIssuerWarned && _issuers.Count > 1)
{
ValidateIssuerWarned = true;
_validateIssuerWarned = true;
WarningLog?.Invoke(
"You do not have a license, and you have processed requests for {issuerCount} issuers. If you are deliberately hosting multiple URLs then this number requires a license per issuer, or the Enterprise Edition tier of license. If not then this might be due to your server being accessed via different URLs or a direct IP and/or you have reverse proxy or a gateway involved, and this suggests a network infrastructure configuration problem. The issuers used were: {issuers}.",
new object[] { _issuers.Count, _issuers.Keys.ToArray() });
[_issuers.Count, _issuers.ToArray()]);
}
}
}
Expand Down Expand Up @@ -179,9 +216,9 @@ public void ValidateResourceIndicators(string resourceIndicator)
bool ValidateParWarned = false;
public void ValidatePar()
{
if(License != null)
if (License != null)
{
if(!License.ParFeature)
if (!License.ParFeature)
{
throw new Exception("A request was made to the pushed authorization endpoint. Your license of Duende IdentityServer does not permit pushed authorization. This features requires the Business Edition or higher tier of license.");
}
Expand Down
2 changes: 1 addition & 1 deletion src/IdentityServer/Licensing/LicenseValidator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal class LicenseValidator<T>
protected Action<string, object[]> WarningLog;
protected Action<string, object[]> DebugLog;

protected T License { get; private set; }
protected T License { get; set; }

// cloned copy meant to be accessible in DI
T _copy;
Expand Down
52 changes: 52 additions & 0 deletions test/IdentityServer.UnitTests/Licensing/IdentityServerLicenseValidatorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using System;
using System.Security.Claims;
using Duende;
using Duende.IdentityServer;
using FluentAssertions;
using Xunit;
Expand Down Expand Up @@ -379,4 +380,55 @@ public void invalid_edition_should_fail()
func.Should().Throw<Exception>();
}
}

private class MockLicenseValidator : IdentityServerLicenseValidator
{
public MockLicenseValidator(IdentityServerLicense license)
{
License = license;
ErrorLog = (str, obj) => { ErrorLogCount++; };
WarningLog = (str, obj) => { WarningLogCount++; };
}

public int ErrorLogCount { get; set; }
public int WarningLogCount { get; set; }
}

[Theory]
[Trait("Category", Category)]
[InlineData(false, 5)]
[InlineData(true, 15)]
public void client_count_exceeded_should_warn(bool hasLicense, int allowedClients)
{
var license = hasLicense ? new IdentityServerLicense(new Claim("edition", "business")) : null;
var subject = new MockLicenseValidator(license);

for (int i = 0; i < allowedClients; i++)
{
subject.ValidateClient("client" + i);
}

// Adding the allowed number of clients shouldn't log.
subject.ErrorLogCount.Should().Be(0);
subject.WarningLogCount.Should().Be(0);

// Validating same client again shouldn't log.
subject.ValidateClient("client3");
subject.ErrorLogCount.Should().Be(0);
subject.WarningLogCount.Should().Be(0);

subject.ValidateClient("extra1");
subject.ValidateClient("extra2");

if (hasLicense)
{
subject.ErrorLogCount.Should().Be(2);
subject.WarningLogCount.Should().Be(0);
}
else
{
subject.ErrorLogCount.Should().Be(0);
subject.WarningLogCount.Should().Be(1);
}
}
}

0 comments on commit f2930e2

Please sign in to comment.