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

verify access to a namespace before listing it in the list of namespaces #7158

Closed
mlbiam opened this issue Jun 6, 2022 · 5 comments
Closed
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@mlbiam
Copy link
Contributor

mlbiam commented Jun 6, 2022

What would you like to be added?

Presently, the dashboard lists all namespaces, even if the user has no access. There's an error in the upper right that doesn't stop users from doing their tasks, but it isn't a great UX. Instead, the dashboard can do a get on each namespace in the list to verify access. If a user can't read a namespace, don't show it. Kiali does this and it makes for a much cleaner UX in a multi-tenant environment. Here's the code kiali runs - https://github.com/kiali/kiali/blob/8191532af24f5f93c9534eada25d0557bbc0996a/business/namespaces.go#L115

I'm happy to submit a PR is the team would be willing to accept it

Why is this needed?

Limiting the namespaces listed provides for a better UX when accessing the dashboard. Users don't try to access resources they don't have access to and don't get flooded with error messages.

@mlbiam mlbiam added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 6, 2022
@floreks
Copy link
Member

floreks commented Jun 14, 2022

Duplicate of #6785

/close

@k8s-ci-robot
Copy link
Contributor

@floreks: Closing this issue.

In response to this:

Duplicate of #6785

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@mecampbellsoup
Copy link

IMO this is not a duplicate of #6785.

This feature is necessary for multi-tenant clusters in which isolation is done via namespaces.

Here is how the endpoint currently responds in e.g. a shared cluster where users' namespaces are segregated (and those users do not have list namespaces RBAC in the cluster):

{
 "listMeta": {
  "totalItems": 0
 },
 "namespaces": [],
 "errors": [
  {
   "ErrStatus": {
    "metadata": {},
    "status": "Failure",
    "message": "namespaces is forbidden: User \"mcampbell+dev+11-16-2023@coreweave.com\" cannot list resource \"namespaces\" in API group \"\" at the cluster scope",
    "reason": "Forbidden",
    "details": {
     "kind": "namespaces"
    },
    "code": 403
   }
  }
 ]
}

With this response, the user sees the following namespace dropdown:
image

So in summary: in clusters where users do not have RBAC to list namespaces, there needs to be a programmatic way to modify the list of namespaces returned by /api/v1/namespace endpoint to answer the question "what namespaces does this [auth proxied] user have access to?" An acceptable fallback alternative would be to make the namespaces list setting dynamic and/or configurable per-user as well.

Kubeapps has implemented something similar (we worked w/ them on it) where you can configure a values trustedNamespaces.headerName and trustedNamespaces.headerPattern that allow the backend API to extract "valid" accessible namespaces from another proxied request header. For kube dashboard, since you already support proxying the Impersonate-* k8s request headers, one of those could probably be used; or we could implement something similar to kubeapps where the proxied header name is entirely configurable.

https://github.com/vmware-tanzu/kubeapps/blob/main/chart/kubeapps/values.yaml#L1614-L1626

I opened a related issue here: #8496

@mlbiam
Copy link
Contributor Author

mlbiam commented Nov 20, 2023

Kubeapps has implemented something similar (we worked w/ them on it) where you can configure a values trustedNamespaces.headerName and trustedNamespaces.headerPattern that allow the backend API to extract "valid" accessible namespaces from another proxied request header.

This makes a lot of sense. there's no security issue because RBAC is enforcing everything, it's just a better UX

For kube dashboard, since you already support proxying the Impersonate-* k8s request headers, one of those could probably be used; or we could implement something similar to kubeapps where the proxied header name is entirely configurable.

I wrote the impersonation implementation, not sure I'd want to mix with impersonation because that could impact security depending on the way the the impersonation headers are enabled via RBAC

@mecampbellsoup
Copy link

@mlbiam do you know if we could open a PR and have it reviewed by the reviewers? Or what the process is for something like this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

4 participants