Repository: solid/community-server (ts)

Issue:
Hide internal storage data
For internal data storage we use a `KeyValueStore` in several cases. One of the implementations for that stores the data in a specific container in the ResourceStore. It is important that this container, or any data in it, can not be accessed by doing a GET request to the location of that container.

The IDP components currently use a key/value store that stores its data in `/idp/data/`. Since all requests to `/idp/*` get redirected to the main IDP handler that data is safe since that request is simply going to error (fortunately, since that's where the oidc keys are stored), but we should have a general solution for this.

One solution is to always put a `.acl` file in such a container that allows no access at all, but that ties the implementation to the authorization that is being used. Another solution would be to put create a new HttpHandler that always returns 401/404 when trying to access those containers (or put them in the `StaticAssetHandler`).

Pull request:
Prevent access to containers used for internal storage
Closes #757. Uses the solution described there.

Patch:
diff --git a/config/README.md b/config/README.md
index 468b4c8d7..6dca90572 100644
--- a/config/README.md
+++ b/config/README.md
@@ -11,7 +11,7 @@ it is always possible to not choose any of them and create your own custom versi
 # How to use
 The easiest way to create a new config is by creating a JSON-LD file
 that imports one option from every component subfolder 
-(such as either `allow-everything.json` or `webacl.json` from `ldp/authorization`).
+(such as either `allow-all.json` or `webacl.json` from `ldp/authorization`).
 In case none of the available options suffice, there are 2 other ways to handle this:
 
 ## Append to an existing config
diff --git a/config/default.json b/config/default.json
index 6a213cd47..ad353cdf5 100644
--- a/config/default.json
+++ b/config/default.json
@@ -19,7 +19,7 @@
     "files-scs:config/ldp/metadata-writer/default.json",
     "files-scs:config/ldp/permissions/acl.json",
     "files-scs:config/storage/backend/memory.json",
-    "files-scs:config/storage/key-value/memory.json",
+    "files-scs:config/storage/key-value/resource-store.json",
     "files-scs:config/storage/middleware/default.json",
     "files-scs:config/util/auxiliary/acl.json",
     "files-scs:config/util/identifiers/suffix.json",
diff --git a/config/dynamic.json b/config/dynamic.json
index eb1429a15..bdcd5e865 100644
--- a/config/dynamic.json
+++ b/config/dynamic.json
@@ -19,7 +19,7 @@
     "files-scs:config/ldp/metadata-writer/default.json",
     "files-scs:config/ldp/permissions/acl.json",
     "files-scs:config/storage/backend/dynamic.json",
-    "files-scs:config/storage/key-value/memory.json",
+    "files-scs:config/storage/key-value/resource-store.json",
     "files-scs:config/storage/middleware/default.json",
     "files-scs:config/util/auxiliary/acl.json",
     "files-scs:config/util/identifiers/suffix.json",
diff --git a/config/example-https-file.json b/config/example-https-file.json
index bb913f216..00ecbf0ff 100644
--- a/config/example-https-file.json
+++ b/config/example-https-file.json
@@ -19,7 +19,7 @@
     "files-scs:config/ldp/metadata-writer/default.json",
     "files-scs:config/ldp/permissions/acl.json",
     "files-scs:config/storage/backend/file.json",
-    "files-scs:config/storage/key-value/memory.json",
+    "files-scs:config/storage/key-value/resource-store.json",
     "files-scs:config/storage/middleware/default.json",
     "files-scs:config/util/auxiliary/acl.json",
     "files-scs:config/util/identifiers/suffix.json",
diff --git a/config/file.json b/config/file.json
index befd58868..d64336f23 100644
--- a/config/file.json
+++ b/config/file.json
@@ -19,7 +19,7 @@
     "files-scs:config/ldp/metadata-writer/default.json",
     "files-scs:config/ldp/permissions/acl.json",
     "files-scs:config/storage/backend/file.json",
-    "files-scs:config/storage/key-value/memory.json",
+    "files-scs:config/storage/key-value/resource-store.json",
     "files-scs:config/storage/middleware/default.json",
     "files-scs:config/util/auxiliary/acl.json",
     "files-scs:config/util/identifiers/suffix.json",
diff --git a/config/ldp/README.md b/config/ldp/README.md
index 561fca860..9642a64b4 100644
--- a/config/ldp/README.md
+++ b/config/ldp/README.md
@@ -11,7 +11,7 @@ Covers how agents are identified.
 
 ## Authorization
 Covers how operations are authorized (or rejected).
-* *allow-everything*: No authorization, everything is allowed.
+* *allow-all*: No authorization, everything is allowed.
 * *webacl*: Use the default Web Access Control.
 
 ## Handler
diff --git a/config/ldp/authorization/allow-everything.json b/config/ldp/authorization/allow-all.json
similarity index 89%
rename from config/ldp/authorization/allow-everything.json
rename to config/ldp/authorization/allow-all.json
index a6dfb8e0a..141b64894 100644
--- a/config/ldp/authorization/allow-everything.json
+++ b/config/ldp/authorization/allow-all.json
@@ -7,7 +7,7 @@
         "Always allows all operations."
       ],
       "@id": "urn:solid-server:default:Authorizer",
-      "@type": "AllowEverythingAuthorizer"
+      "@type": "AllowAllAuthorizer"
     }
   ]
 }
diff --git a/config/ldp/authorization/webacl.json b/config/ldp/authorization/webacl.json
index 6026d8cfa..655481116 100644
--- a/config/ldp/authorization/webacl.json
+++ b/config/ldp/authorization/webacl.json
@@ -9,6 +9,12 @@
       "@id": "urn:solid-server:default:Authorizer",
       "@type": "WaterfallHandler",
       "handlers": [
+        {
+          "comment": "This authorizer will be used to prevent external access to containers used for internal storage.",
+          "@id": "urn:solid-server:default:PathBasedAuthorizer",
+          "@type": "PathBasedAuthorizer",
+          "baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" }
+        },
         {
           "comment": "This authorizer makes sure that for auxiliary resources, the main authorizer gets called with the associated identifier.",
           "@type": "AuxiliaryAuthorizer",
diff --git a/config/memory-subdomains.json b/config/memory-subdomains.json
index 2a495e03d..b1f35cab5 100644
--- a/config/memory-subdomains.json
+++ b/config/memory-subdomains.json
@@ -19,7 +19,7 @@
     "files-scs:config/ldp/metadata-writer/default.json",
     "files-scs:config/ldp/permissions/acl.json",
     "files-scs:config/storage/backend/memory.json",
-    "files-scs:config/storage/key-value/memory.json",
+    "files-scs:config/storage/key-value/resource-store.json",
     "files-scs:config/storage/middleware/default.json",
     "files-scs:config/util/auxiliary/acl.json",
     "files-scs:config/util/identifiers/subdomain.json",
diff --git a/config/path-routing.json b/config/path-routing.json
index 67e3df917..1da9fc335 100644
--- a/config/path-routing.json
+++ b/config/path-routing.json
@@ -19,7 +19,7 @@
     "files-scs:config/ldp/metadata-writer/default.json",
     "files-scs:config/ldp/permissions/acl.json",
     "files-scs:config/storage/backend/regex.json",
-    "files-scs:config/storage/key-value/memory.json",
+    "files-scs:config/storage/key-value/resource-store.json",
     "files-scs:config/storage/middleware/default.json",
     "files-scs:config/util/auxiliary/acl.json",
     "files-scs:config/util/identifiers/suffix.json",
diff --git a/config/sparql-endpoint.json b/config/sparql-endpoint.json
index 284d41bae..5966d1c05 100644
--- a/config/sparql-endpoint.json
+++ b/config/sparql-endpoint.json
@@ -19,7 +19,7 @@
     "files-scs:config/ldp/metadata-writer/default.json",
     "files-scs:config/ldp/permissions/acl.json",
     "files-scs:config/storage/backend/sparql.json",
-    "files-scs:config/storage/key-value/memory.json",
+    "files-scs:config/storage/key-value/resource-store.json",
     "files-scs:config/storage/middleware/default.json",
     "files-scs:config/util/auxiliary/acl.json",
     "files-scs:config/util/identifiers/suffix.json",
diff --git a/config/storage/key-value/resource-store.json b/config/storage/key-value/resource-store.json
index 3394b2d40..b522ec7af 100644
--- a/config/storage/key-value/resource-store.json
+++ b/config/storage/key-value/resource-store.json
@@ -22,6 +22,20 @@
       "source": { "@id": "urn:solid-server:default:ResourceStore" },
       "baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" },
       "container": "/idp/data/"
