Skip to content

Ft/user central rbac #185

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

Open
AnuMonachan wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Ft/user central rbac #185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
84c3183
user central rbac first draft
AnuMonachan Jun 4, 2026
8ddd31c
review changes and pass org roles
AnuMonachan Jun 10, 2026
f3ad943
Merge branch 'main' into ft/user-central-rbac
AnuMonachan Jun 10, 2026
739847d
format fix
AnuMonachan Jun 10, 2026
99ef62b
added db migration files
AnuMonachan Jun 10, 2026
53c98cb
updated db migration files as per review
AnuMonachan Jun 11, 2026
e05b9b9
code review fixes
AnuMonachan Jun 11, 2026
7538074
policy fixes and other code review issues
AnuMonachan Jun 11, 2026
e7f1f72
code review fixes, issues found by code rabbit
AnuMonachan Jun 11, 2026
10df565
minor bug fixes
AnuMonachan Jun 11, 2026
3330273
Merge branch 'main' into ft/user-central-rbac
kaseywright Jun 14, 2026
27719f5
updated code in order to incorporate org switcher ui
AnuMonachan Jun 24, 2026
0ed7be3
error fix
AnuMonachan Jun 25, 2026
5781254
Merge branch 'main' into ft/user-central-rbac
AnuMonachan Jul 13, 2026
1e6e9ac
format fix
AnuMonachan Jul 13, 2026
2a290f3
error fixes
AnuMonachan Jul 14, 2026
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
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"db:seed:roles": "npx tsx src/db/seeds/roles.ts",
"db:seed:org": "npx tsx src/db/seeds/organizations.ts",
"db:seed:dev-users": "npx tsx src/db/seeds/dev-users.ts",
"db:migrate-rbac": "npx tsx src/db/scripts/migrate-to-user-central-rbac.ts",
"db:seed:languages": "npx tsx src/db/seeds/languages.ts",
"db:seed:books": "npx tsx src/db/seeds/books.ts",
"db:seed:bibles": "npx tsx src/db/seeds/bibles.ts",
Expand Down
20 changes: 20 additions & 0 deletions src/db/migrations/0015_create_user_role_grants.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
CREATE TABLE "user_roles" (
"id" serial PRIMARY KEY NOT NULL,
"user_id" integer NOT NULL,
"org_id" integer,
"project_id" integer,
"role_id" integer NOT NULL,
"created_by" integer,
"created_at" timestamp DEFAULT now(),
"updated_at" timestamp DEFAULT now()
);
--> statement-breakpoint
ALTER TABLE "user_roles" ADD CONSTRAINT "user_roles_user_id_users_id_fk" FOREIGN KEY ("user_id") REFERENCES "public"."users"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint
ALTER TABLE "user_roles" ADD CONSTRAINT "user_roles_org_id_organizations_id_fk" FOREIGN KEY ("org_id") REFERENCES "public"."organizations"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint
ALTER TABLE "user_roles" ADD CONSTRAINT "user_roles_project_id_projects_id_fk" FOREIGN KEY ("project_id") REFERENCES "public"."projects"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint
ALTER TABLE "user_roles" ADD CONSTRAINT "user_roles_role_id_roles_id_fk" FOREIGN KEY ("role_id") REFERENCES "public"."roles"("id") ON DELETE no action ON UPDATE no action;--> statement-breakpoint
ALTER TABLE "user_roles" ADD CONSTRAINT "user_roles_created_by_users_id_fk" FOREIGN KEY ("created_by") REFERENCES "public"."users"("id") ON DELETE no action ON UPDATE no action;--> statement-breakpoint
CREATE UNIQUE INDEX "uq_user_role_grant" ON "user_roles" USING btree ("user_id","org_id","project_id","role_id");--> statement-breakpoint

@coderabbitai coderabbitai Bot Jun 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

This unique index does not actually de-duplicate NULL-scoped grants.

In PostgreSQL, NULL values are distinct for ordinary unique indexes, so duplicate global/org grants like (user_id, org_id = NULL, project_id = NULL, role_id) or (user_id, org_id = 7, project_id = NULL, role_id) can still be inserted multiple times. That breaks the one-grant-per-scope contract this table is meant to enforce.

Use partial unique indexes per scope, or NULLS NOT DISTINCT if that matches the database target.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/db/migrations/0012_added_user_role_grants.sql` at line 24, The current
unique index uq_user_role_grant on table user_roles over (user_id, org_id,
project_id, role_id) doesn’t prevent duplicate NULL-scoped grants because
PostgreSQL treats NULLs as distinct; replace it with scope-aware constraints:
drop uq_user_role_grant and add partial unique indexes for each scope (global:
WHERE org_id IS NULL AND project_id IS NULL on (user_id, role_id); org-scoped:
WHERE org_id IS NOT NULL AND project_id IS NULL on (user_id, org_id, role_id);
project-scoped: WHERE project_id IS NOT NULL on (user_id, org_id, project_id,
role_id)), or if your Postgres target supports it use a single index with NULLS
NOT DISTINCT on (user_id, org_id, project_id, role_id); ensure the migration
updates the index creation to one of these options so one-grant-per-scope is
enforced.

CREATE INDEX "idx_user_roles_user" ON "user_roles" USING btree ("user_id");--> statement-breakpoint
CREATE INDEX "idx_user_roles_org" ON "user_roles" USING btree ("org_id");--> statement-breakpoint
CREATE INDEX "idx_user_roles_project" ON "user_roles" USING btree ("project_id");
9 changes: 9 additions & 0 deletions src/db/migrations/0016_drop_legacy_rbac_columns.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
ALTER TABLE "project_users" DISABLE ROW LEVEL SECURITY;--> statement-breakpoint
DROP TABLE "project_users" CASCADE;--> statement-breakpoint

@coderabbitai coderabbitai Bot Jun 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Destructive RBAC drops run without a safety gate.

Lines 2, 8, and 9 remove legacy authorization data with no in-migration precondition/backfill guard, so running this migration early can permanently lose role assignments.

Suggested safety guard before DROP statements 
+DO $$
+BEGIN
+  IF EXISTS (
+    SELECT 1
+    FROM "users"
+    WHERE "role" IS NOT NULL OR "organization" IS NOT NULL
+  )
+  AND NOT EXISTS (SELECT 1 FROM "user_roles")
+  THEN
+    RAISE EXCEPTION 'Refusing to drop legacy RBAC columns: user_roles backfill not detected';
+  END IF;
+END $$;
+
 ALTER TABLE "project_users" DISABLE ROW LEVEL SECURITY;--> statement-breakpoint
 DROP TABLE "project_users" CASCADE;--> statement-breakpoint
 ...

Also applies to: 8-9

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/db/migrations/0013_drop_legacy_rbac_columns.sql` at line 2, The DROP
TABLE "project_users" CASCADE; and the other DROP statements at lines 8-9 remove
legacy RBAC data with no safety gate; add a precondition/backfill guard at the
top of this migration that aborts unless a safe condition is met (for example:
require an environment-controlled migration flag or confirm legacy tables are
empty). Implement this by querying the tables (e.g., SELECT count(*) FROM
"project_users") and RAISE EXCEPTION (or use pg_notify/RAISE NOTICE and exit) if
counts > 0, or by checking a feature_flag table/pg_settings key before executing
the DROP, and include a clear message instructing operators how to enable the
migration after performing backfill/verification.

ALTER TABLE "users" DROP CONSTRAINT "users_role_roles_id_fk";
--> statement-breakpoint
ALTER TABLE "users" DROP CONSTRAINT "users_organization_organizations_id_fk";
--> statement-breakpoint
DROP INDEX "idx_role_permissions_role";--> statement-breakpoint
ALTER TABLE "users" DROP COLUMN "role";--> statement-breakpoint
ALTER TABLE "users" DROP COLUMN "organization";
2 changes: 2 additions & 0 deletions src/db/migrations/0017_add_user_roles_unique_index.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
DROP INDEX "uq_user_role_grant";--> statement-breakpoint
CREATE UNIQUE INDEX "uq_user_role_grant" ON "user_roles" USING btree ("user_id",COALESCE("org_id", -1),COALESCE("project_id", -1),"role_id");
4 changes: 4 additions & 0 deletions src/db/migrations/0018_added_user_session.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
ALTER TABLE "auth_session" ADD COLUMN "active_org_id" integer;--> statement-breakpoint
ALTER TABLE "users" ADD COLUMN "last_active_org_id" integer;--> statement-breakpoint
ALTER TABLE "auth_session" ADD CONSTRAINT "auth_session_active_org_id_organizations_id_fk" FOREIGN KEY ("active_org_id") REFERENCES "public"."organizations"("id") ON DELETE no action ON UPDATE no action;--> statement-breakpoint
ALTER TABLE "users" ADD CONSTRAINT "users_last_active_org_id_organizations_id_fk" FOREIGN KEY ("last_active_org_id") REFERENCES "public"."organizations"("id") ON DELETE no action ON UPDATE no action;
Loading