Remove plaintext auth-token.json cache; rely on OS-protected MSAL cache (#451)

* Remove plaintext auth-token.json cache; rely on OS-protected MSAL cache

Stop writing the plaintext L1 access-token cache (auth-token.json). Token persistence and silent refresh now rely solely on the OS-protected MSAL persistent cache (msal-token-cache: DPAPI on Windows, Keychain on macOS, owner-only file on Linux). forceRefresh is threaded to the MSAL credential (was a no-op once L1 is removed); login-hint resolution reads MSAL cache accounts instead of decoding a stored JWT; any legacy auth-token.json is deleted on cache-clear. Centralizes the msal-token-cache filename, fixes stale doc comments, and updates CHANGELOG + design.md. Build clean (warnings-as-errors); 1898 tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Refine safe-token-cache: correct MSAL cache path, rename clearer, constructor migration cleanup

- get-token reports the real MSAL cache path via MsalBrowserCredential.MsalCacheFilePath
  (fixes wrong ~/.config path on Linux/macOS — Copilot comment)
- rename ClearStaleTokenCachesAsync -> ClearMsalCacheAsync (clears only the MSAL cache now)
- run legacy auth-token.json cleanup once at construction; keep ClearCache hook
- convert ClearCache tests to constructor-cleanup tests; correct CHANGELOG wording

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: sellakumaran

CHANGELOG.md

@@ -95,6 +95,7 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g
 
 ### Changed
 
+- Hardened token storage: the CLI no longer writes access tokens to a plaintext file — they live only in the OS-protected MSAL cache (DPAPI/Keychain/owner-only file). Any legacy plaintext cache is removed automatically; sign-in prompts are unchanged.
 - `develop-mcp register-external-mcp-server` now sets `exit code 1` on failure paths (validation errors, tenant detection failure, Graph unavailable, Entra app creation failure, MCP-Platform AddMcpServer failure). Previously these paths logged an error and exited `0`, which made the command's success/failure status undetectable from scripts and CI. Successful dry-run and user-initiated cancellation at the y/N prompt continue to exit `0`.
 - Admin consent canary path (when the caller lacks `DelegatedPermissionGrant.Read.All`) no longer prompts for Enter immediately. The CLI now polls every 5 seconds, prints a friendly progress message at 30 seconds, and responds promptly to Enter or Ctrl+C. The previous jargon-heavy message about `oauth2PermissionGrants` was rewritten in plain English; technical details are demoted to `Debug`.
 - CLI log file now writes a run-start separator line (`====...====`) with the full command and timestamp before each invocation, making it easier to identify individual runs in a shared log file.

src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopSubcommands/GetTokenSubcommand.cs

@@ -252,9 +252,10 @@ private static async Task<McpServerTokenResult> AcquireTokenAsync(
         AuthenticationService authService,
         ILogger logger)
     {
-        var tokenCachePath = Path.Combine(
-            ConfigService.GetGlobalConfigDirectory(),
-            AuthenticationConstants.TokenCacheFileName);
+        // Report the OS-protected MSAL persistent cache as the token cache location. The CLI no
+        // longer writes a plaintext app-level token file; MSAL owns token persistence (DPAPI on
+        // Windows, Keychain on macOS, 0600 file on Linux).
+        var tokenCachePath = MsalBrowserCredential.MsalCacheFilePath;
 
         try
         {

src/Microsoft.Agents.A365.DevTools.Cli/Constants/AuthenticationConstants.cs

@@ -83,11 +83,6 @@ public static string[] GetRequiredRedirectUris(string clientAppId)
     /// </summary>
     public const string ApplicationName = "Microsoft.Agents.A365.DevTools.Cli";
 
-    /// <summary>
-    /// Token cache file name
-    /// </summary>
-    public const string TokenCacheFileName = "auth-token.json";
-
     /// <summary>
     /// MSAL persistent token cache file name.
     /// Used by MsalBrowserCredential (WAM/browser auth) and referenced by

src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs

@@ -24,12 +24,13 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Services;
 /// Uses Microsoft.Identity.Client.Extensions.Msal to persist tokens across all CLI instances.
 /// This dramatically reduces authentication prompts during multi-step operations like 'a365 setup all'.
 ///
-/// Cache Location: [LocalApplicationData]/Agent365/msal-token-cache (Windows/macOS)
+/// Cache Location: [LocalApplicationData]/Microsoft.Agents.A365.DevTools.Cli/msal-token-cache (all platforms)
 /// Security: Tokens are stored using platform-appropriate mechanisms:
 ///   - Windows: DPAPI (Data Protection API) - tokens encrypted with user credentials, persisted to disk
 ///   - macOS: Keychain - tokens stored in secure keychain, persisted to disk
-///   - Linux: Shared in-memory cache (static, in-process) - tokens never written to disk, shared
-///            across all MsalBrowserCredential instances in the same CLI process to avoid repeated prompts
+///   - Linux: Unprotected plaintext file (0600 permissions, owner-only), persisted to disk.
+///            Same approach used by Azure CLI (~/.azure/msal_token_cache.json); tokens are protected
+///            by filesystem permissions rather than at-rest encryption.
 ///
 /// See: https://learn.microsoft.com/en-us/entra/msal/dotnet/acquiring-tokens/desktop-mobile/wam
 /// Enhancement: Improves the WAM authentication experience by reducing repeated login prompts.
@@ -55,6 +56,16 @@ public sealed class MsalBrowserCredential : TokenCredential
         Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData),
         AuthenticationConstants.ApplicationName);
 
+    /// <summary>
+    /// Full path to the OS-protected MSAL persistent token cache file — the single source of
+    /// truth for callers that report or clear the cache location. The cache lives under
+    /// LocalApplicationData (%LocalAppData% on Windows, ~/.local/share on Linux,
+    /// ~/Library/Application Support on macOS) + the application name. This is deliberately NOT
+    /// ConfigService.GetGlobalConfigDirectory(), which resolves to the XDG config dir
+    /// (~/.config/a365) on Linux/macOS and would report the wrong location.
+    /// </summary>
+    public static string MsalCacheFilePath => Path.Combine(CacheDirectory, CacheFileName);
+
     // P/Invoke is required for WAM window handle in console applications.
     // There is no managed .NET API for console/desktop window handles - these are Windows-specific.
     // This is the standard approach documented by Microsoft for WAM integration:
@@ -267,6 +278,46 @@ private static void RegisterPersistentCache(IPublicClientApplication app, ILogge
         }
     }
 
