From 8339413ab4d7b602710b95a5fc59ce3b83bb15f7 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Fri, 29 Jan 2021 16:48:09 +0100 Subject: [PATCH] feat: Add permissions extractor for acl resources --- config/presets/ldp/permissions-extractor.json | 6 ++++ src/authorization/WebAclAuthorizer.ts | 7 +--- src/index.ts | 1 + .../permissions/AclPermissionsExtractor.ts | 36 +++++++++++++++++++ test/integration/LdpHandlerWithAuth.test.ts | 25 +++++++++---- .../authorization/WebAclAuthorizer.test.ts | 12 ------- .../AclPermissionsExtractor.test.ts | 30 ++++++++++++++++ test/util/TestHelpers.ts | 3 +- 8 files changed, 94 insertions(+), 26 deletions(-) create mode 100644 src/ldp/permissions/AclPermissionsExtractor.ts create mode 100644 test/unit/ldp/permissions/AclPermissionsExtractor.test.ts diff --git a/config/presets/ldp/permissions-extractor.json b/config/presets/ldp/permissions-extractor.json index 46fc66b50..54aeb4d5c 100644 --- a/config/presets/ldp/permissions-extractor.json +++ b/config/presets/ldp/permissions-extractor.json @@ -5,6 +5,12 @@ "@id": "urn:solid-server:default:PermissionsExtractor", "@type": "WaterfallHandler", "WaterfallHandler:_handlers": [ + { + "@type": "AclPermissionsExtractor", + "AclPermissionsExtractor:_aclStrategy": { + "@id": "urn:solid-server:default:AclIdentifierStrategy" + } + }, { "@type": "MethodPermissionsExtractor" }, diff --git a/src/authorization/WebAclAuthorizer.ts b/src/authorization/WebAclAuthorizer.ts index fc232ac9e..5de04e1ef 100644 --- a/src/authorization/WebAclAuthorizer.ts +++ b/src/authorization/WebAclAuthorizer.ts @@ -42,12 +42,7 @@ export class WebAclAuthorizer extends Authorizer { * @param input - Relevant data needed to check if access can be granted. */ public async handle({ identifier, permissions, credentials }: AuthorizerArgs): Promise { - // Solid, §4.3.3: "To discover, read, create, or modify an ACL auxiliary resource, an acl:agent MUST - // have acl:Control privileges per the ACL inheritance algorithm on the resource directly associated with it." - // https://solid.github.io/specification/protocol#auxiliary-resources-reserved - const modes = this.aclStrategy.isAuxiliaryIdentifier(identifier) ? - [ 'control' ] : - (Object.keys(permissions) as (keyof PermissionSet)[]).filter((key): boolean => permissions[key]); + const modes = (Object.keys(permissions) as (keyof PermissionSet)[]).filter((key): boolean => permissions[key]); // Verify that all required modes are set for the given agent this.logger.debug(`Checking if ${credentials.webId} has ${modes.join()} permissions for ${identifier.path}`); diff --git a/src/index.ts b/src/index.ts index 6f4eb8dda..e83a69cf2 100644 --- a/src/index.ts +++ b/src/index.ts @@ -78,6 +78,7 @@ export * from './ldp/operations/PostOperationHandler'; export * from './ldp/operations/PutOperationHandler'; // LDP/Permissions +export * from './ldp/permissions/AclPermissionsExtractor'; export * from './ldp/permissions/PermissionSet'; export * from './ldp/permissions/PermissionsExtractor'; export * from './ldp/permissions/MethodPermissionsExtractor'; diff --git a/src/ldp/permissions/AclPermissionsExtractor.ts b/src/ldp/permissions/AclPermissionsExtractor.ts new file mode 100644 index 000000000..4744fa50c --- /dev/null +++ b/src/ldp/permissions/AclPermissionsExtractor.ts @@ -0,0 +1,36 @@ +import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError'; +import type { AuxiliaryIdentifierStrategy } from '../auxiliary/AuxiliaryIdentifierStrategy'; +import type { Operation } from '../operations/Operation'; +import type { PermissionSet } from './PermissionSet'; +import { PermissionsExtractor } from './PermissionsExtractor'; + +/** + * PermissionsExtractor specifically for acl resources. + * + * Solid, §4.3.3: "To discover, read, create, or modify an ACL auxiliary resource, an acl:agent MUST have + * acl:Control privileges per the ACL inheritance algorithm on the resource directly associated with it." + * https://solid.github.io/specification/protocol#auxiliary-resources-reserved + */ +export class AclPermissionsExtractor extends PermissionsExtractor { + private readonly aclStrategy: AuxiliaryIdentifierStrategy; + + public constructor(aclStrategy: AuxiliaryIdentifierStrategy) { + super(); + this.aclStrategy = aclStrategy; + } + + public async canHandle({ target }: Operation): Promise { + if (!this.aclStrategy.isAuxiliaryIdentifier(target)) { + throw new NotImplementedHttpError('Can only determine permissions of acl resources'); + } + } + + public async handle(): Promise { + return { + read: false, + write: false, + append: false, + control: true, + }; + } +} diff --git a/test/integration/LdpHandlerWithAuth.test.ts b/test/integration/LdpHandlerWithAuth.test.ts index 563eb42c3..061dfea7e 100644 --- a/test/integration/LdpHandlerWithAuth.test.ts +++ b/test/integration/LdpHandlerWithAuth.test.ts @@ -59,8 +59,7 @@ describe.each(stores)('An LDP handler with auth using %s', (name, { storeUrn, te await teardown(); }); - it('can add a file to the store, read it and delete it if allowed.', async(): - Promise => { + it('can add a file to the store, read it and delete it if allowed.', async(): Promise => { // Set acl await aclHelper.setSimpleAcl({ read: true, write: true, append: true, control: false }, 'agent'); @@ -82,8 +81,7 @@ describe.each(stores)('An LDP handler with auth using %s', (name, { storeUrn, te await resourceHelper.shouldNotExist(id); }); - it('can not add a file to the store if not allowed.', async(): - Promise => { + it('can not add a file to the store if not allowed.', async(): Promise => { // Set acl await aclHelper.setSimpleAcl({ read: true, write: true, append: true, control: false }, 'authenticated'); @@ -94,8 +92,7 @@ describe.each(stores)('An LDP handler with auth using %s', (name, { storeUrn, te expect(response.statusCode).toBe(401); }); - it('can not add/delete, but only read files if allowed.', async(): - Promise => { + it('can not add/delete, but only read files if allowed.', async(): Promise => { // Set acl await aclHelper.setSimpleAcl({ read: true, write: false, append: false, control: false }, 'agent'); @@ -135,4 +132,20 @@ describe.each(stores)('An LDP handler with auth using %s', (name, { storeUrn, te ); expect(response.statusCode).toBe(401); }); + + it('can not access an acl file if no control rights are provided.', async(): Promise => { + // Set acl + await aclHelper.setSimpleAcl({ read: true, write: true, append: true, control: false }, 'agent'); + + const response = await resourceHelper.performRequest(new URL('http://test.com/.acl'), 'GET', { accept: '*/*' }); + expect(response.statusCode).toBe(401); + }); + + it('can only access an acl file if control rights are provided.', async(): Promise => { + // Set acl + await aclHelper.setSimpleAcl({ read: false, write: false, append: false, control: true }, 'agent'); + + const response = await resourceHelper.performRequest(new URL('http://test.com/.acl'), 'GET', { accept: '*/*' }); + expect(response.statusCode).toBe(200); + }); }); diff --git a/test/unit/authorization/WebAclAuthorizer.test.ts b/test/unit/authorization/WebAclAuthorizer.test.ts index f574ba2c4..90b8c16d2 100644 --- a/test/unit/authorization/WebAclAuthorizer.test.ts +++ b/test/unit/authorization/WebAclAuthorizer.test.ts @@ -120,18 +120,6 @@ describe('A WebAclAuthorizer', (): void => { await expect(authorizer.handle({ identifier, permissions, credentials })).rejects.toThrow(ForbiddenHttpError); }); - it('allows access to the acl file if control is allowed.', async(): Promise => { - credentials.webId = 'http://test.com/user'; - identifier.path = 'http://test.com/foo'; - store.getRepresentation = async(): Promise => ({ data: streamifyArray([ - quad(nn('auth'), nn(`${acl}agent`), nn(credentials.webId!)), - quad(nn('auth'), nn(`${acl}accessTo`), nn(identifier.path)), - quad(nn('auth'), nn(`${acl}mode`), nn(`${acl}Control`)), - ]) } as Representation); - const aclIdentifier = aclStrategy.getAuxiliaryIdentifier(identifier); - await expect(authorizer.handle({ identifier: aclIdentifier, permissions, credentials })).resolves.toBeUndefined(); - }); - it('errors if an agent tries to edit the acl file without control permissions.', async(): Promise => { credentials.webId = 'http://test.com/user'; identifier.path = 'http://test.com/foo'; diff --git a/test/unit/ldp/permissions/AclPermissionsExtractor.test.ts b/test/unit/ldp/permissions/AclPermissionsExtractor.test.ts new file mode 100644 index 000000000..3b97b6001 --- /dev/null +++ b/test/unit/ldp/permissions/AclPermissionsExtractor.test.ts @@ -0,0 +1,30 @@ +import type { AuxiliaryIdentifierStrategy } from '../../../../src/ldp/auxiliary/AuxiliaryIdentifierStrategy'; +import { AclPermissionsExtractor } from '../../../../src/ldp/permissions/AclPermissionsExtractor'; +import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError'; + +describe('An AclPermissionsExtractor', (): void => { + let extractor: AclPermissionsExtractor; + + beforeEach(async(): Promise => { + const aclStrategy = { + isAuxiliaryIdentifier: (id): boolean => id.path.endsWith('.acl'), + } as AuxiliaryIdentifierStrategy; + extractor = new AclPermissionsExtractor(aclStrategy); + }); + + it('can only handle acl files.', async(): Promise => { + await expect(extractor.canHandle({ target: { path: 'http://test.com/foo' }} as any)) + .rejects.toThrow(NotImplementedHttpError); + await expect(extractor.canHandle({ target: { path: 'http://test.com/foo.acl' }} as any)) + .resolves.toBeUndefined(); + }); + + it('returns control permissions.', async(): Promise => { + await expect(extractor.handle()).resolves.toEqual({ + read: false, + write: false, + append: false, + control: true, + }); + }); +}); diff --git a/test/util/TestHelpers.ts b/test/util/TestHelpers.ts index 39e3c1f9e..861efe773 100644 --- a/test/util/TestHelpers.ts +++ b/test/util/TestHelpers.ts @@ -29,12 +29,11 @@ export class AclHelper { ' a acl:Authorization', ]; - for (const perm of [ 'Read', 'Append', 'Write', 'Delete' ]) { + for (const perm of [ 'Read', 'Append', 'Write', 'Control' ]) { if (permissions[perm.toLowerCase() as keyof PermissionSet]) { acl.push(`;\n acl:mode acl:${perm}`); } } - acl.push(';\n acl:mode acl:Control'); acl.push(`;\n acl:accessTo <${this.id}>`); acl.push(`;\n acl:default <${this.id}>`); acl.push(