diff --git a/src/SelfService.Tests/Application/TestRbacApplicationService.cs b/src/SelfService.Tests/Application/TestRbacApplicationService.cs index 143e0de7..32b8155d 100644 --- a/src/SelfService.Tests/Application/TestRbacApplicationService.cs +++ b/src/SelfService.Tests/Application/TestRbacApplicationService.cs @@ -37,6 +37,34 @@ public static async Task NewInMemoryFixture( var fixture = new RbacInMemoryTestFixture(databaseFactory, dbContext, application); return fixture; } + + public static async Task SeedGuestRole( + SelfServiceDbContext dbContext, + params (RbacNamespace Namespace, string Name, RbacAccessType Type)[] permissions + ) + { + var guest = RbacRole.New("system", "Guest", "Role: Guest", RbacAccessType.Global); + dbContext.RbacRoles.Add(guest); + + foreach (var p in permissions) + { + dbContext.RbacPermissionGrants.Add( + new RbacPermissionGrant( + id: RbacPermissionGrantId.New(), + createdAt: DateTime.Now, + assignedEntityType: AssignedEntityType.Role, + assignedEntityId: guest.Id.ToString(), + @namespace: p.Namespace, + permission: p.Name, + type: p.Type, + resource: "" + ) + ); + } + + await dbContext.SaveChangesAsync(); + return guest.Id; + } } public class RbacInMemoryTestFixture @@ -741,12 +769,8 @@ await rbacSvc.IsUserPermitted( ).Permitted() ); - /* - Reading public topics is allowed for everyone - This is currently handled outside of RBAC in the application logic - [04-11-2025, andfris] Leaving this test here as a reminder - */ - /* + // Reading public topics is allowed for everyone. This now comes from RBAC itself: topics/read-public + // is one of the Guest role's grants, and Guest is the baseline every user holds implicitly. Assert.True( ( await rbacSvc.IsUserPermitted( @@ -756,7 +780,6 @@ await rbacSvc.IsUserPermitted( ) ).Permitted() ); - */ } private static RbacPermissionGrant UserGrant(RbacAccessType type, string resource) => @@ -893,4 +916,201 @@ await rbacSvc.IsUserPermitted("admin@dfds.cloud", [RbacCreate(RbacAccessType.Cap ).Permitted() ); } + + // --------------------------------------------------------------------------------------------- + // Guest baseline + // --------------------------------------------------------------------------------------------- + + private const string Nobody = "nobody@dfds.cloud"; + + private static Permission CatalogueRead(RbacAccessType accessType) => + new() + { + Namespace = RbacNamespace.ServiceCatalogue, + Name = "read", + AccessType = accessType, + }; + + private static Permission TopicsReadPublic(RbacAccessType accessType) => + new() + { + Namespace = RbacNamespace.Topics, + Name = "read-public", + AccessType = accessType, + }; + + private static async Task EmptyFixtureWithGuest( + params (RbacNamespace Namespace, string Name, RbacAccessType Type)[] guestPermissions + ) + { + var fixture = await RbacTestData.NewInMemoryFixture( + true, + new List(), + new List(), + new List() + ); + await RbacTestData.SeedGuestRole(fixture.DbContext, guestPermissions); + return fixture.ApiApplication.Services.GetService()!; + } + + [Fact] + public async Task GuestBaselineSatisfiesGlobalScopedChecks() + { + var rbacSvc = await EmptyFixtureWithGuest((RbacNamespace.ServiceCatalogue, "read", RbacAccessType.Global)); + + // A Global-scoped controller has no {id} route value, so AuthChecker passes a null objectId. + Assert.True((await rbacSvc.IsUserPermitted(Nobody, [CatalogueRead(RbacAccessType.Global)], null!)).Permitted()); + Assert.True((await rbacSvc.IsUserPermitted(Nobody, [CatalogueRead(RbacAccessType.Global)], "")).Permitted()); + } + + [Fact] + public async Task GuestBaselineAppliesToAnyCapability() + { + var rbacSvc = await EmptyFixtureWithGuest((RbacNamespace.Topics, "read-public", RbacAccessType.Capability)); + + Assert.True( + (await rbacSvc.IsUserPermitted(Nobody, [TopicsReadPublic(RbacAccessType.Capability)], "cap-a")).Permitted() + ); + Assert.True( + (await rbacSvc.IsUserPermitted(Nobody, [TopicsReadPublic(RbacAccessType.Capability)], "cap-b")).Permitted() + ); + } + + [Fact] + public async Task GuestBaselineStillAppliesWhenUserHoldsACapabilityRole() + { + var readerRoleId = RbacRoleId.New(); + + var fixture = await RbacTestData.NewInMemoryFixture( + true, + new List + { + new( + id: RbacPermissionGrantId.New(), + createdAt: DateTime.Now, + assignedEntityType: AssignedEntityType.Role, + assignedEntityId: readerRoleId.ToString(), + @namespace: RbacNamespace.Topics, + permission: "read-private", + type: RbacAccessType.Global, + resource: "" + ), + }, + new List + { + new( + id: RbacRoleGrantId.New(), + roleId: readerRoleId, + createdAt: DateTime.Now, + assignedEntityType: AssignedEntityType.User, + assignedEntityId: "reader@dfds.cloud", + type: RbacAccessType.Capability, + resource: "bar" + ), + }, + new List() + ); + await RbacTestData.SeedGuestRole( + fixture.DbContext, + (RbacNamespace.Topics, "read-public", RbacAccessType.Capability) + ); + var rbacSvc = fixture.ApiApplication.Services.GetService()!; + + Assert.True( + ( + await rbacSvc.IsUserPermitted( + "reader@dfds.cloud", + [ + new Permission + { + Namespace = RbacNamespace.Topics, + Name = "read-private", + AccessType = RbacAccessType.Capability, + }, + ], + "bar" + ) + ).Permitted() + ); + + Assert.True( + ( + await rbacSvc.IsUserPermitted("reader@dfds.cloud", [TopicsReadPublic(RbacAccessType.Capability)], "bar") + ).Permitted() + ); + } + + [Fact] + public async Task GuestBaselineIsAdditiveNeverSubtractive() + { + var fixture = await RbacTestData.NewInMemoryFixture( + true, + new List { UserGrant(RbacAccessType.Global, "") }, + new List(), + new List() + ); + await RbacTestData.SeedGuestRole( + fixture.DbContext, + (RbacNamespace.Topics, "read-public", RbacAccessType.Capability) + ); + var rbacSvc = fixture.ApiApplication.Services.GetService()!; + + var own = await rbacSvc.IsUserPermitted("test01@dfds.cloud", [RbacCreate(RbacAccessType.Global)], "test01"); + Assert.True(own.Permitted()); + Assert.NotEmpty(own.PermissionGrants); + + var baseline = await rbacSvc.IsUserPermitted( + "test01@dfds.cloud", + [TopicsReadPublic(RbacAccessType.Capability)], + "test01" + ); + Assert.True(baseline.Permitted()); + Assert.NotEmpty(baseline.PermissionGrants); + } + + [Fact] + public async Task CapabilityScopedGuestGrantDoesNotSatisfyGlobalCheck() + { + var rbacSvc = await EmptyFixtureWithGuest((RbacNamespace.Topics, "read-public", RbacAccessType.Capability)); + + Assert.False( + (await rbacSvc.IsUserPermitted(Nobody, [TopicsReadPublic(RbacAccessType.Global)], "cap-a")).Permitted() + ); + } + + [Fact] + public async Task GuestGrantScopeIgnoresTheStoredTypeColumn() + { + // what Guest reaches. + var storedGlobal = await EmptyFixtureWithGuest((RbacNamespace.ServiceCatalogue, "read", RbacAccessType.Global)); + var storedCapability = await EmptyFixtureWithGuest( + (RbacNamespace.ServiceCatalogue, "read", RbacAccessType.Capability) + ); + + Assert.True( + (await storedGlobal.IsUserPermitted(Nobody, [CatalogueRead(RbacAccessType.Global)], null!)).Permitted() + ); + Assert.True( + (await storedCapability.IsUserPermitted(Nobody, [CatalogueRead(RbacAccessType.Global)], null!)).Permitted() + ); + } + + [Fact] + public async Task NoGuestRoleMeansNoBaseline() + { + var fixture = await RbacTestData.NewInMemoryFixture( + true, + new List(), + new List(), + new List() + ); + var rbacSvc = fixture.ApiApplication.Services.GetService()!; + + Assert.False( + (await rbacSvc.IsUserPermitted(Nobody, [CatalogueRead(RbacAccessType.Global)], null!)).Permitted() + ); + Assert.False( + (await rbacSvc.IsUserPermitted(Nobody, [TopicsReadPublic(RbacAccessType.Capability)], "cap-a")).Permitted() + ); + } } diff --git a/src/SelfService.Tests/Infrastructure/Api/TestRbacMeRoute.cs b/src/SelfService.Tests/Infrastructure/Api/TestRbacMeRoute.cs new file mode 100644 index 00000000..6ffdd38f --- /dev/null +++ b/src/SelfService.Tests/Infrastructure/Api/TestRbacMeRoute.cs @@ -0,0 +1,43 @@ +using System.Net; +using System.Text.Json; +using SelfService.Domain.Models; +using SelfService.Tests.Application; + +namespace SelfService.Tests.Infrastructure.Api; + +public class TestRbacMeRoute +{ + [Fact] + public async Task me_reports_the_guest_baseline_separately_from_the_users_own_grants() + { + var fixture = await RbacTestData.NewInMemoryFixture( + true, + new List(), + new List(), + new List() + ); + await RbacTestData.SeedGuestRole( + fixture.DbContext, + (RbacNamespace.ServiceCatalogue, "read", RbacAccessType.Global) + ); + + var application = fixture.ApiApplication; + await using var _ = application; + using var client = application.CreateClient(); + + var response = await client.GetAsync("/rbac/me"); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + using var document = JsonDocument.Parse(await response.Content.ReadAsStringAsync()); + var root = document.RootElement; + + // The caller holds nothing of their own, but the baseline is still reported — /rbac/me would + // understate their effective access without it. + Assert.Empty(root.GetProperty("permissionGrants").EnumerateArray()); + + var baseline = root.GetProperty("baselinePermissionGrants").EnumerateArray().ToList(); + var single = Assert.Single(baseline); + Assert.Equal("service-catalogue", single.GetProperty("namespace").GetString()); + Assert.Equal("read", single.GetProperty("permission").GetString()); + } +} diff --git a/src/SelfService.Tests/TestDoubles/StubPermissionQuery.cs b/src/SelfService.Tests/TestDoubles/StubPermissionQuery.cs index 537e20f4..231a600a 100644 --- a/src/SelfService.Tests/TestDoubles/StubPermissionQuery.cs +++ b/src/SelfService.Tests/TestDoubles/StubPermissionQuery.cs @@ -5,7 +5,13 @@ namespace SelfService.Tests.TestDoubles; public class StubPermissionQuery : IPermissionQuery { - public StubPermissionQuery() { } + private readonly List _guestPermissions; + + // Defaults to an empty guest baseline so existing tests see no implicit permissions. + public StubPermissionQuery(List? guestPermissions = null) + { + _guestPermissions = guestPermissions ?? new List(); + } public Task> FindUserGroupPermissionsByUserId(string userId) { @@ -19,6 +25,6 @@ public Task> FindUserGroupRolesByUserId(string userId) public Task> FindGuestPermissions() { - return Task.FromResult(new List()); + return Task.FromResult(_guestPermissions); } } diff --git a/src/SelfService/Application/RbacApplicationService.cs b/src/SelfService/Application/RbacApplicationService.cs index 73e65855..f02660ad 100644 --- a/src/SelfService/Application/RbacApplicationService.cs +++ b/src/SelfService/Application/RbacApplicationService.cs @@ -35,6 +35,13 @@ IRbacRoleRepository roleRepository _cache = new RbacCache(); } + private static readonly Dictionary DeclaredScopes = Permission + .BootstrapPermissions() + .ToDictionary(p => $"{p.Namespace}-{p.Name}", p => p.AccessType); + + private static RbacAccessType DeclaredScopeOf(RbacNamespace ns, string name) => + DeclaredScopes.TryGetValue($"{ns}-{name}", out var t) ? t : RbacAccessType.Capability; + public async Task IsUserPermitted(string user, List permissions, string objectId) { var resp = new PermittedResponse(); @@ -59,33 +66,31 @@ public async Task IsUserPermitted(string user, List p.AccessType == RbacAccessType.Capability); - if ( - isCapabilityCheck - && !combinedRoles.Any(rg => rg.Type == RbacAccessType.Capability && rg.Resource == objectId) - ) - { - var guestPermissions = await _cache.GetOrAddAsync( - CacheConst.GuestPermissions, - "global", - () => _permissionQuery.FindGuestPermissions() - ); - combinedPermissions = combinedPermissions - .Concat( - guestPermissions.Select(p => new RbacPermissionGrant( - p.Id, - p.CreatedAt, - p.AssignedEntityType, - p.AssignedEntityId, - p.Namespace, - p.Permission, - RbacAccessType.Capability, - objectId - )) - ) - .ToList(); - } + // Guest is the baseline role every user holds implicitly + var guestPermissions = await _cache.GetOrAddAsync( + CacheConst.GuestPermissions, + "global", + () => _permissionQuery.FindGuestPermissions() + ); + + combinedPermissions = combinedPermissions + .Concat( + guestPermissions.Select(g => + { + var scope = DeclaredScopeOf(g.Namespace, g.Permission); + return new RbacPermissionGrant( + g.Id, + g.CreatedAt, + g.AssignedEntityType, + g.AssignedEntityId, + g.Namespace, + g.Permission, + scope, + scope == RbacAccessType.Global ? "" : (objectId ?? "") + ); + }) + ) + .ToList(); // Shared by both grant sources below (direct/group grants and role-derived grants) so the // matching rule cannot drift between them. @@ -285,8 +290,8 @@ public async Task> GetAllRoles() return await GetAllRolesInternal(); } - // Returns roles that can be assigned to capability members. Guest is excluded as it is a system-level - // default role applied implicitly to users without an explicit capability role. + // Returns roles that can be assigned to capability members. Guest is excluded as it is the baseline + // role held implicitly by every user — assigning it to anyone would be a no-op. public async Task> GetAssignableRoles() { var allRoles = await GetAllRolesInternal(); @@ -521,7 +526,7 @@ await _roleGrantRepository.Add( if (guestRoleCheck != null && roleGrant.RoleId == guestRoleCheck.Id) { throw new BadHttpRequestException( - "Guest role cannot be directly assigned to a capability. It is the implicit default role for users without an explicit capability role." + "Guest role cannot be directly assigned to a capability. It is the baseline role held implicitly by every user, so granting it would have no effect." ); } diff --git a/src/SelfService/Infrastructure/Api/ApiResourceFactory.cs b/src/SelfService/Infrastructure/Api/ApiResourceFactory.cs index 09635cf1..93322fdf 100644 --- a/src/SelfService/Infrastructure/Api/ApiResourceFactory.cs +++ b/src/SelfService/Infrastructure/Api/ApiResourceFactory.cs @@ -1751,20 +1751,25 @@ public ReleaseNoteListApiResource Convert(IEnumerable releaseNotes) public RbacMeApiResource Convert( List permissionGrants, List roleGrants, - List groups + List groups, + List baselinePermissionGrants ) { - var mappedPermissionGrants = permissionGrants.Select(x => new RBAC.Dto.RbacPermissionGrant - { - Id = x.Id.ToString(), - AssignedEntityId = x.AssignedEntityId, - AssignedEntityType = x.AssignedEntityType, - CreatedAt = x.CreatedAt, - Namespace = x.Namespace, - Permission = x.Permission, - Resource = x.Resource, - Type = x.Type.ToString(), - }); + RBAC.Dto.RbacPermissionGrant MapPermissionGrant(RbacPermissionGrant x) => + new() + { + Id = x.Id.ToString(), + AssignedEntityId = x.AssignedEntityId, + AssignedEntityType = x.AssignedEntityType, + CreatedAt = x.CreatedAt, + Namespace = x.Namespace, + Permission = x.Permission, + Resource = x.Resource, + Type = x.Type.ToString(), + }; + + var mappedPermissionGrants = permissionGrants.Select(MapPermissionGrant); + var mappedBaselinePermissionGrants = baselinePermissionGrants.Select(MapPermissionGrant); var mappedRoleGrants = roleGrants.Select(x => new RBAC.Dto.RbacRoleGrant { @@ -1821,7 +1826,8 @@ List groups }, permissionGrants: mappedPermissionGrants.ToArray(), roleGrants: mappedRoleGrants.ToArray(), - groups: mappedGroups.ToArray() + groups: mappedGroups.ToArray(), + baselinePermissionGrants: mappedBaselinePermissionGrants.ToArray() ); return payload; diff --git a/src/SelfService/Infrastructure/Api/RBAC/RbacController.cs b/src/SelfService/Infrastructure/Api/RBAC/RbacController.cs index fe865386..6a52661c 100644 --- a/src/SelfService/Infrastructure/Api/RBAC/RbacController.cs +++ b/src/SelfService/Infrastructure/Api/RBAC/RbacController.cs @@ -75,7 +75,9 @@ public async Task Me() var groups = await _rbacApplicationService.GetGroupsForUser(userId); - return Ok(_apiResourceFactory.Convert(combinedPermissions, combinedRoles, groups)); + var baselinePermissions = await _permissionQuery.FindGuestPermissions(); + + return Ok(_apiResourceFactory.Convert(combinedPermissions, combinedRoles, groups, baselinePermissions)); } [HttpGet("get-assignable-permissions")] diff --git a/src/SelfService/Infrastructure/Api/RBAC/RbacMeListApiResource.cs b/src/SelfService/Infrastructure/Api/RBAC/RbacMeListApiResource.cs index ab95a035..98999de2 100644 --- a/src/SelfService/Infrastructure/Api/RBAC/RbacMeListApiResource.cs +++ b/src/SelfService/Infrastructure/Api/RBAC/RbacMeListApiResource.cs @@ -10,6 +10,8 @@ public class RbacMeApiResource public RbacRoleGrant[] RoleGrants { get; set; } public RbacGroup[] Groups { get; set; } + public RbacPermissionGrant[] BaselinePermissionGrants { get; set; } + [JsonPropertyName("_links")] public RbacMeLinks Links { get; set; } @@ -29,12 +31,14 @@ public RbacMeApiResource( RbacPermissionGrant[] permissionGrants, RbacRoleGrant[] roleGrants, RbacMeLinks links, - RbacGroup[] groups + RbacGroup[] groups, + RbacPermissionGrant[] baselinePermissionGrants ) { PermissionGrants = permissionGrants; RoleGrants = roleGrants; Links = links; Groups = groups; + BaselinePermissionGrants = baselinePermissionGrants; } }