From e9a1b899745e1eaafa5a03743d2a64ff3e71fa7d Mon Sep 17 00:00:00 2001 From: Jacob Parker Date: Wed, 7 Feb 2024 13:01:09 -0500 Subject: [PATCH] Have GetSigningCredentialsAsync refresh its key earlier 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. --- .../Keys/KeyManagementService.cs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/D2L.Security.OAuth2/Keys/KeyManagementService.cs b/src/D2L.Security.OAuth2/Keys/KeyManagementService.cs index 0b8bea9e..1055b1d1 100644 --- a/src/D2L.Security.OAuth2/Keys/KeyManagementService.cs +++ b/src/D2L.Security.OAuth2/Keys/KeyManagementService.cs @@ -51,7 +51,7 @@ async Task 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 ) @@ -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 IKeyManagementService.RefreshKeyAsync() { var now = m_clock.UtcNow; @@ -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