From 1ed84f743b1cac2f2cececa8fd74f28c79362eb9 Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Tue, 8 Aug 2023 10:29:15 +0100 Subject: [PATCH 1/3] fix(content-manager): simplify useSyncRbac hook --- .../02-frontend/using-permissions.mdx | 2 +- .../src/fixtures/store/index.ts | 1 - .../hooks/useSyncRbac/actions.js | 14 ---- .../hooks/useSyncRbac/constants.js | 2 - .../hooks/useSyncRbac/index.js | 30 ++----- .../hooks/useSyncRbac/reducer.js | 33 -------- .../hooks/useSyncRbac/selectors.js | 6 +- .../hooks/useSyncRbac/tests/reducer.test.js | 83 ------------------- .../hooks/useSyncRbac/tests/selectors.test.js | 28 ++----- .../pages/EditViewLayoutManager/index.js | 2 +- .../pages/ListViewLayoutManager/index.js | 4 +- packages/core/admin/admin/src/reducers.js | 2 - 12 files changed, 18 insertions(+), 189 deletions(-) delete mode 100644 packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/actions.js delete mode 100644 packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/constants.js delete mode 100644 packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/reducer.js delete mode 100644 packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/tests/reducer.test.js diff --git a/docs/docs/docs/01-core/admin/02-permissions/02-frontend/using-permissions.mdx b/docs/docs/docs/01-core/admin/02-permissions/02-frontend/using-permissions.mdx index f697b79cef..c94ed03a47 100644 --- a/docs/docs/docs/01-core/admin/02-permissions/02-frontend/using-permissions.mdx +++ b/docs/docs/docs/01-core/admin/02-permissions/02-frontend/using-permissions.mdx @@ -74,7 +74,7 @@ available in the `@strapi/helper-plugin`. ### CheckPagePermissions Used to apply RBAC to a view/page of the application. If the user does not have the permissions to access this page they will -be redirect to the homepage. +be redirected to the homepage. #### Usage diff --git a/packages/admin-test-utils/src/fixtures/store/index.ts b/packages/admin-test-utils/src/fixtures/store/index.ts index e74b0e0400..40f3418d4a 100644 --- a/packages/admin-test-utils/src/fixtures/store/index.ts +++ b/packages/admin-test-utils/src/fixtures/store/index.ts @@ -20,7 +20,6 @@ const reducers = { total: 0, }, })), - 'content-manager_rbacManager': jest.fn(() => ({ permissions: null })), 'content-manager_editViewLayoutManager': jest.fn(() => ({ currentLayout: null })), 'content-manager_editViewCrudReducer': jest.fn(() => ({ componentsDataStructure: {}, diff --git a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/actions.js b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/actions.js deleted file mode 100644 index 511463e38e..0000000000 --- a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/actions.js +++ /dev/null @@ -1,14 +0,0 @@ -import { RESET_PERMISSIONS, SET_PERMISSIONS } from './constants'; - -export const setPermissions = (permissions, plugins, containerName) => { - return { - type: SET_PERMISSIONS, - permissions, - __meta__: { - plugins, - containerName, - }, - }; -}; - -export const resetPermissions = () => ({ type: RESET_PERMISSIONS }); diff --git a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/constants.js b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/constants.js deleted file mode 100644 index 5c82c0f5af..0000000000 --- a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/constants.js +++ /dev/null @@ -1,2 +0,0 @@ -export const SET_PERMISSIONS = 'ContentManager/RBACManager/SET_PERMISSIONS'; -export const RESET_PERMISSIONS = 'ContentManager/RBACManager/RESET_PERMISSIONS'; diff --git a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/index.js b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/index.js index e88b664f7f..6d6ef45c08 100644 --- a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/index.js +++ b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/index.js @@ -1,28 +1,12 @@ -import { useEffect } from 'react'; +import { useSelector } from 'react-redux'; -import { useDispatch, useSelector } from 'react-redux'; +import { selectCollectionTypePermissions } from './selectors'; -import { resetPermissions, setPermissions } from './actions'; -import { selectCollectionTypePermissions, selectPermissions } from './selectors'; - -const useSyncRbac = (query, collectionTypeUID, containerName = 'listView') => { - const collectionTypesRelatedPermissions = useSelector(selectCollectionTypePermissions); - const permissions = useSelector(selectPermissions); - const dispatch = useDispatch(); - - const relatedPermissions = collectionTypesRelatedPermissions[collectionTypeUID]; - - useEffect(() => { - if (relatedPermissions) { - dispatch(setPermissions(relatedPermissions, query ? query.plugins : null, containerName)); - - return () => { - dispatch(resetPermissions()); - }; - } - - return () => {}; - }, [relatedPermissions, dispatch, query, containerName]); +const useSyncRbac = (collectionTypeUID) => { + const relatedPermissions = useSelector((state) => + selectCollectionTypePermissions(state, collectionTypeUID) + ); + const permissions = [].concat(...Object.values(relatedPermissions)); return permissions; }; diff --git a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/reducer.js b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/reducer.js deleted file mode 100644 index 223c75b736..0000000000 --- a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/reducer.js +++ /dev/null @@ -1,33 +0,0 @@ -/** - * - * RBACManager reducer - */ - -import produce from 'immer'; - -import { RESET_PERMISSIONS, SET_PERMISSIONS } from './constants'; - -export const initialState = { - permissions: null, -}; - -const rbacManagerReducer = (state = initialState, action) => - // eslint-disable-next-line consistent-return - produce(state, (draftState) => { - switch (action.type) { - case SET_PERMISSIONS: { - draftState.permissions = Object.entries(action.permissions).reduce((acc, current) => { - return [...acc, ...current[1]]; - }, []); - break; - } - case RESET_PERMISSIONS: { - draftState.permissions = null; - break; - } - default: - return draftState; - } - }); - -export default rbacManagerReducer; diff --git a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/selectors.js b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/selectors.js index ffbd4ee762..bda0044f45 100644 --- a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/selectors.js +++ b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/selectors.js @@ -1,4 +1,2 @@ -export const selectPermissions = (state) => state['content-manager_rbacManager'].permissions; - -export const selectCollectionTypePermissions = (state) => - state.rbacProvider.collectionTypesRelatedPermissions; +export const selectCollectionTypePermissions = (state, collectionTypeUID) => + state.rbacProvider.collectionTypesRelatedPermissions[collectionTypeUID]; diff --git a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/tests/reducer.test.js b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/tests/reducer.test.js deleted file mode 100644 index d325b7e84b..0000000000 --- a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/tests/reducer.test.js +++ /dev/null @@ -1,83 +0,0 @@ -import { resetPermissions, setPermissions } from '../actions'; -import reducer, { initialState } from '../reducer'; - -describe('CONTENT MANAGER | CONTAINERS | RBACMANAGER | reducer', () => { - let state; - - beforeEach(() => { - state = initialState; - }); - - describe('SET_PERMISSIONS', () => { - it('should set the permissions to an empty array when the permissions are empty', () => { - const expected = { - permissions: [], - }; - - expect(reducer(state, setPermissions({}))).toEqual(expected); - }); - - it('should set the permissions correctly when the permissions are not empty', () => { - const permissions = { - create: [ - { - action: 'create', - subject: 'article', - properties: 'test', - }, - { - action: 'create', - subject: 'article', - properties: 'test1', - }, - ], - read: [ - { - action: 'read', - subject: 'article', - properties: 'test', - }, - { - action: 'read', - subject: 'article', - properties: 'test1', - }, - ], - }; - const expected = { - permissions: [ - { - action: 'create', - subject: 'article', - properties: 'test', - }, - { - action: 'create', - subject: 'article', - properties: 'test1', - }, - { - action: 'read', - subject: 'article', - properties: 'test', - }, - { - action: 'read', - subject: 'article', - properties: 'test1', - }, - ], - }; - - expect(reducer(state, setPermissions(permissions))).toEqual(expected); - }); - }); - - describe('RESET_PERMISSIONS', () => { - it('should set the permissions to null', () => { - state.permissions = []; - - expect(reducer(state, resetPermissions())).toEqual({ permissions: null }); - }); - }); -}); diff --git a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/tests/selectors.test.js b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/tests/selectors.test.js index 8d73461177..048a7d7be8 100644 --- a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/tests/selectors.test.js +++ b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/tests/selectors.test.js @@ -1,6 +1,6 @@ import { fixtures } from '@strapi/admin-test-utils'; -import { selectCollectionTypePermissions, selectPermissions } from '../selectors'; +import { selectCollectionTypePermissions } from '../selectors'; describe('Admin | content manager | hooks | useSyncRbac | selectors', () => { let store; @@ -9,31 +9,13 @@ describe('Admin | content manager | hooks | useSyncRbac | selectors', () => { store = { ...fixtures.store.state }; }); - describe('selectPermissions', () => { - it('resolves the permissions key of the "content-manager_rbacManager" store key', () => { - store['content-manager_rbacManager'] = { - permissions: { - some: 'permission', - }, - }; - - const actual = selectPermissions(store); - const expected = { - some: 'permission', - }; - - expect(actual).toEqual(expected); - }); - }); - describe('selectCollectionTypePermissions', () => { it('resolves the permissions key of the "rbacProvider" store key', () => { - store.rbacProvider.collectionTypesRelatedPermissions.some = 'permission again'; + const uid = 'uid'; + store.rbacProvider.collectionTypesRelatedPermissions[uid] = { permission: true }; - const actual = selectCollectionTypePermissions(store); - const expected = { - some: 'permission again', - }; + const actual = selectCollectionTypePermissions(store, uid); + const expected = { permission: true }; expect(actual).toEqual(expected); }); diff --git a/packages/core/admin/admin/src/content-manager/pages/EditViewLayoutManager/index.js b/packages/core/admin/admin/src/content-manager/pages/EditViewLayoutManager/index.js index c811983301..c44bdd91be 100644 --- a/packages/core/admin/admin/src/content-manager/pages/EditViewLayoutManager/index.js +++ b/packages/core/admin/admin/src/content-manager/pages/EditViewLayoutManager/index.js @@ -16,7 +16,7 @@ const EditViewLayoutManager = ({ layout, ...rest }) => { const dispatch = useDispatch(); const [{ query }] = useQueryParams(); const { runHookWaterfall } = useStrapiApp(); - const permissions = useSyncRbac(query, rest.slug, 'editView'); + const permissions = useSyncRbac(rest.slug); useEffect(() => { // Allow the plugins to extend the edit view layout diff --git a/packages/core/admin/admin/src/content-manager/pages/ListViewLayoutManager/index.js b/packages/core/admin/admin/src/content-manager/pages/ListViewLayoutManager/index.js index 870de7b975..35a8506924 100644 --- a/packages/core/admin/admin/src/content-manager/pages/ListViewLayoutManager/index.js +++ b/packages/core/admin/admin/src/content-manager/pages/ListViewLayoutManager/index.js @@ -13,8 +13,8 @@ import Permissions from './Permissions'; const ListViewLayout = ({ layout, ...props }) => { const dispatch = useDispatch(); const { replace } = useHistory(); - const [{ query, rawQuery }] = useQueryParams(); - const permissions = useSyncRbac(query, props.slug, 'listView'); + const [{ rawQuery }] = useQueryParams(); + const permissions = useSyncRbac(props.slug); const redirectionLink = useFindRedirectionLink(props.slug); useEffect(() => { diff --git a/packages/core/admin/admin/src/reducers.js b/packages/core/admin/admin/src/reducers.js index 912cbff289..a8ccfb7f3d 100644 --- a/packages/core/admin/admin/src/reducers.js +++ b/packages/core/admin/admin/src/reducers.js @@ -1,5 +1,4 @@ import rbacProviderReducer from './components/RBACProvider/reducer'; -import rbacManagerReducer from './content-manager/hooks/useSyncRbac/reducer'; import cmAppReducer from './content-manager/pages/App/reducer'; import editViewLayoutManagerReducer from './content-manager/pages/EditViewLayoutManager/reducer'; import listViewReducer from './content-manager/pages/ListView/reducer'; @@ -9,7 +8,6 @@ import appReducer from './pages/App/reducer'; const contentManagerReducers = { 'content-manager_app': cmAppReducer, 'content-manager_listView': listViewReducer, - 'content-manager_rbacManager': rbacManagerReducer, 'content-manager_editViewLayoutManager': editViewLayoutManagerReducer, 'content-manager_editViewCrudReducer': editViewCrudReducer, }; From 9845be9dfebc3025b7476741974326dbc99d21fd Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Tue, 8 Aug 2023 12:44:52 +0100 Subject: [PATCH 2/3] Revert "fix(content-manager): simplify useSyncRbac hook" This reverts commit 1ed84f743b1cac2f2cececa8fd74f28c79362eb9. --- .../02-frontend/using-permissions.mdx | 2 +- .../src/fixtures/store/index.ts | 1 + .../hooks/useSyncRbac/actions.js | 14 ++++ .../hooks/useSyncRbac/constants.js | 2 + .../hooks/useSyncRbac/index.js | 30 +++++-- .../hooks/useSyncRbac/reducer.js | 33 ++++++++ .../hooks/useSyncRbac/selectors.js | 6 +- .../hooks/useSyncRbac/tests/reducer.test.js | 83 +++++++++++++++++++ .../hooks/useSyncRbac/tests/selectors.test.js | 28 +++++-- .../pages/EditViewLayoutManager/index.js | 2 +- .../pages/ListViewLayoutManager/index.js | 4 +- packages/core/admin/admin/src/reducers.js | 2 + 12 files changed, 189 insertions(+), 18 deletions(-) create mode 100644 packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/actions.js create mode 100644 packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/constants.js create mode 100644 packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/reducer.js create mode 100644 packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/tests/reducer.test.js diff --git a/docs/docs/docs/01-core/admin/02-permissions/02-frontend/using-permissions.mdx b/docs/docs/docs/01-core/admin/02-permissions/02-frontend/using-permissions.mdx index c94ed03a47..f697b79cef 100644 --- a/docs/docs/docs/01-core/admin/02-permissions/02-frontend/using-permissions.mdx +++ b/docs/docs/docs/01-core/admin/02-permissions/02-frontend/using-permissions.mdx @@ -74,7 +74,7 @@ available in the `@strapi/helper-plugin`. ### CheckPagePermissions Used to apply RBAC to a view/page of the application. If the user does not have the permissions to access this page they will -be redirected to the homepage. +be redirect to the homepage. #### Usage diff --git a/packages/admin-test-utils/src/fixtures/store/index.ts b/packages/admin-test-utils/src/fixtures/store/index.ts index 40f3418d4a..e74b0e0400 100644 --- a/packages/admin-test-utils/src/fixtures/store/index.ts +++ b/packages/admin-test-utils/src/fixtures/store/index.ts @@ -20,6 +20,7 @@ const reducers = { total: 0, }, })), + 'content-manager_rbacManager': jest.fn(() => ({ permissions: null })), 'content-manager_editViewLayoutManager': jest.fn(() => ({ currentLayout: null })), 'content-manager_editViewCrudReducer': jest.fn(() => ({ componentsDataStructure: {}, diff --git a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/actions.js b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/actions.js new file mode 100644 index 0000000000..511463e38e --- /dev/null +++ b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/actions.js @@ -0,0 +1,14 @@ +import { RESET_PERMISSIONS, SET_PERMISSIONS } from './constants'; + +export const setPermissions = (permissions, plugins, containerName) => { + return { + type: SET_PERMISSIONS, + permissions, + __meta__: { + plugins, + containerName, + }, + }; +}; + +export const resetPermissions = () => ({ type: RESET_PERMISSIONS }); diff --git a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/constants.js b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/constants.js new file mode 100644 index 0000000000..5c82c0f5af --- /dev/null +++ b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/constants.js @@ -0,0 +1,2 @@ +export const SET_PERMISSIONS = 'ContentManager/RBACManager/SET_PERMISSIONS'; +export const RESET_PERMISSIONS = 'ContentManager/RBACManager/RESET_PERMISSIONS'; diff --git a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/index.js b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/index.js index 6d6ef45c08..e88b664f7f 100644 --- a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/index.js +++ b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/index.js @@ -1,12 +1,28 @@ -import { useSelector } from 'react-redux'; +import { useEffect } from 'react'; -import { selectCollectionTypePermissions } from './selectors'; +import { useDispatch, useSelector } from 'react-redux'; -const useSyncRbac = (collectionTypeUID) => { - const relatedPermissions = useSelector((state) => - selectCollectionTypePermissions(state, collectionTypeUID) - ); - const permissions = [].concat(...Object.values(relatedPermissions)); +import { resetPermissions, setPermissions } from './actions'; +import { selectCollectionTypePermissions, selectPermissions } from './selectors'; + +const useSyncRbac = (query, collectionTypeUID, containerName = 'listView') => { + const collectionTypesRelatedPermissions = useSelector(selectCollectionTypePermissions); + const permissions = useSelector(selectPermissions); + const dispatch = useDispatch(); + + const relatedPermissions = collectionTypesRelatedPermissions[collectionTypeUID]; + + useEffect(() => { + if (relatedPermissions) { + dispatch(setPermissions(relatedPermissions, query ? query.plugins : null, containerName)); + + return () => { + dispatch(resetPermissions()); + }; + } + + return () => {}; + }, [relatedPermissions, dispatch, query, containerName]); return permissions; }; diff --git a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/reducer.js b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/reducer.js new file mode 100644 index 0000000000..223c75b736 --- /dev/null +++ b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/reducer.js @@ -0,0 +1,33 @@ +/** + * + * RBACManager reducer + */ + +import produce from 'immer'; + +import { RESET_PERMISSIONS, SET_PERMISSIONS } from './constants'; + +export const initialState = { + permissions: null, +}; + +const rbacManagerReducer = (state = initialState, action) => + // eslint-disable-next-line consistent-return + produce(state, (draftState) => { + switch (action.type) { + case SET_PERMISSIONS: { + draftState.permissions = Object.entries(action.permissions).reduce((acc, current) => { + return [...acc, ...current[1]]; + }, []); + break; + } + case RESET_PERMISSIONS: { + draftState.permissions = null; + break; + } + default: + return draftState; + } + }); + +export default rbacManagerReducer; diff --git a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/selectors.js b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/selectors.js index bda0044f45..ffbd4ee762 100644 --- a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/selectors.js +++ b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/selectors.js @@ -1,2 +1,4 @@ -export const selectCollectionTypePermissions = (state, collectionTypeUID) => - state.rbacProvider.collectionTypesRelatedPermissions[collectionTypeUID]; +export const selectPermissions = (state) => state['content-manager_rbacManager'].permissions; + +export const selectCollectionTypePermissions = (state) => + state.rbacProvider.collectionTypesRelatedPermissions; diff --git a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/tests/reducer.test.js b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/tests/reducer.test.js new file mode 100644 index 0000000000..d325b7e84b --- /dev/null +++ b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/tests/reducer.test.js @@ -0,0 +1,83 @@ +import { resetPermissions, setPermissions } from '../actions'; +import reducer, { initialState } from '../reducer'; + +describe('CONTENT MANAGER | CONTAINERS | RBACMANAGER | reducer', () => { + let state; + + beforeEach(() => { + state = initialState; + }); + + describe('SET_PERMISSIONS', () => { + it('should set the permissions to an empty array when the permissions are empty', () => { + const expected = { + permissions: [], + }; + + expect(reducer(state, setPermissions({}))).toEqual(expected); + }); + + it('should set the permissions correctly when the permissions are not empty', () => { + const permissions = { + create: [ + { + action: 'create', + subject: 'article', + properties: 'test', + }, + { + action: 'create', + subject: 'article', + properties: 'test1', + }, + ], + read: [ + { + action: 'read', + subject: 'article', + properties: 'test', + }, + { + action: 'read', + subject: 'article', + properties: 'test1', + }, + ], + }; + const expected = { + permissions: [ + { + action: 'create', + subject: 'article', + properties: 'test', + }, + { + action: 'create', + subject: 'article', + properties: 'test1', + }, + { + action: 'read', + subject: 'article', + properties: 'test', + }, + { + action: 'read', + subject: 'article', + properties: 'test1', + }, + ], + }; + + expect(reducer(state, setPermissions(permissions))).toEqual(expected); + }); + }); + + describe('RESET_PERMISSIONS', () => { + it('should set the permissions to null', () => { + state.permissions = []; + + expect(reducer(state, resetPermissions())).toEqual({ permissions: null }); + }); + }); +}); diff --git a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/tests/selectors.test.js b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/tests/selectors.test.js index 048a7d7be8..8d73461177 100644 --- a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/tests/selectors.test.js +++ b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/tests/selectors.test.js @@ -1,6 +1,6 @@ import { fixtures } from '@strapi/admin-test-utils'; -import { selectCollectionTypePermissions } from '../selectors'; +import { selectCollectionTypePermissions, selectPermissions } from '../selectors'; describe('Admin | content manager | hooks | useSyncRbac | selectors', () => { let store; @@ -9,13 +9,31 @@ describe('Admin | content manager | hooks | useSyncRbac | selectors', () => { store = { ...fixtures.store.state }; }); + describe('selectPermissions', () => { + it('resolves the permissions key of the "content-manager_rbacManager" store key', () => { + store['content-manager_rbacManager'] = { + permissions: { + some: 'permission', + }, + }; + + const actual = selectPermissions(store); + const expected = { + some: 'permission', + }; + + expect(actual).toEqual(expected); + }); + }); + describe('selectCollectionTypePermissions', () => { it('resolves the permissions key of the "rbacProvider" store key', () => { - const uid = 'uid'; - store.rbacProvider.collectionTypesRelatedPermissions[uid] = { permission: true }; + store.rbacProvider.collectionTypesRelatedPermissions.some = 'permission again'; - const actual = selectCollectionTypePermissions(store, uid); - const expected = { permission: true }; + const actual = selectCollectionTypePermissions(store); + const expected = { + some: 'permission again', + }; expect(actual).toEqual(expected); }); diff --git a/packages/core/admin/admin/src/content-manager/pages/EditViewLayoutManager/index.js b/packages/core/admin/admin/src/content-manager/pages/EditViewLayoutManager/index.js index c44bdd91be..c811983301 100644 --- a/packages/core/admin/admin/src/content-manager/pages/EditViewLayoutManager/index.js +++ b/packages/core/admin/admin/src/content-manager/pages/EditViewLayoutManager/index.js @@ -16,7 +16,7 @@ const EditViewLayoutManager = ({ layout, ...rest }) => { const dispatch = useDispatch(); const [{ query }] = useQueryParams(); const { runHookWaterfall } = useStrapiApp(); - const permissions = useSyncRbac(rest.slug); + const permissions = useSyncRbac(query, rest.slug, 'editView'); useEffect(() => { // Allow the plugins to extend the edit view layout diff --git a/packages/core/admin/admin/src/content-manager/pages/ListViewLayoutManager/index.js b/packages/core/admin/admin/src/content-manager/pages/ListViewLayoutManager/index.js index 35a8506924..870de7b975 100644 --- a/packages/core/admin/admin/src/content-manager/pages/ListViewLayoutManager/index.js +++ b/packages/core/admin/admin/src/content-manager/pages/ListViewLayoutManager/index.js @@ -13,8 +13,8 @@ import Permissions from './Permissions'; const ListViewLayout = ({ layout, ...props }) => { const dispatch = useDispatch(); const { replace } = useHistory(); - const [{ rawQuery }] = useQueryParams(); - const permissions = useSyncRbac(props.slug); + const [{ query, rawQuery }] = useQueryParams(); + const permissions = useSyncRbac(query, props.slug, 'listView'); const redirectionLink = useFindRedirectionLink(props.slug); useEffect(() => { diff --git a/packages/core/admin/admin/src/reducers.js b/packages/core/admin/admin/src/reducers.js index a8ccfb7f3d..912cbff289 100644 --- a/packages/core/admin/admin/src/reducers.js +++ b/packages/core/admin/admin/src/reducers.js @@ -1,4 +1,5 @@ import rbacProviderReducer from './components/RBACProvider/reducer'; +import rbacManagerReducer from './content-manager/hooks/useSyncRbac/reducer'; import cmAppReducer from './content-manager/pages/App/reducer'; import editViewLayoutManagerReducer from './content-manager/pages/EditViewLayoutManager/reducer'; import listViewReducer from './content-manager/pages/ListView/reducer'; @@ -8,6 +9,7 @@ import appReducer from './pages/App/reducer'; const contentManagerReducers = { 'content-manager_app': cmAppReducer, 'content-manager_listView': listViewReducer, + 'content-manager_rbacManager': rbacManagerReducer, 'content-manager_editViewLayoutManager': editViewLayoutManagerReducer, 'content-manager_editViewCrudReducer': editViewCrudReducer, }; From 66f2ed977bda4e9b549b66a0909157a754fff308 Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Tue, 8 Aug 2023 13:17:15 +0100 Subject: [PATCH 3/3] fix(content-manager): check that permissions are valid in useSyncRbac --- .../src/content-manager/hooks/useSyncRbac/index.js | 12 ++++++++++-- .../pages/EditViewLayoutManager/index.js | 4 ++-- .../pages/ListViewLayoutManager/index.js | 4 ++-- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/index.js b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/index.js index e88b664f7f..fc2e37ea4a 100644 --- a/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/index.js +++ b/packages/core/admin/admin/src/content-manager/hooks/useSyncRbac/index.js @@ -6,9 +6,10 @@ import { resetPermissions, setPermissions } from './actions'; import { selectCollectionTypePermissions, selectPermissions } from './selectors'; const useSyncRbac = (query, collectionTypeUID, containerName = 'listView') => { + const dispatch = useDispatch(); + const collectionTypesRelatedPermissions = useSelector(selectCollectionTypePermissions); const permissions = useSelector(selectPermissions); - const dispatch = useDispatch(); const relatedPermissions = collectionTypesRelatedPermissions[collectionTypeUID]; @@ -24,7 +25,14 @@ const useSyncRbac = (query, collectionTypeUID, containerName = 'listView') => { return () => {}; }, [relatedPermissions, dispatch, query, containerName]); - return permissions; + // Check if the permissions are related to the current collectionTypeUID + const isPermissionMismatch = + permissions?.some((permission) => permission.subject !== collectionTypeUID) ?? true; + + return { + isValid: permissions && !isPermissionMismatch, + permissions, + }; }; export default useSyncRbac; diff --git a/packages/core/admin/admin/src/content-manager/pages/EditViewLayoutManager/index.js b/packages/core/admin/admin/src/content-manager/pages/EditViewLayoutManager/index.js index c811983301..6635729609 100644 --- a/packages/core/admin/admin/src/content-manager/pages/EditViewLayoutManager/index.js +++ b/packages/core/admin/admin/src/content-manager/pages/EditViewLayoutManager/index.js @@ -16,7 +16,7 @@ const EditViewLayoutManager = ({ layout, ...rest }) => { const dispatch = useDispatch(); const [{ query }] = useQueryParams(); const { runHookWaterfall } = useStrapiApp(); - const permissions = useSyncRbac(query, rest.slug, 'editView'); + const { permissions, isValid: isValidPermissions } = useSyncRbac(query, rest.slug, 'editView'); useEffect(() => { // Allow the plugins to extend the edit view layout @@ -29,7 +29,7 @@ const EditViewLayoutManager = ({ layout, ...rest }) => { }; }, [layout, dispatch, query, runHookWaterfall]); - if (!currentLayout || !permissions) { + if (!currentLayout || !isValidPermissions) { return ; } diff --git a/packages/core/admin/admin/src/content-manager/pages/ListViewLayoutManager/index.js b/packages/core/admin/admin/src/content-manager/pages/ListViewLayoutManager/index.js index 870de7b975..5addd99b32 100644 --- a/packages/core/admin/admin/src/content-manager/pages/ListViewLayoutManager/index.js +++ b/packages/core/admin/admin/src/content-manager/pages/ListViewLayoutManager/index.js @@ -14,7 +14,7 @@ const ListViewLayout = ({ layout, ...props }) => { const dispatch = useDispatch(); const { replace } = useHistory(); const [{ query, rawQuery }] = useQueryParams(); - const permissions = useSyncRbac(query, props.slug, 'listView'); + const { permissions, isValid: isValidPermissions } = useSyncRbac(query, props.slug, 'listView'); const redirectionLink = useFindRedirectionLink(props.slug); useEffect(() => { @@ -33,7 +33,7 @@ const ListViewLayout = ({ layout, ...props }) => { }; }, [dispatch]); - if (!permissions) { + if (!isValidPermissions) { return null; }