diff --git a/GVFS/GVFS.Common/Git/GitAuthentication.cs b/GVFS/GVFS.Common/Git/GitAuthentication.cs index 37c3563b5..9e294f5d1 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 IGVFSConfigRequestor ConfigRequestorOverride { get; set; } + private GitSsl GitSsl { get; } public void ApproveCredentials(ITracer tracer, string credentialString) @@ -260,13 +280,21 @@ public bool TryInitializeAndQueryGVFSConfig( errorMessage = null; isAuthFailure = false; - using (ConfigHttpRequestor configRequestor = 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(); @@ -276,9 +304,42 @@ 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. + // 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}", 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 95b531bb3..7b46e9fe4 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; @@ -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 0f9767dde..98b8298a9 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 new file mode 100644 index 000000000..641a8c3e6 --- /dev/null +++ b/GVFS/GVFS.Common/Http/IGVFSConfigRequestor.cs @@ -0,0 +1,33 @@ +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 . + /// + internal 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. + /// + /// 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 forceAnonymous = false); + } +} diff --git a/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs b/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs index 25aa675b7..04e00099b 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,147 @@ 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() + { + 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.ConfigRequestorOverride = 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.ConfigRequestorOverride = 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(HttpStatusCode.RequestTimeout)] + [TestCase(HttpStatusCode.InternalServerError)] + [TestCase(HttpStatusCode.ServiceUnavailable)] + [TestCase(HttpStatusCode.BadRequest)] + [TestCase(null)] + public void IndeterminateConfigProbeDoesNotLeaveAuthAnonymous(HttpStatusCode? status) + { + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + + 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.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.ConfigRequestorOverride = 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 000000000..64e35ec29 --- /dev/null +++ b/GVFS/GVFS.UnitTests/Mock/Http/MockGVFSConfigRequestor.cs @@ -0,0 +1,91 @@ +using GVFS.Common; +using GVFS.Common.Http; +using System; +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. + /// + internal class MockGVFSConfigRequestor : IGVFSConfigRequestor + { + private readonly bool succeedAnonymously; + private readonly HttpStatusCode? statusCode; + + private MockGVFSConfigRequestor(bool succeedAnonymously, HttpStatusCode? statusCode) + { + this.succeedAnonymously = succeedAnonymously; + 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. + /// + 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, bool forceAnonymous = false) + { + this.QueryCount++; + this.LastQueryForcedAnonymous = forceAnonymous; + this.OnQuery?.Invoke(forceAnonymous); + + 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() + { + } + } +}