Repository navigation
feat: add --login-token flag for separate IAM database authentication - #958
Conversation
| } | ||
| if conf.LoginToken != "" && conf.Token == "" { | ||
| return newBadCommandError("cannot specify --login-token without --token") | ||
| } |
There was a problem hiding this comment.
I don't think we need to force the user to specify both --token and --login-token here. The --login-token flag should be optional for the customer to use when they want to specify a specific tokens for interacting with our API vs only being able to login into the DB.
Be sure to also update the flag description in the various spots as well
There was a problem hiding this comment.
I see this makes sense. I did more research, and it seems that, especially since it is not a security concern, it does not warrant a warning, and especially not a hard fail.
There was a problem hiding this comment.
commit fixed
| oauth2.StaticTokenSource(&oauth2.Token{AccessToken: c.LoginToken}), | ||
| )) | ||
| case c.Token != "" || c.ImpersonationChain != "": | ||
| l.Infof("Warning: reusing the configured API credential for IAM database authentication login. " + |
There was a problem hiding this comment.
On a similar thought with the comment above - if the --login-token flag is optional then we should change this log to be a Note: instead of a Warning:, and could drop the second sentence since it's covered already in the help/README text
There was a problem hiding this comment.
Makes sense. Not a warning but just a note to let users know. Keep warnings for real threats that should be made known
fba809f to
2d2947a
Compare
enocom
left a comment
enocom
left a comment
There was a problem hiding this comment.
Please fix the two minor formatting issues noted below before merging.
| localFlags.StringVarP(&c.conf.Token, "token", "t", "", | ||
| "Bearer token used for authorization.") | ||
| localFlags.StringVar(&c.conf.LoginToken, "login-token", "", | ||
| "Bearer token used as a separate credential for IAM database authentication login. Only used when --auto-iam-authn is enabled.") |
There was a problem hiding this comment.
This is going to wrap in a small terminal window. Would you format this help string as we've done above and below?
There was a problem hiding this comment.
Got it thanks!
| the cached copy has expired. Use this setting in environments where the | ||
| CPU may be throttled and a background refresh cannot run reliably | ||
| (e.g., Cloud Run) | ||
| --login-token string Bearer token used as a separate credential for IAM database authentication login. Only used when --auto-iam-authn is enabled. |
There was a problem hiding this comment.
Ditto here on wrapping. We try to make this easy to read in the terminal on smaller screens.
There was a problem hiding this comment.
Got it thanks!
enocom
changed the title
4 participants
Overview
Adds a
--login-tokenflag that lets the proxy use a separate OAuth2 token forIAM database authentication login, distinct from
--token, which authenticatesAlloyDB Admin API calls. This keeps the API credentials (Unnecessary scopes) out of the database
login path, supporting least-privilege access.
Requires
--tokenand--auto-iam-authn. When unset, existing behavior isunchanged, and the proxy logs a warning suggesting the new flag.
Fixes #848.
Testing
--token+ valid--login-token→ connects--token+ corrupted--login-token→ rejected at the IAM check, proving no silent fallback to--token--tokenonly → connects via the existing fallback path, warning logged