Skip to content

Commit

Permalink
feat(ESSNTL-5206): Group-level RBAC and actions (#1950)
Browse files Browse the repository at this point in the history 
Implements https://issues.redhat.com/browse/ESSNTL-5206.

- Do additional RBAC check on the group level if a selected host is in a group.
- Enable the bulk Delete button is user has inventory:hosts:write for all
systems selected. Also, if the selected hosts are in a group, then calculate the
minimal RBAC required permissions accordingly.

---------

Co-authored-by: Aleksandr Voznesenskii <[email protected]>
  • Loading branch information
@gkarat @Fewwy
gkarat and Fewwy authored Aug 8, 2023
1 parent 36382d4 commit c678efc
Show file tree
Hide file tree
Showing 5 changed files with 316 additions and 160 deletions.
10 changes: 0 additions & 10 deletions src/Utilities/constants.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import { createContext } from 'react';
import { usePermissionsWithContext } from '@redhat-cloud-services/frontend-components-utilities/RBACHook';
import { GENERAL_HOSTS_WRITE_PERMISSIONS } from '../constants';

export const TEXT_FILTER = 'hostname_or_id';
export const TEXTUAL_CHIP = 'textual';
Expand Down Expand Up @@ -251,12 +249,4 @@ export const generateFilter = (
},
].filter(Boolean);

export const useHostsWritePermissions = () => {
const { hasAccess } = usePermissionsWithContext([
GENERAL_HOSTS_WRITE_PERMISSIONS,
]);

return hasAccess;
};

export const allStaleFilters = ['fresh', 'stale', 'stale_warning', 'unknown'];
56 changes: 56 additions & 0 deletions src/components/InventoryTable/ActionWithRBAC.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/**
* This module contains Button and DropdownItem components wrapped by RBAC checks.
*/
import React from 'react';
import { usePermissionsWithContext } from '@redhat-cloud-services/frontend-components-utilities/RBACHook';
import { Button, DropdownItem, Tooltip } from '@patternfly/react-core';
import PropTypes from 'prop-types';

export const ActionButton = ({
requiredPermissions,
noAccessTooltip,
checkAll,
...props
}) => {
const { hasAccess: enabled } = usePermissionsWithContext(
requiredPermissions,
checkAll
);

return enabled ? (
<Button {...props} />
) : (
<Tooltip content={noAccessTooltip}>
<Button {...props} isAriaDisabled />
</Tooltip>
);
};

ActionButton.propTypes = {
requiredPermissions: PropTypes.array,
noAccessTooltip: PropTypes.string,
checkAll: PropTypes.bool,
};

ActionButton.defaultProps = {
checkAll: false,
};

export const ActionDropdownItem = ({
requiredPermissions,
noAccessTooltip,
...props
}) => {
const { hasAccess: enabled } = usePermissionsWithContext(requiredPermissions);

return enabled ? (
<DropdownItem {...props} />
) : (
<DropdownItem {...props} isAriaDisabled tooltip={noAccessTooltip} />
);
};

ActionDropdownItem.propTypes = {
requiredPermissions: PropTypes.array,
noAccessTooltip: PropTypes.string,
};
106 changes: 92 additions & 14 deletions src/routes/InventoryTable.cy.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ import {
ROW,
} from '@redhat-cloud-services/frontend-components-utilities';

const TEST_GROUP = 'ancd';
const TEST_GROUP_NAME = 'ancd';
const TEST_GROUP_ID = '54b302e4-07d2-45c5-b2f8-92a286847f9d';

