From 21923d7ae10f509d0e0a56c16be588a031502490 Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Mon, 31 Aug 2026 14:33:04 -0700 Subject: [PATCH 1/2] Do not fall back to anonymous when the /gvfs/config probe is indeterminate Mount initializes authentication by querying /gvfs/config without credentials. A success means the server allows anonymous access. A 401 means credentials are required. Any other outcome - a timeout, a 5xx, or a socket error - says nothing about authentication, but the code left IsAnonymous at its default of true and marked initialization complete. That state is unrecoverable for the life of the mount process. HttpRequestor omits the Authorization header while IsAnonymous is true, and the short-circuit in SendRequest means TryGetCredentials is never called, so no credential is ever fetched. The Azure DevOps cache server answers such a request with 400 and the body "A valid Basic Authorization header is required." A 400 is not retryable, RejectCredentials is a no-op because no credential was ever cached, and initialization is already latched, so the probe never runs again. Mount proceeds past this failure when a cache server is configured, so the repo stays mounted but cannot download objects or blob sizes. Directory enumeration then fails with SizesUnavailableException and git reports tracked files as deleted. Every enumeration retries and fails the same way, which floods the mount log. Only a remount clears the state. Default IsAnonymous to false and set it explicitly on the indeterminate branch. Anonymous access is now an affirmative determination that requires a successful unauthenticated probe. If the server does allow anonymous access, it ignores the Authorization header that GVFS sends after an indeterminate probe. Extract IGVFSConfigRequestor from ConfigHttpRequestor and add an internal factory seam so tests can drive each probe outcome without a server. Five tests cover the three branches; the three that pin the fix fail if either half of it is reverted. Assisted-by: Claude Opus 5 Signed-off-by: Tyrie Vella --- GVFS/GVFS.Common/Git/GitAuthentication.cs | 44 +++++++- GVFS/GVFS.Common/Http/ConfigHttpRequestor.cs | 2 +- GVFS/GVFS.Common/Http/IGVFSConfigRequestor.cs | 28 +++++ .../Git/GitAuthenticationTests.cs | 106 ++++++++++++++++++ .../Mock/Http/MockGVFSConfigRequestor.cs | 73 ++++++++++++ 5 files changed, 249 insertions(+), 4 deletions(-) create mode 100644 GVFS/GVFS.Common/Http/IGVFSConfigRequestor.cs create mode 100644 GVFS/GVFS.UnitTests/Mock/Http/MockGVFSConfigRequestor.cs diff --git a/GVFS/GVFS.Common/Git/GitAuthentication.cs b/GVFS/GVFS.Common/Git/GitAuthentication.cs index 37c3563b51..0f4e6335d8 100644 --- a/GVFS/GVFS.Common/Git/GitAuthentication.cs +++ b/GVFS/GVFS.Common/Git/GitAuthentication.cs @@ -56,7 +56,19 @@ public bool IsBackingOff } } - public bool IsAnonymous { get; private set; } = true; + /// + /// True only when the server is known to allow anonymous access. + /// + /// + /// This defaults to false, because anonymous access is an affirmative + /// determination that requires a successful unauthenticated probe of + /// /gvfs/config. While this is true, + /// omits the Authorization + /// header and never calls , so a default of + /// true makes every request unauthenticated when the probe does not run or + /// does not complete. + /// + public bool IsAnonymous { get; private set; } /// /// How long a caller of will wait for @@ -66,6 +78,14 @@ public bool IsBackingOff /// internal int InitializationWaitTimeoutMs { get; set; } = BackgroundCredentialTimeoutMs; + /// + /// Test seam for the /gvfs/config probe. When null, production code uses + /// . Unit tests substitute a fake so the + /// probe outcome - anonymous success, 401, or an indeterminate network + /// failure - can be driven deterministically. + /// + internal Func ConfigRequestorFactory { get; set; } + private GitSsl GitSsl { get; } public void ApproveCredentials(ITracer tracer, string credentialString) @@ -260,7 +280,9 @@ public bool TryInitializeAndQueryGVFSConfig( errorMessage = null; isAuthFailure = false; - using (ConfigHttpRequestor configRequestor = new ConfigHttpRequestor(tracer, enlistment, retryConfig)) + using (IGVFSConfigRequestor configRequestor = this.ConfigRequestorFactory != null + ? this.ConfigRequestorFactory(tracer, enlistment, retryConfig) + : new ConfigHttpRequestor(tracer, enlistment, retryConfig)) { HttpStatusCode? httpStatus; @@ -276,9 +298,25 @@ public bool TryInitializeAndQueryGVFSConfig( if (httpStatus != HttpStatusCode.Unauthorized) { + // The probe did not determine whether the server allows anonymous + // access. It failed for a reason unrelated to authentication - a + // timeout, a 5xx, or a socket error. Assume authentication is + // required. If the server does allow anonymous access it ignores + // the Authorization header we then send. + // + // Treating this as anonymous is unrecoverable for the life of the + // process: SendRequest would omit the Authorization header on every + // later request and never call TryGetCredentials, the Azure DevOps + // cache server answers 400 ("A valid Basic Authorization header is + // required."), a 400 is not retryable, RejectCredentials is a no-op + // because no credential was ever cached, and initialization is + // already latched - so re-initialization throws. Mount proceeds + // when a cache server is configured, so the repo stays mounted but + // cannot hydrate or enumerate until it is remounted. + this.IsAnonymous = false; this.MarkInitialized(); errorMessage = "Unable to query /gvfs/config"; - tracer.RelatedWarning("{0}: Config query failed with status {1}", nameof(this.TryInitializeAndQueryGVFSConfig), httpStatus?.ToString() ?? "None"); + tracer.RelatedWarning("{0}: Config query failed with status {1}; assuming authentication is required", nameof(this.TryInitializeAndQueryGVFSConfig), httpStatus?.ToString() ?? "None"); return false; } diff --git a/GVFS/GVFS.Common/Http/ConfigHttpRequestor.cs b/GVFS/GVFS.Common/Http/ConfigHttpRequestor.cs index 95b531bb36..def2869a4d 100644 --- a/GVFS/GVFS.Common/Http/ConfigHttpRequestor.cs +++ b/GVFS/GVFS.Common/Http/ConfigHttpRequestor.cs @@ -7,7 +7,7 @@ namespace GVFS.Common.Http { - public class ConfigHttpRequestor : HttpRequestor + public class ConfigHttpRequestor : HttpRequestor, IGVFSConfigRequestor { private readonly string repoUrl; diff --git a/GVFS/GVFS.Common/Http/IGVFSConfigRequestor.cs b/GVFS/GVFS.Common/Http/IGVFSConfigRequestor.cs new file mode 100644 index 0000000000..b248b7beae --- /dev/null +++ b/GVFS/GVFS.Common/Http/IGVFSConfigRequestor.cs @@ -0,0 +1,28 @@ +using System; +using System.Net; + +namespace GVFS.Common.Http +{ + /// + /// Queries the server's /gvfs/config endpoint. Extracted from + /// so that + /// can be + /// driven with a deterministic probe outcome in unit tests. Production code + /// always uses . + /// + public interface IGVFSConfigRequestor : IDisposable + { + /// + /// Queries /gvfs/config. + /// + /// Whether to trace failures as errors. + /// The parsed config when this returns true. + /// + /// The HTTP status the server responded with, or null when the request + /// never produced an HTTP response (for example a DNS or socket failure). + /// + /// The failure description when this returns false. + /// True when the config was retrieved. + bool TryQueryGVFSConfig(bool logErrors, out ServerGVFSConfig serverGVFSConfig, out HttpStatusCode? httpStatus, out string errorMessage); + } +} diff --git a/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs b/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs index 25aa675b7e..d968fc3589 100644 --- a/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs +++ b/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs @@ -1,12 +1,15 @@ using System; using System.Linq; +using System.Net; using System.Threading; using System.Threading.Tasks; +using GVFS.Common; using GVFS.Common.Git; using GVFS.Tests; using GVFS.Tests.Should; using GVFS.UnitTests.Mock.Common; using GVFS.UnitTests.Mock.Git; +using GVFS.UnitTests.Mock.Http; using NUnit.Framework; namespace GVFS.UnitTests.Git @@ -342,6 +345,109 @@ public void TryGetCredentialsWaitsForBackgroundInitializationThenSucceeds() authString.ShouldNotBeNull("A credential string should be returned"); } + [TestCase] + public void AuthIsNotAnonymousBeforeInitialization() + { + MockGitProcess gitProcess = this.GetGitProcess(); + + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + + dut.IsAnonymous.ShouldEqual(false, "Auth must not report anonymous before a probe has proven the server allows it"); + } + + [TestCase] + public void AnonymousConfigProbeSuccessLeavesAuthAnonymous() + { + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + MockGVFSConfigRequestor requestor = MockGVFSConfigRequestor.AnonymousSucceeds(); + + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + dut.ConfigRequestorFactory = (t, e, r) => requestor; + + dut.TryInitializeAndQueryGVFSConfig(tracer, null, new RetryConfig(), out _, out _, out bool isAuthFailure) + .ShouldEqual(true, "An anonymous server should initialize successfully"); + + dut.IsAnonymous.ShouldEqual(true, "A successful unauthenticated probe means the server allows anonymous access"); + isAuthFailure.ShouldEqual(false, "An anonymous success is not an auth failure"); + requestor.QueryCount.ShouldEqual(1, "An anonymous server needs only the single unauthenticated query"); + } + + [TestCase] + public void UnauthorizedConfigProbeRequiresAuthentication() + { + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + dut.ConfigRequestorFactory = (t, e, r) => MockGVFSConfigRequestor.RequiresAuthentication(); + + dut.TryInitializeAndQueryGVFSConfig(tracer, null, new RetryConfig(), out _, out _, out _); + + dut.IsAnonymous.ShouldEqual(false, "A 401 from the unauthenticated probe means credentials are required"); + dut.TryGetCredentials(tracer, out string authString, out _).ShouldEqual(true, "Credentials should be available after a 401 probe"); + authString.ShouldNotBeNull("A credential should have been fetched"); + } + + /// + /// A config query that fails for a reason unrelated to authentication - a + /// timeout, a 5xx, or a socket error - says nothing about whether the server + /// allows anonymous access. Treating it as anonymous is unrecoverable: + /// HttpRequestor.SendRequest omits the Authorization header and never calls + /// TryGetCredentials while IsAnonymous is true, the Azure DevOps cache server + /// answers 400, a 400 is not retryable, and initialization is already latched + /// so the probe never runs again. Mount proceeds when a cache server is + /// configured, so the repo stays mounted but cannot hydrate or enumerate. + /// + [TestCase] + public void IndeterminateConfigProbeDoesNotLeaveAuthAnonymous() + { + foreach (HttpStatusCode? status in new HttpStatusCode?[] + { + HttpStatusCode.RequestTimeout, + HttpStatusCode.InternalServerError, + HttpStatusCode.ServiceUnavailable, + HttpStatusCode.BadRequest, + null, + }) + { + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + dut.ConfigRequestorFactory = (t, e, r) => MockGVFSConfigRequestor.Indeterminate(status); + + dut.TryInitializeAndQueryGVFSConfig(tracer, null, new RetryConfig(), out _, out _, out bool isAuthFailure) + .ShouldEqual(false, $"A config query that failed with {status?.ToString() ?? "no response"} should report failure"); + + dut.IsAnonymous.ShouldEqual( + false, + $"An indeterminate config probe ({status?.ToString() ?? "no response"}) must not leave auth in anonymous mode"); + + isAuthFailure.ShouldEqual(false, "An indeterminate failure is not an authentication failure"); + } + } + + [TestCase] + public void IndeterminateConfigProbeStillAuthenticatesLaterRequests() + { + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + dut.ConfigRequestorFactory = (t, e, r) => MockGVFSConfigRequestor.Indeterminate(HttpStatusCode.RequestTimeout); + + dut.TryInitializeAndQueryGVFSConfig(tracer, null, new RetryConfig(), out _, out _, out _); + + // Mount proceeds past this failure when a cache server is configured, so + // object downloads and directory enumeration must still be able to + // authenticate rather than silently issuing unauthenticated requests. + dut.IsAnonymous.ShouldEqual(false, "Later requests must send an Authorization header"); + dut.TryGetCredentials(tracer, out string authString, out string error) + .ShouldEqual(true, "Credentials should still be obtainable after an indeterminate probe: " + error); + authString.ShouldNotBeNull("A credential should have been fetched"); + } + private MockGitProcess GetGitProcess() { MockGitProcess gitProcess = new MockGitProcess(); diff --git a/GVFS/GVFS.UnitTests/Mock/Http/MockGVFSConfigRequestor.cs b/GVFS/GVFS.UnitTests/Mock/Http/MockGVFSConfigRequestor.cs new file mode 100644 index 0000000000..d07a0a2c69 --- /dev/null +++ b/GVFS/GVFS.UnitTests/Mock/Http/MockGVFSConfigRequestor.cs @@ -0,0 +1,73 @@ +using GVFS.Common; +using GVFS.Common.Http; +using System.Net; + +namespace GVFS.UnitTests.Mock.Http +{ + /// + /// Drives + /// with a fixed /gvfs/config probe outcome so the anonymous / authenticated / + /// indeterminate branches can be tested without a server. + /// + public class MockGVFSConfigRequestor : IGVFSConfigRequestor + { + private readonly bool succeedAnonymously; + private readonly HttpStatusCode? statusCode; + + private MockGVFSConfigRequestor(bool succeedAnonymously, HttpStatusCode? statusCode) + { + this.succeedAnonymously = succeedAnonymously; + this.statusCode = statusCode; + } + + public int QueryCount { get; private set; } + + /// + /// The server allows anonymous access: the unauthenticated probe returns the config. + /// + public static MockGVFSConfigRequestor AnonymousSucceeds() + { + return new MockGVFSConfigRequestor(succeedAnonymously: true, statusCode: HttpStatusCode.OK); + } + + /// + /// The server requires authentication: the unauthenticated probe returns 401. + /// + public static MockGVFSConfigRequestor RequiresAuthentication() + { + return new MockGVFSConfigRequestor(succeedAnonymously: false, statusCode: HttpStatusCode.Unauthorized); + } + + /// + /// The probe failed for a reason that says nothing about authentication, so + /// whether the server allows anonymous access is unknown. Pass null for + /// to model a failure with no HTTP response at all. + /// + public static MockGVFSConfigRequestor Indeterminate(HttpStatusCode? statusCode) + { + return new MockGVFSConfigRequestor(succeedAnonymously: false, statusCode: statusCode); + } + + public bool TryQueryGVFSConfig(bool logErrors, out ServerGVFSConfig serverGVFSConfig, out HttpStatusCode? httpStatus, out string errorMessage) + { + this.QueryCount++; + + httpStatus = this.statusCode; + + if (this.succeedAnonymously) + { + serverGVFSConfig = new ServerGVFSConfig(); + errorMessage = null; + return true; + } + + serverGVFSConfig = null; + errorMessage = "Mock config query failure"; + return false; + } + + public void Dispose() + { + } + } +} From 61dd891a0d403c1ac9d9960cea0947b5fe0a4182 Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Wed, 2 Sep 2026 09:59:59 -0700 Subject: [PATCH 2/2] Send the initial /gvfs/config probe without credentials Review found that the previous commit broke the probe it depends on. The probe that determines whether the server allows anonymous access was unauthenticated only as a side effect of IsAnonymous defaulting to true. Defaulting it to false removed that mechanism without replacing it, so HttpRequestor.SendRequest saw IsAnonymous == false, called TryGetCredentials, and waited on initializationComplete - the event that only the same call stack can set. Each wait cost the full 120 second initialization timeout and returned a retryable 401, so a mount stalled for roughly 14 minutes across the default seven attempts and then wrongly concluded that credentials were required. This happened on every mount. The mock-based tests never reached SendRequest, so they could not catch it. Make the probe explicitly unauthenticated instead of implicitly so. SendRequest takes a forceAnonymous parameter, ConfigHttpRequestor.TryQueryGVFSConfig passes it through, and the initial probe sets it. A request that carries no credentials now also skips ApproveCredentials and keeps the existing anonymous handling of a 401, which is the definitive answer that the server requires authentication. SendRequest also resolves the decision once into a local instead of reading IsAnonymous four times. Another thread can change the property mid-request, and the credential gate, the Authorization header, and the response handling must agree on one value. Trace the indeterminate branch with Keywords.Telemetry and structured metadata. The plain RelatedWarning overload traces with Keywords.None, which the telemetry listener filters out, so the affected population could not be measured. Replace the ConfigRequestorFactory Func with a plain ConfigRequestorOverride property, matching the settable-knob style this class already uses for InitializationWaitTimeoutMs, and make IGVFSConfigRequestor internal since no production code needs the polymorphism. Add InitialConfigProbeIsSentWithoutCredentials, which mirrors the SendRequest credential gate and fails fast if the probe ever waits on its own initialization. Convert the indeterminate-status test to real TestCase rows so each status reports independently. Mutation testing, per half: removing forceAnonymous fails the new test; reverting the default fails AuthIsNotAnonymousBeforeInitialization; deleting the explicit IsAnonymous assignment on the indeterminate branch fails nothing, so that line is kept only as defensive symmetry and is commented as such. Full unit suite: 948 tests, 0 failed. Assisted-by: Claude Opus 5 Signed-off-by: Tyrie Vella --- GVFS/GVFS.Common/Git/GitAuthentication.cs | 35 ++++++-- GVFS/GVFS.Common/Http/ConfigHttpRequestor.cs | 5 +- GVFS/GVFS.Common/Http/HttpRequestor.cs | 27 ++++-- GVFS/GVFS.Common/Http/IGVFSConfigRequestor.cs | 9 +- .../Git/GitAuthenticationTests.cs | 88 +++++++++++++------ .../Mock/Http/MockGVFSConfigRequestor.cs | 22 ++++- 6 files changed, 144 insertions(+), 42 deletions(-) diff --git a/GVFS/GVFS.Common/Git/GitAuthentication.cs b/GVFS/GVFS.Common/Git/GitAuthentication.cs index 0f4e6335d8..9e294f5d1f 100644 --- a/GVFS/GVFS.Common/Git/GitAuthentication.cs +++ b/GVFS/GVFS.Common/Git/GitAuthentication.cs @@ -84,7 +84,7 @@ public bool IsBackingOff /// probe outcome - anonymous success, 401, or an indeterminate network /// failure - can be driven deterministically. /// - internal Func ConfigRequestorFactory { get; set; } + internal IGVFSConfigRequestor ConfigRequestorOverride { get; set; } private GitSsl GitSsl { get; } @@ -280,15 +280,21 @@ public bool TryInitializeAndQueryGVFSConfig( errorMessage = null; isAuthFailure = false; - using (IGVFSConfigRequestor configRequestor = this.ConfigRequestorFactory != null - ? this.ConfigRequestorFactory(tracer, enlistment, retryConfig) - : new ConfigHttpRequestor(tracer, enlistment, retryConfig)) + IGVFSConfigRequestor configRequestor = this.ConfigRequestorOverride ?? new ConfigHttpRequestor(tracer, enlistment, retryConfig); + using (configRequestor) { HttpStatusCode? httpStatus; // First attempt without credentials. If anonymous access works, // we get the config in a single request. - if (configRequestor.TryQueryGVFSConfig(false, out serverGVFSConfig, out httpStatus, out _)) + // + // forceAnonymous is required, not incidental: this probe is what + // DETERMINES whether the server allows anonymous access, so it must + // not consult the answer it is computing. Without it, SendRequest + // sees IsAnonymous == false and calls TryGetCredentials, which waits + // for the initialization this very call stack is performing - a + // self-deadlock that stalls every mount until the wait times out. + if (configRequestor.TryQueryGVFSConfig(false, out serverGVFSConfig, out httpStatus, out _, forceAnonymous: true)) { this.IsAnonymous = true; this.MarkInitialized(); @@ -313,10 +319,27 @@ public bool TryInitializeAndQueryGVFSConfig( // already latched - so re-initialization throws. Mount proceeds // when a cache server is configured, so the repo stays mounted but // cannot hydrate or enumerate until it is remounted. + // Assigning IsAnonymous here is defensive: the field already + // defaults to false and no earlier path in this method can set it + // true without returning, so this states the outcome explicitly + // rather than relying on the default staying false. this.IsAnonymous = false; this.MarkInitialized(); errorMessage = "Unable to query /gvfs/config"; - tracer.RelatedWarning("{0}: Config query failed with status {1}; assuming authentication is required", nameof(this.TryInitializeAndQueryGVFSConfig), httpStatus?.ToString() ?? "None"); + + // Emit this with Keywords.Telemetry so the population hitting an + // indeterminate probe is measurable in the field. The plain + // RelatedWarning(string, params object[]) overload traces with + // Keywords.None, which TelemetryDaemonEventListener filters out. + EventMetadata indeterminateMetadata = new EventMetadata(new Dictionary + { + ["Area"] = nameof(GitAuthentication), + ["HttpStatus"] = httpStatus?.ToString() ?? "None", + }); + tracer.RelatedWarning( + indeterminateMetadata, + $"{nameof(this.TryInitializeAndQueryGVFSConfig)}: Config query failed with status {httpStatus?.ToString() ?? "None"}; assuming authentication is required", + Keywords.Telemetry); return false; } diff --git a/GVFS/GVFS.Common/Http/ConfigHttpRequestor.cs b/GVFS/GVFS.Common/Http/ConfigHttpRequestor.cs index def2869a4d..7b46e9fe42 100644 --- a/GVFS/GVFS.Common/Http/ConfigHttpRequestor.cs +++ b/GVFS/GVFS.Common/Http/ConfigHttpRequestor.cs @@ -17,7 +17,7 @@ public ConfigHttpRequestor(ITracer tracer, Enlistment enlistment, RetryConfig re this.repoUrl = enlistment.RepoUrl; } - public bool TryQueryGVFSConfig(bool logErrors, out ServerGVFSConfig serverGVFSConfig, out HttpStatusCode? httpStatus, out string errorMessage) + public bool TryQueryGVFSConfig(bool logErrors, out ServerGVFSConfig serverGVFSConfig, out HttpStatusCode? httpStatus, out string errorMessage, bool forceAnonymous = false) { serverGVFSConfig = null; httpStatus = null; @@ -56,7 +56,8 @@ public bool TryQueryGVFSConfig(bool logErrors, out ServerGVFSConfig serverGVFSCo gvfsConfigEndpoint, HttpMethod.Get, requestContent: null, - cancellationToken: CancellationToken.None)) + cancellationToken: CancellationToken.None, + forceAnonymous: forceAnonymous)) { if (response.HasErrors) { diff --git a/GVFS/GVFS.Common/Http/HttpRequestor.cs b/GVFS/GVFS.Common/Http/HttpRequestor.cs index 0f9767dde1..98b8298a92 100644 --- a/GVFS/GVFS.Common/Http/HttpRequestor.cs +++ b/GVFS/GVFS.Common/Http/HttpRequestor.cs @@ -98,17 +98,31 @@ public void Dispose() } } + /// + /// Sends the request without credentials regardless of the authentication + /// state. Required by the /gvfs/config probe that DETERMINES whether the + /// server allows anonymous access: that probe runs before initialization + /// completes, so it must not call , + /// which would wait for the initialization this very request is part of. + /// protected GitEndPointResponseData SendRequest( long requestId, Uri requestUri, HttpMethod httpMethod, string requestContent, CancellationToken cancellationToken, - MediaTypeWithQualityHeaderValue acceptType = null) + MediaTypeWithQualityHeaderValue acceptType = null, + bool forceAnonymous = false) { + // Resolve the anonymous decision once. Another thread can change + // IsAnonymous while this request is in flight, and the credential + // gate, the Authorization header, and the response handling below + // must all agree on a single value. + bool sendAnonymous = forceAnonymous || this.authentication.IsAnonymous; + string authString = null; string errorMessage; - if (!this.authentication.IsAnonymous && + if (!sendAnonymous && !this.authentication.TryGetCredentials(this.Tracer, out authString, out errorMessage)) { return new GitEndPointResponseData( @@ -127,7 +141,7 @@ protected GitEndPointResponseData SendRequest( request.Headers.UserAgent.Add(this.userAgentHeader); - if (!this.authentication.IsAnonymous) + if (!sendAnonymous) { request.Headers.Authorization = new AuthenticationHeaderValue("Basic", authString); } @@ -205,7 +219,7 @@ protected GitEndPointResponseData SendRequest( string contentType = GetSingleHeaderOrEmpty(response.Content.Headers, "Content-Type"); responseMetadata.Add("ContentType", contentType); - if (!this.authentication.IsAnonymous) + if (!sendAnonymous) { this.authentication.ApproveCredentials(this.Tracer, authString); } @@ -227,8 +241,11 @@ protected GitEndPointResponseData SendRequest( bool shouldRetry = ShouldRetry(response.StatusCode); if (response.StatusCode == HttpStatusCode.Unauthorized && - this.authentication.IsAnonymous) + sendAnonymous) { + // The request carried no credentials, so there is nothing to + // reject. For the initial probe this is the definitive answer + // that the server requires authentication. shouldRetry = false; errorMessage = "Anonymous request was rejected with a 401"; } diff --git a/GVFS/GVFS.Common/Http/IGVFSConfigRequestor.cs b/GVFS/GVFS.Common/Http/IGVFSConfigRequestor.cs index b248b7beae..641a8c3e63 100644 --- a/GVFS/GVFS.Common/Http/IGVFSConfigRequestor.cs +++ b/GVFS/GVFS.Common/Http/IGVFSConfigRequestor.cs @@ -10,7 +10,7 @@ namespace GVFS.Common.Http /// driven with a deterministic probe outcome in unit tests. Production code /// always uses . /// - public interface IGVFSConfigRequestor : IDisposable + internal interface IGVFSConfigRequestor : IDisposable { /// /// Queries /gvfs/config. @@ -22,7 +22,12 @@ public interface IGVFSConfigRequestor : IDisposable /// never produced an HTTP response (for example a DNS or socket failure). /// /// The failure description when this returns false. + /// + /// Sends the request without credentials regardless of the authentication + /// state. The probe that determines whether the server allows anonymous + /// access must set this. + /// /// True when the config was retrieved. - bool TryQueryGVFSConfig(bool logErrors, out ServerGVFSConfig serverGVFSConfig, out HttpStatusCode? httpStatus, out string errorMessage); + bool TryQueryGVFSConfig(bool logErrors, out ServerGVFSConfig serverGVFSConfig, out HttpStatusCode? httpStatus, out string errorMessage, bool forceAnonymous = false); } } diff --git a/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs b/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs index d968fc3589..04e00099b4 100644 --- a/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs +++ b/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs @@ -345,6 +345,50 @@ public void TryGetCredentialsWaitsForBackgroundInitializationThenSucceeds() authString.ShouldNotBeNull("A credential string should be returned"); } + /// + /// The initial /gvfs/config probe is what DETERMINES whether the server + /// allows anonymous access, so it must be sent without credentials. + /// gates the + /// Authorization header on and + /// calls when it is false. + /// Because IsAnonymous now defaults to false, an unforced probe would + /// take that path and block on initializationComplete - the event that + /// only this same call stack can set - stalling every mount for the whole + /// wait timeout, on every retry. + /// + [TestCase] + public void InitialConfigProbeIsSentWithoutCredentials() + { + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + MockGVFSConfigRequestor requestor = MockGVFSConfigRequestor.AnonymousSucceeds(); + + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + + // Keep the failure fast: if the probe ever does wait for initialization, + // this bounds the stall instead of hanging the suite. + dut.InitializationWaitTimeoutMs = 500; + dut.ConfigRequestorOverride = requestor; + + bool credentialsRequestedDuringProbe = false; + requestor.OnQuery = forceAnonymous => + { + // Mirror HttpRequestor.SendRequest's credential gate exactly. + bool sendAnonymous = forceAnonymous || dut.IsAnonymous; + if (!sendAnonymous) + { + credentialsRequestedDuringProbe = true; + dut.TryGetCredentials(tracer, out _, out _); + } + }; + + dut.TryInitializeAndQueryGVFSConfig(tracer, null, new RetryConfig(), out _, out _, out _) + .ShouldEqual(true, "The anonymous probe should have succeeded"); + + requestor.LastQueryForcedAnonymous.ShouldEqual(true, "The initial config probe must be forced anonymous so it cannot wait on its own initialization"); + credentialsRequestedDuringProbe.ShouldEqual(false, "The initial config probe must not require credentials"); + } + [TestCase] public void AuthIsNotAnonymousBeforeInitialization() { @@ -363,7 +407,7 @@ public void AnonymousConfigProbeSuccessLeavesAuthAnonymous() MockGVFSConfigRequestor requestor = MockGVFSConfigRequestor.AnonymousSucceeds(); GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); - dut.ConfigRequestorFactory = (t, e, r) => requestor; + dut.ConfigRequestorOverride = requestor; dut.TryInitializeAndQueryGVFSConfig(tracer, null, new RetryConfig(), out _, out _, out bool isAuthFailure) .ShouldEqual(true, "An anonymous server should initialize successfully"); @@ -380,7 +424,7 @@ public void UnauthorizedConfigProbeRequiresAuthentication() MockGitProcess gitProcess = this.GetGitProcess(); GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); - dut.ConfigRequestorFactory = (t, e, r) => MockGVFSConfigRequestor.RequiresAuthentication(); + dut.ConfigRequestorOverride = MockGVFSConfigRequestor.RequiresAuthentication(); dut.TryInitializeAndQueryGVFSConfig(tracer, null, new RetryConfig(), out _, out _, out _); @@ -399,33 +443,27 @@ public void UnauthorizedConfigProbeRequiresAuthentication() /// so the probe never runs again. Mount proceeds when a cache server is /// configured, so the repo stays mounted but cannot hydrate or enumerate. /// - [TestCase] - public void IndeterminateConfigProbeDoesNotLeaveAuthAnonymous() + [TestCase(HttpStatusCode.RequestTimeout)] + [TestCase(HttpStatusCode.InternalServerError)] + [TestCase(HttpStatusCode.ServiceUnavailable)] + [TestCase(HttpStatusCode.BadRequest)] + [TestCase(null)] + public void IndeterminateConfigProbeDoesNotLeaveAuthAnonymous(HttpStatusCode? status) { - foreach (HttpStatusCode? status in new HttpStatusCode?[] - { - HttpStatusCode.RequestTimeout, - HttpStatusCode.InternalServerError, - HttpStatusCode.ServiceUnavailable, - HttpStatusCode.BadRequest, - null, - }) - { - MockTracer tracer = new MockTracer(); - MockGitProcess gitProcess = this.GetGitProcess(); + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); - GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); - dut.ConfigRequestorFactory = (t, e, r) => MockGVFSConfigRequestor.Indeterminate(status); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + dut.ConfigRequestorOverride = MockGVFSConfigRequestor.Indeterminate(status); - dut.TryInitializeAndQueryGVFSConfig(tracer, null, new RetryConfig(), out _, out _, out bool isAuthFailure) - .ShouldEqual(false, $"A config query that failed with {status?.ToString() ?? "no response"} should report failure"); + dut.TryInitializeAndQueryGVFSConfig(tracer, null, new RetryConfig(), out _, out _, out bool isAuthFailure) + .ShouldEqual(false, $"A config query that failed with {status?.ToString() ?? "no response"} should report failure"); - dut.IsAnonymous.ShouldEqual( - false, - $"An indeterminate config probe ({status?.ToString() ?? "no response"}) must not leave auth in anonymous mode"); + dut.IsAnonymous.ShouldEqual( + false, + $"An indeterminate config probe ({status?.ToString() ?? "no response"}) must not leave auth in anonymous mode"); - isAuthFailure.ShouldEqual(false, "An indeterminate failure is not an authentication failure"); - } + isAuthFailure.ShouldEqual(false, "An indeterminate failure is not an authentication failure"); } [TestCase] @@ -435,7 +473,7 @@ public void IndeterminateConfigProbeStillAuthenticatesLaterRequests() MockGitProcess gitProcess = this.GetGitProcess(); GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); - dut.ConfigRequestorFactory = (t, e, r) => MockGVFSConfigRequestor.Indeterminate(HttpStatusCode.RequestTimeout); + dut.ConfigRequestorOverride = MockGVFSConfigRequestor.Indeterminate(HttpStatusCode.RequestTimeout); dut.TryInitializeAndQueryGVFSConfig(tracer, null, new RetryConfig(), out _, out _, out _); diff --git a/GVFS/GVFS.UnitTests/Mock/Http/MockGVFSConfigRequestor.cs b/GVFS/GVFS.UnitTests/Mock/Http/MockGVFSConfigRequestor.cs index d07a0a2c69..64e35ec29e 100644 --- a/GVFS/GVFS.UnitTests/Mock/Http/MockGVFSConfigRequestor.cs +++ b/GVFS/GVFS.UnitTests/Mock/Http/MockGVFSConfigRequestor.cs @@ -1,5 +1,6 @@ using GVFS.Common; using GVFS.Common.Http; +using System; using System.Net; namespace GVFS.UnitTests.Mock.Http @@ -9,7 +10,7 @@ namespace GVFS.UnitTests.Mock.Http /// with a fixed /gvfs/config probe outcome so the anonymous / authenticated / /// indeterminate branches can be tested without a server. /// - public class MockGVFSConfigRequestor : IGVFSConfigRequestor + internal class MockGVFSConfigRequestor : IGVFSConfigRequestor { private readonly bool succeedAnonymously; private readonly HttpStatusCode? statusCode; @@ -20,8 +21,23 @@ private MockGVFSConfigRequestor(bool succeedAnonymously, HttpStatusCode? statusC this.statusCode = statusCode; } + /// + /// Number of times the probe was issued. + /// public int QueryCount { get; private set; } + /// + /// The value of on the most recent probe. + /// + public bool LastQueryForcedAnonymous { get; private set; } + + /// + /// Runs while the probe is "in flight", so a test can observe the + /// authentication state that a real requestor would see at that moment. + /// The argument is the probe's forceAnonymous value. + /// + public Action OnQuery { get; set; } + /// /// The server allows anonymous access: the unauthenticated probe returns the config. /// @@ -48,9 +64,11 @@ public static MockGVFSConfigRequestor Indeterminate(HttpStatusCode? statusCode) return new MockGVFSConfigRequestor(succeedAnonymously: false, statusCode: statusCode); } - public bool TryQueryGVFSConfig(bool logErrors, out ServerGVFSConfig serverGVFSConfig, out HttpStatusCode? httpStatus, out string errorMessage) + public bool TryQueryGVFSConfig(bool logErrors, out ServerGVFSConfig serverGVFSConfig, out HttpStatusCode? httpStatus, out string errorMessage, bool forceAnonymous = false) { this.QueryCount++; + this.LastQueryForcedAnonymous = forceAnonymous; + this.OnQuery?.Invoke(forceAnonymous); httpStatus = this.statusCode;