Skip to content

feat: better error messages & improved validations #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
niklashaug merged 11 commits into from
Mar 14, 2025
Merged

feat: better error messages & improved validations #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5975662
added parameter and path info in validation error
niklashaug Mar 14, 2025
b2f5d9f
added parseZodSchema utility function that parses the given schema an…
niklashaug Mar 14, 2025
8c76820
added default prefix for zod schema parsing function
niklashaug Mar 14, 2025
4ef47cf
prepare validateCustomFields function for validating request body par…
niklashaug Mar 14, 2025
3cd8a8c
introduced getParametrizedResources method used for validating reques…
niklashaug Mar 14, 2025
4d291ed
added update user to openapi fixture
niklashaug Mar 14, 2025
c8b6a75
show issue instead of issues for 1 additional issue
niklashaug Mar 14, 2025
3d781c6
make sure all resources are returned inside getParametrizedResources
niklashaug Mar 14, 2025
de676d6
added test cases for request body validation
niklashaug Mar 14, 2025
64c93f7
since there are no required individual request body properties, set d…
niklashaug Mar 14, 2025
cd0cc96
formatted with prettier
niklashaug Mar 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
256 changes: 138 additions & 118 deletions src/core/parsers/openapi-parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
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 {
Expand All @@ -37,6 +37,9 @@
parameterLocation?: ParameterLocation;
};

type Operations = ReturnType<OpenAPIParser["transformPathsSchema"]>;
type Operation = Operations[0];

export class OpenAPIParser {
private constructor(
private readonly openApiSource: Oas,
Expand Down Expand Up @@ -89,52 +92,54 @@
// todo: should this be part of the creation process?
validateCustomFields(resources: Array<Resource>) {
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.`,
);
});
}

Expand Down Expand Up @@ -178,7 +183,7 @@
}
}

getPaths(): ReturnType<OpenAPIParser["transformPathsSchema"]> {
getOperations(): Operations {
const oasPaths = this.openApiSource.getPaths();

return this.transformPathsSchema(oasPaths);
Expand All @@ -191,9 +196,9 @@
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),
Expand All @@ -203,79 +208,16 @@
})
: 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 =
Expand All @@ -288,7 +230,7 @@
]
: [];

const securityRequirements = path.getSecurity();
const securityRequirements = operation.getSecurity();
const isPublicPath = securityRequirements.length === 0;

const resources: Array<ResourceLocationDescriptor> = [
Expand All @@ -297,14 +239,92 @@
];

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
Expand Down Expand Up @@ -337,7 +357,7 @@
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
Expand Down Expand Up @@ -415,7 +435,7 @@
}

if (
authenticatorType === AuthenticatorType.API_KEY_COOKIE &&

Check warning on line 438 in src/core/parsers/openapi-parser.ts

View workflow job for this annotation

GitHub Actions / Run all tests 

Unnecessary conditional, comparison is always true, since `AuthenticatorType.API_KEY_COOKIE === AuthenticatorType.API_KEY_COOKIE` is true
"name" in securityScheme
) {
const authResponseParameterDescription = {
Expand All @@ -426,7 +446,7 @@
// todo: check if parameterLocation is of type ParameterLocation
if (
!authResponseParameterDescription.parameterName ||
!authResponseParameterDescription.parameterLocation

Check warning on line 449 in src/core/parsers/openapi-parser.ts

View workflow job for this annotation

GitHub Actions / Run all tests 

Unnecessary conditional, value is always falsy
) {
throw new Error(
"Could not find parameter name (name) or parameter location (in) for Cookie Authentication",
Expand All @@ -449,7 +469,7 @@

// todo: move out filtering part to a separate function (to be reused for identifier/password extract function)
private getTokenParameterDescription(
authEndpoint: ReturnType<OpenAPIParser["getPaths"]>[0],
authEndpoint: ReturnType<OpenAPIParser["getOperations"]>[0],
): AuthParameterLocationDescription {
const responseStatusCodes = authEndpoint.getResponseStatusCodes();

Expand Down Expand Up @@ -496,7 +516,7 @@
httpMethod as HttpMethods,
);

if (!operation) {

Check warning on line 519 in src/core/parsers/openapi-parser.ts

View workflow job for this annotation

GitHub Actions / Run all tests 

Unnecessary conditional, value is always falsy
throw new Error("Operation not found");
}

Expand Down
Loading
Loading