fix: Undo authorization on OPTIONS requests

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",

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" }

src/http/ldp/OptionsOperationHandler.ts

@@ -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<void> {
-    if (operation.method !== 'OPTIONS') {
-      throw new NotImplementedHttpError('This handler only supports OPTIONS operations');
-    }
-  }
-
-  public async handle({ operation }: OperationHandlerInput): Promise<ResponseDescription> {
-    if (!await this.resourceSet.hasResource(operation.target)) {
-      throw new NotFoundHttpError();
-    }
-    return new NoContentResponseDescription();
-  }
-}

src/http/output/response/NoContentResponseDescription.ts

@@ -1,10 +0,0 @@
-import { ResponseDescription } from './ResponseDescription';
-
-/**
- * Corresponds to a 204 response.
- */
-export class NoContentResponseDescription extends ResponseDescription {
-  public constructor() {
-    super(204);
-  }
-}

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';

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',

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 ],
+  // No authorization headers are sent in an OPTIONS request making it impossible to grant permission.
-  [ 'OPTIONS', 'C/R', [],                     [ AM.read ],            '',     '',  204, 404 ],
+  // See https://github.com/CommunitySolidServer/CommunitySolidServer/issues/1246#issuecomment-1087325235
-  [ 'OPTIONS', 'C/R', [ AM.read ],            undefined,              '',     '',  204, 404 ],
+  // 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 ],

test/unit/http/ldp/OptionsOperationHandler.test.ts

@@ -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<ResourceSet>;
-  let handler: OptionsOperationHandler;
-
-  beforeEach(async(): Promise<void> => {
-    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<void> => {
-    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<void> => {
-    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<void> => {
-    resourceSet.hasResource.mockResolvedValueOnce(false);
-    await expect(handler.handle({ operation })).rejects.toThrow(NotFoundHttpError);
-  });
-});