Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move caching stuff off the IAccessTokenProvider API into the factory #373

Merged
merged 1 commit into from
Nov 29, 2024
Merged

Move caching stuff off the IAccessTokenProvider API into the factory #373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/D2L.Security.OAuth2.TestFramework/TestAccessTokenProviderFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 );
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests

}

}
Expand Down
33 changes: 25 additions & 8 deletions src/D2L.Security.OAuth2/Provisioning/AccessTokenProviderFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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" );
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea here is that this factory is either for wrapping your IAccessTokenProvider in our caching layer (for which you provide an ICache for us to use) or for using our jwt-bearer-grant based impl with or without caching.

The reason I'm doing this is I want to implement an IamGrant IAccessTokenProvider impl. I want to do that inside the LMS because I'd really like to use the LMS Lambda framework. It would make sense to implement it in this library, and I mean you could at least define the input/output here, but meh.

}

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
);
}
}
}
4 changes: 2 additions & 2 deletions src/D2L.Security.OAuth2/Provisioning/Default/AccessTokenProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -23,7 +23,7 @@ IAuthServiceClient authServiceClient
}

[GenerateSync]
async Task<IAccessToken> INonCachingAccessTokenProvider.ProvisionAccessTokenAsync(
async Task<IAccessToken> IAccessTokenProvider.ProvisionAccessTokenAsync(
IEnumerable<Claim> claimSet,
IEnumerable<Scope> scopes
) {
Expand Down
32 changes: 19 additions & 13 deletions src/D2L.Security.OAuth2/Provisioning/Default/CachedAccessTokenProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -35,19 +38,15 @@ TimeSpan tokenRefreshGracePeriod
[GenerateSync]
async Task<IAccessToken> IAccessTokenProvider.ProvisionAccessTokenAsync(
IEnumerable<Claim> claims,
IEnumerable<Scope> scopes,
ICache cache
IEnumerable<Scope> 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 );
Expand All @@ -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;
}
}
Expand Down
15 changes: 0 additions & 15 deletions src/D2L.Security.OAuth2/Provisioning/Default/INonCachingAccessTokenProvider.cs
View file
Open in desktop

This file was deleted.

5 changes: 1 addition & 4 deletions src/D2L.Security.OAuth2/Provisioning/IAccessTokenProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -16,15 +15,13 @@ public partial interface IAccessTokenProvider {
/// </summary>
/// <param name="claims">The set of claims to be included in the token.</param>
/// <param name="scopes">The set of scopes to be included in the token.</param>
/// <param name="cache">The provided <see cref="ICache"/> does not need to
/// check for token expiration or grace period because the
/// <see cref="IAccessTokenProvider"/> will handle it internally.</param>
/// <returns>An access token containing an expiry and the provided claims and scopes.</returns>
[GenerateSync]
Task<IAccessToken> ProvisionAccessTokenAsync(
IEnumerable<Claim> claims,
IEnumerable<Scope> scopes,
ICache cache = null
IEnumerable<Scope> scopes
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consolidating on one interface, equivalent to the old non-caching one, and taking the cache inside the factory+constructor.

The LMS cache is instantiated each call but it can be safely cached.

);
}
}
31 changes: 16 additions & 15 deletions test/D2L.Security.OAuth2.UnitTests/Provisioning/CachedAccessTokenProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<INonCachingAccessTokenProvider> m_accessTokenProviderMock;
private Mock<IAccessTokenProvider> m_accessTokenProviderMock;
private Mock<ICache> m_userTokenCacheMock;
private Mock<ICache> m_serviceTokenCacheMock;

[SetUp]
public void Setup() {
m_accessTokenProviderMock = new Mock<INonCachingAccessTokenProvider>( MockBehavior.Strict );
m_accessTokenProviderMock = new Mock<IAccessTokenProvider>( MockBehavior.Strict );
m_serviceTokenCacheMock = new Mock<ICache>( MockBehavior.Strict );
m_userTokenCacheMock = new Mock<ICache>( MockBehavior.Strict );
}
Expand All @@ -51,10 +51,10 @@ public async Task ProvisionAccessTokenAsync_NotCached_CallsThroughToAccessTokenP
m_serviceTokenCacheMock.Setup( x => x.SetAsync( It.IsAny<string>(), It.IsAny<string>(), It.IsAny<TimeSpan>() ) )
.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 );
}

Expand All @@ -64,10 +64,10 @@ public async Task ProvisionAccessTokenAsync_AlreadyCached_UsesCachedValueAndDoes
m_serviceTokenCacheMock.Setup( x => x.GetAsync( It.IsAny<string>() ) )
.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 );
}

Expand All @@ -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 );
}

Expand All @@ -98,11 +98,11 @@ public async Task ProvisionAccessTokenAsync_UserClaimProvided_UserCacheUsed() {
m_userTokenCacheMock.Setup( x => x.GetAsync( It.IsAny<string>() ) )
.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 );
}

Expand All @@ -112,10 +112,10 @@ public async Task ProvisionAccessTokenAsync_ServiceClaimProvided_ServiceCacheUse
m_serviceTokenCacheMock.Setup( x => x.GetAsync( It.IsAny<string>() ) )
.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 );
}

Expand All @@ -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<Scope>(), m_serviceTokenCacheMock.Object ).ConfigureAwait( false );
await cachedAccessTokenProvider.ProvisionAccessTokenAsync( claimSet, Enumerable.Empty<Scope>() ).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 )
Expand Down
2 changes: 1 addition & 1 deletion test/D2L.Security.OAuth2.UnitTests/Provisioning/Default/AccessTokenProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Loading