Skip to content

Commit aa4b675

Browse files
provokateurinprovokateurin
authored
Merge pull request #266 from nextcloud/fix/non-public-page-401-response
2 parents 41d1f54 + 9acc49b commit aa4b675

File tree

10 files changed

+12979
-3740
lines changed

10 files changed

+12979
-3740
lines changed

‎generate-spec.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,7 @@
269269
}
270270
}
271271

272+
/** @var array<string, list<Route>> $routes */
272273
$routes = [];
273274
foreach ($controllers as $controllerName => $stmts) {
274275
$controllerClass = null;
@@ -465,7 +466,6 @@
465466
$isIgnored = Helpers::classMethodHasAnnotationOrAttribute($methodFunction, 'IgnoreOpenAPI');
466467
$isPasswordConfirmation = Helpers::classMethodHasAnnotationOrAttribute($methodFunction, 'PasswordConfirmationRequired');
467468
$isExApp = Helpers::classMethodHasAnnotationOrAttribute($methodFunction, 'ExAppRequired');
468-
$isCORS = Helpers::classMethodHasAnnotationOrAttribute($methodFunction, 'CORS');
469469
$scopes = Helpers::getOpenAPIAttributeScopes($classMethod, $routeName);
470470

471471
if ($isIgnored) {
@@ -519,7 +519,7 @@
519519
];
520520
}
521521

522-
$classMethodInfo = ControllerMethod::parse($routeName, $definitions, $methodFunction, $isAdmin, $isDeprecated, $isPasswordConfirmation, $isCORS);
522+
$classMethodInfo = ControllerMethod::parse($routeName, $definitions, $methodFunction, $isPublic, $isAdmin, $isDeprecated, $isPasswordConfirmation, $isCORS, $isOCS);
523523
if (count($classMethodInfo->responses) == 0) {
524524
Logger::error($routeName, 'Returns no responses');
525525
continue;
@@ -724,10 +724,11 @@
724724
} else {
725725
$mergedContentTypeResponses[$contentType] = [
726726
'schema' => [
727-
[$hasEmpty ? 'anyOf' : 'oneOf' => array_map(function (ControllerMethodResponse $response) use ($route): \stdClass|array {
727+
// At least one should match, but it's possible that multiple match, so oneOf can't be used.
728+
'anyOf' => array_map(function (ControllerMethodResponse $response) use ($route): stdClass|array {
728729
$schema = Helpers::cleanEmptyResponseArray($response->type->toArray());
729730
return Helpers::wrapOCSResponse($route, $response, $schema);
730-
}, $uniqueResponses)],
731+
}, $uniqueResponses),
731732
],
732733
];
733734
}

‎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: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,11 @@
8888
['name' => 'Settings#samePathPost', 'url' => '/api/{apiVersion}/same-path', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
8989
['name' => 'Settings#requestHeader', 'url' => '/api/{apiVersion}/request-header', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
9090
['name' => 'Settings#requestParams', 'url' => '/api/{apiVersion}/request-params', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
91+
['name' => 'Settings#publicPageAnnotation', 'url' => '/api/{apiVersion}/public-page/annotation', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
92+
['name' => 'Settings#publicPageAttribute', 'url' => '/api/{apiVersion}/public-page/attribute', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
93+
['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)']],
9196
['name' => 'V1\SubDir#subDirRoute', 'url' => '/sub-dir', 'verb' => 'GET'],
9297
],
9398
];

‎tests/lib/Controller/SettingsController.php‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,13 @@
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;
19+
use OCP\AppFramework\Http\Attribute\PublicPage;
1820
use OCP\AppFramework\Http\Attribute\RequestHeader;
1921
use OCP\AppFramework\Http\DataResponse;
22+
use OCP\AppFramework\Http\JSONResponse;
2023
use OCP\AppFramework\OCS\OCSNotFoundException;
2124
use OCP\AppFramework\OCSController;
2225

@@ -790,4 +793,62 @@ public function requestParams(): DataResponse {
790793

791794
return new DataResponse();
792795
}
796+
797+
/**
798+
* A public page with annotation.
799+
*
800+
* @PublicPage
801+
* @return DataResponse<Http::STATUS_OK, list<empty>, array{}>
802+
*
803+
* 200: Admin settings updated
804+
*/
805+
public function publicPageAnnotation(): DataResponse {
806+
return new DataResponse();
807+
}
808+
809+
/**
810+
* A public page with attribute.
811+
*
812+
* @return DataResponse<Http::STATUS_OK, list<empty>, array{}>
813+
*
814+
* 200: Admin settings updated
815+
*/
816+
#[PublicPage]
817+
public function publicPageAttribute(): DataResponse {
818+
return new DataResponse();
819+
}
820+
821+
/**
822+
* A with merged responses.
823+
*
824+
* @return DataResponse<Http::STATUS_OK, array{a: string}, array{}>|JSONResponse<Http::STATUS_OK, array{b: int}, array{}>
825+
*
826+
* 200: Admin settings updated
827+
*/
828+
public function mergedResponses(): DataResponse|JSONResponse {
829+
return new DataResponse();
830+
}
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+
}
793854
}

0 commit comments

Comments
 (0)