Skip to content

Commit 8f505a5

Browse files
authored
Merge commit from fork
The informer-based caching introduced in f22ae3b bypassed caller authorization checks when reading templates. This adds explicit auth.CanI() checks before returning data from the informer cache, ensuring proper authorization while preserving performance benefits. Signed-off-by: Alan Clucas <alan@clucas.org>
1 parent 9064c7f commit 8f505a5

4 files changed

Lines changed: 75 additions & 0 deletions

File tree

server/clusterworkflowtemplate/cluster_workflow_template_server.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"sort"
77

88
"google.golang.org/grpc/codes"
9+
"google.golang.org/grpc/status"
910
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1011

1112
clusterwftmplpkg "github.com/argoproj/argo-workflows/v3/pkg/apiclient/clusterworkflowtemplate"
@@ -52,6 +53,13 @@ func (cwts *ClusterWorkflowTemplateServer) CreateClusterWorkflowTemplate(ctx con
5253
}
5354

5455
func (cwts *ClusterWorkflowTemplateServer) GetClusterWorkflowTemplate(ctx context.Context, req *clusterwftmplpkg.ClusterWorkflowTemplateGetRequest) (*v1alpha1.ClusterWorkflowTemplate, error) {
56+
allowed, err := auth.CanI(ctx, "get", "clusterworkflowtemplates", "", req.Name)
57+
if err != nil {
58+
return nil, serverutils.ToStatusError(err, codes.Internal)
59+
}
60+
if !allowed {
61+
return nil, status.Error(codes.PermissionDenied, "permission denied")
62+
}
5563
wfTmpl, err := cwts.getTemplateAndValidate(ctx, req.Name)
5664
if err != nil {
5765
return nil, serverutils.ToStatusError(err, codes.Internal)

server/clusterworkflowtemplate/cluster_workflow_template_server_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ import (
77
"github.com/go-jose/go-jose/v3/jwt"
88
"github.com/stretchr/testify/assert"
99
"github.com/stretchr/testify/require"
10+
authorizationv1 "k8s.io/api/authorization/v1"
11+
"k8s.io/apimachinery/pkg/runtime"
1012
"k8s.io/client-go/kubernetes/fake"
13+
k8stesting "k8s.io/client-go/testing"
1114

1215
clusterwftmplpkg "github.com/argoproj/argo-workflows/v3/pkg/apiclient/clusterworkflowtemplate"
1316
"github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
@@ -151,6 +154,11 @@ const userEmailLabel = "my-sub.at.your.org"
151154

152155
func getClusterWorkflowTemplateServer() (clusterwftmplpkg.ClusterWorkflowTemplateServiceServer, context.Context) {
153156
kubeClientSet := fake.NewSimpleClientset()
157+
kubeClientSet.PrependReactor("create", "selfsubjectaccessreviews", func(action k8stesting.Action) (bool, runtime.Object, error) {
158+
return true, &authorizationv1.SelfSubjectAccessReview{
159+
Status: authorizationv1.SubjectAccessReviewStatus{Allowed: true},
160+
}, nil
161+
})
154162
wfClientset := wftFake.NewSimpleClientset(&unlabelled, &cwftObj2, &cwftObj3)
155163
ctx := context.WithValue(context.WithValue(context.WithValue(context.TODO(), auth.WfKey, wfClientset), auth.KubeKey, kubeClientSet), auth.ClaimsKey, &types.Claims{Claims: jwt.Claims{Subject: "my-sub"}, Email: "my-sub@your.org"})
156164
return NewClusterWorkflowTemplateServer(instanceid.NewService("my-instanceid"), nil, nil), ctx
@@ -203,6 +211,25 @@ func TestWorkflowTemplateServer_GetClusterWorkflowTemplate(t *testing.T) {
203211
})
204212
require.Error(t, err)
205213
})
214+
t.Run("Unauthorized", func(t *testing.T) {
215+
kubeClientSet := fake.NewSimpleClientset()
216+
kubeClientSet.PrependReactor("create", "selfsubjectaccessreviews", func(action k8stesting.Action) (bool, runtime.Object, error) {
217+
return true, &authorizationv1.SelfSubjectAccessReview{
218+
Status: authorizationv1.SubjectAccessReviewStatus{Allowed: false},
219+
}, nil
220+
})
221+
wfClientset := wftFake.NewSimpleClientset(&cwftObj2)
222+
ctx := context.WithValue(context.WithValue(context.WithValue(context.TODO(),
223+
auth.WfKey, wfClientset),
224+
auth.KubeKey, kubeClientSet),
225+
auth.ClaimsKey, &types.Claims{Claims: jwt.Claims{Subject: "my-sub"}})
226+
server := NewClusterWorkflowTemplateServer(instanceid.NewService("my-instanceid"), nil, nil)
227+
_, err := server.GetClusterWorkflowTemplate(ctx, &clusterwftmplpkg.ClusterWorkflowTemplateGetRequest{
228+
Name: "cluster-workflow-template-whalesay-template2",
229+
})
230+
require.Error(t, err)
231+
assert.Contains(t, err.Error(), "PermissionDenied")
232+
})
206233
}
207234

