From 831e1d39bced0322efd31ef4dd049aba6932ac99 Mon Sep 17 00:00:00 2001 From: apoger Date: Mon, 28 Nov 2022 12:29:13 +0200 Subject: [PATCH] Add authorization mechanisms in new Katib UI backend (#1983) * UI(back): Add authorization mechanisms in new Katib UI backend * Introduce helper ENV vars and functions for authentication and authorization checks. The authz checks are using SubjectAcessReviews objects. * BACKEND_MODE={dev,prod}: skip authz when in dev mode * APP_DISABLE_AUTH={bool}: skip authz if explicity requested * Introduce a client-go client to construct SubjectAccessReview objects. * Before any request proceed to K8s api-server: * check if authorization must be skipped (BACKEND_MODE, APP_DISBLE_AUTH) * check if a username is proviced in request Header * query the K8s api-server with SAR to ensure that the user has appropriate access privilleges * Replace the /katib/fetch_experiment/ route with /katib/fetch_namespaces_experiments. This route expects a namespace as a query parameter from which all experiments will be fetched. Signed-off-by: Apostolos Gerakaris * UI(front): Provide a namespace as a query parameter This is needed for the new /katib/fetch_namespaced_experiments route. Signed-off-by: Apostolos Gerakaris * Update README for running locally without auth Update the README of the web app to expose that devs should set APP_DISABLE_AUTH=true when running locally, since there's no authnz when running locally. Signed-off-by: Apostolos Gerakaris * remove duplicated variable types Signed-off-by: Apostolos Gerakaris * Review fixes * proper error handling. * switch to Go's build-in errors package. * set appropriate verbs when constructing SAR objects. Signed-off-by: Apostolos Gerakaris * review: Use controller-runtime client to create SAR objects Signed-off-by: Apostolos Gerakaris * Review fixes * fix backend routes. * '/katib/fetch_namespaces' to fetch experiments in a namespace * 'FetchExperiments' handler * hit the appropriate route from frontend and provide namespace as a query parameter to fetch experiments * remove remove BACKEND_MODE env var in favour of the more specific APP_DISABLE_AUTH Signed-off-by: Apostolos Gerakaris * Review fixes * Add constants for CRUD actions * Add plural for experiments and suggestions as constants * Add GetUsername logic under IsAuthorized and handle errors properly * Have APP_DISABLE_AUTH by default as true, since currently Katib doesn't support this feature in standalone mode. Signed-off-by: Apostolos Gerakaris Signed-off-by: Apostolos Gerakaris --- cmd/new-ui/v1beta1/main.go | 2 +- pkg/controller.v1beta1/consts/const.go | 15 ++ pkg/new-ui/v1beta1/README.md | 1 + pkg/new-ui/v1beta1/authzn.go | 95 +++++++++ pkg/new-ui/v1beta1/backend.go | 187 ++++++++++++++++-- .../experiments/experiments.component.ts | 23 +-- .../src/app/services/backend.service.ts | 4 +- pkg/new-ui/v1beta1/hp.go | 79 +++++++- pkg/new-ui/v1beta1/util.go | 39 +++- 9 files changed, 403 insertions(+), 42 deletions(-) create mode 100644 pkg/new-ui/v1beta1/authzn.go diff --git a/cmd/new-ui/v1beta1/main.go b/cmd/new-ui/v1beta1/main.go index 68d2f1dcd2b..190bd774ae1 100644 --- a/cmd/new-ui/v1beta1/main.go +++ b/cmd/new-ui/v1beta1/main.go @@ -48,7 +48,7 @@ func main() { http.HandleFunc("/katib/", kuh.ServeIndex(*buildDir)) http.Handle("/katib/static/", http.StripPrefix("/katib/", frontend)) - http.HandleFunc("/katib/fetch_experiments/", kuh.FetchAllExperiments) + http.HandleFunc("/katib/fetch_experiments/", kuh.FetchExperiments) http.HandleFunc("/katib/create_experiment/", kuh.CreateExperiment) diff --git a/pkg/controller.v1beta1/consts/const.go b/pkg/controller.v1beta1/consts/const.go index 3bf670fe762..d11580c335a 100644 --- a/pkg/controller.v1beta1/consts/const.go +++ b/pkg/controller.v1beta1/consts/const.go @@ -26,8 +26,23 @@ import ( const ( + // ActionTypeCreate is the create CRUD action + ActionTypeCreate = "create" + // ActionTypeList is the list CRUD action + ActionTypeList = "list" + // ActionTypeGet is the get CRUD action + ActionTypeGet = "get" + // ActionTypeUpdate is the update CRUD action + ActionTypeUpdate = "update" + // ActionTypeDelete is the delete CRUD action + ActionTypeDelete = "delete" + // PluralTrial is the plural for Trial object PluralTrial = "trials" + // PluralExperiment is the plural for Experiment object + PluralExperiment = "experiments" + // PluralSuggestion is the plural for Suggestion object + PluralSuggestion = "suggestions" // ConfigExperimentSuggestionName is the config name of the // suggestion client implementation in experiment controller. diff --git a/pkg/new-ui/v1beta1/README.md b/pkg/new-ui/v1beta1/README.md index 4ace186a618..fd3b0cb8a9b 100755 --- a/pkg/new-ui/v1beta1/README.md +++ b/pkg/new-ui/v1beta1/README.md @@ -92,6 +92,7 @@ This is the recommended way to test the web app e2e. In order to build the UI an For example, if you use port-forwarding to expose `katib-db-manager`, run this command: ``` + export APP_DISABLE_AUTH=true go run main.go --build-dir=../../../pkg/new-ui/v1beta1/frontend/dist --port=8080 --db-manager-address=localhost:6789 ``` diff --git a/pkg/new-ui/v1beta1/authzn.go b/pkg/new-ui/v1beta1/authzn.go new file mode 100644 index 00000000000..c31da3426f7 --- /dev/null +++ b/pkg/new-ui/v1beta1/authzn.go @@ -0,0 +1,95 @@ +package v1beta1 + +import ( + "context" + "errors" + "fmt" + "log" + "net/http" + "strings" + + "github.com/kubeflow/katib/pkg/util/v1beta1/env" + v1 "k8s.io/api/authorization/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// ENV variables +var ( + USER_HEADER = env.GetEnvOrDefault("USERID_HEADER", "kubeflow-userid") + USER_PREFIX = env.GetEnvOrDefault("USERID_PREFIX", ":") + DISABLE_AUTH = env.GetEnvOrDefault("APP_DISABLE_AUTH", "true") +) + +// Function for constructing SubjectAccessReviews (SAR) objects +func CreateSAR(user, verb, namespace, resource, subresource, name string, schema schema.GroupVersion) *v1.SubjectAccessReview { + + sar := &v1.SubjectAccessReview{ + Spec: v1.SubjectAccessReviewSpec{ + User: user, + ResourceAttributes: &v1.ResourceAttributes{ + Namespace: namespace, + Verb: verb, + Group: schema.Group, + Version: schema.Version, + Resource: resource, + Subresource: subresource, + Name: name, + }, + }, + } + return sar +} + +func IsAuthorized(verb, namespace, resource, subresource, name string, schema schema.GroupVersion, client client.Client, r *http.Request) (string, error) { + + // We disable authn/authz checks when in standalone mode. + if DISABLE_AUTH == "true" { + log.Printf("APP_DISABLE_AUTH set to True. Skipping authentication/authorization checks") + return "", nil + } + // Check if an incoming request is from an authenticated user (kubeflow mode: kubeflow-userid header) + if r.Header.Get(USER_HEADER) == "" { + return "", errors.New("user header not present") + } + user := r.Header.Get(USER_HEADER) + user = strings.Replace(user, USER_PREFIX, "", 1) + + // Check if the user is authorized to perform a given action on katib/k8s resources. + sar := CreateSAR(user, verb, namespace, resource, subresource, name, schema) + err := client.Create(context.TODO(), sar) + if err != nil { + log.Printf("Error submitting SubjectAccessReview: %v, %s", sar, err.Error()) + return user, err + } + + if sar.Status.Allowed { + return user, nil + } + + msg := generateUnauthorizedMessage(user, verb, namespace, resource, subresource, schema, sar) + return user, errors.New(msg) +} + +func generateUnauthorizedMessage(user, verb, namespace, resource, subresource string, schema schema.GroupVersion, sar *v1.SubjectAccessReview) string { + + msg := fmt.Sprintf("User: %s is not authorized to %s", user, verb) + + if schema.Group == "" { + msg += fmt.Sprintf(" %s/%s", schema.Version, resource) + } else { + msg += fmt.Sprintf(" %s/%s/%s", schema.Group, schema.Version, resource) + } + + if subresource != "" { + msg += fmt.Sprintf("/%s", subresource) + } + + if namespace != "" { + msg += fmt.Sprintf(" in namespace: %s", namespace) + } + if sar.Status.Reason != "" { + msg += fmt.Sprintf(" ,reason: %s", sar.Status.Reason) + } + return msg +} diff --git a/pkg/new-ui/v1beta1/backend.go b/pkg/new-ui/v1beta1/backend.go index 80c5aff83b4..2594395cac2 100644 --- a/pkg/new-ui/v1beta1/backend.go +++ b/pkg/new-ui/v1beta1/backend.go @@ -18,6 +18,7 @@ package v1beta1 import ( "encoding/json" + "errors" "log" "net/http" "path/filepath" @@ -27,8 +28,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" experimentv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1" + suggestionv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/suggestions/v1beta1" api_pb_v1beta1 "github.com/kubeflow/katib/pkg/apis/manager/v1beta1" + consts "github.com/kubeflow/katib/pkg/controller.v1beta1/consts" "github.com/kubeflow/katib/pkg/util/v1beta1/katibclient" + corev1 "k8s.io/api/core/v1" ) func NewKatibUIHandler(dbManagerAddr string) *KatibUIHandler { @@ -37,6 +41,7 @@ func NewKatibUIHandler(dbManagerAddr string) *KatibUIHandler { log.Printf("NewClient for Katib failed: %v", err) panic(err) } + return &KatibUIHandler{ katibClient: kclient, dbManagerAddr: dbManagerAddr, @@ -75,6 +80,7 @@ func (k *KatibUIHandler) CreateExperiment(w http.ResponseWriter, r *http.Request http.Error(w, err.Error(), http.StatusInternalServerError) return } + dataJSON, ok := data["postData"] if !ok { msg := "Couldn't load the 'postData' field of the request's data" @@ -96,6 +102,20 @@ func (k *KatibUIHandler) CreateExperiment(w http.ResponseWriter, r *http.Request return } + namespace := job.ObjectMeta.Namespace + experimentName := job.ObjectMeta.Name + + user, err := IsAuthorized(consts.ActionTypeCreate, namespace, consts.PluralExperiment, "", experimentName, experimentv1beta1.SchemeGroupVersion, k.katibClient.GetClient(), r) + if user == "" && err != nil { + log.Printf("No user provided in kubeflow-userid header.") + http.Error(w, err.Error(), http.StatusUnauthorized) + return + } else if err != nil { + log.Printf("The user: %s is not authorized to create experiment: %s in namespace: %s \n", user, experimentName, namespace) + http.Error(w, err.Error(), http.StatusForbidden) + return + } + err = k.katibClient.CreateRuntimeObject(&job) if err != nil { log.Printf("CreateRuntimeObject from parameters failed: %v", err) @@ -104,14 +124,30 @@ func (k *KatibUIHandler) CreateExperiment(w http.ResponseWriter, r *http.Request } } -// FetchAllExperiments gets HP and NAS experiments in all namespaces. -func (k *KatibUIHandler) FetchAllExperiments(w http.ResponseWriter, r *http.Request) { - // At first, try to list experiments in cluster scope - experiments, err := k.getExperiments([]string{""}) - if err != nil { - // If failed, just try to list experiments from own namespace - experiments, err = k.getExperiments([]string{}) +func (k *KatibUIHandler) FetchExperiments(w http.ResponseWriter, r *http.Request) { + + namespaces, ok := r.URL.Query()["namespace"] + if !ok { + log.Printf("No 'namespace' query parameter was provided.") + err := errors.New("no 'namespace' query parameter was provided") + http.Error(w, err.Error(), http.StatusBadRequest) + return } + + namespace := namespaces[0] + + user, err := IsAuthorized(consts.ActionTypeList, namespace, consts.PluralExperiment, "", "", experimentv1beta1.SchemeGroupVersion, k.katibClient.GetClient(), r) + if user == "" && err != nil { + log.Printf("No user provided in kubeflow-userid header.") + http.Error(w, err.Error(), http.StatusUnauthorized) + return + } else if err != nil { + log.Printf("The user: %s is not authorized to list experiments in namespace: %s \n", user, namespace) + http.Error(w, err.Error(), http.StatusForbidden) + return + } + + experiments, err := k.getExperiments([]string{namespace}) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return @@ -122,16 +158,44 @@ func (k *KatibUIHandler) FetchAllExperiments(w http.ResponseWriter, r *http.Requ http.Error(w, err.Error(), http.StatusInternalServerError) return } + if _, err = w.Write(response); err != nil { log.Printf("Write experiments failed: %v", err) http.Error(w, err.Error(), http.StatusInternalServerError) return } + } func (k *KatibUIHandler) DeleteExperiment(w http.ResponseWriter, r *http.Request) { - experimentName := r.URL.Query()["experimentName"][0] - namespace := r.URL.Query()["namespace"][0] + + namespaces, ok := r.URL.Query()["namespace"] + if !ok { + log.Printf("No 'namespace' query parameter was provided.") + err := errors.New("no 'namespace' query parameter was provided") + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + experimentNames, ok := r.URL.Query()["experimentName"] + if !ok { + log.Printf("No experimentName provided in Query parameteres! Provide an 'experimentName' param") + err := errors.New("no experimentName provided") + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + experimentName := experimentNames[0] + namespace := namespaces[0] + + user, err := IsAuthorized(consts.ActionTypeDelete, namespace, consts.PluralExperiment, "", experimentName, experimentv1beta1.SchemeGroupVersion, k.katibClient.GetClient(), r) + if user == "" && err != nil { + log.Printf("No user provided in kubeflow-userid header.") + http.Error(w, err.Error(), http.StatusUnauthorized) + return + } else if err != nil { + log.Printf("The user: %s is not authorized to delete experiment: %s in namespace: %s \n", user, experimentName, namespace) + http.Error(w, err.Error(), http.StatusForbidden) + return + } experiment, err := k.katibClient.GetExperiment(experimentName, namespace) if err != nil { @@ -139,6 +203,7 @@ func (k *KatibUIHandler) DeleteExperiment(w http.ResponseWriter, r *http.Request http.Error(w, err.Error(), http.StatusInternalServerError) return } + err = k.katibClient.DeleteRuntimeObject(experiment) if err != nil { log.Printf("DeleteRuntimeObject failed: %v", err) @@ -188,7 +253,7 @@ func (k *KatibUIHandler) DeleteExperiment(w http.ResponseWriter, r *http.Request // FetchTrialTemplates gets all trial templates in all namespaces func (k *KatibUIHandler) FetchTrialTemplates(w http.ResponseWriter, r *http.Request) { - trialTemplatesViewList, err := k.getTrialTemplatesViewList() + trialTemplatesViewList, err := k.getTrialTemplatesViewList(r) if err != nil { log.Printf("getTrialTemplatesViewList failed: %v", err) http.Error(w, err.Error(), http.StatusInternalServerError) @@ -213,6 +278,7 @@ func (k *KatibUIHandler) FetchTrialTemplates(w http.ResponseWriter, r *http.Requ // AddTemplate adds template to ConfigMap func (k *KatibUIHandler) AddTemplate(w http.ResponseWriter, r *http.Request) { + var data map[string]interface{} if err := json.NewDecoder(r.Body).Decode(&data); err != nil { log.Printf("Failed to decode body: %v", err) @@ -225,7 +291,18 @@ func (k *KatibUIHandler) AddTemplate(w http.ResponseWriter, r *http.Request) { updatedConfigMapPath := data["updatedConfigMapPath"].(string) updatedTemplateYaml := data["updatedTemplateYaml"].(string) - newTemplates, err := k.updateTrialTemplates(updatedConfigMapNamespace, updatedConfigMapName, "", updatedConfigMapPath, updatedTemplateYaml, ActionTypeAdd) + user, err := IsAuthorized(consts.ActionTypeCreate, updatedConfigMapNamespace, corev1.ResourceConfigMaps.String(), "", updatedConfigMapName, corev1.SchemeGroupVersion, k.katibClient.GetClient(), r) + if user == "" && err != nil { + log.Printf("No user provided in kubeflow-userid header.") + http.Error(w, err.Error(), http.StatusUnauthorized) + return + } else if err != nil { + log.Printf("The user: %s is not authorized to add configmap: %s in namespace: %s \n", user, updatedConfigMapName, updatedConfigMapNamespace) + http.Error(w, err.Error(), http.StatusForbidden) + return + } + + newTemplates, err := k.updateTrialTemplates(updatedConfigMapNamespace, updatedConfigMapName, "", updatedConfigMapPath, updatedTemplateYaml, ActionTypeAdd, r) if err != nil { log.Printf("updateTrialTemplates failed: %v", err) http.Error(w, err.Error(), http.StatusInternalServerError) @@ -265,7 +342,18 @@ func (k *KatibUIHandler) EditTemplate(w http.ResponseWriter, r *http.Request) { updatedConfigMapPath := data["updatedConfigMapPath"].(string) updatedTemplateYaml := data["updatedTemplateYaml"].(string) - newTemplates, err := k.updateTrialTemplates(updatedConfigMapNamespace, updatedConfigMapName, configMapPath, updatedConfigMapPath, updatedTemplateYaml, ActionTypeEdit) + user, err := IsAuthorized(consts.ActionTypeUpdate, updatedConfigMapNamespace, corev1.ResourceConfigMaps.String(), "", updatedConfigMapName, corev1.SchemeGroupVersion, k.katibClient.GetClient(), r) + if user == "" && err != nil { + log.Printf("No user provided in kubeflow-userid header.") + http.Error(w, err.Error(), http.StatusUnauthorized) + return + } else if err != nil { + log.Printf("The user: %s is not authorized to edit configmap: %s in namespace: %s \n", user, updatedConfigMapName, updatedConfigMapNamespace) + http.Error(w, err.Error(), http.StatusForbidden) + return + } + + newTemplates, err := k.updateTrialTemplates(updatedConfigMapNamespace, updatedConfigMapName, configMapPath, updatedConfigMapPath, updatedTemplateYaml, ActionTypeEdit, r) if err != nil { log.Printf("updateTrialTemplates failed: %v", err) http.Error(w, err.Error(), http.StatusInternalServerError) @@ -302,7 +390,18 @@ func (k *KatibUIHandler) DeleteTemplate(w http.ResponseWriter, r *http.Request) updatedConfigMapName := data["updatedConfigMapName"].(string) updatedConfigMapPath := data["updatedConfigMapPath"].(string) - newTemplates, err := k.updateTrialTemplates(updatedConfigMapNamespace, updatedConfigMapName, "", updatedConfigMapPath, "", ActionTypeDelete) + user, err := IsAuthorized(consts.ActionTypeDelete, updatedConfigMapNamespace, corev1.ResourceConfigMaps.String(), "", updatedConfigMapName, corev1.SchemeGroupVersion, k.katibClient.GetClient(), r) + if user == "" && err != nil { + log.Printf("No user provided in kubeflow-userid header.") + http.Error(w, err.Error(), http.StatusUnauthorized) + return + } else if err != nil { + log.Printf("The user: %s is not authorized to delete configmap: %s in namespace: %s \n", user, updatedConfigMapName, updatedConfigMapNamespace) + http.Error(w, err.Error(), http.StatusForbidden) + return + } + + newTemplates, err := k.updateTrialTemplates(updatedConfigMapNamespace, updatedConfigMapName, "", updatedConfigMapPath, "", ActionTypeDelete, r) if err != nil { log.Printf("updateTrialTemplates failed: %v", err) http.Error(w, err.Error(), http.StatusInternalServerError) @@ -351,8 +450,35 @@ func (k *KatibUIHandler) FetchNamespaces(w http.ResponseWriter, r *http.Request) // FetchExperiment gets experiment in specific namespace. func (k *KatibUIHandler) FetchExperiment(w http.ResponseWriter, r *http.Request) { - experimentName := r.URL.Query()["experimentName"][0] - namespace := r.URL.Query()["namespace"][0] + + namespaces, ok := r.URL.Query()["namespace"] + if !ok { + log.Printf("No 'namespace' query parameter was provided.") + err := errors.New("no 'namespace' query parameter was provided") + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + experimentNames, ok := r.URL.Query()["experimentName"] + if !ok { + log.Printf("No experimentName provided in Query parameteres! Provide an 'experimentName' param") + err := errors.New("no experimentName provided") + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + experimentName := experimentNames[0] + namespace := namespaces[0] + + user, err := IsAuthorized(consts.ActionTypeGet, namespace, consts.PluralExperiment, "", experimentName, experimentv1beta1.SchemeGroupVersion, k.katibClient.GetClient(), r) + if user == "" && err != nil { + log.Printf("No user provided in kubeflow-userid header.") + http.Error(w, err.Error(), http.StatusUnauthorized) + return + } else if err != nil { + log.Printf("The user: %s is not authorized to get experiment: %s in namespace: %s \n", user, experimentName, namespace) + http.Error(w, err.Error(), http.StatusForbidden) + return + } experiment, err := k.katibClient.GetExperiment(experimentName, namespace) if err != nil { @@ -375,8 +501,35 @@ func (k *KatibUIHandler) FetchExperiment(w http.ResponseWriter, r *http.Request) // FetchSuggestion gets suggestion in specific namespace func (k *KatibUIHandler) FetchSuggestion(w http.ResponseWriter, r *http.Request) { - suggestionName := r.URL.Query()["suggestionName"][0] - namespace := r.URL.Query()["namespace"][0] + + namespaces, ok := r.URL.Query()["namespace"] + if !ok { + log.Printf("No namespace provided in Query parameters! Provide a 'namespace' param") + err := errors.New("no 'namespace' provided") + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + suggestionNames, ok := r.URL.Query()["suggestionName"] + if !ok { + log.Printf("No experimentName provided in Query parameteres! Provide an 'experimentName' param") + err := errors.New("no experimentName provided") + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + suggestionName := suggestionNames[0] + namespace := namespaces[0] + + user, err := IsAuthorized(consts.ActionTypeGet, namespace, consts.PluralSuggestion, "", suggestionName, suggestionv1beta1.SchemeGroupVersion, k.katibClient.GetClient(), r) + if user == "" && err != nil { + log.Printf("No user provided in kubeflow-userid header.") + http.Error(w, err.Error(), http.StatusUnauthorized) + return + } else if err != nil { + log.Printf("The user: %s is not authorized to get suggestion: %s in namespace: %s \n", user, suggestionName, namespace) + http.Error(w, err.Error(), http.StatusForbidden) + return + } suggestion, err := k.katibClient.GetSuggestion(suggestionName, namespace) if err != nil { diff --git a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiments/experiments.component.ts b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiments/experiments.component.ts index c1dcacc0f2e..32034aec408 100644 --- a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiments/experiments.component.ts +++ b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiments/experiments.component.ts @@ -134,19 +134,20 @@ export class ExperimentsComponent implements OnInit, OnDestroy { // Poll for new data and reset the poller if different data is found this.subs.add( this.poller.start().subscribe(() => { - this.backend.getExperiments().subscribe(experiments => { - // the backend should have proper namespace isolation - experiments = experiments.filter( - experiment => experiment.namespace === this.currNamespace, - ); + if (!this.currNamespace) { + return; + } - if (isEqual(this.experiments, experiments)) { - return; - } + this.backend + .getExperiments(this.currNamespace) + .subscribe(experiments => { + if (isEqual(this.experiments, experiments)) { + return; + } - this.experiments = experiments; - this.poller.reset(); - }); + this.experiments = experiments; + this.poller.reset(); + }); }), ); } diff --git a/pkg/new-ui/v1beta1/frontend/src/app/services/backend.service.ts b/pkg/new-ui/v1beta1/frontend/src/app/services/backend.service.ts index 8bcd3cc2735..457defc12f2 100644 --- a/pkg/new-ui/v1beta1/frontend/src/app/services/backend.service.ts +++ b/pkg/new-ui/v1beta1/frontend/src/app/services/backend.service.ts @@ -38,10 +38,10 @@ export class KWABackendService extends BackendService { return throwError(msg); } - getExperiments(): Observable { + getExperiments(namespace: string): Observable { // If the route doesn't end in a "/"" then the backend will return a 301 to // the url ending with "/". - const url = '/katib/fetch_experiments/'; + const url = `/katib/fetch_experiments/?namespace=${namespace}`; return this.http.get(url).pipe( catchError(error => this.parseError(error)), diff --git a/pkg/new-ui/v1beta1/hp.go b/pkg/new-ui/v1beta1/hp.go index b4ccd44aa20..3d6eb6631eb 100644 --- a/pkg/new-ui/v1beta1/hp.go +++ b/pkg/new-ui/v1beta1/hp.go @@ -19,6 +19,7 @@ package v1beta1 import ( "context" "encoding/json" + "errors" "log" "net/http" "strconv" @@ -26,15 +27,44 @@ import ( "time" commonv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/common/v1beta1" + experimentv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1" + trialv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/trials/v1beta1" api_pb_v1beta1 "github.com/kubeflow/katib/pkg/apis/manager/v1beta1" + "github.com/kubeflow/katib/pkg/controller.v1beta1/consts" ) const kfpRunIDAnnotation = "kubeflow-kale.org/kfp-run-uuid" func (k *KatibUIHandler) FetchHPJobInfo(w http.ResponseWriter, r *http.Request) { //enableCors(&w) - experimentName := r.URL.Query()["experimentName"][0] - namespace := r.URL.Query()["namespace"][0] + + namespaces, ok := r.URL.Query()["namespace"] + if !ok { + log.Printf("No namespace provided in Query parameters! Provide a 'namespace' param") + err := errors.New("no 'namespace' provided") + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + experimentNames, ok := r.URL.Query()["experimentName"] + if !ok { + log.Printf("No experimentName provided in Query parameteres! Provide an 'experimentName' param") + err := errors.New("no experimentName provided") + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + experimentName := experimentNames[0] + namespace := namespaces[0] + + user, err := IsAuthorized(consts.ActionTypeGet, namespace, consts.PluralExperiment, "", experimentName, experimentv1beta1.SchemeGroupVersion, k.katibClient.GetClient(), r) + if user == "" && err != nil { + log.Printf("No user provided in kubeflow-userid header.") + http.Error(w, err.Error(), http.StatusUnauthorized) + return + } else if err != nil { + log.Printf("The user: %s is not authorized to get experiments from namespace: %s \n", user, namespace) + http.Error(w, err.Error(), http.StatusForbidden) + return + } log.Printf("Start FetchHPJobInfo for Experiment: %v in namespace: %v", experimentName, namespace) @@ -65,6 +95,17 @@ func (k *KatibUIHandler) FetchHPJobInfo(w http.ResponseWriter, r *http.Request) } log.Printf("Got Parameters names") + _, err = IsAuthorized(consts.ActionTypeList, namespace, consts.PluralTrial, "", "", trialv1beta1.SchemeGroupVersion, k.katibClient.GetClient(), r) + if user == "" && err != nil { + log.Printf("No user provided in kubeflow-userid header.") + http.Error(w, err.Error(), http.StatusUnauthorized) + return + } else if err != nil { + log.Printf("The user: %s is not authorized to list trials from namespace: %s \n", user, namespace) + http.Error(w, err.Error(), http.StatusForbidden) + return + } + trialList, err := k.katibClient.GetTrialList(experimentName, namespace) if err != nil { log.Printf("GetTrialList from HP job failed: %v", err) @@ -151,16 +192,46 @@ func (k *KatibUIHandler) FetchHPJobInfo(w http.ResponseWriter, r *http.Request) // FetchHPJobTrialInfo returns all metrics for the HP Job Trial func (k *KatibUIHandler) FetchHPJobTrialInfo(w http.ResponseWriter, r *http.Request) { //enableCors(&w) - trialName := r.URL.Query()["trialName"][0] - namespace := r.URL.Query()["namespace"][0] + + namespaces, ok := r.URL.Query()["namespace"] + if !ok { + log.Printf("No namespace provided in Query parameters! Provide a 'namespace' param") + err := errors.New("no 'namespace' provided") + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + trialNames, ok := r.URL.Query()["trialName"] + if !ok { + log.Printf("No trialName provided in Query parameters! Provide a 'trialName' param") + err := errors.New("no 'trialName' provided") + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + trialName := trialNames[0] + namespace := namespaces[0] + conn, c := k.connectManager() defer conn.Close() + user, err := IsAuthorized(consts.ActionTypeList, namespace, consts.PluralTrial, "", trialName, trialv1beta1.SchemeGroupVersion, k.katibClient.GetClient(), r) + if user == "" && err != nil { + log.Printf("No user provided in kubeflow-userid header.") + http.Error(w, err.Error(), http.StatusUnauthorized) + return + } else if err != nil { + log.Printf("The user: %s is not authorized to get trial: %s from namespace: %s \n", user, trialName, namespace) + http.Error(w, err.Error(), http.StatusForbidden) + return + } + trial, err := k.katibClient.GetTrial(trialName, namespace) if err != nil { log.Printf("GetTrial from HP job failed: %v", err) http.Error(w, err.Error(), http.StatusInternalServerError) + return } objectiveType := trial.Spec.Objective.Type diff --git a/pkg/new-ui/v1beta1/util.go b/pkg/new-ui/v1beta1/util.go index c5d6c5cf13c..bec53cc5873 100644 --- a/pkg/new-ui/v1beta1/util.go +++ b/pkg/new-ui/v1beta1/util.go @@ -19,6 +19,7 @@ package v1beta1 import ( "encoding/json" "log" + "net/http" "sort" "strconv" "strings" @@ -76,7 +77,7 @@ func havePipelineUID(trials []trialv1beta1.Trial) bool { return false } -func (k *KatibUIHandler) getTrialTemplatesViewList() ([]TrialTemplatesDataView, error) { +func (k *KatibUIHandler) getTrialTemplatesViewList(r *http.Request) ([]TrialTemplatesDataView, error) { trialTemplatesDataView := make([]TrialTemplatesDataView, 0) // Get all available namespaces @@ -94,8 +95,31 @@ func (k *KatibUIHandler) getTrialTemplatesViewList() ([]TrialTemplatesDataView, return nil, err } + // Iterate over the trialTemplatesConfigMapList from all namespaces and filter out the + // configmaps that belong to namespaces other than kubeflow and the ones that the user has + // access privileges. + var newTrialTemplatesConfigMapList []apiv1.ConfigMap + for _, cmap := range trialTemplatesConfigMapList.Items { + if ns == consts.DefaultKatibNamespace { + // Add the configmaps from kubeflow namespace (no user can have access to kubeflow ns, so we hardcode it) + newTrialTemplatesConfigMapList = append(newTrialTemplatesConfigMapList, cmap) + } else { + // for all other namespaces check authorization rbac + configmapName := cmap.ObjectMeta.Name + user, err := IsAuthorized(consts.ActionTypeGet, ns, apiv1.ResourceConfigMaps.String(), "", configmapName, apiv1.SchemeGroupVersion, k.katibClient.GetClient(), r) + if err != nil { + log.Printf("The user: %s is not authorized to view configMap: %s in namespace: %s \n", user, configmapName, ns) + return nil, err + } else { + log.Printf("The user: %s is authorized to view configMap: %s in namespace: %s", user, configmapName, ns) + newTrialTemplatesConfigMapList = append(newTrialTemplatesConfigMapList, cmap) + } + } + + } + if len(trialTemplatesConfigMapList.Items) != 0 { - newTrialTemplatesView := getTrialTemplatesView(trialTemplatesConfigMapList) + newTrialTemplatesView := getTrialTemplatesView(newTrialTemplatesConfigMapList) // ConfigMap with templates must exists in namespace if len(newTrialTemplatesView.ConfigMaps) > 0 { trialTemplatesDataView = append(trialTemplatesDataView, newTrialTemplatesView) @@ -120,13 +144,13 @@ func (k *KatibUIHandler) getAvailableNamespaces() ([]string, error) { return namespaces, nil } -func getTrialTemplatesView(templatesConfigMapList *apiv1.ConfigMapList) TrialTemplatesDataView { +func getTrialTemplatesView(templatesConfigMapList []apiv1.ConfigMap) TrialTemplatesDataView { trialTemplatesDataView := TrialTemplatesDataView{ - ConfigMapNamespace: templatesConfigMapList.Items[0].ObjectMeta.Namespace, + ConfigMapNamespace: templatesConfigMapList[0].ObjectMeta.Namespace, ConfigMaps: []ConfigMap{}, } - for _, configMap := range templatesConfigMapList.Items { + for _, configMap := range templatesConfigMapList { newConfigMap := ConfigMap{ ConfigMapName: configMap.ObjectMeta.Name, Templates: []Template{}, @@ -159,7 +183,8 @@ func (k *KatibUIHandler) updateTrialTemplates( configMapPath, updatedConfigMapPath, updatedTemplateYaml, - actionType string) ([]TrialTemplatesDataView, error) { + actionType string, + r *http.Request) ([]TrialTemplatesDataView, error) { templates, err := k.katibClient.GetConfigMap(updatedConfigMapName, updatedConfigMapNamespace) if err != nil && !(errors.IsNotFound(err) && actionType == ActionTypeAdd) { @@ -213,7 +238,7 @@ func (k *KatibUIHandler) updateTrialTemplates( } } - newTemplates, err := k.getTrialTemplatesViewList() + newTemplates, err := k.getTrialTemplatesViewList(r) if err != nil { log.Printf("getTrialTemplatesViewList: %v", err) return nil, err