Add SelfSubjectAccessReview as a fallback for failing SelfSubjectRulesReview (#5025)

* add SelfSubjectAccessReview as a fallback for failing SelfSubjectRulesReview

Signed-off-by: Alvin Wonys <alvin.rw@proton.me>

* add docs for SelfSubjectAccessReview validation fallback explanation

Signed-off-by: Alvin Wonys <alvin.rw@proton.me>

---------

Signed-off-by: Alvin Wonys <alvin.rw@proton.me>
Co-authored-by: Gustavo Fernandes de Carvalho <17139678+gusfcarvalho@users.noreply.github.com>

docs/provider/kubernetes.md

@@ -2,7 +2,7 @@ External Secrets Operator allows to retrieve secrets from a Kubernetes Cluster -
 
 A `SecretStore` points to a **specific namespace** in the target Kubernetes Cluster. You are able to retrieve all secrets from that particular namespace given you have the correct set of RBAC permissions.
 
-The `SecretStore` reconciler checks if you have read access for secrets in that namespace using `SelfSubjectRulesReview`. See below on how to set that up properly.
+The `SecretStore` reconciler checks if you have read access for secrets in that namespace using `SelfSubjectRulesReview` and will fallback to `SelfSubjectAccessReview` when that fails. See below on how to set that up properly.
 
 ### External Secret Spec
 

pkg/provider/kubernetes/provider.go

@@ -46,6 +46,10 @@ type RClient interface {
 	Create(ctx context.Context, selfSubjectRulesReview *authv1.SelfSubjectRulesReview, opts metav1.CreateOptions) (*authv1.SelfSubjectRulesReview, error)
 }
 
+type AClient interface {
+	Create(ctx context.Context, selfSubjectAccessReview *authv1.SelfSubjectAccessReview, opts metav1.CreateOptions) (*authv1.SelfSubjectAccessReview, error)
+}
+
 // Provider implements Secret Provider interface
 // for Kubernetes.
 type Provider struct{}
@@ -62,9 +66,12 @@ type Client struct {
 	// userSecretClient is a client-go CoreV1().Secrets() client
 	// with user-defined scope.
 	userSecretClient KClient
-	// userReviewClient is a SelfSubjectAccessReview client with
+	// userReviewClient is a SelfSubjectRulesReview client with
 	// user-defined scope.
 	userReviewClient RClient
+	// userAccessReviewClient is a SelfSubjectAccessReview client with
+	// user-defined scope.
+	userAccessReviewClient AClient
 
 	// store is the Kubernetes Provider spec
 	// which contains the configuration for this provider.
@@ -130,6 +137,7 @@ func (p *Provider) newClient(ctx context.Context, store esv1.GenericStore, ctrlC
 	}
 	client.userSecretClient = userClientset.CoreV1().Secrets(client.store.RemoteNamespace)
 	client.userReviewClient = userClientset.AuthorizationV1().SelfSubjectRulesReviews()
+	client.userAccessReviewClient = userClientset.AuthorizationV1().SelfSubjectAccessReviews()
 	return client, nil
 }
 

pkg/provider/kubernetes/validate.go

@@ -101,5 +101,23 @@ func (c *Client) Validate() (esv1.ValidationResult, error) {
 			return esv1.ValidationResultReady, nil
 		}
 	}
+
+	a := authv1.SelfSubjectAccessReview{
+		Spec: authv1.SelfSubjectAccessReviewSpec{
+			ResourceAttributes: &authv1.ResourceAttributes{
+				Resource:  "secrets",
+				Namespace: c.store.RemoteNamespace,
+				Verb:      "get",
+			},
+		},
+	}
+	accessReview, err := c.userAccessReviewClient.Create(ctx, &a, metav1.CreateOptions{})
+	if err != nil {
+		return esv1.ValidationResultUnknown, fmt.Errorf("could not verify if client is valid: %w", err)
+	}
+	if accessReview.Status.Allowed {
+		return esv1.ValidationResultReady, nil
+	}
+
 	return esv1.ValidationResultError, errors.New("client is not allowed to get secrets")
 }

pkg/provider/kubernetes/validate_test.go

@@ -39,6 +39,17 @@ func (fk fakeReviewClient) Create(_ context.Context, _ *authv1.SelfSubjectRulesR
 	return fk.authReview, nil
 }
 
+type fakeAccessReviewClient struct {
+	accessReview *authv1.SelfSubjectAccessReview
+}
+
+func (fk fakeAccessReviewClient) Create(_ context.Context, _ *authv1.SelfSubjectAccessReview, _ metav1.CreateOptions) (*authv1.SelfSubjectAccessReview, error) {
+	if fk.accessReview == nil {
+		return nil, errors.New(errSomethingWentWrong)
+	}
+	return fk.accessReview, nil
+}
+
 func TestValidateStore(t *testing.T) {
 	type fields struct {
 		Client       KClient
@@ -326,13 +337,24 @@ func TestValidate(t *testing.T) {
 			},
 		},
 	}
