diff --git a/BuildMonitor.Test/AzureDevOpsAuthFailureTests.cs b/BuildMonitor.Test/AzureDevOpsAuthFailureTests.cs new file mode 100644 index 0000000..fe0c6b0 --- /dev/null +++ b/BuildMonitor.Test/AzureDevOpsAuthFailureTests.cs @@ -0,0 +1,116 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.BuildMonitor.Test; + +using ktsu.CredentialCache.Storage; +using ktsu.Semantics.Strings; + +using Microsoft.VisualStudio.Services.Common; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using CredentialCache = ktsu.CredentialCache.CredentialCache; + +/// +/// Covers a token that Azure DevOps rejects. Building the clients authenticates, and the rejection +/// arrives as , which is not a VssServiceException. The +/// provider caught only the latter, so the exception escaped, faulted the whole update loop, and the +/// bad token was never reported as AuthFailed (ktsu-dev/BuildMonitor#305). +/// +/// +/// The token lives in the process-wide , so these run outside the parallel +/// phase rather than racing another class that swaps the same cache. +/// +[TestClass] +[DoNotParallelize] +public sealed class AzureDevOpsAuthFailureTests +{ + private const string Organization = "contoso"; + private const string ExpiredToken = "expired"; + + private CredentialCache Cache { get; set; } = null!; + + [TestInitialize] + public void SetUp() + { + Cache = new CredentialCache(new InMemoryCredentialStore()); + TokenStorage.UseCache(Cache); + } + + [TestCleanup] + public void TearDown() + { + TokenStorage.UseCache(null); + Cache.Dispose(); + } + + /// + /// A provider configured with credentials whose session factory fails the way dev.azure.com does + /// for an expired or revoked PAT. + /// + private static AzureDevOps CreateProviderWithRejectedToken() + { + AzureDevOps provider = new((_, _) => throw new VssUnauthorizedException("VS30063: You are not authorized to access https://dev.azure.com.")) + { + AccountId = Organization.As(), + }; + Assert.IsTrue(TokenStorage.Write(provider.TokenPersona, ExpiredToken.As())); + return provider; + } + + [TestMethod] + public void ARejectedTokenYieldsNoSessionAndReportsAuthFailed() + { + // Arrange + AzureDevOps provider = CreateProviderWithRejectedToken(); + + // Act + using CredentialedSessionCache.Lease? lease = provider.EnsureAzureDevOpsClients(out _); + + // Assert + Assert.IsNull(lease); + Assert.AreEqual(ProviderStatus.AuthFailed, provider.Status); + } + + [TestMethod] + public void ARejectedTokenClearsTheCredentials() + { + // Arrange + AzureDevOps provider = CreateProviderWithRejectedToken(); + + // Act + _ = provider.EnsureAzureDevOpsClients(out _); + + // Assert + Assert.IsTrue(provider.AccountId.IsEmpty(), "The organization should be cleared so the user is asked again"); + Assert.IsTrue(TokenStorage.Read(provider.TokenPersona).IsEmpty(), "The rejected token should be removed from the secret store"); + } + + [TestMethod] + public async Task ARepositoryUpdateWithARejectedTokenDoesNotThrow() + { + // Arrange + AzureDevOps provider = CreateProviderWithRejectedToken(); + Owner owner = new() { Name = "Project".As() }; + + // Act: an exception here is what faulted UpdateAsync and stopped GitHub polling with it. + await provider.UpdateRepositoriesAsync(owner).ConfigureAwait(false); + + // Assert + Assert.AreEqual(ProviderStatus.AuthFailed, provider.Status); + } + + [TestMethod] + public async Task AnUnauthorizedRequestReportsAuthFailed() + { + // Arrange + AzureDevOps provider = CreateProviderWithRejectedToken(); + + // Act + await provider.MakeAzureDevOpsRequestAsync("test/unauthorized", () => + throw new VssUnauthorizedException("VS30063: You are not authorized.")).ConfigureAwait(false); + + // Assert + Assert.AreEqual(ProviderStatus.AuthFailed, provider.Status); + Assert.IsTrue(provider.AccountId.IsEmpty()); + } +} diff --git a/BuildMonitor/BuildProvider.cs b/BuildMonitor/BuildProvider.cs index dc652b6..f38efac 100644 --- a/BuildMonitor/BuildProvider.cs +++ b/BuildMonitor/BuildProvider.cs @@ -46,7 +46,7 @@ internal abstract class BuildProvider { internal abstract BuildProviderName Name { get; } [JsonInclude] - internal BuildProviderAccountId AccountId { get; private set; } = new(); + internal BuildProviderAccountId AccountId { get; set; } = new(); /// /// The provider token as earlier versions persisted it: plaintext, in the app data file. diff --git a/BuildMonitor/Providers/AzureDevOps.cs b/BuildMonitor/Providers/AzureDevOps.cs index b302803..881c17f 100644 --- a/BuildMonitor/Providers/AzureDevOps.cs +++ b/BuildMonitor/Providers/AzureDevOps.cs @@ -17,9 +17,22 @@ internal sealed class AzureDevOps : BuildProvider internal static BuildProviderName BuildProviderName => nameof(AzureDevOps).As(); internal override BuildProviderName Name => BuildProviderName; - private CredentialedSessionCache Sessions { get; } = new(CreateSession); + private CredentialedSessionCache Sessions { get; } private bool ShouldDiscoverProjects { get; set; } + /// + /// Initializes a new instance of the class that connects to dev.azure.com. + /// + public AzureDevOps() : this(CreateSession) { } + + /// + /// Initializes a new instance of the class with its own session factory, + /// which is how a test stands in for a connection that dev.azure.com refuses. + /// + /// Builds a session from an organization name and a token. + internal AzureDevOps(Func createSession) => + Sessions = new(createSession); + /// /// A connection to an Azure DevOps organization together with the clients built from it. /// @@ -92,6 +105,15 @@ private static AzureDevOpsSession CreateSession(string accountId, string token) { return Sessions.Get(accountId, token, out session); } + catch (VssUnauthorizedException ex) + { + // Building the clients authenticates, so a rejected token surfaces here rather than as a + // 401 from a request. VssUnauthorizedException is not a VssServiceException, and letting it + // escape faulted the whole update loop (ktsu-dev/BuildMonitor#305). + Log.Error($"{Name}: Unauthorized while connecting to '{accountId}' - {ex.Message}"); + OnAuthenticationFailure(); + return null; + } catch (VssServiceException ex) { SetStatus(ProviderStatus.Error, $"{Strings.ConnectionErrorMessage} {ex.Message}"); @@ -429,6 +451,11 @@ internal async Task MakeAzureDevOpsRequestAsync(string name, Func action) Log.Error($"{Name}: 401 Unauthorized for request '{name}' - {ex.Message}"); OnAuthenticationFailure(); } + catch (VssUnauthorizedException ex) + { + Log.Error($"{Name}: Unauthorized for request '{name}' - {ex.Message}"); + OnAuthenticationFailure(); + } catch (VssServiceResponseException ex) when (ex.HttpStatusCode == System.Net.HttpStatusCode.TooManyRequests) { Log.Warning($"{Name}: 429 Too Many Requests for '{name}' - {ex.Message}");