-
Notifications
You must be signed in to change notification settings - Fork 5
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
Update Request #16
Comments
But an 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 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! |
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. |
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
The text was updated successfully, but these errors were encountered: