From 6a7ff8d48acb19a3df4eddcf0cfbd1fecd977123 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Thu, 21 May 2026 00:04:21 +0200 Subject: [PATCH] Address identity secret hashing review feedback --- .../Services/DefaultSecretHasher.cs | 5 +- .../DefaultUserCredentialsValidator.cs | 2 +- .../Services/DefaultSecretHasherTests.cs | 53 +++++++++++++++++-- 3 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/modules/Elsa.Identity/Services/DefaultSecretHasher.cs b/src/modules/Elsa.Identity/Services/DefaultSecretHasher.cs index 696ba255a..aaf27bb15 100644 --- a/src/modules/Elsa.Identity/Services/DefaultSecretHasher.cs +++ b/src/modules/Elsa.Identity/Services/DefaultSecretHasher.cs @@ -1,3 +1,4 @@ +using System.Globalization; using System.Security.Cryptography; using System.Text; using Elsa.Identity.Contracts; @@ -81,7 +82,7 @@ public class DefaultSecretHasher : ISecretHasher { var hash = HashSecret(secret, salt, DefaultIterationCount); var encodedHash = Convert.ToBase64String(hash); - return Encoding.UTF8.GetBytes($"{Algorithm}{Separator}{DefaultIterationCount}{Separator}{encodedHash}"); + return Encoding.UTF8.GetBytes($"{Algorithm}{Separator}{DefaultIterationCount.ToString(CultureInfo.InvariantCulture)}{Separator}{encodedHash}"); } /// @@ -110,7 +111,7 @@ public class DefaultSecretHasher : ISecretHasher if (segments.Length != 3 || !string.Equals(segments[0], Algorithm, StringComparison.Ordinal)) return false; - if (!int.TryParse(segments[1], out iterationCount) || iterationCount <= 0 || iterationCount > MaxIterationCount) + if (!int.TryParse(segments[1], NumberStyles.None, CultureInfo.InvariantCulture, out iterationCount) || iterationCount <= 0 || iterationCount > MaxIterationCount) return false; try diff --git a/src/modules/Elsa.Identity/Services/DefaultUserCredentialsValidator.cs b/src/modules/Elsa.Identity/Services/DefaultUserCredentialsValidator.cs index 3caead79b..b0e2b2dcb 100644 --- a/src/modules/Elsa.Identity/Services/DefaultUserCredentialsValidator.cs +++ b/src/modules/Elsa.Identity/Services/DefaultUserCredentialsValidator.cs @@ -71,7 +71,7 @@ public class DefaultUserCredentialsValidator : IUserCredentialsValidator user.HashedPasswordSalt = previousHashedPasswordSalt; throw; } - catch (InvalidOperationException e) + catch (Exception e) { user.HashedPassword = previousHashedPassword; user.HashedPasswordSalt = previousHashedPasswordSalt; diff --git a/test/unit/Elsa.Identity.UnitTests/Services/DefaultSecretHasherTests.cs b/test/unit/Elsa.Identity.UnitTests/Services/DefaultSecretHasherTests.cs index db76a5d6b..f43d12557 100644 --- a/test/unit/Elsa.Identity.UnitTests/Services/DefaultSecretHasherTests.cs +++ b/test/unit/Elsa.Identity.UnitTests/Services/DefaultSecretHasherTests.cs @@ -1,3 +1,4 @@ +using System.Globalization; using System.Security.Cryptography; using System.Text; using Elsa.Common.Services; @@ -23,6 +24,30 @@ public class DefaultSecretHasherTests Assert.False(needsRehash); } + [Fact] + public void HashSecret_FormatsPbkdf2EnvelopeUsingInvariantCulture() + { + using var _ = new CultureScope("ar-SA"); + + var hashedSecret = _hasher.HashSecret("secret"); + + Assert.StartsWith("pbkdf2-sha256$600000$", Encoding.UTF8.GetString(hashedSecret.Secret)); + Assert.True(_hasher.VerifySecret("secret", hashedSecret, out var needsRehash)); + Assert.False(needsRehash); + } + + [Fact] + public void VerifySecret_ParsesPbkdf2EnvelopeUsingInvariantCulture() + { + var hashedSecret = CreatePbkdf2Hash("secret", 1); + using var _ = new CultureScope("ar-SA"); + + var verified = _hasher.VerifySecret("secret", hashedSecret, out var needsRehash); + + Assert.True(verified); + Assert.True(needsRehash); + } + [Fact] public void HashSecret_UsesUniqueSalts() { @@ -164,7 +189,7 @@ public class DefaultSecretHasherTests HashedPassword = legacyHash.EncodeSecret(), HashedPasswordSalt = legacyHash.EncodeSalt() }; - var userStore = new FailingUserStore(user); + var userStore = new FailingUserStore(user, new TimeoutException("Save timed out.")); var validator = new DefaultUserCredentialsValidator(new StoreBasedUserProvider(userStore), userStore, _hasher); var validatedUser = await validator.ValidateAsync("alice", "secret"); @@ -260,15 +285,16 @@ public class DefaultSecretHasherTests var salt = RandomNumberGenerator.GetBytes(32); var secretBytes = Encoding.UTF8.GetBytes(secret); var hash = Rfc2898DeriveBytes.Pbkdf2(secretBytes, salt, iterationCount, HashAlgorithmName.SHA256, 32); - var envelope = Encoding.UTF8.GetBytes($"pbkdf2-sha256${iterationCount}${Convert.ToBase64String(hash)}"); + var envelope = Encoding.UTF8.GetBytes($"pbkdf2-sha256${iterationCount.ToString(CultureInfo.InvariantCulture)}${Convert.ToBase64String(hash)}"); return HashedSecret.FromBytes(envelope, salt); } - private sealed class FailingUserStore(User user) : IUserStore + private sealed class FailingUserStore(User user, Exception? exception = null) : IUserStore { private readonly User _user = user; + private readonly Exception _exception = exception ?? new InvalidOperationException("Save failed."); - public Task SaveAsync(User user, CancellationToken cancellationToken = default) => throw new InvalidOperationException("Save failed."); + public Task SaveAsync(User user, CancellationToken cancellationToken = default) => throw _exception; public Task DeleteAsync(UserFilter filter, CancellationToken cancellationToken = default) => Task.CompletedTask; @@ -321,4 +347,23 @@ public class DefaultSecretHasherTests return Task.FromResult(application); } } + + private sealed class CultureScope : IDisposable + { + private readonly CultureInfo _currentCulture = CultureInfo.CurrentCulture; + private readonly CultureInfo _currentUICulture = CultureInfo.CurrentUICulture; + + public CultureScope(string cultureName) + { + var culture = CultureInfo.GetCultureInfo(cultureName); + CultureInfo.CurrentCulture = culture; + CultureInfo.CurrentUICulture = culture; + } + + public void Dispose() + { + CultureInfo.CurrentCulture = _currentCulture; + CultureInfo.CurrentUICulture = _currentUICulture; + } + } }