-
Notifications
You must be signed in to change notification settings - Fork 2
185 allow organization specific roles #188
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.
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
Changes from all commits
5b81b32
b0ae182
5cbd5f9
28e174c
31c814c
24bbcaa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
calebbourg marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
//! `SeaORM` Entity, @generated by sea-orm-codegen 1.1.13 | ||
use super::roles::Role; | ||
use sea_orm::entity::prelude::*; | ||
use serde::{Deserialize, Serialize}; | ||
|
||
#[derive(Clone, Debug, PartialEq, DeriveEntityModel, Eq, Serialize, Deserialize)] | ||
#[sea_orm(schema_name = "refactor_platform", table_name = "user_roles")] | ||
pub struct Model { | ||
#[sea_orm(primary_key, auto_increment = false)] | ||
#[serde(skip_deserializing)] | ||
pub id: Uuid, | ||
pub role: Role, | ||
jhodapp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
pub organization_id: Option<Uuid>, | ||
pub user_id: Uuid, | ||
pub created_at: DateTimeWithTimeZone, | ||
pub updated_at: DateTimeWithTimeZone, | ||
} | ||
Comment on lines
+9
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Issue: Missing Validation:
Suggestion:
impl Model {
pub fn validate(&self) -> Result<(), ValidationError> {
match self.role {
Role::SuperAdmin if self.organization_id.is_some() => {
Err(ValidationError::new("SuperAdmin cannot be scoped to organization"))
}
Role::User | Role::Admin if self.organization_id.is_none() => {
Err(ValidationError::new("User/Admin roles must be scoped to organization"))
}
_ => Ok(())
}
}
} More Detailed Explanation: INSERT INTO user_roles (user_id, organization_id, role) VALUES
('user-123', 'some-org-uuid', 'super_admin'); -- ❌ BAD: Admin role with no organization (global admin?) INSERT INTO user_roles (user_id, organization_id, role) VALUES
('user-456', NULL, 'admin'); The database schema allows these invalid combinations because:
What the Validation Does impl Model {
pub fn validate(&self) -> Result<(), ValidationError> {
match self.role {
// Enforce: SuperAdmin MUST be global (organization_id = NULL)
Role::SuperAdmin if self.organization_id.is_some() => {
Err(ValidationError::new("SuperAdmin cannot be scoped to organization"))
}
// Enforce: User/Admin MUST be org-scoped (organization_id != NULL)
Role::User | Role::Admin if self.organization_id.is_none() => {
Err(ValidationError::new("User/Admin roles must be scoped to organization"))
}
_ => Ok(())
}
}
} Perhaps adding some unit tests around this also makes sense? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I plan to add validation in the next iteration when I add access to the data via web API There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I'll leave this here unresolved as a placeholder guide then. |
||
|
||
#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] | ||
pub enum Relation { | ||
#[sea_orm( | ||
belongs_to = "super::organizations::Entity", | ||
from = "Column::OrganizationId", | ||
to = "super::organizations::Column::Id", | ||
on_update = "Cascade", | ||
on_delete = "Cascade" | ||
)] | ||
Organizations, | ||
#[sea_orm( | ||
belongs_to = "super::users::Entity", | ||
from = "Column::UserId", | ||
to = "super::users::Column::Id", | ||
on_update = "Cascade", | ||
on_delete = "Cascade" | ||
)] | ||
Users, | ||
} | ||
|
||
impl Related<super::organizations::Entity> for Entity { | ||
fn to() -> RelationDef { | ||
Relation::Organizations.def() | ||
} | ||
} | ||
|
||
impl Related<super::users::Entity> for Entity { | ||
fn to() -> RelationDef { | ||
Relation::Users.def() | ||
} | ||
} | ||
|
||
impl ActiveModelBehavior for ActiveModel {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
use sea_orm_migration::prelude::*; | ||
|
||
#[derive(DeriveMigrationName)] | ||
pub struct Migration; | ||
|
||
#[async_trait::async_trait] | ||
impl MigrationTrait for Migration { | ||
async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> { | ||
// 1. Add super_admin variant to the existing role enum | ||
// Note: We use execute_unprepared() instead of SeaORM's schema builder because: | ||
// - PostgreSQL's ALTER TYPE ... ADD VALUE has special transaction restrictions | ||
// - It cannot be executed in a transaction block that has other commands | ||
// - execute_unprepared() gives us direct control over the SQL execution | ||
// - The IF NOT EXISTS clause makes it safe for re-runs | ||
manager | ||
.get_connection() | ||
.execute_unprepared( | ||
"ALTER TYPE refactor_platform.role ADD VALUE IF NOT EXISTS 'super_admin'", | ||
) | ||
jhodapp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.await?; | ||
|
||
// 2. Create the user_roles table | ||
// We continue using execute_unprepared() for consistency and to ensure | ||
// proper PostgreSQL schema qualification (refactor_platform.user_roles) | ||
let create_table_sql = "CREATE TABLE IF NOT EXISTS refactor_platform.user_roles ( | ||
id UUID PRIMARY KEY DEFAULT gen_random_uuid(), | ||
role refactor_platform.role NOT NULL, | ||
organization_id UUID, | ||
user_id UUID NOT NULL, | ||
created_at TIMESTAMPTZ NOT NULL DEFAULT now(), | ||
updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), | ||
CONSTRAINT fk_user_roles_organization | ||
FOREIGN KEY (organization_id) | ||
REFERENCES refactor_platform.organizations(id) | ||
ON DELETE CASCADE | ||
ON UPDATE CASCADE, | ||
CONSTRAINT fk_user_roles_user | ||
FOREIGN KEY (user_id) | ||
REFERENCES refactor_platform.users(id) | ||
ON DELETE CASCADE | ||
ON UPDATE CASCADE | ||
)"; | ||
|
||
manager | ||
.get_connection() | ||
.execute_unprepared(create_table_sql) | ||
.await?; | ||
|
||
// 3. Create partial unique indexes to prevent duplicate role assignments | ||
// This handles NULL organization_id properly (for super_admin roles) | ||
|
||
// Partial index for organization-scoped roles (where organization_id is NOT NULL) | ||
let create_org_index_sql = | ||
"CREATE UNIQUE INDEX IF NOT EXISTS user_roles_user_org_role_unique | ||
ON refactor_platform.user_roles(user_id, organization_id, role) | ||
WHERE organization_id IS NOT NULL"; | ||
|
||
manager | ||
.get_connection() | ||
.execute_unprepared(create_org_index_sql) | ||
.await?; | ||
|
||
// Partial index for global roles (prevents duplicate global roles for same user) | ||
// This includes super_admin and any other future global roles | ||
let create_global_role_index_sql = | ||
"CREATE UNIQUE INDEX IF NOT EXISTS user_roles_user_global_role_unique | ||
ON refactor_platform.user_roles(user_id, role) | ||
WHERE organization_id IS NULL"; | ||
|
||
manager | ||
.get_connection() | ||
.execute_unprepared(create_global_role_index_sql) | ||
.await?; | ||
|
||
Ok(()) | ||
} | ||
|
||
async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { | ||
calebbourg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Drop the user_roles table (this will also drop the indexes and foreign keys) | ||
manager | ||
.get_connection() | ||
.execute_unprepared("DROP TABLE IF EXISTS refactor_platform.user_roles") | ||
.await?; | ||
|
||
// Note: We cannot remove the 'super_admin' value from the enum in PostgreSQL | ||
// once it has been added. This is a PostgreSQL limitation. | ||
// If you need to truly remove it, you would need to: | ||
// 1. Create a new enum type without 'super_admin' | ||
// 2. Update all columns using the old type to use the new type | ||
// 3. Drop the old enum type | ||
// This is complex and risky, so we'll leave the enum value in place. | ||
|
||
Ok(()) | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.