Add cmd flag to skip http paths from authentication #1637
Conversation
arajkumar
requested review from
antekresic,
Harkishen-Singh and
a team
as code owners
arajkumar
force-pushed
the
skip-auth
branch
2 times, most recently
from
328090a to
bceb05e
Compare
pkg/api/router.go
Outdated
| @@ -163,8 +164,22 @@ func authHandler(cfg *Config, handler http.Handler) http.Handler { | |||
| return handler | |||
| } | |||
|
|
|||
| isIgnoredPath := func(r *http.Request) bool { | |||
There was a problem hiding this comment.
Maybe we can add this as a method to Auth?
There was a problem hiding this comment.
I agree, this is getting into too much auth logic for the router file. It should probably go into the common.go file, have the Auth method encapsulate all this internal logic about authorization methods and ignored paths.
There was a problem hiding this comment.
Might even make sense to break it up into its own auth file since its kinda overgrown the common file.
pkg/api/router.go
Outdated
| @@ -163,8 +164,22 @@ func authHandler(cfg *Config, handler http.Handler) http.Handler { | |||
| return handler | |||
| } | |||
|
|
|||
| isIgnoredPath := func(r *http.Request) bool { | |||
There was a problem hiding this comment.
I agree, this is getting into too much auth logic for the router file. It should probably go into the common.go file, have the Auth method encapsulate all this internal logic about authorization methods and ignored paths.
pkg/api/router.go
Outdated
| @@ -163,8 +164,22 @@ func authHandler(cfg *Config, handler http.Handler) http.Handler { | |||
| return handler | |||
| } | |||
|
|
|||
| isIgnoredPath := func(r *http.Request) bool { | |||
There was a problem hiding this comment.
Might even make sense to break it up into its own auth file since its kinda overgrown the common file.
arajkumar
force-pushed
the
skip-auth
branch
2 times, most recently
from
9f48f88 to
a28b24f
Compare
|
I will pass this review to other reviewers. |
| @@ -29,7 +28,7 @@ import ( | |||
| "github.com/timescale/promscale/pkg/telemetry" | |||
| ) | |||
|
|
|||
| func GenerateRouter(apiConf *Config, promqlConf *query.Config, client *pgclient.Client, store *jaegerStore.Store, reload func() error) (*mux.Router, error) { | |||
| func GenerateRouter(apiConf *Config, promqlConf *query.Config, client *pgclient.Client, store *jaegerStore.Store, authWrapper mux.MiddlewareFunc, reload func() error) (*mux.Router, error) { | |||
There was a problem hiding this comment.
This is obviously a smell, having so many input parameters on a function. The function itself is too big as well.
Lets put a TODO on this to refactor it in the future so when somebody goes to add another parameter, they can see it and not do it 😃
arajkumar
force-pushed
the
skip-auth
branch
from
a28b24f to
05b5269
Compare
This commit adds a flag `--web.auth.ignore-path` which takes a http path and passed path would be ignored from authentication. The flag shall be repeated to pass array of ignored paths. e.g. ``` ./dist/promscale --web.auth.ignore-path=/heathz --web.auth.ignore-path=/api/query_range ``` Fixes timescale#1636 Signed-off-by: Arunprasad Rajkumar <[email protected]>
Signed-off-by: Arunprasad Rajkumar <[email protected]>
arajkumar
force-pushed
the
skip-auth
branch
from
05b5269 to
c9981cb
Compare
Signed-off-by: Arunprasad Rajkumar [email protected]
Description
This PR adds a flag named
--web.auth.ignore-pathwhich takes a http path and passed path would be ignored from authentication. The flag shall be repeated to pass array of ignored paths.e.g.
Fixes #1636
Code changes are inspired from https://github.com/brancz/kube-rbac-proxy/blob/fc1ca4f969941340a8adb66932bd64dc5773d37a/main.go#L298-L321
Merge requirements
Please take into account the following non-code changes that you may need to make with your PR: