google-auth-library-php icon indicating copy to clipboard operation
google-auth-library-php copied to clipboard

feat: add support for $_ENV and $_SERVER in CredentialsLoader

Open jannes-io opened this issue 9 months ago • 2 comments

Many modern frameworks use dotenv or similar methods of loading environment variables into the superglobals, but do not use putenv in doing so. Which makes it harder to use this library, having to use factories to dynamically build client libs and setting the authentication that way instead of using this default method of authenticating.

Partially fixes #432 (only applies to the credentials loader)

Relates to: symfony/symfony#31062

Backwards compatibility is provided by always first checking getenv. If you want me to extract this function and replace all occurrences of getenv with it, then that's fine too. Just let me know.

jannes-io avatar Mar 05 '25 09:03 jannes-io

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.

google-cla[bot] avatar Mar 05 '25 09:03 google-cla[bot]

CLA signed.

jannes-io avatar Mar 05 '25 09:03 jannes-io

Hello @jannes-io! Thanks for your contributions!

It looks like I don't have permissions to update your branch, so could you update your branch with the changes from main in this repo? That should also trigger the tests, which need to run in order for me to merge this.

Thanks again!

bshaffer avatar Apr 11 '25 19:04 bshaffer

Hi @bshaffer ,

The "Allow edits by maintainers" is enabled, so you should be able to add commits. But it seems like there are no updates to main? The branch appears to be up to date.

image

Let me know if I can help.

jannes-io avatar Apr 11 '25 19:04 jannes-io

@jannes-io You're right, my mistake! Okay, I created an empty commit to trigger the actions.

bshaffer avatar Apr 11 '25 19:04 bshaffer

@jannes-io hmm, now I'm thinking, is there any real reason we want to load from _SERVER? It seems to me that _ENV would be sufficient and preferable.

bshaffer avatar Apr 15 '25 21:04 bshaffer

@jannes-io hmm, now I'm thinking, is there any real reason we want to load from _SERVER? It seems to me that _ENV would be sufficient and preferable.

Honestly? I don't have a reason for it besides symfony/dotenv placing all .env variables in both _SERVER and _ENV.

So my reasoning is that they probably had a good and well thought out reason for doing both, so 3rd party libs that read things from the environment should probably be checking both for existence of env vars? 😆

jannes-io avatar Apr 15 '25 21:04 jannes-io

@jannes-io okay that's what I thought! Thanks for your quick reply.

So it makes sense to WRITE to both (since SERVER also contains ENV), but I can't think of a reason to READ from both (symfony is writing, and we're reading). I think I'll submit a PR to remove it (I get that it's a little pedantic, but this is an auth library, so it's good to keep thing simple and by the book)

bshaffer avatar Apr 15 '25 21:04 bshaffer

@jannes-io released in v1.47.0. Thanks for your contribution!!

bshaffer avatar Apr 15 '25 22:04 bshaffer