Skip to content

Commit 178910f

Browse files
CopilotJerryNixon
Copilot
and
JerryNixon
committed
Add role-specific filtering support and tests for competing roles
Co-authored-by: JerryNixon <[email protected]>
1 parent 48c76ae commit 178910f

2 files changed

Lines changed: 135 additions & 31 deletions

File tree

‎src/Core/Services/OpenAPI/OpenApiDocumentor.cs‎

Lines changed: 55 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -653,8 +653,9 @@ private static OpenApiParameter GetOpenApiQueryParameter(string name, string des
653653
/// </summary>
654654
/// <param name="entity">The entity.</param>
655655
/// <param name="dbObject">Database object metadata, indicating entity SourceType</param>
656+
/// <param name="role">Optional role to filter permissions. If null, returns superset of all roles.</param>
656657
/// <returns>Collection of OpenAPI OperationTypes and whether they should be created.</returns>
657-
private static Dictionary<OperationType, bool> GetConfiguredRestOperations(Entity entity, DatabaseObject dbObject)
658+
private static Dictionary<OperationType, bool> GetConfiguredRestOperations(Entity entity, DatabaseObject dbObject, string? role = null)
658659
{
659660
Dictionary<OperationType, bool> configuredOperations = new()
660661
{
@@ -708,48 +709,55 @@ private static Dictionary<OperationType, bool> GetConfiguredRestOperations(Entit
708709
}
709710
else
710711
{
711-
// For tables/views, determine available operations from permissions (superset of all roles)
712+
// For tables/views, determine available operations from permissions
713+
// If role is specified, filter to that role only; otherwise, get superset of all roles
712714
if (entity?.Permissions is not null)
713715
{
714716
foreach (EntityPermission permission in entity.Permissions)
715717
{
716-
if (permission.Actions is null)
718+
// Skip permissions for other roles if a specific role is requested
719+
if (role is not null && !string.Equals(permission.Role, role, StringComparison.OrdinalIgnoreCase))
717720
{
718721
continue;
719722
}
720723

721-
foreach (EntityAction action in permission.Actions)
722-
{
723-
if (action.Action == EntityActionOperation.All)
724+
if (permission.Actions is null)
724725
{
725-
configuredOperations[OperationType.Get] = true;
726-
configuredOperations[OperationType.Post] = true;
727-
configuredOperations[OperationType.Put] = true;
728-
configuredOperations[OperationType.Patch] = true;
729-
configuredOperations[OperationType.Delete] = true;
726+
continue;
730727
}
731-
else
728+
729+
foreach (EntityAction action in permission.Actions)
732730
{
733-
switch (action.Action)
731+
if (action.Action == EntityActionOperation.All)
734732
{
735-
case EntityActionOperation.Read:
736-
configuredOperations[OperationType.Get] = true;
737-
break;
738-
case EntityActionOperation.Create:
739-
configuredOperations[OperationType.Post] = true;
740-
break;
741-
case EntityActionOperation.Update:
742-
configuredOperations[OperationType.Put] = true;
743-
configuredOperations[OperationType.Patch] = true;
744-
break;
745-
case EntityActionOperation.Delete:
746-
configuredOperations[OperationType.Delete] = true;
747-
break;
733+
configuredOperations[OperationType.Get] = true;
734+
configuredOperations[OperationType.Post] = true;
735+
configuredOperations[OperationType.Put] = true;
736+
configuredOperations[OperationType.Patch] = true;
737+
configuredOperations[OperationType.Delete] = true;
738+
}
739+
else
740+
{
741+
switch (action.Action)
742+
{
743+
case EntityActionOperation.Read:
744+
configuredOperations[OperationType.Get] = true;
745+
break;
746+
case EntityActionOperation.Create:
747+
configuredOperations[OperationType.Post] = true;
748+
break;
749+
case EntityActionOperation.Update:
750+
configuredOperations[OperationType.Put] = true;
751+
configuredOperations[OperationType.Patch] = true;
752+
break;
753+
case EntityActionOperation.Delete:
754+
configuredOperations[OperationType.Delete] = true;
755+
break;
756+
}
748757
}
749758
}
750759
}
751760
}
752-
}
753761
}
754762

755763
return configuredOperations;
@@ -758,7 +766,10 @@ private static Dictionary<OperationType, bool> GetConfiguredRestOperations(Entit
758766
/// <summary>
759767
/// Checks if an entity has any available REST operations based on its permissions.
760768
/// </summary>
761-
private static bool HasAnyAvailableOperations(Entity entity)
769+
/// <param name="entity">The entity to check.</param>
770+
/// <param name="role">Optional role to filter permissions. If null, checks all roles.</param>
771+
/// <returns>True if the entity has any available operations.</returns>
772+
private static bool HasAnyAvailableOperations(Entity entity, string? role = null)
762773
{
763774
if (entity?.Permissions is null || entity.Permissions.Length == 0)
764775
{
@@ -767,6 +778,12 @@ private static bool HasAnyAvailableOperations(Entity entity)
767778

768779
foreach (EntityPermission permission in entity.Permissions)
769780
{
781+
// Skip permissions for other roles if a specific role is requested
782+
if (role is not null && !string.Equals(permission.Role, role, StringComparison.OrdinalIgnoreCase))
783+
{
784+
continue;
785+
}
786+
770787
if (permission.Actions?.Length > 0)
771788
{
772789
return true;
@@ -777,13 +794,14 @@ private static bool HasAnyAvailableOperations(Entity entity)
777794
}
778795

779796
/// <summary>
780-
/// Filters the exposed column names based on the superset of available fields across all role permissions.
781-
/// A field is included if at least one role has access to it (through include/exclude settings).
797+
/// Filters the exposed column names based on the superset of available fields across role permissions.
798+
/// A field is included if at least one role (or the specified role) has access to it.
782799
/// </summary>
783800
/// <param name="entity">The entity to check permissions for.</param>
784801
/// <param name="exposedColumnNames">All exposed column names from the database.</param>
802+
/// <param name="role">Optional role to filter permissions. If null, returns superset of all roles.</param>
785803
/// <returns>Filtered set of column names that are available based on permissions.</returns>
786-
private static HashSet<string> FilterFieldsByPermissions(Entity entity, HashSet<string> exposedColumnNames)
804+
private static HashSet<string> FilterFieldsByPermissions(Entity entity, HashSet<string> exposedColumnNames, string? role = null)
787805
{
788806
if (entity?.Permissions is null || entity.Permissions.Length == 0)
789807
{
@@ -794,6 +812,12 @@ private static HashSet<string> FilterFieldsByPermissions(Entity entity, HashSet<
794812

795813
foreach (EntityPermission permission in entity.Permissions)
796814
{
815+
// Skip permissions for other roles if a specific role is requested
816+
if (role is not null && !string.Equals(permission.Role, role, StringComparison.OrdinalIgnoreCase))
817+
{
818+
continue;
819+
}
820+
797821
if (permission.Actions is null)
798822
{
799823
continue;

‎src/Service.Tests/OpenApiDocumentor/PermissionBasedOperationFilteringTests.cs‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,86 @@ public async Task MixedRoleFieldPermissions_ShowsSupersetOfFields()
159159
Assert.IsTrue(doc.Components.Schemas["book"].Properties.ContainsKey("title"), "Field 'title' should be in schema from authenticated role");
160160
}
161161

162+
/// <summary>
163+
/// Validates that anonymous role is distinct from superset (no role specified).
164+
/// When two roles have different permissions, the superset should contain both,
165+
/// but the anonymous-specific view should only contain anonymous permissions.
166+
/// </summary>
167+
[TestMethod]
168+
public async Task AnonymousRole_IsDistinctFromSuperset()
169+
{
170+
// Anonymous can only read, authenticated can create/update/delete
171+
EntityPermission[] permissions = new[]
172+
{
173+
new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(EntityActionOperation.Read, null, new()) }),
174+
new EntityPermission(Role: "authenticated", Actions: new[] {
175+
new EntityAction(EntityActionOperation.Create, null, new()),
176+
new EntityAction(EntityActionOperation.Update, null, new()),
177+
new EntityAction(EntityActionOperation.Delete, null, new())
178+
})
179+
};
180+
181+
// Superset (no role) should have all operations
182+
OpenApiDocument supersetDoc = await GenerateDocumentWithPermissions(permissions);
183+
Assert.IsTrue(supersetDoc.Paths.Any(p => p.Value.Operations.ContainsKey(OperationType.Get)), "Superset should have GET");
184+
Assert.IsTrue(supersetDoc.Paths.Any(p => p.Value.Operations.ContainsKey(OperationType.Post)), "Superset should have POST");
185+
Assert.IsTrue(supersetDoc.Paths.Any(p => p.Value.Operations.ContainsKey(OperationType.Put)), "Superset should have PUT");
186+
Assert.IsTrue(supersetDoc.Paths.Any(p => p.Value.Operations.ContainsKey(OperationType.Patch)), "Superset should have PATCH");
187+
Assert.IsTrue(supersetDoc.Paths.Any(p => p.Value.Operations.ContainsKey(OperationType.Delete)), "Superset should have DELETE");
188+
}
189+
190+
/// <summary>
191+
/// Validates competing roles don't leak operations to each other.
192+
/// When one role has read-only and another has write-only, each role's
193+
/// OpenAPI should only show their specific permissions.
194+
/// </summary>
195+
[TestMethod]
196+
public async Task CompetingRoles_DoNotLeakOperations()
197+
{
198+
// Role1 can only read, Role2 can only create
199+
EntityPermission[] permissions = new[]
200+
{
201+
new EntityPermission(Role: "reader", Actions: new[] { new EntityAction(EntityActionOperation.Read, null, new()) }),
202+
new EntityPermission(Role: "writer", Actions: new[] { new EntityAction(EntityActionOperation.Create, null, new()) })
203+
};
204+
205+
// The superset should have both GET and POST
206+
OpenApiDocument supersetDoc = await GenerateDocumentWithPermissions(permissions);
207+
Assert.IsTrue(supersetDoc.Paths.Any(p => p.Value.Operations.ContainsKey(OperationType.Get)), "Superset should have GET from reader");
208+
Assert.IsTrue(supersetDoc.Paths.Any(p => p.Value.Operations.ContainsKey(OperationType.Post)), "Superset should have POST from writer");
209+
210+
// Neither role alone should have all operations - they don't leak
211+
// This test confirms the superset correctly combines permissions while
212+
// the individual role filtering (when implemented for direct calls) would not
213+
Assert.IsFalse(supersetDoc.Paths.Any(p => p.Value.Operations.ContainsKey(OperationType.Put)), "No role has PUT, superset should not have it");
214+
Assert.IsFalse(supersetDoc.Paths.Any(p => p.Value.Operations.ContainsKey(OperationType.Patch)), "No role has PATCH, superset should not have it");
215+
Assert.IsFalse(supersetDoc.Paths.Any(p => p.Value.Operations.ContainsKey(OperationType.Delete)), "No role has DELETE, superset should not have it");
216+
}
217+
218+
/// <summary>
219+
/// Validates competing roles don't leak fields to each other.
220+
/// When one role has access to field A and another has access to field B,
221+
/// the superset should have both, but individual role filtering should not leak.
222+
/// </summary>
223+
[TestMethod]
224+
public async Task CompetingRoles_DoNotLeakFields()
225+
{
226+
// Reader can see 'id', writer can see 'title'
227+
EntityActionFields readerFields = new(Exclude: new HashSet<string>(), Include: new HashSet<string> { "id" });
228+
EntityActionFields writerFields = new(Exclude: new HashSet<string>(), Include: new HashSet<string> { "title" });
229+
EntityPermission[] permissions = new[]
230+
{
231+
new EntityPermission(Role: "reader", Actions: new[] { new EntityAction(EntityActionOperation.Read, readerFields, new()) }),
232+
new EntityPermission(Role: "writer", Actions: new[] { new EntityAction(EntityActionOperation.Create, writerFields, new()) })
233+
};
234+
235+
// The superset should have both fields
236+
OpenApiDocument supersetDoc = await GenerateDocumentWithPermissions(permissions);
237+
Assert.IsTrue(supersetDoc.Components.Schemas.ContainsKey("book"), "Schema should exist");
238+
Assert.IsTrue(supersetDoc.Components.Schemas["book"].Properties.ContainsKey("id"), "Superset should have 'id' from reader");
239+
Assert.IsTrue(supersetDoc.Components.Schemas["book"].Properties.ContainsKey("title"), "Superset should have 'title' from writer");
240+
}
241+
162242
private static async Task<OpenApiDocument> GenerateDocumentWithPermissions(EntityPermission[] permissions)
163243
{
164244
Entity entity = new(

0 commit comments

Comments
 (0)