mirror of
https://github.com/Stirling-Tools/Stirling-PDF.git
synced 2026-09-02 21:03:34 +03:00
Require the policy-management role to run a policy against its sources (#7565)
## What
Running a stored policy against its **configured sources** (`POST
/api/v1/policies/{id}/trigger`, the manual "run now") now requires the
policy-management role — global admin self-hosted, team leader on SaaS —
alongside the existing team scoping.
## Why
A source sweep operates on the team's configured sources using the
server's stored connection credentials, so it belongs with the other
policy-management capabilities rather than with ordinary use. Team
scoping on its own didn't express that distinction.
## Not changed
- `POST /{id}/run` — running a policy over documents the **caller
supplied** stays open to every team member. That's ordinary editor
enforcement on upload and export, and gating it would break it.
- Ad-hoc pipelines (`/run`, `/run/stream`).
- The scheduled, folder-watch and webhook triggers.
- Single-user deployments (login disabled), which have no roles.
## Implementation
`PolicyManagementAuthority` gains `canTriggerPolicies()`, kept separate
from `canEditPolicies()` so the two capabilities can diverge later. Both
current implementations grant it to the same principals that may edit
policies.
## Tests
- role absent → 403, rejected before any run starts
- role present → 202
- login disabled → check skipped entirely
- `/{id}/run` asserted to consult neither authority method, so the gate
can't quietly extend to the editor path later
This commit is contained in:
+5
@@ -26,6 +26,11 @@ public class AdminPolicyManagementAuthority implements PolicyManagementAuthority
|
||||
return userService.isCurrentUserAdmin();
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean canTriggerPolicies() {
|
||||
return userService.isCurrentUserAdmin();
|
||||
}
|
||||
|
||||
@Override
|
||||
public Long currentUserTeamId() {
|
||||
String username = userService.getCurrentUsername();
|
||||
|
||||
+11
@@ -12,6 +12,17 @@ public interface PolicyManagementAuthority {
|
||||
/** Whether the current user may create, edit, or delete policies (for their own team). */
|
||||
boolean canEditPolicies();
|
||||
|
||||
/**
|
||||
* Whether the current user may run a policy against its <em>configured sources</em> (the manual
|
||||
* "run now" sweep). Kept separate from {@link #canEditPolicies()} because the two are distinct
|
||||
* capabilities, even where a deployment grants both to the same people: a sweep operates on the
|
||||
* team's configured sources using the server's stored connection credentials, which makes it a
|
||||
* policy-management capability rather than ordinary use. Running a policy over the caller's
|
||||
* <em>own</em> uploaded files is not covered by this and stays open to every team member — that
|
||||
* is ordinary editor enforcement.
|
||||
*/
|
||||
boolean canTriggerPolicies();
|
||||
|
||||
/**
|
||||
* The team that scopes the current user's policies — the team a new policy is stamped with and
|
||||
* the only team whose policies the user may see/run/edit. {@code null} when it can't be
|
||||
|
||||
+26
-4
@@ -432,9 +432,10 @@ public class PolicyController {
|
||||
* admin gets no say on SaaS. Team scoping (which team's policies) is enforced separately by
|
||||
* {@link PolicyAccessGuard}. 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 to the team. Single-user deployments (login
|
||||
* disabled) have no such role, so they trust the local operator. The path allowlist for folder
|
||||
* sources/outputs is enforced separately by {@link PolicyValidator} at validation time.
|
||||
* covers them all; runs over the caller's own files ({@code /{id}/run}) stay open to the team,
|
||||
* while source sweeps are gated by {@link #requirePolicySweepAllowed}. Single-user deployments
|
||||
* (login disabled) have no such role, so they trust the local operator. The path allowlist for
|
||||
* folder sources/outputs is enforced separately by {@link PolicyValidator} at validation time.
|
||||
*/
|
||||
private void requirePolicyEditingAllowed() {
|
||||
if (!applicationProperties.getSecurity().isEnableLogin()) {
|
||||
@@ -447,6 +448,25 @@ public class PolicyController {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Sweeping a policy's configured sources requires the same role as managing policies: the sweep
|
||||
* operates on the team's configured sources using the server's stored connection credentials,
|
||||
* which makes it a policy-management capability rather than ordinary use, and team scoping on
|
||||
* its own does not express that. Deliberately narrower than it looks: it gates only the sweep,
|
||||
* not {@link #runStoredPolicy}, because running a policy over documents the caller supplied is
|
||||
* ordinary editor enforcement that every member performs on upload and export.
|
||||
*/
|
||||
private void requirePolicySweepAllowed() {
|
||||
if (!applicationProperties.getSecurity().isEnableLogin()) {
|
||||
return;
|
||||
}
|
||||
if (!policyManagementAuthority.canTriggerPolicies()) {
|
||||
throw new ResponseStatusException(
|
||||
HttpStatus.FORBIDDEN,
|
||||
"Not permitted to run this policy against its configured sources");
|
||||
}
|
||||
}
|
||||
|
||||
@GetMapping
|
||||
@Operation(
|
||||
summary = "List policies",
|
||||
@@ -571,8 +591,10 @@ public class PolicyController {
|
||||
+ " the enabled flag (which only gates automatic triggering). Returns"
|
||||
+ " the ids of the runs started (poll the run-status endpoint for each)"
|
||||
+ " plus what the sweep skipped - already-processed, parked-by-failure,"
|
||||
+ " and in-flight counts - so an empty result explains itself.")
|
||||
+ " and in-flight counts - so an empty result explains itself. Requires"
|
||||
+ " the policy-management role.")
|
||||
public ResponseEntity<SweepOutcome> trigger(@PathVariable String policyId) {
|
||||
requirePolicySweepAllowed();
|
||||
Policy policy =
|
||||
policyStore
|
||||
.get(policyId)
|
||||
|
||||
+12
@@ -39,6 +39,18 @@ class AdminPolicyManagementAuthorityTest {
|
||||
assertFalse(authority().canEditPolicies());
|
||||
}
|
||||
|
||||
@Test
|
||||
void adminMayTriggerPolicies() {
|
||||
when(userService.isCurrentUserAdmin()).thenReturn(true);
|
||||
assertTrue(authority().canTriggerPolicies());
|
||||
}
|
||||
|
||||
@Test
|
||||
void nonAdminMayNotTriggerPolicies() {
|
||||
when(userService.isCurrentUserAdmin()).thenReturn(false);
|
||||
assertFalse(authority().canTriggerPolicies());
|
||||
}
|
||||
|
||||
@Test
|
||||
void currentUserTeamIdResolvesFromTheCurrentUsersTeam() {
|
||||
Team team = new Team();
|
||||
|
||||
+74
@@ -2,6 +2,7 @@ package stirling.software.proprietary.policy.controller;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
@@ -738,5 +739,78 @@ class PolicyControllerTest {
|
||||
assertThat(((ResponseStatusException) e).getStatusCode())
|
||||
.isEqualTo(HttpStatus.NOT_FOUND));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("trigger is forbidden for a team member who cannot manage policies")
|
||||
void triggerForbiddenForMember() {
|
||||
// Sweeping a policy's configured sources is a policy-management capability, so being
|
||||
// in the policy's team is not on its own enough to perform it.
|
||||
applicationProperties.getSecurity().setEnableLogin(true);
|
||||
when(policyManagementAuthority.canTriggerPolicies()).thenReturn(false);
|
||||
|
||||
assertThatThrownBy(() -> controller.trigger("a"))
|
||||
.isInstanceOf(ResponseStatusException.class)
|
||||
.satisfies(
|
||||
e ->
|
||||
assertThat(((ResponseStatusException) e).getStatusCode())
|
||||
.isEqualTo(HttpStatus.FORBIDDEN));
|
||||
// Rejected before the policy is looked up, so no run starts.
|
||||
verify(policyRunner, never()).run(any());
|
||||
verify(policyStore, never()).get(any());
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("trigger runs for a caller who may manage policies")
|
||||
void triggerAllowedForLeader() {
|
||||
applicationProperties.getSecurity().setEnableLogin(true);
|
||||
when(policyManagementAuthority.canTriggerPolicies()).thenReturn(true);
|
||||
Policy p = policy("a", 1L);
|
||||
when(policyStore.get("a")).thenReturn(Optional.of(p));
|
||||
when(policyAccessGuard.canAccess(p)).thenReturn(true);
|
||||
SweepOutcome outcome = new SweepOutcome(List.of("run-a"), 1, 0, 0, 0);
|
||||
when(policyRunner.run(p)).thenReturn(outcome);
|
||||
|
||||
ResponseEntity<SweepOutcome> response = controller.trigger("a");
|
||||
|
||||
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.ACCEPTED);
|
||||
assertThat(response.getBody()).isEqualTo(outcome);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("trigger skips the role check when login is disabled")
|
||||
void triggerTrustsTheLocalOperator() {
|
||||
// Single-user deployments have no roles at all; the gate must not lock them out of
|
||||
// their
|
||||
// own sweeps.
|
||||
applicationProperties.getSecurity().setEnableLogin(false);
|
||||
Policy p = policy("a", null);
|
||||
when(policyStore.get("a")).thenReturn(Optional.of(p));
|
||||
when(policyAccessGuard.canAccess(p)).thenReturn(true);
|
||||
SweepOutcome outcome = new SweepOutcome(List.of("run-a"), 1, 0, 0, 0);
|
||||
when(policyRunner.run(p)).thenReturn(outcome);
|
||||
|
||||
assertThat(controller.trigger("a").getStatusCode()).isEqualTo(HttpStatus.ACCEPTED);
|
||||
verify(policyManagementAuthority, never()).canTriggerPolicies();
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("running a policy over the caller's own files stays open to any member")
|
||||
void storedRunIsNotGatedByRole() {
|
||||
// Editor enforcement: every member's upload/export runs the team's stored policies on
|
||||
// their own documents. Gating this the way the sweep is gated would break the editor.
|
||||
applicationProperties.getSecurity().setEnableLogin(true);
|
||||
Policy p = policy("a", 1L);
|
||||
when(policyStore.get("a")).thenReturn(Optional.of(p));
|
||||
when(policyAccessGuard.canAccess(p)).thenReturn(true);
|
||||
when(policyRunner.runWith(eq(p), any(), eq(PolicyProgressListener.NOOP)))
|
||||
.thenReturn(handle("run-9"));
|
||||
|
||||
ResponseEntity<JobResponse<Void>> response =
|
||||
assertDoesNotThrow(() -> controller.runStoredPolicy("a", new PolicyRunFiles()));
|
||||
|
||||
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.ACCEPTED);
|
||||
verify(policyManagementAuthority, never()).canTriggerPolicies();
|
||||
verify(policyManagementAuthority, never()).canEditPolicies();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+5
@@ -25,6 +25,11 @@ public class TeamLeaderPolicyManagementAuthority implements PolicyManagementAuth
|
||||
return teamSecurity.isCurrentUserTeamLeader();
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean canTriggerPolicies() {
|
||||
return teamSecurity.isCurrentUserTeamLeader();
|
||||
}
|
||||
|
||||
@Override
|
||||
public Long currentUserTeamId() {
|
||||
return teamSecurity.currentUserTeamId();
|
||||
|
||||
+12
@@ -32,6 +32,18 @@ class TeamLeaderPolicyManagementAuthorityTest {
|
||||
assertFalse(authority().canEditPolicies());
|
||||
}
|
||||
|
||||
@Test
|
||||
void teamLeaderMayTriggerPolicies() {
|
||||
when(teamSecurity.isCurrentUserTeamLeader()).thenReturn(true);
|
||||
assertTrue(authority().canTriggerPolicies());
|
||||
}
|
||||
|
||||
@Test
|
||||
void nonLeaderMayNotTriggerPolicies() {
|
||||
when(teamSecurity.isCurrentUserTeamLeader()).thenReturn(false);
|
||||
assertFalse(authority().canTriggerPolicies());
|
||||
}
|
||||
|
||||
@Test
|
||||
void currentUserTeamIdDelegatesToTeamSecurity() {
|
||||
when(teamSecurity.currentUserTeamId()).thenReturn(9L);
|
||||
|
||||
Reference in New Issue
Block a user