Skip to content

Commit

Permalink
Have GetSigningCredentialsAsync refresh its key earlier
Browse files Browse the repository at this point in the history
Only refreshing after the key is expired will cause us to sign tokens
that might not validate for their entire lifetime.

The spirit was to have GetSigningCredentialsAsync call RefreshAsync even
if there was no background job calling it -- so this change makes it
call it at the same cadence/time we expect a background service to.

This hasn't been a problem in production for various reasons but it was
for a specific use-case in the dev environment, but regardless its a
bug.
  • Loading branch information
j3parker committed Feb 7, 2024
1 parent 49301a7 commit e9a1b89
Showing 1 changed file with 9 additions and 6 deletions.
15 changes: 9 additions & 6 deletions src/D2L.Security.OAuth2/Keys/KeyManagementService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ async Task<D2LSecurityToken> IPrivateKeyProvider.GetSigningCredentialsAsync() {

var now = m_clock.UtcNow;

if ( current == null || current.ValidTo <= now ) {
if ( current == null || ExpectedTimeOfNewUsableKey( current ) < now ) {
// Slow path: RefreshKeyAsync() wasn't called on boot and/or it
// isn't being called in a background job.
await RefreshKeyAsync( now )
Expand All @@ -67,6 +67,13 @@ await RefreshKeyAsync( now )
return current.Ref();
}

private DateTimeOffset ExpectedTimeOfNewUsableKey( D2LSecurityToken current )
// A new key will get generated some time before the current key
// expires, but will only become usable some time after that.
=> current.ValidTo
- m_config.KeyRotationBuffer
+ m_config.KeyTimeUntilUse;

[GenerateSync]
async Task<TimeSpan> IKeyManagementService.RefreshKeyAsync() {
var now = m_clock.UtcNow;
Expand All @@ -81,11 +88,7 @@ await RefreshKeyAsync( now )
return TimeSpan.FromSeconds( 10 );
}

// A new key will get generated some time before the current key
// expires, but will only become usable some time after that.
var expectedTimeOfNewUsableKey = current.ValidTo
- m_config.KeyRotationBuffer
+ m_config.KeyTimeUntilUse;
var expectedTimeOfNewUsableKey = ExpectedTimeOfNewUsableKey( current );

if( now > expectedTimeOfNewUsableKey ) {
// If we would have expected a new key by now, retry again in a
Expand Down

0 comments on commit e9a1b89

Please sign in to comment.