diff --git a/relay/concurrent_keys_views_test.go b/relay/concurrent_keys_views_test.go new file mode 100644 index 00000000..dbf0599a --- /dev/null +++ b/relay/concurrent_keys_views_test.go @@ -0,0 +1,361 @@ +package relay + +// Ingestion-time rejection of credentials scoped to a view (payload filtering). +// +// A view-scoped key may only see a subset of its environment's flags. The Relay Proxy serves the whole +// environment payload and has no view support, so accepting one would silently over-deliver every flag +// in the environment to an SDK entitled to a subset. These tests pin that such a key never reaches the +// accepted set — from either source — while everything around it keeps working. +// +// They mirror the four cases in concurrent_keys_auth_test.go and reuse its harnesses and fixtures +// (multiKeyEnvRep / multiKeyArchiveEnv, assertSDKEndpointsAvailability, awaitClient, awaitStreamClosed). +// The anchor and primary mobile key are the deliberate exception: the marker is ignored on them rather +// than taking the environment down, which is what TestConcurrentKeys*_ViewScopedMarkerOnDesignatedKeys* +// covers. + +import ( + "encoding/json" + "net/http" + "regexp" + "testing" + "time" + + "github.com/launchdarkly/ld-relay/v8/config" + "github.com/launchdarkly/ld-relay/v8/internal/api" + "github.com/launchdarkly/ld-relay/v8/internal/credential" + "github.com/launchdarkly/ld-relay/v8/internal/envfactory" + "github.com/launchdarkly/ld-relay/v8/internal/sdkauth" + "github.com/launchdarkly/ld-relay/v8/internal/sdks" + "github.com/launchdarkly/ld-relay/v8/internal/sharedtest" + "github.com/launchdarkly/ld-relay/v8/internal/sharedtest/configsource" + "github.com/launchdarkly/ld-relay/v8/internal/sharedtest/testclient" + + "github.com/launchdarkly/eventsource" + "github.com/launchdarkly/go-configtypes" + "github.com/launchdarkly/go-sdk-common/v3/ldlog" + "github.com/launchdarkly/go-sdk-common/v3/ldlogtest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + // A non-anchor SDK key and a non-primary mobile key, each scoped to a view. + viewScopedSDKKey = config.SDKKey("sdk-view-scoped") + viewScopedMobileKey = config.MobileKey("mob-view-scoped") + + // The wire identifiers of those two entries. Non-secret, and what the WARN names. + viewScopedSDKID = "view-scoped-sdk" + viewScopedMobID = "view-scoped-mob" +) + +// The standard two-entry arrays plus a third entry scoped to a view, in each of the four shapes the +// two harnesses need (wire reps for RAC, accepted-key params for the offline archive). + +func viewScopedSDKKeyReps() []envfactory.ConcurrentKeyRep { + return append(defaultSDKKeyReps(), + envfactory.ConcurrentKeyRep{Key: viewScopedSDKID, Value: string(viewScopedSDKKey), HasViews: true}) +} + +func viewScopedMobileKeyReps() []envfactory.ConcurrentKeyRep { + return append(defaultMobileKeyReps(), + envfactory.ConcurrentKeyRep{Key: viewScopedMobID, Value: string(viewScopedMobileKey), HasViews: true}) +} + +func viewScopedAcceptedSDKKeys() []envfactory.AcceptedSDKKey { + return append(defaultAcceptedSDKKeys(), + envfactory.AcceptedSDKKey{Key: viewScopedSDKID, Value: viewScopedSDKKey, HasViews: true}) +} + +func viewScopedAcceptedMobileKeys() []envfactory.AcceptedMobileKey { + return append(defaultAcceptedMobileKeys(), + envfactory.AcceptedMobileKey{Key: viewScopedMobID, Value: viewScopedMobileKey, HasViews: true}) +} + +// assertViewScopedKeysAbsentFromStatus verifies the /status sdkKeys[]/mobileKeys[] arrays do not list +// the view-scoped credentials. Those arrays are rendered straight from the accepted set, so this is the +// externally-visible proof that a filtered key carries no state anywhere in the environment. +func assertViewScopedKeysAbsentFromStatus(t *testing.T, relay *Relay) { + t.Helper() + req, _ := http.NewRequest("GET", "/status", nil) + result, body := sharedtest.DoRequest(req, relay) + require.Equal(t, http.StatusOK, result.StatusCode) + + var status api.StatusRep + require.NoError(t, json.Unmarshal(body, &status)) + require.Len(t, status.Environments, 1) + var envStatus api.EnvironmentStatusRep + for _, e := range status.Environments { + envStatus = e + } + + assert.Nil(t, findSDKKeyStatus(envStatus.SDKKeys, sdks.ObscureKey(string(viewScopedSDKKey))), + "a view-scoped SDK key must not appear in the status sdkKeys[] array") + assert.Nil(t, findSDKKeyStatus(envStatus.MobileKeys, sdks.ObscureKey(string(viewScopedMobileKey))), + "a view-scoped mobile key must not appear in the status mobileKeys[] array") + + // The keys that were accepted are still all there — the filter is surgical, not a blanket drop. + assert.NotNil(t, findSDKKeyStatus(envStatus.SDKKeys, sdks.ObscureKey(string(anchorSDKKey)))) + assert.NotNil(t, findSDKKeyStatus(envStatus.SDKKeys, sdks.ObscureKey(string(extraSDKKey)))) + assert.NotNil(t, findSDKKeyStatus(envStatus.MobileKeys, sdks.ObscureKey(string(anchorMobileKey)))) + assert.NotNil(t, findSDKKeyStatus(envStatus.MobileKeys, sdks.ObscureKey(string(extraMobileKey)))) + + // Exact counts, so a filtered key surfacing under an unexpected obscured value is still caught. + assert.Len(t, envStatus.SDKKeys, 2) + assert.Len(t, envStatus.MobileKeys, 2) +} + +// assertViewScopedKeysRejectedWarning verifies the ingestion WARN names the environment and each +// rejected identifier. The identifier is the non-secret wire name, so it is logged unobscured — an +// operator needs it to find the key in the LaunchDarkly UI. +// +// It also pins that the WARN is emitted exactly once. The design deliberately keeps +// StreamManager.validateCredentialPayload silent — it runs on every environment of every payload — so +// logging from there would double every one of these. A count assertion is what makes that regress +// loudly instead of silently. +func assertViewScopedKeysRejectedWarning(t *testing.T, mockLog *ldlogtest.MockLog) { + t.Helper() + pattern := multiKeyIdentifiers.GetDisplayName() + ".*rejecting credentials scoped to a view: " + + viewScopedSDKID + ", " + viewScopedMobID + mockLog.AssertMessageMatch(t, true, ldlog.Warn, pattern) + + re := regexp.MustCompile(pattern) + matches := 0 + for _, line := range mockLog.GetOutput(ldlog.Warn) { + if re.MatchString(line) { + matches++ + } + } + assert.Equal(t, 1, matches, "the view-scoped WARN must be logged exactly once per payload") +} + +// A view-scoped key never enters the accepted set: it is rejected downstream while the anchor and the +// non-view-scoped sibling in the same environment keep working. + +func TestConcurrentKeysRAC_ViewScopedKeysAreRejected(t *testing.T) { + putEvent := configsource.MakeAutoConfigPutEvent( + multiKeyEnvRep(viewScopedSDKKeyReps(), viewScopedMobileKeyReps(), 1)) + autoConfTest(t, testAutoConfDefaultConfig, &putEvent, func(p autoConfTestParams) { + // The anchor still opens the single upstream client; a rejected sibling changes nothing here. + anchorClient := p.awaitClient() + assert.Equal(t, anchorSDKKey, anchorClient.Key) + p.shouldNotCreateClient(200 * time.Millisecond) + + env := p.awaitEnvironment(multiKeyEnvID) + + p.assertSDKEndpointsAvailability(true, anchorSDKKey, anchorMobileKey, multiKeyEnvID) + p.assertSDKEndpointsAvailability(true, extraSDKKey, extraMobileKey, "") + p.assertSDKEndpointsAvailability(false, viewScopedSDKKey, viewScopedMobileKey, "") + + // Absent from the accepted set entirely. That set is the single source for /status, event + // forwarding, and the expiry ticker's schedule, so a filtered key can reach none of them. + accepted := env.GetAcceptedKeys() + assert.NotContains(t, accepted.Server, viewScopedSDKKey) + assert.NotContains(t, accepted.Mobile, viewScopedMobileKey) + + assertViewScopedKeysAbsentFromStatus(t, p.relay) + assertViewScopedKeysRejectedWarning(t, p.mockLog) + }) +} + +func TestConcurrentKeysOffline_ViewScopedKeysAreRejected(t *testing.T) { + offlineModeTest(t, config.Config{}, func(p offlineModeTestParams) { + p.updateHandler.AddEnvironment(multiKeyArchiveEnv( + viewScopedAcceptedSDKKeys(), viewScopedAcceptedMobileKeys())) + + anchorClient := p.awaitClient() + assert.Equal(t, anchorSDKKey, anchorClient.Key) + p.shouldNotCreateClient(200 * time.Millisecond) + + env := p.awaitEnvironment(multiKeyEnvID) + + p.assertSDKEndpointsAvailability(true, anchorSDKKey, anchorMobileKey, multiKeyEnvID) + p.assertSDKEndpointsAvailability(true, extraSDKKey, extraMobileKey, "") + p.assertSDKEndpointsAvailability(false, viewScopedSDKKey, viewScopedMobileKey, "") + + accepted := env.GetAcceptedKeys() + assert.NotContains(t, accepted.Server, viewScopedSDKKey) + assert.NotContains(t, accepted.Mobile, viewScopedMobileKey) + + assertViewScopedKeysAbsentFromStatus(t, p.relay) + assertViewScopedKeysRejectedWarning(t, p.mockLog) + }) +} + +// A malformed payload that also carries view-scoped keys logs the malformed error and stays silent +// about the view-scoped ones. +// +// The handler discards the whole payload and preserves the previous credentials in that case, so it +// never actually rejected anything — a WARN naming keys would describe an action that did not happen. +// BuildAcceptedSet enforces this by returning an empty ViewScopedKeys on every error path; this pins +// that the handler honors it, which moving the log call above the error branch would break. +func TestConcurrentKeysOffline_MalformedPayloadSuppressesViewScopedWarning(t *testing.T) { + offlineModeTest(t, config.Config{}, func(p offlineModeTestParams) { + // The anchor is absent from the accepted SDK keys — structurally malformed — alongside a + // view-scoped entry that would otherwise be reported as rejected. + p.updateHandler.AddEnvironment(multiKeyArchiveEnv( + []envfactory.AcceptedSDKKey{ + {Key: "other-sdk", Value: config.SDKKey("sdk-not-the-anchor")}, + {Key: viewScopedSDKID, Value: viewScopedSDKKey, HasViews: true}, + }, + []envfactory.AcceptedMobileKey{{Key: "anchor-mob", Value: anchorMobileKey}}, + )) + + _ = p.awaitEnvironment(multiKeyEnvID) + + p.mockLog.AssertMessageMatch(t, true, ldlog.Error, "Malformed credential payload for offline environment") + p.mockLog.AssertMessageMatch(t, false, ldlog.Warn, "rejecting credentials scoped to a view") + }) +} + +// A key that gains a view mid-session is revoked on the next payload, and a connected SDK using it is +// disconnected. +// +// This needs no new production code: reconcileAcceptedKeys revokes any key absent from the desired set +// immediately rather than on an expiry timestamp, and RemoveConnectionMapping unmaps before the streams +// are torn down so a reconnect is rejected. These tests assert that behavior rather than build it. The +// live teardown is verified on the offline path, which uses a real SDK client that actually serves +// stream data (mirroring TestConcurrentKeysOffline_ConnectedSDKDisconnectedWhenKeyExpires). +func TestConcurrentKeysOffline_ConnectedSDKDisconnectedWhenKeyGainsView(t *testing.T) { + // The server-side stream (/all) emits "put"; the mobile streams (/meval, /mping) emit "ping". + run := func(t *testing.T, streamPath, firstEvent string, connectKey credential.SDKCredential, viewOnSDK bool) { + // Named entries, unlike the shared default* fixtures — the WARN interpolates the wire + // identifier, so this is also what lets the log assertion below name a specific key. + const ( + revokedSDKID = "extra-sdk" + revokedMobID = "extra-mob" + ) + named := func(viewScoped bool) ([]envfactory.AcceptedSDKKey, []envfactory.AcceptedMobileKey) { + sdkKeys := []envfactory.AcceptedSDKKey{ + {Key: "anchor-sdk", Value: anchorSDKKey}, + {Key: revokedSDKID, Value: extraSDKKey}, + } + mobileKeys := []envfactory.AcceptedMobileKey{ + {Key: "anchor-mob", Value: anchorMobileKey}, + {Key: revokedMobID, Value: extraMobileKey}, + } + if viewOnSDK { + sdkKeys[1].HasViews = viewScoped + } else { + mobileKeys[1].HasViews = viewScoped + } + return sdkKeys, mobileKeys + } + + offlineModeTest(t, config.Config{}, func(p offlineModeTestParams) { + p.updateHandler.AddEnvironment(multiKeyArchiveEnv(named(false))) + _ = p.awaitClient() + env := p.awaitEnvironment(multiKeyEnvID) + require.Eventually(t, func() bool { return env.GetClient() != nil }, 5*time.Second, 5*time.Millisecond) + + req := sharedtest.BuildRequestWithAuth("GET", streamPath, connectKey, nil) + sharedtest.WithStreamRequest(t, req, p.relay, func(eventCh <-chan eventsource.Event) { + // Confirm the stream is live before the key gains a view. + sharedtest.AwaitEventOfType(t, eventCh, firstEvent, 5*time.Second) + + // Reload the archive with the connected non-anchor key now scoped to a view, keeping + // the anchor and primary untouched. + p.updateHandler.UpdateEnvironment(multiKeyArchiveEnv(named(true))) + + // Revocation is immediate on reconcile — no expiry timestamp and no cleanup ticker. + awaitStreamClosed(t, eventCh, 5*time.Second) + }) + + // The revoked key no longer authenticates; the anchor and primary are undisturbed. + revokedID := revokedSDKID + if viewOnSDK { + p.assertSDKEndpointsAvailability(false, extraSDKKey, "", "") + } else { + revokedID = revokedMobID + p.assertSDKEndpointsAvailability(false, "", extraMobileKey, "") + } + p.assertSDKEndpointsAvailability(true, anchorSDKKey, anchorMobileKey, multiKeyEnvID) + + // The offline update handler logs the rejection too, not just the add handler. + p.mockLog.AssertMessageMatch(t, true, ldlog.Warn, + multiKeyIdentifiers.GetDisplayName()+".*rejecting credentials scoped to a view: "+revokedID) + + // Losing the view re-admits the key: the filter is stateless, not a one-way latch. This is + // the remediation an operator performs after seeing the WARN, so it has to work. + p.updateHandler.UpdateEnvironment(multiKeyArchiveEnv(named(false))) + if viewOnSDK { + p.assertSDKEndpointsAvailability(true, extraSDKKey, "", "") + } else { + p.assertSDKEndpointsAvailability(true, "", extraMobileKey, "") + } + p.assertSDKEndpointsAvailability(true, anchorSDKKey, anchorMobileKey, multiKeyEnvID) + }) + } + + t.Run("sdk key", func(t *testing.T) { run(t, "/all", "put", extraSDKKey, true) }) + t.Run("mobile key", func(t *testing.T) { + // base64 of {"key":"userkey","kind":"user"} — a valid context, not the legacy user format. + run(t, "/meval/eyJrZXkiOiJ1c2Vya2V5Iiwia2luZCI6InVzZXIifQ==", "ping", extraMobileKey, false) + }) +} + +// The RAC equivalent, with an open downstream stream held on the anchor while the non-anchor key gains +// a view around it: the revoked key stops routing, a reconnect with it is rejected, and the anchor's +// live connection is untouched. Mirrors TestConcurrentKeysRAC_ArrayPatchAddsAndRemovesNonAnchorKeys. +func TestConcurrentKeysRAC_KeyGainingViewIsRevokedAndOthersUndisturbed(t *testing.T) { + putEvent := configsource.MakeAutoConfigPutEvent(multiKeyEnvRep(defaultSDKKeyReps(), defaultMobileKeyReps(), 1)) + racMock := configsource.NewRACMock(t, &putEvent) + + cfg := config.Config{AutoConfig: config.AutoConfigConfig{Key: testAutoConfKey}} + cfg.Main.StreamURI, _ = configtypes.NewOptURLAbsoluteFromString(racMock.URL) + // A short cleanup interval so the expiry ticker runs during the test: a revoked view-scoped key + // must be gone outright, never left scheduled for a later drop. + cfg.Main.ExpiredCredentialCleanupInterval = configtypes.NewOptDuration(100 * time.Millisecond) + + mockLog := ldlogtest.NewMockLog() + defer mockLog.DumpIfTestFailed(t) + + relay, err := newRelayInternal(cfg, relayInternalOptions{ + loggers: mockLog.Loggers, + clientFactory: testclient.CreateDummyClient, + }) + require.NoError(t, err) + defer relay.Close() + + h := relayTestHelper{t: t, relay: relay} + env := h.awaitEnvironment(multiKeyEnvID) + require.Eventually(t, func() bool { return env.GetClient() != nil }, 5*time.Second, 5*time.Millisecond) + h.assertSDKEndpointsAvailability(true, extraSDKKey, extraMobileKey, "") + + req := sharedtest.BuildRequestWithAuth("GET", "/all", anchorSDKKey, nil) + sharedtest.WithStreamRequest(t, req, relay, func(eventCh <-chan eventsource.Event) { + sharedtest.AwaitEventOfType(t, eventCh, "put", 5*time.Second) + + // One patch marks both non-anchor entries as view-scoped, keeping the anchor and primary. + patch := multiKeyEnvRep( + []envfactory.ConcurrentKeyRep{ + {Key: "anchor-sdk", Value: string(anchorSDKKey)}, + {Key: "extra-sdk", Value: string(extraSDKKey), HasViews: true}, + }, + []envfactory.ConcurrentKeyRep{ + {Key: "anchor-mob", Value: string(anchorMobileKey)}, + {Key: "extra-mob", Value: string(extraMobileKey), HasViews: true}, + }, + 2, + ) + racMock.Send(configsource.MakeAutoConfigPatchEvent(patch)) + + // Both stop routing, so a reconnect presenting either one is rejected. + require.Eventually(t, func() bool { + _, errSDK := relay.getEnvironment(sdkauth.New(extraSDKKey)) + _, errMobile := relay.getEnvironment(sdkauth.New(extraMobileKey)) + return errSDK != nil && errMobile != nil + }, 5*time.Second, 5*time.Millisecond, "keys that gained a view were not revoked") + + // The anchor's open stream is undisturbed by the revocation alongside it. + assertStreamStaysOpen(t, eventCh, 500*time.Millisecond) + }) + + // The anchor never changed, so no re-anchor happened and it still owns the connection. + assert.Equal(t, anchorSDKKey, env.GetAcceptedKeys().Anchor) + h.assertSDKEndpointsAvailability(true, anchorSDKKey, anchorMobileKey, multiKeyEnvID) + h.assertSDKEndpointsAvailability(false, extraSDKKey, extraMobileKey, "") + + mockLog.AssertMessageMatch(t, true, ldlog.Warn, + multiKeyIdentifiers.GetDisplayName()+".*rejecting credentials scoped to a view: extra-sdk, extra-mob") +}