Allow to discover namespaces (#1507)

* Allow to discover namespaces

* TODO comment

chart/kubeapps/templates/kubeops-rbac.yaml

@@ -40,5 +40,42 @@ subjects:
   - kind: ServiceAccount
     name: {{ template "kubeapps.kubeops.fullname" . }}
     namespace: {{ .Release.Namespace }}
+{{- if .Values.allowNamespaceDiscovery }}
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: {{ template "kubeapps.kubeops.fullname" . }}-ns-discovery
+  labels:
+    app: {{ template "kubeapps.kubeops.fullname" . }}
+    chart: {{ template "kubeapps.chart" . }}
+    release: {{ .Release.Name }}
+    heritage: {{ .Release.Service }}
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - namespaces
+  verbs:
+  - list
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: {{ template "kubeapps.kubeops.fullname" . }}-ns-discovery
+  labels:
+    app: {{ template "kubeapps.kubeops.fullname" . }}
+    chart: {{ template "kubeapps.chart" . }}
+    release: {{ .Release.Name }}
+    heritage: {{ .Release.Service }}
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: {{ template "kubeapps.kubeops.fullname" . }}-ns-discovery
+subjects:
+  - kind: ServiceAccount
+    name: {{ template "kubeapps.kubeops.fullname" . }}
+    namespace: {{ .Release.Namespace }}
+{{- end -}}
 {{- end -}}
 {{- end }}{{/* matches useHelm3 */}}

chart/kubeapps/templates/tiller-proxy-rbac.yaml

@@ -40,5 +40,42 @@ subjects:
   - kind: ServiceAccount
     name: {{ template "kubeapps.tiller-proxy.fullname" . }}
     namespace: {{ .Release.Namespace }}
+{{- if .Values.allowNamespaceDiscovery }}
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: {{ template "kubeapps.tiller-proxy.fullname" . }}-ns-discovery
+  labels:
+    app: {{ template "kubeapps.tiller-proxy.fullname" . }}
+    chart: {{ template "kubeapps.chart" . }}
+    release: {{ .Release.Name }}
+    heritage: {{ .Release.Service }}
+rules:
+- apiGroups:
+    - ""
+  resources:
+    - namespaces
+  verbs:
+    - list
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: {{ template "kubeapps.tiller-proxy.fullname" . }}-ns-discovery
+  labels:
+    app: {{ template "kubeapps.tiller-proxy.fullname" . }}
+    chart: {{ template "kubeapps.chart" . }}
+    release: {{ .Release.Name }}
+    heritage: {{ .Release.Service }}
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: {{ template "kubeapps.tiller-proxy.fullname" . }}-ns-discovery
+subjects:
+  - kind: ServiceAccount
+    name: {{ template "kubeapps.tiller-proxy.fullname" . }}
+    namespace: {{ .Release.Namespace }}
+{{- end -}}
 {{- end -}}
 {{- end }}{{/* matches useHelm3 */}}

chart/kubeapps/values.yaml

@@ -12,6 +12,10 @@
 ## If you set it to true, Kubeapps will not work with releases installed with Helm 2.
 useHelm3: false
 
+## Enable this feature flag to allow users to discover available namespaces (only the ones they have access).
+## If you set it to true, Kubeapps create a ClusterRole to be able to list namespaces.
+allowNamespaceDiscovery: false
+
 ## The frontend service is the main reverse proxy used to access the Kubeapps UI
 ## To expose Kubeapps externally either configure the ingress object below or
 ## set frontend.service.type=LoadBalancer in the frontend configuration.

cmd/kubeops/main.go

@@ -86,6 +86,9 @@ func main() {
 	backendAPIv1.Methods("POST").Path("/apprepositories").Handler(negroni.New(
 		negroni.WrapFunc(appreposHandler.Create),
 	))
+	backendAPIv1.Methods("GET").Path("/namespaces").Handler(negroni.New(
+		negroni.WrapFunc(appreposHandler.GetNamespaces),
+	))
 
 	// assetsvc reverse proxy
 	authGate := auth.AuthGate()

cmd/tiller-proxy/main.go

@@ -192,6 +192,9 @@ func main() {
 	backendAPIv1.Methods("POST").Path("/apprepositories").Handler(negroni.New(
 		negroni.WrapFunc(appreposHandler.Create),
 	))
+	backendAPIv1.Methods("GET").Path("/namespaces").Handler(negroni.New(
+		negroni.WrapFunc(appreposHandler.GetNamespaces),
+	))
 
 	// assetsvc reverse proxy
 	parsedAssetsvcURL, err := url.Parse(assetsvcURL)

dashboard/src/actions/namespace.test.tsx

@@ -56,9 +56,7 @@ actionTestCases.forEach(tc => {
 describe("fetchNamespaces", () => {
   it("dispatches the list of namespace names if no error", async () => {
     Namespace.list = jest.fn().mockImplementationOnce(() => {
-      return {
-        items: [{ metadata: { name: "overlook-hotel" } }, { metadata: { name: "room-217" } }],
-      };
+      return [{ metadata: { name: "overlook-hotel" } }, { metadata: { name: "room-217" } }];
     });
     const expectedActions = [
       {
@@ -91,9 +89,7 @@ describe("createNamespace", () => {
   it("dispatches the new namespace and re-fetch namespaces", async () => {
     Namespace.create = jest.fn();
     Namespace.list = jest.fn().mockImplementationOnce(() => {
-      return {
-        items: [{ metadata: { name: "overlook-hotel" } }, { metadata: { name: "room-217" } }],
-      };
+      return [{ metadata: { name: "overlook-hotel" } }, { metadata: { name: "room-217" } }];
     });
     const expectedActions = [
       {

dashboard/src/actions/namespace.ts

@@ -45,7 +45,7 @@ export function fetchNamespaces(): ThunkAction<Promise<void>, IStoreState, null,
   return async dispatch => {
     try {
       const namespaces = await Namespace.list();
-      const namespaceStrings = namespaces.items.map((n: IResource) => n.metadata.name);
+      const namespaceStrings = namespaces.map((n: IResource) => n.metadata.name);
       dispatch(receiveNamespaces(namespaceStrings));
     } catch (e) {
       dispatch(errorNamespaces(e, "list"));

dashboard/src/shared/Namespace.ts

@@ -1,11 +1,12 @@
 import { axiosWithAuth } from "./AxiosInstance";
 import { APIBase } from "./Kube";
+import * as url from "./url";
 
-import { ForbiddenError, IK8sList, IResource, NotFoundError } from "./types";
+import { ForbiddenError, IResource, NotFoundError } from "./types";
 
 export default class Namespace {
   public static async list() {
-    const { data } = await axiosWithAuth.get<IK8sList<IResource, {}>>(`${Namespace.APIEndpoint}`);
+    const { data } = await axiosWithAuth.get<IResource[]>(url.backend.namespaces.list());
     return data;
   }
 

dashboard/src/shared/url.ts

@@ -15,6 +15,10 @@ export const backend = {
     base: "api/v1/apprepositories",
     create: () => `${backend.apprepositories.base}`,
   },
+  namespaces: {
+    base: "api/v1/namespaces",
+    list: () => `${backend.namespaces.base}`,
+  },
 };
 
 export const api = {

go.sum

@@ -568,6 +568,7 @@ k8s.io/apiextensions-apiserver v0.0.0-20191016113550-5357c4baaf65 h1:kThoiqgMsSw
 k8s.io/apiextensions-apiserver v0.0.0-20191016113550-5357c4baaf65/go.mod h1:5BINdGqggRXXKnDgpwoJ7PyQH8f+Ypp02fvVNcIFy9s=
 k8s.io/apimachinery v0.0.0-20191004115801-a2eda9f80ab8 h1:Iieh/ZEgT3BWwbLD5qEKcY06jKuPEl6zC7gPSehoLw4=
 k8s.io/apimachinery v0.0.0-20191004115801-a2eda9f80ab8/go.mod h1:llRdnznGEAqC3DcNm6yEj472xaFVfLM7hnYofMb12tQ=
+k8s.io/apimachinery v0.17.2 h1:hwDQQFbdRlpnnsR64Asdi55GyCaIP/3WQpMmbNBeWr4=
 k8s.io/apiserver v0.0.0-20191016112112-5190913f932d/go.mod h1:7OqfAolfWxUM/jJ/HBLyE+cdaWFBUoo5Q5pHgJVj2ws=
 k8s.io/cli-runtime v0.0.0-20191016114015-74ad18325ed5 h1:8ZfMjkMBzcXEawLsYHg9lDM7aLEVso3NiVKfUTnN56A=
 k8s.io/cli-runtime v0.0.0-20191016114015-74ad18325ed5/go.mod h1:sDl6WKSQkDM6zS1u9F49a0VooQ3ycYFBFLqd2jf2Xfo=

pkg/apprepo/apprepos_handler.go

@@ -22,8 +22,10 @@ import (
 	"net/http"
 
 	log "github.com/sirupsen/logrus"
+	authorizationapi "k8s.io/api/authorization/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/kubernetes"
+	authorizationv1 "k8s.io/client-go/kubernetes/typed/authorization/v1"
 	corev1typed "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
@@ -41,6 +43,7 @@ import (
 type combinedClientsetInterface interface {
 	KubeappsV1alpha1() v1alpha1typed.KubeappsV1alpha1Interface
 	CoreV1() corev1typed.CoreV1Interface
+	AuthorizationV1() authorizationv1.AuthorizationV1Interface
 }
 
 // Need to use a type alias to embed the two Clientset's without a name clash.
@@ -61,6 +64,9 @@ type appRepositoriesHandler struct {
 	// The namespace in which (currently) app repositories are created.
 	kubeappsNamespace string
 
+	// The Kubernetes client using the pod serviceaccount
+	svcKubeClient *kubernetes.Clientset
+
 	// clientsetForConfig is a field on the struct only so it can be switched
 	// for a fake version when testing. NewAppRepositoryHandler sets it to the
 	// proper function below so that production code always has the real
@@ -89,7 +95,7 @@ type appRepositoryResponse struct {
 	AppRepository v1alpha1.AppRepository `json:"appRepository"`
 }
 
-// NewAppRepositoriesHandler returns an AppRepositories handler configured with
+// NewAppRepositoriesHandler returns an AppRepositories and Kubernetes handler configured with
 // the in-cluster config but overriding the token with an empty string, so that
 // ConfigForToken must be called to obtain a valid config.
 func NewAppRepositoriesHandler(kubeappsNamespace string) (*appRepositoriesHandler, error) {
@@ -110,11 +116,22 @@ func NewAppRepositoriesHandler(kubeappsNamespace string) (*appRepositoriesHandle
 	if err != nil {
 		return nil, err
 	}
+
+	svcRestConfig, err := rest.InClusterConfig()
+	if err != nil {
+		return nil, err
+	}
+	svcKubeClient, err := kubernetes.NewForConfig(svcRestConfig)
+	if err != nil {
+		return nil, err
+	}
+
 	return &appRepositoriesHandler{
 		config:            *config,
 		kubeappsNamespace: kubeappsNamespace,
 		// See comment in the struct defn above.
 		clientsetForConfig: clientsetForConfig,
+		svcKubeClient:      svcKubeClient,
 	}, nil
 }
 
@@ -287,9 +304,70 @@ func secretForRequest(appRepoRequest appRepositoryRequest, appRepo *v1alpha1.App
 		},
 		StringData: secrets,
 	}
-	return nil
 }
 
 func secretNameForRepo(repoName string) string {
 	return fmt.Sprintf("apprepo-%s-secrets", repoName)
 }
+
+func filterAllowedNamespaces(userClientset combinedClientsetInterface, namespaces *corev1.NamespaceList) ([]corev1.Namespace, error) {
+	allowedNamespaces := []corev1.Namespace{}
+	for _, namespace := range namespaces.Items {
+		res, err := userClientset.AuthorizationV1().SelfSubjectAccessReviews().Create(&authorizationapi.SelfSubjectAccessReview{
+			Spec: authorizationapi.SelfSubjectAccessReviewSpec{
+				ResourceAttributes: &authorizationapi.ResourceAttributes{
+					Group:     "",
+					Resource:  "secrets",
+					Verb:      "get",
+					Namespace: namespace.Name,
+				},
+			},
+		})
+		if err != nil {
+			return nil, err
+		}
+		if res.Status.Allowed {
+			allowedNamespaces = append(allowedNamespaces, namespace)
+		}
+	}
+	return allowedNamespaces, nil
+}
+
+// GetNamespaces return the list of namespaces that the user has permission to access
+// TODO(andresmgot): I am adding this method in this package for simplicity
+// (since it already allows to impersonate the user)
+// We should refactor this code to make it more generic (not apprepository-specific)
+func (a *appRepositoriesHandler) GetNamespaces(w http.ResponseWriter, req *http.Request) {
+	token := auth.ExtractToken(req.Header.Get("Authorization"))
+	userClientset, err := a.clientsetForConfig(a.ConfigForToken(token))
+	if err != nil {
+		log.Errorf("unable to create clientset: %v", err)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+	// Try to list namespaces with the user token, for backward compatibility
+	namespaces, err := userClientset.CoreV1().Namespaces().List(metav1.ListOptions{})
+	if err != nil {
+		if errors.IsForbidden(err) {
+			// The user doesn't have permissions to list namespaces, use the current serviceaccount
+			namespaces, err = a.svcKubeClient.CoreV1().Namespaces().List(metav1.ListOptions{})
+		}
+		if err != nil && !errors.IsForbidden(err) {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+	}
+
+	allowedNamespaces, err := filterAllowedNamespaces(userClientset, namespaces)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+
+	responseBody, err := json.Marshal(allowedNamespaces)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+	w.Write(responseBody)
+}