Fdc sql perms #7578
Fdc sql perms #7578
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7578 +/- ##
==========================================
- Coverage 53.06% 52.96% -0.11%
==========================================
Files 391 392 +1
Lines 27003 27112 +109
Branches 5571 5593 +22
==========================================
+ Hits 14330 14360 +30
- Misses 11393 11471 +78
- Partials 1280 1281 +1 ☔ View full report in Codecov by Sentry. |
…sers.create requirement for migrate
It should be safe. I tried it on a new database, used the current latest version of |
| @@ -151,6 +151,11 @@ export const githubClientSecret = () => | |||
|
|
|||
| export const dataconnectOrigin = () => | |||
| utils.envOverride("FIREBASE_DATACONNECT_URL", "https://firebasedataconnect.googleapis.com"); | |||
| export const dataconnectP4SADomain = () => | |||
There was a problem hiding this comment.
Not blocking: Is this something that will need to be overridden ever? It seems like all p4sas will have this domain. If it won't be overridden, you could simplify this to just a normal constant in schemaMigration.ts
There was a problem hiding this comment.
I'm not sure if it will be overridden, possibly if testing internally. But this is the pattern I saw other firebase products use for setting the P4SA domain so I feel sticking with how the rest of the CLI does this makes sense.
* Improve SQL permissions setup and security in dataconnenct:sql:migrate command * Improve sql role grant checks * redeuce required iam perms to perform sql setup and remove cloudsql.users.create requirement for migrate * Move P4SA user creation and grants to setupIAMUsers
Description
This pull request changes how we setup SQL permissions inside postgres as part of the dataconnect:migrate command. The purpose of this change is to reduce the radius of permissions we grant to improve the database security.
The new migration flow is the following:
Create Extensioncommands with "cloudsqlsuperuser" role.This change is backward compatible and effects already existing users minimally. We automatically revoke the cloudsqlsuperuser permission from firebaseowner to avoid cyclic dependencies/errors with the new roles setup but we don't revoke any other permissions that have been granted to PUBLIC.
Note, downgrading CLI version will require manual intervention, specifically users will need to manually run
REVOKE "firebaseowner_postgres_public" FROM "cloudsqlsuperuser".Scenarios Tested
Test 1
firebase dataconnect:sql:migrateto apply schema changes and extensions.firebase dataconnect:sql:migrateand confirmed diffs applied with no errors.Test 2
firebase dataconnect:sql:migrateand confirmed diffs applied, extensions installed with no errors.Sample Commands
No changes.