Skip to content

Commit

Permalink
Fixes evaluating whether cluster-admin can access a namespace
Browse files Browse the repository at this point in the history
- We don't handle wildcards in the code right now.
  • Loading branch information
foot committed Aug 10, 2022
1 parent 6ec8134 commit a4316ff
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 8 deletions.
26 changes: 20 additions & 6 deletions core/nsaccess/nsaccess.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ func hasAllRules(status authorizationv1.SubjectRulesReviewStatus, rules []rbacv1
derivedAccess := map[string]map[string]map[string]bool{}

for _, statusRule := range status.ResourceRules {
// cluster-admin etc
if containsWildcard(statusRule.APIGroups) && containsWildcard(statusRule.Resources) && containsWildcard(statusRule.Verbs) {
return true
}

for _, apiGroup := range statusRule.APIGroups {
if _, ok := derivedAccess[apiGroup]; !ok {
derivedAccess[apiGroup] = map[string]map[string]bool{}
Expand All @@ -122,12 +127,12 @@ func hasAllRules(status authorizationv1.SubjectRulesReviewStatus, rules []rbacv1
derivedAccess[apiGroup][resource] = map[string]bool{}
}

for _, verb := range statusRule.Verbs {
if verb == "*" {
for _, v := range allK8sVerbs {
derivedAccess[apiGroup][resource][v] = true
}
} else {
if containsWildcard(statusRule.Verbs) {
for _, v := range allK8sVerbs {
derivedAccess[apiGroup][resource][v] = true
}
} else {
for _, verb := range statusRule.Verbs {
derivedAccess[apiGroup][resource][verb] = true
}
}
Expand Down Expand Up @@ -172,6 +177,15 @@ Rules:
return hasAccess
}

func containsWildcard(permissions []string) bool {
for _, p := range permissions {
if p == "*" {
return true
}
}
return false
}

func newAuthClient(cfg *rest.Config) (typedauth.AuthorizationV1Interface, error) {
cs, err := kubernetes.NewForConfig(cfg)
if err != nil {
Expand Down
42 changes: 40 additions & 2 deletions core/nsaccess/nsaccess_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func TestFilterAccessibleNamespaces(t *testing.T) {

g.Expect(filtered).To(HaveLen(1))
})
t.Run("works when a user has * permissions on a resource", func(t *testing.T) {
t.Run("works when a user has * permissions on verbs", func(t *testing.T) {
g := NewGomegaWithT(t)
ns := newNamespace(context.Background(), adminClient, NewGomegaWithT(t))
defer removeNs(t, adminClient, ns)
Expand Down Expand Up @@ -244,7 +244,45 @@ func TestFilterAccessibleNamespaces(t *testing.T) {

g.Expect(filtered).To(HaveLen(1))
})
t.Run("works with cluster-admin", func(t *testing.T) {
t.Run("works when a user has * permissions on a resource", func(t *testing.T) {
t.Skip("Doesn't work right now")

g := NewGomegaWithT(t)
ns := newNamespace(context.Background(), adminClient, NewGomegaWithT(t))
defer removeNs(t, adminClient, ns)

userName = userName + "-" + rand.String(5)

roleName := makeRole(ns)

roleRules := []rbacv1.PolicyRule{
{
APIGroups: []string{""},
Resources: []string{"*"},
Verbs: []string{"*"},
},
}

userCfg := newRestConfigWithRole(t, testCfg, roleName, roleRules)

requiredRules := []rbacv1.PolicyRule{
{
APIGroups: []string{""},
Resources: []string{"secrets", "events", "namespaces"},
Verbs: []string{"get", "list"},
},
}

list := &corev1.NamespaceList{}
g.Expect(adminClient.List(ctx, list)).To(Succeed())

checker := NewChecker(requiredRules)
filtered, err := checker.FilterAccessibleNamespaces(ctx, userCfg, list.Items)
g.Expect(err).NotTo(HaveOccurred())

g.Expect(filtered).To(HaveLen(1))
})
t.Run("works with cluster-admin who have * for everything", func(t *testing.T) {
g := NewGomegaWithT(t)
ns := newNamespace(context.Background(), adminClient, NewGomegaWithT(t))
defer removeNs(t, adminClient, ns)
Expand Down

0 comments on commit a4316ff

Please sign in to comment.