From 892ebf2d253b3c8369f61d7b60ceae45863201c6 Mon Sep 17 00:00:00 2001
From: Sheng Zhan <49895476+AxeZhan@users.noreply.github.com>
Date: Wed, 26 Apr 2023 00:06:18 +0800
Subject: [PATCH] Ensure version "*" is passed instead of "" for all authz
 checks (#116937)

* ensure version * is passed instead of  for all authz checks

* unexport match function

* remove allversion constant
---
 .../certificates/approver/sarapprove.go       |  5 ++--
 .../subjectaccessreview/rest_test.go          |  4 +--
 pkg/registry/authorization/util/helpers.go    | 10 ++++++-
 .../authorization/util/helpers_test.go        | 29 +++++++++++++++++++
 4 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/pkg/controller/certificates/approver/sarapprove.go b/pkg/controller/certificates/approver/sarapprove.go
index d739fc783b3..381304a9000 100644
--- a/pkg/controller/certificates/approver/sarapprove.go
+++ b/pkg/controller/certificates/approver/sarapprove.go
@@ -29,7 +29,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	certificatesinformers "k8s.io/client-go/informers/certificates/v1"
 	clientset "k8s.io/client-go/kubernetes"
-
 	capihelper "k8s.io/kubernetes/pkg/apis/certificates"
 	"k8s.io/kubernetes/pkg/controller/certificates"
 )
@@ -63,12 +62,12 @@ func recognizers() []csrRecognizer {
 	recognizers := []csrRecognizer{
 		{
 			recognize:      isSelfNodeClientCert,
-			permission:     authorization.ResourceAttributes{Group: "certificates.k8s.io", Resource: "certificatesigningrequests", Verb: "create", Subresource: "selfnodeclient"},
+			permission:     authorization.ResourceAttributes{Group: "certificates.k8s.io", Resource: "certificatesigningrequests", Verb: "create", Subresource: "selfnodeclient", Version: "*"},
 			successMessage: "Auto approving self kubelet client certificate after SubjectAccessReview.",
 		},
 		{
 			recognize:      isNodeClientCert,
-			permission:     authorization.ResourceAttributes{Group: "certificates.k8s.io", Resource: "certificatesigningrequests", Verb: "create", Subresource: "nodeclient"},
+			permission:     authorization.ResourceAttributes{Group: "certificates.k8s.io", Resource: "certificatesigningrequests", Verb: "create", Subresource: "nodeclient", Version: "*"},
 			successMessage: "Auto approving kubelet client certificate after SubjectAccessReview.",
 		},
 	}
diff --git a/pkg/registry/authorization/subjectaccessreview/rest_test.go b/pkg/registry/authorization/subjectaccessreview/rest_test.go
index c768f24391e..35f8b58449c 100644
--- a/pkg/registry/authorization/subjectaccessreview/rest_test.go
+++ b/pkg/registry/authorization/subjectaccessreview/rest_test.go
@@ -19,11 +19,10 @@ package subjectaccessreview
 import (
 	"context"
 	"errors"
+	"reflect"
 	"strings"
 	"testing"
 
-	"reflect"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
@@ -181,6 +180,7 @@ func TestCreate(t *testing.T) {
 			expectedAttrs: authorizer.AttributesRecord{
 				User:            &user.DefaultInfo{Name: "bob"},
 				ResourceRequest: true,
+				APIVersion:      "*",
 			},
 			expectedStatus: authorizationapi.SubjectAccessReviewStatus{
 				Allowed: false,
diff --git a/pkg/registry/authorization/util/helpers.go b/pkg/registry/authorization/util/helpers.go
index c9187f481dc..f939f7b826a 100644
--- a/pkg/registry/authorization/util/helpers.go
+++ b/pkg/registry/authorization/util/helpers.go
@@ -29,7 +29,7 @@ func ResourceAttributesFrom(user user.Info, in authorizationapi.ResourceAttribut
 		Verb:            in.Verb,
 		Namespace:       in.Namespace,
 		APIGroup:        in.Group,
-		APIVersion:      in.Version,
+		APIVersion:      matchAllVersionIfEmpty(in.Version),
 		Resource:        in.Resource,
 		Subresource:     in.Subresource,
 		Name:            in.Name,
@@ -77,3 +77,11 @@ func AuthorizationAttributesFrom(spec authorizationapi.SubjectAccessReviewSpec)
 
 	return authorizationAttributes
 }
+
+// matchAllVersionIfEmpty returns a "*" if the version is unspecified
+func matchAllVersionIfEmpty(version string) string {
+	if len(version) == 0 {
+		return "*"
+	}
+	return version
+}
diff --git a/pkg/registry/authorization/util/helpers_test.go b/pkg/registry/authorization/util/helpers_test.go
index f8c2b914019..c086e70e12a 100644
--- a/pkg/registry/authorization/util/helpers_test.go
+++ b/pkg/registry/authorization/util/helpers_test.go
@@ -133,6 +133,35 @@ func TestAuthorizationAttributesFrom(t *testing.T) {
 				ResourceRequest: true,
 			},
 		},
+		{
+			name: "resource with no version",
+			args: args{
+				spec: authorizationapi.SubjectAccessReviewSpec{
+					User: "bob",
+					ResourceAttributes: &authorizationapi.ResourceAttributes{
+						Namespace:   "myns",
+						Verb:        "create",
+						Group:       "extensions",
+						Resource:    "deployments",
+						Subresource: "scale",
+						Name:        "mydeployment",
+					},
+				},
+			},
+			want: authorizer.AttributesRecord{
+				User: &user.DefaultInfo{
+					Name: "bob",
+				},
+				APIGroup:        "extensions",
+				APIVersion:      "*",
+				Namespace:       "myns",
+				Verb:            "create",
+				Resource:        "deployments",
+				Subresource:     "scale",
+				Name:            "mydeployment",
+				ResourceRequest: true,
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {