Skip to content

Commit 1cda155

Browse files
brunobritodevbrunobritodev
authored
fix: correct new jwe bugs and older ones
Bugfix/correct new jwe bugs and older ones
2 parents 59d3989 + 0464138 commit 1cda155

File tree

12 files changed

+110
-56
lines changed

12 files changed

+110
-56
lines changed

‎src/NetDevPack.Security.Jwt.Core/DefaultStore/InMemoryStore.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,24 @@ namespace NetDevPack.Security.Jwt.Core.DefaultStore;
77

88
internal class InMemoryStore : IJsonWebKeyStore
99
{
10-
internal const string DefaultRevocationReason = "Revoked";
1110
private static readonly List<KeyMaterial> _store = new();
1211
private readonly SemaphoreSlim _slim = new(1);
12+
internal const string DefaultRevocationReason = "Revoked";
13+
1314
public Task Store(KeyMaterial keyMaterial)
1415
{
16+
if (keyMaterial is null) throw new InvalidOperationException("Can't store empty value.");
17+
1518
_slim.Wait();
1619
_store.Add(keyMaterial);
1720
_slim.Release();
1821

1922
return Task.CompletedTask;
2023
}
2124

22-
public Task<KeyMaterial> GetCurrent(JwtKeyType jwtKeyType)
25+
public Task<KeyMaterial> GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws)
2326
{
24-
return Task.FromResult(_store.OrderByDescending(s => s.CreationDate).FirstOrDefault());
27+
return Task.FromResult(_store.Where(s => s.Use == (jwtKeyType == JwtKeyType.Jws ? "sig" : "enc")).OrderByDescending(s => s.CreationDate).FirstOrDefault());
2528
}
2629

2730
public async Task Revoke(KeyMaterial keyMaterial, string reason = null)
@@ -41,7 +44,7 @@ public async Task Revoke(KeyMaterial keyMaterial, string reason = null)
4144
}
4245
}
4346

44-
public Task<ReadOnlyCollection<KeyMaterial>> GetLastKeys(int quantity, JwtKeyType? jwtKeyType)
47+
public Task<ReadOnlyCollection<KeyMaterial>> GetLastKeys(int quantity, JwtKeyType? jwtKeyType = null)
4548
{
4649
return Task.FromResult(
4750
_store

‎src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,7 @@ public async Task<EncryptingCredentials> GetCurrentEncryptingCredentials()
6161

6262
public Task<ReadOnlyCollection<KeyMaterial>> GetLastKeys(int? i = null)
6363
{
64-
JwtKeyType? jwtKeyType = null;
65-
66-
if (_options.Value.ExposedKeyType == JwtType.Jws)
67-
jwtKeyType = JwtKeyType.Jws;
68-
else if (_options.Value.ExposedKeyType == JwtType.Jwe)
69-
jwtKeyType = JwtKeyType.Jwe;
70-
71-
return _store.GetLastKeys(_options.Value.AlgorithmsToKeep, jwtKeyType);
64+
return _store.GetLastKeys(_options.Value.AlgorithmsToKeep, null);
7265
}
7366

7467
public Task<ReadOnlyCollection<KeyMaterial>> GetLastKeys(int i, JwtKeyType jwtKeyType)

‎src/NetDevPack.Security.Jwt.Core/JwtOptions.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,4 @@ public class JwtOptions
1010
public string KeyPrefix { get; set; } = $"{Environment.MachineName}_";
1111
public int AlgorithmsToKeep { get; set; } = 2;
1212
public TimeSpan CacheTime { get; set; } = TimeSpan.FromMinutes(15);
13-
public JwtType ExposedKeyType { get; set; } = JwtType.Jws;
1413
}

‎src/NetDevPack.Security.Jwt.Core/Model/KeyMaterial.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public JsonWebKey GetJsonWebKey()
3535
};
3636

