feat(policies): org-wide policies with admin-only editing (#6625)

Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
Reece Browne
2026-06-11 21:23:06 +01:00
committed by GitHub
co-authored by Claude Opus 4.8
parent 47e5977a31
commit ef65e6b015
7 changed files with 160 additions and 110 deletions
@@ -11,9 +11,10 @@ import stirling.software.common.service.UserServiceInterface;
import stirling.software.proprietary.policy.model.Policy;
/**
* Decides who may act on a stored {@link Policy}: its owner and global admins only, with no
* separate view/edit/run capability. Enforced only when login is enabled; single-user deployments
* pass every check. The owner is assigned server-side, never from client input.
* Policies are org-wide: every user may view and run any stored policy, so reads and runs are open
* to all. Creating, editing, and deleting is gated to admins at the controller (see {@code
* PolicyController#requirePolicyEditingAllowed}). The owner is still recorded server-side (for run
* / usage attribution) but no longer restricts visibility or access.
*/
@Component
@RequiredArgsConstructor
@@ -27,27 +28,9 @@ public class PolicyAccessGuard {
return enforced() ? userService.getCurrentUsername() : null;
}
/** Whether the current user may view, edit, delete, or run the given stored policy. */
public boolean canAccess(Policy policy) {
if (!enforced() || userService.isCurrentUserAdmin()) {
return true;
}
String current = userService.getCurrentUsername();
return current != null && current.equals(policy.owner());
}
/**
* The subset of {@code policies} the current user may see (their own; everything for an admin).
*/
/** All stored policies are visible to every user (org-wide). */
public List<Policy> visible(List<Policy> policies) {
if (!enforced() || userService.isCurrentUserAdmin()) {
return policies;
}
String current = userService.getCurrentUsername();
if (current == null) {
return List.of();
}
return policies.stream().filter(policy -> current.equals(policy.owner())).toList();
return policies;
}
private boolean enforced() {
@@ -39,7 +39,6 @@ import stirling.software.common.model.job.JobResponse;
import stirling.software.common.service.UserServiceInterface;
import stirling.software.common.util.TempFile;
import stirling.software.common.util.TempFileManager;
import stirling.software.proprietary.policy.config.FolderAccessGuard;
import stirling.software.proprietary.policy.config.PolicyAccessGuard;
import stirling.software.proprietary.policy.engine.PolicyRunHandle;
import stirling.software.proprietary.policy.engine.PolicyRunRegistry;
@@ -72,7 +71,6 @@ public class PolicyController {
private final PolicyRunRegistry runRegistry;
private final PolicyStore policyStore;
private final PolicyValidator policyValidator;
private final FolderAccessGuard folderAccessGuard;
private final PolicyAccessGuard policyAccessGuard;
private final UserServiceInterface userService;
private final ApplicationProperties applicationProperties;
@@ -156,8 +154,8 @@ public class PolicyController {
"Stores a policy (trigger config + steps + output + metadata). A blank id is"
+ " assigned; returns the stored policy with its id.")
public ResponseEntity<Policy> savePolicy(@RequestBody Policy policy) {
requirePolicyEditingAllowed();
Policy owned = resolveOwnership(policy);
requireAuthorizedForFolderAccess(owned);
try {
policyValidator.validate(owned);
} catch (IllegalArgumentException e) {
@@ -167,18 +165,16 @@ public class PolicyController {
}
/**
* Assign the owner: create stamps the current user; update preserves the existing owner after
* an access check. So the client can neither forge ownership on create nor reassign it on
* update.
* Assign the owner: create stamps the current user; update preserves the existing owner — so
* the client can neither forge ownership on create nor reassign it on update. Editing is gated
* to admins by {@link #requirePolicyEditingAllowed}; policies are org-wide, so there is no
* per-owner access check here.
*/
private Policy resolveOwnership(Policy incoming) {
String id = incoming.id();
if (id != null && !id.isBlank()) {
Policy existing = policyStore.get(id).orElse(null);
if (existing != null) {
if (!policyAccessGuard.canAccess(existing)) {
throw new ResponseStatusException(HttpStatus.NOT_FOUND, "No policy: " + id);
}
return withOwner(incoming, existing.owner());
}
}
@@ -198,28 +194,28 @@ public class PolicyController {
}
/**
* Folder sources/outputs grant whoever saves the policy access to that path, so gate them to
* admins on multi-user deployments. Single-user (login disabled) trusts the local operator;
* {@link FolderAccessGuard} still enforces SaaS-off and the path allowlist at validation time.
* Creating, editing, pausing/resuming, and deleting policies is admin-only on multi-user
* deployments. Every mutation routes through {@link #savePolicy} (pause/resume re-save with a
* flipped {@code enabled} flag) or {@link #deletePolicy}, so gating those two covers them all;
* runs ({@code /run}) stay open. Single-user deployments (login disabled) have no admin
* concept, so they trust the local operator. The path allowlist for folder sources/outputs is
* enforced separately by {@link PolicyValidator} at validation time.
*/
private void requireAuthorizedForFolderAccess(Policy policy) {
if (!folderAccessGuard.usesFolderAccess(policy)) {
return;
}
private void requirePolicyEditingAllowed() {
if (!applicationProperties.getSecurity().isEnableLogin()) {
return;
}
if (!userService.isCurrentUserAdmin()) {
throw new ResponseStatusException(
HttpStatus.FORBIDDEN,
"Folder sources and outputs may only be configured by an administrator");
"Policies may only be created or modified by an administrator");
}
}
@GetMapping
@Operation(
summary = "List policies",
description = "Lists the caller's policies; admins see all.")
description = "Lists all policies (org-wide; every user sees them all).")
public List<Policy> listPolicies() {
return policyAccessGuard.visible(policyStore.all());
}
@@ -229,7 +225,6 @@ public class PolicyController {
public ResponseEntity<Policy> getPolicy(@PathVariable String policyId) {
return policyStore
.get(policyId)
.filter(policyAccessGuard::canAccess)
.map(ResponseEntity::ok)
.orElseGet(() -> ResponseEntity.notFound().build());
}
@@ -237,9 +232,8 @@ public class PolicyController {
@DeleteMapping("/{policyId}")
@Operation(summary = "Delete a policy by id")
public ResponseEntity<Void> deletePolicy(@PathVariable String policyId) {
boolean accessible =
policyStore.get(policyId).filter(policyAccessGuard::canAccess).isPresent();
if (accessible && policyStore.delete(policyId)) {
requirePolicyEditingAllowed();
if (policyStore.get(policyId).isPresent() && policyStore.delete(policyId)) {
return ResponseEntity.noContent().build();
}
return ResponseEntity.notFound().build();
@@ -259,7 +253,6 @@ public class PolicyController {
Policy policy =
policyStore
.get(policyId)
.filter(policyAccessGuard::canAccess)
.orElseThrow(
() ->
new ResponseStatusException(
@@ -26,6 +26,7 @@ import stirling.software.common.service.JobQueue;
import stirling.software.common.service.ResourceMonitor;
import stirling.software.common.service.TaskManager;
import stirling.software.common.util.ExecutorFactory;
import stirling.software.common.util.JobContext;
import stirling.software.proprietary.policy.model.OutputSpec;
import stirling.software.proprietary.policy.model.PipelineDefinition;
import stirling.software.proprietary.policy.model.Policy;
@@ -74,22 +75,34 @@ public class PolicyEngine {
*/
public PolicyRunHandle submit(
PipelineDefinition definition, PolicyInputs inputs, PolicyProgressListener listener) {
// Ad-hoc run (no stored policy): bill whoever kicked it off. Capture the principal on this
// (request) thread — it does not survive the hop onto the async worker.
return submitForPrincipal(currentActingPrincipal(), definition, inputs, listener);
// Ad-hoc run (no stored policy): bill whoever kicked it off and own the outputs as them
// too.
// Capture the principal on this (request) thread — it does not survive the hop onto the
// async
// worker.
String principal = currentActingPrincipal();
return submitForPrincipal(principal, principal, definition, inputs, listener);
}
/** Run a stored policy on demand. {@code enabled} gates triggers, not explicit runs. */
public PolicyRunHandle runPolicy(
Policy policy, PolicyInputs inputs, PolicyProgressListener listener) {
// Bill the policy owner. Trigger-fired runs have no security context at all, and even an
// on-demand run executes on a background worker that doesn't inherit the caller's context —
// so the owner (a username stamped at policy creation) is the reliable billing identity.
return submitForPrincipal(policy.owner(), policy.toDefinition(), inputs, listener);
// Bill the policy owner: trigger-fired runs have no security context, and the async worker
// doesn't inherit the caller's, so the owner (stamped at policy creation) is the reliable
// billing identity — and for org-wide policies the org/owner is meant to pay. But own the
// OUTPUT files as the user who triggered the run (captured here on the request thread) so
// they can download their enforced file; otherwise an org-wide policy's output is owned by
// the admin and the triggering user is denied it. Trigger-fired runs have no such user, so
// the owner owns those outputs.
String triggeringUser = currentActingPrincipal();
String fileOwner = triggeringUser != null ? triggeringUser : policy.owner();
return submitForPrincipal(
policy.owner(), fileOwner, policy.toDefinition(), inputs, listener);
}
private PolicyRunHandle submitForPrincipal(
String actingPrincipal,
String billingPrincipal,
String fileOwner,
PipelineDefinition definition,
PolicyInputs inputs,
PolicyProgressListener listener) {
@@ -109,7 +122,8 @@ public class PolicyEngine {
Runnable task =
() ->
runAsPrincipal(
actingPrincipal,
billingPrincipal,
fileOwner,
() -> runToCompletion(run, inputs, tracking, completion));
// One admission unit per run; steps run synchronously within it, so this gates heavy work
@@ -297,21 +311,30 @@ public class PolicyEngine {
* dispatch attributes (and charges) usage to that user. A null/blank principal runs as-is.
* Restores the previous MDC value afterward (defensive — worker threads aren't pooled).
*/
private static void runAsPrincipal(String principal, Runnable body) {
if (principal == null || principal.isBlank()) {
body.run();
return;
private static void runAsPrincipal(String billingPrincipal, String fileOwner, Runnable body) {
// Billing identity (MDC auditPrincipal) and output-file ownership (JobContext owner) are
// set
// independently: usage is charged to billingPrincipal, but stored output files are owned by
// fileOwner — the user who triggered an org-wide policy — so they can fetch their results.
// Either may be null (e.g. login disabled, or a trigger-fired run); each is applied only
// when present and restored afterward (defensive — worker threads aren't pooled).
String previousPrincipal = MDC.get(AUDIT_PRINCIPAL_MDC_KEY);
String previousOwner = JobContext.getOwner();
if (billingPrincipal != null && !billingPrincipal.isBlank()) {
MDC.put(AUDIT_PRINCIPAL_MDC_KEY, billingPrincipal);
}
if (fileOwner != null && !fileOwner.isBlank()) {
JobContext.setOwner(fileOwner);
}
String previous = MDC.get(AUDIT_PRINCIPAL_MDC_KEY);
MDC.put(AUDIT_PRINCIPAL_MDC_KEY, principal);
try {
body.run();
} finally {
if (previous != null) {
MDC.put(AUDIT_PRINCIPAL_MDC_KEY, previous);
if (previousPrincipal != null) {
MDC.put(AUDIT_PRINCIPAL_MDC_KEY, previousPrincipal);
} else {
MDC.remove(AUDIT_PRINCIPAL_MDC_KEY);
}
JobContext.setOwner(previousOwner);
}
}
}
@@ -1,9 +1,7 @@
package stirling.software.proprietary.policy.config;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.when;
import java.util.List;
@@ -19,8 +17,9 @@ import stirling.software.proprietary.policy.model.OutputSpec;
import stirling.software.proprietary.policy.model.Policy;
/**
* Tests for {@link PolicyAccessGuard}: owner-or-admin access, no-op when login is disabled, and
* server-side owner assignment.
* Tests for {@link PolicyAccessGuard}. Policies are org-wide: every user sees them all (no
* owner-based filtering). The owner is still assigned server-side — the current user when login is
* enabled, {@code null} otherwise — purely for run/usage attribution.
*/
@ExtendWith(MockitoExtension.class)
class PolicyAccessGuardTest {
@@ -34,53 +33,30 @@ class PolicyAccessGuardTest {
}
@Test
void loginDisabledAllowsEverythingAndAssignsNoOwner() {
PolicyAccessGuard guard = guard(false);
void visibleReturnsEveryPolicyWhenLoginEnabled() {
// Org-wide: a non-admin sees every policy, not just the ones they own.
List<Policy> all = List.of(ownedBy("alice"), ownedBy("bob"), ownedBy("alice"));
assertEquals(all, guard(true).visible(all));
}
@Test
void visibleReturnsEveryPolicyWhenLoginDisabled() {
List<Policy> all = List.of(ownedBy("alice"), ownedBy("bob"));
assertTrue(guard.canAccess(ownedBy("someone-else")));
assertNull(guard.ownerForNewPolicy());
assertEquals(all, guard.visible(all));
assertEquals(all, guard(false).visible(all));
}
@Test
void adminCanAccessAnyPolicy() {
when(userService.isCurrentUserAdmin()).thenReturn(true);
assertTrue(guard(true).canAccess(ownedBy("alice")));
}
@Test
void ownerCanAccessTheirOwnPolicy() {
when(userService.isCurrentUserAdmin()).thenReturn(false);
when(userService.getCurrentUsername()).thenReturn("alice");
assertTrue(guard(true).canAccess(ownedBy("alice")));
}
@Test
void nonOwnerNonAdminCannotAccess() {
when(userService.isCurrentUserAdmin()).thenReturn(false);
when(userService.getCurrentUsername()).thenReturn("bob");
assertFalse(guard(true).canAccess(ownedBy("alice")));
}
@Test
void visibleFiltersToOwnedPoliciesForANonAdmin() {
when(userService.isCurrentUserAdmin()).thenReturn(false);
when(userService.getCurrentUsername()).thenReturn("alice");
List<Policy> visible =
guard(true).visible(List.of(ownedBy("alice"), ownedBy("bob"), ownedBy("alice")));
assertEquals(2, visible.size());
assertTrue(visible.stream().allMatch(policy -> "alice".equals(policy.owner())));
}
@Test
void ownerForNewPolicyIsTheCurrentUserWhenEnforced() {
void ownerForNewPolicyIsTheCurrentUserWhenLoginEnabled() {
when(userService.getCurrentUsername()).thenReturn("alice");
assertEquals("alice", guard(true).ownerForNewPolicy());
}
@Test
void ownerForNewPolicyIsNullWhenLoginDisabled() {
// Single-user deployment: no identity to attribute to.
assertNull(guard(false).ownerForNewPolicy());
}
private static Policy ownedBy(String owner) {
return new Policy("p1", "p", owner, true, null, List.of(), List.of(), OutputSpec.inline());
}
@@ -0,0 +1,65 @@
import { test, expect } from "@app/tests/helpers/stub-test-base";
/**
* Policy editing is admin-only (mirrors the backend's enforcement on the
* save/delete endpoints). The frontend gate is `canConfigure = !enableLogin ||
* isAdmin`, surfaced through `usePolicies()`:
* - login enabled + non-admin → read-only: the setup wizard shows the
* "Managed by your organization" locked state instead of the steps.
* - login enabled + admin → full setup flow ("Step 1 of 2").
* - login disabled (single-user/desktop) → open to the local operator.
*
* Backend-free: the app-config (`enableLogin`/`isAdmin`) and the empty policy
* list are stubbed via `mockAppApis`, so this asserts the gating wiring without
* a live Spring Boot server. "Security" is the only non-coming-soon policy, so
* it's the one we open.
*/
const LOCKED_TITLE = "Managed by your organization";
const LOCKED_DESC = "Contact an admin to change this policy.";
/** Open the Security policy from the right-sidebar Policies list. */
async function openSecurityPolicy(page: import("@playwright/test").Page) {
const row = page.locator("button.pol-row").filter({ hasText: "Security" });
await expect(row).toBeVisible({ timeout: 15_000 });
await row.click();
// The wizard header confirms we opened the right policy in either state.
await expect(page.getByText("Set up Security Policy")).toBeVisible();
}
test.describe("Policy editing gate — non-admin (login on)", () => {
test.use({
stubOptions: { enableLogin: true, isAdmin: false },
seedJwt: true,
});
test("non-admin gets the read-only locked state", async ({ page }) => {
await openSecurityPolicy(page);
await expect(page.getByText(LOCKED_TITLE)).toBeVisible();
await expect(page.getByText(LOCKED_DESC)).toBeVisible();
// The editable flow must NOT be reachable.
await expect(page.getByText(/Step \d+ of \d+/)).toHaveCount(0);
});
});
test.describe("Policy editing gate — admin (login on)", () => {
test.use({ stubOptions: { enableLogin: true, isAdmin: true }, seedJwt: true });
test("admin can reach the setup wizard", async ({ page }) => {
await openSecurityPolicy(page);
await expect(page.getByText(/Step \d+ of \d+/)).toBeVisible();
await expect(page.getByText(LOCKED_TITLE)).toHaveCount(0);
});
});
test.describe("Policy editing gate — single-user (login off)", () => {
test.use({ stubOptions: { enableLogin: false, isAdmin: false } });
test("local operator can reach the setup wizard with no admin role", async ({
page,
}) => {
await openSecurityPolicy(page);
await expect(page.getByText(/Step \d+ of \d+/)).toBeVisible();
await expect(page.getByText(LOCKED_TITLE)).toHaveCount(0);
});
});
@@ -249,7 +249,7 @@ export function PolicyDetailPanel({
<Banner
tone="neutral"
icon={<LockIcon sx={{ fontSize: "1rem" }} />}
description="Managed by your organization. Contact an admin to change settings."
description="Managed by your organization. Contact an admin to change this policy."
/>
)}
</div>
@@ -7,6 +7,7 @@
*/
import { useState, useEffect, useCallback } from "react";
import { useAppConfig } from "@app/contexts/AppConfigContext";
import {
loadPolicies,
onPoliciesChange,
@@ -63,6 +64,7 @@ function toStoreRequest(
export function usePolicies() {
const [policies, setPolicies] = useState<PoliciesByCategory>(loadPolicies);
const { config } = useAppConfig();
useEffect(() => onPoliciesChange(() => setPolicies(loadPolicies())), []);
@@ -291,9 +293,17 @@ export function usePolicies() {
return folder.id;
}, []);
// Configuration is open to signed-in users; real per-org gating is a backend
// concern (the mock owner/admin/member permission model has been removed).
const canConfigure = true;
// Editing/creating policies is admin-only, mirroring the backend's enforcement
// on the save/delete endpoints. On single-user deployments (login disabled)
// there's no admin concept, so the local operator can always configure. When
// login is enabled, only admins can — non-admins get the read-only surface.
// The `config != null` guard keeps the gate CLOSED until app-config resolves:
// config is null while loading (and in bare test renders), and a failed/partial
// fetch can yield `{ enableLogin: true }` with isAdmin omitted. Without it a
// non-admin would briefly see edit controls during load, and an admin would be
// wrongly locked out on a transient config-fetch error.
const canConfigure =
config != null && (!config.enableLogin || config.isAdmin === true);
return {
policies,