Merge pull request #14 from pdsinterop/fix-1339-1340

michielbdejong · web-flow · commit ce74e024d455 · 2022-06-30T11:58:20.000+02:00
Fix #1339 and #1340
diff --git a/src/authorization/AllStaticReader.ts b/src/authorization/AllStaticReader.ts
@@ -1,5 +1,6 @@
 import type { CredentialGroup } from '../authentication/Credentials';
-import type { PermissionReaderInput } from './PermissionReader';
+import { ResourceIdentifier } from '../http/representation/ResourceIdentifier';
+import type { PermissionReaderInput, PermissionReaderOutput } from './PermissionReader';
 import { PermissionReader } from './PermissionReader';
 import type { Permission, PermissionSet } from './permissions/Permissions';
 
@@ -21,13 +22,13 @@ export class AllStaticReader extends PermissionReader {
     });
   }
 
-  public async handle({ credentials }: PermissionReaderInput): Promise<PermissionSet> {
+  public async handle({ credentials }: PermissionReaderInput): Promise<PermissionReaderOutput> {
     const result: PermissionSet = {};
     for (const [ key, value ] of Object.entries(credentials) as [CredentialGroup, Permission][]) {
       if (value) {
         result[key] = this.permissions;
       }
     }
-    return result;
+    return { permissions:result };
   }
 }
