From a28f759ff5ac771e4c98d85e612fd9cccd90b03f Mon Sep 17 00:00:00 2001 From: Alex Creasy Date: Fri, 17 Jan 2025 17:38:11 +0000 Subject: [PATCH] Adds BFF support for CORS Signed-off-by: Alex Creasy --- clients/ui/bff/Makefile | 3 +- clients/ui/bff/README.md | 38 ++++++++++++++++++ clients/ui/bff/cmd/main.go | 1 + clients/ui/bff/go.mod | 3 +- clients/ui/bff/go.sum | 2 + clients/ui/bff/internal/api/app.go | 2 +- clients/ui/bff/internal/api/helpers.go | 12 ++++++ clients/ui/bff/internal/api/helpers_test.go | 39 +++++++++++++++++++ clients/ui/bff/internal/api/middleware.go | 22 ++++++++--- clients/ui/bff/internal/config/environment.go | 1 + clients/ui/bff/internal/constants/keys.go | 2 + 11 files changed, 116 insertions(+), 9 deletions(-) diff --git a/clients/ui/bff/Makefile b/clients/ui/bff/Makefile index f2d67cd46..4cbf7d79e 100644 --- a/clients/ui/bff/Makefile +++ b/clients/ui/bff/Makefile @@ -9,6 +9,7 @@ STATIC_ASSETS_DIR ?= ./static # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary. ENVTEST_K8S_VERSION = 1.29.0 LOG_LEVEL ?= info +ALLOWED_ORIGINS ?= "" .PHONY: all all: build @@ -49,7 +50,7 @@ build: fmt vet test ## Builds the project to produce a binary executable. .PHONY: run run: fmt vet envtest ## Runs the project. ENVTEST_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" \ - go run ./cmd/main.go --port=$(PORT) --static-assets-dir=$(STATIC_ASSETS_DIR) --mock-k8s-client=$(MOCK_K8S_CLIENT) --mock-mr-client=$(MOCK_MR_CLIENT) --dev-mode=$(DEV_MODE) --dev-mode-port=$(DEV_MODE_PORT) --standalone-mode=$(STANDALONE_MODE) --log-level=$(LOG_LEVEL) + go run ./cmd/main.go --port=$(PORT) --static-assets-dir=$(STATIC_ASSETS_DIR) --mock-k8s-client=$(MOCK_K8S_CLIENT) --mock-mr-client=$(MOCK_MR_CLIENT) --dev-mode=$(DEV_MODE) --dev-mode-port=$(DEV_MODE_PORT) --standalone-mode=$(STANDALONE_MODE) --log-level=$(LOG_LEVEL) --allowed-origins=$(ALLOWED_ORIGINS) ##@ Dependencies diff --git a/clients/ui/bff/README.md b/clients/ui/bff/README.md index 37bda0b59..474bbde35 100644 --- a/clients/ui/bff/README.md +++ b/clients/ui/bff/README.md @@ -287,3 +287,41 @@ Access to Model Registry List: Access to Specific Model Registry Endpoints: - For other endpoints (e.g., /v1/model_registry/{model_registry_id}/...), we perform a SAR check for get and list verbs on the specific service (identified by model_registry_id) within the namespace. - If the user or any group has permission to get or list the specific service, the request is authorized. + +#### 4. How do I allow CORS requests from other origins + +When serving the UI directly from the BFF there is no need for any CORS headers to be served, by default they are turned off for security reasons. + +If you need to enable CORS for any reasons you can add origins to the allow-list in several ways: + +##### Via the make command +Add the following parameter to your command: `ALLOWED_ORIGINS` this takes a comma separated list of origins to permit serving to, alterantively you can specify the value `*` to allow all origins, **Note this is not recommended in production deployments as it poses a security risk** + +Examples: + +```shell +# Allow only the origin http://example.com:8081 +make run ALLOWED_ORIGINS="http://example.com:8081" + +# Allow the origins http://example.com and http://very-nice.com +make run ALLOWED_ORIGINS="http://example.com,http://very-nice.com" + +# Allow all origins +make run ALLOWED_ORIGINS="*" + +# Explicitly disable CORS (default behaviour) +make run ALLOWED_ORIGINS="" +``` + +#### Via environment variable +Setting CORS via environment variable follows the same rules as using the Makefile, simply set the environment variable `ALLOWED_ORIGINS` with the same value as above. + +#### Via the command line arguments + +Setting CORS via command line arguments follows the same rules as using the Makefile. Simply add the `--allowed-origins=` flag to your command. + +Examples: +```shell +./bff --allowed-origins="http://my-domain.com,http://my-other-domain.com" +``` + diff --git a/clients/ui/bff/cmd/main.go b/clients/ui/bff/cmd/main.go index bb2aecaf5..eb2657ddd 100644 --- a/clients/ui/bff/cmd/main.go +++ b/clients/ui/bff/cmd/main.go @@ -28,6 +28,7 @@ func main() { flag.BoolVar(&cfg.StandaloneMode, "standalone-mode", false, "Use standalone mode for enabling endpoints in standalone mode") flag.StringVar(&cfg.StaticAssetsDir, "static-assets-dir", "./static", "Configure frontend static assets root directory") flag.StringVar(&cfg.LogLevel, "log-level", getEnvAsString("LOG_LEVEL", "info"), "Sets server log level, possible values: debug, info, warn, error, fatal") + flag.StringVar(&cfg.AllowedOrigins, "allowed-origins", getEnvAsString("ALLOWED_ORIGINS", ""), "Sets allowed origins for CORS purposes, accepts a comma separated list of origins or * to allow all, default none") flag.Parse() logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ diff --git a/clients/ui/bff/go.mod b/clients/ui/bff/go.mod index 58102e714..4c95d98f3 100644 --- a/clients/ui/bff/go.mod +++ b/clients/ui/bff/go.mod @@ -4,10 +4,12 @@ go 1.22.2 require ( github.com/brianvoe/gofakeit/v7 v7.1.2 + github.com/google/uuid v1.6.0 github.com/julienschmidt/httprouter v1.3.0 github.com/kubeflow/model-registry v0.2.9 github.com/onsi/ginkgo/v2 v2.21.0 github.com/onsi/gomega v1.35.1 + github.com/rs/cors v1.11.1 github.com/stretchr/testify v1.9.0 k8s.io/api v0.31.2 k8s.io/apimachinery v0.31.4 @@ -36,7 +38,6 @@ require ( github.com/google/go-cmp v0.6.0 // indirect github.com/google/gofuzz v1.2.0 // indirect github.com/google/pprof v0.0.0-20241029153458-d1b30febd7db // indirect - github.com/google/uuid v1.6.0 // indirect github.com/imdario/mergo v0.3.6 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect diff --git a/clients/ui/bff/go.sum b/clients/ui/bff/go.sum index bc28ba1d6..4d5c921e3 100644 --- a/clients/ui/bff/go.sum +++ b/clients/ui/bff/go.sum @@ -97,6 +97,8 @@ github.com/prometheus/procfs v0.15.1 h1:YagwOFzUgYfKKHX6Dr+sHT7km/hxC76UB0leargg github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoGhij/e3PBqk= github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8= github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4= +github.com/rs/cors v1.11.1 h1:eU3gRzXLRK57F5rKMGMZURNdIG4EoAmX8k94r9wXWHA= +github.com/rs/cors v1.11.1/go.mod h1:XyqrcTp5zjWr1wsJ8PIRZssZ8b/WMcMf71DJnit4EMU= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/clients/ui/bff/internal/api/app.go b/clients/ui/bff/internal/api/app.go index bdbdbad27..054434e08 100644 --- a/clients/ui/bff/internal/api/app.go +++ b/clients/ui/bff/internal/api/app.go @@ -138,5 +138,5 @@ func (app *App) Routes() http.Handler { http.ServeFile(w, r, path.Join(app.config.StaticAssetsDir, "index.html")) }) - return app.RecoverPanic(app.EnableTelemetry(app.enableCORS(app.InjectUserHeaders(appMux)))) + return app.RecoverPanic(app.EnableTelemetry(app.EnableCORS(app.InjectUserHeaders(appMux)))) } diff --git a/clients/ui/bff/internal/api/helpers.go b/clients/ui/bff/internal/api/helpers.go index 4f0fb58b8..c11876676 100644 --- a/clients/ui/bff/internal/api/helpers.go +++ b/clients/ui/bff/internal/api/helpers.go @@ -106,3 +106,15 @@ func ParseURLTemplate(tmpl string, params map[string]string) string { return r.Replace(tmpl) } + +func ParseOriginList(origins string) ([]string, bool) { + if origins == "" { + return []string{}, false + } + + if origins == "*" { + return []string{"*"}, true + } + originsTrimmed := strings.ReplaceAll(origins, " ", "") + return strings.Split(originsTrimmed, ","), true +} diff --git a/clients/ui/bff/internal/api/helpers_test.go b/clients/ui/bff/internal/api/helpers_test.go index 4cf02821b..492693fe8 100644 --- a/clients/ui/bff/internal/api/helpers_test.go +++ b/clients/ui/bff/internal/api/helpers_test.go @@ -19,3 +19,42 @@ func TestParseURLTemplateWhenEmpty(t *testing.T) { actual := ParseURLTemplate("", nil) assert.Empty(t, actual) } + +func TestParseOriginListAllowAll(t *testing.T) { + expected := []string{"*"} + + actual, ok := ParseOriginList("*") + + assert.True(t, ok) + assert.Equal(t, expected, actual) +} + +func TestParseOriginListEmpty(t *testing.T) { + actual, ok := ParseOriginList("") + + assert.False(t, ok) + assert.Empty(t, actual) +} + +func TestParseOriginListSingle(t *testing.T) { + expected := []string{"http://test.com"} + + actual, ok := ParseOriginList("http://test.com") + + assert.True(t, ok) + assert.Equal(t, expected, actual) +} + +func TestParseOriginListMultiple(t *testing.T) { + expected := []string{"http://test.com", "http://test2.com"} + actual, ok := ParseOriginList("http://test.com,http://test2.com") + assert.True(t, ok) + assert.Equal(t, expected, actual) +} + +func TestParseOriginListMultipleAndSpaces(t *testing.T) { + expected := []string{"http://test.com", "http://test2.com"} + actual, ok := ParseOriginList("http://test.com, http://test2.com") + assert.True(t, ok) + assert.Equal(t, expected, actual) +} diff --git a/clients/ui/bff/internal/api/middleware.go b/clients/ui/bff/internal/api/middleware.go index 6e4e299c5..cce30bcbf 100644 --- a/clients/ui/bff/internal/api/middleware.go +++ b/clients/ui/bff/internal/api/middleware.go @@ -9,6 +9,7 @@ import ( "github.com/kubeflow/model-registry/ui/bff/internal/config" "github.com/kubeflow/model-registry/ui/bff/internal/constants" "github.com/kubeflow/model-registry/ui/bff/internal/integrations" + "github.com/rs/cors" "log/slog" "net/http" "runtime/debug" @@ -66,14 +67,23 @@ func (app *App) InjectUserHeaders(next http.Handler) http.Handler { }) } -func (app *App) enableCORS(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // TODO(ederign) restrict CORS to a much smaller set of trusted origins. - // TODO(ederign) deal with preflight requests - w.Header().Set("Access-Control-Allow-Origin", "*") +func (app *App) EnableCORS(next http.Handler) http.Handler { + allowedOrigins, ok := ParseOriginList(app.config.AllowedOrigins) - next.ServeHTTP(w, r) + if !ok { + return next + } + + c := cors.New(cors.Options{ + AllowedOrigins: allowedOrigins, + AllowCredentials: true, + AllowedMethods: []string{"GET", "PUT", "POST", "PATCH", "DELETE"}, + AllowedHeaders: []string{constants.KubeflowUserIDHeader, constants.KubeflowUserGroupsIdHeader}, + Debug: app.config.DevMode, + OptionsPassthrough: false, }) + + return c.Handler(next) } func (app *App) EnableTelemetry(next http.Handler) http.Handler { diff --git a/clients/ui/bff/internal/config/environment.go b/clients/ui/bff/internal/config/environment.go index 80449363e..1efa5c8d2 100644 --- a/clients/ui/bff/internal/config/environment.go +++ b/clients/ui/bff/internal/config/environment.go @@ -9,4 +9,5 @@ type EnvConfig struct { DevModePort int StaticAssetsDir string LogLevel string + AllowedOrigins string } diff --git a/clients/ui/bff/internal/constants/keys.go b/clients/ui/bff/internal/constants/keys.go index 3051679f5..e4c9f1aba 100644 --- a/clients/ui/bff/internal/constants/keys.go +++ b/clients/ui/bff/internal/constants/keys.go @@ -2,6 +2,8 @@ package constants type contextKey string +// NOTE: If you are adding any HTTP headers, they need to also be added to the EnableCORS middleware +// to ensure requests are not blocked when using CORS. const ( ModelRegistryHttpClientKey contextKey = "ModelRegistryHttpClientKey" NamespaceHeaderParameterKey contextKey = "namespace"