From 6125b09f73c72655f8d7d5aa738496d59c2621a8 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Tue, 2 Jun 2026 11:32:49 +0200 Subject: [PATCH 01/14] Add permissions catalog with constants for access control Add Authorize attribute to all controller APIs --- .../Particular.LicensingComponent.csproj | 1 + .../WebApi/LicensingController.cs | 10 ++ .../MessagesView/GetMessages2Controller.cs | 3 + .../MessagesView/GetMessagesController.cs | 8 + .../MessagesConversationController.cs | 3 + .../Connection/ConnectionController.cs | 3 + .../KnownEndpointsController.cs | 3 + .../SagaAudit/SagasController.cs | 3 + .../Auth/Permissions.cs | 137 ++++++++++++++++++ .../Connection/ConnectionController.cs | 3 + .../Http/Diagrams/DiagramApiController.cs | 6 + .../Http/LicenseController.cs | 3 + .../Messages/GetMessages2Controller.cs | 3 + .../GetMessagesByConversationController.cs | 3 + .../Messages/GetMessagesController.cs | 10 ++ .../Connection/ConnectionController.cs | 3 + .../CustomChecks/Web/CustomCheckController.cs | 4 + .../EventLog/EventLogApiController.cs | 3 + .../Licensing/LicenseController.cs | 3 + .../Api/ArchiveMessagesController.cs | 6 + .../Api/EditFailedMessagesController.cs | 4 + .../Api/GetAllErrorsController.cs | 6 + .../Api/GetErrorByIdController.cs | 4 + .../Api/PendingRetryMessagesController.cs | 4 + .../Api/QueueAddressController.cs | 4 + .../Api/ResolveMessagesController.cs | 4 + .../Api/RetryMessagesController.cs | 7 + .../Api/UnArchiveMessagesController.cs | 4 + .../Api/MessageRedirectsController.cs | 7 + .../Web/EndpointsMonitoringController.cs | 6 + .../Web/EndpointsSettingsController.cs | 4 + .../Api/NotificationsController.cs | 6 + .../API/FailureGroupsArchiveController.cs | 3 + .../API/FailureGroupsController.cs | 10 ++ .../API/FailureGroupsRetryController.cs | 3 + .../API/FailureGroupsUnarchiveController.cs | 3 + .../API/UnacknowledgedGroupsController.cs | 3 + .../SagaAudit/SagasController.cs | 3 + 38 files changed, 303 insertions(+) create mode 100644 src/ServiceControl.Infrastructure/Auth/Permissions.cs diff --git a/src/Particular.LicensingComponent/Particular.LicensingComponent.csproj b/src/Particular.LicensingComponent/Particular.LicensingComponent.csproj index 4f87cd4a63..d1136120d9 100644 --- a/src/Particular.LicensingComponent/Particular.LicensingComponent.csproj +++ b/src/Particular.LicensingComponent/Particular.LicensingComponent.csproj @@ -13,6 +13,7 @@ + diff --git a/src/Particular.LicensingComponent/WebApi/LicensingController.cs b/src/Particular.LicensingComponent/WebApi/LicensingController.cs index 0b094fbb94..f898c0cc72 100644 --- a/src/Particular.LicensingComponent/WebApi/LicensingController.cs +++ b/src/Particular.LicensingComponent/WebApi/LicensingController.cs @@ -5,10 +5,12 @@ using System.Text.Json; using System.Threading; using Contracts; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.Net.Http.Headers; using Particular.LicensingComponent.Report; + using ServiceControl.Infrastructure.Auth; [ApiController] [Route("api/licensing")] @@ -19,6 +21,7 @@ public LicensingController(IThroughputCollector throughputCollector) this.throughputCollector = throughputCollector; } + [Authorize(Policy = Permissions.ErrorThroughputView)] [Route("endpoints")] [HttpGet] public async Task> GetEndpointThroughput(CancellationToken cancellationToken) @@ -26,6 +29,7 @@ public async Task> GetEndpointThroughput(Cancell return await throughputCollector.GetThroughputSummary(cancellationToken); } + [Authorize(Policy = Permissions.ErrorThroughputManage)] [Route("endpoints/update")] [HttpPost] public async Task UpdateUserSelectionOnEndpointThroughput(List updateUserIndicators, CancellationToken cancellationToken) @@ -34,6 +38,7 @@ public async Task UpdateUserSelectionOnEndpointThroughput(List CanThroughputReportBeGenerated(CancellationToken cancellationToken) @@ -41,6 +46,7 @@ public async Task CanThroughputReportBeGenerated(Cancella return await throughputCollector.GetReportGenerationState(cancellationToken); } + [Authorize(Policy = Permissions.ErrorThroughputView)] [Route("report/file")] [HttpGet] public async Task GetThroughputReportFile([FromQuery(Name = "spVersion")] string? spVersion, CancellationToken cancellationToken) @@ -77,6 +83,7 @@ public async Task GetThroughputReportFile([FromQuery(Name = "spVersion")] string await JsonSerializer.SerializeAsync(entryStream, report, SerializationOptions.IndentedWithNoEscaping, cancellationToken); } + [Authorize(Policy = Permissions.ErrorThroughputView)] [Route("settings/info")] [HttpGet] public async Task GetThroughputSettingsInformation(CancellationToken cancellationToken) @@ -84,10 +91,12 @@ public async Task GetThroughputSettingsInformation return await throughputCollector.GetThroughputConnectionSettingsInformation(cancellationToken); } + [Authorize(Policy = Permissions.ErrorThroughputView)] [Route("settings/test")] [HttpGet] public async Task TestThroughputConnectionSettings(CancellationToken cancellationToken) => await throughputCollector.TestConnectionSettings(cancellationToken); + [Authorize(Policy = Permissions.ErrorThroughputView)] [Route("settings/masks")] [HttpGet] public async Task> GetMasks(CancellationToken cancellationToken) @@ -95,6 +104,7 @@ public async Task> GetMasks(CancellationToken cancellationToken) return await throughputCollector.GetReportMasks(cancellationToken); } + [Authorize(Policy = Permissions.ErrorThroughputManage)] [Route("settings/masks/update")] [HttpPost] public async Task UpdateMasks(List updateMasks, CancellationToken cancellationToken) diff --git a/src/ServiceControl.Audit/Auditing/MessagesView/GetMessages2Controller.cs b/src/ServiceControl.Audit/Auditing/MessagesView/GetMessages2Controller.cs index db187bcd77..5027f7f43f 100644 --- a/src/ServiceControl.Audit/Auditing/MessagesView/GetMessages2Controller.cs +++ b/src/ServiceControl.Audit/Auditing/MessagesView/GetMessages2Controller.cs @@ -5,13 +5,16 @@ namespace ServiceControl.Audit.Auditing.MessagesView; using System.Threading.Tasks; using Infrastructure; using Infrastructure.WebApi; +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Persistence; +using ServiceControl.Infrastructure.Auth; [ApiController] [Route("api")] public class GetMessages2Controller(IAuditDataStore dataStore) : ControllerBase { + [Authorize(Policy = Permissions.AuditMessageView)] [Route("messages2")] [HttpGet] public async Task> GetAllMessages( diff --git a/src/ServiceControl.Audit/Auditing/MessagesView/GetMessagesController.cs b/src/ServiceControl.Audit/Auditing/MessagesView/GetMessagesController.cs index 7a81b7294d..e88d1fbb6f 100644 --- a/src/ServiceControl.Audit/Auditing/MessagesView/GetMessagesController.cs +++ b/src/ServiceControl.Audit/Auditing/MessagesView/GetMessagesController.cs @@ -9,11 +9,13 @@ namespace ServiceControl.Audit.Auditing.MessagesView using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Persistence; + using ServiceControl.Infrastructure.Auth; [ApiController] [Route("api")] public class GetMessagesController(IAuditDataStore dataStore) : ControllerBase { + [Authorize(Policy = Permissions.AuditMessageView)] [Route("messages")] [HttpGet] public async Task> GetAllMessages([FromQuery] PagingInfo pagingInfo, [FromQuery] SortInfo sortInfo, [FromQuery(Name = "include_system_messages")] bool includeSystemMessages, CancellationToken cancellationToken) @@ -23,6 +25,7 @@ public async Task> GetAllMessages([FromQuery] PagingInfo pag return result.Results; } + [Authorize(Policy = Permissions.AuditMessageView)] [Route("endpoints/{endpoint}/messages")] [HttpGet] public async Task> GetEndpointMessages([FromQuery] PagingInfo pagingInfo, [FromQuery] SortInfo sortInfo, [FromQuery(Name = "include_system_messages")] bool includeSystemMessages, string endpoint, CancellationToken cancellationToken) @@ -43,6 +46,7 @@ public async Task> GetEndpointAuditCounts([FromQuery] PagingIn return result.Results; } + [Authorize(Policy = Permissions.AuditMessageView)] [Route("messages/{id}/body")] [HttpGet] public async Task Get(string id, CancellationToken cancellationToken) @@ -69,6 +73,7 @@ public async Task Get(string id, CancellationToken cancellationTo return result.StringContent != null ? Content(result.StringContent, contentType) : File(result.StreamContent, contentType); } + [Authorize(Policy = Permissions.AuditMessageView)] [Route("messages/search")] [HttpGet] public async Task> Search([FromQuery] PagingInfo pagingInfo, [FromQuery] SortInfo sortInfo, string q, CancellationToken cancellationToken) @@ -78,6 +83,7 @@ public async Task> Search([FromQuery] PagingInfo pagingInfo, return result.Results; } + [Authorize(Policy = Permissions.AuditMessageView)] [Route("messages/search/{keyword}")] [HttpGet] public async Task> SearchByKeyWord([FromQuery] PagingInfo pagingInfo, [FromQuery] SortInfo sortInfo, string keyword, CancellationToken cancellationToken) @@ -87,6 +93,7 @@ public async Task> SearchByKeyWord([FromQuery] PagingInfo pa return result.Results; } + [Authorize(Policy = Permissions.AuditMessageView)] [Route("endpoints/{endpoint}/messages/search")] [HttpGet] public async Task> Search([FromQuery] PagingInfo pagingInfo, [FromQuery] SortInfo sortInfo, string endpoint, string q, CancellationToken cancellationToken) @@ -96,6 +103,7 @@ public async Task> Search([FromQuery] PagingInfo pagingInfo, return result.Results; } + [Authorize(Policy = Permissions.AuditMessageView)] [Route("endpoints/{endpoint}/messages/search/{keyword}")] [HttpGet] public async Task> SearchByKeyword([FromQuery] PagingInfo pagingInfo, [FromQuery] SortInfo sortInfo, string endpoint, string keyword, CancellationToken cancellationToken) diff --git a/src/ServiceControl.Audit/Auditing/MessagesView/MessagesConversationController.cs b/src/ServiceControl.Audit/Auditing/MessagesView/MessagesConversationController.cs index 4fcc04cd88..7ba20c1a11 100644 --- a/src/ServiceControl.Audit/Auditing/MessagesView/MessagesConversationController.cs +++ b/src/ServiceControl.Audit/Auditing/MessagesView/MessagesConversationController.cs @@ -5,13 +5,16 @@ namespace ServiceControl.Audit.Auditing.MessagesView using System.Threading.Tasks; using Infrastructure; using Infrastructure.WebApi; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Persistence; + using ServiceControl.Infrastructure.Auth; [ApiController] [Route("api")] public class MessagesConversationController(IAuditDataStore dataStore) : ControllerBase { + [Authorize(Policy = Permissions.AuditMessageView)] [Route("conversations/{conversationId}")] [HttpGet] public async Task> Get([FromQuery] PagingInfo pagingInfo, [FromQuery] SortInfo sortInfo, string conversationId, CancellationToken cancellationToken) diff --git a/src/ServiceControl.Audit/Connection/ConnectionController.cs b/src/ServiceControl.Audit/Connection/ConnectionController.cs index 75daeb593d..8d8d0fa4dc 100644 --- a/src/ServiceControl.Audit/Connection/ConnectionController.cs +++ b/src/ServiceControl.Audit/Connection/ConnectionController.cs @@ -2,7 +2,9 @@ namespace ServiceControl.Audit.Connection { using System.Text.Json; using Infrastructure.Settings; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; + using ServiceControl.Infrastructure.Auth; [ApiController] [Route("api")] @@ -11,6 +13,7 @@ public class ConnectionController(Settings settings) : ControllerBase // This controller doesn't use the default serialization settings because // ServicePulse and the Platform Connector Plugin expect the connection // details the be serialized and formatted in a specific way + [Authorize(Policy = Permissions.AuditConnectionView)] [Route("connection")] [HttpGet] public IActionResult GetConnectionDetails() => diff --git a/src/ServiceControl.Audit/Monitoring/KnownEndpoints/KnownEndpointsController.cs b/src/ServiceControl.Audit/Monitoring/KnownEndpoints/KnownEndpointsController.cs index 2ed0b2a278..3b7fd5871b 100644 --- a/src/ServiceControl.Audit/Monitoring/KnownEndpoints/KnownEndpointsController.cs +++ b/src/ServiceControl.Audit/Monitoring/KnownEndpoints/KnownEndpointsController.cs @@ -5,13 +5,16 @@ namespace ServiceControl.Audit.Monitoring using System.Threading.Tasks; using Infrastructure; using Infrastructure.WebApi; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Persistence; + using ServiceControl.Infrastructure.Auth; [ApiController] [Route("api")] public class KnownEndpointsController(IAuditDataStore dataStore) : ControllerBase { + [Authorize(Policy = Permissions.AuditEndpointView)] [Route("endpoints/known")] [HttpGet] public async Task> GetAll([FromQuery] PagingInfo pagingInfo, CancellationToken cancellationToken) diff --git a/src/ServiceControl.Audit/SagaAudit/SagasController.cs b/src/ServiceControl.Audit/SagaAudit/SagasController.cs index cd2356784d..3d92d5f913 100644 --- a/src/ServiceControl.Audit/SagaAudit/SagasController.cs +++ b/src/ServiceControl.Audit/SagaAudit/SagasController.cs @@ -5,14 +5,17 @@ namespace ServiceControl.Audit.SagaAudit using System.Threading.Tasks; using Infrastructure; using Infrastructure.WebApi; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Persistence; + using ServiceControl.Infrastructure.Auth; using ServiceControl.SagaAudit; [ApiController] [Route("api")] public class SagasController(IAuditDataStore dataStore) : ControllerBase { + [Authorize(Policy = Permissions.AuditSagaView)] [Route("sagas/{id}")] [HttpGet] public async Task Sagas([FromQuery] PagingInfo pagingInfo, Guid id, CancellationToken cancellationToken) diff --git a/src/ServiceControl.Infrastructure/Auth/Permissions.cs b/src/ServiceControl.Infrastructure/Auth/Permissions.cs new file mode 100644 index 0000000000..006fccf0f5 --- /dev/null +++ b/src/ServiceControl.Infrastructure/Auth/Permissions.cs @@ -0,0 +1,137 @@ +#nullable enable +namespace ServiceControl.Infrastructure.Auth; + +using System.Collections.Generic; +using System.Reflection; + +/// +/// Catalogue of all known permission constants in the format instance:resource:action. +/// Each ServiceControl instance (error/audit/monitoring) is a separate process and namespaces its +/// permissions with an instance prefix. +/// +/// The set is automatically derived from all public const string +/// fields on this class, so adding a new constant is sufficient — no separate registration needed. +/// +/// +public static class Permissions +{ + // ───────────────────────────── Error instance (Primary) ───────────────────────────── + + /// Messages area — viewing, retrying, archiving, and editing failed messages. + public const string ErrorMessagesView = "error:messages:view"; + /// + public const string ErrorMessagesRetry = "error:messages:retry"; + /// + public const string ErrorMessagesArchive = "error:messages:archive"; + /// + public const string ErrorMessagesUnarchive = "error:messages:unarchive"; + /// + public const string ErrorMessagesEdit = "error:messages:edit"; + + /// Recoverability groups area — viewing, retrying, archiving, and unarchiving failure groups. + public const string ErrorRecoverabilityGroupsView = "error:recoverabilitygroups:view"; + /// + public const string ErrorRecoverabilityGroupsRetry = "error:recoverabilitygroups:retry"; + /// + public const string ErrorRecoverabilityGroupsArchive = "error:recoverabilitygroups:archive"; + /// + public const string ErrorRecoverabilityGroupsUnarchive = "error:recoverabilitygroups:unarchive"; + + /// Endpoints area — viewing, managing, and deleting monitored endpoints. + public const string ErrorEndpointsView = "error:endpoints:view"; + /// + public const string ErrorEndpointsManage = "error:endpoints:manage"; + /// + public const string ErrorEndpointsDelete = "error:endpoints:delete"; + + /// Heartbeats area — viewing heartbeat status for endpoints. + public const string ErrorHeartbeatsView = "error:heartbeats:view"; + + /// Custom checks area — viewing and deleting custom check results. + public const string ErrorCustomChecksView = "error:customchecks:view"; + /// + public const string ErrorCustomChecksDelete = "error:customchecks:delete"; + + /// Sagas area — viewing saga audit data. + public const string ErrorSagasView = "error:sagas:view"; + + /// Event log area — viewing the event log. + public const string ErrorEventLogView = "error:eventlog:view"; + + /// Licensing area — viewing and managing license configuration. + public const string ErrorLicensingView = "error:licensing:view"; + /// + public const string ErrorLicensingManage = "error:licensing:manage"; + + /// Notifications area — viewing, managing, and testing notification settings. + public const string ErrorNotificationsView = "error:notifications:view"; + /// + public const string ErrorNotificationsManage = "error:notifications:manage"; + /// + public const string ErrorNotificationsTest = "error:notifications:test"; + + /// Retry redirects area — viewing and managing message redirect rules. + public const string ErrorRedirectsView = "error:redirects:view"; + /// + public const string ErrorRedirectsManage = "error:redirects:manage"; + + /// Queue addresses area — viewing and deleting queue address entries. + public const string ErrorQueuesView = "error:queues:view"; + /// + public const string ErrorQueuesDelete = "error:queues:delete"; + + /// Throughput area — viewing and managing throughput reports and settings. + public const string ErrorThroughputView = "error:throughput:view"; + /// + public const string ErrorThroughputManage = "error:throughput:manage"; + + /// Platform connections area — viewing and managing broker/platform connection settings. + public const string ErrorConnectionsView = "error:connections:view"; + /// + public const string ErrorConnectionsManage = "error:connections:manage"; + + // ───────────────────────────── Audit instance ───────────────────────────── + + /// Audit instance (separate process) — read-only audit message log. + public const string AuditMessageView = "audit:message:view"; + /// Audit instance — viewing platform connection details. + public const string AuditConnectionView = "audit:connection:view"; + /// Audit instance — viewing known endpoints. + public const string AuditEndpointView = "audit:endpoint:view"; + /// Audit instance — viewing saga audit data. + public const string AuditSagaView = "audit:saga:view"; + + // ───────────────────────────── Monitoring instance ───────────────────────────── + + /// Monitoring instance (separate process) — viewing endpoint metrics. + public const string MonitoringEndpointView = "monitoring:endpoint:view"; + /// Monitoring instance — removing a monitored endpoint instance. + public const string MonitoringEndpointDelete = "monitoring:endpoint:delete"; + /// Monitoring instance — viewing platform connection details. + public const string MonitoringConnectionView = "monitoring:connection:view"; + /// Monitoring instance — viewing license status. + public const string MonitoringLicenseView = "monitoring:license:view"; + + /// + /// The complete set of known permissions, derived from all public const string + /// fields declared on this class. Used by the policy provider and coverage tests. + /// + public static readonly IReadOnlySet All = BuildAll(); + + static IReadOnlySet BuildAll() + { + var set = new HashSet(); + foreach (var field in typeof(Permissions).GetFields(BindingFlags.Public | BindingFlags.Static)) + { + if (field.IsLiteral && !field.IsInitOnly && field.FieldType == typeof(string)) + { + var value = (string?)field.GetValue(null); + if (value != null) + { + set.Add(value); + } + } + } + return set; + } +} diff --git a/src/ServiceControl.Monitoring/Connection/ConnectionController.cs b/src/ServiceControl.Monitoring/Connection/ConnectionController.cs index e765007b73..28978672a6 100644 --- a/src/ServiceControl.Monitoring/Connection/ConnectionController.cs +++ b/src/ServiceControl.Monitoring/Connection/ConnectionController.cs @@ -2,8 +2,10 @@ { using System; using System.Text.Json; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using NServiceBus; + using ServiceControl.Infrastructure.Auth; [ApiController] public class ConnectionController(ReceiveAddresses receiveAddresses) : ControllerBase @@ -11,6 +13,7 @@ public class ConnectionController(ReceiveAddresses receiveAddresses) : Controlle readonly string mainInputQueue = receiveAddresses.MainReceiveAddress; readonly TimeSpan defaultInterval = TimeSpan.FromSeconds(1); + [Authorize(Policy = Permissions.MonitoringConnectionView)] [Route("connection")] [HttpGet] public IActionResult GetConnectionDetails() => diff --git a/src/ServiceControl.Monitoring/Http/Diagrams/DiagramApiController.cs b/src/ServiceControl.Monitoring/Http/Diagrams/DiagramApiController.cs index 134bf92716..04bd58ac86 100644 --- a/src/ServiceControl.Monitoring/Http/Diagrams/DiagramApiController.cs +++ b/src/ServiceControl.Monitoring/Http/Diagrams/DiagramApiController.cs @@ -1,21 +1,26 @@ namespace ServiceControl.Monitoring.Http.Diagrams { using Infrastructure.Api; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; + using ServiceControl.Infrastructure.Auth; [ApiController] public class DiagramApiController(IEndpointMetricsApi endpointMetricsApi) : ControllerBase { + [Authorize(Policy = Permissions.MonitoringEndpointView)] [Route("monitored-endpoints")] [HttpGet] public MonitoredEndpoint[] GetAllEndpointsMetrics([FromQuery] int? history = null) => endpointMetricsApi.GetAllEndpointsMetrics(history); + [Authorize(Policy = Permissions.MonitoringEndpointView)] [Route("monitored-endpoints/{endpointName}")] [HttpGet] public ActionResult GetSingleEndpointMetrics(string endpointName, [FromQuery] int? history = null) => endpointMetricsApi.GetSingleEndpointMetrics(endpointName, history); + [Authorize(Policy = Permissions.MonitoringEndpointDelete)] [Route("monitored-instance/{endpointName}/{instanceId}")] [HttpDelete] public IActionResult DeleteEndpointInstance(string endpointName, string instanceId) @@ -25,6 +30,7 @@ public IActionResult DeleteEndpointInstance(string endpointName, string instance return Ok(); } + [Authorize(Policy = Permissions.MonitoringEndpointView)] [Route("monitored-endpoints/disconnected")] [HttpGet] public ActionResult DisconnectedEndpointCount() => endpointMetricsApi.DisconnectedEndpointCount(); diff --git a/src/ServiceControl.Monitoring/Http/LicenseController.cs b/src/ServiceControl.Monitoring/Http/LicenseController.cs index d76e2e9361..55bcdfbbb5 100644 --- a/src/ServiceControl.Monitoring/Http/LicenseController.cs +++ b/src/ServiceControl.Monitoring/Http/LicenseController.cs @@ -1,11 +1,14 @@ namespace ServiceControl.Monitoring.Http { + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; + using ServiceControl.Infrastructure.Auth; using ServiceControl.Monitoring.Licensing; [ApiController] public class LicenseController(ActiveLicense activeLicense) : ControllerBase { + [Authorize(Policy = Permissions.MonitoringLicenseView)] [Route("license")] [HttpGet] public ActionResult License(bool refresh) diff --git a/src/ServiceControl/CompositeViews/Messages/GetMessages2Controller.cs b/src/ServiceControl/CompositeViews/Messages/GetMessages2Controller.cs index 1ee79cfbbc..eb72656fb1 100644 --- a/src/ServiceControl/CompositeViews/Messages/GetMessages2Controller.cs +++ b/src/ServiceControl/CompositeViews/Messages/GetMessages2Controller.cs @@ -2,7 +2,9 @@ namespace ServiceControl.CompositeViews.Messages; using System.Collections.Generic; using System.Threading.Tasks; +using Infrastructure.Auth; using Infrastructure.WebApi; +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc; using Persistence.Infrastructure; @@ -16,6 +18,7 @@ public class GetMessages2Controller( SearchEndpointApi searchEndpointApi) : ControllerBase { + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("messages2")] [HttpGet] public async Task> Messages( diff --git a/src/ServiceControl/CompositeViews/Messages/GetMessagesByConversationController.cs b/src/ServiceControl/CompositeViews/Messages/GetMessagesByConversationController.cs index 7bd650b453..ab18a6da9e 100644 --- a/src/ServiceControl/CompositeViews/Messages/GetMessagesByConversationController.cs +++ b/src/ServiceControl/CompositeViews/Messages/GetMessagesByConversationController.cs @@ -2,7 +2,9 @@ { using System.Collections.Generic; using System.Threading.Tasks; + using Infrastructure.Auth; using Infrastructure.WebApi; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc; using Persistence.Infrastructure; @@ -12,6 +14,7 @@ public class GetMessagesByConversationController(MessagesByConversationApi byConversationApi) : ControllerBase { + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("conversations/{conversationId:required:minlength(1)}")] [HttpGet] public async Task> Messages([FromQuery] PagingInfo pagingInfo, diff --git a/src/ServiceControl/CompositeViews/Messages/GetMessagesController.cs b/src/ServiceControl/CompositeViews/Messages/GetMessagesController.cs index e7133ba20a..d586fe34cd 100644 --- a/src/ServiceControl/CompositeViews/Messages/GetMessagesController.cs +++ b/src/ServiceControl/CompositeViews/Messages/GetMessagesController.cs @@ -5,8 +5,10 @@ namespace ServiceControl.CompositeViews.Messages using System.Net.Http; using System.Threading.Tasks; using Api.Contracts; + using Infrastructure.Auth; using Infrastructure.WebApi; using MessageCounting; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc; @@ -33,6 +35,7 @@ public class GetMessagesController( ILogger logger) : ControllerBase { + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("messages")] [HttpGet] public async Task> Messages([FromQuery] PagingInfo pagingInfo, @@ -48,6 +51,7 @@ public async Task> Messages([FromQuery] PagingInfo pagingInf return result.Results; } + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("endpoints/{endpoint}/messages")] [HttpGet] public async Task> MessagesForEndpoint([FromQuery] PagingInfo pagingInfo, @@ -64,6 +68,7 @@ public async Task> MessagesForEndpoint([FromQuery] PagingInf } // the endpoint name is needed in the route to match the route and forward it as path and query to the remotes + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("endpoints/{endpoint}/audit-count")] [HttpGet] public async Task> GetEndpointAuditCounts([FromQuery] PagingInfo pagingInfo, string endpoint) @@ -75,6 +80,7 @@ public async Task> GetEndpointAuditCounts([FromQuery] PagingIn return result.Results; } + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("messages/{id}/body")] [HttpGet] public async Task Get(string id, [FromQuery(Name = "instance_id")] string instanceId) @@ -114,6 +120,7 @@ public async Task Get(string id, [FromQuery(Name = "instance_id") return Empty; } + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("messages/search")] [HttpGet] public async Task> Search([FromQuery] PagingInfo pagingInfo, [FromQuery] SortInfo sortInfo, @@ -126,6 +133,7 @@ public async Task> Search([FromQuery] PagingInfo pagingInfo, return result.Results; } + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("messages/search/{keyword}")] [HttpGet] public async Task> SearchByKeyWord([FromQuery] PagingInfo pagingInfo, @@ -139,6 +147,7 @@ public async Task> SearchByKeyWord([FromQuery] PagingInfo pa return result.Results; } + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("endpoints/{endpoint}/messages/search")] [HttpGet] public async Task> Search([FromQuery] PagingInfo pagingInfo, [FromQuery] SortInfo sortInfo, @@ -151,6 +160,7 @@ public async Task> Search([FromQuery] PagingInfo pagingInfo, return result.Results; } + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("endpoints/{endpoint}/messages/search/{keyword}")] [HttpGet] public async Task> SearchByKeyword([FromQuery] PagingInfo pagingInfo, diff --git a/src/ServiceControl/Connection/ConnectionController.cs b/src/ServiceControl/Connection/ConnectionController.cs index a85522a84c..b87d4d8dc7 100644 --- a/src/ServiceControl/Connection/ConnectionController.cs +++ b/src/ServiceControl/Connection/ConnectionController.cs @@ -4,6 +4,8 @@ using System.Text.Json; using System.Text.Json.Serialization; using System.Threading.Tasks; + using Infrastructure.Auth; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; [ApiController] @@ -13,6 +15,7 @@ public class ConnectionController(IPlatformConnectionBuilder builder) : Controll // This controller doesn't use the default serialization settings because // ServicePulse and the Platform Connector Plugin expect the connection // details the be serialized and formatted in a specific way + [Authorize(Policy = Permissions.ErrorConnectionsView)] [Route("connection")] [HttpGet] public async Task GetConnectionDetails() diff --git a/src/ServiceControl/CustomChecks/Web/CustomCheckController.cs b/src/ServiceControl/CustomChecks/Web/CustomCheckController.cs index 0cc06bbd0c..f9fb690757 100644 --- a/src/ServiceControl/CustomChecks/Web/CustomCheckController.cs +++ b/src/ServiceControl/CustomChecks/Web/CustomCheckController.cs @@ -4,7 +4,9 @@ using System.Collections.Generic; using System.Threading.Tasks; using Contracts.CustomChecks; + using Infrastructure.Auth; using Infrastructure.WebApi; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using NServiceBus; using ServiceControl.Persistence; @@ -15,6 +17,7 @@ public class CustomCheckController(ICustomChecksDataStore checksDataStore, IMessageSession session) : ControllerBase { + [Authorize(Policy = Permissions.ErrorCustomChecksView)] [Route("customchecks")] [HttpGet] public async Task> CustomChecks([FromQuery] PagingInfo pagingInfo, string status = null) @@ -27,6 +30,7 @@ public async Task> CustomChecks([FromQuery] PagingInfo paging return stats.Results; } + [Authorize(Policy = Permissions.ErrorCustomChecksDelete)] [Route("customchecks/{id}")] [HttpDelete] public async Task Delete(Guid id) diff --git a/src/ServiceControl/EventLog/EventLogApiController.cs b/src/ServiceControl/EventLog/EventLogApiController.cs index 1872154be7..605efc453d 100644 --- a/src/ServiceControl/EventLog/EventLogApiController.cs +++ b/src/ServiceControl/EventLog/EventLogApiController.cs @@ -2,7 +2,9 @@ { using System.Collections.Generic; using System.Threading.Tasks; + using Infrastructure.Auth; using Infrastructure.WebApi; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Persistence.Infrastructure; using ServiceControl.Persistence; @@ -11,6 +13,7 @@ [Route("api")] public class EventLogApiController(IEventLogDataStore logDataStore) : ControllerBase { + [Authorize(Policy = Permissions.ErrorEventLogView)] [Route("eventlogitems")] [HttpGet] public async Task> Items([FromQuery] PagingInfo pagingInfo) diff --git a/src/ServiceControl/Licensing/LicenseController.cs b/src/ServiceControl/Licensing/LicenseController.cs index 15539d9db0..9f9e56cb6e 100644 --- a/src/ServiceControl/Licensing/LicenseController.cs +++ b/src/ServiceControl/Licensing/LicenseController.cs @@ -2,6 +2,8 @@ { using System.Threading; using System.Threading.Tasks; + using Infrastructure.Auth; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Monitoring.HeartbeatMonitoring; using Particular.ServiceControl.Licensing; @@ -11,6 +13,7 @@ [Route("api")] public class LicenseController(ActiveLicense activeLicense, Settings settings, MassTransitConnectorHeartbeatStatus connectorHeartbeatStatus) : ControllerBase { + [Authorize(Policy = Permissions.ErrorLicensingView)] [HttpGet] [Route("license")] public async Task> License(bool refresh, string clientName, CancellationToken cancellationToken) diff --git a/src/ServiceControl/MessageFailures/Api/ArchiveMessagesController.cs b/src/ServiceControl/MessageFailures/Api/ArchiveMessagesController.cs index bad1ec4cf5..84384bec6f 100644 --- a/src/ServiceControl/MessageFailures/Api/ArchiveMessagesController.cs +++ b/src/ServiceControl/MessageFailures/Api/ArchiveMessagesController.cs @@ -2,8 +2,10 @@ namespace ServiceControl.MessageFailures.Api { using System.Linq; using System.Threading.Tasks; + using Infrastructure.Auth; using Infrastructure.WebApi; using InternalMessages; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using NServiceBus; using ServiceControl.Persistence; @@ -13,6 +15,7 @@ namespace ServiceControl.MessageFailures.Api [Route("api")] public class ArchiveMessagesController(IMessageSession messageSession, IErrorMessageDataStore dataStore) : ControllerBase { + [Authorize(Policy = Permissions.ErrorMessagesArchive)] [Route("errors/archive")] [HttpPost] [HttpPatch] @@ -34,6 +37,7 @@ public async Task ArchiveBatch(string[] messageIds) return Accepted(); } + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("errors/groups/{classifier?}")] [HttpGet] public async Task GetArchiveMessageGroups(string classifier = "Exception Type and Stack Trace") @@ -45,6 +49,7 @@ public async Task GetArchiveMessageGroups(string classifier = "Ex return Ok(results); } + [Authorize(Policy = Permissions.ErrorMessagesArchive)] [Route("errors/{messageId:required:minlength(1)}/archive")] [HttpPost] [HttpPatch] @@ -55,6 +60,7 @@ public async Task Archive(string messageId) return Accepted(); } + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("archive/groups/id/{groupId:required:minlength(1)}")] [HttpGet] public async Task> GetGroup(string groupId, string status = default, string modified = default) diff --git a/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs b/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs index 3ce42fc3ff..d5f5d587f6 100644 --- a/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs +++ b/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs @@ -5,6 +5,8 @@ using System.Linq; using System.Text; using System.Threading.Tasks; + using Infrastructure.Auth; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; using NServiceBus; @@ -21,10 +23,12 @@ public class EditFailedMessagesController( ILogger logger) : ControllerBase { + [Authorize(Policy = Permissions.ErrorMessagesEdit)] [Route("edit/config")] [HttpGet] public EditConfigurationModel Config() => GetEditConfiguration(); + [Authorize(Policy = Permissions.ErrorMessagesEdit)] [Route("edit/{failedMessageId:required:minlength(1)}")] [HttpPost] public async Task> Edit(string failedMessageId, [FromBody] EditMessageModel edit) diff --git a/src/ServiceControl/MessageFailures/Api/GetAllErrorsController.cs b/src/ServiceControl/MessageFailures/Api/GetAllErrorsController.cs index 60f9f08ca9..06c7260d26 100644 --- a/src/ServiceControl/MessageFailures/Api/GetAllErrorsController.cs +++ b/src/ServiceControl/MessageFailures/Api/GetAllErrorsController.cs @@ -2,7 +2,9 @@ { using System.Collections.Generic; using System.Threading.Tasks; + using Infrastructure.Auth; using Infrastructure.WebApi; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Persistence.Infrastructure; using ServiceControl.Persistence; @@ -11,6 +13,7 @@ [Route("api")] public class GetAllErrorsController(IErrorMessageDataStore store) : ControllerBase { + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("errors")] [HttpGet] public async Task> ErrorsGet([FromQuery] PagingInfo pagingInfo, [FromQuery] SortInfo sortInfo, string status, string modified, string queueAddress) @@ -28,6 +31,7 @@ public async Task> ErrorsGet([FromQuery] PagingInfo pag return results.Results; } + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("errors")] [HttpHead] public async Task ErrorsHead(string status, string modified, string queueAddress) @@ -41,6 +45,7 @@ public async Task ErrorsHead(string status, string modified, string queueAddress Response.WithQueryStatsInfo(queryResult); } + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("endpoints/{endpointname}/errors")] [HttpGet] public async Task> ErrorsByEndpointName([FromQuery] PagingInfo pagingInfo, [FromQuery] SortInfo sortInfo, string status, string modified, string endpointName) @@ -58,6 +63,7 @@ public async Task> ErrorsByEndpointName([FromQuery] Pag return results.Results; } + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("errors/summary")] [HttpGet] public async Task> ErrorsSummary() => await store.ErrorsSummary(); diff --git a/src/ServiceControl/MessageFailures/Api/GetErrorByIdController.cs b/src/ServiceControl/MessageFailures/Api/GetErrorByIdController.cs index 437b6fc5a3..bb419a014c 100644 --- a/src/ServiceControl/MessageFailures/Api/GetErrorByIdController.cs +++ b/src/ServiceControl/MessageFailures/Api/GetErrorByIdController.cs @@ -1,6 +1,8 @@ namespace ServiceControl.MessageFailures.Api { using System.Threading.Tasks; + using Infrastructure.Auth; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Persistence; @@ -8,6 +10,7 @@ [Route("api")] public class GetErrorByIdController(IErrorMessageDataStore store) : ControllerBase { + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("errors/{failedMessageId:required:minlength(1)}")] [HttpGet] public async Task> ErrorBy(string failedMessageId) @@ -17,6 +20,7 @@ public async Task> ErrorBy(string failedMessageId) return result == null ? NotFound() : result; } + [Authorize(Policy = Permissions.ErrorMessagesView)] [Route("errors/last/{failedMessageId:required:minlength(1)}")] [HttpGet] public async Task> ErrorLastBy(string failedMessageId) diff --git a/src/ServiceControl/MessageFailures/Api/PendingRetryMessagesController.cs b/src/ServiceControl/MessageFailures/Api/PendingRetryMessagesController.cs index 611ee01e5c..1ad75d3bee 100644 --- a/src/ServiceControl/MessageFailures/Api/PendingRetryMessagesController.cs +++ b/src/ServiceControl/MessageFailures/Api/PendingRetryMessagesController.cs @@ -5,7 +5,9 @@ using System.Linq; using System.Text.Json.Serialization; using System.Threading.Tasks; + using Infrastructure.Auth; using InternalMessages; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using NServiceBus; @@ -13,6 +15,7 @@ [Route("api")] public class PendingRetryMessagesController(IMessageSession session) : ControllerBase { + [Authorize(Policy = Permissions.ErrorMessagesRetry)] [Route("pendingretries/retry")] [HttpPost] public async Task RetryBy(string[] ids) @@ -28,6 +31,7 @@ public async Task RetryBy(string[] ids) return Accepted(); } + [Authorize(Policy = Permissions.ErrorMessagesRetry)] [Route("pendingretries/queues/retry")] [HttpPost] public async Task RetryBy(PendingRetryRequest request) diff --git a/src/ServiceControl/MessageFailures/Api/QueueAddressController.cs b/src/ServiceControl/MessageFailures/Api/QueueAddressController.cs index 9e01c66e15..44e043426c 100644 --- a/src/ServiceControl/MessageFailures/Api/QueueAddressController.cs +++ b/src/ServiceControl/MessageFailures/Api/QueueAddressController.cs @@ -2,7 +2,9 @@ { using System.Collections.Generic; using System.Threading.Tasks; + using Infrastructure.Auth; using Infrastructure.WebApi; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Persistence.Infrastructure; using ServiceControl.Persistence; @@ -11,6 +13,7 @@ [Route("api")] public class QueueAddressController(IQueueAddressStore store) : ControllerBase { + [Authorize(Policy = Permissions.ErrorQueuesView)] [Route("errors/queues/addresses")] [HttpGet] public async Task> GetAddresses([FromQuery] PagingInfo pagingInfo) @@ -22,6 +25,7 @@ public async Task> GetAddresses([FromQuery] PagingInfo pagin return result.Results; } + [Authorize(Policy = Permissions.ErrorQueuesView)] [Route("errors/queues/addresses/search/{search}")] [HttpGet] public async Task>> GetAddressesBySearchTerm([FromQuery] PagingInfo pagingInfo, string search = null) diff --git a/src/ServiceControl/MessageFailures/Api/ResolveMessagesController.cs b/src/ServiceControl/MessageFailures/Api/ResolveMessagesController.cs index dc4eddf431..21c05d2cc1 100644 --- a/src/ServiceControl/MessageFailures/Api/ResolveMessagesController.cs +++ b/src/ServiceControl/MessageFailures/Api/ResolveMessagesController.cs @@ -8,7 +8,9 @@ namespace ServiceControl.MessageFailures.Api using System.Linq; using System.Text.Json.Serialization; using System.Threading.Tasks; + using Infrastructure.Auth; using InternalMessages; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.ModelBinding; using NServiceBus; @@ -17,6 +19,7 @@ namespace ServiceControl.MessageFailures.Api [Route("api")] public class ResolveMessagesController(IMessageSession session) : ControllerBase { + [Authorize(Policy = Permissions.ErrorMessagesRetry)] [Route("pendingretries/resolve")] [HttpPatch] public async Task ResolveBy(UniqueMessageIdsModel request) @@ -61,6 +64,7 @@ await session.SendLocal(m => return Accepted(); } + [Authorize(Policy = Permissions.ErrorMessagesRetry)] [Route("pendingretries/queues/resolve")] [HttpPatch] public async Task ResolveBy(QueueModel queueModel) diff --git a/src/ServiceControl/MessageFailures/Api/RetryMessagesController.cs b/src/ServiceControl/MessageFailures/Api/RetryMessagesController.cs index c486129df9..29fb12966c 100644 --- a/src/ServiceControl/MessageFailures/Api/RetryMessagesController.cs +++ b/src/ServiceControl/MessageFailures/Api/RetryMessagesController.cs @@ -4,7 +4,9 @@ using System.Linq; using System.Net.Http; using System.Threading.Tasks; + using Infrastructure.Auth; using InternalMessages; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc; @@ -23,6 +25,7 @@ public class RetryMessagesController( IMessageSession messageSession, ILogger logger) : ControllerBase { + [Authorize(Policy = Permissions.ErrorMessagesRetry)] [Route("errors/{failedMessageId:required:minlength(1)}/retry")] [HttpPost] public async Task RetryMessageBy([FromQuery(Name = "instance_id")] string instanceId, string failedMessageId) @@ -49,6 +52,7 @@ public async Task RetryMessageBy([FromQuery(Name = "instance_id") return Empty; } + [Authorize(Policy = Permissions.ErrorMessagesRetry)] [Route("errors/retry")] [HttpPost] public async Task RetryAllBy(List messageIds) @@ -63,6 +67,7 @@ public async Task RetryAllBy(List messageIds) return Accepted(); } + [Authorize(Policy = Permissions.ErrorMessagesRetry)] [Route("errors/queues/{queueAddress:required:minlength(1)}/retry")] [HttpPost] public async Task RetryAllBy(string queueAddress) @@ -76,6 +81,7 @@ await messageSession.SendLocal(m => return Accepted(); } + [Authorize(Policy = Permissions.ErrorMessagesRetry)] [Route("errors/retry/all")] [HttpPost] public async Task RetryAll() @@ -85,6 +91,7 @@ public async Task RetryAll() return Accepted(); } + [Authorize(Policy = Permissions.ErrorMessagesRetry)] [Route("errors/{endpointName:required:minlength(1)}/retry/all")] [HttpPost] public async Task RetryAllByEndpoint(string endpointName) diff --git a/src/ServiceControl/MessageFailures/Api/UnArchiveMessagesController.cs b/src/ServiceControl/MessageFailures/Api/UnArchiveMessagesController.cs index 0a0271d9fa..d36940bc99 100644 --- a/src/ServiceControl/MessageFailures/Api/UnArchiveMessagesController.cs +++ b/src/ServiceControl/MessageFailures/Api/UnArchiveMessagesController.cs @@ -4,7 +4,9 @@ using System.Globalization; using System.Linq; using System.Threading.Tasks; + using Infrastructure.Auth; using InternalMessages; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using NServiceBus; @@ -12,6 +14,7 @@ [Route("api")] public class UnArchiveMessagesController(IMessageSession session) : ControllerBase { + [Authorize(Policy = Permissions.ErrorMessagesUnarchive)] [Route("errors/unarchive")] [HttpPatch] public async Task Unarchive(string[] ids) @@ -28,6 +31,7 @@ public async Task Unarchive(string[] ids) return Accepted(); } + [Authorize(Policy = Permissions.ErrorMessagesUnarchive)] [Route("errors/{from}...{to}/unarchive")] [HttpPatch] public async Task Unarchive(string from, string to) diff --git a/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs b/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs index 4899498683..2eed6206cc 100644 --- a/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs +++ b/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs @@ -7,9 +7,11 @@ using System.Text.Json.Serialization; using System.Threading.Tasks; using Contracts.MessageRedirects; + using Infrastructure.Auth; using Infrastructure.DomainEvents; using Infrastructure.WebApi; using MessageFailures.InternalMessages; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using NServiceBus; using ServiceControl.Persistence.Infrastructure; @@ -25,6 +27,7 @@ public class MessageRedirectsController( IDomainEvents events) : ControllerBase { + [Authorize(Policy = Permissions.ErrorRedirectsManage)] [Route("redirects")] [HttpPost] public async Task NewRedirects(MessageRedirectRequest request) @@ -93,6 +96,7 @@ await session.SendLocal(new RetryPendingMessages return StatusCode((int)HttpStatusCode.Created, messageRedirect); } + [Authorize(Policy = Permissions.ErrorRedirectsManage)] [Route("redirects/{messageRedirectId:guid}")] [HttpPut] public async Task UpdateRedirect(Guid messageRedirectId, MessageRedirectRequest request) @@ -135,6 +139,7 @@ public async Task UpdateRedirect(Guid messageRedirectId, MessageR return NoContent(); } + [Authorize(Policy = Permissions.ErrorRedirectsManage)] [Route("redirects/{messageRedirectId:guid}")] [HttpDelete] public async Task DeleteRedirect(Guid messageRedirectId) @@ -162,6 +167,7 @@ await events.Raise(new MessageRedirectRemoved return NoContent(); } + [Authorize(Policy = Permissions.ErrorRedirectsView)] [Route("redirect")] [HttpHead] public async Task CountRedirects() @@ -172,6 +178,7 @@ public async Task CountRedirects() Response.WithTotalCount(redirects.Redirects.Count); } + [Authorize(Policy = Permissions.ErrorRedirectsView)] [Route("redirects")] [HttpGet] public async Task> Redirects(string sort, string direction, [FromQuery] PagingInfo pagingInfo) diff --git a/src/ServiceControl/Monitoring/Web/EndpointsMonitoringController.cs b/src/ServiceControl/Monitoring/Web/EndpointsMonitoringController.cs index 5b64d5a22d..0926075b59 100644 --- a/src/ServiceControl/Monitoring/Web/EndpointsMonitoringController.cs +++ b/src/ServiceControl/Monitoring/Web/EndpointsMonitoringController.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Threading.Tasks; using CompositeViews.Messages; + using Infrastructure.Auth; using Infrastructure.WebApi; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http.Extensions; @@ -25,10 +26,12 @@ public class EndpointsMonitoringController( IMonitoringDataStore dataStore) : ControllerBase { + [Authorize(Policy = Permissions.ErrorHeartbeatsView)] [Route("heartbeats/stats")] [HttpGet] public EndpointMonitoringStats HeartbeatStats() => monitoring.GetStats(); + [Authorize(Policy = Permissions.ErrorEndpointsView)] [Route("endpoints")] [HttpGet] public EndpointsView[] Endpoints() => monitoring.GetEndpoints(); @@ -44,6 +47,7 @@ public void GetSupportedOperations() Response.Headers.AccessControlExposeHeaders = "Allow"; } + [Authorize(Policy = Permissions.ErrorEndpointsDelete)] [Route("endpoints/{endpointId}")] [HttpDelete] public async Task DeleteEndpoint(Guid endpointId) @@ -59,6 +63,7 @@ public async Task DeleteEndpoint(Guid endpointId) return NoContent(); } + [Authorize(Policy = Permissions.ErrorEndpointsView)] [Route("endpoints/known")] [HttpGet] public async Task> KnownEndpoints([FromQuery] PagingInfo pagingInfo) @@ -70,6 +75,7 @@ public async Task> KnownEndpoints([FromQuery] PagingIn return result.Results; } + [Authorize(Policy = Permissions.ErrorEndpointsManage)] [Route("endpoints/{endpointId}")] [HttpPatch] public async Task Monitoring(Guid endpointId, [FromBody] EndpointUpdateModel data) diff --git a/src/ServiceControl/Monitoring/Web/EndpointsSettingsController.cs b/src/ServiceControl/Monitoring/Web/EndpointsSettingsController.cs index be1807e54c..a1d55138db 100644 --- a/src/ServiceControl/Monitoring/Web/EndpointsSettingsController.cs +++ b/src/ServiceControl/Monitoring/Web/EndpointsSettingsController.cs @@ -4,6 +4,8 @@ using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; +using Infrastructure.Auth; +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Persistence; using ServiceBus.Management.Infrastructure.Settings; @@ -25,6 +27,7 @@ public class EndpointsSettingsController( IEndpointSettingsStore dataStore, Settings settings) : ControllerBase { + [Authorize(Policy = Permissions.ErrorEndpointsView)] [Route("endpointssettings")] [HttpGet] public async IAsyncEnumerable Endpoints([EnumeratorCancellation] CancellationToken token) @@ -49,6 +52,7 @@ public async IAsyncEnumerable Endpoints([EnumeratorCancellation] C } } + [Authorize(Policy = Permissions.ErrorEndpointsManage)] [Route("endpointssettings/{endpointName?}")] [HttpPatch] public async Task diff --git a/src/ServiceControl/Notifications/Api/NotificationsController.cs b/src/ServiceControl/Notifications/Api/NotificationsController.cs index af5f90cfe0..e42cfa5473 100644 --- a/src/ServiceControl/Notifications/Api/NotificationsController.cs +++ b/src/ServiceControl/Notifications/Api/NotificationsController.cs @@ -4,6 +4,8 @@ using System.Net; using System.Threading.Tasks; using Email; + using Infrastructure.Auth; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Persistence; using ServiceBus.Management.Infrastructure.Settings; @@ -12,6 +14,7 @@ [Route("api")] public class NotificationsController(IErrorMessageDataStore store, Settings settings, EmailSender emailSender) : ControllerBase { + [Authorize(Policy = Permissions.ErrorNotificationsView)] [Route("notifications/email")] [HttpGet] public async Task GetEmailNotificationsSettings() @@ -22,6 +25,7 @@ public async Task GetEmailNotificationsSettings() return notificationsSettings.Email; } + [Authorize(Policy = Permissions.ErrorNotificationsManage)] [Route("notifications/email/toggle")] [HttpPost] public async Task ToggleEmailNotifications(ToggleEmailNotifications request) @@ -36,6 +40,7 @@ public async Task ToggleEmailNotifications(ToggleEmailNotificatio return Ok(); } + [Authorize(Policy = Permissions.ErrorNotificationsManage)] [Route("notifications/email")] [HttpPost] public async Task UpdateSettings(UpdateEmailNotificationsSettingsRequest request) @@ -60,6 +65,7 @@ public async Task UpdateSettings(UpdateEmailNotificationsSettings return Ok(); } + [Authorize(Policy = Permissions.ErrorNotificationsTest)] [Route("notifications/email/test")] [HttpPost] public async Task SendTestEmail() diff --git a/src/ServiceControl/Recoverability/API/FailureGroupsArchiveController.cs b/src/ServiceControl/Recoverability/API/FailureGroupsArchiveController.cs index f40611db85..69c805f199 100644 --- a/src/ServiceControl/Recoverability/API/FailureGroupsArchiveController.cs +++ b/src/ServiceControl/Recoverability/API/FailureGroupsArchiveController.cs @@ -1,6 +1,8 @@ namespace ServiceControl.Recoverability.API { using System.Threading.Tasks; + using Infrastructure.Auth; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using NServiceBus; using ServiceControl.Persistence.Recoverability; @@ -9,6 +11,7 @@ [Route("api")] public class FailureGroupsArchiveController(IMessageSession bus, IArchiveMessages archiver) : ControllerBase { + [Authorize(Policy = Permissions.ErrorRecoverabilityGroupsArchive)] [Route("recoverability/groups/{groupId:required:minlength(1)}/errors/archive")] [HttpPost] public async Task ArchiveGroupErrors(string groupId) diff --git a/src/ServiceControl/Recoverability/API/FailureGroupsController.cs b/src/ServiceControl/Recoverability/API/FailureGroupsController.cs index b03b2b702d..7a3964f9cb 100644 --- a/src/ServiceControl/Recoverability/API/FailureGroupsController.cs +++ b/src/ServiceControl/Recoverability/API/FailureGroupsController.cs @@ -3,8 +3,10 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; + using Infrastructure.Auth; using Infrastructure.WebApi; using MessageFailures.Api; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Persistence.Infrastructure; using ServiceControl.Persistence; @@ -18,6 +20,7 @@ public class FailureGroupsController( IRetryHistoryDataStore retryStore) : ControllerBase { + [Authorize(Policy = Permissions.ErrorRecoverabilityGroupsView)] [Route("recoverability/classifiers")] [HttpGet] public string[] GetSupportedClassifiers() @@ -32,6 +35,7 @@ public string[] GetSupportedClassifiers() return result; } + [Authorize(Policy = Permissions.ErrorRecoverabilityGroupsView)] [Route("recoverability/groups/{groupId:required:minlength(1)}/comment")] [HttpPost] public async Task EditComment(string groupId, string comment) @@ -41,6 +45,7 @@ public async Task EditComment(string groupId, string comment) return Accepted(); } + [Authorize(Policy = Permissions.ErrorRecoverabilityGroupsView)] [Route("recoverability/groups/{groupId:required:minlength(1)}/comment")] [HttpDelete] public async Task DeleteComment(string groupId) @@ -50,6 +55,7 @@ public async Task DeleteComment(string groupId) return Accepted(); } + [Authorize(Policy = Permissions.ErrorRecoverabilityGroupsView)] [Route("recoverability/groups/{classifier?}")] [HttpGet] public async Task GetAllGroups(string classifier = "Exception Type and Stack Trace", string classifierFilter = default) @@ -64,6 +70,7 @@ public async Task GetAllGroups(string classifier = "Exception return results; } + [Authorize(Policy = Permissions.ErrorRecoverabilityGroupsView)] [Route("recoverability/groups/{groupId:required:minlength(1)}/errors")] [HttpGet] public async Task> GetGroupErrors(string groupId, [FromQuery] SortInfo sortInfo, [FromQuery] PagingInfo pagingInfo, string status = default, string modified = default) @@ -75,6 +82,7 @@ public async Task> GetGroupErrors(string groupId, [From } + [Authorize(Policy = Permissions.ErrorRecoverabilityGroupsView)] [Route("recoverability/groups/{groupId:required:minlength(1)}/errors")] [HttpHead] public async Task GetGroupErrorsCount(string groupId, string status = default, string modified = default) @@ -84,6 +92,7 @@ public async Task GetGroupErrorsCount(string groupId, string status = default, s Response.WithQueryStatsInfo(results); } + [Authorize(Policy = Permissions.ErrorRecoverabilityGroupsView)] [Route("recoverability/history")] [HttpGet] public async Task GetRetryHistory() @@ -95,6 +104,7 @@ public async Task GetRetryHistory() return retryHistory; } + [Authorize(Policy = Permissions.ErrorRecoverabilityGroupsView)] [Route("recoverability/groups/id/{groupId:required:minlength(1)}")] [HttpGet] public async Task GetGroup(string groupId, string status = default, string modified = default) diff --git a/src/ServiceControl/Recoverability/API/FailureGroupsRetryController.cs b/src/ServiceControl/Recoverability/API/FailureGroupsRetryController.cs index 308d427217..8ef2ec86d2 100644 --- a/src/ServiceControl/Recoverability/API/FailureGroupsRetryController.cs +++ b/src/ServiceControl/Recoverability/API/FailureGroupsRetryController.cs @@ -2,6 +2,8 @@ namespace ServiceControl.Recoverability.API { using System; using System.Threading.Tasks; + using Infrastructure.Auth; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using NServiceBus; using ServiceControl.Persistence; @@ -10,6 +12,7 @@ namespace ServiceControl.Recoverability.API [Route("api")] public class FailureGroupsRetryController(IMessageSession bus, RetryingManager retryingManager) : ControllerBase { + [Authorize(Policy = Permissions.ErrorRecoverabilityGroupsRetry)] [Route("recoverability/groups/{groupId:required:minlength(1)}/errors/retry")] [HttpPost] public async Task ArchiveGroupErrors(string groupId) diff --git a/src/ServiceControl/Recoverability/API/FailureGroupsUnarchiveController.cs b/src/ServiceControl/Recoverability/API/FailureGroupsUnarchiveController.cs index 5661c8dd78..37e2f0b6d9 100644 --- a/src/ServiceControl/Recoverability/API/FailureGroupsUnarchiveController.cs +++ b/src/ServiceControl/Recoverability/API/FailureGroupsUnarchiveController.cs @@ -1,6 +1,8 @@ namespace ServiceControl.Recoverability.API { using System.Threading.Tasks; + using Infrastructure.Auth; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using NServiceBus; using ServiceControl.Persistence.Recoverability; @@ -9,6 +11,7 @@ [Route("api")] public class FailureGroupsUnarchiveController(IMessageSession bus, IArchiveMessages archiver) : ControllerBase { + [Authorize(Policy = Permissions.ErrorRecoverabilityGroupsUnarchive)] [Route("recoverability/groups/{groupId:required:minlength(1)}/errors/unarchive")] [HttpPost] public async Task UnarchiveGroupErrors(string groupId) diff --git a/src/ServiceControl/Recoverability/API/UnacknowledgedGroupsController.cs b/src/ServiceControl/Recoverability/API/UnacknowledgedGroupsController.cs index 2e85be691f..9be6bec2b2 100644 --- a/src/ServiceControl/Recoverability/API/UnacknowledgedGroupsController.cs +++ b/src/ServiceControl/Recoverability/API/UnacknowledgedGroupsController.cs @@ -1,6 +1,8 @@ namespace ServiceControl.Recoverability.API { using System.Threading.Tasks; + using Infrastructure.Auth; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using ServiceControl.Persistence; using ServiceControl.Persistence.Recoverability; @@ -9,6 +11,7 @@ [Route("api")] public class UnacknowledgedGroupsController(IRetryHistoryDataStore retryStore, IArchiveMessages archiver) : ControllerBase { + [Authorize(Policy = Permissions.ErrorRecoverabilityGroupsView)] [Route("recoverability/unacknowledgedgroups/{groupId:required:minlength(1)}")] [HttpDelete] public async Task AcknowledgeOperation(string groupId) diff --git a/src/ServiceControl/SagaAudit/SagasController.cs b/src/ServiceControl/SagaAudit/SagasController.cs index 9b581b9758..d3c082de8f 100644 --- a/src/ServiceControl/SagaAudit/SagasController.cs +++ b/src/ServiceControl/SagaAudit/SagasController.cs @@ -2,7 +2,9 @@ namespace ServiceControl.SagaAudit { using System; using System.Threading.Tasks; + using Infrastructure.Auth; using Infrastructure.WebApi; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc; using Persistence.Infrastructure; @@ -11,6 +13,7 @@ namespace ServiceControl.SagaAudit [Route("api")] public class SagasController(GetSagaByIdApi getSagaByIdApi) : ControllerBase { + [Authorize(Policy = Permissions.ErrorSagasView)] [Route("sagas/{id}")] [HttpGet] public async Task Sagas([FromQuery] PagingInfo pagingInfo, Guid id) From e1082ad1a3779aecac907be40d2048ceedb84dc8 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Thu, 4 Jun 2026 14:00:11 +0200 Subject: [PATCH 02/14] 1st pass of registering an authorization provider --- .../Hosting/Commands/RunCommand.cs | 1 + .../Auth/PermissionAuthorizationExtensions.cs | 43 ++++++ .../Auth/PermissionPolicyProvider.cs | 59 ++++++++ .../Auth/PermissionRequirement.cs | 11 ++ .../Auth/PermissionVerbHandler.cs | 44 ++++++ .../Auth/RolePermissions.cs | 127 ++++++++++++++++++ .../Hosting/Commands/RunCommand.cs | 1 + .../Hosting/Commands/RunCommand.cs | 1 + 8 files changed, 287 insertions(+) create mode 100644 src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs create mode 100644 src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs create mode 100644 src/ServiceControl.Hosting/Auth/PermissionRequirement.cs create mode 100644 src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs create mode 100644 src/ServiceControl.Infrastructure/Auth/RolePermissions.cs diff --git a/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/RunCommand.cs b/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/RunCommand.cs index 22e2fff776..d8229e8774 100644 --- a/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/RunCommand.cs +++ b/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/RunCommand.cs @@ -19,6 +19,7 @@ public override async Task Execute(HostArguments args, Settings settings) var hostBuilder = WebApplication.CreateBuilder(); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); hostBuilder.AddServiceControlHttps(settings.HttpsSettings); hostBuilder.AddServiceControlAudit((_, __) => { diff --git a/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs b/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs new file mode 100644 index 0000000000..07229849eb --- /dev/null +++ b/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs @@ -0,0 +1,43 @@ +#nullable enable +namespace ServiceControl.Hosting.Auth; + +using Microsoft.AspNetCore.Authorization; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Options; + +/// +/// Registers the permission-based policy authorization services: a dynamic +/// that resolves [Authorize(Policy = "<permission>")] +/// attributes, and — when OIDC is enabled — the that evaluates them +/// against the user's roles. +/// +/// The provider is registered unconditionally so the policy attributes resolve in every configuration +/// (without it, annotated endpoints fail with "AuthorizationPolicy not found"). When OIDC is disabled the +/// provider returns allow-all policies that carry no requirement, so the verb handler is not registered. +/// Wire this into every instance that hosts annotated controllers (Error, Audit, Monitoring). +/// +/// +public static class PermissionAuthorizationExtensions +{ + public static void AddServiceControlAuthorization(this IHostApplicationBuilder hostBuilder, bool oidcEnabled) + { + var services = hostBuilder.Services; + + // Ensure the authorization core services and options are present (idempotent). + services.AddAuthorization(); + + // Resolve permission policy names dynamically. Registered last so it supersedes the default + // policy provider registered by AddAuthorization(). When OIDC is disabled it returns allow-all + // policies (no requirement); when enabled it emits a PermissionRequirement for the verb handler. + services.AddSingleton(sp => + new PermissionPolicyProvider(sp.GetRequiredService>(), oidcEnabled)); + + // The role-based handler is only needed when OIDC is enabled — otherwise the provider produces + // no PermissionRequirement for it to evaluate. + if (oidcEnabled) + { + services.AddSingleton(); + } + } +} diff --git a/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs b/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs new file mode 100644 index 0000000000..1d5a52d4ff --- /dev/null +++ b/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs @@ -0,0 +1,59 @@ +#nullable enable +namespace ServiceControl.Hosting.Auth; + +using System; +using System.Collections.Frozen; +using System.Collections.Generic; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authorization; +using Microsoft.Extensions.Options; +using ServiceControl.Infrastructure.Auth; + +/// +/// A dynamic that resolves a verb-level authorization policy +/// for each known permission string (e.g. error:messages:retry). +/// +/// The set of valid policy names is known up front (), so every policy is +/// built once at construction into a . The framework +/// calls on every request to a protected endpoint, so this makes that call +/// an O(1) lookup with no per-request policy allocation. (Authorization policies and requirements are +/// immutable, so the prebuilt instances are safely shared across all requests.) +/// +/// +/// When OIDC is enabled each permission maps to a policy carrying a +/// (evaluated by ). When OIDC is disabled the platform runs +/// unauthenticated, so every permission maps to a shared allow-all policy — no requirement, no handler. +/// Unknown policy names resolve to ; the default and fallback policies are +/// delegated to the configured . +/// +/// +public sealed class PermissionPolicyProvider(IOptions authorizationOptions, bool oidcEnabled) + : IAuthorizationPolicyProvider +{ + // Carries no requirement, so it succeeds without any IAuthorizationHandler being registered. + static readonly AuthorizationPolicy AllowAll = + new AuthorizationPolicyBuilder().RequireAssertion(_ => true).Build(); + + readonly FrozenDictionary policies = BuildPolicies(oidcEnabled); + + static FrozenDictionary BuildPolicies(bool oidcEnabled) => + Permissions.All.ToFrozenDictionary( + permission => permission, + permission => oidcEnabled + ? new AuthorizationPolicyBuilder().AddRequirements(new PermissionRequirement(permission)).Build() + : AllowAll, + StringComparer.Ordinal); + + public Task GetPolicyAsync(string policyName) => + Task.FromResult(policies.GetValueOrDefault(policyName)); + + public Task GetDefaultPolicyAsync() + { + var defaultPolicy = authorizationOptions.Value.DefaultPolicy + ?? new AuthorizationPolicyBuilder().RequireAuthenticatedUser().Build(); + return Task.FromResult(defaultPolicy); + } + + public Task GetFallbackPolicyAsync() + => Task.FromResult(authorizationOptions.Value.FallbackPolicy); +} diff --git a/src/ServiceControl.Hosting/Auth/PermissionRequirement.cs b/src/ServiceControl.Hosting/Auth/PermissionRequirement.cs new file mode 100644 index 0000000000..77039457b7 --- /dev/null +++ b/src/ServiceControl.Hosting/Auth/PermissionRequirement.cs @@ -0,0 +1,11 @@ +#nullable enable +namespace ServiceControl.Hosting.Auth; + +using Microsoft.AspNetCore.Authorization; + +/// +/// An that carries the permission string enforced by a +/// [Authorize(Policy = "<permission>")] attribute (e.g. error:messages:view). +/// Evaluated by . +/// +public sealed record PermissionRequirement(string Permission) : IAuthorizationRequirement; diff --git a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs new file mode 100644 index 0000000000..f246234c7b --- /dev/null +++ b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs @@ -0,0 +1,44 @@ +#nullable enable +namespace ServiceControl.Hosting.Auth; + +using System.Linq; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authorization; +using ServiceControl.Infrastructure.Auth; + +/// +/// Verb-level authorization handler for . It resolves the user's +/// roles and checks them against the hardcoded policy: the user must hold +/// a role (e.g. reader / writer) that grants the requested permission. +/// +/// Only registered — and only reached — when OIDC is enabled. When it is disabled, +/// returns an allow-all policy that carries no +/// , so this handler is not needed. +/// +/// +public sealed class PermissionVerbHandler : AuthorizationHandler +{ + // TODO: The claim that carries a user's roles is identity-provider specific and must become + // configurable (per-IdP) rather than hardcoded. Roles are expected as a flat, multivalued claim; + // the token handler splits a top-level JSON array into individual claims, so no parsing is needed. + // For Keycloak, add a "User Realm Role" protocol mapper with Multivalued = ON and Token Claim Name + // = "roles" (a dotted name like "realm_access.roles" would nest it into an object instead). + const string RoleClaimType = "roles"; + + protected override Task HandleRequirementAsync( + AuthorizationHandlerContext context, + PermissionRequirement requirement) + { + var roles = context.User.FindAll(RoleClaimType).Select(claim => claim.Value); + + + // TODO: Although plural, likely roles will only contain a single value unless we want to define a role for each instance but likely customers don't care about instances + if (RolePermissions.IsGranted(roles, requirement.Permission)) + { + context.Succeed(requirement); + } + + // Otherwise leave the requirement unmet → the request is denied (403/401). + return Task.CompletedTask; + } +} diff --git a/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs b/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs new file mode 100644 index 0000000000..f0ff2028a1 --- /dev/null +++ b/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs @@ -0,0 +1,127 @@ +#nullable enable +namespace ServiceControl.Infrastructure.Auth; + +using System; +using System.Collections.Frozen; +using System.Collections.Generic; +using System.Linq; + +/// +/// Hardcoded role → permission policy. Two roles for now: +/// +/// 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. +/// +/// TODO: interim hardcoded model — replace with a configurable role/permission mapping (loaded from +/// configuration or the IdP) when more than these two coarse roles are needed. +/// +/// +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"; + + // Source of truth: the wildcard pattern(s) each role grants. + static readonly Dictionary RolePatterns = new(StringComparer.OrdinalIgnoreCase) + { + [Reader] = ["*:*:view"], + [Writer] = ["*:*:*"], + }; + + // Expanded once against the full permission catalogue: role -> concrete granted permissions. + static readonly FrozenDictionary> PermissionsByRole = Expand(); + + /// + /// 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) + { + foreach (var role in roles) + { + if (PermissionsByRole.TryGetValue(role, out var granted) && granted.Contains(permission)) + { + return true; + } + } + + 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/Hosting/Commands/RunCommand.cs b/src/ServiceControl.Monitoring/Hosting/Commands/RunCommand.cs index ca648ac222..da23814397 100644 --- a/src/ServiceControl.Monitoring/Hosting/Commands/RunCommand.cs +++ b/src/ServiceControl.Monitoring/Hosting/Commands/RunCommand.cs @@ -16,6 +16,7 @@ public override async Task Execute(HostArguments args, Settings settings) var hostBuilder = WebApplication.CreateBuilder(); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); hostBuilder.AddServiceControlHttps(settings.HttpsSettings); hostBuilder.AddServiceControlMonitoring((_, __) => Task.CompletedTask, settings, endpointConfiguration); hostBuilder.AddServiceControlMonitoringApi(); diff --git a/src/ServiceControl/Hosting/Commands/RunCommand.cs b/src/ServiceControl/Hosting/Commands/RunCommand.cs index ebc08958cf..1f895ec9d3 100644 --- a/src/ServiceControl/Hosting/Commands/RunCommand.cs +++ b/src/ServiceControl/Hosting/Commands/RunCommand.cs @@ -25,6 +25,7 @@ public override async Task Execute(HostArguments args, Settings settings) var hostBuilder = WebApplication.CreateBuilder(); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); hostBuilder.AddServiceControlHttps(settings.HttpsSettings); hostBuilder.AddServiceControl(settings, endpointConfiguration); hostBuilder.AddServiceControlApi(settings.CorsSettings); From 9cb0f0918f8f5b99ab486448180be4777751f7f6 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Thu, 4 Jun 2026 14:45:35 +0200 Subject: [PATCH 03/14] =?UTF-8?q?=E2=9C=A8=20Per-IdP=20roles=20claim=20pat?= =?UTF-8?q?h=20via=20IClaimsTransformation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds Authentication.RolesClaim (default "realm_access.roles") and a RolesClaimsTransformation that normalises per-IdP role claim shapes — Keycloak's nested realm_access.roles, flat repeated claims (Entra app roles, Keycloak with a User Realm Role mapper, Cognito groups), and JSON-array-as-string values — into canonical "roles" claims that PermissionVerbHandler reads unchanged. The parsing lives in a pure static RolesClaimExtractor in Infrastructure so it can be unit-tested without an ASP.NET host; the IClaimsTransformation implementation in Hosting is a thin wrapper that adds idempotency via a sentinel claim. Removes the TODO on PermissionVerbHandler that the transformation now resolves. --- .../Auth/HostApplicationBuilderExtensions.cs | 7 + .../Auth/PermissionVerbHandler.cs | 7 +- .../Auth/RolesClaimsTransformation.cs | 47 ++++++ .../Auth/RolesClaimExtractorTests.cs | 124 +++++++++++++++ .../Auth/RolesClaimExtractor.cs | 141 ++++++++++++++++++ .../OpenIdConnectSettings.cs | 20 ++- 6 files changed, 339 insertions(+), 7 deletions(-) create mode 100644 src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs create mode 100644 src/ServiceControl.Infrastructure.Tests/Auth/RolesClaimExtractorTests.cs create mode 100644 src/ServiceControl.Infrastructure/Auth/RolesClaimExtractor.cs diff --git a/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs b/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs index f425e7afb2..3aea1f63dd 100644 --- a/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs +++ b/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs @@ -3,6 +3,7 @@ using System; using System.Text.Json; using System.Threading.Tasks; + using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; @@ -99,6 +100,12 @@ public static void AddServiceControlAuthentication(this IHostApplicationBuilder configure.FallbackPolicy = new Microsoft.AspNetCore.Authorization.AuthorizationPolicyBuilder() .RequireAuthenticatedUser() .Build()); + + // Normalise per-IdP role claim shapes (Keycloak's nested realm_access.roles, Entra app + // roles, Cognito groups) into canonical "roles" claims for the verb handler. The source + // path is configurable via Authentication.RolesClaim. + hostBuilder.Services.AddSingleton( + new RolesClaimsTransformation(oidcSettings.RolesClaim)); } static string GetErrorMessage(JwtBearerChallengeContext context) diff --git a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs index f246234c7b..7155bde555 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs @@ -18,11 +18,8 @@ namespace ServiceControl.Hosting.Auth; /// public sealed class PermissionVerbHandler : AuthorizationHandler { - // TODO: The claim that carries a user's roles is identity-provider specific and must become - // configurable (per-IdP) rather than hardcoded. Roles are expected as a flat, multivalued claim; - // the token handler splits a top-level JSON array into individual claims, so no parsing is needed. - // For Keycloak, add a "User Realm Role" protocol mapper with Multivalued = ON and Token Claim Name - // = "roles" (a dotted name like "realm_access.roles" would nest it into an object instead). + // The per-IdP variability of the source claim is absorbed by RolesClaimsTransformation, which + // reads from the path configured in Authentication.RolesClaim and emits canonical "roles" claims. const string RoleClaimType = "roles"; protected override Task HandleRequirementAsync( diff --git a/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs b/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs new file mode 100644 index 0000000000..74a62c4eeb --- /dev/null +++ b/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs @@ -0,0 +1,47 @@ +#nullable enable +namespace ServiceControl.Hosting.Auth; + +using System.Linq; +using System.Security.Claims; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authentication; +using ServiceControl.Infrastructure.Auth; + +/// +/// Normalises per-IdP role claim shapes into a flat set of roles claims that +/// can read directly. The source path is configured via +/// Authentication.RolesClaim (default realm_access.roles — the Keycloak out-of-box +/// shape). Flat claim names work too (roles for Keycloak with a "User Realm Role" mapper or +/// Microsoft Entra ID app roles, cognito:groups for AWS Cognito). +/// +/// ASP.NET may invoke multiple times for the same principal; a sentinel +/// claim makes the transformation idempotent and returns the same principal on subsequent calls. +/// +/// +public sealed class RolesClaimsTransformation(string rolesClaimPath) : IClaimsTransformation +{ + const string SentinelClaimType = "_roles_transformed"; + const string RoleClaimType = "roles"; + + public Task TransformAsync(ClaimsPrincipal principal) + { + if (principal.Identity?.IsAuthenticated != true || principal.HasClaim(SentinelClaimType, "1")) + { + return Task.FromResult(principal); + } + + var roles = RolesClaimExtractor.Extract(principal, rolesClaimPath); + + var claims = new Claim[roles.Count + 1]; + claims[0] = new Claim(SentinelClaimType, "1"); + for (var i = 0; i < roles.Count; i++) + { + claims[i + 1] = new Claim(RoleClaimType, roles[i]); + } + + // Build a new principal so the original (cached) instance is left untouched. + var transformed = new ClaimsPrincipal(principal.Identities.ToArray()); + transformed.AddIdentity(new ClaimsIdentity(claims)); + return Task.FromResult(transformed); + } +} diff --git a/src/ServiceControl.Infrastructure.Tests/Auth/RolesClaimExtractorTests.cs b/src/ServiceControl.Infrastructure.Tests/Auth/RolesClaimExtractorTests.cs new file mode 100644 index 0000000000..42f2387515 --- /dev/null +++ b/src/ServiceControl.Infrastructure.Tests/Auth/RolesClaimExtractorTests.cs @@ -0,0 +1,124 @@ +#nullable enable +namespace ServiceControl.Infrastructure.Tests.Auth; + +using System.Security.Claims; +using NUnit.Framework; +using ServiceControl.Infrastructure.Auth; + +[TestFixture] +public class RolesClaimExtractorTests +{ + [Test] + public void Flat_claim_with_repeated_string_values_returns_each_value() + { + var principal = PrincipalWith( + new Claim("roles", "operator"), + new Claim("roles", "viewer")); + + var result = RolesClaimExtractor.Extract(principal, "roles"); + + Assert.That(result, Is.EquivalentTo(new[] { "operator", "viewer" })); + } + + [Test] + public void Flat_claim_serialized_as_json_array_string_is_decoded() + { + var principal = PrincipalWith(new Claim("roles", "[\"admin\",\"writer\"]")); + + var result = RolesClaimExtractor.Extract(principal, "roles"); + + Assert.That(result, Is.EquivalentTo(new[] { "admin", "writer" })); + } + + [Test] + public void Nested_keycloak_path_extracts_realm_access_roles() + { + var principal = PrincipalWith(new Claim( + "realm_access", + "{\"roles\":[\"sc-admin\",\"sc-operator\"]}")); + + var result = RolesClaimExtractor.Extract(principal, "realm_access.roles"); + + Assert.That(result, Is.EquivalentTo(new[] { "sc-admin", "sc-operator" })); + } + + [Test] + public void Nested_path_with_single_string_value_returns_one_value() + { + var principal = PrincipalWith(new Claim( + "realm_access", + "{\"role\":\"sc-admin\"}")); + + var result = RolesClaimExtractor.Extract(principal, "realm_access.role"); + + Assert.That(result, Is.EqualTo(new[] { "sc-admin" })); + } + + [Test] + public void Missing_top_level_claim_returns_empty() + { + var principal = PrincipalWith(new Claim("other", "anything")); + + var result = RolesClaimExtractor.Extract(principal, "realm_access.roles"); + + Assert.That(result, Is.Empty); + } + + [Test] + public void Missing_nested_property_returns_empty() + { + var principal = PrincipalWith(new Claim( + "realm_access", + "{\"resource_access\":{}}")); + + var result = RolesClaimExtractor.Extract(principal, "realm_access.roles"); + + Assert.That(result, Is.Empty); + } + + [Test] + public void Malformed_json_in_nested_claim_returns_empty() + { + var principal = PrincipalWith(new Claim("realm_access", "not json")); + + var result = RolesClaimExtractor.Extract(principal, "realm_access.roles"); + + Assert.That(result, Is.Empty); + } + + [Test] + public void Empty_or_whitespace_path_returns_empty() + { + var principal = PrincipalWith(new Claim("roles", "viewer")); + + Assert.That(RolesClaimExtractor.Extract(principal, ""), Is.Empty); + Assert.That(RolesClaimExtractor.Extract(principal, " "), Is.Empty); + } + + [Test] + public void Non_string_array_entries_are_skipped() + { + var principal = PrincipalWith(new Claim( + "realm_access", + "{\"roles\":[\"valid\",42,null,\"alsovalid\"]}")); + + var result = RolesClaimExtractor.Extract(principal, "realm_access.roles"); + + Assert.That(result, Is.EquivalentTo(new[] { "valid", "alsovalid" })); + } + + [Test] + public void Multiple_top_level_claims_with_dotted_path_aggregate_values() + { + var principal = PrincipalWith( + new Claim("resource_access", "{\"client-a\":{\"roles\":[\"role-a\"]}}"), + new Claim("resource_access", "{\"client-a\":{\"roles\":[\"role-b\"]}}")); + + var result = RolesClaimExtractor.Extract(principal, "resource_access.client-a.roles"); + + Assert.That(result, Is.EquivalentTo(new[] { "role-a", "role-b" })); + } + + static ClaimsPrincipal PrincipalWith(params Claim[] claims) => + new(new ClaimsIdentity(claims, authenticationType: "Test")); +} diff --git a/src/ServiceControl.Infrastructure/Auth/RolesClaimExtractor.cs b/src/ServiceControl.Infrastructure/Auth/RolesClaimExtractor.cs new file mode 100644 index 0000000000..6a1c40c917 --- /dev/null +++ b/src/ServiceControl.Infrastructure/Auth/RolesClaimExtractor.cs @@ -0,0 +1,141 @@ +#nullable enable +namespace ServiceControl.Infrastructure.Auth; + +using System; +using System.Collections.Generic; +using System.Security.Claims; +using System.Text.Json; + +/// +/// Reads role values out of a at a configurable path. +/// Supports a flat claim name (roles) or a dotted path into a nested JSON object claim +/// (realm_access.roles). Used by RolesClaimsTransformation to normalize per-IdP token +/// shapes (Keycloak, Microsoft Entra ID, AWS Cognito, etc.) into a canonical set of role values. +/// +public static class RolesClaimExtractor +{ + /// + /// Extracts every role value reachable at on . + /// Returns an empty list when the path is absent or the value cannot be interpreted as a string or + /// array of strings — never throws on malformed input. + /// + public static IReadOnlyList Extract(ClaimsPrincipal principal, string rolesClaimPath) + { + if (principal is null || string.IsNullOrWhiteSpace(rolesClaimPath)) + { + return Array.Empty(); + } + + var segments = rolesClaimPath.Split('.'); + var topClaimType = segments[0]; + var results = new List(); + + foreach (var claim in principal.FindAll(topClaimType)) + { + if (segments.Length == 1) + { + AddFlatClaimValues(results, claim.Value); + } + else + { + AddNestedClaimValues(results, claim.Value, segments); + } + } + + return results; + } + + static void AddFlatClaimValues(List results, string claimValue) + { + // Flat claim values are typically a single role string per claim (the JWT bearer middleware + // explodes a top-level JSON array of strings into one claim per element). The fallback path + // handles the rare case where an IdP serialises the array into a single claim value. + if (LooksLikeJsonArray(claimValue)) + { + if (TryParse(claimValue, out var doc)) + { + using (doc) + { + AppendStringArray(results, doc.RootElement); + } + return; + } + } + + results.Add(claimValue); + } + + static void AddNestedClaimValues(List results, string claimValue, string[] segments) + { + if (!TryParse(claimValue, out var doc)) + { + return; + } + + using (doc) + { + var node = doc.RootElement; + for (var i = 1; i < segments.Length; i++) + { + if (node.ValueKind != JsonValueKind.Object || !node.TryGetProperty(segments[i], out var next)) + { + return; + } + node = next; + } + + AppendStringOrArray(results, node); + } + } + + static void AppendStringOrArray(List results, JsonElement node) + { + if (node.ValueKind == JsonValueKind.String) + { + var single = node.GetString(); + if (!string.IsNullOrEmpty(single)) + { + results.Add(single); + } + } + else if (node.ValueKind == JsonValueKind.Array) + { + AppendStringArray(results, node); + } + } + + static void AppendStringArray(List results, JsonElement array) + { + foreach (var item in array.EnumerateArray()) + { + if (item.ValueKind == JsonValueKind.String) + { + var value = item.GetString(); + if (!string.IsNullOrEmpty(value)) + { + results.Add(value); + } + } + } + } + + static bool LooksLikeJsonArray(string value) + { + var trimmed = value.AsSpan().TrimStart(); + return trimmed.Length > 0 && trimmed[0] == '['; + } + + static bool TryParse(string value, out JsonDocument document) + { + try + { + document = JsonDocument.Parse(value); + return true; + } + catch (JsonException) + { + document = null!; + return false; + } + } +} diff --git a/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs b/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs index efbba73239..23c4f58565 100644 --- a/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs +++ b/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs @@ -32,6 +32,13 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC ValidateIssuerSigningKey = SettingsReader.Read(rootNamespace, "Authentication.ValidateIssuerSigningKey", true); RequireHttpsMetadata = SettingsReader.Read(rootNamespace, "Authentication.RequireHttpsMetadata", true); + // Path within the JWT to the user's role values. May be a flat claim name (e.g. "roles" — the + // shape produced by Keycloak with a "User Realm Role" mapper, by Microsoft Entra ID, or by + // AWS Cognito as "cognito:groups") or a dotted path into a nested object claim (e.g. the + // Keycloak out-of-box shape "realm_access.roles"). The RolesClaimsTransformation reads from + // this path and flattens the values into canonical "roles" claims for the authorization handler. + RolesClaim = SettingsReader.Read(rootNamespace, "Authentication.RolesClaim", "realm_access.roles"); + // ServicePulse settings are only relevant for the primary ServiceControl instance // which serves the OIDC configuration endpoint that ServicePulse uses for login if (requireServicePulseSettings) @@ -96,6 +103,15 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC /// public bool RequireHttpsMetadata { get; } + /// + /// Path within the JWT where the user's role values live. Defaults to realm_access.roles + /// to match Keycloak's out-of-box token shape. A flat claim name like roles is used when + /// the identity provider emits role values as top-level claims (Keycloak with a "User Realm Role" + /// mapper, Microsoft Entra ID app roles, AWS Cognito groups, etc.). The dotted form navigates + /// into a nested JSON object claim. + /// + public string RolesClaim { get; } + /// /// Optional override for the authority URL that ServicePulse should use for authentication. /// If not specified, ServicePulse uses the main Authority value. @@ -187,8 +203,8 @@ void LogConfiguration(bool requireServicePulseSettings) var servicePulseAuthorityDisplay = requireServicePulseSettings ? (ServicePulseAuthority ?? "(not configured)") : "(n/a)"; var servicePulseApiScopesDisplay = requireServicePulseSettings ? (ServicePulseApiScopes ?? "(not configured)") : "(n/a)"; - logger.LogInformation("Authentication settings: Enabled={Enabled}, Authority={Authority}, Audience={Audience}, ValidateIssuer={ValidateIssuer}, ValidateAudience={ValidateAudience}, ValidateLifetime={ValidateLifetime}, ValidateIssuerSigningKey={ValidateIssuerSigningKey}, RequireHttpsMetadata={RequireHttpsMetadata}, ServicePulseClientId={ServicePulseClientId}, ServicePulseAuthority={ServicePulseAuthority}, ServicePulseApiScopes={ServicePulseApiScopes}", - Enabled, authorityDisplay, audienceDisplay, ValidateIssuer, ValidateAudience, ValidateLifetime, ValidateIssuerSigningKey, RequireHttpsMetadata, servicePulseClientIdDisplay, servicePulseAuthorityDisplay, servicePulseApiScopesDisplay); + logger.LogInformation("Authentication settings: Enabled={Enabled}, Authority={Authority}, Audience={Audience}, ValidateIssuer={ValidateIssuer}, ValidateAudience={ValidateAudience}, ValidateLifetime={ValidateLifetime}, ValidateIssuerSigningKey={ValidateIssuerSigningKey}, RequireHttpsMetadata={RequireHttpsMetadata}, RolesClaim={RolesClaim}, ServicePulseClientId={ServicePulseClientId}, ServicePulseAuthority={ServicePulseAuthority}, ServicePulseApiScopes={ServicePulseApiScopes}", + Enabled, authorityDisplay, audienceDisplay, ValidateIssuer, ValidateAudience, ValidateLifetime, ValidateIssuerSigningKey, RequireHttpsMetadata, RolesClaim, servicePulseClientIdDisplay, servicePulseAuthorityDisplay, servicePulseApiScopesDisplay); // Warn about potential misconfigurations var hasAuthConfig = !string.IsNullOrWhiteSpace(Authority) || !string.IsNullOrWhiteSpace(Audience); From 5ae3c440e4979e83e39695f68043f51bab77211a Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 5 Jun 2026 09:46:35 +0200 Subject: [PATCH 04/14] =?UTF-8?q?=F0=9F=90=9B=20Authorize=20policies=20req?= =?UTF-8?q?uire=20authenticated=20user?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without RequireAuthenticatedUser() in the policy, an unauthenticated request reaches PermissionVerbHandler, finds no roles, and fails the requirement — which ASP.NET classifies as FailedRequirements and turns into a 403 Forbid. The OIDC acceptance tests expect 401 Challenge (Should_reject_requests_without_bearer_token, ..._with_invalid/expired /wrong_audience/wrong_issuer). Adding the auth requirement first ensures the failure is classified as FailedAuthentication when no valid principal is present. --- .../Auth/PermissionPolicyProvider.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs b/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs index 1d5a52d4ff..1a95ec2ab8 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs @@ -40,7 +40,14 @@ static FrozenDictionary BuildPolicies(bool oidcEnab Permissions.All.ToFrozenDictionary( permission => permission, permission => oidcEnabled - ? new AuthorizationPolicyBuilder().AddRequirements(new PermissionRequirement(permission)).Build() + ? new AuthorizationPolicyBuilder() + // RequireAuthenticatedUser() must come first so an unauthenticated request fails as + // FailedAuthentication (→ 401 challenge) rather than FailedRequirements (→ 403 + // forbid). Without it, PermissionVerbHandler is reached for anonymous callers and a + // missing-roles outcome is classified as a forbidden permission failure. + .RequireAuthenticatedUser() + .AddRequirements(new PermissionRequirement(permission)) + .Build() : AllowAll, StringComparer.Ordinal); From 8e06d2a978c7ee59f9e4e2f23731288c244e654c Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 5 Jun 2026 09:47:44 +0200 Subject: [PATCH 05/14] =?UTF-8?q?=F0=9F=90=9B=20Wire=20authorization=20in?= =?UTF-8?q?=20acceptance-test=20runners?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Primary, Audit, and Monitoring acceptance-test runners called AddServiceControlAuthentication but not AddServiceControlAuthorization, while production RunCommand wires both. Without the authorization provider, ASP.NET cannot resolve the policy names emitted by the Permissions catalogue and any request to a controller carrying [Authorize(Policy = Permissions.X)] throws "AuthorizationPolicy named '...' was not found" — which breaks the test hosts and times out the audit acceptance tests that exercise authorized endpoints. --- .../TestSupport/ServiceControlComponentRunner.cs | 1 + .../TestSupport/ServiceControlComponentRunner.cs | 1 + .../TestSupport/ServiceControlComponentRunner.cs | 1 + 3 files changed, 3 insertions(+) diff --git a/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs b/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs index 657a84244d..45c6726718 100644 --- a/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs +++ b/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs @@ -123,6 +123,7 @@ async Task InitializeServiceControl(ScenarioContext context) EnvironmentName = Environments.Development }); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); hostBuilder.AddServiceControl(settings, configuration); hostBuilder.AddServiceControlHttps(settings.HttpsSettings); hostBuilder.AddServiceControlApi(settings.CorsSettings); diff --git a/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs b/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs index efcd99c0f6..6e0d233f12 100644 --- a/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs +++ b/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs @@ -120,6 +120,7 @@ async Task InitializeServiceControl(ScenarioContext context) EnvironmentName = Environments.Development }); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); hostBuilder.AddServiceControlAudit((criticalErrorContext, cancellationToken) => { var logitem = new ScenarioContext.LogItem diff --git a/src/ServiceControl.Monitoring.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs b/src/ServiceControl.Monitoring.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs index 1d233e7cc7..319ea95329 100644 --- a/src/ServiceControl.Monitoring.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs +++ b/src/ServiceControl.Monitoring.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs @@ -100,6 +100,7 @@ async Task InitializeServiceControl(ScenarioContext context) hostBuilder.Logging.ConfigureLogging(LogLevel.Information); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); hostBuilder.AddServiceControlMonitoring((criticalErrorContext, cancellationToken) => { var logitem = new ScenarioContext.LogItem From 99923fe2258360733f270908b34d1fdb0ff85c57 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 5 Jun 2026 09:58:03 +0200 Subject: [PATCH 06/14] =?UTF-8?q?=F0=9F=90=9B=20Accept-token=20tests=20mus?= =?UTF-8?q?t=20supply=20a=20role-bearing=20claim?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should_accept_requests_with_valid_bearer_token previously passed in CI by accident: AddServiceControlAuthorization was missing from the test runners, so the policy provider could not resolve the [Authorize] policy name and threw, returning 500 — which the assertion "not 401 and not 403" accepted. With the policy provider now wired up, a valid token without any role claim correctly fails the permission requirement and returns 403. The test must therefore supply the canonical "roles" claim with a value that maps to a role granting the endpoint's permission. "reader" grants every *:*:view permission, which covers the endpoints the three acceptance-test variants exercise (error:messages:view, audit:message:view, monitoring:endpoint:view). --- .../OpenIdConnect/When_authentication_is_enabled.cs | 6 +++++- .../OpenIdConnect/When_authentication_is_enabled.cs | 6 +++++- .../OpenIdConnect/When_authentication_is_enabled.cs | 7 ++++++- 3 files changed, 16 insertions(+), 3 deletions(-) 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 e19736ff17..139c840b77 100644 --- a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs +++ b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs @@ -1,6 +1,7 @@ namespace ServiceControl.AcceptanceTests.Security.OpenIdConnect { using System.Net.Http; + using System.Security.Claims; using System.Threading.Tasks; using AcceptanceTesting; using AcceptanceTesting.OpenIdConnect; @@ -124,7 +125,10 @@ public async Task Should_accept_requests_with_valid_bearer_token() _ = await Define() .Done(async ctx => { - var validToken = mockOidcServer.GenerateToken(); + // 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") }); response = await OpenIdConnectAssertions.SendRequestWithBearerToken( HttpClient, HttpMethod.Get, 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 9d57a41316..1d44d2e64f 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 @@ -1,6 +1,7 @@ namespace ServiceControl.Audit.AcceptanceTests.Security.OpenIdConnect { using System.Net.Http; + using System.Security.Claims; using System.Threading.Tasks; using AcceptanceTesting; using AcceptanceTesting.OpenIdConnect; @@ -92,7 +93,10 @@ public async Task Should_accept_requests_with_valid_bearer_token() _ = await Define() .Done(async ctx => { - var validToken = mockOidcServer.GenerateToken(); + // 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") }); response = await OpenIdConnectAssertions.SendRequestWithBearerToken( HttpClient, HttpMethod.Get, 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 4781e5e0fc..da0940e81c 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 @@ -1,6 +1,7 @@ namespace ServiceControl.Monitoring.AcceptanceTests.Security.OpenIdConnect { using System.Net.Http; + using System.Security.Claims; using System.Threading.Tasks; using AcceptanceTesting; using AcceptanceTesting.OpenIdConnect; @@ -92,7 +93,11 @@ public async Task Should_accept_requests_with_valid_bearer_token() _ = await Define() .Done(async ctx => { - var validToken = mockOidcServer.GenerateToken(); + // 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( + additionalClaims: new[] { new Claim("roles", "reader") }); response = await OpenIdConnectAssertions.SendRequestWithBearerToken( HttpClient, HttpMethod.Get, From 746cefb4165dbd233c7003ba54960668dd78460c Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 5 Jun 2026 10:01:36 +0200 Subject: [PATCH 07/14] =?UTF-8?q?=E2=9C=85=20Approve=20RolesClaim=20in=20P?= =?UTF-8?q?latformSampleSettings=20snapshots?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PlatformSampleSettings approval tests in SC, Audit, and Monitoring unit-test projects serialise the settings object and diff it against an approved JSON snapshot. The earlier Authentication.RolesClaim addition needs the snapshots updated to include the new property. --- .../APIApprovals.PlatformSampleSettings.approved.txt | 1 + .../SettingsTests.PlatformSampleSettings.approved.txt | 1 + .../APIApprovals.PlatformSampleSettings.approved.txt | 1 + 3 files changed, 3 insertions(+) diff --git a/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt b/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt index 83897faeba..82cf9de13f 100644 --- a/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt @@ -12,6 +12,7 @@ "ValidateLifetime": true, "ValidateIssuerSigningKey": true, "RequireHttpsMetadata": true, + "RolesClaim": "realm_access.roles", "ServicePulseAuthority": null, "ServicePulseClientId": null, "ServicePulseApiScopes": null diff --git a/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt b/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt index c4cfe5ad09..b29ddc3856 100644 --- a/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt @@ -12,6 +12,7 @@ "ValidateLifetime": true, "ValidateIssuerSigningKey": true, "RequireHttpsMetadata": true, + "RolesClaim": "realm_access.roles", "ServicePulseAuthority": null, "ServicePulseClientId": null, "ServicePulseApiScopes": null diff --git a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt index 6873e229b3..5ca8fcdf90 100644 --- a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt @@ -12,6 +12,7 @@ "ValidateLifetime": true, "ValidateIssuerSigningKey": true, "RequireHttpsMetadata": true, + "RolesClaim": "realm_access.roles", "ServicePulseAuthority": null, "ServicePulseClientId": null, "ServicePulseApiScopes": null From 387a4167244386fe86c66ef527ddc36f7e31b436 Mon Sep 17 00:00:00 2001 From: williambza Date: Fri, 5 Jun 2026 10:18:57 +0200 Subject: [PATCH 08/14] Fix rebase issues --- .../ServiceControlComponentRunner.cs | 2 +- .../ServiceControlComponentRunner.cs | 2 +- .../Hosting/Commands/RunCommand.cs | 2 +- .../Auth/HostApplicationBuilderExtensions.cs | 3 +- .../Auth/PermissionAuthorizationExtensions.cs | 17 +++++----- .../Auth/PermissionPolicyProvider.cs | 2 +- .../Auth/PermissionVerbHandler.cs | 10 +++--- .../Auth/RolePermissions.cs | 6 +--- .../OpenIdConnectSettings.cs | 32 ++++++++++--------- .../ServiceControlComponentRunner.cs | 2 +- .../Hosting/Commands/RunCommand.cs | 2 +- .../Hosting/Commands/RunCommand.cs | 2 +- 12 files changed, 42 insertions(+), 40 deletions(-) diff --git a/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs b/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs index 45c6726718..c3b87d68fd 100644 --- a/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs +++ b/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs @@ -123,7 +123,7 @@ async Task InitializeServiceControl(ScenarioContext context) EnvironmentName = Environments.Development }); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControl(settings, configuration); hostBuilder.AddServiceControlHttps(settings.HttpsSettings); hostBuilder.AddServiceControlApi(settings.CorsSettings); diff --git a/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs b/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs index 6e0d233f12..53a548f038 100644 --- a/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs +++ b/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs @@ -120,7 +120,7 @@ async Task InitializeServiceControl(ScenarioContext context) EnvironmentName = Environments.Development }); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControlAudit((criticalErrorContext, cancellationToken) => { var logitem = new ScenarioContext.LogItem diff --git a/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/RunCommand.cs b/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/RunCommand.cs index d8229e8774..2bfdb9c065 100644 --- a/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/RunCommand.cs +++ b/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/RunCommand.cs @@ -19,7 +19,7 @@ public override async Task Execute(HostArguments args, Settings settings) var hostBuilder = WebApplication.CreateBuilder(); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControlHttps(settings.HttpsSettings); hostBuilder.AddServiceControlAudit((_, __) => { diff --git a/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs b/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs index 3aea1f63dd..eba714e32d 100644 --- a/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs +++ b/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs @@ -37,7 +37,8 @@ public static void AddServiceControlAuthentication(this IHostApplicationBuilder ValidateLifetime = oidcSettings.ValidateLifetime, ValidateIssuerSigningKey = oidcSettings.ValidateIssuerSigningKey, ValidAudience = oidcSettings.Audience, - ClockSkew = TimeSpan.FromMinutes(5) // Allow 5 minutes clock skew + ClockSkew = TimeSpan.FromMinutes(5), // Allow 5 minutes clock skew + RoleClaimType = oidcSettings.RolesClaim }; options.RequireHttpsMetadata = oidcSettings.RequireHttpsMetadata; // Don't map inbound claims to legacy Microsoft claim types diff --git a/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs b/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs index 07229849eb..ca20392461 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs @@ -5,6 +5,7 @@ namespace ServiceControl.Hosting.Auth; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Options; +using ServiceControl.Infrastructure; /// /// Registers the permission-based policy authorization services: a dynamic @@ -20,8 +21,13 @@ namespace ServiceControl.Hosting.Auth; /// public static class PermissionAuthorizationExtensions { - public static void AddServiceControlAuthorization(this IHostApplicationBuilder hostBuilder, bool oidcEnabled) + public static void AddServiceControlAuthorization(this IHostApplicationBuilder hostBuilder, OpenIdConnectSettings oidcSettings) { + if (!oidcSettings.RoleBasedAuthorizationEnabled) + { + return; + } + var services = hostBuilder.Services; // Ensure the authorization core services and options are present (idempotent). @@ -31,13 +37,8 @@ public static void AddServiceControlAuthorization(this IHostApplicationBuilder h // policy provider registered by AddAuthorization(). When OIDC is disabled it returns allow-all // policies (no requirement); when enabled it emits a PermissionRequirement for the verb handler. services.AddSingleton(sp => - new PermissionPolicyProvider(sp.GetRequiredService>(), oidcEnabled)); + new PermissionPolicyProvider(sp.GetRequiredService>(), oidcSettings.Enabled)); - // The role-based handler is only needed when OIDC is enabled — otherwise the provider produces - // no PermissionRequirement for it to evaluate. - if (oidcEnabled) - { - services.AddSingleton(); - } + services.AddSingleton(service => new PermissionVerbHandler(oidcSettings.RolesClaim)); } } diff --git a/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs b/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs index 1a95ec2ab8..efd07b37e5 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs @@ -52,7 +52,7 @@ static FrozenDictionary BuildPolicies(bool oidcEnab StringComparer.Ordinal); public Task GetPolicyAsync(string policyName) => - Task.FromResult(policies.GetValueOrDefault(policyName)); + Task.FromResult(policies.GetValueOrDefault(policyName)); public Task GetDefaultPolicyAsync() { diff --git a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs index 7155bde555..46167d2484 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs @@ -18,9 +18,10 @@ namespace ServiceControl.Hosting.Auth; /// public sealed class PermissionVerbHandler : AuthorizationHandler { - // The per-IdP variability of the source claim is absorbed by RolesClaimsTransformation, which - // reads from the path configured in Authentication.RolesClaim and emits canonical "roles" claims. - const string RoleClaimType = "roles"; + public PermissionVerbHandler(string rolesClaimName) + { + RoleClaimType = rolesClaimName; + } protected override Task HandleRequirementAsync( AuthorizationHandlerContext context, @@ -28,7 +29,6 @@ protected override Task HandleRequirementAsync( { var roles = context.User.FindAll(RoleClaimType).Select(claim => claim.Value); - // TODO: Although plural, likely roles will only contain a single value unless we want to define a role for each instance but likely customers don't care about instances if (RolePermissions.IsGranted(roles, requirement.Permission)) { @@ -38,4 +38,6 @@ protected override Task HandleRequirementAsync( // Otherwise leave the requirement unmet → the request is denied (403/401). return Task.CompletedTask; } + + internal string RoleClaimType = "roles"; } diff --git a/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs b/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs index f0ff2028a1..7375919263 100644 --- a/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs +++ b/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs @@ -7,7 +7,7 @@ namespace ServiceControl.Infrastructure.Auth; using System.Linq; /// -/// Hardcoded role → permission policy. Two roles for now: +/// Role → permission policy. Two roles: /// /// reader — granted every *:*:view permission (read-only access). /// writer — granted every permission (*:*:*). @@ -17,10 +17,6 @@ namespace ServiceControl.Infrastructure.Auth; /// immutable of granted permissions per role. As a result both /// and are O(1) hash lookups with no /// per-call pattern matching or allocation. -/// -/// TODO: interim hardcoded model — replace with a configurable role/permission mapping (loaded from -/// configuration or the IdP) when more than these two coarse roles are needed. -/// /// public static class RolePermissions { diff --git a/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs b/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs index 23c4f58565..9c2d74158f 100644 --- a/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs +++ b/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs @@ -32,12 +32,8 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC ValidateIssuerSigningKey = SettingsReader.Read(rootNamespace, "Authentication.ValidateIssuerSigningKey", true); RequireHttpsMetadata = SettingsReader.Read(rootNamespace, "Authentication.RequireHttpsMetadata", true); - // Path within the JWT to the user's role values. May be a flat claim name (e.g. "roles" — the - // shape produced by Keycloak with a "User Realm Role" mapper, by Microsoft Entra ID, or by - // AWS Cognito as "cognito:groups") or a dotted path into a nested object claim (e.g. the - // Keycloak out-of-box shape "realm_access.roles"). The RolesClaimsTransformation reads from - // this path and flattens the values into canonical "roles" claims for the authorization handler. - RolesClaim = SettingsReader.Read(rootNamespace, "Authentication.RolesClaim", "realm_access.roles"); + RolesClaim = SettingsReader.Read(rootNamespace, "Authentication.RolesClaim", "roles"); + RoleBasedAuthorizationEnabled = SettingsReader.Read(rootNamespace, "Authentication.RoleBasedAuthorizationEnabled", false); // ServicePulse settings are only relevant for the primary ServiceControl instance // which serves the OIDC configuration endpoint that ServicePulse uses for login @@ -103,15 +99,6 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC /// public bool RequireHttpsMetadata { get; } - /// - /// Path within the JWT where the user's role values live. Defaults to realm_access.roles - /// to match Keycloak's out-of-box token shape. A flat claim name like roles is used when - /// the identity provider emits role values as top-level claims (Keycloak with a "User Realm Role" - /// mapper, Microsoft Entra ID app roles, AWS Cognito groups, etc.). The dotted form navigates - /// into a nested JSON object claim. - /// - public string RolesClaim { get; } - /// /// Optional override for the authority URL that ServicePulse should use for authentication. /// If not specified, ServicePulse uses the main Authority value. @@ -130,6 +117,21 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC /// public string ServicePulseApiScopes { get; } + /// + /// Path within the JWT where the user's role values live. Defaults to realm_access.roles + /// to match Keycloak's out-of-box token shape. A flat claim name like roles is used when + /// the identity provider emits role values as top-level claims (Keycloak with a "User Realm Role" + /// mapper, Microsoft Entra ID app roles, AWS Cognito groups, etc.). The dotted form navigates + /// into a nested JSON object claim. + /// + public string RolesClaim { get; } + + /// + /// Is RBAC enabled. When false, all authenticated users have access to all methods. When true, + /// role based authorization rules are applied. + /// + public bool RoleBasedAuthorizationEnabled { get; } + void Validate(bool requireServicePulseSettings) { if (Enabled) diff --git a/src/ServiceControl.Monitoring.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs b/src/ServiceControl.Monitoring.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs index 319ea95329..aaab866c82 100644 --- a/src/ServiceControl.Monitoring.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs +++ b/src/ServiceControl.Monitoring.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs @@ -100,7 +100,7 @@ async Task InitializeServiceControl(ScenarioContext context) hostBuilder.Logging.ConfigureLogging(LogLevel.Information); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControlMonitoring((criticalErrorContext, cancellationToken) => { var logitem = new ScenarioContext.LogItem diff --git a/src/ServiceControl.Monitoring/Hosting/Commands/RunCommand.cs b/src/ServiceControl.Monitoring/Hosting/Commands/RunCommand.cs index da23814397..6e197e463a 100644 --- a/src/ServiceControl.Monitoring/Hosting/Commands/RunCommand.cs +++ b/src/ServiceControl.Monitoring/Hosting/Commands/RunCommand.cs @@ -16,7 +16,7 @@ public override async Task Execute(HostArguments args, Settings settings) var hostBuilder = WebApplication.CreateBuilder(); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControlHttps(settings.HttpsSettings); hostBuilder.AddServiceControlMonitoring((_, __) => Task.CompletedTask, settings, endpointConfiguration); hostBuilder.AddServiceControlMonitoringApi(); diff --git a/src/ServiceControl/Hosting/Commands/RunCommand.cs b/src/ServiceControl/Hosting/Commands/RunCommand.cs index 1f895ec9d3..c25485bf5c 100644 --- a/src/ServiceControl/Hosting/Commands/RunCommand.cs +++ b/src/ServiceControl/Hosting/Commands/RunCommand.cs @@ -25,7 +25,7 @@ public override async Task Execute(HostArguments args, Settings settings) var hostBuilder = WebApplication.CreateBuilder(); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControlHttps(settings.HttpsSettings); hostBuilder.AddServiceControl(settings, endpointConfiguration); hostBuilder.AddServiceControlApi(settings.CorsSettings); From b29e1b12c7501b2d8de2055762ca6087b75e9b77 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 5 Jun 2026 10:19:00 +0200 Subject: [PATCH 09/14] magic number --- .../Auth/RolesClaimsTransformation.cs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs b/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs index 74a62c4eeb..2f59829edc 100644 --- a/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs +++ b/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs @@ -21,11 +21,15 @@ namespace ServiceControl.Hosting.Auth; public sealed class RolesClaimsTransformation(string rolesClaimPath) : IClaimsTransformation { const string SentinelClaimType = "_roles_transformed"; + // The sentinel's value is irrelevant; only the claim's presence matters. A non-empty + // placeholder is required because a Claim value cannot be null. + const string SentinelClaimValue = "1"; const string RoleClaimType = "roles"; public Task TransformAsync(ClaimsPrincipal principal) { - if (principal.Identity?.IsAuthenticated != true || principal.HasClaim(SentinelClaimType, "1")) + var isAuthenticated = principal.Identity?.IsAuthenticated == true; + if (!isAuthenticated || AlreadyTransformed(principal)) { return Task.FromResult(principal); } @@ -33,7 +37,7 @@ public Task TransformAsync(ClaimsPrincipal principal) var roles = RolesClaimExtractor.Extract(principal, rolesClaimPath); var claims = new Claim[roles.Count + 1]; - claims[0] = new Claim(SentinelClaimType, "1"); + claims[0] = new Claim(SentinelClaimType, SentinelClaimValue); for (var i = 0; i < roles.Count; i++) { claims[i + 1] = new Claim(RoleClaimType, roles[i]); @@ -44,4 +48,9 @@ public Task TransformAsync(ClaimsPrincipal principal) transformed.AddIdentity(new ClaimsIdentity(claims)); return Task.FromResult(transformed); } + + // True once this transformation has stamped its sentinel claim, keeping TransformAsync + // idempotent across the repeated calls ASP.NET makes for the same principal. + static bool AlreadyTransformed(ClaimsPrincipal principal) => + principal.HasClaim(SentinelClaimType, SentinelClaimValue); } From 41dcf2ad1d64f0fa20a0fadecfa7e6f7552c5749 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 5 Jun 2026 13:59:49 +0200 Subject: [PATCH 10/14] =?UTF-8?q?=F0=9F=90=9B=20Always=20register=20the=20?= =?UTF-8?q?permission=20policy=20provider?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recent refactor moved Authentication.RoleBasedAuthorizationEnabled to a separate master switch (default false) and made the entire authorization registration short-circuit when it was off. That left the permission policy provider unregistered in every default deployment, so ASP.NET could not resolve the policy names emitted by [Authorize(Policy = Permissions.X)] attributes — every annotated endpoint returned 500 with "AuthorizationPolicy named '...' was not found", which is what was timing out the audit acceptance tests at 90s each. PermissionPolicyProvider already returns allow-all policies for every known permission when its oidcEnabled flag is false. Registering it unconditionally and passing RoleBasedAuthorizationEnabled to that flag gives the right behaviour in all three combinations: RBAC off → allow-all (controllers reachable, no permission check), RBAC on → require auth + the permission requirement evaluated by PermissionVerbHandler. The handler itself remains gated on RoleBasedAuthorizationEnabled since it has nothing to evaluate when RBAC is off. --- .../Auth/PermissionAuthorizationExtensions.cs | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs b/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs index ca20392461..008b1f37ea 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs @@ -23,22 +23,23 @@ public static class PermissionAuthorizationExtensions { public static void AddServiceControlAuthorization(this IHostApplicationBuilder hostBuilder, OpenIdConnectSettings oidcSettings) { - if (!oidcSettings.RoleBasedAuthorizationEnabled) - { - return; - } - var services = hostBuilder.Services; // Ensure the authorization core services and options are present (idempotent). services.AddAuthorization(); - // Resolve permission policy names dynamically. Registered last so it supersedes the default - // policy provider registered by AddAuthorization(). When OIDC is disabled it returns allow-all - // policies (no requirement); when enabled it emits a PermissionRequirement for the verb handler. + // The policy provider is registered UNCONDITIONALLY: every instance hosts controllers with + // [Authorize(Policy = Permissions.X)] attributes, and without a provider that knows those + // policy names ASP.NET throws "AuthorizationPolicy named '...' was not found" → 500 on every + // request to an annotated endpoint. When RBAC is disabled the provider returns allow-all + // policies (no requirement), so anonymous-to-the-policy calls pass through and the verb + // handler is unnecessary. services.AddSingleton(sp => - new PermissionPolicyProvider(sp.GetRequiredService>(), oidcSettings.Enabled)); + new PermissionPolicyProvider(sp.GetRequiredService>(), oidcSettings.RoleBasedAuthorizationEnabled)); - services.AddSingleton(service => new PermissionVerbHandler(oidcSettings.RolesClaim)); + if (oidcSettings.RoleBasedAuthorizationEnabled) + { + services.AddSingleton(_ => new PermissionVerbHandler(oidcSettings.RolesClaim)); + } } } From 95c6c63b9f1a9e9f5001bf9c49cd88a45fca49dc Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 5 Jun 2026 13:59:58 +0200 Subject: [PATCH 11/14] =?UTF-8?q?=F0=9F=90=9B=20OIDC=20enabled=20tests=20m?= =?UTF-8?q?ust=20enable=20RBAC=20explicitly?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Authentication.RoleBasedAuthorizationEnabled defaults to false, so without an explicit opt-in the policy provider returns allow-all and unauthenticated requests reach the controller — breaking every Should_reject_requests_* test in the three When_authentication_is_enabled classes. Add WithRoleBasedAuthorizationEnabled() to the test configuration helper and call it alongside WithAuthenticationEnabled() in all three OIDC enabled fixtures. --- .../OpenIdConnect/OpenIdConnectTestConfiguration.cs | 13 +++++++++++++ .../OpenIdConnect/When_authentication_is_enabled.cs | 1 + .../OpenIdConnect/When_authentication_is_enabled.cs | 1 + .../OpenIdConnect/When_authentication_is_enabled.cs | 1 + 4 files changed, 16 insertions(+) diff --git a/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectTestConfiguration.cs b/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectTestConfiguration.cs index dc056b05c3..3148b18275 100644 --- a/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectTestConfiguration.cs +++ b/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectTestConfiguration.cs @@ -34,6 +34,18 @@ public OpenIdConnectTestConfiguration WithAuthenticationDisabled() return this; } + /// + /// Enables role-based authorization. When on, controllers carrying + /// [Authorize(Policy = Permissions.X)] require the caller's "roles" claim to map to a + /// role that grants the permission via RolePermissions. When off, the policy provider + /// returns allow-all policies and any authenticated request reaches the controller. + /// + public OpenIdConnectTestConfiguration WithRoleBasedAuthorizationEnabled() + { + SetEnvironmentVariable("AUTHENTICATION_ROLEBASEDAUTHORIZATIONENABLED", "true"); + return this; + } + /// /// Disables settings validation. This allows testing with placeholder/fake OIDC settings. /// Should only be used in test scenarios where a real OIDC provider is not available. @@ -164,6 +176,7 @@ public void ClearConfiguration() ClearEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_CLIENTID"); ClearEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_APISCOPES"); ClearEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_AUTHORITY"); + ClearEnvironmentVariable("AUTHENTICATION_ROLEBASEDAUTHORIZATIONENABLED"); ClearEnvironmentVariable("VALIDATECONFIG"); } 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 139c840b77..425b8b6e92 100644 --- a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs +++ b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs @@ -36,6 +36,7 @@ public void ConfigureAuth() configuration = new OpenIdConnectTestConfiguration(ServiceControlInstanceType.Primary) .WithConfigurationValidationDisabled() .WithAuthenticationEnabled() + .WithRoleBasedAuthorizationEnabled() .WithAuthority(mockOidcServer.Authority) .WithAudience(TestAudience) .WithServicePulseClientId(TestClientId) 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 1d44d2e64f..1891b545d3 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 @@ -33,6 +33,7 @@ public void ConfigureAuth() configuration = new OpenIdConnectTestConfiguration(ServiceControlInstanceType.Audit) .WithConfigurationValidationDisabled() .WithAuthenticationEnabled() + .WithRoleBasedAuthorizationEnabled() .WithAuthority(mockOidcServer.Authority) .WithAudience(TestAudience) .WithRequireHttpsMetadata(false); 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 da0940e81c..0f94dba925 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 @@ -33,6 +33,7 @@ public void ConfigureAuth() configuration = new OpenIdConnectTestConfiguration(ServiceControlInstanceType.Monitoring) .WithConfigurationValidationDisabled() .WithAuthenticationEnabled() + .WithRoleBasedAuthorizationEnabled() .WithAuthority(mockOidcServer.Authority) .WithAudience(TestAudience) .WithRequireHttpsMetadata(false); From 89977de60c3b72b18b2f784cebd823cab38c58e0 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 5 Jun 2026 14:00:09 +0200 Subject: [PATCH 12/14] =?UTF-8?q?=E2=9C=85=20Refresh=20PlatformSampleSetti?= =?UTF-8?q?ngs=20snapshots=20for=20RBAC=20settings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two settings shifted in the OpenIdConnectSettings serialisation: RolesClaim moved after the ServicePulse block and its default changed from "realm_access.roles" to "roles", and the new RoleBasedAuthorizationEnabled property (default false) was added. Sync the three approved snapshots so the approval tests reflect the current shape. --- .../APIApprovals.PlatformSampleSettings.approved.txt | 5 +++-- .../SettingsTests.PlatformSampleSettings.approved.txt | 5 +++-- .../APIApprovals.PlatformSampleSettings.approved.txt | 5 +++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt b/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt index 82cf9de13f..c73fd28a46 100644 --- a/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt @@ -12,10 +12,11 @@ "ValidateLifetime": true, "ValidateIssuerSigningKey": true, "RequireHttpsMetadata": true, - "RolesClaim": "realm_access.roles", "ServicePulseAuthority": null, "ServicePulseClientId": null, - "ServicePulseApiScopes": null + "ServicePulseApiScopes": null, + "RolesClaim": "roles", + "RoleBasedAuthorizationEnabled": false }, "ForwardedHeadersSettings": { "Enabled": true, diff --git a/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt b/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt index b29ddc3856..fed5a2acf4 100644 --- a/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt @@ -12,10 +12,11 @@ "ValidateLifetime": true, "ValidateIssuerSigningKey": true, "RequireHttpsMetadata": true, - "RolesClaim": "realm_access.roles", "ServicePulseAuthority": null, "ServicePulseClientId": null, - "ServicePulseApiScopes": null + "ServicePulseApiScopes": null, + "RolesClaim": "roles", + "RoleBasedAuthorizationEnabled": false }, "ForwardedHeadersSettings": { "Enabled": true, diff --git a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt index 5ca8fcdf90..72e33e6946 100644 --- a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt @@ -12,10 +12,11 @@ "ValidateLifetime": true, "ValidateIssuerSigningKey": true, "RequireHttpsMetadata": true, - "RolesClaim": "realm_access.roles", "ServicePulseAuthority": null, "ServicePulseClientId": null, - "ServicePulseApiScopes": null + "ServicePulseApiScopes": null, + "RolesClaim": "roles", + "RoleBasedAuthorizationEnabled": false }, "ForwardedHeadersSettings": { "Enabled": true, From af98ef4daea0dff7cbe8b9c756e3fefec9fc238f Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 5 Jun 2026 10:19:00 +0200 Subject: [PATCH 13/14] magic number --- .../Auth/RolesClaimsTransformation.cs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs b/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs index 74a62c4eeb..2f59829edc 100644 --- a/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs +++ b/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs @@ -21,11 +21,15 @@ namespace ServiceControl.Hosting.Auth; public sealed class RolesClaimsTransformation(string rolesClaimPath) : IClaimsTransformation { const string SentinelClaimType = "_roles_transformed"; + // The sentinel's value is irrelevant; only the claim's presence matters. A non-empty + // placeholder is required because a Claim value cannot be null. + const string SentinelClaimValue = "1"; const string RoleClaimType = "roles"; public Task TransformAsync(ClaimsPrincipal principal) { - if (principal.Identity?.IsAuthenticated != true || principal.HasClaim(SentinelClaimType, "1")) + var isAuthenticated = principal.Identity?.IsAuthenticated == true; + if (!isAuthenticated || AlreadyTransformed(principal)) { return Task.FromResult(principal); } @@ -33,7 +37,7 @@ public Task TransformAsync(ClaimsPrincipal principal) var roles = RolesClaimExtractor.Extract(principal, rolesClaimPath); var claims = new Claim[roles.Count + 1]; - claims[0] = new Claim(SentinelClaimType, "1"); + claims[0] = new Claim(SentinelClaimType, SentinelClaimValue); for (var i = 0; i < roles.Count; i++) { claims[i + 1] = new Claim(RoleClaimType, roles[i]); @@ -44,4 +48,9 @@ public Task TransformAsync(ClaimsPrincipal principal) transformed.AddIdentity(new ClaimsIdentity(claims)); return Task.FromResult(transformed); } + + // True once this transformation has stamped its sentinel claim, keeping TransformAsync + // idempotent across the repeated calls ASP.NET makes for the same principal. + static bool AlreadyTransformed(ClaimsPrincipal principal) => + principal.HasClaim(SentinelClaimType, SentinelClaimValue); } From 1811c14fa2a925f762995867c69f7dfe3fd3aec3 Mon Sep 17 00:00:00 2001 From: williambza Date: Mon, 8 Jun 2026 11:49:21 +0200 Subject: [PATCH 14/14] Make role based always on, but only filter if auth enabled --- .../Auth/PermissionAuthorizationExtensions.cs | 7 ++----- .../Auth/PermissionPolicyProvider.cs | 9 +++++---- src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs | 3 +-- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs b/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs index 008b1f37ea..82d6c3fcbc 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs @@ -35,11 +35,8 @@ public static void AddServiceControlAuthorization(this IHostApplicationBuilder h // policies (no requirement), so anonymous-to-the-policy calls pass through and the verb // handler is unnecessary. services.AddSingleton(sp => - new PermissionPolicyProvider(sp.GetRequiredService>(), oidcSettings.RoleBasedAuthorizationEnabled)); + new PermissionPolicyProvider(sp.GetRequiredService>(), oidcSettings)); - if (oidcSettings.RoleBasedAuthorizationEnabled) - { - services.AddSingleton(_ => new PermissionVerbHandler(oidcSettings.RolesClaim)); - } + services.AddSingleton(_ => new PermissionVerbHandler(oidcSettings.RolesClaim)); } } diff --git a/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs b/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs index efd07b37e5..500e50c335 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs @@ -7,6 +7,7 @@ namespace ServiceControl.Hosting.Auth; using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; using Microsoft.Extensions.Options; +using ServiceControl.Infrastructure; using ServiceControl.Infrastructure.Auth; /// @@ -27,19 +28,19 @@ namespace ServiceControl.Hosting.Auth; /// delegated to the configured . /// /// -public sealed class PermissionPolicyProvider(IOptions authorizationOptions, bool oidcEnabled) +public sealed class PermissionPolicyProvider(IOptions authorizationOptions, OpenIdConnectSettings oidcSettings) : IAuthorizationPolicyProvider { // Carries no requirement, so it succeeds without any IAuthorizationHandler being registered. static readonly AuthorizationPolicy AllowAll = new AuthorizationPolicyBuilder().RequireAssertion(_ => true).Build(); - readonly FrozenDictionary policies = BuildPolicies(oidcEnabled); + readonly FrozenDictionary policies = BuildPolicies(oidcSettings); - static FrozenDictionary BuildPolicies(bool oidcEnabled) => + static FrozenDictionary BuildPolicies(OpenIdConnectSettings oidcSettings) => Permissions.All.ToFrozenDictionary( permission => permission, - permission => oidcEnabled + permission => oidcSettings.RoleBasedAuthorizationEnabled ? new AuthorizationPolicyBuilder() // RequireAuthenticatedUser() must come first so an unauthenticated request fails as // FailedAuthentication (→ 401 challenge) rather than FailedRequirements (→ 403 diff --git a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs index 46167d2484..68a56e29ba 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs @@ -29,7 +29,6 @@ protected override Task HandleRequirementAsync( { var roles = context.User.FindAll(RoleClaimType).Select(claim => claim.Value); - // TODO: Although plural, likely roles will only contain a single value unless we want to define a role for each instance but likely customers don't care about instances if (RolePermissions.IsGranted(roles, requirement.Permission)) { context.Succeed(requirement); @@ -40,4 +39,4 @@ protected override Task HandleRequirementAsync( } internal string RoleClaimType = "roles"; -} +} \ No newline at end of file