Skip to content

Commit 9acc49b

Browse files
committed
fix: Add 401 response for non-public pages and 403 response for admin pages
Signed-off-by: provokateurin <kate@provokateurin.de>
1 parent a59df43 commit 9acc49b

File tree

10 files changed

+12115
-3402
lines changed

10 files changed

+12115
-3402
lines changed

generate-spec.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,6 @@
466466
$isIgnored = Helpers::classMethodHasAnnotationOrAttribute($methodFunction, 'IgnoreOpenAPI');
467467
$isPasswordConfirmation = Helpers::classMethodHasAnnotationOrAttribute($methodFunction, 'PasswordConfirmationRequired');
468468
$isExApp = Helpers::classMethodHasAnnotationOrAttribute($methodFunction, 'ExAppRequired');
469-
$isCORS = Helpers::classMethodHasAnnotationOrAttribute($methodFunction, 'CORS');
470469
$scopes = Helpers::getOpenAPIAttributeScopes($classMethod, $routeName);
471470

472471
if ($isIgnored) {
@@ -520,7 +519,7 @@
520519
];
521520
}
522521

523-
$classMethodInfo = ControllerMethod::parse($routeName, $definitions, $methodFunction, $isAdmin, $isDeprecated, $isPasswordConfirmation, $isCORS);
522+
$classMethodInfo = ControllerMethod::parse($routeName, $definitions, $methodFunction, $isPublic, $isAdmin, $isDeprecated, $isPasswordConfirmation, $isCORS, $isOCS);
524523
if (count($classMethodInfo->responses) == 0) {
525524
Logger::error($routeName, 'Returns no responses');
526525
continue;

src/ControllerMethod.php

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,10 +304,12 @@ public function __construct(
304304
public static function parse(string $context,
305305
array $definitions,
306306
ClassMethod $method,
307+
bool $isPublic,
307308
bool $isAdmin,
308309
bool $isDeprecated,
309310
bool $isPasswordConfirmation,
310311
bool $isCORS,
312+
bool $isOCS,
311313
): ControllerMethod {
312314
global $phpDocParser, $lexer, $nodeFinder, $allowMissingDocs;
313315

@@ -409,6 +411,46 @@ public static function parse(string $context,
409411
Logger::error($context, 'Missing @return annotation');
410412
}
411413

414+
if (!$isPublic || $isAdmin) {
415+
$statusCodes = [];
416+
if (!$isPublic) {
417+
$responseDescriptions[401] ??= 'Current user is not logged in';
418+
$statusCodes[] = 401;
419+
}
420+
if ($isAdmin) {
421+
$responseDescriptions[403] ??= 'Logged in account must be an admin';
422+
$statusCodes[] = 403;
423+
}
424+
425+
foreach ($statusCodes as $statusCode) {
426+
if ($isOCS) {
427+
$responses[] = new ControllerMethodResponse(
428+
'DataResponse',
429+
$statusCode,
430+
'application/json',
431+
new OpenApiType($context),
432+
);
433+
} else {
434+
$responses[] = new ControllerMethodResponse(
435+
'JsonResponse',
436+
$statusCode,
437+
'application/json',
438+
new OpenApiType(
439+
$context,
440+
type: 'object',
441+
properties: [
442+
'message' => new OpenApiType(
443+
$context,
444+
type: 'string',
445+
),
446+
],
447+
required: ['message'],
448+
),
449+
);
450+
}
451+
}
452+
}
453+
412454
$responseStatusCodes = array_unique(array_map(static fn (ControllerMethodResponse $response): int => $response->statusCode, $responses));
413455
$unusedResponseDescriptions = array_diff(array_keys($responseDescriptions), $responseStatusCodes);
414456
if ($unusedResponseDescriptions !== []) {

src/Helpers.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,8 @@ public static function wrapOCSResponse(Route $route, ControllerMethodResponse $r
140140
return $schema;
141141
}
142142

143-
public static function cleanEmptyResponseArray(array $schema): array|stdClass {
144-
if (array_key_exists('type', $schema) && $schema['type'] === 'array' && array_key_exists('maxItems', $schema) && $schema['maxItems'] === 0) {
143+
public static function cleanEmptyResponseArray(array|stdClass $schema): array|stdClass {
144+
if (is_array($schema) && array_key_exists('type', $schema) && $schema['type'] === 'array' && array_key_exists('maxItems', $schema) && $schema['maxItems'] === 0) {
145145
return new stdClass();
146146
}
147147

tests/appinfo/routes.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@
9191
['name' => 'Settings#publicPageAnnotation', 'url' => '/api/{apiVersion}/public-page/annotation', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
9292
['name' => 'Settings#publicPageAttribute', 'url' => '/api/{apiVersion}/public-page/attribute', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
9393
['name' => 'Settings#mergedResponses', 'url' => '/api/{apiVersion}/merged-responses', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
94+
['name' => 'Settings#custom401', 'url' => '/api/{apiVersion}/custom/401', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
95+
['name' => 'Settings#custom403', 'url' => '/api/{apiVersion}/custom/403', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
9496
['name' => 'V1\SubDir#subDirRoute', 'url' => '/sub-dir', 'verb' => 'GET'],
9597
],
9698
];

tests/lib/Controller/SettingsController.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use OCP\AppFramework\Http;
1414
use OCP\AppFramework\Http\Attribute\CORS;
1515
use OCP\AppFramework\Http\Attribute\IgnoreOpenAPI;
16+
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
1617
use OCP\AppFramework\Http\Attribute\OpenAPI;
1718
use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
1819
use OCP\AppFramework\Http\Attribute\PublicPage;
@@ -827,4 +828,27 @@ public function publicPageAttribute(): DataResponse {
827828
public function mergedResponses(): DataResponse|JSONResponse {
828829
return new DataResponse();
829830
}
831+
832+
/**
833+
* A page with a custom 401.
834+
*
835+
* @return DataResponse<Http::STATUS_UNAUTHORIZED, array{a: string}, array{}>
836+
*
837+
* 401: Admin settings updated
838+
*/
839+
#[NoAdminRequired]
840+
public function custom401(): DataResponse {
841+
return new DataResponse();
842+
}
843+
844+
/**
845+
* A page with a custom 403.
846+
*
847+
* @return DataResponse<Http::STATUS_FORBIDDEN, array{a: string}, array{}>
848+
*
849+
* 403: Admin settings updated
850+
*/
851+
public function custom403(): DataResponse {
852+
return new DataResponse();
853+
}
830854
}

0 commit comments

Comments
 (0)