Skip to content
This repository has been archived by the owner on Dec 6, 2023. It is now read-only.

Commit

Permalink
Merge pull request #14 from pdsinterop/fix-1339-1340
Browse files Browse the repository at this point in the history
Fix #1339 and #1340
  • Loading branch information
michielbdejong committed Jun 30, 2022
2 parents 9ecb769 + f2567f5 commit ce74e02
Show file tree
Hide file tree
Showing 11 changed files with 112 additions and 37 deletions.
7 changes: 4 additions & 3 deletions src/authorization/AllStaticReader.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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 };
}
}
5 changes: 5 additions & 0 deletions src/authorization/Authorizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
11 changes: 6 additions & 5 deletions src/authorization/AuxiliaryReader.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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 {
Expand Down
25 changes: 14 additions & 11 deletions src/authorization/OwnerPermissionReader.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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 }
};
}

/**
Expand Down
7 changes: 4 additions & 3 deletions src/authorization/PathBasedReader.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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) };
}

/**
Expand Down
6 changes: 5 additions & 1 deletion src/authorization/PermissionBasedAuthorizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
Expand Down
7 changes: 6 additions & 1 deletion src/authorization/PermissionReader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> {}
12 changes: 8 additions & 4 deletions src/authorization/UnionPermissionReader.ts
Original file line number Diff line number Diff line change
@@ -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';

/**
Expand All @@ -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 };
}

/**
Expand Down
21 changes: 17 additions & 4 deletions src/authorization/WebAclReader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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,
Expand All @@ -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}`);

Expand Down Expand Up @@ -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) };
}

/**
Expand Down
4 changes: 4 additions & 0 deletions src/authorization/permissions/N3PatchModesExtractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
44 changes: 39 additions & 5 deletions src/server/AuthorizingHttpHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
/**
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit ce74e02

Please sign in to comment.