fix: ensure crucial security measure is implemented per https://rxdb.info/replication.html#security (#82)

rbeauchamp · web-flow · commit c7d3a6e25b25 · 2024-09-16T07:52:54.000-07:00
diff --git a/README.md b/README.md
@@ -1,16 +1,8 @@
 # RxDBDotNet
 
-<p align="left">
-  <a href="https://www.nuget.org/packages/RxDBDotNet/" style="text-decoration:none;">
-    <img src="https://img.shields.io/nuget/v/RxDBDotNet.svg" alt="NuGet Version" style="margin-right: 10px;">
-  </a>
-  <a href="https://www.nuget.org/packages/RxDBDotNet/" style="text-decoration:none;">
-    <img src="https://img.shields.io/nuget/dt/RxDBDotNet.svg" alt="NuGet Downloads" style="margin-right: 10px;">
-  </a>
-  <a href="https://codecov.io/github/Ziptility/RxDBDotNet" style="text-decoration:none;">
-    <img src="https://codecov.io/github/Ziptility/RxDBDotNet/graph/badge.svg?token=VvuBJEsIHT" alt="codecov">
-  </a>
-</p>
+[![NuGet Version](https://img.shields.io/nuget/v/RxDBDotNet.svg)](https://www.nuget.org/packages/RxDBDotNet/)
+[![NuGet Downloads](https://img.shields.io/nuget/dt/RxDBDotNet.svg)](https://www.nuget.org/packages/RxDBDotNet/)
+[![codecov](https://codecov.io/github/Ziptility/RxDBDotNet/graph/badge.svg?token=VvuBJEsIHT)](https://codecov.io/github/Ziptility/RxDBDotNet)
 
 RxDBDotNet is a powerful .NET library that implements the [RxDB replication protocol](https://rxdb.info/replication.html), enabling real-time data synchronization between RxDB clients and .NET servers using GraphQL and Hot Chocolate. It extends the standard RxDB replication protocol with .NET-specific enhancements.
 
@@ -37,6 +29,7 @@ Ready to dive in? [Get started](#getting-started) or [contribute](#contributing)
 - [Sample Implementation](#sample-implementation)
 - [RxDB Replication Protocol Details](#rxdb-replication-protocol-details)
 - [Advanced Features](#advanced-features)
+- [Security Considerations](#security-considerations)
 - [Contributing](#contributing)
 - [Code of Conduct](#code-of-conduct)
 - [License](#license)
@@ -642,6 +635,20 @@ builder.Services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
 
 This feature allows for more robust and flexible authentication scenarios, particularly in environments where signing keys may change dynamically or where you're integrating with external OIDC providers like IdentityServer.
 
+## Security Considerations
+
+### Server-Side Timestamp Overwriting
+
+RxDBDotNet implements a [crucial security measure](https://rxdb.info/replication.html#security) to prevent potential issues with untrusted client-side clocks. When the server receives a document creation or update request, it always overwrites the `UpdatedAt` timestamp with its own server-side timestamp. This approach ensures that:
+
+1. The integrity of the document's timeline is maintained.
+2. Potential time-based attacks or inconsistencies due to client clock discrepancies are mitigated.
+3. The server maintains authoritative control over the timestamp for all document changes.
+
+This security measure is implemented in the `MutationResolver<TDocument>` class, which handles document push operations. Developers using RxDBDotNet should be aware that any client-provided `UpdatedAt` value will be ignored and replaced with the server's timestamp.
+
+Important: While the `IReplicatedDocument` interface defines `UpdatedAt` with both a getter and a setter, developers should not manually set this property in their application code. Always rely on the server to set the correct `UpdatedAt` value during replication operations. The setter is present solely to allow the server to overwrite the timestamp as a security measure.
+
 ## Contributing
 
 We welcome contributions to RxDBDotNet! Here's how you can contribute:
diff --git a/example/LiveDocs.GraphQLApi/Models/Replication/ReplicatedDocument.cs b/example/LiveDocs.GraphQLApi/Models/Replication/ReplicatedDocument.cs
@@ -10,7 +10,7 @@ namespace LiveDocs.GraphQLApi.Models.Replication;
 public abstract record ReplicatedDocument : IReplicatedDocument
 {
     private readonly List<string>? _topics;
-    private readonly DateTimeOffset _updatedAt;
+    private DateTimeOffset _updatedAt;
 
     /// <inheritdoc />
     [Required]
@@ -27,7 +27,7 @@ public abstract record ReplicatedDocument : IReplicatedDocument
     public required DateTimeOffset UpdatedAt
     {
         get => _updatedAt;
-        init =>
+        set =>
             // Strip microseconds by setting the ticks to zero, keeping only up to milliseconds.
             // Doing this because microseconds are not supported by Hot Chocolate's DateTime serializer.
             // Now Equals() and GetHashCode() will work correctly.
diff --git a/src/RxDBDotNet/Documents/IReplicatedDocument.cs b/src/RxDBDotNet/Documents/IReplicatedDocument.cs
@@ -23,8 +23,9 @@ public interface IReplicatedDocument
     /// <remarks>
     /// This property is crucial for conflict resolution and determining the most recent version of a document.
     /// It should be updated every time the document is modified.
+    /// The server will always overwrite this value with its own timestamp to ensure security and consistency.
     /// </remarks>
-    DateTimeOffset UpdatedAt { get; }
+    DateTimeOffset UpdatedAt { get; set; }
 
     /// <summary>
     /// A value indicating whether the document has been marked as deleted.
diff --git a/src/RxDBDotNet/Resolvers/MutationResolver.cs b/src/RxDBDotNet/Resolvers/MutationResolver.cs
@@ -198,6 +198,10 @@ private static async Task<List<TDocument>> ApplyChangesAsync(
             await AuthorizeOperationAsync(authorizationHelper, currentUser, securityOptions, Operation.Create)
                 .ConfigureAwait(false);
 
+            // Set the server timestamp to ensure data integrity and security
+            // This overrides any client-provided timestamp, as client-side clocks cannot be trusted
+            // It's crucial for maintaining a reliable timeline of document changes and preventing potential exploits
+            create.UpdatedAt = DateTimeOffset.UtcNow;
             await documentService.CreateDocumentAsync(create, cancellationToken)
                 .ConfigureAwait(false);
         }
@@ -236,6 +240,13 @@ private static async Task HandleDocumentUpdateAsync(
         SecurityOptions<TDocument>? securityOptions,
         CancellationToken cancellationToken)
     {
+        // Set the server timestamp for updates
+        // This is a critical security measure that ensures:
+        // 1. The integrity of the document's timeline is maintained
+        // 2. Potential time-based attacks or inconsistencies due to client clock discrepancies are mitigated
+        // 3. The server has the authoritative timestamp for all document changes
+        update.UpdatedAt = DateTimeOffset.UtcNow;
+
         if (update.IsDeleted)
         {
             await AuthorizeOperationAsync(authorizationHelper, currentUser, securityOptions, Operation.Delete)
diff --git a/tests/RxDBDotNet.Tests/AdditionalAuthorizationTests.cs b/tests/RxDBDotNet.Tests/AdditionalAuthorizationTests.cs
@@ -204,7 +204,7 @@ public async Task SystemAdmin_ShouldHaveFullAccessToWorkspace()
         var systemAdmin = await TestContext.CreateUserAsync(workspace, UserRole.SystemAdmin, TestContext.CancellationToken);
 
         // Act & Assert - Create
-        var newWorkspace = new WorkspaceInputGql
+        var newWorkspaceInput = new WorkspaceInputGql
         {
             Id = Provider.Sql.Create(),
             Name = Strings.CreateString(),
@@ -220,7 +220,7 @@ public async Task SystemAdmin_ShouldHaveFullAccessToWorkspace()
         var createWorkspaceInputPushRowGql = new WorkspaceInputPushRowGql
         {
             AssumedMasterState = null,
-            NewDocumentState = newWorkspace,
+            NewDocumentState = newWorkspaceInput,
         };
 
         var createWorkspaceInputGql = new PushWorkspaceInputGql
@@ -253,22 +253,24 @@ public async Task SystemAdmin_ShouldHaveFullAccessToWorkspace()
         readResponse.Data.PullWorkspace.Should()
             .NotBeNull();
         readResponse.Data.PullWorkspace?.Documents.Should()
-            .Contain(w => w.Name == newWorkspace.Name.Value);
+            .Contain(w => w.Name == newWorkspaceInput.Name.Value);
+
+        var newWorkspace = await TestContext.HttpClient.GetWorkspaceByIdAsync(newWorkspaceInput.Id, TestContext.CancellationToken, systemAdmin.JwtAccessToken);
 
         // Act & Assert - Update
-        var updatedWorkspace = new WorkspaceInputGql
+        var updatedWorkspaceInput = new WorkspaceInputGql
         {
-            Id = newWorkspace.Id,
+            Id = newWorkspaceInput.Id,
             Name = Strings.CreateString(),
-            IsDeleted = newWorkspace.IsDeleted,
+            IsDeleted = newWorkspaceInput.IsDeleted,
             UpdatedAt = DateTimeOffset.UtcNow,
-            Topics = newWorkspace.Topics,
+            Topics = newWorkspaceInput.Topics,
         };
 
         var updateWorkspaceInputPushRowGql = new WorkspaceInputPushRowGql
         {
-            AssumedMasterState = newWorkspace,
-            NewDocumentState = updatedWorkspace,
+            AssumedMasterState = newWorkspace.ToWorkspaceInputGql(),
+            NewDocumentState = updatedWorkspaceInput,
         };
 
         var updateWorkspaceInputGql = new PushWorkspaceInputGql
@@ -293,18 +295,20 @@ public async Task SystemAdmin_ShouldHaveFullAccessToWorkspace()
             .BeNullOrEmpty();
 
         // Act & Assert - Delete
+        var updatedWorkspace = await TestContext.HttpClient.GetWorkspaceByIdAsync(newWorkspaceInput.Id, TestContext.CancellationToken, systemAdmin.JwtAccessToken);
+
         var deleteWorkspace = new WorkspaceInputGql
         {
-            Id = updatedWorkspace.Id,
-            Name = updatedWorkspace.Name,
+            Id = updatedWorkspaceInput.Id,
+            Name = updatedWorkspaceInput.Name,
             IsDeleted = true,
             UpdatedAt = DateTimeOffset.UtcNow,
-            Topics = updatedWorkspace.Topics,
+            Topics = updatedWorkspaceInput.Topics,
         };
 
         var deleteWorkspaceInputPushRowGql = new WorkspaceInputPushRowGql
         {
-            AssumedMasterState = updatedWorkspace,
+            AssumedMasterState = updatedWorkspace.ToWorkspaceInputGql(),
             NewDocumentState = deleteWorkspace,
         };
 
diff --git a/tests/RxDBDotNet.Tests/PushDocumentsTests.cs b/tests/RxDBDotNet.Tests/PushDocumentsTests.cs
@@ -18,6 +18,64 @@ public async Task DisposeAsync()
         await TestContext.DisposeAsync();
     }
 
+    [Fact]
+    public async Task PushDocuments_ShouldOverwriteClientProvidedTimestamp()
+    {
+        // Arrange
+        TestContext = new TestScenarioBuilder().Build();
+        var workspaceId = Provider.Sql.Create();
+        var clientProvidedTimestamp = DateTimeOffset.UtcNow.AddDays(-1); // Simulate an old timestamp
+
+        var newWorkspace = new WorkspaceInputGql
+        {
+            Id = workspaceId,
+            Name = Strings.CreateString(),
+            UpdatedAt = clientProvidedTimestamp, // Use the old timestamp
+            IsDeleted = false,
+            Topics = new List<string>
+            {
+                workspaceId.ToString(),
+            },
+        };
+
+        var workspaceInputPushRowGql = new WorkspaceInputPushRowGql
+        {
+            AssumedMasterState = null,
+            NewDocumentState = newWorkspace,
+        };
+
+        var pushWorkspaceInputGql = new PushWorkspaceInputGql
+        {
+            WorkspacePushRow = new List<WorkspaceInputPushRowGql?>
+            {
+                workspaceInputPushRowGql,
+            },
+        };
+
+        var createWorkspace =
+            new MutationQueryBuilderGql().WithPushWorkspace(new PushWorkspacePayloadQueryBuilderGql().WithAllFields(), pushWorkspaceInputGql);
+
+        // Act
+        var pushResponse = await TestContext.HttpClient.PostGqlMutationAsync(createWorkspace, TestContext.CancellationToken);
+
+        // Assert push response
+        pushResponse.Errors.Should().BeNullOrEmpty();
+        pushResponse.Data.PushWorkspace?.Errors.Should().BeNullOrEmpty();
+        pushResponse.Data.PushWorkspace?.Workspace.Should().BeNullOrEmpty();
+
+        // Verify the created workspace
+        var createdWorkspace = await TestContext.HttpClient.GetWorkspaceByIdAsync(workspaceId, TestContext.CancellationToken);
+
+        createdWorkspace.Should().NotBeNull();
+        createdWorkspace.Id.Should().Be(workspaceId);
+        createdWorkspace.Name.Should().Be(newWorkspace.Name.Value);
+        createdWorkspace.IsDeleted.Should().Be(newWorkspace.IsDeleted);
+
+        // Check if the timestamp was overwritten by the server
+        createdWorkspace.UpdatedAt.Should().BeAfter(clientProvidedTimestamp);
+        createdWorkspace.UpdatedAt.Should().BeCloseTo(DateTimeOffset.UtcNow, TimeSpan.FromSeconds(1));
+    }
+
     [Fact]
     public async Task PushDocuments_WithNullDocumentList_ShouldReturnEmptyResult()
     {
diff --git a/tests/RxDBDotNet.Tests/SecurityTests.cs b/tests/RxDBDotNet.Tests/SecurityTests.cs
@@ -1,5 +1,6 @@
 ﻿using RxDBDotNet.Tests.Model;
 using RxDBDotNet.Tests.Utils;
+using static RxDBDotNet.Tests.Setup.Strings;
 
 namespace RxDBDotNet.Tests;
 
@@ -28,13 +29,45 @@ public async Task AWorkspaceAdminShouldBeAbleToCreateAWorkspace()
             .Build();
 
         var workspace = await TestContext.CreateWorkspaceAsync(TestContext.CancellationToken);
-        var admin = await TestContext.CreateUserAsync(workspace, UserRole.WorkspaceAdmin, TestContext.CancellationToken);
+        var workspaceAdmin = await TestContext.CreateUserAsync(workspace, UserRole.WorkspaceAdmin, TestContext.CancellationToken);
+
+        var workspaceId = Provider.Sql.Create();
+
+        var workspaceInputGql = new WorkspaceInputGql
+        {
+            Id = workspaceId,
+            Name = CreateString(),
+            UpdatedAt = DateTimeOffset.UtcNow,
+            IsDeleted = false,
+            Topics = new List<string>
+            {
+                workspaceId.ToString(),
+            },
+        };
+
+        var workspaceInputPushRowGql = new WorkspaceInputPushRowGql
+        {
+            AssumedMasterState = null,
+            NewDocumentState = workspaceInputGql,
+        };
+
+        var pushWorkspaceInputGql = new PushWorkspaceInputGql
+        {
+            WorkspacePushRow = new List<WorkspaceInputPushRowGql?>
+            {
+                workspaceInputPushRowGql,
+            },
+        };
+
+        var createWorkspace = new MutationQueryBuilderGql().WithPushWorkspace(new PushWorkspacePayloadQueryBuilderGql().WithAllFields()
+            .WithErrors(new PushWorkspaceErrorQueryBuilderGql().WithAuthenticationErrorFragment(
+                new AuthenticationErrorQueryBuilderGql().WithAllFields())), pushWorkspaceInputGql);
 
         // Act
-        var response = await TestContext.HttpClient.CreateWorkspaceAsync(TestContext.CancellationToken, admin.JwtAccessToken);
+        await TestContext.HttpClient.PostGqlMutationAsync(createWorkspace, TestContext.CancellationToken, workspaceAdmin.JwtAccessToken);
 
         // Assert
-        await TestContext.HttpClient.VerifyWorkspaceAsync(response.workspaceInputGql, TestContext.CancellationToken);
+        await TestContext.HttpClient.VerifyWorkspaceAsync(workspaceInputGql, TestContext.CancellationToken);
     }
 
     [Fact]
@@ -47,10 +80,46 @@ public async Task AStandardUserShouldNotBeAbleToCreateAWorkspace()
             .Build();
 
         var workspace = await TestContext.CreateWorkspaceAsync(TestContext.CancellationToken);
+
         var standardUser = await TestContext.CreateUserAsync(workspace, UserRole.StandardUser, TestContext.CancellationToken);
 
+        var workspaceId = Provider.Sql.Create();
+
+        var newWorkspace = new WorkspaceInputGql
+        {
+            Id = workspaceId,
+            Name = CreateString(),
+            UpdatedAt = DateTimeOffset.UtcNow,
+            IsDeleted = false,
+            Topics = new List<string>
+            {
+                workspaceId.ToString(),
+            },
+        };
+
+        var workspaceInputPushRowGql = new WorkspaceInputPushRowGql
+        {
+            AssumedMasterState = null,
+            NewDocumentState = newWorkspace,
+        };
+
+        var pushWorkspaceInputGql = new PushWorkspaceInputGql
+        {
+            WorkspacePushRow = new List<WorkspaceInputPushRowGql?>
+            {
+                workspaceInputPushRowGql,
+            },
+        };
+
+        var createWorkspace = new MutationQueryBuilderGql().WithPushWorkspace(new PushWorkspacePayloadQueryBuilderGql().WithAllFields()
+            .WithErrors(new PushWorkspaceErrorQueryBuilderGql().WithAuthenticationErrorFragment(
+                new AuthenticationErrorQueryBuilderGql().WithAllFields())), pushWorkspaceInputGql);
+
         // Act
-        var (_, response) = await TestContext.HttpClient.CreateWorkspaceAsync(TestContext.CancellationToken, standardUser.JwtAccessToken);
+        var response = await TestContext.HttpClient.PostGqlMutationAsync(
+            createWorkspace,
+            TestContext.CancellationToken,
+            standardUser.JwtAccessToken);
 
         // Assert
         response.Data.PushWorkspace?.Workspace.Should()
@@ -123,7 +192,7 @@ public async Task AWorkspaceAdminShouldBeAbleToUpdateAWorkspace()
             .Build();
 
         var workspace = await TestContext.CreateWorkspaceAsync(TestContext.CancellationToken);
-        var admin = await TestContext.CreateUserAsync(workspace, UserRole.WorkspaceAdmin, TestContext.CancellationToken);
+        var workspaceAdmin = await TestContext.CreateUserAsync(workspace, UserRole.WorkspaceAdmin, TestContext.CancellationToken);
 
         // Act
         var workspaceToUpdate = new WorkspaceInputGql
@@ -161,15 +230,19 @@ public async Task AWorkspaceAdminShouldBeAbleToUpdateAWorkspace()
         var updateWorkspace =
             new MutationQueryBuilderGql().WithPushWorkspace(new PushWorkspacePayloadQueryBuilderGql().WithAllFields(), pushWorkspaceInputGql);
 
-        var response = await TestContext.HttpClient.PostGqlMutationAsync(updateWorkspace, TestContext.CancellationToken, admin.JwtAccessToken);
+        var response = await TestContext.HttpClient.PostGqlMutationAsync(updateWorkspace, TestContext.CancellationToken, workspaceAdmin.JwtAccessToken);
 
         // Assert
         response.Errors.Should()
             .BeNullOrEmpty();
         response.Data.PushWorkspace?.Workspace.Should()
             .BeNullOrEmpty();
 
-        var updatedWorkspace = await TestContext.HttpClient.GetWorkspaceByIdAsync(workspace.ReplicatedDocumentId, TestContext.CancellationToken);
+        var updatedWorkspace = await TestContext.HttpClient.GetWorkspaceByIdAsync(
+            workspace.ReplicatedDocumentId,
+            TestContext.CancellationToken,
+            workspaceAdmin.JwtAccessToken);
+
         updatedWorkspace.Name.Should()
             .Be(workspaceToUpdate.Name.Value);
     }
@@ -290,7 +363,11 @@ public async Task ASystemAdminShouldBeAbleToDeleteAWorkspace()
         response.Data.PushWorkspace?.Workspace.Should()
             .BeNullOrEmpty();
 
-        var deletedWorkspace = await TestContext.HttpClient.GetWorkspaceByIdAsync(workspace.ReplicatedDocumentId, TestContext.CancellationToken);
+        var deletedWorkspace = await TestContext.HttpClient.GetWorkspaceByIdAsync(
+            workspace.ReplicatedDocumentId,
+            TestContext.CancellationToken,
+            systemAdmin.JwtAccessToken);
+
         deletedWorkspace.IsDeleted.Should()
             .BeTrue();
     }
diff --git a/tests/RxDBDotNet.Tests/SubscriptionTests.cs b/tests/RxDBDotNet.Tests/SubscriptionTests.cs
diff --git a/tests/RxDBDotNet.Tests/Utils/TestUtils.cs b/tests/RxDBDotNet.Tests/Utils/TestUtils.cs
