feat: Return 404 for read/delete requests if there is no resource

RELEASE_NOTES.md

@@ -23,7 +23,7 @@ These changes are relevant if you wrote custom modules for the server that depen
 - `PermissionReader`s take an additional `modes` parameter as input.
 - The `ResourceStore` function `resourceExists` has been renamed to `hasResource`
   and has been moved to a separate `ResourceSet` interface.
-- Several `ModesExtractor`s now take a `ResourceSet` as constructor parameter.
+- Several `ModesExtractor`s `PermissionBasedAuthorizer` now take a `ResourceSet` as constructor parameter.
 
 ## v3.0.0
 ### New features

config/ldp/handler/components/authorizer.json

@@ -4,7 +4,8 @@
     {
       "comment": "Matches requested permissions with those available.",
       "@id": "urn:solid-server:default:Authorizer",
-      "@type": "PermissionBasedAuthorizer"
+      "@type": "PermissionBasedAuthorizer",
+      "resourceSet": { "@id": "urn:solid-server:default:CachedResourceSet" }
     }
   ]
 }

src/authorization/PermissionBasedAuthorizer.ts

@@ -1,10 +1,13 @@
 import type { CredentialSet } from '../authentication/Credentials';
 import { getLoggerFor } from '../logging/LogUtil';
+import type { ResourceSet } from '../storage/ResourceSet';
 import { ForbiddenHttpError } from '../util/errors/ForbiddenHttpError';
+import { NotFoundHttpError } from '../util/errors/NotFoundHttpError';
 import { UnauthorizedHttpError } from '../util/errors/UnauthorizedHttpError';
 import type { AuthorizerInput } from './Authorizer';
 import { Authorizer } from './Authorizer';
-import type { AccessMode, PermissionSet } from './permissions/Permissions';
+import type { PermissionSet } from './permissions/Permissions';
+import { AccessMode } from './permissions/Permissions';
 
 /**
  * Authorizer that bases its decision on the output it gets from its PermissionReader.
@@ -15,14 +18,39 @@ import type { AccessMode, PermissionSet } from './permissions/Permissions';
 export class PermissionBasedAuthorizer extends Authorizer {
   protected readonly logger = getLoggerFor(this);
 
+  private readonly resourceSet: ResourceSet;
+
+  /**
+   * The existence of the target resource determines the output status code for certain situations.
+   * The provided {@link ResourceSet} will be used for that.
+   * @param resourceSet - {@link ResourceSet} that can verify the target resource existence.
+   */
+  public constructor(resourceSet: ResourceSet) {
+    super();
+    this.resourceSet = resourceSet;
+  }
+
   public async handle(input: AuthorizerInput): Promise<void> {
     const { credentials, modes, identifier, permissionSet } = input;
 
     const modeString = [ ...modes ].join(',');
     this.logger.debug(`Checking if ${credentials.agent?.webId} has ${modeString} permissions for ${identifier.path}`);
 
+    // Ensure all required modes are within the agent's permissions.
     for (const mode of modes) {
-      this.requireModePermission(credentials, permissionSet, mode);
+      try {
+        this.requireModePermission(credentials, permissionSet, mode);
+      } catch (error: unknown) {
+        // If we know the operation will return a 404 regardless (= resource does not exist and is not being created),
+        // and the agent is allowed to know about its existence (= the agent has Read permissions),
+        // then immediately send the 404 here, as it makes any other agent permissions irrelevant.
+        const exposeExistence = this.hasModePermission(permissionSet, AccessMode.read);
+        if (exposeExistence && !modes.has(AccessMode.create) && !await this.resourceSet.hasResource(identifier)) {
+          throw new NotFoundHttpError();
+        }
+        // Otherwise, deny access based on existing grounds.
+        throw error;
+      }
     }
     this.logger.debug(`${JSON.stringify(credentials)} has ${modeString} permissions for ${identifier.path}`);
   }

test/unit/authorization/PermissionBasedAuthorizer.test.ts

@@ -2,11 +2,14 @@ import { CredentialGroup } from '../../../src/authentication/Credentials';
 import type { AuthorizerInput } from '../../../src/authorization/Authorizer';
 import { PermissionBasedAuthorizer } from '../../../src/authorization/PermissionBasedAuthorizer';
 import { AccessMode } from '../../../src/authorization/permissions/Permissions';
+import type { ResourceSet } from '../../../src/storage/ResourceSet';
 import { ForbiddenHttpError } from '../../../src/util/errors/ForbiddenHttpError';
+import { NotFoundHttpError } from '../../../src/util/errors/NotFoundHttpError';
 import { UnauthorizedHttpError } from '../../../src/util/errors/UnauthorizedHttpError';
 
 describe('A PermissionBasedAuthorizer', (): void => {
   let input: AuthorizerInput;
+  let resourceSet: jest.Mocked<ResourceSet>;
   let authorizer: PermissionBasedAuthorizer;
 
   beforeEach(async(): Promise<void> => {
@@ -16,8 +19,11 @@ describe('A PermissionBasedAuthorizer', (): void => {
       permissionSet: {},
       credentials: {},
     };
+    resourceSet = {
+      hasResource: jest.fn().mockResolvedValue(true),
+    };
 
-    authorizer = new PermissionBasedAuthorizer();
+    authorizer = new PermissionBasedAuthorizer(resourceSet);
   });
 
   it('can handle any input.', async(): Promise<void> => {
@@ -53,4 +59,13 @@ describe('A PermissionBasedAuthorizer', (): void => {
   it('defaults to empty permissions for the Authorization.', async(): Promise<void> => {
     await expect(authorizer.handle(input)).resolves.toBeUndefined();
   });
+
+  it('throws a 404 in case the target resource does not exist and would not be written to.', async(): Promise<void> => {
+    resourceSet.hasResource.mockResolvedValueOnce(false);
+    input.modes = new Set([ AccessMode.delete ]);
+    input.permissionSet = {
+      [CredentialGroup.public]: { read: true },
+    };
+    await expect(authorizer.handle(input)).rejects.toThrow(NotFoundHttpError);
+  });
 });