Skip to content

fix: Common user folder permission #4288

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 29, 2025
Merged

fix: Common user folder permission #4288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions ...plication/flow/step_node/variable_aggregation_node/impl/base_variable_aggregation_node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ def get_first_non_null(self, variable_list):
v = self.workflow_manage.get_reference_field(
variable.get('variable')[0],
variable.get('variable')[1:])
if v is not None and not(isinstance(v, (str,list,dict)) and len(v) == 0) :
if v is not None and not (isinstance(v, (str, list, dict)) and len(v) == 0):
return v
return None

def set_variable_to_json(self, variable_list):

return {variable.get('variable')[1:][0]: self.workflow_manage.get_reference_field(
return [self.workflow_manage.get_reference_field(
variable.get('variable')[0],
variable.get('variable')[1:]) for variable in variable_list}
variable.get('variable')[1:]) for variable in variable_list]

def reset_variable(self, variable):
value = self.workflow_manage.get_reference_field(
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 29, 2025

Choose a reason for hiding this comment

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

The code appears to be functioning correctly, but I can suggest a few improvements:

  1. Readability: It would be helpful to add more meaningful names to variables and method parameters for better understanding.
    • Example: 
      def get_first_non_empty_value(self, variable_list):
    # ... existing code ...

2. **Type Hints**: Adding type hints to `set_variable_to_json`'s return type could make it clearer what kind of data is expected to be returned.
   - Proposed change:
     ```python
     def set_variable_to_json(self, variable_list) -> List[Union[str, list, dict]]:
         # ... existing code ...

  1. Functionality Duplication: The logic within get_first_non_null and set_variable_to_json might become redundant if you combine them into one function based on some conditions.

  2. Error Handling: Consider adding error handling to manage cases where the input values or references cannot be retrieved from workflow_manage.

Here's an improved version incorporating these comments and suggestions:

from typing import Union, List

class ReferenceManager:
    @staticmethod
    def get_reference_field(reference_id: str, path: List[str]) -> Union[str, list, dict]:
        # Placeholder implementation
        pass

class WorkflowManager:
    class Variable:
        def __init__(self, name: str, reference: tuple, optional_parts: list):
            self.name = name
            self.reference = reference
            self.optional_parts = optional_parts

    def __init__(self):
        self.variables = {}

    def register_variable(self, name: str, reference: tuple, optional_parts: list = []):
        self.variables[name] = self.Variable(name, reference, optional_parts)

def process_variables(workflow_manager: WorkflowManager) -> None:
    def get_first_non_empty_value(variable_list: List['WorkflowManager.Variable']) -> Union[None, str, list, dict]:
        for variable in variable_list:
            value = workflow_manager.register_variable.get_reference_field(
                variable.reference,
                variable.optional_parts)
            if value is not None and not (
                    isinstance(value, (str, list, dict)) and len(value) == 0):
                return value
        return None

    def map_variablesToJson(variable_list: List['WorkflowManager.Variable']) -> Dict[str, Union[str, list, dict]]:
        return {
            variable.name: workflow_manager.register_variable.get_reference_field(
                variable.reference,
                variable.optional_parts)
            for variable in variable_list}

    def reset_variables(variable: 'WorkflowManager.Variable') -> None:
        value = workflow_manager.register_variable.get_reference_field(
            variable.reference,
            variable.optional_parts)
        if value is not None:
            # Implement resetting logic here using the obtained value
            print(f"Resetting variable {variable.name} with value {value}")

# Example usage
manager = WorkflowManager()
process_variables(manager)
reset_variables(manager.register_variable("example", ("ref1", "part1")))

This revised code includes the suggested improvements and organizes the functionality into methods that perform specific tasks related to processing and managing workflows and their variables.

Expand Down
6 changes: 3 additions & 3 deletions apps/folders/views/folder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class FolderView(APIView):
),
lambda r, kwargs: ViewPermission([RoleConstants.USER.get_workspace_role()],
[Permission(group=Group(f"{kwargs.get('source')}_FOLDER"),
operate=Operate.SELF,
operate=Operate.EDIT,
resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/{kwargs.get('source')}/{r.data.get('parent_id')}"
)], CompareConstants.AND),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role()
Expand Down Expand Up @@ -107,7 +107,7 @@ class Operate(APIView):
),
lambda r, kwargs: ViewPermission([RoleConstants.USER.get_workspace_role()],
[Permission(group=Group(f"{kwargs.get('source')}_FOLDER"),
operate=Operate.SELF,
operate=Operate.EDIT,
resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/{kwargs.get('source')}/{kwargs.get('folder_id')}"
)], CompareConstants.AND),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role()
Expand Down Expand Up @@ -159,7 +159,7 @@ def get(self, request: Request, workspace_id: str, source: str, folder_id: str):
),
lambda r, kwargs: ViewPermission([RoleConstants.USER.get_workspace_role()],
[Permission(group=Group(f"{kwargs.get('source')}_FOLDER"),
operate=Operate.SELF,
operate=Operate.EDIT,
resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/{kwargs.get('source')}/{kwargs.get('folder_id')}"
)], CompareConstants.AND),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role()
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 29, 2025

Choose a reason for hiding this comment

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

The provided code appears correct and does not contain significant issues or optimizations at this time. It sets up permissions that allow users to view their own folders within a workspace, but it allows others (Workspace Managers) to edit any folder regardless of who owns it.

Suggestions:

  1. Permissions Clarification: Consider adding more specific role-based checks if you want to distinguish between the owner and other editors.

    # Example: Only owners can edit their own folders
lambda r, kwargs: ViewPermission(
    [RoleConstants.USER.get_workspace_role(kwargs['source'], 'owner'),
     RoleConstants.WORKSPACE_MANAGE.get_workspace_role()],
    [Permission(...)], CompareConstants.OR
)

  2. Parameterization: If these functions could be reused for different sources (e.g., files), consider parameterizing them with the source type for better reusability and readability.

  3. Testing: Ensure thorough testing is performed with various scenarios to validate the permission handling logic correctly.

  4. Documentation: Add comments or documentation to explain the purpose of each function and how permissions are applied.

By implementing some of the proposed changes, you can further enhance the security and flexibility of your application's access control mechanisms.

Expand Down
Loading