Open
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
thabofletcher
added a commit
that referenced
this pull request
Feb 3, 2026
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
Greptile OverviewGreptile SummaryThis PR introduces the initial database schema and ORM models for a dynamic RBAC system, including Alembic migrations to create RBAC tables and seed permissions/roles, plus unit tests covering basic model behavior (roles, permissions, assignments, and constraints). The RBAC models are wired into the existing SQLAlchemy model registry via Key issues to address before merging:
After those are fixed, consider aligning server_default usage (SQL expressions vs strings) and making time-dependent tests deterministic. Confidence Score: 2/5
Important Files Changed
|
src/fides/api/alembic/migrations/versions/xx_2026_01_31_1000_d9ee4ea46797_add_rbac_tables.py
Show resolved
Hide resolved
18 tasks
Add industry-standard RBAC (Role-Based Access Control) system with: Database tables: - rbac_role: Dynamic role definitions with hierarchy support - rbac_permission: Permission definitions (seeded from SCOPE_REGISTRY) - rbac_role_permission: Role-permission mapping (many-to-many) - rbac_user_role: User role assignments with resource scoping + temporal validity - rbac_role_constraint: Separation of duties constraints SQLAlchemy models: - RBACRole: Supports role hierarchy via parent_role_id - RBACPermission: Permission definitions - RBACRolePermission: Junction table - RBACUserRole: Supports resource-scoped and time-bounded assignments - RBACRoleConstraint: Static/dynamic SoD and cardinality constraints Migrations: - Add RBAC tables with indexes and constraints - Seed default roles and permissions from existing role definitions Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ations Changed revision IDs to unique values: - add_rbac_tables: a1b2c3d4e5f6 -> d9ee4ea46797 - seed_rbac_defaults: b2c3d4e5f6a7 -> f5f526cbc35a The original IDs conflicted with existing migrations: - xx_2025_11_10_1200_a1b2c3d4e5f6_add_test_datastore_to_connectiontype.py - xx_2025_11_12_1430_b2c3d4e5f6a7_add_default_identity_definitions.py Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit adds comprehensive tests for the RBAC models: - TestRBACPermission: Tests for creating and persisting permission records - TestRBACRole: Tests for custom roles, role hierarchy, and permission inheritance - TestRBACUserRole: Tests for role assignments, resource scoping, and temporal validity - TestRBACRoleConstraint: Tests for SoD and cardinality constraints Tests cover: - Basic CRUD operations - Parent-child role relationships - Permission inheritance through role hierarchy - Scoped role assignments (resource_type + resource_id) - Temporal role validity (valid_from, valid_until) - Separation of duties constraints - Cardinality constraints Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add passive_deletes=True to the user relationship so SQLAlchemy defers to the database's ON DELETE CASCADE behavior instead of trying to set user_id=NULL (which causes IntegrityError). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add scopes required for RBAC management UI: - rbac_role:create/read/update/delete - rbac_permission:read - rbac_user_role:create/read/update/delete - rbac_constraint:create/read/update/delete - rbac:evaluate Also add migration for existing installations to add these scopes and assign them to the Owner role. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This migration adds fidesplus-specific permissions (discovery_monitor, custom_field, taxonomy, etc.) to the rbac_permission table and assigns them to the appropriate system roles. This replaces the runtime seeding that was previously in fidesplus startup, following the pattern of keeping all DB changes in fides. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Updated down_revision from 6d5f70dd0ba5 to 627c230d9917 to align with the current head on main branch. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use RelationshipProperty[Type] for SQLAlchemy relationship type annotations to match the pattern used elsewhere in the fides codebase. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Addresses Greptile review feedback: 1. Fix migration docstring to match actual down_revision (627c230d9917) 2. Change rbac_role_permission from id-based PK to composite PK: - Migration now creates table with (role_id, permission_id) as composite PK - Removed id column and updated_at column (not needed for junction table) - Updated all seed migrations to not insert id column 3. Update RBACRolePermission model to override Base columns: - Set id = None to remove inherited id column - Set updated_at = None to remove inherited updated_at column - Composite PK on (role_id, permission_id) matches migration Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…et down_revision to main head Co-authored-by: Cursor <cursoragent@cursor.com>
4e10302 to
ae00de0
Compare
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.
Ticket: N/A - Internal infrastructure for RBAC system
Description Of Changes
Adds database migrations and SQLAlchemy models to support the new dynamic Role-Based Access Control (RBAC) system. This creates the foundational database schema that enables:
Important: These migrations only CREATE new tables - no existing tables are modified.
Code Changes
New Database Tables (5):
rbac_role- Dynamic role definitions with hierarchy supportrbac_permission- Permission definitions (seeded from SCOPE_REGISTRY)rbac_role_permission- Role to permission mapping (many-to-many)rbac_user_role- User role assignments with resource scoping + temporal validityrbac_role_constraint- Separation of duties constraintsMigrations (3):
d9ee4ea46797_add_rbac_tables.py- Creates all 5 RBAC tables with indexesf5f526cbc35a_seed_rbac_defaults.py- Seeds permissions from SCOPE_REGISTRY and creates system roles9f6555f12ad1_add_rbac_management_scopes.py- Adds RBAC management scopes (idempotent)SQLAlchemy Models:
src/fides/api/models/rbac/- New model package with all RBAC entitiessql_models.pyto import RBAC modelsSteps to Confirm
alembic upgrade head\dt rbac_*in psqlSELECT COUNT(*) FROM rbac_permission;(should be ~135)SELECT * FROM rbac_role;(should have 7 system roles)alembic downgrade -1(repeat 3x), verify tables removedPre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works