Repository: solid/community-server
Pull request #51 (2020-08-07 14:44:04)

Add more extensive permission parsing support
The SimplePermissionsExtractor now specifies which methods it supports and correctly matches them to permissions. Also renamed to BasePermissionsExtractor since it does what it's supposed to do (and I probably am too strict when I add Simple to class names).

Also added PermissionsExtractor for (SPARQL DELETE/INSERT) PATCH operations to determine if append or read is required.

Depends on #50 due to the permissions interface changing.


Patch:
diff --git a/bin/server.ts b/bin/server.ts
index 213e8e0db..6408ddb74 100644
--- a/bin/server.ts
+++ b/bin/server.ts
@@ -5,6 +5,7 @@ import yargs from 'yargs';
 import {
   AcceptPreferenceParser,
   AuthenticatedLdpHandler,
+  BasePermissionsExtractor,
   CompositeAsyncHandler,
   ExpressHttpServer,
   HttpRequest,
@@ -19,7 +20,6 @@ import {
   SimpleExtensionAclManager,
   SimpleGetOperationHandler,
   SimplePatchOperationHandler,
-  SimplePermissionsExtractor,
   SimplePostOperationHandler,
   SimplePutOperationHandler,
   SimpleRequestParser,
@@ -29,6 +29,7 @@ import {
   SimpleSparqlUpdatePatchHandler,
   SimpleTargetExtractor,
   SingleThreadedResourceLocker,
+  SparqlPatchPermissionsExtractor,
   TurtleToQuadConverter,
   UrlContainerManager,
 } from '..';
@@ -56,7 +57,10 @@ const requestParser = new SimpleRequestParser({
 });
 
 const credentialsExtractor = new SimpleCredentialsExtractor();
-const permissionsExtractor = new SimplePermissionsExtractor();
+const permissionsExtractor = new CompositeAsyncHandler([
+  new BasePermissionsExtractor(),
+  new SparqlPatchPermissionsExtractor(),
+]);
 
 // Will have to see how to best handle this
 const store = new SimpleResourceStore(base);
diff --git a/index.ts b/index.ts
index cbae5a621..a811b8483 100644
--- a/index.ts
+++ b/index.ts
@@ -38,7 +38,8 @@ export * from './src/ldp/operations/SimplePutOperationHandler';
 // LDP/Permissions
 export * from './src/ldp/permissions/PermissionSet';
 export * from './src/ldp/permissions/PermissionsExtractor';
-export * from './src/ldp/permissions/SimplePermissionsExtractor';
+export * from './src/ldp/permissions/BasePermissionsExtractor';
+export * from './src/ldp/permissions/SparqlPatchPermissionsExtractor';
 
 // LDP/Representation
 export * from './src/ldp/representation/BinaryRepresentation';
diff --git a/src/ldp/http/SparqlUpdatePatch.ts b/src/ldp/http/SparqlUpdatePatch.ts
index 1165df0b5..c3e50f4a5 100644
--- a/src/ldp/http/SparqlUpdatePatch.ts
+++ b/src/ldp/http/SparqlUpdatePatch.ts
@@ -1,5 +1,5 @@
+import { Algebra } from 'sparqlalgebrajs';
 import { Patch } from './Patch';
-import { Update } from 'sparqlalgebrajs/lib/algebra';
 
 /**
  * A specific type of {@link Patch} corresponding to a SPARQL update.
@@ -8,5 +8,5 @@ export interface SparqlUpdatePatch extends Patch {
   /**
    * Algebra corresponding to the SPARQL update.
    */
-  algebra: Update;
+  algebra: Algebra.Update;
 }
diff --git a/src/ldp/permissions/BasePermissionsExtractor.ts b/src/ldp/permissions/BasePermissionsExtractor.ts
new file mode 100644
index 000000000..e6fd0c53b
--- /dev/null
+++ b/src/ldp/permissions/BasePermissionsExtractor.ts
@@ -0,0 +1,32 @@
+import { Operation } from '../operations/Operation';
+import { PermissionSet } from './PermissionSet';
+import { PermissionsExtractor } from './PermissionsExtractor';
+import { UnsupportedHttpError } from '../../util/errors/UnsupportedHttpError';
+
+/**
+ * Generates permissions for the base set of methods that always require the same permissions.
+ * Specifically: GET, HEAD, POST, PUT and DELETE.
+ */
+export class BasePermissionsExtractor extends PermissionsExtractor {
+  public async canHandle(input: Operation): Promise<void> {
+    if (!/^(?:HEAD|GET|POST|PUT|DELETE)$/u.test(input.method)) {
+      throw new UnsupportedHttpError(`Unsupported method: ${input.method}`);
+    }
+  }
+
+  public async handle(input: Operation): Promise<PermissionSet> {
+    const requiredPermissions = {
+      read: /^(?:HEAD|GET)$/u.test(input.method),
+      append: false,
+      write: /^(?:POST|PUT|DELETE)$/u.test(input.method),
+    };
+
+    const read = /^(?:HEAD|GET)$/u.test(input.method);
+    const write = /^(?:POST|PUT|DELETE)$/u.test(input.method);
+
+    // Since `append` is a specific type of write, it is true if `write` is true.
+    const append = requiredPermissions.write;
+
+    return { read, append, write };
+  }
+}
diff --git a/src/ldp/permissions/SimplePermissionsExtractor.ts b/src/ldp/permissions/SimplePermissionsExtractor.ts
deleted file mode 100644
index 34b0b1b7e..000000000
--- a/src/ldp/permissions/SimplePermissionsExtractor.ts
+++ /dev/null
@@ -1,22 +0,0 @@
-import { Operation } from '../operations/Operation';
-import { PermissionSet } from './PermissionSet';
-import { PermissionsExtractor } from './PermissionsExtractor';
-
-/**
- * Makes some simplified assumption based on the {@link Operation} method to generate a {@link PermissionSet}.
- */
-export class SimplePermissionsExtractor extends PermissionsExtractor {
-  public async canHandle(): Promise<void> {
-    // Supports all operations
-  }
-
-  public async handle(input: Operation): Promise<PermissionSet> {
-    const result = {
-      read: input.method === 'GET',
-      append: false,
-      write: input.method === 'POST' || input.method === 'PUT' || input.method === 'DELETE',
-    };
-    result.append = result.write;
-    return result;
-  }
-}
diff --git a/src/ldp/permissions/SparqlPatchPermissionsExtractor.ts b/src/ldp/permissions/SparqlPatchPermissionsExtractor.ts
new file mode 100644
index 000000000..da0aaeeb1
--- /dev/null
+++ b/src/ldp/permissions/SparqlPatchPermissionsExtractor.ts
@@ -0,0 +1,58 @@
+import { Algebra } from 'sparqlalgebrajs';
+import { Operation } from '../operations/Operation';
+import { PermissionSet } from './PermissionSet';
+import { PermissionsExtractor } from './PermissionsExtractor';
+import { Representation } from '../representation/Representation';
+import { SparqlUpdatePatch } from '../http/SparqlUpdatePatch';
+import { UnsupportedHttpError } from '../../util/errors/UnsupportedHttpError';
+
+/**
+ * Generates permissions for a SPARQL DELETE/INSERT patch.
+ * Updates with only an INSERT can be done with just append permissions,
+ * while DELETEs require write permissions as well.
+ */
+export class SparqlPatchPermissionsExtractor extends PermissionsExtractor {
+  public async canHandle(input: Operation): Promise<void> {
+    if (input.method !== 'PATCH') {
+      throw new UnsupportedHttpError('Only PATCH operations are supported.');
+    }
+    if (!input.body) {
+      throw new UnsupportedHttpError('PATCH body is required to determine permissions.');
+    }
+    if (!this.isSparql(input.body)) {
+      throw new UnsupportedHttpError('Only SPARQL update PATCHes are supported.');
+    }
+    if (!this.isDeleteInsert(input.body.algebra)) {
+      throw new UnsupportedHttpError('Only DELETE/INSERT SPARQL update operations are supported.');
+    }
+  }
+
+  public async handle(input: Operation): Promise<PermissionSet> {
+    // Verified in `canHandle` call
+    const op = (input.body as SparqlUpdatePatch).algebra as Algebra.DeleteInsert;
+
+    const read = false;
+    const write = this.needsWrite(op);
+
+    // Since `append` is a specific type of write, it is true if `write` is true.
+    const append = write || this.needsAppend(op);
+
+    return { read, write, append };
+  }
+
+  private isSparql(data: Representation): data is SparqlUpdatePatch {
+    return Boolean((data as SparqlUpdatePatch).algebra);
+  }
+
+  private isDeleteInsert(op: Algebra.Operation): op is Algebra.DeleteInsert {
+    return op.type === Algebra.types.DELETE_INSERT;
+  }
+
+  private needsAppend(update: Algebra.DeleteInsert): boolean {
+    return Boolean(update.insert && update.insert.length > 0);
+  }
+
+  private needsWrite(update: Algebra.DeleteInsert): boolean {
+    return Boolean(update.delete && update.delete.length > 0);
+  }
+}


Test patch:
diff --git a/test/integration/AuthenticatedLdpHandler.test.ts b/test/integration/AuthenticatedLdpHandler.test.ts
index 04e527def..d5434ad9e 100644
--- a/test/integration/AuthenticatedLdpHandler.test.ts
+++ b/test/integration/AuthenticatedLdpHandler.test.ts
@@ -1,5 +1,6 @@
 import { AcceptPreferenceParser } from '../../src/ldp/http/AcceptPreferenceParser';
 import { AuthenticatedLdpHandler } from '../../src/ldp/AuthenticatedLdpHandler';
+import { BasePermissionsExtractor } from '../../src/ldp/permissions/BasePermissionsExtractor';
 import { BodyParser } from '../../src/ldp/http/BodyParser';
 import { call } from '../util/Util';
 import { CompositeAsyncHandler } from '../../src/util/CompositeAsyncHandler';
@@ -18,7 +19,6 @@ import { SimpleCredentialsExtractor } from '../../src/authentication/SimpleCrede
 import { SimpleDeleteOperationHandler } from '../../src/ldp/operations/SimpleDeleteOperationHandler';
 import { SimpleGetOperationHandler } from '../../src/ldp/operations/SimpleGetOperationHandler';
 import { SimplePatchOperationHandler } from '../../src/ldp/operations/SimplePatchOperationHandler';
-import { SimplePermissionsExtractor } from '../../src/ldp/permissions/SimplePermissionsExtractor';
 import { SimplePostOperationHandler } from '../../src/ldp/operations/SimplePostOperationHandler';
 import { SimpleRequestParser } from '../../src/ldp/http/SimpleRequestParser';
 import { SimpleResourceStore } from '../../src/storage/SimpleResourceStore';
@@ -27,6 +27,7 @@ import { SimpleSparqlUpdateBodyParser } from '../../src/ldp/http/SimpleSparqlUpd
 import { SimpleSparqlUpdatePatchHandler } from '../../src/storage/patch/SimpleSparqlUpdatePatchHandler';
 import { SimpleTargetExtractor } from '../../src/ldp/http/SimpleTargetExtractor';
 import { SingleThreadedResourceLocker } from '../../src/storage/SingleThreadedResourceLocker';
+import { SparqlPatchPermissionsExtractor } from '../../src/ldp/permissions/SparqlPatchPermissionsExtractor';
 import { TurtleToQuadConverter } from '../../src/storage/conversion/TurtleToQuadConverter';
 import { namedNode, quad } from '@rdfjs/data-model';
 import * as url from 'url';
@@ -40,7 +41,7 @@ describe('An integrated AuthenticatedLdpHandler', (): void => {
     });
 
     const credentialsExtractor = new SimpleCredentialsExtractor();
-    const permissionsExtractor = new SimplePermissionsExtractor();
+    const permissionsExtractor = new BasePermissionsExtractor();
     const authorizer = new SimpleAuthorizer();
 
     const store = new SimpleResourceStore('http://test.com/');
@@ -108,7 +109,10 @@ describe('An integrated AuthenticatedLdpHandler', (): void => {
     });
 
     const credentialsExtractor = new SimpleCredentialsExtractor();
-    const permissionsExtractor = new SimplePermissionsExtractor();
+    const permissionsExtractor = new CompositeAsyncHandler([
+      new BasePermissionsExtractor(),
+      new SparqlPatchPermissionsExtractor(),
+    ]);
     const authorizer = new SimpleAuthorizer();
 
     const store = new SimpleResourceStore('http://test.com/');
diff --git a/test/integration/Authorization.test.ts b/test/integration/Authorization.test.ts
index 8cfec2e89..d47df48a5 100644
--- a/test/integration/Authorization.test.ts
+++ b/test/integration/Authorization.test.ts
@@ -1,5 +1,6 @@
 import { AcceptPreferenceParser } from '../../src/ldp/http/AcceptPreferenceParser';
 import { AuthenticatedLdpHandler } from '../../src/ldp/AuthenticatedLdpHandler';
+import { BasePermissionsExtractor } from '../../src/ldp/permissions/BasePermissionsExtractor';
 import { BodyParser } from '../../src/ldp/http/BodyParser';
 import { call } from '../util/Util';
 import { CompositeAsyncHandler } from '../../src/util/CompositeAsyncHandler';
@@ -17,7 +18,6 @@ import { SimpleCredentialsExtractor } from '../../src/authentication/SimpleCrede
 import { SimpleDeleteOperationHandler } from '../../src/ldp/operations/SimpleDeleteOperationHandler';
 import { SimpleExtensionAclManager } from '../../src/authorization/SimpleExtensionAclManager';
 import { SimpleGetOperationHandler } from '../../src/ldp/operations/SimpleGetOperationHandler';
-import { SimplePermissionsExtractor } from '../../src/ldp/permissions/SimplePermissionsExtractor';
 import { SimplePostOperationHandler } from '../../src/ldp/operations/SimplePostOperationHandler';
 import { SimplePutOperationHandler } from '../../src/ldp/operations/SimplePutOperationHandler';
 import { SimpleRequestParser } from '../../src/ldp/http/SimpleRequestParser';
@@ -88,7 +88,7 @@ describe('A server with authorization', (): void => {
   const convertingStore = new RepresentationConvertingStore(store, converter);
 
   const credentialsExtractor = new SimpleCredentialsExtractor();
-  const permissionsExtractor = new SimplePermissionsExtractor();
+  const permissionsExtractor = new BasePermissionsExtractor();
   const authorizer = new SimpleAclAuthorizer(
     new SimpleExtensionAclManager(),
     new UrlContainerManager('http://test.com/'),
diff --git a/test/unit/ldp/permissions/BasePermissionsExtractor.test.ts b/test/unit/ldp/permissions/BasePermissionsExtractor.test.ts
new file mode 100644
index 000000000..6a509bb00
--- /dev/null
+++ b/test/unit/ldp/permissions/BasePermissionsExtractor.test.ts
@@ -0,0 +1,56 @@
+import { BasePermissionsExtractor } from '../../../../src/ldp/permissions/BasePermissionsExtractor';
+import { Operation } from '../../../../src/ldp/operations/Operation';
+import { UnsupportedHttpError } from '../../../../src/util/errors/UnsupportedHttpError';
+
+describe('A BasePermissionsExtractor', (): void => {
+  const extractor = new BasePermissionsExtractor();
+
+  it('can handle HEAD/GET/POST/PUT/DELETE.', async(): Promise<void> => {
+    await expect(extractor.canHandle({ method: 'HEAD' } as Operation)).resolves.toBeUndefined();
+    await expect(extractor.canHandle({ method: 'GET' } as Operation)).resolves.toBeUndefined();
+    await expect(extractor.canHandle({ method: 'POST' } as Operation)).resolves.toBeUndefined();
+    await expect(extractor.canHandle({ method: 'PUT' } as Operation)).resolves.toBeUndefined();
+    await expect(extractor.canHandle({ method: 'DELETE' } as Operation)).resolves.toBeUndefined();
+    await expect(extractor.canHandle({ method: 'PATCH' } as Operation)).rejects.toThrow(UnsupportedHttpError);
+  });
+
+  it('requires read for HEAD operations.', async(): Promise<void> => {
+    await expect(extractor.handle({ method: 'HEAD' } as Operation)).resolves.toEqual({
+      read: true,
+      append: false,
+      write: false,
+    });
+  });
+
+  it('requires read for GET operations.', async(): Promise<void> => {
+    await expect(extractor.handle({ method: 'GET' } as Operation)).resolves.toEqual({
+      read: true,
+      append: false,
+      write: false,
+    });
+  });
+
+  it('requires write for POST operations.', async(): Promise<void> => {
+    await expect(extractor.handle({ method: 'POST' } as Operation)).resolves.toEqual({
+      read: false,
+      append: true,
+      write: true,
+    });
+  });
+
+  it('requires write for PUT operations.', async(): Promise<void> => {
+    await expect(extractor.handle({ method: 'PUT' } as Operation)).resolves.toEqual({
+      read: false,
+      append: true,
+      write: true,
+    });
+  });
+
+  it('requires write for DELETE operations.', async(): Promise<void> => {
+    await expect(extractor.handle({ method: 'DELETE' } as Operation)).resolves.toEqual({
+      read: false,
+      append: true,
+      write: true,
+    });
+  });
+});
diff --git a/test/unit/ldp/permissions/SimplePermissionsExtractor.test.ts b/test/unit/ldp/permissions/SimplePermissionsExtractor.test.ts
deleted file mode 100644
index 637f4a6a9..000000000
--- a/test/unit/ldp/permissions/SimplePermissionsExtractor.test.ts
+++ /dev/null
@@ -1,42 +0,0 @@
-import { Operation } from '../../../../src/ldp/operations/Operation';
-import { SimplePermissionsExtractor } from '../../../../src/ldp/permissions/SimplePermissionsExtractor';
-
-describe('A SimplePermissionsExtractor', (): void => {
-  const extractor = new SimplePermissionsExtractor();
-
-  it('can handle all input.', async(): Promise<void> => {
-    await expect(extractor.canHandle()).resolves.toBeUndefined();
-  });
-
-  it('requires read for GET operations.', async(): Promise<void> => {
-    await expect(extractor.handle({ method: 'GET' } as Operation)).resolves.toEqual({
-      read: true,
-      append: false,
-      write: false,
-    });
-  });
-
-  it('requires write for POST operations.', async(): Promise<void> => {
-    await expect(extractor.handle({ method: 'POST' } as Operation)).resolves.toEqual({
-      read: false,
-      append: true,
-      write: true,
-    });
-  });
-
-  it('requires write for PUT operations.', async(): Promise<void> => {
-    await expect(extractor.handle({ method: 'PUT' } as Operation)).resolves.toEqual({
-      read: false,
-      append: true,
-      write: true,
-    });
-  });
-
-  it('requires write for DELETE operations.', async(): Promise<void> => {
-    await expect(extractor.handle({ method: 'DELETE' } as Operation)).resolves.toEqual({
-      read: false,
-      append: true,
-      write: true,
-    });
-  });
-});
diff --git a/test/unit/ldp/permissions/SparqlPatchPermissionsExtractor.test.ts b/test/unit/ldp/permissions/SparqlPatchPermissionsExtractor.test.ts
new file mode 100644
index 000000000..3a37ddd90
--- /dev/null
+++ b/test/unit/ldp/permissions/SparqlPatchPermissionsExtractor.test.ts
@@ -0,0 +1,52 @@
+import { Factory } from 'sparqlalgebrajs';
+import { Operation } from '../../../../src/ldp/operations/Operation';
+import { SparqlPatchPermissionsExtractor } from '../../../../src/ldp/permissions/SparqlPatchPermissionsExtractor';
+import { SparqlUpdatePatch } from '../../../../src/ldp/http/SparqlUpdatePatch';
+import { UnsupportedHttpError } from '../../../../src/util/errors/UnsupportedHttpError';
+
+describe('A SparqlPatchPermissionsExtractor', (): void => {
+  const extractor = new SparqlPatchPermissionsExtractor();
+  const factory = new Factory();
+
+  it('can only handle SPARQL DELETE/INSERT PATCH operations.', async(): Promise<void> => {
+    const operation = { method: 'PATCH', body: { algebra: factory.createDeleteInsert() }} as unknown as Operation;
+    await expect(extractor.canHandle(operation)).resolves.toBeUndefined();
+    await expect(extractor.canHandle({ ...operation, method: 'GET' }))
+      .rejects.toThrow(new UnsupportedHttpError('Only PATCH operations are supported.'));
+    await expect(extractor.canHandle({ ...operation, body: undefined }))
+      .rejects.toThrow(new UnsupportedHttpError('PATCH body is required to determine permissions.'));
+    await expect(extractor.canHandle({ ...operation, body: {} as SparqlUpdatePatch }))
+      .rejects.toThrow(new UnsupportedHttpError('Only SPARQL update PATCHes are supported.'));
+    await expect(extractor.canHandle({ ...operation,
+      body: { algebra: factory.createMove('DEFAULT', 'DEFAULT') } as unknown as SparqlUpdatePatch }))
+      .rejects.toThrow(new UnsupportedHttpError('Only DELETE/INSERT SPARQL update operations are supported.'));
+  });
+
+  it('requires append for INSERT operations.', async(): Promise<void> => {
+    const operation = {
+      method: 'PATCH',
+      body: { algebra: factory.createDeleteInsert(undefined, [
+        factory.createPattern(factory.createTerm('
'), factory.createTerm('<p>'), factory.createTerm('<o>')),
+      ]) },
+    } as unknown as Operation;
+    await expect(extractor.handle(operation)).resolves.toEqual({
+      read: false,
+      append: true,
+      write: false,
+    });
+  });
+
+  it('requires write for DELETE operations.', async(): Promise<void> => {
+    const operation = {
+      method: 'PATCH',
+      body: { algebra: factory.createDeleteInsert([
+        factory.createPattern(factory.createTerm('
'), factory.createTerm('<p>'), factory.createTerm('<o>')),
+      ]) },
+    } as unknown as Operation;
+    await expect(extractor.handle(operation)).resolves.toEqual({
+      read: false,
+      append: true,
+      write: true,
+    });
+  });
+});