Skip to content

Commit

Permalink
#47483 - Adding NonResourceURLs support to AccessStore (#299)
Browse files Browse the repository at this point in the history
* adding NonResourceURLs support to access_store

* added tests to AccessSet NonResourceURLs handling

* change on test script suggested by @tomleb + go mod tidy

* added nonresource to ext api authorization

* added NonResourceURLs implementation in Authorizes + test

* removed non-resource-url tests from the main test

* added new tests for non-resource-urls

* removed unused test data

* changed nonResourceKey to point to struct{}

* addressed comments from @tomleb

* addressed more comments

* fixing typo

* check for empty accessSet
  • Loading branch information
gehrkefc authored Nov 5, 2024
1 parent 2175e09 commit 6ee8201
Show file tree
Hide file tree
Showing 10 changed files with 588 additions and 39 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ require (
k8s.io/klog v1.0.0
k8s.io/kube-aggregator v0.31.1
k8s.io/kube-openapi v0.0.0-20240411171206-dc4e619f62f3
k8s.io/kubernetes v1.31.1
sigs.k8s.io/controller-runtime v0.19.0
)

Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,8 @@ k8s.io/kube-aggregator v0.31.1/go.mod h1:+aW4NX50uneozN+BtoCxI4g7ND922p8Wy3tWKFD
k8s.io/kube-openapi v0.0.0-20200121204235-bf4fb3bd569c/go.mod h1:GRQhZsXIAJ1xR0C9bd8UpWHZ5plfAS9fzPjJuQ6JL3E=
k8s.io/kube-openapi v0.0.0-20240411171206-dc4e619f62f3 h1:SbdLaI6mM6ffDSJCadEaD4IkuPzepLDGlkd2xV0t1uA=
k8s.io/kube-openapi v0.0.0-20240411171206-dc4e619f62f3/go.mod h1:yD4MZYeKMBwQKVht279WycxKyM84kkAx2DPrTXaeb98=
k8s.io/kubernetes v1.31.1 h1:1fcYJe8SAhtannpChbmnzHLwAV9Je99PrGaFtBvCxms=
k8s.io/kubernetes v1.31.1/go.mod h1:/YGPL//Fb9mdv5vukvAQ7Xon+Bqwry52bmjTdORAw+Q=
k8s.io/utils v0.0.0-20200324210504-a9aa75ae1b89/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew=
k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 h1:pUdcCO1Lk/tbT5ztQWOBi5HBgbBP1J8+AsQnQCKsi8A=
k8s.io/utils v0.0.0-20240711033017-18e509b52bc8/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
Expand Down
67 changes: 63 additions & 4 deletions pkg/accesscontrol/access_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,19 @@ package accesscontrol
import (
"sort"

"github.com/rancher/apiserver/pkg/types"
"github.com/rancher/steve/pkg/attributes"
v1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/sets"
rbacv1 "k8s.io/kubernetes/pkg/apis/rbac/v1"

"github.com/rancher/apiserver/pkg/types"
"github.com/rancher/steve/pkg/attributes"
)

type AccessSet struct {
ID string
set map[key]resourceAccessSet
ID string
set map[key]resourceAccessSet
nonResourceSet map[nonResourceKey]struct{}
}

type resourceAccessSet map[Access]bool
Expand All @@ -21,6 +25,11 @@ type key struct {
gr schema.GroupResource
}

type nonResourceKey struct {
verb string
url string
}

func (a *AccessSet) Namespaces() (result []string) {
set := map[string]bool{}
for k, as := range a.set {
Expand Down Expand Up @@ -56,6 +65,17 @@ func (a *AccessSet) Merge(right *AccessSet) {
m[k] = v
}
}

if a.nonResourceSet == nil {
a.nonResourceSet = map[nonResourceKey]struct{}{}
}

for k, v := range right.nonResourceSet {
_, ok := a.nonResourceSet[k]
if !ok {
a.nonResourceSet[k] = v
}
}
}

func (a AccessSet) Grants(verb string, gr schema.GroupResource, namespace, name string) bool {
Expand All @@ -80,6 +100,26 @@ func (a AccessSet) Grants(verb string, gr schema.GroupResource, namespace, name
return false
}

func (a *AccessSet) GrantsNonResource(verb, url string) bool {
if a.nonResourceSet == nil {
return false
}

if _, ok := a.nonResourceSet[nonResourceKey{url: url, verb: verb}]; ok {
rule := &v1.PolicyRule{NonResourceURLs: []string{url}, Verbs: []string{verb}}
return rbacv1.NonResourceURLMatches(rule, url) && rbacv1.VerbMatches(rule, verb)
}

for key := range a.nonResourceSet {
rule := &v1.PolicyRule{NonResourceURLs: []string{key.url}, Verbs: []string{key.verb}}
if rbacv1.NonResourceURLMatches(rule, url) && rbacv1.VerbMatches(rule, verb) {
return true
}
}

return false
}

func (a AccessSet) AccessListFor(verb string, gr schema.GroupResource) (result AccessList) {
dedup := map[Access]bool{}
for _, v := range []string{All, verb} {
Expand Down Expand Up @@ -120,6 +160,25 @@ func (a *AccessSet) Add(verb string, gr schema.GroupResource, access Access) {
}
}

func (a *AccessSet) AddNonResourceURLs(verbs, urls []string) {
if len(verbs) == 0 || len(urls) == 0 {
return
}

if a.nonResourceSet == nil {
a.nonResourceSet = map[nonResourceKey]struct{}{}
}

for _, verb := range verbs {
for _, url := range urls {
a.nonResourceSet[nonResourceKey{
verb: verb,
url: url,
}] = struct{}{}
}
}
}

type AccessListByVerb map[string]AccessList

func (a AccessListByVerb) Grants(verb, namespace, name string) bool {
Expand Down
212 changes: 212 additions & 0 deletions pkg/accesscontrol/access_set_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,212 @@
package accesscontrol

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestAccessSet_AddNonResourceURLs(t *testing.T) {
testCases := []struct {
name string
verbs []string
urls []string
want []nonResourceKey
}{
{
name: "valid case",
verbs: []string{"get", "post"},
urls: []string{"/healthz", "/metrics"},
want: []nonResourceKey{
{"get", "/healthz"},
{"get", "/metrics"},
{"post", "/healthz"},
{"post", "/metrics"},
},
},
{
name: "url wildcard",
verbs: []string{"get"},
urls: []string{"/metrics/*"},
want: []nonResourceKey{
{"get", "/metrics/*"},
},
},
{
name: "verb wildcard",
verbs: []string{"*"},
urls: []string{"/metrics"},
want: []nonResourceKey{
{"*", "/metrics"},
},
},
{
name: "empty urls",
verbs: []string{"get", "post"},
urls: []string{},
want: nil,
},
{
name: "empty verbs",
verbs: []string{},
urls: []string{"/healthz", "/metrics"},
want: nil,
},
}

for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
accessSet := &AccessSet{}
accessSet.AddNonResourceURLs(tt.verbs, tt.urls)

if len(tt.want) > 0 {
for _, key := range tt.want {
assert.Contains(t, accessSet.nonResourceSet, key)
}
} else {
assert.Len(t, accessSet.nonResourceSet, 0)
}
})
}
}

func TestAccessSet_GrantsNonResource(t *testing.T) {
testCases := []struct {
name string
verb string
url string
keys map[nonResourceKey]struct{}
expect bool
}{
{
name: "direct match",
verb: "get",
url: "/healthz",
keys: map[nonResourceKey]struct{}{
{verb: "get", url: "/healthz"}: {},
},
expect: true,
},
{
name: "wildcard in url",
verb: "get",
url: "/api/resource",
keys: map[nonResourceKey]struct{}{
{verb: "get", url: "/api/*"}: {},
},
expect: true,
},
{
name: "wildcard in verb",
verb: "get",
url: "/healthz",
keys: map[nonResourceKey]struct{}{
{verb: "*", url: "/healthz"}: {},
},
expect: true,
},
{
name: "invalid wildcard",
verb: "get",
url: "/*", // that's invalid according to k8s rules
keys: map[nonResourceKey]struct{}{
{verb: "get", url: "/api/*"}: {},
},
expect: false,
},
{
name: "wrong verb",
verb: "post",
url: "/healthz",
keys: map[nonResourceKey]struct{}{
{verb: "get", url: "/healthz"}: {},
},
expect: false,
},
{
name: "wrong url",
verb: "post",
url: "/metrics",
keys: map[nonResourceKey]struct{}{
{verb: "post", url: "/healthz"}: {},
},
expect: false,
},
{
name: "no matching rule",
verb: "post",
url: "/healthz",
keys: map[nonResourceKey]struct{}{},
expect: false,
},
}

for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
accessSet := &AccessSet{}

for rule := range tt.keys {
accessSet.AddNonResourceURLs([]string{rule.verb}, []string{rule.url})
}

res := accessSet.GrantsNonResource(tt.verb, tt.url)
assert.Equal(t, tt.expect, res)
})
}
}

func TestAccessSet_Merge(t *testing.T) {
testCases := []struct {
name string
left *AccessSet
right *AccessSet
want *AccessSet
}{
{
name: "merging NonResouceURLs",
left: &AccessSet{
nonResourceSet: map[nonResourceKey]struct{}{
{url: "/healthz", verb: "get"}: {},
},
},
right: &AccessSet{
nonResourceSet: map[nonResourceKey]struct{}{
{url: "/metrics", verb: "post"}: {},
},
},
want: &AccessSet{
nonResourceSet: map[nonResourceKey]struct{}{
{url: "/healthz", verb: "get"}: {},
{url: "/metrics", verb: "post"}: {},
},
},
},
{
name: "merging NonResouceURLs - repeated items",
left: &AccessSet{
nonResourceSet: map[nonResourceKey]struct{}{
{url: "/healthz", verb: "get"}: {},
{url: "/metrics", verb: "post"}: {},
},
},
right: &AccessSet{
nonResourceSet: map[nonResourceKey]struct{}{
{url: "/metrics", verb: "post"}: {},
},
},
want: &AccessSet{
nonResourceSet: map[nonResourceKey]struct{}{
{url: "/healthz", verb: "get"}: {},
{url: "/metrics", verb: "post"}: {},
},
},
},
}

for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
tt.left.Merge(tt.right)
assert.Equal(t, tt.want, tt.left)
})
}
}
Loading

0 comments on commit 6ee8201

Please sign in to comment.