Skip to content

Conversation

@jeromegamez
Copy link
Contributor

@jeromegamez jeromegamez commented Oct 12, 2025

googleapis/google-cloud-php#8617 marked keyFile and keyFilePath options as deprecated.

In kreait/firebase-php, I currently read GOOGLE_APPLICATION_CREDENTIALS and check if it includes a inline JSON string. If it does, I decode it and use it to set the keyFile option.

This allows projects using the SDK to set the GOOGLE_APPLICATION_CREDENTIALS environment variable without having to provide a "physical" file.

With this I create, for example, a FirestoreClient with the keyFile option.

If I remove the option and rely on the credentialFetcher option, as documented, it doesn't seem to be used. FirestoreClient uses ClientTrait::getKeyFile(), and when it doesn't find the deprecated options, it falls back to CredentialsLoader::fromEnv(), which requires the GOOGLE_APPLICATION_CREDENTIALS to be a string.

While this change does solve my particular issue, the correct way might be to have ClientTrait::configureAuthentication() check for the credentialFetcher option before checking for the keyFile option…

However, I wonder if it might make sense to allow inline JSON in the GOOGLE_APPLICATION_CREDENTIALS in general.

:octocat:

@jeromegamez jeromegamez requested a review from a team as a code owner October 12, 2025 08:42
@jeromegamez
Copy link
Contributor Author

Here is the PR for the configureAuthentication() method: googleapis/google-cloud-php#8657

@bshaffer
Copy link
Contributor

I believe this is no longer necessary because we merged googleapis/google-cloud-php#8657, is that right?

... although we still haven't released it yet (working on it!))

@jeromegamez
Copy link
Contributor Author

I think it might still be useful - I do inject other credentials as inline JSON to AWS ECS Containers and GOOGLE_APPLICATION_CREDENTIALS specifically in GitHub Actions (although I could echo them into a file and point the variable with a run step 🤔)

I don't know if this has security implications, though.

@bshaffer
Copy link
Contributor

@jeromegamez The reason we deprecated the ability to load from any file is due to a security implication. It's included in the deprecation message here:

     * @deprecated This method is being deprecated because of a potential security risk.
     *
     * This method does not validate the credential configuration. The security
     * risk occurs when a credential configuration is accepted from a source
     * that is not under your control and used without validation on your side.
     *
     * If you know that you will be loading credential configurations of a
     * specific type, it is recommended to use a credential-type-specific
     * method.
     * This will ensure that an unexpected credential type with potential for
     * malicious intent is not loaded unintentionally. You might still have to do
     * validation for certain credential types. Please follow the recommendation
     * for that method. For example, if you want to load only service accounts,
     * you can create the {@see ServiceAccountCredentials} explicitly:
     *
     * ```
     * use Google\Auth\Credentials\ServiceAccountCredentials;
     * $creds = new ServiceAccountCredentials($scopes, $json);
     * ```
     *
     * If you are loading your credential configuration from an untrusted source and have
     * not mitigated the risks (e.g. by validating the configuration yourself), make
     * these changes as soon as possible to prevent security risks to your environment.
     *
     * Regardless of the method used, it is always your responsibility to validate
     * configurations received from external sources.
     *
     * @see https://cloud.google.com/docs/authentication/external/externally-sourced-credentials

Loading a JSON string from the environment variable may not be exactly the same thing, but I do not want to include non-standard capabilities in a credentials-loading function. This is a pretty precise specification, and while I can't say that loading a raw JSON string from an environment variable is definitively less secure than loading an arbitrary file from that same environment variable, I don't want to include new functionality unless it's been approved by a security council.

I am closing this for now as WAI. You can save your credentials to a temp file and set the environment variable to that if you need to. But please take a look at the above security implications before doing that, as it may be recommended for you to simply create the credentials explicitly. And please provide feedback if this option doesn't work for you and we can look into that as well.

Thanks again for your contribution!

@bshaffer bshaffer closed this Oct 23, 2025
@jeromegamez
Copy link
Contributor Author

I absolutely understand - the other PR is what I actually wanted/need, so I'm totally fine with this one not being done. Thanks for your time, and all the work you're doing here!

@jeromegamez
Copy link
Contributor Author

jeromegamez commented Oct 23, 2025

PS: I'm doing the validation in the unofficial Firebase SDK. You should check it out and start a Sponsorship program at Google 😇

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.

2 participants