Skip to content

Commit

Permalink
Added async capability for all Access Control resolvers (#2872)
Browse files Browse the repository at this point in the history
* Added async capability for all Access Control resolvers

* change all instances to micro-memoize

* fix linter

* fix prettier

* fix tests

* run format

* updated changeset messaging to better reflect major breaking changes

* Update .changeset/nine-crews-flow.md

Co-authored-by: Caleb Gray <caleb.gray@augustineinstitute.org>
Co-authored-by: Jess Telford <jess+github@jes.st>
  • Loading branch information
3 people authored May 15, 2020
1 parent c234ad4 commit 839666e
Show file tree
Hide file tree
Showing 13 changed files with 222 additions and 155 deletions.
34 changes: 34 additions & 0 deletions .changeset/nine-crews-flow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
'@keystonejs/access-control': major
'@keystonejs/keystone': major
'@keystonejs/logger': patch
---

Added async capability for all Access Control resolvers. This changes the below methods to async functions, returning Promises:

```
access-control
- validateCustomAccessControl
- validateListAccessControl
- validateFieldAccessControl
- validateAuthAccessControl
keystone/List
- checkFieldAccess
- checkListAccess
keystone/providers/custom
- computeAccess
keystone/providers/listAuth
- checkAccess
```

Changed `keystone/Keystone`'s `getGraphQlContext` return object (context) to include async resolvers for the following methods:
```
- context.getCustomAccessControlForUser
- context.getListAccessControlForUser
- context.getFieldAccessControlForUser
- context.getAuthAccessControlForUser
```
24 changes: 12 additions & 12 deletions packages/access-control/lib/access-control.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ module.exports = {
const accessTypes = [];
const parseAndValidate = access => {
const type = getType(access);
if (!['Boolean', 'Function', 'Object'].includes(type)) {
if (!['Boolean', 'AsyncFunction', 'Function', 'Object'].includes(type)) {
throw new Error(
`Expected a Boolean, Object, or Function for custom access, but got ${type}`
);
Expand Down Expand Up @@ -111,11 +111,11 @@ module.exports = {
}),
(type, accessType) => {
if (accessType === 'create') {
if (!['Boolean', 'Function'].includes(type)) {
if (!['Boolean', 'AsyncFunction', 'Function'].includes(type)) {
return `Expected a Boolean, or Function for ${listKey}.access.${accessType}, but got ${type}. (NOTE: 'create' cannot have a Declarative access control config)`;
}
} else {
if (!['Object', 'Boolean', 'Function'].includes(type)) {
if (!['Object', 'Boolean', 'AsyncFunction', 'Function'].includes(type)) {
return `Expected a Boolean, Object, or Function for ${listKey}.access.${accessType}, but got ${type}`;
}
}
Expand Down Expand Up @@ -143,21 +143,21 @@ module.exports = {
},
}),
(type, accessType) => {
if (!['Boolean', 'Function'].includes(type)) {
if (!['Boolean', 'AsyncFunction', 'Function'].includes(type)) {
return `Expected a Boolean or Function for ${listKey}.fields.${fieldKey}.access.${accessType}, but got ${type}. (NOTE: Fields cannot have declarative access control config)`;
}
}
);
return parseAccess({ schemaNames, accessTypes, access, defaultAccess, parseAndValidate });
},

validateCustomAccessControl({ access, authentication = {} }) {
async validateCustomAccessControl({ access, authentication = {} }) {
// Either a boolean or an object describing a where clause
let result;
if (typeof access !== 'function') {
result = access;
} else {
result = access({ authentication: authentication.item ? authentication : {} });
result = await access({ authentication: authentication.item ? authentication : {} });
}
const type = getType(result);

Expand All @@ -169,7 +169,7 @@ module.exports = {
return result;
},

validateListAccessControl({
async validateListAccessControl({
access,
listKey,
operation,
Expand All @@ -184,7 +184,7 @@ module.exports = {
if (typeof access[operation] !== 'function') {
result = access[operation];
} else {
result = access[operation]({
result = await access[operation]({
authentication: authentication.item ? authentication : {},
listKey,
operation,
Expand Down Expand Up @@ -213,7 +213,7 @@ module.exports = {
return result;
},

validateFieldAccessControl({
async validateFieldAccessControl({
access,
listKey,
fieldKey,
Expand All @@ -229,7 +229,7 @@ module.exports = {
if (typeof access[operation] !== 'function') {
result = access[operation];
} else {
result = access[operation]({
result = await access[operation]({
authentication: authentication.item ? authentication : {},
listKey,
fieldKey,
Expand All @@ -253,14 +253,14 @@ module.exports = {
return result;
},

validateAuthAccessControl({ access, listKey, authentication = {}, gqlName }) {
async validateAuthAccessControl({ access, listKey, authentication = {}, gqlName }) {
const operation = 'auth';
// Either a boolean or an object describing a where clause
let result;
if (typeof access[operation] !== 'function') {
result = access[operation];
} else {
result = access[operation]({
result = await access[operation]({
authentication: authentication.item ? authentication : {},
listKey,
operation,
Expand Down
119 changes: 68 additions & 51 deletions packages/access-control/tests/access-control.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,21 +253,29 @@ describe('Access control package tests', () => {
});
});

test('validateListAccessControl', () => {
test('validateListAccessControl', async () => {
let operation = 'read';
const access = { [operation]: true };

// Test the static case: returning a boolean
expect(validateListAccessControl({ access: { [operation]: true }, operation })).toBe(true);
expect(validateListAccessControl({ access: { [operation]: false }, operation })).toBe(false);
expect(() => validateListAccessControl({ access: { [operation]: 10 }, operation })).toThrow(
Error
);
await expect(
validateListAccessControl({ access: { [operation]: true }, operation })
).resolves.toBe(true);
await expect(
validateListAccessControl({ access: { [operation]: false }, operation })
).resolves.toBe(false);
await expect(
validateListAccessControl({ access: { [operation]: 10 }, operation })
).rejects.toThrow(Error);

const originalInput = {};
const accessFn = jest.fn(() => true);

validateListAccessControl({ access: { [operation]: accessFn }, operation, originalInput });
await validateListAccessControl({
access: { [operation]: accessFn },
operation,
originalInput,
});

expect(accessFn).toHaveBeenCalledTimes(1);
expect(accessFn).toHaveBeenCalledWith(
Expand All @@ -276,65 +284,70 @@ describe('Access control package tests', () => {
})
);

[{}, { item: {} }].forEach(authentication => {
const items = [{}, { item: {} }];
for (const authentication of items) {
operation = 'read';

// Boolean function
access[operation] = () => true;
expect(
await expect(
validateListAccessControl({
access: { [operation]: () => true },
operation,
authentication,
})
).toBe(true);
expect(
).resolves.toBe(true);
await expect(
validateListAccessControl({
access: { [operation]: () => false },
operation,
authentication,
})
).toBe(false);
).resolves.toBe(false);
// Object function
expect(
await expect(
validateListAccessControl({
access: { [operation]: () => ({ a: 1 }) },
operation,
authentication,
})
).toEqual({ a: 1 });
).resolves.toEqual({ a: 1 });

// Object function with create operation
operation = 'create';
expect(() =>
await expect(
validateListAccessControl({
access: { [operation]: () => ({ a: 1 }) },
operation,
authentication,
})
).toThrow(Error);
).rejects.toThrow(Error);

// Number function
expect(() =>
await expect(
validateListAccessControl({ access: { [operation]: () => 10 }, operation, authentication })
).toThrow(Error);
});
).rejects.toThrow(Error);
}
});

test('validateFieldAccessControl', () => {
test('validateFieldAccessControl', async () => {
const operation = 'read';
// Test the StaticAccess case: returning a boolean
expect(validateFieldAccessControl({ access: { [operation]: true }, operation })).toBe(true);
expect(validateFieldAccessControl({ access: { [operation]: false }, operation })).toBe(false);
expect(() => validateFieldAccessControl({ access: { [operation]: 10 }, operation })).toThrow(
Error
);
await expect(
validateFieldAccessControl({ access: { [operation]: true }, operation })
).resolves.toBe(true);
await expect(
validateFieldAccessControl({ access: { [operation]: false }, operation })
).resolves.toBe(false);
await expect(
validateFieldAccessControl({ access: { [operation]: 10 }, operation })
).rejects.toThrow(Error);

const originalInput = {};
const existingItem = {};
const accessFn = jest.fn(() => true);

validateFieldAccessControl({
await validateFieldAccessControl({
access: { [operation]: accessFn },
operation,
originalInput,
Expand All @@ -349,71 +362,75 @@ describe('Access control package tests', () => {
})
);

[{}, { item: {} }].forEach(authentication => {
const items = [{}, { item: {} }];
for (const authentication of items) {
// Test the ImperativeAccess case: a function which should return boolean
expect(
await expect(
validateFieldAccessControl({
access: { [operation]: () => true },
operation,
authentication,
})
).toBe(true);
).resolves.toBe(true);

expect(
await expect(
validateFieldAccessControl({
access: { [operation]: () => false },
operation,
authentication,
})
).toBe(false);
).resolves.toBe(false);

expect(() =>
await expect(
validateFieldAccessControl({ access: { [operation]: () => 10 }, operation, authentication })
).toThrow(Error);
});
).rejects.toThrow(Error);
}
});

test('validateAuthAccessControl', () => {
test('validateAuthAccessControl', async () => {
let operation = 'auth';
const access = { [operation]: true };

// Test the static case: returning a boolean
expect(validateAuthAccessControl({ access: { [operation]: true } })).toBe(true);
expect(validateAuthAccessControl({ access: { [operation]: false } })).toBe(false);
expect(() => validateAuthAccessControl({ access: { [operation]: 10 } })).toThrow(Error);
await expect(validateAuthAccessControl({ access: { [operation]: true } })).resolves.toBe(true);
await expect(validateAuthAccessControl({ access: { [operation]: false } })).resolves.toBe(
false
);
await expect(validateAuthAccessControl({ access: { [operation]: 10 } })).rejects.toThrow(Error);

const accessFn = jest.fn(() => true);

validateAuthAccessControl({ access: { [operation]: accessFn } });
await validateAuthAccessControl({ access: { [operation]: accessFn } });

expect(accessFn).toHaveBeenCalledTimes(1);

[{}, { item: {} }].forEach(authentication => {
const items = [{}, { item: {} }];
for (const authentication of items) {
operation = 'auth';

// Boolean function
access[operation] = () => true;
expect(
await expect(
validateAuthAccessControl({ access: { [operation]: () => true }, authentication })
).toBe(true);
expect(
).resolves.toBe(true);
await expect(
validateAuthAccessControl({ access: { [operation]: () => false }, authentication })
).toBe(false);
).resolves.toBe(false);
// Object function
expect(
await expect(
validateAuthAccessControl({ access: { [operation]: () => ({ a: 1 }) }, authentication })
).toEqual({ a: 1 });
).resolves.toEqual({ a: 1 });

// Object function with create operation
operation = 'create';
expect(() =>
await expect(
validateAuthAccessControl({ access: { [operation]: () => ({ a: 1 }) }, authentication })
).toThrow(Error);
).rejects.toThrow(Error);

// Number function
expect(() =>
await expect(
validateAuthAccessControl({ access: { [operation]: () => 10 }, authentication })
).toThrow(Error);
});
).rejects.toThrow(Error);
}
});
});
Loading

0 comments on commit 839666e

Please sign in to comment.