Merge pull request #7810 from mabashian/7564-access-add-btn

Fix rbac on Add button on User Access/Team Roles lists

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul

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;

awx/ui_next/src/screens/Team/Team.jsx

@@ -11,12 +11,13 @@ 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';
 import TeamEdit from './TeamEdit';
 import { TeamsAPI } from '../../api';
-import TeamAccessList from './TeamRoles';
+import TeamRolesList from './TeamRoles';
 import { ResourceAccessList } from '../../components/ResourceAccessList';
 
 function Team({ i18n, setBreadcrumb }) {
@@ -102,7 +103,9 @@ function Team({ i18n, setBreadcrumb }) {
           )}
           {team && (
             <Route path="/teams/:id/roles">
-              <TeamAccessList />
+              <Config>
+                {({ me }) => <>{me && <TeamRolesList me={me} team={team} />}</>}
+              </Config>
             </Route>
           )}
           <Route key="not-found" path="*">

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`)}
                     </Button>,
                   ]
                 : []),

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('<TeamRolesList />', () => {
   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(<TeamRolesList />);
+      wrapper = mountWithContexts(<TeamRolesList me={me} team={team} />);
     });
     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(<TeamRolesList />);
+      wrapper = mountWithContexts(<TeamRolesList me={me} team={team} />);
     });
     waitForElement(wrapper, 'ContentEmpty', el => el.length === 0);
 
@@ -134,9 +203,10 @@ describe('<TeamRolesList />', () => {
       '/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('<TeamRolesList />', () => {
         count: 1,
       },
     });
+
     await act(async () => {
-      wrapper = mountWithContexts(<TeamRolesList />);
+      wrapper = mountWithContexts(<TeamRolesList me={me} team={team} />);
     });
 
     waitForElement(wrapper, 'ContentEmpty', el => el.length === 0);
@@ -172,10 +243,9 @@ describe('<TeamRolesList />', () => {
 
   test('should render disassociate modal', async () => {
     TeamsAPI.readRoles.mockResolvedValue(roles);
-    TeamsAPI.readRoleOptions.mockResolvedValue(options);
 
     await act(async () => {
-      wrapper = mountWithContexts(<TeamRolesList />);
+      wrapper = mountWithContexts(<TeamRolesList me={me} team={team} />);
     });
 
     waitForElement(wrapper, 'ContentEmpty', el => el.length === 0);
@@ -225,10 +295,9 @@ describe('<TeamRolesList />', () => {
         },
       })
     );
-    TeamsAPI.readRoleOptions.mockResolvedValue(options);
 
     await act(async () => {
-      wrapper = mountWithContexts(<TeamRolesList />);
+      wrapper = mountWithContexts(<TeamRolesList me={me} team={team} />);
     });
 
     waitForElement(wrapper, 'ContentEmpty', el => el.length === 0);
@@ -282,10 +351,9 @@ describe('<TeamRolesList />', () => {
         count: 1,
       },
     });
-    TeamsAPI.readRoleOptions.mockResolvedValue(options);
 
     await act(async () => {
-      wrapper = mountWithContexts(<TeamRolesList />);
+      wrapper = mountWithContexts(<TeamRolesList me={me} team={team} />);
     });
 
     waitForElement(

awx/ui_next/src/screens/User/User.jsx

@@ -127,7 +127,7 @@ function User({ i18n, setBreadcrumb, me }) {
             </Route>
             {user && (
               <Route path="/users/:id/access">
-                <UserAccessList />
+                <UserAccessList user={user} />
               </Route>
             )}
             <Route path="/users/:id/tokens">

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`)}
                     </Button>,
                   ]
                 : []),
