Skip to content
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

feat: add support for Postgres IAM authentication in JDBC and R2DBC connectors #490

Merged
merged 15 commits into from May 24, 2021

Conversation

shubha-rajan
Copy link
Contributor

@shubha-rajan shubha-rajan commented May 11, 2021

Change Description

Adds IAM Auth support. Some notes to keep in mind:

  • Users need to include a nonempty string for the password when connecting. This will be ignored, but is required to bypass driver errors for null and empty passwords.
  • sslmode needs to be disabled or the driver will hang indefinitely.

Checklist

  • Make sure to open an issue as a
    bug/issue
    before writing your code! That way we can discuss the change, evaluate
    designs, and agree on the general idea.
  • Ensure the tests and linter pass
  • Appropriate documentation is updated (if necessary)

Relevant issues:

@google-cla google-cla bot added the cla: yes label May 11, 2021
@shubha-rajan shubha-rajan force-pushed the iam-auth branch 2 times, most recently from a289613 to d8db5d2 Compare May 11, 2021 21:20
@shubha-rajan shubha-rajan added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels May 11, 2021
@shubha-rajan shubha-rajan marked this pull request as ready for review May 11, 2021 21:26
/**
* Name of system property that can specify an alternative credential factory.
*/
String TOKEN_SOURCE_FACTORY_PROPERTY = "cloudSql.socketFactory.TokenSourceFactory";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a credential socket factory right? Can we get a token source from the credentials?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess what I'm saying is we already have a property for setting the user credentials, can we reuse the user credentials instead of trying to ask for the token source from somewhere else?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the Credential Factory returns credentials that are wrapped in a HttpCredentialsAdapter and there's no way to get credentials back from that, because they're private https://github.com/googleapis/google-auth-library-java/blob/master/oauth2_http/java/com/google/auth/http/HttpCredentialsAdapter.java#L64

We could change the Credential Factory to return a GoogleCredentials object and then convert that to the HttpCredentialsAdapter later, but I'm hesitant to change the existing interface

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a DPE owned library - can we PR a getter so we can have access to it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shubha-rajan shubha-rajan force-pushed the iam-auth branch 4 times, most recently from 4732aa0 to 6f8f197 Compare May 14, 2021 16:48
@shubha-rajan shubha-rajan added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 14, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 14, 2021
@shubha-rajan shubha-rajan merged commit 3799c78 into main May 24, 2021
@shubha-rajan shubha-rajan deleted the iam-auth branch May 24, 2021 16:41
@meltsufin
Copy link
Member

@shubha-rajan @kurtisvg

sslmode needs to be disabled or the driver will hang indefinitely.

Are there any security implications to disabling sslmode?

@kurtisvg
Copy link
Contributor

kurtisvg commented Jun 3, 2021

@meltsufin No - there are two potential layers of encryption - at the connection level, and at the database protocol level. Disabling at the database level is fine because the connection level is always persistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for IAM database authentication
4 participants