Skip to content

Commit 80b18e6

Browse files
committed
addressing comments, deep copy
1 parent 04819a6 commit 80b18e6

3 files changed

Lines changed: 41 additions & 4 deletions

File tree

src/Auth/AuthorizationMetadataHelpers.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,23 @@ public class RoleMetadata
5555
/// Given the key (operation) returns the associated OperationMetadata object.
5656
/// </summary>
5757
public Dictionary<EntityActionOperation, OperationMetadata> OperationToColumnMap { get; set; } = new();
58+
59+
/// <summary>
60+
/// Creates a deep clone of this RoleMetadata instance so that mutations
61+
/// to the clone do not affect the original (and vice versa).
62+
/// This is critical when copying permissions from one role to another
63+
/// (e.g., anonymous → authenticated) to prevent shared mutable state.
64+
/// </summary>
65+
public RoleMetadata DeepClone()
66+
{
67+
RoleMetadata clone = new();
68+
foreach ((EntityActionOperation operation, OperationMetadata metadata) in OperationToColumnMap)
69+
{
70+
clone.OperationToColumnMap[operation] = metadata.DeepClone();
71+
}
72+
73+
return clone;
74+
}
5875
}
5976

6077
/// <summary>
@@ -68,4 +85,19 @@ public class OperationMetadata
6885
public HashSet<string> Included { get; set; } = new();
6986
public HashSet<string> Excluded { get; set; } = new();
7087
public HashSet<string> AllowedExposedColumns { get; set; } = new();
88+
89+
/// <summary>
90+
/// Creates a deep clone of this OperationMetadata instance so that
91+
/// mutations to the clone do not affect the original (and vice versa).
92+
/// </summary>
93+
public OperationMetadata DeepClone()
94+
{
95+
return new OperationMetadata
96+
{
97+
DatabasePolicy = DatabasePolicy,
98+
Included = new HashSet<string>(Included),
99+
Excluded = new HashSet<string>(Excluded),
100+
AllowedExposedColumns = new HashSet<string>(AllowedExposedColumns)
101+
};
102+
}
71103
}

src/Core/Authorization/AuthorizationResolver.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,8 @@ private void SetEntityPermissionMap(RuntimeConfig runtimeConfig)
395395
/// <summary>
396396
/// Helper method to copy over permissions from anonymous role to authenticated role in the case
397397
/// when anonymous role is defined for an entity in the config but authenticated role is not.
398+
/// Uses deep cloning to ensure the authenticated role's RoleMetadata is a separate instance
399+
/// from anonymous, preventing shared mutable state between the two roles.
398400
/// </summary>
399401
/// <param name="entityToRoleMap">The EntityMetadata for the entity for which we want to copy permissions
400402
/// from anonymous to authenticated role.</param>
@@ -403,9 +405,10 @@ private static void CopyOverPermissionsFromAnonymousToAuthenticatedRole(
403405
EntityMetadata entityToRoleMap,
404406
HashSet<string> allowedColumnsForAnonymousRole)
405407
{
406-
// Using assignment operator overrides the existing value for the key /
407-
// adds a new entry for (key,value) pair if absent, to the map.
408-
entityToRoleMap.RoleToOperationMap[ROLE_AUTHENTICATED] = entityToRoleMap.RoleToOperationMap[ROLE_ANONYMOUS];
408+
// Deep clone the RoleMetadata so that anonymous and authenticated roles
409+
// do not share mutable OperationMetadata instances. Without deep cloning,
410+
// any future mutation of one role's permissions would silently affect the other.
411+
entityToRoleMap.RoleToOperationMap[ROLE_AUTHENTICATED] = entityToRoleMap.RoleToOperationMap[ROLE_ANONYMOUS].DeepClone();
409412

410413
// Copy over OperationToRolesMap for authenticated role from anonymous role.
411414
Dictionary<EntityActionOperation, OperationMetadata> allowedOperationMap =

src/Service.Tests/UnitTests/RequestParserUnitTests.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4+
#nullable enable
5+
46
using Azure.DataApiBuilder.Core.Parsers;
57
using Microsoft.VisualStudio.TestTools.UnitTesting;
68

@@ -52,7 +54,7 @@ public void ExtractRawQueryParameter_PreservesEncoding(string queryString, strin
5254
public void ExtractRawQueryParameter_ReturnsNull_WhenParameterNotFound(string? queryString, string parameterName)
5355
{
5456
// Call the internal method directly (no reflection needed)
55-
string? result = RequestParser.ExtractRawQueryParameter(queryString, parameterName);
57+
string? result = RequestParser.ExtractRawQueryParameter(queryString!, parameterName);
5658

5759
Assert.IsNull(result,
5860
$"Expected null but got '{result}' for parameter '{parameterName}' in query '{queryString}'");

0 commit comments

Comments
 (0)