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

fix: Differentiating explicitly loaded vs default credentials #764

Merged
merged 3 commits into from Jan 21, 2020

Conversation

hiranya911
Copy link
Contributor

@hiranya911 hiranya911 commented Jan 16, 2020

Modules like Firestore and Storage used to have the following bit of code:

} else if (app.options.credential instanceof ApplicationDefaultCredential) {
// Try to use the Google application default credentials.
// If an explicit project ID is not available, let Firestore client discover one from the
// environment. This prevents the users from having to set GOOGLE_CLOUD_PROJECT in GCP runtimes.
return validator.isNonEmptyString(projectId) ? {projectId, firebaseVersion} : {firebaseVersion};
}
throw new FirebaseFirestoreError({
code: 'invalid-credential',
message: 'Failed to initialize Google Cloud Firestore client with the available credentials. ' +
'Must initialize the SDK with a certificate credential or application default credentials ' +
'to use Cloud Firestore API.',
});

This allowed checking if the SDK was initialized with ADC, and if so delegating the credential loading back to the corresponding GCP library. In other cases (e.g. custom credentials), an error was thrown.

But in the v8.9.1 release this code was refactored into this:

} else if (app.options.credential instanceof ComputeEngineCredential) {
// Try to use the Google application default credentials.
// If an explicit project ID is not available, let Firestore client discover one from the
// environment. This prevents the users from having to set GOOGLE_CLOUD_PROJECT in GCP runtimes.
return validator.isNonEmptyString(projectId) ? {projectId, firebaseVersion} : {firebaseVersion};
}
throw new FirebaseFirestoreError({
code: 'invalid-credential',
message: 'Failed to initialize Google Cloud Firestore client with the available credentials. ' +
'Must initialize the SDK with a certificate credential or application default credentials ' +
'to use Cloud Firestore API.',
});

But this is not quite equivalent to the logic we used to have, because:

ApplicationDefaultCredential = 
  MetadataServiceCredential |
  RefreshTokenCredential loaded from well-known path |
  CertCredential loaded from environment variable

The new ComputeEngineCredential only accounts for the MetadataServiceCredential.

In order to cover the remaining 2 cases, we need to differentiate the credentials that were explicitly instantiated by the developer and the credentials that were implicitly loaded by the SDK. This PR implements this capability and updates the rest of the code that depends on this behavior.

Resolves #763

RELEASE NOTE: Fixed a credential loading issue that prevented some Cloud Functions from being deployed via the Firebase CLI.

src/auth/credential.ts Show resolved Hide resolved
Copy link
Member

@lahirumaramba lahirumaramba left a comment

Choose a reason for hiding this comment

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

LGTM

@lahirumaramba lahirumaramba removed their assignment Jan 21, 2020
@hiranya911 hiranya911 merged commit 70c4d0f into master Jan 21, 2020
@hiranya911 hiranya911 deleted the hkj-implicit-creds branch January 21, 2020 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants