Skip to content

Commit

Permalink
Always have read permission on config
Browse files Browse the repository at this point in the history
  • Loading branch information
bosschaert committed Jan 10, 2025
1 parent a7948cc commit bda99ac
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 5 deletions.
9 changes: 6 additions & 3 deletions src/routes/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ export async function postConfig({ req, env, daCtx }) {
}

export async function getConfig({ env, daCtx }) {
if (!hasPermission(daCtx, 'CONFIG', 'read', true)) {
return { status: 403 };
}
// // TODO maybe we should turn the order around?
// if (!hasPermission(daCtx, 'CONFIG', 'read', true)) {
// const key = daCtx.key.startsWith('/') ? daCtx.key : `/${daCtx.key}`
// if (!hasPermission(daCtx, key, 'read', true)) { // TODO
// return { status: 403 };
// }
return getKv(env, daCtx);
}
14 changes: 14 additions & 0 deletions src/utils/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,20 @@ export async function getAclCtx(env, org, users, key, api) {
// Expose the action trace or not?
actionTrace = users.every((u) => aclTrace.includes(u.email)) ? actionTrace : undefined;

if (k === 'CONFIG') {
actionSet.add('read');
}

// // TODO maybe we should turn the order around because it's more likely to have read
// // permission on the content than explicitly on CONFIG

// // If the user doesn't have read persmissions on config, get them from the content
// const pathActions = getAllUserActions(pathLookup, users, key.startsWith('/') ? key : `/${key}`);

Check failure on line 201 in src/utils/auth.js

View workflow job for this annotation

GitHub Actions / Running tests (20.x)

This line has a length of 101. Maximum allowed is 100
// if (pathActions.actionSet.has('read')) {
// actionSet.add('read');
// actionTrace = pathActions.actionTrace;
// }

return { pathLookup, actionSet, actionTrace };
}

Expand Down
3 changes: 1 addition & 2 deletions test/routes/config.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,7 @@ describe('Config', () => {
);

const res = await getConfig({ env, daCtx: ctx });
assert.strictEqual(res.status, 403);
assert.strictEqual(getKVCalled.length, 0);
assert.strictEqual(getKVCalled.length, 1, "Should always have get permission on config");

const res2 = await postConfig({ req, env, daCtx: ctx });
assert.strictEqual(res2.status, 403);
Expand Down
6 changes: 6 additions & 0 deletions test/utils/auth.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,12 @@ describe('DA auth', () => {
assert(hasPermission({users, org: 'test', aclCtx, key: ''}, 'CONFIG', 'write', true));
assert(hasPermission({users, org: 'test', aclCtx, key: '/somewhere'}, 'CONFIG', 'write', true));
});

it('test CONFIG always has read permission', async () => {
const users = [{ident: "blah@foo.org"}];
const aclCtx = await getAclCtx(env2, 'test', users, '/', 'config');
assert(aclCtx.actionSet.has('read'));
})
});

describe('persmissions single sheet', () => {
Expand Down

0 comments on commit bda99ac

Please sign in to comment.