(refactor): code review

This commit is contained in:
Maksym Sadovnychyy 2025-11-13 21:04:15 +01:00
parent e2f930587a
commit c6f7e47240
8 changed files with 122 additions and 138 deletions

View File

@ -987,7 +987,7 @@ using System.Security.Cryptography;
using MaksIT.Core.Security.JWK; using MaksIT.Core.Security.JWK;
using var rsa = RSA.Create(2048); using var rsa = RSA.Create(2048);
var result = JwkGenerator.TryGenerateFromRCA(rsa, out var jwk, out var errorMessage); var result = JwkGenerator.TryGenerateFromRSA(rsa, out var jwk, out var errorMessage);
if (result) if (result)
{ {
// jwk contains KeyType, RsaExponent, RsaModulus // jwk contains KeyType, RsaExponent, RsaModulus
@ -1004,9 +1004,9 @@ else
#### API #### API
```csharp ```csharp
public static bool TryGenerateFromRCA( public static bool TryGenerateFromRSA(
RSA rsa, RSA rsa,
out Jwk? jwk, [NotNullWhen(true)] out Jwk? jwk,
[NotNullWhen(false)] out string? errorMessage [NotNullWhen(false)] out string? errorMessage
) )
``` ```
@ -1068,7 +1068,7 @@ public static bool TryEncode(
RSA rsa, RSA rsa,
Jwk jwk, Jwk jwk,
JwsHeader protectedHeader, JwsHeader protectedHeader,
out JwsMessage? message, [NotNullWhen(true)]out JwsMessage? message,
[NotNullWhen(false)] out string? errorMessage [NotNullWhen(false)] out string? errorMessage
) )
``` ```
@ -1080,7 +1080,7 @@ public static bool TryEncode<T>(
Jwk jwk, Jwk jwk,
JwsHeader protectedHeader, JwsHeader protectedHeader,
T? payload, T? payload,
out JwsMessage? message, [NotNullWhen(true)] out JwsMessage? message,
[NotNullWhen(false)] out string? errorMessage [NotNullWhen(false)] out string? errorMessage
) )
``` ```
@ -1153,7 +1153,7 @@ else
```csharp ```csharp
public static bool TryGetSha256Thumbprint( public static bool TryGetSha256Thumbprint(
Jwk jwk, Jwk jwk,
out string? thumbprint, [NotNullWhen(true)] out string? thumbprint,
[NotNullWhen(false)] out string? errorMessage [NotNullWhen(false)] out string? errorMessage
) )
``` ```
@ -1163,7 +1163,7 @@ public static bool TryGetSha256Thumbprint(
public static bool TryGetKeyAuthorization( public static bool TryGetKeyAuthorization(
Jwk jwk, Jwk jwk,
string token, string token,
out string? keyAuthorization, [NotNullWhen(true)] out string? keyAuthorization,
[NotNullWhen(false)] out string? errorMessage [NotNullWhen(false)] out string? errorMessage
) )
``` ```

View File

@ -4,67 +4,61 @@ using MaksIT.Core.Security.JWK;
namespace MaksIT.Core.Tests.Security.JWK; namespace MaksIT.Core.Tests.Security.JWK;
public class JwkGeneratorTests public class JwkGeneratorTests {
{ [Fact]
[Fact] public void TryGenerateFromRSA_ValidRsa_ReturnsTrueAndJwk() {
public void TryGenerateFromRCA_ValidRsa_ReturnsTrueAndJwk() using var rsa = RSA.Create(2048);
{ var result = JwkGenerator.TryGenerateFromRSA(rsa, out var jwk, out var errorMessage);
using var rsa = RSA.Create(2048); Assert.True(result);
var result = JwkGenerator.TryGenerateFromRCA(rsa, out var jwk, out var errorMessage); Assert.NotNull(jwk);
Assert.True(result); Assert.Null(errorMessage);
Assert.NotNull(jwk); Assert.Equal(JwkKeyType.Rsa.Name, jwk!.KeyType);
Assert.Null(errorMessage); Assert.False(string.IsNullOrEmpty(jwk.RsaExponent));
Assert.Equal(JwkKeyType.Rsa.Name, jwk!.KeyType); Assert.False(string.IsNullOrEmpty(jwk.RsaModulus));
Assert.False(string.IsNullOrEmpty(jwk.RsaExponent)); }
Assert.False(string.IsNullOrEmpty(jwk.RsaModulus));
}
[Fact] [Fact]
public void TryGenerateFromRCA_MissingExponentOrModulus_ReturnsFalseAndError() public void TryGenerateFromRSA_MissingExponentOrModulus_ReturnsFalseAndError() {
{ using var rsa = RSA.Create();
using var rsa = RSA.Create(); // ExportParameters returns valid values, so we simulate missing exponent/modulus by mocking
// ExportParameters returns valid values, so we simulate missing exponent/modulus by mocking // Instead, test with a custom RSA implementation that throws
// Instead, test with a custom RSA implementation that throws var fakeRsa = new FakeRsaMissingParams();
var fakeRsa = new FakeRsaMissingParams(); var result = JwkGenerator.TryGenerateFromRSA(fakeRsa, out var jwk, out var errorMessage);
var result = JwkGenerator.TryGenerateFromRCA(fakeRsa, out var jwk, out var errorMessage); Assert.False(result);
Assert.False(result); Assert.Null(jwk);
Assert.Null(jwk); Assert.Contains("missing exponent or modulus", errorMessage);
Assert.Contains("missing exponent or modulus", errorMessage); }
}
[Fact] [Fact]
public void TryGenerateFromRCA_ExportParametersThrows_ReturnsFalseAndError() public void TryGenerateFromRSA_ExportParametersThrows_ReturnsFalseAndError() {
{ var fakeRsa = new FakeRsaThrows();
var fakeRsa = new FakeRsaThrows(); var result = JwkGenerator.TryGenerateFromRSA(fakeRsa, out var jwk, out var errorMessage);
var result = JwkGenerator.TryGenerateFromRCA(fakeRsa, out var jwk, out var errorMessage); Assert.False(result);
Assert.False(result); Assert.Null(jwk);
Assert.Null(jwk); Assert.Contains("ExportParameters failed", errorMessage);
Assert.Contains("ExportParameters failed", errorMessage); }
}
private class FakeRsaMissingParams : RSA private class FakeRsaMissingParams : RSA {
{ public override RSAParameters ExportParameters(bool includePrivateParameters)
public override RSAParameters ExportParameters(bool includePrivateParameters) => new RSAParameters { Exponent = null, Modulus = null };
=> new RSAParameters { Exponent = null, Modulus = null }; // ...other abstract members throw NotImplementedException
// ...other abstract members throw NotImplementedException public override byte[] Decrypt(byte[] data, RSAEncryptionPadding padding) => throw new NotImplementedException();
public override byte[] Decrypt(byte[] data, RSAEncryptionPadding padding) => throw new NotImplementedException(); public override byte[] Encrypt(byte[] data, RSAEncryptionPadding padding) => throw new NotImplementedException();
public override byte[] Encrypt(byte[] data, RSAEncryptionPadding padding) => throw new NotImplementedException(); public override byte[] SignHash(byte[] hash, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding) => throw new NotImplementedException();
public override byte[] SignHash(byte[] hash, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding) => throw new NotImplementedException(); public override bool VerifyHash(byte[] hash, byte[] signature, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding) => throw new NotImplementedException();
public override bool VerifyHash(byte[] hash, byte[] signature, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding) => throw new NotImplementedException(); public override void ImportParameters(RSAParameters parameters) => throw new NotImplementedException();
public override void ImportParameters(RSAParameters parameters) => throw new NotImplementedException(); protected override void Dispose(bool disposing) { }
protected override void Dispose(bool disposing) { } }
}
private class FakeRsaThrows : RSA private class FakeRsaThrows : RSA {
{ public override RSAParameters ExportParameters(bool includePrivateParameters)
public override RSAParameters ExportParameters(bool includePrivateParameters) => throw new Exception("ExportParameters failed");
=> throw new Exception("ExportParameters failed"); // ...other abstract members throw NotImplementedException
// ...other abstract members throw NotImplementedException public override byte[] Decrypt(byte[] data, RSAEncryptionPadding padding) => throw new NotImplementedException();
public override byte[] Decrypt(byte[] data, RSAEncryptionPadding padding) => throw new NotImplementedException(); public override byte[] Encrypt(byte[] data, RSAEncryptionPadding padding) => throw new NotImplementedException();
public override byte[] Encrypt(byte[] data, RSAEncryptionPadding padding) => throw new NotImplementedException(); public override byte[] SignHash(byte[] hash, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding) => throw new NotImplementedException();
public override byte[] SignHash(byte[] hash, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding) => throw new NotImplementedException(); public override bool VerifyHash(byte[] hash, byte[] signature, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding) => throw new NotImplementedException();
public override bool VerifyHash(byte[] hash, byte[] signature, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding) => throw new NotImplementedException(); public override void ImportParameters(RSAParameters parameters) => throw new NotImplementedException();
public override void ImportParameters(RSAParameters parameters) => throw new NotImplementedException(); protected override void Dispose(bool disposing) { }
protected override void Dispose(bool disposing) { } }
}
} }

View File

@ -5,59 +5,54 @@ using MaksIT.Core.Security.JWK;
namespace MaksIT.Core.Tests.Security.JWK; namespace MaksIT.Core.Tests.Security.JWK;
public class JwkThumbprintUtilityTests public class JwkThumbprintUtilityTests {
{ [Fact]
[Fact] public void TryGetSha256Thumbprint_ValidRsaJwk_ReturnsTrueAndThumbprint() {
public void TryGetSha256Thumbprint_ValidRsaJwk_ReturnsTrueAndThumbprint() using var rsa = RSA.Create(2048);
{ var genResult = JwkGenerator.TryGenerateFromRSA(rsa, out var jwk, out var genError);
using var rsa = RSA.Create(2048); Assert.True(genResult);
var genResult = JwkGenerator.TryGenerateFromRCA(rsa, out var jwk, out var genError); Assert.NotNull(jwk);
Assert.True(genResult); var result = JwkThumbprintUtility.TryGetSha256Thumbprint(jwk!, out var thumbprint, out var error);
Assert.NotNull(jwk); Assert.True(result);
var result = JwkThumbprintUtility.TryGetSha256Thumbprint(jwk!, out var thumbprint, out var error); Assert.False(string.IsNullOrEmpty(thumbprint));
Assert.True(result); Assert.Null(error);
Assert.False(string.IsNullOrEmpty(thumbprint)); // Should be base64url encoded and of expected length (SHA256 =32 bytes)
Assert.Null(error); var decoded = Base64UrlUtility.Decode(thumbprint!);
// Should be base64url encoded and of expected length (SHA256 =32 bytes) Assert.Equal(32, decoded.Length);
var decoded = Base64UrlUtility.Decode(thumbprint!); }
Assert.Equal(32, decoded.Length);
}
[Fact] [Fact]
public void TryGetSha256Thumbprint_NullExponentOrModulus_ReturnsFalseAndError() public void TryGetSha256Thumbprint_NullExponentOrModulus_ReturnsFalseAndError() {
{ var jwk = new Jwk { RsaExponent = null, RsaModulus = null };
var jwk = new Jwk { RsaExponent = null, RsaModulus = null }; var result = JwkThumbprintUtility.TryGetSha256Thumbprint(jwk, out var thumbprint, out var error);
var result = JwkThumbprintUtility.TryGetSha256Thumbprint(jwk, out var thumbprint, out var error); Assert.False(result);
Assert.False(result); Assert.Null(thumbprint);
Assert.Null(thumbprint); Assert.Contains("exponent or modulus", error);
Assert.Contains("exponent or modulus", error); }
}
[Fact] [Fact]
public void TryGetKeyAuthorization_ValidJwk_ReturnsTrueAndKeyAuthorization() public void TryGetKeyAuthorization_ValidJwk_ReturnsTrueAndKeyAuthorization() {
{ using var rsa = RSA.Create(2048);
using var rsa = RSA.Create(2048); var genResult = JwkGenerator.TryGenerateFromRSA(rsa, out var jwk, out var genError);
var genResult = JwkGenerator.TryGenerateFromRCA(rsa, out var jwk, out var genError); Assert.True(genResult);
Assert.True(genResult); Assert.NotNull(jwk);
Assert.NotNull(jwk); var token = "test-token";
var token = "test-token"; var result = JwkThumbprintUtility.TryGetKeyAuthorization(jwk!, token, out var keyAuth, out var error);
var result = JwkThumbprintUtility.TryGetKeyAuthorization(jwk!, token, out var keyAuth, out var error); Assert.True(result);
Assert.True(result); Assert.Null(error);
Assert.Null(error); Assert.StartsWith(token + ".", keyAuth);
Assert.StartsWith(token + ".", keyAuth); var parts = keyAuth!.Split('.');
var parts = keyAuth!.Split('.'); Assert.Equal(2, parts.Length);
Assert.Equal(2, parts.Length); Assert.False(string.IsNullOrEmpty(parts[1]));
Assert.False(string.IsNullOrEmpty(parts[1])); }
}
[Fact] [Fact]
public void TryGetKeyAuthorization_NullExponentOrModulus_ReturnsFalseAndError() public void TryGetKeyAuthorization_NullExponentOrModulus_ReturnsFalseAndError() {
{ var jwk = new Jwk { RsaExponent = null, RsaModulus = null };
var jwk = new Jwk { RsaExponent = null, RsaModulus = null }; var token = "test-token";
var token = "test-token"; var result = JwkThumbprintUtility.TryGetKeyAuthorization(jwk, token, out var keyAuth, out var error);
var result = JwkThumbprintUtility.TryGetKeyAuthorization(jwk, token, out var keyAuth, out var error); Assert.False(result);
Assert.False(result); Assert.Null(keyAuth);
Assert.Null(keyAuth); Assert.Contains("Failed to compute thumbprint", error);
Assert.Contains("Failed to compute thumbprint", error); }
}
} }

View File

@ -8,10 +8,9 @@ namespace MaksIT.Core.Tests.Security;
public class JwsGeneratorTests { public class JwsGeneratorTests {
[Fact] [Fact]
public void TryEncode_ValidRsaAndJwk_ReturnsTrueAndMessage() public void TryEncode_ValidRsaAndJwk_ReturnsTrueAndMessage() {
{
using var rsa = RSA.Create(2048); using var rsa = RSA.Create(2048);
var jwkResult = JwkGenerator.TryGenerateFromRCA(rsa, out var jwk, out var jwkError); var jwkResult = JwkGenerator.TryGenerateFromRSA(rsa, out var jwk, out var jwkError);
Assert.True(jwkResult); Assert.True(jwkResult);
Assert.NotNull(jwk); Assert.NotNull(jwk);
var header = new JwsHeader(); var header = new JwsHeader();
@ -24,10 +23,9 @@ public class JwsGeneratorTests {
} }
[Fact] [Fact]
public void TryEncode_WithPayload_ReturnsEncodedPayload() public void TryEncode_WithPayload_ReturnsEncodedPayload() {
{
using var rsa = RSA.Create(2048); using var rsa = RSA.Create(2048);
var jwkResult = JwkGenerator.TryGenerateFromRCA(rsa, out var jwk, out var jwkError); var jwkResult = JwkGenerator.TryGenerateFromRSA(rsa, out var jwk, out var jwkError);
Assert.True(jwkResult); Assert.True(jwkResult);
Assert.NotNull(jwk); Assert.NotNull(jwk);
var header = new JwsHeader(); var header = new JwsHeader();
@ -43,8 +41,7 @@ public class JwsGeneratorTests {
} }
[Fact] [Fact]
public void TryEncode_InvalidRsa_ReturnsFalseAndError() public void TryEncode_InvalidRsa_ReturnsFalseAndError() {
{
var fakeRsa = new FakeRsaThrows(); var fakeRsa = new FakeRsaThrows();
var jwk = new Jwk { KeyType = JwkKeyType.Rsa.Name }; var jwk = new Jwk { KeyType = JwkKeyType.Rsa.Name };
var header = new JwsHeader(); var header = new JwsHeader();
@ -55,8 +52,7 @@ public class JwsGeneratorTests {
} }
[Fact] [Fact]
public void TryEncode_JwkWithKeyId_SetsHeaderKid() public void TryEncode_JwkWithKeyId_SetsHeaderKid() {
{
using var rsa = RSA.Create(2048); using var rsa = RSA.Create(2048);
var jwk = new Jwk { KeyType = JwkKeyType.Rsa.Name, KeyId = "my-key-id" }; var jwk = new Jwk { KeyType = JwkKeyType.Rsa.Name, KeyId = "my-key-id" };
var header = new JwsHeader(); var header = new JwsHeader();
@ -70,8 +66,7 @@ public class JwsGeneratorTests {
} }
[Fact] [Fact]
public void TryEncode_JwkWithoutKeyId_SetsHeaderJwk() public void TryEncode_JwkWithoutKeyId_SetsHeaderJwk() {
{
using var rsa = RSA.Create(2048); using var rsa = RSA.Create(2048);
var jwk = new Jwk { KeyType = JwkKeyType.Rsa.Name }; var jwk = new Jwk { KeyType = JwkKeyType.Rsa.Name };
var header = new JwsHeader(); var header = new JwsHeader();
@ -83,8 +78,7 @@ public class JwsGeneratorTests {
Assert.Contains("jwk", protectedJson); Assert.Contains("jwk", protectedJson);
} }
private class FakeRsaThrows : RSA private class FakeRsaThrows : RSA {
{
public override RSAParameters ExportParameters(bool includePrivateParameters) public override RSAParameters ExportParameters(bool includePrivateParameters)
=> throw new Exception("ExportParameters failed"); => throw new Exception("ExportParameters failed");
public override byte[] Decrypt(byte[] data, RSAEncryptionPadding padding) => throw new NotImplementedException(); public override byte[] Decrypt(byte[] data, RSAEncryptionPadding padding) => throw new NotImplementedException();

View File

@ -8,7 +8,7 @@
<!-- NuGet package metadata --> <!-- NuGet package metadata -->
<PackageId>MaksIT.Core</PackageId> <PackageId>MaksIT.Core</PackageId>
<Version>1.5.7</Version> <Version>1.5.8</Version>
<Authors>Maksym Sadovnychyy</Authors> <Authors>Maksym Sadovnychyy</Authors>
<Company>MAKS-IT</Company> <Company>MAKS-IT</Company>
<Product>MaksIT.Core</Product> <Product>MaksIT.Core</Product>

View File

@ -8,8 +8,9 @@ namespace MaksIT.Core.Security.JWK;
/// Provides utilities for JWK (JSON Web Key) operations, including RFC7638 thumbprint computation and key generation. /// Provides utilities for JWK (JSON Web Key) operations, including RFC7638 thumbprint computation and key generation.
/// </summary> /// </summary>
public static class JwkGenerator { public static class JwkGenerator {
public static bool TryGenerateFromRCA( public static bool TryGenerateFromRSA(
RSA rsa, RSA rsa,
[NotNullWhen(true)]
out Jwk? jwk, out Jwk? jwk,
[NotNullWhen(false)] out string? errorMessage [NotNullWhen(false)] out string? errorMessage
) { ) {

View File

@ -13,7 +13,7 @@ public static class JwkThumbprintUtility {
public static bool TryGetKeyAuthorization( public static bool TryGetKeyAuthorization(
Jwk jwk, Jwk jwk,
string token, string token,
out string? keyAuthorization, [NotNullWhen(true)] out string? keyAuthorization,
[NotNullWhen(false)] out string? errorMessage [NotNullWhen(false)] out string? errorMessage
) { ) {
keyAuthorization = null; keyAuthorization = null;
@ -32,7 +32,7 @@ public static class JwkThumbprintUtility {
/// </summary> /// </summary>
public static bool TryGetSha256Thumbprint( public static bool TryGetSha256Thumbprint(
Jwk jwk, Jwk jwk,
out string? thumbprint, [NotNullWhen(true)] out string? thumbprint,
[NotNullWhen(false)] out string? errorMessage [NotNullWhen(false)] out string? errorMessage
) { ) {
thumbprint = null; thumbprint = null;

View File

@ -12,7 +12,7 @@ public static class JwsGenerator {
RSA rsa, RSA rsa,
Jwk jwk, Jwk jwk,
JwsHeader protectedHeader, JwsHeader protectedHeader,
out JwsMessage? message, [NotNullWhen(true)] out JwsMessage? message,
[NotNullWhen(false)] out string? errorMessage [NotNullWhen(false)] out string? errorMessage
) => TryEncode<string>(rsa, jwk, protectedHeader, null, out message, out errorMessage); ) => TryEncode<string>(rsa, jwk, protectedHeader, null, out message, out errorMessage);
@ -22,7 +22,7 @@ public static class JwsGenerator {
Jwk jwk, Jwk jwk,
JwsHeader protectedHeader, JwsHeader protectedHeader,
T? payload, T? payload,
out JwsMessage? message, [NotNullWhen(true)] out JwsMessage? message,
[NotNullWhen(false)] out string? errorMessage [NotNullWhen(false)] out string? errorMessage
) { ) {
try { try {