Skip to content

Commit 38da9ff

Browse files
committed
Fix scope enforcement: fail-closed + inject stub token in tests [gh-967]
Restore the 403 deny path in requireScope() when apiCurrentToken is null — the previous fail-open was a security regression (if the token row is ever missing during a real request, scope enforcement silently vanished). Fix the test infrastructure instead: loginAs() now also injects a synthetic token row with scopes=null (all scopes allowed) onto Container::apiCurrentToken, so handler-level integration tests that bypass TokenAuthentication satisfy the fail-closed gate. A companion injectTokenScopes() helper lets scope-restriction tests set an explicit allow-list. Signed-off-by: Nicholas K. Dionysopoulos <nicholas@akeeba.com>
1 parent bdec01c commit 38da9ff

2 files changed

Lines changed: 34 additions & 4 deletions

File tree

src/Controller/Api/AbstractApiHandler.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,12 +141,12 @@ protected function requireScope(ApiScope $scope): void
141141
{
142142
$tokenRow = $this->container->apiCurrentToken;
143143

144-
// No token row on the container: authentication was either bypassed (e.g. in test
145-
// infrastructure that calls handlers directly) or the token has no scope column yet
146-
// (pre-scope-feature rows). Treat as "all scopes allowed" in both cases — the same
147-
// semantics as a token whose scopes column is NULL.
144+
// No token row: this should never happen in production (TokenAuthentication always
145+
// sets apiCurrentToken before any handler runs). Fail closed.
148146
if ($tokenRow === null)
149147
{
148+
$this->sendJsonError(403, 'Access denied: missing token context.', 'auth.forbidden');
149+
150150
return;
151151
}
152152

tests/Integration/Api/AbstractApiIntegrationTestCase.php

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,12 @@ protected function forceUnknownCmsType(int $siteId): void
239239
/**
240240
* Set an authenticated user on the container's userManager directly (skip token round-trip)
241241
* for tests that only care about the post-auth behaviour of a handler.
242+
*
243+
* Also injects a synthetic token row with scopes = null (all scopes allowed) so that
244+
* {@see \Akeeba\Panopticon\Controller\Api\AbstractApiHandler::requireScope()} does not
245+
* reject the request with a "missing token context" 403. This keeps scope enforcement
246+
* fail-closed in production while allowing handler-level integration tests to run
247+
* without a real token round-trip.
242248
*/
243249
protected function loginAs(int $userId): void
244250
{
@@ -249,5 +255,29 @@ protected function loginAs(int $userId): void
249255
$property = $ref->getProperty('currentUser');
250256
$property->setAccessible(true);
251257
$property->setValue($manager, $user);
258+
259+
// Inject a synthetic token row with null scopes so requireScope() treats this
260+
// request as having all scopes. Real scope-restriction tests should call
261+
// injectTokenScopes() instead to set a restricted scope list.
262+
$this->container->apiCurrentToken = (object) [
263+
'id' => 0,
264+
'user_id' => $userId,
265+
'scopes' => null,
266+
];
267+
}
268+
269+
/**
270+
* Override the synthetic token row's scopes for tests that specifically exercise
271+
* scope-restricted behaviour. Pass null to restore the "all scopes allowed" default.
272+
*
273+
* @param string[]|null $scopes Scope value strings (e.g. ['sites:read']), or null for all.
274+
*/
275+
protected function injectTokenScopes(?array $scopes): void
276+
{
277+
$existing = $this->container->apiCurrentToken ?? (object) ['id' => 0, 'user_id' => 0, 'scopes' => null];
278+
279+
$existing->scopes = $scopes === null ? null : json_encode($scopes);
280+
281+
$this->container->apiCurrentToken = $existing;
252282
}
253283
}

0 commit comments

Comments
 (0)