From e43b579ae7a14a0b7be5c89545922346c7fb3833 Mon Sep 17 00:00:00 2001
From: Joachim Van Herwegen <joachimvh@gmail.com>
Date: Thu, 15 Jul 2021 13:01:44 +0200
Subject: [PATCH] fix: Throw internal error with invalid ACL.

---
 src/authorization/WebAclAuthorizer.ts            | 9 ++++++---
 src/init/AclInitializer.ts                       | 6 +++---
 test/unit/authorization/WebAclAuthorizer.test.ts | 7 +++++--
 test/unit/init/AclInitializer.test.ts            | 2 +-
 4 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/authorization/WebAclAuthorizer.ts b/src/authorization/WebAclAuthorizer.ts
index 94cd5cf4f..de7ac53e2 100644
--- a/src/authorization/WebAclAuthorizer.ts
+++ b/src/authorization/WebAclAuthorizer.ts
@@ -8,7 +8,9 @@ import type { ResourceIdentifier } from '../ldp/representation/ResourceIdentifie
 import { getLoggerFor } from '../logging/LogUtil';
 import type { ResourceStore } from '../storage/ResourceStore';
 import { INTERNAL_QUADS } from '../util/ContentTypes';
+import { createErrorMessage } from '../util/errors/ErrorUtil';
 import { ForbiddenHttpError } from '../util/errors/ForbiddenHttpError';
+import { InternalServerError } from '../util/errors/InternalServerError';
 import { NotFoundHttpError } from '../util/errors/NotFoundHttpError';
 import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError';
 import { UnauthorizedHttpError } from '../util/errors/UnauthorizedHttpError';
@@ -214,13 +216,14 @@ export class WebAclAuthorizer extends Authorizer {
       const data = await this.resourceStore.getRepresentation(acl, { type: { [INTERNAL_QUADS]: 1 }});
       this.logger.info(`Reading ACL statements from ${acl.path}`);
 
-      return this.filterData(data, recurse ? ACL.default : ACL.accessTo, id.path);
+      return await this.filterData(data, recurse ? ACL.default : ACL.accessTo, id.path);
     } catch (error: unknown) {
       if (NotFoundHttpError.isInstance(error)) {
         this.logger.debug(`No direct ACL document found for ${id.path}`);
       } else {
-        this.logger.error(`Error reading ACL for ${id.path}: ${(error as Error).message}`, { error });
-        throw error;
+        const message = `Error reading ACL for ${id.path}: ${createErrorMessage(error)}`;
+        this.logger.error(message);
+        throw new InternalServerError(message, { cause: error });
       }
     }
 
diff --git a/src/init/AclInitializer.ts b/src/init/AclInitializer.ts
index 44e86afb4..59d2e0b69 100644
--- a/src/init/AclInitializer.ts
+++ b/src/init/AclInitializer.ts
@@ -48,9 +48,9 @@ export class AclInitializer extends Initializer {
       try {
         await this.store.setRepresentation(rootAcl, new BasicRepresentation(aclDocument, rootAcl, TEXT_TURTLE));
       } catch (error: unknown) {
-        const msg = `There was an issue initializing the root .acl resource: ${createErrorMessage(error)}`;
-        this.logger.error(msg);
-        throw new InternalServerError(msg, { cause: error });
+        const message = `Issue initializing the root ACL resource: ${createErrorMessage(error)}`;
+        this.logger.error(message);
+        throw new InternalServerError(message, { cause: error });
       }
     }
   }
diff --git a/test/unit/authorization/WebAclAuthorizer.test.ts b/test/unit/authorization/WebAclAuthorizer.test.ts
index 6136f3f44..09590c409 100644
--- a/test/unit/authorization/WebAclAuthorizer.test.ts
+++ b/test/unit/authorization/WebAclAuthorizer.test.ts
@@ -8,6 +8,7 @@ import type { Representation } from '../../../src/ldp/representation/Representat
 import type { ResourceIdentifier } from '../../../src/ldp/representation/ResourceIdentifier';
 import type { ResourceStore } from '../../../src/storage/ResourceStore';
 import { ForbiddenHttpError } from '../../../src/util/errors/ForbiddenHttpError';
+import { InternalServerError } from '../../../src/util/errors/InternalServerError';
 import { NotFoundHttpError } from '../../../src/util/errors/NotFoundHttpError';
 import { NotImplementedHttpError } from '../../../src/util/errors/NotImplementedHttpError';
 import { UnauthorizedHttpError } from '../../../src/util/errors/UnauthorizedHttpError';
@@ -145,11 +146,13 @@ describe('A WebAclAuthorizer', (): void => {
     await expect(authorizer.handle({ identifier, permissions, credentials })).rejects.toThrow(ForbiddenHttpError);
   });
 
-  it('passes errors of the ResourceStore along.', async(): Promise<void> => {
+  it('re-throws ResourceStore errors as internal errors.', async(): Promise<void> => {
     store.getRepresentation = async(): Promise<Representation> => {
       throw new Error('TEST!');
     };
-    await expect(authorizer.handle({ identifier, permissions, credentials })).rejects.toThrow('TEST!');
+    const promise = authorizer.handle({ identifier, permissions, credentials });
+    await expect(promise).rejects.toThrow(`Error reading ACL for ${identifier.path}: TEST!`);
+    await expect(promise).rejects.toThrow(InternalServerError);
   });
 
   it('errors if the root container has no corresponding acl document.', async(): Promise<void> => {
diff --git a/test/unit/init/AclInitializer.test.ts b/test/unit/init/AclInitializer.test.ts
index bf08f09c2..67cc460e3 100644
--- a/test/unit/init/AclInitializer.test.ts
+++ b/test/unit/init/AclInitializer.test.ts
@@ -77,7 +77,7 @@ describe('AclInitializer', (): void => {
 
     const initializer = new AclInitializer({ baseUrl, store, aclStrategy });
     const prom = initializer.handle();
-    await expect(prom).rejects.toThrow('There was an issue initializing the root .acl resource: Fatal');
+    await expect(prom).rejects.toThrow('Issue initializing the root ACL resource: Fatal');
     await expect(prom).rejects.toThrow(InternalServerError);
   });
 });