Add basic auth middleware for basic protection of web ui #113
Conversation
| Logger: logger, | ||
| Prefix: pathPrefix, | ||
| BasicAuthUser: os.Getenv("BASIC_AUTH_USER"), | ||
| BasicAuthPassword: os.Getenv("BASIC_AUTH_PASSWORD"), |
There was a problem hiding this comment.
You use "user" here, but "username" below.
Do you want to change all references to user/pass/BASIC_AUTH_USER/BASIC_AUTH_PASS? The symmetry in length appeals to me, and it's a little shorter.
Also, can you keep this list alphabetized?
| - `RIVER_DEBUG=true` - enable debugging logs | ||
| - `CORS_ORIGINS=url1,url2` - define allowed CORS origins | ||
| - `OTEL_ENABLED=true` - enable OTEL integration | ||
| - `BASIC_AUTH_USER=admin`, `BASIC_AUTH_PASSWORD=changeme` - enable basic auth username/password |
There was a problem hiding this comment.
Good idea on some basic documentation for this stuff.
Can you sort this list? Better if there's some sort of scheme in how things like this are organized.
| return | ||
| } | ||
|
|
||
| data, err := base64.StdEncoding.DecodeString(basicAuthHeader[len("Basic "):]) |
There was a problem hiding this comment.
Do you know about Request.BasicAuth? Probably better to use that than implement it yourself.
| } | ||
| next.ServeHTTP(w, r) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Do you think you could take a stab at a basic couple of tests for this middleware? I know the project is hit or miss testing wise, but we're trying to shore that up.
|
@cinemast whether or not you pick up on the fixes to this PR, you may also be interested in this prerelease |
This should fix #111 in a very simple way.
BASIC_AUTH_USERandBASIC_AUTH_PASSWORDwhich both need to be defined, otherwise Basic Auth middleware will not be enabled.