Skip to content

Commit 8f5d619

Browse files
committedOct 5, 2021
feat: Always grant control permissions to pod owners
1 parent 6c4ccb3 commit 8f5d619

File tree

10 files changed

+224
-6
lines changed

10 files changed

+224
-6
lines changed
 
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^1.0.0/components/context.jsonld",
3+
"@graph": [
4+
{
5+
"comment": "Allows pod owners to always edit permissions on the data.",
6+
"@id": "urn:solid-server:default:OwnerPermissionReader",
7+
"@type": "OwnerPermissionReader",
8+
"accountStore": { "@id": "urn:solid-server:auth:password:AccountStore" },
9+
"aclStrategy": { "@id": "urn:solid-server:default:AclStrategy" }
10+
}
11+
]
12+
}

‎config/ldp/authorization/webacl.json

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
{
22
"@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^1.0.0/components/context.jsonld",
33
"import": [
4-
"files-scs:config/ldp/authorization/readers/acl.json"
4+
"files-scs:config/ldp/authorization/readers/acl.json",
5+
"files-scs:config/ldp/authorization/readers/ownership.json"
56
],
67
"@graph": [
78
{
@@ -15,6 +16,7 @@
1516
"@type": "PathBasedReader",
1617
"baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" }
1718
},
19+
{ "@id": "urn:solid-server:default:OwnerPermissionReader" },
1820
{
1921
"comment": "This PermissionReader makes sure that for auxiliary resources, the main reader gets called with the associated identifier.",
2022
"@type": "AuxiliaryReader",
+70
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import { CredentialGroup } from '../authentication/Credentials';
2+
import type { AccountSettings, AccountStore } from '../identity/interaction/email-password/storage/AccountStore';
3+
import type { AuxiliaryIdentifierStrategy } from '../ldp/auxiliary/AuxiliaryIdentifierStrategy';
4+
import type { AclPermission } from '../ldp/permissions/AclPermission';
5+
import type { PermissionSet } from '../ldp/permissions/Permissions';
6+
import { getLoggerFor } from '../logging/LogUtil';
7+
import { createErrorMessage } from '../util/errors/ErrorUtil';
8+
import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError';
9+
import type { PermissionReaderInput } from './PermissionReader';
10+
import { PermissionReader } from './PermissionReader';
11+
12+
/**
13+
* Allows control access if the request is being made by the owner of the pod containing the resource.
14+
*/
15+
export class OwnerPermissionReader extends PermissionReader {
16+
protected readonly logger = getLoggerFor(this);
17+
18+
private readonly accountStore: AccountStore;
19+
private readonly aclStrategy: AuxiliaryIdentifierStrategy;
20+
21+
public constructor(accountStore: AccountStore, aclStrategy: AuxiliaryIdentifierStrategy) {
22+
super();
23+
this.accountStore = accountStore;
24+
this.aclStrategy = aclStrategy;
25+
}
26+
27+
public async handle(input: PermissionReaderInput): Promise<PermissionSet> {
28+
try {
29+
await this.ensurePodOwner(input);
30+
} catch (error: unknown) {
31+
this.logger.debug(`No pod owner Control permissions: ${createErrorMessage(error)}`);
32+
return {};
33+
}
34+
this.logger.debug(`Granting Control permissions to owner on ${input.identifier.path}`);
35+
36+
return { [CredentialGroup.agent]: {
37+
read: true,
38+
write: true,
39+
append: true,
40+
create: true,
41+
delete: true,
42+
control: true,
43+
} as AclPermission };
44+
}
45+
46+
/**
47+
* Verify that all conditions are fulfilled to give the owner access.
48+
*/
49+
private async ensurePodOwner({ credentials, identifier }: PermissionReaderInput): Promise<void> {
50+
// We only check ownership when an ACL resource is targeted to reduce the number of storage calls
51+
if (!this.aclStrategy.isAuxiliaryIdentifier(identifier)) {
52+
throw new NotImplementedHttpError('Exception is only granted when accessing ACL resources');
53+
}
54+
if (!credentials.agent?.webId) {
55+
throw new NotImplementedHttpError('Only authenticated agents could be owners');
56+
}
57+
let settings: AccountSettings;
58+
try {
59+
settings = await this.accountStore.getSettings(credentials.agent.webId);
60+
} catch {
61+
throw new NotImplementedHttpError('No account registered for this WebID');
62+
}
63+
if (!settings.podBaseUrl) {
64+
throw new NotImplementedHttpError('This agent has no pod on the server');
65+
}
66+
if (!identifier.path.startsWith(settings.podBaseUrl)) {
67+
throw new NotImplementedHttpError('Not targeting the pod owned by this agent');
68+
}
69+
}
70+
}

‎src/identity/interaction/email-password/storage/AccountStore.ts

+4
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ export interface AccountSettings {
66
* If this account can be used to identify as the corresponding WebID in the IDP.
77
*/
88
useIdp: boolean;
9+
/**
10+
* The base URL of the pod associated with this account, if there is one.
11+
*/
12+
podBaseUrl?: string;
913
}
1014

1115
/**

‎src/identity/interaction/email-password/util/RegistrationManager.ts

+1
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ export class RegistrationManager {
191191
// Register the account
192192
const settings: AccountSettings = {
193193
useIdp: input.register,
194+
podBaseUrl: podBaseUrl?.path,
194195
};
195196
await this.accountStore.create(input.email, input.webId!, input.password, settings);
196197

‎src/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export * from './authorization/access-checkers/AgentClassAccessChecker';
1515
export * from './authorization/access-checkers/AgentGroupAccessChecker';
1616

1717
// Authorization
18+
export * from './authorization/OwnerPermissionReader';
1819
export * from './authorization/AllStaticReader';
1920
export * from './authorization/Authorizer';
2021
export * from './authorization/AuxiliaryReader';

‎test/integration/Identity.test.ts

+37
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,43 @@ describe('A Solid server with IDP', (): void => {
328328
res = await state.session.fetch(newWebId, patchOptions);
329329
expect(res.status).toBe(205);
330330
});
331+
332+
it('always has control over data in the pod.', async(): Promise<void> => {
333+
const podBaseUrl = `${baseUrl}${podName}/`;
334+
const brokenAcl = '<#authorization> a <http://www.w3.org/ns/auth/acl#Authorization> .';
335+
336+
// Make the acl file unusable
337+
let res = await state.session.fetch(`${podBaseUrl}.acl`, {
338+
method: 'PUT',
339+
headers: { 'content-type': 'text/turtle' },
340+
body: brokenAcl,
341+
});
342+
expect(res.status).toBe(205);
343+
344+
// The owner is locked out of their own pod due to a faulty acl file
345+
res = await state.session.fetch(podBaseUrl);
346+
expect(res.status).toBe(403);
347+
348+
const fixedAcl = `@prefix acl: <http://www.w3.org/ns/auth/acl#>.
349+
@prefix foaf: <http://xmlns.com/foaf/0.1/>.
350+
351+
<#authorization>
352+
a acl:Authorization;
353+
acl:agentClass foaf:Agent;
354+
acl:mode acl:Read;
355+
acl:accessTo <./>.`;
356+
// Owner can still update the acl
357+
res = await state.session.fetch(`${podBaseUrl}.acl`, {
358+
method: 'PUT',
359+
headers: { 'content-type': 'text/turtle' },
360+
body: fixedAcl,
361+
});
362+
expect(res.status).toBe(205);
363+
364+
// Access is possible again
365+
res = await state.session.fetch(podBaseUrl);
366+
expect(res.status).toBe(200);
367+
});
331368
});
332369

333370
describe('setup', (): void => {

‎test/integration/config/ldp-with-auth.json

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
"files-scs:config/http/middleware/no-websockets.json",
99
"files-scs:config/http/server-factory/no-websockets.json",
1010
"files-scs:config/http/static/default.json",
11+
"files-scs:config/identity/handler/default.json",
1112
"files-scs:config/ldp/authentication/debug-auth-header.json",
1213
"files-scs:config/ldp/authorization/webacl.json",
1314
"files-scs:config/ldp/handler/default.json",
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
import type { CredentialSet } from '../../../src/authentication/Credentials';
2+
import { CredentialGroup } from '../../../src/authentication/Credentials';
3+
import { OwnerPermissionReader } from '../../../src/authorization/OwnerPermissionReader';
4+
import type {
5+
AccountSettings,
6+
AccountStore,
7+
} from '../../../src/identity/interaction/email-password/storage/AccountStore';
8+
import type { AuxiliaryIdentifierStrategy } from '../../../src/ldp/auxiliary/AuxiliaryIdentifierStrategy';
9+
import type { ResourceIdentifier } from '../../../src/ldp/representation/ResourceIdentifier';
10+
11+
describe('An OwnerPermissionReader', (): void => {
12+
const owner = 'http://test.com/alice/profile/card#me';
13+
const podBaseUrl = 'http://test.com/alice/';
14+
let credentials: CredentialSet;
15+
let identifier: ResourceIdentifier;
16+
let settings: AccountSettings;
17+
let accountStore: jest.Mocked<AccountStore>;
18+
let aclStrategy: jest.Mocked<AuxiliaryIdentifierStrategy>;
19+
let reader: OwnerPermissionReader;
20+
21+
beforeEach(async(): Promise<void> => {
22+
credentials = { [CredentialGroup.agent]: { webId: owner }};
23+
24+
identifier = { path: `${podBaseUrl}.acl` };
25+
26+
settings = {
27+
useIdp: true,
28+
podBaseUrl,
29+
};
30+
31+
accountStore = {
32+
getSettings: jest.fn(async(webId: string): Promise<AccountSettings> => {
33+
if (webId === owner) {
34+
return settings;
35+
}
36+
throw new Error('No account');
37+
}),
38+
} as any;
39+
40+
aclStrategy = {
41+
isAuxiliaryIdentifier: jest.fn((id): boolean => id.path.endsWith('.acl')),
42+
} as any;
43+
44+
reader = new OwnerPermissionReader(accountStore, aclStrategy);
45+
});
46+
47+
it('returns empty permissions for non-ACL resources.', async(): Promise<void> => {
48+
identifier.path = podBaseUrl;
49+
await expect(reader.handle({ credentials, identifier })).resolves.toEqual({});
50+
});
51+
52+
it('returns empty permissions if there is no agent WebID.', async(): Promise<void> => {
53+
credentials = {};
54+
await expect(reader.handle({ credentials, identifier })).resolves.toEqual({});
55+
});
56+
57+
it('returns empty permissions if the agent has no account.', async(): Promise<void> => {
58+
credentials.agent!.webId = 'http://test.com/someone/else';
59+
await expect(reader.handle({ credentials, identifier })).resolves.toEqual({});
60+
});
61+
62+
it('returns empty permissions if the account has no pod.', async(): Promise<void> => {
63+
delete settings.podBaseUrl;
64+
await expect(reader.handle({ credentials, identifier })).resolves.toEqual({});
65+
});
66+
67+
it('returns empty permissions if the target identifier is not in the pod.', async(): Promise<void> => {
68+
identifier.path = 'http://somewhere.else/.acl';
69+
await expect(reader.handle({ credentials, identifier })).resolves.toEqual({});
70+
});
71+
72+
it('returns full permissions if the owner is accessing an ACL resource in their pod.', async(): Promise<void> => {
73+
await expect(reader.handle({ credentials, identifier })).resolves.toEqual({
74+
[CredentialGroup.agent]: {
75+
read: true,
76+
write: true,
77+
append: true,
78+
create: true,
79+
delete: true,
80+
control: true,
81+
},
82+
});
83+
});
84+
});

‎test/unit/identity/interaction/email-password/util/RegistrationManager.test.ts

+11-5
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ describe('A RegistrationManager', (): void => {
200200
expect(podManager.createPod).toHaveBeenCalledTimes(1);
201201
expect(podManager.createPod).toHaveBeenLastCalledWith({ path: `${baseUrl}${podName}/` }, podSettings, false);
202202
expect(accountStore.create).toHaveBeenCalledTimes(1);
203-
expect(accountStore.create).toHaveBeenLastCalledWith(email, webId, password, { useIdp: false });
203+
expect(accountStore.create).toHaveBeenLastCalledWith(email, webId, password, { useIdp: false, podBaseUrl });
204204
expect(accountStore.verify).toHaveBeenCalledTimes(1);
205205

206206
expect(accountStore.deleteAccount).toHaveBeenCalledTimes(0);
@@ -222,7 +222,7 @@ describe('A RegistrationManager', (): void => {
222222
expect(ownershipValidator.handleSafe).toHaveBeenCalledTimes(1);
223223
expect(ownershipValidator.handleSafe).toHaveBeenLastCalledWith({ webId });
224224
expect(accountStore.create).toHaveBeenCalledTimes(1);
225-
expect(accountStore.create).toHaveBeenLastCalledWith(email, webId, password, { useIdp: true });
225+
expect(accountStore.create).toHaveBeenLastCalledWith(email, webId, password, { useIdp: true, podBaseUrl });
226226
expect(identifierGenerator.generate).toHaveBeenCalledTimes(1);
227227
expect(identifierGenerator.generate).toHaveBeenLastCalledWith(podName);
228228
expect(podManager.createPod).toHaveBeenCalledTimes(1);
@@ -242,7 +242,7 @@ describe('A RegistrationManager', (): void => {
242242
expect(ownershipValidator.handleSafe).toHaveBeenCalledTimes(1);
243243
expect(ownershipValidator.handleSafe).toHaveBeenLastCalledWith({ webId });
244244
expect(accountStore.create).toHaveBeenCalledTimes(1);
245-
expect(accountStore.create).toHaveBeenLastCalledWith(email, webId, password, { useIdp: true });
245+
expect(accountStore.create).toHaveBeenLastCalledWith(email, webId, password, { useIdp: true, podBaseUrl });
246246
expect(identifierGenerator.generate).toHaveBeenCalledTimes(1);
247247
expect(identifierGenerator.generate).toHaveBeenLastCalledWith(podName);
248248
expect(podManager.createPod).toHaveBeenCalledTimes(1);
@@ -272,7 +272,10 @@ describe('A RegistrationManager', (): void => {
272272
expect(identifierGenerator.generate).toHaveBeenCalledTimes(1);
273273
expect(identifierGenerator.generate).toHaveBeenLastCalledWith(podName);
274274
expect(accountStore.create).toHaveBeenCalledTimes(1);
275-
expect(accountStore.create).toHaveBeenLastCalledWith(email, generatedWebID, password, { useIdp: true });
275+
expect(accountStore.create).toHaveBeenLastCalledWith(email,
276+
generatedWebID,
277+
password,
278+
{ useIdp: true, podBaseUrl });
276279
expect(accountStore.verify).toHaveBeenCalledTimes(1);
277280
expect(accountStore.verify).toHaveBeenLastCalledWith(email);
278281
expect(podManager.createPod).toHaveBeenCalledTimes(1);
@@ -300,7 +303,10 @@ describe('A RegistrationManager', (): void => {
300303
expect(podManager.createPod).toHaveBeenCalledTimes(1);
301304
expect(podManager.createPod).toHaveBeenLastCalledWith({ path: baseUrl }, podSettings, true);
302305
expect(accountStore.create).toHaveBeenCalledTimes(1);
303-
expect(accountStore.create).toHaveBeenLastCalledWith(email, webId, password, { useIdp: false });
306+
expect(accountStore.create).toHaveBeenLastCalledWith(email,
307+
webId,
308+
password,
309+
{ useIdp: false, podBaseUrl: baseUrl });
304310
expect(accountStore.verify).toHaveBeenCalledTimes(1);
305311

306312
expect(identifierGenerator.generate).toHaveBeenCalledTimes(0);

0 commit comments

Comments
 (0)
Please sign in to comment.