merge(agent-a): rows 37/86/87/88 — deployment reads consolidated on QueryDeploymentListPageAsync (QueryDeploymentSummariesAsync deleted); SiteScopeService honors the Administrator system-wide rule (claims no longer scope admins — the documented ManagementActor semantic now holds on both surfaces); site scope rides the query join (whole-table instance read gone), PageSize bounded (default 50, cap 1000); unparseable --status fails loudly naming valid values
This commit is contained in:
@@ -51,7 +51,9 @@ Because `CookieAuthenticationStateProvider` serves a frozen principal, the circu
|
||||
|
||||
### Site-scoped authorization
|
||||
|
||||
`SiteScopeService` (scoped, registered in `AddCentralUI`) reads `SiteId` claims from the circuit principal. Deployment users whose LDAP mapping carries site-scope rules carry one integer `SiteId` claim per permitted site; Admin and Design users carry none (system-wide). Pages that enumerate sites or issue cross-site commands call `SiteScopeService.FilterSitesAsync` and `IsSiteAllowedAsync` to enforce the grant before any operation.
|
||||
`SiteScopeService` (scoped, registered in `AddCentralUI`) reads the role and `SiteId` claims off the circuit principal. Deployment users whose LDAP mapping carries site-scope rules carry one integer `SiteId` claim per permitted site; a user with no `SiteId` claims is system-wide. Pages that enumerate sites or issue cross-site commands call `SiteScopeService.FilterSitesAsync` and `IsSiteAllowedAsync` to enforce the grant before any operation.
|
||||
|
||||
**A user holding the `Administrator` role is system-wide regardless of the `SiteId` claims they carry** (deferred-work register row 86). `ManagementActor.EnforceSiteScope` has always exempted Administrator, so before this the same user could act fleet-wide through the CLI/HTTP surface while the UI hid the same sites — one user, one dataset, two answers. The role claim is read explicitly (`JwtTokenService.RoleClaimType`, `OrdinalIgnoreCase`) rather than via `ClaimsPrincipal.IsInRole`, because `IsInRole` compares claim *values* ordinally and only resolves at all when the identity was built with the matching `roleType`; the actor compares case-insensitively, and the two sides must not disagree on a differently-cased role name. **`Designer` is NOT exempt** — the enforced rule has only ever bypassed Administrator, and this document previously claimed "Admin and Design users carry none (system-wide)", which was true of neither side. An Administrator's `SiteId` claims are not collected at all, so the invariant *system-wide ⇒ empty permitted-id set* holds for every caller and a consumer reading the id set without consulting `IsSystemWideAsync` cannot silently narrow an Administrator to a subset.
|
||||
|
||||
### Repository access pattern
|
||||
|
||||
|
||||
@@ -2,8 +2,10 @@
|
||||
--
|
||||
-- Replaces the single-column [IX_DeploymentRecords_DeployedAt] with the composite
|
||||
-- [IX_DeploymentRecords_DeployedAt_Id] ([DeployedAt] DESC, [Id] DESC), which is exactly
|
||||
-- the order DeploymentManagerRepository.QueryDeploymentListPageAsync (and
|
||||
-- QueryDeploymentSummariesAsync) asks for. With both key columns descending, a forward
|
||||
-- the order DeploymentManagerRepository.QueryDeploymentListPageAsync asks for. (It named
|
||||
-- QueryDeploymentSummariesAsync too; register row 37 folded that second paged read into
|
||||
-- QueryDeploymentListPageAsync, so both the UI and the CLI now share this one key.)
|
||||
-- With both key columns descending, a forward
|
||||
-- scan delivers the requested order, so a paged Skip/Take reads a Top-N off the index
|
||||
-- instead of sorting the whole filtered set on every page request.
|
||||
--
|
||||
|
||||
@@ -149,6 +149,7 @@ Central cluster only. Sites have no user interface.
|
||||
- `IDeploymentManagerRepository.QueryDeploymentListPageAsync(filter, pageNumber, pageSize)` returns one page of `DeploymentListRow` — `DeploymentRecord` **inner-joined to `Instance`**, so the instance's display name and site travel with the rows that need them (the join is exact: the FK is `Restrict` and instance deletion removes the records first) — plus the **total count** of the filtered set.
|
||||
- `GetDeploymentStatusCountsAsync(filter)` returns the tile counts from **one grouped aggregation**. It deliberately **ignores the filter's `Status`** — the tiles are the status *breakdown* of the otherwise-identically-filtered set, so each keeps its own total while it is the selected one.
|
||||
- **Site scoping runs in the query.** The permitted-site grant is pushed in as `SiteIdScope` and resolved through the record's instance; an **empty** grant is a real filter matching nothing, never "unconstrained". `DeploymentRecord` has no `SiteId` of its own — the join is what makes this expressible.
|
||||
- **This is now the ONLY paged read of `DeploymentRecords`.** The CLI/HTTP `QueryDeployments` command had a second one (`QueryDeploymentSummariesAsync`: offset instead of page number, no total, no instance join, instance-id-list scoping); it is **deleted**, and both surfaces share this query. See `Component-ManagementService.md` → *QueryDeployments* for the three behavioral drifts the duplication had accumulated (deferred-work register rows 37, 86–88).
|
||||
- **Offset paging, not the Audit Log's keyset cursor — deliberately.** This page's pager is numbered and jump-to-any-page, so it needs a page count, which only a total can give it; a keyset cursor can express neither. The total is required for the tiles regardless. The deep-offset cost that pushes high-volume tables to keyset is bounded here by the terminal-record retention purge, unlike the 365-day central `AuditLog`. This mirrors the Notification Outbox, which is offset-paged for the same reason. Ordering is `DeployedAt DESC, Id DESC`; the `Id` tie-break is load-bearing, because `DeployedAt` ties on rapid redeploys and an unstable sort key makes offset paging repeat or drop rows between pages.
|
||||
- The whole-table `GetAllDeploymentRecordsAsync` read was **deleted** with its last caller.
|
||||
- **Filtering.** Status filter (the tiles double as the control — clicking one applies it, clicking it again clears) plus a free-text search matched DB-side against instance unique name, deployment id, revision hash and initiating user. A filter change resets to page 1. Search input is **trailing-edge debounced at 500 ms**, so a typed term is one query rather than one per keystroke.
|
||||
|
||||
@@ -147,7 +147,14 @@ The whole folder surface is reachable from the CLI as `template folder list|crea
|
||||
- **DeployArtifacts**: Deploy system-wide artifacts (shared scripts, external system definitions, DB connections, data connections) to all sites or a specific site. The command's `SiteId` is honored (arch-review C2): a value routes to the single-site deploy path (`ArtifactDeploymentService.DeployToSiteAsync`), while `null` deploys fleet-wide (`DeployToAllSitesAsync`). Site scope is enforced — a site-scoped (non-Administrator) Deployer may only target a site within its `PermittedSiteIds`, and may **not** deploy fleet-wide (`SiteId is null` is rejected with `SiteScopeViolationException` → `ManagementUnauthorized`, since `EnforceSiteScope(null)` is a deliberate no-op); fleet-wide deployment requires a system-wide Deployer or Administrator.
|
||||
- **DeploySite** (`MgmtDeploySite`, WP2.5): Bulk-deploy every deployable instance at one site — see the Instances section above and `Component-DeploymentManager.md`.
|
||||
- **GetDeploymentStatus**: Query deployment status.
|
||||
- **QueryDeployments** (`InstanceId?`, `Status?`, `Page`, `PageSize`): List deployment records. **All four arguments are honoured DB-side (WP2.5)** and the result is a `DeploymentRecordSummary` projection. The handler previously ignored `Status`, `Page` and `PageSize` entirely (the CLI had been sending them all along) and loaded the whole insert-only `DeploymentRecords` table; for a site-scoped user it additionally loaded every instance and intersected the two sets in memory. Site scope is now resolved to the set of in-scope instance ids and pushed into the query as an id filter — an **empty** scope stays a real filter (a user permitted no in-scope instances sees nothing), never a no-op. This read path also drives the opportunistic, rate-limited terminal-record retention sweep described in `Component-DeploymentManager.md`.
|
||||
- **QueryDeployments** (`InstanceId?`, `Status?`, `Page`, `PageSize`): List deployment records. **All four arguments are honoured DB-side (WP2.5)** and the result is a `DeploymentQueryResult` envelope — `{ deployments, totalCount, page, pageSize }` over `DeploymentListRow`. The handler previously ignored `Status`, `Page` and `PageSize` entirely (the CLI had been sending them all along) and loaded the whole insert-only `DeploymentRecords` table; for a site-scoped user it additionally loaded every instance and intersected the two sets in memory. This read path also drives the opportunistic, rate-limited terminal-record retention sweep described in `Component-DeploymentManager.md`.
|
||||
|
||||
**One paged read, three drifts closed (deferred-work register rows 37, 86–88).** This handler used to own a second repository query, `QueryDeploymentSummariesAsync`, alongside the Central UI page's `QueryDeploymentListPageAsync`. That query is **deleted**; both surfaces now go through `QueryDeploymentListPageAsync` with a `DeploymentListFilter`, and the `DeploymentRecordSummary` projection went with it (`DeploymentListRow` is the single row shape). The duplication had drifted into three behavioral differences, all resolved here:
|
||||
- **Site scope is a SITE-ID filter, resolved by the query's own join to `Instance`** (row 87). It used to be resolved in the handler by loading **every instance in the system** on every scoped query, purely to turn the site grant into an instance-id list — a second whole-table read, non-transactional with the one that mattered. An **empty** scope remains a real filter (a user permitted no sites sees nothing), never a no-op, and a permitted-site id that is not an integer is **dropped rather than ignored** — dropping can only narrow the grant, whereas discarding the grant would widen it to fleet-wide.
|
||||
- **`PageSize` is bounded** (row 87). A non-positive value used to mean *unlimited* all the way down to the repository, so `--page-size 0` materialized the entire retention window into one response. It now means "unspecified" and takes `DeploymentListPage.DefaultPageSize` (50, the CLI's own default); every value is capped at `DeploymentListPage.MaxPageSize` (1000). `totalCount` travels with the bounded page precisely so a truncated page is distinguishable from a complete one.
|
||||
- **An unrecognised `Status` is an ERROR, not "no filter"** (row 88). `--status Sucess` used to fall back to unfiltered and return every status — a plausible, complete-looking answer to a question the caller never asked. It now returns `ManagementError` naming the valid values (`Pending, InProgress, Success, Failed`). Parsing stays case-insensitive; a numeric string is rejected too (`Enum.TryParse` accepts `"7"` and yields an undefined value, so the parse is guarded by `Enum.IsDefined`). Same loud-failure principle as register row 46.
|
||||
|
||||
**Administrator is system-wide** on this path, as on every other site-scoped handler (`EnforceSiteScope`). The Central UI's `SiteScopeService` was claims-only and disagreed — an Administrator carrying `SiteId` claims saw the fleet here and only their sites on the Deployments page. The UI was corrected to match (register row 86); see `Component-CentralUI.md`.
|
||||
|
||||
### Secured Writes (MxGateway, two-person)
|
||||
|
||||
|
||||
@@ -116,6 +116,7 @@ Set in a local or docker-dev environment via the environment variable `ScadaBrid
|
||||
- Monitor and manage the Notification Outbox (retry and discard parked notifications).
|
||||
- View site event logs.
|
||||
- **Site scoping**: A user with site-scoped Deployer role can only perform these actions for instances at their permitted sites.
|
||||
- **Administrator overrides site scoping.** A user who holds `Administrator` is system-wide **regardless of the `SiteId` claims they carry** — a scoped Deployer grant does not narrow an Administrator. This is enforced identically on both surfaces: `ManagementActor.EnforceSiteScope` (CLI / HTTP management API) and the Central UI's `SiteScopeService`. The two disagreed until deferred-work register row 86 — the UI was claims-only, so the same user saw the whole fleet through the CLI and only their sites in the UI. **No other role carries this bypass**; in particular `Designer` does not (Designer has no site-scoped permissions to bypass in the first place).
|
||||
|
||||
> **Area management (arch-review C6)**: Managing area definitions per site is gated **any-of [Designer, Deployer]**, not Administrator. Areas are authored both in the Designer tooling and inside the Central UI Deployment Topology workflow (`Topology.razor` is `RequireDeployment`), so either role qualifies; Administrator is not implicitly included in the actor gate.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user