-
Notifications
You must be signed in to change notification settings - Fork 7
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
Resource credentials should be sourced from environment variables #75
Comments
This seems like it would be helpful in certain situations, but I'm a little bit worried about how to actually implement this. What would these environment variables be named? How would they be created by layman developers? |
Responding to the comment left in #78:
It's not really JSON vs. ENV, but rather allowing values in a JSON config to be sourced from ENV. I see two possible implementations: interpolation or sourcing the ENV as an object and merging it with the JSON after it's parsed to provide it's "context" for the JSON. For example: {
"resources": {
"doges": {
"url": "https://example.com/api/v2/resources/4521zb/such-doges",
"headers": {
"key": DOGE_KEY
},
"auth": DOGE_AUTH
}
}
} To me, this implementation seems most natural within Solidus. The templating route would need to introduce a different syntax, like
The developer should control the names. In either implementation referenced here there's no real connection to the keys in the config JSON and there really shouldn't need to be. There are many ways to specify them if you are simply sourcing the ENV. It's as simple as running You could take a note from Foreman and define your own file as well, and include it in the template .gitignore. This dodges the potential for conflicts (That often necessitates namespacing the variables), providing a namespace that's just for the app, as would exist in the deployment environment (aside from any platform vars, which are almost always namespaced). I'd call this file You could add some CLI commands to maintain that file, similar to the various PaaS CLI tools: Heroku, Modulus and Nodejitsu. Modulus even provides a GUI. I'd argue that values for typically named header keys (containing "key", "password", etc) and the auth string be required to come from environment variables to guide developers into this practice. It's pretty sensible if communicated properly. A warning in the log at the very least would be a good idea. |
#73 and #74 allow the separation of credentials from resource endpoint URLs, a good next step in this direction would be to allow for the separation of credentials from the code by way of sourcing them from environment variables. This is an established best practice pattern well-reasoned in The Twelve-Factor App . It's also easy to setup with the recommended deployment targets (Modulus, Heroku, and Nodejitsu).
Aside from improving general security I think this adds a lot of value in other areas, allowing for any Solidus site to be made a public repo (for others to learn from) without compromising credentials in the process. It also allows for resource configurations to be templated.
The text was updated successfully, but these errors were encountered: