From 4ce2235f68d85edd3bb35c451734c3efcbb0452f Mon Sep 17 00:00:00 2001 From: mabashian Date: Tue, 4 Aug 2020 08:49:51 -0400 Subject: [PATCH 1/2] Fix rbac on Add button on User Access/Team Roles lists --- awx/ui_next/src/api/models/Users.js | 6 + awx/ui_next/src/screens/Team/Team.jsx | 7 +- .../screens/Team/TeamRoles/TeamRolesList.jsx | 39 +++--- .../Team/TeamRoles/TeamRolesList.test.jsx | 116 ++++++++++++++---- awx/ui_next/src/screens/User/User.jsx | 2 +- .../User/UserAccess/UserAccessList.jsx | 22 ++-- .../User/UserAccess/UserAccessList.test.jsx | 84 +++++++------ 7 files changed, 186 insertions(+), 90 deletions(-) diff --git a/awx/ui_next/src/api/models/Users.js b/awx/ui_next/src/api/models/Users.js index 97c7a6976c..02099c2b67 100644 --- a/awx/ui_next/src/api/models/Users.js +++ b/awx/ui_next/src/api/models/Users.js @@ -54,6 +54,12 @@ class Users extends Base { params, }); } + + readAdminOfOrganizations(userId, params) { + return this.http.get(`${this.baseUrl}${userId}/admin_of_organizations/`, { + params, + }); + } } export default Users; diff --git a/awx/ui_next/src/screens/Team/Team.jsx b/awx/ui_next/src/screens/Team/Team.jsx index 2d46f30360..e5a9d70b85 100644 --- a/awx/ui_next/src/screens/Team/Team.jsx +++ b/awx/ui_next/src/screens/Team/Team.jsx @@ -11,6 +11,7 @@ import { } from 'react-router-dom'; import { CaretLeftIcon } from '@patternfly/react-icons'; import { Card, PageSection } from '@patternfly/react-core'; +import { Config } from '../../contexts/Config'; import RoutedTabs from '../../components/RoutedTabs'; import ContentError from '../../components/ContentError'; import TeamDetail from './TeamDetail'; @@ -102,7 +103,11 @@ function Team({ i18n, setBreadcrumb }) { )} {team && ( - + + {({ me }) => ( + <>{me && } + )} + )} diff --git a/awx/ui_next/src/screens/Team/TeamRoles/TeamRolesList.jsx b/awx/ui_next/src/screens/Team/TeamRoles/TeamRolesList.jsx index 25c3a59220..3fcf84a417 100644 --- a/awx/ui_next/src/screens/Team/TeamRoles/TeamRolesList.jsx +++ b/awx/ui_next/src/screens/Team/TeamRoles/TeamRolesList.jsx @@ -1,5 +1,5 @@ import React, { useCallback, useEffect, useState } from 'react'; -import { useLocation, useParams } from 'react-router-dom'; +import { useLocation } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; @@ -12,7 +12,7 @@ import { Title, } from '@patternfly/react-core'; import { CubesIcon } from '@patternfly/react-icons'; -import { TeamsAPI, RolesAPI } from '../../../api'; +import { TeamsAPI, RolesAPI, UsersAPI } from '../../../api'; import useRequest, { useDeleteItems } from '../../../util/useRequest'; import DataListToolbar from '../../../components/DataListToolbar'; import PaginatedDataList from '../../../components/PaginatedDataList'; @@ -28,17 +28,16 @@ const QS_CONFIG = getQSConfig('roles', { order_by: 'id', }); -function TeamRolesList({ i18n }) { +function TeamRolesList({ i18n, me, team }) { const [isWizardOpen, setIsWizardOpen] = useState(false); const { search } = useLocation(); - const { id } = useParams(); const [roleToDisassociate, setRoleToDisassociate] = useState(null); const { isLoading, request: fetchRoles, contentError, - result: { roleCount, roles, options }, + result: { roleCount, roles, isAdminOfOrg }, } = useRequest( useCallback(async () => { const params = parseQueryString(QS_CONFIG, search); @@ -46,18 +45,23 @@ function TeamRolesList({ i18n }) { { data: { results, count }, }, - { - data: { actions }, - }, + { count: orgAdminCount }, ] = await Promise.all([ - TeamsAPI.readRoles(id, params), - TeamsAPI.readRoleOptions(id), + TeamsAPI.readRoles(team.id, params), + UsersAPI.readAdminOfOrganizations(me.id, { + id: team.organization, + }), ]); - return { roleCount: count, roles: results, options: actions }; - }, [id, search]), + return { + roleCount: count, + roles: results, + isAdminOfOrg: orgAdminCount > 0, + }; + }, [me.id, team.id, team.organization, search]), { roles: [], roleCount: 0, + isAdminOfOrg: false, } ); @@ -79,14 +83,13 @@ function TeamRolesList({ i18n }) { setRoleToDisassociate(null); await RolesAPI.disassociateTeamRole( roleToDisassociate.id, - parseInt(id, 10) + parseInt(team.id, 10) ); - }, [roleToDisassociate, id]), + }, [roleToDisassociate, team.id]), { qsConfig: QS_CONFIG, fetchItems: fetchRoles } ); - const canAdd = - options && Object.prototype.hasOwnProperty.call(options, 'POST'); + const canAdd = team?.summary_fields?.user_capabilities?.edit || isAdminOfOrg; const detailUrl = role => { const { resource_id, resource_type } = role.summary_fields; @@ -128,7 +131,7 @@ function TeamRolesList({ i18n }) { hasContentLoading={isLoading || isDisassociateLoading} items={roles} itemCount={roleCount} - pluralizedItemName={i18n._(t`Teams`)} + pluralizedItemName={i18n._(t`Team Roles`)} qsConfig={QS_CONFIG} toolbarSearchColumns={[ { @@ -157,7 +160,7 @@ function TeamRolesList({ i18n }) { setIsWizardOpen(true); }} > - Add + {i18n._(t`Add`)} , ] : []), diff --git a/awx/ui_next/src/screens/Team/TeamRoles/TeamRolesList.test.jsx b/awx/ui_next/src/screens/Team/TeamRoles/TeamRolesList.test.jsx index 5475470486..e8371af4d4 100644 --- a/awx/ui_next/src/screens/Team/TeamRoles/TeamRolesList.test.jsx +++ b/awx/ui_next/src/screens/Team/TeamRoles/TeamRolesList.test.jsx @@ -1,6 +1,6 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; -import { TeamsAPI, RolesAPI } from '../../../api'; +import { TeamsAPI, RolesAPI, UsersAPI } from '../../../api'; import { mountWithContexts, waitForElement, @@ -9,13 +9,74 @@ import TeamRolesList from './TeamRolesList'; jest.mock('../../../api/models/Teams'); jest.mock('../../../api/models/Roles'); +jest.mock('../../../api/models/Users'); -jest.mock('react-router-dom', () => ({ - ...jest.requireActual('react-router-dom'), - useParams: () => ({ - id: 18, - }), -})); +const me = { + id: 1, +}; + +const team = { + id: 18, + type: 'team', + url: '/api/v2/teams/1/', + related: { + created_by: '/api/v2/users/1/', + modified_by: '/api/v2/users/1/', + projects: '/api/v2/teams/1/projects/', + users: '/api/v2/teams/1/users/', + credentials: '/api/v2/teams/1/credentials/', + roles: '/api/v2/teams/1/roles/', + object_roles: '/api/v2/teams/1/object_roles/', + activity_stream: '/api/v2/teams/1/activity_stream/', + access_list: '/api/v2/teams/1/access_list/', + organization: '/api/v2/organizations/1/', + }, + summary_fields: { + organization: { + id: 1, + name: 'Default', + description: '', + }, + created_by: { + id: 1, + username: 'admin', + first_name: '', + last_name: '', + }, + modified_by: { + id: 1, + username: 'admin', + first_name: '', + last_name: '', + }, + object_roles: { + admin_role: { + description: 'Can manage all aspects of the team', + name: 'Admin', + id: 33, + }, + member_role: { + description: 'User is a member of the team', + name: 'Member', + id: 34, + }, + read_role: { + description: 'May view settings for the team', + name: 'Read', + id: 35, + }, + }, + user_capabilities: { + edit: false, + delete: false, + }, + }, + created: '2020-07-22T18:21:54.233411Z', + modified: '2020-07-22T18:21:54.233442Z', + name: 'a team', + description: '', + organization: 1, +}; const roles = { data: { @@ -89,32 +150,40 @@ const roles = { count: 5, }, }; -const options = { - data: { actions: { POST: { id: 1, disassociate: true } } }, -}; + describe('', () => { let wrapper; + beforeEach(() => { + UsersAPI.readAdminOfOrganizations.mockResolvedValue({ + count: 1, + results: [ + { + id: 1, + name: 'Foo Org', + }, + ], + }); + }); + afterEach(() => { jest.clearAllMocks(); wrapper.unmount(); }); test('should render properly', async () => { TeamsAPI.readRoles.mockResolvedValue(roles); - TeamsAPI.readRoleOptions.mockResolvedValue(options); await act(async () => { - wrapper = mountWithContexts(); + wrapper = mountWithContexts(); }); expect(wrapper.find('TeamRolesList').length).toBe(1); }); test('should create proper detailUrl', async () => { TeamsAPI.readRoles.mockResolvedValue(roles); - TeamsAPI.readRoleOptions.mockResolvedValue(options); await act(async () => { - wrapper = mountWithContexts(); + wrapper = mountWithContexts(); }); waitForElement(wrapper, 'ContentEmpty', el => el.length === 0); @@ -134,9 +203,10 @@ describe('', () => { '/inventories/smart_inventory/77/details' ); }); - test('should not render add button', async () => { - TeamsAPI.readRoleOptions.mockResolvedValueOnce({ - data: {}, + test('should not render add button when user cannot edit team and is not an admin of the org', async () => { + UsersAPI.readAdminOfOrganizations.mockResolvedValueOnce({ + count: 0, + results: [], }); TeamsAPI.readRoles.mockResolvedValue({ @@ -160,8 +230,9 @@ describe('', () => { count: 1, }, }); + await act(async () => { - wrapper = mountWithContexts(); + wrapper = mountWithContexts(); }); waitForElement(wrapper, 'ContentEmpty', el => el.length === 0); @@ -172,10 +243,9 @@ describe('', () => { test('should render disassociate modal', async () => { TeamsAPI.readRoles.mockResolvedValue(roles); - TeamsAPI.readRoleOptions.mockResolvedValue(options); await act(async () => { - wrapper = mountWithContexts(); + wrapper = mountWithContexts(); }); waitForElement(wrapper, 'ContentEmpty', el => el.length === 0); @@ -225,10 +295,9 @@ describe('', () => { }, }) ); - TeamsAPI.readRoleOptions.mockResolvedValue(options); await act(async () => { - wrapper = mountWithContexts(); + wrapper = mountWithContexts(); }); waitForElement(wrapper, 'ContentEmpty', el => el.length === 0); @@ -282,10 +351,9 @@ describe('', () => { count: 1, }, }); - TeamsAPI.readRoleOptions.mockResolvedValue(options); await act(async () => { - wrapper = mountWithContexts(); + wrapper = mountWithContexts(); }); waitForElement( diff --git a/awx/ui_next/src/screens/User/User.jsx b/awx/ui_next/src/screens/User/User.jsx index 282a193ab8..95dcb487b4 100644 --- a/awx/ui_next/src/screens/User/User.jsx +++ b/awx/ui_next/src/screens/User/User.jsx @@ -127,7 +127,7 @@ function User({ i18n, setBreadcrumb, me }) { {user && ( - + )} diff --git a/awx/ui_next/src/screens/User/UserAccess/UserAccessList.jsx b/awx/ui_next/src/screens/User/UserAccess/UserAccessList.jsx index 4bc108ff83..73fb0031e5 100644 --- a/awx/ui_next/src/screens/User/UserAccess/UserAccessList.jsx +++ b/awx/ui_next/src/screens/User/UserAccess/UserAccessList.jsx @@ -1,5 +1,5 @@ import React, { useCallback, useEffect, useState } from 'react'; -import { useParams, useLocation } from 'react-router-dom'; +import { useLocation } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { @@ -29,8 +29,7 @@ const QS_CONFIG = getQSConfig('roles', { // TODO Figure out how to best conduct a search of this list. // Since we only have a role ID in the top level of each role object // we can't really search using the normal search parameters. -function UserAccessList({ i18n }) { - const { id } = useParams(); +function UserAccessList({ i18n, user }) { const { search } = useLocation(); const [isWizardOpen, setIsWizardOpen] = useState(false); @@ -51,11 +50,11 @@ function UserAccessList({ i18n }) { data: { actions }, }, ] = await Promise.all([ - UsersAPI.readRoles(id, params), - UsersAPI.readRoleOptions(id), + UsersAPI.readRoles(user.id, params), + UsersAPI.readOptions(), ]); return { roleCount: count, roles: results, options: actions }; - }, [id, search]), + }, [user.id, search]), { roles: [], roleCount: 0, @@ -75,14 +74,15 @@ function UserAccessList({ i18n }) { setRoleToDisassociate(null); await RolesAPI.disassociateUserRole( roleToDisassociate.id, - parseInt(id, 10) + parseInt(user.id, 10) ); - }, [roleToDisassociate, id]), + }, [roleToDisassociate, user.id]), { qsConfig: QS_CONFIG, fetchItems: fetchRoles } ); const canAdd = - options && Object.prototype.hasOwnProperty.call(options, 'POST'); + user?.summary_fields?.user_capabilities?.edit || + (options && Object.prototype.hasOwnProperty.call(options, 'POST')); const saveRoles = () => { setIsWizardOpen(false); @@ -170,7 +170,7 @@ function UserAccessList({ i18n }) { setIsWizardOpen(true); }} > - Add + {i18n._(t`Add`)} , ] : []), @@ -198,7 +198,7 @@ function UserAccessList({ i18n }) {