-
Notifications
You must be signed in to change notification settings - Fork 426
Conversation
Improvements and fixed tests. Thank you, @donbobka!
Thank you @donbobka for fixing the tests and doing some general improvements! |
Just wanted to follow up and check if there's anything else blocking this from being merged? |
I can't get this to work. I've forked it and used it. I'm presented with basic-auth username and password but it doesn't seem to recognise the username and password. I assumed if I set config vars within heroku for In the logs I see: |
@paulmsmith, BASIC_AUTH_PASSWORD must be a hash of your password. Use command |
@donbobka Oh of course! Thanks for the mighty swift reply! |
@donbobka Actually, still not working. Where am I supposed to stored the hashed password? In the config vars? (Long day! :)) Edit. I think I was being dumb... will post back...... |
@donbobka I was being dumb. I paste the username and hashed that. Thank you for the work. Hopefully will make it in to the final repo. |
I've been using this for almost a month now in our 2 pre-production environments with great success. https://github.com/Aculeus/create-react-app-buildpack/blob/master/.buildpacks |
Hi @hone, what do you think about these changes? ;) |
Hey @randallagordon I'm trying to use this via your branch, but for some reason even when adding I have a feeling the buildpack isn't actually taking effect, even though I've run:
(I've also tried...)
Anyone know why? |
@ianstormtaylor it should work with or without the |
Is there anything holding this PR to be merged ? Thx |
Happy Monday! I'd love to see this feature. Could today be the day? /bump |
TGIF! I'd love to see this feature as well. Could today be the day? /bumpagain |
Ping! Any plans on when this will be merged? We could use it to add basic auth to our review apps. /bumpagain |
Ping |
Why are we not merging this PR? |
I merged it manually on my repo, https://github.com/viperfx07/heroku-buildpack-static, just in case anyone needs it. Cheers |
Hi @viperfx07, I'm using your fork, and I'm having an issue. I'm getting a 401 error when I request my backend through a proxy. Is anyone having the same issue? Or anyone knows how I can fix it? |
Why has this PR been open since 2016 with no justification for not being merged? Can't be inspiring for contributors... 😬 |
Ping on this? Would be great to have this! |
@hone This is well tested by the community. It would be great to get it in. |
Related: #111 |
@Dhaulagiri @hone Are there any updates on this? |
Please move this forward, there're so many forks in this repo because of this small feature! Listen to your users /cc @Dhaulagiri @hone |
thx @Dhaulagiri 🎉 |
Because heroku/heroku-buildpack-static#45 looks for `/app/.htpasswd` when: - `"basic_auth": true` (`static.json`), and - `BASIC_AUTH_USERNAME` and `BASIC_AUTH_PASSWORD` are not set
Is there any way to enable / disable basic auth based on an env var? |
@cancan101 I'd recommend starting a new issue for this support/feature request <3 |
* upstream/master: Basic Auth Configuration (heroku#45) # Conflicts: # scripts/config/templates/nginx.conf.erb
* Basic Basic Auth * Allow username and password to be passed in via env vars * Add test spec for basic auth returning 401 * Update README with Basic Auth config info * Add .htpasswd to test fixture * Set env vars in test * Request the file that actually exists, maybe? * Is context the key? * Perhaps app.run needs to come to the party too? * Move `auth_basic` from `location` to `server` * Set `basic_auth` as true if env `basic_auth_username` exists * Fix htpasswd generation when basic_auth is false * Append env password instead truncating * Fix typo * Fix config example in README
What's this PR do?
Adds the ability to configure Basic Authentication.
Will generate
.htpasswd
if environment variablesBASIC_AUTH_USERNAME
andBASIC_AUTH_PASSWORD
are present; otherwise use an.htpasswd
file present in theapp
directory.To configure, in
static.json
:Background
Closes #43, where Basic Auth support was requested by @josephabrahams.