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

Fix #1339 and #1340 #14

Merged
merged 2 commits into from
Jun 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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