diff --git a/.gitignore b/.gitignore
index dc0470a..e043c9f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -203,6 +203,11 @@ PublishScripts/
**/[Pp]ackages/*
# except build/, which is used as an MSBuild target.
!**/[Pp]ackages/build/
+# and except a Unity project's Packages/, which is source: Unity's package manifest and its
+# resolved lock file are both meant to be committed, and a NuGet restore folder never contains
+# a file by either name.
+!**/[Pp]ackages/manifest.json
+!**/[Pp]ackages/packages-lock.json
# Uncomment if necessary however generally it will be regenerated when needed
#!**/[Pp]ackages/repositories.config
# NuGet v3's project.json files produces more ignorable files
@@ -651,3 +656,16 @@ Temporary Items
# ImGui.ini files
imgui.ini
+
+# Game engine projects
+#
+# Godot: the import cache, and the mono/temp bin+obj a C# build writes.
+.godot/
+
+# Unity: .meta files are source, not the Visual Studio C++ build artifact that the `*.meta` rule
+# further up targets. Unity generates one per asset and it carries the GUID that scenes, prefabs
+# and serialized references point at, so ignoring them gives every clone fresh GUIDs and silently
+# breaks those references - including for a plug-in whose .dll is itself a build output. This
+# negation has to come after that rule to win, and is scoped to the asset tree so the Visual
+# Studio artifact stays ignored everywhere else.
+!**/[Aa]ssets/**/*.meta
diff --git a/BuildMonitor.Test/GitHubRequestOutcomeTests.cs b/BuildMonitor.Test/GitHubRequestOutcomeTests.cs
index 178ca2a..02c86cf 100644
--- a/BuildMonitor.Test/GitHubRequestOutcomeTests.cs
+++ b/BuildMonitor.Test/GitHubRequestOutcomeTests.cs
@@ -104,7 +104,7 @@ public async Task ARateLimited403ReportsFailure()
}
///
- /// The other headline case: a plain 403, which is what a just-revoked token looks like.
+ /// The other headline case: a plain 403, a request the token is not permitted to make.
///
[TestMethod]
public async Task APlain403ReportsFailure()
@@ -114,9 +114,76 @@ public async Task APlain403ReportsFailure()
bool succeeded = await RunFailing(provider, ApiFailure(HttpStatusCode.Forbidden)).ConfigureAwait(false);
Assert.IsFalse(succeeded, "A request refused for lack of authorization did not happen, and must not report success.");
+ Assert.AreEqual(ProviderStatus.Error, provider.Status);
+ }
+
+ ///
+ /// A 403 that is not a rate limit refuses one request -- a workflow dispatch on a repository the
+ /// token lacks actions: write for, say. The token still works for everything else, so the
+ /// refusal must not erase it along with every owner that depends on it.
+ ///
+ [TestMethod]
+ public async Task APlain403LeavesTheCredentialsInPlace()
+ {
+ GitHub provider = new();
+ SetProviderToken(provider, "provider-pat");
+ Owner owner = OwnerWithToken(provider);
+
+ bool succeeded = await provider.MakeGitHubRequestAsync(
+ RequestName, () => Task.FromException(ApiFailure(HttpStatusCode.Forbidden)), owner).ConfigureAwait(false);
+
+ Assert.IsFalse(succeeded);
+ Assert.AreEqual("provider-pat", ProviderToken(provider));
+ Assert.AreEqual("alpha-pat", owner.Token.ToString());
+ }
+
+ ///
+ /// A request sent with an owner's override token that GitHub rejects condemns that token, not
+ /// the provider's. Clearing the provider token took every other owner offline, while the owner
+ /// token that actually failed stayed in place and was rejected again every cycle.
+ ///
+ [TestMethod]
+ public async Task ARejectedOwnerTokenClearsOnlyThatOwnersToken()
+ {
+ GitHub provider = new();
+ SetProviderToken(provider, "provider-pat");
+ Owner owner = OwnerWithToken(provider);
+ AuthorizationException unauthorized = new(new FakeResponse(HttpStatusCode.Unauthorized, new Dictionary()));
+
+ bool succeeded = await provider.MakeGitHubRequestAsync(
+ RequestName, () => Task.FromException(unauthorized), owner).ConfigureAwait(false);
+
+ Assert.IsFalse(succeeded);
+ Assert.IsFalse(owner.HasToken, "The owner token GitHub rejected must not be sent again.");
+ Assert.AreEqual("provider-pat", ProviderToken(provider), "The provider token was not the one rejected.");
+ Assert.AreEqual(ProviderStatus.AuthFailed, provider.Status);
+ }
+
+ ///
+ /// An owner without its own token uses the provider's, so a rejection there still clears the
+ /// provider credentials as before.
+ ///
+ [TestMethod]
+ public async Task ARejectedProviderTokenStillClearsTheProviderCredentials()
+ {
+ GitHub provider = new();
+ SetProviderToken(provider, "provider-pat");
+ Owner owner = provider.CreateOwner(OwnerName.Create("beta"));
+ AuthorizationException unauthorized = new(new FakeResponse(HttpStatusCode.Unauthorized, new Dictionary()));
+
+ bool succeeded = await provider.MakeGitHubRequestAsync(
+ RequestName, () => Task.FromException(unauthorized), owner).ConfigureAwait(false);
+
+ Assert.IsFalse(succeeded);
+ Assert.AreEqual(string.Empty, ProviderToken(provider));
Assert.AreEqual(ProviderStatus.AuthFailed, provider.Status);
}
+ private static void SetProviderToken(GitHub provider, string token) =>
+ Assert.IsTrue(TokenStorage.Write(provider.TokenPersona, BuildProviderToken.Create(token)));
+
+ private static string ProviderToken(GitHub provider) => TokenStorage.Read(provider.TokenPersona).ToString();
+
[TestMethod]
public async Task A429ReportsFailure()
{
diff --git a/BuildMonitor/Providers/GitHub.cs b/BuildMonitor/Providers/GitHub.cs
index b0dd2ab..1f88bfa 100644
--- a/BuildMonitor/Providers/GitHub.cs
+++ b/BuildMonitor/Providers/GitHub.cs
@@ -680,6 +680,9 @@ internal async Task MakeGitHubRequestAsync(string name, Func action,
// concurrent requests use different owner tokens
SetCurrentClient(owner);
+ // Which credential this request is sent with decides which one a rejection condemns.
+ Owner? tokenOwner = owner?.HasToken == true ? owner : null;
+
// Use smart waiting: if rate limited with a known reset time, wait until reset
TimeSpan waitTime = GetRateLimitWaitTime();
if (waitTime > BaseRequestDelay)
@@ -701,7 +704,15 @@ internal async Task MakeGitHubRequestAsync(string name, Func action,
catch (AuthorizationException)
{
Log.Error($"{Name}: AuthorizationException for request '{name}'");
- OnAuthenticationFailure();
+ if (tokenOwner is not null)
+ {
+ OnOwnerAuthenticationFailure(tokenOwner);
+ }
+ else
+ {
+ OnAuthenticationFailure();
+ }
+
return false;
}
catch (ApiException e)
@@ -718,8 +729,11 @@ internal async Task MakeGitHubRequestAsync(string name, Func action,
}
else
{
+ // A token that is not allowed to make this one request -- a workflow
+ // dispatch without actions: write, say -- is still a working token.
+ // The refusal belongs to this request, not to the credentials.
Log.Error($"{Name}: 403 Forbidden for request '{name}' - {e.Message}");
- OnAuthenticationFailure();
+ SetStatus(ProviderStatus.Error, $"{Strings.Forbidden}: {name}");
}
break;
case System.Net.HttpStatusCode.TooManyRequests:
@@ -746,6 +760,23 @@ internal async Task MakeGitHubRequestAsync(string name, Func action,
}
}
+ ///
+ /// Clears an owner's override token after GitHub rejected it.
+ ///
+ /// The owner whose token the rejected request was sent with.
+ ///
+ /// The provider credentials were not used for the request, so they are left alone: clearing them
+ /// took every other owner offline while the token that actually failed stayed in place and kept
+ /// being sent, and rejected, every cycle. Clearing the owner token instead lets that owner fall
+ /// back to the provider token until a new one is set.
+ ///
+ private void OnOwnerAuthenticationFailure(Owner owner)
+ {
+ Log.Error($"{Name}: Authentication failed for owner '{owner.Name}' - owner token cleared");
+ owner.Token = new();
+ SetStatus(ProviderStatus.AuthFailed, $"{Strings.AuthFailedMessage} ({owner.Name})");
+ }
+
///
/// Updates the rate limit budget from the last API response.
/// Uses Octokit's GetLastApiInfo() to retrieve rate limit headers.
diff --git a/BuildMonitor/Strings.cs b/BuildMonitor/Strings.cs
index 5a161f1..36c64f1 100644
--- a/BuildMonitor/Strings.cs
+++ b/BuildMonitor/Strings.cs
@@ -44,6 +44,7 @@ internal static class Strings
internal static string RateLimitedMessage { get; } = "Rate limited.";
internal static string AuthFailed { get; } = nameof(AuthFailed).Titleize();
internal static string AuthFailedMessage { get; } = "Authentication failed. Please update credentials.";
+ internal static string Forbidden { get; } = "Forbidden";
internal static string ConnectionError { get; } = nameof(ConnectionError).Titleize();
internal static string ConnectionErrorMessage { get; } = "Connection error.";
internal static string Delay { get; } = nameof(Delay).Titleize();