Skip to content

Commit

Permalink
Adds BFF support for CORS
Browse files Browse the repository at this point in the history
Signed-off-by: Alex Creasy <[email protected]>
  • Loading branch information
alexcreasy committed Jan 17, 2025
1 parent 350d705 commit a28f759
Show file tree
Hide file tree
Showing 11 changed files with 116 additions and 9 deletions.
3 changes: 2 additions & 1 deletion clients/ui/bff/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
38 changes: 38 additions & 0 deletions clients/ui/bff/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
```

1 change: 1 addition & 0 deletions clients/ui/bff/cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
3 changes: 2 additions & 1 deletion clients/ui/bff/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions clients/ui/bff/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
2 changes: 1 addition & 1 deletion clients/ui/bff/internal/api/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))))
}
12 changes: 12 additions & 0 deletions clients/ui/bff/internal/api/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
39 changes: 39 additions & 0 deletions clients/ui/bff/internal/api/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
22 changes: 16 additions & 6 deletions clients/ui/bff/internal/api/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions clients/ui/bff/internal/config/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ type EnvConfig struct {
DevModePort int
StaticAssetsDir string
LogLevel string
AllowedOrigins string
}
2 changes: 2 additions & 0 deletions clients/ui/bff/internal/constants/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit a28f759

Please sign in to comment.