3737
jsonWebKey.Use = Algorithm.CryptographyType == CryptographyType.DigitalSignature ? "sig" : "enc";
38-
jsonWebKey.Alg = Algorithm.Alg; // Assure-toi que `Algorithm.Name` contient l'algorithme correct
38+
jsonWebKey.Alg = Algorithm.Alg;
3939

4040
return jsonWebKey;
4141
}

‎src/NetDevPack.Security.Jwt.Store.EntityFrameworkCore/DatabaseJsonWebKeyStore.cs

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ internal class DatabaseJsonWebKeyStore<TContext> : IJsonWebKeyStore
2222
private readonly IOptions<JwtOptions> _options;
2323
private readonly IMemoryCache _memoryCache;
2424
private readonly ILogger<DatabaseJsonWebKeyStore<TContext>> _logger;
25+
internal const string DefaultRevocationReason = "Revoked";
2526

2627
public DatabaseJsonWebKeyStore(TContext context, ILogger<DatabaseJsonWebKeyStore<TContext>> logger, IOptions<JwtOptions> options, IMemoryCache memoryCache)
2728
{
@@ -46,10 +47,11 @@ public async Task<KeyMaterial> GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws
4647

4748
if (!_memoryCache.TryGetValue(cacheKey, out KeyMaterial credentials))
4849
{
50+
var keyType = (jwtKeyType == JwtKeyType.Jws ? "sig" : "enc");
4951
#if NET5_0_OR_GREATER
50-
credentials = await _context.SecurityKeys.Where(X => X.IsRevoked == false).OrderByDescending(d => d.CreationDate).AsNoTrackingWithIdentityResolution().FirstOrDefaultAsync();
52+
credentials = await _context.SecurityKeys.Where(X => X.IsRevoked == false).Where(s => s.Use == keyType).OrderByDescending(d => d.CreationDate).AsNoTrackingWithIdentityResolution().FirstOrDefaultAsync();
5153
#else
52-
credentials = await _context.SecurityKeys.Where(X => X.IsRevoked == false).OrderByDescending(d => d.CreationDate).AsNoTracking().FirstOrDefaultAsync();
54+
credentials = await _context.SecurityKeys.Where(X => X.IsRevoked == false).Where(s => s.Use == keyType).OrderByDescending(d => d.CreationDate).AsNoTracking().FirstOrDefaultAsync();
5355
#endif
5456

5557
// Set cache options.
@@ -73,9 +75,11 @@ public async Task<ReadOnlyCollection<KeyMaterial>> GetLastKeys(int quantity = 5,
7375
if (!_memoryCache.TryGetValue(cacheKey, out ReadOnlyCollection<KeyMaterial> keys))
7476
{
7577
#if NET5_0_OR_GREATER
76-
keys = _context.SecurityKeys.OrderByDescending(d => d.CreationDate).Take(quantity).AsNoTrackingWithIdentityResolution().ToList().AsReadOnly();
78+
keys = (await _context.SecurityKeys.Where(s => jwtKeyType == null || s.Use == (jwtKeyType == JwtKeyType.Jws ? "sig" : "enc"))
79+
.OrderByDescending(d => d.CreationDate).AsNoTrackingWithIdentityResolution().ToListAsync()).AsReadOnly();
7780
#else
78-
keys = _context.SecurityKeys.OrderByDescending(d => d.CreationDate).Take(quantity).AsNoTracking().ToList().AsReadOnly();
81+
keys = _context.SecurityKeys.Where(s => jwtKeyType == null || s.Use == (jwtKeyType == JwtKeyType.Jws ? "sig" : "enc"))
82+
.OrderByDescending(d => d.CreationDate).AsNoTracking().ToList().AsReadOnly();
7983
#endif
8084
// Set cache options.
8185
var cacheEntryOptions = new MemoryCacheEntryOptions()
@@ -84,11 +88,11 @@ public async Task<ReadOnlyCollection<KeyMaterial>> GetLastKeys(int quantity = 5,
8488

8589
if (keys.Any())
8690
_memoryCache.Set(cacheKey, keys, cacheEntryOptions);
87-
88-
return keys;
8991
}
9092

91-
return keys;
93+
return keys.GroupBy(s => s.Use)
94+
.SelectMany(g => g.Take(quantity))
95+
.ToList().AsReadOnly();
9296
}
9397

9498
public Task<KeyMaterial> Get(string keyId)
@@ -113,7 +117,7 @@ public async Task Revoke(KeyMaterial securityKeyWithPrivate, string reason = nul
113117
if (securityKeyWithPrivate == null)
114118
return;
115119

116-
securityKeyWithPrivate.Revoke(reason);
120+
securityKeyWithPrivate.Revoke(reason ?? DefaultRevocationReason);
117121
_context.Attach(securityKeyWithPrivate);
118122
_context.SecurityKeys.Update(securityKeyWithPrivate);
119123
await _context.SaveChangesAsync();

‎src/NetDevPack.Security.Jwt.Store.FileSystem/FileSystemStore.cs

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ namespace NetDevPack.Security.Jwt.Store.FileSystem
1212
{
1313
public class FileSystemStore : IJsonWebKeyStore
1414
{
15+
internal const string DefaultRevocationReason = "Revoked";
1516
private readonly IOptions<JwtOptions> _options;
1617
private readonly IMemoryCache _memoryCache;
1718
public DirectoryInfo KeysPath { get; }
@@ -25,39 +26,43 @@ public FileSystemStore(DirectoryInfo keysPath, IOptions<JwtOptions> options, IMe
2526
KeysPath.Create();
2627
}
2728

28-
private string GetCurrentFile()
29+
private string GetCurrentFile(JwtKeyType jwtKeyType)
2930
{
30-
var files = Directory.GetFiles(KeysPath.FullName, $"*current*.key");
31+
var files = Directory.GetFiles(KeysPath.FullName, $"*current*.{jwtKeyType}.key");
3132
if (files.Any())
32-
return Path.Combine(KeysPath.FullName, files.First());
33+
return files.First();
3334

34-
return Path.Combine(KeysPath.FullName, $"{_options.Value.KeyPrefix}current.key");
35+
return Path.Combine(KeysPath.FullName, $"{_options.Value.KeyPrefix}current.{jwtKeyType}.key");
3536
}
3637

3738
public async Task Store(KeyMaterial securityParamteres)
3839
{
3940
if (!KeysPath.Exists)
4041
KeysPath.Create();
4142

43+
JwtKeyType keyType = securityParamteres.Use.Equals("enc", StringComparison.InvariantCultureIgnoreCase) ? JwtKeyType.Jwe : JwtKeyType.Jws;
44+
4245
// Datetime it's just to be easy searchable.
43-
if (File.Exists(GetCurrentFile()))
44-
File.Copy(GetCurrentFile(), Path.Combine(KeysPath.FullName, $"{_options.Value.KeyPrefix}old-{DateTime.Now:yyyy-MM-dd}-{securityParamteres.KeyId}.key"));
46+
if (File.Exists(GetCurrentFile(keyType)))
47+
File.Copy(GetCurrentFile(keyType), Path.Combine(KeysPath.FullName, $"{_options.Value.KeyPrefix}old-{DateTime.Now:yyyy-MM-dd}-{securityParamteres.KeyId}.key"));
4548

46-
await File.WriteAllTextAsync(Path.Combine(KeysPath.FullName, $"{_options.Value.KeyPrefix}current-{securityParamteres.KeyId}.key"), JsonSerializer.Serialize(securityParamteres, new JsonSerializerOptions() { DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull }));
49+
await File.WriteAllTextAsync(Path.Combine(KeysPath.FullName, $"{_options.Value.KeyPrefix}current-{securityParamteres.KeyId}.{keyType}.key"), JsonSerializer.Serialize(securityParamteres, new JsonSerializerOptions() { DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull }));
4750
ClearCache();
4851
}
4952

50-
public bool NeedsUpdate()
53+
public bool NeedsUpdate(KeyMaterial current)
5154
{
52-
return !File.Exists(GetCurrentFile()) || File.GetCreationTimeUtc(GetCurrentFile()).AddDays(_options.Value.DaysUntilExpire) < DateTime.UtcNow.Date;
55+
JwtKeyType keyType = current.Use.Equals("enc", StringComparison.InvariantCultureIgnoreCase) ? JwtKeyType.Jwe : JwtKeyType.Jws;
56+
57+
return !File.Exists(GetCurrentFile(keyType)) || File.GetCreationTimeUtc(GetCurrentFile(keyType)).AddDays(_options.Value.DaysUntilExpire) < DateTime.UtcNow.Date;
5358
}
5459

5560
public async Task Revoke(KeyMaterial securityKeyWithPrivate, string reason = null)
5661
{
5762
if (securityKeyWithPrivate == null)
5863
return;
5964

60-
securityKeyWithPrivate?.Revoke();
65+
securityKeyWithPrivate?.Revoke(reason ?? DefaultRevocationReason);
6166
foreach (var fileInfo in KeysPath.GetFiles("*.key"))
6267
{
6368
var key = GetKey(fileInfo.FullName);
@@ -75,7 +80,7 @@ public async Task Revoke(KeyMaterial securityKeyWithPrivate, string reason = nul
7580

7681
if (!_memoryCache.TryGetValue(cacheKey, out KeyMaterial credentials))
7782
{
78-
credentials = GetKey(GetCurrentFile());
83+
credentials = GetKey(GetCurrentFile(jwtKeyType));
7984
// Set cache options.
8085
var cacheEntryOptions = new MemoryCacheEntryOptions()
8186
// Keep in cache for this time, reset time if accessed.
@@ -87,9 +92,9 @@ public async Task Revoke(KeyMaterial securityKeyWithPrivate, string reason = nul
8792
return Task.FromResult(credentials);
8893
}
8994

90-
private KeyMaterial GetKey(string file)
95+
private KeyMaterial? GetKey(string file)
9196
{
92-
if (!File.Exists(file)) throw new FileNotFoundException("Check configuration - cannot find auth key file: " + file);
97+
if (!File.Exists(file)) return null;
9398
var keyParams = JsonSerializer.Deserialize<KeyMaterial>(File.ReadAllText(file));
9499
return keyParams!;
95100

@@ -101,8 +106,9 @@ public Task<ReadOnlyCollection<KeyMaterial>> GetLastKeys(int quantity = 5, JwtKe
101106

102107
if (!_memoryCache.TryGetValue(cacheKey, out IReadOnlyCollection<KeyMaterial> keys))
103108
{
104-
keys = KeysPath.GetFiles("*.key")
105-
.Take(quantity)
109+
var type = jwtKeyType == null ? "*" : jwtKeyType.ToString();
110+
111+
keys = KeysPath.GetFiles($"*.{type}.key")
106112
.Select(s => s.FullName)
107113
.Select(GetKey).ToList().AsReadOnly();
108114

@@ -115,7 +121,10 @@ public Task<ReadOnlyCollection<KeyMaterial>> GetLastKeys(int quantity = 5, JwtKe
115121
_memoryCache.Set(cacheKey, keys, cacheEntryOptions);
116122
}
117123

118-
return Task.FromResult(keys.ToList().AsReadOnly());
124+
return Task.FromResult(keys
125+
.GroupBy(s => s.Use)
126+
.SelectMany(g => g.Take(quantity))
127+
.ToList().AsReadOnly());
119128
}
120129

121130
public Task<KeyMaterial?> Get(string keyId)

‎tests/NetDevPack.Security.Jwt.Tests/JwtTests/JwtServiceTest.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ public JwtServiceTest(WarmupInMemoryStore warmup)
2222
_jwksService = warmup.Services.GetRequiredService<IJwtService>();
2323
}
2424

25-
26-
2725
[Fact]
2826
public async Task Should_Create_New_Key()
2927
{

‎tests/NetDevPack.Security.Jwt.Tests/StoreTests/DatabaseInMemoryStoreTests.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
using NetDevPack.Security.Jwt.Tests.Warmups;
2+
using Xunit;
3+
4+
namespace NetDevPack.Security.Jwt.Tests.StoreTests;
5+
6+
[Trait("Category", "DatabaseInMemory Tests")]
7+
public class DatabaseInMemoryStoreTests : GenericStoreServiceTest<WarmupDatabaseInMemoryStore>
8+
{
9+
public DatabaseInMemoryStoreTests(WarmupDatabaseInMemoryStore unifiedContext) : base(unifiedContext)
10+
{
11+
}
12+
}

‎tests/NetDevPack.Security.Jwt.Tests/StoreTests/FileSystemStoreTests.cs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@
33

44
namespace NetDevPack.Security.Jwt.Tests.StoreTests
55
{
6-
//[Trait("Category", "InMemory Tests")]
7-
//public class FileSystemStoreTests : GenericStoreServiceTest<WarmupFileStore>
8-
//{
9-
// public FileSystemStoreTests(WarmupFileStore unifiedContext) : base(unifiedContext)
10-
// {
11-
// }
12-
//}
6+
[Trait("Category", "InMemory Tests")]
7+
public class FileSystemStoreTests : GenericStoreServiceTest<WarmupFileStore>
8+
{
9+
public FileSystemStoreTests(WarmupFileStore unifiedContext) : base(unifiedContext)
10+
{
11+
}
12+
13+
}
1314
}

‎tests/NetDevPack.Security.Jwt.Tests/StoreTests/GenericStoreServiceTest.cs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Collections.Generic;
23
using System.Linq;
34
using System.Security.Claims;
45
using System.Threading;
@@ -23,13 +24,15 @@ public abstract class GenericStoreServiceTest<TWarmup> : IClassFixture<TWarmup>
2324
{
2425
private static SemaphoreSlim TestSync = new(1);
2526
protected readonly IJsonWebKeyStore _store;
27+
protected readonly IJwtService _jwtService;
2628
private readonly IOptions<JwtOptions> _options;
2729
public TWarmup WarmupData { get; }
2830

2931
public GenericStoreServiceTest(TWarmup warmup)
3032
{
3133
WarmupData = warmup;
3234
_store = WarmupData.Services.GetRequiredService<IJsonWebKeyStore>();
35+
_jwtService = WarmupData.Services.GetRequiredService<IJwtService>();
3336
_options = WarmupData.Services.GetRequiredService<IOptions<JwtOptions>>();
3437
this.WarmupData.Clear();
3538
}
@@ -519,6 +522,28 @@ public async Task Should_Read_NonDefault_Revocation_Reason(string reason)
519522
await CheckRevocationReasonIsStored(keyMaterial.KeyId, reason);
520523
}
521524

525+
[Fact]
526+
public async Task Should_Generate_Different_Keys_For_JWS_And_JWE_And_Retrieve_Them_Correctly()
527+
{
528+
var defaultVal = await _jwtService.GetCurrentSecurityKey();
529+
var jwe = await _jwtService.GetCurrentSecurityKey(JwtKeyType.Jwe);
530+
var jws = await _jwtService.GetCurrentSecurityKey(JwtKeyType.Jws);
531+
532+
var getLast2DefaultVal = await _jwtService.GetLastKeys(1);
533+
var getLastJwe = (await _jwtService.GetLastKeys(1, JwtKeyType.Jwe)).First();
534+
var getLastJws = (await _jwtService.GetLastKeys(1, JwtKeyType.Jws)).First();
535+
536+
jws.KeyId.Should().NotBe(jwe.KeyId);
537+
getLastJws.KeyId.Should().NotBe(getLastJwe.KeyId);
538+
defaultVal.KeyId.Should().Be(jws.KeyId);
539+
jwe.KeyId.Should().Be(getLastJwe.KeyId);
540+
jws.KeyId.Should().Be(getLastJws.KeyId);
541+
542+
getLast2DefaultVal.Should().HaveCount(2);
543+
getLast2DefaultVal.Should().ContainSingle(x => x.Use == "enc");
544+
getLast2DefaultVal.Should().ContainSingle(x => x.Use == "sig");
545+
}
546+
522547
private async Task CheckRevocationReasonIsStored(string keyId, string revocationReason)
523548
{
524549
var dbKey = (await _store.GetLastKeys(5)).First(w => w.KeyId == keyId);

‎tests/NetDevPack.Security.Jwt.Tests/Warmups/WarmupDatabaseInMemory.cs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@
66

77
namespace NetDevPack.Security.Jwt.Tests.Warmups;
88

9-
public class WarmupDatabaseInMemory : IWarmupTest
9+
public class WarmupDatabaseInMemoryStore : IWarmupTest
1010
{
1111
private readonly IJsonWebKeyStore _jsonWebKeyStore;
1212
public ServiceProvider Services { get; set; }
1313

14-
public WarmupDatabaseInMemory()
14+
public WarmupDatabaseInMemoryStore()
1515
{
1616
var serviceCollection = new ServiceCollection();
1717

@@ -21,11 +21,7 @@ public WarmupDatabaseInMemory()
2121
serviceCollection.AddLogging();
2222
serviceCollection.AddDbContext<AspNetGeneralContext>(DatabaseOptions);
2323

24-
serviceCollection.AddJwksManager(o =>
25-
{
26-
o.Jws = Algorithm.Create(AlgorithmType.AES, JwtType.Jws);
27-
o.Jwe = Algorithm.Create(AlgorithmType.AES, JwtType.Jwe);
28-
})
24+
serviceCollection.AddJwksManager()
2925
.PersistKeysToDatabaseStore<AspNetGeneralContext>();
3026
Services = serviceCollection.BuildServiceProvider();
3127
_jsonWebKeyStore = Services.GetRequiredService<IJsonWebKeyStore>();

‎tests/NetDevPack.Security.Jwt.Tests/Warmups/WarmupFileStore.cs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
using System.IO;
1+
using System;
2+
using System.IO;
23
using System.Threading.Tasks;
34
using Microsoft.Extensions.DependencyInjection;
45
using NetDevPack.Security.Jwt.Core;
@@ -11,19 +12,32 @@ public class WarmupFileStore : IWarmupTest
1112
{
1213
private readonly IJsonWebKeyStore _jsonWebKeyStore;
1314
public ServiceProvider Services { get; set; }
15+
public DirectoryInfo _directoryInfo;
16+
1417
public WarmupFileStore()
1518
{
19+
_directoryInfo = TempDirectoryTest();
20+
1621
var serviceCollection = new ServiceCollection();
1722
serviceCollection.AddLogging();
1823
serviceCollection.AddMemoryCache();
19-
serviceCollection.AddJwksManager().PersistKeysToFileSystem(new DirectoryInfo(Path.Combine(Directory.GetCurrentDirectory(), "files")));
24+
serviceCollection.AddJwksManager().PersistKeysToFileSystem(_directoryInfo);
2025
Services = serviceCollection.BuildServiceProvider();
2126
_jsonWebKeyStore = Services.GetRequiredService<IJsonWebKeyStore>();
2227
}
2328

29+
public DirectoryInfo TempDirectoryTest()
30+
{
31+
// Créer un répertoire temporaire unique
32+
var tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
33+
return Directory.CreateDirectory(tempDir);
34+
}
35+
2436
public async Task Clear()
2537
{
2638
await _jsonWebKeyStore.Clear();
39+
_directoryInfo.Delete(true);
40+
Directory.CreateDirectory(_directoryInfo.FullName);
2741
}
2842
}
2943
}

0 commit comments

Comments
 (0)