feat: Return correct status codes for invalid requests

config/ldp/handler/components/operation-handler.json

@@ -28,6 +28,10 @@
         {
           "@type": "PatchOperationHandler",
           "store": { "@id": "urn:solid-server:default:ResourceStore" }
+        },
+        {
+          "@type": "StaticThrowHandler",
+          "error": { "@type": "MethodNotAllowedHttpError" }
         }
       ]
     }

config/ldp/modes/default.json

@@ -7,7 +7,24 @@
       "@type": "WaterfallHandler",
       "handlers": [
         { "@type": "MethodModesExtractor" },
-        { "@type": "SparqlPatchModesExtractor" }
+        {
+          "@type": "WrappedMethodModesExtractor",
+          "methods": [ "PATCH" ],
+          "extractor": {
+            "@type": "WaterfallHandler",
+            "handlers": [
+              { "@type": "SparqlUpdateModesExtractor" },
+              {
+                "@type": "StaticThrowHandler",
+                "error": { "@type": "UnsupportedMediaTypeHttpError" }
+              }
+            ]
+          }
+        },
+        {
+          "@type": "StaticThrowHandler",
+          "error": { "@type": "MethodNotAllowedHttpError" }
+        }
       ]
     }
   ]

config/storage/middleware/stores/patching.json

@@ -22,6 +22,10 @@
               "converter": { "@id": "urn:solid-server:default:RepresentationConverter" },
               "intermediateType": "internal/quads",
               "defaultType": "text/turtle"
+            },
+            {
+              "@type": "StaticThrowHandler",
+              "error": { "@type": "UnsupportedMediaTypeHttpError" }
             }
           ]
         }

src/authorization/permissions/ModesExtractor.ts

@@ -2,4 +2,7 @@ import type { Operation } from '../../http/Operation';
 import { AsyncHandler } from '../../util/handlers/AsyncHandler';
 import type { AccessMode } from './Permissions';
 
+/**
+ * Extracts all {@link AccessMode}s that are necessary to execute the given {@link Operation}.
+ */
 export abstract class ModesExtractor extends AsyncHandler<Operation, Set<AccessMode>> {}

src/authorization/permissions/SparqlPatchModesExtractor.ts → src/authorization/permissions/SparqlUpdateModesExtractor.ts

@@ -6,11 +6,13 @@ import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpErr
 import { ModesExtractor } from './ModesExtractor';
 import { AccessMode } from './Permissions';
 
