redwoodjs / cookbook-third-party-apis

Codebase for the Third Party APIs cookbook example app
MIT License
10 stars 5 forks source link

Update Request #16

Open B-Haller opened 3 years ago

B-Haller commented 3 years ago

Hi RedwoodJS Devs,

In recently following this cookbook, I noticed that you have exposed a key in your client/server calls to Openweather API. I understand that this key is invalid; but I believe, for the purpose of the cookbook, new developers would be best served with code that depicts best practices. I suggest an update to the code to move the key into the .env file so as to prevent accidental uploads by developers of these secrets to their own online repos.

Additionally, you have mixed use of protocols on your calls--http and https-- that could be updated.

I've made these changes locally and am happy to open a PR if you find these comments helpful.

Thanks, Brandon

thedavidprice commented 3 years ago

Thanks @B-Haller

Looping in @cannikin

cannikin commented 3 years ago

But an .env file shouldn't be checked in either—best practice paradox! 😉 Or you mean that we recommend to the developer that they do that themselves?

I was just trying to make the code and setup as simple as possible, and not go down the road of explaining ENV vars, and the .env file, version control best practices, etc.

How would you feel about a sidebar/callout that mentions this, with a note that "we didn't do any of that in this code for simplicity"?

We should definitely be consistent with HTTPS though. The Openweather docs show getting the image from an HTTP endpoint, which is why I used that scheme: https://openweathermap.org/weather-conditions However it appears they do respond on HTTPS now, so we should be safe to switch over!

B-Haller commented 3 years ago

As you rightly point out, .env files shouldn't be checked in either. What I was attempting to point out is that there should, at the very least, be a callout. This doesn't require going down a rabbit hole on .env, or how they work. I would argue how to treat API keys in RedwoodJS is an integral part of a cookbook on integrating with a third party APIs.

Again, just a friendly suggestion as dev dropping by checking out the framework.