From ebd44131c94ef62d4f192552591d3a739d031603 Mon Sep 17 00:00:00 2001 From: Mritunjay Kumar Sharma Date: Tue, 26 Apr 2022 20:03:58 +0530 Subject: [PATCH] Logic of match service account is fixed for namespace (#3662) * attempt to implement new logic for roleRef Signed-off-by: Mritunjay Sharma * fixes match subject map logic Signed-off-by: Mritunjay Sharma * changes namespace for clusterRolebinding Signed-off-by: Mritunjay Sharma * adds tests Signed-off-by: Mritunjay Sharma * fixes in tests Signed-off-by: Mritunjay Sharma * fixes in tests Signed-off-by: Mritunjay Sharma --- pkg/userinfo/roleRef.go | 15 +++++++++------ pkg/userinfo/roleRef_test.go | 12 ++++++------ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/pkg/userinfo/roleRef.go b/pkg/userinfo/roleRef.go index 636432de7f..1a56dcd190 100644 --- a/pkg/userinfo/roleRef.go +++ b/pkg/userinfo/roleRef.go @@ -11,6 +11,7 @@ import ( rbacv1 "k8s.io/api/rbac/v1" labels "k8s.io/apimachinery/pkg/labels" rbaclister "k8s.io/client-go/listers/rbac/v1" + "sigs.k8s.io/controller-runtime/pkg/log" ) @@ -45,7 +46,7 @@ func GetRoleRef(rbLister rbaclister.RoleBindingLister, crbLister rbaclister.Clus func getRoleRefByRoleBindings(roleBindings []*rbacv1.RoleBinding, userInfo authenticationv1.UserInfo) (roles []string, clusterRoles []string) { for _, rolebinding := range roleBindings { for _, subject := range rolebinding.Subjects { - if matchSubjectsMap(subject, userInfo) { + if matchSubjectsMap(subject, userInfo, rolebinding.Namespace) { switch rolebinding.RoleRef.Kind { case roleKind: roles = append(roles, rolebinding.Namespace+":"+rolebinding.RoleRef.Name) @@ -62,7 +63,7 @@ func getRoleRefByRoleBindings(roleBindings []*rbacv1.RoleBinding, userInfo authe func getRoleRefByClusterRoleBindings(clusterroleBindings []*rbacv1.ClusterRoleBinding, userInfo authenticationv1.UserInfo) (clusterRoles []string) { for _, clusterRoleBinding := range clusterroleBindings { for _, subject := range clusterRoleBinding.Subjects { - if matchSubjectsMap(subject, userInfo) { + if matchSubjectsMap(subject, userInfo, subject.Namespace) { if clusterRoleBinding.RoleRef.Kind == clusterroleKind { clusterRoles = append(clusterRoles, clusterRoleBinding.RoleRef.Name) } @@ -75,17 +76,19 @@ func getRoleRefByClusterRoleBindings(clusterroleBindings []*rbacv1.ClusterRoleBi // matchSubjectsMap checks if userInfo found in subject // return true directly if found a match // subject.kind can only be ServiceAccount, User and Group -func matchSubjectsMap(subject rbacv1.Subject, userInfo authenticationv1.UserInfo) bool { +func matchSubjectsMap(subject rbacv1.Subject, userInfo authenticationv1.UserInfo, namespace string) bool { + if strings.Contains(userInfo.Username, saPrefix) { - return matchServiceAccount(subject, userInfo) + return matchServiceAccount(subject, userInfo, namespace) + } return matchUserOrGroup(subject, userInfo) } // matchServiceAccount checks if userInfo sa matche the subject sa // serviceaccount represents as saPrefix:namespace:name in userInfo -func matchServiceAccount(subject rbacv1.Subject, userInfo authenticationv1.UserInfo) bool { - subjectServiceAccount := subject.Namespace + ":" + subject.Name +func matchServiceAccount(subject rbacv1.Subject, userInfo authenticationv1.UserInfo, namespace string) bool { + subjectServiceAccount := namespace + ":" + subject.Name if userInfo.Username[len(saPrefix):] != subjectServiceAccount { return false } diff --git a/pkg/userinfo/roleRef_test.go b/pkg/userinfo/roleRef_test.go index 3d6575c57f..bb84e2f99c 100644 --- a/pkg/userinfo/roleRef_test.go +++ b/pkg/userinfo/roleRef_test.go @@ -61,7 +61,7 @@ func Test_matchServiceAccount_subject_variants(t *testing.T) { } for _, test := range tests { - res := matchServiceAccount(test.subject, userInfo) + res := matchServiceAccount(test.subject, userInfo, test.subject.Namespace) assert.Equal(t, test.expected, res) } } @@ -131,10 +131,10 @@ func Test_matchSubjectsMap(t *testing.T) { Name: "fakeGroup", } - res := matchSubjectsMap(sasubject, sa) + res := matchSubjectsMap(sasubject, sa, sasubject.Namespace) assert.Assert(t, res) - res = matchSubjectsMap(groupsubject, group) + res = matchSubjectsMap(groupsubject, group, "") assert.Assert(t, !res) } @@ -158,7 +158,7 @@ func Test_getRoleRefByRoleBindings(t *testing.T) { list := make([]*rbacv1.RoleBinding, 2) - list[0] = newRoleBinding("test1", "mynamespace", + list[0] = newRoleBinding("test1", "default", []rbacv1.Subject{ { Kind: "ServiceAccount", @@ -171,7 +171,7 @@ func Test_getRoleRefByRoleBindings(t *testing.T) { }, ) - list[1] = newRoleBinding("test2", "mynamespace", + list[1] = newRoleBinding("test2", "default", []rbacv1.Subject{ { Kind: "ServiceAccount", @@ -188,7 +188,7 @@ func Test_getRoleRefByRoleBindings(t *testing.T) { Username: "system:serviceaccount:default:saconfig", } - expectedrole := []string{"mynamespace:myrole"} + expectedrole := []string{list[0].Namespace + ":" + "myrole"} expectedClusterRole := []string{"myclusterrole"} roles, clusterroles := getRoleRefByRoleBindings(list, sa) assert.DeepEqual(t, roles, expectedrole)