From 2e83eedea9e8fe3e4ecf2d58d0fb574d54b53def Mon Sep 17 00:00:00 2001 From: Hans Kristian Flaatten Date: Tue, 18 Aug 2026 13:51:38 +0200 Subject: [PATCH 1/4] feat(unleash): send the pre-shared key to bifrost MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bifrost added pre-shared-key authentication (nais/bifrost#549) and is running in its accept phase: unauthenticated calls are allowed but logged as "Request without a valid API key allowed (authentication not enforced)". Every call nais-api makes is currently in that log. Sends the key as "Authorization: Bearer " on every bifrost request, read from UNLEASH_BIFROST_API_KEY and provisioned through fasit to both deployments. An unset key sends no header at all rather than an empty one — a present-but-empty credential is worse than none, since it looks like a failed authentication rather than an unauthenticated caller. Startup logs a warning in that case, because it is the state that breaks the moment bifrost sets auth.enforced. This is step 3 of nais/bifrost#576. It must be deployed and confirmed before that flag is flipped; until then nothing changes behaviourally, since bifrost accepts both. Tests cover that the header is sent with the configured key and that no header is sent without one; the first fails if the request editor is removed. --- internal/cmd/api/api.go | 3 +- internal/cmd/api/config.go | 4 ++ internal/cmd/api/http.go | 3 +- internal/unleash/bifrost.go | 22 +++++++++- internal/unleash/bifrost_auth_test.go | 62 +++++++++++++++++++++++++++ internal/unleash/bifrost_test.go | 16 +++---- internal/unleash/config_test.go | 2 +- internal/unleash/dataloader.go | 8 ++-- 8 files changed, 103 insertions(+), 17 deletions(-) create mode 100644 internal/unleash/bifrost_auth_test.go diff --git a/internal/cmd/api/api.go b/internal/cmd/api/api.go index 38b8e97e8..226dcd9f0 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,7 @@ 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")) + 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/unleash/bifrost.go b/internal/unleash/bifrost.go index fa45384b2..a67ebcd64 100644 --- a/internal/unleash/bifrost.go +++ b/internal/unleash/bifrost.go @@ -29,12 +29,30 @@ type bifrostClientImpl struct { // 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. It is sent as +// "Authorization: Bearer " on every request. An empty key sends no header, +// which bifrost currently accepts and logs — that is the accept-then-enforce +// phase. Once bifrost sets auth.enforced, an empty key means every call is +// rejected with 401, so the key must be configured before that flag is flipped. +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 apiKey != "" { + opts = append(opts, bifrostclient.WithRequestEditorFn( + func(ctx context.Context, req *http.Request) error { + req.Header.Set("Authorization", "Bearer "+apiKey) + return nil + }, + )) + } else { + log.Warn("No bifrost API key configured; requests will be unauthenticated. This fails once bifrost enforces authentication.") + } + + 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..0aff7784b --- /dev/null +++ b/internal/unleash/bifrost_auth_test.go @@ -0,0 +1,62 @@ +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") + } +} 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)) From fb64a759a87567336dce82425b7384f54aaba629 Mon Sep 17 00:00:00 2001 From: Hans Kristian Flaatten Date: Tue, 18 Aug 2026 15:16:29 +0200 Subject: [PATCH 2/4] fix(unleash): actually deliver the bifrost key to the pod MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit reads UNLEASH_BIFROST_API_KEY but nothing set it. The chart wires UNLEASH_BIFROST_API_URL and no key, so the client would have logged "No bifrost API key configured" and kept sending unauthenticated requests — the rollout would have looked done while changing nothing. Wires it end to end: the value is sourced from the fasit management value bifrost_api_key, lands in the release Secret alongside the other pre-shared keys, and reaches the container through the existing envFrom. Same pattern as HOOKD_PSK and REST_PRE_SHARED_KEY, which is where a secret belongs rather than a plain env value in the deployment. --- charts/nais-api/Feature.yaml | 10 ++++++++++ charts/nais-api/templates/secret.yaml | 1 + charts/nais-api/values.yaml | 3 +++ 3 files changed, 14 insertions(+) diff --git a/charts/nais-api/Feature.yaml b/charts/nais-api/Feature.yaml index c8d660e31..9f08e4ca2 100644 --- a/charts/nais-api/Feature.yaml +++ b/charts/nais-api/Feature.yaml @@ -200,6 +200,16 @@ 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. Same value the bifrost deployment accepts. + config: + type: string + secret: true + 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 From 57a6fa6092a1a8951b68932b00f3f95b5e9e7c49 Mon Sep 17 00:00:00 2001 From: Hans Kristian Flaatten Date: Tue, 18 Aug 2026 19:27:49 +0200 Subject: [PATCH 3/4] feat(chart): source the bifrost key from fasit The key is computed from the management value. It is deliberately not overridable per environment: staging the rollout is done with bifrost's enforcement toggle, not by varying who holds the credential. One key, one place it comes from. Refs nais/bifrost#576 --- charts/nais-api/Feature.yaml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/charts/nais-api/Feature.yaml b/charts/nais-api/Feature.yaml index 9f08e4ca2..81640646d 100644 --- a/charts/nais-api/Feature.yaml +++ b/charts/nais-api/Feature.yaml @@ -202,10 +202,7 @@ values: unleash.bifrostApiKey: displayName: Bifrost API pre-shared key - description: Pre-shared key authenticating nais-api to the bifrost API. Same value the bifrost deployment accepts. - config: - type: string - secret: true + 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 }} From bc58f44918a81d372b2dafd21d581fd8236bee55 Mon Sep 17 00:00:00 2001 From: Hans Kristian Flaatten Date: Tue, 18 Aug 2026 19:58:37 +0200 Subject: [PATCH 4/4] fix(unleash): send one key, fix the tagged build, log the warning once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings. The integration harness did not compile. api.ConfigureGraph gained a parameter and internal/integration/manager.go was not updated. The package sits behind //go:build integration_test, so go build ./... and go vet ./... both passed while CI — which runs go test -tags integration_test ./... — would have failed. Key rotation could not work. bifrost and nais-api read the same fasit value, and bifrost accepts a comma-separated list so keys can be rotated without downtime. nais-api sent the list verbatim, so a rotation value of "new,old" produced a credential matching neither key. Before enforcement that shows up as unauthenticated_allowed; after it, a full outage at the worst possible moment. The client now presents the first entry, so rotation is: set "new,old", let both sides settle, then drop the old one. The missing-key warning fired per request. NewBifrostClient is constructed per GraphQL request via the dataloader, so the warning would have emitted thousands of lines an hour in every environment until the key was provisioned. It is reported once at startup instead. --- internal/cmd/api/api.go | 6 ++++ internal/integration/manager.go | 1 + internal/unleash/bifrost.go | 33 +++++++++++++------ internal/unleash/bifrost_auth_test.go | 46 +++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 9 deletions(-) diff --git a/internal/cmd/api/api.go b/internal/cmd/api/api.go index 226dcd9f0..420ba0e77 100644 --- a/internal/cmd/api/api.go +++ b/internal/cmd/api/api.go @@ -429,6 +429,12 @@ func run(ctx context.Context, cfg *Config, log logrus.FieldLogger) error { if cfg.Unleash.BifrostAPIURL == unleash.FakeBifrostURL { bifrostClient = unleash.NewFakeBifrostClient(watchers.UnleashWatcher) } else { + // 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")) } 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 a67ebcd64..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,30 +28,44 @@ 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. // -// apiKey is the pre-shared key bifrost authenticates with. It is sent as -// "Authorization: Bearer " on every request. An empty key sends no header, -// which bifrost currently accepts and logs — that is the accept-then-enforce -// phase. Once bifrost sets auth.enforced, an empty key means every call is -// rejected with 401, so the key must be configured before that flag is flipped. +// 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), } opts := []bifrostclient.ClientOption{bifrostclient.WithHTTPClient(httpClient)} - if apiKey != "" { + if key := ActiveBifrostAPIKey(apiKey); key != "" { opts = append(opts, bifrostclient.WithRequestEditorFn( func(ctx context.Context, req *http.Request) error { - req.Header.Set("Authorization", "Bearer "+apiKey) + req.Header.Set("Authorization", "Bearer "+key) return nil }, )) - } else { - log.Warn("No bifrost API key configured; requests will be unauthenticated. This fails once bifrost enforces authentication.") } + // 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 { diff --git a/internal/unleash/bifrost_auth_test.go b/internal/unleash/bifrost_auth_test.go index 0aff7784b..879487b83 100644 --- a/internal/unleash/bifrost_auth_test.go +++ b/internal/unleash/bifrost_auth_test.go @@ -60,3 +60,49 @@ func TestNewBifrostClient_NoKeySendsNoHeader(t *testing.T) { 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) + } +}