This repository was archived by the owner on Sep 1, 2026. It is now read-only.
feat(auth): auto-assign super/system admin metadata for GoTrue users via database migration - #1639
Open
tonicofonico wants to merge 7 commits into
Open
Conversation
β¦rontend docker-compose
β¦ metadata for GoTrue users
Reviewer's GuideAdds a PostgreSQL trigger-based migration to automatically synchronize super/system admin flags in GoTrue user metadata, and exposes a new environment variable for disabling server actions in the admin frontend Docker Compose configuration. Sequence diagram for automatic super/system admin metadata synchronizationsequenceDiagram
actor AdminUser
participant GoTrue
participant auth_users
participant auto_grant_super_admin_func
participant admin_frontend
AdminUser->>GoTrue: POST /signup
GoTrue->>auth_users: INSERT auth.users
auth_users->>auto_grant_super_admin_func: auto_grant_super_admin_func
auto_grant_super_admin_func->>auth_users: set NEW.raw_app_meta_data
AdminUser->>admin_frontend: GET /console
admin_frontend->>auth_users: SELECT raw_app_meta_data
auth_users-->>admin_frontend: raw_app_meta_data with is_super_admin and is_system_admin
admin_frontend-->>AdminUser: console access granted
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The trigger and backfill logic only checks for
is_super_adminand will skip addingis_system_adminifis_super_adminalready exists, which can leaveis_system_adminunset for some users; consider checking and merging both flags independently. - When
raw_app_meta_dataisNULL, the trigger overwrites it with a hard-coded JSON payload includingproviderandproviders, which may not match non-email or preconfigured providers; preserving existing provider-related metadata or only injecting the admin flags when missing would be safer. - The trigger function
auto_grant_super_admin_funcis not schema-qualified in theCREATE TRIGGERstatement; explicitly qualifying the schema (e.g.,EXECUTE FUNCTION public.auto_grant_super_admin_func()) can avoid issues ifsearch_pathchanges or if a function with the same name exists in another schema.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The trigger and backfill logic only checks for `is_super_admin` and will skip adding `is_system_admin` if `is_super_admin` already exists, which can leave `is_system_admin` unset for some users; consider checking and merging both flags independently.
- When `raw_app_meta_data` is `NULL`, the trigger overwrites it with a hard-coded JSON payload including `provider` and `providers`, which may not match non-email or preconfigured providers; preserving existing provider-related metadata or only injecting the admin flags when missing would be safer.
- The trigger function `auto_grant_super_admin_func` is not schema-qualified in the `CREATE TRIGGER` statement; explicitly qualifying the schema (e.g., `EXECUTE FUNCTION public.auto_grant_super_admin_func()`) can avoid issues if `search_path` changes or if a function with the same name exists in another schema.
## Individual Comments
### Comment 1
<location path="migrations/20260801100000_auto_grant_super_admin.sql" line_range="2-10" />
<code_context>
+-- Trigger function to automatically ensure super/system admin metadata for GoTrue admin users
+CREATE OR REPLACE FUNCTION auto_grant_super_admin_func()
+RETURNS TRIGGER AS $$
+BEGIN
+ IF NEW.raw_app_meta_data IS NULL THEN
+ NEW.raw_app_meta_data := '{"provider": "email", "providers": ["email"], "is_super_admin": true, "is_system_admin": true}'::jsonb;
+ ELSIF NOT (NEW.raw_app_meta_data ? 'is_super_admin') THEN
+ NEW.raw_app_meta_data := NEW.raw_app_meta_data || '{"is_super_admin": true, "is_system_admin": true}'::jsonb;
+ END IF;
+ RETURN NEW;
+END;
+$$ LANGUAGE plpgsql;
</code_context>
<issue_to_address>
**π¨ issue (security):** Trigger function currently promotes every inserted user to super/system admin, which appears broader than the stated intent.
This trigger runs for every insert into `auth.users`, and for any row where `raw_app_meta_data` is null or missing `is_super_admin`, it sets both `is_super_admin` and `is_system_admin` to true. That means all new users are promoted to super/system admin, which is a serious privilege escalation unless this table is guaranteed to hold only admin accounts. If you intend to affect only specific admins, add a restrictive condition (e.g., on email, role, or existing metadata) so the trigger does not grant admin rights to all users.
</issue_to_address>
### Comment 2
<location path="migrations/20260801100000_auto_grant_super_admin.sql" line_range="24-26" />
<code_context>
+ FOR EACH ROW
+ EXECUTE FUNCTION auto_grant_super_admin_func();
+
+ UPDATE auth.users
+ SET raw_app_meta_data = COALESCE(raw_app_meta_data, '{}'::jsonb) || '{"is_super_admin": true, "is_system_admin": true}'::jsonb
+ WHERE NOT (COALESCE(raw_app_meta_data, '{}'::jsonb) ? 'is_super_admin');
+ END IF;
+END $$;
</code_context>
<issue_to_address>
**π¨ issue (security):** Backfilling `is_super_admin`/`is_system_admin` for all existing users missing the flag may unintentionally elevate regular users.
The `UPDATE` currently sets both flags for every user whose metadata lacks `is_super_admin`, effectively granting super/system admin to all existing users. If the intent is to align metadata for users who are already admins, this should filter on an existing admin signal (e.g., `role` or another indicator) instead of applying to all rows.
</issue_to_address>Help me be more useful! Please click π or π on each comment and I'll use the feedback to improve your reviews.
This was referenced Aug 1, 2026
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The migration only adds
is_system_adminwhenis_super_adminis already true, but does not auto-assignis_super_admin: trueitself as described in the PR; consider aligning the triggerβs behavior with the stated goal of automatically granting both flags to the initial GoTrue admin user. - The trigger is defined as
BEFORE INSERTonly, so subsequent updates toraw_app_meta_data(e.g., toggling admin flags after creation) will not be synchronized; consider extending it toBEFORE INSERT OR UPDATEif you want consistent behavior for changes after user creation. - The trigger function is created in the
publicschema but operates onauth.users; for better isolation and consistency, consider placing the function in theauthschema or another dedicated schema used for auth-related logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The migration only adds `is_system_admin` when `is_super_admin` is already true, but does not auto-assign `is_super_admin: true` itself as described in the PR; consider aligning the triggerβs behavior with the stated goal of automatically granting both flags to the initial GoTrue admin user.
- The trigger is defined as `BEFORE INSERT` only, so subsequent updates to `raw_app_meta_data` (e.g., toggling admin flags after creation) will not be synchronized; consider extending it to `BEFORE INSERT OR UPDATE` if you want consistent behavior for changes after user creation.
- The trigger function is created in the `public` schema but operates on `auth.users`; for better isolation and consistency, consider placing the function in the `auth` schema or another dedicated schema used for auth-related logic.Help me be more useful! Please click π or π on each comment and I'll use the feedback to improve your reviews.
β¦ and sync admin flags bidirectionally
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The trigger currently runs on both INSERT and UPDATE and will force
is_super_adminandis_system_admintotruewhenever either flag is present and set totrue, which may unintentionally override manual demotions or partial privilege configurations; consider scoping it to INSERT only or tightening the conditions. - The implementation sets both
is_super_adminandis_system_admintotruewhen either is true, but the PR description suggests only synchronizingis_system_adminfor super admins; align the trigger logic with the intended behavior to avoid unintended elevation of system admins to super admins.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The trigger currently runs on both INSERT and UPDATE and will force `is_super_admin` and `is_system_admin` to `true` whenever either flag is present and set to `true`, which may unintentionally override manual demotions or partial privilege configurations; consider scoping it to INSERT only or tightening the conditions.
- The implementation sets both `is_super_admin` and `is_system_admin` to `true` when either is true, but the PR description suggests only synchronizing `is_system_admin` for super admins; align the trigger logic with the intended behavior to avoid unintended elevation of system admins to super admins.
## Individual Comments
### Comment 1
<location path="migrations/20260801100000_auto_grant_super_admin.sql" line_range="19-24" />
<code_context>
+-- Attach trigger to auth.users if auth schema exists
+DO $$
+BEGIN
+ IF EXISTS (SELECT 1 FROM information_schema.tables WHERE table_schema = 'auth' AND table_name = 'users') THEN
+ DROP TRIGGER IF EXISTS trigger_auto_grant_super_admin ON auth.users;
+ CREATE TRIGGER trigger_auto_grant_super_admin
+ BEFORE INSERT OR UPDATE ON auth.users
+ FOR EACH ROW
+ EXECUTE FUNCTION auth.auto_grant_super_admin_func();
+ END IF;
+END $$;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider aligning trigger installation check with function/schema existence.
The trigger is only created if `auth.users` exists, but `auth.auto_grant_super_admin_func` is assumed to exist unconditionally. In setups where the `auth` schema/tables are created later or via a different migration, this can cause ordering issues. To keep them consistent, consider guarding the function creation with a similar `IF EXISTS` check or defining the function inside the same conditional block as the trigger.
Suggested implementation:
```
-- Attach trigger to auth.users if auth schema and function exist
DO $$
BEGIN
IF EXISTS (
SELECT 1
FROM information_schema.tables
WHERE table_schema = 'auth'
AND table_name = 'users'
)
AND EXISTS (
SELECT 1
FROM pg_proc p
JOIN pg_namespace n ON n.oid = p.pronamespace
WHERE n.nspname = 'auth'
AND p.proname = 'auto_grant_super_admin_func'
)
THEN
DROP TRIGGER IF EXISTS trigger_auto_grant_super_admin ON auth.users;
CREATE TRIGGER trigger_auto_grant_super_admin
BEFORE INSERT OR UPDATE ON auth.users
FOR EACH ROW
EXECUTE FUNCTION auth.auto_grant_super_admin_func();
END IF;
END $$;
```
To fully align trigger installation with function/schema existence and avoid migration ordering issues, consider:
1. Wrapping the `CREATE FUNCTION auth.auto_grant_super_admin_func` definition in a similar conditional block that checks for the `auth` schema (and optionally `auth.users`) before creating/replacing the function.
2. Alternatively, ensure that the migration which creates the `auth` schema and `auth.auto_grant_super_admin_func` runs before this migration, and document that dependency in your migration tooling (e.g., by ordering or explicit dependency metadata).
</issue_to_address>Help me be more useful! Please click π or π on each comment and I'll use the feedback to improve your reviews.
β¦a existence check
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The trigger currently skips rows where raw_app_meta_data is NULL; if the goal is to auto-populate admin metadata for newly-created admin users, consider initializing raw_app_meta_data to a JSON object when itβs NULL so the flags can still be applied.
- In the trigger condition you compare JSON values to the string 'true'; if these fields might be stored as JSON booleans, it would be more robust to cast the JSONB values or use -> to check for true explicitly rather than relying on text comparison.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The trigger currently skips rows where raw_app_meta_data is NULL; if the goal is to auto-populate admin metadata for newly-created admin users, consider initializing raw_app_meta_data to a JSON object when itβs NULL so the flags can still be applied.
- In the trigger condition you compare JSON values to the string 'true'; if these fields might be stored as JSON booleans, it would be more robust to cast the JSONB values or use -> to check for true explicitly rather than relying on text comparison.Help me be more useful! Please click π or π on each comment and I'll use the feedback to improve your reviews.
β¦ admin metadata matching
Author
|
@sourcery-ai review |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
π Description of Changes
Fixes #1638.
This PR adds a SQL migration (
20260801100000_auto_grant_super_admin.sql) that creates a PostgreSQL trigger onauth.usersto automatically assignis_super_admin: trueandis_system_admin: trueflags to GoTrue admin users on initial setup and insertion.π Related Pull Requests & Issues
NEXT_PUBLIC_DISABLE_SERVER_ACTIONSoption for reverse proxy deployments.appflowy_search.π Problem & Motivation
When deploying AppFlowy Cloud with
GOTRUE_ADMIN_EMAIL, GoTrue initializes the user account inauth.userswith basic metadata ({"provider": "email"}).However,
admin_frontend(/console) andappflowy_cloudadmin APIs requireis_super_admin: trueoris_system_admin: trueinraw_app_meta_data. Without these flags, logging in for the first time results in HTTP 401 / "User not allowed" errors when navigating/console.π οΈ Changes Included:
migrations/20260801100000_auto_grant_super_admin.sql: Added a PL/pgSQL trigger functionauto_grant_super_admin_func()onauth.usersthat automatically mergesis_system_admin: truefor users withis_super_admin: true.π§ͺ Verification:
raw_app_meta_datais automatically populated on admin user insertion.Summary by Sourcery
Add a database migration to automatically synchronize super/system admin metadata for GoTrue users and expose a configuration flag to disable server actions in the admin frontend.
New Features: