fix: Require create permission for empty PATCH bodies

config/ldp/modes/default.json

@@ -37,6 +37,9 @@
     },
     {
       "@id": "urn:solid-server:default:PatchModesExtractor",
+      "@type": "CreateModesExtractor",
+      "resourceSet": { "@id": "urn:solid-server:default:CachedResourceSet" },
+      "source": {
         "@type": "MethodFilterHandler",
         "methods": [ "PATCH" ],
         "source": {
@@ -57,5 +60,6 @@
           ]
         }
       }
+    }
   ]
 }

src/authorization/permissions/CreateModesExtractor.ts

@@ -0,0 +1,34 @@
+import type { Operation } from '../../http/Operation';
+import type { ResourceSet } from '../../storage/ResourceSet';
+import { ModesExtractor } from './ModesExtractor';
+import type { AccessMap } from './Permissions';
+import { AccessMode } from './Permissions';
+
+/**
+ * Adds the `create` access mode to the result of the source in case the target resource does not exist.
+ */
+export class CreateModesExtractor extends ModesExtractor {
+  private readonly source: ModesExtractor;
+  private readonly resourceSet: ResourceSet;
+
+  public constructor(source: ModesExtractor, resourceSet: ResourceSet) {
+    super();
+    this.source = source;
+    this.resourceSet = resourceSet;
+  }
+
+  public async canHandle(operation: Operation): Promise<void> {
+    await this.source.canHandle(operation);
+  }
+
+  public async handle(operation: Operation): Promise<AccessMap> {
+    const accessMap = await this.source.handle(operation);
+
+    if (!accessMap.hasEntry(operation.target, AccessMode.create) &&
+      !await this.resourceSet.hasResource(operation.target)) {
+      accessMap.add(operation.target, AccessMode.create);
+    }
+
+    return accessMap;
+  }
+}

src/index.ts

@@ -16,6 +16,7 @@ export * from './authorization/access/AgentGroupAccessChecker';
 
 // Authorization/Permissions
 export * from './authorization/permissions/AclPermission';
+export * from './authorization/permissions/CreateModesExtractor';
 export * from './authorization/permissions/DeleteParentExtractor';
 export * from './authorization/permissions/IntermediateCreateExtractor';
 export * from './authorization/permissions/ModesExtractor';

test/integration/PermissionTable.test.ts

@@ -85,17 +85,20 @@ const table: [string, string, AM[], AM[] | undefined, string, string, number, nu
   [ 'PUT',     'C/R', [ AM.write ],           undefined,              '',     TXT, 205, 201 ],
   [ 'PUT',     'C/R', [ AM.append ],          [ AM.write ],           '',     TXT, 205, 201 ],
 
+  // All PATCH operations with read permissions return 401 instead of 404 if the target does not exist.
+  // This is a consequence of PATCH always creating a resource in case it does not exist.
+  // https://solidproject.org/TR/2021/protocol-20211217#n3-patch
+  // "Start from the RDF dataset in the target document,
+  // or an empty RDF dataset if the target resource does not exist yet."
   [ 'PATCH',   'C/R', [],                     undefined,              DELETE, N3,  401, 401 ],
-  [ 'PATCH',   'C/R', [],                     [ AM.read ],            DELETE, N3,  401, 404 ],
+  [ 'PATCH',   'C/R', [],                     [ AM.read ],            DELETE, N3,  401, 401 ],
   [ 'PATCH',   'C/R', [],                     [ AM.append ],          INSERT, N3,  205, 401 ],
   [ 'PATCH',   'C/R', [],                     [ AM.append ],          DELETE, N3,  401, 401 ],
   [ 'PATCH',   'C/R', [],                     [ AM.write ],           INSERT, N3,  205, 401 ],
   [ 'PATCH',   'C/R', [],                     [ AM.write ],           DELETE, N3,  401, 401 ],
   [ 'PATCH',   'C/R', [ AM.append ],          [ AM.write ],           INSERT, N3,  205, 201 ],
   [ 'PATCH',   'C/R', [ AM.append ],          [ AM.write ],           DELETE, N3,  401, 401 ],
-  // We currently return 409 instead of 404 in case a PATCH has no inserts and C/R does not exist.
-  // This is an agreed upon deviation from the original table
-  [ 'PATCH',   'C/R', [],                     [ AM.read, AM.write ],  DELETE, N3,  205, 409 ],
+  [ 'PATCH',   'C/R', [],                     [ AM.read, AM.write ],  DELETE, N3,  205, 401 ],
 
   [ 'DELETE',  'C/R', [],                     undefined,              '',     '',  401, 401 ],
   [ 'DELETE',  'C/R', [],                     [ AM.read ],            '',     '',  401, 404 ],

test/unit/authorization/permissions/CreateModesExtractor.test.ts

@@ -0,0 +1,59 @@
+import { CreateModesExtractor } from '../../../../src/authorization/permissions/CreateModesExtractor';
+import type { ModesExtractor } from '../../../../src/authorization/permissions/ModesExtractor';
+import type { AccessMap } from '../../../../src/authorization/permissions/Permissions';
+import { AccessMode } from '../../../../src/authorization/permissions/Permissions';
+import type { Operation } from '../../../../src/http/Operation';
+import { BasicRepresentation } from '../../../../src/http/representation/BasicRepresentation';
+import type { ResourceIdentifier } from '../../../../src/http/representation/ResourceIdentifier';
+import type { ResourceSet } from '../../../../src/storage/ResourceSet';
+import { IdentifierSetMultiMap } from '../../../../src/util/map/IdentifierMap';
+import { compareMaps } from '../../../util/Util';
+
+describe('A CreateModesExtractor', (): void => {
+  const target: ResourceIdentifier = { path: 'http://example.com/foo' };
+  let operation: Operation;
+  let result: AccessMap;
+  let resourceSet: jest.Mocked<ResourceSet>;
+  let source: jest.Mocked<ModesExtractor>;
+  let extractor: CreateModesExtractor;
+
+  beforeEach(async(): Promise<void> => {
+    operation = {
+      method: 'PATCH',
+      target,
+      body: new BasicRepresentation(),
+      preferences: {},
+    };
+
+    result = new IdentifierSetMultiMap<AccessMode>([[ target, AccessMode.read ]]);
+
+    resourceSet = {
+      hasResource: jest.fn().mockResolvedValue(true),
+    };
+
+    source = {
+      canHandle: jest.fn(),
+      handle: jest.fn().mockResolvedValue(result),
+    } as any;
+
+    extractor = new CreateModesExtractor(source, resourceSet);
+  });
+
+  it('checks if the source can handle the input.', async(): Promise<void> => {
+    await expect(extractor.canHandle(operation)).resolves.toBeUndefined();
+
+    source.canHandle.mockRejectedValue(new Error('bad data'));
+    await expect(extractor.canHandle(operation)).rejects.toThrow('bad data');
+  });
+
+  it('does nothing if the resource exists.', async(): Promise<void> => {
+    await expect(extractor.handle(operation)).resolves.toBe(result);
+    compareMaps(result, new IdentifierSetMultiMap([[ target, AccessMode.read ]]));
+  });
+
+  it('adds the create mode if the resource does not exist.', async(): Promise<void> => {
+    resourceSet.hasResource.mockResolvedValue(false);
+    await expect(extractor.handle(operation)).resolves.toBe(result);
+    compareMaps(result, new IdentifierSetMultiMap([[ target, AccessMode.read ], [ target, AccessMode.create ]]));
+  });
+});