diff --git a/.claude/commands b/.claude/commands index 1a366ba9..5b2d918a 160000 --- a/.claude/commands +++ b/.claude/commands @@ -1 +1 @@ -Subproject commit 1a366ba9c83ff100c9d0721c5cbcdcade62fdd46 +Subproject commit 5b2d918abe53ac2bb2ea7e39bbd0fa17adbd7d82 diff --git a/internal b/internal index 46b8e931..363530d4 160000 --- a/internal +++ b/internal @@ -1 +1 @@ -Subproject commit 46b8e9318e62763322224edc0bfbf0df42744f88 +Subproject commit 363530d4948857f38d0656fd31ebd3eddad5d962 diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/AutopilotDeviceValidatorTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/AutopilotDeviceValidatorTests.cs index 5f1eb2e0..9ee463a7 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/AutopilotDeviceValidatorTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/AutopilotDeviceValidatorTests.cs @@ -47,6 +47,12 @@ public class AutopilotDeviceValidatorTests new(status) { Content = new StringContent(body) }; private static (AutopilotDeviceValidator Sut, DelayingHandler Handler, MemoryCache Cache) BuildSut(params HttpResponseMessage?[] responses) + { + var built = BuildSutWithTokens(responses); + return (built.Sut, built.Handler, built.Cache); + } + + private static (AutopilotDeviceValidator Sut, DelayingHandler Handler, MemoryCache Cache, Mock Tokens) BuildSutWithTokens(params HttpResponseMessage?[] responses) { var handler = new DelayingHandler(responses); var factory = new Mock(); @@ -71,10 +77,52 @@ public class AutopilotDeviceValidatorTests var sut = new AutopilotDeviceValidator( NullLogger.Instance, factory.Object, cache, tokenService.Object); - return (sut, handler, cache); + return (sut, handler, cache, tokenService); } private const string FoundBody = "{\"value\":[{\"id\":\"ap-1\",\"serialNumber\":\"PF3XKQ7\"}]}"; + private const string ForbiddenBody = "{\"error\":{\"code\":\"Authorization_RequestDenied\",\"message\":\"Application is not authorized to perform this operation.\"}}"; + + [Fact] + public async Task StaleTokenForbidden_ThenSuccess_InvalidatesOnce_AndValidatesWithTheFreshToken() + { + // Field case 2026-09-02: a second instance held a roleless app-only token minted before the + // tenant's consent; Graph answered 403 for 45 minutes. Attempt 1 must drop that token and + // attempt 2 must succeed with the re-minted one — no 503 loop, no cached negative. + var (sut, handler, cache, tokens) = BuildSutWithTokens( + Json(ForbiddenBody, HttpStatusCode.Forbidden), + Json(FoundBody)); + + var result = await sut.ValidateAutopilotDeviceAsync(TenantId, Serial, null, CancellationToken.None); + + Assert.True(result.IsValid); + Assert.Equal("ap-1", result.AutopilotDeviceId); + Assert.Equal(2, handler.Requests.Count); + tokens.Verify(t => t.InvalidateTenant(TenantId), Times.Once); + tokens.Verify(t => t.GetAccessTokenAsync(TenantId, It.IsAny()), Times.Exactly(2)); + Assert.True(cache.TryGetValue($"autopilot-device-validation:{TenantId}:{Serial}", out _)); + } + + [Fact] + public async Task ForbiddenWithFreshToken_StaysTransient_WithLongerRetryAfter_NotCached() + { + // Consent genuinely missing (or not yet propagated): the agent must keep getting 503 (a 403 + // would make it shut down as "device not registered"), but with the permission-gap + // Retry-After and nothing cached so a granted consent takes effect on the next call. + var (sut, handler, cache, tokens) = BuildSutWithTokens( + Json(ForbiddenBody, HttpStatusCode.Forbidden), + Json(ForbiddenBody, HttpStatusCode.Forbidden)); + + var result = await sut.ValidateAutopilotDeviceAsync(TenantId, Serial, null, CancellationToken.None); + + Assert.False(result.IsValid); + Assert.True(result.IsTransient); + Assert.Equal(GraphAuthFailure.PermissionMissingRetryAfterSeconds, result.RetryAfterSeconds); + Assert.Contains("permission missing", result.ErrorMessage); + Assert.Equal(2, handler.Requests.Count); + tokens.Verify(t => t.InvalidateTenant(TenantId), Times.Once); + Assert.False(cache.TryGetValue($"autopilot-device-validation:{TenantId}:{Serial}", out _)); + } [Fact] public async Task Budget_defaults_sit_inside_the_agents_30s_client_timeout() diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/CorporateIdentifierValidatorTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/CorporateIdentifierValidatorTests.cs index 8b914766..8e56ebe7 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/CorporateIdentifierValidatorTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/CorporateIdentifierValidatorTests.cs @@ -224,17 +224,36 @@ public class CorporateIdentifierValidatorTests } [Fact] - public async Task ValidateAsync_graphPermissionDenied_isCached_notRetried() + public async Task ValidateAsync_graphPermissionDenied_isCached_afterOneFreshTokenRetry() { // Definitive permission failures are negative-cached so an agent retry storm does not - // hammer Graph while consent is being fixed; the in-call retry loop must not fire either. + // hammer Graph while consent is being fixed. Attempt 1 may have used a token minted before + // the consent (GraphAuthFailure), so exactly ONE retry with a fresh token precedes the + // verdict; the cached verdict then answers the next call without Graph. var (sut, handler) = BuildSut(Json("""{"error":{"code":"Forbidden"}}""", HttpStatusCode.Forbidden)); - await sut.ValidateAsync(TenantId, "Microsoft Corporation", "Virtual Machine", Serial); + var first = await sut.ValidateAsync(TenantId, "Microsoft Corporation", "Virtual Machine", Serial); var second = await sut.ValidateAsync(TenantId, "Microsoft Corporation", "Virtual Machine", Serial); + Assert.False(first.IsTransient); Assert.False(second.IsValid); - Assert.Single(handler.Requests); + Assert.Equal(2, handler.Requests.Count); + } + + [Fact] + public async Task ValidateAsync_staleTokenForbidden_thenFound_validatesWithTheFreshToken() + { + // Field case 2026-09-02 in this validator's shape: a stale roleless token gets 403, the + // fresh one finds the identifier. Without the retry the stale token would have been cached + // as "not permitted" for 5 minutes, every 5 minutes, until its 55-minute TTL. + var (sut, handler) = BuildSut( + Json("""{"error":{"code":"Forbidden"}}""", HttpStatusCode.Forbidden), + Json($$"""{"value":[{"importedDeviceIdentityType":"manufacturerModelSerial","importedDeviceIdentifier":"{{StoredIdentifier}}"}]}""")); + + var result = await sut.ValidateAsync(TenantId, "Microsoft Corporation", "Virtual Machine", Serial); + + Assert.True(result.IsValid); + Assert.Equal(2, handler.Requests.Count); } [Fact] diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/GraphAuthFailureTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/GraphAuthFailureTests.cs new file mode 100644 index 00000000..05770502 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/GraphAuthFailureTests.cs @@ -0,0 +1,100 @@ +using System.Net; +using AutopilotMonitor.Functions.Security; +using AutopilotMonitor.Functions.Services; +using AutopilotMonitor.Shared.DataAccess; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Pins the shared 401/403 contract of the Graph-backed device validators: attempt 1 drops the +/// tenant's cached token (it may predate the consent grant on this instance) and reports +/// transient so the loop re-mints; attempt 2 and every other status leave the token alone. +/// +public class GraphAuthFailureTests +{ + private const string TenantId = "11111111-1111-1111-1111-111111111111"; + + private static Mock TokenService() + { + var cache = new MemoryCache(new MemoryCacheOptions()); + var configuration = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary + { + ["EntraId:ClientId"] = "aaaaaaaa-0000-0000-0000-000000000001", + ["EntraId:ClientSecret"] = "secret", + }).Build(); + var registry = new EntraAppRegistry(configuration, NullLogger.Instance); + var tenantConfig = new Mock( + Mock.Of(), NullLogger.Instance, cache) + { CallBase = false }; + return new Mock( + NullLogger.Instance, Mock.Of(), cache, + configuration, registry, tenantConfig.Object) + { CallBase = false }; + } + + [Theory] + [InlineData(HttpStatusCode.Unauthorized)] + [InlineData(HttpStatusCode.Forbidden)] + public void AuthFailure_OnFirstAttempt_InvalidatesOnce_AndAsksForRetry(HttpStatusCode status) + { + var tokens = TokenService(); + + var retry = GraphAuthFailure.TryRecoverStaleToken(tokens.Object, NullLogger.Instance, "Test", TenantId, status, attempt: 1); + + Assert.True(retry); + tokens.Verify(t => t.InvalidateTenant(TenantId), Times.Once); + } + + [Theory] + [InlineData(HttpStatusCode.Unauthorized)] + [InlineData(HttpStatusCode.Forbidden)] + public void AuthFailure_OnSecondAttempt_LeavesTheFreshToken_AndDefersToTheValidator(HttpStatusCode status) + { + var tokens = TokenService(); + + var retry = GraphAuthFailure.TryRecoverStaleToken(tokens.Object, NullLogger.Instance, "Test", TenantId, status, attempt: 2); + + Assert.False(retry); + tokens.Verify(t => t.InvalidateTenant(It.IsAny()), Times.Never); + } + + [Theory] + [InlineData(HttpStatusCode.NotFound)] + [InlineData(HttpStatusCode.BadRequest)] + [InlineData(HttpStatusCode.TooManyRequests)] + [InlineData(HttpStatusCode.InternalServerError)] + [InlineData(HttpStatusCode.GatewayTimeout)] + public void OtherStatuses_NeverTouchTheToken(HttpStatusCode status) + { + var tokens = TokenService(); + + var retry = GraphAuthFailure.TryRecoverStaleToken(tokens.Object, NullLogger.Instance, "Test", TenantId, status, attempt: 1); + + Assert.False(retry); + Assert.False(GraphAuthFailure.IsAuthFailure(status)); + tokens.Verify(t => t.InvalidateTenant(It.IsAny()), Times.Never); + } + + [Fact] + public void PermissionMissingRetryAfter_IsLongerThanTheDefault_ButInsideTheAgentsPatience() + { + // 30 s is the SecurityValidator default for a transient failure; a consent gap needs more + // than that to close, and the agent's replay logic honours any Retry-After. + Assert.True(GraphAuthFailure.PermissionMissingRetryAfterSeconds > 30); + Assert.True(GraphAuthFailure.PermissionMissingRetryAfterSeconds <= 300); + } + + [Theory] + [InlineData(null, null, null)] + [InlineData(null, 120, 120)] + [InlineData(30, null, 30)] + [InlineData(30, 120, 120)] + [InlineData(120, 30, 120)] + public void SecurityValidator_KeepsTheLargestRetryAfter(int? accumulated, int? next, int? expected) + => Assert.Equal(expected, SecurityValidator.MaxRetryAfter(accumulated, next)); +} diff --git a/src/Backend/AutopilotMonitor.Functions/Security/AutopilotDeviceValidator.cs b/src/Backend/AutopilotMonitor.Functions/Security/AutopilotDeviceValidator.cs index 3707e048..c4b2d56b 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/AutopilotDeviceValidator.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/AutopilotDeviceValidator.cs @@ -144,6 +144,36 @@ namespace AutopilotMonitor.Functions.Security "Autopilot device validation Graph query failed for tenant {TenantId} (attempt {Attempt}). Status: {StatusCode}. Body: {ResponseBody}", tenantId, attempt, (int)response.StatusCode, responseBody); + // 401/403 on attempt 1: the cached token may predate the tenant's consent + // (see GraphAuthFailure) — drop it and let the loop retry with a fresh one. + if (GraphAuthFailure.TryRecoverStaleToken(_graphTokenService, _logger, nameof(AutopilotDeviceValidator), tenantId, response.StatusCode, attempt)) + { + return new AutopilotDeviceValidationResult + { + IsValid = false, + IsTransient = true, + SerialNumber = normalizedSerial, + ErrorMessage = $"Graph auth failure {(int)response.StatusCode}; token refreshed" + }; + } + + // 401/403 with a fresh token = consent missing or not yet propagated. Still + // transient for the agent (a definitive 403 would make it shut down as + // "device not registered"), but with a longer Retry-After and never cached so a + // granted consent takes effect on the next call. + if (GraphAuthFailure.IsAuthFailure(response.StatusCode)) + { + GraphAuthFailure.LogPermissionMissing(_logger, nameof(AutopilotDeviceValidator), tenantId, response.StatusCode); + return new AutopilotDeviceValidationResult + { + IsValid = false, + IsTransient = true, + RetryAfterSeconds = GraphAuthFailure.PermissionMissingRetryAfterSeconds, + SerialNumber = normalizedSerial, + ErrorMessage = $"Graph permission missing (status {(int)response.StatusCode})" + }; + } + // Graph errors are transient — do NOT cache return new AutopilotDeviceValidationResult { @@ -266,6 +296,12 @@ namespace AutopilotMonitor.Functions.Security /// public bool IsTransient { get; set; } + /// + /// Retry-After the agent should see for this transient failure; null = the caller's default. + /// Set when a fresh token still lacks the Graph permission (). + /// + public int? RetryAfterSeconds { get; set; } + public string? SerialNumber { get; set; } public string? AutopilotDeviceId { get; set; } public string? ErrorMessage { get; set; } diff --git a/src/Backend/AutopilotMonitor.Functions/Security/CloudPcDeviceValidator.cs b/src/Backend/AutopilotMonitor.Functions/Security/CloudPcDeviceValidator.cs index f5dc81bb..e6b57f3d 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/CloudPcDeviceValidator.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/CloudPcDeviceValidator.cs @@ -150,12 +150,27 @@ namespace AutopilotMonitor.Functions.Security "Cloud PC validation Graph query failed for tenant {TenantId} (attempt {Attempt}). Status: {StatusCode}. Body: {ResponseBody}", tenantId, attempt, (int)response.StatusCode, responseBody); + // 401/403 on attempt 1: the cached token may predate the tenant's consent (see + // GraphAuthFailure) — drop it and let the loop retry with a fresh one before the + // definitive "not granted" verdict below is cached against a stale token. + if (GraphAuthFailure.TryRecoverStaleToken(_graphTokenService, _logger, nameof(CloudPcDeviceValidator), tenantId, response.StatusCode, attempt)) + { + return new CloudPcValidationResult + { + IsValid = false, + IsTransient = true, + IntuneDeviceId = normalizedId, + ErrorMessage = $"Graph auth failure {(int)response.StatusCode}; token refreshed" + }; + } + // 403 = CloudPC.Read.All not granted in this tenant. That is a configuration // state, not an outage: retrying cannot fix it, and a 503 Retry-After would // keep every W365 agent in a futile retry loop. Cache it as a definitive // negative (5 min) so a fresh grant is picked up quickly. if (response.StatusCode == System.Net.HttpStatusCode.Forbidden) { + GraphAuthFailure.LogPermissionMissing(_logger, nameof(CloudPcDeviceValidator), tenantId, response.StatusCode); return CacheAndReturn(cacheKey, new CloudPcValidationResult { IsValid = false, diff --git a/src/Backend/AutopilotMonitor.Functions/Security/CorporateIdentifierValidator.cs b/src/Backend/AutopilotMonitor.Functions/Security/CorporateIdentifierValidator.cs index fcab627c..de8e63f5 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/CorporateIdentifierValidator.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/CorporateIdentifierValidator.cs @@ -189,7 +189,22 @@ namespace AutopilotMonitor.Functions.Security ErrorMessage = $"Device '{rawIdentifier}' (normalized '{normalizedIdentifier}') is not registered as a Corporate Identifier (manufacturerModelSerial)" }, isPositive: false); + case ScanOutcome.PermissionDenied when GraphAuthFailure.TryRecoverStaleToken( + _graphTokenService, _logger, nameof(CorporateIdentifierValidator), tenantId, scan.Status ?? System.Net.HttpStatusCode.Forbidden, attempt): + // Attempt 1: the cached token may predate the tenant's consent (see + // GraphAuthFailure). Transient so the loop retries with a fresh token; the + // definitive branch below is reserved for a fresh token that still lacks the + // permission — without this step a stale token would be cached as + // "not permitted" every 5 minutes for up to 55 minutes. + return new CorporateIdentifierValidationResult + { + IsValid = false, + IsTransient = true, + ErrorMessage = "Graph auth failure; token refreshed" + }; + case ScanOutcome.PermissionDenied: + GraphAuthFailure.LogPermissionMissing(_logger, nameof(CorporateIdentifierValidator), tenantId, scan.Status ?? System.Net.HttpStatusCode.Forbidden); // 401/403 = missing application permission / admin consent. Retries can // never heal that, so it is a DEFINITIVE failure — a transient // classification would trap every agent in an endless 503 Retry-After @@ -250,7 +265,7 @@ namespace AutopilotMonitor.Functions.Security private enum ScanOutcome { Found, NotFound, PermissionDenied, FilterRejected, Transient } - private readonly record struct ScanResult(ScanOutcome Outcome, string? Error = null); + private readonly record struct ScanResult(ScanOutcome Outcome, string? Error = null, System.Net.HttpStatusCode? Status = null); /// /// Pages through an importedDeviceIdentities GET url (following @odata.nextLink) and @@ -277,7 +292,7 @@ namespace AutopilotMonitor.Functions.Security return response.StatusCode switch { System.Net.HttpStatusCode.Unauthorized or System.Net.HttpStatusCode.Forbidden - => new ScanResult(ScanOutcome.PermissionDenied), + => new ScanResult(ScanOutcome.PermissionDenied, Status: response.StatusCode), // 400 on the first page = the $filter was rejected; mid-pagination a 400 // is unexpected and treated as transient like any other Graph error. System.Net.HttpStatusCode.BadRequest when page == 0 diff --git a/src/Backend/AutopilotMonitor.Functions/Security/DeviceAssociationResult.cs b/src/Backend/AutopilotMonitor.Functions/Security/DeviceAssociationResult.cs index 61998d81..e6205355 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/DeviceAssociationResult.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/DeviceAssociationResult.cs @@ -20,6 +20,12 @@ namespace AutopilotMonitor.Functions.Security /// public bool IsTransient { get; set; } + /// + /// Retry-After the agent should see for this transient failure; null = the caller's default. + /// Set when a fresh token still lacks the Graph permission (). + /// + public int? RetryAfterSeconds { get; set; } + public string? SerialNumber { get; set; } // ---- Fields populated when a matching tenantAssociatedDevices entry is found ---- @@ -69,6 +75,7 @@ namespace AutopilotMonitor.Functions.Security { IsValid = IsValid, IsTransient = IsTransient, + RetryAfterSeconds = RetryAfterSeconds, SerialNumber = SerialNumber, AssociationState = AssociationState, DevicePreparationPolicyId = DevicePreparationPolicyId, diff --git a/src/Backend/AutopilotMonitor.Functions/Security/DeviceAssociationValidator.cs b/src/Backend/AutopilotMonitor.Functions/Security/DeviceAssociationValidator.cs index 9b63e053..21b5c8d7 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/DeviceAssociationValidator.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/DeviceAssociationValidator.cs @@ -144,6 +144,33 @@ namespace AutopilotMonitor.Functions.Security "Device association Graph query failed for tenant {TenantId} (attempt {Attempt}). Status: {StatusCode}. Body: {ResponseBody}", tenantId, attempt, (int)response.StatusCode, responseBody); + // Same contract as AutopilotDeviceValidator: attempt 1 drops a possibly stale + // token and retries; a fresh token that still gets 401/403 stays transient for + // the agent with a longer Retry-After (see GraphAuthFailure). + if (GraphAuthFailure.TryRecoverStaleToken(_graphTokenService, _logger, nameof(DeviceAssociationValidator), tenantId, response.StatusCode, attempt)) + { + return new DeviceAssociationResult + { + IsValid = false, + IsTransient = true, + SerialNumber = normalizedSerial, + ErrorMessage = $"Graph auth failure {(int)response.StatusCode}; token refreshed" + }; + } + + if (GraphAuthFailure.IsAuthFailure(response.StatusCode)) + { + GraphAuthFailure.LogPermissionMissing(_logger, nameof(DeviceAssociationValidator), tenantId, response.StatusCode); + return new DeviceAssociationResult + { + IsValid = false, + IsTransient = true, + RetryAfterSeconds = GraphAuthFailure.PermissionMissingRetryAfterSeconds, + SerialNumber = normalizedSerial, + ErrorMessage = $"Graph permission missing (status {(int)response.StatusCode})" + }; + } + return new DeviceAssociationResult { IsValid = false, diff --git a/src/Backend/AutopilotMonitor.Functions/Security/GraphAuthFailure.cs b/src/Backend/AutopilotMonitor.Functions/Security/GraphAuthFailure.cs new file mode 100644 index 00000000..27101d9d --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions/Security/GraphAuthFailure.cs @@ -0,0 +1,66 @@ +using System.Net; +using Microsoft.Extensions.Logging; + +namespace AutopilotMonitor.Functions.Security +{ + /// + /// Shared handling of a Graph 401/403 across the device validators. + /// + /// The app-only Graph token is cached per Function instance for up to 55 minutes + /// (). When a tenant grants admin consent, only the instance that + /// served the consent flow invalidates its cache; every other instance keeps a token that was + /// minted BEFORE the grant and therefore carries no roles. Graph answers 403 for that + /// token forever, and a validator that classifies the 403 as transient parks the agent in a + /// 503 Retry-After loop until the token expires (field case 2026-09-02: 45 minutes, 312 rejected + /// ingest calls of one freshly onboarded tenant). A validator that classifies it as definitive is + /// worse: the agent treats a 403 as "device not registered" and shuts down after a few in a row. + /// + /// + /// The fix is per request and needs no cross-instance channel: on the FIRST attempt an auth + /// failure drops the tenant's cached token, the validator reports the failure as transient, and + /// its existing retry loop mints a fresh token for the second attempt. A 401/403 on the second + /// attempt is then a real permission gap (consent missing or not yet propagated); each validator + /// keeps its own semantics for that case, logged through so + /// the operator sees the tenant. + /// + /// + internal static class GraphAuthFailure + { + /// + /// Retry-After handed to the agent when a fresh token still gets 401/403: a consent gap does + /// not close in 30 seconds, and a quarter of the retry volume is enough to notice when it does. + /// + public const int PermissionMissingRetryAfterSeconds = 120; + + public static bool IsAuthFailure(HttpStatusCode status) + => status == HttpStatusCode.Unauthorized || status == HttpStatusCode.Forbidden; + + /// + /// On the first attempt an auth failure is treated as a possibly stale token: the tenant's + /// cached token is invalidated so the retry mints a fresh one. Returns true when the caller + /// must classify the failure as transient (so its loop retries) instead of applying its + /// definitive 401/403 handling. Never invalidates on a later attempt or on any other status. + /// + public static bool TryRecoverStaleToken( + GraphTokenService tokenService, ILogger logger, string validator, string tenantId, HttpStatusCode status, int attempt) + { + if (attempt != 1 || !IsAuthFailure(status)) + return false; + + tokenService.InvalidateTenant(tenantId); + logger.LogWarning( + "{Validator}: Graph auth failure {StatusCode} for tenant {TenantId} on attempt 1 — token invalidated, retrying with a fresh token", + validator, (int)status, tenantId); + return true; + } + + /// + /// Stable marker for the operator: the fresh token still lacks the permission. The caller + /// decides what the agent sees (transient 503 or the validator's definitive negative). + /// + public static void LogPermissionMissing(ILogger logger, string validator, string tenantId, HttpStatusCode status) + => logger.LogWarning( + "{Validator}: Graph permission missing for tenant {TenantId} after fresh token (status {StatusCode})", + validator, tenantId, (int)status); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Security/IntuneDeviceBindingValidator.cs b/src/Backend/AutopilotMonitor.Functions/Security/IntuneDeviceBindingValidator.cs index 2793f4d9..c1d934eb 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/IntuneDeviceBindingValidator.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/IntuneDeviceBindingValidator.cs @@ -146,8 +146,22 @@ namespace AutopilotMonitor.Functions.Security }, isPositive: false); } + // 401/403 on attempt 1: the cached token may predate the tenant's consent (see + // GraphAuthFailure) — drop it and let the loop retry with a fresh one before the + // definitive "not granted" verdict below is cached against a stale token. + if (GraphAuthFailure.TryRecoverStaleToken(_graphTokenService, _logger, nameof(IntuneDeviceBindingValidator), tenantId, response.StatusCode, attempt)) + { + return new IntuneDeviceBindingResult + { + Outcome = IntuneDeviceBindingOutcome.Transient, + IntuneDeviceId = normalizedId, + ErrorMessage = $"Graph auth failure {(int)response.StatusCode}; token refreshed" + }; + } + if (response.StatusCode == System.Net.HttpStatusCode.Forbidden) { + GraphAuthFailure.LogPermissionMissing(_logger, nameof(IntuneDeviceBindingValidator), tenantId, response.StatusCode); // Configuration state, not an outage: retrying cannot fix a missing grant. // Cached briefly so a fresh grant is picked up quickly. return CacheAndReturn(cacheKey, new IntuneDeviceBindingResult diff --git a/src/Backend/AutopilotMonitor.Functions/Security/SecurityValidator.cs b/src/Backend/AutopilotMonitor.Functions/Security/SecurityValidator.cs index f4958636..3b69a61b 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/SecurityValidator.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/SecurityValidator.cs @@ -408,6 +408,9 @@ namespace AutopilotMonitor.Functions.Security string? autopilotDeviceId = null; bool deviceValidated = false; bool deviceValidationTransient = false; + // Largest Retry-After any transient validator asked for (GraphAuthFailure: a fresh + // token that still lacks the permission wants 120 s, not the 30-s default). + int? deviceValidationRetryAfter = null; string? deviceValidationError = null; ValidatorType validatedBy = ValidatorType.Unknown; @@ -430,6 +433,7 @@ namespace AutopilotMonitor.Functions.Security { deviceValidationError = autopilotResult.ErrorMessage; deviceValidationTransient = autopilotResult.IsTransient; + deviceValidationRetryAfter = MaxRetryAfter(deviceValidationRetryAfter, autopilotResult.RetryAfterSeconds); } } @@ -470,6 +474,7 @@ namespace AutopilotMonitor.Functions.Security { deviceValidationError = CombineValidationErrors(deviceValidationError, associationResult.ErrorMessage); deviceValidationTransient |= associationResult.IsTransient; + deviceValidationRetryAfter = MaxRetryAfter(deviceValidationRetryAfter, associationResult.RetryAfterSeconds); } } @@ -511,7 +516,7 @@ namespace AutopilotMonitor.Functions.Security StatusCode = HttpStatusCode.ServiceUnavailable, ErrorMessage = "Device validation temporarily unavailable", Details = deviceValidationError, - RetryAfterSeconds = 30 + RetryAfterSeconds = deviceValidationRetryAfter ?? 30 }; } @@ -618,6 +623,10 @@ namespace AutopilotMonitor.Functions.Security return accumulated + " | " + next; } + /// Largest Retry-After requested so far (null when no validator asked for one). + internal static int? MaxRetryAfter(int? accumulated, int? next) + => next is int n && (accumulated is not int a || n > a) ? n : accumulated; + /// /// Extracts the Intune device id from an MDM client certificate subject. Certs issued by /// the Microsoft Intune MDM Device CA carry the Intune managedDevice id as the CN