diff --git a/charts/nais-api/Feature.yaml b/charts/nais-api/Feature.yaml index c8d660e31..81640646d 100644 --- a/charts/nais-api/Feature.yaml +++ b/charts/nais-api/Feature.yaml @@ -200,6 +200,13 @@ values: template: | {{ .Management.bifrost_unleash_namespace | quote }} + unleash.bifrostApiKey: + displayName: Bifrost API pre-shared key + description: Pre-shared key authenticating nais-api to the bifrost API — the same value the bifrost deployment accepts. + computed: + template: | + {{ .Management.bifrost_api_key | quote }} + replaceEnvironmentNames: displayName: Replace environment names description: Mapping of environment names from current name to expected name. Format `currentName1:expectedName1,currentName2:expectedName2` diff --git a/charts/nais-api/templates/secret.yaml b/charts/nais-api/templates/secret.yaml index ecfde16d6..219b79bc0 100644 --- a/charts/nais-api/templates/secret.yaml +++ b/charts/nais-api/templates/secret.yaml @@ -13,3 +13,4 @@ stringData: DATABASE_URL: "postgres://{{ .Values.database.user }}:{{ .Values.database.password }}@127.0.0.1:5432/{{ .Values.database.name }}?sslmode=disable" ZITADEL_KEY: {{ .Values.zitadel.key | quote }} AIVEN_TOKEN: "{{ .Values.aiven.token }}" + UNLEASH_BIFROST_API_KEY: "{{ .Values.unleash.bifrostApiKey }}" diff --git a/charts/nais-api/values.yaml b/charts/nais-api/values.yaml index 5933dc536..853c09193 100644 --- a/charts/nais-api/values.yaml +++ b/charts/nais-api/values.yaml @@ -74,6 +74,9 @@ unleash: enabled: false namespace: "bifrost-unleash" bifrostApiUrl: "http://bifrost-backend" + # Pre-shared key for the bifrost API; provisioned in fasit as + # bifrost_api_key and shared with the bifrost deployment. + bifrostApiKey: "" replicas: 2 diff --git a/internal/cmd/api/api.go b/internal/cmd/api/api.go index 38b8e97e8..420ba0e77 100644 --- a/internal/cmd/api/api.go +++ b/internal/cmd/api/api.go @@ -323,6 +323,7 @@ func run(ctx context.Context, cfg *Config, log logrus.FieldLogger) error { cfg.K8s.AllClusterNames(), hookdClient, cfg.Unleash.BifrostAPIURL, + cfg.Unleash.BifrostAPIKey, cfg.K8s.AllClusterNames(), cfg.Logging.DefaultLogDestinations(), notifier, @@ -428,7 +429,13 @@ func run(ctx context.Context, cfg *Config, log logrus.FieldLogger) error { if cfg.Unleash.BifrostAPIURL == unleash.FakeBifrostURL { bifrostClient = unleash.NewFakeBifrostClient(watchers.UnleashWatcher) } else { - bifrostClient = unleash.NewBifrostClient(cfg.Unleash.BifrostAPIURL, log.WithField("subsystem", "bifrost_client")) + // Reported once here rather than in the constructor, which runs per + // request via the dataloader. This is the state that starts failing the + // moment bifrost enables enforcement. + if unleash.ActiveBifrostAPIKey(cfg.Unleash.BifrostAPIKey) == "" { + log.Warn("No bifrost API key configured; requests to bifrost will be unauthenticated and will be rejected once bifrost enforces authentication") + } + bifrostClient = unleash.NewBifrostClient(cfg.Unleash.BifrostAPIURL, cfg.Unleash.BifrostAPIKey, log.WithField("subsystem", "bifrost_client")) } issueChecker, err := checker.New( diff --git a/internal/cmd/api/config.go b/internal/cmd/api/config.go index 9d8de849d..4a6004ca7 100644 --- a/internal/cmd/api/config.go +++ b/internal/cmd/api/config.go @@ -105,6 +105,10 @@ type oAuthConfig struct { type unleashConfig struct { // BifrostApiEndpoint is the endpoint for the Bifrost API BifrostAPIURL string `env:"UNLEASH_BIFROST_API_URL,default=*fake*"` + // BifrostAPIKey is the pre-shared key sent to the Bifrost API. Provisioned + // via fasit to both deployments; empty means unauthenticated requests, which + // bifrost accepts only until it enables enforcement. + BifrostAPIKey string `env:"UNLEASH_BIFROST_API_KEY"` } type loggingConfig struct { diff --git a/internal/cmd/api/http.go b/internal/cmd/api/http.go index 155180265..9d4c19372 100644 --- a/internal/cmd/api/http.go +++ b/internal/cmd/api/http.go @@ -192,6 +192,7 @@ func ConfigureGraph( clusters []string, hookdClient hookd.Client, bifrostAPIURL string, + bifrostAPIKey string, allowedClusters []string, defaultLogDestinations []logging.SupportedLogDestination, notifier *notify.Notifier, @@ -374,7 +375,7 @@ func ConfigureGraph( ctx = serviceaccount.NewLoaderContext(ctx, pool) ctx = session.NewLoaderContext(ctx, pool) ctx = search.NewLoaderContext(ctx, pool, searcher) - ctx = unleash.NewLoaderContext(ctx, tenantName, watchers.UnleashWatcher, bifrostAPIURL, allowedClusters, log) + ctx = unleash.NewLoaderContext(ctx, tenantName, watchers.UnleashWatcher, bifrostAPIURL, bifrostAPIKey, allowedClusters, log) ctx = tunnel.WithLoaders(ctx, tunnel.NewLoaders(watchers.TunnelWatcher)) ctx = logging.NewPackageContext(ctx, tenantName, defaultLogDestinations) ctx = environment.NewLoaderContext(ctx, pool) diff --git a/internal/integration/manager.go b/internal/integration/manager.go index e673c6b4f..061d3b4e5 100644 --- a/internal/integration/manager.go +++ b/internal/integration/manager.go @@ -284,6 +284,7 @@ func newGQLRunner( clusters(), fakeHookd.New(), unleash.FakeBifrostURL, + "", // bifrost API key: the harness uses the fake client, which needs none []string{"dev", "staging", "dev-fss", "dev-gcp"}, []logging.SupportedLogDestination{logging.Loki}, notifier, diff --git a/internal/unleash/bifrost.go b/internal/unleash/bifrost.go index fa45384b2..99cd7707f 100644 --- a/internal/unleash/bifrost.go +++ b/internal/unleash/bifrost.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "strings" "github.com/nais/bifrost/pkg/bifrostclient" "github.com/sirupsen/logrus" @@ -27,14 +28,46 @@ type bifrostClientImpl struct { log logrus.FieldLogger } +// ActiveBifrostAPIKey returns the key to present to bifrost, given the +// configured value. +// +// bifrost and nais-api read the same fasit value, and bifrost accepts a +// comma-separated list so keys can be rotated without downtime. A client must +// send exactly one of them, so the first entry is the active key. Rotation is +// therefore: set "new,old" (bifrost accepts both, nais-api presents new), then +// drop the old one. Sending the list verbatim would match nothing. +func ActiveBifrostAPIKey(configured string) string { + first, _, _ := strings.Cut(configured, ",") + return strings.TrimSpace(first) +} + // NewBifrostClient creates a new BifrostClient with the given base URL and logger. // The client uses OpenTelemetry-instrumented HTTP transport for tracing. -func NewBifrostClient(baseURL string, log logrus.FieldLogger) BifrostClient { +// +// apiKey is the pre-shared key bifrost authenticates with, sent as +// "Authorization: Bearer " on every request. An empty key sends no header +// at all — not an empty one — which bifrost accepts and counts during the +// accept-then-enforce phase. Once bifrost sets auth.enforced, an empty key +// means every call is rejected, so the key must be in place before that flip. +func NewBifrostClient(baseURL, apiKey string, log logrus.FieldLogger) BifrostClient { httpClient := &http.Client{ Transport: otelhttp.NewTransport(http.DefaultTransport), } - client, err := bifrostclient.NewClientWithResponses(baseURL, bifrostclient.WithHTTPClient(httpClient)) + opts := []bifrostclient.ClientOption{bifrostclient.WithHTTPClient(httpClient)} + if key := ActiveBifrostAPIKey(apiKey); key != "" { + opts = append(opts, bifrostclient.WithRequestEditorFn( + func(ctx context.Context, req *http.Request) error { + req.Header.Set("Authorization", "Bearer "+key) + return nil + }, + )) + } + // No warning here: this constructor runs per request via the dataloader, so + // logging the missing-key state would emit a line on every GraphQL call. + // It is reported once at startup instead. + + client, err := bifrostclient.NewClientWithResponses(baseURL, opts...) if err != nil { // This should only fail if the base URL is invalid log.WithError(err).Fatal("failed to create bifrost client") diff --git a/internal/unleash/bifrost_auth_test.go b/internal/unleash/bifrost_auth_test.go new file mode 100644 index 000000000..879487b83 --- /dev/null +++ b/internal/unleash/bifrost_auth_test.go @@ -0,0 +1,108 @@ +package unleash + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/sirupsen/logrus" +) + +// The pre-shared key must reach bifrost on every request. bifrost accepts +// "Authorization: Bearer "; sending nothing is only tolerated until it +// enables enforcement, after which an unauthenticated call is a 401. +func TestNewBifrostClient_SendsPreSharedKey(t *testing.T) { + var gotAuth string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotAuth = r.Header.Get("Authorization") + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`[]`)) + })) + t.Cleanup(srv.Close) + + logger := logrus.New() + logger.SetOutput(io.Discard) + + client := NewBifrostClient(srv.URL, "s3cret", logger) + if _, err := client.ListInstances(context.Background()); err != nil { + t.Fatalf("list: %v", err) + } + + if want := "Bearer s3cret"; gotAuth != want { + t.Fatalf("Authorization header = %q, want %q", gotAuth, want) + } +} + +// An unset key must not send an empty or malformed header — that would be worse +// than sending none, since bifrost would see a present-but-invalid credential. +func TestNewBifrostClient_NoKeySendsNoHeader(t *testing.T) { + var hadAuth bool + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, hadAuth = r.Header["Authorization"] + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`[]`)) + })) + t.Cleanup(srv.Close) + + logger := logrus.New() + logger.SetOutput(io.Discard) + + client := NewBifrostClient(srv.URL, "", logger) + if _, err := client.ListInstances(context.Background()); err != nil { + t.Fatalf("list: %v", err) + } + + if hadAuth { + t.Fatal("no key configured, but an Authorization header was sent") + } +} + +// bifrost and nais-api read the same fasit value, and bifrost accepts a +// comma-separated list so keys can be rotated without downtime. A client must +// present exactly one of them — sending the list verbatim matches nothing, and +// under enforcement that is a full outage at the worst possible moment. +func TestActiveBifrostAPIKey(t *testing.T) { + for _, tc := range []struct { + name, configured, want string + }{ + {"single key", "abc123", "abc123"}, + {"rotation: first is active", "new,old", "new"}, + {"whitespace is trimmed", " new , old ", "new"}, + {"empty stays empty", "", ""}, + {"only separators is empty", " , ", ""}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := ActiveBifrostAPIKey(tc.configured); got != tc.want { + t.Fatalf("ActiveBifrostAPIKey(%q) = %q, want %q", tc.configured, got, tc.want) + } + }) + } +} + +// The whole point of picking one key: a rotation value must produce a header +// bifrost can actually match. +func TestNewBifrostClient_SendsOneKeyDuringRotation(t *testing.T) { + var gotAuth string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotAuth = r.Header.Get("Authorization") + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`[]`)) + })) + t.Cleanup(srv.Close) + + logger := logrus.New() + logger.SetOutput(io.Discard) + + client := NewBifrostClient(srv.URL, "newkey,oldkey", logger) + if _, err := client.ListInstances(context.Background()); err != nil { + t.Fatalf("list: %v", err) + } + + if want := "Bearer newkey"; gotAuth != want { + t.Fatalf("Authorization header = %q, want %q — the list must not be sent verbatim", gotAuth, want) + } +} diff --git a/internal/unleash/bifrost_test.go b/internal/unleash/bifrost_test.go index 522c1c9f2..ea70760cc 100644 --- a/internal/unleash/bifrost_test.go +++ b/internal/unleash/bifrost_test.go @@ -91,7 +91,7 @@ func TestBifrostClient_CreateInstance(t *testing.T) { defer s.Close() logger, _ := test.NewNullLogger() - bifrostClient := unleash.NewBifrostClient(s.URL, logger) + bifrostClient := unleash.NewBifrostClient(s.URL, "", logger) name := "test" allowedTeams := "team1,team2" @@ -182,7 +182,7 @@ func TestBifrostClient_UpdateInstance(t *testing.T) { defer s.Close() logger, _ := test.NewNullLogger() - client := unleash.NewBifrostClient(s.URL, logger) + client := unleash.NewBifrostClient(s.URL, "", logger) releaseChannel := "rapid" req := bifrostclient.UnleashConfigRequest{ @@ -240,7 +240,7 @@ func TestBifrostClient_GetInstance(t *testing.T) { defer s.Close() logger, _ := test.NewNullLogger() - client := unleash.NewBifrostClient(s.URL, logger) + client := unleash.NewBifrostClient(s.URL, "", logger) resp, err := client.GetInstance(context.Background(), "my-team") if err != nil { @@ -271,7 +271,7 @@ func TestBifrostClient_DeleteInstance(t *testing.T) { defer s.Close() logger, _ := test.NewNullLogger() - client := unleash.NewBifrostClient(s.URL, logger) + client := unleash.NewBifrostClient(s.URL, "", logger) _, err := client.DeleteInstance(context.Background(), "my-team") if err != nil { @@ -322,7 +322,7 @@ func TestBifrostClient_ListChannels(t *testing.T) { defer s.Close() logger, _ := test.NewNullLogger() - client := unleash.NewBifrostClient(s.URL, logger) + client := unleash.NewBifrostClient(s.URL, "", logger) resp, err := client.ListChannels(context.Background()) if err != nil { @@ -382,7 +382,7 @@ func TestBifrostClient_ErrorHandling_CreateInstance(t *testing.T) { defer s.Close() logger, _ := test.NewNullLogger() - client := unleash.NewBifrostClient(s.URL, logger) + client := unleash.NewBifrostClient(s.URL, "", logger) name := "test" _, err := client.CreateInstance(context.Background(), bifrostclient.UnleashConfigRequest{ @@ -410,7 +410,7 @@ func TestBifrostClient_ErrorHandling_UpdateInstance(t *testing.T) { defer s.Close() logger, _ := test.NewNullLogger() - client := unleash.NewBifrostClient(s.URL, logger) + client := unleash.NewBifrostClient(s.URL, "", logger) releaseChannel := "stable" _, err := client.UpdateInstance(context.Background(), "my-team", bifrostclient.UnleashConfigRequest{ @@ -436,7 +436,7 @@ func TestBifrostClient_ErrorHandling_ListChannels(t *testing.T) { defer s.Close() logger, _ := test.NewNullLogger() - client := unleash.NewBifrostClient(s.URL, logger) + client := unleash.NewBifrostClient(s.URL, "", logger) _, err := client.ListChannels(context.Background()) diff --git a/internal/unleash/config_test.go b/internal/unleash/config_test.go index af696f5d5..637eac0b4 100644 --- a/internal/unleash/config_test.go +++ b/internal/unleash/config_test.go @@ -102,7 +102,7 @@ func TestAllowedClustersMapping(t *testing.T) { logger, _ := test.NewNullLogger() // Create a bifrost client - client := NewBifrostClient(s.URL, logger) + client := NewBifrostClient(s.URL, "", logger) // Simulate what happens in newLoaders function mappedClusters := make([]string, len(tt.clusters)) diff --git a/internal/unleash/dataloader.go b/internal/unleash/dataloader.go index 697f98548..88f812cea 100644 --- a/internal/unleash/dataloader.go +++ b/internal/unleash/dataloader.go @@ -38,8 +38,8 @@ func (r mimirRoundTrip) RoundTrip(req *http.Request) (*http.Response, error) { // NewLoaderContext creates a new context with a loaders value. // If *fake* is provided as bifrostAPIURL, a fake client will be used. -func NewLoaderContext(ctx context.Context, tenantName string, appWatcher *watcher.Watcher[*UnleashInstance], bifrostAPIURL string, allowedClusters []string, log logrus.FieldLogger) context.Context { - return context.WithValue(ctx, loadersKey, newLoaders(tenantName, appWatcher, bifrostAPIURL, allowedClusters, log)) +func NewLoaderContext(ctx context.Context, tenantName string, appWatcher *watcher.Watcher[*UnleashInstance], bifrostAPIURL, bifrostAPIKey string, allowedClusters []string, log logrus.FieldLogger) context.Context { + return context.WithValue(ctx, loadersKey, newLoaders(tenantName, appWatcher, bifrostAPIURL, bifrostAPIKey, allowedClusters, log)) } func NewWatcher(ctx context.Context, mgr *watcher.Manager) *watcher.Watcher[*UnleashInstance] { @@ -60,14 +60,14 @@ type loaders struct { log logrus.FieldLogger } -func newLoaders(tenantName string, appWatcher *watcher.Watcher[*UnleashInstance], bifrostAPIURL string, allowedClusters []string, log logrus.FieldLogger) *loaders { +func newLoaders(tenantName string, appWatcher *watcher.Watcher[*UnleashInstance], bifrostAPIURL, bifrostAPIKey string, allowedClusters []string, log logrus.FieldLogger) *loaders { var client BifrostClient var prometheus Prometheus if bifrostAPIURL == FakeBifrostURL { client = NewFakeBifrostClient(appWatcher) prometheus = NewFakePrometheusClient() } else { - client = NewBifrostClient(bifrostAPIURL, log) + client = NewBifrostClient(bifrostAPIURL, bifrostAPIKey, log) promClient, err := promapi.NewClient(promapi.Config{Address: prometheusURL, RoundTripper: mimirRoundTrip{HeaderValue: "nais"}}) if err != nil { panic(fmt.Errorf("failed to create prometheus client: %w", err))