diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/TableBootstrapRepositoryRevokeOwnershipTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/TableBootstrapRepositoryRevokeOwnershipTests.cs new file mode 100644 index 00000000..33676d28 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/TableBootstrapRepositoryRevokeOwnershipTests.cs @@ -0,0 +1,105 @@ +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using AutopilotMonitor.Functions.DataAccess.TableStorage; +using AutopilotMonitor.Functions.Services; +using Azure; +using Azure.Data.Tables; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Object-level authorization for bootstrap revoke: the short code is a platform-global +/// identifier, so the repository must refuse to revoke a code owned by a tenant other than +/// the caller's authorized tenant — and must do so without leaking who owns it (same +/// "not found" result as an unknown code). +/// +public sealed class TableBootstrapRepositoryRevokeOwnershipTests +{ + private const string TenantA = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"; + private const string TenantB = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"; + private const string Code = "k7m3pq"; + + private sealed class Harness + { + public TableBootstrapRepository Sut { get; } + public Mock Table { get; } = new(); + public List Updates { get; } = new(); + + public Harness(string owningTenantId) + { + var lookup = new TableEntity("CodeLookup", Code) + { + { "TenantId", owningTenantId }, + { "Token", "token-1" }, + { "IsRevoked", false } + }; + var main = new TableEntity(owningTenantId, Code) + { + { "Token", "token-1" }, + { "IsRevoked", false } + }; + + Table.Setup(c => c.GetEntityAsync("CodeLookup", Code, null, It.IsAny())) + .ReturnsAsync(Response.FromValue(lookup, Mock.Of())); + Table.Setup(c => c.GetEntityAsync(owningTenantId, Code, null, It.IsAny())) + .ReturnsAsync(Response.FromValue(main, Mock.Of())); + Table.Setup(c => c.UpdateEntityAsync( + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((e, _, _, _) => + { + Updates.Add(e); + return Task.FromResult(Mock.Of()); + }); + + var serviceClient = new Mock(); + serviceClient.Setup(c => c.GetTableClient(It.IsAny())).Returns(Table.Object); + Sut = new TableBootstrapRepository( + new TableStorageService(serviceClient.Object, NullLogger.Instance), + NullLogger.Instance); + } + } + + [Fact] + public async Task Revoke_ForeignTenantCode_IsRefusedWithoutStateChange() + { + var h = new Harness(owningTenantId: TenantB); + + var result = await h.Sut.RevokeBootstrapSessionAsync(TenantA, Code); + + Assert.False(result); + Assert.Empty(h.Updates); + // The owning tenant's main partition must not even be read on a foreign-tenant attempt. + h.Table.Verify(c => c.GetEntityAsync(TenantB, Code, null, It.IsAny()), Times.Never); + } + + [Fact] + public async Task Revoke_OwnTenantCode_RevokesMainAndLookupEntities() + { + var h = new Harness(owningTenantId: TenantA); + + var result = await h.Sut.RevokeBootstrapSessionAsync(TenantA, Code); + + Assert.True(result); + Assert.Equal(2, h.Updates.Count); + Assert.All(h.Updates, e => Assert.True(e.GetBoolean("IsRevoked"))); + Assert.Contains(h.Updates, e => e.PartitionKey == TenantA && e.RowKey == Code); + Assert.Contains(h.Updates, e => e.PartitionKey == "CodeLookup" && e.RowKey == Code); + } + + [Fact] + public async Task Revoke_UnknownCode_ReturnsFalse_SameAsForeignTenant() + { + var h = new Harness(owningTenantId: TenantA); + h.Table.Setup(c => c.GetEntityAsync("CodeLookup", "zzzzzz", null, It.IsAny())) + .ThrowsAsync(new RequestFailedException(404, "not found")); + + var result = await h.Sut.RevokeBootstrapSessionAsync(TenantA, "zzzzzz"); + + Assert.False(result); + Assert.Empty(h.Updates); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableBootstrapRepository.cs b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableBootstrapRepository.cs index 78d5c792..32f9636b 100644 --- a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableBootstrapRepository.cs +++ b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableBootstrapRepository.cs @@ -167,16 +167,26 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage } /// - /// Revokes a bootstrap session. Looks up the tenant via CodeLookup partition first. + /// Revokes a bootstrap session. Resolves the owning tenant via the CodeLookup partition + /// and refuses (returns false, same as not-found) when it differs from the caller's tenant: + /// short codes are platform-global, so tenant scope must be checked against the object itself. /// - public async Task RevokeBootstrapSessionAsync(string shortCode) + public async Task RevokeBootstrapSessionAsync(string tenantId, string shortCode) { try { - // Look up the tenant from the CodeLookup partition + // Look up the owning tenant from the CodeLookup partition var lookupResult = await _tableClient.GetEntityAsync(CodeLookupPartition, shortCode); var lookup = lookupResult.Value; - var tenantId = lookup.GetString("TenantId") ?? ""; + var owningTenantId = lookup.GetString("TenantId") ?? ""; + + if (!string.Equals(owningTenantId, tenantId, StringComparison.Ordinal)) + { + // Do not leak the owner; the caller sees the same 404 as for an unknown code. + _logger.LogWarning("Bootstrap session {ShortCode} revoke refused: code is not owned by tenant {TenantId}", + shortCode, tenantId); + return false; + } // Update main entity var mainResult = await _tableClient.GetEntityAsync(tenantId, shortCode); diff --git a/src/Backend/AutopilotMonitor.Functions/Services/BootstrapSessionService.cs b/src/Backend/AutopilotMonitor.Functions/Services/BootstrapSessionService.cs index 759e1b41..d924b964 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/BootstrapSessionService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/BootstrapSessionService.cs @@ -92,7 +92,9 @@ namespace AutopilotMonitor.Functions.Services } /// - /// Revokes a bootstrap session. Already-running agents with the token will be rejected on next request. + /// Revokes a bootstrap session owned by . Already-running agents with + /// the token will be rejected on next request. Codes owned by another tenant are reported as + /// not found (false) — the repository enforces ownership against the CodeLookup partition. /// public async Task RevokeAsync(string tenantId, string shortCode) { @@ -105,7 +107,7 @@ namespace AutopilotMonitor.Functions.Services _cache.Remove($"bootstrap-token:{session.Token}"); } - var result = await _bootstrapRepo.RevokeBootstrapSessionAsync(shortCode); + var result = await _bootstrapRepo.RevokeBootstrapSessionAsync(tenantId, shortCode); if (result) { _logger.LogInformation("Revoked bootstrap session {ShortCode} for tenant {TenantId}", shortCode, tenantId); diff --git a/src/Shared/AutopilotMonitor.Shared/DataAccess/IBootstrapRepository.cs b/src/Shared/AutopilotMonitor.Shared/DataAccess/IBootstrapRepository.cs index b75691cc..ca764781 100644 --- a/src/Shared/AutopilotMonitor.Shared/DataAccess/IBootstrapRepository.cs +++ b/src/Shared/AutopilotMonitor.Shared/DataAccess/IBootstrapRepository.cs @@ -14,7 +14,13 @@ namespace AutopilotMonitor.Shared.DataAccess Task GetBootstrapSessionByCodeAsync(string shortCode); Task ValidateBootstrapTokenAsync(string token); Task> GetBootstrapSessionsAsync(string tenantId); - Task RevokeBootstrapSessionAsync(string shortCode); + /// + /// Revokes a bootstrap session only when the code belongs to . + /// Returns false (indistinguishable from "not found") when the code does not exist + /// or is owned by another tenant — the short code is a platform-global identifier + /// and the caller's tenant scope must be enforced here, not just at the route. + /// + Task RevokeBootstrapSessionAsync(string tenantId, string shortCode); Task IncrementBootstrapUsageAsync(string shortCode); Task CleanupExpiredAsync(); }