Skip to content

Commit

Permalink
Merge pull request #507 from dhirajsb/fix/k8s-authz-groups
Browse files Browse the repository at this point in the history 
fix: add authorizationGroups property to K8s SAR authorization, fixes #506
  • Loading branch information
@guicassolato
guicassolato authored Nov 21, 2024
2 parents ca4e46e + 1353e29 commit a1d035d
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 47 deletions.
4 changes: 4 additions & 0 deletions api/v1beta3/auth_config_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -620,8 +620,12 @@ type KubernetesSubjectAccessReviewAuthorizationSpec struct {
User *ValueOrSelector `json:"user,omitempty"`

// Groups the user must be a member of or, if `user` is omitted, the groups to check for authorization in the Kubernetes RBAC.
// Deprecated: Use authorizationGroups instead.
Groups []string `json:"groups,omitempty"`

// Groups to check for existing permission in the Kubernetes RBAC alternatively to a specific user. This is typically obtained from a list of groups the user is a member of. Must be a static list of group names or dynamically resolve to one from the Authorization JSON.
AuthorizationGroups *ValueOrSelector `json:"authorizationGroups,omitempty"`

// Use resourceAttributes to check permissions on Kubernetes resources.
// If omitted, it performs a non-resource SubjectAccessReview, with verb and path inferred from the request.
// +optional
Expand Down
5 changes: 5 additions & 0 deletions api/v1beta3/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 12 additions & 1 deletion controllers/auth_config_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,18 @@ func (r *AuthConfigReconciler) translateAuthConfig(ctx context.Context, authConf
}
}

