Merge pull request #124792 from mjudeikis/mjudeikis/ctx.wiring

Wire in ctx into rbac plugins

test/integration/auth/rbac_test.go

@@ -27,6 +27,7 @@ import (
 	"strings"
 	"testing"
 
+	corev1 "k8s.io/api/core/v1"
 	rbacapi "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -40,6 +41,7 @@ import (
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	unionauthz "k8s.io/apiserver/pkg/authorization/union"
+	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/generic"
 	clientset "k8s.io/client-go/kubernetes"
 	restclient "k8s.io/client-go/rest"
@@ -133,26 +135,27 @@ type bootstrapRoles struct {
 //
 // client should be authenticated as the RBAC super user.
 func (b bootstrapRoles) bootstrap(client clientset.Interface) error {
+	ctx := context.TODO()
 	for _, r := range b.clusterRoles {
-		_, err := client.RbacV1().ClusterRoles().Create(context.TODO(), &r, metav1.CreateOptions{})
+		_, err := client.RbacV1().ClusterRoles().Create(ctx, &r, metav1.CreateOptions{})
 		if err != nil {
 			return fmt.Errorf("failed to make request: %v", err)
 		}
 	}
 	for _, r := range b.roles {
-		_, err := client.RbacV1().Roles(r.Namespace).Create(context.TODO(), &r, metav1.CreateOptions{})
+		_, err := client.RbacV1().Roles(r.Namespace).Create(ctx, &r, metav1.CreateOptions{})
 		if err != nil {
 			return fmt.Errorf("failed to make request: %v", err)
 		}
 	}
 	for _, r := range b.clusterRoleBindings {
-		_, err := client.RbacV1().ClusterRoleBindings().Create(context.TODO(), &r, metav1.CreateOptions{})
+		_, err := client.RbacV1().ClusterRoleBindings().Create(ctx, &r, metav1.CreateOptions{})
 		if err != nil {
 			return fmt.Errorf("failed to make request: %v", err)
 		}
 	}
 	for _, r := range b.roleBindings {
-		_, err := client.RbacV1().RoleBindings(r.Namespace).Create(context.TODO(), &r, metav1.CreateOptions{})
+		_, err := client.RbacV1().RoleBindings(r.Namespace).Create(ctx, &r, metav1.CreateOptions{})
 		if err != nil {
 			return fmt.Errorf("failed to make request: %v", err)
 		}
@@ -817,3 +820,220 @@ func TestDiscoveryUpgradeBootstrapping(t *testing.T) {
 		t.Errorf("`system:public-info-viewer` should have inherited Subjects from `system:discovery` Wanted: %v, got %v", newDiscRoleBinding.Subjects, publicInfoViewerRoleBinding.Subjects)
 	}
 }
+
+type authorizeRequest struct {
+	ar       authorizer.AttributesRecord
+	expected authorizer.Decision
+}
+
+// For 1.31 ctx was wired into the authorizers. This tests check that context values
+// are not used inside the code to resolve namespaces or users with the goal of
+// preventing regressions in the future.
+func TestRBACContextContamination(t *testing.T) {
+	superUser := "admin/system:masters"
+	validNamespace := "pod-namespace"
+	invalidNamespace := "forbidden-namespace"
+
+	roles := bootstrapRoles{}
+	testcases := []authorizeRequest{}
+
+	// Tests itself is bit oververbose and each case creates its own objects.
+	// This makes readability bit easier over trying to overoptimize test case.
+
+	// Case 1: clusterrole+clusterbinding
+	// should allow cluster-scoped request
+	// should allow namespace-scoped request in any namespace
+	// should disallow request for resource not in rules
+	{
+		roles.clusterRoles = append(roles.clusterRoles, rbacapi.ClusterRole{
+			ObjectMeta: metav1.ObjectMeta{Name: "c1-clusterrole"},
+			Rules:      []rbacapi.PolicyRule{ruleReadPods},
+		})
+		roles.clusterRoleBindings = append(roles.clusterRoleBindings, rbacapi.ClusterRoleBinding{
+			ObjectMeta: metav1.ObjectMeta{Name: "c1-clusterrolebinding"},
+			Subjects:   []rbacapi.Subject{{Kind: "User", Name: "c1-user"}},
+			RoleRef:    rbacapi.RoleRef{Kind: "ClusterRole", Name: "c1-clusterrole"},
+		})
+
+		user := &user.DefaultInfo{Name: "c1-user"}
+		testcases = append(testcases, []authorizeRequest{
+			{
+				ar:       authorizer.AttributesRecord{Verb: "list", Resource: "pods", Namespace: "", ResourceRequest: true, User: user},
+				expected: authorizer.DecisionAllow,
+			},
+			{
+				ar:       authorizer.AttributesRecord{Verb: "list", Resource: "pods", Namespace: validNamespace, ResourceRequest: true, User: user},
+				expected: authorizer.DecisionAllow,
+			},
+			{
+				ar:       authorizer.AttributesRecord{Verb: "list", Resource: "configmaps", Namespace: "", ResourceRequest: true, User: user},
+				expected: authorizer.DecisionNoOpinion,
+			},
+		}...,
+		)
+	}
+
+	// case 2: clusterrole+rolebinding
+	// should disallow cluster-scoped request
+	// should allow namespace-scoped request in rolebinding namespace
+	// should disallow namespace-scoped request in other namespace
+	// should disallow request for resource not in rules
+	{
+		roles.clusterRoles = append(roles.clusterRoles, rbacapi.ClusterRole{
+			ObjectMeta: metav1.ObjectMeta{Name: "c2-clusterrole"},
+			Rules:      []rbacapi.PolicyRule{ruleReadPods},
+		})
+		roles.roleBindings = append(roles.roleBindings, rbacapi.RoleBinding{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "c2-rolebinding",
+				Namespace: validNamespace,
+			},
+			Subjects: []rbacapi.Subject{{Kind: "User", Name: "c2-user"}},
+			RoleRef:  rbacapi.RoleRef{Kind: "ClusterRole", Name: "c2-clusterrole"},
+		})
+
+		user := &user.DefaultInfo{Name: "c2-user"}
+		testcases = append(testcases, []authorizeRequest{
+			{
+				ar:       authorizer.AttributesRecord{Verb: "list", Resource: "pods", Namespace: "", ResourceRequest: true, User: user},
+				expected: authorizer.DecisionNoOpinion,
+			},
+			{
+				ar:       authorizer.AttributesRecord{Verb: "list", Resource: "pods", Namespace: validNamespace, ResourceRequest: true, User: user},
+				expected: authorizer.DecisionAllow,
+			},
+			{
+				ar:       authorizer.AttributesRecord{Verb: "list", Resource: "configmaps", Namespace: validNamespace, ResourceRequest: true, User: user},
+				expected: authorizer.DecisionNoOpinion,
+			},
+			{
+				ar:       authorizer.AttributesRecord{Verb: "list", Resource: "pods", Namespace: invalidNamespace, ResourceRequest: true, User: user},
+				expected: authorizer.DecisionNoOpinion,
+			},
+		}...,
+		)
+	}
+
+	// case 3: role+rolebinding
+	// should disallow cluster-scoped request
+	// should allow namespace-scoped request in rolebinding namespace
+	// should disallow namespace-scoped request in other namespace
+	// should disallow request for resource not in rules
+	{
+		roles.roles = append(roles.roles, rbacapi.Role{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "c3-role",
+				Namespace: validNamespace,
+			},
+			Rules: []rbacapi.PolicyRule{ruleReadPods},
+		})
+		roles.roleBindings = append(roles.roleBindings, rbacapi.RoleBinding{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "c3-rolebinding",
+				Namespace: validNamespace,
+			},
+			Subjects: []rbacapi.Subject{{Kind: "User", Name: "c3-user"}},
+			RoleRef:  rbacapi.RoleRef{Kind: "Role", Name: "c3-role"},
+		})
+
+		user := &user.DefaultInfo{Name: "c3-user"}
+		testcases = append(testcases, []authorizeRequest{
+			{
+				ar:       authorizer.AttributesRecord{Verb: "list", Resource: "pods", Namespace: "", ResourceRequest: true, User: user},
+				expected: authorizer.DecisionNoOpinion,
+			},
+			{
+				ar:       authorizer.AttributesRecord{Verb: "list", Resource: "pods", Namespace: validNamespace, ResourceRequest: true, User: user},
+				expected: authorizer.DecisionAllow,
+			},
+			{
+				ar:       authorizer.AttributesRecord{Verb: "list", Resource: "configmaps", Namespace: validNamespace, ResourceRequest: true, User: user},
+				expected: authorizer.DecisionNoOpinion,
+			},
+			{
+				ar:       authorizer.AttributesRecord{Verb: "list", Resource: "pods", Namespace: invalidNamespace, ResourceRequest: true, User: user},
+				expected: authorizer.DecisionNoOpinion,
+			},
+		}...,
+		)
+	}
+
+	authenticator := group.NewAuthenticatedGroupAdder(bearertoken.New(tokenfile.New(map[string]*user.DefaultInfo{
+		superUser: {Name: "admin", Groups: []string{"system:masters"}},
+	})))
+
+	var tearDownAuthorizerFn func()
+	defer func() {
+		if tearDownAuthorizerFn != nil {
+			tearDownAuthorizerFn()
+		}
+	}()
+	var rbacAuthz authorizer.Authorizer
+	_, kubeConfig, tearDownFn := framework.StartTestServer(context.Background(), t, framework.TestServerSetup{
+		ModifyServerRunOptions: func(opts *options.ServerRunOptions) {
+			// Disable ServiceAccount admission plugin as we don't have serviceaccount controller running.
+			// Also disable namespace lifecycle to workaroung the test limitation that first creates
+			// roles/rolebindings and only then creates corresponding namespaces.
+			opts.Admission.GenericAdmission.DisablePlugins = []string{"ServiceAccount", "NamespaceLifecycle"}
+			// Disable built-in authorizers
+			opts.Authorization.Modes = []string{"AlwaysDeny"}
+		},
+		ModifyServerConfig: func(config *controlplane.Config) {
+			// Append our custom test authenticator
+			config.ControlPlane.Generic.Authentication.Authenticator = unionauthn.New(config.ControlPlane.Generic.Authentication.Authenticator, authenticator)
+			// Append our custom test authorizer
+			rbacAuthz, tearDownAuthorizerFn = newRBACAuthorizer(t, config)
+			config.ControlPlane.Generic.Authorization.Authorizer = unionauthz.New(config.ControlPlane.Generic.Authorization.Authorizer, rbacAuthz)
+		},
+	})
+	defer tearDownFn()
+
+	// Bootstrap the API Server with the test case's initial roles.
+	superuserClient, _ := clientsetForToken(superUser, kubeConfig)
+	if err := roles.bootstrap(superuserClient); err != nil {
+		t.Errorf("failed to apply initial roles: %v", err)
+		return
+	}
+
+	for _, ns := range []string{validNamespace, invalidNamespace} {
+		_, err := superuserClient.CoreV1().Namespaces().Create(context.TODO(), &corev1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: ns,
+			},
+		}, metav1.CreateOptions{})
+		if err != nil {
+			t.Errorf("failed to create namespace: %v", err)
+			return
+		}
+	}
+
+	// 3 cases shared for all test cases:
+	// 1. Default context
+	// 2. Empty namespace
+	// 3. Invalid namespace in the context
+	for j, r := range testcases {
+		ctx := context.Background()
+		// 1. Default context
+		if decision, _, err := rbacAuthz.Authorize(ctx, &r.ar); err != nil {
+			t.Errorf("req %d: unexpected error: %v", j, err)
+			return
+		} else if decision != r.expected {
+			t.Errorf("req %d: expected %v, got %v", j, r.expected, decision)
+		}
+		// 2. Empty namespace
+		if decision, _, err := rbacAuthz.Authorize(genericapirequest.WithNamespace(ctx, ""), &r.ar); err != nil {
+			t.Errorf("req %d: unexpected error: %v", j, err)
+			return
+		} else if decision != r.expected {
+			t.Errorf("req %d: expected %v, got %v", j, r.expected, decision)
+		}
+		// 3. Invalid namespace in the context
+		if decision, _, err := rbacAuthz.Authorize(genericapirequest.WithNamespace(ctx, invalidNamespace), &r.ar); err != nil {
+			t.Errorf("req %d: unexpected error: %v", j, err)
+			return
+		} else if decision != r.expected {
+			t.Errorf("req %d: expected %v, got %v", j, r.expected, decision)
+		}
+
+	}
+}