From 7bd53d5cdc6c77330afef9af4754bf77a18f5ba4 Mon Sep 17 00:00:00 2001 From: caryxychen <101852328+caryxychen@users.noreply.github.com> Date: Mon, 9 May 2022 09:43:49 +0800 Subject: [PATCH] fix(auth): authz bug fix (#1916) Co-authored-by: caryxychen --- pkg/apiserver/filter/authentication.go | 10 ++++++ pkg/auth/authorization/local/authorizer.go | 36 ++++++++++++++++++++-- pkg/auth/filter/filter.go | 33 +++++++++++++++----- 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/pkg/apiserver/filter/authentication.go b/pkg/apiserver/filter/authentication.go index d3865ed68..7b23ab5ff 100644 --- a/pkg/apiserver/filter/authentication.go +++ b/pkg/apiserver/filter/authentication.go @@ -189,6 +189,16 @@ func GroupWithProject(project string) string { return fmt.Sprintf("project:%s", project) } +func FindValueFromGroups(groups []string, key string) (bool, string) { + prefix := fmt.Sprintf("%s:", key) + for _, group := range groups { + if strings.HasPrefix(group, prefix) { + return true, strings.TrimPrefix(group, prefix) + } + } + return false, "" +} + func GetValueFromGroups(groups []string, key string) string { prefix := fmt.Sprintf("%s:", key) for _, group := range groups { diff --git a/pkg/auth/authorization/local/authorizer.go b/pkg/auth/authorization/local/authorizer.go index 002c1a88a..031cda3ac 100644 --- a/pkg/auth/authorization/local/authorizer.go +++ b/pkg/auth/authorization/local/authorizer.go @@ -22,6 +22,7 @@ import ( "context" "encoding/json" "fmt" + "k8s.io/apimachinery/pkg/util/sets" "strings" genericfilter "tkestack.io/tke/pkg/apiserver/filter" @@ -42,6 +43,10 @@ var ( debugKey = "debug" ) +// the kube verb associated with API requests (this includes get, list, watch, create, update, patch, delete, deletecollection, and proxy), +// or the lowercased HTTP verb associated with non-API requests (this includes get, put, post, patch, and delete) +var verbMap = sets.NewString("get", "list", "watch", "create", "update", "patch", "delete", "proxy", "put", "post", "deletecollection") + // Authorizer implement the authorize interface that use local repository to // authorize the subject access review. type Authorizer struct { @@ -75,6 +80,8 @@ func (a *Authorizer) Authorize(ctx context.Context, attr authorizer.Attributes) if tenantIDs, ok := extra[genericoidc.TenantIDKey]; ok { if len(tenantIDs) > 0 { tenantID = tenantIDs[0] + } else { + tenantID = "default" } } @@ -84,8 +91,12 @@ func (a *Authorizer) Authorize(ctx context.Context, attr authorizer.Attributes) } } } + find := false if tenantID == "" { - tenantID = genericfilter.GetValueFromGroups(attr.GetUser().GetGroups(), "tenant") + find, tenantID = genericfilter.FindValueFromGroups(attr.GetUser().GetGroups(), "tenant") + if find && tenantID == "" { + tenantID = "default" + } } projectID = genericfilter.GetValueFromGroups(attr.GetUser().GetGroups(), "project") log.Debug("Authorize", log.String("subject", subject), log.String("action", action), @@ -139,6 +150,28 @@ func (a *Authorizer) Authorize(ctx context.Context, attr authorizer.Attributes) } } + if tenantID != "" && verbMap.Has(action) { + record := &authorizer.AttributesRecord{ + User: attr.GetUser(), + Verb: attr.GetVerb(), + Namespace: attr.GetNamespace(), + APIGroup: attr.GetAPIGroup(), + Resource: attr.GetResource(), + Subresource: attr.GetSubresource(), + Name: attr.GetName(), + ResourceRequest: attr.IsResourceRequest(), + Path: attr.GetPath(), + } + tkeAttributes := filter.ConvertTKEAttributes(ctx, record) + attrStr, _ := json.Marshal(attr) + tkeAttributesStr, _ := json.Marshal(tkeAttributes) + log.Debugf("Attribute '%s' converted to TKEAttributes '%s'", string(attrStr), string(tkeAttributesStr)) + attr = tkeAttributes + } + return a.casbinDecision(attr, tenantID, subject, projectID, attr.GetResource(), attr.GetVerb(), debug) +} + +func (a *Authorizer) casbinDecision(attr authorizer.Attributes, tenantID, subject, projectID, resource, action string, debug bool) (authorized authorizer.Decision, reason string, err error) { allow, err := a.enforcer.Enforce(authutil.UserKey(tenantID, subject), projectID, resource, action) if err != nil { log.Error("Casbin enforcer failed", log.Any("att", attr), log.String("projectID", projectID), log.String("subj", subject), log.String("act", action), log.String("res", resource), log.Err(err)) @@ -156,6 +189,5 @@ func (a *Authorizer) Authorize(ctx context.Context, attr authorizer.Attributes) return authorizer.DecisionDeny, fmt.Sprintf("permission for %s on %s not verify", action, resource), nil } log.Debug("Casbin enforcer: ", log.Any("att", attr), log.String("projectID", projectID), log.String("subj", subject), log.String("act", action), log.String("res", resource), log.String("allow", "true")) - return authorizer.DecisionAllow, reason, nil } diff --git a/pkg/auth/filter/filter.go b/pkg/auth/filter/filter.go index 2be5469e2..3b81f9caa 100644 --- a/pkg/auth/filter/filter.go +++ b/pkg/auth/filter/filter.go @@ -24,6 +24,7 @@ import ( "net/http/httputil" "strconv" "strings" + genericoidc "tkestack.io/tke/pkg/apiserver/authentication/authenticator/oidc" "unicode" "github.com/go-openapi/inflect" @@ -37,6 +38,7 @@ import ( "k8s.io/apiserver/pkg/endpoints/handlers/responsewriters" "k8s.io/apiserver/pkg/endpoints/request" genericapiserver "k8s.io/apiserver/pkg/server" + genericfilter "tkestack.io/tke/pkg/apiserver/filter" "tkestack.io/tke/api/business" "tkestack.io/tke/api/registry" @@ -133,15 +135,32 @@ func WithTKEAuthorization(handler http.Handler, a authorizer.Authorizer, s runti reason string ) - // firstly check if resource is unprotected - authorized = UnprotectedAuthorized(attributes) - if authorized != authorizer.DecisionAllow { - authorized, reason, err = a.Authorize(ctx, attributes) + tenantID := "" + extra := attributes.GetUser().GetExtra() + if len(extra) > 0 { + if tenantIDs, ok := extra[genericoidc.TenantIDKey]; ok { + if len(tenantIDs) > 0 { + tenantID = tenantIDs[0] + } else { + tenantID = "default" + } + } + } + find := false + if tenantID == "" { + find, tenantID = genericfilter.FindValueFromGroups(attributes.GetUser().GetGroups(), "tenant") + if find && tenantID == "" { + tenantID = "default" + } } - // secondly check k8s resource authz result + // firstly check if resource is unprotected + authorized = UnprotectedAuthorized(attributes) if authorized != authorizer.DecisionAllow { - attributes = ConvertTKEAttributes(ctx, attributes) + if tenantID != "" { + log.Debugf("TKEStack user '%v'", attributes.GetUser()) + attributes = ConvertTKEAttributes(ctx, attributes) + } authorized, reason, err = a.Authorize(ctx, attributes) } else { setK8sDecision(req, true) @@ -194,7 +213,7 @@ func UnprotectedAuthorized(attributes authorizer.Attributes) authorizer.Decision } // specialSubResources contains resources which get verb use get instead of list -var specialSubResources = sets.NewString("status", "log", "finalize") +var specialSubResources = sets.NewString("status", "log", "finalize", "proxy") // ConvertTKEAttributes converts attributes parsed by apiserver compatible with casbin enforcer func ConvertTKEAttributes(ctx context.Context, attr authorizer.Attributes) authorizer.Attributes {