+    },
+    {
+      "comment": "Block external access to the storage containers to avoid exposing internal data.",
+      "@id": "urn:solid-server:default:PathBasedAuthorizer",
+      "PathBasedAuthorizer:_paths": [
+        {
+          "PathBasedAuthorizer:_paths_key": "^/locks(/.*)?$",
+          "PathBasedAuthorizer:_paths_value": { "@type": "DenyAllAuthorizer" }
+        },
+        {
+          "PathBasedAuthorizer:_paths_key": "^/idp/data(/.*)?$",
+          "PathBasedAuthorizer:_paths_value": { "@type": "DenyAllAuthorizer" }
+        }
+      ]
     }
   ]
 }
diff --git a/src/authorization/AllowEverythingAuthorizer.ts b/src/authorization/AllowAllAuthorizer.ts
similarity index 70%
rename from src/authorization/AllowEverythingAuthorizer.ts
rename to src/authorization/AllowAllAuthorizer.ts
index 7344d1a0c..12a2a12dc 100644
--- a/src/authorization/AllowEverythingAuthorizer.ts
+++ b/src/authorization/AllowAllAuthorizer.ts
@@ -2,7 +2,7 @@ import type { PermissionSet } from '../ldp/permissions/PermissionSet';
 import { Authorizer } from './Authorizer';
 import { WebAclAuthorization } from './WebAclAuthorization';
 