-export class SparqlPatchModesExtractor extends ModesExtractor {
-  public async canHandle({ method, body }: Operation): Promise<void> {
-    if (method !== 'PATCH') {
-      throw new NotImplementedHttpError(`Cannot determine permissions of ${method}, only PATCH.`);
-    }
+/**
+ * Generates permissions for a SPARQL DELETE/INSERT body.
+ * Updates with only an INSERT can be done with just append permissions,
+ * while DELETEs require write permissions as well.
+ */
+export class SparqlUpdateModesExtractor extends ModesExtractor {
+  public async canHandle({ body }: Operation): Promise<void> {
     if (!this.isSparql(body)) {
       throw new NotImplementedHttpError('Cannot determine permissions of non-SPARQL patches.');
     }

src/authorization/permissions/WrappedMethodModesExtractor.ts

@@ -0,0 +1,32 @@
+import type { Operation } from '../../http/Operation';
+import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError';
+import { ModesExtractor } from './ModesExtractor';
+import type { AccessMode } from './Permissions';
+
+/**
+ * Only allows {@link Operation}s with the specified methods.
+ * If the method matches the Operation will be sent to the provided extractor.
+ */
+export class WrappedMethodModesExtractor extends ModesExtractor {
+  private readonly methods: string[];
+  private readonly extractor: ModesExtractor;
+
+  public constructor(methods: string[], extractor: ModesExtractor) {
+    super();
+    this.methods = methods;
+    this.extractor = extractor;
+  }
+
+  public async canHandle(input: Operation): Promise<void> {
+    if (!this.methods.includes(input.method)) {
+      throw new NotImplementedHttpError(
+        `Cannot determine permissions of ${input.method}, only ${this.methods.join(',')}.`,
+      );
+    }
+    await this.extractor.canHandle(input);
+  }
+
+  public async handle(input: Operation): Promise<Set<AccessMode>> {
+    return this.extractor.handle(input);
+  }
+}

src/index.ts

@@ -18,7 +18,8 @@ export * from './authorization/access/AgentGroupAccessChecker';
 export * from './authorization/permissions/Permissions';
 export * from './authorization/permissions/ModesExtractor';
 export * from './authorization/permissions/MethodModesExtractor';
-export * from './authorization/permissions/SparqlPatchModesExtractor';
+export * from './authorization/permissions/SparqlUpdateModesExtractor';
+export * from './authorization/permissions/WrappedMethodModesExtractor';
 
 // Authorization
 export * from './authorization/OwnerPermissionReader';
@@ -344,6 +345,7 @@ export * from './util/handlers/ConditionalHandler';
 export * from './util/handlers/ParallelHandler';
 export * from './util/handlers/SequenceHandler';
 export * from './util/handlers/StaticHandler';
+export * from './util/handlers/StaticThrowHandler';
 export * from './util/handlers/UnionHandler';
 export * from './util/handlers/UnsupportedAsyncHandler';
 export * from './util/handlers/WaterfallHandler';

src/util/handlers/StaticThrowHandler.ts

@@ -0,0 +1,18 @@
+import type { HttpError } from '../errors/HttpError';
+import { AsyncHandler } from './AsyncHandler';
+
+/**
+ * Utility handler that can handle all input and always throws the given error.
+ */
+export class StaticThrowHandler extends AsyncHandler<any, never> {
+  private readonly error: HttpError;
+
+  public constructor(error: HttpError) {
+    super();
+    this.error = error;
+  }
+
+  public async handle(): Promise<never> {
+    throw this.error;
+  }
+}

test/integration/LdpHandlerWithoutAuth.test.ts

@@ -386,4 +386,14 @@ describe.each(stores)('An LDP handler allowing all requests %s', (name, { storeC
     // DELETE
     expect(await deleteResource(documentUrl)).toBeUndefined();
   });
+
+  it('returns 405 for unsupported methods.', async(): Promise<void> => {
+    const response = await fetch(baseUrl, { method: 'TRACE' });
+    expect(response.status).toBe(405);
+  });
+
+  it('returns 415 for unsupported PATCH types.', async(): Promise<void> => {
+    const response = await fetch(baseUrl, { method: 'PATCH', headers: { 'content-type': 'text/plain' }, body: 'abc' });
+    expect(response.status).toBe(415);
+  });
 });

test/unit/authorization/permissions/SparqlPatchModesExtractor.test.ts → test/unit/authorization/permissions/SparqlUpdateModesExtractor.test.ts

@@ -1,25 +1,21 @@
 import { Factory } from 'sparqlalgebrajs';
 import { AccessMode } from '../../../../src/authorization/permissions/Permissions';
-import { SparqlPatchModesExtractor } from '../../../../src/authorization/permissions/SparqlPatchModesExtractor';
+import { SparqlUpdateModesExtractor } from '../../../../src/authorization/permissions/SparqlUpdateModesExtractor';
 import type { Operation } from '../../../../src/http/Operation';
 import type { SparqlUpdatePatch } from '../../../../src/http/representation/SparqlUpdatePatch';
 import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError';
 
-describe('A SparqlPatchModesExtractor', (): void => {
-  const extractor = new SparqlPatchModesExtractor();
+describe('A SparqlUpdateModesExtractor', (): void => {
+  const extractor = new SparqlUpdateModesExtractor();
   const factory = new Factory();
 
-  it('can only handle (composite) SPARQL DELETE/INSERT PATCH operations.', async(): Promise<void> => {
+  it('can only handle (composite) SPARQL DELETE/INSERT operations.', async(): Promise<void> => {
     const operation = { method: 'PATCH', body: { algebra: factory.createDeleteInsert() }} as unknown as Operation;
     await expect(extractor.canHandle(operation)).resolves.toBeUndefined();
     (operation.body as SparqlUpdatePatch).algebra = factory.createCompositeUpdate([ factory.createDeleteInsert() ]);
     await expect(extractor.canHandle(operation)).resolves.toBeUndefined();
 
-    let result = extractor.canHandle({ ...operation, method: 'GET' });
-    await expect(result).rejects.toThrow(NotImplementedHttpError);
-    await expect(result).rejects.toThrow('Cannot determine permissions of GET, only PATCH.');
-
-    result = extractor.canHandle({ ...operation, body: {} as SparqlUpdatePatch });
+    let result = extractor.canHandle({ ...operation, body: {} as SparqlUpdatePatch });
     await expect(result).rejects.toThrow(NotImplementedHttpError);
     await expect(result).rejects.toThrow('Cannot determine permissions of non-SPARQL patches.');
 

test/unit/authorization/permissions/WrappedMethodModesExtractor.test.ts

@@ -0,0 +1,53 @@
+import type { ModesExtractor } from '../../../../src/authorization/permissions/ModesExtractor';
+import { AccessMode } from '../../../../src/authorization/permissions/Permissions';
+import {
+  WrappedMethodModesExtractor,
+} from '../../../../src/authorization/permissions/WrappedMethodModesExtractor';
+import type { Operation } from '../../../../src/http/Operation';
+import { BasicRepresentation } from '../../../../src/http/representation/BasicRepresentation';
+import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError';
+
+describe('A WrappedMethodModesExtractor', (): void => {
+  const modes = [ 'PATCH', 'POST' ];
+  const result = new Set([ AccessMode.read ]);
+  let operation: Operation;
+  let source: jest.Mocked<ModesExtractor>;
+  let extractor: WrappedMethodModesExtractor;
+
+  beforeEach(async(): Promise<void> => {
+    operation = {
+      method: 'PATCH',
+      preferences: {},
+      permissionSet: {},
+      target: { path: 'http://example.com/foo' },
+      body: new BasicRepresentation(),
+    };
+
+    source = {
+      canHandle: jest.fn(),
+      handle: jest.fn().mockResolvedValue(result),
+    } as any;
+
+    extractor = new WrappedMethodModesExtractor(modes, source);
+  });
+
+  it('rejects unknown methods.', async(): Promise<void> => {
+    operation.method = 'GET';
+    await expect(extractor.canHandle(operation)).rejects.toThrow(NotImplementedHttpError);
+  });
+
+  it('checks if the source handle supports the request.', async(): Promise<void> => {
+    operation.method = 'PATCH';
+    await expect(extractor.canHandle(operation)).resolves.toBeUndefined();
+    operation.method = 'POST';
+    await expect(extractor.canHandle(operation)).resolves.toBeUndefined();
+    source.canHandle.mockRejectedValueOnce(new Error('not supported'));
+    await expect(extractor.canHandle(operation)).rejects.toThrow('not supported');
+    expect(source.canHandle).toHaveBeenLastCalledWith(operation);
+  });
+
+  it('calls the source extractor.', async(): Promise<void> => {
+    await expect(extractor.handle(operation)).resolves.toBe(result);
+    expect(source.handle).toHaveBeenLastCalledWith(operation);
+  });
+});

test/unit/util/handlers/StaticThrowHandler.test.ts

@@ -0,0 +1,15 @@
+import { BadRequestHttpError } from '../../../../src/util/errors/BadRequestHttpError';
+import { StaticThrowHandler } from '../../../../src/util/handlers/StaticThrowHandler';
+
+describe('A StaticThrowHandler', (): void => {
+  const error = new BadRequestHttpError();
+  const handler = new StaticThrowHandler(error);
+
+  it('can handle all requests.', async(): Promise<void> => {
+    await expect(handler.canHandle({})).resolves.toBeUndefined();
+  });
+
+  it('always throws the given error.', async(): Promise<void> => {
+    await expect(handler.handle()).rejects.toThrow(error);
+  });
+});