@@ -198,7 +198,7 @@ function UserAccessList({ i18n }) {
             <Button
               key="disassociate"
               variant="danger"
-              aria-label={i18n._(t`confirm disassociate`)}
+              aria-label={i18n._(t`Confirm disassociate`)}
               onClick={() => disassociateRole()}
             >
               {i18n._(t`Disassociate`)}

awx/ui_next/src/screens/User/UserAccess/UserAccessList.test.jsx

@@ -10,12 +10,25 @@ import UserAccessList from './UserAccessList';
 jest.mock('../../../api/models/Users');
 jest.mock('../../../api/models/Roles');
 
-jest.mock('react-router-dom', () => ({
-  ...jest.requireActual('react-router-dom'),
-  useParams: () => ({
-    id: 18,
-  }),
-}));
+UsersAPI.readOptions.mockResolvedValue({
+  data: {
+    actions: {
+      GET: {},
+    },
+  },
+});
+
+const user = {
+  id: 18,
+  username: 'Foo User',
+  summary_fields: {
+    user_capabilities: {
+      edit: true,
+      delete: true,
+    },
+  },
+};
+
 const roles = {
   data: {
     results: [
@@ -88,21 +101,18 @@ const roles = {
     count: 5,
   },
 };
-const options = {
-  data: { actions: { POST: { id: 1, disassociate: true } } },
-};
+
 describe('<UserAccessList />', () => {
   let wrapper;
   afterEach(() => {
     jest.clearAllMocks();
-    // wrapper.unmount();
+    wrapper.unmount();
   });
   test('should render properly', async () => {
     UsersAPI.readRoles.mockResolvedValue(roles);
-    UsersAPI.readRoleOptions.mockResolvedValue(options);
 
     await act(async () => {
-      wrapper = mountWithContexts(<UserAccessList />);
+      wrapper = mountWithContexts(<UserAccessList user={user} />);
     });
 
     expect(wrapper.find('UserAccessList').length).toBe(1);
@@ -110,13 +120,12 @@ describe('<UserAccessList />', () => {
 
   test('should create proper detailUrl', async () => {
     UsersAPI.readRoles.mockResolvedValue(roles);
-    UsersAPI.readRoleOptions.mockResolvedValue(options);
 
     await act(async () => {
-      wrapper = mountWithContexts(<UserAccessList />);
+      wrapper = mountWithContexts(<UserAccessList user={user} />);
     });
 
-    waitForElement(wrapper, 'ContentEmpty', el => el.length === 0);
+    wrapper.update();
 
     expect(wrapper.find(`Link#userRole-2`).prop('to')).toBe(
       '/templates/job_template/15/details'
@@ -134,11 +143,7 @@ describe('<UserAccessList />', () => {
       '/inventories/smart_inventory/77/details'
     );
   });
-  test('should not render add button', async () => {
-    UsersAPI.readRoleOptions.mockResolvedValueOnce({
-      data: {},
-    });
-
+  test('should not render add button when user cannot create other users and user cannot edit this user', async () => {
     UsersAPI.readRoles.mockResolvedValue({
       data: {
         results: [
@@ -177,21 +182,33 @@ describe('<UserAccessList />', () => {
       },
     });
     await act(async () => {
-      wrapper = mountWithContexts(<UserAccessList />);
+      wrapper = mountWithContexts(
+        <UserAccessList
+          user={{
+            ...user,
+            summary_fields: {
+              user_capabilities: {
+                edit: false,
+                delete: false,
+              },
+            },
+          }}
+        />
+      );
     });
 
-    waitForElement(wrapper, 'ContentEmpty', el => el.length === 0);
+    wrapper.update();
+
     expect(wrapper.find('Button[aria-label="Add resource roles"]').length).toBe(
       0
     );
   });
   test('should open and close wizard', async () => {
     UsersAPI.readRoles.mockResolvedValue(roles);
-    UsersAPI.readRoleOptions.mockResolvedValue(options);
     await act(async () => {
-      wrapper = mountWithContexts(<UserAccessList />);
+      wrapper = mountWithContexts(<UserAccessList user={user} />);
     });
-    waitForElement(wrapper, 'ContentEmpty', el => el.length === 0);
+    wrapper.update();
     await act(async () =>
       wrapper.find('Button[aria-label="Add resource roles"]').prop('onClick')()
     );
@@ -205,13 +222,12 @@ describe('<UserAccessList />', () => {
   });
   test('should render disassociate modal', async () => {
     UsersAPI.readRoles.mockResolvedValue(roles);
-    UsersAPI.readRoleOptions.mockResolvedValue(options);
 
     await act(async () => {
-      wrapper = mountWithContexts(<UserAccessList />);
+      wrapper = mountWithContexts(<UserAccessList user={user} />);
     });
 
-    waitForElement(wrapper, 'ContentEmpty', el => el.length === 0);
+    wrapper.update();
 
     await act(async () =>
       wrapper.find('Chip[aria-label="Execute"]').prop('onClick')({
@@ -234,7 +250,7 @@ describe('<UserAccessList />', () => {
     ).toBe(1);
     await act(async () =>
       wrapper
-        .find('button[aria-label="confirm disassociate"]')
+        .find('button[aria-label="Confirm disassociate"]')
         .prop('onClick')()
     );
     expect(RolesAPI.disassociateUserRole).toBeCalledWith(4, 18);
@@ -257,13 +273,12 @@ describe('<UserAccessList />', () => {
         },
       })
     );
-    UsersAPI.readRoleOptions.mockResolvedValue(options);
 
     await act(async () => {
-      wrapper = mountWithContexts(<UserAccessList />);
+      wrapper = mountWithContexts(<UserAccessList user={user} />);
     });
 
-    waitForElement(wrapper, 'ContentEmpty', el => el.length === 0);
+    wrapper.update();
 
     await act(async () =>
       wrapper.find('Chip[aria-label="Execute"]').prop('onClick')({
@@ -286,7 +301,7 @@ describe('<UserAccessList />', () => {
     ).toBe(1);
     await act(async () =>
       wrapper
-        .find('button[aria-label="confirm disassociate"]')
+        .find('button[aria-label="Confirm disassociate"]')
         .prop('onClick')()
     );
     wrapper.update();
@@ -313,10 +328,9 @@ describe('<UserAccessList />', () => {
         count: 1,
       },
     });
-    UsersAPI.readRoleOptions.mockResolvedValue(options);
 
     await act(async () => {
-      wrapper = mountWithContexts(<UserAccessList />);
+      wrapper = mountWithContexts(<UserAccessList user={user} />);
     });
 
     waitForElement(

awx/ui_next/src/screens/User/UserAccess/index.js

@@ -1,2 +1,2 @@
-export { default as UserAccessListList } from './UserAccessList';
+export { default as UserAccessList } from './UserAccessList';
 export { default as UserAccessListItem } from './UserAccessListItem';