Skip to content
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

Storing passport keys #242

Closed
devjoca opened this issue Jan 20, 2017 · 23 comments
Closed

Storing passport keys #242

devjoca opened this issue Jan 20, 2017 · 23 comments

Comments

@devjoca
Copy link

devjoca commented Jan 20, 2017

Hi,

I'm actually working in Heroku and I figured it out that they use an especial file system, where any deploy run as a fresh copy of the deployed code, so every time I deployed my app, my passport keys get losts. Are there any approach to work with that, maybe some config to store these keys in a external file system like s3?

Regards

@sifex
Copy link

sifex commented Jan 21, 2017

Same issue here. The only solution seems to push the keys live atm.

@mms-gianni
Copy link

I got the same problem with my Heroku installation. My preferred solution would be to have them in a configuration variable. So they can be stored in env vars. Then they are persistent and configurable per environment, even on Heroku.

@sifex
Copy link

sifex commented Jan 22, 2017

@mms-gianni Agreed, this works with there 32kb limit. (Private key usually around 3.5KB and public 800B)

Source: https://devcenter.heroku.com/articles/config-vars#limits

@mms-gianni
Copy link

mms-gianni commented Jan 23, 2017

This is a quick draft:
mms-gianni#1

i did not changed any tests and had no time to test it. Just to get an Idea.

With this solution you would be able to store the key's where ever you want in the filesystem, config direktory or even ENV vars.

@mms-gianni
Copy link

mms-gianni commented Jan 23, 2017

dammit. after i got the code running i figured it out how it works on Heroku:

add "php artisan passport:keys" to your "post-install-cmd"

"post-install-cmd": [
    "Illuminate\\Foundation\\ComposerScripts::postInstall",
    "php artisan optimize",
    "php artisan passport:keys"
],

Let me know if it works for you too.

What do you guy's think: is it a good idea to have the keylocation configurable ?

@sifex
Copy link

sifex commented Jan 25, 2017

@mms-gianni I'll get onto this tonight and give it a try.

@mikew1
Copy link

mikew1 commented Mar 14, 2017

Thanks for the hint about post-install-cmd, but php artisan passport:keys means each of your dynos will have different keys. On Heroku (see https://12factor.net/), that won't scale beyond one dyno. (e.g. with 2 dynos, half of your api requests coming back will fail to authenticate, and so on as you scale).

I believe the solution is, following the 12 factor principles above, to add the key as a (multiline) config var using the Heroku CLI. Wrap the .key in double quotes when you add it on the CLI to make this work.

Then all you need to do is echo out the config var to the necessary file location as a post-install-cmd.

Note that this is defining the build step, which every dyno will follow exactly. So you have the same key across your dynos.

Note, even if you do heroku run bash, and, say, delete the key, it will still work.
Reason? heroku run bash gives you a one off dyno, and anything you do there does not affect your web worker... It takes some thought getting used to this, but I believe it is really worth it for the incredible graceful scaling it gives you. Love Heroku. And thanks to other commenters above for the initial work on this problem.

@mms-gianni
Copy link

Yes you are absulute right. I run currently only one Dyno and did not considered that with my post-install-cmd.

In my draft here mms-gianni#1 keep the key also as a config variable. But it did not made sense to write down the String into a file and read it then on every request from this file. So pass the config variable directly to League\OAuth2\Server\ResourceServer which is capable to read strings or filepointers. see the two cases here : https://github.com/thephpleague/oauth2-server/blob/b8b92e59255ffe586ddd50a3975d7219ca9a8c38/src/CryptKey.php#L33-L49

so i rathere prefered it, to not write down the key in the config resource but keep it there and let it read from the config instead of a ENV variable. (Which would surely be a cooler Heroku-way to do it)

@m-bymike
Copy link

Inspired by the ideas in this issue, I put these lines into my composer.json file:

    "post-install-cmd": [
            "Illuminate\\Foundation\\ComposerScripts::postInstall",
            "@php artisan optimize",
            "echo \"$OAUTH_PUBLIC_KEY\" > storage/oauth-public.key",
            "echo \"$OAUTH_PRIVATE_KEY\" > storage/oauth-private.key"
    ],

This way I can work with environment variables with the existing version. Works for me with heroku...

@themsaid
Copy link
Member

themsaid commented Jul 5, 2017

Closing for lack of activity, hope you got the help you needed :)

@themsaid themsaid closed this as completed Jul 5, 2017
@bkuhl
Copy link

bkuhl commented Nov 29, 2017

I deploy my laravel apps within docker containers where the composer install is a part of the Dockerfile itself. With sensitive information like keys it's insecure to leave them on the container. It's best practice to add them as secrets to your container orchestration tooling and mount them into the container at runtime.

To handle this, I put this segment of code in the AppServiceProvider.boot():

if (env('KEY_PATH', false) !== false) {
    Passport::loadKeysFrom(env('KEY_PATH'));
}

@sgasser
Copy link

sgasser commented Feb 14, 2018

Inspired by @m-bymike I added an if to not override on locale machine:

"post-autoload-dump": [
    "Illuminate\\Foundation\\ComposerScripts::postAutoloadDump",
    "@php artisan package:discover",
    "if [ \"$OAUTH_PUBLIC_KEY\" != \"\" ]; then echo \"$OAUTH_PUBLIC_KEY\" > storage/oauth-public.key; fi",
    "if [ \"$OAUTH_PRIVATE_KEY\" != \"\" ]; then echo \"$OAUTH_PRIVATE_KEY\" > storage/oauth-private.key; fi"
]

@reinink
Copy link
Contributor

reinink commented Apr 4, 2018

I'm trying to solve this issue. Input welcome and appreciated: #683

@reinink
Copy link
Contributor

reinink commented Apr 9, 2018

My PR has been merged in and tagged. You can now set encryption keys via the config (env vars) in Passport v6.0. 🎊

@nestordgs
Copy link

@reinink Thanks a lot, just upgrade laravel and laravel passport to 6.0 and it work

@pdsorensen
Copy link

@reinink this saved me quite a lot of work, since i had to take rotation of environment variables into account on aws. Made it much easier, thanks again.

@sethphillips
Copy link

sethphillips commented Jul 24, 2018

it looks like the vlucas/phpdotenv 2.5.0 update broke the ability to do this. Sorry I dont have more information at the time but hopefully this saves someone else some troubleshooting time. I locked the version to 2.4 and was able to use keys in my .env again.

@m-bymike
Copy link

The issue with phpdotenv is reported here: vlucas/phpdotenv#279
It only affects private/public keys placed in a .env file. If your key is an actual environment variable, it will work.
Workaround: Place the keys into appropriate files (public.key/private.key) and set the filename/path to these files in .env.

@sethphillips
Copy link

sethphillips commented Jul 25, 2018 via email

@m-bymike
Copy link

@sethphillips on a production & load balanced system you should be able to add these keys as actual environment variables w/o the need of having a .env file. This still works. I use .env only for my local development machine...

@sethphillips
Copy link

sethphillips commented Jul 25, 2018 via email

@m-bymike
Copy link

@sethphillips check out this laracast: https://laracasts.com/series/server-management-with-forge/episodes/5

(we're running off topic here, sorry for that...)

@sethphillips
Copy link

sethphillips commented Jul 26, 2018 via email

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

No branches or pull requests