diff --git a/backend/src/Taskdeck.Application/Interfaces/IUnitOfWork.cs b/backend/src/Taskdeck.Application/Interfaces/IUnitOfWork.cs index 9be0965e54..6db10d4b2c 100644 --- a/backend/src/Taskdeck.Application/Interfaces/IUnitOfWork.cs +++ b/backend/src/Taskdeck.Application/Interfaces/IUnitOfWork.cs @@ -1,3 +1,5 @@ +using System.Data; + namespace Taskdeck.Application.Interfaces; public interface IUnitOfWork @@ -58,6 +60,17 @@ public interface IUnitOfWork Task BeginReadTransactionAsync(CancellationToken cancellationToken = default); Task BeginTransactionAsync(CancellationToken cancellationToken = default); + + /// + /// Begins a transaction at an explicit isolation level. The default implementation keeps + /// lightweight test doubles source-compatible; the production unit of work overrides it so + /// authorization reads that guard a write can share a serializable snapshot. + /// + Task BeginTransactionAsync( + IsolationLevel isolationLevel, + CancellationToken cancellationToken = default) + => BeginTransactionAsync(cancellationToken); + Task CommitTransactionAsync(CancellationToken cancellationToken = default); Task RollbackTransactionAsync(CancellationToken cancellationToken = default); } diff --git a/backend/src/Taskdeck.Application/Services/CaptureService.cs b/backend/src/Taskdeck.Application/Services/CaptureService.cs index 7128c2f64b..58e3141d5d 100644 --- a/backend/src/Taskdeck.Application/Services/CaptureService.cs +++ b/backend/src/Taskdeck.Application/Services/CaptureService.cs @@ -1,3 +1,4 @@ +using System.Data; using Microsoft.Extensions.Logging; using Taskdeck.Application.DTOs; using Taskdeck.Application.Interfaces; @@ -183,6 +184,17 @@ public async Task> CreateAsync( if (userId == Guid.Empty) return Result.Failure(ErrorCodes.ValidationError, "UserId cannot be empty"); + var boardTransactionStarted = false; + + async Task RollbackBoardTransactionAsync() + { + if (!boardTransactionStarted) + return; + + await _unitOfWork.RollbackTransactionAsync(cancellationToken); + boardTransactionStarted = false; + } + try { var user = await _unitOfWork.Users.GetByIdAsync(userId, cancellationToken); @@ -191,17 +203,39 @@ public async Task> CreateAsync( if (dto.BoardId.HasValue) { - var permissionResult = await _authorizationService.CanReadBoardAsync(userId, dto.BoardId.Value); + // The authorization read and queue insert must share one serializable snapshot. + // Otherwise a board owner can demote this caller after CanWriteBoardAsync returns + // but before SaveChangesAsync, admitting a capture under stale write authority. + await _unitOfWork.BeginTransactionAsync( + IsolationLevel.Serializable, + cancellationToken); + boardTransactionStarted = true; + + // A board-scoped capture can enter that board's proposal queue. Keep the + // attachment boundary aligned with triage: readable Viewer access is not + // authority to inject work into a board only writers can modify (#3291). + var permissionResult = await _authorizationService.CanWriteBoardAsync(userId, dto.BoardId.Value); if (!permissionResult.IsSuccess) + { + await RollbackBoardTransactionAsync(); return Result.Failure(permissionResult.ErrorCode, permissionResult.ErrorMessage); + } if (!permissionResult.Value) - return Result.Failure(ErrorCodes.Forbidden, "You do not have access to this board"); + { + await RollbackBoardTransactionAsync(); + return Result.Failure( + ErrorCodes.Forbidden, + "You do not have permission to attach captures to this board"); + } } var sourceResult = ResolveSource(dto.Source); if (!sourceResult.IsSuccess) + { + await RollbackBoardTransactionAsync(); return Result.Failure(sourceResult.ErrorCode, sourceResult.ErrorMessage); + } var payload = new CapturePayloadV1( CaptureRequestContract.CurrentSchemaVersion, @@ -243,6 +277,12 @@ public async Task> CreateAsync( await _unitOfWork.SaveChangesAsync(cancellationToken); + if (boardTransactionStarted) + { + await _unitOfWork.CommitTransactionAsync(cancellationToken); + boardTransactionStarted = false; + } + return Result.Success(MapToDetailDto( request, attributedPayload, @@ -251,8 +291,14 @@ public async Task> CreateAsync( } catch (DomainException ex) { + await RollbackBoardTransactionAsync(); return Result.Failure(ex.ErrorCode, ex.Message); } + catch + { + await RollbackBoardTransactionAsync(); + throw; + } } public async Task>> ListAsync( diff --git a/backend/src/Taskdeck.Application/Services/LlmQueueService.cs b/backend/src/Taskdeck.Application/Services/LlmQueueService.cs index a1ca53643d..a0f49948d4 100644 --- a/backend/src/Taskdeck.Application/Services/LlmQueueService.cs +++ b/backend/src/Taskdeck.Application/Services/LlmQueueService.cs @@ -1,3 +1,4 @@ +using System.Data; using Taskdeck.Application.DTOs; using Taskdeck.Application.Interfaces; using Taskdeck.Domain.Common; @@ -43,33 +44,33 @@ public LlmQueueService( public async Task> AddToQueueAsync(Guid userId, CreateLlmRequestDto dto) { + var boardTransactionStarted = false; + + async Task RollbackBoardTransactionAsync() + { + if (!boardTransactionStarted) + return; + + await _unitOfWork.RollbackTransactionAsync(); + boardTransactionStarted = false; + } + try { var user = await _unitOfWork.Users.GetByIdAsync(userId); if (user == null) return Result.Failure(ErrorCodes.NotFound, $"User with ID {userId} not found"); - if (dto.BoardId.HasValue) - { - var permissionResult = await _authorizationService.CanReadBoardAsync(userId, dto.BoardId.Value); - if (!permissionResult.IsSuccess) - { - return Result.Failure(permissionResult.ErrorCode, permissionResult.ErrorMessage); - } - - if (!permissionResult.Value) - { - return Result.Failure(ErrorCodes.Forbidden, "You do not have access to this board"); - } - } - var requestTypeValidation = CaptureRequestContract.ValidateRequestType(dto.RequestType); if (!requestTypeValidation.IsSuccess) { return Result.Failure(requestTypeValidation.ErrorCode, requestTypeValidation.ErrorMessage); } - var requestType = dto.RequestType; + // Validation accepts surrounding whitespace, so carry the same normalized value + // into capture classification. Otherwise a padded capture type could select the + // readable-board gate instead of the write gate below. + var requestType = dto.RequestType.Trim(); var payload = dto.Payload; CapturePayloadV1? capturePayload = null; if (CaptureRequestContract.IsCaptureRequestType(requestType)) @@ -88,6 +89,37 @@ public async Task> AddToQueueAsync(Guid userId, CreateLlmR payload = CaptureRequestContract.SerializePayload(capturePayload); } + if (dto.BoardId.HasValue) + { + // Capture-shaped queue requests are an alternate capture intake path, not merely + // readable board metadata. Keep their write gate and transaction boundary aligned + // with CaptureService so a Viewer cannot attach new work through this endpoint. + if (capturePayload is not null) + { + await _unitOfWork.BeginTransactionAsync( + IsolationLevel.Serializable); + boardTransactionStarted = true; + } + + var permissionResult = capturePayload is not null + ? await _authorizationService.CanWriteBoardAsync(userId, dto.BoardId.Value) + : await _authorizationService.CanReadBoardAsync(userId, dto.BoardId.Value); + if (!permissionResult.IsSuccess) + { + await RollbackBoardTransactionAsync(); + return Result.Failure(permissionResult.ErrorCode, permissionResult.ErrorMessage); + } + + if (!permissionResult.Value) + { + await RollbackBoardTransactionAsync(); + var message = capturePayload is not null + ? "You do not have permission to attach captures to this board" + : "You do not have access to this board"; + return Result.Failure(ErrorCodes.Forbidden, message); + } + } + var request = new LlmRequest(userId, requestType, payload, dto.BoardId); await _unitOfWork.LlmQueue.AddAsync(request); @@ -102,12 +134,24 @@ public async Task> AddToQueueAsync(Guid userId, CreateLlmR await _unitOfWork.SaveChangesAsync(); + if (boardTransactionStarted) + { + await _unitOfWork.CommitTransactionAsync(); + boardTransactionStarted = false; + } + return Result.Success(MapToDto(request)); } catch (DomainException ex) { + await RollbackBoardTransactionAsync(); return Result.Failure(ex.ErrorCode, ex.Message); } + catch + { + await RollbackBoardTransactionAsync(); + throw; + } } public async Task>> GetUserQueueAsync(Guid userId) diff --git a/backend/src/Taskdeck.Infrastructure/Repositories/BoardAccessRepository.cs b/backend/src/Taskdeck.Infrastructure/Repositories/BoardAccessRepository.cs index 4429770071..f130e9561b 100644 --- a/backend/src/Taskdeck.Infrastructure/Repositories/BoardAccessRepository.cs +++ b/backend/src/Taskdeck.Infrastructure/Repositories/BoardAccessRepository.cs @@ -18,6 +18,7 @@ public BoardAccessRepository(TaskdeckDbContext context) : base(context) public async Task GetByBoardAndUserAsync(Guid boardId, Guid userId, CancellationToken cancellationToken = default) { return await _context.BoardAccesses + .AsNoTracking() .Include(ba => ba.User) .Include(ba => ba.Board) .FirstOrDefaultAsync(ba => ba.BoardId == boardId && ba.UserId == userId, cancellationToken); diff --git a/backend/src/Taskdeck.Infrastructure/Repositories/UnitOfWork.cs b/backend/src/Taskdeck.Infrastructure/Repositories/UnitOfWork.cs index 1955d6c38d..d967e949ea 100644 --- a/backend/src/Taskdeck.Infrastructure/Repositories/UnitOfWork.cs +++ b/backend/src/Taskdeck.Infrastructure/Repositories/UnitOfWork.cs @@ -262,6 +262,15 @@ public async Task BeginTransactionAsync(CancellationToken cancellationToken = de _transaction = await _context.Database.BeginTransactionAsync(cancellationToken); } + public async Task BeginTransactionAsync( + IsolationLevel isolationLevel, + CancellationToken cancellationToken = default) + { + _transaction = await _context.Database.BeginTransactionAsync( + isolationLevel, + cancellationToken); + } + public async Task CommitTransactionAsync(CancellationToken cancellationToken = default) { var transaction = _transaction; diff --git a/backend/tests/Taskdeck.Api.Tests/BoardAccessRepositoryFreshnessTests.cs b/backend/tests/Taskdeck.Api.Tests/BoardAccessRepositoryFreshnessTests.cs new file mode 100644 index 0000000000..950ae21d56 --- /dev/null +++ b/backend/tests/Taskdeck.Api.Tests/BoardAccessRepositoryFreshnessTests.cs @@ -0,0 +1,62 @@ +using FluentAssertions; +using Microsoft.EntityFrameworkCore; +using Taskdeck.Domain.Entities; +using Taskdeck.Domain.Enums; +using Taskdeck.Infrastructure.Persistence; +using Taskdeck.Infrastructure.Repositories; +using Xunit; + +namespace Taskdeck.Api.Tests; + +public sealed class BoardAccessRepositoryFreshnessTests +{ + [Fact] + public async Task GetByBoardAndUserAsync_reads_role_changes_after_an_earlier_read() + { + var dbPath = Path.Combine(Path.GetTempPath(), $"taskdeck-board-access-freshness-{Guid.NewGuid():N}.db"); + try + { + var options = new DbContextOptionsBuilder() + .UseSqlite(TestSqlite.ConnectionString(dbPath)) + .Options; + + var owner = new User("access-freshness-owner", $"access-freshness-owner-{Guid.NewGuid():N}@example.com", "hash"); + var member = new User("access-freshness-member", $"access-freshness-member-{Guid.NewGuid():N}@example.com", "hash"); + var board = new Board("Access freshness", ownerId: owner.Id); + var access = new BoardAccess(board.Id, member.Id, UserRole.Editor, owner.Id); + + await using (var seed = new TaskdeckDbContext(options)) + { + await seed.Database.MigrateAsync(); + seed.Users.AddRange(owner, member); + seed.Boards.Add(board); + seed.BoardAccesses.Add(access); + await seed.SaveChangesAsync(); + } + + await using var reader = new TaskdeckDbContext(options); + var repository = new BoardAccessRepository(reader); + var initial = await repository.GetByBoardAndUserAsync(board.Id, member.Id); + initial!.Role.Should().Be(UserRole.Editor); + + await using (var writer = new TaskdeckDbContext(options)) + { + var persisted = await writer.BoardAccesses.SingleAsync(value => + value.BoardId == board.Id && value.UserId == member.Id); + persisted.UpdateRole(UserRole.Viewer, owner.Id); + await writer.SaveChangesAsync(); + } + + var refreshed = await repository.GetByBoardAndUserAsync(board.Id, member.Id); + refreshed!.Role.Should().Be(UserRole.Viewer); + } + finally + { + foreach (var suffix in new[] { "", "-wal", "-shm", "-journal" }) + { + try { File.Delete(dbPath + suffix); } + catch (IOException) { } + } + } + } +} diff --git a/backend/tests/Taskdeck.Api.Tests/CaptureApiTests.cs b/backend/tests/Taskdeck.Api.Tests/CaptureApiTests.cs index 284aaa6a44..126ed39f8a 100644 --- a/backend/tests/Taskdeck.Api.Tests/CaptureApiTests.cs +++ b/backend/tests/Taskdeck.Api.Tests/CaptureApiTests.cs @@ -888,8 +888,8 @@ public async Task Triage_ShouldSucceed_WhenTargetBoardMemberIsEditor() [Fact] public async Task Triage_ShouldReturnForbidden_WhenAlreadyLinkedBoardIsReadOnlyForCaller() { - // The gate has to sit on the effective board, not only on the triage body: a capture created - // with a readable board (create is read-gated) and accepted with no body is the same vector. + // Create while the caller is write-capable, then demote the membership before triage. The + // already-linked board gate must still reject the read-only caller with no target body. var ownerClient = _factory.CreateClient(); var viewerClient = _factory.CreateClient(); await ApiTestHarness.AuthenticateAsync(ownerClient, "capture-triage-linked-gate-owner"); @@ -898,8 +898,10 @@ public async Task Triage_ShouldReturnForbidden_WhenAlreadyLinkedBoardIsReadOnlyF var grantResponse = await ownerClient.PostAsJsonAsync( $"/api/boards/{board.Id}/access", - new GrantAccessDto(board.Id, viewer.UserId, UserRole.Viewer)); + new GrantAccessDto(board.Id, viewer.UserId, UserRole.Editor)); grantResponse.StatusCode.Should().Be(HttpStatusCode.OK); + var access = await grantResponse.Content.ReadFromJsonAsync(); + access.Should().NotBeNull(); var createResponse = await viewerClient.PostAsJsonAsync( "/api/capture/items", @@ -909,6 +911,11 @@ public async Task Triage_ShouldReturnForbidden_WhenAlreadyLinkedBoardIsReadOnlyF created.Should().NotBeNull(); created!.BoardId.Should().Be(board.Id); + var demoteResponse = await ownerClient.PutAsJsonAsync( + $"/api/boards/{board.Id}/access/{access!.Id}", + new UpdateAccessDto(UserRole.Viewer)); + demoteResponse.StatusCode.Should().Be(HttpStatusCode.OK); + var triageResponse = await viewerClient.PostAsync($"/api/capture/items/{created.Id}/triage", null); await ApiTestHarness.AssertErrorContractAsync(triageResponse, HttpStatusCode.Forbidden, "Forbidden"); diff --git a/backend/tests/Taskdeck.Api.Tests/CaptureBoardAttachmentAuthorizationApiTests.cs b/backend/tests/Taskdeck.Api.Tests/CaptureBoardAttachmentAuthorizationApiTests.cs new file mode 100644 index 0000000000..45e90fc64a --- /dev/null +++ b/backend/tests/Taskdeck.Api.Tests/CaptureBoardAttachmentAuthorizationApiTests.cs @@ -0,0 +1,97 @@ +using System.Net; +using System.Net.Http.Json; +using FluentAssertions; +using Microsoft.Extensions.DependencyInjection; +using Taskdeck.Api.Tests.Support; +using Taskdeck.Application.DTOs; +using Taskdeck.Domain.Entities; +using Taskdeck.Domain.Enums; +using Taskdeck.Infrastructure.Persistence; +using Xunit; + +namespace Taskdeck.Api.Tests; + +public class CaptureBoardAttachmentAuthorizationApiTests : IClassFixture +{ + private readonly TestWebApplicationFactory _factory; + private readonly HttpClient _client; + + public CaptureBoardAttachmentAuthorizationApiTests(TestWebApplicationFactory factory) + { + _factory = factory; + _client = factory.CreateClient(); + } + + [Fact] + public async Task Viewer_CannotCreateCaptureAttachedToReadableBoard() + { + var suffix = Guid.NewGuid().ToString("N")[..8]; + var owner = await ApiTestHarness.AuthenticateAsync(_client, "cap-owner"); + var board = await ApiTestHarness.CreateBoardAsync( + _client, + $"Viewer capture boundary {suffix}"); + var viewer = await ApiTestHarness.AuthenticateAsync(_client, "cap-viewer"); + + using (var scope = _factory.Services.CreateScope()) + { + var db = scope.ServiceProvider.GetRequiredService(); + db.BoardAccesses.Add(new BoardAccess( + board.Id, + viewer.UserId, + UserRole.Viewer, + owner.UserId)); + await db.SaveChangesAsync(); + } + + var response = await _client.PostAsJsonAsync( + "/api/capture/items", + new CreateCaptureItemDto( + board.Id, + "A Viewer must not attach a capture to this board", + "paste")); + + await ApiTestHarness.AssertErrorContractAsync( + response, + HttpStatusCode.Forbidden, + "Forbidden"); + var captures = await _client.GetFromJsonAsync>( + "/api/capture/items"); + captures.Should().BeEmpty( + "a refused board attachment must not persist a board-scoped capture"); + } + + [Fact] + public async Task Editor_CanCreateCaptureAttachedToWritableBoard() + { + var suffix = Guid.NewGuid().ToString("N")[..8]; + var owner = await ApiTestHarness.AuthenticateAsync(_client, "cap-owner"); + var board = await ApiTestHarness.CreateBoardAsync( + _client, + $"Editor capture boundary {suffix}"); + var editor = await ApiTestHarness.AuthenticateAsync(_client, "cap-editor"); + + using (var scope = _factory.Services.CreateScope()) + { + var db = scope.ServiceProvider.GetRequiredService(); + db.BoardAccesses.Add(new BoardAccess( + board.Id, + editor.UserId, + UserRole.Editor, + owner.UserId)); + await db.SaveChangesAsync(); + } + + var response = await _client.PostAsJsonAsync( + "/api/capture/items", + new CreateCaptureItemDto( + board.Id, + "An Editor may attach a capture to this board", + "paste")); + + response.StatusCode.Should().Be(HttpStatusCode.Created); + var capture = await response.Content.ReadFromJsonAsync(); + capture.Should().NotBeNull(); + capture!.BoardId.Should().Be(board.Id); + capture.UserId.Should().Be(editor.UserId); + } +} diff --git a/backend/tests/Taskdeck.Api.Tests/LlmQueueApiTests.cs b/backend/tests/Taskdeck.Api.Tests/LlmQueueApiTests.cs index 1a327157c9..fc3fef3b65 100644 --- a/backend/tests/Taskdeck.Api.Tests/LlmQueueApiTests.cs +++ b/backend/tests/Taskdeck.Api.Tests/LlmQueueApiTests.cs @@ -2,8 +2,12 @@ using System.Net.Http.Json; using System.Text.Json; using FluentAssertions; +using Microsoft.Extensions.DependencyInjection; using Taskdeck.Api.Tests.Support; using Taskdeck.Application.DTOs; +using Taskdeck.Domain.Entities; +using Taskdeck.Domain.Enums; +using Taskdeck.Infrastructure.Persistence; using Xunit; namespace Taskdeck.Api.Tests; @@ -118,6 +122,45 @@ public async Task AddToQueue_ShouldReturnForbidden_WhenBoardBelongsToDifferentUs await ApiTestHarness.AssertForbiddenAsync(response); } + [Fact] + public async Task AddToQueue_ShouldReturnForbidden_WhenViewerSubmitsCaptureForReadableBoard() + { + using var ownerClient = _factory.CreateClient(); + using var viewerClient = _factory.CreateClient(); + + var owner = await ApiTestHarness.AuthenticateAsync(ownerClient, "llm-capture-owner"); + var viewer = await ApiTestHarness.AuthenticateAsync(viewerClient, "llm-capture-viewer"); + var board = await ApiTestHarness.CreateBoardAsync(ownerClient, "llm-capture-protected-board"); + + using (var scope = _factory.Services.CreateScope()) + { + var db = scope.ServiceProvider.GetRequiredService(); + db.BoardAccesses.Add(new BoardAccess( + board.Id, + viewer.UserId, + UserRole.Viewer, + owner.UserId)); + await db.SaveChangesAsync(); + } + + var response = await viewerClient.PostAsJsonAsync( + "/api/llm-queue", + new CreateLlmRequestDto( + CaptureRequestContract.RequestTypeV1, + "A Viewer must not attach a capture through the queue", + board.Id)); + + await ApiTestHarness.AssertErrorContractAsync( + response, + HttpStatusCode.Forbidden, + "Forbidden"); + + var requests = await viewerClient.GetFromJsonAsync>( + "/api/llm-queue/user"); + requests.Should().BeEmpty( + "a refused capture attachment must not persist a queue request"); + } + [Fact] public async Task AddToQueue_ShouldReturnBadRequest_WhenCapturePayloadSpoofsProvenanceAttribution() { diff --git a/backend/tests/Taskdeck.Api.Tests/McpResourcesTests.cs b/backend/tests/Taskdeck.Api.Tests/McpResourcesTests.cs index cff4e37e03..49aaa54a68 100644 --- a/backend/tests/Taskdeck.Api.Tests/McpResourcesTests.cs +++ b/backend/tests/Taskdeck.Api.Tests/McpResourcesTests.cs @@ -314,7 +314,11 @@ private async Task RevokeBoardAccessAsync(IServiceScope scope, Guid boardId, Gui var uow = scope.ServiceProvider.GetRequiredService(); var access = await uow.BoardAccesses.GetByBoardAndUserAsync(boardId, userId); access.Should().NotBeNull(); - await uow.BoardAccesses.DeleteAsync(access!); + // The authorization query is intentionally no-tracking so repeated reads observe + // membership changes. Reload the entity through the tracked identity before mutating it. + var trackedAccess = await uow.BoardAccesses.GetByIdAsync(access!.Id); + trackedAccess.Should().NotBeNull(); + await uow.BoardAccesses.DeleteAsync(trackedAccess!); await uow.SaveChangesAsync(); } @@ -328,8 +332,12 @@ private async Task DemoteToViewerAsync(IServiceScope scope, Guid boardId, Guid u var uow = scope.ServiceProvider.GetRequiredService(); var access = await uow.BoardAccesses.GetByBoardAndUserAsync(boardId, userId); access.Should().NotBeNull(); - access!.UpdateRole(UserRole.Viewer, access.GrantedBy); - await uow.BoardAccesses.UpdateAsync(access); + // The authorization query is intentionally no-tracking so repeated reads observe + // membership changes. Reload the entity through the tracked identity before mutating it. + var trackedAccess = await uow.BoardAccesses.GetByIdAsync(access!.Id); + trackedAccess.Should().NotBeNull(); + trackedAccess!.UpdateRole(UserRole.Viewer, trackedAccess.GrantedBy); + await uow.BoardAccesses.UpdateAsync(trackedAccess); await uow.SaveChangesAsync(); } diff --git a/backend/tests/Taskdeck.Application.Tests/Services/CaptureBoardAttachmentAuthorizationTests.cs b/backend/tests/Taskdeck.Application.Tests/Services/CaptureBoardAttachmentAuthorizationTests.cs new file mode 100644 index 0000000000..10cc76aace --- /dev/null +++ b/backend/tests/Taskdeck.Application.Tests/Services/CaptureBoardAttachmentAuthorizationTests.cs @@ -0,0 +1,134 @@ +using FluentAssertions; +using Moq; +using Taskdeck.Application.DTOs; +using Taskdeck.Application.Interfaces; +using Taskdeck.Application.Services; +using Taskdeck.Domain.Common; +using Taskdeck.Domain.Entities; +using Taskdeck.Domain.Exceptions; +using Xunit; + +namespace Taskdeck.Application.Tests.Services; + +public class CaptureBoardAttachmentAuthorizationTests +{ + private readonly Mock _unitOfWork = new(); + private readonly Mock _authorization = new(); + private readonly Mock _users = new(); + private readonly Mock _llmQueue = new(); + private readonly User _user = new("capture-viewer", "capture-viewer@example.com", "Password1!"); + + public CaptureBoardAttachmentAuthorizationTests() + { + _unitOfWork.SetupGet(unit => unit.Users).Returns(_users.Object); + _unitOfWork.SetupGet(unit => unit.LlmQueue).Returns(_llmQueue.Object); + _users + .Setup(repository => repository.GetByIdAsync( + _user.Id, + It.IsAny())) + .ReturnsAsync(_user); + _llmQueue + .Setup(repository => repository.AddAsync( + It.IsAny(), + It.IsAny())) + .ReturnsAsync((LlmRequest request, CancellationToken _) => request); + _unitOfWork + .Setup(unit => unit.SaveChangesAsync(It.IsAny())) + .ReturnsAsync(1); + } + + private CaptureService BuildService() => new( + _unitOfWork.Object, + _authorization.Object); + + [Fact] + public async Task CreateAsync_RejectsReadableBoard_WhenCallerCannotWrite() + { + var boardId = Guid.NewGuid(); + _authorization + .Setup(service => service.CanReadBoardAsync(_user.Id, boardId)) + .ReturnsAsync(Result.Success(true)); + _authorization + .Setup(service => service.CanWriteBoardAsync(_user.Id, boardId)) + .ReturnsAsync(Result.Success(false)); + + var result = await BuildService().CreateAsync( + _user.Id, + new CreateCaptureItemDto(boardId, "Viewer must not attach this capture", "paste")); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.Forbidden); + _authorization.Verify( + service => service.CanWriteBoardAsync(_user.Id, boardId), + Times.Once); + _llmQueue.Verify( + repository => repository.AddAsync( + It.IsAny(), + It.IsAny()), + Times.Never); + _unitOfWork.Verify( + unit => unit.SaveChangesAsync(It.IsAny()), + Times.Never); + } + + [Fact] + public async Task CreateAsync_PropagatesWriteAuthorizationFailure_WithoutPersistence() + { + var boardId = Guid.NewGuid(); + _authorization + .Setup(service => service.CanReadBoardAsync(_user.Id, boardId)) + .ReturnsAsync(Result.Success(true)); + _authorization + .Setup(service => service.CanWriteBoardAsync(_user.Id, boardId)) + .ReturnsAsync(Result.Failure( + ErrorCodes.NotFound, + "Board not found")); + + var result = await BuildService().CreateAsync( + _user.Id, + new CreateCaptureItemDto(boardId, "Missing board", "paste")); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.NotFound); + result.ErrorMessage.Should().Be("Board not found"); + _llmQueue.Verify( + repository => repository.AddAsync( + It.IsAny(), + It.IsAny()), + Times.Never); + _unitOfWork.Verify( + unit => unit.SaveChangesAsync(It.IsAny()), + Times.Never); + } + + [Fact] + public async Task CreateAsync_AllowsWritableBoard_WithoutConsultingReadPermission() + { + var boardId = Guid.NewGuid(); + _authorization + .Setup(service => service.CanWriteBoardAsync(_user.Id, boardId)) + .ReturnsAsync(Result.Success(true)); + _authorization + .Setup(service => service.CanReadBoardAsync(_user.Id, boardId)) + .ThrowsAsync(new InvalidOperationException("Read permission is not the attachment contract")); + + var result = await BuildService().CreateAsync( + _user.Id, + new CreateCaptureItemDto(boardId, "Editor may attach this capture", "paste")); + + result.IsSuccess.Should().BeTrue(); + result.Value.BoardId.Should().Be(boardId); + _authorization.Verify( + service => service.CanReadBoardAsync(It.IsAny(), It.IsAny()), + Times.Never); + _llmQueue.Verify( + repository => repository.AddAsync( + It.Is(request => + request.UserId == _user.Id && request.BoardId == boardId), + It.IsAny()), + Times.Once); + _unitOfWork.Verify( + unit => unit.SaveChangesAsync(It.IsAny()), + Times.Once); + } +} diff --git a/backend/tests/Taskdeck.Application.Tests/Services/CaptureServiceDualWriteTests.cs b/backend/tests/Taskdeck.Application.Tests/Services/CaptureServiceDualWriteTests.cs index 589a64cf0c..4e44b5266e 100644 --- a/backend/tests/Taskdeck.Application.Tests/Services/CaptureServiceDualWriteTests.cs +++ b/backend/tests/Taskdeck.Application.Tests/Services/CaptureServiceDualWriteTests.cs @@ -176,7 +176,7 @@ public async Task CreateAsync_WithDualWriteEnabled_ShouldCarryTheBoardAsContextH var boardId = Guid.NewGuid(); Capture? mirrored = null; _authorizationServiceMock - .Setup(s => s.CanReadBoardAsync(_userId, boardId)) + .Setup(s => s.CanWriteBoardAsync(_userId, boardId)) .ReturnsAsync(Result.Success(true)); _captureStoreMock .Setup(s => s.AddAsync(It.IsAny(), It.IsAny())) diff --git a/backend/tests/Taskdeck.Application.Tests/Services/CaptureServiceTests.cs b/backend/tests/Taskdeck.Application.Tests/Services/CaptureServiceTests.cs index fddbdf5dd9..088e700169 100644 --- a/backend/tests/Taskdeck.Application.Tests/Services/CaptureServiceTests.cs +++ b/backend/tests/Taskdeck.Application.Tests/Services/CaptureServiceTests.cs @@ -125,7 +125,7 @@ public async Task CreateAsync_ShouldPersistCaptureRequestAndReturnDetail() .Setup(r => r.GetByIdAsync(userId, default)) .ReturnsAsync(user); _authorizationServiceMock - .Setup(s => s.CanReadBoardAsync(userId, boardId)) + .Setup(s => s.CanWriteBoardAsync(userId, boardId)) .ReturnsAsync(Result.Success(true)); _llmQueueRepositoryMock .Setup(r => r.AddAsync(It.IsAny(), default)) @@ -177,7 +177,7 @@ public async Task CreateAsync_ShouldAssignTranscriptRequestType_ForTranscriptSou .Setup(r => r.GetByIdAsync(userId, default)) .ReturnsAsync(user); _authorizationServiceMock - .Setup(s => s.CanReadBoardAsync(userId, boardId)) + .Setup(s => s.CanWriteBoardAsync(userId, boardId)) .ReturnsAsync(Result.Success(true)); _llmQueueRepositoryMock .Setup(r => r.AddAsync(It.IsAny(), default)) @@ -250,7 +250,7 @@ public async Task CreateAsync_ShouldReturnForbidden_WhenBoardAccessIsDenied() .Setup(r => r.GetByIdAsync(userId, default)) .ReturnsAsync(user); _authorizationServiceMock - .Setup(s => s.CanReadBoardAsync(userId, boardId)) + .Setup(s => s.CanWriteBoardAsync(userId, boardId)) .ReturnsAsync(Result.Success(false)); var result = await _service.CreateAsync(userId, dto); diff --git a/backend/tests/Taskdeck.Application.Tests/Services/LlmQueueServiceTests.cs b/backend/tests/Taskdeck.Application.Tests/Services/LlmQueueServiceTests.cs index a1ec21ccdc..7f02fae2cb 100644 --- a/backend/tests/Taskdeck.Application.Tests/Services/LlmQueueServiceTests.cs +++ b/backend/tests/Taskdeck.Application.Tests/Services/LlmQueueServiceTests.cs @@ -74,7 +74,7 @@ public async Task AddToQueueAsync_ShouldNormalizeCapturePayload_WhenCaptureReque _userRepoMock.Setup(r => r.GetByIdAsync(userId, default)) .ReturnsAsync(user); - _authorizationServiceMock.Setup(s => s.CanReadBoardAsync(userId, boardId)) + _authorizationServiceMock.Setup(s => s.CanWriteBoardAsync(userId, boardId)) .ReturnsAsync(Result.Success(true)); _llmQueueRepoMock.Setup(r => r.AddAsync(It.IsAny(), default)) .Callback((request, _) => persistedRequest = request) @@ -110,7 +110,7 @@ public async Task AddToQueueAsync_ShouldNormalizeToTranscriptRequestType_WhenPay _userRepoMock.Setup(r => r.GetByIdAsync(userId, default)) .ReturnsAsync(user); - _authorizationServiceMock.Setup(s => s.CanReadBoardAsync(userId, boardId)) + _authorizationServiceMock.Setup(s => s.CanWriteBoardAsync(userId, boardId)) .ReturnsAsync(Result.Success(true)); _llmQueueRepoMock.Setup(r => r.AddAsync(It.IsAny(), default)) .Callback((request, _) => persistedRequest = request) @@ -225,6 +225,63 @@ public async Task AddToQueueAsync_ShouldReturnForbidden_WhenUserCannotAccessBoar _unitOfWorkMock.Verify(u => u.SaveChangesAsync(default), Times.Never); } + [Fact] + public async Task AddToQueueAsync_ShouldRejectViewerCaptureAttachment() + { + var userId = Guid.NewGuid(); + var boardId = Guid.NewGuid(); + var user = new User("testuser", "test@example.com", "hashedpassword"); + var dto = new CreateLlmRequestDto( + CaptureRequestContract.RequestTypeV1, + "Capture this quick note", + boardId); + + _userRepoMock.Setup(r => r.GetByIdAsync(userId, default)) + .ReturnsAsync(user); + _authorizationServiceMock.Setup(s => s.CanWriteBoardAsync(userId, boardId)) + .ReturnsAsync(Result.Success(false)); + + var result = await _service.AddToQueueAsync(userId, dto); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.Forbidden); + result.ErrorMessage.Should().Contain("attach captures"); + _llmQueueRepoMock.Verify(r => r.AddAsync(It.IsAny(), default), Times.Never); + _unitOfWorkMock.Verify(u => u.SaveChangesAsync(default), Times.Never); + _unitOfWorkMock.Verify(u => u.RollbackTransactionAsync(default), Times.Once); + } + + [Fact] + public async Task AddToQueueAsync_ShouldUseWriteGateForWhitespacePaddedCaptureRequestType() + { + var userId = Guid.NewGuid(); + var boardId = Guid.NewGuid(); + var user = new User("testuser", "test@example.com", "hashedpassword"); + var dto = new CreateLlmRequestDto( + $" {CaptureRequestContract.RequestTypeV1} ", + "Capture this padded quick note", + boardId); + + _userRepoMock.Setup(r => r.GetByIdAsync(userId, default)) + .ReturnsAsync(user); + _authorizationServiceMock.Setup(s => s.CanWriteBoardAsync(userId, boardId)) + .ReturnsAsync(Result.Success(false)); + _authorizationServiceMock.Setup(s => s.CanReadBoardAsync(userId, boardId)) + .ReturnsAsync(Result.Success(true)); + + var result = await _service.AddToQueueAsync(userId, dto); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.Forbidden); + _authorizationServiceMock.Verify(s => s.CanWriteBoardAsync(userId, boardId), Times.Once); + _authorizationServiceMock.Verify(s => s.CanReadBoardAsync(userId, boardId), Times.Never); + _unitOfWorkMock.Verify( + u => u.BeginTransactionAsync(System.Data.IsolationLevel.Serializable, default), + Times.Once); + _unitOfWorkMock.Verify(u => u.RollbackTransactionAsync(default), Times.Once); + _llmQueueRepoMock.Verify(r => r.AddAsync(It.IsAny(), default), Times.Never); + } + [Fact] public async Task AddToQueueAsync_ShouldReturnNotFound_WhenBoardDoesNotExist() { diff --git a/frontend/taskdeck-web/src/components/board/BoardActionRail.vue b/frontend/taskdeck-web/src/components/board/BoardActionRail.vue index 60e37262d8..19c45c7f88 100644 --- a/frontend/taskdeck-web/src/components/board/BoardActionRail.vue +++ b/frontend/taskdeck-web/src/components/board/BoardActionRail.vue @@ -1,4 +1,11 @@