diff --git a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs index 658db5cade..b88df37305 100644 --- a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs +++ b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs @@ -126,7 +126,7 @@ public async Task Should_accept_requests_with_valid_bearer_token() _ = await Define() .Done(async ctx => { - // The "reader" role grants every *:*:view permission, including error:messages:view + // The "reader" role grants every :view permission, including error:messages:view // required by /api/errors. Without a role-bearing claim the request would be 403. var validToken = mockOidcServer.GenerateToken( additionalClaims: new[] { new Claim("roles", "reader") }); diff --git a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_my_routes_are_requested.cs b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_my_routes_are_requested.cs index 2093227e72..97888c183c 100644 --- a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_my_routes_are_requested.cs +++ b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_my_routes_are_requested.cs @@ -63,48 +63,5 @@ public async Task Should_reject_requests_without_bearer_token() OpenIdConnectAssertions.AssertUnauthorized(response); } - [Test] - public async Task Reader_can_view_but_cannot_retry() - { - var routes = await GetRoutes(RolePermissions.Reader); - - using (Assert.EnterMultipleScope()) - { - Assert.That(routes.Any(r => r.UrlTemplate == "/api/configuration"), Is.True, - "reader holds :view permissions, so view routes are allowed"); - Assert.That(routes.Any(r => r.Method == "POST" && r.UrlTemplate.EndsWith("/retry")), Is.False, - "reader has no retry permission, so retry routes are excluded"); - } - } - - [Test] - public async Task Writer_can_retry() - { - var routes = await GetRoutes(RolePermissions.Writer); - - Assert.That(routes.Any(r => r.Method == "POST" && r.UrlTemplate.EndsWith("/retry")), Is.True, - "writer holds every permission, so retry routes are allowed"); - } - - async Task> GetRoutes(string role) - { - HttpResponseMessage response = null; - - _ = await Define() - .Done(async ctx => - { - var token = mockOidcServer.GenerateToken(additionalClaims: [new Claim("roles", role)]); - response = await OpenIdConnectAssertions.SendRequestWithBearerToken( - HttpClient, HttpMethod.Get, "/api/my/routes", token); - return response != null; - }) - .Run(); - - OpenIdConnectAssertions.AssertAuthenticated(response); - - var content = await response.Content.ReadAsStringAsync(); - return JsonSerializer.Deserialize>(content, SerializerOptions); - } - class Context : ScenarioContext; } diff --git a/src/ServiceControl.Audit.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs b/src/ServiceControl.Audit.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs index ab5053a505..5926e71c41 100644 --- a/src/ServiceControl.Audit.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs +++ b/src/ServiceControl.Audit.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs @@ -94,7 +94,7 @@ public async Task Should_accept_requests_with_valid_bearer_token() _ = await Define() .Done(async ctx => { - // The "reader" role grants every *:*:view permission, including audit:message:view + // The "reader" role grants every :view permission, including audit:message:view // required by /api/messages. Without a role-bearing claim the request would be 403. var validToken = mockOidcServer.GenerateToken( additionalClaims: new[] { new Claim("roles", "reader") }); diff --git a/src/ServiceControl.Infrastructure.Tests/Auth/EffectivePermissionsTests.cs b/src/ServiceControl.Infrastructure.Tests/Auth/EffectivePermissionsTests.cs deleted file mode 100644 index c4383e8dcb..0000000000 --- a/src/ServiceControl.Infrastructure.Tests/Auth/EffectivePermissionsTests.cs +++ /dev/null @@ -1,46 +0,0 @@ -#nullable enable -namespace ServiceControl.Infrastructure.Tests.Auth; - -using System; -using System.Linq; -using System.Security.Claims; -using NUnit.Framework; -using ServiceControl.Configuration; -using ServiceControl.Infrastructure; -using ServiceControl.Infrastructure.Auth; - -[TestFixture] -class EffectivePermissionsTests -{ - static readonly SettingsRootNamespace TestNamespace = new("ServiceControl"); - - static ClaimsPrincipal PrincipalWithRoles(params string[] roles) => - new(new ClaimsIdentity(roles.Select(r => new Claim(ClaimTypes.Role, r)), "test")); - - [TearDown] - public void TearDown() - { - Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_ROLEBASEDAUTHORIZATIONENABLED", null); - } - - [Test] - public void Rbac_enabled_returns_the_union_of_role_permissions() - { - Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_ROLEBASEDAUTHORIZATIONENABLED", "true"); - var settings = new OpenIdConnectSettings(TestNamespace, validateConfiguration: false); - - var result = EffectivePermissions.ForUser(PrincipalWithRoles(RolePermissions.Reader), settings); - - Assert.That(result, Is.EquivalentTo(RolePermissions.GetPermissions(RolePermissions.Reader))); - } - - [Test] - public void Rbac_disabled_returns_all_permissions() - { - var settings = new OpenIdConnectSettings(TestNamespace, validateConfiguration: false); - - var result = EffectivePermissions.ForUser(PrincipalWithRoles("anything"), settings); - - Assert.That(result, Is.EquivalentTo(Permissions.All)); - } -} diff --git a/src/ServiceControl.Infrastructure.Tests/Auth/RolePermissionsTests.cs b/src/ServiceControl.Infrastructure.Tests/Auth/RolePermissionsTests.cs new file mode 100644 index 0000000000..b7edf5a562 --- /dev/null +++ b/src/ServiceControl.Infrastructure.Tests/Auth/RolePermissionsTests.cs @@ -0,0 +1,22 @@ +#nullable enable +namespace ServiceControl.Infrastructure.Tests.Auth; + +using System.Linq; +using NUnit.Framework; +using ServiceControl.Infrastructure.Auth; + +[TestFixture] +class RolePermissionsTests +{ + [Test] + public void Every_permission_is_assigned_to_a_role() + { + var unassigned = Permissions.All + .Except(RolePermissions.Roles[RolePermissions.Admin]) + .Order() + .ToArray(); + + Assert.That(unassigned, Is.Empty, + $"Every permission constant must be assigned to a role group in RolePermissions. Unassigned: [{string.Join(", ", unassigned)}]"); + } +} diff --git a/src/ServiceControl.Infrastructure/Auth/EffectivePermissions.cs b/src/ServiceControl.Infrastructure/Auth/EffectivePermissions.cs index 1cff8bd5e6..7454a6516f 100644 --- a/src/ServiceControl.Infrastructure/Auth/EffectivePermissions.cs +++ b/src/ServiceControl.Infrastructure/Auth/EffectivePermissions.cs @@ -1,14 +1,14 @@ #nullable enable namespace ServiceControl.Infrastructure.Auth; +using System; using System.Collections.Generic; -using System.Linq; using System.Security.Claims; /// /// The set of permissions a principal effectively holds, computed per request. Mirrors the inputs the /// enforcement handler uses: when role-based authorization is enabled, the union of the permissions -/// granted by the principal's claims (via ); +/// granted by the principal's claims (via ); /// when it is disabled the platform runs allow-all, so every known permission is held. /// public static class EffectivePermissions @@ -20,7 +20,15 @@ public static IReadOnlySet ForUser(ClaimsPrincipal user, OpenIdConnectSe return Permissions.All; } - var roles = user.FindAll(ClaimTypes.Role).Select(claim => claim.Value); - return RolePermissions.GetPermissions(roles); + var permissions = new HashSet(StringComparer.Ordinal); + foreach (var claim in user.FindAll(ClaimTypes.Role)) + { + if (RolePermissions.Roles.TryGetValue(claim.Value, out var granted)) + { + permissions.UnionWith(granted); + } + } + + return permissions; } } diff --git a/src/ServiceControl.Infrastructure/Auth/Permissions.cs b/src/ServiceControl.Infrastructure/Auth/Permissions.cs index 006fccf0f5..04f6582c14 100644 --- a/src/ServiceControl.Infrastructure/Auth/Permissions.cs +++ b/src/ServiceControl.Infrastructure/Auth/Permissions.cs @@ -85,7 +85,7 @@ public static class Permissions /// public const string ErrorThroughputManage = "error:throughput:manage"; - /// Platform connections area — viewing and managing broker/platform connection settings. + /// Platform connections area — viewing and managing platform connection settings. public const string ErrorConnectionsView = "error:connections:view"; /// public const string ErrorConnectionsManage = "error:connections:manage"; diff --git a/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs b/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs index bb79f98f02..473e9e19d3 100644 --- a/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs +++ b/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs @@ -4,63 +4,82 @@ namespace ServiceControl.Infrastructure.Auth; using System; using System.Collections.Frozen; using System.Collections.Generic; -using System.Linq; -/// -/// Role → permission policy. Two roles: -/// -/// reader — granted every *:*:view permission (read-only access). -/// writer — granted every permission (*:*:*). -/// -/// The wildcard patterns (* is a colon-segment wildcard) are the source of truth, but they are -/// expanded once at type initialization against into a concrete, -/// immutable of granted permissions per role. As a result both -/// and are O(1) hash lookups with no -/// per-call pattern matching or allocation. -/// public static class RolePermissions { - /// Read-only role: every *:*:view permission. public const string Reader = "reader"; - - /// Full-access role: every permission. public const string Writer = "writer"; - - /// - /// Platform-administrator role: read-only on everything, plus full management of the configuration / - /// admin-area resources (licensing, notifications, retry redirects, throughput, connections) — but - /// not the message-triage write actions (retry/edit/archive/restore). - /// public const string Admin = "admin"; - // Source of truth: the wildcard pattern(s) each role grants. - static readonly Dictionary RolePatterns = new(StringComparer.OrdinalIgnoreCase) - { - [Reader] = ["*:*:view"], - [Writer] = ["*:*:*"], - [Admin] = - [ - "*:*:view", - "error:licensing:*", - "error:notifications:*", - "error:redirects:*", - "error:throughput:*", - "error:connections:*", - ], - }; + static readonly string[] Read = + [ + Permissions.ErrorMessagesView, + Permissions.ErrorRecoverabilityGroupsView, + Permissions.ErrorEndpointsView, + Permissions.ErrorHeartbeatsView, + Permissions.ErrorCustomChecksView, + Permissions.ErrorSagasView, + Permissions.ErrorEventLogView, + Permissions.ErrorQueuesView, + Permissions.ErrorConnectionsView, + Permissions.AuditMessageView, + Permissions.AuditConnectionView, + Permissions.AuditEndpointView, + Permissions.AuditSagaView, + Permissions.MonitoringEndpointView, + Permissions.MonitoringConnectionView, + Permissions.MonitoringLicenseView, + ]; + + static readonly string[] ReadConfiguration = + [ + Permissions.ErrorLicensingView, + Permissions.ErrorNotificationsView, + Permissions.ErrorRedirectsView, + Permissions.ErrorThroughputView, + ]; + + static readonly string[] Operate = + [ + Permissions.ErrorMessagesRetry, + Permissions.ErrorMessagesArchive, + Permissions.ErrorMessagesUnarchive, + Permissions.ErrorMessagesEdit, + Permissions.ErrorRecoverabilityGroupsRetry, + Permissions.ErrorRecoverabilityGroupsArchive, + Permissions.ErrorRecoverabilityGroupsUnarchive, + Permissions.ErrorEndpointsManage, + Permissions.ErrorEndpointsDelete, + Permissions.ErrorCustomChecksDelete, + Permissions.ErrorQueuesDelete, + Permissions.ErrorConnectionsManage, + Permissions.MonitoringEndpointDelete, + ]; + + static readonly string[] Configure = + [ + Permissions.ErrorLicensingManage, + Permissions.ErrorNotificationsManage, + Permissions.ErrorNotificationsTest, + Permissions.ErrorRedirectsManage, + Permissions.ErrorThroughputManage, + ]; + + public static readonly FrozenDictionary> Roles = + new Dictionary>(StringComparer.OrdinalIgnoreCase) + { + [Reader] = ToSet([.. Read, .. ReadConfiguration]), + [Writer] = ToSet([.. Read, .. Operate]), + [Admin] = ToSet([.. Read, .. ReadConfiguration, .. Operate, .. Configure]), + }.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); - // Expanded once against the full permission catalogue: role -> concrete granted permissions. - static readonly FrozenDictionary> PermissionsByRole = Expand(); + static FrozenSet ToSet(string[] permissions) => permissions.ToFrozenSet(StringComparer.Ordinal); - /// - /// Returns if any of the supplied grants the - /// requested . O(1) per role — a frozen-set membership test. - /// - public static bool IsGranted(IEnumerable roles, string permission) + public static bool IsGranted(string[] roles, string permission) { foreach (var role in roles) { - if (PermissionsByRole.TryGetValue(role, out var granted) && granted.Contains(permission)) + if (Roles.TryGetValue(role, out var granted) && granted.Contains(permission)) { return true; } @@ -68,72 +87,4 @@ public static bool IsGranted(IEnumerable roles, string permission) return false; } - - /// - /// The complete set of permissions granted to a single role (empty if the role is unknown). - /// O(1) and allocation-free — returns the precomputed frozen set. - /// - public static IReadOnlySet GetPermissions(string role) => - PermissionsByRole.TryGetValue(role, out var granted) ? granted : FrozenSet.Empty; - - /// - /// The union of permissions granted across several . Allocation-free for the - /// common single-role case; only the multi-role union allocates. - /// - public static IReadOnlySet GetPermissions(IEnumerable roles) - { - var list = roles as IReadOnlyList ?? roles.ToList(); - if (list.Count <= 1) - { - return list.Count == 0 ? FrozenSet.Empty : GetPermissions(list[0]); - } - - var union = new HashSet(StringComparer.Ordinal); - foreach (var role in list) - { - if (PermissionsByRole.TryGetValue(role, out var granted)) - { - union.UnionWith(granted); - } - } - - return union; - } - - static FrozenDictionary> Expand() - { - var expanded = new Dictionary>(StringComparer.OrdinalIgnoreCase); - - foreach (var (role, patterns) in RolePatterns) - { - expanded[role] = Permissions.All - .Where(permission => patterns.Any(pattern => Matches(pattern, permission))) - .ToFrozenSet(StringComparer.Ordinal); - } - - return expanded.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); - } - - /// Matches a colon-delimited permission against a pattern where * is a segment wildcard. - static bool Matches(string pattern, string permission) - { - var patternSegments = pattern.Split(':'); - var permissionSegments = permission.Split(':'); - - if (patternSegments.Length != permissionSegments.Length) - { - return false; - } - - for (var i = 0; i < patternSegments.Length; i++) - { - if (patternSegments[i] != "*" - && !string.Equals(patternSegments[i], permissionSegments[i], StringComparison.OrdinalIgnoreCase)) - { - return false; - } - } - - return true; - } } diff --git a/src/ServiceControl.Monitoring.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs b/src/ServiceControl.Monitoring.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs index 82ce6a9d20..963f727b86 100644 --- a/src/ServiceControl.Monitoring.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs +++ b/src/ServiceControl.Monitoring.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs @@ -94,7 +94,7 @@ public async Task Should_accept_requests_with_valid_bearer_token() _ = await Define() .Done(async ctx => { - // The "reader" role grants every *:*:view permission, including + // The "reader" role grants every :view permission, including // monitoring:endpoint:view required by /monitored-endpoints. Without a // role-bearing claim the request would be 403. var validToken = mockOidcServer.GenerateToken(