langgenius/dify
1
0
Fork 0
mirror of https://github.com/langgenius/dify.git synced 2026-07-21 12:05:30 -04:00
Code Issues 775 Packages Projects Releases 10 Wiki Activity

[PR #28760] test: add unit tests for dataset permission service #32168

New Issue
Closed
opened 2026-02-21 20:50:53 -05:00 by yindo · 0 comments
yindo commented 2026-02-21 20:50:53 -05:00
Owner
Copy Link
Copy Source

Original Pull Request: https://github.com/langgenius/dify/pull/28760

State: closed
Merged: Yes

Summary

This PR adds comprehensive unit tests for DatasetPermissionService and DatasetService permission methods, covering critical dataset access control and security functionality.

Fix: #28757

Test Coverage

1. Dataset Permission Service — Partial Member List Operations

get_dataset_partial_member_list

  • ✅ Retrieving lists with multiple members
  • ✅ Retrieving lists with single member
  • ✅ Retrieving empty lists (no partial members)
  • ✅ Database query validation

update_partial_member_list

  • ✅ Adding new partial members to dataset
  • ✅ Replacing existing partial members
  • ✅ Handling empty member lists (clearing all members)
  • ✅ Database transaction handling
  • ✅ Error handling and rollback on database errors

clear_partial_member_list

  • ✅ Clearing lists with existing members
  • ✅ Clearing empty lists (no-op)
  • ✅ Database transaction handling
  • ✅ Error handling and rollback on database errors

2. Dataset Permission Service — Permission Validation

check_permission

  • ✅ Dataset editor permission validation
  • ✅ Dataset operator restrictions (cannot change permission levels)
  • ✅ Dataset operator restrictions (cannot modify partial member lists)
  • ✅ Partial member list validation for operators
  • ✅ Missing dataset editor permissions error handling
  • ✅ Invalid permission change error handling

3. Dataset Service — Permission Checking

check_dataset_permission

  • ✅ Tenant boundary enforcement (cross-tenant access denied)
  • ✅ OWNER role bypass (access granted automatically)
  • ✅ only_me permission validation (creator only)
  • ✅ partial_members permission validation (explicit permission required)
  • ✅ all_team_members permission validation (all tenant members)
  • ✅ Creator privilege validation (creators can access their own datasets)
  • ✅ Permission denial scenarios with appropriate error messages

check_dataset_operator_permission

  • ✅ Dataset not found error handling
  • ✅ User not found error handling
  • ✅ OWNER role bypass
  • ✅ only_me permission validation
  • ✅ partial_members permission validation
  • ✅ Permission denial scenarios

Permission Enum Handling

The test suite comprehensively covers all three permission levels:

  • only_me: Only the dataset creator can access
  • all_team_members: All members of the tenant can access
  • partial_members: Only specific users with explicit DatasetPermission records can access

Test Structure

  • Test Data Factory: Centralized mock object creation for consistency
  • Comprehensive Documentation: Extensive comments explaining test scenarios, architecture, and security boundaries
  • Error Path Coverage: All error conditions and edge cases are tested
  • Mock Strategy: Proper isolation using unittest.mock for dependencies
  • Security Testing: Tenant isolation, role-based access control, and permission boundary enforcement

Files Changed

  • api/tests/unit_tests/services/dataset_permission_service.py (1,476 lines)

Testing Approach

  • Follows existing test patterns from other DatasetService test files
  • Uses Arrange-Act-Assert structure
  • Comprehensive mocking of dependencies (database, user context, permission models)
  • Tests both success and error paths
  • Validates security boundaries and access control rules
  • Ensures proper transaction handling and rollback on errors

Security Considerations

These tests ensure that:

  • Tenant boundaries are strictly enforced
  • Permission levels are correctly validated
  • Dataset operators have appropriate restrictions
  • Explicit permissions are required for partial_members access
  • Error messages don't leak sensitive information
  • Database transactions are properly handled

Related Issues

This test suite addresses the need for comprehensive coverage of dataset permission management functionality, ensuring safe refactoring and preventing regressions in critical access control logic.

Contribution by Gittensor, learn more at https://gittensor.io/

**Original Pull Request:** https://github.com/langgenius/dify/pull/28760 **State:** closed **Merged:** Yes --- ## Summary This PR adds comprehensive unit tests for DatasetPermissionService and DatasetService permission methods, covering critical dataset access control and security functionality. Fix: #28757 ## Test Coverage ### 1. Dataset Permission Service — Partial Member List Operations #### `get_dataset_partial_member_list` - ✅ Retrieving lists with multiple members - ✅ Retrieving lists with single member - ✅ Retrieving empty lists (no partial members) - ✅ Database query validation #### `update_partial_member_list` - ✅ Adding new partial members to dataset - ✅ Replacing existing partial members - ✅ Handling empty member lists (clearing all members) - ✅ Database transaction handling - ✅ Error handling and rollback on database errors #### `clear_partial_member_list` - ✅ Clearing lists with existing members - ✅ Clearing empty lists (no-op) - ✅ Database transaction handling - ✅ Error handling and rollback on database errors ### 2. Dataset Permission Service — Permission Validation #### `check_permission` - ✅ Dataset editor permission validation - ✅ Dataset operator restrictions (cannot change permission levels) - ✅ Dataset operator restrictions (cannot modify partial member lists) - ✅ Partial member list validation for operators - ✅ Missing dataset editor permissions error handling - ✅ Invalid permission change error handling ### 3. Dataset Service — Permission Checking #### `check_dataset_permission` - ✅ Tenant boundary enforcement (cross-tenant access denied) - ✅ OWNER role bypass (access granted automatically) - ✅ `only_me` permission validation (creator only) - ✅ `partial_members` permission validation (explicit permission required) - ✅ `all_team_members` permission validation (all tenant members) - ✅ Creator privilege validation (creators can access their own datasets) - ✅ Permission denial scenarios with appropriate error messages #### `check_dataset_operator_permission` - ✅ Dataset not found error handling - ✅ User not found error handling - ✅ OWNER role bypass - ✅ `only_me` permission validation - ✅ `partial_members` permission validation - ✅ Permission denial scenarios ## Permission Enum Handling The test suite comprehensively covers all three permission levels: - **`only_me`**: Only the dataset creator can access - **`all_team_members`**: All members of the tenant can access - **`partial_members`**: Only specific users with explicit DatasetPermission records can access ## Test Structure - **Test Data Factory**: Centralized mock object creation for consistency - **Comprehensive Documentation**: Extensive comments explaining test scenarios, architecture, and security boundaries - **Error Path Coverage**: All error conditions and edge cases are tested - **Mock Strategy**: Proper isolation using `unittest.mock` for dependencies - **Security Testing**: Tenant isolation, role-based access control, and permission boundary enforcement ## Files Changed - `api/tests/unit_tests/services/dataset_permission_service.py` (1,476 lines) ## Testing Approach - Follows existing test patterns from other DatasetService test files - Uses Arrange-Act-Assert structure - Comprehensive mocking of dependencies (database, user context, permission models) - Tests both success and error paths - Validates security boundaries and access control rules - Ensures proper transaction handling and rollback on errors ## Security Considerations These tests ensure that: - Tenant boundaries are strictly enforced - Permission levels are correctly validated - Dataset operators have appropriate restrictions - Explicit permissions are required for partial_members access - Error messages don't leak sensitive information - Database transactions are properly handled ## Related Issues This test suite addresses the need for comprehensive coverage of dataset permission management functionality, ensuring safe refactoring and preventing regressions in critical access control logic. Contribution by Gittensor, learn more at https://gittensor.io/
yindo added the pull-request label 2026-02-21 20:50:53 -05:00
yindo closed this issue 2026-02-21 20:50:53 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Knowledge Pipeline
Proposal
WIP
cloud
dependencies
feat:webapp
good first issue
outdated
performance
pull-request
Mirrored from GitHub Pull Request
 python:uv
refactor
size:L
stale
status: accepting prs
status: blocked
status: in discussion
stt & tts
vulnerablity
web
wont-fix
🌊 feat:workflow
🌚 invalid
🐞 bug
👻 feat:rag
💪 enhancement
📚 documentation
🔨 feat:tools
🙋‍♂️ question
🙏 help wanted
🤔 cant-reproduce
🤖 feat:agent
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
yindo
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: langgenius/dify#32168
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?