SECURITY: Remove conflicting record when IAccessControlProvider says you can't see it #445

Activity

1. ahallctr-jpg commented on Mar 15, 2026

   

   ahallctr-jpg

   on Mar 15, 2026

   Author

   More actions

   Analysis: Hide Conflicting Record from Unauthorized Clients

   Summary

   When Repository.CreateAsync detects a duplicate ID, it throws HttpException(409) { Payload = existingEntity }. This propagates through DatasyncControllerAttribute.OnException which serializes the payload as the 409 response body. No EntityIsInView check is performed — the conflicting entity is always returned, even if the client's IAccessControlProvider data view would hide it. This is a data leak.

   The fix: wrap Repository.CreateAsync in a try/catch for HttpException where StatusCode == 409, then check AccessControlProvider.EntityIsInView() on the payload entity. If in view, re-throw (existing behavior). If not in view, throw HttpException(400) without the payload.

   T-Shirt Size: M

   • 2 source files to change, 1 test helper to update
   • ~4 new test methods (unit + integration)
   • Estimated effort: 2–4 hours

   Breaking Change Assessment

   Conditionally breaking — but safe for existing clients.

   This behavior change ONLY occurs when ALL of these conditions are true simultaneously:

   1. The server has an IAccessControlProvider configured with a non-null GetDataView()
   2. The data view EXCLUDES the conflicting entity
   3. A client attempts to create an entity whose ID collides with the hidden entity

   For users NOT using IAccessControlProvider (or using the default AccessControlProvider which returns null from GetDataView()), EntityIsInView always returns true — so the existing 409-with-payload behavior is 100% preserved.

   Client-side compatibility (confirmed via code review)

   Online path (DatasyncServiceClient.AddAsync):

   1. result.ThrowIfConflict(entity) — checks IsConflictStatusCode which is StatusCode is 409 or 412. A 400 does NOT match, so this is skipped.
   2. result.ThrowIfNotSuccessful(requireContent: true) — 400 is not successful, so this throws DatasyncHttpException.
   3. Result: Old client code that catches ConflictException<TEntity> will NOT catch the 400. Instead, DatasyncHttpException is thrown. Since ConflictException<TEntity> extends DatasyncHttpException, any code that catches the base type still works.

   Offline push path (OperationsQueueManager.PushOperationAsync):

   1. response.IsConflictStatusCode is false for 400, so conflict resolution is skipped entirely.
   2. !isSuccessful is true, so the operation is marked OperationState.Failed with HttpStatusCode = 400 and added to PushResult.FailedRequests.
   3. Result: The push operation fails cleanly. No conflict resolver is invoked (which is correct — the client can't see the conflicting entity, so conflict resolution is meaningless).

   Bottom line: Existing client code handles a 400 gracefully. The behavior change is scoped to the narrow intersection of users who both configure an IAccessControlProvider with a data view AND have ID collisions against hidden entities — and for those users, the old behavior was a data leak.

   Implementation

   The change to TableController.Create.cs follows the exact EntityIsInView pattern used in Delete (TableController.Delete.cs:28-32), Read (TableController.Read.cs:29-33), and Replace (TableController.Replace.cs:38-42):

   // In CreateAsync, wrap the Repository.CreateAsync call:
   try
   {
       await Repository.CreateAsync(entity, cancellationToken).ConfigureAwait(false);
   }
   catch (HttpException ex) when (ex.StatusCode == StatusCodes.Status409Conflict && ex.Payload is TEntity conflictEntity)
   {
       if (!AccessControlProvider.EntityIsInView(conflictEntity))
       {
           Logger.LogWarning("CreateAsync: {id} statusCode=400 conflict not in view", entity.Id);
           throw new HttpException(StatusCodes.Status400BadRequest);
       }
   
       throw; // re-throw original 409 with payload
   }

   Files to Change

   File	Change
   TableController.Create.cs	Add try/catch around Repository.CreateAsync with EntityIsInView check
   BaseTest.cs	Modify FakeRepository to support conflict with payload (currently new HttpException(409) has no Payload)

   Testing Impact

   The existing FakeRepository at BaseTest.cs:41 throws new HttpException(409) WITHOUT a payload. The real repositories (InMemory, EF Core, LiteDb, MongoDB) throw with { Payload = existingEntity }. The test helper must be updated to optionally include a payload.

   Existing Test	Action
   CreateAsync_RepositoryException_Throws (line 30)	No change — uses throwConflict = true with no payload, catch only matches Payload is TEntity so this path is unaffected
   Create_ExistingId_Returns409 (integration, line 41)	No change — uses default AccessControlProvider (no data view)
   Create_SoftDeleted_Returns409 (integration, line 56)	No change — uses default AccessControlProvider

   New unit tests needed:

   New Test	Purpose
   CreateAsync_Conflict_EntityInView_Returns409	ACP with data view that INCLUDES the conflicting entity → 409 with payload (existing behavior preserved)
   CreateAsync_Conflict_EntityNotInView_Returns400	ACP with data view that EXCLUDES the conflicting entity → 400 with no payload

   New integration tests needed:

   New Test	Purpose
   Create_ExistingId_WithACP_InView_Returns409	Full pipeline: ACP allows view → 409
   Create_ExistingId_WithACP_NotInView_Returns400	Full pipeline: ACP hides entity → 400

   The test helper FakeRepository needs a new overload or parameter to include a payload entity on conflict:

   // BaseTest.cs FakeRepository, when throwConflict == true and conflictEntity is provided:
   mock.CreateAsync(...).Returns(ValueTask.FromException(
       new HttpException(409) { Payload = conflictEntity }));
2. adrianhall

   added this to the 10.0.1 milestone on Mar 15, 2026
3. adrianhall

   added

   ServerImprovements or additions to the server codeImprovements or additions to the server code

   on Mar 15, 2026
4. adrianhall

   added the

   Task

   issue type on Mar 15, 2026
5. adrianhall

   modified the milestones: 10.0.1, 10.0.2 on May 30, 2026
6. adrianhall

   added a commit that references this issue on Jun 15, 2026

   fix: hide conflicting record on create when not in IAccessControlProv…

   ...

   a02ea92
7. adrianhall

   closed this as completedin #471on Jun 15, 2026