From 8d5af182b9a1f049f5dc3796980aa5b65cc77eec Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 19 Jun 2026 11:40:38 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Typed=20PermissionId=20for=20instan?= =?UTF-8?q?ce:component:access=20permissions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a strongly-typed, additive layer over the colon-delimited authorization permission strings: - InstanceId / AccessLevel / Component enums (wire value = name lowercased) - PermissionId readonly record struct with Parse/TryParse/ToString and an All catalogue derived from Permissions.All; rejects well-typed but unknown triples (e.g. audit:messages:retry) - PermissionPattern readonly record struct for *:*:view wildcard matching RolePermissions now expresses its reader/writer patterns as typed PermissionPattern values and matches via PermissionPattern.Matches, removing the hand-rolled Split(':') matcher. IsGranted keeps its string signature, so nothing downstream changes. The hand-authored const strings in Permissions.cs remain the source of truth (required by [Authorize(Policy=...)]); a guard test asserts they round-trip through the typed model and that the two catalogues stay in sync. Type named PermissionId rather than Permission to satisfy CA1711 (enforced as error) and to pair with InstanceId. --- .../Auth/PermissionTests.cs | 59 ++++++++++++++ .../Auth/AccessLevel.cs | 18 +++++ .../Auth/Component.cs | 39 +++++++++ .../Auth/InstanceId.cs | 14 ++++ .../Auth/PermissionId.cs | 79 +++++++++++++++++++ .../Auth/PermissionPattern.cs | 51 ++++++++++++ .../Auth/RolePermissions.cs | 36 ++------- 7 files changed, 267 insertions(+), 29 deletions(-) create mode 100644 src/ServiceControl.Infrastructure.Tests/Auth/PermissionTests.cs create mode 100644 src/ServiceControl.Infrastructure/Auth/AccessLevel.cs create mode 100644 src/ServiceControl.Infrastructure/Auth/Component.cs create mode 100644 src/ServiceControl.Infrastructure/Auth/InstanceId.cs create mode 100644 src/ServiceControl.Infrastructure/Auth/PermissionId.cs create mode 100644 src/ServiceControl.Infrastructure/Auth/PermissionPattern.cs diff --git a/src/ServiceControl.Infrastructure.Tests/Auth/PermissionTests.cs b/src/ServiceControl.Infrastructure.Tests/Auth/PermissionTests.cs new file mode 100644 index 0000000000..f7f0b3ada2 --- /dev/null +++ b/src/ServiceControl.Infrastructure.Tests/Auth/PermissionTests.cs @@ -0,0 +1,59 @@ +#nullable enable +namespace ServiceControl.Infrastructure.Tests.Auth; + +using System.Linq; +using NUnit.Framework; +using ServiceControl.Infrastructure.Auth; + +[TestFixture] +public class PermissionTests +{ + [Test] + public void Every_const_permission_round_trips_through_the_typed_model() + { + foreach (var value in Permissions.All) + { + Assert.That(PermissionId.TryParse(value, out var permission), Is.True, $"'{value}' did not parse."); + Assert.That(permission.ToString(), Is.EqualTo(value), $"'{value}' did not round-trip."); + } + } + + [Test] + public void Typed_catalogue_matches_the_const_catalogue() + { + var typed = PermissionId.All.Select(p => p.ToString()); + + Assert.That(typed, Is.EquivalentTo(Permissions.All)); + } + + [Test] + public void TryParse_rejects_a_well_typed_but_unknown_triple() + { + // audit:messages:retry uses valid segments but is not a real permission. + Assert.That(PermissionId.TryParse("audit:messages:retry", out _), Is.False); + } + + [Test] + public void TryParse_rejects_malformed_input() + { + Assert.That(PermissionId.TryParse("error:messages", out _), Is.False); + Assert.That(PermissionId.TryParse("error:messages:view:extra", out _), Is.False); + Assert.That(PermissionId.TryParse("nope:nope:nope", out _), Is.False); + } + + [Test] + public void TryParse_is_case_insensitive() + { + Assert.That(PermissionId.TryParse("ERROR:Messages:VIEW", out var permission), Is.True); + Assert.That(permission, Is.EqualTo(new PermissionId(InstanceId.Error, Component.Messages, AccessLevel.View))); + } + + [Test] + public void Pattern_matches_expected_permissions() + { + var viewPattern = PermissionPattern.Parse("*:*:view"); + + Assert.That(viewPattern.Matches(PermissionId.Parse("error:messages:view")), Is.True); + Assert.That(viewPattern.Matches(PermissionId.Parse("error:messages:retry")), Is.False); + } +} diff --git a/src/ServiceControl.Infrastructure/Auth/AccessLevel.cs b/src/ServiceControl.Infrastructure/Auth/AccessLevel.cs new file mode 100644 index 0000000000..c2f681689f --- /dev/null +++ b/src/ServiceControl.Infrastructure/Auth/AccessLevel.cs @@ -0,0 +1,18 @@ +#nullable enable +namespace ServiceControl.Infrastructure.Auth; + +/// +/// The action a permission authorizes. is the read-only level; every other value is a +/// write/mutating action. The wire value is the name lowercased (e.g. view). +/// +public enum AccessLevel +{ + View, + Retry, + Archive, + Unarchive, + Edit, + Manage, + Delete, + Test +} diff --git a/src/ServiceControl.Infrastructure/Auth/Component.cs b/src/ServiceControl.Infrastructure/Auth/Component.cs new file mode 100644 index 0000000000..84d91c8e7b --- /dev/null +++ b/src/ServiceControl.Infrastructure/Auth/Component.cs @@ -0,0 +1,39 @@ +#nullable enable +namespace ServiceControl.Infrastructure.Auth; + +/// +/// The functional area a permission applies to, flat across all instances. The wire value is the name +/// lowercased (e.g. recoverabilitygroups). +/// +/// The set is flat (not nested per instance), so not every is valid for every +/// . The error instance uses the plural forms (, +/// , …) while the audit and monitoring instances use the singular forms +/// (, , …). Validity of a full +/// instance:component:access triple is enforced by against +/// the known catalogue. +/// +/// +public enum Component +{ + // Error instance (plural). + Messages, + RecoverabilityGroups, + Endpoints, + Heartbeats, + CustomChecks, + Sagas, + EventLog, + Licensing, + Notifications, + Redirects, + Queues, + Throughput, + Connections, + + // Audit / Monitoring instances (singular). + Message, + Connection, + Endpoint, + Saga, + License +} diff --git a/src/ServiceControl.Infrastructure/Auth/InstanceId.cs b/src/ServiceControl.Infrastructure/Auth/InstanceId.cs new file mode 100644 index 0000000000..bf2cb61620 --- /dev/null +++ b/src/ServiceControl.Infrastructure/Auth/InstanceId.cs @@ -0,0 +1,14 @@ +#nullable enable +namespace ServiceControl.Infrastructure.Auth; + +/// +/// The ServiceControl instance a permission belongs to. Each instance is a separate process and +/// namespaces its permissions with this prefix. The wire value is the name lowercased +/// (e.g. error). +/// +public enum InstanceId +{ + Error, + Audit, + Monitoring +} diff --git a/src/ServiceControl.Infrastructure/Auth/PermissionId.cs b/src/ServiceControl.Infrastructure/Auth/PermissionId.cs new file mode 100644 index 0000000000..c3c7d9b4d7 --- /dev/null +++ b/src/ServiceControl.Infrastructure/Auth/PermissionId.cs @@ -0,0 +1,79 @@ +#nullable enable +namespace ServiceControl.Infrastructure.Auth; + +using System; +using System.Collections.Generic; +using System.Linq; + +/// +/// A strongly-typed authorization permission in the canonical wire format +/// instance:component:access (e.g. error:messages:view). +/// +/// The hand-authored const string constants in remain the source of +/// truth (they are required by [Authorize(Policy = …)] attributes). is derived +/// from , and only accepts a triple that is a member +/// of that catalogue — so a well-typed but non-existent combination such as audit:messages:retry +/// is rejected. +/// +/// +public readonly record struct PermissionId(InstanceId Instance, Component Component, AccessLevel Access) +{ + /// The complete set of known permissions, parsed from . + public static IReadOnlySet All { get; } = BuildAll(); + + /// The canonical wire representation, instance:component:access (lowercased). + public override string ToString() => + $"{Instance.ToString().ToLowerInvariant()}:{Component.ToString().ToLowerInvariant()}:{Access.ToString().ToLowerInvariant()}"; + + /// + /// Parses a instance:component:access string. Case-insensitive. Returns + /// for malformed input or for a well-typed triple that is not part of the known catalogue. + /// + public static bool TryParse(string value, out PermissionId permission) => + TryParseSegments(value, out permission) && All.Contains(permission); + + /// Parses a instance:component:access string, throwing on an unknown or malformed value. + public static PermissionId Parse(string value) => + TryParse(value, out var permission) + ? permission + : throw new FormatException($"'{value}' is not a known permission."); + + static IReadOnlySet BuildAll() + { + var set = new HashSet(); + foreach (var value in Permissions.All) + { + if (TryParseSegments(value, out var permission)) + { + set.Add(permission); + } + } + + return set; + } + + // Enum-level parse only; does not validate the triple against the catalogue (used to build it). + static bool TryParseSegments(string value, out PermissionId permission) + { + permission = default; + + var segments = value.Split(':'); + if (segments.Length != 3) + { + return false; + } + + if (TryParseEnum(segments[0], out var instance) + && TryParseEnum(segments[1], out var component) + && TryParseEnum(segments[2], out var access)) + { + permission = new PermissionId(instance, component, access); + return true; + } + + return false; + } + + static bool TryParseEnum(string value, out T result) where T : struct, Enum => + Enum.TryParse(value, ignoreCase: true, out result) && Enum.IsDefined(result); +} diff --git a/src/ServiceControl.Infrastructure/Auth/PermissionPattern.cs b/src/ServiceControl.Infrastructure/Auth/PermissionPattern.cs new file mode 100644 index 0000000000..e4dfe7827b --- /dev/null +++ b/src/ServiceControl.Infrastructure/Auth/PermissionPattern.cs @@ -0,0 +1,51 @@ +#nullable enable +namespace ServiceControl.Infrastructure.Auth; + +using System; + +/// +/// A wildcard pattern over the three permission segments, where a segment is the +/// * wildcard that matches any value. For example *:*:view is +/// new PermissionPattern(null, null, AccessLevel.View) and matches every view permission. +/// +public readonly record struct PermissionPattern(InstanceId? Instance, Component? Component, AccessLevel? Access) +{ + /// Returns if matches every non-wildcard segment. + public bool Matches(PermissionId permission) => + (Instance is null || Instance == permission.Instance) + && (Component is null || Component == permission.Component) + && (Access is null || Access == permission.Access); + + /// + /// Parses a colon-delimited pattern (e.g. *:*:view) where * is a segment wildcard. + /// Throws on a malformed pattern or an unknown segment value. + /// + public static PermissionPattern Parse(string value) + { + var segments = value.Split(':'); + if (segments.Length != 3) + { + throw new FormatException($"'{value}' is not a valid permission pattern (expected instance:component:access)."); + } + + return new PermissionPattern( + ParseSegment(segments[0]), + ParseSegment(segments[1]), + ParseSegment(segments[2])); + } + + static T? ParseSegment(string value) where T : struct, Enum + { + if (value == "*") + { + return null; + } + + if (Enum.TryParse(value, ignoreCase: true, out var result) && Enum.IsDefined(result)) + { + return result; + } + + throw new FormatException($"'{value}' is not a valid {typeof(T).Name} segment."); + } +} diff --git a/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs b/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs index 7375919263..bc1d7e44d0 100644 --- a/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs +++ b/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs @@ -26,11 +26,11 @@ public static class RolePermissions /// Full-access role: every permission. public const string Writer = "writer"; - // Source of truth: the wildcard pattern(s) each role grants. - static readonly Dictionary RolePatterns = new(StringComparer.OrdinalIgnoreCase) + // Source of truth: the wildcard pattern(s) each role grants (null segment = '*'). + static readonly Dictionary RolePatterns = new(StringComparer.OrdinalIgnoreCase) { - [Reader] = ["*:*:view"], - [Writer] = ["*:*:*"], + [Reader] = [new PermissionPattern(null, null, AccessLevel.View)], + [Writer] = [new PermissionPattern(null, null, null)], }; // Expanded once against the full permission catalogue: role -> concrete granted permissions. @@ -90,34 +90,12 @@ static FrozenDictionary> Expand() foreach (var (role, patterns) in RolePatterns) { - expanded[role] = Permissions.All - .Where(permission => patterns.Any(pattern => Matches(pattern, permission))) + expanded[role] = PermissionId.All + .Where(permission => patterns.Any(pattern => pattern.Matches(permission))) + .Select(permission => permission.ToString()) .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; - } }