Add basic auth support for interface #353
Conversation
| return false | ||
| } | ||
|
|
||
| func (a *basicAuth) ServeHTTP(w http.ResponseWriter, req *http.Request, next http.HandlerFunc) { |
There was a problem hiding this comment.
do you mind adding a unit test for it? thanks
jheth
force-pushed
the
basic-auth-support
branch
from
76794ba
to
daa3326
Compare
Codecov Report
@@ Coverage Diff @@
## master #353 +/- ##
==========================================
+ Coverage 81.75% 81.81% +0.05%
==========================================
Files 26 26
Lines 1540 1567 +27
==========================================
+ Hits 1259 1282 +23
- Misses 211 214 +3
- Partials 70 71 +1
Continue to review full report at Codecov.
|
|
PS: You could also do that on your http LB, either nginx or aws ELB, or k8s... it's much easier... |
|
Agreed, that would be the easier path. We have this on heroku though which always has a public reachable domain. *.herokuapp.com |
|
@jheth I see, yeap, Heroku is extremely customizable on the infra-level. I think you could run |
| res.Body = new(bytes.Buffer) | ||
| req, _ := http.NewRequest("GET", "http://localhost:18000/api/v1/flags", nil) | ||
| hh.ServeHTTP(res, req) | ||
| assert.Equal(t, http.StatusOK, res.Code) |
There was a problem hiding this comment.
actually one more thing to test - a happy code path that the basicauth is serving the traffic with username/password passed. this test didn't cover that because /api/v1/flags was one of the default whitelist.
That's why this line wasn't covered yet. https://codecov.io/gh/checkr/flagr/pull/353/diff?src=pr&el=tree#D2-266
There was a problem hiding this comment.
Good call. Thanks for catching that. It's been added.
Add optional basic auth support to keep the interface from being publicly available.
Description
Motivation and Context
How Has This Been Tested?
Types of changes
Checklist: