fix(Users Page): Prevents errors when users without "manage roles" scope expand users. (#1451 - [LL-143](https://learningpool.atlassian.net/browse/LL-143))

h-kanazawa · ryasmi · commit 8ef5a115534a · 2019-09-20T10:20:15.000+01:00
diff --git a/lib/services/auth/filters/getAdminModelFilter.js b/lib/services/auth/filters/getAdminModelFilter.js
@@ -1,5 +1,3 @@
-import includes from 'lodash/includes';
-import intersection from 'lodash/intersection';
 import getScopesFromAuthInfo from 'lib/services/auth/authInfoSelectors/getScopesFromAuthInfo';
 import getOrgFilter from 'lib/services/auth/filters/getOrgFilter';
 import NoAccessError from 'lib/errors/NoAccessError';
@@ -10,18 +8,24 @@ import getModelsFilter,
     checkAllScope
   } from 'lib/services/auth/filters/utils/getModelsFilter';
 
-const adminModelFilter = ({ viewAllScope, editAllScope }) =>
+/**
+ * @param {string[]} _.viewAllScopes
+ * @param {string[]} _.editAllScopes
+ * @return {({ actionName, authInfo }) => Promise}
+ */
+const adminModelFilter = ({ viewAllScopes, editAllScopes }) =>
   async ({ actionName, authInfo }) => {
     const scopes = getScopesFromAuthInfo(authInfo);
 
     switch (actionName) {
       case 'view': {
-        const validScopes = intersection(scopes, [viewAllScope, editAllScope]);
-        if (validScopes.length > 0) return getOrgFilter(authInfo);
+        const hasValidViewScopes = [...viewAllScopes, ...editAllScopes].some(s => scopes.includes(s));
+        if (hasValidViewScopes) return getOrgFilter(authInfo);
         throw new NoAccessError();
       }
       default: {
-        if (includes(scopes, editAllScope)) return getOrgFilter(authInfo);
+        const hasValidEditScopes = editAllScopes.some(s => scopes.includes(s));
+        if (hasValidEditScopes) return getOrgFilter(authInfo);
         throw new NoAccessError();
       }
     }
diff --git a/lib/services/auth/filters/getGlobalModelFilter.js b/lib/services/auth/filters/getGlobalModelFilter.js
@@ -1,4 +1,3 @@
-import includes from 'lodash/includes';
 import getScopesFromAuthInfo from 'lib/services/auth/authInfoSelectors/getScopesFromAuthInfo';
 import getOrgFilter from 'lib/services/auth/filters/getOrgFilter';
 import NoAccessError from 'lib/errors/NoAccessError';
@@ -9,7 +8,7 @@ import getModelsFilter,
     checkAllScope
   } from 'lib/services/auth/filters/utils/getModelsFilter';
 
-const globalModelFilter = ({ editAllScope }) =>
+const globalModelFilter = ({ editAllScopes }) =>
   async ({ actionName, authInfo }) => {
     const scopes = getScopesFromAuthInfo(authInfo);
 
@@ -20,13 +19,15 @@ const globalModelFilter = ({ editAllScope }) =>
           case 'organisation':
             return getOrgFilter(authInfo);
           default: {
-            if (includes(scopes, editAllScope)) return getOrgFilter(authInfo);
+            const isValid = editAllScopes.some(s => scopes.includes(s));
+            if (isValid) return getOrgFilter(authInfo);
             throw new NoAccessError();
           }
         }
       }
       default: {
-        if (includes(scopes, editAllScope)) return getOrgFilter(authInfo);
+        const isValid = editAllScopes.some(s => scopes.includes(s));
+        if (isValid) return getOrgFilter(authInfo);
         throw new NoAccessError();
       }
     }
diff --git a/lib/services/auth/filters/getShareableModelFilter.js b/lib/services/auth/filters/getShareableModelFilter.js
@@ -1,5 +1,3 @@
-import includes from 'lodash/includes';
-import intersection from 'lodash/intersection';
 import getScopesFromAuthInfo
   from 'lib/services/auth/authInfoSelectors/getScopesFromAuthInfo';
 import getPublicOrgFilter from 'lib/services/auth/filters/getPublicOrgFilter';
