-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: Folder Permission #4222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Folder Permission #4222
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,17 +20,56 @@ const workspace = { | |
| ], | ||
| 'OR', | ||
| ), | ||
| folderRead: () => true, | ||
| folderRead: (folder_id: string) => | ||
| hasPermission( | ||
| [ | ||
| new ComplexPermission([RoleConst.USER],[PermissionConst.KNOWLEDGE.getKnowledgeWorkspaceResourcePermission(folder_id)],[],'AND'), | ||
| RoleConst.WORKSPACE_MANAGE.getWorkspaceRole, | ||
| PermissionConst.KNOWLEDGE_FOLDER_READ.getKnowledgeWorkspaceResourcePermission(folder_id), | ||
| PermissionConst.KNOWLEDGE_READ.getWorkspacePermissionWorkspaceManageRole, | ||
| ], | ||
| 'OR' | ||
| ), | ||
| folderManage: () => true, | ||
| folderCreate: () => | ||
| hasPermission( | ||
| [ | ||
| RoleConst.USER.getWorkspaceRole, | ||
| RoleConst.WORKSPACE_MANAGE.getWorkspaceRole, | ||
| PermissionConst.KNOWLEDGE_CREATE.getWorkspacePermission, | ||
| PermissionConst.KNOWLEDGE_CREATE.getWorkspacePermissionWorkspaceManageRole, | ||
| ], | ||
| 'OR', | ||
| folderAuth: (folder_id: string) => | ||
| hasPermission( | ||
| [ | ||
| new ComplexPermission([RoleConst.USER],[PermissionConst.KNOWLEDGE.getKnowledgeWorkspaceResourcePermission(folder_id)],[],'AND'), | ||
| RoleConst.WORKSPACE_MANAGE.getWorkspaceRole, | ||
| PermissionConst.KNOWLEDGE_FOLDER_EDIT.getKnowledgeWorkspaceResourcePermission(folder_id), | ||
| PermissionConst.KNOWLEDGE_RESOURCE_AUTHORIZATION.getWorkspacePermissionWorkspaceManageRole, | ||
| ], | ||
| 'OR' | ||
| ), | ||
| folderCreate: (folder_id: string) => | ||
| hasPermission( | ||
| [ | ||
| new ComplexPermission([RoleConst.USER],[PermissionConst.KNOWLEDGE.getKnowledgeWorkspaceResourcePermission(folder_id)],[],'AND'), | ||
| RoleConst.WORKSPACE_MANAGE.getWorkspaceRole, | ||
| PermissionConst.KNOWLEDGE_FOLDER_EDIT.getKnowledgeWorkspaceResourcePermission(folder_id), | ||
| PermissionConst.KNOWLEDGE_CREATE.getWorkspacePermissionWorkspaceManageRole, | ||
| ], | ||
| 'OR' | ||
| ), | ||
| folderDelete: (folder_id: string) => | ||
| hasPermission( | ||
| [ | ||
| new ComplexPermission([RoleConst.USER],[PermissionConst.KNOWLEDGE.getKnowledgeWorkspaceResourcePermission(folder_id)],[],'AND'), | ||
| RoleConst.WORKSPACE_MANAGE.getWorkspaceRole, | ||
| PermissionConst.KNOWLEDGE_FOLDER_EDIT.getKnowledgeWorkspaceResourcePermission(folder_id), | ||
| PermissionConst.KNOWLEDGE_DELETE.getWorkspacePermissionWorkspaceManageRole, | ||
| ], | ||
| 'OR' | ||
| ), | ||
| folderEdit: (folder_id: string) => | ||
| hasPermission( | ||
| [ | ||
| new ComplexPermission([RoleConst.USER],[PermissionConst.KNOWLEDGE.getKnowledgeWorkspaceResourcePermission(folder_id)],[],'AND'), | ||
| RoleConst.WORKSPACE_MANAGE.getWorkspaceRole, | ||
| PermissionConst.KNOWLEDGE_FOLDER_EDIT.getKnowledgeWorkspaceResourcePermission(folder_id), | ||
| PermissionConst.KNOWLEDGE_EDIT.getWorkspacePermissionWorkspaceManageRole, | ||
| ], | ||
| 'OR' | ||
| ), | ||
| sync: (source_id:string) => | ||
| hasPermission( | ||
|
|
@@ -82,16 +121,6 @@ const workspace = { | |
| ], | ||
| 'OR', | ||
| ), | ||
| folderEdit: () => | ||
| hasPermission( | ||
| [ | ||
| RoleConst.USER.getWorkspaceRole, | ||
| RoleConst.WORKSPACE_MANAGE.getWorkspaceRole, | ||
| PermissionConst.KNOWLEDGE_EDIT.getWorkspacePermission, | ||
| PermissionConst.KNOWLEDGE_EDIT.getWorkspacePermissionWorkspaceManageRole, | ||
| ], | ||
| 'OR', | ||
| ), | ||
| export: (source_id:string) => | ||
| hasPermission( | ||
| [ | ||
|
|
@@ -112,16 +141,6 @@ const workspace = { | |
| ], | ||
| 'OR', | ||
| ), | ||
| folderDelete: () => | ||
| hasPermission( | ||
| [ | ||
| RoleConst.USER.getWorkspaceRole, | ||
| RoleConst.WORKSPACE_MANAGE.getWorkspaceRole, | ||
| PermissionConst.KNOWLEDGE_DELETE.getWorkspacePermission, | ||
| PermissionConst.KNOWLEDGE_DELETE.getWorkspacePermissionWorkspaceManageRole, | ||
| ], | ||
| 'OR', | ||
| ), | ||
| doc_read: () => false, | ||
| doc_create: (source_id:string) => | ||
| hasPermission( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The provided code appears to be implementing permission checks using a complex
Here's an optimized version of your code: const workspace = {
knowledgeRead: (workspaceId: string) => {
return hasPermission([
hasPermission(RoleConst.USER, WorkspaceRoles.workspaceManager),
hasPermission(
PermissionConst.KNOWLEDGE_WORKSPACE_RESOURCE_PERMISSION,
{ resourceIdType: ResourceTypeEnum.WORKSPACE, resourceId: workspaceId }
)
]).OR;
},
// Consolidated folders management methods:
folderPermissions(basePermissions: IHasBasePermissions): IFolderPermissions {
[RoleConst.USER.getWorkspaceRole(), RoleConst.WORKSPACE_MANAGE.getWorkspaceRole()].
forEach(role => basePermissions.addRole(role));
['read', 'create', 'edit', 'delete'].forEach(action => {
const actionPermissionKey = PermissionConst[`${action.toUpperCase()}${capitalize('knowledge')}_${ResourceTypeEnum.FOLDER}_${action}`];
if (!Object.keys(PermissionConst).includes(actionPermissionKey))
throw new Error(`Missing ${action} permission key in PermissionConst`);
basePermissions.addPermission({
role: '',
permissionKey: actionPermissionKey,
target: ResourceTypeEnum.FOLDER,
action
});
});
return basePermissions;
},
folderAccess: (folderId: string) => this.folderPermissions(new HasBasePermissions())
.checkPermission(ResourceTypeEnum.FOLDER),
};Key Changes:
These changes improve maintainability, reduce redundancy, and add better structure to the permission logic. |
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The provided TypeScript code snippet is a configuration file that defines permissions for various actions related to folders within a system. The
hasPermissionfunction checks if an actor has the necessary roles or permissions specified in the role-based access control (RBAC) setup.Overall, there are no significant irregularities in the structure of the code but some optimizations can be made:
Code Duplication: Functions like
folderCreate,folderEdit,folderAuth,folderDelete, andoverview_embedhave almost identical logic except for different parameters and permission IDs. Consider creating a helper function to handle these repeated sections.Documentation: Adding comments explaining each part of the RBAC rules could improve maintainability and understanding.
Here's an optimized version with minimal changes and additions:
Key Changes and Additions:
checkFolderxxxPermission) which are more descriptive and easier to understand when using the class methods.