diff --git a/src/core/parsers/openapi-parser.ts b/src/core/parsers/openapi-parser.ts index 685b3e1..c785bbf 100644 --- a/src/core/parsers/openapi-parser.ts +++ b/src/core/parsers/openapi-parser.ts @@ -14,9 +14,9 @@ import { type AuthParameterLocationDescription, type ParameterLocation, } from "../authentication/http/types.ts"; -import { OpenApiFieldNames } from "../constants.ts"; +import { OPENAPI_FIELD_PREFIX, OpenApiFieldNames } from "../constants.ts"; import type { Resource } from "../policy/entities/resource.ts"; -import { createResourceDescriptorSchema } from "../schemas.ts"; +import { createResourceDescriptorSchema, parseZodSchema } from "../schemas.ts"; import { Route } from "../tests/test-utils.ts"; import type { AuthEndpointInformation } from "../types.ts"; import { @@ -37,6 +37,9 @@ export type ResourceLocationDescriptor = { parameterLocation?: ParameterLocation; }; +type Operations = ReturnType; +type Operation = Operations[0]; + export class OpenAPIParser { private constructor( private readonly openApiSource: Oas, @@ -89,52 +92,54 @@ export class OpenAPIParser { // todo: should this be part of the creation process? validateCustomFields(resources: Array) { const resourceNames = resources.map((resource) => resource.getName()); - const resourceDescriptorSchema = - createResourceDescriptorSchema(resourceNames); - - this.getPaths().forEach((path) => { - path.getParameters().forEach((parameter) => { - const resourceAccess = getOpenApiField( - parameter, - OpenApiFieldNames.RESOURCE_ACCESS, - ); - const resourceName = getOpenApiField( - parameter, - OpenApiFieldNames.RESOURCE_NAME, - ); - - const parameterSchema = parameter.schema as SchemaObject; - const parameterDefaultProvided = Boolean(parameterSchema.default); - const resourceDescriptionNeeded = - Boolean(parameter.required) && !parameterDefaultProvided; - // todo: use default parameter values in requests when they are provided for required params - // validate that required parameters are annotated with resource name and resource access - if (resourceDescriptionNeeded && (!resourceAccess || !resourceName)) { - throw new Error( - "To describe required resources in routes, both 'resourceName' and 'resourceAccess' must be defined at the same time.", - ); - } + this.getOperations().forEach((operation) => { + const parametrizedResources = this.getParametrizedResources(operation); - // todo: better custom error message - // create a ResourceDescriptorParser - // it should accept path.schema/parameter.schema and provide a nicer error message - resourceDescriptorSchema.parse({ + parametrizedResources.forEach( + ({ resourceName, resourceAccess, - }); - }); + descriptorsRequired, + parameterName, + parameterLocation, + }) => { + parseZodSchema( + createResourceDescriptorSchema({ + allowedResourceNames: resourceNames, + descriptorsRequired, + }), + { + resourceName, + resourceAccess, + }, + [ + "To describe", + descriptorsRequired ? "required" : "", + `resources in routes, both '${OPENAPI_FIELD_PREFIX}-${OpenApiFieldNames.RESOURCE_NAME}' and '${OPENAPI_FIELD_PREFIX}-${OpenApiFieldNames.RESOURCE_ACCESS}' must be defined at the same time. +Parameter '${parameterName}' of type '${parameterLocation}' in path '${operation.method.toUpperCase()} ${operation.path}' must be annotated properly.`, + ].join(" "), + ); + }, + ); - resourceDescriptorSchema.parse({ - resourceName: getOpenApiField( - path.schema, - OpenApiFieldNames.RESOURCE_NAME, - ), - resourceAccess: getOpenApiField( - path.schema, - OpenApiFieldNames.RESOURCE_ACCESS, - ), - }); + parseZodSchema( + createResourceDescriptorSchema({ + allowedResourceNames: resourceNames, + }), + { + resourceName: getOpenApiField( + operation.schema, + OpenApiFieldNames.RESOURCE_NAME, + ), + resourceAccess: getOpenApiField( + operation.schema, + OpenApiFieldNames.RESOURCE_ACCESS, + ), + }, + `To describe resources in routes, both '${OPENAPI_FIELD_PREFIX}-${OpenApiFieldNames.RESOURCE_NAME}' and '${OPENAPI_FIELD_PREFIX}-${OpenApiFieldNames.RESOURCE_ACCESS}' must be defined at the same time. +Path '${operation.path}' must be annotated properly.`, + ); }); } @@ -178,7 +183,7 @@ export class OpenAPIParser { } } - getPaths(): ReturnType { + getOperations(): Operations { const oasPaths = this.openApiSource.getPaths(); return this.transformPathsSchema(oasPaths); @@ -191,9 +196,9 @@ export class OpenAPIParser { getPathResourceMappings(filterAuthEndpointsOut = true) { // todo: ensure that validation happens before // parameterName, parameterLocation, resourceAccess resourceName need to be valid - const paths = this.getPaths(); + const paths = this.getOperations(); - const filteredPaths = filterAuthEndpointsOut + const filteredOperations = filterAuthEndpointsOut ? paths.filter((path) => { const isAuthEndpoint = Boolean( getOpenApiField(path.schema, OpenApiFieldNames.AUTH_ENDPOINT), @@ -203,79 +208,16 @@ export class OpenAPIParser { }) : paths; - return filteredPaths.map((path) => { - // todo: getParametersAsJSONSchema seems to return null (which is not present in the type definition), open an issue - - // unfortunately, getParametersAsJSONSchema strips x-act fields for other parameter types such as path or query - // this is why there are separate calls for requestBody and parameters - // todo: maybe find a better way to handle this/open an issue - const resourcesFromRequestBody = path.hasRequestBody() - ? path.getParametersAsJSONSchema().flatMap((parameterType) => { - const parameterLocation = parameterType.type as ParameterLocation; - - if ( - !parameterType.schema.properties || - parameterLocation !== "body" - ) { - return []; - } - - return Object.entries(parameterType.schema.properties).flatMap( - ([parameterName, property]) => { - if (typeof property !== "object") { - return []; - } - - const resourceName = getOpenApiField( - property, - OpenApiFieldNames.RESOURCE_NAME, - ) as string; - - const resourceAccess = getOpenApiField( - property, - OpenApiFieldNames.RESOURCE_ACCESS, - ) as string; - - if (!resourceName || !resourceAccess) { - return []; - } - - return { - resourceName, - resourceAccess, - parameterName, - parameterLocation, - }; - }, - ); - }) - : []; - - const resourcesFromParameters = path.getParameters().map((parameter) => ({ - parameterName: parameter.name, - parameterLocation: parameter.in, - resourceName: getOpenApiField( - parameter, - OpenApiFieldNames.RESOURCE_NAME, - ) as string, // todo: replace this with zod parsing - resourceAccess: getOpenApiField( - parameter, - OpenApiFieldNames.RESOURCE_ACCESS, - ) as string, - })); - - const parametrizedResources = [ - ...resourcesFromRequestBody, - ...resourcesFromParameters, - ]; + return filteredOperations.map((operation) => { + const parametrizedResources = this.getParametrizedResources(operation); // todo: at the moment it is considered that there can be at most one non-parametrized resource per path (e.g. /users) const nonParametrizedResourceName = getOpenApiField( - path.schema, + operation.schema, OpenApiFieldNames.RESOURCE_NAME, ) as string; const nonParametrizedResourceAccess = getOpenApiField( - path.schema, + operation.schema, OpenApiFieldNames.RESOURCE_ACCESS, ) as string; const nonParametrizedResources = @@ -288,7 +230,7 @@ export class OpenAPIParser { ] : []; - const securityRequirements = path.getSecurity(); + const securityRequirements = operation.getSecurity(); const isPublicPath = securityRequirements.length === 0; const resources: Array = [ @@ -297,14 +239,92 @@ export class OpenAPIParser { ]; return { - path: path.path, - method: path.method, + path: operation.path, + method: operation.method, isPublicPath, resources, }; }); } + /** + * Returns all resources for the given path enriched with resource information + * coming from various parameters and the request body + * + * @private + * @param operation + */ + private getParametrizedResources(operation: Operation) { + // todo: getParametersAsJSONSchema seems to return null (which is not present in the type definition), open an issue + + // unfortunately, getParametersAsJSONSchema strips x-act fields for other parameter types such as path or query + // this is why there are separate calls for requestBody and parameters + // todo: maybe find a better way to handle this/open an issue + const resourcesFromRequestBody = operation.hasRequestBody() + ? operation.getParametersAsJSONSchema().flatMap((parameterType) => { + const parameterLocation = parameterType.type as ParameterLocation; + + if ( + !parameterType.schema.properties || + parameterLocation !== "body" + ) { + return []; + } + + return Object.entries(parameterType.schema.properties).flatMap( + ([parameterName, property]) => { + if (typeof property !== "object") { + return []; + } + + const resourceName = getOpenApiField( + property, + OpenApiFieldNames.RESOURCE_NAME, + ) as string; + + const resourceAccess = getOpenApiField( + property, + OpenApiFieldNames.RESOURCE_ACCESS, + ) as string; + + return { + resourceName, + resourceAccess, + descriptorsRequired: false, + parameterName, + parameterLocation, + }; + }, + ); + }) + : []; + + const resourcesFromParameters = operation + .getParameters() + .map((parameter) => { + const parameterSchema = parameter.schema as SchemaObject; + const parameterDefaultProvided = Boolean(parameterSchema.default); + const descriptorsRequired = + Boolean(parameter.required) && !parameterDefaultProvided; + + return { + parameterName: parameter.name, + parameterLocation: parameter.in, + resourceName: getOpenApiField( + parameter, + OpenApiFieldNames.RESOURCE_NAME, + ) as string, // todo: replace this with zod parsing + resourceAccess: getOpenApiField( + parameter, + OpenApiFieldNames.RESOURCE_ACCESS, + ) as string, + descriptorsRequired, + }; + }); + + return [...resourcesFromRequestBody, ...resourcesFromParameters]; + } + /** * Transforms the paths schema from Oas to a minimal schema containing only * the necessary information @@ -337,7 +357,7 @@ export class OpenAPIParser { return null; } - const paths = this.getPaths(); + const paths = this.getOperations(); // todo: handle cases when 0 or more than 1 is found // maybe 0 or more than 1 are cases to be handled by validation / parsing @@ -449,7 +469,7 @@ export class OpenAPIParser { // todo: move out filtering part to a separate function (to be reused for identifier/password extract function) private getTokenParameterDescription( - authEndpoint: ReturnType[0], + authEndpoint: ReturnType[0], ): AuthParameterLocationDescription { const responseStatusCodes = authEndpoint.getResponseStatusCodes(); diff --git a/src/core/schemas.ts b/src/core/schemas.ts index c198754..d1cf2ad 100644 --- a/src/core/schemas.ts +++ b/src/core/schemas.ts @@ -1,26 +1,34 @@ -import { z } from "zod"; +import { z, ZodError, ZodSchema, type ZodIssue } from "zod"; + +export const createResourceDescriptorSchema = ({ + allowedResourceNames, + descriptorsRequired = false, +}: { + allowedResourceNames: Array; + descriptorsRequired?: boolean; +}) => { + const resourceAccessSchema = z.enum(["create", "read", "update", "delete"]); + const resourceNameSchema = z.enum( + allowedResourceNames as [string, ...Array], + ); -export const createResourceDescriptorSchema = ( - allowedResourceNames: Array, -) => { return z .object({ - resourceAccess: z.enum(["create", "read", "update", "delete"]).optional(), // todo: unify naming access/action etc. - resourceName: z - .enum(allowedResourceNames as [string, ...Array]) - .optional(), + resourceAccess: descriptorsRequired + ? resourceAccessSchema + : resourceAccessSchema.optional(), + resourceName: descriptorsRequired + ? resourceNameSchema + : resourceNameSchema.optional(), }) - .refine( - (data) => - (data.resourceAccess !== undefined && - data.resourceName !== undefined) || - (data.resourceAccess === undefined && data.resourceName === undefined), - { - message: - "To describe resources in routes, both 'resourceName' and 'resourceAccess' must be defined at the same time.", - path: ["resourceAccess", "resourceName"], - }, - ); + .refine((data) => { + const bothOrOneIsMissing = + (data.resourceAccess === undefined && + data.resourceName === undefined) || + (data.resourceAccess !== undefined && data.resourceName !== undefined); + + return bothOrOneIsMissing; + }); }; export const AuthFieldSchema = z @@ -28,3 +36,41 @@ export const AuthFieldSchema = z type: z.enum(["identifier", "password", "token"]), }) .optional(); + +// from https://stackoverflow.com/a/76642589/13156621 +const formatZodIssue = (issue: ZodIssue): string => { + const { path, message } = issue; + const pathString = path.join("."); + + return `${pathString}: ${message}`; +}; + +const formatZodError = (error: ZodError, prefix: string): string => { + const { issues } = error; + if (issues[0] === undefined) return prefix; + + const firstIssue = formatZodIssue(issues[0]); + + const additionalIssuesCount = issues.length - 1; + const additionalIssuesHint = + additionalIssuesCount >= 1 + ? ` (and ${additionalIssuesCount} more ${additionalIssuesCount > 1 ? "issues" : "issue"})` + : ""; + + return `${prefix}\n\n${firstIssue}${additionalIssuesHint}`; +}; + +export const parseZodSchema = ( + schema: ZodSchema, + data: unknown, + prefix = "An error occurred while trying to parse the provided data.", +): SchemaType => { + try { + return schema.parse(data); + } catch (error) { + if (error instanceof ZodError) { + throw new TypeError(formatZodError(error, prefix)); + } + throw error; + } +}; diff --git a/src/core/types.ts b/src/core/types.ts index 055c8bc..ea7e100 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -2,7 +2,7 @@ import type { AuthParameterLocationDescription } from "./authentication/http/typ import type { OpenAPIParser } from "./parsers/openapi-parser.ts"; export type AuthEndpointInformation = { - authEndpoint: ReturnType[0]; + authEndpoint: ReturnType[0]; authRequestParameterDescription: { identifier: AuthParameterLocationDescription; password: AuthParameterLocationDescription; diff --git a/tests/fixtures/openapi.json b/tests/fixtures/openapi.json index 3bc0348..292aef2 100644 --- a/tests/fixtures/openapi.json +++ b/tests/fixtures/openapi.json @@ -231,6 +231,98 @@ "cookieAuthentication": [] } ] + }, + "patch": { + "summary": "Benutzer updaten", + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "name": { + "type": "string", + "description": "Name des Benutzers", + "example": "Max Mustermann" + }, + "id": { + "type": "string", + "description": "ID des Benutzers", + "x-act": { + "resource-name": "User", + "resource-access": "update" + } + }, + "password": { + "type": "string", + "format": "password", + "description": "Passwort des Benutzers", + "example": "secretpassword" + } + } + } + } + } + }, + "responses": { + "201": { + "description": "Benutzer erfolgreich erstellt", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "id": { + "type": "integer", + "example": 1 + }, + "name": { + "type": "string", + "example": "Max Mustermann" + }, + "email": { + "type": "string", + "example": "max@beispiel.de" + } + } + } + } + } + }, + "400": { + "description": "Ungültige Eingabe", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "errors": { + "type": "array", + "items": { + "type": "object", + "properties": { + "message": { + "type": "string", + "example": "Ungültige E-Mail-Adresse" + } + } + } + } + } + } + } + } + } + }, + "security": [ + { + "bearerHttpAuthentication": [] + }, + { + "cookieAuthentication": [] + } + ] } }, "/admin/users/{id}": { diff --git a/tests/unit/openapi-parser.spec.ts b/tests/unit/openapi-parser.spec.ts index 3ebda46..e7f5df5 100644 --- a/tests/unit/openapi-parser.spec.ts +++ b/tests/unit/openapi-parser.spec.ts @@ -120,7 +120,9 @@ test.group("OpenAPIParser", (group) => { const openAPIParser = await OpenAPIParser.create(specUrl, apiBaseUrl); openAPIParser.validateCustomFields(resources); - }).throws(/both 'resourceName' and 'resourceAccess' must be defined/); + }).throws( + /both 'x-act-resource-name' and 'x-act-resource-access' must be defined/, + ); test("should throw when resource name but not resource access is specified", async () => { const resources = [new Resource("User")]; @@ -133,7 +135,9 @@ test.group("OpenAPIParser", (group) => { const openAPIParser = await OpenAPIParser.create(specUrl, apiBaseUrl); openAPIParser.validateCustomFields(resources); - }).throws(/both 'resourceName' and 'resourceAccess' must be defined/); + }).throws( + /both 'x-act-resource-name' and 'x-act-resource-access' must be defined/, + ); test("should throw when resource description is omitted for required parameters", async () => { const resources = [new Resource("User")]; @@ -146,8 +150,20 @@ test.group("OpenAPIParser", (group) => { openAPIParser.validateCustomFields(resources); }).throws( - "To describe required resources in routes, both 'resourceName' and 'resourceAccess' must be defined at the same time.", - ); + /To describe required resources in routes, both 'x-act-resource-name' and 'x-act-resource-access' must be defined at the same time./, + ); // todo: also check for "Parameter 'id' in path '/admin/users/{id}' must be annotated properly." + + test("should throw when resource name is invalid in request body property", async () => { + const resources = [new Resource("User")]; + + currentSpec.paths["/admin/users"].patch.requestBody.content[ + "application/json" + ].schema.properties.id["x-act"]["resource-name"] = "test"; + + const openAPIParser = await OpenAPIParser.create(specUrl, apiBaseUrl); + + openAPIParser.validateCustomFields(resources); + }).throws(/Expected 'User', received 'test'/); test("should properly combine api base url containing path with given path", async ({ expect,