const mountTable = () => {
cy.mountWithContext(Inventory);
Expand All @@ -47,21 +48,22 @@ before(() => {
});

describe('test data', () => {
it(`first two hosts in the group ${TEST_GROUP}`, () => {
it(`first two hosts in the group ${TEST_GROUP_NAME}`, () => {
expect(
hostsFixtures.results
.slice(0, 2)
.every(({ groups }) => groups[0].name === TEST_GROUP)
.every(({ groups }) => groups[0].name === TEST_GROUP_NAME)
).to.be.true;
});

it(`the third host has a group differente that ${TEST_GROUP}`, () => {
expect(hostsFixtures.results[2].groups[0].name !== TEST_GROUP).to.be.true;
it(`the third host has a group different to ${TEST_GROUP_NAME}`, () => {
expect(hostsFixtures.results[2].groups[0].name !== TEST_GROUP_NAME).to.be
.true;
});

it(`groups has the group ${TEST_GROUP}`, () => {
expect(groupsFixtures.results.some(({ name }) => name === TEST_GROUP)).to.be
.true;
it(`groups has the group ${TEST_GROUP_NAME}`, () => {
expect(groupsFixtures.results.some(({ name }) => name === TEST_GROUP_NAME))
.to.be.true;
});

it('the fourth host is not in a group', () => {
Expand Down Expand Up @@ -92,7 +94,7 @@ describe('inventory table', () => {
describe('has groups actions', () => {
it('cannot add host to another group', () => {
cy.get(ROW).eq(1).find(DROPDOWN).click();
cy.get(DROPDOWN_ITEM)
cy.get(`${DROPDOWN_ITEM} a`)
.contains('Add to group')
.should('have.attr', 'aria-disabled', 'true');
});
Expand Down Expand Up @@ -120,7 +122,7 @@ describe('inventory table', () => {
`/api/inventory/v1/groups/${hostsFixtures.results[0].groups[0].id}/hosts/${hostsFixtures.results[0].id}`
).as('request');
cy.get(ROW).eq(1).find(DROPDOWN).click();
cy.get(DROPDOWN_ITEM).contains('Remove from group').click();
cy.get(`${DROPDOWN_ITEM} a`).contains('Remove from group').click();
cy.get(MODAL).within(() => {
cy.get('h1').should('have.text', 'Remove from group');
cy.get('button[type="submit"]').click();
Expand All @@ -135,7 +137,7 @@ describe('inventory table', () => {
`/api/inventory/v1/groups/${groupsFixtures.results[0].id}/hosts`
).as('request');
cy.get(ROW).eq(4).find(DROPDOWN).click();
cy.get(DROPDOWN_ITEM).contains('Add to group').click();
cy.get(`${DROPDOWN_ITEM} a`).contains('Add to group').click();
cy.get(MODAL).within(() => {
cy.get('h1').should('have.text', 'Add to group');
cy.wait('@getGroups');
Expand Down Expand Up @@ -191,7 +193,8 @@ describe('inventory table', () => {
cy.intercept(
'POST',
`/api/inventory/v1/groups/${
groupsFixtures.results.find(({ name }) => name === TEST_GROUP)?.id
groupsFixtures.results.find(({ name }) => name === TEST_GROUP_NAME)
?.id
}/hosts`
).as('request');

Expand All @@ -207,7 +210,7 @@ describe('inventory table', () => {
cy.get('h1').should('have.text', 'Add to group');
cy.wait('@getGroups');
cy.get('.pf-c-select__toggle').click(); // TODO: implement ouia selector for this component
cy.get('.pf-c-select__menu-item').contains(TEST_GROUP).click();
cy.get('.pf-c-select__menu-item').contains(TEST_GROUP_NAME).click();
cy.get('button[type="submit"]').click();
cy.wait('@request')
.its('request.body')
Expand Down Expand Up @@ -240,12 +243,14 @@ describe('inventory table', () => {

it('all per-row actions are disabled', () => {
cy.get(ROW).eq(1).find(DROPDOWN).click();
cy.get(DROPDOWN_ITEM).each(($el) =>
cy.get(`${DROPDOWN_ITEM} a`).each(($el) =>
cy.wrap($el).should('have.attr', 'aria-disabled', 'true')
);
});

it('bulk actions are disabled', () => {
cy.get(ROW).find('[type="checkbox"]').eq(0).click();

cy.get('button')
.contains('Delete')
.should('have.attr', 'aria-disabled', 'true');
Expand All @@ -264,5 +269,78 @@ describe('inventory table', () => {
.should('have.attr', 'aria-disabled', 'true');
});
});

describe('with group-level hosts write permissions', () => {
before(() => {
cy.mockWindowChrome({
userPermissions: [
'inventory:*:read',
{
permission: 'inventory:hosts:write',
resourceDefinitions: [
{
attributeFilter: {
key: 'group.id',
operation: 'equal',
value: TEST_GROUP_ID,
},
},
],
},
],
});
});

beforeEach(prepareTest);

it('can edit hosts that in the test group', () => {
cy.get(ROW).eq(1).find(DROPDOWN).click();
cy.get(`${DROPDOWN_ITEM} a`)
.contains('Edit')
.should('have.attr', 'aria-disabled', 'false')
.click();
cy.get('button').contains('Cancel').click();
cy.get(ROW).eq(1).find(DROPDOWN).click();
});

it('can delete hosts in the test group', () => {
cy.get(ROW).eq(1).find(DROPDOWN).click();
cy.get(`${DROPDOWN_ITEM} a`)
.contains('Delete')
.should('have.attr', 'aria-disabled', 'false')
.click();
cy.get('button').contains('Cancel').click();
});

it('cannot edit nor delete hosts that are not in the test group', () => {
cy.get(ROW).eq(3).find(DROPDOWN).click();
cy.get(`${DROPDOWN_ITEM} a`)
.contains('Edit')
.should('have.attr', 'aria-disabled', 'true');
cy.get(`${DROPDOWN_ITEM} a`)
.contains('Delete')
.should('have.attr', 'aria-disabled', 'true');
});

it('can delete hosts that are in the test group', () => {
cy.get(ROW).find('[type="checkbox"]').eq(0).click();
cy.get(ROW).find('[type="checkbox"]').eq(1).click();

cy.get('button')
.contains('Delete')
.should('have.attr', 'aria-disabled', 'false')
.click();
});

it('cannot delete hosts that are not in the test group', () => {
cy.get(ROW).find('[type="checkbox"]').eq(2).click();

cy.get('button')
.contains('Delete')
.should('have.attr', 'aria-disabled', 'true');
});
});

// TODO: test group bulk actions once granular RBAC is implemented there too
});
});
Loading

0 comments on commit c678efc

Please sign in to comment.