diff --git a/config/http/middleware/handlers/cors.json b/config/http/middleware/handlers/cors.json index c0298b0260..2f19b46b6c 100644 --- a/config/http/middleware/handlers/cors.json +++ b/config/http/middleware/handlers/cors.json @@ -15,7 +15,7 @@ "DELETE" ], "options_credentials": true, - "options_preflightContinue": true, + "options_preflightContinue": false, "options_exposedHeaders": [ "Accept-Patch", "Accept-Post", diff --git a/config/ldp/handler/components/operation-handler.json b/config/ldp/handler/components/operation-handler.json index 93fdcb71cd..8494a9139b 100644 --- a/config/ldp/handler/components/operation-handler.json +++ b/config/ldp/handler/components/operation-handler.json @@ -5,10 +5,6 @@ "@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" } diff --git a/src/http/ldp/OptionsOperationHandler.ts b/src/http/ldp/OptionsOperationHandler.ts deleted file mode 100644 index 866580090b..0000000000 --- a/src/http/ldp/OptionsOperationHandler.ts +++ /dev/null @@ -1,36 +0,0 @@ -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 deleted file mode 100644 index e9601dfe74..0000000000 --- a/src/http/output/response/NoContentResponseDescription.ts +++ /dev/null @@ -1,10 +0,0 @@ -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 e785ed0c66..949379706b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -83,7 +83,6 @@ 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'; @@ -106,7 +105,6 @@ 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/test/integration/Middleware.test.ts b/test/integration/Middleware.test.ts index f94b85b436..0d9be03f96 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(200); + .expect(204); 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 13354b4344..d0d4acd51e 100644 --- a/test/integration/PermissionTable.test.ts +++ b/test/integration/PermissionTable.test.ts @@ -44,9 +44,13 @@ 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][] = [ - [ 'OPTIONS', 'C/R', [], undefined, '', '', 401, 401 ], - [ 'OPTIONS', 'C/R', [], [ AM.read ], '', '', 204, 404 ], - [ 'OPTIONS', 'C/R', [ AM.read ], undefined, '', '', 204, 404 ], + // No authorization headers are sent in an OPTIONS request making it impossible to grant permission. + // See https://github.com/CommunitySolidServer/CommunitySolidServer/issues/1246#issuecomment-1087325235 + // From https://fetch.spec.whatwg.org/#cors-preflight-fetch it follows + // that a preflight check should always return an OK response. + [ 'OPTIONS', 'C/R', [], undefined, '', '', 204, 204 ], + [ 'OPTIONS', 'C/R', [], [ AM.read ], '', '', 204, 204 ], + [ 'OPTIONS', 'C/R', [ AM.read ], undefined, '', '', 204, 204 ], [ '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 deleted file mode 100644 index fbb501a04b..0000000000 --- a/test/unit/http/ldp/OptionsOperationHandler.test.ts +++ /dev/null @@ -1,44 +0,0 @@ -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); - }); -});