fix(management-service): resolve ManagementService-001/002/003 — enforce site scope on query/snapshot handlers and DebugStreamHub
This commit is contained in:
@@ -8,7 +8,7 @@
|
|||||||
| Last reviewed | 2026-05-16 |
|
| Last reviewed | 2026-05-16 |
|
||||||
| Reviewer | claude-agent |
|
| Reviewer | claude-agent |
|
||||||
| Commit reviewed | `9c60592` |
|
| Commit reviewed | `9c60592` |
|
||||||
| Open findings | 13 |
|
| Open findings | 10 |
|
||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
|
|
||||||
@@ -51,7 +51,7 @@ authorization bypass with no workaround.
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | High |
|
| Severity | High |
|
||||||
| Category | Security |
|
| Category | Security |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.ManagementService/ManagementActor.cs:1465`, `:1481`, `:1493`, `:641`, `:649` |
|
| Location | `src/ScadaLink.ManagementService/ManagementActor.cs:1465`, `:1481`, `:1493`, `:641`, `:649` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -76,7 +76,16 @@ is already loaded — call `EnforceSiteScope(user, instance.SiteId)` (which requ
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit `<pending>`). Threaded `AuthenticatedUser` into
|
||||||
|
`HandleQueryEventLogs`, `HandleQueryParkedMessages`, `HandleRetryParkedMessage`,
|
||||||
|
`HandleDiscardParkedMessage`, and `HandleDebugSnapshot`; added an
|
||||||
|
`EnforceSiteScopeForIdentifier` helper that resolves the site by identifier and applies
|
||||||
|
`EnforceSiteScope`. `HandleDebugSnapshot` enforces against the already-loaded instance's
|
||||||
|
`SiteId`. Regression tests: `QueryEventLogs_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`,
|
||||||
|
`QueryParkedMessages_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`,
|
||||||
|
`RetryParkedMessage_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`,
|
||||||
|
`DiscardParkedMessage_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`,
|
||||||
|
`DebugSnapshot_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`.
|
||||||
|
|
||||||
### ManagementService-002 — Single-entity query handlers leak data across site scope
|
### ManagementService-002 — Single-entity query handlers leak data across site scope
|
||||||
|
|
||||||
@@ -84,7 +93,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | High |
|
| Severity | High |
|
||||||
| Category | Security |
|
| Category | Security |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.ManagementService/ManagementActor.cs:510`, `:673`, `:733`, `:774`, `:631`, `:624` |
|
| Location | `src/ScadaLink.ManagementService/ManagementActor.cs:510`, `:673`, `:733`, `:774`, `:631`, `:624` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -106,7 +115,16 @@ the resolved site ID in `HandleGetSite`, `HandleListAreas`, and `HandleGetDataCo
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit `<pending>`). `HandleGetInstance`, `HandleGetSite`,
|
||||||
|
`HandleGetDataConnection` now take `AuthenticatedUser` and call `EnforceSiteScope` against
|
||||||
|
the resolved entity's site ID (instance `SiteId`, site `Id`, data-connection `SiteId`);
|
||||||
|
`HandleListAreas` enforces against the requested `SiteId` before querying. Regression tests:
|
||||||
|
`GetInstance_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`,
|
||||||
|
`GetInstance_InScopeForSiteScopedUser_ReturnsSuccess`,
|
||||||
|
`GetSite_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`,
|
||||||
|
`GetSite_OutOfScopeForAdminUser_ReturnsSuccess`,
|
||||||
|
`ListAreas_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`,
|
||||||
|
`GetDataConnection_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`.
|
||||||
|
|
||||||
### ManagementService-003 — DebugStreamHub.SubscribeInstance performs no per-instance authorization
|
### ManagementService-003 — DebugStreamHub.SubscribeInstance performs no per-instance authorization
|
||||||
|
|
||||||
@@ -114,7 +132,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | High |
|
| Severity | High |
|
||||||
| Category | Security |
|
| Category | Security |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.ManagementService/DebugStreamHub.cs:104` |
|
| Location | `src/ScadaLink.ManagementService/DebugStreamHub.cs:104` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -135,7 +153,17 @@ established in `OnConnectedAsync` must be persisted on the connection (e.g. in
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit `<pending>`). `OnConnectedAsync` now persists the resolved
|
||||||
|
roles and `PermittedSiteIds` in `Context.Items`. `SubscribeInstance` resolves the
|
||||||
|
instance's site via `ITemplateEngineRepository` and rejects the subscription (sending
|
||||||
|
`OnStreamTerminated`) when the new pure `DebugStreamHub.IsInstanceAccessAllowed` check
|
||||||
|
fails. The check grants access for the Admin role or system-wide Deployment (empty
|
||||||
|
permitted set) and otherwise requires the instance's site in the permitted set. Regression
|
||||||
|
tests: `IsInstanceAccessAllowed_SiteScopedUser_OutOfScopeInstance_Denied`,
|
||||||
|
`IsInstanceAccessAllowed_SiteScopedUser_InScopeInstance_Allowed`,
|
||||||
|
`IsInstanceAccessAllowed_SystemWideDeployment_AnySiteAllowed`,
|
||||||
|
`IsInstanceAccessAllowed_AdminRole_BypassesSiteScope`,
|
||||||
|
`IsInstanceAccessAllowed_AdminRoleCheck_IsCaseInsensitive`.
|
||||||
|
|
||||||
### ManagementService-004 — Actor offloads work to Task.Run instead of using PipeTo
|
### ManagementService-004 — Actor offloads work to Task.Run instead of using PipeTo
|
||||||
|
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ using System.Text;
|
|||||||
using Microsoft.AspNetCore.SignalR;
|
using Microsoft.AspNetCore.SignalR;
|
||||||
using Microsoft.Extensions.DependencyInjection;
|
using Microsoft.Extensions.DependencyInjection;
|
||||||
using Microsoft.Extensions.Logging;
|
using Microsoft.Extensions.Logging;
|
||||||
|
using ScadaLink.Commons.Interfaces.Repositories;
|
||||||
using ScadaLink.Commons.Messages.DebugView;
|
using ScadaLink.Commons.Messages.DebugView;
|
||||||
using ScadaLink.Commons.Messages.Streaming;
|
using ScadaLink.Commons.Messages.Streaming;
|
||||||
using ScadaLink.Communication;
|
using ScadaLink.Communication;
|
||||||
@@ -17,6 +18,26 @@ namespace ScadaLink.ManagementService;
|
|||||||
public class DebugStreamHub : Hub
|
public class DebugStreamHub : Hub
|
||||||
{
|
{
|
||||||
private const string SessionIdKey = "DebugStreamSessionId";
|
private const string SessionIdKey = "DebugStreamSessionId";
|
||||||
|
private const string RolesKey = "DebugStreamRoles";
|
||||||
|
private const string PermittedSiteIdsKey = "DebugStreamPermittedSiteIds";
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Pure site-scope authorization check for a debug-stream subscription.
|
||||||
|
/// Returns true when the caller may subscribe to a debug stream for an instance
|
||||||
|
/// belonging to <paramref name="instanceSiteId"/>.
|
||||||
|
/// Admin role, or an empty <paramref name="permittedSiteIds"/> (system-wide
|
||||||
|
/// Deployment), grants access to any site; otherwise the instance's site must be
|
||||||
|
/// in the permitted set.
|
||||||
|
/// </summary>
|
||||||
|
public static bool IsInstanceAccessAllowed(
|
||||||
|
IReadOnlyCollection<string> roles,
|
||||||
|
IReadOnlyCollection<string> permittedSiteIds,
|
||||||
|
int instanceSiteId)
|
||||||
|
{
|
||||||
|
if (roles.Contains("Admin", StringComparer.OrdinalIgnoreCase)) return true;
|
||||||
|
if (permittedSiteIds.Count == 0) return true; // system-wide deployment
|
||||||
|
return permittedSiteIds.Contains(instanceSiteId.ToString());
|
||||||
|
}
|
||||||
|
|
||||||
private readonly DebugStreamService _debugStreamService;
|
private readonly DebugStreamService _debugStreamService;
|
||||||
private readonly IHubContext<DebugStreamHub> _hubContext;
|
private readonly IHubContext<DebugStreamHub> _hubContext;
|
||||||
@@ -93,6 +114,11 @@ public class DebugStreamHub : Hub
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Persist the resolved identity on the connection so per-instance site-scope
|
||||||
|
// enforcement can be applied to SubscribeInstance calls.
|
||||||
|
Context.Items[RolesKey] = mappingResult.Roles.ToArray();
|
||||||
|
Context.Items[PermittedSiteIdsKey] = mappingResult.PermittedSiteIds.ToArray();
|
||||||
|
|
||||||
_logger.LogInformation("DebugStreamHub connection established for {Username}", username);
|
_logger.LogInformation("DebugStreamHub connection established for {Username}", username);
|
||||||
await base.OnConnectedAsync();
|
await base.OnConnectedAsync();
|
||||||
}
|
}
|
||||||
@@ -108,6 +134,41 @@ public class DebugStreamHub : Hub
|
|||||||
|
|
||||||
var connectionId = Context.ConnectionId;
|
var connectionId = Context.ConnectionId;
|
||||||
|
|
||||||
|
// Per-instance site-scope enforcement: a site-scoped Deployment user must not
|
||||||
|
// be able to stream an instance belonging to a site outside their scope.
|
||||||
|
var httpContext = Context.GetHttpContext();
|
||||||
|
if (httpContext == null)
|
||||||
|
{
|
||||||
|
_logger.LogWarning("DebugStreamHub: {ConnectionId} subscribe rejected — no HTTP context", connectionId);
|
||||||
|
await Clients.Caller.SendAsync("OnStreamTerminated", "Authorization context unavailable.");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
var roles = Context.Items.TryGetValue(RolesKey, out var rolesObj) && rolesObj is string[] r
|
||||||
|
? r : Array.Empty<string>();
|
||||||
|
var permittedSiteIds = Context.Items.TryGetValue(PermittedSiteIdsKey, out var sitesObj) && sitesObj is string[] s
|
||||||
|
? s : Array.Empty<string>();
|
||||||
|
|
||||||
|
var instanceRepo = httpContext.RequestServices.GetRequiredService<ITemplateEngineRepository>();
|
||||||
|
var instance = await instanceRepo.GetInstanceByIdAsync(instanceId);
|
||||||
|
if (instance == null)
|
||||||
|
{
|
||||||
|
_logger.LogWarning("DebugStreamHub: {ConnectionId} subscribe rejected — instance {InstanceId} not found",
|
||||||
|
connectionId, instanceId);
|
||||||
|
await Clients.Caller.SendAsync("OnStreamTerminated", $"Instance {instanceId} not found.");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!IsInstanceAccessAllowed(roles, permittedSiteIds, instance.SiteId))
|
||||||
|
{
|
||||||
|
_logger.LogWarning(
|
||||||
|
"DebugStreamHub: {ConnectionId} subscribe to instance {InstanceId} denied — site {SiteId} outside permitted scope",
|
||||||
|
connectionId, instanceId, instance.SiteId);
|
||||||
|
await Clients.Caller.SendAsync("OnStreamTerminated",
|
||||||
|
$"Access denied: instance {instanceId} belongs to a site outside your Deployment scope.");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
// Use IHubContext for callbacks — the hub instance is transient (disposed after method returns),
|
// Use IHubContext for callbacks — the hub instance is transient (disposed after method returns),
|
||||||
|
|||||||
@@ -164,7 +164,7 @@ public class ManagementActor : ReceiveActor
|
|||||||
|
|
||||||
// Instances
|
// Instances
|
||||||
ListInstancesCommand cmd => await HandleListInstances(sp, cmd, user),
|
ListInstancesCommand cmd => await HandleListInstances(sp, cmd, user),
|
||||||
GetInstanceCommand cmd => await HandleGetInstance(sp, cmd),
|
GetInstanceCommand cmd => await HandleGetInstance(sp, cmd, user),
|
||||||
CreateInstanceCommand cmd => await HandleCreateInstance(sp, cmd, user),
|
CreateInstanceCommand cmd => await HandleCreateInstance(sp, cmd, user),
|
||||||
MgmtDeployInstanceCommand cmd => await HandleDeployInstance(sp, cmd, user),
|
MgmtDeployInstanceCommand cmd => await HandleDeployInstance(sp, cmd, user),
|
||||||
MgmtEnableInstanceCommand cmd => await HandleEnableInstance(sp, cmd, user),
|
MgmtEnableInstanceCommand cmd => await HandleEnableInstance(sp, cmd, user),
|
||||||
@@ -179,18 +179,18 @@ public class ManagementActor : ReceiveActor
|
|||||||
|
|
||||||
// Sites
|
// Sites
|
||||||
ListSitesCommand => await HandleListSites(sp, user),
|
ListSitesCommand => await HandleListSites(sp, user),
|
||||||
GetSiteCommand cmd => await HandleGetSite(sp, cmd),
|
GetSiteCommand cmd => await HandleGetSite(sp, cmd, user),
|
||||||
CreateSiteCommand cmd => await HandleCreateSite(sp, cmd, user.Username),
|
CreateSiteCommand cmd => await HandleCreateSite(sp, cmd, user.Username),
|
||||||
UpdateSiteCommand cmd => await HandleUpdateSite(sp, cmd, user.Username),
|
UpdateSiteCommand cmd => await HandleUpdateSite(sp, cmd, user.Username),
|
||||||
DeleteSiteCommand cmd => await HandleDeleteSite(sp, cmd, user.Username),
|
DeleteSiteCommand cmd => await HandleDeleteSite(sp, cmd, user.Username),
|
||||||
ListAreasCommand cmd => await HandleListAreas(sp, cmd),
|
ListAreasCommand cmd => await HandleListAreas(sp, cmd, user),
|
||||||
CreateAreaCommand cmd => await HandleCreateArea(sp, cmd, user.Username),
|
CreateAreaCommand cmd => await HandleCreateArea(sp, cmd, user.Username),
|
||||||
DeleteAreaCommand cmd => await HandleDeleteArea(sp, cmd, user.Username),
|
DeleteAreaCommand cmd => await HandleDeleteArea(sp, cmd, user.Username),
|
||||||
UpdateAreaCommand cmd => await HandleUpdateArea(sp, cmd, user.Username),
|
UpdateAreaCommand cmd => await HandleUpdateArea(sp, cmd, user.Username),
|
||||||
|
|
||||||
// Data Connections
|
// Data Connections
|
||||||
ListDataConnectionsCommand cmd => await HandleListDataConnections(sp, cmd),
|
ListDataConnectionsCommand cmd => await HandleListDataConnections(sp, cmd),
|
||||||
GetDataConnectionCommand cmd => await HandleGetDataConnection(sp, cmd),
|
GetDataConnectionCommand cmd => await HandleGetDataConnection(sp, cmd, user),
|
||||||
CreateDataConnectionCommand cmd => await HandleCreateDataConnection(sp, cmd, user.Username),
|
CreateDataConnectionCommand cmd => await HandleCreateDataConnection(sp, cmd, user.Username),
|
||||||
UpdateDataConnectionCommand cmd => await HandleUpdateDataConnection(sp, cmd, user.Username),
|
UpdateDataConnectionCommand cmd => await HandleUpdateDataConnection(sp, cmd, user.Username),
|
||||||
DeleteDataConnectionCommand cmd => await HandleDeleteDataConnection(sp, cmd, user.Username),
|
DeleteDataConnectionCommand cmd => await HandleDeleteDataConnection(sp, cmd, user.Username),
|
||||||
@@ -263,11 +263,11 @@ public class ManagementActor : ReceiveActor
|
|||||||
GetSiteHealthCommand cmd => HandleGetSiteHealth(sp, cmd),
|
GetSiteHealthCommand cmd => HandleGetSiteHealth(sp, cmd),
|
||||||
|
|
||||||
// Remote Queries
|
// Remote Queries
|
||||||
QueryEventLogsCommand cmd => await HandleQueryEventLogs(sp, cmd),
|
QueryEventLogsCommand cmd => await HandleQueryEventLogs(sp, cmd, user),
|
||||||
QueryParkedMessagesCommand cmd => await HandleQueryParkedMessages(sp, cmd),
|
QueryParkedMessagesCommand cmd => await HandleQueryParkedMessages(sp, cmd, user),
|
||||||
RetryParkedMessageCommand cmd => await HandleRetryParkedMessage(sp, cmd),
|
RetryParkedMessageCommand cmd => await HandleRetryParkedMessage(sp, cmd, user),
|
||||||
DiscardParkedMessageCommand cmd => await HandleDiscardParkedMessage(sp, cmd),
|
DiscardParkedMessageCommand cmd => await HandleDiscardParkedMessage(sp, cmd, user),
|
||||||
DebugSnapshotCommand cmd => await HandleDebugSnapshot(sp, cmd),
|
DebugSnapshotCommand cmd => await HandleDebugSnapshot(sp, cmd, user),
|
||||||
|
|
||||||
// Role resolution (for CLI LDAP auth)
|
// Role resolution (for CLI LDAP auth)
|
||||||
ResolveRolesCommand cmd => await HandleResolveRoles(sp, cmd),
|
ResolveRolesCommand cmd => await HandleResolveRoles(sp, cmd),
|
||||||
@@ -329,6 +329,21 @@ public class ManagementActor : ReceiveActor
|
|||||||
EnforceSiteScope(user, instance.SiteId);
|
EnforceSiteScope(user, instance.SiteId);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Resolves a site by its string identifier and enforces site-scope.
|
||||||
|
/// Used by remote-query handlers that key off the site identifier rather than its ID.
|
||||||
|
/// </summary>
|
||||||
|
private static async Task EnforceSiteScopeForIdentifier(IServiceProvider sp, AuthenticatedUser user, string siteIdentifier)
|
||||||
|
{
|
||||||
|
if (user.PermittedSiteIds.Length == 0) return;
|
||||||
|
if (user.Roles.Contains("Admin", StringComparer.OrdinalIgnoreCase)) return;
|
||||||
|
|
||||||
|
var repo = sp.GetRequiredService<ISiteRepository>();
|
||||||
|
var site = await repo.GetSiteByIdentifierAsync(siteIdentifier);
|
||||||
|
if (site != null)
|
||||||
|
EnforceSiteScope(user, site.Id);
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Helper to log an audit entry after a successful mutation.
|
/// Helper to log an audit entry after a successful mutation.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
@@ -507,10 +522,13 @@ public class ManagementActor : ReceiveActor
|
|||||||
return instances;
|
return instances;
|
||||||
}
|
}
|
||||||
|
|
||||||
private static async Task<object?> HandleGetInstance(IServiceProvider sp, GetInstanceCommand cmd)
|
private static async Task<object?> HandleGetInstance(IServiceProvider sp, GetInstanceCommand cmd, AuthenticatedUser user)
|
||||||
{
|
{
|
||||||
var repo = sp.GetRequiredService<ITemplateEngineRepository>();
|
var repo = sp.GetRequiredService<ITemplateEngineRepository>();
|
||||||
return await repo.GetInstanceByIdAsync(cmd.InstanceId);
|
var instance = await repo.GetInstanceByIdAsync(cmd.InstanceId);
|
||||||
|
if (instance != null)
|
||||||
|
EnforceSiteScope(user, instance.SiteId);
|
||||||
|
return instance;
|
||||||
}
|
}
|
||||||
|
|
||||||
private static async Task<object?> HandleCreateInstance(IServiceProvider sp, CreateInstanceCommand cmd, AuthenticatedUser user)
|
private static async Task<object?> HandleCreateInstance(IServiceProvider sp, CreateInstanceCommand cmd, AuthenticatedUser user)
|
||||||
@@ -638,16 +656,18 @@ public class ManagementActor : ReceiveActor
|
|||||||
: throw new InvalidOperationException(result.Error);
|
: throw new InvalidOperationException(result.Error);
|
||||||
}
|
}
|
||||||
|
|
||||||
private static async Task<object?> HandleRetryParkedMessage(IServiceProvider sp, RetryParkedMessageCommand cmd)
|
private static async Task<object?> HandleRetryParkedMessage(IServiceProvider sp, RetryParkedMessageCommand cmd, AuthenticatedUser user)
|
||||||
{
|
{
|
||||||
|
await EnforceSiteScopeForIdentifier(sp, user, cmd.SiteIdentifier);
|
||||||
var commService = sp.GetRequiredService<CommunicationService>();
|
var commService = sp.GetRequiredService<CommunicationService>();
|
||||||
var request = new Commons.Messages.RemoteQuery.ParkedMessageRetryRequest(
|
var request = new Commons.Messages.RemoteQuery.ParkedMessageRetryRequest(
|
||||||
Guid.NewGuid().ToString("N"), cmd.SiteIdentifier, cmd.MessageId, DateTimeOffset.UtcNow);
|
Guid.NewGuid().ToString("N"), cmd.SiteIdentifier, cmd.MessageId, DateTimeOffset.UtcNow);
|
||||||
return await commService.RetryParkedMessageAsync(cmd.SiteIdentifier, request);
|
return await commService.RetryParkedMessageAsync(cmd.SiteIdentifier, request);
|
||||||
}
|
}
|
||||||
|
|
||||||
private static async Task<object?> HandleDiscardParkedMessage(IServiceProvider sp, DiscardParkedMessageCommand cmd)
|
private static async Task<object?> HandleDiscardParkedMessage(IServiceProvider sp, DiscardParkedMessageCommand cmd, AuthenticatedUser user)
|
||||||
{
|
{
|
||||||
|
await EnforceSiteScopeForIdentifier(sp, user, cmd.SiteIdentifier);
|
||||||
var commService = sp.GetRequiredService<CommunicationService>();
|
var commService = sp.GetRequiredService<CommunicationService>();
|
||||||
var request = new Commons.Messages.RemoteQuery.ParkedMessageDiscardRequest(
|
var request = new Commons.Messages.RemoteQuery.ParkedMessageDiscardRequest(
|
||||||
Guid.NewGuid().ToString("N"), cmd.SiteIdentifier, cmd.MessageId, DateTimeOffset.UtcNow);
|
Guid.NewGuid().ToString("N"), cmd.SiteIdentifier, cmd.MessageId, DateTimeOffset.UtcNow);
|
||||||
@@ -670,10 +690,13 @@ public class ManagementActor : ReceiveActor
|
|||||||
return sites;
|
return sites;
|
||||||
}
|
}
|
||||||
|
|
||||||
private static async Task<object?> HandleGetSite(IServiceProvider sp, GetSiteCommand cmd)
|
private static async Task<object?> HandleGetSite(IServiceProvider sp, GetSiteCommand cmd, AuthenticatedUser user)
|
||||||
{
|
{
|
||||||
var repo = sp.GetRequiredService<ISiteRepository>();
|
var repo = sp.GetRequiredService<ISiteRepository>();
|
||||||
return await repo.GetSiteByIdAsync(cmd.SiteId);
|
var site = await repo.GetSiteByIdAsync(cmd.SiteId);
|
||||||
|
if (site != null)
|
||||||
|
EnforceSiteScope(user, site.Id);
|
||||||
|
return site;
|
||||||
}
|
}
|
||||||
|
|
||||||
private static async Task<object?> HandleCreateSite(IServiceProvider sp, CreateSiteCommand cmd, string user)
|
private static async Task<object?> HandleCreateSite(IServiceProvider sp, CreateSiteCommand cmd, string user)
|
||||||
@@ -730,8 +753,9 @@ public class ManagementActor : ReceiveActor
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
private static async Task<object?> HandleListAreas(IServiceProvider sp, ListAreasCommand cmd)
|
private static async Task<object?> HandleListAreas(IServiceProvider sp, ListAreasCommand cmd, AuthenticatedUser user)
|
||||||
{
|
{
|
||||||
|
EnforceSiteScope(user, cmd.SiteId);
|
||||||
var repo = sp.GetRequiredService<ICentralUiRepository>();
|
var repo = sp.GetRequiredService<ICentralUiRepository>();
|
||||||
return await repo.GetAreaTreeBySiteIdAsync(cmd.SiteId);
|
return await repo.GetAreaTreeBySiteIdAsync(cmd.SiteId);
|
||||||
}
|
}
|
||||||
@@ -771,10 +795,13 @@ public class ManagementActor : ReceiveActor
|
|||||||
return await repo.GetAllDataConnectionsAsync();
|
return await repo.GetAllDataConnectionsAsync();
|
||||||
}
|
}
|
||||||
|
|
||||||
private static async Task<object?> HandleGetDataConnection(IServiceProvider sp, GetDataConnectionCommand cmd)
|
private static async Task<object?> HandleGetDataConnection(IServiceProvider sp, GetDataConnectionCommand cmd, AuthenticatedUser user)
|
||||||
{
|
{
|
||||||
var repo = sp.GetRequiredService<ISiteRepository>();
|
var repo = sp.GetRequiredService<ISiteRepository>();
|
||||||
return await repo.GetDataConnectionByIdAsync(cmd.DataConnectionId);
|
var conn = await repo.GetDataConnectionByIdAsync(cmd.DataConnectionId);
|
||||||
|
if (conn != null)
|
||||||
|
EnforceSiteScope(user, conn.SiteId);
|
||||||
|
return conn;
|
||||||
}
|
}
|
||||||
|
|
||||||
private static async Task<object?> HandleCreateDataConnection(IServiceProvider sp, CreateDataConnectionCommand cmd, string user)
|
private static async Task<object?> HandleCreateDataConnection(IServiceProvider sp, CreateDataConnectionCommand cmd, string user)
|
||||||
@@ -1462,8 +1489,9 @@ public class ManagementActor : ReceiveActor
|
|||||||
// Remote Query handlers
|
// Remote Query handlers
|
||||||
// ========================================================================
|
// ========================================================================
|
||||||
|
|
||||||
private static async Task<object?> HandleQueryEventLogs(IServiceProvider sp, QueryEventLogsCommand cmd)
|
private static async Task<object?> HandleQueryEventLogs(IServiceProvider sp, QueryEventLogsCommand cmd, AuthenticatedUser user)
|
||||||
{
|
{
|
||||||
|
await EnforceSiteScopeForIdentifier(sp, user, cmd.SiteIdentifier);
|
||||||
var commService = sp.GetRequiredService<CommunicationService>();
|
var commService = sp.GetRequiredService<CommunicationService>();
|
||||||
var request = new EventLogQueryRequest(
|
var request = new EventLogQueryRequest(
|
||||||
Guid.NewGuid().ToString("N"),
|
Guid.NewGuid().ToString("N"),
|
||||||
@@ -1478,8 +1506,9 @@ public class ManagementActor : ReceiveActor
|
|||||||
return await commService.QueryEventLogsAsync(cmd.SiteIdentifier, request);
|
return await commService.QueryEventLogsAsync(cmd.SiteIdentifier, request);
|
||||||
}
|
}
|
||||||
|
|
||||||
private static async Task<object?> HandleQueryParkedMessages(IServiceProvider sp, QueryParkedMessagesCommand cmd)
|
private static async Task<object?> HandleQueryParkedMessages(IServiceProvider sp, QueryParkedMessagesCommand cmd, AuthenticatedUser user)
|
||||||
{
|
{
|
||||||
|
await EnforceSiteScopeForIdentifier(sp, user, cmd.SiteIdentifier);
|
||||||
var commService = sp.GetRequiredService<CommunicationService>();
|
var commService = sp.GetRequiredService<CommunicationService>();
|
||||||
var request = new ParkedMessageQueryRequest(
|
var request = new ParkedMessageQueryRequest(
|
||||||
Guid.NewGuid().ToString("N"),
|
Guid.NewGuid().ToString("N"),
|
||||||
@@ -1490,12 +1519,14 @@ public class ManagementActor : ReceiveActor
|
|||||||
return await commService.QueryParkedMessagesAsync(cmd.SiteIdentifier, request);
|
return await commService.QueryParkedMessagesAsync(cmd.SiteIdentifier, request);
|
||||||
}
|
}
|
||||||
|
|
||||||
private static async Task<object?> HandleDebugSnapshot(IServiceProvider sp, DebugSnapshotCommand cmd)
|
private static async Task<object?> HandleDebugSnapshot(IServiceProvider sp, DebugSnapshotCommand cmd, AuthenticatedUser user)
|
||||||
{
|
{
|
||||||
var instanceRepo = sp.GetRequiredService<ITemplateEngineRepository>();
|
var instanceRepo = sp.GetRequiredService<ITemplateEngineRepository>();
|
||||||
var instance = await instanceRepo.GetInstanceByIdAsync(cmd.InstanceId)
|
var instance = await instanceRepo.GetInstanceByIdAsync(cmd.InstanceId)
|
||||||
?? throw new InvalidOperationException($"Instance {cmd.InstanceId} not found.");
|
?? throw new InvalidOperationException($"Instance {cmd.InstanceId} not found.");
|
||||||
|
|
||||||
|
EnforceSiteScope(user, instance.SiteId);
|
||||||
|
|
||||||
var siteRepo = sp.GetRequiredService<ISiteRepository>();
|
var siteRepo = sp.GetRequiredService<ISiteRepository>();
|
||||||
var site = await siteRepo.GetSiteByIdAsync(instance.SiteId)
|
var site = await siteRepo.GetSiteByIdAsync(instance.SiteId)
|
||||||
?? throw new InvalidOperationException($"Site {instance.SiteId} not found.");
|
?? throw new InvalidOperationException($"Site {instance.SiteId} not found.");
|
||||||
|
|||||||
@@ -0,0 +1,66 @@
|
|||||||
|
using ScadaLink.ManagementService;
|
||||||
|
|
||||||
|
namespace ScadaLink.ManagementService.Tests;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Tests for <see cref="DebugStreamHub"/> per-instance site-scope authorization
|
||||||
|
/// (finding ManagementService-003).
|
||||||
|
/// </summary>
|
||||||
|
public class DebugStreamHubTests
|
||||||
|
{
|
||||||
|
[Fact]
|
||||||
|
public void IsInstanceAccessAllowed_SiteScopedUser_InScopeInstance_Allowed()
|
||||||
|
{
|
||||||
|
var allowed = DebugStreamHub.IsInstanceAccessAllowed(
|
||||||
|
roles: new[] { "Deployment" },
|
||||||
|
permittedSiteIds: new[] { "1", "2" },
|
||||||
|
instanceSiteId: 2);
|
||||||
|
|
||||||
|
Assert.True(allowed);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void IsInstanceAccessAllowed_SiteScopedUser_OutOfScopeInstance_Denied()
|
||||||
|
{
|
||||||
|
var allowed = DebugStreamHub.IsInstanceAccessAllowed(
|
||||||
|
roles: new[] { "Deployment" },
|
||||||
|
permittedSiteIds: new[] { "1", "2" },
|
||||||
|
instanceSiteId: 99);
|
||||||
|
|
||||||
|
Assert.False(allowed);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void IsInstanceAccessAllowed_SystemWideDeployment_AnySiteAllowed()
|
||||||
|
{
|
||||||
|
// Empty permitted set == system-wide Deployment.
|
||||||
|
var allowed = DebugStreamHub.IsInstanceAccessAllowed(
|
||||||
|
roles: new[] { "Deployment" },
|
||||||
|
permittedSiteIds: Array.Empty<string>(),
|
||||||
|
instanceSiteId: 99);
|
||||||
|
|
||||||
|
Assert.True(allowed);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void IsInstanceAccessAllowed_AdminRole_BypassesSiteScope()
|
||||||
|
{
|
||||||
|
var allowed = DebugStreamHub.IsInstanceAccessAllowed(
|
||||||
|
roles: new[] { "Admin", "Deployment" },
|
||||||
|
permittedSiteIds: new[] { "1" },
|
||||||
|
instanceSiteId: 99);
|
||||||
|
|
||||||
|
Assert.True(allowed);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void IsInstanceAccessAllowed_AdminRoleCheck_IsCaseInsensitive()
|
||||||
|
{
|
||||||
|
var allowed = DebugStreamHub.IsInstanceAccessAllowed(
|
||||||
|
roles: new[] { "admin" },
|
||||||
|
permittedSiteIds: new[] { "1" },
|
||||||
|
instanceSiteId: 99);
|
||||||
|
|
||||||
|
Assert.True(allowed);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -44,6 +44,10 @@ public class ManagementActorTests : TestKit, IDisposable
|
|||||||
new(new AuthenticatedUser("testuser", "Test User", roles, Array.Empty<string>()),
|
new(new AuthenticatedUser("testuser", "Test User", roles, Array.Empty<string>()),
|
||||||
command, Guid.NewGuid().ToString("N"));
|
command, Guid.NewGuid().ToString("N"));
|
||||||
|
|
||||||
|
private static ManagementEnvelope ScopedEnvelope(object command, string[] permittedSiteIds, params string[] roles) =>
|
||||||
|
new(new AuthenticatedUser("scopeduser", "Scoped User", roles, permittedSiteIds),
|
||||||
|
command, Guid.NewGuid().ToString("N"));
|
||||||
|
|
||||||
void IDisposable.Dispose()
|
void IDisposable.Dispose()
|
||||||
{
|
{
|
||||||
Shutdown();
|
Shutdown();
|
||||||
@@ -478,4 +482,190 @@ public class ManagementActorTests : TestKit, IDisposable
|
|||||||
Assert.Equal("COMMAND_FAILED", response.ErrorCode);
|
Assert.Equal("COMMAND_FAILED", response.ErrorCode);
|
||||||
Assert.Contains("Connection refused", response.Error);
|
Assert.Contains("Connection refused", response.Error);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ========================================================================
|
||||||
|
// Site-scope enforcement tests (findings ManagementService-001 / -002)
|
||||||
|
//
|
||||||
|
// A site-scoped Deployment user (PermittedSiteIds set, no Admin role) must
|
||||||
|
// not be able to read or act on entities belonging to a site outside their
|
||||||
|
// scope. The handlers must throw SiteScopeViolationException, which the
|
||||||
|
// actor maps to ManagementUnauthorized.
|
||||||
|
// ========================================================================
|
||||||
|
|
||||||
|
private void AddSiteRepoWithSite(int siteId, string identifier)
|
||||||
|
{
|
||||||
|
var siteRepo = Substitute.For<ISiteRepository>();
|
||||||
|
siteRepo.GetSiteByIdAsync(siteId, Arg.Any<CancellationToken>())
|
||||||
|
.Returns(new Commons.Entities.Sites.Site($"Site{siteId}", identifier) { Id = siteId });
|
||||||
|
siteRepo.GetSiteByIdentifierAsync(identifier, Arg.Any<CancellationToken>())
|
||||||
|
.Returns(new Commons.Entities.Sites.Site($"Site{siteId}", identifier) { Id = siteId });
|
||||||
|
_services.AddScoped(_ => siteRepo);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void GetInstance_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
|
||||||
|
{
|
||||||
|
// Instance belongs to site 2; user is scoped to site 1.
|
||||||
|
_templateRepo.GetInstanceByIdAsync(7, Arg.Any<CancellationToken>())
|
||||||
|
.Returns(new Instance("Pump7") { Id = 7, SiteId = 2 });
|
||||||
|
|
||||||
|
var actor = CreateActor();
|
||||||
|
var envelope = ScopedEnvelope(new GetInstanceCommand(7), new[] { "1" }, "Deployment");
|
||||||
|
|
||||||
|
actor.Tell(envelope);
|
||||||
|
|
||||||
|
var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
|
||||||
|
Assert.Equal(envelope.CorrelationId, response.CorrelationId);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void GetInstance_InScopeForSiteScopedUser_ReturnsSuccess()
|
||||||
|
{
|
||||||
|
_templateRepo.GetInstanceByIdAsync(7, Arg.Any<CancellationToken>())
|
||||||
|
.Returns(new Instance("Pump7") { Id = 7, SiteId = 1 });
|
||||||
|
|
||||||
|
var actor = CreateActor();
|
||||||
|
var envelope = ScopedEnvelope(new GetInstanceCommand(7), new[] { "1" }, "Deployment");
|
||||||
|
|
||||||
|
actor.Tell(envelope);
|
||||||
|
|
||||||
|
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void GetSite_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
|
||||||
|
{
|
||||||
|
AddSiteRepoWithSite(2, "SITE2");
|
||||||
|
|
||||||
|
var actor = CreateActor();
|
||||||
|
var envelope = ScopedEnvelope(new GetSiteCommand(2), new[] { "1" }, "Deployment");
|
||||||
|
|
||||||
|
actor.Tell(envelope);
|
||||||
|
|
||||||
|
var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
|
||||||
|
Assert.Equal(envelope.CorrelationId, response.CorrelationId);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void ListAreas_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
|
||||||
|
{
|
||||||
|
AddSiteRepoWithSite(2, "SITE2");
|
||||||
|
var uiRepo = Substitute.For<ICentralUiRepository>();
|
||||||
|
uiRepo.GetAreaTreeBySiteIdAsync(2, Arg.Any<CancellationToken>())
|
||||||
|
.Returns(new List<Commons.Entities.Instances.Area>());
|
||||||
|
_services.AddScoped(_ => uiRepo);
|
||||||
|
|
||||||
|
var actor = CreateActor();
|
||||||
|
var envelope = ScopedEnvelope(new ListAreasCommand(2), new[] { "1" }, "Deployment");
|
||||||
|
|
||||||
|
actor.Tell(envelope);
|
||||||
|
|
||||||
|
var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
|
||||||
|
Assert.Equal(envelope.CorrelationId, response.CorrelationId);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void GetDataConnection_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
|
||||||
|
{
|
||||||
|
var siteRepo = Substitute.For<ISiteRepository>();
|
||||||
|
siteRepo.GetDataConnectionByIdAsync(5, Arg.Any<CancellationToken>())
|
||||||
|
.Returns(new Commons.Entities.Sites.DataConnection("Conn5", "OpcUa", 2) { Id = 5 });
|
||||||
|
_services.AddScoped(_ => siteRepo);
|
||||||
|
|
||||||
|
var actor = CreateActor();
|
||||||
|
var envelope = ScopedEnvelope(new GetDataConnectionCommand(5), new[] { "1" }, "Deployment");
|
||||||
|
|
||||||
|
actor.Tell(envelope);
|
||||||
|
|
||||||
|
var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
|
||||||
|
Assert.Equal(envelope.CorrelationId, response.CorrelationId);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void GetSite_OutOfScopeForAdminUser_ReturnsSuccess()
|
||||||
|
{
|
||||||
|
// Admin role bypasses site scoping even when PermittedSiteIds is set.
|
||||||
|
AddSiteRepoWithSite(2, "SITE2");
|
||||||
|
|
||||||
|
var actor = CreateActor();
|
||||||
|
var envelope = ScopedEnvelope(new GetSiteCommand(2), new[] { "1" }, "Admin");
|
||||||
|
|
||||||
|
actor.Tell(envelope);
|
||||||
|
|
||||||
|
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void QueryEventLogs_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
|
||||||
|
{
|
||||||
|
// Site SITE2 has Id 2; user scoped to site 1.
|
||||||
|
AddSiteRepoWithSite(2, "SITE2");
|
||||||
|
|
||||||
|
var actor = CreateActor();
|
||||||
|
var envelope = ScopedEnvelope(new QueryEventLogsCommand("SITE2"), new[] { "1" }, "Deployment");
|
||||||
|
|
||||||
|
actor.Tell(envelope);
|
||||||
|
|
||||||
|
var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
|
||||||
|
Assert.Equal(envelope.CorrelationId, response.CorrelationId);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void QueryParkedMessages_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
|
||||||
|
{
|
||||||
|
AddSiteRepoWithSite(2, "SITE2");
|
||||||
|
|
||||||
|
var actor = CreateActor();
|
||||||
|
var envelope = ScopedEnvelope(new QueryParkedMessagesCommand("SITE2"), new[] { "1" }, "Deployment");
|
||||||
|
|
||||||
|
actor.Tell(envelope);
|
||||||
|
|
||||||
|
var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
|
||||||
|
Assert.Equal(envelope.CorrelationId, response.CorrelationId);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void RetryParkedMessage_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
|
||||||
|
{
|
||||||
|
AddSiteRepoWithSite(2, "SITE2");
|
||||||
|
|
||||||
|
var actor = CreateActor();
|
||||||
|
var envelope = ScopedEnvelope(new RetryParkedMessageCommand("SITE2", "msg-1"), new[] { "1" }, "Deployment");
|
||||||
|
|
||||||
|
actor.Tell(envelope);
|
||||||
|
|
||||||
|
var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
|
||||||
|
Assert.Equal(envelope.CorrelationId, response.CorrelationId);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void DiscardParkedMessage_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
|
||||||
|
{
|
||||||
|
AddSiteRepoWithSite(2, "SITE2");
|
||||||
|
|
||||||
|
var actor = CreateActor();
|
||||||
|
var envelope = ScopedEnvelope(new DiscardParkedMessageCommand("SITE2", "msg-1"), new[] { "1" }, "Deployment");
|
||||||
|
|
||||||
|
actor.Tell(envelope);
|
||||||
|
|
||||||
|
var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
|
||||||
|
Assert.Equal(envelope.CorrelationId, response.CorrelationId);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void DebugSnapshot_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
|
||||||
|
{
|
||||||
|
// Instance 9 belongs to site 2; user scoped to site 1.
|
||||||
|
_templateRepo.GetInstanceByIdAsync(9, Arg.Any<CancellationToken>())
|
||||||
|
.Returns(new Instance("Pump9") { Id = 9, SiteId = 2 });
|
||||||
|
AddSiteRepoWithSite(2, "SITE2");
|
||||||
|
|
||||||
|
var actor = CreateActor();
|
||||||
|
var envelope = ScopedEnvelope(new DebugSnapshotCommand(9), new[] { "1" }, "Deployment");
|
||||||
|
|
||||||
|
actor.Tell(envelope);
|
||||||
|
|
||||||
|
var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
|
||||||
|
Assert.Equal(envelope.CorrelationId, response.CorrelationId);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user