From 017ab19b42969ed662d11291d399294c8f231a4e Mon Sep 17 00:00:00 2001 From: Peter Amstutz Date: Tue, 22 Aug 2023 14:16:29 -0400 Subject: [PATCH] 20829: Migrate to canWrite and canManage for permission checks Replace use of writableBy with canWrite and canManage, and remove writableBy from workbench 2 entirely. Arvados-DCO-1.1-Signed-off-by: Peter Amstutz --- src/models/group.ts | 2 - src/models/test-utils.ts | 1 - src/models/user.ts | 3 +- src/services/auth-service/auth-service.ts | 4 + src/services/user-service/user-service.ts | 3 +- src/store/advanced-tab/advanced-tab.tsx | 5 +- .../context-menu/context-menu-actions.test.ts | 4 +- src/store/resources/resources.test.ts | 6 +- src/store/resources/resources.ts | 22 +---- .../run-process-panel-actions.ts | 2 +- src/store/tree-picker/tree-picker-actions.ts | 2 +- .../workflow-panel/workflow-panel-actions.ts | 2 +- .../data-explorer/renderers.tsx | 65 ++++++------- .../side-panel-button/side-panel-button.tsx | 3 +- .../collection-panel/collection-panel.tsx | 6 +- .../group-details-panel.tsx | 96 +++++++++---------- .../inputs/project-input.tsx | 2 +- 17 files changed, 102 insertions(+), 126 deletions(-) diff --git a/src/models/group.ts b/src/models/group.ts index df4ea6aafa..0932b3c95e 100644 --- a/src/models/group.ts +++ b/src/models/group.ts @@ -15,7 +15,6 @@ export interface GroupResource extends TrashableResource, ResourceWithProperties name: string; groupClass: GroupClass | null; description: string; - writableBy: string[]; ensure_unique_name: boolean; canWrite: boolean; canManage: boolean; @@ -49,7 +48,6 @@ export const selectedFieldsOfGroup = [ "group_class", "description", "properties", - "writable_by", "can_write", "can_manage", "trash_at", diff --git a/src/models/test-utils.ts b/src/models/test-utils.ts index a2a980bf28..74667a915e 100644 --- a/src/models/test-utils.ts +++ b/src/models/test-utils.ts @@ -23,7 +23,6 @@ export const mockGroupResource = (data: Partial = {}): GroupResou properties: "", trashAt: "", uuid: "", - writableBy: [], ensure_unique_name: true, canWrite: false, canManage: false, diff --git a/src/models/user.ts b/src/models/user.ts index 87a2e8c13d..0df6eac241 100644 --- a/src/models/user.ts +++ b/src/models/user.ts @@ -24,6 +24,8 @@ export interface User { prefs: UserPrefs; isAdmin: boolean; isActive: boolean; + canWrite: boolean; + canManage: boolean; } export const getUserFullname = (user: User) => { @@ -62,5 +64,4 @@ export const getUserClusterID = (user: User): string | undefined => { export interface UserResource extends Resource, User { kind: ResourceKind.USER; defaultOwnerUuid: string; - writableBy: string[]; } diff --git a/src/services/auth-service/auth-service.ts b/src/services/auth-service/auth-service.ts index b530e4cd3e..79a6b7e196 100644 --- a/src/services/auth-service/auth-service.ts +++ b/src/services/auth-service/auth-service.ts @@ -35,6 +35,8 @@ export interface UserDetailsResponse { is_active: boolean; username: string; prefs: UserPrefs; + can_write: boolean; + can_manage: boolean; } export class AuthService { @@ -146,6 +148,8 @@ export class AuthService { isAdmin: resp.data.is_admin, isActive: resp.data.is_active, username: resp.data.username, + canWrite: resp.data.can_write, + canManage: resp.data.can_manage, prefs }; }) diff --git a/src/services/user-service/user-service.ts b/src/services/user-service/user-service.ts index 8581b26766..75131f922d 100644 --- a/src/services/user-service/user-service.ts +++ b/src/services/user-service/user-service.ts @@ -12,8 +12,7 @@ export class UserService extends CommonResourceService { constructor(serverApi: AxiosInstance, actions: ApiActions, readOnlyFields: string[] = []) { super(serverApi, "users", actions, readOnlyFields.concat([ 'fullName', - 'isInvited', - 'writableBy', + 'isInvited' ])); } diff --git a/src/store/advanced-tab/advanced-tab.tsx b/src/store/advanced-tab/advanced-tab.tsx index 6333711783..fedd551864 100644 --- a/src/store/advanced-tab/advanced-tab.tsx +++ b/src/store/advanced-tab/advanced-tab.tsx @@ -468,7 +468,7 @@ const collectionApiResponse = (apiResponse: CollectionResource): JSX.Element => const groupRequestApiResponse = (apiResponse: ProjectResource): JSX.Element => { const { uuid, ownerUuid, createdAt, modifiedAt, modifiedByClientUuid, modifiedByUserUuid, name, - description, groupClass, trashAt, isTrashed, deleteAt, properties, writableBy, + description, groupClass, trashAt, isTrashed, deleteAt, properties, canWrite, canManage } = apiResponse; const response = ` "uuid": "${uuid}", @@ -485,8 +485,7 @@ const groupRequestApiResponse = (apiResponse: ProjectResource): JSX.Element => { "delete_at": ${stringify(deleteAt)}, "properties": ${stringifyObject(properties)}, "can_write": ${stringify(canWrite)}, -"can_manage": ${stringify(canManage)}, -"writable_by": ${stringifyObject(writableBy)}`; +"can_manage": ${stringify(canManage)}`; return {'{'} {response} {'\n'} {'}'}; }; diff --git a/src/store/context-menu/context-menu-actions.test.ts b/src/store/context-menu/context-menu-actions.test.ts index d9e87b1a76..623c45088c 100644 --- a/src/store/context-menu/context-menu-actions.test.ts +++ b/src/store/context-menu/context-menu-actions.test.ts @@ -107,13 +107,13 @@ describe('context-menu-actions', () => { [projectUuid]: { uuid: projectUuid, ownerUuid: isEditable ? userUuid : otherUserUuid, - writableBy: isEditable ? [userUuid] : [otherUserUuid], + canWrite: isEditable, groupClass: GroupClass.PROJECT, }, [filterGroupUuid]: { uuid: filterGroupUuid, ownerUuid: isEditable ? userUuid : otherUserUuid, - writableBy: isEditable ? [userUuid] : [otherUserUuid], + canWrite: isEditable, groupClass: GroupClass.FILTER, }, [linkUuid]: { diff --git a/src/store/resources/resources.test.ts b/src/store/resources/resources.test.ts index 300e21d106..503e19a2e8 100644 --- a/src/store/resources/resources.test.ts +++ b/src/store/resources/resources.test.ts @@ -27,7 +27,7 @@ describe('resources', () => { modifiedAt: 'string', href: 'string', kind: ResourceKind.PROJECT, - writableBy: [groupFixtures.user_uuid], + canWrite: true, etag: 'string', }, [groupFixtures.editable_collection_resource_uuid]: { @@ -50,7 +50,7 @@ describe('resources', () => { modifiedAt: 'string', href: 'string', kind: ResourceKind.PROJECT, - writableBy: [groupFixtures.unknown_user_resource_uuid], + canWrite: false, etag: 'string', }, [groupFixtures.not_editable_collection_resource_uuid]: { @@ -137,4 +137,4 @@ describe('resources', () => { expect(result!.isEditable).toBeFalsy(); }); }); -}); \ No newline at end of file +}); diff --git a/src/store/resources/resources.ts b/src/store/resources/resources.ts index 6f7acadae0..a063db9709 100644 --- a/src/store/resources/resources.ts +++ b/src/store/resources/resources.ts @@ -9,26 +9,6 @@ import { GroupResource } from "models/group"; export type ResourcesState = { [key: string]: Resource }; -const getResourceWritableBy = (state: ResourcesState, id: string, userUuid: string): string[] => { - if (!id) { - return []; - } - - if (id === userUuid) { - return [userUuid]; - } - - const resource = (state[id] as ProjectResource); - - if (!resource) { - return []; - } - - const { writableBy } = resource; - - return writableBy || getResourceWritableBy(state, resource.ownerUuid, userUuid); -}; - export const getResourceWithEditableStatus = (id: string, userUuid?: string) => (state: ResourcesState): T | undefined => { if (state[id] === undefined) { return; } @@ -36,7 +16,7 @@ export const getResourceWithEditableStatus = -1 : false; + resource.isEditable = resource.canWrite; } return resource; diff --git a/src/store/run-process-panel/run-process-panel-actions.ts b/src/store/run-process-panel/run-process-panel-actions.ts index 58796295f8..67a89e4c77 100644 --- a/src/store/run-process-panel/run-process-panel-actions.ts +++ b/src/store/run-process-panel/run-process-panel-actions.ts @@ -104,7 +104,7 @@ export const setWorkflow = (workflow: WorkflowResource, isWorkflowChanged = true let owner = getResource(getState().runProcessPanel.processOwnerUuid)(getState().resources); const userUuid = getUserUuid(getState()); - if (!owner || !userUuid || owner.writableBy.indexOf(userUuid) === -1) { + if (!owner || !owner.canWrite) { owner = undefined; } diff --git a/src/store/tree-picker/tree-picker-actions.ts b/src/store/tree-picker/tree-picker-actions.ts index ad657f14e6..72d1cb65d9 100644 --- a/src/store/tree-picker/tree-picker-actions.ts +++ b/src/store/tree-picker/tree-picker-actions.ts @@ -390,7 +390,7 @@ export const loadFavoritesProject = (params: LoadFavoritesProjectParams, id: 'Favorites', pickerId, data: items.filter((item) => { - if (options.showOnlyWritable && (item as GroupResource).writableBy && (item as GroupResource).writableBy.indexOf(uuid) === -1) { + if (options.showOnlyWritable && !(item as GroupResource).canWrite) { return false; } diff --git a/src/store/workflow-panel/workflow-panel-actions.ts b/src/store/workflow-panel/workflow-panel-actions.ts index eab16882e0..2c44fae4e2 100644 --- a/src/store/workflow-panel/workflow-panel-actions.ts +++ b/src/store/workflow-panel/workflow-panel-actions.ts @@ -65,7 +65,7 @@ export const openRunProcess = (workflowUuid: string, ownerUuid?: string, name?: // Must be writable. const userUuid = getUserUuid(getState()); owner = getResource(ownerUuid)(getState().resources); - if (!owner || !userUuid || owner.writableBy.indexOf(userUuid) === -1) { + if (!owner || !owner.canWrite) { owner = undefined; } } diff --git a/src/views-components/data-explorer/renderers.tsx b/src/views-components/data-explorer/renderers.tsx index d274157c48..251304075b 100644 --- a/src/views-components/data-explorer/renderers.tsx +++ b/src/views-components/data-explorer/renderers.tsx @@ -91,7 +91,7 @@ const renderName = (dispatch: Dispatch, item: GroupContentsResource) => { }; -const FrozenProject = (props: {item: ProjectResource}) => { +const FrozenProject = (props: { item: ProjectResource }) => { const [fullUsername, setFullusername] = React.useState(null); const getFullName = React.useCallback(() => { if (props.item.frozenByUuid) { @@ -102,7 +102,7 @@ const FrozenProject = (props: {item: ProjectResource}) => { if (props.item.frozenByUuid) { return Project was frozen by {fullUsername}}> - + ; } else { return null; @@ -115,7 +115,7 @@ export const ResourceName = connect( return resource; })((resource: GroupContentsResource & DispatchProp) => renderName(resource.dispatch, resource)); - + const renderIcon = (item: GroupContentsResource) => { switch (item.kind) { case ResourceKind.PROJECT: @@ -230,12 +230,12 @@ export const UserResourceFullName = connect( const renderUuid = (item: { uuid: string }) => {item.uuid} - {(item.uuid && ) || '-' } + {(item.uuid && ) || '-'} ; const renderUuidCopyIcon = (item: { uuid: string }) => - {(item.uuid && ) || '-' } + {(item.uuid && ) || '-'} ; export const ResourceUuid = connect((state: RootState, props: { uuid: string }) => ( @@ -617,11 +617,8 @@ export const ResourceLinkTailPermissionLevel = connect( const getResourceLinkCanManage = (state: RootState, link: LinkResource) => { const headResource = getResource(link.headUuid)(state.resources); - // const tailResource = getResource(link.tailUuid)(state.resources); - const userUuid = getUserUuid(state); - if (headResource && headResource.kind === ResourceKind.GROUP) { - return userUuid ? (headResource as GroupResource).writableBy?.includes(userUuid) : false; + return (headResource as GroupResource).canManage; } else { // true for now return true; @@ -687,12 +684,12 @@ const renderUuidLinkWithCopyIcon = (dispatch: Dispatch, item: ProcessResource, c const selectedColumnUuid = item[column] return - {selectedColumnUuid ? - dispatch(navigateTo(selectedColumnUuid))}> - {selectedColumnUuid} - - : '-' } + {selectedColumnUuid} + + : '-'} {selectedColumnUuid && renderUuidCopyIcon({ uuid: selectedColumnUuid })} @@ -716,20 +713,20 @@ export const ResourceParentProcess = connect( (state: RootState, props: { uuid: string }) => { const process = getProcess(props.uuid)(state.resources) return { parentProcess: process?.containerRequest?.requestingContainerUuid || '' }; - })((props: { parentProcess: string }) => renderUuid({uuid: props.parentProcess})); + })((props: { parentProcess: string }) => renderUuid({ uuid: props.parentProcess })); export const ResourceModifiedByUserUuid = connect( (state: RootState, props: { uuid: string }) => { const process = getProcess(props.uuid)(state.resources) return { userUuid: process?.containerRequest?.modifiedByUserUuid || '' }; - })((props: { userUuid: string }) => renderUuid({uuid: props.userUuid})); + })((props: { userUuid: string }) => renderUuid({ uuid: props.userUuid })); - export const ResourceCreatedAtDate = connect( +export const ResourceCreatedAtDate = connect( (state: RootState, props: { uuid: string }) => { const resource = getResource(props.uuid)(state.resources); return { date: resource ? resource.createdAt : '' }; })((props: { date: string }) => renderDate(props.date)); - + export const ResourceLastModifiedDate = connect( (state: RootState, props: { uuid: string }) => { const resource = getResource(props.uuid)(state.resources); @@ -787,39 +784,39 @@ export const ResourceUUID = connect( (state: RootState, props: { uuid: string }) => { const resource = getResource(props.uuid)(state.resources); return { uuid: resource ? resource.uuid : '' }; - })((props: { uuid: string }) => renderUuid({uuid: props.uuid})); + })((props: { uuid: string }) => renderUuid({ uuid: props.uuid })); -const renderVersion = (version: number) =>{ +const renderVersion = (version: number) => { return {version ?? '-'} } export const ResourceVersion = connect( (state: RootState, props: { uuid: string }) => { const resource = getResource(props.uuid)(state.resources); - return { version: resource ? resource.version: '' }; + return { version: resource ? resource.version : '' }; })((props: { version: number }) => renderVersion(props.version)); - -const renderPortableDataHash = (portableDataHash:string | null) => + +const renderPortableDataHash = (portableDataHash: string | null) => {portableDataHash ? <>{portableDataHash} - : '-' } + : '-'} - + export const ResourcePortableDataHash = connect( (state: RootState, props: { uuid: string }) => { const resource = getResource(props.uuid)(state.resources); - return { portableDataHash: resource ? resource.portableDataHash : '' }; + return { portableDataHash: resource ? resource.portableDataHash : '' }; })((props: { portableDataHash: string }) => renderPortableDataHash(props.portableDataHash)); -const renderFileCount = (fileCount: number) =>{ +const renderFileCount = (fileCount: number) => { return {fileCount ?? '-'} } export const ResourceFileCount = connect( (state: RootState, props: { uuid: string }) => { const resource = getResource(props.uuid)(state.resources); - return { fileCount: resource ? resource.fileCount: '' }; + return { fileCount: resource ? resource.fileCount : '' }; })((props: { fileCount: number }) => renderFileCount(props.fileCount)); const userFromID = @@ -968,12 +965,12 @@ export const CollectionStatus = connect((state: RootState, props: { uuid: string export const CollectionName = connect((state: RootState, props: { uuid: string, className?: string }) => { return { - collection: getResource(props.uuid)(state.resources), - uuid: props.uuid, - className: props.className, - }; + collection: getResource(props.uuid)(state.resources), + uuid: props.uuid, + className: props.className, + }; })((props: { collection: CollectionResource, uuid: string, className?: string }) => - {props.collection?.name || props.uuid} + {props.collection?.name || props.uuid} ); export const ProcessStatus = compose( @@ -994,7 +991,7 @@ export const ProcessStatus = compose( }} /> : - - ); + ); export const ProcessStartDate = connect( (state: RootState, props: { uuid: string }) => { diff --git a/src/views-components/side-panel-button/side-panel-button.tsx b/src/views-components/side-panel-button/side-panel-button.tsx index 7874441588..6acbb16118 100644 --- a/src/views-components/side-panel-button/side-panel-button.tsx +++ b/src/views-components/side-panel-button/side-panel-button.tsx @@ -89,8 +89,7 @@ export const SidePanelButton = withStyles(styles)( enabled = true; } else if (matchProjectRoute(location ? location.pathname : '')) { const currentProject = getResource(currentItemId)(resources); - if (currentProject && currentProject.writableBy && - currentProject.writableBy.indexOf(currentUserUUID || '') >= 0 && + if (currentProject && currentProject.canWrite && !currentProject.frozenByUuid && !isProjectTrashed(currentProject, resources) && currentProject.groupClass !== GroupClass.FILTER) { diff --git a/src/views/collection-panel/collection-panel.tsx b/src/views/collection-panel/collection-panel.tsx index 79cf07bfb4..8cf19c03fe 100644 --- a/src/views/collection-panel/collection-panel.tsx +++ b/src/views/collection-panel/collection-panel.tsx @@ -150,8 +150,8 @@ export const CollectionPanel = withStyles(styles)(connect( isWritable = true; } else { const itemOwner = getResource(item.ownerUuid)(state.resources); - if (itemOwner && itemOwner.writableBy) { - isWritable = itemOwner.writableBy.indexOf(currentUserUUID || '') >= 0; + if (itemOwner) { + isWritable = itemOwner.canWrite; } } } @@ -351,7 +351,7 @@ export const CollectionDetailsAttributes = (props: CollectionDetailsProps) => { {/* NOTE: The property list should be kept at the bottom, because it spans the entire available width, without regards of the twoCol prop. - */} + */} diff --git a/src/views/group-details-panel/group-details-panel.tsx b/src/views/group-details-panel/group-details-panel.tsx index 798a7b67e3..fdbc204ee7 100644 --- a/src/views/group-details-panel/group-details-panel.tsx +++ b/src/views/group-details-panel/group-details-panel.tsx @@ -136,8 +136,8 @@ const mapStateToProps = (state: RootState) => { return { resources: state.resources, groupCanManage: userUuid && !isBuiltinGroup(group?.uuid || '') - ? group?.writableBy?.includes(userUuid) - : false, + ? group?.canManage + : false, }; }; @@ -158,7 +158,7 @@ export const GroupDetailsPanel = withStyles(styles)(connect( )( class GroupDetailsPanel extends React.Component> { state = { - value: 0, + value: 0, }; componentDidMount() { @@ -169,56 +169,56 @@ export const GroupDetailsPanel = withStyles(styles)(connect( const { value } = this.state; return ( - - - - -
- {value === 0 && - + + + +
+ {value === 0 && + - } - paperProps={{ - elevation: 0, - }} /> - } - {value === 1 && - - } -
+ } + paperProps={{ + elevation: 0, + }} /> + } + {value === 1 && + + } +
); } diff --git a/src/views/run-process-panel/inputs/project-input.tsx b/src/views/run-process-panel/inputs/project-input.tsx index 688af4aafa..d91a6b8483 100644 --- a/src/views/run-process-panel/inputs/project-input.tsx +++ b/src/views/run-process-panel/inputs/project-input.tsx @@ -99,7 +99,7 @@ export const ProjectInputComponent = connect(mapStateToProps)( } } - invalid = () => (!this.state.project || this.state.project.writableBy.indexOf(this.props.userUuid) === -1); + invalid = () => (!this.state.project || !this.state.project.canWrite); renderInput() { return