Skip to content

Commit

Permalink
the key caching has some race conditions
Browse files Browse the repository at this point in the history
  • Loading branch information
daryl-mcmillan committed Feb 8, 2024
1 parent 49301a7 commit f6ae6ec
Showing 1 changed file with 22 additions and 12 deletions.
34 changes: 22 additions & 12 deletions src/D2L.Security.OAuth2/Keys/KeyManagementService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,22 @@ public sealed partial class KeyManagementService : IKeyManagementService, IPriva
private readonly IDateTimeProvider m_clock;
private readonly OAuth2Configuration m_config;

private readonly object m_keyRefreshLock = new object();
private D2LSecurityToken m_current = null;

private D2LSecurityToken GetCurrentToken() {
lock( m_keyRefreshLock ) {
return m_current?.Ref();
}
}

private void SetCurrentToken( D2LSecurityToken token ) {
lock( m_keyRefreshLock ) {
m_current?.Dispose();
m_current = token;
}
}

internal KeyManagementService(
IPublicKeyDataProvider publicKeys,
IPrivateKeyDataProvider privateKeys,
Expand Down Expand Up @@ -47,7 +61,7 @@ public KeyManagementService(

[GenerateSync]
async Task<D2LSecurityToken> IPrivateKeyProvider.GetSigningCredentialsAsync() {
var current = Volatile.Read( ref m_current );
using var current = GetCurrentToken();

var now = m_clock.UtcNow;

Expand All @@ -57,14 +71,11 @@ async Task<D2LSecurityToken> IPrivateKeyProvider.GetSigningCredentialsAsync() {
await RefreshKeyAsync( now )
.ConfigureAwait( false );

current = Volatile.Read( ref m_current );
using var refreshed = GetCurrentToken();
return refreshed?.Ref();
}

if( current == null ) {
return null;
}

return current.Ref();
return current?.Ref();
}

[GenerateSync]
Expand All @@ -74,7 +85,7 @@ async Task<TimeSpan> IKeyManagementService.RefreshKeyAsync() {
await RefreshKeyAsync( now )
.ConfigureAwait( false );

var current = Volatile.Read( ref m_current );
using var current = GetCurrentToken();

if( current == null || now > current.ValidTo ) {
// If the key is expired or doesn't exist, retry quickly.
Expand Down Expand Up @@ -166,9 +177,7 @@ private async Task RefreshKeyAsync( DateTimeOffset now ) {
return;
}

var prev = Interlocked.Exchange( ref m_current, best );

prev?.Dispose();
SetCurrentToken( best );
}

private D2LSecurityToken ChooseKey(
Expand Down Expand Up @@ -241,7 +250,8 @@ DateTimeOffset now
}

public void Dispose() {
m_current?.Dispose();
// release the current token
SetCurrentToken( null );
}
}
}

0 comments on commit f6ae6ec

Please sign in to comment.