+    /// <summary>
+    /// Reads the username (UPN) of the first account in the MSAL persistent token cache without
+    /// triggering any interactive authentication. Used to resolve a login hint for pre-selecting
+    /// the correct account when the Azure CLI is not available.
+    /// </summary>
+    /// <param name="clientId">The application (client) ID whose cache to inspect — must match the
+    /// client used for interactive acquisition so the same persisted accounts are visible.</param>
+    /// <param name="logger">Optional logger for diagnostic output.</param>
+    /// <returns>The UPN of the first cached account, or <c>null</c> if the cache is empty,
+    /// unreadable, or any error occurs. This is a best-effort fallback and never throws.</returns>
+    public static async Task<string?> TryGetCachedAccountUsernameAsync(string clientId, ILogger? logger)
+    {
+        if (string.IsNullOrWhiteSpace(clientId))
+        {
+            return null;
+        }
+
+        try
+        {
+            // Build a minimal PublicClientApplication on the common authority. We never call any
+            // Acquire* method here — only GetAccountsAsync, which reads the persisted cache and
+            // cannot prompt. The authority/tenant does not affect which accounts are enumerated.
+            var app = PublicClientApplicationBuilder
+                .Create(clientId)
+                .WithAuthority(AzureCloudInstance.AzurePublic, AuthenticationConstants.CommonTenantId)
+                .Build();
+
+            RegisterPersistentCache(app, logger);
+
+            var accounts = await app.GetAccountsAsync();
+            return accounts.FirstOrDefault()?.Username;
+        }
+        catch (Exception ex)
+        {
+            // Best-effort only — login-hint resolution falls back to no hint (account picker).
+            logger?.LogDebug(ex, "Failed to read cached account username from MSAL cache");
+            return null;
+        }
+    }
+
 
     /// <inheritdoc/>
     public override AccessToken GetToken(TokenRequestContext requestContext, CancellationToken cancellationToken)

src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs

@@ -182,12 +182,12 @@ All callers (GraphApiService, ArmApiService, TeamsGraphBackendConfigurator, ...)
         v
 AuthenticationService.GetAccessTokenAsync(resource, tenantId)
         |
