fix: Correctly handle acl behaviour for acl identifiers

src/authorization/AclManager.ts

@@ -1,26 +1,37 @@
 import type { ResourceIdentifier } from '../ldp/representation/ResourceIdentifier';
 
 /**
- * Handles where acl files are stored.
+ * Handles where acl resources are stored.
  */
 export interface AclManager {
   /**
-   * Returns the identifier of the acl file corresponding to the given resource.
-   * This does not guarantee that this acl file exists.
-   * In the case the input is already an acl file that will also be the response.
-   * @param id - The ResourceIdentifier of which we need the corresponding acl file.
+   * Returns the identifier of the acl resource corresponding to the given resource.
+   * This does not guarantee that this acl resource exists.
+   * In the case the input is already an acl resource that will also be the response.
+   * @param id - The ResourceIdentifier of which we need the corresponding acl resource.
    *
-   * @returns The ResourceIdentifier of the corresponding acl file.
+   * @returns The ResourceIdentifier of the corresponding acl resource.
    */
-  getAcl: (id: ResourceIdentifier) => Promise<ResourceIdentifier>;
+  getAclDocument: (id: ResourceIdentifier) => Promise<ResourceIdentifier>;
 
   /**
-   * Checks if the input identifier corresponds to an acl file.
-   * This does not check if that acl file exists,
-   * only if the identifier indicates that there could be an acl file there.
+   * Checks if the input identifier corresponds to an acl resource.
+   * This does not check if that acl resource exists,
+   * only if the identifier indicates that there could be an acl resource there.
    * @param id - Identifier to check.
    *
-   * @returns true if the input identifier points to an acl file.
+   * @returns true if the input identifier points to an acl resource.
    */
-  isAcl: (id: ResourceIdentifier) => Promise<boolean>;
+  isAclDocument: (id: ResourceIdentifier) => Promise<boolean>;
+
+  /**
+   * Returns the identifier of the resource on which the acl constraints are placed.
+   * In general, this is the resource identifier when the input is a normal resource,
+   * or the non-acl version if the input is an acl resource.
+   * This does not guarantee that this resource exists.
+   * @param aclId - Identifier of the acl resource.
+   *
+   * @returns The ResourceIdentifier of the corresponding resource.
+   */
+  getAclConstrainedResource: (id: ResourceIdentifier) => Promise<ResourceIdentifier>;
 }

src/authorization/UrlBasedAclManager.ts