-const allowEverything: PermissionSet = {
+const allowAll: PermissionSet = {
   read: true,
   write: true,
   append: true,
@@ -12,8 +12,8 @@ const allowEverything: PermissionSet = {
 /**
  * Authorizer which allows all access independent of the identifier and requested permissions.
  */
-export class AllowEverythingAuthorizer extends Authorizer {
+export class AllowAllAuthorizer extends Authorizer {
   public async handle(): Promise<WebAclAuthorization> {
-    return new WebAclAuthorization(allowEverything, allowEverything);
+    return new WebAclAuthorization(allowAll, allowAll);
   }
 }
diff --git a/src/authorization/DenyAllAuthorizer.ts b/src/authorization/DenyAllAuthorizer.ts
new file mode 100644
index 000000000..93cb26417
--- /dev/null
+++ b/src/authorization/DenyAllAuthorizer.ts
@@ -0,0 +1,11 @@
+import { ForbiddenHttpError } from '../util/errors/ForbiddenHttpError';
+import { Authorizer } from './Authorizer';
+
+/**
+ * An authorizer that rejects all requests.
+ */
+export class DenyAllAuthorizer extends Authorizer {
+  public async handle(): Promise<never> {
+    throw new ForbiddenHttpError();
+  }
+}
diff --git a/src/authorization/PathBasedAuthorizer.ts b/src/authorization/PathBasedAuthorizer.ts
new file mode 100644
index 000000000..39fc697a5
--- /dev/null
+++ b/src/authorization/PathBasedAuthorizer.ts
@@ -0,0 +1,52 @@
+import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError';
+import { ensureTrailingSlash, trimTrailingSlashes } from '../util/PathUtil';
+import type { Authorization } from './Authorization';
+import type { AuthorizerArgs } from './Authorizer';
+import { Authorizer } from './Authorizer';
+
+/**
+ * Redirects requests to specific authorizers based on their identifier.
+ * The keys in the input map will be converted to regular expressions.
+ * The regular expressions should all start with a slash
+ * and will be evaluated relative to the base URL.
+ *
+ * Will error if no match is found.
+ */
+export class PathBasedAuthorizer extends Authorizer {
+  private readonly baseUrl: string;
+  private readonly paths: Map<RegExp, Authorizer>;
+
+  public constructor(baseUrl: string, paths: Record<string, Authorizer>) {
+    super();
+    this.baseUrl = ensureTrailingSlash(baseUrl);
+    const entries = Object.entries(paths).map(([ key, val ]): [RegExp, Authorizer] => [ new RegExp(key, 'u'), val ]);
+    this.paths = new Map(entries);
+  }
+
+  public async canHandle(input: AuthorizerArgs): Promise<void> {
+    const authorizer = this.findAuthorizer(input.identifier.path);
+    await authorizer.canHandle(input);
+  }
+
+  public async handle(input: AuthorizerArgs): Promise<Authorization> {
+    const authorizer = this.findAuthorizer(input.identifier.path);
+    return authorizer.handle(input);
+  }
+
+  /**
+   * Find the authorizer corresponding to the given path.
+   * Errors if there is no match.
+   */
+  private findAuthorizer(path: string): Authorizer {
+    if (path.startsWith(this.baseUrl)) {
+      // We want to keep the leading slash
+      const relative = path.slice(trimTrailingSlashes(this.baseUrl).length);
+      for (const [ regex, authorizer ] of this.paths) {
+        if (regex.test(relative)) {
+          return authorizer;
+        }
+      }
+    }
+    throw new NotImplementedHttpError('No regex matches the given path.');
+  }
+}
diff --git a/src/index.ts b/src/index.ts
index aa95300a2..7c075528e 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -8,10 +8,12 @@ export * from './authentication/UnsecureConstantCredentialsExtractor';
 export * from './authentication/UnsecureWebIdExtractor';
 
 // Authorization
-export * from './authorization/AllowEverythingAuthorizer';
+export * from './authorization/AllowAllAuthorizer';
 export * from './authorization/Authorization';
 export * from './authorization/Authorizer';
 export * from './authorization/AuxiliaryAuthorizer';
+export * from './authorization/DenyAllAuthorizer';
+export * from './authorization/PathBasedAuthorizer';
 export * from './authorization/WebAclAuthorization';
 export * from './authorization/WebAclAuthorizer';
 


Test patch:
diff --git a/test/integration/config/run-with-redlock.json b/test/integration/config/run-with-redlock.json
index 55b925291..9b902ce03 100644
--- a/test/integration/config/run-with-redlock.json
+++ b/test/integration/config/run-with-redlock.json
@@ -8,7 +8,7 @@
     "files-scs:config/http/server-factory/no-websockets.json",
     "files-scs:config/http/static/default.json",
     "files-scs:config/ldp/authentication/debug-auth-header.json",
-    "files-scs:config/ldp/authorization/allow-everything.json",
+    "files-scs:config/ldp/authorization/allow-all.json",
     "files-scs:config/ldp/handler/default.json",
     "files-scs:config/ldp/metadata-parser/default.json",
     "files-scs:config/ldp/metadata-writer/default.json",
diff --git a/test/integration/config/server-without-auth.json b/test/integration/config/server-without-auth.json
index 3c8a32110..7cd7a4ab4 100644
--- a/test/integration/config/server-without-auth.json
+++ b/test/integration/config/server-without-auth.json
@@ -8,7 +8,7 @@
     "files-scs:config/http/server-factory/websockets.json",
     "files-scs:config/http/static/default.json",
     "files-scs:config/ldp/authentication/dpop-bearer.json",
-    "files-scs:config/ldp/authorization/allow-everything.json",
+    "files-scs:config/ldp/authorization/allow-all.json",
     "files-scs:config/ldp/handler/default.json",
     "files-scs:config/ldp/metadata-parser/default.json",
     "files-scs:config/ldp/metadata-writer/default.json",
diff --git a/test/unit/authorization/AllowAllAuthorizer.test.ts b/test/unit/authorization/AllowAllAuthorizer.test.ts
new file mode 100644
index 000000000..e55366d90
--- /dev/null
+++ b/test/unit/authorization/AllowAllAuthorizer.test.ts
@@ -0,0 +1,23 @@
+import { AllowAllAuthorizer } from '../../../src/authorization/AllowAllAuthorizer';
+import type { PermissionSet } from '../../../src/ldp/permissions/PermissionSet';
+
+describe('An AllowAllAuthorizer', (): void => {
+  const authorizer = new AllowAllAuthorizer();
+  const allowAll: PermissionSet = {
+    read: true,
+    write: true,
+    append: true,
+    control: true,
+  };
+
+  it('can handle everything.', async(): Promise<void> => {
+    await expect(authorizer.canHandle({} as any)).resolves.toBeUndefined();
+  });
+
+  it('always returns a full access Authorization.', async(): Promise<void> => {
+    await expect(authorizer.handle()).resolves.toEqual({
+      user: allowAll,
+      everyone: allowAll,
+    });
+  });
+});
diff --git a/test/unit/authorization/AllowEverythingAuthorizer.test.ts b/test/unit/authorization/AllowEverythingAuthorizer.test.ts
deleted file mode 100644
index 3c0f8d4a0..000000000
--- a/test/unit/authorization/AllowEverythingAuthorizer.test.ts
+++ /dev/null
@@ -1,23 +0,0 @@
-import { AllowEverythingAuthorizer } from '../../../src/authorization/AllowEverythingAuthorizer';
-import type { PermissionSet } from '../../../src/ldp/permissions/PermissionSet';
-
-describe('An AllowEverythingAuthorizer', (): void => {
-  const authorizer = new AllowEverythingAuthorizer();
-  const allowEverything: PermissionSet = {
-    read: true,
-    write: true,
-    append: true,
-    control: true,
-  };
-
-  it('can handle everything.', async(): Promise<void> => {
-    await expect(authorizer.canHandle({} as any)).resolves.toBeUndefined();
-  });
-
-  it('always returns an empty Authorization.', async(): Promise<void> => {
-    await expect(authorizer.handle()).resolves.toEqual({
-      user: allowEverything,
-      everyone: allowEverything,
-    });
-  });
-});
diff --git a/test/unit/authorization/DenyAllAuthorizer.test.ts b/test/unit/authorization/DenyAllAuthorizer.test.ts
new file mode 100644
index 000000000..730d7095e
--- /dev/null
+++ b/test/unit/authorization/DenyAllAuthorizer.test.ts
@@ -0,0 +1,14 @@
+import { DenyAllAuthorizer } from '../../../src/authorization/DenyAllAuthorizer';
+import { ForbiddenHttpError } from '../../../src/util/errors/ForbiddenHttpError';
+
+describe('A DenyAllAuthorizer', (): void => {
+  const authorizer = new DenyAllAuthorizer();
+
+  it('can handle all requests.', async(): Promise<void> => {
+    await expect(authorizer.canHandle({} as any)).resolves.toBeUndefined();
+  });
+
+  it('rejects all requests.', async(): Promise<void> => {
+    await expect(authorizer.handle()).rejects.toThrow(ForbiddenHttpError);
+  });
+});
diff --git a/test/unit/authorization/PathBasedAuthorizer.test.ts b/test/unit/authorization/PathBasedAuthorizer.test.ts
new file mode 100644
index 000000000..ecc773f29
--- /dev/null
+++ b/test/unit/authorization/PathBasedAuthorizer.test.ts
@@ -0,0 +1,51 @@
+import type { Authorizer, AuthorizerArgs } from '../../../src/authorization/Authorizer';
+import { PathBasedAuthorizer } from '../../../src/authorization/PathBasedAuthorizer';
+import { NotImplementedHttpError } from '../../../src/util/errors/NotImplementedHttpError';
+
+describe('A PathBasedAuthorizer', (): void => {
+  const baseUrl = 'http://test.com/foo/';
+  let input: AuthorizerArgs;
+  let authorizers: jest.Mocked<Authorizer>[];
+  let authorizer: PathBasedAuthorizer;
+
+  beforeEach(async(): Promise<void> => {
+    input = {
+      identifier: { path: `${baseUrl}first` },
+      permissions: { read: true, append: false, write: false, control: false },
+      credentials: { webId: 'http://alice.test.com/card#me' },
+    };
+
+    authorizers = [
+      { canHandle: jest.fn(), handle: jest.fn() },
+      { canHandle: jest.fn(), handle: jest.fn() },
+    ] as any;
+    const paths = {
+      '/first': authorizers[0],
+      '/second': authorizers[1],
+    };
+    authorizer = new PathBasedAuthorizer(baseUrl, paths);
+  });
+
+  it('can only handle requests with a matching path.', async(): Promise<void> => {
+    input.identifier.path = 'http://wrongsite/';
+    await expect(authorizer.canHandle(input)).rejects.toThrow(NotImplementedHttpError);
+    input.identifier.path = `${baseUrl}third`;
+    await expect(authorizer.canHandle(input)).rejects.toThrow(NotImplementedHttpError);
+    input.identifier.path = `${baseUrl}first`;
+    await expect(authorizer.canHandle(input)).resolves.toBeUndefined();
+    input.identifier.path = `${baseUrl}second`;
+    await expect(authorizer.canHandle(input)).resolves.toBeUndefined();
+  });
+
+  it('can only handle requests supported by the stored authorizers.', async(): Promise<void> => {
+    await expect(authorizer.canHandle(input)).resolves.toBeUndefined();
+    authorizers[0].canHandle.mockRejectedValueOnce(new Error('not supported'));
+    await expect(authorizer.canHandle(input)).rejects.toThrow('not supported');
+  });
+
+  it('passes the handle requests to the matching authorizer.', async(): Promise<void> => {
+    await expect(authorizer.handle(input)).resolves.toBeUndefined();
+    expect(authorizers[0].handle).toHaveBeenCalledTimes(1);
+    expect(authorizers[0].handle).toHaveBeenLastCalledWith(input);
+  });
+});