diff --git a/GVFS/GVFS.Common/Git/GVFSGitObjects.cs b/GVFS/GVFS.Common/Git/GVFSGitObjects.cs index 06a1af9908..38065f28f0 100644 --- a/GVFS/GVFS.Common/Git/GVFSGitObjects.cs +++ b/GVFS/GVFS.Common/Git/GVFSGitObjects.cs @@ -67,10 +67,14 @@ public enum BlobHydrationFailureCategory /// internal class DownloadAttemptResult { - public DownloadAttemptResult(DownloadAndSaveObjectResult result, HttpStatusCode? httpStatusCode) + public DownloadAttemptResult( + DownloadAndSaveObjectResult result, + HttpStatusCode? httpStatusCode, + bool shouldRecordFailure = true) { this.Result = result; this.HttpStatusCode = httpStatusCode; + this.ShouldRecordFailure = shouldRecordFailure; } public DownloadAndSaveObjectResult Result { get; } @@ -78,6 +82,8 @@ public DownloadAttemptResult(DownloadAndSaveObjectResult result, HttpStatusCode? // The HTTP status of the last download attempt, or null when no HTTP response was // received (for example an exhausted retry that ended in an exception). public HttpStatusCode? HttpStatusCode { get; } + + public bool ShouldRecordFailure { get; } } protected GVFSContext Context { get; private set; } @@ -217,7 +223,11 @@ public virtual bool TryCopyBlobContentStream( downloadSucceededButCopyFailed = true; } - return new RetryWrapper.CallbackResult(error: null, shouldRetry: true); + return new RetryWrapper.CallbackResult( + error: null, + shouldRetry: true, + result: false, + shouldRecordFailure: lastDownloadResult?.ShouldRecordFailure ?? true); } }); @@ -367,17 +377,26 @@ private DownloadAttemptResult DoDownloadAndSaveObject( { if (output.Succeeded && output.Result.Success) { - return new DownloadAttemptResult(DownloadAndSaveObjectResult.Success, httpStatusCode); + return new DownloadAttemptResult( + DownloadAndSaveObjectResult.Success, + httpStatusCode, + output.ShouldRecordFailure); } if (output.Result.HttpStatusCodeResult == HttpStatusCode.NotFound) { this.objectNegativeCache.AddOrUpdate(objectId, DateTime.Now, (unused1, unused2) => DateTime.Now); - return new DownloadAttemptResult(DownloadAndSaveObjectResult.ObjectNotOnServer, httpStatusCode); + return new DownloadAttemptResult( + DownloadAndSaveObjectResult.ObjectNotOnServer, + httpStatusCode, + output.ShouldRecordFailure); } } - return new DownloadAttemptResult(DownloadAndSaveObjectResult.Error, httpStatusCode); + return new DownloadAttemptResult( + DownloadAndSaveObjectResult.Error, + httpStatusCode, + output.ShouldRecordFailure); } } } \ No newline at end of file diff --git a/GVFS/GVFS.Common/Git/GitAuthentication.cs b/GVFS/GVFS.Common/Git/GitAuthentication.cs index c35a11a823..5f9db0c724 100644 --- a/GVFS/GVFS.Common/Git/GitAuthentication.cs +++ b/GVFS/GVFS.Common/Git/GitAuthentication.cs @@ -12,6 +12,13 @@ namespace GVFS.Common.Git { public class GitAuthentication { + private enum AuthenticationState + { + Unknown = 0, + Anonymous, + Authenticated, + } + private const double MaxBackoffSeconds = 30; public const int DefaultCredentialTimeoutMs = 30_000; public const int BackgroundCredentialTimeoutMs = 120_000; @@ -36,6 +43,8 @@ public class GitAuthentication private bool isCachedCredentialStringApproved = false; private bool isInitialized; + private int authenticationState; + private int initializationStarted; public GitAuthentication(GitProcess git, string repoUrl) { @@ -56,7 +65,38 @@ public bool IsBackingOff } } - public bool IsAnonymous { get; private set; } = true; + /// + /// True only when the server is known to allow anonymous access. + /// + /// + /// This is false while the state is unknown. While true, + /// omits the Authorization + /// header and never calls . + /// + public bool IsAnonymous + { + get => this.GetAuthenticationState() == AuthenticationState.Anonymous; + } + + public bool IsSendingAnonymousRequests => this.ShouldSendAnonymous; + + internal bool ShouldSendAnonymous => + !this.IsInitializationStarted || + (this.initializationComplete.IsSet && this.GetAuthenticationState() != AuthenticationState.Authenticated); + + /// + /// Indicates whether authentication initialization has started. + /// + internal bool IsInitializationStarted => Volatile.Read(ref this.initializationStarted) != 0; + + /// + /// Marks authentication initialization as scheduled so that requests wait + /// for its result instead of being sent anonymously. + /// + internal void MarkInitializationStarted() + { + Interlocked.Exchange(ref this.initializationStarted, 1); + } /// /// How long a caller of will wait for @@ -66,6 +106,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) @@ -169,6 +217,19 @@ public void RejectCredentials(ITracer tracer, string credentialString) } } + /// + /// Gets the credential to send with a request. + /// + /// + /// The Basic credential, or null when this returns true because the server + /// allows anonymous access or its authentication requirement remains unknown. + /// Callers must check for null before using it. + /// + /// The failure reason when this returns false. + /// + /// True when the request can proceed: with the credential in + /// , or anonymously when it is null. + /// public bool TryGetCredentials(ITracer tracer, out string credentialString, out string errorMessage) { if (!this.isInitialized) @@ -186,6 +247,13 @@ public bool TryGetCredentials(ITracer tracer, out string credentialString, out s } } + if (this.GetAuthenticationState() != AuthenticationState.Authenticated) + { + credentialString = null; + errorMessage = null; + return true; + } + credentialString = this.cachedCredentialString; if (credentialString == null) { @@ -256,19 +324,51 @@ public bool TryInitializeAndQueryGVFSConfig( throw new InvalidOperationException("Already initialized"); } + this.MarkInitializationStarted(); + + try + { + return this.TryInitializeAndQueryGVFSConfigCore( + tracer, + enlistment, + retryConfig, + out serverGVFSConfig, + out errorMessage, + out isAuthFailure, + credentialTimeoutMs); + } + finally + { + if (!this.isInitialized) + { + this.MarkInitialized(); + } + } + } + + private bool TryInitializeAndQueryGVFSConfigCore( + ITracer tracer, + Enlistment enlistment, + RetryConfig retryConfig, + out ServerGVFSConfig serverGVFSConfig, + out string errorMessage, + out bool isAuthFailure, + int credentialTimeoutMs) + { serverGVFSConfig = null; 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; + string probeError; - // 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 _)) + // The probe must not consult the authentication state it determines. + if (configRequestor.TryQueryGVFSConfig(false, out serverGVFSConfig, out httpStatus, out probeError, forceAnonymous: true)) { - this.IsAnonymous = true; + this.MarkAnonymousAccessSucceeded(); this.MarkInitialized(); tracer.RelatedInfo("{0}: Anonymous access succeeded, config obtained in one request", nameof(this.TryInitializeAndQueryGVFSConfig)); return true; @@ -276,14 +376,31 @@ public bool TryInitializeAndQueryGVFSConfig( if (httpStatus != HttpStatusCode.Unauthorized) { + // The probe did not determine whether the server allows anonymous + // access. Keep the state unknown so later requests can try + // anonymously and promote to authenticated if the server rejects them. this.MarkInitialized(); - errorMessage = "Unable to query /gvfs/config"; - tracer.RelatedWarning("{0}: Config query failed with status {1}", nameof(this.TryInitializeAndQueryGVFSConfig), httpStatus?.ToString() ?? "None"); + errorMessage = "Unable to query /gvfs/config" + + (string.IsNullOrEmpty(probeError) ? string.Empty : ": " + probeError) + + ". The authentication requirement remains unknown."; + + // The probe is unauthenticated, so the error text carries no credential. + EventMetadata indeterminateMetadata = new EventMetadata(new Dictionary + { + ["Area"] = nameof(GitAuthentication), + ["HttpStatus"] = httpStatus?.ToString() ?? "None", + ["ProbeError"] = probeError ?? string.Empty, + ["AuthenticationState"] = AuthenticationState.Unknown.ToString(), + }); + tracer.RelatedWarning( + indeterminateMetadata, + $"{nameof(this.TryInitializeAndQueryGVFSConfig)}: Config query failed with status {httpStatus?.ToString() ?? "None"}; authentication requirement remains unknown", + Keywords.Telemetry); return false; } // Server requires authentication — fetch credentials - this.IsAnonymous = false; + this.MarkAuthenticationRequired(); if (!this.TryCallGitCredential(tracer, out errorMessage, credentialTimeoutMs)) { @@ -327,13 +444,60 @@ internal bool TryInitializeAndRequireAuth(ITracer tracer, out string errorMessag throw new InvalidOperationException("Already initialized"); } - if (this.TryCallGitCredential(tracer, out errorMessage)) + this.MarkInitializationStarted(); + this.MarkAuthenticationRequired(); + + try + { + if (this.TryCallGitCredential(tracer, out errorMessage)) + { + this.MarkInitialized(); + return true; + } + + return false; + } + finally + { + if (!this.isInitialized) + { + this.MarkInitialized(); + } + } + } + + internal void MarkAnonymousAccessSucceeded() + { + Interlocked.CompareExchange( + ref this.authenticationState, + (int)AuthenticationState.Anonymous, + (int)AuthenticationState.Unknown); + } + + internal void MarkAuthenticationRequired() + { + Interlocked.Exchange(ref this.authenticationState, (int)AuthenticationState.Authenticated); + } + + internal bool PromoteToAuthenticationRequired(out string previousAuthenticationState) + { + AuthenticationState previousState = (AuthenticationState)Interlocked.Exchange( + ref this.authenticationState, + (int)AuthenticationState.Authenticated); + + previousAuthenticationState = previousState.ToString(); + + if (Interlocked.CompareExchange(ref this.initializationStarted, 1, 0) == 0) { this.MarkInitialized(); - return true; } - return false; + return previousState != AuthenticationState.Authenticated; + } + + private AuthenticationState GetAuthenticationState() + { + return (AuthenticationState)Volatile.Read(ref this.authenticationState); } public void ConfigureHttpClientHandlerSslIfNeeded(ITracer tracer, HttpClientHandler httpClientHandler, GitProcess gitProcess) diff --git a/GVFS/GVFS.Common/Http/ConfigHttpRequestor.cs b/GVFS/GVFS.Common/Http/ConfigHttpRequestor.cs index 95b531bb36..a341e778d2 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,11 +56,16 @@ public bool TryQueryGVFSConfig(bool logErrors, out ServerGVFSConfig serverGVFSCo gvfsConfigEndpoint, HttpMethod.Get, requestContent: null, - cancellationToken: CancellationToken.None)) + cancellationToken: CancellationToken.None, + forceAnonymous: forceAnonymous)) { if (response.HasErrors) { - return new RetryWrapper.CallbackResult(response.Error, response.ShouldRetry); + return new RetryWrapper.CallbackResult( + response.Error, + response.ShouldRetry, + result: null, + shouldRecordFailure: response.ShouldRecordFailure); } try @@ -89,6 +94,12 @@ public bool TryQueryGVFSConfig(bool logErrors, out ServerGVFSConfig serverGVFSCo httpStatus = httpException.StatusCode; errorMessage = httpException.Message; } + else + { + // No HTTP response (TLS, DNS, proxy, or socket failure): there is no + // status to report, so keep the exception text as the only diagnostic. + errorMessage = output.Error.Message; + } if (logErrors) { diff --git a/GVFS/GVFS.Common/Http/GitEndPointResponseData.cs b/GVFS/GVFS.Common/Http/GitEndPointResponseData.cs index 9eb6164219..0be18adc01 100644 --- a/GVFS/GVFS.Common/Http/GitEndPointResponseData.cs +++ b/GVFS/GVFS.Common/Http/GitEndPointResponseData.cs @@ -17,11 +17,18 @@ public class GitEndPointResponseData : IDisposable /// /// Constructor used when GitEndPointResponseData contains an error response /// - public GitEndPointResponseData(HttpStatusCode statusCode, Exception error, bool shouldRetry, HttpResponseMessage message, Action onResponseDisposed) + public GitEndPointResponseData( + HttpStatusCode statusCode, + Exception error, + bool shouldRetry, + HttpResponseMessage message, + Action onResponseDisposed, + bool shouldRecordFailure = true) { this.StatusCode = statusCode; this.Error = error; this.ShouldRetry = shouldRetry; + this.ShouldRecordFailure = shouldRetry && shouldRecordFailure; this.message = message; this.onResponseDisposed = onResponseDisposed; } @@ -40,6 +47,8 @@ public GitEndPointResponseData(HttpStatusCode statusCode, string contentType, St public bool ShouldRetry { get; } + public bool ShouldRecordFailure { get; } + public HttpStatusCode StatusCode { get; } public Stream Stream { get; private set; } diff --git a/GVFS/GVFS.Common/Http/GitObjectsHttpRequestor.cs b/GVFS/GVFS.Common/Http/GitObjectsHttpRequestor.cs index 278351b8ee..80d64030ec 100644 --- a/GVFS/GVFS.Common/Http/GitObjectsHttpRequestor.cs +++ b/GVFS/GVFS.Common/Http/GitObjectsHttpRequestor.cs @@ -121,7 +121,11 @@ public virtual List QueryForFileSizes(IEnumerable objectI if (response.HasErrors) { - return new RetryWrapper>.CallbackResult(response.Error, response.ShouldRetry); + return new RetryWrapper>.CallbackResult( + response.Error, + response.ShouldRetry, + result: null, + shouldRecordFailure: response.ShouldRecordFailure); } string objectSizesString = response.RetryableReadToEnd(); @@ -176,7 +180,11 @@ public virtual GitRefs QueryInfoRefs(string branch) { if (response.HasErrors) { - return new RetryWrapper.CallbackResult(response.Error, response.ShouldRetry); + return new RetryWrapper.CallbackResult( + response.Error, + response.ShouldRetry, + result: null, + shouldRecordFailure: response.ShouldRecordFailure); } List infoRefsResponse = response.RetryableReadAllLines(); @@ -438,7 +446,7 @@ public virtual RetryWrapper.InvocationResult TrySendProtoco response.Error, shouldFallBack || response.ShouldRetry, new GitObjectTaskResult(response.StatusCode, requestEndPoint), - shouldRecordFailure: response.ShouldRetry && !shouldFallBack); + shouldRecordFailure: response.ShouldRecordFailure && !shouldFallBack); } RetryWrapper.CallbackResult result; diff --git a/GVFS/GVFS.Common/Http/HttpRequestor.cs b/GVFS/GVFS.Common/Http/HttpRequestor.cs index ee1558abf2..4ea8e41d57 100644 --- a/GVFS/GVFS.Common/Http/HttpRequestor.cs +++ b/GVFS/GVFS.Common/Http/HttpRequestor.cs @@ -38,6 +38,20 @@ static HttpRequestor() } protected HttpRequestor(ITracer tracer, RetryConfig retryConfig, Enlistment enlistment) + : this(tracer, retryConfig, enlistment, handlerOverride: null) + { + } + + /// + /// Test-only constructor that injects the so + /// can run without network I/O. Production code uses + /// the overload that builds a configured . + /// + internal HttpRequestor( + ITracer tracer, + RetryConfig retryConfig, + Enlistment enlistment, + HttpMessageHandler handlerOverride) { this.RetryConfig = retryConfig; @@ -52,23 +66,28 @@ protected HttpRequestor(ITracer tracer, RetryConfig retryConfig, Enlistment enli TryApplyConnectionLimitFromConfig(tracer, enlistment); } - // WARNING: Do NOT set Credentials or ServerCredentials on this handler. - // - // Setting Credentials = CredentialCache.DefaultCredentials causes the handler - // to perform an NTLM/Negotiate challenge-response on every new connection. - // On SocketsHttpHandler this adds ~400ms per request vs ~14ms without. - // - // GVFS cache servers and Azure DevOps accept PAT/OAuth tokens via the - // "Authorization: Basic " header that SendRequest already attaches. - // Transport-level credentials are redundant and purely wasteful. - SocketsHttpHandler handler = new SocketsHttpHandler() + HttpMessageHandler handler = handlerOverride; + if (handler == null) { - MaxConnectionsPerServer = Environment.ProcessorCount, - PooledConnectionLifetime = Timeout.InfiniteTimeSpan, - PooledConnectionIdleTimeout = TimeSpan.FromMinutes(5), - }; + // WARNING: Do NOT set Credentials or ServerCredentials on this handler. + // + // Setting Credentials = CredentialCache.DefaultCredentials causes the handler + // to perform an NTLM/Negotiate challenge-response on every new connection. + // On SocketsHttpHandler this adds ~400ms per request vs ~14ms without. + // + // GVFS cache servers and Azure DevOps accept PAT/OAuth tokens via the + // "Authorization: Basic " header that SendRequest already attaches. + // Transport-level credentials are redundant and purely wasteful. + SocketsHttpHandler socketsHandler = new SocketsHttpHandler() + { + MaxConnectionsPerServer = Environment.ProcessorCount, + PooledConnectionLifetime = Timeout.InfiniteTimeSpan, + PooledConnectionIdleTimeout = TimeSpan.FromMinutes(5), + }; - this.authentication.ConfigureSocketsHandlerSslIfNeeded(this.Tracer, handler, enlistment.CreateGitProcess()); + this.authentication.ConfigureSocketsHandlerSslIfNeeded(this.Tracer, socketsHandler, enlistment.CreateGitProcess()); + handler = socketsHandler; + } this.client = new HttpClient(handler) { @@ -98,25 +117,37 @@ public void Dispose() } } + /// Sends without credentials regardless of authentication state. protected GitEndPointResponseData SendRequest( long requestId, Uri requestUri, HttpMethod httpMethod, string requestContent, CancellationToken cancellationToken, - MediaTypeWithQualityHeaderValue acceptType = null) + MediaTypeWithQualityHeaderValue acceptType = null, + bool forceAnonymous = false) { + // Before initialization starts, preserve anonymous requests. During + // initialization, callers wait for the auth decision to complete. + // Resolve the decision once so all request handling uses one value. + bool sendAnonymous = forceAnonymous || this.authentication.ShouldSendAnonymous; + string authString = null; string errorMessage; - if (!this.authentication.IsAnonymous && - !this.authentication.TryGetCredentials(this.Tracer, out authString, out errorMessage)) + if (!sendAnonymous) { - return new GitEndPointResponseData( - HttpStatusCode.Unauthorized, - new GitObjectsHttpException(HttpStatusCode.Unauthorized, errorMessage), - shouldRetry: true, - message: null, - onResponseDisposed: null); + if (!this.authentication.TryGetCredentials(this.Tracer, out authString, out errorMessage)) + { + return new GitEndPointResponseData( + HttpStatusCode.Unauthorized, + new GitObjectsHttpException(HttpStatusCode.Unauthorized, errorMessage), + shouldRetry: true, + message: null, + onResponseDisposed: null); + } + + // Initialization may finish anonymously while this request waits. + sendAnonymous = authString == null; } HttpRequestMessage request = new HttpRequestMessage(httpMethod, requestUri); @@ -127,7 +158,7 @@ protected GitEndPointResponseData SendRequest( request.Headers.UserAgent.Add(this.userAgentHeader); - if (!this.authentication.IsAnonymous) + if (!sendAnonymous) { request.Headers.Authorization = new AuthenticationHeaderValue("Basic", authString); } @@ -205,10 +236,14 @@ 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); } + else if (!forceAnonymous) + { + this.authentication.MarkAnonymousAccessSucceeded(); + } Stream responseStream = response.Content.ReadAsStreamAsync().GetAwaiter().GetResult(); @@ -225,12 +260,37 @@ protected GitEndPointResponseData SendRequest( int statusInt = (int)response.StatusCode; bool shouldRetry = ShouldRetry(response.StatusCode); + bool shouldRecordFailure = shouldRetry; - if (response.StatusCode == HttpStatusCode.Unauthorized && - this.authentication.IsAnonymous) + if (sendAnonymous && + ShouldRejectCredentials(response.StatusCode, errorMessage)) { - shouldRetry = false; - errorMessage = "Anonymous request was rejected with a 401"; + // A forced anonymous request is the initialization probe. Its + // caller owns the state transition and authenticated retry. + shouldRetry = !forceAnonymous; + if (!forceAnonymous) + { + shouldRecordFailure = false; + if (this.authentication.PromoteToAuthenticationRequired(out string previousAuthenticationState)) + { + EventMetadata authenticationMetadata = new EventMetadata + { + ["PreviousAuthenticationState"] = previousAuthenticationState, + ["AuthenticationState"] = "Authenticated", + ["HttpStatus"] = response.StatusCode, + }; + + this.Tracer.RelatedInfo( + authenticationMetadata, + "Anonymous request was rejected; authentication is now required"); + } + } + + errorMessage = string.Format( + "Anonymous request was rejected with error code {0} ({1}). Original error message from server: {2}", + statusInt, + response.StatusCode, + errorMessage); } else if (ShouldRejectCredentials(response.StatusCode, errorMessage)) { @@ -254,7 +314,8 @@ protected GitEndPointResponseData SendRequest( new GitObjectsHttpException(response.StatusCode, errorMessage), shouldRetry, message: response, - onResponseDisposed: () => availableConnections.Release()); + onResponseDisposed: () => availableConnections.Release(), + shouldRecordFailure: shouldRecordFailure); } } catch (TaskCanceledException) diff --git a/GVFS/GVFS.Common/Http/IGVFSConfigRequestor.cs b/GVFS/GVFS.Common/Http/IGVFSConfigRequestor.cs new file mode 100644 index 0000000000..85126896ba --- /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 authentication + /// state. The initial probe must set this because it determines whether + /// the server allows anonymous access. + /// + /// 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.Common/InternalsVisibleTo.cs b/GVFS/GVFS.Common/InternalsVisibleTo.cs index 0ba48d81be..f25a75048b 100644 --- a/GVFS/GVFS.Common/InternalsVisibleTo.cs +++ b/GVFS/GVFS.Common/InternalsVisibleTo.cs @@ -1,3 +1,4 @@ using System.Runtime.CompilerServices; [assembly: InternalsVisibleTo("GVFS.UnitTests")] +[assembly: InternalsVisibleTo("GVFS.Mount")] diff --git a/GVFS/GVFS.Common/RetryWrapper.cs b/GVFS/GVFS.Common/RetryWrapper.cs index 4d56ccc1fa..ae2e4bd9ba 100644 --- a/GVFS/GVFS.Common/RetryWrapper.cs +++ b/GVFS/GVFS.Common/RetryWrapper.cs @@ -95,7 +95,11 @@ public InvocationResult Invoke(Func toInvoke) if (!this.ShouldRetry(tryCount, null, result)) { - return new InvocationResult(tryCount, result.Error, result.Result); + return new InvocationResult( + tryCount, + result.Error, + result.Result, + result.ShouldRecordFailure); } } else @@ -201,15 +205,22 @@ public InvocationResult(int tryCount, Exception error) } public InvocationResult(int tryCount, Exception error, T result) + : this(tryCount, error, result, shouldRecordFailure: true) + { + } + + public InvocationResult(int tryCount, Exception error, T result, bool shouldRecordFailure) : this(tryCount, error) { this.Result = result; + this.ShouldRecordFailure = shouldRecordFailure; } public T Result { get; } public int Attempts { get; } public bool Succeeded { get; } public Exception Error { get; } + public bool ShouldRecordFailure { get; } = true; } public class CallbackResult diff --git a/GVFS/GVFS.Mount/InProcessMount.cs b/GVFS/GVFS.Mount/InProcessMount.cs index 579c65207e..0224a167e4 100644 --- a/GVFS/GVFS.Mount/InProcessMount.cs +++ b/GVFS/GVFS.Mount/InProcessMount.cs @@ -160,6 +160,10 @@ private void MountWithLockAcquired(EventLevel verbosity, Keywords keywords) }, Keywords.Telemetry); + // Deliberately unconditional: callbacks must wait for the auth result in + // every mount, not only when background auth is enabled. + this.enlistment.Authentication.MarkInitializationStarted(); + // With background auth enabled, auth/config is best-effort: mount can // proceed without it. We still attempt it so GCM can pop up a renewal // prompt for stale tokens, but we don't block mount on the result. A diff --git a/GVFS/GVFS.UnitTests/Common/RetryWrapperTests.cs b/GVFS/GVFS.UnitTests/Common/RetryWrapperTests.cs index 528252fa38..d023b7f336 100644 --- a/GVFS/GVFS.UnitTests/Common/RetryWrapperTests.cs +++ b/GVFS/GVFS.UnitTests/Common/RetryWrapperTests.cs @@ -317,6 +317,23 @@ public void CircuitBreakerIgnoresNonRetryableErrors() RetryCircuitBreaker.ConsecutiveFailures.ShouldEqual(0); } + [TestCase] + public void InvocationResultPreservesFailureAccountingDecision() + { + RetryWrapper wrapper = new RetryWrapper(1, CancellationToken.None); + + RetryWrapper.InvocationResult result = wrapper.Invoke( + tryCount => new RetryWrapper.CallbackResult( + new Exception("Expected retry"), + shouldRetry: true, + result: false, + shouldRecordFailure: false)); + + result.Succeeded.ShouldEqual(false); + result.ShouldRecordFailure.ShouldEqual(false); + RetryCircuitBreaker.ConsecutiveFailures.ShouldEqual(0); + } + [TestCase] [Category(CategoryConstants.ExceptionExpected)] public void CircuitBreakerClosesAfterCooldown() diff --git a/GVFS/GVFS.UnitTests/Git/GVFSGitObjectsTests.cs b/GVFS/GVFS.UnitTests/Git/GVFSGitObjectsTests.cs index 60826ca694..0d522bb5c8 100644 --- a/GVFS/GVFS.UnitTests/Git/GVFSGitObjectsTests.cs +++ b/GVFS/GVFS.UnitTests/Git/GVFSGitObjectsTests.cs @@ -83,6 +83,32 @@ public void CatchesFileNotFoundAfterFileDeleted() } } + [TestCase] + public void ExpectedAuthenticationChallengeDoesNotIncrementHydrationCircuitBreaker() + { + MockHttpGitObjects httpObjects = new MockHttpGitObjects + { + StatusCodeToReturn = HttpStatusCode.Unauthorized, + ShouldRecordFailureToReturn = false, + }; + GVFSGitObjects dut = this.CreateTestableGVFSGitObjects( + httpObjects, + new MissingGitRepo(new MockTracer()), + out _); + + dut.TryCopyBlobContentStream( + ValidTestObjectFileSha1, + CancellationToken.None, + GVFSGitObjects.RequestSource.FileStreamCallback, + (stream, length) => Assert.Fail("The challenged download should not provide content"), + out GVFSGitObjects.BlobHydrationFailureCategory _) + .ShouldEqual(false); + + RetryCircuitBreaker.ConsecutiveFailures.ShouldEqual( + 0, + "The outer hydration retry must preserve the expected-challenge classification"); + } + [TestCase] [Category(CategoryConstants.ExceptionExpected)] public void TerminalBlobHydrationFailureIsTaggedWithCategory() @@ -1050,6 +1076,7 @@ private MockHttpGitObjects(MockGVFSEnlistment enlistment) public Stream InputStream { get; set; } public string MediaType { get; set; } public HttpStatusCode? StatusCodeToReturn { get; set; } + public bool ShouldRecordFailureToReturn { get; set; } = true; // When true, TryDownloadObjects returns a failing result built from GitObjectTaskResult(bool), // i.e. Result is non-null but carries no HTTP status (HttpStatusCodeResult == 0). This @@ -1100,7 +1127,8 @@ public override RetryWrapper.InvocationResult TryDownloadOb return new RetryWrapper.InvocationResult( 0, error: null, - result: new GitObjectTaskResult(this.StatusCodeToReturn.Value)); + result: new GitObjectTaskResult(this.StatusCodeToReturn.Value), + shouldRecordFailure: this.ShouldRecordFailureToReturn); } if (this.FailWithoutStatus) @@ -1164,6 +1192,19 @@ public override bool TryCopyBlobContentStream(string blobSha, Action writeAction) + { + return false; + } + } + private class CoalescingTestHttpGitObjects : GitObjectsHttpRequestor { private readonly byte[] objectContents; diff --git a/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs b/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs index 6ebc4cb9de..4995577719 100644 --- a/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs +++ b/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs @@ -1,15 +1,21 @@ using System; +using System.Collections.Generic; using System.Diagnostics; using System.Linq; +using System.Net; +using System.Net.Http; using System.Text; using System.Threading; using System.Threading.Tasks; using GVFS.Common; using GVFS.Common.Git; +using GVFS.Common.Http; +using GVFS.Common.Tracing; 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 @@ -639,6 +645,437 @@ public void TryGetCertificatePasswordLogsConsoleVisibilityWhenCredentialFillFail } } + [TestCase] + public void InitialConfigProbeIsSentWithoutCredentials() + { + 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 _) + .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"); + } + + [TestCase] + public void SendRequestOmitsAuthorizationBeforeInitializationAndWhenForcedAnonymous() + { + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + MockGVFSEnlistment enlistment = new MockGVFSEnlistment(dut); + TestHttpMessageHandler handler = new TestHttpMessageHandler(); + + using (TestHttpRequestor requestor = new TestHttpRequestor(tracer, new RetryConfig(), enlistment, handler)) + { + using (GitEndPointResponseData response = requestor.SendRequest( + new Uri("https://mock.invalid/gvfs/config"), + forceAnonymous: false)) + { + response.HasErrors.ShouldEqual(false, "The uninitialized request should succeed anonymously"); + } + + using (GitEndPointResponseData response = requestor.SendRequest( + new Uri("https://mock.invalid/gvfs/config"), + forceAnonymous: true)) + { + response.HasErrors.ShouldEqual(false, "The forced-anonymous request should succeed"); + } + } + + handler.AuthorizationHeaders.Count.ShouldEqual(2, "The handler should receive both requests"); + handler.AuthorizationHeaders[0].ShouldEqual(null, "A request before initialization must remain anonymous"); + handler.AuthorizationHeaders[1].ShouldEqual(null, "A forced-anonymous request must omit Authorization"); + } + + [TestCase] + public void SendRequestSendsAuthorizationAfterAuthentication() + { + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + MockGVFSEnlistment enlistment = new MockGVFSEnlistment(dut); + TestHttpMessageHandler handler = new TestHttpMessageHandler(); + + dut.TryInitializeAndRequireAuth(tracer, out _) + .ShouldEqual(true, "The mock credential should initialize successfully"); + dut.TryGetCredentials(tracer, out string authString, out _) + .ShouldEqual(true, "The initialized credential should be available"); + + using (TestHttpRequestor requestor = new TestHttpRequestor(tracer, new RetryConfig(), enlistment, handler)) + { + using (GitEndPointResponseData response = requestor.SendRequest( + new Uri("https://mock.invalid/gvfs/config"), + forceAnonymous: false)) + { + response.HasErrors.ShouldEqual(false, "The authenticated request should succeed"); + } + + using (GitEndPointResponseData response = requestor.SendRequest( + new Uri("https://mock.invalid/gvfs/config"), + forceAnonymous: true)) + { + response.HasErrors.ShouldEqual(false, "The forced-anonymous request should succeed"); + } + } + + handler.AuthorizationHeaders.Count.ShouldEqual(2, "The handler should receive both requests"); + handler.AuthorizationHeaders[0].ShouldEqual( + "Basic " + authString, + "An authenticated request should send the cached credential"); + handler.AuthorizationHeaders[1].ShouldEqual(null, "forceAnonymous must suppress Authorization after initialization too"); + } + + [TestCase] + public void SendRequestWaitsForAnonymousInitializationWithoutFetchingCredentials() + { + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + MockGVFSEnlistment enlistment = new MockGVFSEnlistment(dut); + MockGVFSConfigRequestor configRequestor = MockGVFSConfigRequestor.AnonymousSucceeds(); + TestHttpMessageHandler handler = new TestHttpMessageHandler(); + using ManualResetEventSlim probeStarted = new ManualResetEventSlim(false); + using ManualResetEventSlim continueProbe = new ManualResetEventSlim(false); + using ManualResetEventSlim requestStarted = new ManualResetEventSlim(false); + configRequestor.BeforeQueryResult = _ => + { + probeStarted.Set(); + continueProbe.Wait(); + }; + dut.ConfigRequestorOverride = configRequestor; + dut.MarkInitializationStarted(); + + Task initialize = Task.Run(() => + dut.TryInitializeAndQueryGVFSConfig(tracer, enlistment, new RetryConfig(), out _, out _, out _)); + using TestHttpRequestor requestor = new TestHttpRequestor(tracer, new RetryConfig(), enlistment, handler); + Task send = null; + try + { + probeStarted.Wait(TimeSpan.FromSeconds(5)).ShouldBeTrue("Authentication probe should start"); + send = Task.Run(() => + { + requestStarted.Set(); + using GitEndPointResponseData response = requestor.SendRequest( + new Uri("https://mock.invalid/gvfs/config"), + forceAnonymous: false); + return !response.HasErrors; + }); + + requestStarted.Wait(TimeSpan.FromSeconds(5)).ShouldBeTrue("HTTP request should start"); + Thread.Sleep(100); + send.IsCompleted.ShouldEqual(false, "The HTTP request should wait for auth initialization"); + } + finally + { + continueProbe.Set(); + } + + Task.WaitAll(send == null ? new[] { initialize } : new Task[] { initialize, send }); + initialize.Result.ShouldEqual(true, "Anonymous config initialization should succeed"); + if (send != null) + { + send.Result.ShouldEqual(true, "The waiting request should continue after anonymous initialization"); + } + + handler.AuthorizationHeaders.Count.ShouldEqual(1, "The handler should receive one request"); + handler.AuthorizationHeaders[0].ShouldEqual(null, "The waiting request must not attach Authorization"); + gitProcess.CommandsRun.Any(command => command.Contains("credential fill")) + .ShouldEqual(false, "Anonymous initialization must not trigger credential lookup"); + } + + [TestCase] + public void SendRequestOmitsAuthorizationWhenForcedAnonymousAfterInitialization() + { + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + MockGVFSEnlistment enlistment = new MockGVFSEnlistment(dut); + TestHttpMessageHandler handler = new TestHttpMessageHandler(); + + dut.TryInitializeAndRequireAuth(tracer, out _).ShouldEqual(true, "Authentication should initialize"); + + using (TestHttpRequestor requestor = new TestHttpRequestor(tracer, new RetryConfig(), enlistment, handler)) + using (GitEndPointResponseData response = requestor.SendRequest( + new Uri("https://mock.invalid/gvfs/config"), + forceAnonymous: true)) + { + response.HasErrors.ShouldEqual(false, "The forced-anonymous request should succeed"); + } + + handler.AuthorizationHeaders.Count.ShouldEqual(1, "The handler should receive one request"); + handler.AuthorizationHeaders[0].ShouldEqual(null, "forceAnonymous must suppress Authorization after initialization"); + } + + [TestCase] + public void InitializationExceptionReleasesCredentialWaiters() + { + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl") + { + InitializationWaitTimeoutMs = 100, + ConfigRequestorOverride = MockGVFSConfigRequestor.Indeterminate(HttpStatusCode.RequestTimeout), + }; + MockGVFSConfigRequestor requestor = (MockGVFSConfigRequestor)dut.ConfigRequestorOverride; + requestor.BeforeQueryResult = _ => throw new InvalidOperationException("probe failed"); + + Assert.Throws(() => + dut.TryInitializeAndQueryGVFSConfig(tracer, null, new RetryConfig(), out _, out _, out _)); + + dut.TryGetCredentials(tracer, out string authString, out string error) + .ShouldEqual(true, "Waiters should be released after the probe throws: " + error); + authString.ShouldEqual(null, "An indeterminate probe should leave later requests anonymous until rejected"); + } + + [TestCase] + public void FailedTestAuthInitializationReleasesCredentialWaiters() + { + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl") + { + InitializationWaitTimeoutMs = 100, + }; + gitProcess.ShouldFail = true; + + dut.TryInitializeAndRequireAuth(tracer, out _) + .ShouldEqual(false, "The forced credential failure should be reported"); + + dut.TryGetCredentials(tracer, out _, out string error) + .ShouldEqual(false, "A failed credential fetch should remain a failure"); + error.Contains("Timed out waiting for authentication to initialize") + .ShouldEqual(false, "Credential failure must not be reported as an initialization timeout"); + } + + [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 TryGetCredentialsDoesNotFetchCredentialsForAnonymousServer() + { + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + dut.ConfigRequestorOverride = MockGVFSConfigRequestor.AnonymousSucceeds(); + + dut.TryInitializeAndQueryGVFSConfig(tracer, null, new RetryConfig(), out _, out _, out _) + .ShouldEqual(true, "The anonymous config probe should succeed"); + dut.TryGetCredentials(tracer, out string authString, out string error) + .ShouldEqual(true, "An anonymous server does not require credentials: " + error); + + authString.ShouldEqual(null, "No credential is needed for an anonymous server"); + gitProcess.CommandsRun.Any(command => command.Contains("credential fill")) + .ShouldEqual(false, "Do not prompt for credentials after an anonymous probe"); + } + + [TestCase] + public void UnauthorizedConfigProbeRequiresAuthentication() + { + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + MockGVFSConfigRequestor requestor = MockGVFSConfigRequestor.RequiresAuthenticationThenSucceeds(); + dut.ConfigRequestorOverride = requestor; + + dut.TryInitializeAndQueryGVFSConfig(tracer, null, new RetryConfig(), out ServerGVFSConfig config, out _, out bool isAuthFailure) + .ShouldEqual(true, "The authenticated retry should retrieve config"); + + dut.IsAnonymous.ShouldEqual(false, "A 401 from the unauthenticated probe means credentials are required"); + config.ShouldNotBeNull("The authenticated probe should return config"); + isAuthFailure.ShouldEqual(false, "A successful authenticated retry is not an auth failure"); + requestor.QueryCount.ShouldEqual(2, "The requestor should probe once anonymously and once with credentials"); + 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"); + } + + [TestCase] + public void UnauthorizedConfigProbeWithRejectedCredentialsIsAuthFailure() + { + MockTracer tracer = new MockTracer(); + GitAuthentication dut = new GitAuthentication(this.GetGitProcess(), "mock://repoUrl"); + MockGVFSConfigRequestor requestor = MockGVFSConfigRequestor.RequiresAuthentication(); + dut.ConfigRequestorOverride = requestor; + + dut.TryInitializeAndQueryGVFSConfig(tracer, null, new RetryConfig(), out _, out _, out bool isAuthFailure) + .ShouldEqual(false, "Rejected credentials must fail initialization"); + + isAuthFailure.ShouldEqual(true, "A 401 after authenticating is an authentication failure"); + dut.IsAnonymous.ShouldEqual(false, "A 401 probe must never leave auth anonymous"); + } + + [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"); + tracer.RelatedWarningEvents.Count.ShouldEqual(1, "The probe should emit one warning"); + tracer.RelatedWarningKeywords.Count.ShouldEqual(1, "Each warning should record its keyword"); + tracer.RelatedWarningKeywords[0].ShouldEqual(Keywords.Telemetry, "Indeterminate authentication telemetry must use the telemetry keyword"); + tracer.RelatedWarningEvents[0].ShouldContain("HttpStatus", status?.ToString() ?? "None"); + tracer.RelatedWarningEvents[0].ShouldContain("AuthenticationState", "Unknown"); + tracer.RelatedWarningEvents[0].ShouldContain("ProbeError", "Mock config query failure"); + } + + [TestCase] + public void IndeterminateConfigProbeAllowsLaterAnonymousSuccess() + { + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + MockGVFSEnlistment enlistment = new MockGVFSEnlistment(dut); + TestHttpMessageHandler handler = new TestHttpMessageHandler(); + dut.ConfigRequestorOverride = MockGVFSConfigRequestor.Indeterminate(HttpStatusCode.RequestTimeout); + + dut.TryInitializeAndQueryGVFSConfig(tracer, null, new RetryConfig(), out _, out _, out _); + + using (TestHttpRequestor requestor = new TestHttpRequestor(tracer, new RetryConfig(), enlistment, handler)) + using (GitEndPointResponseData response = requestor.SendRequest( + new Uri("https://mock.invalid/gvfs/objects"), + forceAnonymous: false)) + { + response.HasErrors.ShouldEqual(false, "A later anonymous success should complete the request"); + } + + handler.AuthorizationHeaders.Single().ShouldEqual(null, "Unknown authentication must try anonymously first"); + dut.IsAnonymous.ShouldEqual(true, "A successful anonymous request should resolve the state"); + gitProcess.CommandsRun.Any(command => command.Contains("credential fill")) + .ShouldEqual(false, "Anonymous success must not prompt for credentials"); + } + + [TestCase(false, HttpStatusCode.Unauthorized, "Unauthorized")] + [TestCase(false, HttpStatusCode.BadRequest, HttpRequestor.CacheServerAuthRequiredBadRequestMessage)] + [TestCase(true, HttpStatusCode.Unauthorized, "Unauthorized")] + [TestCase(true, HttpStatusCode.BadRequest, HttpRequestor.CacheServerAuthRequiredBadRequestMessage)] + public void AnonymousAuthFailurePromotesLaterRequestToAuthenticated( + bool startKnownAnonymous, + HttpStatusCode statusCode, + string responseBody) + { + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + MockGVFSEnlistment enlistment = new MockGVFSEnlistment(dut); + TestHttpMessageHandler handler = new TestHttpMessageHandler(); + dut.ConfigRequestorOverride = startKnownAnonymous + ? MockGVFSConfigRequestor.AnonymousSucceeds() + : MockGVFSConfigRequestor.Indeterminate(HttpStatusCode.RequestTimeout); + + dut.TryInitializeAndQueryGVFSConfig(tracer, null, new RetryConfig(), out _, out _, out _); + handler.EnqueueResponse(statusCode, responseBody); + handler.EnqueueResponse(HttpStatusCode.OK, "{}"); + + using (TestHttpRequestor requestor = new TestHttpRequestor(tracer, new RetryConfig(), enlistment, handler)) + { + using (GitEndPointResponseData response = requestor.SendRequest( + new Uri("https://mock.invalid/gvfs/objects"), + forceAnonymous: false)) + { + response.HasErrors.ShouldEqual(true, "The rejected anonymous attempt should report its error"); + response.ShouldRetry.ShouldEqual(true, "The rejected anonymous attempt should retry with credentials"); + response.ShouldRecordFailure.ShouldEqual(false, "An expected authentication challenge must not count against the circuit breaker"); + response.Error.Message.ShouldContain(responseBody); + } + + using (GitEndPointResponseData response = requestor.SendRequest( + new Uri("https://mock.invalid/gvfs/objects"), + forceAnonymous: false)) + { + response.HasErrors.ShouldEqual(false, "The authenticated retry should succeed"); + } + } + + handler.AuthorizationHeaders.Count.ShouldEqual(2, "Both attempts should reach the server"); + handler.AuthorizationHeaders[0].ShouldEqual(null, "The first attempt should be anonymous"); + handler.AuthorizationHeaders[1].StartsWith("Basic ", StringComparison.Ordinal) + .ShouldEqual(true, "The retry should carry the credential"); + dut.IsAnonymous.ShouldEqual(false, "The auth-required response should promote the state"); + tracer.RelatedInfoEvents.Any(info => info.Contains("PreviousAuthenticationState")) + .ShouldEqual(true, "Runtime authentication promotion should be traced"); + } + + [TestCase] + public void AnonymousAuthFailurePromotesWithoutPriorInitialization() + { + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + MockGVFSEnlistment enlistment = new MockGVFSEnlistment(dut); + TestHttpMessageHandler handler = new TestHttpMessageHandler(); + handler.EnqueueResponse(HttpStatusCode.Unauthorized, "Authentication required"); + handler.EnqueueResponse(HttpStatusCode.OK, "{}"); + + using (TestHttpRequestor requestor = new TestHttpRequestor(tracer, new RetryConfig(), enlistment, handler)) + { + using (GitEndPointResponseData response = requestor.SendRequest( + new Uri("https://mock.invalid/gvfs/objects"), + forceAnonymous: false)) + { + response.HasErrors.ShouldEqual(true, "The anonymous request should report the authentication challenge"); + response.ShouldRetry.ShouldEqual(true, "The challenge should request an authenticated retry"); + response.ShouldRecordFailure.ShouldEqual(false, "The expected challenge must not count against the circuit breaker"); + } + + using (GitEndPointResponseData response = requestor.SendRequest( + new Uri("https://mock.invalid/gvfs/objects"), + forceAnonymous: false)) + { + response.HasErrors.ShouldEqual(false, "The retry should fetch and send a credential"); + } + } + + handler.AuthorizationHeaders.Count.ShouldEqual(2, "Both attempts should reach the server"); + handler.AuthorizationHeaders[0].ShouldEqual(null, "The first request should be anonymous"); + handler.AuthorizationHeaders[1].StartsWith("Basic ", StringComparison.Ordinal) + .ShouldEqual(true, "Promotion must affect the next request without prior initialization"); + } + private MockGitProcess GetGitProcess() { MockGitProcess gitProcess = new MockGitProcess(); @@ -677,5 +1114,53 @@ private MockGitProcess GetGitProcess() }); return gitProcess; } + + private sealed class TestHttpRequestor : HttpRequestor + { + public TestHttpRequestor( + MockTracer tracer, + RetryConfig retryConfig, + MockGVFSEnlistment enlistment, + HttpMessageHandler httpMessageHandler) + : base(tracer, retryConfig, enlistment, httpMessageHandler) + { + } + + public GitEndPointResponseData SendRequest(Uri requestUri, bool forceAnonymous) + { + return this.SendRequest( + HttpRequestor.GetNewRequestId(), + requestUri, + HttpMethod.Get, + requestContent: null, + CancellationToken.None, + forceAnonymous: forceAnonymous); + } + } + + private sealed class TestHttpMessageHandler : HttpMessageHandler + { + private readonly Queue> responses = new Queue>(); + + public List AuthorizationHeaders { get; } = new List(); + + public void EnqueueResponse(HttpStatusCode statusCode, string content) + { + this.responses.Enqueue(new KeyValuePair(statusCode, content)); + } + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + this.AuthorizationHeaders.Add(request.Headers.Authorization?.ToString()); + KeyValuePair response = this.responses.Count == 0 + ? new KeyValuePair(HttpStatusCode.OK, "{}") + : this.responses.Dequeue(); + + return Task.FromResult(new HttpResponseMessage(response.Key) + { + Content = new StringContent(response.Value), + }); + } + } } } diff --git a/GVFS/GVFS.UnitTests/Http/GitObjectsHttpRequestorTests.cs b/GVFS/GVFS.UnitTests/Http/GitObjectsHttpRequestorTests.cs index ee89a68ae2..1e7236202e 100644 --- a/GVFS/GVFS.UnitTests/Http/GitObjectsHttpRequestorTests.cs +++ b/GVFS/GVFS.UnitTests/Http/GitObjectsHttpRequestorTests.cs @@ -103,6 +103,31 @@ public void PrefetchRequestFallsBackToGlobalCacheServer() requestor.RequestUris[1].AbsoluteUri.ShouldEqual(GlobalCacheServerUrl + "/gvfs/prefetch?lastPackTimestamp=0"); } + [TestCase] + public void RetryableAuthenticationChallengeDoesNotIncrementCircuitBreaker() + { + TestGitObjectsHttpRequestor requestor = this.CreateRequestor(maxRetries: 1); + requestor.EnqueueResponse( + HttpStatusCode.Unauthorized, + shouldRetry: true, + shouldRecordFailure: false); + requestor.EnqueueResponse(HttpStatusCode.OK); + + RetryWrapper.InvocationResult result = + requestor.TrySendProtocolRequest( + requestId: 1, + onSuccess: SuccessfulRequest, + onFailure: null, + method: HttpMethod.Get, + endPointGenerator: () => new Uri(EndpointCacheServerUrl + "/gvfs/objects"), + requestBodyGenerator: () => null, + cancellationToken: CancellationToken.None); + + result.Succeeded.ShouldEqual(true); + requestor.RequestUris.Count.ShouldEqual(2); + RetryCircuitBreaker.ConsecutiveFailures.ShouldEqual(0); + } + [TestCase] public void TransportExceptionFallsBackToGlobalCacheServer() { @@ -405,9 +430,13 @@ private TestGitObjectsHttpRequestor( public MockTracer TestTracer { get; } public List RequestUris { get; } - public void EnqueueResponse(HttpStatusCode statusCode, string body = "", bool shouldRetry = false) + public void EnqueueResponse( + HttpStatusCode statusCode, + string body = "", + bool shouldRetry = false, + bool shouldRecordFailure = true) { - this.responses.Enqueue(Tuple.Create(statusCode, body, shouldRetry)); + this.responses.Enqueue(Tuple.Create(statusCode, body, shouldRetry, shouldRecordFailure)); } public void EnqueueException(Exception exception) @@ -445,7 +474,7 @@ protected override GitEndPointResponseData SendProtocolRequest( onResponseDisposed: null); } - Tuple response = (Tuple)nextResponse; + Tuple response = (Tuple)nextResponse; if (response.Item1 == HttpStatusCode.OK) { @@ -462,7 +491,8 @@ protected override GitEndPointResponseData SendProtocolRequest( new GitObjectsHttpException(response.Item1, "Test failure"), shouldRetry: response.Item3, message: null, - onResponseDisposed: null); + onResponseDisposed: null, + shouldRecordFailure: response.Item4); } } diff --git a/GVFS/GVFS.UnitTests/Mock/Common/MockGVFSEnlistment.cs b/GVFS/GVFS.UnitTests/Mock/Common/MockGVFSEnlistment.cs index 2292149b65..6fb128930d 100644 --- a/GVFS/GVFS.UnitTests/Mock/Common/MockGVFSEnlistment.cs +++ b/GVFS/GVFS.UnitTests/Mock/Common/MockGVFSEnlistment.cs @@ -10,7 +10,12 @@ public class MockGVFSEnlistment : GVFSEnlistment private MockGitProcess gitProcess; public MockGVFSEnlistment() - : base(Path.Combine("mock:", "path"), "mock://repoUrl", Path.Combine("mock:", "git"), authentication: null) + : this(authentication: null) + { + } + + public MockGVFSEnlistment(GitAuthentication authentication) + : base(Path.Combine("mock:", "path"), "mock://repoUrl", Path.Combine("mock:", "git"), authentication) { this.GitObjectsRoot = Path.Combine("mock:", "path", ".git", "objects"); this.LocalObjectsRoot = this.GitObjectsRoot; diff --git a/GVFS/GVFS.UnitTests/Mock/Common/MockTracer.cs b/GVFS/GVFS.UnitTests/Mock/Common/MockTracer.cs index 1ad0c9690d..caa2faa15d 100644 --- a/GVFS/GVFS.UnitTests/Mock/Common/MockTracer.cs +++ b/GVFS/GVFS.UnitTests/Mock/Common/MockTracer.cs @@ -19,6 +19,7 @@ public MockTracer() this.waitEvent = new AutoResetEvent(false); this.RelatedInfoEvents = new List(); this.RelatedWarningEvents = new List(); + this.RelatedWarningKeywords = new List(); this.RelatedErrorEvents = new List(); this.RelatedEventNames = new List(); this.RelatedEventKeywords = new List(); @@ -33,6 +34,7 @@ public MockTracer() public List RelatedInfoEvents { get; } public List RelatedWarningEvents { get; } + public List RelatedWarningKeywords { get; } public List RelatedErrorEvents { get; } // Names of events reported via RelatedEvent (which, unlike RelatedInfo/Warning/Error, @@ -106,21 +108,29 @@ public void RelatedInfo(string format, params object[] args) } public void RelatedWarning(EventMetadata metadata, string message) + { + this.AddRelatedWarning(metadata, message, Keywords.None); + } + + public void RelatedWarning(EventMetadata metadata, string message, Keywords keyword) + { + this.AddRelatedWarning(metadata, message, keyword); + } + + private void AddRelatedWarning(EventMetadata metadata, string message, Keywords keyword) { if (metadata != null) { metadata[TracingConstants.MessageKey.WarningMessage] = message; this.RelatedWarningEvents.Add(GVFSJsonOptions.Serialize(metadata)); + this.RelatedWarningKeywords.Add(keyword); } else if (message != null) { - this.RelatedWarning(message); + this.RelatedWarningEvents.Add(message); + this.RelatedWarningKeywords.Add(keyword); } - } - public void RelatedWarning(EventMetadata metadata, string message, Keywords keyword) - { - this.RelatedWarning(metadata, message); if (metadata != null && (keyword & Keywords.Telemetry) != 0) { this.TelemetryWarnings.Add(metadata); @@ -129,11 +139,13 @@ public void RelatedWarning(EventMetadata metadata, string message, Keywords keyw public void RelatedWarning(string message) { + this.RelatedWarningKeywords.Add(Keywords.None); this.RelatedWarningEvents.Add(message); } public void RelatedWarning(string format, params object[] args) { + this.RelatedWarningKeywords.Add(Keywords.None); this.RelatedWarningEvents.Add(string.Format(format, args)); } diff --git a/GVFS/GVFS.UnitTests/Mock/Http/MockGVFSConfigRequestor.cs b/GVFS/GVFS.UnitTests/Mock/Http/MockGVFSConfigRequestor.cs new file mode 100644 index 0000000000..b104a77ad2 --- /dev/null +++ b/GVFS/GVFS.UnitTests/Mock/Http/MockGVFSConfigRequestor.cs @@ -0,0 +1,101 @@ +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 bool succeedAfterAuthentication; + private readonly HttpStatusCode? statusCode; + + private MockGVFSConfigRequestor(bool succeedAnonymously, HttpStatusCode? statusCode, bool succeedAfterAuthentication = false) + { + this.succeedAnonymously = succeedAnonymously; + this.statusCode = statusCode; + this.succeedAfterAuthentication = succeedAfterAuthentication; + } + + /// + /// 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; } + + public Action BeforeQueryResult { 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 initial unauthenticated query is rejected, then the authenticated retry succeeds. + /// + public static MockGVFSConfigRequestor RequiresAuthenticationThenSucceeds() + { + return new MockGVFSConfigRequestor( + succeedAnonymously: false, + statusCode: HttpStatusCode.Unauthorized, + succeedAfterAuthentication: true); + } + + /// + /// 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.BeforeQueryResult?.Invoke(forceAnonymous); + + bool querySucceeded = this.succeedAnonymously || + (this.succeedAfterAuthentication && this.QueryCount > 1 && !forceAnonymous); + httpStatus = querySucceeded ? HttpStatusCode.OK : this.statusCode; + + if (querySucceeded) + { + serverGVFSConfig = new ServerGVFSConfig(); + errorMessage = null; + return true; + } + + serverGVFSConfig = null; + errorMessage = "Mock config query failure"; + return false; + } + + public void Dispose() + { + } + } +} diff --git a/GVFS/GVFS.Virtualization/FileSystemCallbacks.cs b/GVFS/GVFS.Virtualization/FileSystemCallbacks.cs index 2f07ecccd2..81945fa58e 100644 --- a/GVFS/GVFS.Virtualization/FileSystemCallbacks.cs +++ b/GVFS/GVFS.Virtualization/FileSystemCallbacks.cs @@ -314,7 +314,7 @@ public EventMetadata GetAndResetHeartBeatMetadata(out bool logToFile) // header to outgoing requests" (see HttpRequestor.cs), so a mount stuck // reporting true for long stretches against a server that requires auth // is the signature of the anonymous-auth-latch bug this field detects. - metadata.Add("IsSendingAnonymousRequests", this.context.Enlistment.Authentication.IsAnonymous); + metadata.Add("IsSendingAnonymousRequests", this.context.Enlistment.Authentication.IsSendingAnonymousRequests); return metadata; }