+	successAccessReview := authv1.SelfSubjectAccessReview{
+		Status: authv1.SubjectAccessReviewStatus{
+			Allowed: true,
+		},
+	}
+	failAccessReview := authv1.SelfSubjectAccessReview{
+		Status: authv1.SubjectAccessReviewStatus{
+			Allowed: false,
+		},
+	}
 
 	type fields struct {
-		Client       KClient
-		ReviewClient RClient
-		Namespace    string
-		store        *esv1.KubernetesProvider
-		storeKind    string
+		Client             KClient
+		ReviewClient       RClient
+		AccessReviewClient AClient
+		Namespace          string
+		store              *esv1.KubernetesProvider
+		storeKind          string
 	}
 	tests := []struct {
 		name    string
@@ -351,7 +373,8 @@ func TestValidate(t *testing.T) {
 						},
 					},
 				},
-				ReviewClient: fakeReviewClient{authReview: &successReview},
+				ReviewClient:       fakeReviewClient{authReview: &successReview},
+				AccessReviewClient: fakeAccessReviewClient{accessReview: &successAccessReview},
 			},
 			want:    esv1.ValidationResultUnknown,
 			wantErr: false,
@@ -359,39 +382,54 @@ func TestValidate(t *testing.T) {
 		{
 			name: "review results in unknown",
 			fields: fields{
-				Namespace:    "default",
-				ReviewClient: fakeReviewClient{},
-				store:        &esv1.KubernetesProvider{},
+				Namespace:          "default",
+				ReviewClient:       fakeReviewClient{},
+				AccessReviewClient: fakeAccessReviewClient{},
+				store:              &esv1.KubernetesProvider{},
 			},
 			want:    esv1.ValidationResultUnknown,
 			wantErr: true,
 		},
 		{
-			name: "not allowed results in error",
+			name: "rules & access review not allowed results in error",
 			fields: fields{
-				Namespace:    "default",
-				ReviewClient: fakeReviewClient{authReview: &failReview},
-				store:        &esv1.KubernetesProvider{},
+				Namespace:          "default",
+				ReviewClient:       fakeReviewClient{authReview: &failReview},
+				AccessReviewClient: fakeAccessReviewClient{accessReview: &failAccessReview},
+				store:              &esv1.KubernetesProvider{},
 			},
 			want:    esv1.ValidationResultError,
 			wantErr: true,
 		},
 		{
-			name: "allowed results in no error",
+			name: "rules review allowed results in no error",
+			fields: fields{
+				Namespace:          "default",
+				ReviewClient:       fakeReviewClient{authReview: &successReview},
+				AccessReviewClient: fakeAccessReviewClient{accessReview: &failAccessReview},
+				store:              &esv1.KubernetesProvider{},
+			},
+			want:    esv1.ValidationResultReady,
+			wantErr: false,
+		},
+		{
+			name: "rules review allowed results in no error",
 			fields: fields{
-				Namespace:    "default",
-				ReviewClient: fakeReviewClient{authReview: &successReview},
-				store:        &esv1.KubernetesProvider{},
+				Namespace:          "default",
+				ReviewClient:       fakeReviewClient{authReview: &successWildcardReview},
+				AccessReviewClient: fakeAccessReviewClient{accessReview: &failAccessReview},
+				store:              &esv1.KubernetesProvider{},
 			},
 			want:    esv1.ValidationResultReady,
 			wantErr: false,
 		},
 		{
-			name: "allowed results in no error",
+			name: "access review allowed results in no error",
 			fields: fields{
-				Namespace:    "default",
-				ReviewClient: fakeReviewClient{authReview: &successWildcardReview},
-				store:        &esv1.KubernetesProvider{},
+				Namespace:          "default",
+				ReviewClient:       fakeReviewClient{authReview: &failReview},
+				AccessReviewClient: fakeAccessReviewClient{accessReview: &successAccessReview},
+				store:              &esv1.KubernetesProvider{},
 			},
 			want:    esv1.ValidationResultReady,
 			wantErr: false,
@@ -400,11 +438,12 @@ func TestValidate(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			k := &Client{
-				userSecretClient: tt.fields.Client,
-				userReviewClient: tt.fields.ReviewClient,
-				namespace:        tt.fields.Namespace,
-				store:            tt.fields.store,
-				storeKind:        tt.fields.storeKind,
+				userSecretClient:       tt.fields.Client,
+				userReviewClient:       tt.fields.ReviewClient,
+				userAccessReviewClient: tt.fields.AccessReviewClient,
+				namespace:              tt.fields.Namespace,
+				store:                  tt.fields.store,
+				storeKind:              tt.fields.storeKind,
 			}
 			got, err := k.Validate()
 			if (err != nil) != tt.wantErr {