Refactor access-control internals to better support future changes (#…

…1640)

.changeset/yellow-penguins-warn/changes.json

@@ -0,0 +1 @@
+{ "releases": [{ "name": "@keystone-alpha/access-control", "type": "patch" }], "dependents": [] }

.changeset/yellow-penguins-warn/changes.md

@@ -0,0 +1 @@
+Refactor access-control internals to better support future changes

CONTRIBUTING.md

@@ -134,7 +134,9 @@ We also build commonjs builds to run in node (for testing with jest or etc.) and
 Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/docs/en/emoji-key)):
 
 <!-- ALL-CONTRIBUTORS-LIST:START - Do not remove or modify this section -->
+
 <!-- prettier-ignore -->
+
 <table>
   <tr>
     <td align="center"><a href="http://www.thinkmill.com.au"><img src="https://avatars3.githubusercontent.com/u/872310?v=4" width="100px;" alt="Jed Watson"/><br /><sub><b>Jed Watson</b></sub></a><br /><a href="https://github.com/keystonejs/keystone-5/commits?author=JedWatson" title="Code">💻</a></td>

packages/access-control/lib/access-control.js

@@ -8,54 +8,24 @@ const validateGranularConfigTypes = (longHandAccess, validationError) => {
   if (errors.length) {
     throw new Error(errors.join('\n'));
   }
+  return longHandAccess;
 };
 
-const parseGranularAccessConfig = ({
-  accessTypes,
-  access,
-  defaultAccess,
-  onGranularParseError,
-  validateGranularType,
-}) => {
-  const longHandAccess = pick(access, accessTypes);
-
-  // An object was supplied, but it has the wrong keys (it's probably a
-  // declarative access control config being used as a shorthand, which
-  // isn't possible [due to `create` not supporting declarative config])
-  if (Object.keys(longHandAccess).length === 0) {
-    onGranularParseError();
-  }
-  // Construct an object with all keys
-  const finalAccess = {
-    ...defaultObj(accessTypes, defaultAccess),
-    ...longHandAccess,
-  };
-  validateGranularConfigTypes(finalAccess, validateGranularType);
-
-  return finalAccess;
-};
-
-const parseAccessCore = ({
-  accessTypes,
-  access,
-  defaultAccess,
-  onGranularParseError,
-  validateGranularType,
-}) => {
+const parseAccessCore = ({ accessTypes, access, defaultAccess, onGranularParseError }) => {
   const type = getType(access);
   switch (type) {
     case 'Boolean':
     case 'Function':
       return defaultObj(accessTypes, access);
 
     case 'Object':
-      return parseGranularAccessConfig({
-        accessTypes,
-        access,
-        defaultAccess,
-        onGranularParseError,
-        validateGranularType,
-      });
+      // An object was supplied, but it has the wrong keys (it's probably a
+      // declarative access control config being used as a shorthand, which
+      // isn't possible [due to `create` not supporting declarative config])
+      if (Object.keys(pick(access, accessTypes)).length === 0) {
+        onGranularParseError();
+      }
+      return { ...defaultObj(accessTypes, defaultAccess), ...pick(access, accessTypes) };
 
     default:
       throw new Error(
@@ -83,39 +53,40 @@ const parseAccess = ({
 
   const providedNameCount = intersection(Object.keys(access), schemaNames).length;
   const type = getType(access);
-  if (type === 'Object' && providedNameCount === Object.keys(access).length) {
-    // If all the keys are in schemaNames, parse each on their own
-    return schemaNames.reduce(
-      (acc, schemaName) => ({
-        ...acc,
-        [schemaName]: parseAccessCore({
-          accessTypes,
-          access: access.hasOwnProperty(schemaName) ? access[schemaName] : defaultAccess,
-          defaultAccess,
-          onGranularParseError,
-          validateGranularType,
-        }),
-      }),
-      {}
-    );
-  } else if (type === 'Object' && providedNameCount > 0) {
+
+  if (
+    type === 'Object' &&
+    providedNameCount > 0 &&
+    providedNameCount < Object.keys(access).length
+  ) {
     // If some are in, and some are out, throw an error!
     throw new Error(
       `Invalid schema names: ${JSON.stringify(
         Object.keys(access).filter(k => !schemaNames.includes(k))
       )}`
     );
-  } else {
-    // Otherwise, treat it as common across all schemaNames
-    const commonAccess = parseAccessCore({
-      accessTypes,
-      access,
-      defaultAccess,
-      onGranularParseError,
-      validateGranularType,
-    });
-    return schemaNames.reduce((acc, schemaName) => ({ ...acc, [schemaName]: commonAccess }), {});
   }
+
+  const namesProvided = type === 'Object' && providedNameCount === Object.keys(access).length;
+  return schemaNames.reduce(
+    (acc, schemaName) => ({
+      ...acc,
+      [schemaName]: validateGranularConfigTypes(
+        parseAccessCore({
+          accessTypes,
+          access: namesProvided
+            ? access.hasOwnProperty(schemaName) // If all the keys are in schemaNames, parse each on their own
+              ? access[schemaName]
+              : defaultAccess
+            : access, // Otherwise, treat it as common across all schemaNames
+          defaultAccess,
+          onGranularParseError,
+        }),
+        validateGranularType
+      ),
+    }),
+    {}
+  );
 };
 
 module.exports = {