Skip to content

Commit

Permalink
refactor(rbac): reduce cognitive stress for create, update and delete…
Browse files Browse the repository at this point in the history 
… role functions (#1878)
  • Loading branch information
@divyanshiGupta
divyanshiGupta authored Jul 11, 2024
1 parent 5ed2587 commit 38c3144
Show file tree
Hide file tree
Showing 7 changed files with 398 additions and 163 deletions.
69 changes: 40 additions & 29 deletions plugins/rbac/src/__fixtures__/mockConditions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,41 +5,52 @@ import {
RoleConditionalPolicyDecision,
} from '@janus-idp/backstage-plugin-rbac-common';

import { RoleBasedConditions } from '../types';

export const mockNewConditions: RoleBasedConditions[] = [
{
result: AuthorizeResult.CONDITIONAL,
pluginId: 'catalog',
resourceType: 'catalog-entity',
conditions: {
rule: 'HAS_ANNOTATION',
resourceType: 'catalog-entity',
params: { annotation: 'temp' },
},
roleEntityRef: 'role:default/rbac_admin',
permissionMapping: ['read'],
},
{
result: AuthorizeResult.CONDITIONAL,
pluginId: 'catalog',
resourceType: 'catalog-entity',
conditions: {
allOf: [
{
rule: 'HAS_LABEL',
resourceType: 'catalog-entity',
params: { label: 'temp' },
},
{
rule: 'HAS_METADATA',
resourceType: 'catalog-entity',
params: { key: 'status' },
},
],
},
roleEntityRef: 'role:default/rbac_admin',
permissionMapping: ['delete', 'update'],
},
];

export const mockConditions: RoleConditionalPolicyDecision<PermissionAction>[] =
[
{
id: 1,
result: AuthorizeResult.CONDITIONAL,
pluginId: 'catalog',
resourceType: 'catalog-entity',
conditions: {
rule: 'HAS_ANNOTATION',
resourceType: 'catalog-entity',
params: { annotation: 'temp' },
},
roleEntityRef: 'role:default/rbac_admin',
permissionMapping: ['read'],
...mockNewConditions[0],
},
{
id: 2,
result: AuthorizeResult.CONDITIONAL,
pluginId: 'catalog',
resourceType: 'catalog-entity',
conditions: {
allOf: [
{
rule: 'HAS_LABEL',
resourceType: 'catalog-entity',
params: { label: 'temp' },
},
{
rule: 'HAS_METADATA',
resourceType: 'catalog-entity',
params: { key: 'status' },
},
],
},
roleEntityRef: 'role:default/rbac_admin',
permissionMapping: ['delete', 'update'],
...mockNewConditions[1],
},
];
119 changes: 21 additions & 98 deletions plugins/rbac/src/components/CreateRole/RoleForm.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ import {
isSamePermissionPolicy,
onlyInLeft,
} from '../../utils/rbac-utils';
import {
createConditions,
createPermissions,
modifyConditions,
removeConditions,
removePermissions,
} from '../../utils/role-form-utils';
import { AddedMembersTable } from './AddedMembersTable';
import { AddMembersForm } from './AddMembersForm';
import { PermissionPoliciesForm } from './PermissionPoliciesForm';
Expand Down Expand Up @@ -114,79 +121,12 @@ export const RoleForm = ({
isSamePermissionPolicy,
);

if (newPermissions.length > 0) {
const permissionsRes = await rbacApi.createPolicies(newPermissions);
if ((permissionsRes as unknown as RoleError).error) {
throw new Error(
`Unable to create the permissions. ${
(permissionsRes as unknown as RoleError).error.message
}`,
);
}
}

if (deletePermissions.length > 0) {
const permissionsRes = await rbacApi.deletePolicies(
name,
deletePermissions,
);
if ((permissionsRes as unknown as RoleError).error) {
throw new Error(
`Unable to delete the permissions. ${
(permissionsRes as unknown as RoleError).error.message
}`,
);
}
}

if (deleteConditions.length > 0) {
const promises = deleteConditions.map(cid =>
rbacApi.deleteConditionalPolicies(cid),
);

const cppRes: (Response | RoleError)[] = await Promise.all(promises);
const cpErr = cppRes
.map(r => (r as unknown as RoleError).error?.message)
.filter(m => m);

if (cpErr.length > 0) {
throw new Error(
`Unable to remove conditions from the role. ${cpErr.join('\n')}`,
);
}
}

if (newConditions.length > 0) {
const promises = newConditions.map(cpp =>
rbacApi.createConditionalPermission(cpp),
);

const cppRes: (Response | RoleError)[] = await Promise.all(promises);
const cpErr = cppRes
.map(r => (r as unknown as RoleError).error?.message)
.filter(m => m);

if (cpErr.length > 0) {
throw new Error(
`Unable to add conditions to the role. ${cpErr.join('\n')}`,
);
}
}

if (updateConditions.length > 0) {
const promises = updateConditions.map(({ id, updateCondition }) =>
rbacApi.updateConditionalPolicies(id, updateCondition),
);
await removePermissions(name, deletePermissions, rbacApi);
await createPermissions(newPermissions, rbacApi);

const cppRes: (Response | RoleError)[] = await Promise.all(promises);
const cpErr = cppRes
.map(r => (r as unknown as RoleError).error?.message)
.filter(m => m);

if (cpErr.length > 0) {
throw new Error(`Unable to update conditions. ${cpErr.join('\n')}`);
}
}
await removeConditions(deleteConditions, rbacApi);
await modifyConditions(updateConditions, rbacApi);
await createConditions(newConditions, rbacApi);

navigateTo(`${name} updated`);
}
Expand All @@ -212,34 +152,17 @@ export const RoleForm = ({
);
}

if (newPermissionsData?.length > 0) {
const permissionsRes: Response | RoleError =
await rbacApi.createPolicies(newPermissionsData);
if ((permissionsRes as unknown as RoleError).error) {
throw new Error(
`Role was created successfully but unable to add permissions to the role. ${
(permissionsRes as unknown as RoleError).error.message
}`,
);
}
}

const promises = newConditionalPermissionPoliciesData.map(cpp =>
rbacApi.createConditionalPermission(cpp),
await createPermissions(
newPermissionsData,
rbacApi,
'Role was created successfully but unable to add permission policies to the role.',
);

const cppRes: (Response | RoleError)[] = await Promise.all(promises);
const cpErr = cppRes
.map(r => (r as unknown as RoleError).error?.message)
.filter(m => m);

if (cpErr.length > 0) {
throw new Error(
`Role was created successfully but unable to add conditions to the role. ${cpErr.join(
'\n',
)}`,
);
}
await createConditions(
newConditionalPermissionPoliciesData,
rbacApi,
'Role created successfully but unable to add conditions to the role.',
);

navigateTo(`${newData.name} created`);
} catch (e) {
Expand Down
43 changes: 9 additions & 34 deletions plugins/rbac/src/components/RolesList/DeleteRoleDialog.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@ import { Alert } from '@material-ui/lab';
import { RoleBasedPolicy } from '@janus-idp/backstage-plugin-rbac-common';

import { rbacApiRef } from '../../api/RBACBackendClient';
import { RoleError } from '../../types';
import { getMembers } from '../../utils/rbac-utils';
import {
removeConditions,
removePermissions,
} from '../../utils/role-form-utils';
import { useToast } from '../ToastContext';

const useStyles = makeStyles((theme: Theme) =>
Expand Down Expand Up @@ -70,45 +73,17 @@ const DeleteRoleDialog = ({
try {
const policies = await rbacApi.getAssociatedPolicies(roleName);
const conditionalPolicies = await rbacApi.getRoleConditions(roleName);

if (Array.isArray(policies)) {
const allowedPolicies = policies.filter(
(policy: RoleBasedPolicy) => policy.effect !== 'deny',
);
if (allowedPolicies.length > 0) {
const cleanupPoliciesResponse = await rbacApi.deletePolicies(
roleName,
allowedPolicies,
);
if ((cleanupPoliciesResponse as unknown as RoleError)?.error) {
setError(
`Unable to delete policies. ${
(cleanupPoliciesResponse as unknown as RoleError).error.message
}`,
);
return;
}
}
await removePermissions(roleName, allowedPolicies, rbacApi);
}

if (
Array.isArray(conditionalPolicies) &&
conditionalPolicies.length > 0
) {
const cleanupPoliciesPromises = conditionalPolicies.map(cp =>
rbacApi.deleteConditionalPolicies(cp.id),
);
const res: (Response | RoleError)[] = await Promise.all(
cleanupPoliciesPromises,
);
const err = res
.map(r => (r as unknown as RoleError).error?.message)
.filter(m => m);
if (err.length > 0) {
setError(
`Unable to remove conditions from the role. ${err.join('\n')}`,
);
return;
}
if (Array.isArray(conditionalPolicies)) {
const conditionalPoliciesIds = conditionalPolicies.map(cp => cp.id);
await removeConditions(conditionalPoliciesIds, rbacApi);
}

const response = await rbacApi.deleteRole(roleName);
Expand Down
5 changes: 5 additions & 0 deletions plugins/rbac/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,8 @@ export type PluginConditionRules = {
pluginId: string;
rules: ConditionRule[];
};

export type UpdatedConditionsData = {
id: number;
updateCondition: RoleBasedConditions;
}[];
9 changes: 7 additions & 2 deletions plugins/rbac/src/utils/create-role-utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ import {
RoleFormValues,
SelectedMember,
} from '../components/CreateRole/types';
import { MemberEntity, PermissionsData, RoleBasedConditions } from '../types';
import {
MemberEntity,
PermissionsData,
RoleBasedConditions,
UpdatedConditionsData,
} from '../types';

export const uniqBy = (arr: string[], iteratee: (arg: string) => any) => {
return arr.filter(
Expand Down Expand Up @@ -218,7 +223,7 @@ export const getConditionalPermissionPoliciesData = (
export const getUpdatedConditionalPolicies = (
values: RoleFormValues,
initialValues: RoleFormValues,
) => {
): UpdatedConditionsData => {
const initialConditionsWithId = initialValues.permissionPoliciesRows.filter(
ppr => ppr.id,
);
Expand Down
Loading

0 comments on commit 38c3144

Please sign in to comment.