Skip to content

Keep authentication adaptive after indeterminate /gvfs/config probes #2104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Tyrie Vella (tyrielv) merged 3 commits into from
Nov 3, 2026
Merged

Keep authentication adaptive after indeterminate /gvfs/config probes #2104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7648b2e
Keep authentication adaptive after indeterminate config probes
tyrielv Oct 6, 2026
a33ca20
Fix authentication promotion and retry accounting
tyrielv Nov 3, 2026
8511f62
Preserve retry accounting through blob hydration
tyrielv Nov 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 24 additions & 5 deletions GVFS/GVFS.Common/Git/GVFSGitObjects.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,23 @@ public enum BlobHydrationFailureCategory
/// </summary>
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; }

// 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; }
Expand Down Expand Up @@ -217,7 +223,11 @@ public virtual bool TryCopyBlobContentStream(
downloadSucceededButCopyFailed = true;
}

return new RetryWrapper<bool>.CallbackResult(error: null, shouldRetry: true);
return new RetryWrapper<bool>.CallbackResult(
error: null,
shouldRetry: true,
result: false,
shouldRecordFailure: lastDownloadResult?.ShouldRecordFailure ?? true);
}
});

Expand Down Expand Up @@ -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);
}
}
}
188 changes: 176 additions & 12 deletions GVFS/GVFS.Common/Git/GitAuthentication.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
{
Expand All @@ -56,7 +65,38 @@ public bool IsBackingOff
}
}

public bool IsAnonymous { get; private set; } = true;
/// <summary>
/// True only when the server is known to allow anonymous access.
/// </summary>
/// <remarks>
/// This is false while the state is unknown. While true,
/// <see cref="Http.HttpRequestor.SendRequest"/> omits the Authorization
/// header and never calls <see cref="TryGetCredentials"/>.
/// </remarks>
public bool IsAnonymous
{
get => this.GetAuthenticationState() == AuthenticationState.Anonymous;
}

public bool IsSendingAnonymousRequests => this.ShouldSendAnonymous;

internal bool ShouldSendAnonymous =>
!this.IsInitializationStarted ||
Comment thread
tyrielv marked this conversation as resolved.
Resolved
(this.initializationComplete.IsSet && this.GetAuthenticationState() != AuthenticationState.Authenticated);

/// <summary>
/// Indicates whether authentication initialization has started.
/// </summary>
internal bool IsInitializationStarted => Volatile.Read(ref this.initializationStarted) != 0;

/// <summary>
/// Marks authentication initialization as scheduled so that requests wait
/// for its result instead of being sent anonymously.
/// </summary>
internal void MarkInitializationStarted()
{
Interlocked.Exchange(ref this.initializationStarted, 1);
}
Comment thread
tyrielv marked this conversation as resolved.
Resolved

/// <summary>
/// How long a caller of <see cref="TryGetCredentials"/> will wait for
Expand All @@ -66,6 +106,14 @@ public bool IsBackingOff
/// </summary>
internal int InitializationWaitTimeoutMs { get; set; } = BackgroundCredentialTimeoutMs;

/// <summary>
/// Test seam for the /gvfs/config probe. When null, production code uses
/// <see cref="ConfigHttpRequestor"/>. Unit tests substitute a fake so the
/// probe outcome - anonymous success, 401, or an indeterminate network
/// failure - can be driven deterministically.
/// </summary>
internal IGVFSConfigRequestor ConfigRequestorOverride { get; set; }

private GitSsl GitSsl { get; }

public void ApproveCredentials(ITracer tracer, string credentialString)
Expand Down Expand Up @@ -169,6 +217,19 @@ public void RejectCredentials(ITracer tracer, string credentialString)
}
}

/// <summary>
/// Gets the credential to send with a request.
/// </summary>
/// <param name="credentialString">
/// 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.
/// </param>
/// <param name="errorMessage">The failure reason when this returns false.</param>
/// <returns>
/// True when the request can proceed: with the credential in
/// <paramref name="credentialString"/>, or anonymously when it is null.
/// </returns>
public bool TryGetCredentials(ITracer tracer, out string credentialString, out string errorMessage)
{
if (!this.isInitialized)
Expand All @@ -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;
}
Comment thread
tyrielv marked this conversation as resolved.
Resolved

credentialString = this.cachedCredentialString;
if (credentialString == null)
{
Expand Down Expand Up @@ -256,34 +324,83 @@ 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;
}

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<string, object>
{
["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))
{
Expand Down Expand Up @@ -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))
Comment thread
tyrielv marked this conversation as resolved.
Resolved
{
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)
Expand Down
19 changes: 15 additions & 4 deletions GVFS/GVFS.Common/Http/ConfigHttpRequestor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

namespace GVFS.Common.Http
{
public class ConfigHttpRequestor : HttpRequestor
public class ConfigHttpRequestor : HttpRequestor, IGVFSConfigRequestor
{
private readonly string repoUrl;

Expand All @@ -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;
Expand Down Expand Up @@ -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<ServerGVFSConfig>.CallbackResult(response.Error, response.ShouldRetry);
return new RetryWrapper<ServerGVFSConfig>.CallbackResult(
response.Error,
response.ShouldRetry,
result: null,
shouldRecordFailure: response.ShouldRecordFailure);
}

try
Expand Down Expand Up @@ -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)
{
Expand Down
Loading
Loading