fix(auth): inject ITokenCacheService and add registration guard (#37)#52
Merged
Conversation
AuthService currently takes only 2 constructor args (IPublicClientApplication, ILogger<AuthService>). These tests require a 3rd arg (ITokenCacheService) and assert that: - SignInInteractiveAsync calls ITokenCacheService.RegisterAsync exactly once - AcquireTokenSilentAsync calls ITokenCacheService.RegisterAsync exactly once - Calling SignInInteractiveAsync twice only triggers RegisterAsync once (guard) - MsalUiRequiredException in AcquireTokenSilentAsync logs at Error level Build result: 4 errors (2 pre-existing in GivenAResult.cs; 2 new CS1729 'AuthService does not contain a constructor that takes 3 arguments'). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add ITokenCacheService as third constructor parameter to AuthService - Add EnsureCacheRegisteredAsync guard — calls RegisterAsync once per instance; subsequent calls on either SignIn or AcquireTokenSilent are no-ops so the file-backed cache is registered exactly once per session - Call EnsureCacheRegisteredAsync at the top of SignInInteractiveAsync and AcquireTokenSilentAsync so every token operation activates the cache - Add LogAuthFailed call in MsalUiRequiredException catch block — error paths must log per functional-usage.md rule 10 - Update GivenAnAuthService factory and add 8 new tests covering cache registration, cross-method guard, failure retry, and logging Closes #37 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ITokenCacheServicewas never injected intoAuthService, soRegisterAsyncwas never called — MSAL used only an in-memory cache, losing all sessions on every restartITokenCacheService tokenCacheServiceas third constructor parameterEnsureCacheRegisteredAsyncguard — registers the file-backed cache exactly once per instance; shared acrossSignInInteractiveAsyncandAcquireTokenSilentAsyncLogAuthFailedcall inMsalUiRequiredExceptioncatch block (was missing, violating functional-usage.md rule 10)Test plan
dotnet build— 0 errors, 0 warningsdotnet run --project test/AStar.Dev.CloudSyncFunctional.Tests.Unit— 207 passed, 0 faileddotnet run --project test/AStar.Dev.CloudSyncFunctional.Tests.Integration— 31 passed, 0 failedRegisterAsynccalled exactly once even when bothSignInInteractiveAsyncandAcquireTokenSilentAsyncare called on the same instanceRegisterAsyncfailure leaves guard unset so next call retriesCloses #37
🤖 Generated with Claude Code