From b9d48f56814735a3b5cc913ef0b1d1eb89971ba8 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Thu, 23 Apr 2026 19:41:13 +0200 Subject: [PATCH] feat(adr-023): ActionAuthService skeleton + repair step + empty seed + 8 unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ports the ADR-023 action-authorization pattern from decidesk's reference implementation (decidesk@88844d0) to the template so every new app picks it up for free. What's included: * lib/Service/ActionAuthService.php — verbatim service: - requireAction (throws OCSForbiddenException) - can (non-throwing bool variant) - getAllowedGroups / getMatrix / setMatrix / getActions - Admin always passes (break-glass) - Default-deny on missing or malformed matrix - Matrix stored in IAppConfig under 'actions' key * lib/Repair/InitializeActions.php — seeds lib/actions.seed.json on install; preserves admin-customized matrix on upgrade * lib/actions.seed.json — empty "actions" object (template ships with no actions; implementers add one entry per requireAction() call site as the app grows) * tests/Unit/Service/ActionAuthServiceTest.php — 8 unit tests covering the contract: 1. Admin always passes 2. Default-deny on missing matrix entry 3. Admin-only matrix entry blocks non-admin 4. Group-matching non-admin passes 5. Admin in entry doesn't auto-pass non-admin 6. can() returns bool 7. Malformed matrix JSON falls back to deny 8. getAllowedGroups defaults to ['admin'] * appinfo/info.xml — registered Repair\InitializeActions on install + post-migration * openspec/architecture/adr-023-action-authorization.md — ADR copied from hydra for the app's local reference Implementers declare an action name in their controller (e.g. 'item.publish'), add it to actions.seed.json with ['admin'] default, then call $this->actionAuth->requireAction($user, 'item.publish') wrapped in try/catch for OCSForbiddenException. Admin broadens the group list via a settings UI (not included in this template port — a follow-up change can port decidesk's SettingsController matrix API). @spec openspec/architecture/adr-023-action-authorization.md --- appinfo/info.xml | 10 + lib/Repair/InitializeActions.php | 127 +++++++++ lib/Service/ActionAuthService.php | 242 +++++++++++++++++ lib/actions.seed.json | 4 + .../adr-023-action-authorization.md | 183 +++++++++++++ tests/Unit/Service/ActionAuthServiceTest.php | 246 ++++++++++++++++++ 6 files changed, 812 insertions(+) create mode 100644 lib/Repair/InitializeActions.php create mode 100644 lib/Service/ActionAuthService.php create mode 100644 lib/actions.seed.json create mode 100644 openspec/architecture/adr-023-action-authorization.md create mode 100644 tests/Unit/Service/ActionAuthServiceTest.php diff --git a/appinfo/info.xml b/appinfo/info.xml index 63c6eae..9113dc4 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -65,6 +65,16 @@ Vrij en open source onder de EUPL-1.2-licentie. + + + + OCA\AppTemplate\Repair\InitializeActions + + + OCA\AppTemplate\Repair\InitializeActions + + + OCA\AppTemplate\Settings\AdminSettings OCA\AppTemplate\Sections\SettingsSection diff --git a/lib/Repair/InitializeActions.php b/lib/Repair/InitializeActions.php new file mode 100644 index 0000000..c3290b5 --- /dev/null +++ b/lib/Repair/InitializeActions.php @@ -0,0 +1,127 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @link https://conduction.nl + * + * @spec openspec/architecture/adr-023-action-authorization.md + */ + +declare(strict_types=1); + +namespace OCA\AppTemplate\Repair; + +use OCA\AppTemplate\Service\ActionAuthService; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; +use Psr\Log\LoggerInterface; + +/** + * Seed the action-authorization matrix from lib/actions.seed.json on install. + * + * @spec openspec/architecture/adr-023-action-authorization.md + */ +class InitializeActions implements IRepairStep +{ + private const SEED_PATH = __DIR__ . '/../actions.seed.json'; + + /** + * Constructor. + * + * @param ActionAuthService $actionAuth The action authorization service. + * @param LoggerInterface $logger Logger. + */ + public function __construct( + private ActionAuthService $actionAuth, + private LoggerInterface $logger, + ) { + }//end __construct() + + /** + * Repair-step name. + * + * @return string + */ + public function getName(): string + { + return 'Initialize action-authorization matrix (ADR-023)'; + + }//end getName() + + /** + * Seed the matrix if empty; preserve any existing admin-customized matrix. + * + * @param IOutput $output Repair output channel. + * + * @return void + */ + public function run(IOutput $output): void + { + $existing = $this->actionAuth->getMatrix(); + if (count($existing) > 0) { + $output->info( + sprintf( + 'Action matrix already has %d entr%s — preserving.', + count($existing), + (count($existing) === 1 ? 'y' : 'ies') + ) + ); + return; + } + + if (file_exists(self::SEED_PATH) === false) { + $output->warning('actions.seed.json not found — matrix left empty (default-deny).'); + $this->logger->warning('[app-template] ADR-023 seed file missing at ' . self::SEED_PATH); + return; + } + + $raw = file_get_contents(self::SEED_PATH); + if ($raw === false) { + $output->warning('Could not read actions.seed.json — matrix left empty (default-deny).'); + return; + } + + try { + $parsed = json_decode($raw, associative: true, depth: 512, flags: JSON_THROW_ON_ERROR); + } catch (\JsonException $e) { + $output->warning('actions.seed.json invalid JSON: ' . $e->getMessage()); + $this->logger->error('[app-template] ADR-023 seed malformed: ' . $e->getMessage()); + return; + } + + $actions = ($parsed['actions'] ?? null); + if (is_array($actions) === false) { + $output->warning('actions.seed.json missing `actions` object — matrix left empty.'); + return; + } + + try { + $this->actionAuth->setMatrix($actions); + } catch (\JsonException $e) { + $output->warning('Failed to write matrix: ' . $e->getMessage()); + return; + } + + $output->info( + sprintf( + 'Seeded action matrix with %d action%s (default: admin-only).', + count($actions), + (count($actions) === 1 ? '' : 's') + ) + ); + + }//end run() + +}//end class diff --git a/lib/Service/ActionAuthService.php b/lib/Service/ActionAuthService.php new file mode 100644 index 0000000..2ea79a0 --- /dev/null +++ b/lib/Service/ActionAuthService.php @@ -0,0 +1,242 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @link https://conduction.nl + * + * @spec openspec/architecture/adr-023-action-authorization.md + */ + +declare(strict_types=1); + +namespace OCA\AppTemplate\Service; + +use OCA\AppTemplate\AppInfo\Application; +use OCP\AppFramework\OCS\OCSForbiddenException; +use OCP\IAppConfig; +use OCP\IGroupManager; +use OCP\IUser; + +/** + * Action-level authorization service. + * + * Enforces ADR-023 action RBAC: controllers call requireAction with a + * dot-separated action name; this service checks the admin-configured + * action-to-group mapping stored in IAppConfig. + */ +class ActionAuthService +{ + private const CONFIG_KEY = 'actions'; + + /** + * Constructor. + * + * @param IAppConfig $appConfig IAppConfig for reading/writing the matrix + * @param IGroupManager $groupManager Group manager for resolving user groups + */ + public function __construct( + private IAppConfig $appConfig, + private IGroupManager $groupManager, + ) { + }//end __construct() + + /** + * Require that the user may perform the named action. + * + * Admin users always pass (break-glass). Non-admins pass only when any + * of their groups intersects the matrix entry for the action. + * + * @param IUser $user The authenticated user. + * @param string $action Dot-separated action name (e.g. "item.publish"). + * + * @return void + * + * @throws OCSForbiddenException When the user's groups don't match the action's allowed groups. + */ + public function requireAction(IUser $user, string $action): void + { + // Admin always passes — break-glass for ops / debugging. + if ($this->groupManager->isAdmin($user->getUID()) === true) { + return; + } + + $allowedGroups = $this->getAllowedGroups($action); + + // An "admin"-only entry means non-admins never pass (admin already + // returned above). Empty entry means nobody is allowed. + if (count($allowedGroups) === 0 || $allowedGroups === ['admin']) { + throw new OCSForbiddenException( + "Action '{$action}' requires admin rights" + ); + } + + $userGroups = $this->groupManager->getUserGroupIds($user); + + // Exclude "admin" from matrix entry before intersection — admin was + // already checked above; its presence in the entry is a display hint, + // not a group membership check. + $nonAdminAllowed = array_values(array_diff($allowedGroups, ['admin'])); + + if (count(array_intersect($userGroups, $nonAdminAllowed)) === 0) { + throw new OCSForbiddenException( + "Action '{$action}' not allowed for your groups" + ); + } + + }//end requireAction() + + /** + * Check whether the user may perform the named action (non-throwing). + * + * @param IUser $user The authenticated user. + * @param string $action Dot-separated action name. + * + * @return bool True if the user may perform the action. + */ + public function can(IUser $user, string $action): bool + { + try { + $this->requireAction($user, $action); + return true; + } catch (OCSForbiddenException $e) { + return false; + } + + }//end can() + + /** + * Get the list of groups allowed to perform the action. + * + * Returns the matrix entry for the action, or ["admin"] as the safe + * default when the action is not in the matrix. + * + * @param string $action Dot-separated action name. + * + * @return array + */ + public function getAllowedGroups(string $action): array + { + $matrix = $this->getMatrix(); + return $matrix[$action] ?? ['admin']; + + }//end getAllowedGroups() + + /** + * Get the full action-to-groups matrix. + * + * Reads the JSON-encoded matrix from IAppConfig. Missing or malformed + * config returns an empty array (default-deny — admin-only for every + * action since getAllowedGroups falls back to ["admin"]). + * + * @return array> + */ + public function getMatrix(): array + { + $json = $this->appConfig->getValueString(Application::APP_ID, self::CONFIG_KEY, '{}'); + + try { + $decoded = json_decode($json, associative: true, depth: 512, flags: JSON_THROW_ON_ERROR); + } catch (\JsonException $e) { + return []; + } + + if (is_array($decoded) === false) { + return []; + } + + // Normalize: discard any non-array values + any non-string group entries. + $matrix = []; + foreach ($decoded as $action => $groups) { + if (is_string($action) === false || is_array($groups) === false) { + continue; + } + + $clean = []; + foreach ($groups as $g) { + if (is_string($g) === true && $g !== '') { + $clean[] = $g; + } + } + + $matrix[$action] = array_values(array_unique($clean)); + } + + return $matrix; + + }//end getMatrix() + + /** + * Set the full action-to-groups matrix. + * + * Caller MUST enforce admin-only before invoking (this method does not + * gate writes — it's called from an admin-only settings endpoint). + * + * @param array> $matrix The new matrix. + * + * @return void + * + * @throws \JsonException When the matrix cannot be encoded. + */ + public function setMatrix(array $matrix): void + { + // Normalize on write — same shape as getMatrix returns. + $normalized = []; + foreach ($matrix as $action => $groups) { + if (is_string($action) === false || is_array($groups) === false) { + continue; + } + + $clean = []; + foreach ($groups as $g) { + if (is_string($g) === true && $g !== '') { + $clean[] = $g; + } + } + + $normalized[$action] = array_values(array_unique($clean)); + } + + $json = json_encode($normalized, flags: JSON_THROW_ON_ERROR); + $this->appConfig->setValueString(Application::APP_ID, self::CONFIG_KEY, $json); + + }//end setMatrix() + + /** + * List all action keys currently in the matrix. + * + * @return array + */ + public function getActions(): array + { + return array_keys($this->getMatrix()); + + }//end getActions() + +}//end class diff --git a/lib/actions.seed.json b/lib/actions.seed.json new file mode 100644 index 0000000..dc85a1b --- /dev/null +++ b/lib/actions.seed.json @@ -0,0 +1,4 @@ +{ + "$comment": "ADR-023 action-authorization matrix seed. Each entry maps a dot-separated action name to an array of group IDs allowed to invoke it. The special 'admin' group means Nextcloud admins; admins always pass regardless. Empty array or ['admin'] means admin-only. Admins customize via Admin Settings > App Template > Actions. Add entries below as the app grows — one per ActionAuthService::requireAction() call site. Template ships with an example commented pattern; remove the $comment field and add real actions when implementing your app.", + "actions": {} +} diff --git a/openspec/architecture/adr-023-action-authorization.md b/openspec/architecture/adr-023-action-authorization.md new file mode 100644 index 0000000..8a32855 --- /dev/null +++ b/openspec/architecture/adr-023-action-authorization.md @@ -0,0 +1,183 @@ +# ADR-023: Action-level authorization via admin-configured action/group mappings + +**Status:** accepted +**Date:** 2026-04-23 + +## Context + +Conduction apps mix **data authorization** (who can read/write which OpenRegister objects) and **action authorization** (who can invoke which controller methods / workflow steps). The two are related but not the same: + +- A chair of "Board A" can read all Board A minutes (data RBAC → OpenRegister) AND can invoke `generateMinutesDraft()` on them (action RBAC → app). +- A regular member of Board A can read the same minutes (data RBAC → OpenRegister) but CANNOT invoke `generateMinutesDraft()` (action RBAC denies). +- A Nextcloud admin can invoke `create()` on `SettingsController` (action RBAC → admin-only) regardless of any board membership. + +OpenRegister already owns the **data** layer: object-level ownership, schema/register permissions, per-relation filtering (ADR-022 lists RBAC as one of the shared abstractions it provides). Apps consume this cleanly. + +Apps DO NOT have a shared pattern for the **action** layer. Observed across decidesk / docudesk / pipelinq, the action-auth implementations range from: + +- `IGroupManager::isAdmin()` hardcoded checks in controller bodies (wrong — locks governance actions to Nextcloud sysadmins, not to chairs/secretaries — see #44 / #45 on 2026-04-23) +- Missing entirely (the endpoint gates on data RBAC alone — wrong for actions that cross objects, like "generate report across all boards I chair") +- Inline `!in_array('chair', $roles)` checks that are (a) not discoverable by admins, (b) require a code change to adjust, (c) duplicated across controllers + +The consistent answer needs to: live in app code (each app has its own actions), be **declarative** (admin can see and change the matrix without touching code), and be **testable** (gate-7 / gate-9 can mechanically verify each routed action either delegates to this service or is explicitly marked admin-only). + +## Decision + +### Rule 1 — Data RBAC is OpenRegister's job; apps never roll their own + +OpenRegister decides for itself who may read / write / list which objects. App code that fetches, lists, or mutates domain objects MUST go through OpenRegister's `ObjectService` and trust the service's filtering + per-object permissions. Apps do not implement: + +- Object-ownership checks (OpenRegister does it via `createdBy` / `owner` / schema settings) +- Register/schema-level access gates (OpenRegister does it via register permissions) +- Group-based read/write filtering on data (OpenRegister does it via `relations.group` / schema RBAC) +- Schema / register configuration (that's OpenRegister's own admin UI, not the consuming app's) + +If the data-layer RBAC has a gap, **fix it in OpenRegister** (ADR-012 — push logic up to the shared foundation, don't re-implement per app). + +### Rule 2 — Action RBAC is the app's job, declared in admin settings + +Every app defines a registry of **actions** — named operations that a controller method executes. Examples (decidesk): + +- `minutes.generate-draft` — produces a draft from a meeting transcript +- `minutes.distribute` — sends final minutes to the governance body +- `decision.publish` — marks a decision as published, triggers notifications +- `analytics.view-summary` — reads aggregate metrics across bodies +- `settings.write` — admin-only settings writes + +Each action is mapped to a set of **user groups** via an admin-configured matrix, stored in `IAppConfig` under a well-known key. Every app maintains its own seed data for the initial mapping; the template ships a skeleton file per app that declares the action list with `["admin"]` as the default for every action. This default is **the safest first-install posture** — nothing is accidentally opened to non-admins until an admin explicitly broadens it. The admin settings panel is the only place to edit the matrix. + +```json +// stored as IAppConfig["decidesk"]["actions"] +// +// First-install values (seed from the app, admin-only everywhere). +// The admin editing the matrix is the only path to broaden — code +// changes must not relax the default. +{ + "minutes.generate-draft": ["admin"], + "minutes.distribute": ["admin"], + "decision.publish": ["admin"], + "analytics.view-summary": ["admin"], + "settings.write": ["admin"] +} +``` + +After admin customization (example — illustrative, not default): + +```json +{ + "minutes.generate-draft": ["chairs", "secretaries"], + "minutes.distribute": ["chairs", "secretaries"], + "decision.publish": ["chairs"], + "analytics.view-summary": ["chairs", "secretaries", "board-members"], + "settings.write": ["admin"] +} +``` + +**Naming convention**: `.` with dot as separator, lowercase, hyphens-in-phrases. `minutes.generate-draft`, `decision.publish`, `analytics.view-summary`. NOT `decidesk:minutes:generateDraft`. This keeps the keys grep-friendly, stable across refactors, and matches how schema keys look in OpenRegister. + +The **admin settings panel** (registered via `\OCP\Settings\ISection`, route carries `#[AuthorizedAdminSetting(Application::APP_ID)]`) renders this matrix: rows = actions, columns = user groups, checkboxes = allowed. Admin edits + saves → `IAppConfig` updated. NO code change required to adjust who can do what. + +Controllers enforce the mapping with a single helper call: + +```php +#[NoAdminRequired] +public function generateDraft(string $minutesId): JSONResponse { + $user = $this->userSession->getUser(); + if ($user === null) { + return new JSONResponse(['error' => 'Not authenticated'], Http::STATUS_UNAUTHORIZED); + } + + $this->actionAuth->requireAction($user, 'minutes.generate-draft'); + // Throws OCSForbiddenException if none of $user's groups are mapped + // to 'minutes.generate-draft' in the admin matrix. + + // ... data-layer work via ObjectService (OpenRegister enforces its own + // per-object permissions on top of this action check). +} +``` + +### Rule 3 — When admin IS required (not delegated to action RBAC) + +The following stay `#[AuthorizedAdminSetting(Application::APP_ID)]` and live **only on the admin settings page** — they are NOT expressible as action mappings because they are the plumbing the action matrix itself depends on: + +- **Configuring the action ↔ group matrix** (the admin settings panel itself) +- **App configuration** — any `IAppConfig` writes (feature flags, feature toggles, workflow parameters, anything that affects app-wide behavior) +- **Backup / restore operations** — data export, re-import, cross-environment migration +- **App integration configuration** — connections to external systems (n8n, SOLR, external APIs), webhook URLs, integration feature flags +- **Credential management** — API keys, OAuth tokens, basic-auth credentials for any third-party service +- **One-off admin operations** — re-import seed data, purge caches, run migrations, trigger re-indexing + +Everything a non-admin (chair / secretary / board-member / agent / regular user) might legitimately invoke during normal operation = an **action**, gated via `requireAction()`. Admin settings page handles the plumbing; user settings page / per-user UI never touches the plumbing. The user settings page is for user-personal preferences only (UI theme, notification opt-ins) — not for anything the action matrix references. + +Rule of thumb: if the operation mutates something the action matrix references (keys the matrix looks up, values the matrix resolves to, integrations the actions depend on) → admin. Everything else → action. + +### Rule 4 — Middleware attribute + body check layered + +Per ADR-005 and ADR-016: + +- `#[PublicPage]` — genuinely public (login pages, OAuth callbacks). Body does NO auth check. +- `#[NoAdminRequired]` — any authenticated user may reach the endpoint. Body **MUST** call `$this->actionAuth->requireAction($user, 'action.name')` for action-level gating. Absence of this call is a gate-9 failure — see enforcement below. +- `#[AuthorizedAdminSetting(Application::APP_ID)]` — framework-level admin gate for the exceptions in Rule 3. Body does no further admin check (the middleware already enforced it). + +### Rule 5 — Gate-9 enforces the action-auth pattern mechanically + +`hydra-gate-9` (semantic-auth) is extended to check: + +| Pattern | Verdict | +|---|---| +| `#[NoAdminRequired]` + body calls `$this->actionAuth->requireAction(...)` | PASS | +| `#[NoAdminRequired]` + body calls `$this->authorize*(...)` (per-object auth helper per ADR-005 Rule 3) | PASS | +| `#[NoAdminRequired]` + body calls `$this->requireAdmin()` / `isAdmin()===false`→403 | FAIL — the wrong layer; use `#[AuthorizedAdminSetting]` for admin-only or `requireAction()` for role-based | +| `#[NoAdminRequired]` + no recognized auth gate in body | FAIL — inadequately gated, open endpoint | +| `#[PublicPage]` + any body auth check | FAIL — public is public, no body checks | +| `#[AuthorizedAdminSetting]` + `requireAction()` in body | PASS but redundant (middleware already gated to admin) — not a fail, but the lint could suggest removal | + +Enforcement rolls out in two phases to give apps time to migrate without breaking their pipelines: + +1. **Soft-fail phase** (announce in ADR): gate emits warnings, doesn't fail the gate. Apps that haven't migrated yet stay green. +2. **Hard-fail phase** (date-stamped): gate treats missing `requireAction()` as FAIL. Decided when majority of apps have adopted the pattern. + +## Consequences + +### Positive +- Governance actions (minutes drafting, decision publishing, quorum checks) can be delegated to chairs / secretaries / board members — NOT Nextcloud sysadmins. Current decidesk bug class (#44 + #45) goes away structurally. +- Admins can re-map actions to groups without a code change — useful when an org shifts responsibilities mid-deployment. +- One helper (`$this->actionAuth->requireAction()`) per gated method — consistent, grep-able, testable. +- Gate-7 / gate-9 enforcement has a clear target to check for (`requireAction()` call in body). +- Template repo ships this out of the box — new apps inherit the pattern instead of each rolling their own. + +### Negative +- Initial setup burden: admin must populate the action matrix on first install. Mitigated with sensible defaults in `create-labels`-style seed data per app. +- Two layers of auth per request (action matrix check + OpenRegister per-object check) = two service calls per gated endpoint. Negligible cost (both are app-local memory or indexed DB). +- Admin who mis-configures the matrix can lock chairs out of essential actions. Mitigated with a "reset to defaults" button + `occ decidesk:actions:reset`. + +### Neutral +- Replaces "lock everything to admin" over-restriction with "configurable by admin" flexibility. For ops that currently have only Nextcloud admins, the first-install default can be "admin-only" per action — the matrix is editable but the safe default survives if nobody touches it. + +## Implementation plan + +1. **This ADR** — accepted. +2. **Reference implementation in decidesk**: + - New `OCA\Decidesk\Service\ActionAuthService` with `requireAction(IUser $user, string $action): void` — throws `OCSForbiddenException` when $user's groups don't intersect the matrix entry for $action + - New `OCA\Decidesk\Settings\ActionMatrixAdmin` settings section (`\OCP\Settings\ISettings` + template) showing the action×group matrix, admin-only + - `IAppConfig` key `decidesk.actions` storing the JSON mapping + - Refactor the 13 + 2 controller methods caught by gate-9 on #44 / #45 to use `requireAction()` + - **Seed data per app** — each app ships its own `actions.seed.json` (or equivalent) declaring the action list with `["admin"]` as default. App migration runs it on first install. +3. **Port to `nextcloud-app-template`**: copy `ActionAuthService` + skeleton settings panel + seed-data pattern. Parametrized so new apps just declare their action names. Default values all `["admin"]`. +4. **Gate-9 extension (soft-fail phase first)**: + - Detect `#[NoAdminRequired]` + body-has-`requireAction()`-call → PASS + - Detect `#[NoAdminRequired]` + body-has-`authorize*()`-call (per-object auth per ADR-005) → PASS + - Detect `#[NoAdminRequired]` + no recognized gate → emit warning (soft-fail) + - Detect `#[NoAdminRequired]` + `requireAdmin()` / `isAdmin()===false` → FAIL (hard — the wrong layer) + - Warnings hit the verdict JSON but do not set the gate to FAIL during migration. +5. **Migrate existing apps** (hydra, decidesk first, then docudesk / pipelinq / procest / …) to the new pattern. +6. **Gate-9 hard-fail phase**: after apps are migrated, flip warnings → fails. Date-stamp to set on the PR that ships the hard-fail variant. +7. **Unblock #44 + #45**: once decidesk has `ActionAuthService`, their 13+2 methods plug into `requireAction('minutes.generate-draft')` etc. The current parked state resolves as a retry cycle. + +## References + +- ADR-005 (security) — per-object authorization rule + admin checks +- ADR-016 (routes) — auth attribute rules + gate layering +- ADR-021 (bounded-fix scope) — mentions `checkUserRole($uid, ['chair','secretary'])` as the correct shape (now formalized via `requireAction`) +- ADR-022 (apps consume OR abstractions) — lists RBAC as one of OpenRegister's shared abstractions; this ADR clarifies that the scope is **data** RBAC, not **action** RBAC +- decidesk#44 / #45 — both pending role-based fix that this ADR unblocks diff --git a/tests/Unit/Service/ActionAuthServiceTest.php b/tests/Unit/Service/ActionAuthServiceTest.php new file mode 100644 index 0000000..7034dd5 --- /dev/null +++ b/tests/Unit/Service/ActionAuthServiceTest.php @@ -0,0 +1,246 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @link https://conduction.nl + * + * @spec openspec/architecture/adr-023-action-authorization.md + */ + +declare(strict_types=1); + +namespace OCA\AppTemplate\Tests\Unit\Service; + +use OCA\AppTemplate\Service\ActionAuthService; +use OCP\AppFramework\OCS\OCSForbiddenException; +use OCP\IAppConfig; +use OCP\IGroupManager; +use OCP\IUser; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; + +/** + * Tests for the ADR-023 action-authorization service. + * + * @spec openspec/architecture/adr-023-action-authorization.md + */ +class ActionAuthServiceTest extends TestCase +{ + + /** + * Service under test. + * + * @var ActionAuthService + */ + private ActionAuthService $service; + + /** + * Mock IAppConfig. + * + * @var IAppConfig&MockObject + */ + private IAppConfig $appConfig; + + /** + * Mock IGroupManager. + * + * @var IGroupManager&MockObject + */ + private IGroupManager $groupManager; + + /** + * Wire up fresh mocks before each test. + * + * @return void + */ + protected function setUp(): void + { + parent::setUp(); + $this->appConfig = $this->createMock(IAppConfig::class); + $this->groupManager = $this->createMock(IGroupManager::class); + $this->service = new ActionAuthService( + appConfig: $this->appConfig, + groupManager: $this->groupManager, + ); + + }//end setUp() + + /** + * Helper to create a mock IUser that reports the given UID. + * + * @param string $uid The user ID to report. + * + * @return IUser&MockObject + */ + private function mockUser(string $uid): IUser + { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn($uid); + return $user; + + }//end mockUser() + + /** + * Set the appConfig to return the given matrix as JSON. + * + * @param array> $matrix The matrix to serve. + * + * @return void + */ + private function setMatrix(array $matrix): void + { + $this->appConfig->method('getValueString') + ->willReturn(json_encode($matrix)); + + }//end setMatrix() + + /** + * Admin always passes, regardless of the matrix. + * + * @return void + */ + public function testAdminAlwaysPasses(): void + { + $user = $this->mockUser('sysadmin'); + $this->groupManager->method('isAdmin')->willReturn(true); + // Matrix explicitly denies everyone including nonexistent action. + $this->setMatrix(['minutes.generate-draft' => []]); + + $this->service->requireAction($user, 'minutes.generate-draft'); + // No exception → pass. + $this->assertTrue($this->service->can($user, 'any.action.not.in.matrix')); + + }//end testAdminAlwaysPasses() + + /** + * Non-admin is denied when the action is not in the matrix (default ["admin"]). + * + * @return void + */ + public function testNonAdminDeniedByDefaultWhenActionMissing(): void + { + $user = $this->mockUser('jane'); + $this->groupManager->method('isAdmin')->willReturn(false); + $this->groupManager->method('getUserGroupIds')->willReturn(['users']); + $this->setMatrix([]); + + $this->expectException(OCSForbiddenException::class); + $this->service->requireAction($user, 'minutes.generate-draft'); + + }//end testNonAdminDeniedByDefaultWhenActionMissing() + + /** + * Non-admin is denied when the matrix entry is ["admin"] only. + * + * @return void + */ + public function testNonAdminDeniedWhenMatrixIsAdminOnly(): void + { + $user = $this->mockUser('jane'); + $this->groupManager->method('isAdmin')->willReturn(false); + $this->groupManager->method('getUserGroupIds')->willReturn(['users', 'chairs']); + $this->setMatrix(['minutes.generate-draft' => ['admin']]); + + $this->expectException(OCSForbiddenException::class); + $this->service->requireAction($user, 'minutes.generate-draft'); + + }//end testNonAdminDeniedWhenMatrixIsAdminOnly() + + /** + * Non-admin passes when their group intersects the matrix entry. + * + * @return void + */ + public function testNonAdminPassesWhenGroupMatches(): void + { + $user = $this->mockUser('jane'); + $this->groupManager->method('isAdmin')->willReturn(false); + $this->groupManager->method('getUserGroupIds')->willReturn(['chairs', 'users']); + $this->setMatrix(['minutes.generate-draft' => ['chairs', 'secretaries']]); + + $this->service->requireAction($user, 'minutes.generate-draft'); + $this->assertTrue(true, 'No exception thrown — user is authorized.'); + + }//end testNonAdminPassesWhenGroupMatches() + + /** + * Non-admin is denied when their groups don't intersect — even if admin is in the entry. + * + * Admin in the entry is a display hint for the matrix UI, not a real group + * membership check for non-admins. + * + * @return void + */ + public function testNonAdminDeniedEvenWithAdminInEntry(): void + { + $user = $this->mockUser('jane'); + $this->groupManager->method('isAdmin')->willReturn(false); + $this->groupManager->method('getUserGroupIds')->willReturn(['users']); + $this->setMatrix(['minutes.generate-draft' => ['admin', 'chairs']]); + + $this->expectException(OCSForbiddenException::class); + $this->service->requireAction($user, 'minutes.generate-draft'); + + }//end testNonAdminDeniedEvenWithAdminInEntry() + + /** + * `can()` is the non-throwing variant that returns bool. + * + * @return void + */ + public function testCanReturnsBool(): void + { + $user = $this->mockUser('jane'); + $this->groupManager->method('isAdmin')->willReturn(false); + $this->groupManager->method('getUserGroupIds')->willReturn(['users']); + $this->setMatrix(['minutes.generate-draft' => ['chairs']]); + + $this->assertFalse($this->service->can($user, 'minutes.generate-draft')); + + }//end testCanReturnsBool() + + /** + * Malformed matrix JSON falls back to empty matrix (default-deny). + * + * @return void + */ + public function testMalformedMatrixFallsBackToDeny(): void + { + $user = $this->mockUser('jane'); + $this->groupManager->method('isAdmin')->willReturn(false); + $this->groupManager->method('getUserGroupIds')->willReturn(['chairs']); + $this->appConfig->method('getValueString')->willReturn('not valid json {['); + + $this->expectException(OCSForbiddenException::class); + $this->service->requireAction($user, 'minutes.generate-draft'); + + }//end testMalformedMatrixFallsBackToDeny() + + /** + * `getAllowedGroups` returns ["admin"] for an unknown action. + * + * @return void + */ + public function testGetAllowedGroupsDefaultsToAdmin(): void + { + $this->setMatrix([]); + + $this->assertSame(['admin'], $this->service->getAllowedGroups('unknown.action')); + + }//end testGetAllowedGroupsDefaultsToAdmin() + +}//end class