From 4a9840b3cda50a02ba6920e4c367756fe1a540bd Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Thu, 21 May 2026 02:03:06 +0200 Subject: [PATCH] Address identity review feedback --- .../Services/DefaultApiKeyGeneratorAndParser.cs | 3 +-- .../Services/DefaultRandomStringGenerator.cs | 12 +----------- .../Elsa.Identity/Services/DefaultSecretHasher.cs | 12 ++++-------- ...rTests.cs => SecretHasherDefaultOverloadTests.cs} | 2 +- 4 files changed, 7 insertions(+), 22 deletions(-) rename test/unit/Elsa.Identity.UnitTests/Services/{ISecretHasherTests.cs => SecretHasherDefaultOverloadTests.cs} (97%) diff --git a/src/modules/Elsa.Identity/Services/DefaultApiKeyGeneratorAndParser.cs b/src/modules/Elsa.Identity/Services/DefaultApiKeyGeneratorAndParser.cs index 0d91a535b..ccf1e8c62 100644 --- a/src/modules/Elsa.Identity/Services/DefaultApiKeyGeneratorAndParser.cs +++ b/src/modules/Elsa.Identity/Services/DefaultApiKeyGeneratorAndParser.cs @@ -1,4 +1,3 @@ -using System.Security.Cryptography; using System.Text; using Elsa.Identity.Contracts; @@ -13,7 +12,7 @@ public class DefaultApiKeyGeneratorAndParser : IApiKeyGenerator, IApiKeyParser public string Generate(string clientId) { var hexIdentifier = Convert.ToHexString(Encoding.UTF8.GetBytes(clientId)); - var id = new Guid(RandomNumberGenerator.GetBytes(16)).ToString("D"); + var id = Guid.NewGuid().ToString("D"); return $"{hexIdentifier}-{id}"; } diff --git a/src/modules/Elsa.Identity/Services/DefaultRandomStringGenerator.cs b/src/modules/Elsa.Identity/Services/DefaultRandomStringGenerator.cs index 313f51405..87071ea9d 100644 --- a/src/modules/Elsa.Identity/Services/DefaultRandomStringGenerator.cs +++ b/src/modules/Elsa.Identity/Services/DefaultRandomStringGenerator.cs @@ -1,5 +1,4 @@ using System.Security.Cryptography; -using System.Text; using Elsa.Identity.Constants; using Elsa.Identity.Contracts; @@ -11,16 +10,7 @@ public class DefaultRandomStringGenerator : IRandomStringGenerator /// public string Generate(int length = 32, char[]? chars = null) { - var identifierBuilder = new StringBuilder(length); - chars ??= CharacterSequences.AlphanumericSequence; - - for (var i = 0; i < length; i++) - { - var randomIndex = RandomNumberGenerator.GetInt32(chars.Length); - identifierBuilder.Append(chars[randomIndex]); - } - - return identifierBuilder.ToString(); + return RandomNumberGenerator.GetString(chars, length); } } diff --git a/src/modules/Elsa.Identity/Services/DefaultSecretHasher.cs b/src/modules/Elsa.Identity/Services/DefaultSecretHasher.cs index 3dca73026..f55d2bc8d 100644 --- a/src/modules/Elsa.Identity/Services/DefaultSecretHasher.cs +++ b/src/modules/Elsa.Identity/Services/DefaultSecretHasher.cs @@ -66,13 +66,13 @@ public class DefaultSecretHasher : ISecretHasher var storedSecretBytes = hashedSecret.Secret; var saltBytes = hashedSecret.Salt; var clearTextBytes = Encoding.UTF8.GetBytes(clearTextSecret); - byte[]? expectedHash = null; + Span expectedHash = stackalloc byte[KeySize]; byte[]? providedHash = null; byte[]? legacyHash = null; try { - if (TryReadPbkdf2Hash(storedSecretBytes, out var iterationCount, out expectedHash)) + if (TryReadPbkdf2Hash(storedSecretBytes, expectedHash, out var iterationCount)) { providedHash = HashSecret(clearTextBytes, saltBytes, iterationCount); var matches = CryptographicOperations.FixedTimeEquals(providedHash, expectedHash); @@ -101,8 +101,7 @@ public class DefaultSecretHasher : ISecretHasher if (legacyHash is not null) CryptographicOperations.ZeroMemory(legacyHash); - if (expectedHash is not null) - CryptographicOperations.ZeroMemory(expectedHash); + CryptographicOperations.ZeroMemory(expectedHash); } } @@ -160,10 +159,9 @@ public class DefaultSecretHasher : ISecretHasher return sha256.GetHashAndReset(); } - private static bool TryReadPbkdf2Hash(byte[] storedSecretBytes, out int iterationCount, out byte[] hash) + private static bool TryReadPbkdf2Hash(byte[] storedSecretBytes, Span hash, out int iterationCount) { iterationCount = 0; - hash = []; var envelope = storedSecretBytes.AsSpan(); var algorithmSeparatorIndex = envelope.IndexOf(SeparatorByte); @@ -184,14 +182,12 @@ public class DefaultSecretHasher : ISecretHasher return false; var encodedHashBytes = iterationAndHashBytes[(iterationSeparatorIndex + 1)..]; - hash = new byte[KeySize]; var status = Base64.DecodeFromUtf8(encodedHashBytes, hash, out var consumed, out var written); if (status == OperationStatus.Done && consumed == encodedHashBytes.Length && written == KeySize) return true; iterationCount = 0; CryptographicOperations.ZeroMemory(hash); - hash = []; return false; } } diff --git a/test/unit/Elsa.Identity.UnitTests/Services/ISecretHasherTests.cs b/test/unit/Elsa.Identity.UnitTests/Services/SecretHasherDefaultOverloadTests.cs similarity index 97% rename from test/unit/Elsa.Identity.UnitTests/Services/ISecretHasherTests.cs rename to test/unit/Elsa.Identity.UnitTests/Services/SecretHasherDefaultOverloadTests.cs index 5c3b1c8cf..4ed92787a 100644 --- a/test/unit/Elsa.Identity.UnitTests/Services/ISecretHasherTests.cs +++ b/test/unit/Elsa.Identity.UnitTests/Services/SecretHasherDefaultOverloadTests.cs @@ -4,7 +4,7 @@ using Elsa.Identity.Models; namespace Elsa.Identity.UnitTests.Services; -public class ISecretHasherTests +public class SecretHasherDefaultOverloadTests { private readonly ISecretHasher _hasher = new BackwardCompatibleSecretHasher();