@@ -15,22 +13,22 @@ import getModelsFilter,
 
 
 export const shareableModelFilter = ({
-  viewAllScope,
-  viewPublicScope,
-  editAllScope,
-  editPublicScope
+  viewAllScopes,
+  viewPublicScopes,
+  editAllScopes,
+  editPublicScopes,
 }) => async ({ actionName, authInfo }) => {
   const scopes = getScopesFromAuthInfo(authInfo);
 
   switch (actionName) {
     case 'view': {
-      const validAllScopes = intersection(scopes, [viewAllScope, editAllScope]);
-      if (validAllScopes.length > 0) {
+      const hasValidAllScopes = [...viewAllScopes, ...editAllScopes].some(s => scopes.includes(s));
+      if (hasValidAllScopes) {
         return getOrgFilter(authInfo);
       }
 
-      const validPublicScopes = intersection(scopes, [viewPublicScope, editPublicScope]);
-      if (validPublicScopes.length > 0) {
+      const hasValidPublicScopes = [...viewPublicScopes, ...editPublicScopes].some(s => scopes.includes(s));
+      if (hasValidPublicScopes) {
         return getPublicOrgFilter(authInfo);
       }
 
@@ -42,10 +40,10 @@ export const shareableModelFilter = ({
       return privateFilter;
     }
     default: {
-      if (includes(scopes, editAllScope)) {
+      if (editAllScopes.some(s => scopes.includes(s))) {
         return getOrgFilter(authInfo);
       }
-      if (includes(scopes, editPublicScope)) {
+      if (editPublicScopes.some(s => scopes.includes(s))) {
         return getPublicOrgFilter(authInfo);
       }
       const privateFilter = getPrivateOrgFilter(authInfo);
diff --git a/lib/services/auth/modelFilters/client.js b/lib/services/auth/modelFilters/client.js
@@ -3,6 +3,6 @@ import getAdminModelFilter
   from 'lib/services/auth/filters/getAdminModelFilter';
 
 export default getAdminModelFilter({
-  viewAllScope: MANAGE_ALL_CLIENTS,
-  editAllScope: MANAGE_ALL_CLIENTS,
+  viewAllScopes: [MANAGE_ALL_CLIENTS],
+  editAllScopes: [MANAGE_ALL_CLIENTS],
 });
diff --git a/lib/services/auth/modelFilters/dashboard.js b/lib/services/auth/modelFilters/dashboard.js
@@ -48,10 +48,10 @@ export const filters = [
 ];
 
 export default getModelsFilter({
-  viewAllScope: VIEW_ALL_DASHBOARDS,
-  editAllScope: EDIT_ALL_DASHBOARDS,
-  viewPublicScope: VIEW_PUBLIC_DASHBOARDS,
-  editPublicScope: EDIT_PUBLIC_DASHBOARDS,
+  viewAllScopes: [VIEW_ALL_DASHBOARDS],
+  editAllScopes: [EDIT_ALL_DASHBOARDS],
+  viewPublicScopes: [VIEW_PUBLIC_DASHBOARDS],
+  editPublicScopes: [EDIT_PUBLIC_DASHBOARDS],
   allowedTokenTypes: ['organisation', 'dashboard', 'client'],
   filters
 });
diff --git a/lib/services/auth/modelFilters/download.js b/lib/services/auth/modelFilters/download.js
@@ -8,8 +8,8 @@ import getShareableModelFilter
   from 'lib/services/auth/filters/getShareableModelFilter';
 
 export default getShareableModelFilter({
-  viewAllScope: VIEW_ALL_DOWNLOADS,
-  editAllScope: EDIT_ALL_DOWNLOADS,
-  viewPublicScope: VIEW_PUBLIC_DOWNLOADS,
-  editPublicScope: EDIT_PUBLIC_DOWNLOADS
+  viewAllScopes: [VIEW_ALL_DOWNLOADS],
+  editAllScopes: [EDIT_ALL_DOWNLOADS],
+  viewPublicScopes: [VIEW_PUBLIC_DOWNLOADS],
+  editPublicScopes: [EDIT_PUBLIC_DOWNLOADS],
 });
diff --git a/lib/services/auth/modelFilters/export.js b/lib/services/auth/modelFilters/export.js
@@ -8,8 +8,8 @@ import getShareableModelFilter
   from 'lib/services/auth/filters/getShareableModelFilter';
 
 export default getShareableModelFilter({
-  viewAllScope: VIEW_ALL_EXPORTS,
-  editAllScope: EDIT_ALL_EXPORTS,
-  viewPublicScope: VIEW_PUBLIC_EXPORTS,
-  editPublicScope: EDIT_PUBLIC_EXPORTS
+  viewAllScopes: [VIEW_ALL_EXPORTS],
+  editAllScopes: [EDIT_ALL_EXPORTS],
+  viewPublicScopes: [VIEW_PUBLIC_EXPORTS],
+  editPublicScopes: [EDIT_PUBLIC_EXPORTS],
 });
diff --git a/lib/services/auth/modelFilters/lrs.js b/lib/services/auth/modelFilters/lrs.js
@@ -3,5 +3,5 @@ import getGlobalModelFilter
   from 'lib/services/auth/filters/getGlobalModelFilter';
 
 export default getGlobalModelFilter({
-  editAllScope: MANAGE_ALL_STORES,
+  editAllScopes: [MANAGE_ALL_STORES],
 });
diff --git a/lib/services/auth/modelFilters/persona.js b/lib/services/auth/modelFilters/persona.js
@@ -3,5 +3,5 @@ import getGlobalModelFilter
   from 'lib/services/auth/filters/getGlobalModelFilter';
 
 export default getGlobalModelFilter({
-  editAllScope: MANAGE_ALL_PERSONAS,
+  editAllScopes: [MANAGE_ALL_PERSONAS],
 });
diff --git a/lib/services/auth/modelFilters/query.js b/lib/services/auth/modelFilters/query.js
@@ -8,8 +8,8 @@ import getShareableModelFilter
   from 'lib/services/auth/filters/getShareableModelFilter';
 
 export default getShareableModelFilter({
-  viewAllScope: VIEW_ALL_QUERIES,
-  editAllScope: EDIT_ALL_QUERIES,
-  viewPublicScope: VIEW_PUBLIC_QUERIES,
-  editPublicScope: EDIT_PUBLIC_QUERIES
+  viewAllScopes: [VIEW_ALL_QUERIES],
+  editAllScopes: [EDIT_ALL_QUERIES],
+  viewPublicScopes: [VIEW_PUBLIC_QUERIES],
+  editPublicScopes: [EDIT_PUBLIC_QUERIES],
 });
diff --git a/lib/services/auth/modelFilters/querybuildercache.js b/lib/services/auth/modelFilters/querybuildercache.js
@@ -3,5 +3,5 @@ import getGlobalModelFilter
   from 'lib/services/auth/filters/getGlobalModelFilter';
 
 export default getGlobalModelFilter({
-  editAllScope: ALL,
+  editAllScopes: [ALL],
 });
diff --git a/lib/services/auth/modelFilters/role.js b/lib/services/auth/modelFilters/role.js
@@ -1,8 +1,8 @@
-import { MANAGE_ALL_ROLES } from 'lib/constants/orgScopes';
+import { MANAGE_ALL_ROLES, MANAGE_ALL_USERS } from 'lib/constants/orgScopes';
 import getAdminModelFilter
   from 'lib/services/auth/filters/getAdminModelFilter';
 
 export default getAdminModelFilter({
-  viewAllScope: MANAGE_ALL_ROLES,
-  editAllScope: MANAGE_ALL_ROLES,
+  viewAllScopes: [MANAGE_ALL_ROLES, MANAGE_ALL_USERS],
+  editAllScopes: [MANAGE_ALL_ROLES],
 });
diff --git a/lib/services/auth/modelFilters/statementForwarding.js b/lib/services/auth/modelFilters/statementForwarding.js
@@ -7,8 +7,8 @@ import {
 import getShareableModelFilter from 'lib/services/auth/filters/getShareableModelFilter';
 
 export default getShareableModelFilter({
-  viewAllScope: VIEW_ALL_STATEMENTFORWARDING,
-  editAllScope: EDIT_ALL_STATEMENTFORWARDING,
-  viewPublicScope: VIEW_PUBLIC_STATEMENTFORWARDING,
-  editPublicScope: EDIT_PUBLIC_STATEMENTFORWARDING,
+  viewAllScopes: [VIEW_ALL_STATEMENTFORWARDING],
+  editAllScopes: [EDIT_ALL_STATEMENTFORWARDING],
+  viewPublicScopes: [VIEW_PUBLIC_STATEMENTFORWARDING],
+  editPublicScopes: [EDIT_PUBLIC_STATEMENTFORWARDING],
 });
diff --git a/lib/services/auth/modelFilters/visualisation.js b/lib/services/auth/modelFilters/visualisation.js
@@ -62,10 +62,10 @@ const filters = [
 ];
 
 export default getModelsFilter({
-  viewAllScope: VIEW_ALL_VISUALISATIONS,
-  editAllScope: EDIT_ALL_VISUALISATIONS,
-  viewPublicScope: VIEW_PUBLIC_VISUALISATIONS,
-  editPublicScope: EDIT_PUBLIC_VISUALISATIONS,
+  viewAllScopes: [VIEW_ALL_VISUALISATIONS],
+  editAllScopes: [EDIT_ALL_VISUALISATIONS],
+  viewPublicScopes: [VIEW_PUBLIC_VISUALISATIONS],
+  editPublicScopes: [EDIT_PUBLIC_VISUALISATIONS],
   allowedTokenTypes: ['organisation', 'dashboard', 'client'],
   filters
 });
diff --git a/lib/services/auth/tests/modelFilters/client-test.js b/lib/services/auth/tests/modelFilters/client-test.js
@@ -3,6 +3,6 @@ import { MANAGE_ALL_CLIENTS } from 'lib/constants/orgScopes';
 
 testAdminModel({
   modelName: 'client',
-  viewAllScope: MANAGE_ALL_CLIENTS,
-  editAllScope: MANAGE_ALL_CLIENTS,
+  viewAllScopes: [MANAGE_ALL_CLIENTS],
+  editAllScopes: [MANAGE_ALL_CLIENTS],
 });
diff --git a/lib/services/auth/tests/modelFilters/dashboard-test.js b/lib/services/auth/tests/modelFilters/dashboard-test.js
@@ -13,10 +13,10 @@ import testDashScopeFilter
 describe('dashboard-test', () => {
   testShareableModel({
     modelName: 'dashboard',
-    viewAllScope: VIEW_ALL_DASHBOARDS,
-    editAllScope: EDIT_ALL_DASHBOARDS,
-    viewPublicScope: VIEW_PUBLIC_DASHBOARDS,
-    editPublicScope: EDIT_PUBLIC_DASHBOARDS,
+    viewAllScopes: [VIEW_ALL_DASHBOARDS],
+    editAllScopes: [EDIT_ALL_DASHBOARDS],
+    viewPublicScopes: [VIEW_PUBLIC_DASHBOARDS],
+    editPublicScopes: [EDIT_PUBLIC_DASHBOARDS],
   });
 
   testDashScopeFilter('dashboard', 'view', [VIEW_SHAREABLE_DASHBOARD], TEST_DASH_DASHBOARD_FILTER);
diff --git a/lib/services/auth/tests/modelFilters/download-test.js b/lib/services/auth/tests/modelFilters/download-test.js
@@ -9,8 +9,8 @@ import {
 
 testShareableModel({
   modelName: 'download',
-  viewAllScope: VIEW_ALL_DOWNLOADS,
-  editAllScope: EDIT_ALL_DOWNLOADS,
-  viewPublicScope: VIEW_PUBLIC_DOWNLOADS,
-  editPublicScope: EDIT_PUBLIC_DOWNLOADS
+  viewAllScopes: [VIEW_ALL_DOWNLOADS],
+  editAllScopes: [EDIT_ALL_DOWNLOADS],
+  viewPublicScopes: [VIEW_PUBLIC_DOWNLOADS],
+  editPublicScopes: [EDIT_PUBLIC_DOWNLOADS],
 });
diff --git a/lib/services/auth/tests/modelFilters/export-test.js b/lib/services/auth/tests/modelFilters/export-test.js
@@ -9,8 +9,8 @@ import {
 
 testShareableModel({
   modelName: 'export',
-  viewAllScope: VIEW_ALL_EXPORTS,
-  editAllScope: EDIT_ALL_EXPORTS,
-  viewPublicScope: VIEW_PUBLIC_EXPORTS,
-  editPublicScope: EDIT_PUBLIC_EXPORTS
+  viewAllScopes: [VIEW_ALL_EXPORTS],
+  editAllScopes: [EDIT_ALL_EXPORTS],
+  viewPublicScopes: [VIEW_PUBLIC_EXPORTS],
+  editPublicScopes: [EDIT_PUBLIC_EXPORTS],
 });
diff --git a/lib/services/auth/tests/modelFilters/lrs-test.js b/lib/services/auth/tests/modelFilters/lrs-test.js
@@ -3,5 +3,5 @@ import { MANAGE_ALL_STORES } from 'lib/constants/orgScopes';
 
 testGlobalModel({
   modelName: 'lrs',
-  editAllScope: MANAGE_ALL_STORES,
+  editAllScopes: [MANAGE_ALL_STORES],
 });
diff --git a/lib/services/auth/tests/modelFilters/persona-test.js b/lib/services/auth/tests/modelFilters/persona-test.js
@@ -4,5 +4,5 @@ import { MANAGE_ALL_PERSONAS } from 'lib/constants/orgScopes';
 
 testGlobalModel({
   modelName: 'persona',
-  editAllScope: MANAGE_ALL_PERSONAS,
+  editAllScopes: [MANAGE_ALL_PERSONAS],
 });
diff --git a/lib/services/auth/tests/modelFilters/query-test.js b/lib/services/auth/tests/modelFilters/query-test.js
@@ -9,8 +9,8 @@ import {
 
 testShareableModel({
   modelName: 'query',
-  viewAllScope: VIEW_ALL_QUERIES,
-  editAllScope: EDIT_ALL_QUERIES,
-  viewPublicScope: VIEW_PUBLIC_QUERIES,
-  editPublicScope: EDIT_PUBLIC_QUERIES
+  viewAllScopes: [VIEW_ALL_QUERIES],
+  editAllScopes: [EDIT_ALL_QUERIES],
+  viewPublicScopes: [VIEW_PUBLIC_QUERIES],
+  editPublicScopes: [EDIT_PUBLIC_QUERIES],
 });
diff --git a/lib/services/auth/tests/modelFilters/role-test.js b/lib/services/auth/tests/modelFilters/role-test.js
@@ -1,8 +1,8 @@
 import testAdminModel from 'lib/services/auth/tests/utils/testAdminModel';
-import { MANAGE_ALL_ROLES } from 'lib/constants/orgScopes';
+import { MANAGE_ALL_ROLES, MANAGE_ALL_USERS } from 'lib/constants/orgScopes';
 
 testAdminModel({
   modelName: 'role',
-  viewAllScope: MANAGE_ALL_ROLES,
-  editAllScope: MANAGE_ALL_ROLES,
+  viewAllScopes: [MANAGE_ALL_ROLES, MANAGE_ALL_USERS],
+  editAllScopes: [MANAGE_ALL_ROLES],
 });
diff --git a/lib/services/auth/tests/modelFilters/statementForwarding-test.js b/lib/services/auth/tests/modelFilters/statementForwarding-test.js
@@ -9,8 +9,8 @@ import {
 
 testShareableModel({
   modelName: 'statementforwarding',
-  viewAllScope: VIEW_ALL_STATEMENTFORWARDING,
-  editAllScope: EDIT_ALL_STATEMENTFORWARDING,
-  viewPublicScope: VIEW_PUBLIC_STATEMENTFORWARDING,
-  editPublicScope: EDIT_PUBLIC_STATEMENTFORWARDING,
+  viewAllScopes: [VIEW_ALL_STATEMENTFORWARDING],
+  editAllScopes: [EDIT_ALL_STATEMENTFORWARDING],
+  viewPublicScopes: [VIEW_PUBLIC_STATEMENTFORWARDING],
+  editPublicScopes: [EDIT_PUBLIC_STATEMENTFORWARDING],
 });
diff --git a/lib/services/auth/tests/modelFilters/visualisation-test.js b/lib/services/auth/tests/modelFilters/visualisation-test.js
@@ -15,10 +15,10 @@ import testDashScopeFilter
 describe('test visualisation model', () => {
   testShareableModel({
     modelName: 'visualisation',
-    viewAllScope: VIEW_ALL_VISUALISATIONS,
-    editAllScope: EDIT_ALL_VISUALISATIONS,
-    viewPublicScope: VIEW_PUBLIC_VISUALISATIONS,
-    editPublicScope: EDIT_PUBLIC_VISUALISATIONS,
+    viewAllScopes: [VIEW_ALL_VISUALISATIONS],
+    editAllScopes: [EDIT_ALL_VISUALISATIONS],
+    viewPublicScopes: [VIEW_PUBLIC_VISUALISATIONS],
+    editPublicScopes: [EDIT_PUBLIC_VISUALISATIONS],
   });
 
   testDashScopeFilter('visualisation', 'view', [VIEW_SHAREABLE_DASHBOARD], TEST_DASH_FILTER);
diff --git a/lib/services/auth/tests/utils/testAdminModel.js b/lib/services/auth/tests/utils/testAdminModel.js
diff --git a/lib/services/auth/tests/utils/testGlobalModel.js b/lib/services/auth/tests/utils/testGlobalModel.js
diff --git a/lib/services/auth/tests/utils/testShareableModel.js b/lib/services/auth/tests/utils/testShareableModel.js

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,3 @@`
`1`		`-import includes from 'lodash/includes';`
`2`	`1`	`import getScopesFromAuthInfo from 'lib/services/auth/authInfoSelectors/getScopesFromAuthInfo';`
`3`	`2`	`import getOrgFilter from 'lib/services/auth/filters/getOrgFilter';`
`4`	`3`	`import NoAccessError from 'lib/errors/NoAccessError';`
`@@ -9,7 +8,7 @@ import getModelsFilter,`
`9`	`8`	`checkAllScope`
`10`	`9`	`} from 'lib/services/auth/filters/utils/getModelsFilter';`
`11`	`10`
`12`		`-const globalModelFilter = ({ editAllScope }) =>`
	`11`	`+const globalModelFilter = ({ editAllScopes }) =>`
`13`	`12`	`async ({ actionName, authInfo }) => {`
`14`	`13`	`const scopes = getScopesFromAuthInfo(authInfo);`
`15`	`14`
`@@ -20,13 +19,15 @@ const globalModelFilter = ({ editAllScope }) =>`
`20`	`19`	`case 'organisation':`
`21`	`20`	`return getOrgFilter(authInfo);`
`22`	`21`	`default: {`
`23`		`- if (includes(scopes, editAllScope)) return getOrgFilter(authInfo);`
	`22`	`+ const isValid = editAllScopes.some(s => scopes.includes(s));`
	`23`	`+ if (isValid) return getOrgFilter(authInfo);`
`24`	`24`	`throw new NoAccessError();`
`25`	`25`	`}`
`26`	`26`	`}`
`27`	`27`	`}`
`28`	`28`	`default: {`
`29`		`- if (includes(scopes, editAllScope)) return getOrgFilter(authInfo);`
	`29`	`+ const isValid = editAllScopes.some(s => scopes.includes(s));`
	`30`	`+ if (isValid) return getOrgFilter(authInfo);`
`30`	`31`	`throw new NoAccessError();`
`31`	`32`	`}`
`32`	`33`	`}`