mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-24 04:32:32 +00:00
fix(backend): bootstrap revoke enforces code ownership — DELETE /api/bootstrap/sessions/{code} only scoped the caller's ?tenantId while the repository resolved the owning tenant from CodeLookup and revoked any tenant's code; RevokeBootstrapSessionAsync now takes the caller tenant and refuses a mismatch as not-found (no owner leak, foreign partition never read), with regression tests
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
+105
@@ -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;
|
||||
|
||||
/// <summary>
|
||||
/// 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).
|
||||
/// </summary>
|
||||
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<TableClient> Table { get; } = new();
|
||||
public List<TableEntity> 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<TableEntity>("CodeLookup", Code, null, It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(Response.FromValue(lookup, Mock.Of<Response>()));
|
||||
Table.Setup(c => c.GetEntityAsync<TableEntity>(owningTenantId, Code, null, It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(Response.FromValue(main, Mock.Of<Response>()));
|
||||
Table.Setup(c => c.UpdateEntityAsync(
|
||||
It.IsAny<TableEntity>(), It.IsAny<ETag>(), It.IsAny<TableUpdateMode>(), It.IsAny<CancellationToken>()))
|
||||
.Returns<TableEntity, ETag, TableUpdateMode, CancellationToken>((e, _, _, _) =>
|
||||
{
|
||||
Updates.Add(e);
|
||||
return Task.FromResult(Mock.Of<Response>());
|
||||
});
|
||||
|
||||
var serviceClient = new Mock<TableServiceClient>();
|
||||
serviceClient.Setup(c => c.GetTableClient(It.IsAny<string>())).Returns(Table.Object);
|
||||
Sut = new TableBootstrapRepository(
|
||||
new TableStorageService(serviceClient.Object, NullLogger<TableStorageService>.Instance),
|
||||
NullLogger<TableBootstrapRepository>.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<TableEntity>(TenantB, Code, null, It.IsAny<CancellationToken>()), 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<TableEntity>("CodeLookup", "zzzzzz", null, It.IsAny<CancellationToken>()))
|
||||
.ThrowsAsync(new RequestFailedException(404, "not found"));
|
||||
|
||||
var result = await h.Sut.RevokeBootstrapSessionAsync(TenantA, "zzzzzz");
|
||||
|
||||
Assert.False(result);
|
||||
Assert.Empty(h.Updates);
|
||||
}
|
||||
}
|
||||
+14
-4
@@ -167,16 +167,26 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
public async Task<bool> RevokeBootstrapSessionAsync(string shortCode)
|
||||
public async Task<bool> 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<TableEntity>(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<TableEntity>(tenantId, shortCode);
|
||||
|
||||
@@ -92,7 +92,9 @@ namespace AutopilotMonitor.Functions.Services
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Revokes a bootstrap session. Already-running agents with the token will be rejected on next request.
|
||||
/// Revokes a bootstrap session owned by <paramref name="tenantId"/>. 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.
|
||||
/// </summary>
|
||||
public async Task<bool> 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);
|
||||
|
||||
@@ -14,7 +14,13 @@ namespace AutopilotMonitor.Shared.DataAccess
|
||||
Task<BootstrapSession?> GetBootstrapSessionByCodeAsync(string shortCode);
|
||||
Task<BootstrapSession?> ValidateBootstrapTokenAsync(string token);
|
||||
Task<List<BootstrapSession>> GetBootstrapSessionsAsync(string tenantId);
|
||||
Task<bool> RevokeBootstrapSessionAsync(string shortCode);
|
||||
/// <summary>
|
||||
/// Revokes a bootstrap session only when the code belongs to <paramref name="tenantId"/>.
|
||||
/// 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.
|
||||
/// </summary>
|
||||
Task<bool> RevokeBootstrapSessionAsync(string tenantId, string shortCode);
|
||||
Task<bool> IncrementBootstrapUsageAsync(string shortCode);
|
||||
Task<int> CleanupExpiredAsync();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user