diff --git a/src/authorization/Authorizer.ts b/src/authorization/Authorizer.ts
@@ -20,6 +20,11 @@ export interface AuthorizerInput {
    * Permissions that are available for the request.
    */
   permissionSet: PermissionSet;
+  /**
+   * Only perform the check if the resource does not exist
+   * Used for ancestor creation checks
+   */
+  onlyIfNotExist?: boolean;
 }
 
 /**
diff --git a/src/authorization/AuxiliaryReader.ts b/src/authorization/AuxiliaryReader.ts
@@ -1,7 +1,8 @@
 import type { AuxiliaryStrategy } from '../http/auxiliary/AuxiliaryStrategy';
+import { ResourceIdentifier } from '../http/representation/ResourceIdentifier';
 import { getLoggerFor } from '../logging/LogUtil';
 import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError';
-import type { PermissionReaderInput } from './PermissionReader';
+import type { PermissionReaderInput, PermissionReaderOutput } from './PermissionReader';
 import { PermissionReader } from './PermissionReader';
 import type { PermissionSet } from './permissions/Permissions';
 
@@ -27,16 +28,16 @@ export class AuxiliaryReader extends PermissionReader {
     return this.resourceReader.canHandle(resourceAuth);
   }
 
-  public async handle(auxiliaryAuth: PermissionReaderInput): Promise<PermissionSet> {
+  public async handle(auxiliaryAuth: PermissionReaderInput): Promise<PermissionReaderOutput> {
     const resourceAuth = this.getRequiredAuthorization(auxiliaryAuth);
     this.logger.debug(`Checking auth request for ${auxiliaryAuth.identifier.path} on ${resourceAuth.identifier.path}`);
-    return this.resourceReader.handle(resourceAuth);
+    return { permissions: (await this.resourceReader.handle(resourceAuth) as PermissionSet)};
   }
 
-  public async handleSafe(auxiliaryAuth: PermissionReaderInput): Promise<PermissionSet> {
+  public async handleSafe(auxiliaryAuth: PermissionReaderInput): Promise<PermissionReaderOutput> {
     const resourceAuth = this.getRequiredAuthorization(auxiliaryAuth);
     this.logger.debug(`Checking auth request for ${auxiliaryAuth.identifier.path} to ${resourceAuth.identifier.path}`);
-    return this.resourceReader.handleSafe(resourceAuth);
+    return { permissions: (await this.resourceReader.handleSafe(resourceAuth) as PermissionSet)};
   }
 
   private getRequiredAuthorization(auxiliaryAuth: PermissionReaderInput): PermissionReaderInput {
diff --git a/src/authorization/OwnerPermissionReader.ts b/src/authorization/OwnerPermissionReader.ts
@@ -1,10 +1,11 @@
 import { CredentialGroup } from '../authentication/Credentials';
 import type { AuxiliaryIdentifierStrategy } from '../http/auxiliary/AuxiliaryIdentifierStrategy';
+import { ResourceIdentifier } from '../http/representation/ResourceIdentifier';
 import type { AccountSettings, AccountStore } from '../identity/interaction/email-password/storage/AccountStore';
 import { getLoggerFor } from '../logging/LogUtil';
 import { createErrorMessage } from '../util/errors/ErrorUtil';
 import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError';
-import type { PermissionReaderInput } from './PermissionReader';
+import type { PermissionReaderInput, PermissionReaderOutput } from './PermissionReader';
 import { PermissionReader } from './PermissionReader';
 import type { AclPermission } from './permissions/AclPermission';
 import type { PermissionSet } from './permissions/Permissions';
@@ -24,23 +25,25 @@ export class OwnerPermissionReader extends PermissionReader {
     this.aclStrategy = aclStrategy;
   }
 
-  public async handle(input: PermissionReaderInput): Promise<PermissionSet> {
+  public async handle(input: PermissionReaderInput): Promise<PermissionReaderOutput> {
     try {
       await this.ensurePodOwner(input);
     } catch (error: unknown) {
       this.logger.debug(`No pod owner Control permissions: ${createErrorMessage(error)}`);
-      return {};
+      return { permissions: {}};
     }
     this.logger.debug(`Granting Control permissions to owner on ${input.identifier.path}`);
 
-    return { [CredentialGroup.agent]: {
-      read: true,
-      write: true,
-      append: true,
-      create: true,
-      delete: true,
-      control: true,
-    } as AclPermission };
+    return {
+      permissions: { [CredentialGroup.agent]: {
+        read: true,
+        write: true,
+        append: true,
+        create: true,
+        delete: true,
+        control: true,
+      } as AclPermission }
+    };
   }
 
   /**
diff --git a/src/authorization/PathBasedReader.ts b/src/authorization/PathBasedReader.ts
@@ -1,7 +1,8 @@
+import { ResourceIdentifier } from '../http/representation/ResourceIdentifier';
 import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError';
 import { ensureTrailingSlash, trimTrailingSlashes } from '../util/PathUtil';
 
-import type { PermissionReaderInput } from './PermissionReader';
+import type { PermissionReaderInput, PermissionReaderOutput } from './PermissionReader';
 import { PermissionReader } from './PermissionReader';
 import type { PermissionSet } from './permissions/Permissions';
 
@@ -30,9 +31,9 @@ export class PathBasedReader extends PermissionReader {
     await reader.canHandle(input);
   }
 
-  public async handle(input: PermissionReaderInput): Promise<PermissionSet> {
+  public async handle(input: PermissionReaderInput): Promise<PermissionReaderOutput> {
     const reader = this.findReader(input.identifier.path);
-    return reader.handle(input);
+    return { permissions: (await reader.handle(input) as PermissionSet) };
   }
 
   /**
diff --git a/src/authorization/PermissionBasedAuthorizer.ts b/src/authorization/PermissionBasedAuthorizer.ts
@@ -31,7 +31,11 @@ export class PermissionBasedAuthorizer extends Authorizer {
   }
 
   public async handle(input: AuthorizerInput): Promise<void> {
-    const { credentials, modes, identifier, permissionSet } = input;
+    const { credentials, modes, identifier, permissionSet, onlyIfNotExist } = input;
+
+    if (onlyIfNotExist && await this.resourceSet.hasResource(identifier)) {
+      return
+    }
 
     const modeString = [ ...modes ].join(',');
     this.logger.debug(`Checking if ${credentials.agent?.webId} has ${modeString} permissions for ${identifier.path}`);
diff --git a/src/authorization/PermissionReader.ts b/src/authorization/PermissionReader.ts
@@ -20,7 +20,12 @@ export interface PermissionReaderInput {
   modes: Set<AccessMode>;
 }
 
+export interface PermissionReaderOutput {
+  permissions: PermissionSet;
+  ancestors?: ResourceIdentifier[];
+}
+
 /**
  * Discovers the permissions of the given credentials on the given identifier.
  */
-export abstract class PermissionReader extends AsyncHandler<PermissionReaderInput, PermissionSet> {}
+export abstract class PermissionReader extends AsyncHandler<PermissionReaderInput, PermissionReaderOutput> {}
diff --git a/src/authorization/UnionPermissionReader.ts b/src/authorization/UnionPermissionReader.ts
@@ -1,6 +1,6 @@
 import type { CredentialGroup } from '../authentication/Credentials';
 import { UnionHandler } from '../util/handlers/UnionHandler';