-        +-- Check persistent disk cache (%LocalAppData%\Agent365\token-cache.json)
-        |       Cache key: {resource}[:tenant:{tenantId}][:user:{userId}]
+        +-- MsalBrowserCredential.GetTokenAsync(scopes) - silent-first against the
+        |       OS-protected MSAL cache (msal-token-cache: DPAPI/Keychain/owner-only file)
         |
-        +-- Cache hit + not expiring: return token immediately (0 prompts)
+        +-- Valid token in MSAL cache: returned silently (0 prompts)
         |
-        +-- Cache miss / expired: MsalBrowserCredential.GetTokenAsync(scopes)
+        +-- Miss / expired: interactive acquisition
                 |
                 +-- Windows:  WAM broker (no browser, SSO, CAP-compliant)
                 +-- macOS:    System browser → device code fallback if restricted
@@ -205,7 +205,7 @@ AuthenticationService.GetAccessTokenAsync(resource, tenantId)
 
 To prevent WAM from selecting a stale or wrong account, a login hint (UPN) is resolved before interactive auth:
 1. `AzCliHelper.ResolveLoginHintAsync()` — reads `az account show` if az CLI is present
-2. `AuthenticationService.ResolveLoginHintFromCacheAsync()` — decodes `upn`/`preferred_username` from a cached JWT if az CLI is unavailable
+2. `AuthenticationService.ResolveLoginHintFromCacheAsync()` — reads the first account's UPN from the OS-protected MSAL persistent cache if az CLI is unavailable
 
 ### IAuthenticationService Interface
 
@@ -215,7 +215,7 @@ Only `GetAccessTokenAsync` and `ResolveLoginHintFromCacheAsync` are on the inter
 
 ### Token Caching
 
-- **Persistent cache** (`AuthenticationService`): survives across CLI invocations, keyed by resource + tenant + user
+- **MSAL persistent cache** (`MsalBrowserCredential`): the sole token store - survives across CLI invocations, OS-protected at rest (DPAPI on Windows, Keychain on macOS, owner-only file on Linux). `AuthenticationService` no longer writes a separate cache.
 - **Process-level login hint cache** (`AzCliHelper`): caches the result of `az account show` for the process lifetime — invalidated after `az login` operations
 
 ### Platform Notes

src/Microsoft.Agents.A365.DevTools.Cli/design.md

@@ -37,9 +37,9 @@ public void ApplicationName_ShouldBeCorrect()
     }
 
     [Fact]
-    public void TokenCacheFileName_ShouldBeCorrect()
+    public void MsalCacheFileName_ShouldBeCorrect()
     {
-        AuthenticationConstants.TokenCacheFileName.Should().Be("auth-token.json");
+        AuthenticationConstants.MsalCacheFileName.Should().Be("msal-token-cache");
     }
 
     [Fact]

src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Constants/AuthenticationConstantsTests.cs

@@ -32,35 +32,36 @@ public AuthenticationServiceTests()
         _mockLogger = Substitute.For<ILogger<AuthenticationService>>();
         _authService = new AuthenticationService(_mockLogger);
 
-        // Get the actual cache path that the service uses
+        // The constructor best-effort deletes any legacy plaintext auth-token.json (migration cleanup).
+        // These tests pin that behavior against the same legacy path the service computes internally.
         var appDataPath = Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData);
         _testCachePath = Path.Combine(appDataPath, "Microsoft.Agents.A365.DevTools.Cli", "auth-token.json");
     }
 
     public void Dispose()
     {
-        // Clean up test cache
-        _authService.ClearCache();
+        // Best-effort cleanup of any legacy file a test created (service no longer exposes ClearCache).
+        try { if (File.Exists(_testCachePath)) File.Delete(_testCachePath); } catch (Exception) { /* best-effort */ }
         GC.SuppressFinalize(this);
     }
 
     [Fact]
-    public void ClearCache_WhenCacheExists_RemovesFile()
+    public void Constructor_WhenLegacyPlaintextCacheExists_RemovesIt()
     {
-        // Arrange
+        // Arrange — simulate a plaintext auth-token.json left by an older CLI version
         var cacheDir = Path.GetDirectoryName(_testCachePath)!;
         Directory.CreateDirectory(cacheDir);
         File.WriteAllText(_testCachePath, "test content");
 
-        // Act
-        _authService.ClearCache();
+        // Act — constructing the service runs the one-time migration cleanup
+        _ = new AuthenticationService(_mockLogger);
 
         // Assert
         File.Exists(_testCachePath).Should().BeFalse();
     }
 
     [Fact]
-    public void ClearCache_WhenCacheDoesNotExist_DoesNotThrow()
+    public void Constructor_WhenNoLegacyCache_DoesNotThrow()
     {
         // Arrange
         if (File.Exists(_testCachePath))
@@ -69,7 +70,7 @@ public void ClearCache_WhenCacheDoesNotExist_DoesNotThrow()
         }
 
         // Act
-        Action act = () => _authService.ClearCache();
+        Action act = () => new AuthenticationService(_mockLogger);
 
         // Assert
         act.Should().NotThrow();
@@ -801,13 +802,59 @@ public TestableAuthenticationService(
             _deviceCodeCredential = deviceCodeCredential;
         }
 
-        protected override TokenCredential CreateBrowserCredential(string clientId, string tenantId, string? loginHint = null)
+        protected override TokenCredential CreateBrowserCredential(string clientId, string tenantId, string? loginHint = null, bool forceRefresh = false)
             => _browserCredential;
 
         protected override TokenCredential CreateDeviceCodeCredential(string clientId, string tenantId)
             => _deviceCodeCredential;
     }
 
+    [Fact]
+    public async Task GetAccessTokenAsync_OnSuccess_DoesNotWritePlaintextTokenFile()
+    {
+        // Regression guard for the Safe-Secrets fix: the service must NOT persist access tokens to
+        // a plaintext file (the removed auth-token.json). Token persistence is delegated entirely
+        // to the OS-protected MSAL persistent cache. We drive a successful acquisition through the
+        // browser-credential seam (no real auth) and assert no plaintext token file appears in the
+        // cache directory.
+        var expectedToken = "stub-access-token";
+        var browserCredential = new StubTokenCredential(expectedToken, DateTimeOffset.UtcNow.AddHours(1));
+        var deviceCodeCredential = new StubTokenCredential("unused", DateTimeOffset.UtcNow.AddHours(1));
+
+        var logger = Substitute.For<ILogger<AuthenticationService>>();
+        var sut = new TestableAuthenticationService(logger, browserCredential, deviceCodeCredential);
+
+        var cacheDir = Path.Combine(
+            Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData),
+            AuthenticationConstants.ApplicationName);
+        var legacyPlaintextPath = Path.Combine(cacheDir, "auth-token.json");
+
+        // Ensure no stale legacy file is present before the act.
+        if (File.Exists(legacyPlaintextPath))
+        {
+            File.Delete(legacyPlaintextPath);
+        }
+
+        try
+        {
+            // Act
+            var result = await sut.GetAccessTokenAsync(
+                "https://agent365.svc.cloud.microsoft",
+                forceRefresh: true,
+                useInteractiveBrowser: true);
+
+            // Assert — token returned, but nothing written to the plaintext cache path.
+            result.Should().Be(expectedToken);
+            File.Exists(legacyPlaintextPath).Should().BeFalse(
+                because: "the Safe-Secrets fix removed the plaintext access-token cache; only the " +
+                         "OS-protected MSAL cache may persist tokens");
+        }
+        finally
+        {
+            sut.ClearCache();
+        }
+    }
+
     [Fact]
     public async Task GetAccessTokenAsync_WhenBrowserThrowsPlatformNotSupported_FallsBackToDeviceCode()
     {
@@ -992,70 +1039,31 @@ private static string BuildJwt(object payload)
         return $"header.{payloadB64Url}.signature";
     }
 
