Project Id detection fails when manually providing "credentials" option

[matthieusieben]

Environment details

• OS: Darwin Kernel Version 19.5.0
• Node.js version: v14.4.0
• npm version: 6.14.4
• google-auth-library version: 6.0.1

Steps to reproduce

import { Datastore } from '@google-cloud/datastore'

const datastore = new Datastore({
  credentials: JSON.parse(process.env.GOOGLE_APPLICATION_KEY) // content of the JSON file
})

datastore.get(datastore.key(["kind", "name"])).catch(err => {
  console.error(err) // Unable to detect a Project Id in the current environment.
})

Current workaround:

const datastore = new Datastore({
  keyFilename: 'foo', // Any truthy value here
  credentials: { ... },
})

[sofisl]

@matthieusieben, Thank you for submitting this pull request! This is actually a bug that we're tracking across some libraries. @bcoe, do we know how we want to tackle this bug holistically? (I think its also related to this one)

[bcoe]

@sofisl assuming that the project ID is baked into the credentials file @matthieusieben is providing (@matthieusieben can you confirm this?), I think we should make an effort to reproduce the bug in this repository and see if we can't figure out how to pull in the project ID appropriately.

Once we fix it in this library, it will potentially address the issue in upstream libraries that depend on it (like bigquery.)

[matthieusieben]

This is what the credentials look like (real values obfuscated):

{
  "type": "service_account",
  "project_id": "my-app-name",
  "private_key_id": "dab324234...",
  "private_key": "-----BEGIN PRIVATE KEY-----\nXXX==\n-----END PRIVATE KEY-----\n",
  "client_email": "[email protected]",
  "client_id": "765434567876543456765456",
  "auth_uri": "https://accounts.google.com/o/oauth2/auth",
  "token_uri": "https://oauth2.googleapis.com/token",
  "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs",
  "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/app-access%40my-app-name.iam.gserviceaccount.com"
}

[sofisl]

Hey @matthieusieben, thanks for creating this issue!

I'm looking into implementing this feature request, but I was wondering if you could clarify your use case for me a bit.

I'm curious why you're setting your credentials file to credentials, instead of keyFileName or GOOGLE_APPLICATION_CREDENTIALS. The issue is that the credentials property expects a response of type Credential Body, which is only looking for a client_email and private_key. It does not expect a project ID, so it doesn't look for it. However, there are other places that are looking for the projectId (namely, keyfileName and the CREDENTIALS env variable).

Moreover, if you wanted to you could also do something like this:

const datastore = new Datastore({
    credentials: {
        private_key: 'abc123'
        client_email: '[email protected]',
    }
    projectId: 'projectId',
}

So I guess my question here is, is there a reason you need to put the JSON credentials in the credentials property, rather than in those other places?

Thanks in advance!

Sofia

[sofisl]

Actually, I want to clarify something: it seems like it is a bug that we can't find the projectId from keyFileName (that I linked earlier. But I just want to understand if there's a distinct use case why you would want to get projectId from credentials as well.

[matthieusieben]

My use case is pretty simple: I instantiate the Datastore from inside a docker container, which is run using an environment variable that contains the whole credentials as a json string. Of course I could write that env variable in a key file before instantiating but that seems just wrong. I could also use another environment variable for the project_id, but since the env variable (which is juste a copy-paste of the key file) already contains it, and since credentials should conceptually contain exactly the same information as keyFileName, that also feels weird.

[matthieusieben]

Consider the following code:

const credentials = JSON.parse(process.env.GOOGLE_APPLICATION_KEY)
const datastore = new Datastore({
  credentials,
  projectId: credentials['project_id']
})

It feels weird because

1. It part of it is redundant
2. It requires to know the internals of credentials, which is not the case when using a key file.
3. It makes the implementation of google-auth-library-nodejs work differently when reading the credentials from the file, from when credentials are provided as an object.

[sofisl]

I see - thanks for clarifying!

So, I'm still a bit confused about why you want to use the credentials property instead of the keyFileName property, even if your credentials are stored in a string.

Would this kind of code solve your use case? (assuming GOOGLE_CREDENTIALS is a string of your JSON credentials)?

const { Datastore } = require('@google-cloud/datastore');

const datastore = new Datastore({
    keyFileName: JSON.parse(process.env.GOOGLE_CREDENTIALS)
})

datastore.get(datastore.key(["kind", "name"])).catch(err => {
  console.error(err) 
})

This code currently doesn't work, due to this bug. But, I feel like this is a more appropriate solution, since keyFileName at least expects a projectId, but (currently) is not actually retrieving it. credentials does not expect it, so I'm not sure why we want to add functionality where it looks for a projectId.

Let me know if this would work for you, and I can get started on working on the bug I linked above (I will re-file it under this library, since it's an auth issue).

[sofisl]

Hi @matthieusieben,

After lots of internal discussion, I think we've come up with a plan of attack for this bug:

1. There needs to be better and more consistent documentation as to how authentication gets done, and in what order of operations. I think one of the big reasons why we were getting so mixed up is because it is not clear what is by design or what is a bug.
2. There is a real bug with authenticating with a keyFile! It should get a projectId, since it is provided (and expected) in the file. But, currently, it is not. I will be creating a bug and assigning to this repo.
3. For the time being (until we can dig into Step 1 more), we think that your use case is actually identified through the clientOptions property, which is looking for a JWT with all the properties you've listed. If it were in an actual path to a keyFile, then it would be keyFileName.

Given that we'll need to explore Step 1, I am going to close your corresponding PR for the time being as we haven't yet cleared up what is intentional and what is by design. But, thanks to your comment and PR, we've uncovered some inconsistencies/bugs. So thank you!