-
-
Notifications
You must be signed in to change notification settings - Fork 315
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
fix: Reduce postgres user grants #1159
base: main
Are you sure you want to change the base?
Conversation
Reduces the permissions given to the postgres user to reduce error prone operations from users when interacting with the schema 'realtime'
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
1 similar comment
lib/realtime/tenants/repo/migrations/20240919140541_reduce_grants_postgres_user.ex
Outdated
Show resolved
Hide resolved
lib/realtime/tenants/repo/migrations/20240919140541_reduce_grants_postgres_user.ex
Show resolved
Hide resolved
lib/realtime/tenants/repo/migrations/20240919140541_reduce_grants_postgres_user.ex
Outdated
Show resolved
Hide resolved
…nts_postgres_user.ex Co-authored-by: Bobbie Soedirgo <[email protected]>
…nts_postgres_user.ex Co-authored-by: Bobbie Soedirgo <[email protected]>
lib/realtime/tenants/repo/migrations/20240919140541_reduce_grants_postgres_user.ex
Outdated
Show resolved
Hide resolved
…nts_postgres_user.ex Co-authored-by: Bobbie Soedirgo <[email protected]>
execute("revoke supabase_realtime_admin from postgres") | ||
execute("alter default privileges for role supabase_admin in schema realtime revoke all on tables from postgres") | ||
execute("alter default privileges for role supabase_admin in schema realtime revoke all on functions from postgres") | ||
execute("alter default privileges for role supabase_admin in schema realtime revoke all on sequences from postgres") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this actually breaks our tests and implementations as supabase_admin
is the user we use to connect to tenant database and we still need them 😭
so I will need to remove this 3 execute
@soedirgo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm how does it break? It shouldn't affect existing tables/functions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
running required migrations fails:
18:14:41.532 project=dev_tenant external_id=dev_tenant [error] MigrationsFailedToRun: %Postgrex.Error{
message: nil,
postgres: %{
code: :undefined_object,
line: "5101",
message: "role \"supabase_admin\" does not exist",
file: "acl.c",
unknown: "ERROR",
severity: "ERROR",
pg_code: "42704",
routine: "get_role_oid"
},
connection_id: 156,
query: nil
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh we granted the default privileges here 😭 then yeah we need to move these out of Realtime tenant migrations
What kind of change does this PR introduce?
Reduces the permissions given to the postgres user to reduce error prone operations from users when interacting with the schema 'realtime'