Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ui/src/permission/application/system-manage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const systemManage = {
'OR'
),
folderDelete: () => false,
auth: () => false,
overview_embed: () =>
hasPermission(
[
Expand Down
10 changes: 10 additions & 0 deletions ui/src/permission/application/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@ const workspace = {
],
'OR'
),
auth: (source_id:string) =>
hasPermission(
[
new ComplexPermission([RoleConst.USER],[PermissionConst.APPLICATION.getApplicationWorkspaceResourcePermission(source_id)],[],'AND'),
RoleConst.WORKSPACE_MANAGE.getWorkspaceRole,
PermissionConst.APPLICATION_RESOURCE_AUTHORIZATION.getWorkspacePermissionWorkspaceManageRole,
PermissionConst.APPLICATION_RESOURCE_AUTHORIZATION.getApplicationWorkspaceResourcePermission(source_id)
],
'OR'
),
folderEdit: () =>
hasPermission(
[
Copy link
Contributor Author

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 appears to be part of an API definition that defines several permission checks within a workspace object. Some key points to consider:

  1. Code Structure: The structure is clear with multiple function definitions and permissions, but it looks like there might not be any actual logic implemented for these functions yet.

  2. Function Names:

    • auth: This suggests this function should return a boolean value indicating if the user/auth source has the necessary permissions.
    • folderEdit: Similarily, this function should also return a boolean value indicating permission for editing folders.
  3. Permissions:

    • Each check involves different arrays and conditions. It's unclear what exactly each array represents without further context on the hasPermission, ComplexPermission, RoleConst, and PermissionConst.
  4. Documentation and Comments: There seems to be some intended documentation or comments around these methods, but they are incomplete (// at the end of lines).

  5. Suggested Improvements:

    • Comments and Documentation: Add more detailed explanations and comments about the purpose and functionality of each method and associated constants.
    • Code Clarity: Make sure each function name corresponds clearly to its intended use and actions (e.g., checkUserAuth, canEditFolder).
    • Type Annotations: Consider adding type annotations if using JavaScript/TypeScript to help clarify parameter types and expected return values.

Here’s a revised version with added comments and improved clarity:

const workspace = {
    /**
     * Checks if the user/source has the specified permissions for authentication.
     * @param {string} sourceId - ID of the user source.
     * @returns {boolean} True if all requirements are met, false otherwise.
     */
    auth: (sourceId: string): boolean => {
        return hasPermission([
            new ComplexPermission([RoleConst.USER], [PermissionConst.APPLICATION.getApplicationWorkspaceResourcePermission(sourceId)], [], 'AND'),
            RoleConst.WORKSPACE_MANAGE.getWorkspaceRole,
            PermissionConst.APPLICATION_RESOURCE_AUTHORIZATION.getWorkspacePermissionWorkspaceManageRole,
            PermissionConst.APPLICATION_RESOURCE_AUTHORIZATION.getApplicationWorkspaceResourcePermission(sourceId)
        ], 'OR');
    },

    /**
     * Check if the user/source has the necessary permissions to edit folders.
     * @returns {boolean} True if allowed, false otherwise.
     */
    folderEdit: (): boolean => {
        return hasPermission([
            // Add permission checks here if needed
            // For example:
            // PermissionConst.FOLDER_EDIT.getFolderPermission,
            // ...
        ], 'OR');
    }
};

/**
 * Utility function to determine if user/auth meets given permissions.
 * @param {Array<IPermissionCheck>} checks - Array of permission checks to evaluate.
 * @returns {boolean} True if all checks PASS, else false.
 */
function hasPermission(checks: IPermissionCheck[], logicalOperator: string = 'AND'): boolean {
    // Implementation details would go here
}

interface IComplexPermission extends IPermissionCheck {
    roles: Role[];
    resources: Resource[];
    operators?: Operator;
    logicalOperands?: LogicalOperand[];
}

enum PermissionConst {
    APPLICATION = 'application',
    WORKSPACE_MANAGE = 'workspaceManage',
    APPLICATION_RESOURCE_AUTHORIZATION = 'applicationResourceAuthorization',
    FOLDER_EDIT = 'folderEdit',
    ...

}

This revision includes comments, docstrings, and placeholders for the implementation of the hasPermission function, which could include more complex logic based on your application's permission system.

Expand Down
1 change: 1 addition & 0 deletions ui/src/permission/knowledge/system-manage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ const systemManage = {
],'OR'
),

auth: () => false,
folderCreate: () => false,
folderEdit: () => false,
folderDelete: () => false,
Expand Down
1 change: 1 addition & 0 deletions ui/src/permission/knowledge/system-share.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ const share = {
],
'OR'
),
auth: () => false,
folderCreate: () => false,
folderEdit: () => false,
folderDelete: () => false,
Expand Down
1 change: 1 addition & 0 deletions ui/src/permission/knowledge/workspace-share.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const workspaceShare = {
edit: () => false,
export: () => false,
delete: () => false,
auth: () => false,

doc_read: () => false,
doc_create: () => false,
Expand Down
10 changes: 10 additions & 0 deletions ui/src/permission/knowledge/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,16 @@ const workspace = {
],
'OR',
),
auth: (source_id:string) =>
hasPermission(
[
new ComplexPermission([RoleConst.USER],[PermissionConst.KNOWLEDGE.getKnowledgeWorkspaceResourcePermission(source_id)],[],'AND'),
RoleConst.WORKSPACE_MANAGE.getWorkspaceRole,
PermissionConst.KNOWLEDGE_RESOURCE_AUTHORIZATION.getKnowledgeWorkspaceResourcePermission(source_id),
PermissionConst.KNOWLEDGE_RESOURCE_AUTHORIZATION.getWorkspacePermissionWorkspaceManageRole,
],
'OR',
),
folderEdit: () =>
hasPermission(
[
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided JavaScript code defines an object workspace with several methods that handle permission checks using a hypothetical function hasPermission. The methods include "search", "details", "delete", and "view". Each method takes parameters related to permissions.

Here's a summary of the issues, potential improvements, and some suggestions:

Issues:

  1. Missing Parameter Handling: Some methods do not explicitly handle missing parameters, which could lead to errors if called without proper arguments.

  2. Redundant Permissions: There are multiple uses of similar permission combinations across different functions, which might indicate redundancy that can be streamlined.

  3. Complexity in Permission Logic: The logic for defining complex conditions is complex, making it hard to read and understand at a glance.

  4. Comments: While comments explain how specific permissions work, they don't clarify where other parts of the logic should go or why certain decisions were made about individual permissions.

  5. Consistency: The overall structure of the methods does not fully follow best practices for readability and maintainability.

Potential Improvements/Cleanups:

  1. Parameter Validation: Ensure all parameter validation before calling hasPermission to prevent runtime errors related to unexpected input types. For example:

    if (!source_id) throw new Error("source_id is required");
  2. Inline Comments or Inline Documentation Blocks Instead of placing comments after each line, consider adding inline doc blocks above each method or function call explaining what it does. This makes it easier to understand the role of each part without additional scrolling through many lines of comments.

  3. Simplify Complex Conditions: Use named constants or variables instead of string literals for roles and permissions names. This improves readability and reduces errors due to mispellings.

  4. Refactor Redundant Code: Extract common permission logic into shared utility functions or reuse existing permission definitions when applicable.

  5. Documentation: Consider creating API documentation outside the source code for developers who will interact with these functions. A comprehensive guide on how to use them would also help users.

Example Refactored Snippet:

const { RoleConst, PermissionConst } = require('./constants');

const workspace = {
  search: () => hasPermission([
    // ... (existing permission array)
  ]),

  delete: () => hasPermission([
    // ... (existing permission array)
  ]),

  view: () => hasPermission([
    // ... (existing permission array)
  ]),
};

// Utility Function for Creating Complex Permissions
function makeComplexPermission(roleOrRoles, permissions, exclusions = [], conjunction = 'AND') {
  return { roleOrRoles, permissions, exclusions, conjunction };
}

// Example Usage in 'auth'
workspace.auth = (sourceId) => {
  if (!sourceId) throw new Error("source_id is required");

  const permissions = [makeComplexPermission(RoleConst.USER, [
    PermissionConst.KNOWLEDGE.getKnowledgeWorkspaceResourcePermission(sourceId),
  ], [])];

  permissions.push(RoleConst.WORKSPACE_MANAGE.getWorkspaceRole);
  permissions.push(PermissionConst.KNOWLEDGE_RESOURCE_AUTHORIZATION.getKnowledgeWorkspaceResourcePermission(sourceId));
  permissions.push(PermissionConst.KNOWLEDGE_RESOURCE_AUTHORIZATION.getWorkspacePermissionWorkspaceManageRole);

  return hasPermission(permissions, 'OR');
};

In this refactored version:

  • A utility function makeComplexPermission is used to encapsulate common permission structures.
  • Specific error handling is implemented directly in the method calls.
  • Overall clarity improves by organizing the permission logic more logically within each method.

Expand Down
1 change: 1 addition & 0 deletions ui/src/permission/model/system-manage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const systemManage = {
delete: () =>
hasPermission([RoleConst.ADMIN, PermissionConst.RESOURCE_MODEL_DELETE], 'OR'),

auth: () => false,
folderCreate: () => false,
folderEdit: () => false,
folderDelete: () => false,
Expand Down
1 change: 1 addition & 0 deletions ui/src/permission/model/system-share.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const share = {
],
'OR',
),
auth: () => false,
folderCreate: () => false,
folderEdit: () => false,
folderDelete: () => false,
Expand Down
10 changes: 10 additions & 0 deletions ui/src/permission/model/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,16 @@ const workspace = {
],
'OR'
),
auth: (source_id:string) =>
hasPermission(
[
new ComplexPermission([RoleConst.USER],[PermissionConst.MODEL.getModelWorkspaceResourcePermission(source_id)],[],'AND'),
RoleConst.WORKSPACE_MANAGE.getWorkspaceRole,
PermissionConst.MODEL_RESOURCE_AUTHORIZATION.getModelWorkspaceResourcePermission(source_id),
PermissionConst.MODEL_RESOURCE_AUTHORIZATION.getWorkspacePermissionWorkspaceManageRole
],
'OR'
),
folderEdit: () =>
hasPermission(
[
Expand Down
1 change: 1 addition & 0 deletions ui/src/permission/tool/system-manage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const systemManage = {
'OR',
),

auth: () => false,
folderCreate: () => false,
folderEdit: () => false,
folderDelete: () => false,
Expand Down
1 change: 1 addition & 0 deletions ui/src/permission/tool/system-share.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ const share = {
'OR',
),

auth: () => false,
folderCreate: () => false,
folderEdit: () => false,
folderDelete: () => false,
Expand Down
10 changes: 10 additions & 0 deletions ui/src/permission/tool/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,16 @@ const workspace = {
],
'OR'
),
auth: (source_id:string) =>
hasPermission(
[
new ComplexPermission([RoleConst.USER],[PermissionConst.TOOL.getToolWorkspaceResourcePermission(source_id)],[],'AND'),
RoleConst.WORKSPACE_MANAGE.getWorkspaceRole,
PermissionConst.TOOL_RESOURCE_AUTHORIZATION.getToolWorkspaceResourcePermission(source_id),
PermissionConst.TOOL_RESOURCE_AUTHORIZATION.getWorkspacePermissionWorkspaceManageRole
],
'OR'
),
debug: () =>
hasPermission(
[
Expand Down
13 changes: 13 additions & 0 deletions ui/src/utils/permission/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,19 @@ const PermissionConst = {
CHANGE_PASSWORD: new Permission('OTHER:READ+CREATE'),
SYSTEM_API_KEY_EDIT: new Permission('OTHER:READ+DELETE'),

APPLICATION_RESOURCE_AUTHORIZATION: new Permission(
'APPLICATION:READ+AUTH',
),
KNOWLEDGE_RESOURCE_AUTHORIZATION: new Permission(
'KNOWLEDGE:READ+AUTH',
),
TOOL_RESOURCE_AUTHORIZATION: new Permission(
'TOOL:READ+AUTH',
),
MODEL_RESOURCE_AUTHORIZATION: new Permission(
'MODEL:READ+AUTH',
),

APPLICATION_WORKSPACE_USER_RESOURCE_PERMISSION_READ: new Permission(
'APPLICATION_WORKSPACE_USER_RESOURCE_PERMISSION:READ',
),
Expand Down
Loading
Loading