@@ -4,23 +4,23 @@ import type { AclManager } from './AclManager';
 /**
  * Generates acl URIs by adding an .acl file extension.
  *
- * What actually should happen in getAcl:
- * 1. Return id if it isAcl
- * 2. Check store if id exists
- * 3a. (true) Close/destroy data stream! To prevent potential locking issues.
- * 4a. Check metadata if it is a container or a resource.
- * 3b. (false) Use input metadata/heuristic to check if container or resource.
- * 5. Generate the correct identifier (.acl right of / for containers, left for resources if there is a /)
- *
- * It is potentially possible that an agent wants to generate the acl file before generating the actual file.
- * (Unless this is not allowed by the spec, need to verify).
+ * Needs to be updated according to issue #113.
  */
 export class UrlBasedAclManager implements AclManager {
-  public async getAcl(id: ResourceIdentifier): Promise<ResourceIdentifier> {
-    return await this.isAcl(id) ? id : { path: `${id.path}.acl` };
+  public async getAclDocument(id: ResourceIdentifier): Promise<ResourceIdentifier> {
+    return await this.isAclDocument(id) ? id : { path: `${id.path}.acl` };
   }
 
-  public async isAcl(id: ResourceIdentifier): Promise<boolean> {
+  public async isAclDocument(id: ResourceIdentifier): Promise<boolean> {
     return /\.acl\/?/u.test(id.path);
   }
+
+  public async getAclConstrainedResource(id: ResourceIdentifier): Promise<ResourceIdentifier> {
+    if (!await this.isAclDocument(id)) {
+      return id;
+    }
+
+    // Slice off `.acl`
+    return { path: id.path.slice(0, -4) };
+  }
 }

src/authorization/WebAclAuthorizer.ts

@@ -40,7 +40,7 @@ export class WebAclAuthorizer extends Authorizer {
    */
   public async handle(input: AuthorizerArgs): Promise<void> {
     const store = await this.getAclRecursive(input.identifier);
-    if (await this.aclManager.isAcl(input.identifier)) {
+    if (await this.aclManager.isAclDocument(input.identifier)) {
       this.checkPermission(input.credentials, store, 'control');
     } else {
       (Object.keys(input.permissions) as (keyof PermissionSet)[]).forEach((key): void => {
@@ -117,11 +117,13 @@ export class WebAclAuthorizer extends Authorizer {
   private async getAclRecursive(id: ResourceIdentifier, recurse?: boolean): Promise<Store> {
     this.logger.debug(`Trying to read the direct ACL document of ${id.path}`);
     try {
-      const acl = await this.aclManager.getAcl(id);
+      const acl = await this.aclManager.getAclDocument(id);
       this.logger.debug(`Trying to read the ACL document ${acl.path}`);
       const data = await this.resourceStore.getRepresentation(acl, { type: [{ value: INTERNAL_QUADS, weight: 1 }]});
       this.logger.info(`Reading ACL statements from ${acl.path}`);
-      return this.filterData(data, recurse ? ACL.default : ACL.accessTo, id.path);
+
+      const resourceId = await this.aclManager.getAclConstrainedResource(id);
+      return this.filterData(data, recurse ? ACL.default : ACL.accessTo, resourceId.path);
     } catch (error: unknown) {
       if (error instanceof NotFoundHttpError) {
         this.logger.debug(`No direct ACL document found for ${id.path}`);

src/init/Setup.ts

@@ -59,7 +59,7 @@ export class Setup {
     acl:mode        acl:Control;
     acl:accessTo    <${this.base}>;
     acl:default     <${this.base}>.`;
-      const baseAclId = await this.aclManager.getAcl({ path: this.base });
+      const baseAclId = await this.aclManager.getAclDocument({ path: this.base });
       const metadata = new RepresentationMetadata(baseAclId.path, { [CONTENT_TYPE]: TEXT_TURTLE });
       await this.store.setRepresentation(
         baseAclId,

test/unit/authorization/UrlBasedAclManager.test.ts

@@ -3,18 +3,32 @@ import { UrlBasedAclManager } from '../../../src/authorization/UrlBasedAclManage
 describe('An UrlBasedAclManager', (): void => {
   const manager = new UrlBasedAclManager();
 
-  it('generates acl URLs by adding an .acl extension.', async(): Promise<void> => {
-    await expect(manager.getAcl({ path: '/foo/bar' })).resolves.toEqual({ path: '/foo/bar.acl' });
+  describe('#getAcl', (): void => {
+    it('generates acl URLs by adding an .acl extension.', async(): Promise<void> => {
+      await expect(manager.getAclDocument({ path: '/foo/bar' })).resolves.toEqual({ path: '/foo/bar.acl' });
+    });
+
+    it('returns the identifier if the input is already an acl resource.', async(): Promise<void> => {
+      await expect(manager.getAclDocument({ path: '/foo/bar.acl' })).resolves.toEqual({ path: '/foo/bar.acl' });
+    });
   });
 
-  it('returns the identifier if the input is already an acl file.', async(): Promise<void> => {
-    await expect(manager.getAcl({ path: '/foo/bar.acl' })).resolves.toEqual({ path: '/foo/bar.acl' });
+  describe('#isAcl', (): void => {
+    it('checks if a resource is an acl resource by looking at the extension.', async(): Promise<void> => {
+      await expect(manager.isAclDocument({ path: '/foo/bar' })).resolves.toBeFalsy();
+      await expect(manager.isAclDocument({ path: '/foo/bar/' })).resolves.toBeFalsy();
+      await expect(manager.isAclDocument({ path: '/foo/bar.acl' })).resolves.toBeTruthy();
+      await expect(manager.isAclDocument({ path: '/foo/bar.acl/' })).resolves.toBeTruthy();
+    });
   });
 
-  it('checks if a resource is an acl file by looking at the extension.', async(): Promise<void> => {
-    await expect(manager.isAcl({ path: '/foo/bar' })).resolves.toBeFalsy();
-    await expect(manager.isAcl({ path: '/foo/bar/' })).resolves.toBeFalsy();
-    await expect(manager.isAcl({ path: '/foo/bar.acl' })).resolves.toBeTruthy();
-    await expect(manager.isAcl({ path: '/foo/bar.acl/' })).resolves.toBeTruthy();
+  describe('#getResource', (): void => {
+    it('generates non-acl resource URLs by removing the .acl extension.', async(): Promise<void> => {
+      await expect(manager.getAclConstrainedResource({ path: '/foo/bar.acl' })).resolves.toEqual({ path: '/foo/bar' });
+    });
+
+    it('returns the identifier if the input is already a non-acl resource.', async(): Promise<void> => {
+      await expect(manager.getAclConstrainedResource({ path: '/foo/bar' })).resolves.toEqual({ path: '/foo/bar' });
+    });
   });
 });

test/unit/authorization/WebAclAuthorizer.test.ts

@@ -19,9 +19,11 @@ const acl = 'http://www.w3.org/ns/auth/acl#';
 describe('A WebAclAuthorizer', (): void => {
   let authorizer: WebAclAuthorizer;
   const aclManager: AclManager = {
-    getAcl: async(id: ResourceIdentifier): Promise<ResourceIdentifier> =>
+    getAclDocument: async(id: ResourceIdentifier): Promise<ResourceIdentifier> =>
       id.path.endsWith('.acl') ? id : { path: `${id.path}.acl` },
-    isAcl: async(id: ResourceIdentifier): Promise<boolean> => id.path.endsWith('.acl'),
+    isAclDocument: async(id: ResourceIdentifier): Promise<boolean> => id.path.endsWith('.acl'),
+    getAclConstrainedResource: async(id: ResourceIdentifier): Promise<ResourceIdentifier> =>
+      !id.path.endsWith('.acl') ? id : { path: id.path.slice(0, -4) },
   };
   let permissions: PermissionSet;
   let credentials: Credentials;
@@ -134,9 +136,9 @@ describe('A WebAclAuthorizer', (): void => {
         quad(nn('auth'), nn(`${acl}mode`), nn(`${acl}Control`)),
       ]) } as Representation),
     } as unknown as ResourceStore;
-    identifier = await aclManager.getAcl(identifier);
+    const aclIdentifier = await aclManager.getAclDocument(identifier);
     authorizer = new WebAclAuthorizer(aclManager, store);
-    await expect(authorizer.handle({ identifier, permissions, credentials })).resolves.toBeUndefined();
+    await expect(authorizer.handle({ identifier: aclIdentifier, permissions, credentials })).resolves.toBeUndefined();
   });
 
   it('errors if an agent tries to edit the acl file without control permissions.', async(): Promise<void> => {
@@ -149,7 +151,7 @@ describe('A WebAclAuthorizer', (): void => {
         quad(nn('auth'), nn(`${acl}mode`), nn(`${acl}Read`)),
       ]) } as Representation),
     } as unknown as ResourceStore;
-    identifier = await aclManager.getAcl(identifier);
+    identifier = await aclManager.getAclDocument(identifier);
     authorizer = new WebAclAuthorizer(aclManager, store);
     await expect(authorizer.handle({ identifier, permissions, credentials })).rejects.toThrow(ForbiddenHttpError);
   });

test/unit/init/Setup.test.ts

@@ -1,19 +1,20 @@
+import type { AclManager } from '../../../src/authorization/AclManager';
 import { Setup } from '../../../src/init/Setup';
 import type { ResourceIdentifier } from '../../../src/ldp/representation/ResourceIdentifier';
 import { VoidLoggerFactory } from '../../../src/logging/VoidLoggerFactory';
 
 describe('Setup', (): void => {
   let serverFactory: any;
   let store: any;
-  let aclManager: any;
+  let aclManager: AclManager;
   let setup: Setup;
   beforeEach(async(): Promise<void> => {
     store = {
       setRepresentation: jest.fn(async(): Promise<void> => undefined),
     };
     aclManager = {
-      getAcl: jest.fn(async(): Promise<ResourceIdentifier> => ({ path: 'http://test.com/.acl' })),
-    };
+      getAclDocument: jest.fn(async(): Promise<ResourceIdentifier> => ({ path: 'http://test.com/.acl' })),
+    } as any;
     serverFactory = {
       startServer: jest.fn(),
     };
@@ -27,7 +28,7 @@ describe('Setup', (): void => {
 
   it('invokes ACL initialization.', async(): Promise<void> => {
     await setup.setup();
-    expect(aclManager.getAcl).toHaveBeenCalledWith({ path: 'http://localhost:3000/' });
+    expect(aclManager.getAclDocument).toHaveBeenCalledWith({ path: 'http://localhost:3000/' });
     expect(store.setRepresentation).toHaveBeenCalledTimes(1);
   });
 });