feat(workspaces): add rwx perms to blueprints and ranges#147
Closed
Adamkadaban wants to merge 20 commits intoworkspaces-stagingfrom
Closed
feat(workspaces): add rwx perms to blueprints and ranges#147Adamkadaban wants to merge 20 commits intoworkspaces-stagingfrom
Adamkadaban wants to merge 20 commits intoworkspaces-stagingfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces a comprehensive permissions system for managing access to blueprint and deployed ranges in the application, enabling fine-grained control over who can read, write, and execute ranges beyond simple ownership.
- Added CRUD operations for granting and revoking permissions for both blueprint and deployed ranges
- Updated range-related methods to enforce permission checks before allowing access or operations
- Extended data models with permission relationships and created new permission models
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| api/src/app/models/permission_models.py | Created new permission models for blueprint and deployed ranges with proper constraints |
| api/src/app/crud/crud_permissions.py | Implemented CRUD functions for granting and revoking permissions with error handling |
| api/src/app/crud/crud_ranges.py | Updated range operations to include permission checks and grant permissions during creation |
| api/src/app/schemas/range_schemas.py | Added permission fields to create schemas and computed fields to response schemas |
| api/src/app/models/range_models.py | Added permission relationships to range models |
| api/tests/unit/crud/test_crud_permissions.py | Added comprehensive tests for permission CRUD operations |
| api/tests/unit/crud/test_crud_ranges.py | Updated range key access tests to reflect new permission-based logic |
Comments suppressed due to low confidence (1)
api/src/app/crud/crud_ranges.py:163
- The
db.delete()method is being called but the actual SQLAlchemy delete operation requiresdb.delete(permission)to be called on the session. The code should useawait db.delete(permission)instead ofdb.delete(permission).
)
alexchristy
reviewed
Jul 28, 2025
alexchristy
reviewed
Jul 28, 2025
alexchristy
reviewed
Jul 28, 2025
alexchristy
reviewed
Jul 28, 2025
alexchristy
reviewed
Jul 28, 2025
Member
alexchristy
left a comment
There was a problem hiding this comment.
Looking good so far! I love to see the tests. For the permission types, do you think you could clarify the meanings of each type of permission?
From the review this is my understanding:
- Readers can read the corresponding database record
- Blueprint writers can deploy and edit the blueprint?
- Deployed range writers can edit the resources of a live range?
- Deployed range executors can interact with a live range
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
Description
This pull request introduces a permissions system for managing access to blueprint and deployed ranges in the application. Key changes include adding CRUD operations for permissions, updating range-related methods to enforce permissions, and extending the data models to support these changes.
Permissions Management:
Added CRUD functions for granting and revoking permissions (
grant_blueprint_permission,grant_deployed_permission,revoke_blueprint_permission,revoke_deployed_permission) incrud_permissions.py. These functions handle database operations and include logging for success and failure cases.Introduced helper functions (
can_read_blueprint,can_write_blueprint,can_read_deployed,can_write_deployed,can_execute_deployed) incrud_ranges.pyto check user permissions for blueprint and deployed ranges. These functions ensure that access control is enforced consistently.Updates to Range Operations:
Updated methods like
get_blueprint_range_headers,get_blueprint_range,create_blueprint_range, anddelete_blueprint_rangeincrud_ranges.pyto incorporate permission checks and manage permissions during creation and deletion. Similar changes were made for deployed ranges. [1] [2] [3] [4] [5] [6] [7] [8]Modified range creation methods to grant permissions to specified readers, writers, and executors during the creation of blueprint and deployed ranges. [1] [2]
Data Model Enhancements:
Added
BlueprintRangePermissionModelandDeployedRangePermissionModelto represent permissions in the database. These models were integrated into the existing data model structure. [1] [2]Updated range models to include a
permissionsrelationship for loading associated permissions, enabling efficient permission checks. [1] [2]Query Adjustments:
These changes collectively improve the application's access control mechanisms, ensuring that permissions are consistently enforced across all operations involving blueprint and deployed ranges.
Fixes: #66