Fdc grant cmds #7656
Fdc grant cmds #7656
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
tammam-g
force-pushed
the
fdc_grant_cmds
branch
from
b08b2a9
to
28fe9a9
Compare
| "-E, --email <email>", | ||
| "The email of the user or service account we would like to grant the role to.", | ||
| ) | ||
| .before(requirePermissions, ["firebasedataconnect.services.list"]) |
There was a problem hiding this comment.
and cloudsql.instance.connect
We can move cloudsql.users.create here for simplicity.
There was a problem hiding this comment.
These are in the iamUserIsCSQLAdmin check so the user can get a nice error message. I'm ok with removing the iamUserIsCSQLAdmin check and just moving things here.
src/dataconnect/schemaMigration.ts
Outdated
| throw new FirebaseError( | ||
| "-E, --email <email> is required. Run the command with -h for more info.", | ||
| ); | ||
| } |
There was a problem hiding this comment.
Should these validations be at cmd file?
There was a problem hiding this comment.
+1 - consider using https://github.com/tj/commander.js?tab=readme-ov-file#required-option instead of option to make these required.
There was a problem hiding this comment.
Moved to cmd file.
@joehan, I tried using requiredOption but it wasn't possible. We have our own Command type and we don't define requiredOption on it. I also looked around and it seems like we don't have any usage of this.
src/dataconnect/schemaMigration.ts
Outdated
| // Make sure current user can perform this action. | ||
| const userIsCSQLAdmin = await iamUserIsCSQLAdmin(options); | ||
| if (!userIsCSQLAdmin) { | ||
| throw new FirebaseError(`Only users with 'roles/cloudsql.admin' can grant SQL roles.`); |
There was a problem hiding this comment.
Wonder if requiring cloudsql.users.create in the cmd permission will do it.
This error message is nice though.
There was a problem hiding this comment.
This might be a good place to print out instructions telling users to ask their DB admin - ie:
Only users with 'roles/cloudsql.admin' can grant SQL roles. If you do not have this role, ask your database administrator to run this command or manually grant <role> to <user>
There was a problem hiding this comment.
Updated the error message. The error message also will take care of the service account extension.
| "-E, --email <email>", | ||
| "The email of the user or service account we would like to grant the role to.", | ||
| ) | ||
| .before(requirePermissions, ["firebasedataconnect.services.list"]) |
Description
Add
firebase dataconnect:sql:grant -R <role> -E <email>command. The command grants the corresponding role in the FDC sql instance.Scenarios Tested
Not passing -R or --role ->
Error: -R, --role <role> is required. Run the command with -h for more info.Note passing -E or --email ->
Error: -E, --email <email> is required. Run the command with -h for more info.passing non existing role ->
Error: Role should be one of owner | writer | reader.Sample Commands