From ad3edcf1a89257344d2209edd76b91f3eed9bc82 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Thu, 17 Mar 2022 15:34:03 +0100 Subject: [PATCH] feat: Handle OPTIONS requests in OperationHandler --- config/http/middleware/handlers/cors.json | 1 + .../handler/components/operation-handler.json | 8 +++- .../permissions/MethodModesExtractor.ts | 2 +- src/http/ldp/OptionsOperationHandler.ts | 36 +++++++++++++++ .../response/NoContentResponseDescription.ts | 10 +++++ src/index.ts | 2 + src/server/middleware/CorsHandler.ts | 11 ++--- test/integration/Middleware.test.ts | 2 +- test/integration/PermissionTable.test.ts | 7 ++- .../http/ldp/OptionsOperationHandler.test.ts | 44 +++++++++++++++++++ 10 files changed, 110 insertions(+), 13 deletions(-) create mode 100644 src/http/ldp/OptionsOperationHandler.ts create mode 100644 src/http/output/response/NoContentResponseDescription.ts create mode 100644 test/unit/http/ldp/OptionsOperationHandler.test.ts diff --git a/config/http/middleware/handlers/cors.json b/config/http/middleware/handlers/cors.json index f818b6c36..00371890a 100644 --- a/config/http/middleware/handlers/cors.json +++ b/config/http/middleware/handlers/cors.json @@ -15,6 +15,7 @@ "DELETE" ], "options_credentials": true, + "options_preflightContinue": true, "options_exposedHeaders": [ "Accept-Patch", "ETag", diff --git a/config/ldp/handler/components/operation-handler.json b/config/ldp/handler/components/operation-handler.json index 0a15f9ecc..93fdcb71c 100644 --- a/config/ldp/handler/components/operation-handler.json +++ b/config/ldp/handler/components/operation-handler.json @@ -5,9 +5,13 @@ "@id": "urn:solid-server:default:OperationHandler", "@type": "WaterfallHandler", "handlers": [ + { + "@type": "OptionsOperationHandler", + "resourceSet": { "@id": "urn:solid-server:default:CachedResourceSet" } + }, { "@type": "GetOperationHandler", - "store": { "@id": "urn:solid-server:default:ResourceStore" }, + "store": { "@id": "urn:solid-server:default:ResourceStore" } }, { "@type": "PostOperationHandler", @@ -23,7 +27,7 @@ }, { "@type": "HeadOperationHandler", - "store": { "@id": "urn:solid-server:default:ResourceStore" }, + "store": { "@id": "urn:solid-server:default:ResourceStore" } }, { "@type": "PatchOperationHandler", diff --git a/src/authorization/permissions/MethodModesExtractor.ts b/src/authorization/permissions/MethodModesExtractor.ts index 7db15d26b..944e6a556 100644 --- a/src/authorization/permissions/MethodModesExtractor.ts +++ b/src/authorization/permissions/MethodModesExtractor.ts @@ -5,7 +5,7 @@ import { isContainerIdentifier } from '../../util/PathUtil'; import { ModesExtractor } from './ModesExtractor'; import { AccessMode } from './Permissions'; -const READ_METHODS = new Set([ 'GET', 'HEAD' ]); +const READ_METHODS = new Set([ 'OPTIONS', 'GET', 'HEAD' ]); const SUPPORTED_METHODS = new Set([ ...READ_METHODS, 'PUT', 'POST', 'DELETE' ]); /** diff --git a/src/http/ldp/OptionsOperationHandler.ts b/src/http/ldp/OptionsOperationHandler.ts new file mode 100644 index 000000000..866580090 --- /dev/null +++ b/src/http/ldp/OptionsOperationHandler.ts @@ -0,0 +1,36 @@ +import type { ResourceSet } from '../../storage/ResourceSet'; +import { NotFoundHttpError } from '../../util/errors/NotFoundHttpError'; +import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError'; +import { NoContentResponseDescription } from '../output/response/NoContentResponseDescription'; +import type { ResponseDescription } from '../output/response/ResponseDescription'; +import type { OperationHandlerInput } from './OperationHandler'; +import { OperationHandler } from './OperationHandler'; + +/** + * Handles OPTIONS {@link Operation}s by always returning a 204. + */ +export class OptionsOperationHandler extends OperationHandler { + private readonly resourceSet: ResourceSet; + + /** + * Uses a {@link ResourceSet} to determine the existence of the target resource which impacts the response code. + * @param resourceSet - {@link ResourceSet} that knows if the target resource exists or not. + */ + public constructor(resourceSet: ResourceSet) { + super(); + this.resourceSet = resourceSet; + } + + public async canHandle({ operation }: OperationHandlerInput): Promise { + if (operation.method !== 'OPTIONS') { + throw new NotImplementedHttpError('This handler only supports OPTIONS operations'); + } + } + + public async handle({ operation }: OperationHandlerInput): Promise { + if (!await this.resourceSet.hasResource(operation.target)) { + throw new NotFoundHttpError(); + } + return new NoContentResponseDescription(); + } +} diff --git a/src/http/output/response/NoContentResponseDescription.ts b/src/http/output/response/NoContentResponseDescription.ts new file mode 100644 index 000000000..e9601dfe7 --- /dev/null +++ b/src/http/output/response/NoContentResponseDescription.ts @@ -0,0 +1,10 @@ +import { ResponseDescription } from './ResponseDescription'; + +/** + * Corresponds to a 204 response. + */ +export class NoContentResponseDescription extends ResponseDescription { + public constructor() { + super(204); + } +} diff --git a/src/index.ts b/src/index.ts index 2f30aa872..542cf9a77 100644 --- a/src/index.ts +++ b/src/index.ts @@ -82,6 +82,7 @@ export * from './http/ldp/DeleteOperationHandler'; export * from './http/ldp/GetOperationHandler'; export * from './http/ldp/HeadOperationHandler'; export * from './http/ldp/OperationHandler'; +export * from './http/ldp/OptionsOperationHandler'; export * from './http/ldp/PatchOperationHandler'; export * from './http/ldp/PostOperationHandler'; export * from './http/ldp/PutOperationHandler'; @@ -103,6 +104,7 @@ export * from './http/output/metadata/WwwAuthMetadataWriter'; // HTTP/Output/Response export * from './http/output/response/CreatedResponseDescription'; +export * from './http/output/response/NoContentResponseDescription'; export * from './http/output/response/OkResponseDescription'; export * from './http/output/response/ResetResponseDescription'; export * from './http/output/response/ResponseDescription'; diff --git a/src/server/middleware/CorsHandler.ts b/src/server/middleware/CorsHandler.ts index 23af49c5a..d06116645 100644 --- a/src/server/middleware/CorsHandler.ts +++ b/src/server/middleware/CorsHandler.ts @@ -21,12 +21,13 @@ interface SimpleCorsOptions { /** * Handler that sets CORS options on the response. - * In case of an OPTIONS request this handler will close the connection after adding its headers. + * In case of an OPTIONS request this handler will close the connection after adding its headers + * if `preflightContinue` is set to `false`. * - * Solid, §7.1: "A data pod MUST implement the CORS protocol [FETCH] such that, to the extent possible, - * the browser allows Solid apps to send any request and combination of request headers to the data pod, - * and the Solid app can read any response and response headers received from the data pod." - * Full details: https://solid.github.io/specification/protocol#cors-server + * Solid, §8.1: "A server MUST implement the CORS protocol [FETCH] such that, to the extent possible, + * the browser allows Solid apps to send any request and combination of request headers to the server, + * and the Solid app can read any response and response headers received from the server." + * Full details: https://solidproject.org/TR/2021/protocol-20211217#cors-server */ export class CorsHandler extends HttpHandler { private readonly corsHandler: ( diff --git a/test/integration/Middleware.test.ts b/test/integration/Middleware.test.ts index 48ef13674..75a208636 100644 --- a/test/integration/Middleware.test.ts +++ b/test/integration/Middleware.test.ts @@ -70,7 +70,7 @@ describe('An http server with middleware', (): void => { .set('Access-Control-Request-Headers', 'content-type') .set('Access-Control-Request-Method', 'POST') .set('Host', 'test.com') - .expect(204); + .expect(200); expect(res.header).toEqual(expect.objectContaining({ 'access-control-allow-origin': '*', 'access-control-allow-headers': 'content-type', diff --git a/test/integration/PermissionTable.test.ts b/test/integration/PermissionTable.test.ts index 078516db7..13354b434 100644 --- a/test/integration/PermissionTable.test.ts +++ b/test/integration/PermissionTable.test.ts @@ -44,10 +44,9 @@ const allModes = [ AM.read, AM.append, AM.create, AM.write, AM.delete ]; // For PUT/PATCH/DELETE we return 205 instead of 200/204 /* eslint-disable no-multi-spaces */ const table: [string, string, AM[], AM[] | undefined, string, string, number, number][] = [ - // We currently handle OPTIONS before authorization - // [ 'OPTIONS', 'C/R', [], undefined, '', '', 401, 401 ], - // [ 'OPTIONS', 'C/R', [], [ AM.read ], '', '', 200, 404 ], - // [ 'OPTIONS', 'C/R', [ AM.read ], undefined, '', '', 200, 404 ], + [ 'OPTIONS', 'C/R', [], undefined, '', '', 401, 401 ], + [ 'OPTIONS', 'C/R', [], [ AM.read ], '', '', 204, 404 ], + [ 'OPTIONS', 'C/R', [ AM.read ], undefined, '', '', 204, 404 ], [ 'HEAD', 'C/R', [], undefined, '', '', 401, 401 ], [ 'HEAD', 'C/R', [], [ AM.read ], '', '', 200, 404 ], diff --git a/test/unit/http/ldp/OptionsOperationHandler.test.ts b/test/unit/http/ldp/OptionsOperationHandler.test.ts new file mode 100644 index 000000000..fbb501a04 --- /dev/null +++ b/test/unit/http/ldp/OptionsOperationHandler.test.ts @@ -0,0 +1,44 @@ +import { OptionsOperationHandler } from '../../../../src/http/ldp/OptionsOperationHandler'; +import type { Operation } from '../../../../src/http/Operation'; +import { BasicRepresentation } from '../../../../src/http/representation/BasicRepresentation'; +import { BasicConditions } from '../../../../src/storage/BasicConditions'; +import type { ResourceSet } from '../../../../src/storage/ResourceSet'; +import { NotFoundHttpError } from '../../../../src/util/errors/NotFoundHttpError'; +import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError'; + +describe('An OptionsOperationHandler', (): void => { + let operation: Operation; + const conditions = new BasicConditions({}); + const preferences = {}; + const body = new BasicRepresentation(); + let resourceSet: jest.Mocked; + let handler: OptionsOperationHandler; + + beforeEach(async(): Promise => { + operation = { method: 'OPTIONS', target: { path: 'http://test.com/foo' }, preferences, conditions, body }; + resourceSet = { + hasResource: jest.fn().mockResolvedValue(true), + }; + handler = new OptionsOperationHandler(resourceSet); + }); + + it('only supports Options operations.', async(): Promise => { + await expect(handler.canHandle({ operation })).resolves.toBeUndefined(); + operation.method = 'GET'; + await expect(handler.canHandle({ operation })).rejects.toThrow(NotImplementedHttpError); + operation.method = 'HEAD'; + await expect(handler.canHandle({ operation })).rejects.toThrow(NotImplementedHttpError); + }); + + it('returns a 204 response.', async(): Promise => { + const result = await handler.handle({ operation }); + expect(result.statusCode).toBe(204); + expect(result.metadata).toBeUndefined(); + expect(result.data).toBeUndefined(); + }); + + it('returns a 404 if the target resource does not exist.', async(): Promise => { + resourceSet.hasResource.mockResolvedValueOnce(false); + await expect(handler.handle({ operation })).rejects.toThrow(NotFoundHttpError); + }); +});