-    private void WriteTokenCache(string accessToken)
-    {
-        var cacheDir = Path.GetDirectoryName(_testCachePath)!;
-        Directory.CreateDirectory(cacheDir);
-        var cache = new
-        {
-            Tokens = new Dictionary<string, object>
-            {
-                ["key"] = new { AccessToken = accessToken, ExpiresOn = DateTime.UtcNow.AddHours(1), TenantId = "tid" }
-            }
-        };
-        File.WriteAllText(_testCachePath, System.Text.Json.JsonSerializer.Serialize(cache));
-    }
-
-    [Fact]
-    public async Task ResolveLoginHintFromCacheAsync_WhenCacheHasUpnClaim_ReturnsUpn()
-    {
-        WriteTokenCache(BuildJwt(new { upn = "user@test.com" }));
-
-        var result = await _authService.ResolveLoginHintFromCacheAsync();
-
-        result.Should().Be("user@test.com",
-            because: "the upn claim in the cached JWT should be returned as the login hint");
-    }
+    // NOTE: ResolveLoginHintFromCacheAsync now reads the first account's UPN from the OS-protected
+    // MSAL persistent cache (DPAPI/Keychain/0600 file) via
+    // MsalBrowserCredential.TryGetCachedAccountUsernameAsync, rather than decoding a JWT from the
+    // removed plaintext auth-token.json. Populating that cache requires a real MSAL token exchange,
+    // which cannot run in a unit test without interactive auth. The contract verified here is the
+    // safe-degradation path: when no account is cached, the method must return null (a fallback
+    // behind az CLI) and never throw. End-to-end UPN resolution is covered by manual/integration runs.
 
     [Fact]
-    public async Task ResolveLoginHintFromCacheAsync_WhenCacheHasPreferredUsernameClaim_ReturnsIt()
-    {
-        WriteTokenCache(BuildJwt(new { preferred_username = "user@contoso.com" }));
-
-        var result = await _authService.ResolveLoginHintFromCacheAsync();
-
-        result.Should().Be("user@contoso.com",
-            because: "preferred_username is the fallback claim when upn is absent");
-    }
-
-    [Fact]
-    public async Task ResolveLoginHintFromCacheAsync_WhenCacheFileDoesNotExist_ReturnsNull()
+    public async Task ResolveLoginHintFromCacheAsync_WhenNoAccountCached_ReturnsNullAndDoesNotThrow()
     {
+        // Arrange — ensure no legacy plaintext cache lingers (the method no longer reads it anyway).
         _authService.ClearCache();
 
-        var result = await _authService.ResolveLoginHintFromCacheAsync();
-
-        result.Should().BeNull(because: "no cache file means no login hint can be resolved");
-    }
-
-    [Fact]
-    public async Task ResolveLoginHintFromCacheAsync_WhenJwtHasNoUpnClaims_ReturnsNull()
-    {
-        WriteTokenCache(BuildJwt(new { sub = "some-subject", oid = "some-oid" }));
-
-        var result = await _authService.ResolveLoginHintFromCacheAsync();
-
-        result.Should().BeNull(because: "a JWT without upn or preferred_username cannot provide a login hint");
-    }
-
-    [Fact]
-    public async Task ResolveLoginHintFromCacheAsync_WhenJwtIsMalformed_ReturnsNull()
-    {
-        WriteTokenCache("not-a-valid-jwt");
-
-        var result = await _authService.ResolveLoginHintFromCacheAsync();
-
-        result.Should().BeNull(because: "a malformed JWT should be swallowed and null returned");
+        // Act
+        Func<Task<string?>> act = async () => await _authService.ResolveLoginHintFromCacheAsync();
+
+        // Assert — best-effort fallback: empty MSAL cache resolves to no hint, never an exception.
+        string? result = null;
+        await act.Should().NotThrowAsync(
+            because: "login-hint resolution is a fallback behind az CLI; an empty or unreadable MSAL " +
+                     "cache must yield null rather than surfacing an error to the caller");
+        result = await _authService.ResolveLoginHintFromCacheAsync();
+        result.Should().BeNull(
+            because: "no cached MSAL account means no login hint can be resolved");
     }
 
     #endregion
@@ -1081,8 +1089,8 @@ public SequencedAuthenticationService(
             : base(logger)
             => _credentials = new Queue<TokenCredential>(credentials);
 
-        protected override TokenCredential CreateBrowserCredential(string clientId, string tenantId, string? loginHint = null)
-            => _credentials.Count > 0 ? _credentials.Dequeue() : base.CreateBrowserCredential(clientId, tenantId, loginHint);
+        protected override TokenCredential CreateBrowserCredential(string clientId, string tenantId, string? loginHint = null, bool forceRefresh = false)
+            => _credentials.Count > 0 ? _credentials.Dequeue() : base.CreateBrowserCredential(clientId, tenantId, loginHint, forceRefresh);
     }
 
     // Computes the MSAL cache path so tests can back it up and restore it.