From 3462caa2b6d141a5d56f756d35f5df280610f0bf Mon Sep 17 00:00:00 2001 From: Michiel de Jong Date: Wed, 29 Jun 2022 14:23:35 +0200 Subject: [PATCH 1/2] Require Write on c/r for PATCH-to-create, fix https://github.com/solid-contrib/test-suite/issues/146 --- src/authorization/permissions/N3PatchModesExtractor.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/authorization/permissions/N3PatchModesExtractor.ts b/src/authorization/permissions/N3PatchModesExtractor.ts index 659346dc1f..b446bbea22 100644 --- 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); } } From f2567f579af12337be294b88df57e58b8c6277cf Mon Sep 17 00:00:00 2001 From: Michiel de Jong Date: Wed, 29 Jun 2022 15:06:06 +0200 Subject: [PATCH 2/2] Check ancestor create permissions, fix https://github.com/solid-contrib/test-suite/issues/145 --- src/authorization/AllStaticReader.ts | 7 +-- src/authorization/Authorizer.ts | 5 +++ src/authorization/AuxiliaryReader.ts | 11 ++--- src/authorization/OwnerPermissionReader.ts | 25 ++++++----- src/authorization/PathBasedReader.ts | 7 +-- .../PermissionBasedAuthorizer.ts | 6 ++- src/authorization/PermissionReader.ts | 7 ++- src/authorization/UnionPermissionReader.ts | 12 +++-- src/authorization/WebAclReader.ts | 21 +++++++-- src/server/AuthorizingHttpHandler.ts | 44 ++++++++++++++++--- 10 files changed, 108 insertions(+), 37 deletions(-) diff --git a/src/authorization/AllStaticReader.ts b/src/authorization/AllStaticReader.ts index 9b98226241..d8ff5f9f0b 100644 --- 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 { + public async handle({ credentials }: PermissionReaderInput): Promise { 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 index eeb5c4f04d..ec36d7d787 100644 --- 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 index 092a24bc56..c5d9d4cfb4 100644 --- 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 { + public async handle(auxiliaryAuth: PermissionReaderInput): Promise { 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 { + public async handleSafe(auxiliaryAuth: PermissionReaderInput): Promise { 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 index c752cd1c4e..3851bdc0ae 100644 --- 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 { + public async handle(input: PermissionReaderInput): Promise { 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 index 7e8ea1e2b2..e8d5783a3f 100644 --- 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 { + public async handle(input: PermissionReaderInput): Promise { 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 index d71eacce3b..69181776dc 100644 --- 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 { - 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 index c615b65139..b0f89d0e30 100644 --- a/src/authorization/PermissionReader.ts +++ b/src/authorization/PermissionReader.ts @@ -20,7 +20,12 @@ export interface PermissionReaderInput { modes: Set; } +export interface PermissionReaderOutput { + permissions: PermissionSet; + ancestors?: ResourceIdentifier[]; +} + /** * Discovers the permissions of the given credentials on the given identifier. */ -export abstract class PermissionReader extends AsyncHandler {} +export abstract class PermissionReader extends AsyncHandler {} diff --git a/src/authorization/UnionPermissionReader.ts b/src/authorization/UnionPermissionReader.ts index 34bd417432..d9e52e7b30 100644 --- 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 { super(readers); } - protected async combine(results: PermissionSet[]): Promise { + protected async combine(results: PermissionReaderOutput[]): Promise { 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 index f2ef65b4a1..62789f1b4b 100644 --- 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 { + Promise { // 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/server/AuthorizingHttpHandler.ts b/src/server/AuthorizingHttpHandler.ts index b23ea35845..96f5b49ec5 100644 --- 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(); + 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;