diff --git a/docs/hostprovider.md b/docs/hostprovider.md index 4fb56df00..d49b092f9 100644 --- a/docs/hostprovider.md +++ b/docs/hostprovider.md @@ -215,6 +215,17 @@ available and a terminal/TTY is attached then a provider MUST first check if terminal prompts are enabled (`ISettings.IsTerminalPromptsEnabled`), and an exception MUST be thrown if interaction has been disallowed. +Several GCM processes can need the same credential at the same time, for +example one per Git worktree when a shared cached token has expired. Prompting +the user once per process is not useful when a single sign-in satisfies all of +them. Where the providers share a token cache, a host provider SHOULD serialize +interactive authentication across processes and, once it holds the lock, retry +the non-interactive path before prompting, because another process may have +completed the sign-in in the meantime. The Microsoft authentication (Entra) +helper does this with a lock file in the GCM data directory, `entra-signin-*.lock` +(one per client, authority and cache). If the lock cannot be taken, for example +because the data directory is not writable, authentication continues without it. + User-facing output and prompts SHOULD use `ICommandContext.Console`. Output-only messages go to stderr; interactive prompts use the controlling terminal. Providers MUST NOT consume Git's stdin or write user messages to its diff --git a/src/Core.Tests/Authentication/Entra/EntraAuthenticationInteractiveLockTests.cs b/src/Core.Tests/Authentication/Entra/EntraAuthenticationInteractiveLockTests.cs new file mode 100644 index 000000000..be1824dff --- /dev/null +++ b/src/Core.Tests/Authentication/Entra/EntraAuthenticationInteractiveLockTests.cs @@ -0,0 +1,128 @@ +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using GitCredentialManager.Authentication.Entra; +using GitCredentialManager.Tests.Objects; +using Xunit; + +namespace GitCredentialManager.Tests.Authentication.Entra; + +public class EntraAuthenticationInteractiveLockTests : IDisposable +{ + private const string Authority = "https://login.microsoftonline.com/tenant-1"; + + private readonly TestCommandContext _context = new(); + private readonly string _root; + + public EntraAuthenticationInteractiveLockTests() + { + // The lock is a real file, so back the test file system with a real directory. + _root = Path.Combine(Path.GetTempPath(), $"gcmtest-lock-{Guid.NewGuid():N}"); + _context.FileSystem.UserDataDirectoryPath = Path.Combine(_root, ".gcm"); + Directory.CreateDirectory(_context.FileSystem.UserDataDirectoryPath); + } + + public void Dispose() + { + try { Directory.Delete(_root, recursive: true); } catch { /* best effort */ } + } + + private EntraAuthentication CreateAuth(string clientId = "client-1") => + new(_context, new PublicClientConfig { ClientId = clientId }); + + [Fact] + public async Task AcquireInteractiveLockAsync_NotHeld_ReturnsLock() + { + var auth = CreateAuth(); + + using IDisposable handle = await auth.AcquireInteractiveLockAsync(Authority, CancellationToken.None); + + Assert.NotNull(handle); + } + + [Fact] + public async Task AcquireInteractiveLockAsync_HeldByOther_WaitsUntilReleased() + { + var auth = CreateAuth(); + IDisposable first = await auth.AcquireInteractiveLockAsync(Authority, CancellationToken.None); + Assert.NotNull(first); + + Task second = auth.AcquireInteractiveLockAsync( + Authority, CancellationToken.None, TimeSpan.FromSeconds(30)); + + await Task.Delay(TimeSpan.FromSeconds(1)); + Assert.False(second.IsCompleted, "The second caller must wait while the first holds the lock."); + + first.Dispose(); + + using IDisposable handle = await second.WaitAsync(TimeSpan.FromSeconds(10)); + Assert.NotNull(handle); + } + + [Fact] + public async Task AcquireInteractiveLockAsync_HeldByOtherPastTimeout_ReturnsNull() + { + var auth = CreateAuth(); + using IDisposable first = await auth.AcquireInteractiveLockAsync(Authority, CancellationToken.None); + Assert.NotNull(first); + + IDisposable second = await auth.AcquireInteractiveLockAsync( + Authority, CancellationToken.None, TimeSpan.FromMilliseconds(100)); + + // Locking must never block authentication for good: the caller continues without it. + Assert.Null(second); + } + + [Fact] + public async Task AcquireInteractiveLockAsync_DifferentAuthority_DoesNotWait() + { + var auth = CreateAuth(); + using IDisposable first = await auth.AcquireInteractiveLockAsync(Authority, CancellationToken.None); + Assert.NotNull(first); + + using IDisposable second = await auth.AcquireInteractiveLockAsync( + "https://login.microsoftonline.com/tenant-2", CancellationToken.None, TimeSpan.FromMilliseconds(100)); + + Assert.NotNull(second); + } + + [Fact] + public async Task AcquireInteractiveLockAsync_DifferentClientId_DoesNotWait() + { + using IDisposable first = await CreateAuth("client-1") + .AcquireInteractiveLockAsync(Authority, CancellationToken.None); + Assert.NotNull(first); + + using IDisposable second = await CreateAuth("client-2") + .AcquireInteractiveLockAsync(Authority, CancellationToken.None, TimeSpan.FromMilliseconds(100)); + + Assert.NotNull(second); + } + + [Fact] + public async Task AcquireInteractiveLockAsync_Canceled_Throws() + { + var auth = CreateAuth(); + using IDisposable first = await auth.AcquireInteractiveLockAsync(Authority, CancellationToken.None); + Assert.NotNull(first); + + using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(200)); + + await Assert.ThrowsAnyAsync(() => + auth.AcquireInteractiveLockAsync(Authority, cts.Token, TimeSpan.FromSeconds(30))); + } + + [Fact] + public async Task AcquireInteractiveLockAsync_UserDataDirectoryUnusable_ReturnsNull() + { + // A file where the directory should be: the lock file can never be created. + string blocker = Path.Combine(_root, "blocker"); + File.WriteAllText(blocker, string.Empty); + _context.FileSystem.UserDataDirectoryPath = Path.Combine(blocker, ".gcm"); + + IDisposable handle = await CreateAuth().AcquireInteractiveLockAsync(Authority, CancellationToken.None); + + Assert.Null(handle); + } +} diff --git a/src/Core/Authentication/Entra/EntraAuthentication.PublicClient.cs b/src/Core/Authentication/Entra/EntraAuthentication.PublicClient.cs index e0d506499..37019ba1f 100644 --- a/src/Core/Authentication/Entra/EntraAuthentication.PublicClient.cs +++ b/src/Core/Authentication/Entra/EntraAuthentication.PublicClient.cs @@ -218,6 +218,26 @@ public async Task GetTokenForUserAsync(string[] scop ThrowIfUserInteractionDisabled(); + // Many GCM processes can need to sign in at the same time (e.g. one per worktree after the cached + // token expired). They all share one token cache, so only one needs to prompt the user. Take a lock + // so the others wait, then retry silent authentication to pick up the token the first one stored. + using var interactiveLock = await AcquireInteractiveLockAsync(authority, ct); + if (interactiveLock is not null) + { + // The account may only have reached the cache while we were waiting, so resolve it again. + if (account is not null) + { + msalAccount = await ResolveAccountAsync(app, account) ?? msalAccount; + } + + result = await GetTokenForUserSilentAsync(app, scopes, msalAccount, ct); + if (result is not null) + { + Trace2.WriteData(Trace2Category, "flow", "silent_after_wait"); + return AuthResult.FromMsalResult(result); + } + } + // Try interactive auth if we couldn't do so with a cached account if (useBroker) { @@ -233,6 +253,60 @@ public async Task GetTokenForUserAsync(string[] scop return AuthResult.FromMsalResult(result); } + /// + /// Serialize interactive sign-in across GCM processes that share the same token cache. + /// + /// + /// The held lock, or if it could not be taken (in which case the caller proceeds + /// without it, as before). + /// + internal /* for testing purposes */ async Task AcquireInteractiveLockAsync( + string authority, CancellationToken ct, TimeSpan? timeout = null) + { + string key = $"{PublicClientConfig.ClientId}|{authority}|{PublicClientConfig.UseSharedCache}" + .ToLowerInvariant(); + string hash = Convert.ToHexString( + System.Security.Cryptography.SHA256.HashData(Encoding.UTF8.GetBytes(key)))[..16]; + + string dir = Context.FileSystem.UserDataDirectoryPath; + string path = System.IO.Path.Combine(dir, $"entra-signin-{hash}.lock"); + + timeout ??= TimeSpan.FromMinutes(5); + var sw = Stopwatch.StartNew(); + try + { + Context.FileSystem.CreateDirectory(dir); + + while (true) + { + ct.ThrowIfCancellationRequested(); + try + { + // The OS releases the handle if this process exits or crashes, so no stale locks. + var stream = new System.IO.FileStream(path, System.IO.FileMode.OpenOrCreate, + System.IO.FileAccess.ReadWrite, System.IO.FileShare.None); + Trace2.WriteData(Trace2Category, "signin_lock", "acquired"); + return stream; + } + catch (System.IO.IOException ex) when (ex is not (System.IO.FileNotFoundException or + System.IO.DirectoryNotFoundException) && + sw.Elapsed < timeout) + { + Context.Trace.WriteLine("Another GCM process is signing in; waiting for it to finish..."); + await Task.Delay(TimeSpan.FromMilliseconds(500), ct); + } + } + } + catch (Exception ex) when (ex is System.IO.IOException or UnauthorizedAccessException) + { + // Locking is an optimisation; never fail authentication because of it. + Trace2.WriteData(Trace2Category, "signin_lock", "unavailable"); + Context.Trace.WriteLine("Unable to take the sign-in lock; continuing without it."); + Context.Trace.WriteException(ex); + return null; + } + } + private async Task GetTokenForUserSilentAsync( IPublicClientApplication app, string[] scopes, IAccount msalAccount, CancellationToken ct) {