From e5591f31d50b501998e1673cc57cae75248cd949 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Thu, 28 May 2026 11:53:08 +0200 Subject: [PATCH] fix(security): scope audit-trail + search-trail to admin (wave-3 C6/C7 cross-tenant leak) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C6 — AuditTrailController::index/show were @NoAdminRequired, so any authenticated user (including users restricted to a single app group) could enumerate every other tenant's audit trail. Each row carries a full diff of an object change — register/schema/object UUID, the new and old values of every JSON property, the actor UID, IP address and processing-activity tag. With sequential IDs, show() doubled as a trivial enumeration path across the whole instance. LogService:: getAllLogs/getLog delegate straight to AuditTrailMapper::findAll/find with no scope filter, so the surface had no tenant boundary at all. C7 — SearchTrailController::index/show had the same shape against the search-trail table. Rows record the full query string, register/schema identifiers, result counts, the user's IP, user-agent and session id for every search anyone has ever run — a GDPR-sensitive leak and an obvious recon path ("what is tenant X searching for"). Both surfaces now go through a body-level requireAdmin() helper that returns 401 when anonymous and 403 when non-admin, mirroring the existing pattern used by AuditTrailController::export() / clearAll() and the wave-2 NotificationHistoryController. Framework-level @NoAdminRequired annotations are removed so the gate sits at both layers (defence-in-depth — losing either still keeps the surface closed). The non-list audit-trail surfaces (verify, verwerkingsregister, inzageverzoek, per-object objects()) stay reachable because they already key off identifiers the caller must know up front. SearchTrailController gains IUserSession + IGroupManager constructor deps; NC's DI container auto-wires them so info.xml needs no change. Tests updated for both controllers — denial paths assert 401/403 and verify the service layer is not invoked on a forbidden call (no DB hit, no row read). Integration-test SearchTrail sites get an explicit setupAdminUser() call to match production. Pre-existing PHPCS violations on the buildSortFromParams / buildFiltersFromParams pair (inline-if + multi-line-array nits) and a PHPMD ExcessiveClassComplexity bump (50 -> 51 from the added requireAdmin() calls) are fixed in the same commit so the diff lands on a green gate. Follow-up issue (Option B "auditor group"): a future change should allow admins to grant an "auditor" group read-only access to audit / search trails without granting full admin — useful for AVG/GDPR Art 30 reviewers. Not blocking; admin-only is the safer default for now. Refs: wave-3 triage (/tmp/triage-openregister.md) findings C6, C7. --- lib/Controller/AuditTrailController.php | 99 +++++++++++------ lib/Controller/SearchTrailController.php | 64 ++++++++++- tests/Service/ControllersIntegrationTest.php | 16 ++- .../Controller/AuditTrailControllerTest.php | 88 +++++++++++++++ .../SearchTrailControllerCoverageTest.php | 16 ++- .../SearchTrailControllerDeepTest.php | 16 ++- .../Controller/SearchTrailControllerTest.php | 100 +++++++++++++++++- 7 files changed, 359 insertions(+), 40 deletions(-) diff --git a/lib/Controller/AuditTrailController.php b/lib/Controller/AuditTrailController.php index a4adcda0df..226588e17d 100644 --- a/lib/Controller/AuditTrailController.php +++ b/lib/Controller/AuditTrailController.php @@ -47,9 +47,14 @@ * * @psalm-suppress UnusedClass * - * @SuppressWarnings(PHPMD.ExcessiveClassLength) Controller covers audit trail, verification, verwerkingsregister - * @SuppressWarnings(PHPMD.CouplingBetweenObjects) Necessary service dependencies - * @SuppressWarnings(PHPMD.TooManyPublicMethods) One public method per audit trail route + * @SuppressWarnings(PHPMD.ExcessiveClassLength) Controller covers audit trail, verification, verwerkingsregister + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) Necessary service dependencies + * @SuppressWarnings(PHPMD.TooManyPublicMethods) One public method per audit trail route + * @SuppressWarnings(PHPMD.ExcessiveClassComplexity) Audit-trail surface fans out into hash-verify, + * verwerkingsregister, inzageverzoek, export, immutability + admin-gating; the controller's + * overall cyclomatic complexity sits one step above the default threshold (51 vs 50) after + * the wave-3 C6 admin-only gates on index()/show() were added. Splitting the controller + * would force a route-table reshuffle without removing any actual branching. */ class AuditTrailController extends Controller { @@ -77,12 +82,17 @@ public function __construct( }//end __construct() /** - * Gate destructive / bulk-export audit operations on admin membership. + * Gate destructive / bulk-export / bulk-read audit operations on admin membership. * * SECURITY: `clearAll` wipes the entire audit table — a chain of * trust for AVG/GDPR Art 30 reviews. `export` dumps every row in - * bulk and is an obvious recon path across tenants. Both surfaces - * are admin-only at the framework level (the methods carry no + * bulk and is an obvious recon path across tenants. `index` and + * `show` expose cross-tenant audit-trail rows (per-row diffs of + * every object change in every register/schema, frequently PII or + * IP-heavy); without this gate any authenticated user — including + * users restricted to a single app group — could enumerate every + * other tenant's change history (wave-3 C6). All surfaces are + * admin-only at the framework level for sensitive routes (no * `@NoAdminRequired`) and this body-level helper stays as * defence-in-depth so removing the framework gate by accident does * not silently open the surface. @@ -157,8 +167,8 @@ private function extractRequestParameters(): array // Extract search parameter. $search = $params['search'] ?? $params['_search'] ?? null; - $sort = $this->buildSortFromParams($params); - $filters = $this->buildFiltersFromParams($params); + $sort = $this->buildSortFromParams(params: $params); + $filters = $this->buildFiltersFromParams(params: $params); return [ 'limit' => $limit, @@ -191,12 +201,18 @@ private function buildSortFromParams(array $params): array if (is_array($sortRaw) === true) { // Bracket format: _sort[created]=DESC. foreach ($sortRaw as $field => $direction) { - $sort[$field] = strtoupper($direction) === 'ASC' ? 'ASC' : 'DESC'; + $sort[$field] = 'DESC'; + if (strtoupper($direction) === 'ASC') { + $sort[$field] = 'ASC'; + } } } else if ($sortRaw !== null) { // Flat format: sort=created&order=DESC. $sortOrder = $params['order'] ?? $params['_order'] ?? 'DESC'; - $sort[$sortRaw] = strtoupper($sortOrder) === 'ASC' ? 'ASC' : 'DESC'; + $sort[$sortRaw] = 'DESC'; + if (strtoupper($sortOrder) === 'ASC') { + $sort[$sortRaw] = 'ASC'; + } } if (empty($sort) === true) { @@ -217,9 +233,25 @@ private function buildSortFromParams(array $params): array private function buildFiltersFromParams(array $params): array { $systemKeys = [ - 'limit', '_limit', 'offset', '_offset', 'page', '_page', - 'search', '_search', 'sort', '_sort', 'order', '_order', - '_route', 'id', 'register', 'schema', 'format', 'from', 'to', + 'limit', + '_limit', + 'offset', + '_offset', + 'page', + '_page', + 'search', + '_search', + 'sort', + '_sort', + 'order', + '_order', + '_route', + 'id', + 'register', + 'schema', + 'format', + 'from', + 'to', 'identifier', ]; @@ -235,22 +267,26 @@ function ($key) use ($systemKeys) { /** * Get all audit trail logs * - * @NoAdminRequired + * Admin-only at the framework level (no @NoAdminRequired). Body + * `requireAdmin()` stays as defence-in-depth. The cross-tenant + * audit-trail index leaks per-row diffs of every object change + * across every register/schema — wave-3 C6. Returns 200 on + * success, 401 when anonymous, 403 when non-admin. * * @return JSONResponse JSON response containing list of audit trails * * @NoCSRFRequired * - * @psalm-return JSONResponse<200, - * array{results: array<\OCA\OpenRegister\Db\AuditTrail>, - * total: int<0, max>, page: int|null, pages: float, limit: int, - * offset: int|null}, array> - * * @spec openspec/changes/retrofit-2026-04-23-annotate-openregister/tasks.md#task-8 * @spec openspec/changes/retrofit-2026-04-30-annotate-openregister/tasks.md#task-17 */ public function index(): JSONResponse { + $denial = $this->requireAdmin(); + if ($denial !== null) { + return $denial; + } + // Extract common parameters. $params = $this->extractRequestParameters(); @@ -276,29 +312,30 @@ public function index(): JSONResponse /** * Get a specific audit trail log by ID * + * Admin-only at the framework level (no @NoAdminRequired). Body + * `requireAdmin()` stays as defence-in-depth. Without the gate, + * any authed caller could fetch any audit-trail row by ID — and + * IDs are sequential, so this is also a trivial enumeration path + * across every tenant's change history (wave-3 C6). Returns 200 + * on success, 401 when anonymous, 403 when non-admin, 404 if + * the row does not exist. + * * @param int $id The audit trail ID * * @return JSONResponse A JSON response containing the log * - * @NoAdminRequired - * * @NoCSRFRequired * - * @psalm-return JSONResponse< - * 200, - * array, - * array - * >|JSONResponse< - * 404, - * array{error: 'Audit trail not found'}, - * array - * > - * * @spec openspec/changes/retrofit-2026-04-23-annotate-openregister/tasks.md#task-8 * @spec openspec/changes/retrofit-2026-04-30-annotate-openregister/tasks.md#task-15 */ public function show(int $id): JSONResponse { + $denial = $this->requireAdmin(); + if ($denial !== null) { + return $denial; + } + try { $log = $this->logService->getLog($id); return new JSONResponse(data: $log); diff --git a/lib/Controller/SearchTrailController.php b/lib/Controller/SearchTrailController.php index b0194da1b8..aa85c2bd3e 100644 --- a/lib/Controller/SearchTrailController.php +++ b/lib/Controller/SearchTrailController.php @@ -30,7 +30,9 @@ use OCP\AppFramework\Controller; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\TemplateResponse; +use OCP\IGroupManager; use OCP\IRequest; +use OCP\IUserSession; /** * Class SearchTrailController @@ -49,15 +51,54 @@ class SearchTrailController extends Controller * @param string $appName The name of the app * @param IRequest $request The request object * @param SearchTrailService $searchTrailService The search trail service + * @param IUserSession $userSession Active user session for caller identity + * @param IGroupManager $groupManager Group manager for admin / role checks */ public function __construct( string $appName, IRequest $request, - private readonly SearchTrailService $searchTrailService + private readonly SearchTrailService $searchTrailService, + private readonly IUserSession $userSession, + private readonly IGroupManager $groupManager ) { parent::__construct(appName: $appName, request: $request); }//end __construct() + /** + * Gate sensitive search-trail read operations on admin membership. + * + * SECURITY: search-trail rows record per-search IP address, user + * ID, user-agent and full query string for every search across + * every register/schema. Returning them to non-admin callers leaks + * PII (GDPR) and gives any authenticated user — including users + * restricted to a single app group — a recon view of what every + * other tenant is searching for (wave-3 C7). Surface stays + * admin-only at both the framework level (no `@NoAdminRequired`) + * and the body level (defence-in-depth). + * + * @return JSONResponse|null 401/403 response when blocked, null when allowed. + */ + private function requireAdmin(): ?JSONResponse + { + $user = $this->userSession->getUser(); + if ($user === null) { + return new JSONResponse( + data: ['error' => 'Authentication required'], + statusCode: 401 + ); + } + + if ($this->groupManager->isAdmin($user->getUID()) === false) { + return new JSONResponse( + data: ['error' => 'Forbidden: this search-trail operation is admin-only'], + statusCode: 403 + ); + } + + return null; + + }//end requireAdmin() + /** * Extract pagination, filter, and search parameters from request * @@ -305,7 +346,8 @@ private function paginate(array $results, ?int $total=0, ?int $limit=20, ?int $o /** * Get all search trail logs * - * @NoAdminRequired + * Admin-only at the framework level (no @NoAdminRequired). Body + * `requireAdmin()` stays as defence-in-depth — wave-3 C7. * * @NoCSRFRequired * @@ -315,6 +357,11 @@ private function paginate(array $results, ?int $total=0, ?int $limit=20, ?int $o */ public function index(): JSONResponse { + $denial = $this->requireAdmin(); + if ($denial !== null) { + return $denial; + } + try { // Get raw request parameters (this is what the service expects). $rawParams = $this->request->getParams(); @@ -353,9 +400,13 @@ public function index(): JSONResponse /** * Get a specific search trail log by ID * - * @param int $id The search trail ID + * Admin-only at the framework level (no @NoAdminRequired). Body + * `requireAdmin()` stays as defence-in-depth — wave-3 C7. IDs + * are sequential so without the gate any authed caller could + * enumerate every tenant's recorded searches (IP, user ID, + * user-agent, query string). * - * @NoAdminRequired + * @param int $id The search trail ID * * @NoCSRFRequired * @@ -365,6 +416,11 @@ public function index(): JSONResponse */ public function show(int $id): JSONResponse { + $denial = $this->requireAdmin(); + if ($denial !== null) { + return $denial; + } + try { $log = $this->searchTrailService->getSearchTrail($id); return new JSONResponse(data: $log); diff --git a/tests/Service/ControllersIntegrationTest.php b/tests/Service/ControllersIntegrationTest.php index 9a2d61e960..699692384a 100644 --- a/tests/Service/ControllersIntegrationTest.php +++ b/tests/Service/ControllersIntegrationTest.php @@ -54,6 +54,7 @@ use OCP\AppFramework\Http\JSONResponse; use OCP\IAppConfig; use OCP\IDBConnection; +use OCP\IGroupManager; use OCP\IRequest; use OCP\IUser; use OCP\IUserSession; @@ -277,11 +278,15 @@ protected function setUp(): void \OC::$server->get(LoggerInterface::class) ); - // Build SearchTrailController. + // Build SearchTrailController. Reuse the shared userSession mock and + // wire a real IGroupManager so the search-trail admin-only gate + // (wave-3 C7) reflects production behaviour in integration runs. $this->searchTrailController = new SearchTrailController( 'openregister', $this->request, - \OC::$server->get(SearchTrailService::class) + \OC::$server->get(SearchTrailService::class), + $this->userSession, + \OC::$server->get(IGroupManager::class) ); // Build EndpointsController. @@ -1780,6 +1785,8 @@ public function testSettingsReindexNegativeMaxObjects(): void */ public function testSearchTrailIndex(): void { + // SearchTrail index is admin-only (wave-3 C7). + $this->setupAdminUser(); $this->request->method('getParams')->willReturn([]); $this->request->method('getRequestUri')->willReturn('/api/search-trails'); @@ -1798,6 +1805,7 @@ public function testSearchTrailIndex(): void */ public function testSearchTrailIndexWithPagination(): void { + $this->setupAdminUser(); $this->request->method('getParams')->willReturn([ '_limit' => 5, '_offset' => 0, @@ -1818,6 +1826,7 @@ public function testSearchTrailIndexWithPagination(): void */ public function testSearchTrailIndexWithPage(): void { + $this->setupAdminUser(); $this->request->method('getParams')->willReturn([ '_limit' => 10, '_page' => 2, @@ -1838,6 +1847,7 @@ public function testSearchTrailIndexWithPage(): void */ public function testSearchTrailShowNotFound(): void { + $this->setupAdminUser(); $response = $this->searchTrailController->show(999999); $this->assertInstanceOf(JSONResponse::class, $response); @@ -2119,6 +2129,7 @@ public function testSearchTrailExportCsv(): void */ public function testSearchTrailIndexWithDateFilters(): void { + $this->setupAdminUser(); $this->request->method('getParams')->willReturn([ 'from' => '2024-01-01', 'to' => '2025-12-31', @@ -2138,6 +2149,7 @@ public function testSearchTrailIndexWithDateFilters(): void */ public function testSearchTrailIndexWithSort(): void { + $this->setupAdminUser(); $this->request->method('getParams')->willReturn([ '_sort' => 'created', '_order' => 'ASC', diff --git a/tests/Unit/Controller/AuditTrailControllerTest.php b/tests/Unit/Controller/AuditTrailControllerTest.php index 1a64e4c30f..dcf3e926ef 100644 --- a/tests/Unit/Controller/AuditTrailControllerTest.php +++ b/tests/Unit/Controller/AuditTrailControllerTest.php @@ -536,4 +536,92 @@ public function testDestroyMultipleWithArrayIds(): void $this->assertEquals(405, $result->getStatus()); } + + // ── Wave-3 C6 admin-only gate (cross-tenant audit-trail leak) ── + + /** + * Build a fresh controller with a non-admin user for gate tests. + * + * The shared `setUp()` wires an admin user so the rest of the suite + * exercises the happy path. C6 tests need a non-admin (or anon) + * user, so we rebuild the controller with overridden mocks rather + * than fight the sticky `willReturn` already wired in setUp(). + * + * @param IUser|null $user The user the session returns; null => anon. + * @param bool $isAdmin Whether $user is in the admin group. + */ + private function makeControllerWithUser(?IUser $user, bool $isAdmin): AuditTrailController + { + $session = $this->createMock(IUserSession::class); + $groupMgr = $this->createMock(IGroupManager::class); + $session->method('getUser')->willReturn($user); + if ($user !== null) { + $groupMgr->method('isAdmin')->with($user->getUID())->willReturn($isAdmin); + } + + return new AuditTrailController( + 'openregister', + $this->request, + $this->logService, + $this->auditTrailMapper, + $this->auditHashService, + $session, + $groupMgr + ); + } + + public function testIndexReturns401WhenAnonymous(): void + { + $controller = $this->makeControllerWithUser(null, false); + + $result = $controller->index(); + + $this->assertEquals(401, $result->getStatus()); + $this->assertEquals('Authentication required', $result->getData()['error']); + } + + public function testIndexReturns403WhenNonAdmin(): void + { + $bob = $this->createMock(IUser::class); + $bob->method('getUID')->willReturn('bob'); + $controller = $this->makeControllerWithUser($bob, false); + + $result = $controller->index(); + + $this->assertEquals(403, $result->getStatus()); + $this->assertStringContainsString('admin-only', $result->getData()['error']); + } + + public function testShowReturns401WhenAnonymous(): void + { + $controller = $this->makeControllerWithUser(null, false); + + $result = $controller->show(1); + + $this->assertEquals(401, $result->getStatus()); + } + + public function testShowReturns403WhenNonAdmin(): void + { + $bob = $this->createMock(IUser::class); + $bob->method('getUID')->willReturn('bob'); + $controller = $this->makeControllerWithUser($bob, false); + + $result = $controller->show(42); + + $this->assertEquals(403, $result->getStatus()); + } + + public function testShowDoesNotInvokeServiceWhenForbidden(): void + { + // Defence-in-depth check: a non-admin call must short-circuit + // before the service is touched (no DB hit, no log read). + $bob = $this->createMock(IUser::class); + $bob->method('getUID')->willReturn('bob'); + $controller = $this->makeControllerWithUser($bob, false); + + $this->logService->expects($this->never())->method('getLog'); + + $controller->show(42); + } } diff --git a/tests/Unit/Controller/SearchTrailControllerCoverageTest.php b/tests/Unit/Controller/SearchTrailControllerCoverageTest.php index a1ab579c7d..b66859eaf7 100644 --- a/tests/Unit/Controller/SearchTrailControllerCoverageTest.php +++ b/tests/Unit/Controller/SearchTrailControllerCoverageTest.php @@ -6,7 +6,10 @@ use OCA\OpenRegister\Controller\SearchTrailController; use OCA\OpenRegister\Service\SearchTrailService; +use OCP\IGroupManager; use OCP\IRequest; +use OCP\IUser; +use OCP\IUserSession; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -18,6 +21,8 @@ class SearchTrailControllerCoverageTest extends TestCase private SearchTrailController $controller; private IRequest&MockObject $request; private SearchTrailService&MockObject $searchTrailService; + private IUserSession&MockObject $userSession; + private IGroupManager&MockObject $groupManager; protected function setUp(): void { @@ -25,11 +30,20 @@ protected function setUp(): void $this->request = $this->createMock(IRequest::class); $this->searchTrailService = $this->createMock(SearchTrailService::class); + $this->userSession = $this->createMock(IUserSession::class); + $this->groupManager = $this->createMock(IGroupManager::class); + + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('admin'); + $this->userSession->method('getUser')->willReturn($user); + $this->groupManager->method('isAdmin')->with('admin')->willReturn(true); $this->controller = new SearchTrailController( 'openregister', $this->request, - $this->searchTrailService + $this->searchTrailService, + $this->userSession, + $this->groupManager ); } diff --git a/tests/Unit/Controller/SearchTrailControllerDeepTest.php b/tests/Unit/Controller/SearchTrailControllerDeepTest.php index bbc98a725d..4fc551c6ad 100644 --- a/tests/Unit/Controller/SearchTrailControllerDeepTest.php +++ b/tests/Unit/Controller/SearchTrailControllerDeepTest.php @@ -7,7 +7,10 @@ use OCA\OpenRegister\Controller\SearchTrailController; use OCA\OpenRegister\Service\SearchTrailService; use OCP\AppFramework\Db\DoesNotExistException; +use OCP\IGroupManager; use OCP\IRequest; +use OCP\IUser; +use OCP\IUserSession; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use ReflectionClass; @@ -17,6 +20,8 @@ class SearchTrailControllerDeepTest extends TestCase private SearchTrailController $controller; private IRequest|MockObject $request; private SearchTrailService|MockObject $searchTrailService; + private IUserSession|MockObject $userSession; + private IGroupManager|MockObject $groupManager; protected function setUp(): void { @@ -24,11 +29,20 @@ protected function setUp(): void $this->request = $this->createMock(IRequest::class); $this->searchTrailService = $this->createMock(SearchTrailService::class); + $this->userSession = $this->createMock(IUserSession::class); + $this->groupManager = $this->createMock(IGroupManager::class); + + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('admin'); + $this->userSession->method('getUser')->willReturn($user); + $this->groupManager->method('isAdmin')->with('admin')->willReturn(true); $this->controller = new SearchTrailController( 'openregister', $this->request, - $this->searchTrailService + $this->searchTrailService, + $this->userSession, + $this->groupManager ); } diff --git a/tests/Unit/Controller/SearchTrailControllerTest.php b/tests/Unit/Controller/SearchTrailControllerTest.php index 559d92eb2b..9243321838 100644 --- a/tests/Unit/Controller/SearchTrailControllerTest.php +++ b/tests/Unit/Controller/SearchTrailControllerTest.php @@ -9,7 +9,10 @@ use OCA\OpenRegister\Service\SearchTrailService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http\JSONResponse; +use OCP\IGroupManager; use OCP\IRequest; +use OCP\IUser; +use OCP\IUserSession; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -23,6 +26,8 @@ class SearchTrailControllerTest extends TestCase private SearchTrailController $controller; private IRequest&MockObject $request; private SearchTrailService&MockObject $searchTrailService; + private IUserSession&MockObject $userSession; + private IGroupManager&MockObject $groupManager; protected function setUp(): void { @@ -30,11 +35,23 @@ protected function setUp(): void $this->request = $this->createMock(IRequest::class); $this->searchTrailService = $this->createMock(SearchTrailService::class); + $this->userSession = $this->createMock(IUserSession::class); + $this->groupManager = $this->createMock(IGroupManager::class); + + // Default: authenticated admin so the requireAdmin() gate on + // index()/show() lets the happy-path assertions through. Tests + // exercising the non-admin rejection override these expectations. + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('admin'); + $this->userSession->method('getUser')->willReturn($user); + $this->groupManager->method('isAdmin')->with('admin')->willReturn(true); $this->controller = new SearchTrailController( 'openregister', $this->request, - $this->searchTrailService + $this->searchTrailService, + $this->userSession, + $this->groupManager ); } @@ -1775,4 +1792,85 @@ public function testUserAgentStatsWithDateFilters(): void $this->assertSame(200, $result->getStatus()); } + + // ── Wave-3 C7 admin-only gate (cross-tenant search-trail leak) ── + + /** + * Build a fresh controller with a non-admin / anonymous user. + * + * The shared `setUp()` wires an admin user so the rest of the suite + * exercises the happy path. C7 denial tests need to override that. + */ + private function makeControllerWithUser(?IUser $user, bool $isAdmin): SearchTrailController + { + $session = $this->createMock(IUserSession::class); + $groupMgr = $this->createMock(IGroupManager::class); + $session->method('getUser')->willReturn($user); + if ($user !== null) { + $groupMgr->method('isAdmin')->with($user->getUID())->willReturn($isAdmin); + } + + return new SearchTrailController( + 'openregister', + $this->request, + $this->searchTrailService, + $session, + $groupMgr + ); + } + + public function testIndexReturns401WhenAnonymous(): void + { + $controller = $this->makeControllerWithUser(null, false); + + $result = $controller->index(); + + $this->assertEquals(401, $result->getStatus()); + $this->assertEquals('Authentication required', $result->getData()['error']); + } + + public function testIndexReturns403WhenNonAdmin(): void + { + $bob = $this->createMock(IUser::class); + $bob->method('getUID')->willReturn('bob'); + $controller = $this->makeControllerWithUser($bob, false); + + $result = $controller->index(); + + $this->assertEquals(403, $result->getStatus()); + $this->assertStringContainsString('admin-only', $result->getData()['error']); + } + + public function testShowReturns401WhenAnonymous(): void + { + $controller = $this->makeControllerWithUser(null, false); + + $result = $controller->show(1); + + $this->assertEquals(401, $result->getStatus()); + } + + public function testShowReturns403WhenNonAdmin(): void + { + $bob = $this->createMock(IUser::class); + $bob->method('getUID')->willReturn('bob'); + $controller = $this->makeControllerWithUser($bob, false); + + $result = $controller->show(42); + + $this->assertEquals(403, $result->getStatus()); + } + + public function testShowDoesNotInvokeServiceWhenForbidden(): void + { + // Defence-in-depth: non-admin must short-circuit before the + // search-trail service is touched (no DB hit, no row read). + $bob = $this->createMock(IUser::class); + $bob->method('getUID')->willReturn('bob'); + $controller = $this->makeControllerWithUser($bob, false); + + $this->searchTrailService->expects($this->never())->method('getSearchTrail'); + + $controller->show(42); + } }