-
Notifications
You must be signed in to change notification settings - Fork 373
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
fix: Make ADC + human account work with firebase-admin #2553
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you!
Hi @foxrafa could you please take a look at the lint errors in the failing CIs? Thanks! |
@lahirumaramba fixed it. |
Oh, all of the test cases are not expecting the x-goog-user-project header. Can I change them @lahirumaramba ? |
Thanks! We also need to update the unit tests :) |
I updated them now. |
When will a new version with this change be released? |
@lahirumaramba any ETA on when this fix will be generally available? |
Thanks for the updating the tests! This issue will also be addressed in the credentials migration work #2466 (which I think is the proper way to address this issue). For now, I am okay with us merging this PR as a stopgap |
Thanks @foxrafa ! this change is now included in the |
- Picks up firebase/firebase-admin-node#2553 which unlocks using application default crendentials for local development.
This fixes so much pain, thank you for that. |
Picks up firebase/firebase-admin-node#2553 which unlocks using application default credentials for local development.
This PR is intended to fix a current bug in firebase-admin.
Google Cloud recommends utilizing [Application Default Credentials (ADC)] (https://cloud.google.com/docs/authentication/application-default-credentials), however this feature does not fully work with the firebase-admin-node library when you want to utilize a human account (person@domain.com) instead of a service account.
Some GCP APIs require you to specify the quota project associated to the request, however, the current implementation of the firebase-admin-node library does not provide the project ID when making requests to GCP APIs, even if you set them through
gcloud auth application-default set-quota-project YOUR_PROJECT
, because the code that handles the requests never adds the project ID to the request. And you get errors like this:Therefore, this pull request aims to fix this issue by checking if a
projectId
was provided when calling theinitializeApp()
function and adds it to thex-goog-user-project
header of requests made to GCP APIs, just like the documentation states it should be set.This implementation is similar to how the (working) google-auth-library library handles ADC authentication (src/auth/authclient.ts:270):
This should also solve some problems described in #1377