Repository: solid/community-server (ts)

Issue:
Prevent deletion of pim:Storage containers
We currently have a check to prevent deletion of the root container. As mentioned in https://github.com/solid/community-server/issues/381#issuecomment-737277297 we should instead check if the container is a `pim:Storage` container instead.

We probably also want to update our pod provision to make sure the root of the pod is set as such a storage then.

Pull request:
fix: Prevent deletion of root storage containers
Depends on #424 .

Closes #411 .

Was going to also update the pod provisioning so they count as storage, but #341 should be resolved first so we have a cleaner way to handle metadata.

Patch:
diff --git a/src/storage/DataAccessorBasedStore.ts b/src/storage/DataAccessorBasedStore.ts
index cad36e803..b6e29fa80 100644
--- a/src/storage/DataAccessorBasedStore.ts
+++ b/src/storage/DataAccessorBasedStore.ts
@@ -22,7 +22,7 @@ import {
 import { parseQuads } from '../util/QuadUtil';
 import { generateResourceQuads } from '../util/ResourceUtil';
 import { guardedStreamFrom } from '../util/StreamUtil';
-import { CONTENT_TYPE, HTTP, LDP, RDF } from '../util/UriConstants';
+import { CONTENT_TYPE, HTTP, LDP, PIM, RDF } from '../util/UriConstants';
 import type { DataAccessor } from './accessors/DataAccessor';
 import type { ResourceStore } from './ResourceStore';
 
@@ -148,10 +148,13 @@ export class DataAccessorBasedStore implements ResourceStore {
 
   public async deleteResource(identifier: ResourceIdentifier): Promise<void> {
     this.validateIdentifier(identifier);
-    if (this.identifierStrategy.isRootContainer(identifier)) {
-      throw new MethodNotAllowedHttpError('Cannot delete root container.');
-    }
     const metadata = await this.accessor.getMetadata(identifier);
+    // "When a DELETE request targets storage’s root container or its associated ACL resource,
+    //  the server MUST respond with the 405 status code."
+    // https://solid.github.io/specification/#deleting-resources
+    if (this.isRootStorage(metadata)) {
+      throw new MethodNotAllowedHttpError('Cannot delete a root storage container.');
+    }
     if (metadata.getAll(LDP.contains).length > 0) {
       throw new ConflictHttpError('Can only delete empty containers.');
     }
@@ -348,6 +351,13 @@ export class DataAccessorBasedStore implements ResourceStore {
     return types.some((type): boolean => type.value === LDP.Container || type.value === LDP.BasicContainer);
   }
 
+  /**
+   * Verifies if this is the metadata of a root storage container.
+   */
+  protected isRootStorage(metadata: RepresentationMetadata): boolean {
+    return metadata.getAll(RDF.type).some((term): boolean => term.value === PIM.Storage);
+  }
+
   /**
    * Create containers starting from the root until the given identifier corresponds to an existing container.
    * Will throw errors if the identifier of the last existing "container" corresponds to an existing document.


Test patch:
diff --git a/test/unit/storage/DataAccessorBasedStore.test.ts b/test/unit/storage/DataAccessorBasedStore.test.ts
index 560f092d8..d6f358a91 100644
--- a/test/unit/storage/DataAccessorBasedStore.test.ts
+++ b/test/unit/storage/DataAccessorBasedStore.test.ts
@@ -18,7 +18,7 @@ import type { Guarded } from '../../../src/util/GuardedStream';
 import { SingleRootIdentifierStrategy } from '../../../src/util/identifiers/SingleRootIdentifierStrategy';
 import * as quadUtil from '../../../src/util/QuadUtil';
 import { guardedStreamFrom } from '../../../src/util/StreamUtil';
-import { CONTENT_TYPE, HTTP, LDP, RDF } from '../../../src/util/UriConstants';
+import { CONTENT_TYPE, HTTP, LDP, PIM, RDF } from '../../../src/util/UriConstants';
 import { toNamedNode } from '../../../src/util/UriUtil';
 import quad = DataFactory.quad;
 import namedNode = DataFactory.namedNode;
@@ -389,9 +389,11 @@ describe('A DataAccessorBasedStore', (): void => {
         .rejects.toThrow(NotFoundHttpError);
     });
 
-    it('will error when deleting the root.', async(): Promise<void> => {
-      await expect(store.deleteResource({ path: root }))
-        .rejects.toThrow(new MethodNotAllowedHttpError('Cannot delete root container.'));
+    it('will error when deleting a root storage container.', async(): Promise<void> => {
+      representation.metadata.add(RDF.type, toNamedNode(PIM.Storage));
+      accessor.data[`${root}container`] = representation;
+      await expect(store.deleteResource({ path: `${root}container` }))
+        .rejects.toThrow(new MethodNotAllowedHttpError('Cannot delete a root storage container.'));
     });
 
     it('will error when deleting non-empty containers.', async(): Promise<void> => {