kcp-dev · sttts · Aug 23, 2024
diff --git a/pkg/registry/rbac/validation/kcp.go b/pkg/registry/rbac/validation/kcp.go
@@ -0,0 +1,126 @@
+package validation
+
+import (
+	"context"
+	"encoding/json"
+	"strings"
+
+	"github.com/kcp-dev/logicalcluster/v3"
+	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
+	authserviceaccount "k8s.io/apiserver/pkg/authentication/serviceaccount"
+	"k8s.io/apiserver/pkg/authentication/user"
+	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
+)
+
+const (
+	// WarrantExtraKey is the key used in a user's "extra" to specify
+	// JSON-encoded user infos for attached extra permissions for that user
+	// evaluated by the authorizer.
+	WarrantExtraKey = "authorization.kcp.io/warrants"
+
+	// ScopeExtraKey is the key used in a user's "extra" to specify
+	// that the user is restricted to a given scope. Valid values are:
+	// - "cluster:<name>"
+	// In the future, we might add:
+	// - "interval:from:to".
+	// Scopes are and'ed. Scoping to multiple clusters invalidates it for all.
+	ScopeExtraKey = "authentication.kcp.io/scopes"
+)
+
+// Warrant is serialized into the user's "extra" field authorization.kcp.io/warrants
+// to hold user information for extra permissions.
+type Warrant struct {
+	// User is the user you're testing for.
+	// If you specify "User" but not "Groups", then is it interpreted as "What if User were not a member of any groups
+	// +optional
+	User string `json:"user,omitempty"`
+	// Groups is the groups you're testing for.
+	// +optional
+	// +listType=atomic
+	Groups []string `json:"groups,omitempty"`
+	// Extra corresponds to the user.Info.GetExtra() method from the authenticator.  Since that is input to the authorizer
+	// it needs a reflection here.
+	// +optional
+	Extra map[string][]string `json:"extra,omitempty"`
+	// UID information about the requesting user.
+	// +optional
+	UID string `json:"uid,omitempty"`
+}
+
+type appliesToUserFunc func(user user.Info, subject rbacv1.Subject, namespace string) bool
+type appliesToUserFuncCtx func(ctx context.Context, user user.Info, subject rbacv1.Subject, namespace string) bool
+
+var appliesToUserWithWarrants = withWarrants(appliesToUser)
+
+// withWarrants wraps the appliesToUser predicate to check for the base user and any warrants.
+func withWarrants(appliesToUser appliesToUserFunc) appliesToUserFuncCtx {
+	var recursive appliesToUserFuncCtx
+	recursive = func(ctx context.Context, u user.Info, bindingSubject rbacv1.Subject, namespace string) bool {
+		cluster := genericapirequest.ClusterFrom(ctx)
+		if IsInScope(u, cluster.Name) && appliesToUser(u, bindingSubject, namespace) {
+			return true
+		}
+
+		for _, v := range u.GetExtra()[WarrantExtraKey] {
+			var w Warrant
+			if err := json.Unmarshal([]byte(v), &w); err != nil {
+				continue
+			}
+
+			wu := &user.DefaultInfo{
+				Name:   w.User,
+				UID:    w.UID,
+				Groups: w.Groups,
+				Extra:  w.Extra,
+			}
+			if IsServiceAccount(wu) && len(w.Extra[authserviceaccount.ClusterNameKey]) == 0 {
+				continue
+			}
+			if recursive(ctx, wu, bindingSubject, namespace) {
+				return true
+			}
+		}
+
+		return false
+	}
+	return recursive
+}
+
+// IsServiceAccount returns true if the user is a service account.
+func IsServiceAccount(attr user.Info) bool {
+	return strings.HasPrefix(attr.GetName(), "system:serviceaccount:")
+}
+
+// IsForeign returns true if the service account is not from the given cluster.
+func IsForeign(attr user.Info, cluster logicalcluster.Name) bool {
+	clusters := attr.GetExtra()[authserviceaccount.ClusterNameKey]
+	if clusters == nil {
+		// an unqualified service account is considered local: think of some
+		// local SubjectAccessReview specifying a service account without the
+		// cluster scope.
+		return false
+	}
+	return !sets.New(clusters...).Has(string(cluster))
+}
+
+// IsInScope checks if the user is valid for the given cluster.
+func IsInScope(attr user.Info, cluster logicalcluster.Name) bool {
+	if IsServiceAccount(attr) && IsForeign(attr, cluster) {
+		return false
+	}
+
+	scopes := attr.GetExtra()[ScopeExtraKey]
+	for _, scope := range scopes {
+		switch {
+		case strings.HasPrefix(scope, "cluster:"):
+			if scope != "cluster:"+string(cluster) {
+				return false
+			}
+		default:
+			// Unknown scope, ignore.
+		}
+	}
+
+	return true
+}
diff --git a/pkg/registry/rbac/validation/kcp_test.go b/pkg/registry/rbac/validation/kcp_test.go
@@ -0,0 +1,238 @@
+package validation
+
+import (
+	"context"
+	"testing"
+
+	"github.com/kcp-dev/logicalcluster/v3"
+	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apiserver/pkg/authentication/user"
+	"k8s.io/apiserver/pkg/endpoints/request"
+)
+
+func TestIsInScope(t *testing.T) {
+	tests := []struct {
+		name    string
+		info    user.DefaultInfo
+		cluster logicalcluster.Name
+		want    bool
+	}{
+		{name: "empty", cluster: logicalcluster.Name("cluster"), want: true},
+		{
+			name:    "serviceaccount from other cluster",
+			info:    user.DefaultInfo{Name: "system:serviceaccount:default:foo", Extra: map[string][]string{"authentication.kubernetes.io/cluster-name": {"anotherws"}}},
+			cluster: logicalcluster.Name("this"),
+			want:    false,
+		},
+		{
+			name:    "serviceaccount from same cluster",
+			info:    user.DefaultInfo{Name: "system:serviceaccount:default:foo", Extra: map[string][]string{"authentication.kubernetes.io/cluster-name": {"this"}}},
+			cluster: logicalcluster.Name("this"),
+			want:    true,
+		},
+		{
+			name:    "serviceaccount without a cluster",
+			info:    user.DefaultInfo{Name: "system:serviceaccount:default:foo"},
+			cluster: logicalcluster.Name("this"),
+			// an unqualified service account is considered local: think of some
+			// local SubjectAccessReview specifying a service account without the
+			// cluster scope.
+			want: true,
+		},
+		{
+			name:    "scoped user",
+			info:    user.DefaultInfo{Extra: map[string][]string{"authentication.kcp.io/scopes": {"cluster:this"}}},
+			cluster: logicalcluster.Name("this"),
+			want:    true,
+		},
+		{
+			name:    "scoped user to another cluster",
+			info:    user.DefaultInfo{Extra: map[string][]string{"authentication.kcp.io/scopes": {"cluster:another"}}},
+			cluster: logicalcluster.Name("this"),
+			want:    false,
+		},
+		{
+			name:    "scoped user to multiple clusters",
+			info:    user.DefaultInfo{Extra: map[string][]string{"authentication.kcp.io/scopes": {"cluster:this", "cluster:another"}}},
+			cluster: logicalcluster.Name("this"),
+			want:    false,
+		},
+		{
+			name:    "unknown scope",
+			info:    user.DefaultInfo{Extra: map[string][]string{"authentication.kcp.io/scopes": {"unknown:foo"}}},
+			cluster: logicalcluster.Name("this"),
+			want:    true,
+		},
+		{
+			name: "scoped service account",
+			info: user.DefaultInfo{Name: "system:serviceaccount:default:foo", Extra: map[string][]string{
+				"authentication.kubernetes.io/cluster-name": {"this"},
+				"authentication.kcp.io/scopes":              {"cluster:this"},
+			}},
+			cluster: logicalcluster.Name("this"),
+			want:    true,
+		},
+		{
+			name: "scoped foreign service account",
+			info: user.DefaultInfo{Name: "system:serviceaccount:default:foo", Extra: map[string][]string{
+				"authentication.kubernetes.io/cluster-name": {"another"},
+				"authentication.kcp.io/scopes":              {"cluster:this"},
+			}},
+			cluster: logicalcluster.Name("this"),
+			want:    false,
+		},
+		{
+			name: "scoped service account to another clusters",
+			info: user.DefaultInfo{Name: "system:serviceaccount:default:foo", Extra: map[string][]string{
+				"authentication.kubernetes.io/cluster-name": {"this"},
+				"authentication.kcp.io/scopes":              {"cluster:another"},
+			}},
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := IsInScope(&tt.info, tt.cluster); got != tt.want {
+				t.Errorf("IsInScope() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestAppliesToUserWithWarrants(t *testing.T) {
+	tests := []struct {
+		name string
+		user user.Info
+		sub  rbacv1.Subject
+		want bool
+	}{
+		{
+			name: "simple matching user without warrants",
+			user: &user.DefaultInfo{Name: "user-a"},
+			sub:  rbacv1.Subject{Kind: "User", Name: "user-a"},
+			want: true,
+		},
+		{
+			name: "simple non-matching user without warrants",
+			user: &user.DefaultInfo{Name: "user-a"},
+			sub:  rbacv1.Subject{Kind: "User", Name: "user-b"},
+			want: false,
+		},
+		{
+			name: "simple matching user with warrants",
+			user: &user.DefaultInfo{Name: "user-a", Extra: map[string][]string{WarrantExtraKey: {`{"user":"user-b"}`}}},
+			sub:  rbacv1.Subject{Kind: "User", Name: "user-a"},
+			want: true,
+		},
+		{
+			name: "simple non-matching user with matching warrants",
+			user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{WarrantExtraKey: {`{"user":"user-a"}`}}},
+			sub:  rbacv1.Subject{Kind: "User", Name: "user-a"},
+			want: true,
+		},
+		{
+			name: "simple non-matching user with non-matching warrants",
+			user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{WarrantExtraKey: {`{"user":"user-b"}`}}},
+			sub:  rbacv1.Subject{Kind: "User", Name: "user-a"},
+			want: false,
+		},
+		{
+			name: "simple non-matching user with multiple warrants",
+			user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{WarrantExtraKey: {`{"user":"user-b"}`, `{"user":"user-a"}`, `{"user":"user-c"}`}}},
+			sub:  rbacv1.Subject{Kind: "User", Name: "user-a"},
+			want: true,
+		},
+		{
+			name: "simple non-matching user with nested warrants",
+			user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{WarrantExtraKey: {`{"user":"user-b","extra":{"authorization.kcp.io/warrants":["{\"user\":\"user-a\"}"]}}`}}},
+			sub:  rbacv1.Subject{Kind: "User", Name: "user-a"},
+			want: true,
+		},
+		{
+			name: "foreign service account",
+			user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kubernetes.io/cluster-name": {"other"}}},
+			sub:  rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"},
+			want: false,
+		},
+		{
+			name: "local service account",
+			user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kubernetes.io/cluster-name": {"this"}}},
+			sub:  rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"},
+			want: true,
+		},
+		{
+			name: "foreign service account with local warrant",
+			user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kubernetes.io/cluster-name": {"other"}, WarrantExtraKey: {`{"user":"system:serviceaccount:ns:sa","extra":{"authentication.kubernetes.io/cluster-name":["this"]}}`}}},
+			sub:  rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"},
+			want: true,
+		},
+		{
+			name: "foreign service account with foreign warrant",
+			user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kubernetes.io/cluster-name": {"other"}, WarrantExtraKey: {`{"user":"system:serviceaccount:ns:sa","extra":{"authentication.kubernetes.io/cluster-name":["other"]}}`}}},
+			sub:  rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"},
+			want: false,
+		},
+		{
+			name: "non-cluster-aware service account",
+			user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa"},
+			sub:  rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"},
+			want: true,
+		},
+		{
+			name: "non-cluster-aware service account as warrant",
+			user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{WarrantExtraKey: {`{"user":"system:serviceaccount:ns:sa"}`}}},
+			sub:  rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"},
+			want: false,
+		},
+		{
+			name: "in-scope scoped user",
+			user: &user.DefaultInfo{Name: "user-a", Extra: map[string][]string{"authentication.kcp.io/scopes": {"cluster:this"}}},
+			sub:  rbacv1.Subject{Kind: "User", Name: "user-a"},
+			want: true,
+		},
+		{
+			name: "out-of-scope user",
+			user: &user.DefaultInfo{Name: "user-a", Extra: map[string][]string{"authentication.kcp.io/scopes": {"cluster:other"}}},
+			sub:  rbacv1.Subject{Kind: "User", Name: "user-a"},
+			want: false,
+		},
+		{
+			name: "out-of-scope user with warrent",
+			user: &user.DefaultInfo{Name: "user-a", Extra: map[string][]string{"authentication.kcp.io/scopes": {"cluster:other"}, WarrantExtraKey: {`{"user":"user-a"}`}}},
+			sub:  rbacv1.Subject{Kind: "User", Name: "user-a"},
+			want: true,
+		},
+		{
+			name: "out-of-scope warrant",
+			user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{WarrantExtraKey: {`{"user":"user-a","extra":{"authentication.kcp.io/scopes":["cluster:other"]}}`}}},
+			sub:  rbacv1.Subject{Kind: "User", Name: "user-a"},
+			want: false,
+		},
+		{
+			name: "in-scope warrant",
+			user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{WarrantExtraKey: {`{"user":"user-a","extra":{"authentication.kcp.io/scopes":["cluster:this"]}}`}}},
+			sub:  rbacv1.Subject{Kind: "User", Name: "user-a"},
+			want: true,
+		},
+		{
+			name: "in-scope service account",
+			user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/scopes": {"cluster:this"}}},
+			sub:  rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"},
+			want: true,
+		},
+		{
+			name: "out-of-scope service account",
+			user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/scopes": {"cluster:other"}}},
+			sub:  rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"},
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := request.WithCluster(context.Background(), request.Cluster{Name: "this"})
+			if got := appliesToUserWithWarrants(ctx, tt.user, tt.sub, "ns"); got != tt.want {
+				t.Errorf("withWarrants(base) = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
diff --git a/pkg/registry/rbac/validation/rule.go b/pkg/registry/rbac/validation/rule.go
@@ -184,7 +184,7 @@ func (r *DefaultRuleResolver) VisitRulesFor(ctx context.Context, user user.Info,
 	} else {
 		sourceDescriber := &clusterRoleBindingDescriber{}
 		for _, clusterRoleBinding := range clusterRoleBindings {
-			subjectIndex, applies := appliesTo(user, clusterRoleBinding.Subjects, "")
+			subjectIndex, applies := appliesTo(ctx, user, clusterRoleBinding.Subjects, "")
 			if !applies {
 				continue
 			}
@@ -213,7 +213,7 @@ func (r *DefaultRuleResolver) VisitRulesFor(ctx context.Context, user user.Info,
 		} else {
 			sourceDescriber := &roleBindingDescriber{}
 			for _, roleBinding := range roleBindings {
-				subjectIndex, applies := appliesTo(user, roleBinding.Subjects, namespace)
+				subjectIndex, applies := appliesTo(ctx, user, roleBinding.Subjects, namespace)
 				if !applies {
 					continue
 				}
@@ -260,9 +260,9 @@ func (r *DefaultRuleResolver) GetRoleReferenceRules(ctx context.Context, roleRef
 
 // appliesTo returns whether any of the bindingSubjects applies to the specified subject,
 // and if true, the index of the first subject that applies
-func appliesTo(user user.Info, bindingSubjects []rbacv1.Subject, namespace string) (int, bool) {
+func appliesTo(ctx context.Context, user user.Info, bindingSubjects []rbacv1.Subject, namespace string) (int, bool) {
 	for i, bindingSubject := range bindingSubjects {
-		if appliesToUser(user, bindingSubject, namespace) {
+		if appliesToUserWithWarrants(ctx, user, bindingSubject, namespace) {
 			return i, true
 		}
 	}