fix(auth): authz bug fix (#1916)

Co-authored-by: caryxychen <[email protected]>

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 {

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
 }

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 {