Skip to content

Commit

Permalink
Filter out forbidden and terminating namespaces (#2158)
Browse files Browse the repository at this point in the history
  • Loading branch information
Andres Martinez Gotor committed Nov 12, 2020
1 parent 472e1ac commit 9d7070b
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 28 deletions.
39 changes: 26 additions & 13 deletions pkg/kube/kube_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -629,9 +629,9 @@ func KubeappsSecretNameForRepo(repoName, namespace string) string {
return fmt.Sprintf("%s-%s", namespace, secretNameForRepo(repoName))
}

func filterAllowedNamespaces(userClientset combinedClientsetInterface, namespaces *corev1.NamespaceList) (*corev1.NamespaceList, error) {
func filterAllowedNamespaces(userClientset combinedClientsetInterface, namespaces []corev1.Namespace) ([]corev1.Namespace, error) {
allowedNamespaces := []corev1.Namespace{}
for _, namespace := range namespaces.Items {
for _, namespace := range namespaces {
res, err := userClientset.AuthorizationV1().SelfSubjectAccessReviews().Create(context.TODO(), &authorizationapi.SelfSubjectAccessReview{
Spec: authorizationapi.SelfSubjectAccessReviewSpec{
ResourceAttributes: &authorizationapi.ResourceAttributes{
Expand All @@ -649,36 +649,49 @@ func filterAllowedNamespaces(userClientset combinedClientsetInterface, namespace
allowedNamespaces = append(allowedNamespaces, namespace)
}
}
namespaces.Items = allowedNamespaces
return namespaces, nil
return allowedNamespaces, nil
}

func filterActiveNamespaces(namespaces []corev1.Namespace) []corev1.Namespace {
readyNamespaces := []corev1.Namespace{}
for _, namespace := range namespaces {
if namespace.Status.Phase == corev1.NamespaceActive {
readyNamespaces = append(readyNamespaces, namespace)
}
}
return readyNamespaces
}

// GetNamespaces return the list of namespaces that the user has permission to access
func (a *userHandler) GetNamespaces() ([]corev1.Namespace, error) {
// Try to list namespaces with the user token, for backward compatibility
namespaces, err := a.clientset.CoreV1().Namespaces().List(context.TODO(), metav1.ListOptions{})
if err != nil {
// TODO: #1763 Check if we have a service token for getting namespaces on other clusters.
if k8sErrors.IsForbidden(err) {
// The user doesn't have permissions to list namespaces, use the current serviceaccount
namespaces, err = a.svcClientset.CoreV1().Namespaces().List(context.TODO(), metav1.ListOptions{})
if err != nil && k8sErrors.IsForbidden(err) {
// If the configured svcclient doesn't have permission, just return an empty list.
return []corev1.Namespace{}, nil
}

// Only if we obtained the namespaces from the svc client do we filter it using
// the user clientset.
namespaces, err = filterAllowedNamespaces(a.clientset, namespaces)
if err != nil {
return nil, err
}
} else {
return nil, err
}
}

return namespaces.Items, nil
// Filter namespaces in which the user has permissions to write (secrets) only
namespaceList, err := filterAllowedNamespaces(a.clientset, namespaces.Items)
if err != nil {
return nil, err
}

// Filter namespaces that are in terminating state
namespaceList = filterActiveNamespaces(namespaceList)
if err != nil {
return nil, err
}

return namespaceList, nil
}

// GetSecret return the a secret from a namespace using a token if given
Expand Down
62 changes: 47 additions & 15 deletions pkg/kube/kube_handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -801,49 +801,78 @@ func TestSecretForRequest(t *testing.T) {
}
}

type existingNs struct {
name string
phase corev1.NamespacePhase
}

func TestGetNamespaces(t *testing.T) {

testCases := []struct {
name string
existingNamespaces []string
existingNamespaces []existingNs
allowed bool
userClientErr error
svcClientErr error
expectedNamespaces []string
}{
{
name: "it lists namespaces if the user client returns the namespaces",
existingNamespaces: []string{"foo", "bar", "zed"},
name: "it lists namespaces if the user client returns the namespaces",
existingNamespaces: []existingNs{
{"foo", corev1.NamespaceActive},
{"bar", corev1.NamespaceActive},
{"zed", corev1.NamespaceActive},
},
expectedNamespaces: []string{"foo", "bar", "zed"},
allowed: true,
},
{
name: "it does not filter the namespaces if returned by the user client",
existingNamespaces: []string{"foo", "bar", "zed"},
expectedNamespaces: []string{"foo", "bar", "zed"},
name: "it does filter the namespaces if returned by the user client",
existingNamespaces: []existingNs{
{"foo", corev1.NamespaceActive},
{"bar", corev1.NamespaceActive},
{"zed", corev1.NamespaceActive},
},
expectedNamespaces: []string{},
allowed: false,
},
{
name: "it lists namespaces if the userclient fails but the service client succeeds",
existingNamespaces: []string{"foo"},
name: "it lists namespaces if the userclient fails but the service client succeeds",
existingNamespaces: []existingNs{
{"foo", corev1.NamespaceActive},
},
userClientErr: k8sErrors.NewForbidden(schema.GroupResource{}, "bang", fmt.Errorf("Bang")),
expectedNamespaces: []string{"foo"},
allowed: true,
},
{
name: "it filters the namespaces if the userclient fails but the service client succeeds",
existingNamespaces: []string{"foo"},
name: "it filters the namespaces if the userclient fails but the service client succeeds",
existingNamespaces: []existingNs{
{"foo", corev1.NamespaceActive},
},
userClientErr: k8sErrors.NewForbidden(schema.GroupResource{}, "bang", fmt.Errorf("Bang")),
expectedNamespaces: []string{},
allowed: false,
},
{
name: "it returns an empty list if both the user and service account forbidden",
existingNamespaces: []string{"foo"},
name: "it returns an empty list if both the user and service account forbidden",
existingNamespaces: []existingNs{
{"foo", corev1.NamespaceActive},
},
userClientErr: k8sErrors.NewForbidden(schema.GroupResource{}, "bang", fmt.Errorf("Bang")),
svcClientErr: k8sErrors.NewForbidden(schema.GroupResource{}, "bang", fmt.Errorf("Bang")),
expectedNamespaces: []string{},
allowed: true,
},
{
name: "it filters namespaces in terminating status",
existingNamespaces: []existingNs{
{"foo", corev1.NamespaceTerminating},
{"bar", corev1.NamespaceActive},
},
expectedNamespaces: []string{"bar"},
allowed: true,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
Expand Down Expand Up @@ -903,18 +932,21 @@ func TestGetNamespaces(t *testing.T) {
namespaceNames = append(namespaceNames, ns.ObjectMeta.Name)
}
if !cmp.Equal(namespaceNames, tc.expectedNamespaces) {
t.Errorf("Unexpected response: %s", cmp.Diff(namespaces, tc.expectedNamespaces))
t.Errorf("Unexpected response: %s", cmp.Diff(namespaceNames, tc.expectedNamespaces))
}
})
}
}

// setClientsetData configures the fake clientset with the return and error.
func setClientsetData(cs fakeCombinedClientset, namespaceNames []string, err error) {
func setClientsetData(cs fakeCombinedClientset, namespaceNames []existingNs, err error) {
namespaces := []corev1.Namespace{}
for _, ns := range namespaceNames {
namespaces = append(namespaces, corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: ns},
ObjectMeta: metav1.ObjectMeta{Name: ns.name},
Status: corev1.NamespaceStatus{
Phase: ns.phase,
},
})
}
cs.Clientset.Fake.PrependReactor(
Expand Down

0 comments on commit 9d7070b

Please sign in to comment.