From 7289e5cd48ff434b128c366a83b07813c7de8c1a Mon Sep 17 00:00:00 2001 From: ahall Date: Mon, 15 Jun 2026 10:47:45 +0100 Subject: [PATCH] fix: hide conflicting record on create when not in IAccessControlProvider view (#445) --- .../Controllers/TableController.Create.cs | 20 +++++++++++- .../TableController_Create_Tests.cs | 32 +++++++++++++++++++ .../Helpers/BaseTest.cs | 8 ++--- .../Helpers/ServiceApplicationFactory.cs | 3 +- .../Service/Create_Tests.cs | 30 +++++++++++++++++ .../MovieAccessControlProvider.cs | 10 ++++++ 6 files changed, 97 insertions(+), 6 deletions(-) diff --git a/src/CommunityToolkit.Datasync.Server/Controllers/TableController.Create.cs b/src/CommunityToolkit.Datasync.Server/Controllers/TableController.Create.cs index cb1dd1ae..2ad5d012 100644 --- a/src/CommunityToolkit.Datasync.Server/Controllers/TableController.Create.cs +++ b/src/CommunityToolkit.Datasync.Server/Controllers/TableController.Create.cs @@ -32,7 +32,25 @@ public virtual async Task CreateAsync(CancellationToken cancellat await AuthorizeRequestAsync(TableOperation.Create, entity, cancellationToken).ConfigureAwait(false); await AccessControlProvider.PreCommitHookAsync(TableOperation.Create, entity, cancellationToken).ConfigureAwait(false); - await Repository.CreateAsync(entity, cancellationToken).ConfigureAwait(false); + + try + { + await Repository.CreateAsync(entity, cancellationToken).ConfigureAwait(false); + } + catch (HttpException ex) when (ex.StatusCode == StatusCodes.Status409Conflict && ex.Payload is TEntity conflictingEntity) + { + // A conflicting entity exists. If the conflicting entity is not in the client's data view, then + // returning it (or even acknowledging the conflict) would leak data the client is not allowed to see. + // In that case, return a generic Bad Request without the payload instead. + if (!AccessControlProvider.EntityIsInView(conflictingEntity)) + { + Logger.LogWarning("CreateAsync: {id} statusCode=400 conflicting entity not in view", entity.Id); + throw new HttpException(StatusCodes.Status400BadRequest); + } + + throw; + } + await PostCommitHookAsync(TableOperation.Create, entity, cancellationToken).ConfigureAwait(false); Logger.LogInformation("CreateAsync: created {id}", entity.Id); diff --git a/tests/CommunityToolkit.Datasync.Server.Test/Controllers/TableController_Create_Tests.cs b/tests/CommunityToolkit.Datasync.Server.Test/Controllers/TableController_Create_Tests.cs index 26fa6bd9..197fd335 100644 --- a/tests/CommunityToolkit.Datasync.Server.Test/Controllers/TableController_Create_Tests.cs +++ b/tests/CommunityToolkit.Datasync.Server.Test/Controllers/TableController_Create_Tests.cs @@ -76,6 +76,38 @@ public async Task CreateAsync_RepositoryException_Throws() (await act.Should().ThrowAsync()).WithStatusCode(409); } + [Fact] + public async Task CreateAsync_Conflict_EntityInView_Returns409() + { + TableData conflictingEntity = new() { Id = "0da7fb24-3606-442f-9f68-c47c6e7d09d4" }; + // The data view includes the conflicting entity, so the conflict should be surfaced as a 409. + IAccessControlProvider accessProvider = FakeAccessControlProvider(TableOperation.Create, true, x => x.Id == conflictingEntity.Id); + IRepository repository = FakeRepository(null, true, conflictingEntity); + ExposedTableController controller = new(repository, accessProvider); + TableData entity = new() { Id = conflictingEntity.Id }; + controller.ControllerContext.HttpContext = CreateHttpContext(HttpMethod.Post, "https://localhost/table", entity); + + Func act = async () => await controller.CreateAsync(); + + (await act.Should().ThrowAsync()).WithStatusCode(409); + } + + [Fact] + public async Task CreateAsync_Conflict_EntityNotInView_Returns400() + { + TableData conflictingEntity = new() { Id = "0da7fb24-3606-442f-9f68-c47c6e7d09d4" }; + // The data view excludes the conflicting entity, so the conflict must NOT be surfaced - return 400 instead. + IAccessControlProvider accessProvider = FakeAccessControlProvider(TableOperation.Create, true, x => x.Id == "some-other-id"); + IRepository repository = FakeRepository(null, true, conflictingEntity); + ExposedTableController controller = new(repository, accessProvider); + TableData entity = new() { Id = conflictingEntity.Id }; + controller.ControllerContext.HttpContext = CreateHttpContext(HttpMethod.Post, "https://localhost/table", entity); + + Func act = async () => await controller.CreateAsync(); + + (await act.Should().ThrowAsync()).WithStatusCode(400).And.Which.Payload.Should().BeNull(); + } + [Fact] public async Task CreateAsync_NonJsonData_Throws() { diff --git a/tests/CommunityToolkit.Datasync.Server.Test/Helpers/BaseTest.cs b/tests/CommunityToolkit.Datasync.Server.Test/Helpers/BaseTest.cs index 0548cb28..6314fae5 100644 --- a/tests/CommunityToolkit.Datasync.Server.Test/Helpers/BaseTest.cs +++ b/tests/CommunityToolkit.Datasync.Server.Test/Helpers/BaseTest.cs @@ -32,15 +32,15 @@ protected static IAccessControlProvider FakeAccessControlProvider FakeRepository(TEntity entity = null, bool throwConflict = false) where TEntity : class, ITableData + protected static IRepository FakeRepository(TEntity entity = null, bool throwConflict = false, TEntity conflictPayload = null) where TEntity : class, ITableData { AbstractRepository mock = Substitute.ForPartsOf>(); if (throwConflict) { - mock.CreateAsync(Arg.Any(), Arg.Any()).Returns(ValueTask.FromException(new HttpException(409))); - mock.DeleteAsync(Arg.Any(), Arg.Any(), Arg.Any()).Returns(ValueTask.FromException(new HttpException(409))); - mock.ReplaceAsync(Arg.Any(), Arg.Any(), Arg.Any()).Returns(ValueTask.FromException(new HttpException(409))); + mock.CreateAsync(Arg.Any(), Arg.Any()).Returns(ValueTask.FromException(new HttpException(409) { Payload = conflictPayload })); + mock.DeleteAsync(Arg.Any(), Arg.Any(), Arg.Any()).Returns(ValueTask.FromException(new HttpException(409) { Payload = conflictPayload })); + mock.ReplaceAsync(Arg.Any(), Arg.Any(), Arg.Any()).Returns(ValueTask.FromException(new HttpException(409) { Payload = conflictPayload })); } else { diff --git a/tests/CommunityToolkit.Datasync.Server.Test/Helpers/ServiceApplicationFactory.cs b/tests/CommunityToolkit.Datasync.Server.Test/Helpers/ServiceApplicationFactory.cs index 681a21d7..d37d27ff 100644 --- a/tests/CommunityToolkit.Datasync.Server.Test/Helpers/ServiceApplicationFactory.cs +++ b/tests/CommunityToolkit.Datasync.Server.Test/Helpers/ServiceApplicationFactory.cs @@ -57,11 +57,12 @@ internal TEntity GetServerEntityById(string id) where TEntity : InMemor return repository.GetEntity(id); } - internal void SetupAccessControlProvider(bool isAuthorized) + internal void SetupAccessControlProvider(bool isAuthorized, Expression> dataView = null) { using IServiceScope scope = Services.CreateScope(); IAccessControlProvider provider = scope.ServiceProvider.GetRequiredService>(); (provider as MovieAccessControlProvider).CanBeAuthorized = isAuthorized; + (provider as MovieAccessControlProvider).DataView = dataView; } internal InMemoryMovie GetAuthorizedEntity() diff --git a/tests/CommunityToolkit.Datasync.Server.Test/Service/Create_Tests.cs b/tests/CommunityToolkit.Datasync.Server.Test/Service/Create_Tests.cs index 9b90cb0a..481487fb 100644 --- a/tests/CommunityToolkit.Datasync.Server.Test/Service/Create_Tests.cs +++ b/tests/CommunityToolkit.Datasync.Server.Test/Service/Create_Tests.cs @@ -68,6 +68,36 @@ public async Task Create_SoftDeleted_Returns409() response.Headers.ETag.Should().BeETag($"\"{clientMovie.Version}\""); } + [Fact] + public async Task Create_ExistingId_WithACP_InView_Returns409() + { + InMemoryMovie existingMovie = this.factory.GetRandomMovie(); + // The data view INCLUDES the conflicting entity, so the conflict should be surfaced as a 409 with payload. + this.factory.SetupAccessControlProvider(true, m => m.Id == existingMovie.Id); + ClientMovie source = new(TestData.Movies.BlackPanther) { Id = existingMovie.Id }; + + HttpResponseMessage response = await this.client.PostAsJsonAsync(this.factory.AuthorizedMovieEndpoint, source, this.serializerOptions); + response.StatusCode.Should().Be(HttpStatusCode.Conflict); + + ClientMovie clientMovie = await response.Content.ReadFromJsonAsync(this.serializerOptions); + clientMovie.Should().NotBeNull().And.HaveEquivalentMetadataTo(existingMovie).And.BeEquivalentTo(existingMovie); + } + + [Fact] + public async Task Create_ExistingId_WithACP_NotInView_Returns400() + { + InMemoryMovie existingMovie = this.factory.GetRandomMovie(); + // The data view EXCLUDES the conflicting entity, so the conflict must NOT be surfaced - return 400 with no payload. + this.factory.SetupAccessControlProvider(true, m => m.Id == "id-that-does-not-exist"); + ClientMovie source = new(TestData.Movies.BlackPanther) { Id = existingMovie.Id }; + + HttpResponseMessage response = await this.client.PostAsJsonAsync(this.factory.AuthorizedMovieEndpoint, source, this.serializerOptions); + response.StatusCode.Should().Be(HttpStatusCode.BadRequest); + + string content = await response.Content.ReadAsStringAsync(); + content.Should().NotContain(existingMovie.Id); + } + [Fact] public async Task Create_CanRoundtrip_Types() { diff --git a/tests/CommunityToolkit.Datasync.TestService/AccessControlProviders/MovieAccessControlProvider.cs b/tests/CommunityToolkit.Datasync.TestService/AccessControlProviders/MovieAccessControlProvider.cs index 0922e9fb..33e141c9 100644 --- a/tests/CommunityToolkit.Datasync.TestService/AccessControlProviders/MovieAccessControlProvider.cs +++ b/tests/CommunityToolkit.Datasync.TestService/AccessControlProviders/MovieAccessControlProvider.cs @@ -4,6 +4,7 @@ using CommunityToolkit.Datasync.Server; using CommunityToolkit.Datasync.TestCommon.Models; +using System.Linq.Expressions; namespace CommunityToolkit.Datasync.TestService.AccessControlProviders; @@ -20,6 +21,15 @@ public class MovieAccessControlProvider : AccessControlProvider where T : /// public bool CanBeAuthorized { get; set; } = true; + /// + /// An optional data view filter that restricts which entities are visible to the client. + /// + public Expression> DataView { get; set; } = null; + + /// + public override Expression> GetDataView() + => DataView; + /// public override ValueTask IsAuthorizedAsync(TableOperation operation, T entity, CancellationToken cancellationToken = default) {