[ACL-234] Add support for caching auth token #236
Conversation
tl-Roberto-Mancinelli
commented
Dec 4, 2024
•
tl-Roberto-Mancinelli
commented
- Added TrueLayerClientFactory to manage AuthClient creation.
- Added IAuthTokenCache interface to enable clients to provide their own caching implementation.
- Added AddAuthTokenInMemoryCaching and AddAuthTokenCaching to enable caching.
| @@ -115,7 +115,9 @@ public void ConfigureServices(IServiceCollection services) | |||
| // For demo purposes only. Private key should be stored securely | |||
| options.Payments.SigningKey.PrivateKey = File.ReadAllText("ec512-private-key.pem"); | |||
| } | |||
| }); | |||
| }) | |||
| // We advice to cache the auth token | |||
There was a problem hiding this comment.
Do you want to do suggest the same on Java's readme? 😄
I think we could do that in lieu of the environment and/or logs opt-in
There was a problem hiding this comment.
yeah I think it's a good advice
| CancellationToken cancellationToken = default) | ||
| { | ||
| var key = $"{KeyPrefix}{authTokenRequest.Scope}"; | ||
| if (_authTokenCache.TryGetValue(key, out ApiResponse<GetAuthTokenResponse>? cachedResponse)) |
There was a problem hiding this comment.
What happens if the token is invalidated for some reason, before the expiry kicks in?
Because of this potential problem, on the Java lib we clear the cache every time we get a 401 even with the Authorization header set. To do that we use a concept which is specific to the internal HTTP client that we use, not sure what we could use here. Perhaps we could an interceptor here?
Also, because of the above, our caching interface also expose a clear method
There was a problem hiding this comment.
I think adding a clear method is reasonable.
Have we confirmed that the invalidation of a token is a realistic scenario?
There was a problem hiding this comment.
Let me try raising this with the team. @lighe do you know more about this? what's the approach on the PHP lib?
There was a problem hiding this comment.
Actually, I think we clear as well on the PHP lib if we get 401s. Alex to keep me honest.
I think in our implementation we assumed that "external" token invalidation events might happen, and we implemented the cleanup task to avoid impacting the integrations.
tl-Roberto-Mancinelli
force-pushed
the
token-cache
branch
from
172cbec to
99b38cd
Compare
fix NullMemoryCache removing logic minor update readme minor add tests moved Factory
