diff --git a/demo-application/public/openapi.yml b/demo-application/public/openapi.yml index 8bd6b9b..24d19df 100644 --- a/demo-application/public/openapi.yml +++ b/demo-application/public/openapi.yml @@ -150,6 +150,65 @@ paths: security: - bearerHttpAuthentication: [ ] - 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}: get: summary: Einzelner Benutzer diff --git a/demo-application/start/routes.ts b/demo-application/start/routes.ts index 5cd2676..92fbef0 100644 --- a/demo-application/start/routes.ts +++ b/demo-application/start/routes.ts @@ -40,6 +40,19 @@ router return User.query() }) + router.patch('/users', async ({ request, response }) => { + // no check if user can be accessed + // for now just a dummy endpoint for changing some user data + + const { id } = request.body() + + if (!id) { + return response.status(400).send({}) + } + + return User.findOrFail(id) + }) + router.get('/users/:id', async ({ params }) => { return User.findOrFail(params.id) }) diff --git a/src/core/parsers/openapi-parser.ts b/src/core/parsers/openapi-parser.ts index 5dc90b9..685b3e1 100644 --- a/src/core/parsers/openapi-parser.ts +++ b/src/core/parsers/openapi-parser.ts @@ -30,11 +30,18 @@ import { type SpecificationUrl = ConstructorParameters[0]; type SecurityScheme = KeyedSecuritySchemeObject; +export type ResourceLocationDescriptor = { + resourceName: string; + resourceAccess: string; + parameterName?: string; + parameterLocation?: ParameterLocation; +}; + export class OpenAPIParser { private constructor( private readonly openApiSource: Oas, private readonly apiBaseUrl: string, - ) {} // private specificationPath: ConstructorParameters[0], + ) {} /** * Parses the OpenAPI specification and returns a new instance of the @@ -197,21 +204,70 @@ export class OpenAPIParser { : paths; return filteredPaths.map((path) => { - const parameters = path.getParameters(); - - const parametrizedResources = - parameters.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, - })) ?? []; + // 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, + ]; // 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( @@ -235,12 +291,10 @@ export class OpenAPIParser { const securityRequirements = path.getSecurity(); const isPublicPath = securityRequirements.length === 0; - const resources: Array<{ - resourceName: string; - resourceAccess: string; - parameterName?: string; - parameterLocation?: string; - }> = [...parametrizedResources, ...nonParametrizedResources]; + const resources: Array = [ + ...parametrizedResources, + ...nonParametrizedResources, + ]; return { path: path.path, diff --git a/src/core/tests/runner/test-runner.ts b/src/core/tests/runner/test-runner.ts index dcebcbf..536236b 100644 --- a/src/core/tests/runner/test-runner.ts +++ b/src/core/tests/runner/test-runner.ts @@ -1,13 +1,14 @@ import chalk from "chalk"; import CliTable3 from "cli-table3"; import type { User } from "../../policy/entities/user.ts"; -import type { Route } from "../test-utils.ts"; +import type { RequestBody, Route } from "../test-utils.ts"; export type AccessControlResult = "permitted" | "denied"; export type TestResult = { user: User | null; route: Route; + requestBody?: RequestBody; expected: AccessControlResult; actual?: AccessControlResult; result?: "✅" | "❌" | "⏭️"; @@ -53,6 +54,14 @@ export abstract class TestRunner { abstract run(testCases: Array): Promise; + protected formatRequestBody(body: object): string { + try { + return chalk.cyan(JSON.stringify(body, null, 2)); + } catch { + return chalk.red("Invalid JSON"); + } + } + protected printReport(): void { console.log("\n=== Test Report ==="); @@ -69,9 +78,14 @@ export abstract class TestRunner { }); this.testResults.forEach((result) => { + const requestInfo = + typeof result.requestBody === "object" + ? `${result.route}\n${this.formatRequestBody(result.requestBody)}` + : result.route.toString(); + reportTable.push([ result.user?.toString() ?? "anonymous", - result.route.toString(), + requestInfo, result.expected, result.actual, result.statusCode, diff --git a/src/core/tests/test-case-generator.ts b/src/core/tests/test-case-generator.ts index 2b0b1e8..0725cb0 100644 --- a/src/core/tests/test-case-generator.ts +++ b/src/core/tests/test-case-generator.ts @@ -10,12 +10,13 @@ import type { } from "../policy/types.ts"; import { removeObjectDuplicatesFromArray } from "../utils.ts"; import type { TestCase } from "./runner/test-runner.ts"; -import { Route } from "./test-utils.ts"; +import { createRequestData, Route } from "./test-utils.ts"; import { TestCaseExecutor } from "./testcase-executor.ts"; export type TestCombination = { user: User | null; route: Route; + requestBody?: object; expectedRequestToBeAllowed: boolean; }; @@ -117,27 +118,19 @@ export class TestCaseGenerator { path, currentResource.parameterName, ) && - resourceIdentifier == undefined; + resourceIdentifier === undefined; if (resourceIdentifierRequiredButNotProvided) { return []; } - // todo: currently only parameterLocation path supported - // function should support parameterLocation, parameterName and parameterValue - - // resourceIdentifier can be undefined when resource access is create for instance - // or when access for all resources of a type is described - const expandedPath = - resourceIdentifier == undefined || - currentResource.parameterName === undefined || - currentResource.parameterLocation === undefined - ? path - : OpenAPIParser.expandUrlTemplate(path, { - [currentResource.parameterName]: resourceIdentifier, - }); // todo: for multiple resources and therefore parameters, multiple keys in object -> dynamic mapping required - - const url = this.openApiParser.constructFullApiUrl(expandedPath); + const { route, requestBody } = createRequestData({ + path, + method, + currentResource, + resourceIdentifier, + openApiParser: this.openApiParser, + }); const expectedRequestToBeAllowed = PolicyDecisionPoint.isAllowed( user, @@ -148,7 +141,8 @@ export class TestCaseGenerator { return { user, - route: new Route(url, method), + route, + requestBody, expectedRequestToBeAllowed, resourceAction, }; diff --git a/src/core/tests/test-utils.ts b/src/core/tests/test-utils.ts index 343ac12..be5384c 100644 --- a/src/core/tests/test-utils.ts +++ b/src/core/tests/test-utils.ts @@ -1,12 +1,19 @@ import type { URL } from "node:url"; import got, { HTTPError, type Method, type PromiseCookieJar } from "got"; -import { type RequestAuthenticator } from "../authentication/http/authenticator.ts"; +import type { RequestAuthenticator } from "../authentication/http/authenticator"; import { SessionManager } from "../authentication/http/session-manager.ts"; -import type { AuthenticationCredentials } from "../authentication/http/types.ts"; +import type { AuthenticationCredentials } from "../authentication/http/types"; import { API_CLIENT_MAX_REQUEST_RETRIES, HTTP_UNAUTHORIZED_STATUS_CODE, } from "../constants.ts"; +import { + OpenAPIParser, + type ResourceLocationDescriptor, +} from "../parsers/openapi-parser.ts"; +import type { ResourceIdentifier } from "../policy/types.ts"; + +export type RequestBody = object | undefined; export class Route { constructor( @@ -25,19 +32,24 @@ export class Route { * request continues to fail with a non 2xx/3xx status code. * * @param route + * @param requestBody The request body to send as JSON * @param authenticator * @param credentials * @throws {Error} If the request needed prior authentication but failed to - * authenticate See - * {@link https://github.com/sindresorhus/got/blob/main/documentation/8-errors.md list of errors} - * @throws See + * authenticate. See * {@link https://github.com/sindresorhus/got/blob/main/documentation/8-errors.md list of errors} */ -export async function performRequest( - route: Route, - authenticator: RequestAuthenticator | null, - credentials: AuthenticationCredentials | null, -) { +export async function performRequest({ + route, + requestBody, + authenticator, + credentials, +}: { + route: Route; + requestBody?: RequestBody; + authenticator: RequestAuthenticator | null; + credentials: AuthenticationCredentials | null; +}) { const routeRequiresAuthentication = authenticator !== null; const userCredentialsAvailable = credentials !== null; @@ -52,6 +64,7 @@ export async function performRequest( return got(route.url, { method: route.method, + json: requestBody, retry, throwHttpErrors: false, hooks: { @@ -120,3 +133,54 @@ export async function performRequest( }, }); } + +/** + * Creates the request data for an individual test combination by returning the + * route to use and optionally the request body. + */ +export function createRequestData({ + path, + method, + currentResource, + resourceIdentifier, + openApiParser, +}: { + path: string; + method: Method; + currentResource: ResourceLocationDescriptor; + resourceIdentifier?: ResourceIdentifier; + openApiParser: OpenAPIParser; +}) { + // todo: currently only parameterLocation path supported + // function should support parameterLocation, parameterName and parameterValue + + // resourceIdentifier can be undefined when resource access is create for instance + // or when access for all resources of a type is described + let requestBody: RequestBody; + + const expandedPath = + resourceIdentifier === undefined || + currentResource.parameterName === undefined || + currentResource.parameterLocation !== "path" + ? path + : OpenAPIParser.expandUrlTemplate(path, { + [currentResource.parameterName]: resourceIdentifier, + }); // todo: for multiple resources and therefore parameters, multiple keys in object -> dynamic mapping required + + const url = openApiParser.constructFullApiUrl(expandedPath); + + // todo: this only works for params on the top level of the request body + if ( + currentResource.parameterLocation === "body" && + currentResource.parameterName !== undefined + ) { + requestBody = { + [currentResource.parameterName]: resourceIdentifier, + }; + } + + return { + route: new Route(url, method), + requestBody, + }; +} diff --git a/src/core/tests/testcase-executor.ts b/src/core/tests/testcase-executor.ts index f473bdc..b663c18 100644 --- a/src/core/tests/testcase-executor.ts +++ b/src/core/tests/testcase-executor.ts @@ -38,11 +38,18 @@ export class TestCaseExecutor { testCombination: TestCombination, { expect, skip }: TestContext, ): Promise { - const { user, route, expectedRequestToBeAllowed } = testCombination; + const { user, route, requestBody, expectedRequestToBeAllowed } = + testCombination; const expected: AccessControlResult = expectedRequestToBeAllowed ? "permitted" : "denied"; - const testResult: TestResult = { user, route, expected, result: "❌" }; + const testResult: TestResult = { + user, + route, + requestBody, + expected, + result: "❌", + }; const userHasBeenBlocked = user !== null && @@ -62,7 +69,12 @@ export class TestCaseExecutor { let response; try { - response = await performRequest(route, authenticator, credentials); + response = await performRequest({ + route, + requestBody, + authenticator, + credentials, + }); } catch (error: unknown) { if (error instanceof Error) { console.error(error.message); diff --git a/tests/unit/test-utils.spec.ts b/tests/unit/test-utils.spec.ts index dae7126..ae1a599 100644 --- a/tests/unit/test-utils.spec.ts +++ b/tests/unit/test-utils.spec.ts @@ -22,22 +22,26 @@ test.group("TestUtils", (group) => { test("performRequest should properly authenticate via cookie-based authentication", async ({ expect, }) => { - const response = await performRequest( - protectedRoute, - createCookieAuthenticator(), - { + const response = await performRequest({ + route: protectedRoute, + authenticator: createCookieAuthenticator(), + credentials: { identifier: validUsername, password: validPassword, }, - ); + }); expect(response.statusCode).toBe(200); }); test("performRequest should throw when using invalid credentials", async () => { - await performRequest(protectedRoute, createCookieAuthenticator(), { - identifier: validUsername, - password: "wrongpassword", + await performRequest({ + route: protectedRoute, + authenticator: createCookieAuthenticator(), + credentials: { + identifier: validUsername, + password: "wrongpassword", + }, }); }).throws(/authentication endpoint returned a non-successful response/); @@ -61,11 +65,11 @@ test.group("TestUtils", (group) => { sessionStore.set(credentials.identifier, temporarySession); - const response = await performRequest( - protectedRoute, - cookieAuthenticator, + const response = await performRequest({ + route: protectedRoute, + authenticator: cookieAuthenticator, credentials, - ); + }); // since the existing session can't be used, it should have been removed // and a new one should have been created