Skip to content

Commit 32afec3

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 4cac12c commit 32afec3

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/v4/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/v4/pkg/apiclient/clusterworkflowtemplate"
1316
"github.com/argoproj/argo-workflows/v4/pkg/apis/workflow/v1alpha1"
@@ -153,6 +156,11 @@ const userEmailLabel = "my-sub.at.your.org"
153156
func getClusterWorkflowTemplateServer(t *testing.T) (clusterwftmplpkg.ClusterWorkflowTemplateServiceServer, context.Context) {
154157
t.Helper()
155158
kubeClientSet := fake.NewSimpleClientset()
159+
kubeClientSet.PrependReactor("create", "selfsubjectaccessreviews", func(action k8stesting.Action) (bool, runtime.Object, error) {
160+
return true, &authorizationv1.SelfSubjectAccessReview{
161+
Status: authorizationv1.SubjectAccessReviewStatus{Allowed: true},
162+
}, nil
163+
})
156164
wfClientset := wftFake.NewSimpleClientset(&unlabelled, &cwftObj2, &cwftObj3)
157165
ctx := context.WithValue(logging.TestContext(t.Context()), auth.WfKey, wfClientset)
158166
ctx = context.WithValue(ctx, auth.KubeKey, kubeClientSet)
@@ -207,6 +215,25 @@ func TestWorkflowTemplateServer_GetClusterWorkflowTemplate(t *testing.T) {
207215
})
208216
require.Error(t, err)
209217
})
218+
t.Run("Unauthorized", func(t *testing.T) {
219+
kubeClientSet := fake.NewSimpleClientset()
220+
kubeClientSet.PrependReactor("create", "selfsubjectaccessreviews", func(action k8stesting.Action) (bool, runtime.Object, error) {
221+
return true, &authorizationv1.SelfSubjectAccessReview{
222+
Status: authorizationv1.SubjectAccessReviewStatus{Allowed: false},
223+
}, nil
224+
})
225+
wfClientset := wftFake.NewSimpleClientset(&cwftObj2)
226+
ctx := context.WithValue(context.WithValue(context.WithValue(logging.TestContext(t.Context()),
227+
auth.WfKey, wfClientset),
228+
auth.KubeKey, kubeClientSet),
229+
auth.ClaimsKey, &types.Claims{Claims: jwt.Claims{Subject: "my-sub"}})
230+
server := NewClusterWorkflowTemplateServer(instanceid.NewService("my-instanceid"), nil, nil)
231+
_, err := server.GetClusterWorkflowTemplate(ctx, &clusterwftmplpkg.ClusterWorkflowTemplateGetRequest{
232+
Name: "cluster-workflow-template-whalesay-template2",
233+
})
234+
require.Error(t, err)
235+
assert.Contains(t, err.Error(), "PermissionDenied")
236+
})
210237
}
211238

212239
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/v4/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
@@ -9,8 +9,11 @@ import (
99
"github.com/go-jose/go-jose/v3/jwt"
1010
"github.com/stretchr/testify/assert"
1111
"github.com/stretchr/testify/require"
12+
authorizationv1 "k8s.io/api/authorization/v1"
1213
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
14+
"k8s.io/apimachinery/pkg/runtime"
1315
"k8s.io/client-go/kubernetes/fake"
16+
k8stesting "k8s.io/client-go/testing"
1417

1518
workflowtemplatepkg "github.com/argoproj/argo-workflows/v4/pkg/apiclient/workflowtemplate"
1619
"github.com/argoproj/argo-workflows/v4/pkg/apis/workflow/v1alpha1"
@@ -174,6 +177,11 @@ func getWorkflowTemplateServer(t *testing.T) (workflowtemplatepkg.WorkflowTempla
174177
v1alpha1.MustUnmarshal(wftStr2, &wftObj1)
175178
v1alpha1.MustUnmarshal(wftStr3, &wftObj2)
176179
kubeClientSet := fake.NewSimpleClientset()
180+
kubeClientSet.PrependReactor("create", "selfsubjectaccessreviews", func(action k8stesting.Action) (bool, runtime.Object, error) {
181+
return true, &authorizationv1.SelfSubjectAccessReview{
182+
Status: authorizationv1.SubjectAccessReviewStatus{Allowed: true},
183+
}, nil
184+
})
177185
wfClientset := wftFake.NewSimpleClientset(&unlabelledObj, &wftObj1, &wftObj2)
178186
ctx := context.WithValue(context.WithValue(context.WithValue(logging.TestContext(t.Context()), auth.WfKey, wfClientset), auth.KubeKey, kubeClientSet), auth.ClaimsKey, &types.Claims{Claims: jwt.Claims{Subject: "my-sub"}, Email: "my-sub@your.org"})
179187
wftmplStore := NewWorkflowTemplateClientStore()
@@ -229,6 +237,30 @@ func TestWorkflowTemplateServer_GetWorkflowTemplate(t *testing.T) {
229237
_, err := server.GetWorkflowTemplate(ctx, &workflowtemplatepkg.WorkflowTemplateGetRequest{Name: "unlabelled", Namespace: "default"})
230238
require.Error(t, err)
231239
})
240+
t.Run("Unauthorized", func(t *testing.T) {
241+
kubeClientSet := fake.NewSimpleClientset()
242+
kubeClientSet.PrependReactor("create", "selfsubjectaccessreviews", func(action k8stesting.Action) (bool, runtime.Object, error) {
243+
return true, &authorizationv1.SelfSubjectAccessReview{
244+
Status: authorizationv1.SubjectAccessReviewStatus{Allowed: false},
245+
}, nil
246+
})
247+
var wftObj1 v1alpha1.WorkflowTemplate
248+
v1alpha1.MustUnmarshal(wftStr2, &wftObj1)
249+
wfClientset := wftFake.NewSimpleClientset(&wftObj1)
250+
ctx := context.WithValue(context.WithValue(context.WithValue(logging.TestContext(t.Context()),
251+
auth.WfKey, wfClientset),
252+
auth.KubeKey, kubeClientSet),
253+
auth.ClaimsKey, &types.Claims{Claims: jwt.Claims{Subject: "my-sub"}})
254+
wftmplStore := NewWorkflowTemplateClientStore()
255+
cwftmplStore := clusterworkflowtemplate.NewClusterWorkflowTemplateClientStore()
256+
server := NewWorkflowTemplateServer(instanceid.NewService("my-instanceid"), wftmplStore, cwftmplStore)
257+
_, err := server.GetWorkflowTemplate(ctx, &workflowtemplatepkg.WorkflowTemplateGetRequest{
258+
Name: "workflow-template-whalesay-template2",
259+
Namespace: "default",
260+
})
261+
require.Error(t, err)
262+
assert.Contains(t, err.Error(), "PermissionDenied")
263+
})
232264
}
233265

234266
func TestWorkflowTemplateServer_ListWorkflowTemplates(t *testing.T) {

0 commit comments

Comments
 (0)