Skip to content

Commit

Permalink
Merge pull request opendatahub-io#3003 from emilys314/defer-project-k…
Browse files Browse the repository at this point in the history
…ebab-check-and-disable

Defer project kebab check and disable
  • Loading branch information
openshift-merge-bot[bot] authored Jul 17, 2024
2 parents be47fe4 + b9392f5 commit 81961d4
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ import {
ProjectModel,
ProjectRequestModel,
RouteModel,
SelfSubjectAccessReviewModel,
} from '~/__tests__/cypress/cypress/utils/models';
import { mock200Status } from '~/__mocks__/mockK8sStatus';
import { mockNotebookK8sResource, mockRouteK8sResource } from '~/__mocks__';
import { mockPodK8sResource } from '~/__mocks__/mockPodK8sResource';
import { mockSelfSubjectAccessReview } from '~/__mocks__/mockSelfSubjectAccessReview';
import { asProjectAdminUser } from '~/__tests__/cypress/cypress/utils/users';
import { notebookConfirmModal } from '~/__tests__/cypress/cypress/pages/workbench';
import { testPagination } from '~/__tests__/cypress/cypress/utils/pagination';
Expand Down Expand Up @@ -78,7 +80,16 @@ describe('Data science projects details', () => {
it('should delete project', () => {
initIntercepts();
projectListPage.visit();
projectListPage.getProjectRow('Test Project').findKebabAction('Delete project').click();
cy.interceptK8s(
'POST',
SelfSubjectAccessReviewModel,
mockSelfSubjectAccessReview({ allowed: true }),
).as('selfSubjectAccessReviewsCall');
const deleteProject = projectListPage
.getProjectRow('Test Project')
.findKebabAction('Delete project');
cy.wait('@selfSubjectAccessReviewsCall');
deleteProject.click();
deleteModal.shouldBeOpen();
deleteModal.findSubmitButton().should('be.disabled');
deleteModal.findCancelButton().should('be.enabled').click();
Expand Down Expand Up @@ -154,6 +165,31 @@ describe('Data science projects details', () => {
projectListPage.findProjectLink('renamed').should('not.exist');
});

it('should disable kebab actions with insufficient permissions', () => {
initIntercepts();
projectListPage.visit();
cy.interceptK8s(
'POST',
SelfSubjectAccessReviewModel,
mockSelfSubjectAccessReview({ allowed: false }),
).as('selfSubjectAccessReviewsCall');

const editProject = projectListPage
.getProjectRow('Test Project')
.findKebabAction('Edit project');
const editPermission = projectListPage
.getProjectRow('Test Project')
.findKebabAction('Edit permissions');
const deleteProject = projectListPage
.getProjectRow('Test Project')
.findKebabAction('Delete project');
cy.wait('@selfSubjectAccessReviewsCall');

editProject.should('have.attr', 'aria-disabled', 'true');
editPermission.should('have.attr', 'aria-disabled', 'true');
deleteProject.should('have.attr', 'aria-disabled', 'true');
});

describe('Table filter', () => {
it('filter by name', () => {
initIntercepts();
Expand Down
33 changes: 18 additions & 15 deletions frontend/src/api/useAccessReview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const checkAccess = ({

export const useAccessReview = (
resourceAttributes: AccessReviewResourceAttributes,
shouldRunCheck = true,
): [boolean, boolean] => {
const [loaded, setLoaded] = React.useState(false);
const [isAllowed, setAllowed] = React.useState(false);
Expand All @@ -51,22 +52,24 @@ export const useAccessReview = (
} = resourceAttributes;

React.useEffect(() => {
checkAccess({ group, resource, subresource, verb, name, namespace })
.then((result) => {
if (result.status) {
setAllowed(result.status.allowed);
} else {
if (shouldRunCheck) {
checkAccess({ group, resource, subresource, verb, name, namespace })
.then((result) => {
if (result.status) {
setAllowed(result.status.allowed);
} else {
setAllowed(true);
}
setLoaded(true);
})
.catch((e) => {
// eslint-disable-next-line no-console
console.warn('SelfSubjectAccessReview failed', e);
setAllowed(true);
}
setLoaded(true);
})
.catch((e) => {
// eslint-disable-next-line no-console
console.warn('SelfSubjectAccessReview failed', e);
setAllowed(true);
setLoaded(true);
});
}, [group, name, namespace, resource, subresource, verb]);
setLoaded(true);
});
}
}, [group, name, namespace, resource, subresource, verb, shouldRunCheck]);

return [isAllowed, loaded];
};
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@ const ProjectTableRow: React.FC<ProjectTableRowProps> = ({
const navigate = useNavigate();
const owner = getProjectOwner(project);

const item = useProjectTableRowItems(project, isRefreshing, setEditData, setDeleteData);
const [item, runAccessCheck] = useProjectTableRowItems(
project,
isRefreshing,
setEditData,
setDeleteData,
);
const [notebookStates, loaded] = useProjectNotebookStates(project.metadata.name);

return (
Expand Down Expand Up @@ -130,6 +135,8 @@ const ProjectTableRow: React.FC<ProjectTableRowProps> = ({
className="odh-project-table__action-column"
isActionCell
rowSpan={notebookStates.length || 1}
onMouseEnter={runAccessCheck}
onClick={runAccessCheck}
>
<ActionsColumn items={item} />
</Td>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import * as React from 'react';
import { useNavigate } from 'react-router-dom';
import { TooltipProps } from '@patternfly/react-core';
import { useAccessReview } from '~/api';
import { AccessReviewResourceAttributes, ProjectKind } from '~/k8sTypes';

type KebabItem = {
title?: string;
isDisabled?: boolean;
isAriaDisabled?: boolean;
isSeparator?: boolean;
onClick?: () => void;
tooltipProps?: TooltipProps;
};
const accessReviewResource: AccessReviewResourceAttributes = {
group: 'rbac.authorization.k8s.io',
Expand All @@ -18,40 +21,72 @@ const useProjectTableRowItems = (
isRefreshing: boolean,
setEditData: (data: ProjectKind) => void,
setDeleteData: (data: ProjectKind) => void,
): KebabItem[] => {
const [allowCreate] = useAccessReview({
...accessReviewResource,
namespace: project.metadata.name,
});
): [KebabItem[], () => void] => {
const navigate = useNavigate();
const [shouldRunCheck, setShouldRunCheck] = React.useState(false);

const [allowUpdate, allowUpdateLoaded] = useAccessReview(
{
...accessReviewResource,
namespace: project.metadata.name,
verb: 'update',
},
shouldRunCheck,
);
const [allowCreate, allowCreateLoaded] = useAccessReview(
{
...accessReviewResource,
namespace: project.metadata.name,
},
shouldRunCheck,
);
const [allowDelete, allowDeleteLoaded] = useAccessReview(
{
...accessReviewResource,
namespace: project.metadata.name,
verb: 'delete',
},
shouldRunCheck,
);

const runAccesCheck = React.useCallback(() => {
setShouldRunCheck(true);
}, []);

const noPermissionToolTip = (allow: boolean, loaded: boolean): Partial<KebabItem> | undefined =>
!allow && loaded
? { tooltipProps: { content: 'You do not have permissions to perform this action' } }
: undefined;

const item: KebabItem[] = [
{
title: 'Edit project',
isDisabled: isRefreshing,
isAriaDisabled: isRefreshing || !allowUpdate || !allowUpdateLoaded,
onClick: () => {
setEditData(project);
},
...noPermissionToolTip(allowUpdate, allowUpdateLoaded),
},
{
title: 'Edit permissions',
isAriaDisabled: !allowCreate || !allowCreateLoaded,
onClick: () => {
navigate(`/projects/${project.metadata.name}?section=permissions`);
},
...noPermissionToolTip(allowCreate, allowCreateLoaded),
},
...(allowCreate
? [
{
title: 'Edit permissions',
onClick: () => {
navigate(`/projects/${project.metadata.name}?section=permissions`);
},
},
]
: []),
{
isSeparator: true,
},
{
title: 'Delete project',
isAriaDisabled: !allowDelete || !allowDeleteLoaded,
onClick: () => {
setDeleteData(project);
},
...noPermissionToolTip(allowDelete, allowDeleteLoaded),
},
];
return item;
return [item, runAccesCheck];
};
export default useProjectTableRowItems;

0 comments on commit 81961d4

Please sign in to comment.