davidjameshowell / vaultwarden_heroku

Vaultwarden (Bitwarden Rust implementation) self hosted in Heroku for Free!
96 stars 176 forks source link

Replace sed calls with env variable #4

Closed bryanjhv closed 3 years ago

bryanjhv commented 3 years ago

I was checking out your project, and it's awesome! Thanks in advance for that, Heroku helps a lot hosting it.

Now, I've got a suggestion from the official repository after suggesting part of your code:

https://github.com/davidjameshowell/bitwarden_rs_heroku/blob/819e8829a0ffb3143405fa2258823afcb2bddc16/bitwarden_rs_heroku.sh#L67-L71

If you check out https://github.com/dani-garcia/bitwarden_rs/issues/861#issuecomment-776225183, it says it'd be easier to just heroku config:set _ENABLE_DUO=true, given configuration keys are mapped to env variables.

What do you think? Is it something that would make your script simpler?

davidjameshowell commented 3 years ago

I can definitely revisit this to see if it's changed. When I had last attempted this, due to my workflow, it essentially locked my user account out since Duo was not enabled globally. I take bi-daily snapshots of my production Bitwarden and replica it to my Heroku instance via MySQL dumps, so my database and users (including Duo) are already configured. However as a result during the time I had been building it, it would essentially lock out my user from using Duo (and thus logging in since it's my only 2FA).

But I think it's definitely worth taking another look and seeing if that env flag would resolve the issue/has been fixed properly!

bryanjhv commented 3 years ago

Thanks in advance for taking a look at this!

From what I understand (not a Rust expert), that env variable would set explicitly a value for that _enable_duo setting. Downside is that if you forget to set that variable it would default to false. But given Heroku is driven by env variables, that shouldn't be a problem.

davidjameshowell commented 3 years ago

It looks good to me! #5 should resolve this and requires less hackiness!