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

Add support for specifying custom credentials #200

Closed
Anoupz opened this issue Sep 13, 2023 · 15 comments · Fixed by #238
Closed

Add support for specifying custom credentials #200

Anoupz opened this issue Sep 13, 2023 · 15 comments · Fixed by #238
Assignees
Labels
priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@Anoupz
Copy link

Anoupz commented Sep 13, 2023

Question

Getting this error

Not authorized to access resource. Possibly missing permission cloudsql.instances.get on resource

I am connecting with my service account and I verified that the user has cloudsql.admin. Any help?

Additional Context

@Anoupz Anoupz added the type: question Request for information or clarification. label Sep 13, 2023
@Anoupz Anoupz closed this as completed Sep 13, 2023
@Anoupz Anoupz reopened this Sep 13, 2023
@Anoupz
Copy link
Author

Anoupz commented Sep 13, 2023

The issue seems to be the auth being different. Can we update the connector instance to accept custom auth?

@edosrecki
Copy link
Contributor

The issue seems to be the auth being different. Can we update the connector instance to accept custom auth?

I am curious, what do you mean by custom auth?

@Anoupz
Copy link
Author

Anoupz commented Sep 13, 2023

The issue seems to be the auth being different. Can we update the connector instance to accept custom auth?

I am curious, what do you mean by custom auth?

We have a service account, and our API impersonates the service account using cloud Impersonated. When we connect via the Impersonated service account using this connector we get 403 error since it defaults to application default login.

@edosrecki
Copy link
Contributor

edosrecki commented Sep 13, 2023

@Anoupz I don't fully understand your setup from the description that you provided.

However, Application Default Credentials support service account impersonation. For example, locally you can try it out by running:

gcloud auth application-default login --impersonate-service-account $SERVICE_ACCOUNT_EMAIL

Cloud SQL Node.js Connector uses standard google-auth-library, which supports Application Default Credentials JSON with service account impersonation.

There is also an example of Application Default Credentials file that uses impersonation.

This is something that I use and it works as expected.

@Anoupz
Copy link
Author

Anoupz commented Sep 13, 2023

Thanks @edosrecki.

I'm using a backend API in Node.js, and we make use of the Impersonated class provided by the google-auth-library. You can find more information about it here: https://cloud.google.com/nodejs/docs/reference/google-auth-library/latest/google-auth-library/impersonated

We utilize the authentication credentials generated by this instantiation of the Impersonated class across our various services to authenticate our service account. When it comes to testing our APIs locally, we establish a connection using our developer's gcloud credentials, and the API impersonates the service account, granting us access to cloud services.

To initiate this process, I execute the command gcloud auth application-default login, connecting with my developer account before starting the service. However, I've encountered an issue with the cloud-sql-nodejs-connector. It appears that even though the sqladmin-fetcher accepts loginAuth as an optional parameter, it defaults to using application default login.

https://github.com/GoogleCloudPlatform/cloud-sql-nodejs-connector/blob/main/src/sqladmin-fetcher.ts#L75

If the connector can indeed accept custom authentication and pass it down to the sqladmin-fetcher that would work. I was able to test it locally.
https://github.com/GoogleCloudPlatform/cloud-sql-nodejs-connector/blob/main/src/connector.ts#L155-L157

I am open to more ideas.

@edosrecki
Copy link
Contributor

@Anoupz

Impersonated class is exactly what gets automatically instantiated by google-auth-library when you use Application Default Credentials in a way that I described. This is a feature of the google-auth-library to automatically decide which auth method to use based on the environment setup. Therefore I believe that your use case should already be supported.

loginAuth is currently accepted in the constructor only to be able to provide a mock implementation in tests.

@jackwotherspoon
Copy link
Collaborator

jackwotherspoon commented Sep 14, 2023

Hi @Anoupz, as @edosrecki mentioned, google-auth-library should be able to pick up impersonated credentials and set them as the Application Default Credentials.

To initiate this process, I execute the command gcloud auth application-default login

@Anoupz are you initializing this command with the --impersonate-service-account flag set as @edosrecki showed?

@Anoupz
Copy link
Author

Anoupz commented Sep 14, 2023

@edosrecki & @jackwotherspoon,
Our API operates using a service account (let's call it SA_A) to interact with our customers' Google services. We employ impersonation techniques by assuming the identity of another service account (let's call it SA_B). For each customer, SA_A connects to different SAs for enhanced security.
So here the impersonated auth is not the default auth. I need to use impersonated auth and connect to the customer cloud SQL instance.

Again thanks for all the help and info.

@jackwotherspoon jackwotherspoon added the priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. label Sep 14, 2023
@jackwotherspoon jackwotherspoon changed the title Not authorized to access resource. Possibly missing permission cloudsql.instances.get on resource Add support for specifying custom credentials Sep 14, 2023
@jackwotherspoon jackwotherspoon added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed type: question Request for information or clarification. labels Sep 14, 2023
@jackwotherspoon
Copy link
Collaborator

@Anoupz I understand your use-case now! Appreciate the detailed explanation 😄 I have changed this issue into a feature request for allowing a custom google-auth-library credentials object to be passed into the connector. This is currently something we support in our other language connectors (Python Connector feature) so it shouldn't be too difficult to add here for node as well.

Thanks for raising this!

@Anoupz
Copy link
Author

Anoupz commented Sep 14, 2023

Thanks @jackwotherspoon. I am happy to contribute as well. I have submitted the Contributor License Agreement. But still don't have access to create a PR. Let me know if I need follow up something else.

@jackwotherspoon
Copy link
Collaborator

I am happy to contribute as well. I have submitted the Contributor License Agreement. But still don't have access to create a PR. Let me know if I need follow up something else.

We always welcome external contributions 😄 If you create a fork of the repository you can then push changes to your fork for development. You should then be able to open a PR using your fork. Let me know if this doesn't work for you and I can provide more details.

Here is the Python PR that implemented this feature in the Python Connector just in case it helps provide any guidance.

@Anoupz
Copy link
Author

Anoupz commented Sep 14, 2023

Thanks @jackwotherspoon I was able to create the PR.

@Anoupz
Copy link
Author

Anoupz commented Oct 6, 2023

@jackwotherspoon and @edosrecki can you help me with this PR. Appreciate all the help

@enocom
Copy link
Member

enocom commented Oct 9, 2023

cc @ruyadorno.

@ruyadorno
Copy link
Contributor

Hi @Anoupz you can join the follow up conversation in original PR: #204

It looks like at this point in time we're blocked on a GoogleAuth type problem documented by @edosrecki here: googleapis/google-api-nodejs-client#3357, luckily a possible fix is already on its way: googleapis/google-auth-library-nodejs#1624. In the short term here we'll be waiting to see this fix landing before reassessing your original PR, I see it has been closed but feel free to reopen it and leave it open meanwhile 😊

Thanks for the help!

ruyadorno pushed a commit that referenced this issue Nov 13, 2023
Adds a new `auth` property to the connector constructor that
can be used by `SQLAdminFetcher` to extend from or provide
support to a custom auth object defined by the user.

Fixes: #200
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
5 participants