translatedAuthorization.KubernetesAuthz, err = authorization_evaluators.NewKubernetesAuthz(authorinoUser, authorization.KubernetesSubjectAccessReview.Groups, authorinoResourceAttributes)
var authorinoGroups expressions.Value
// look for authorizationGroups first
if authorization.KubernetesSubjectAccessReview.AuthorizationGroups != nil {
authorinoGroups, err = valueFrom(authorization.KubernetesSubjectAccessReview.AuthorizationGroups)
if err != nil {
return nil, err
}
} else if len(authorization.KubernetesSubjectAccessReview.Groups) > 0 {
// use deprecated Groups property otherwise
authorinoGroups = &json.JSONValue{Static: authorization.KubernetesSubjectAccessReview.Groups}
}
translatedAuthorization.KubernetesAuthz, err = authorization_evaluators.NewKubernetesAuthz(authorinoUser, authorinoGroups, authorinoResourceAttributes)
if err != nil {
return nil, err
}
Expand Down
28 changes: 25 additions & 3 deletions install/crd/authorino.kuadrant.io_authconfigs.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2796,10 +2796,32 @@ spec:
kubernetesSubjectAccessReview:
description: Authorization by Kubernetes SubjectAccessReview
properties:
authorizationGroups:
description: Groups to check for existing permission in
the Kubernetes RBAC alternatively to a specific user.
This is typically obtained from a list of groups the user
is a member of. Must be a static list of group names or
dynamically resolve to one from the Authorization JSON.
properties:
expression:
description: |-
A Common Expression Language (CEL) expression that evaluates to a value.
String expressions are supported (https://pkg.go.dev/github.com/google/cel-go/ext#Strings).
type: string
selector:
description: |-
Simple path selector to fetch content from the authorization JSON (e.g. 'request.method') or a string template with variables that resolve to patterns (e.g. "Hello, {auth.identity.name}!").
Any pattern supported by https://pkg.go.dev/github.com/tidwall/gjson can be used.
The following Authorino custom modifiers are supported: @extract:{sep:" ",pos:0}, @replace{old:"",new:""}, @case:upper|lower, @base64:encode|decode and @strip.
type: string
value:
description: Static value
x-kubernetes-preserve-unknown-fields: true
type: object
groups:
description: Groups the user must be a member of or, if
`user` is omitted, the groups to check for authorization
in the Kubernetes RBAC.
description: |-
Groups the user must be a member of or, if `user` is omitted, the groups to check for authorization in the Kubernetes RBAC.
Deprecated: Use authorizationGroups instead.
items:
type: string
type: array
Expand Down
28 changes: 25 additions & 3 deletions install/manifests.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3104,10 +3104,32 @@ spec:
kubernetesSubjectAccessReview:
description: Authorization by Kubernetes SubjectAccessReview
properties:
authorizationGroups:
description: Groups to check for existing permission in
the Kubernetes RBAC alternatively to a specific user.
This is typically obtained from a list of groups the user
is a member of. Must be a static list of group names or
dynamically resolve to one from the Authorization JSON.
properties:
expression:
description: |-
A Common Expression Language (CEL) expression that evaluates to a value.
String expressions are supported (https://pkg.go.dev/github.com/google/cel-go/ext#Strings).
type: string
selector:
description: |-
Simple path selector to fetch content from the authorization JSON (e.g. 'request.method') or a string template with variables that resolve to patterns (e.g. "Hello, {auth.identity.name}!").
Any pattern supported by https://pkg.go.dev/github.com/tidwall/gjson can be used.
The following Authorino custom modifiers are supported: @extract:{sep:" ",pos:0}, @replace{old:"",new:""}, @case:upper|lower, @base64:encode|decode and @strip.
type: string
value:
description: Static value
x-kubernetes-preserve-unknown-fields: true
type: object
groups:
description: Groups the user must be a member of or, if
`user` is omitted, the groups to check for authorization
in the Kubernetes RBAC.
description: |-
Groups the user must be a member of or, if `user` is omitted, the groups to check for authorization in the Kubernetes RBAC.
Deprecated: Use authorizationGroups instead.
items:
type: string
type: array
Expand Down
30 changes: 20 additions & 10 deletions pkg/evaluators/authorization/kubernetes_authz.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package authorization

import (
gocontext "context"
gojson "encoding/json"
"fmt"
"strings"

Expand All @@ -22,7 +23,7 @@ type kubernetesSubjectAccessReviewer interface {
SubjectAccessReviews() kubeAuthzClient.SubjectAccessReviewInterface
}

func NewKubernetesAuthz(user expressions.Value, groups []string, resourceAttributes *KubernetesAuthzResourceAttributes) (*KubernetesAuthz, error) {
func NewKubernetesAuthz(user expressions.Value, authorizationGroups expressions.Value, resourceAttributes *KubernetesAuthzResourceAttributes) (*KubernetesAuthz, error) {
config, err := rest.InClusterConfig()
if err != nil {
return nil, err
Expand All @@ -34,10 +35,10 @@ func NewKubernetesAuthz(user expressions.Value, groups []string, resourceAttribu
}

return &KubernetesAuthz{
User: user,
Groups: groups,
ResourceAttributes: resourceAttributes,
authorizer: k8sClient.AuthorizationV1(),
User: user,
AuthorizationGroups: authorizationGroups,
ResourceAttributes: resourceAttributes,
authorizer: k8sClient.AuthorizationV1(),
}, nil
}

Expand All @@ -51,9 +52,9 @@ type KubernetesAuthzResourceAttributes struct {
}

type KubernetesAuthz struct {
User expressions.Value
Groups []string
ResourceAttributes *KubernetesAuthzResourceAttributes
User expressions.Value
AuthorizationGroups expressions.Value
ResourceAttributes *KubernetesAuthzResourceAttributes

authorizer kubernetesSubjectAccessReviewer
}
Expand Down Expand Up @@ -129,8 +130,17 @@ func (k *KubernetesAuthz) Call(pipeline auth.AuthPipeline, ctx gocontext.Context
}
}

if len(k.Groups) > 0 {
subjectAccessReview.Spec.Groups = k.Groups
if k.AuthorizationGroups != nil {
stringJson, err := jsonValueToStr(k.AuthorizationGroups)
if err != nil {
return nil, err
}
var resolvedGroups []string
err = gojson.Unmarshal([]byte(stringJson), &resolvedGroups)
if err != nil {
return nil, err
}
subjectAccessReview.Spec.Groups = resolvedGroups
}

log.FromContext(ctx).WithName("kubernetesauthz").V(1).Info("calling kubernetes subject access review api", "subjectaccessreview", subjectAccessReview)
Expand Down
45 changes: 15 additions & 30 deletions pkg/evaluators/authorization/kubernetes_authz_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
envoy_auth "github.com/envoyproxy/go-control-plane/envoy/service/auth/v3"
"github.com/golang/mock/gomock"
"gotest.tools/assert"
is "gotest.tools/assert/cmp"
kubeAuthz "k8s.io/api/authorization/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kubeAuthzClient "k8s.io/client-go/kubernetes/typed/authorization/v1"
Expand Down Expand Up @@ -59,11 +60,11 @@ func (client *k8sAuthorizationClientMock) GetRequest() kubeAuthz.SubjectAccessRe
return client.request
}

func newKubernetesAuthz(user expressions.Value, groups []string, resourceAttributes *KubernetesAuthzResourceAttributes, subjectAccessReviewResponseStatus kubeAuthz.SubjectAccessReviewStatus) *KubernetesAuthz {
func newKubernetesAuthz(user expressions.Value, authorizationGroups expressions.Value, resourceAttributes *KubernetesAuthzResourceAttributes, subjectAccessReviewResponseStatus kubeAuthz.SubjectAccessReviewStatus) *KubernetesAuthz {
return &KubernetesAuthz{
User: user,
Groups: groups,
ResourceAttributes: resourceAttributes,
User: user,
AuthorizationGroups: authorizationGroups,
ResourceAttributes: resourceAttributes,

// mock the authorizer so we can control the response
authorizer: &k8sAuthorizationClientMock{SubjectAccessReviewStatus: subjectAccessReviewResponseStatus},
Expand All @@ -75,22 +76,18 @@ func TestKubernetesAuthzNonResource_Allowed(t *testing.T) {
defer ctrl.Finish()

pipelineMock := mock_auth.NewMockAuthPipeline(ctrl)
pipelineMock.EXPECT().GetAuthorizationJSON().Return(`{"context":{"request":{"http":{"method":"GET","path":"/hello"}}},"auth":{"identity":{"username":"john"}}}`)
pipelineMock.EXPECT().GetAuthorizationJSON().Return(`{"context":{"request":{"http":{"method":"GET","path":"/hello"}}},"auth":{"identity":{"username":"john", "groups":["group1","group2"]}}}`)

request := &envoy_auth.AttributeContext_HttpRequest{Method: "GET", Path: "/hello"}
pipelineMock.EXPECT().GetHttp().Return(request)

kubernetesAuth := newKubernetesAuthz(
&json.JSONValue{Pattern: "auth.identity.username"},
[]string{},
nil,
kubeAuthz.SubjectAccessReviewStatus{Allowed: true, Reason: ""},
)
kubernetesAuth := newKubernetesAuthz(&json.JSONValue{Pattern: "auth.identity.username"}, &json.JSONValue{Pattern: "auth.identity.groups"}, nil, kubeAuthz.SubjectAccessReviewStatus{Allowed: true, Reason: ""})
authorized, err := kubernetesAuth.Call(pipelineMock, context.TODO())

client, _ := kubernetesAuth.authorizer.(subjectAccessReviewTestClient)
requestData := client.GetRequest()
assert.Equal(t, requestData.User, "john")
assert.DeepEqual(t, requestData.Groups, []string{"group1", "group2"})
assert.Equal(t, requestData.NonResourceAttributes.Path, "/hello")
assert.Equal(t, requestData.NonResourceAttributes.Verb, "get")

Expand All @@ -108,17 +105,13 @@ func TestKubernetesAuthzNonResource_Denied(t *testing.T) {
request := &envoy_auth.AttributeContext_HttpRequest{Method: "GET", Path: "/hello"}
pipelineMock.EXPECT().GetHttp().Return(request)

kubernetesAuth := newKubernetesAuthz(
&json.JSONValue{Pattern: "auth.identity.username"},
[]string{},
nil,
kubeAuthz.SubjectAccessReviewStatus{Allowed: false, Reason: "some-reason"},
)
kubernetesAuth := newKubernetesAuthz(&json.JSONValue{Pattern: "auth.identity.username"}, nil, nil, kubeAuthz.SubjectAccessReviewStatus{Allowed: false, Reason: "some-reason"})
authorized, err := kubernetesAuth.Call(pipelineMock, context.TODO())

client, _ := kubernetesAuth.authorizer.(subjectAccessReviewTestClient)
requestData := client.GetRequest()
assert.Equal(t, requestData.User, "john")
assert.Assert(t, is.Len(requestData.Groups, 0))
assert.Equal(t, requestData.NonResourceAttributes.Path, "/hello")
assert.Equal(t, requestData.NonResourceAttributes.Verb, "get")

Expand All @@ -131,14 +124,9 @@ func TestKubernetesAuthzResource_Allowed(t *testing.T) {
defer ctrl.Finish()

pipelineMock := mock_auth.NewMockAuthPipeline(ctrl)
pipelineMock.EXPECT().GetAuthorizationJSON().Return(`{"context":{"request":{"http":{"method":"GET","path":"/hello"}}},"auth":{"identity":{"username":"john"}}}`)
pipelineMock.EXPECT().GetAuthorizationJSON().Return(`{"context":{"request":{"http":{"method":"GET","path":"/hello"}}},"auth":{"identity":{"username":"john", "groups":["group1","group2"]}}}`)

kubernetesAuth := newKubernetesAuthz(
&json.JSONValue{Pattern: "auth.identity.username"},
[]string{},
&KubernetesAuthzResourceAttributes{Namespace: &json.JSONValue{Static: "default"}},
kubeAuthz.SubjectAccessReviewStatus{Allowed: true, Reason: ""},
)
kubernetesAuth := newKubernetesAuthz(&json.JSONValue{Pattern: "auth.identity.username"}, &json.JSONValue{Pattern: "auth.identity.groups"}, &KubernetesAuthzResourceAttributes{Namespace: &json.JSONValue{Static: "default"}}, kubeAuthz.SubjectAccessReviewStatus{Allowed: true, Reason: ""})
authorized, err := kubernetesAuth.Call(pipelineMock, context.TODO())

assert.Check(t, authorized.(bool))
Expand All @@ -147,6 +135,7 @@ func TestKubernetesAuthzResource_Allowed(t *testing.T) {
client, _ := kubernetesAuth.authorizer.(subjectAccessReviewTestClient)
requestData := client.GetRequest()
assert.Equal(t, requestData.User, "john")
assert.DeepEqual(t, requestData.Groups, []string{"group1", "group2"})
assert.Equal(t, requestData.ResourceAttributes.Namespace, "default")
}

Expand All @@ -157,12 +146,7 @@ func TestKubernetesAuthzResource_Denied(t *testing.T) {
pipelineMock := mock_auth.NewMockAuthPipeline(ctrl)
pipelineMock.EXPECT().GetAuthorizationJSON().Return(`{"context":{"request":{"http":{"method":"GET","path":"/hello"}}},"auth":{"identity":{"username":"john"}}}`)

kubernetesAuth := newKubernetesAuthz(
&json.JSONValue{Pattern: "auth.identity.username"},
[]string{},
&KubernetesAuthzResourceAttributes{Namespace: &json.JSONValue{Static: "default"}},
kubeAuthz.SubjectAccessReviewStatus{Allowed: false, Reason: "some-reason"},
)
kubernetesAuth := newKubernetesAuthz(&json.JSONValue{Pattern: "auth.identity.username"}, &json.JSONValue{Static: []string{"group1", "group2"}}, &KubernetesAuthzResourceAttributes{Namespace: &json.JSONValue{Static: "default"}}, kubeAuthz.SubjectAccessReviewStatus{Allowed: false, Reason: "some-reason"})
authorized, err := kubernetesAuth.Call(pipelineMock, context.TODO())

assert.Check(t, !authorized.(bool))
Expand All @@ -171,5 +155,6 @@ func TestKubernetesAuthzResource_Denied(t *testing.T) {
client, _ := kubernetesAuth.authorizer.(subjectAccessReviewTestClient)
requestData := client.GetRequest()
assert.Equal(t, requestData.User, "john")
assert.DeepEqual(t, requestData.Groups, []string{"group1", "group2"})
assert.Equal(t, requestData.ResourceAttributes.Namespace, "default")
}

0 comments on commit a1d035d

Please sign in to comment.