fix(#12954): restrict roles and policy in settings for admins only (#13073)

* fix(#12954): restrict roles and policy in settings for admins only

* chore: address comments
This commit is contained in:
Sachin Chaurasiya 2023-09-05 15:19:24 +05:30 committed by GitHub
parent 011eaf8ad5
commit 80c11ab4f5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 282 additions and 440 deletions

View File

@ -19,15 +19,16 @@ import { ROUTES } from '../../constants/constants';
import { useAuth } from '../../hooks/authHooks'; import { useAuth } from '../../hooks/authHooks';
interface AdminProtectedRouteProps extends RouteProps { interface AdminProtectedRouteProps extends RouteProps {
hasPermission: boolean; hasPermission?: boolean;
} }
const AdminProtectedRoute = (routeProps: AdminProtectedRouteProps) => { const AdminProtectedRoute = (routeProps: AdminProtectedRouteProps) => {
const { isAdminUser } = useAuth(); const { isAdminUser } = useAuth();
const hasPermission = Boolean(routeProps.hasPermission);
if (isAdminUser || routeProps.hasPermission) { if (isAdminUser || hasPermission) {
return <Route {...routeProps} />; return <Route {...routeProps} />;
} else if (!routeProps.hasPermission) { } else if (!hasPermission) {
return <ErrorPlaceHolder type={ERROR_PLACEHOLDER_TYPE.PERMISSION} />; return <ErrorPlaceHolder type={ERROR_PLACEHOLDER_TYPE.PERMISSION} />;
} else { } else {
return <Redirect to={ROUTES.SIGNIN} />; return <Redirect to={ROUTES.SIGNIN} />;

View File

@ -175,17 +175,13 @@ const GlobalSettingRouter = () => {
<AdminProtectedRoute <AdminProtectedRoute
exact exact
component={RolesListPage} component={RolesListPage}
hasPermission={userPermissions.hasViewPermissions(
ResourceEntity.ROLE,
permissions
)}
path={getSettingPath( path={getSettingPath(
GlobalSettingsMenuCategory.ACCESS, GlobalSettingsMenuCategory.ACCESS,
GlobalSettingOptions.ROLES GlobalSettingOptions.ROLES
)} )}
/> />
<Route <AdminProtectedRoute
exact exact
component={RolesDetailPage} component={RolesDetailPage}
path={getSettingPath( path={getSettingPath(
@ -201,16 +197,12 @@ const GlobalSettingRouter = () => {
<AdminProtectedRoute <AdminProtectedRoute
exact exact
component={PoliciesListPage} component={PoliciesListPage}
hasPermission={userPermissions.hasViewPermissions(
ResourceEntity.POLICY,
permissions
)}
path={getSettingPath( path={getSettingPath(
GlobalSettingsMenuCategory.ACCESS, GlobalSettingsMenuCategory.ACCESS,
GlobalSettingOptions.POLICIES GlobalSettingOptions.POLICIES
)} )}
/> />
<Route <AdminProtectedRoute
exact exact
component={PoliciesDetailPage} component={PoliciesDetailPage}
path={getSettingPath( path={getSettingPath(

View File

@ -57,20 +57,6 @@ jest.mock('components/Loader/Loader', () =>
jest.fn().mockReturnValue(<div>Loader</div>) jest.fn().mockReturnValue(<div>Loader</div>)
); );
jest.mock('components/PermissionProvider/PermissionProvider', () => ({
usePermissionProvider: jest.fn().mockReturnValue({
getEntityPermissionByFqn: jest.fn().mockReturnValue({
Create: true,
Delete: true,
ViewAll: true,
EditAll: true,
EditDescription: true,
EditDisplayName: true,
EditCustomFields: true,
}),
}),
}));
jest.mock('../../../constants/HelperTextUtil', () => ({ jest.mock('../../../constants/HelperTextUtil', () => ({
NO_PERMISSION_FOR_ACTION: '', NO_PERMISSION_FOR_ACTION: '',
NO_PERMISSION_TO_VIEW: '', NO_PERMISSION_TO_VIEW: '',
@ -80,18 +66,6 @@ jest.mock('../../../utils/CommonUtils', () => ({
getEntityName: jest.fn().mockReturnValue(''), getEntityName: jest.fn().mockReturnValue(''),
})); }));
jest.mock('../../../utils/PermissionsUtils', () => ({
DEFAULT_ENTITY_PERMISSION: {
Create: true,
Delete: true,
ViewAll: true,
EditAll: true,
EditDescription: true,
EditDisplayName: true,
EditCustomFields: true,
},
}));
jest.mock('../../../utils/RouterUtils', () => ({ jest.mock('../../../utils/RouterUtils', () => ({
getAddPolicyRulePath: jest.fn(), getAddPolicyRulePath: jest.fn(),
getEditPolicyRulePath: jest.fn(), getEditPolicyRulePath: jest.fn(),

View File

@ -22,7 +22,6 @@ import {
Row, Row,
Space, Space,
Tabs, Tabs,
Tooltip,
Typography, Typography,
} from 'antd'; } from 'antd';
import { ReactComponent as EditIcon } from 'assets/svg/edit-new.svg'; import { ReactComponent as EditIcon } from 'assets/svg/edit-new.svg';
@ -32,12 +31,6 @@ import ErrorPlaceHolder from 'components/common/error-with-placeholder/ErrorPlac
import RichTextEditorPreviewer from 'components/common/rich-text-editor/RichTextEditorPreviewer'; import RichTextEditorPreviewer from 'components/common/rich-text-editor/RichTextEditorPreviewer';
import TitleBreadcrumb from 'components/common/title-breadcrumb/title-breadcrumb.component'; import TitleBreadcrumb from 'components/common/title-breadcrumb/title-breadcrumb.component';
import Loader from 'components/Loader/Loader'; import Loader from 'components/Loader/Loader';
import { usePermissionProvider } from 'components/PermissionProvider/PermissionProvider';
import {
OperationPermission,
ResourceEntity,
} from 'components/PermissionProvider/PermissionProvider.interface';
import { ERROR_PLACEHOLDER_TYPE } from 'enums/common.enum';
import { compare } from 'fast-json-patch'; import { compare } from 'fast-json-patch';
import { isEmpty, isUndefined, startCase } from 'lodash'; import { isEmpty, isUndefined, startCase } from 'lodash';
import React, { useCallback, useEffect, useMemo, useState } from 'react'; import React, { useCallback, useEffect, useMemo, useState } from 'react';
@ -59,7 +52,6 @@ import { EntityType } from '../../../enums/entity.enum';
import { Rule } from '../../../generated/api/policies/createPolicy'; import { Rule } from '../../../generated/api/policies/createPolicy';
import { Policy } from '../../../generated/entity/policies/policy'; import { Policy } from '../../../generated/entity/policies/policy';
import { EntityReference } from '../../../generated/type/entityReference'; import { EntityReference } from '../../../generated/type/entityReference';
import { DEFAULT_ENTITY_PERMISSION } from '../../../utils/PermissionsUtils';
import { import {
getAddPolicyRulePath, getAddPolicyRulePath,
getEditPolicyRulePath, getEditPolicyRulePath,
@ -78,7 +70,6 @@ const PoliciesDetailPage = () => {
const { t } = useTranslation(); const { t } = useTranslation();
const history = useHistory(); const history = useHistory();
const { fqn } = useParams<{ fqn: string }>(); const { fqn } = useParams<{ fqn: string }>();
const { getEntityPermissionByFqn } = usePermissionProvider();
const [policy, setPolicy] = useState<Policy>({} as Policy); const [policy, setPolicy] = useState<Policy>({} as Policy);
const [isLoading, setLoading] = useState<boolean>(false); const [isLoading, setLoading] = useState<boolean>(false);
@ -87,10 +78,6 @@ const PoliciesDetailPage = () => {
const [selectedEntity, setEntity] = const [selectedEntity, setEntity] =
useState<{ attribute: Attribute; record: EntityReference }>(); useState<{ attribute: Attribute; record: EntityReference }>();
const [policyPermission, setPolicyPermission] = useState<OperationPermission>(
DEFAULT_ENTITY_PERMISSION
);
const policiesPath = getSettingPath( const policiesPath = getSettingPath(
GlobalSettingsMenuCategory.ACCESS, GlobalSettingsMenuCategory.ACCESS,
GlobalSettingOptions.POLICIES GlobalSettingOptions.POLICIES
@ -110,21 +97,6 @@ const PoliciesDetailPage = () => {
[policy] [policy]
); );
const fetchPolicyPermission = async () => {
setLoading(true);
try {
const response = await getEntityPermissionByFqn(
ResourceEntity.POLICY,
fqn
);
setPolicyPermission(response);
} catch (error) {
showErrorToast(error as AxiosError);
} finally {
setLoading(false);
}
};
const fetchPolicy = async () => { const fetchPolicy = async () => {
setLoading(true); setLoading(true);
try { try {
@ -256,7 +228,6 @@ const PoliciesDetailPage = () => {
(rule: Rule) => { (rule: Rule) => {
return ( return (
<Dropdown <Dropdown
disabled={!policyPermission.EditAll}
overlay={ overlay={
<Menu <Menu
items={[ items={[
@ -307,15 +278,8 @@ const PoliciesDetailPage = () => {
} }
placement="bottomRight" placement="bottomRight"
trigger={['click']}> trigger={['click']}>
<Tooltip
title={
policyPermission.EditAll
? t('label.manage-rule')
: t('message.no-permission-for-action')
}>
<Button <Button
data-testid={`manage-button-${rule.name}`} data-testid={`manage-button-${rule.name}`}
disabled={!policyPermission.EditAll}
icon={<EllipsisOutlined className="text-grey-body" rotate={90} />} icon={<EllipsisOutlined className="text-grey-body" rotate={90} />}
size="small" size="small"
type="text" type="text"
@ -323,22 +287,15 @@ const PoliciesDetailPage = () => {
e.stopPropagation(); e.stopPropagation();
}} }}
/> />
</Tooltip>
</Dropdown> </Dropdown>
); );
}, },
[policy, policyPermission] [policy]
); );
useEffect(() => { useEffect(() => {
fetchPolicyPermission();
}, [fqn]);
useEffect(() => {
if (policyPermission.ViewAll || policyPermission.ViewBasic) {
fetchPolicy(); fetchPolicy();
} }, [fqn]);
}, [policyPermission, fqn]);
if (isLoading) { if (isLoading) {
return <Loader />; return <Loader />;
@ -347,7 +304,7 @@ const PoliciesDetailPage = () => {
return ( return (
<div data-testid="policy-details-container"> <div data-testid="policy-details-container">
<TitleBreadcrumb titleLinks={breadcrumb} /> <TitleBreadcrumb titleLinks={breadcrumb} />
{policyPermission.ViewAll || policyPermission.ViewBasic ? (
<> <>
{isEmpty(policy) ? ( {isEmpty(policy) ? (
<ErrorPlaceHolder> <ErrorPlaceHolder>
@ -375,14 +332,12 @@ const PoliciesDetailPage = () => {
{getEntityName(policy)} {getEntityName(policy)}
</Typography.Title> </Typography.Title>
<Description <Description
hasEditAccess
className="m-b-md" className="m-b-md"
description={policy.description || ''} description={policy.description || ''}
entityFqn={policy.fullyQualifiedName} entityFqn={policy.fullyQualifiedName}
entityName={getEntityName(policy)} entityName={getEntityName(policy)}
entityType={EntityType.POLICY} entityType={EntityType.POLICY}
hasEditAccess={
policyPermission.EditAll || policyPermission.EditDescription
}
isEdit={editDescription} isEdit={editDescription}
onCancel={() => setEditDescription(false)} onCancel={() => setEditDescription(false)}
onDescriptionEdit={() => setEditDescription(true)} onDescriptionEdit={() => setEditDescription(true)}
@ -394,35 +349,19 @@ const PoliciesDetailPage = () => {
{isEmpty(policy.rules) ? ( {isEmpty(policy.rules) ? (
<ErrorPlaceHolder /> <ErrorPlaceHolder />
) : ( ) : (
<Space <Space className="w-full tabpane-space" direction="vertical">
className="w-full tabpane-space"
direction="vertical">
<Tooltip
title={
policyPermission.EditAll
? t('label.add-entity', {
entity: t('label.rule'),
})
: t('message.no-permission-for-action')
}>
<Button <Button
data-testid="add-rule" data-testid="add-rule"
disabled={!policyPermission.EditAll}
type="primary" type="primary"
onClick={() => onClick={() => history.push(getAddPolicyRulePath(fqn))}>
history.push(getAddPolicyRulePath(fqn))
}>
{t('label.add-entity', { {t('label.add-entity', {
entity: t('label.rule'), entity: t('label.rule'),
})} })}
</Button> </Button>
</Tooltip>
<Space className="w-full" direction="vertical" size={20}> <Space className="w-full" direction="vertical" size={20}>
{policy.rules.map((rule) => ( {policy.rules.map((rule) => (
<Card <Card data-testid="rule-card" key={rule.name || 'rule'}>
data-testid="rule-card"
key={rule.name || 'rule'}>
<Space <Space
align="baseline" align="baseline"
className="w-full justify-between p-b-lg" className="w-full justify-between p-b-lg"
@ -514,7 +453,7 @@ const PoliciesDetailPage = () => {
</TabPane> </TabPane>
<TabPane key="roles" tab={t('label.role-plural')}> <TabPane key="roles" tab={t('label.role-plural')}>
<PoliciesDetailsList <PoliciesDetailsList
hasAccess={policyPermission.EditAll} hasAccess
list={policy.roles ?? []} list={policy.roles ?? []}
type="role" type="role"
onDelete={(record) => onDelete={(record) =>
@ -524,7 +463,7 @@ const PoliciesDetailPage = () => {
</TabPane> </TabPane>
<TabPane key="teams" tab={t('label.team-plural')}> <TabPane key="teams" tab={t('label.team-plural')}>
<PoliciesDetailsList <PoliciesDetailsList
hasAccess={policyPermission.EditAll} hasAccess
list={policy.teams ?? []} list={policy.teams ?? []}
type="team" type="team"
onDelete={(record) => onDelete={(record) =>
@ -536,9 +475,7 @@ const PoliciesDetailPage = () => {
</div> </div>
)} )}
</> </>
) : (
<ErrorPlaceHolder type={ERROR_PLACEHOLDER_TYPE.PERMISSION} />
)}
{selectedEntity && ( {selectedEntity && (
<Modal <Modal
centered centered

View File

@ -60,20 +60,6 @@ jest.mock('../../../utils/RouterUtils', () => ({
getTeamsWithFqnPath: jest.fn(), getTeamsWithFqnPath: jest.fn(),
})); }));
jest.mock('components/PermissionProvider/PermissionProvider', () => ({
usePermissionProvider: jest.fn().mockReturnValue({
getEntityPermissionByFqn: jest.fn().mockReturnValue({
Create: true,
Delete: true,
ViewAll: true,
EditAll: true,
EditDescription: true,
EditDisplayName: true,
EditCustomFields: true,
}),
}),
}));
describe('Test Roles Details Page', () => { describe('Test Roles Details Page', () => {
it('Should render the detail component', async () => { it('Should render the detail component', async () => {
render(<RolesDetailPage />); render(<RolesDetailPage />);

View File

@ -11,17 +11,12 @@
* limitations under the License. * limitations under the License.
*/ */
import { Button, Modal, Space, Tabs, Tooltip, Typography } from 'antd'; import { Button, Modal, Space, Tabs, Typography } from 'antd';
import { AxiosError } from 'axios'; import { AxiosError } from 'axios';
import Description from 'components/common/description/Description'; import Description from 'components/common/description/Description';
import ErrorPlaceHolder from 'components/common/error-with-placeholder/ErrorPlaceHolder'; import ErrorPlaceHolder from 'components/common/error-with-placeholder/ErrorPlaceHolder';
import TitleBreadcrumb from 'components/common/title-breadcrumb/title-breadcrumb.component'; import TitleBreadcrumb from 'components/common/title-breadcrumb/title-breadcrumb.component';
import Loader from 'components/Loader/Loader'; import Loader from 'components/Loader/Loader';
import { usePermissionProvider } from 'components/PermissionProvider/PermissionProvider';
import {
OperationPermission,
ResourceEntity,
} from 'components/PermissionProvider/PermissionProvider.interface';
import { ERROR_PLACEHOLDER_TYPE } from 'enums/common.enum'; import { ERROR_PLACEHOLDER_TYPE } from 'enums/common.enum';
import { compare } from 'fast-json-patch'; import { compare } from 'fast-json-patch';
import { isEmpty, isUndefined } from 'lodash'; import { isEmpty, isUndefined } from 'lodash';
@ -39,7 +34,6 @@ import {
import { EntityType } from '../../../enums/entity.enum'; import { EntityType } from '../../../enums/entity.enum';
import { Role } from '../../../generated/entity/teams/role'; import { Role } from '../../../generated/entity/teams/role';
import { EntityReference } from '../../../generated/type/entityReference'; import { EntityReference } from '../../../generated/type/entityReference';
import { DEFAULT_ENTITY_PERMISSION } from '../../../utils/PermissionsUtils';
import { getSettingPath } from '../../../utils/RouterUtils'; import { getSettingPath } from '../../../utils/RouterUtils';
import { showErrorToast } from '../../../utils/ToastUtils'; import { showErrorToast } from '../../../utils/ToastUtils';
import AddAttributeModal from '../AddAttributeModal/AddAttributeModal'; import AddAttributeModal from '../AddAttributeModal/AddAttributeModal';
@ -58,7 +52,6 @@ interface AddAttribute {
const RolesDetailPage = () => { const RolesDetailPage = () => {
const history = useHistory(); const history = useHistory();
const { t } = useTranslation(); const { t } = useTranslation();
const { getEntityPermissionByFqn } = usePermissionProvider();
const { fqn } = useParams<{ fqn: string }>(); const { fqn } = useParams<{ fqn: string }>();
const [role, setRole] = useState<Role>({} as Role); const [role, setRole] = useState<Role>({} as Role);
@ -70,10 +63,6 @@ const RolesDetailPage = () => {
const [addAttribute, setAddAttribute] = useState<AddAttribute>(); const [addAttribute, setAddAttribute] = useState<AddAttribute>();
const [rolePermission, setRolePermission] = useState<OperationPermission>(
DEFAULT_ENTITY_PERMISSION
);
const rolesPath = getSettingPath( const rolesPath = getSettingPath(
GlobalSettingsMenuCategory.ACCESS, GlobalSettingsMenuCategory.ACCESS,
GlobalSettingOptions.ROLES GlobalSettingOptions.ROLES
@ -93,18 +82,6 @@ const RolesDetailPage = () => {
[role] [role]
); );
const fetchRolePermission = async () => {
setLoading(true);
try {
const response = await getEntityPermissionByFqn(ResourceEntity.ROLE, fqn);
setRolePermission(response);
} catch (error) {
showErrorToast(error as AxiosError);
} finally {
setLoading(false);
}
};
const fetchRole = async () => { const fetchRole = async () => {
setLoading(true); setLoading(true);
try { try {
@ -238,14 +215,8 @@ const RolesDetailPage = () => {
}; };
useEffect(() => { useEffect(() => {
fetchRolePermission();
}, [fqn]);
useEffect(() => {
if (rolePermission.ViewAll || rolePermission.ViewBasic) {
fetchRole(); fetchRole();
} }, [fqn]);
}, [rolePermission, fqn]);
if (isLoading) { if (isLoading) {
return <Loader />; return <Loader />;
@ -254,7 +225,7 @@ const RolesDetailPage = () => {
return ( return (
<div data-testid="role-details-container"> <div data-testid="role-details-container">
<TitleBreadcrumb titleLinks={breadcrumb} /> <TitleBreadcrumb titleLinks={breadcrumb} />
{rolePermission.ViewAll || rolePermission.ViewBasic ? (
<> <>
{isEmpty(role) ? ( {isEmpty(role) ? (
<ErrorPlaceHolder type={ERROR_PLACEHOLDER_TYPE.CUSTOM}> <ErrorPlaceHolder type={ERROR_PLACEHOLDER_TYPE.CUSTOM}>
@ -283,14 +254,12 @@ const RolesDetailPage = () => {
{getEntityName(role)} {getEntityName(role)}
</Typography.Title> </Typography.Title>
<Description <Description
hasEditAccess
className="m-b-md" className="m-b-md"
description={role.description || ''} description={role.description || ''}
entityFqn={role.fullyQualifiedName} entityFqn={role.fullyQualifiedName}
entityName={getEntityName(role)} entityName={getEntityName(role)}
entityType={EntityType.ROLE} entityType={EntityType.ROLE}
hasEditAccess={
rolePermission.EditAll || rolePermission.EditDescription
}
isEdit={editDescription} isEdit={editDescription}
onCancel={() => setEditDescription(false)} onCancel={() => setEditDescription(false)}
onDescriptionEdit={() => setEditDescription(true)} onDescriptionEdit={() => setEditDescription(true)}
@ -300,17 +269,8 @@ const RolesDetailPage = () => {
<Tabs data-testid="tabs" defaultActiveKey="policies"> <Tabs data-testid="tabs" defaultActiveKey="policies">
<TabPane key="policies" tab={t('label.policy-plural')}> <TabPane key="policies" tab={t('label.policy-plural')}>
<Space className="w-full" direction="vertical"> <Space className="w-full" direction="vertical">
<Tooltip
title={
rolePermission.EditAll
? t('label.add-entity', {
entity: t('label.policy'),
})
: t('message.no-permission-for-action')
}>
<Button <Button
data-testid="add-policy" data-testid="add-policy"
disabled={!rolePermission.EditAll}
type="primary" type="primary"
onClick={() => onClick={() =>
setAddAttribute({ setAddAttribute({
@ -322,9 +282,9 @@ const RolesDetailPage = () => {
entity: t('label.policy'), entity: t('label.policy'),
})} })}
</Button> </Button>
</Tooltip>
<RolesDetailPageList <RolesDetailPageList
hasAccess={rolePermission.EditAll} hasAccess
list={role.policies ?? []} list={role.policies ?? []}
type="policy" type="policy"
onDelete={(record) => onDelete={(record) =>
@ -335,7 +295,7 @@ const RolesDetailPage = () => {
</TabPane> </TabPane>
<TabPane key="teams" tab={t('label.team-plural')}> <TabPane key="teams" tab={t('label.team-plural')}>
<RolesDetailPageList <RolesDetailPageList
hasAccess={rolePermission.EditAll} hasAccess
list={role.teams ?? []} list={role.teams ?? []}
type="team" type="team"
onDelete={(record) => onDelete={(record) =>
@ -345,7 +305,7 @@ const RolesDetailPage = () => {
</TabPane> </TabPane>
<TabPane key="users" tab={t('label.user-plural')}> <TabPane key="users" tab={t('label.user-plural')}>
<RolesDetailPageList <RolesDetailPageList
hasAccess={rolePermission.EditAll} hasAccess
list={role.users ?? []} list={role.users ?? []}
type="user" type="user"
onDelete={(record) => onDelete={(record) =>
@ -357,9 +317,7 @@ const RolesDetailPage = () => {
</div> </div>
)} )}
</> </>
) : (
<ErrorPlaceHolder type={ERROR_PLACEHOLDER_TYPE.PERMISSION} />
)}
{selectedEntity && ( {selectedEntity && (
<Modal <Modal
centered centered

View File

@ -102,19 +102,13 @@ export const getGlobalSettingsMenuWithPermission = (
items: [ items: [
{ {
label: i18next.t('label.role-plural'), label: i18next.t('label.role-plural'),
isProtected: userPermissions.hasViewPermissions( isProtected: Boolean(isAdminUser),
ResourceEntity.ROLE,
permissions
),
key: 'access.roles', key: 'access.roles',
icon: <RolesIcon className="side-panel-icons" />, icon: <RolesIcon className="side-panel-icons" />,
}, },
{ {
label: i18next.t('label.policy-plural'), label: i18next.t('label.policy-plural'),
isProtected: userPermissions.hasViewPermissions( isProtected: Boolean(isAdminUser),
ResourceEntity.POLICY,
permissions
),
key: 'access.policies', key: 'access.policies',
icon: <PoliciesIcon className="side-panel-icons" />, icon: <PoliciesIcon className="side-panel-icons" />,
}, },