From 8f5d61911d771c623cfb20b0ecded3ea913fc899 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Tue, 31 Aug 2021 16:36:42 +0200 Subject: [PATCH] feat: Always grant control permissions to pod owners --- .../ldp/authorization/readers/ownership.json | 12 +++ config/ldp/authorization/webacl.json | 4 +- src/authorization/OwnerPermissionReader.ts | 70 ++++++++++++++++ .../email-password/storage/AccountStore.ts | 4 + .../util/RegistrationManager.ts | 1 + src/index.ts | 1 + test/integration/Identity.test.ts | 37 ++++++++ test/integration/config/ldp-with-auth.json | 1 + .../OwnerPermissionReader.test.ts | 84 +++++++++++++++++++ .../util/RegistrationManager.test.ts | 16 ++-- 10 files changed, 224 insertions(+), 6 deletions(-) create mode 100644 config/ldp/authorization/readers/ownership.json create mode 100644 src/authorization/OwnerPermissionReader.ts create mode 100644 test/unit/authorization/OwnerPermissionReader.test.ts diff --git a/config/ldp/authorization/readers/ownership.json b/config/ldp/authorization/readers/ownership.json new file mode 100644 index 000000000..6c94b0330 --- /dev/null +++ b/config/ldp/authorization/readers/ownership.json @@ -0,0 +1,12 @@ +{ + "@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^1.0.0/components/context.jsonld", + "@graph": [ + { + "comment": "Allows pod owners to always edit permissions on the data.", + "@id": "urn:solid-server:default:OwnerPermissionReader", + "@type": "OwnerPermissionReader", + "accountStore": { "@id": "urn:solid-server:auth:password:AccountStore" }, + "aclStrategy": { "@id": "urn:solid-server:default:AclStrategy" } + } + ] +} diff --git a/config/ldp/authorization/webacl.json b/config/ldp/authorization/webacl.json index 8b25b8648..286d7db7b 100644 --- a/config/ldp/authorization/webacl.json +++ b/config/ldp/authorization/webacl.json @@ -1,7 +1,8 @@ { "@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^1.0.0/components/context.jsonld", "import": [ - "files-scs:config/ldp/authorization/readers/acl.json" + "files-scs:config/ldp/authorization/readers/acl.json", + "files-scs:config/ldp/authorization/readers/ownership.json" ], "@graph": [ { @@ -15,6 +16,7 @@ "@type": "PathBasedReader", "baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" } }, + { "@id": "urn:solid-server:default:OwnerPermissionReader" }, { "comment": "This PermissionReader makes sure that for auxiliary resources, the main reader gets called with the associated identifier.", "@type": "AuxiliaryReader", diff --git a/src/authorization/OwnerPermissionReader.ts b/src/authorization/OwnerPermissionReader.ts new file mode 100644 index 000000000..f0b3b7549 --- /dev/null +++ b/src/authorization/OwnerPermissionReader.ts @@ -0,0 +1,70 @@ +import { CredentialGroup } from '../authentication/Credentials'; +import type { AccountSettings, AccountStore } from '../identity/interaction/email-password/storage/AccountStore'; +import type { AuxiliaryIdentifierStrategy } from '../ldp/auxiliary/AuxiliaryIdentifierStrategy'; +import type { AclPermission } from '../ldp/permissions/AclPermission'; +import type { PermissionSet } from '../ldp/permissions/Permissions'; +import { getLoggerFor } from '../logging/LogUtil'; +import { createErrorMessage } from '../util/errors/ErrorUtil'; +import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError'; +import type { PermissionReaderInput } from './PermissionReader'; +import { PermissionReader } from './PermissionReader'; + +/** + * Allows control access if the request is being made by the owner of the pod containing the resource. + */ +export class OwnerPermissionReader extends PermissionReader { + protected readonly logger = getLoggerFor(this); + + private readonly accountStore: AccountStore; + private readonly aclStrategy: AuxiliaryIdentifierStrategy; + + public constructor(accountStore: AccountStore, aclStrategy: AuxiliaryIdentifierStrategy) { + super(); + this.accountStore = accountStore; + this.aclStrategy = aclStrategy; + } + + 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 {}; + } + 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 }; + } + + /** + * Verify that all conditions are fulfilled to give the owner access. + */ + private async ensurePodOwner({ credentials, identifier }: PermissionReaderInput): Promise { + // We only check ownership when an ACL resource is targeted to reduce the number of storage calls + if (!this.aclStrategy.isAuxiliaryIdentifier(identifier)) { + throw new NotImplementedHttpError('Exception is only granted when accessing ACL resources'); + } + if (!credentials.agent?.webId) { + throw new NotImplementedHttpError('Only authenticated agents could be owners'); + } + let settings: AccountSettings; + try { + settings = await this.accountStore.getSettings(credentials.agent.webId); + } catch { + throw new NotImplementedHttpError('No account registered for this WebID'); + } + if (!settings.podBaseUrl) { + throw new NotImplementedHttpError('This agent has no pod on the server'); + } + if (!identifier.path.startsWith(settings.podBaseUrl)) { + throw new NotImplementedHttpError('Not targeting the pod owned by this agent'); + } + } +} diff --git a/src/identity/interaction/email-password/storage/AccountStore.ts b/src/identity/interaction/email-password/storage/AccountStore.ts index 6de898163..f21a0cb2b 100644 --- a/src/identity/interaction/email-password/storage/AccountStore.ts +++ b/src/identity/interaction/email-password/storage/AccountStore.ts @@ -6,6 +6,10 @@ export interface AccountSettings { * If this account can be used to identify as the corresponding WebID in the IDP. */ useIdp: boolean; + /** + * The base URL of the pod associated with this account, if there is one. + */ + podBaseUrl?: string; } /** diff --git a/src/identity/interaction/email-password/util/RegistrationManager.ts b/src/identity/interaction/email-password/util/RegistrationManager.ts index 476ffab23..59b05bf9d 100644 --- a/src/identity/interaction/email-password/util/RegistrationManager.ts +++ b/src/identity/interaction/email-password/util/RegistrationManager.ts @@ -191,6 +191,7 @@ export class RegistrationManager { // Register the account const settings: AccountSettings = { useIdp: input.register, + podBaseUrl: podBaseUrl?.path, }; await this.accountStore.create(input.email, input.webId!, input.password, settings); diff --git a/src/index.ts b/src/index.ts index 87540bdf6..d9940540e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -15,6 +15,7 @@ export * from './authorization/access-checkers/AgentClassAccessChecker'; export * from './authorization/access-checkers/AgentGroupAccessChecker'; // Authorization +export * from './authorization/OwnerPermissionReader'; export * from './authorization/AllStaticReader'; export * from './authorization/Authorizer'; export * from './authorization/AuxiliaryReader'; diff --git a/test/integration/Identity.test.ts b/test/integration/Identity.test.ts index 2c1026a9d..04d8dbc84 100644 --- a/test/integration/Identity.test.ts +++ b/test/integration/Identity.test.ts @@ -328,6 +328,43 @@ describe('A Solid server with IDP', (): void => { res = await state.session.fetch(newWebId, patchOptions); expect(res.status).toBe(205); }); + + it('always has control over data in the pod.', async(): Promise => { + const podBaseUrl = `${baseUrl}${podName}/`; + const brokenAcl = '<#authorization> a .'; + + // Make the acl file unusable + let res = await state.session.fetch(`${podBaseUrl}.acl`, { + method: 'PUT', + headers: { 'content-type': 'text/turtle' }, + body: brokenAcl, + }); + expect(res.status).toBe(205); + + // The owner is locked out of their own pod due to a faulty acl file + res = await state.session.fetch(podBaseUrl); + expect(res.status).toBe(403); + + const fixedAcl = `@prefix acl: . +@prefix foaf: . + +<#authorization> + a acl:Authorization; + acl:agentClass foaf:Agent; + acl:mode acl:Read; + acl:accessTo <./>.`; + // Owner can still update the acl + res = await state.session.fetch(`${podBaseUrl}.acl`, { + method: 'PUT', + headers: { 'content-type': 'text/turtle' }, + body: fixedAcl, + }); + expect(res.status).toBe(205); + + // Access is possible again + res = await state.session.fetch(podBaseUrl); + expect(res.status).toBe(200); + }); }); describe('setup', (): void => { diff --git a/test/integration/config/ldp-with-auth.json b/test/integration/config/ldp-with-auth.json index 5e994f547..7f2266b07 100644 --- a/test/integration/config/ldp-with-auth.json +++ b/test/integration/config/ldp-with-auth.json @@ -8,6 +8,7 @@ "files-scs:config/http/middleware/no-websockets.json", "files-scs:config/http/server-factory/no-websockets.json", "files-scs:config/http/static/default.json", + "files-scs:config/identity/handler/default.json", "files-scs:config/ldp/authentication/debug-auth-header.json", "files-scs:config/ldp/authorization/webacl.json", "files-scs:config/ldp/handler/default.json", diff --git a/test/unit/authorization/OwnerPermissionReader.test.ts b/test/unit/authorization/OwnerPermissionReader.test.ts new file mode 100644 index 000000000..903614fc1 --- /dev/null +++ b/test/unit/authorization/OwnerPermissionReader.test.ts @@ -0,0 +1,84 @@ +import type { CredentialSet } from '../../../src/authentication/Credentials'; +import { CredentialGroup } from '../../../src/authentication/Credentials'; +import { OwnerPermissionReader } from '../../../src/authorization/OwnerPermissionReader'; +import type { + AccountSettings, + AccountStore, +} from '../../../src/identity/interaction/email-password/storage/AccountStore'; +import type { AuxiliaryIdentifierStrategy } from '../../../src/ldp/auxiliary/AuxiliaryIdentifierStrategy'; +import type { ResourceIdentifier } from '../../../src/ldp/representation/ResourceIdentifier'; + +describe('An OwnerPermissionReader', (): void => { + const owner = 'http://test.com/alice/profile/card#me'; + const podBaseUrl = 'http://test.com/alice/'; + let credentials: CredentialSet; + let identifier: ResourceIdentifier; + let settings: AccountSettings; + let accountStore: jest.Mocked; + let aclStrategy: jest.Mocked; + let reader: OwnerPermissionReader; + + beforeEach(async(): Promise => { + credentials = { [CredentialGroup.agent]: { webId: owner }}; + + identifier = { path: `${podBaseUrl}.acl` }; + + settings = { + useIdp: true, + podBaseUrl, + }; + + accountStore = { + getSettings: jest.fn(async(webId: string): Promise => { + if (webId === owner) { + return settings; + } + throw new Error('No account'); + }), + } as any; + + aclStrategy = { + isAuxiliaryIdentifier: jest.fn((id): boolean => id.path.endsWith('.acl')), + } as any; + + reader = new OwnerPermissionReader(accountStore, aclStrategy); + }); + + it('returns empty permissions for non-ACL resources.', async(): Promise => { + identifier.path = podBaseUrl; + await expect(reader.handle({ credentials, identifier })).resolves.toEqual({}); + }); + + it('returns empty permissions if there is no agent WebID.', async(): Promise => { + credentials = {}; + await expect(reader.handle({ credentials, identifier })).resolves.toEqual({}); + }); + + it('returns empty permissions if the agent has no account.', async(): Promise => { + credentials.agent!.webId = 'http://test.com/someone/else'; + await expect(reader.handle({ credentials, identifier })).resolves.toEqual({}); + }); + + it('returns empty permissions if the account has no pod.', async(): Promise => { + delete settings.podBaseUrl; + await expect(reader.handle({ credentials, identifier })).resolves.toEqual({}); + }); + + it('returns empty permissions if the target identifier is not in the pod.', async(): Promise => { + identifier.path = 'http://somewhere.else/.acl'; + await expect(reader.handle({ credentials, identifier })).resolves.toEqual({}); + }); + + it('returns full permissions if the owner is accessing an ACL resource in their pod.', async(): Promise => { + await expect(reader.handle({ credentials, identifier })).resolves.toEqual({ + [CredentialGroup.agent]: { + read: true, + write: true, + append: true, + create: true, + delete: true, + control: true, + }, + }); + }); +}); diff --git a/test/unit/identity/interaction/email-password/util/RegistrationManager.test.ts b/test/unit/identity/interaction/email-password/util/RegistrationManager.test.ts index b99f3f0ba..c1208e439 100644 --- a/test/unit/identity/interaction/email-password/util/RegistrationManager.test.ts +++ b/test/unit/identity/interaction/email-password/util/RegistrationManager.test.ts @@ -200,7 +200,7 @@ describe('A RegistrationManager', (): void => { expect(podManager.createPod).toHaveBeenCalledTimes(1); expect(podManager.createPod).toHaveBeenLastCalledWith({ path: `${baseUrl}${podName}/` }, podSettings, false); expect(accountStore.create).toHaveBeenCalledTimes(1); - expect(accountStore.create).toHaveBeenLastCalledWith(email, webId, password, { useIdp: false }); + expect(accountStore.create).toHaveBeenLastCalledWith(email, webId, password, { useIdp: false, podBaseUrl }); expect(accountStore.verify).toHaveBeenCalledTimes(1); expect(accountStore.deleteAccount).toHaveBeenCalledTimes(0); @@ -222,7 +222,7 @@ describe('A RegistrationManager', (): void => { expect(ownershipValidator.handleSafe).toHaveBeenCalledTimes(1); expect(ownershipValidator.handleSafe).toHaveBeenLastCalledWith({ webId }); expect(accountStore.create).toHaveBeenCalledTimes(1); - expect(accountStore.create).toHaveBeenLastCalledWith(email, webId, password, { useIdp: true }); + expect(accountStore.create).toHaveBeenLastCalledWith(email, webId, password, { useIdp: true, podBaseUrl }); expect(identifierGenerator.generate).toHaveBeenCalledTimes(1); expect(identifierGenerator.generate).toHaveBeenLastCalledWith(podName); expect(podManager.createPod).toHaveBeenCalledTimes(1); @@ -242,7 +242,7 @@ describe('A RegistrationManager', (): void => { expect(ownershipValidator.handleSafe).toHaveBeenCalledTimes(1); expect(ownershipValidator.handleSafe).toHaveBeenLastCalledWith({ webId }); expect(accountStore.create).toHaveBeenCalledTimes(1); - expect(accountStore.create).toHaveBeenLastCalledWith(email, webId, password, { useIdp: true }); + expect(accountStore.create).toHaveBeenLastCalledWith(email, webId, password, { useIdp: true, podBaseUrl }); expect(identifierGenerator.generate).toHaveBeenCalledTimes(1); expect(identifierGenerator.generate).toHaveBeenLastCalledWith(podName); expect(podManager.createPod).toHaveBeenCalledTimes(1); @@ -272,7 +272,10 @@ describe('A RegistrationManager', (): void => { expect(identifierGenerator.generate).toHaveBeenCalledTimes(1); expect(identifierGenerator.generate).toHaveBeenLastCalledWith(podName); expect(accountStore.create).toHaveBeenCalledTimes(1); - expect(accountStore.create).toHaveBeenLastCalledWith(email, generatedWebID, password, { useIdp: true }); + expect(accountStore.create).toHaveBeenLastCalledWith(email, + generatedWebID, + password, + { useIdp: true, podBaseUrl }); expect(accountStore.verify).toHaveBeenCalledTimes(1); expect(accountStore.verify).toHaveBeenLastCalledWith(email); expect(podManager.createPod).toHaveBeenCalledTimes(1); @@ -300,7 +303,10 @@ describe('A RegistrationManager', (): void => { expect(podManager.createPod).toHaveBeenCalledTimes(1); expect(podManager.createPod).toHaveBeenLastCalledWith({ path: baseUrl }, podSettings, true); expect(accountStore.create).toHaveBeenCalledTimes(1); - expect(accountStore.create).toHaveBeenLastCalledWith(email, webId, password, { useIdp: false }); + expect(accountStore.create).toHaveBeenLastCalledWith(email, + webId, + password, + { useIdp: false, podBaseUrl: baseUrl }); expect(accountStore.verify).toHaveBeenCalledTimes(1); expect(identifierGenerator.generate).toHaveBeenCalledTimes(0);