NETOBSERV-870 implement TokenReview #283
Conversation
pkg/kubernetes/auth/check_auth.go
Outdated
| type ValidBearerTokenChecker struct { | ||
| Checker | ||
| apiProvider client.APIProvider | ||
| } | ||
|
|
||
| func (c *ValidBearerTokenChecker) CheckAuth(ctx context.Context, header http.Header) error { | ||
| hlog.Debug("Checking authenticated user") | ||
| token, err := getUserToken(header) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| hlog.Debug("Checking auth: token found") | ||
| if err = runTokenReview(ctx, c.apiProvider, token, []tokenReviewPredicate{mustBeAuthenticated}); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| hlog.Debug("Checking auth: passed") | ||
| return nil | ||
| } | ||
|
|
||
| type AdminBearerTokenChecker struct { | ||
| Checker | ||
| apiProvider client.APIProvider | ||
| } | ||
|
|
||
| func (c *AdminBearerTokenChecker) CheckAuth(ctx context.Context, header http.Header) error { | ||
| hlog.Debug("Checking authenticated user") | ||
| token, err := getUserToken(header) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| hlog.Debug("Checking auth: token found") | ||
| if err = runTokenReview(ctx, c.apiProvider, token, []tokenReviewPredicate{mustBeAuthenticated, mustBeClusterAdmin}); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| hlog.Debug("Checking auth: passed") | ||
| return nil | ||
| } |
There was a problem hiding this comment.
I guess this code was duplicated for future improvments but at the end I don't see the value to do it.
[]tokenReviewPredicate could be passed as a simple argument with the apiProvider in NewChecker function. Is it going to be other differences than that ?
There was a problem hiding this comment.
I started with a slightly different implementation between the two checkers but after one or two iterations it ended up being almost the same, modulo predicates .. so yeah we can merge them into a single impl; if they diverge again later then we would split them back
|
/lgtm |
|
I changed it a little bit, to add a new mode (which is the default and the one used from operator) : "auto" This is necessary for the multi-tenancy work |
|
cc @jpinsonneau ^ |
Requires operator PR to grant permission for TokenReviews - 3 auth modes: check for cluster-admin, check for any user, no check (insecure; only for debugging/dev mode) - add tests - fix broken dev mode
Auto mode acts either as admin or authenticated depending on the loki authtoken mode (forward => authenticated) Also improve loki errors hanfling
|
New changes are detected. LGTM label has been removed. |
|
/ok-to-test |
openshift-ci
bot
added
the
ok-to-test
|
New image: ["quay.io/netobserv/network-observability-console-plugin:11aa4b5"]. It will expire after two weeks. |
|
/label qe-approved |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jotak The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Requires operator PR to grant permission for TokenReviews: netobserv/network-observability-operator#263