include caeEnabled: true so token is correctly cached#3573
include caeEnabled: true so token is correctly cached#3573ramsessanchez wants to merge 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes -UseDeviceCode follow-on call failures by ensuring the initial token acquisition uses a CAE-enabled TokenRequestContext, matching the CAE-enabled context used during subsequent API calls so MSAL can locate the token in the correct cache. It also hardens device-code message output to avoid misleading null-reference errors when the normal output writer is unavailable.
Changes:
- Acquire the sign-in token with
isCaeEnabled: trueto ensure CAE-capable tokens are cached and retrievable for later requests. - Wrap device-code message output in a fallback path to always display the device code URI/message.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try | ||
| { | ||
| GraphSession.Instance.OutputWriter.WriteObject(code.Message); | ||
| } | ||
| catch |
There was a problem hiding this comment.
The catch { ... } in the device code callback catches all exceptions and then silently falls back to Console.WriteLine, which can mask unexpected failures (and may trigger analyzer warnings like CA1031). Prefer avoiding exceptions for control flow here: e.g., null-check GraphSession.Instance/OutputWriter first, or catch a narrow set of expected exceptions (like NullReferenceException/InvalidOperationException) and optionally emit a debug/warning with the original exception details before falling back to console output.
| try | |
| { | |
| GraphSession.Instance.OutputWriter.WriteObject(code.Message); | |
| } | |
| catch | |
| if (GraphSession.Instance != null && GraphSession.Instance.OutputWriter != null) | |
| { | |
| GraphSession.Instance.OutputWriter.WriteObject(code.Message); | |
| } | |
| else |
| // Use isCaeEnabled: true to match the TokenRequestContext that AzureIdentityAccessTokenProvider will use | ||
| // during API calls, ensuring MSAL caches a CAE-capable token that can be found silently later. | ||
| var token = await tokenCredential.GetTokenAsync(new TokenRequestContext(GetScopes(authContext), isCaeEnabled: true), cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
This change is addressing a subtle caching/behavioral issue (CAE token caching vs non-CAE). There doesn’t appear to be a unit test covering the TokenRequestContext used during sign-in, so this could regress silently. Consider adding a test that exercises AuthenticateAsync/SignInAsync with a stub TokenCredential and asserts GetTokenAsync is called with isCaeEnabled: true (or an equivalent observable effect) to lock in the expected behavior for -UseDeviceCode scenarios.
Ensure CaeEnabled is set to true when aquiring token so that the retrieved token is cached correctly. MSAL looks for tokens in separate CaeEnabled cache. Enabling this ensures subsequent calls made to the api's are able to retrieve the token prior to executing the call.
Add try catch to ensure device code uri is always shown and doesn't result in misleading
Object reference not set to an instance of an object.messagefixes #3495