Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add authorizationGroups property to K8s SAR authorization, fixes #506 #507

Merged
merged 5 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions api/v1beta3/auth_config_types.go
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.
dhirajsb marked this conversation as resolved.
Show resolved Hide resolved
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"`
dhirajsb marked this conversation as resolved.
Show resolved Hide resolved

// 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

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
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
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
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
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
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")
}
Loading