208235
func TestWorkflowTemplateServer_ListClusterWorkflowTemplates(t *testing.T) {

server/workflowtemplate/workflow_template_server.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"strings"
99

1010
"google.golang.org/grpc/codes"
11+
"google.golang.org/grpc/status"
1112
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1213

1314
workflowtemplatepkg "github.com/argoproj/argo-workflows/v3/pkg/apiclient/workflowtemplate"
@@ -58,6 +59,13 @@ func (wts *WorkflowTemplateServer) CreateWorkflowTemplate(ctx context.Context, r
5859
}
5960

6061
func (wts *WorkflowTemplateServer) GetWorkflowTemplate(ctx context.Context, req *workflowtemplatepkg.WorkflowTemplateGetRequest) (*v1alpha1.WorkflowTemplate, error) {
62+
allowed, err := auth.CanI(ctx, "get", "workflowtemplates", req.Namespace, req.Name)
63+
if err != nil {
64+
return nil, sutils.ToStatusError(err, codes.Internal)
65+
}
66+
if !allowed {
67+
return nil, status.Error(codes.PermissionDenied, "permission denied")
68+
}
6169
wfTmpl, err := wts.getTemplateAndValidate(ctx, req.Namespace, req.Name)
6270
if err != nil {
6371
return nil, sutils.ToStatusError(err, codes.Internal)

server/workflowtemplate/workflow_template_server_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@ import (
77
"github.com/go-jose/go-jose/v3/jwt"
88
"github.com/stretchr/testify/assert"
99
"github.com/stretchr/testify/require"
10+
authorizationv1 "k8s.io/api/authorization/v1"
1011
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
12+
"k8s.io/apimachinery/pkg/runtime"
1113
"k8s.io/client-go/kubernetes/fake"
14+
k8stesting "k8s.io/client-go/testing"
1215

1316
workflowtemplatepkg "github.com/argoproj/argo-workflows/v3/pkg/apiclient/workflowtemplate"
1417
"github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
@@ -171,6 +174,11 @@ func getWorkflowTemplateServer() (workflowtemplatepkg.WorkflowTemplateServiceSer
171174
v1alpha1.MustUnmarshal(wftStr2, &wftObj1)
172175
v1alpha1.MustUnmarshal(wftStr3, &wftObj2)
173176
kubeClientSet := fake.NewSimpleClientset()
177+
kubeClientSet.PrependReactor("create", "selfsubjectaccessreviews", func(action k8stesting.Action) (bool, runtime.Object, error) {
178+
return true, &authorizationv1.SelfSubjectAccessReview{
179+
Status: authorizationv1.SubjectAccessReviewStatus{Allowed: true},
180+
}, nil
181+
})
174182
wfClientset := wftFake.NewSimpleClientset(&unlabelledObj, &wftObj1, &wftObj2)
175183
ctx := context.WithValue(context.WithValue(context.WithValue(context.TODO(), auth.WfKey, wfClientset), auth.KubeKey, kubeClientSet), auth.ClaimsKey, &types.Claims{Claims: jwt.Claims{Subject: "my-sub"}, Email: "my-sub@your.org"})
176184
wftmplStore := NewWorkflowTemplateClientStore()
@@ -226,6 +234,30 @@ func TestWorkflowTemplateServer_GetWorkflowTemplate(t *testing.T) {
226234
_, err := server.GetWorkflowTemplate(ctx, &workflowtemplatepkg.WorkflowTemplateGetRequest{Name: "unlabelled", Namespace: "default"})
227235
require.Error(t, err)
228236
})
237+
t.Run("Unauthorized", func(t *testing.T) {
238+
kubeClientSet := fake.NewSimpleClientset()
239+
kubeClientSet.PrependReactor("create", "selfsubjectaccessreviews", func(action k8stesting.Action) (bool, runtime.Object, error) {
240+
return true, &authorizationv1.SelfSubjectAccessReview{
241+
Status: authorizationv1.SubjectAccessReviewStatus{Allowed: false},
242+
}, nil
243+
})
244+
var wftObj1 v1alpha1.WorkflowTemplate
245+
v1alpha1.MustUnmarshal(wftStr2, &wftObj1)
246+
wfClientset := wftFake.NewSimpleClientset(&wftObj1)
247+
ctx := context.WithValue(context.WithValue(context.WithValue(context.TODO(),
248+
auth.WfKey, wfClientset),
249+
auth.KubeKey, kubeClientSet),
250+
auth.ClaimsKey, &types.Claims{Claims: jwt.Claims{Subject: "my-sub"}})
251+
wftmplStore := NewWorkflowTemplateClientStore()
252+
cwftmplStore := clusterworkflowtemplate.NewClusterWorkflowTemplateClientStore()
253+
server := NewWorkflowTemplateServer(instanceid.NewService("my-instanceid"), wftmplStore, cwftmplStore)
254+
_, err := server.GetWorkflowTemplate(ctx, &workflowtemplatepkg.WorkflowTemplateGetRequest{
255+
Name: "workflow-template-whalesay-template2",
256+
Namespace: "default",
257+
})
258+
require.Error(t, err)
259+
assert.Contains(t, err.Error(), "PermissionDenied")
260+
})
229261
}
230262

231263
func TestWorkflowTemplateServer_ListWorkflowTemplates(t *testing.T) {

0 commit comments

Comments
 (0)