From 5adb1c8735ab0a4cda8883f46b5754a4c57124f0 Mon Sep 17 00:00:00 2001 From: Jacob Parker Date: Fri, 29 Nov 2024 13:54:04 -0500 Subject: [PATCH] Move caching stuff off the IAccessTokenProvider API into the factory (#373) --- .../TestAccessTokenProviderFactory.cs | 5 +-- .../AccessTokenProviderFactory.cs | 33 ++++++++++++++----- .../Default/AccessTokenProvider.cs | 4 +-- .../Default/CachedAccessTokenProvider.cs | 32 ++++++++++-------- .../Default/INonCachingAccessTokenProvider.cs | 15 --------- .../Provisioning/IAccessTokenProvider.cs | 5 +-- .../CachedAccessTokenProviderTests.cs | 31 ++++++++--------- .../Default/AccessTokenProviderTests.cs | 2 +- 8 files changed, 67 insertions(+), 60 deletions(-) delete mode 100644 src/D2L.Security.OAuth2/Provisioning/Default/INonCachingAccessTokenProvider.cs diff --git a/src/D2L.Security.OAuth2.TestFramework/TestAccessTokenProviderFactory.cs b/src/D2L.Security.OAuth2.TestFramework/TestAccessTokenProviderFactory.cs index d23b8575..ac237694 100644 --- a/src/D2L.Security.OAuth2.TestFramework/TestAccessTokenProviderFactory.cs +++ b/src/D2L.Security.OAuth2.TestFramework/TestAccessTokenProviderFactory.cs @@ -2,6 +2,7 @@ using System.Net.Http; using System.Security.Cryptography; using System.Threading; +using D2L.Security.OAuth2.Caching; using D2L.Security.OAuth2.Keys; using D2L.Security.OAuth2.Keys.Default; using D2L.Security.OAuth2.Keys.Development; @@ -38,9 +39,9 @@ public static IAccessTokenProvider Create( HttpClient httpClient, String tokenPr Uri authEndpoint = new Uri( tokenProvisioningEndpoint ); ITokenSigner tokenSigner = new TokenSigner( privateKeyProvider ); IAuthServiceClient authServiceClient = new AuthServiceClient( httpClient, authEndpoint ); - INonCachingAccessTokenProvider noCacheTokenProvider = new AccessTokenProvider( tokenSigner, authServiceClient ); + IAccessTokenProvider noCacheTokenProvider = new AccessTokenProvider( tokenSigner, authServiceClient ); - return new CachedAccessTokenProvider( noCacheTokenProvider, authEndpoint, Timeout.InfiniteTimeSpan ); + return new CachedAccessTokenProvider( new NullCache(), noCacheTokenProvider, authEndpoint, Timeout.InfiniteTimeSpan ); } } diff --git a/src/D2L.Security.OAuth2/Provisioning/AccessTokenProviderFactory.cs b/src/D2L.Security.OAuth2/Provisioning/AccessTokenProviderFactory.cs index 04aed488..0919f981 100644 --- a/src/D2L.Security.OAuth2/Provisioning/AccessTokenProviderFactory.cs +++ b/src/D2L.Security.OAuth2/Provisioning/AccessTokenProviderFactory.cs @@ -1,5 +1,6 @@ using System; using System.Net.Http; +using D2L.Security.OAuth2.Caching; using D2L.Security.OAuth2.Keys; using D2L.Security.OAuth2.Provisioning.Default; @@ -18,18 +19,34 @@ public static IAccessTokenProvider Create( ITokenSigner tokenSigner, HttpClient httpClient, Uri authEndpoint, - TimeSpan tokenRefreshGracePeriod + TimeSpan tokenRefreshGracePeriod, + IAccessTokenProvider inner = null, + ICache cache = null ) { + if( inner != null && cache == null ) { + throw new InvalidOperationException( "If you provide an inner you need to also provide its cache" ); + } - IAuthServiceClient authServiceClient = new AuthServiceClient( - httpClient, - authEndpoint - ); + if( inner == null ) { + IAuthServiceClient authServiceClient = new AuthServiceClient( + httpClient, + authEndpoint + ); + + inner = new AccessTokenProvider( tokenSigner, authServiceClient ); + } - INonCachingAccessTokenProvider accessTokenProvider = - new AccessTokenProvider( tokenSigner, authServiceClient ); - return new CachedAccessTokenProvider( accessTokenProvider, authEndpoint, tokenRefreshGracePeriod ); + if( cache != null ) { + return inner; + } + + return new CachedAccessTokenProvider( + cache, + inner, + authEndpoint, + tokenRefreshGracePeriod + ); } } } diff --git a/src/D2L.Security.OAuth2/Provisioning/Default/AccessTokenProvider.cs b/src/D2L.Security.OAuth2/Provisioning/Default/AccessTokenProvider.cs index 2be344fc..e77c7102 100644 --- a/src/D2L.Security.OAuth2/Provisioning/Default/AccessTokenProvider.cs +++ b/src/D2L.Security.OAuth2/Provisioning/Default/AccessTokenProvider.cs @@ -9,7 +9,7 @@ namespace D2L.Security.OAuth2.Provisioning.Default { - internal sealed partial class AccessTokenProvider : INonCachingAccessTokenProvider { + internal sealed partial class AccessTokenProvider : IAccessTokenProvider { private readonly IAuthServiceClient m_client; private readonly ITokenSigner m_tokenSigner; @@ -23,7 +23,7 @@ IAuthServiceClient authServiceClient } [GenerateSync] - async Task INonCachingAccessTokenProvider.ProvisionAccessTokenAsync( + async Task IAccessTokenProvider.ProvisionAccessTokenAsync( IEnumerable claimSet, IEnumerable scopes ) { diff --git a/src/D2L.Security.OAuth2/Provisioning/Default/CachedAccessTokenProvider.cs b/src/D2L.Security.OAuth2/Provisioning/Default/CachedAccessTokenProvider.cs index 5c3dc3cd..b8a7d472 100644 --- a/src/D2L.Security.OAuth2/Provisioning/Default/CachedAccessTokenProvider.cs +++ b/src/D2L.Security.OAuth2/Provisioning/Default/CachedAccessTokenProvider.cs @@ -15,17 +15,20 @@ namespace D2L.Security.OAuth2.Provisioning.Default { internal sealed partial class CachedAccessTokenProvider : IAccessTokenProvider { - private readonly INonCachingAccessTokenProvider m_accessTokenProvider; + private readonly ICache m_cache; + private readonly IAccessTokenProvider m_inner; private readonly Uri m_authEndpoint; private readonly TimeSpan m_tokenRefreshGracePeriod; private readonly JwtSecurityTokenHandler m_tokenHandler; public CachedAccessTokenProvider( - INonCachingAccessTokenProvider accessTokenProvider, + ICache cache, + IAccessTokenProvider inner, Uri authEndpoint, TimeSpan tokenRefreshGracePeriod ) { - m_accessTokenProvider = accessTokenProvider; + m_cache = cache; + m_inner = inner; m_authEndpoint = authEndpoint; m_tokenRefreshGracePeriod = tokenRefreshGracePeriod; @@ -35,19 +38,15 @@ TimeSpan tokenRefreshGracePeriod [GenerateSync] async Task IAccessTokenProvider.ProvisionAccessTokenAsync( IEnumerable claims, - IEnumerable scopes, - ICache cache + IEnumerable scopes ) { - if( cache == null ) { - cache = new NullCache(); - } - claims = claims.ToList(); scopes = scopes.ToList(); string cacheKey = TokenCacheKeyBuilder.BuildKey( m_authEndpoint, claims, scopes ); - CacheResponse cacheResponse = await cache.GetAsync( cacheKey ).ConfigureAwait( false ); + CacheResponse cacheResponse = await m_cache.GetAsync( cacheKey ) + .ConfigureAwait( false ); if( cacheResponse.Success ) { SecurityToken securityToken = m_tokenHandler.ReadToken( cacheResponse.Value ); @@ -56,12 +55,19 @@ ICache cache } } - IAccessToken token = - await m_accessTokenProvider.ProvisionAccessTokenAsync( claims, scopes ).ConfigureAwait( false ); + IAccessToken token = await m_inner.ProvisionAccessTokenAsync( + claims, + scopes + ).ConfigureAwait( false ); DateTime validTo = m_tokenHandler.ReadToken( token.Token ).ValidTo; - await cache.SetAsync( cacheKey, token.Token, validTo - DateTime.UtcNow ).ConfigureAwait( false ); + await m_cache.SetAsync( + cacheKey, + token.Token, + expiry: validTo - DateTime.UtcNow + ).ConfigureAwait( false ); + return token; } } diff --git a/src/D2L.Security.OAuth2/Provisioning/Default/INonCachingAccessTokenProvider.cs b/src/D2L.Security.OAuth2/Provisioning/Default/INonCachingAccessTokenProvider.cs deleted file mode 100644 index 1bd494a9..00000000 --- a/src/D2L.Security.OAuth2/Provisioning/Default/INonCachingAccessTokenProvider.cs +++ /dev/null @@ -1,15 +0,0 @@ -using System.Collections.Generic; -using System.Security.Claims; -using System.Threading.Tasks; -using D2L.CodeStyle.Annotations; -using D2L.Security.OAuth2.Scopes; - -namespace D2L.Security.OAuth2.Provisioning.Default { - internal partial interface INonCachingAccessTokenProvider { - [GenerateSync] - Task ProvisionAccessTokenAsync( - IEnumerable claims, - IEnumerable scopes - ); - } -} diff --git a/src/D2L.Security.OAuth2/Provisioning/IAccessTokenProvider.cs b/src/D2L.Security.OAuth2/Provisioning/IAccessTokenProvider.cs index 13981f89..1ac7d7af 100644 --- a/src/D2L.Security.OAuth2/Provisioning/IAccessTokenProvider.cs +++ b/src/D2L.Security.OAuth2/Provisioning/IAccessTokenProvider.cs @@ -2,7 +2,6 @@ using System.Security.Claims; using System.Threading.Tasks; using D2L.CodeStyle.Annotations; -using D2L.Security.OAuth2.Caching; using D2L.Security.OAuth2.Scopes; namespace D2L.Security.OAuth2.Provisioning { @@ -16,15 +15,13 @@ public partial interface IAccessTokenProvider { /// /// The set of claims to be included in the token. /// The set of scopes to be included in the token. - /// The provided does not need to /// check for token expiration or grace period because the /// will handle it internally. /// An access token containing an expiry and the provided claims and scopes. [GenerateSync] Task ProvisionAccessTokenAsync( IEnumerable claims, - IEnumerable scopes, - ICache cache = null + IEnumerable scopes ); } } diff --git a/test/D2L.Security.OAuth2.UnitTests/Provisioning/CachedAccessTokenProviderTests.cs b/test/D2L.Security.OAuth2.UnitTests/Provisioning/CachedAccessTokenProviderTests.cs index fbaa0d0a..59daf30c 100644 --- a/test/D2L.Security.OAuth2.UnitTests/Provisioning/CachedAccessTokenProviderTests.cs +++ b/test/D2L.Security.OAuth2.UnitTests/Provisioning/CachedAccessTokenProviderTests.cs @@ -19,13 +19,13 @@ internal sealed class CachedAccessTokenProviderTests { private readonly Claim[] m_claims = { new Claim( "abc", "123" ), new Claim( "xyz", "789" ) }; private readonly Scope[] m_scopes = { new Scope( "a", "b", "c" ), new Scope( "7", "8", "9" ) }; - private Mock m_accessTokenProviderMock; + private Mock m_accessTokenProviderMock; private Mock m_userTokenCacheMock; private Mock m_serviceTokenCacheMock; [SetUp] public void Setup() { - m_accessTokenProviderMock = new Mock( MockBehavior.Strict ); + m_accessTokenProviderMock = new Mock( MockBehavior.Strict ); m_serviceTokenCacheMock = new Mock( MockBehavior.Strict ); m_userTokenCacheMock = new Mock( MockBehavior.Strict ); } @@ -51,10 +51,10 @@ public async Task ProvisionAccessTokenAsync_NotCached_CallsThroughToAccessTokenP m_serviceTokenCacheMock.Setup( x => x.SetAsync( It.IsAny(), It.IsAny(), It.IsAny() ) ) .Returns( Task.CompletedTask ); - IAccessTokenProvider cachedAccessTokenProvider = GetCachedAccessTokenProvider(); + IAccessTokenProvider cachedAccessTokenProvider = GetCachedAccessTokenProvider( m_serviceTokenCacheMock.Object ); IAccessToken token = - await cachedAccessTokenProvider.ProvisionAccessTokenAsync( m_claims, m_scopes, m_serviceTokenCacheMock.Object ).ConfigureAwait( false ); + await cachedAccessTokenProvider.ProvisionAccessTokenAsync( m_claims, m_scopes ).ConfigureAwait( false ); Assert.AreEqual( accessToken.Token, token.Token ); } @@ -64,10 +64,10 @@ public async Task ProvisionAccessTokenAsync_AlreadyCached_UsesCachedValueAndDoes m_serviceTokenCacheMock.Setup( x => x.GetAsync( It.IsAny() ) ) .Returns( Task.FromResult( new CacheResponse( true, BuildTestToken() ) ) ); - IAccessTokenProvider cachedAccessTokenProvider = GetCachedAccessTokenProvider(); + IAccessTokenProvider cachedAccessTokenProvider = GetCachedAccessTokenProvider( m_serviceTokenCacheMock.Object ); IAccessToken token = - await cachedAccessTokenProvider.ProvisionAccessTokenAsync( m_claims, m_scopes, m_serviceTokenCacheMock.Object ).ConfigureAwait( false ); + await cachedAccessTokenProvider.ProvisionAccessTokenAsync( m_claims, m_scopes ).ConfigureAwait( false ); Assert.NotNull( token ); } @@ -85,10 +85,10 @@ public async Task ProvisionAccessTokenAsync_TokenIsAlreadyCachedButIsWithinGrace .Returns( Task.FromResult( accessToken ) ); const int gracePeriodThatIsBiggerThanTimeToExpiry = TOKEN_EXPIRY_IN_SECONDS + 60; - IAccessTokenProvider cachedAccessTokenProvider = GetCachedAccessTokenProvider( gracePeriodThatIsBiggerThanTimeToExpiry ); + IAccessTokenProvider cachedAccessTokenProvider = GetCachedAccessTokenProvider( m_serviceTokenCacheMock.Object, gracePeriodThatIsBiggerThanTimeToExpiry ); IAccessToken token = - await cachedAccessTokenProvider.ProvisionAccessTokenAsync( m_claims, m_scopes, m_serviceTokenCacheMock.Object ).ConfigureAwait( false ); + await cachedAccessTokenProvider.ProvisionAccessTokenAsync( m_claims, m_scopes ).ConfigureAwait( false ); Assert.NotNull( token ); } @@ -98,11 +98,11 @@ public async Task ProvisionAccessTokenAsync_UserClaimProvided_UserCacheUsed() { m_userTokenCacheMock.Setup( x => x.GetAsync( It.IsAny() ) ) .Returns( Task.FromResult( new CacheResponse( true, BuildTestToken( specifyUserClaim: true ) ) ) ); - IAccessTokenProvider cachedAccessTokenProvider = GetCachedAccessTokenProvider(); + IAccessTokenProvider cachedAccessTokenProvider = GetCachedAccessTokenProvider( m_userTokenCacheMock.Object ); Claim userClaim = new Claim( Constants.Claims.USER_ID, "user" ); IAccessToken token = - await cachedAccessTokenProvider.ProvisionAccessTokenAsync( new[] { userClaim }, m_scopes, m_userTokenCacheMock.Object ).ConfigureAwait( false ); + await cachedAccessTokenProvider.ProvisionAccessTokenAsync( new[] { userClaim }, m_scopes ).ConfigureAwait( false ); Assert.NotNull( token ); } @@ -112,10 +112,10 @@ public async Task ProvisionAccessTokenAsync_ServiceClaimProvided_ServiceCacheUse m_serviceTokenCacheMock.Setup( x => x.GetAsync( It.IsAny() ) ) .Returns( Task.FromResult( new CacheResponse( true, BuildTestToken( specifyUserClaim: false ) ) ) ); - IAccessTokenProvider cachedAccessTokenProvider = GetCachedAccessTokenProvider(); + IAccessTokenProvider cachedAccessTokenProvider = GetCachedAccessTokenProvider( m_serviceTokenCacheMock.Object ); IAccessToken token = - await cachedAccessTokenProvider.ProvisionAccessTokenAsync( m_claims, m_scopes, m_serviceTokenCacheMock.Object ).ConfigureAwait( false ); + await cachedAccessTokenProvider.ProvisionAccessTokenAsync( m_claims, m_scopes ).ConfigureAwait( false ); Assert.NotNull( token ); } @@ -131,14 +131,15 @@ public async Task ProvisionAccessTokenAsync_CallPassThroughOverload_CallsOtherOv new Claim( Constants.Claims.ISSUER, "TheIssuer" ) }; - IAccessTokenProvider cachedAccessTokenProvider = GetCachedAccessTokenProvider(); + IAccessTokenProvider cachedAccessTokenProvider = GetCachedAccessTokenProvider( m_serviceTokenCacheMock.Object ); IAccessToken token = - await cachedAccessTokenProvider.ProvisionAccessTokenAsync( claimSet, Enumerable.Empty(), m_serviceTokenCacheMock.Object ).ConfigureAwait( false ); + await cachedAccessTokenProvider.ProvisionAccessTokenAsync( claimSet, Enumerable.Empty() ).ConfigureAwait( false ); Assert.NotNull( token ); } - private IAccessTokenProvider GetCachedAccessTokenProvider( int tokenRefreshGracePeriod = 120 ) { + private IAccessTokenProvider GetCachedAccessTokenProvider( ICache cache, int tokenRefreshGracePeriod = 120 ) { return new CachedAccessTokenProvider( + cache, m_accessTokenProviderMock.Object, new Uri( "https://example.com" ), TimeSpan.FromSeconds( tokenRefreshGracePeriod ) diff --git a/test/D2L.Security.OAuth2.UnitTests/Provisioning/Default/AccessTokenProviderTests.cs b/test/D2L.Security.OAuth2.UnitTests/Provisioning/Default/AccessTokenProviderTests.cs index c6abc755..4f36965f 100644 --- a/test/D2L.Security.OAuth2.UnitTests/Provisioning/Default/AccessTokenProviderTests.cs +++ b/test/D2L.Security.OAuth2.UnitTests/Provisioning/Default/AccessTokenProviderTests.cs @@ -24,7 +24,7 @@ private static class TestData { private IPublicKeyDataProvider m_publicKeyDataProvider; private ITokenSigner m_tokenSigner; - private INonCachingAccessTokenProvider m_accessTokenProvider; + private IAccessTokenProvider m_accessTokenProvider; private JwtSecurityToken m_actualAssertion; [SetUp]