From e2587f74b5d35684af71bcbf2c913c50c9d82215 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 20 May 2026 14:11:19 +0200 Subject: [PATCH] Address identity validator compatibility feedback --- doc/changelogs/3.6.0.md | 2 +- .../DefaultApplicationCredentialsValidator.cs | 13 ++- .../Services/DefaultSecretHasher.cs | 7 +- .../DefaultUserCredentialsValidator.cs | 13 ++- .../Services/DefaultSecretHasherTests.cs | 81 +++++++++++++++++++ 5 files changed, 108 insertions(+), 8 deletions(-) diff --git a/doc/changelogs/3.6.0.md b/doc/changelogs/3.6.0.md index c7ec6b0b7..e57ee8208 100644 --- a/doc/changelogs/3.6.0.md +++ b/doc/changelogs/3.6.0.md @@ -6,7 +6,7 @@ Compare: [`3.5.3...3.6.0`](https://github.com/elsa-workflows/elsa-core/compare/3 ## ⚠️ Breaking changes / upgrade notes -- **Identity secret hashing hardened**: New user passwords, client secrets, and API keys are hashed with PBKDF2-SHA256 using 600,000 iterations, per-record salts, and version metadata instead of fast SHA-256 hashes. Existing legacy hashes still verify and are upgraded opportunistically after successful login/API-key validation. Generated identity secrets now use cryptographic randomness. `DefaultUserCredentialsValidator` and `DefaultApplicationCredentialsValidator` now require `IUserStore` and `IApplicationStore` respectively; update direct instantiations to pass these additional dependencies. +- **Identity secret hashing hardened**: New user passwords, client secrets, and API keys are hashed with PBKDF2-SHA256 using 600,000 iterations, per-record salts, and version metadata instead of fast SHA-256 hashes. Existing legacy hashes still verify and are upgraded opportunistically after successful login/API-key validation. Generated identity secrets now use cryptographic randomness. New `DefaultUserCredentialsValidator` and `DefaultApplicationCredentialsValidator` constructor overloads accept `IUserStore` and `IApplicationStore` respectively to persist opportunistic hash upgrades; existing direct instantiations remain supported but do not persist rehashes unless the store dependency is supplied. - **EF Core package names have changed**: EF Core persistence packages were renamed from `Elsa.EntityFrameworkCore.*` to `Elsa.Persistence.EFCore.*`. If your application references any of the old package names, you must update them to the new package names when upgrading to 3.6.0. Be sure to review your project files, internal package feeds, CI pipelines, and deployment manifests for old package references. diff --git a/src/modules/Elsa.Identity/Services/DefaultApplicationCredentialsValidator.cs b/src/modules/Elsa.Identity/Services/DefaultApplicationCredentialsValidator.cs index 148396d45..f70c28c3c 100644 --- a/src/modules/Elsa.Identity/Services/DefaultApplicationCredentialsValidator.cs +++ b/src/modules/Elsa.Identity/Services/DefaultApplicationCredentialsValidator.cs @@ -13,10 +13,17 @@ public class DefaultApplicationCredentialsValidator : IApplicationCredentialsVal { private readonly IApiKeyParser _apiKeyParser; private readonly IApplicationProvider _applicationProvider; - private readonly IApplicationStore _applicationStore; + private readonly IApplicationStore? _applicationStore; private readonly ISecretHasher _secretHasher; private readonly ILogger _logger; + /// + /// Initializes a new instance of the class. + /// + public DefaultApplicationCredentialsValidator(IApiKeyParser apiKeyParser, IApplicationProvider applicationProvider, ISecretHasher secretHasher) : this(apiKeyParser, applicationProvider, null, secretHasher, null) + { + } + /// /// Initializes a new instance of the class. /// @@ -27,7 +34,7 @@ public class DefaultApplicationCredentialsValidator : IApplicationCredentialsVal /// /// Initializes a new instance of the class. /// - public DefaultApplicationCredentialsValidator(IApiKeyParser apiKeyParser, IApplicationProvider applicationProvider, IApplicationStore applicationStore, ISecretHasher secretHasher, ILogger? logger) + public DefaultApplicationCredentialsValidator(IApiKeyParser apiKeyParser, IApplicationProvider applicationProvider, IApplicationStore? applicationStore, ISecretHasher secretHasher, ILogger? logger) { _apiKeyParser = apiKeyParser; _applicationProvider = applicationProvider; @@ -53,7 +60,7 @@ public class DefaultApplicationCredentialsValidator : IApplicationCredentialsVal if (!isValidApiKey) return null; - if (needsRehash) + if (needsRehash && _applicationStore != null) { var hashedApiKey = _secretHasher.HashSecret(apiKey); application.HashedApiKey = hashedApiKey.EncodeSecret(); diff --git a/src/modules/Elsa.Identity/Services/DefaultSecretHasher.cs b/src/modules/Elsa.Identity/Services/DefaultSecretHasher.cs index 46a4dec67..f727ec35e 100644 --- a/src/modules/Elsa.Identity/Services/DefaultSecretHasher.cs +++ b/src/modules/Elsa.Identity/Services/DefaultSecretHasher.cs @@ -107,7 +107,12 @@ public class DefaultSecretHasher : ISecretHasher try { hash = Convert.FromBase64String(segments[2]); - return true; + if (hash.Length == KeySize) + return true; + + iterationCount = 0; + hash = []; + return false; } catch (FormatException) { diff --git a/src/modules/Elsa.Identity/Services/DefaultUserCredentialsValidator.cs b/src/modules/Elsa.Identity/Services/DefaultUserCredentialsValidator.cs index 3aa880091..6f2cc8cf9 100644 --- a/src/modules/Elsa.Identity/Services/DefaultUserCredentialsValidator.cs +++ b/src/modules/Elsa.Identity/Services/DefaultUserCredentialsValidator.cs @@ -12,10 +12,17 @@ namespace Elsa.Identity.Services; public class DefaultUserCredentialsValidator : IUserCredentialsValidator { private readonly IUserProvider _userProvider; - private readonly IUserStore _userStore; + private readonly IUserStore? _userStore; private readonly ISecretHasher _secretHasher; private readonly ILogger _logger; + /// + /// Initializes a new instance of the class. + /// + public DefaultUserCredentialsValidator(IUserProvider userProvider, ISecretHasher secretHasher) : this(userProvider, null, secretHasher, null) + { + } + /// /// Initializes a new instance of the class. /// @@ -26,7 +33,7 @@ public class DefaultUserCredentialsValidator : IUserCredentialsValidator /// /// Initializes a new instance of the class. /// - public DefaultUserCredentialsValidator(IUserProvider userProvider, IUserStore userStore, ISecretHasher secretHasher, ILogger? logger) + public DefaultUserCredentialsValidator(IUserProvider userProvider, IUserStore? userStore, ISecretHasher secretHasher, ILogger? logger) { _userProvider = userProvider; _userStore = userStore; @@ -47,7 +54,7 @@ public class DefaultUserCredentialsValidator : IUserCredentialsValidator if (!isValidPassword) return null; - if (needsRehash) + if (needsRehash && _userStore != null) { var hashedPassword = _secretHasher.HashSecret(password); user.HashedPassword = hashedPassword.EncodeSecret(); diff --git a/test/unit/Elsa.Identity.UnitTests/Services/DefaultSecretHasherTests.cs b/test/unit/Elsa.Identity.UnitTests/Services/DefaultSecretHasherTests.cs index 2012864e9..7f51601af 100644 --- a/test/unit/Elsa.Identity.UnitTests/Services/DefaultSecretHasherTests.cs +++ b/test/unit/Elsa.Identity.UnitTests/Services/DefaultSecretHasherTests.cs @@ -68,6 +68,19 @@ public class DefaultSecretHasherTests Assert.False(needsRehash); } + [Fact] + public void VerifySecret_RejectsPbkdf2HashWithInvalidKeyLength() + { + var salt = _hasher.GenerateSalt(); + var storedHash = Encoding.UTF8.GetBytes("pbkdf2-sha256$600000$" + Convert.ToBase64String(RandomNumberGenerator.GetBytes(16))); + var hashedSecret = HashedSecret.FromBytes(storedHash, salt); + + var verified = _hasher.VerifySecret("secret", hashedSecret, out var needsRehash); + + Assert.False(verified); + Assert.False(needsRehash); + } + [Fact] public async Task ValidateAsync_RehashesLegacyUserPassword() { @@ -137,6 +150,26 @@ public class DefaultSecretHasherTests Assert.Same(user, validatedUser); } + [Fact] + public async Task ValidateAsync_WithOldUserValidatorConstructor_ReturnsUserWithoutPersistingRehash() + { + var legacyHash = CreateLegacyHash("secret"); + var encodedLegacyHash = legacyHash.EncodeSecret(); + var user = new User + { + Id = "user-1", + Name = "alice", + HashedPassword = encodedLegacyHash, + HashedPasswordSalt = legacyHash.EncodeSalt() + }; + var validator = new DefaultUserCredentialsValidator(new StaticUserProvider(user), _hasher); + + var validatedUser = await validator.ValidateAsync("alice", "secret"); + + Assert.Same(user, validatedUser); + Assert.Equal(encodedLegacyHash, user.HashedPassword); + } + [Fact] public async Task ValidateAsync_ReturnsApplicationWhenLegacyApiKeyRehashSaveFails() { @@ -162,6 +195,32 @@ public class DefaultSecretHasherTests Assert.Same(application, validatedApplication); } + [Fact] + public async Task ValidateAsync_WithOldApplicationValidatorConstructor_ReturnsApplicationWithoutPersistingRehash() + { + var apiKeyGenerator = new DefaultApiKeyGeneratorAndParser(); + var apiKey = apiKeyGenerator.Generate("client-1"); + var legacyHash = CreateLegacyHash(apiKey); + var encodedLegacyHash = legacyHash.EncodeSecret(); + var application = new Application + { + Id = "app-1", + ClientId = "client-1", + Name = "Client 1", + HashedApiKey = encodedLegacyHash, + HashedApiKeySalt = legacyHash.EncodeSalt(), + HashedClientSecret = "", + HashedClientSecretSalt = "" + }; + var applicationProvider = new StaticApplicationProvider(application); + var validator = new DefaultApplicationCredentialsValidator(apiKeyGenerator, applicationProvider, _hasher); + + var validatedApplication = await validator.ValidateAsync(apiKey); + + Assert.Same(application, validatedApplication); + Assert.Equal(encodedLegacyHash, application.HashedApiKey); + } + private static HashedSecret CreateLegacyHash(string secret) { var salt = RandomNumberGenerator.GetBytes(32); @@ -214,4 +273,26 @@ public class DefaultSecretHasherTests return Task.FromResult(application); } } + + private sealed class StaticUserProvider(User user) : IUserProvider + { + private readonly User _user = user; + + public Task FindAsync(UserFilter filter, CancellationToken cancellationToken = default) + { + var user = filter.Apply(new[] { _user }.AsQueryable()).FirstOrDefault(); + return Task.FromResult(user); + } + } + + private sealed class StaticApplicationProvider(Application application) : IApplicationProvider + { + private readonly Application _application = application; + + public Task FindAsync(ApplicationFilter filter, CancellationToken cancellationToken = default) + { + var application = filter.Apply(new[] { _application }.AsQueryable()).FirstOrDefault(); + return Task.FromResult(application); + } + } }