-import type { PermissionReader } from './PermissionReader';
+import type { PermissionReader, PermissionReaderOutput } from './PermissionReader';
 import type { Permission, PermissionSet } from './permissions/Permissions';
 
 /**
@@ -12,14 +12,18 @@ export class UnionPermissionReader extends UnionHandler<PermissionReader> {
     super(readers);
   }
 
-  protected async combine(results: PermissionSet[]): Promise<PermissionSet> {
+  protected async combine(results: PermissionReaderOutput[]): Promise<PermissionReaderOutput> {
     const result: PermissionSet = {};
+    let ancestors;
     for (const permissionSet of results) {
-      for (const [ key, value ] of Object.entries(permissionSet) as [ CredentialGroup, Permission | undefined ][]) {
+      for (const [ key, value ] of Object.entries(permissionSet.permissions) as [ CredentialGroup, Permission | undefined ][]) {
         result[key] = this.applyPermissions(value, result[key]);
+        if (permissionSet.ancestors) {
+          ancestors = permissionSet.ancestors;
+        }
       }
     }
-    return result;
+    return { permissions: result, ancestors };
   }
 
   /**
diff --git a/src/authorization/WebAclReader.ts b/src/authorization/WebAclReader.ts
@@ -14,7 +14,7 @@ import type { IdentifierStrategy } from '../util/identifiers/IdentifierStrategy'
 import { readableToQuads } from '../util/StreamUtil';
 import { ACL, RDF } from '../util/Vocabularies';
 import type { AccessChecker } from './access/AccessChecker';
-import type { PermissionReaderInput } from './PermissionReader';
+import type { PermissionReaderInput, PermissionReaderOutput } from './PermissionReader';
 import { PermissionReader } from './PermissionReader';
 import type { AclPermission } from './permissions/AclPermission';
 import { AclMode } from './permissions/AclPermission';
@@ -40,7 +40,7 @@ export class WebAclReader extends PermissionReader {
 
   private readonly aclStrategy: AuxiliaryIdentifierStrategy;
   private readonly aclStore: ResourceStore;
-  private readonly identifierStrategy: IdentifierStrategy;
+  public readonly identifierStrategy: IdentifierStrategy;
   private readonly accessChecker: AccessChecker;
 
   public constructor(aclStrategy: AuxiliaryIdentifierStrategy, aclStore: ResourceStore,
@@ -52,13 +52,26 @@ export class WebAclReader extends PermissionReader {
     this.accessChecker = accessChecker;
   }
 
+  // FIXME: this utility function is unrelated to permission
+  // so should be moved elsewhere by editing the componentjs config:
+  private getAncestors(identifier: ResourceIdentifier): ResourceIdentifier[] {
+    let ancestor = this.identifierStrategy.getParentContainer(identifier);
+    let ancestors: ResourceIdentifier[] = [];
+    ancestors.push(ancestor);
+    while(!this.identifierStrategy.isRootContainer(ancestor)) {
+      ancestor = this.identifierStrategy.getParentContainer(ancestor);
+      ancestors.push(ancestor);
+    }
+    return ancestors;
+  }
+
   /**
    * Checks if an agent is allowed to execute the requested actions.
    * Will throw an error if this is not the case.
    * @param input - Relevant data needed to check if access can be granted.
    */
   public async handle({ identifier, credentials, modes }: PermissionReaderInput):
-  Promise<PermissionSet> {
+  Promise<PermissionReaderOutput> {
     // Determine the required access modes
     this.logger.debug(`Retrieving permissions of ${credentials.agent?.webId} for ${identifier.path}`);
 
@@ -95,7 +108,7 @@ export class WebAclReader extends PermissionReader {
       permissions[CredentialGroup.public]!.delete =
         permissions[CredentialGroup.public]!.write && parentPermissions[CredentialGroup.public]!.write;
     }
-    return permissions;
+    return { permissions, ancestors: this.getAncestors(identifier) };
   }
 
   /**
diff --git a/src/authorization/permissions/N3PatchModesExtractor.ts b/src/authorization/permissions/N3PatchModesExtractor.ts
@@ -45,7 +45,11 @@ export class N3PatchModesExtractor extends ModesExtractor {
     // When ?insertions is non-empty, servers MUST (also) treat the request as an Append operation.
     if (inserts.length > 0) {
       accessModes.add(AccessMode.append);
+      // require Write on c/r and Append-or-Write on c/
+      // for PATCH-to-create c/r
+      // ref https://github.com/solid-contrib/test-suite/issues/146
       if (!await this.resourceSet.hasResource(target)) {
+        accessModes.add(AccessMode.write);
         accessModes.add(AccessMode.create);
       }
     }
diff --git a/src/server/AuthorizingHttpHandler.ts b/src/server/AuthorizingHttpHandler.ts
@@ -7,6 +7,9 @@ import type { ResponseDescription } from '../http/output/response/ResponseDescri
 import { getLoggerFor } from '../logging/LogUtil';
 import type { OperationHttpHandlerInput } from './OperationHttpHandler';
 import { OperationHttpHandler } from './OperationHttpHandler';
+import type { IdentifierStrategy } from '../util/identifiers/IdentifierStrategy';
+import type { ResourceSet } from '../storage/ResourceSet';
+import { AccessMode } from '../authorization/permissions/Permissions';
 
 export interface AuthorizingHttpHandlerArgs {
   /**
@@ -65,13 +68,44 @@ export class AuthorizingHttpHandler extends OperationHttpHandler {
 
     const modes = await this.modesExtractor.handleSafe(operation);
     this.logger.verbose(`Required modes are read: ${[ ...modes ].join(',')}`);
-
-    const permissionSet = await this.permissionReader.handleSafe({ credentials, identifier: operation.target, modes });
-    this.logger.verbose(`Available permissions are ${JSON.stringify(permissionSet)}`);
+    const permissionReaderOutput = await this.permissionReader.handleSafe({ credentials, identifier: operation.target, modes });
+    this.logger.verbose(`Available permissions are ${JSON.stringify(permissionReaderOutput)}`);
 
     try {
-      await this.authorizer.handleSafe({ credentials, identifier: operation.target, modes, permissionSet });
-      operation.permissionSet = permissionSet;
+      // check for effect on resource itself
+      await this.authorizer.handleSafe({
+        credentials,
+        identifier: operation.target,
+        modes,
+        permissionSet: permissionReaderOutput.permissions,
+        onlyIfNotExist: false
+      });
+
+      // if that didn't throw an error, check for any ancestors that may get created on the fly:
+      const ancestors = permissionReaderOutput.ancestors;
+
+      // TODO: only do ancestor existence check if the operation itself
+      // requires create
+      // TODO: as soon as an ancestor is found that does exist, we can
+      // assume that its ancestors also exist, so no further
+      // checks are needed, so we can break from the loop.
+      if (ancestors) {
+        // FIXME: move this into a special modes extractor for ancestors
+        const ancestorModes = new Set<AccessMode>();
+        ancestorModes.add(AccessMode.write);
+        ancestorModes.add(AccessMode.create);
+        for (let i = 0; i < ancestors.length; i++) {
+          const ancestorPermissions = await this.permissionReader.handleSafe({ credentials, identifier: ancestors[i], modes: ancestorModes });
+          await this.authorizer.handleSafe({
+            credentials,
+            identifier: ancestors[i],
+            modes: ancestorModes,
+            permissionSet: ancestorPermissions.permissions,
+            onlyIfNotExist: true,
+          });
+        }
+      }
+      operation.permissionSet = permissionReaderOutput.permissions;
     } catch (error: unknown) {
       this.logger.verbose(`Authorization failed: ${(error as any).message}`);
       throw error;

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,6 @@`
`1`	`1`	`import type { CredentialGroup } from '../authentication/Credentials';`
`2`		`-import type { PermissionReaderInput } from './PermissionReader';`
	`2`	`+import { ResourceIdentifier } from '../http/representation/ResourceIdentifier';`
	`3`	`+import type { PermissionReaderInput, PermissionReaderOutput } from './PermissionReader';`
`3`	`4`	`import { PermissionReader } from './PermissionReader';`
`4`	`5`	`import type { Permission, PermissionSet } from './permissions/Permissions';`
`5`	`6`
`@@ -21,13 +22,13 @@ export class AllStaticReader extends PermissionReader {`
`21`	`22`	`});`
`22`	`23`	`}`
`23`	`24`
`24`		`- public async handle({ credentials }: PermissionReaderInput): Promise<PermissionSet> {`
	`25`	`+ public async handle({ credentials }: PermissionReaderInput): Promise<PermissionReaderOutput> {`
`25`	`26`	`const result: PermissionSet = {};`
`26`	`27`	`for (const [ key, value ] of Object.entries(credentials) as [CredentialGroup, Permission][]) {`
`27`	`28`	`if (value) {`
`28`	`29`	`result[key] = this.permissions;`
`29`	`30`	`}`
`30`	`31`	`}`
`31`		`- return result;`
	`32`	`+ return { permissions:result };`
`32`	`33`	`}`
`33`	`34`	`}`
Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,11 @@ export class N3PatchModesExtractor extends ModesExtractor {`
`45`	`45`	`// When ?insertions is non-empty, servers MUST (also) treat the request as an Append operation.`
`46`	`46`	`if (inserts.length > 0) {`
`47`	`47`	`accessModes.add(AccessMode.append);`
	`48`	`+ // require Write on c/r and Append-or-Write on c/`
	`49`	`+ // for PATCH-to-create c/r`
	`50`	`+ // ref https://github.com/solid-contrib/test-suite/issues/146`
`48`	`51`	`if (!await this.resourceSet.hasResource(target)) {`
	`52`	`+ accessModes.add(AccessMode.write);`
`49`	`53`	`accessModes.add(AccessMode.create);`
`50`	`54`	`}`
`51`	`55`	`}`