diff --git a/gateway/gateway-controller/pkg/controlplane/client.go b/gateway/gateway-controller/pkg/controlplane/client.go index e50480a59d..686157fa0f 100644 --- a/gateway/gateway-controller/pkg/controlplane/client.go +++ b/gateway/gateway-controller/pkg/controlplane/client.go @@ -110,6 +110,7 @@ type ControlPlaneClient interface { // *secrets.SecretService satisfies this interface. type secretSyncer interface { UpsertFromPlatform(handle, displayName, plaintext string) error + Delete(handle, correlationID string) error } // WebhookSecretSnapshotRefresher is the extension point through which an @@ -158,6 +159,7 @@ type Client struct { webhookSecretSnapshotManager WebhookSecretSnapshotRefresher secretSyncer secretSyncer secretHashCache sync.Map // handle → last-known Platform API hash (string) + secretRevisionCache sync.Map // handle → last-applied event Revision (int64); never cleared on evict, so a stale event can't undo a later one eventGatewayHooks ControlPlaneEventGatewayHooks // DP->CP push retry tuning. @@ -1342,9 +1344,19 @@ func (c *Client) handleMessage(messageType int, message []byte) { return } - // Parse as generic event to extract type + // Parse as generic event to extract type. UseNumber() keeps JSON numbers as + // json.Number (exact decimal text) instead of the default float64 — float64 + // only has ~53 bits of integer precision, which silently rounds a UnixNano + // revision (~60 bits) to the nearest ~256ns at this magnitude. Two events for + // the same handle within that window would decode to the identical revision, + // letting a stale, reordered event's revision compare as "not older" than the + // newer one already applied (see isStaleSecretEvent). utils.MapToStruct's own + // marshal/unmarshal re-encodes json.Number as the original digits verbatim, so + // this one change is sufficient — no downstream struct/comparison needs to change. var event map[string]interface{} - if err := json.Unmarshal(message, &event); err != nil { + dec := json.NewDecoder(bytes.NewReader(message)) + dec.UseNumber() + if err := dec.Decode(&event); err != nil { c.logger.Error("Failed to parse WebSocket message", slog.Any("error", err), slog.String("message", string(message)), @@ -1440,6 +1452,10 @@ func (c *Client) handleMessage(messageType int, message []byte) { c.dispatchEventGatewayHook(event["type"], func(h ControlPlaneEventGatewayHooks) { h.HandleWebBrokerAPIDeleted(c, event) }) case "application.updated": c.handleApplicationUpdatedEvent(event) + case "secret.updated": + c.handleSecretUpdatedEvent(event) + case "secret.deleted": + c.handleSecretDeletedEvent(event) default: c.logger.Info("Received unknown event type (will be processed when handlers are implemented)", slog.String("type", eventType), @@ -3745,6 +3761,126 @@ func (c *Client) handleSubscriptionPlanDeletedEvent(event map[string]interface{} } } +// handleSecretUpdatedEvent processes secret.updated events, pushed when a secret is +// rotated. It re-fetches the plaintext over the authenticated internal secret-value +// endpoint (the event payload never carries it) and upserts it into local storage, so +// {{ secret "handle" }} placeholders resolve to the new value immediately instead of +// waiting for the next reconnect's incremental sync. +func (c *Client) handleSecretUpdatedEvent(event map[string]interface{}) { + baseLogger := c.logger + if c.apiUtilsService == nil || c.secretSyncer == nil { + baseLogger.Debug("Skipping secret.updated event: secret sync not configured") + return + } + + var updated SecretUpdatedEvent + if err := utils.MapToStruct(event, &updated); err != nil { + baseLogger.Error("Failed to parse secret.updated event", slog.Any("error", err)) + return + } + payload := updated.Payload + if payload.Handle == "" { + baseLogger.Error("secret.updated event missing handle") + return + } + logger := baseLogger.With( + slog.String("correlation_id", updated.CorrelationID), + slog.String("secret_handle", payload.Handle), + ) + + c.applySecretUpdatedPayload(payload, logger, c.apiUtilsService.FetchPlatformSecretValue) +} + +// applySecretUpdatedPayload is the testable core of handleSecretUpdatedEvent: it +// fetches the rotated plaintext via fetchValue and upserts it into local storage. +// Extracted so unit tests can stub fetchValue instead of the concrete +// *utils.APIUtilsService (mirroring syncSecretsIncrementalFromMetas in +// sync_secrets_test.go, which works around the same constraint). +func (c *Client) applySecretUpdatedPayload(payload SecretUpdatedEventPayload, logger *slog.Logger, fetchValue func(handle string) (string, error)) { + if c.isStaleSecretEvent(payload.Handle, payload.Revision, logger) { + return + } + + plaintext, err := fetchValue(payload.Handle) + if err != nil { + logger.Error("Failed to fetch rotated secret value", slog.Any("error", err)) + return + } + + if err := c.secretSyncer.UpsertFromPlatform(payload.Handle, payload.DisplayName, plaintext); err != nil { + logger.Error("Failed to upsert rotated secret", slog.Any("error", err)) + return + } + + c.secretHashCache.Store(payload.Handle, payload.Hash) + c.secretRevisionCache.Store(payload.Handle, payload.Revision) + logger.Info("Applied secret rotation from secret.updated event") +} + +// isStaleSecretEvent reports whether revision is older than the last revision this +// gateway already applied for handle. The comparison is strict-less-than so a +// redelivery of the same event (revision equal to the cached one — e.g. at-least-once +// retry from the EventHub) still applies normally, preserving existing idempotency. +// The cache entry is never cleared on eviction (see handleSecretDeletedEvent), so a +// deletion followed by the same handle being reused by a later create, followed by a +// stale, redelivered copy of the original deletion, cannot evict the newly created +// secret: the stale deletion's revision is lower than the new secret's. +func (c *Client) isStaleSecretEvent(handle string, revision int64, logger *slog.Logger) bool { + if cached, ok := c.secretRevisionCache.Load(handle); ok { + if lastApplied, ok := cached.(int64); ok && revision < lastApplied { + logger.Warn("Ignoring stale secret event", + slog.Int64("event_revision", revision), + slog.Int64("last_applied_revision", lastApplied), + ) + return true + } + } + return false +} + +// handleSecretDeletedEvent processes secret.deleted events, pushed when a secret is +// permanently deleted. Deletion only succeeds once no artifact — current config or +// any deployed snapshot, on any gateway — still references the handle, so evicting +// the local copy here is always safe. +func (c *Client) handleSecretDeletedEvent(event map[string]interface{}) { + baseLogger := c.logger + if c.secretSyncer == nil { + baseLogger.Debug("Skipping secret.deleted event: secret sync not configured") + return + } + + var deleted SecretDeletedEvent + if err := utils.MapToStruct(event, &deleted); err != nil { + baseLogger.Error("Failed to parse secret.deleted event", slog.Any("error", err)) + return + } + payload := deleted.Payload + if payload.Handle == "" { + baseLogger.Error("secret.deleted event missing handle") + return + } + logger := baseLogger.With( + slog.String("correlation_id", deleted.CorrelationID), + slog.String("secret_handle", payload.Handle), + ) + + if c.isStaleSecretEvent(payload.Handle, payload.Revision, logger) { + return + } + + if err := c.secretSyncer.Delete(payload.Handle, deleted.CorrelationID); err != nil { + logger.Warn("Failed to evict deleted secret from local store", slog.Any("error", err)) + return + } + c.secretHashCache.Delete(payload.Handle) + // Deliberately Store, not Delete: a later secret.updated for the same handle (the + // handle reused by a newly created secret) must still be able to detect a + // subsequently-redelivered copy of *this* deletion event as stale. Clearing the + // cache entry here would let that stale redelivery through and evict the new secret. + c.secretRevisionCache.Store(payload.Handle, payload.Revision) + logger.Info("Evicted deleted secret from local store") +} + // setState updates the connection state func (c *Client) setState(newState State) { c.state.mu.Lock() diff --git a/gateway/gateway-controller/pkg/controlplane/events.go b/gateway/gateway-controller/pkg/controlplane/events.go index ecde9ae796..23726eeba8 100644 --- a/gateway/gateway-controller/pkg/controlplane/events.go +++ b/gateway/gateway-controller/pkg/controlplane/events.go @@ -399,6 +399,46 @@ type SubscriptionPlanDeletedEvent struct { CorrelationID string `json:"correlationId"` } +// SecretUpdatedEventPayload represents the payload of a secret.updated event, fired +// when a secret is rotated. It never carries the plaintext value — Hash is the +// HMAC-SHA256 change-detection digest, safe to transmit since it cannot be reversed +// into the plaintext. The receiving handler fetches the fresh plaintext separately +// over the authenticated internal secret-value endpoint. +type SecretUpdatedEventPayload struct { + Handle string `json:"handle"` + DisplayName string `json:"name"` + Hash string `json:"hash"` + // Revision orders events for the same handle so a redelivered or reordered + // event cannot undo a change already applied locally. See Client.secretRevisionCache. + Revision int64 `json:"revision"` +} + +// SecretUpdatedEvent represents the complete secret.updated event. +type SecretUpdatedEvent struct { + Type string `json:"type"` + Payload SecretUpdatedEventPayload `json:"payload"` + Timestamp string `json:"timestamp"` + CorrelationID string `json:"correlationId"` +} + +// SecretDeletedEventPayload represents the payload of a secret.deleted event, +// fired when a secret is permanently deleted. +type SecretDeletedEventPayload struct { + Handle string `json:"handle"` + // Revision — see SecretUpdatedEventPayload.Revision. Compared against the same + // cache so a late deletion cannot evict a secret that was recreated under the + // same handle after it. + Revision int64 `json:"revision"` +} + +// SecretDeletedEvent represents the complete secret.deleted event. +type SecretDeletedEvent struct { + Type string `json:"type"` + Payload SecretDeletedEventPayload `json:"payload"` + Timestamp string `json:"timestamp"` + CorrelationID string `json:"correlationId"` +} + // ApplicationKeyMappingPayload represents a single application to API key mapping entry. type ApplicationKeyMappingPayload struct { ApiKeyUuid string `json:"apiKeyUuid"` diff --git a/gateway/gateway-controller/pkg/controlplane/sync_secrets.go b/gateway/gateway-controller/pkg/controlplane/sync_secrets.go index 285f08fc2c..43976309d5 100644 --- a/gateway/gateway-controller/pkg/controlplane/sync_secrets.go +++ b/gateway/gateway-controller/pkg/controlplane/sync_secrets.go @@ -20,8 +20,10 @@ package controlplane import ( "log/slog" + "time" "github.com/wso2/api-platform/gateway/gateway-controller/pkg/constants" + "github.com/wso2/api-platform/gateway/gateway-controller/pkg/utils" ) // syncSecrets pulls secrets from the Platform API and upserts them into local @@ -73,12 +75,14 @@ func (c *Client) syncSecretsBulk() { } synced, skipped, failed := 0, 0, 0 + activeHandles := make(map[string]struct{}, len(metas)) for _, meta := range metas { if meta.Status != "ACTIVE" { skipped++ continue } + activeHandles[meta.Handle] = struct{}{} if meta.Value == nil { c.logger.Warn("Bulk fetch returned no value for secret — skipping", @@ -101,10 +105,13 @@ func (c *Client) syncSecretsBulk() { synced++ } + evicted := c.evictSecretsNotIn(activeHandles) + c.logger.Info("Bulk Platform API secret sync complete", slog.Int("synced", synced), slog.Int("skipped", skipped), slog.Int("failed", failed), + slog.Int("evicted", evicted), ) } @@ -120,12 +127,14 @@ func (c *Client) syncSecretsIncremental() { } synced, skipped, failed := 0, 0, 0 + activeHandles := make(map[string]struct{}, len(metas)) for _, meta := range metas { if meta.Status != "ACTIVE" { skipped++ continue } + activeHandles[meta.Handle] = struct{}{} // Skip if hash unchanged since last sync. if cached, ok := c.secretHashCache.Load(meta.Handle); ok && cached.(string) == meta.Hash { @@ -156,13 +165,60 @@ func (c *Client) syncSecretsIncremental() { synced++ } + evicted := c.evictSecretsNotIn(activeHandles) + c.logger.Info("Incremental Platform API secret sync complete", slog.Int("synced", synced), slog.Int("skipped", skipped), slog.Int("failed", failed), + slog.Int("evicted", evicted), ) } +// evictSecretsNotIn is the poll-based recovery path for the same eviction that +// handleSecretDeletedEvent applies live: a gateway that is disconnected at the +// moment a secret is deleted only receives that secret.deleted WebSocket event if +// it's connected when the event is broadcast. This diffs the +// latest Platform API response against secretHashCache so that, on the next +// reconnect/poll, any cached handle that is no longer ACTIVE (permanently deleted, +// or flipped to a non-ACTIVE status) gets evicted from local storage even though +// the live event was missed. +// +// activeHandles is the set of handles the just-completed poll returned with +// status ACTIVE; every other handle currently in secretHashCache is stale. +func (c *Client) evictSecretsNotIn(activeHandles map[string]struct{}) int { + var stale []string + c.secretHashCache.Range(func(key, _ any) bool { + handle, ok := key.(string) + if !ok { + return true + } + if _, ok := activeHandles[handle]; !ok { + stale = append(stale, handle) + } + return true + }) + + for _, handle := range stale { + correlationID := utils.GenerateDeterministicUUIDv7(handle, time.Now()) + if err := c.secretSyncer.Delete(handle, correlationID); err != nil { + c.logger.Warn("Failed to evict stale secret from local store", + slog.String("secret_handle", handle), + slog.String("correlation_id", correlationID), + slog.Any("error", err), + ) + continue + } + c.secretHashCache.Delete(handle) + c.logger.Info("Evicted stale secret from local store during poll sync", + slog.String("secret_handle", handle), + slog.String("correlation_id", correlationID), + ) + } + + return len(stale) +} + // syncSecretRefsFromYAML extracts {{ secret "handle" }} placeholders from the // supplied YAML, then fetches and upserts any handle that is not already in the // local hash cache. This is called from deployment event handlers so that secrets diff --git a/gateway/gateway-controller/pkg/controlplane/sync_secrets_test.go b/gateway/gateway-controller/pkg/controlplane/sync_secrets_test.go index 2e787bc2e5..db57bcfc42 100644 --- a/gateway/gateway-controller/pkg/controlplane/sync_secrets_test.go +++ b/gateway/gateway-controller/pkg/controlplane/sync_secrets_test.go @@ -19,12 +19,19 @@ package controlplane import ( + "encoding/json" "errors" + "fmt" "log/slog" + "net/http" + "net/http/httptest" + "strings" "testing" "time" + "github.com/gorilla/websocket" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/wso2/api-platform/gateway/gateway-controller/pkg/utils" ) @@ -39,8 +46,10 @@ import ( // wrapper on the Client for testing. type mockSecretSyncer struct { - upserted map[string]string // handle → plaintext - err error // if non-nil, UpsertFromPlatform returns this + upserted map[string]string // handle → plaintext + deleted []string // handles passed to Delete, in call order + err error // if non-nil, UpsertFromPlatform returns this + deleteErr error // if non-nil, Delete returns this } func newMockSecretSyncer() *mockSecretSyncer { @@ -55,6 +64,15 @@ func (m *mockSecretSyncer) UpsertFromPlatform(handle, _, plaintext string) error return nil } +func (m *mockSecretSyncer) Delete(handle, _ string) error { + if m.deleteErr != nil { + return m.deleteErr + } + m.deleted = append(m.deleted, handle) + delete(m.upserted, handle) + return nil +} + // stubClient builds the minimal Client needed for syncSecrets* methods. // It does NOT call NewClient (which dials a real control-plane), so it is // purely in-memory. @@ -503,3 +521,596 @@ func TestSecretHashCache_IsolatedPerHandle(t *testing.T) { // Stub to make compilation succeed — the real time.Time argument is used by // syncSecretsIncremental but not needed by our extracted helpers. var _ = time.Now + +// --------------------------------------------------------------------------- +// secret.updated / secret.deleted push-event handlers +// --------------------------------------------------------------------------- + +func TestApplySecretUpdatedPayload_FetchesAndUpserts_CachesHash(t *testing.T) { + syncer := newMockSecretSyncer() + c := stubClient(syncer) + + payload := SecretUpdatedEventPayload{Handle: "openai-key", DisplayName: "OpenAI Key", Hash: "hmac-sha256:new"} + fetchValue := func(handle string) (string, error) { + assert.Equal(t, "openai-key", handle) + return "sk-rotated", nil + } + + c.applySecretUpdatedPayload(payload, slog.Default(), fetchValue) + + assert.Equal(t, "sk-rotated", syncer.upserted["openai-key"]) + cached, ok := c.secretHashCache.Load("openai-key") + assert.True(t, ok) + assert.Equal(t, "hmac-sha256:new", cached) +} + +func TestApplySecretUpdatedPayload_FetchError_DoesNotUpsertOrCacheHash(t *testing.T) { + syncer := newMockSecretSyncer() + c := stubClient(syncer) + + payload := SecretUpdatedEventPayload{Handle: "openai-key", Hash: "hmac-sha256:new"} + fetchValue := func(handle string) (string, error) { return "", errors.New("upstream unreachable") } + + c.applySecretUpdatedPayload(payload, slog.Default(), fetchValue) + + assert.Empty(t, syncer.upserted, "must not upsert when the value fetch fails") + _, ok := c.secretHashCache.Load("openai-key") + assert.False(t, ok, "must not cache the new hash when the value fetch fails") +} + +func TestApplySecretUpdatedPayload_UpsertError_DoesNotCacheHash(t *testing.T) { + syncer := newMockSecretSyncer() + syncer.err = errors.New("storage full") + c := stubClient(syncer) + + payload := SecretUpdatedEventPayload{Handle: "openai-key", Hash: "hmac-sha256:new"} + fetchValue := func(handle string) (string, error) { return "sk-rotated", nil } + + c.applySecretUpdatedPayload(payload, slog.Default(), fetchValue) + + _, ok := c.secretHashCache.Load("openai-key") + assert.False(t, ok, "must not cache the new hash when the local upsert fails") +} + +func TestHandleSecretUpdatedEvent_MissingHandle_NoFetchAttempted(t *testing.T) { + syncer := newMockSecretSyncer() + c := stubClient(syncer) + c.apiUtilsService = &utils.APIUtilsService{} // non-nil so the guard under test is the handle check, not this one + + event := map[string]interface{}{ + "type": "secret.updated", + "correlationId": "corr-1", + "payload": map[string]interface{}{"handle": "", "hash": "hmac-sha256:x"}, + } + + assert.NotPanics(t, func() { c.handleSecretUpdatedEvent(event) }) + assert.Empty(t, syncer.upserted) +} + +func TestHandleSecretUpdatedEvent_NilDependencies_NoPanic(t *testing.T) { + c := stubClient(newMockSecretSyncer()) + // apiUtilsService left nil (zero value of *utils.APIUtilsService) + + event := map[string]interface{}{ + "type": "secret.updated", + "correlationId": "corr-1", + "payload": map[string]interface{}{"handle": "openai-key", "hash": "hmac-sha256:x"}, + } + + assert.NotPanics(t, func() { c.handleSecretUpdatedEvent(event) }) +} + +func TestHandleSecretDeletedEvent_EvictsFromLocalStoreAndHashCache(t *testing.T) { + syncer := newMockSecretSyncer() + syncer.upserted["old-key"] = "sk-stale" + c := stubClient(syncer) + populateCache(c, map[string]string{"old-key": "hmac-sha256:stale"}) + + event := map[string]interface{}{ + "type": "secret.deleted", + "correlationId": "corr-2", + "payload": map[string]interface{}{"handle": "old-key"}, + } + + c.handleSecretDeletedEvent(event) + + assert.Equal(t, []string{"old-key"}, syncer.deleted) + assert.NotContains(t, syncer.upserted, "old-key", "local copy must be evicted") + _, ok := c.secretHashCache.Load("old-key") + assert.False(t, ok, "hash cache entry must be cleared on eviction") +} + +func TestHandleSecretDeletedEvent_MissingHandle_NoEviction(t *testing.T) { + syncer := newMockSecretSyncer() + c := stubClient(syncer) + + event := map[string]interface{}{ + "type": "secret.deleted", + "correlationId": "corr-2", + "payload": map[string]interface{}{"handle": ""}, + } + + assert.NotPanics(t, func() { c.handleSecretDeletedEvent(event) }) + assert.Empty(t, syncer.deleted) +} + +func TestHandleSecretDeletedEvent_NilSyncer_NoPanic(t *testing.T) { + c := &Client{logger: slog.Default()} // secretSyncer left nil + + event := map[string]interface{}{ + "type": "secret.deleted", + "correlationId": "corr-2", + "payload": map[string]interface{}{"handle": "old-key"}, + } + + assert.NotPanics(t, func() { c.handleSecretDeletedEvent(event) }) +} + +func TestHandleSecretDeletedEvent_DeleteError_HashCacheNotCleared(t *testing.T) { + syncer := newMockSecretSyncer() + syncer.deleteErr = errors.New("storage locked") + c := stubClient(syncer) + populateCache(c, map[string]string{"old-key": "hmac-sha256:stale"}) + + event := map[string]interface{}{ + "type": "secret.deleted", + "correlationId": "corr-2", + "payload": map[string]interface{}{"handle": "old-key"}, + } + + c.handleSecretDeletedEvent(event) + + _, ok := c.secretHashCache.Load("old-key") + assert.True(t, ok, "hash cache must be left intact when eviction fails, so a later retry doesn't skip it") +} + +// --------------------------------------------------------------------------- +// Revision-based staleness rejection (out-of-order / redelivered events) +// --------------------------------------------------------------------------- + +func TestApplySecretUpdatedPayload_StaleRevision_Ignored(t *testing.T) { + syncer := newMockSecretSyncer() + c := stubClient(syncer) + c.secretRevisionCache.Store("openai-key", int64(10)) + + payload := SecretUpdatedEventPayload{Handle: "openai-key", Hash: "hmac-sha256:new", Revision: 5} + fetchValue := func(handle string) (string, error) { + t.Fatal("fetchValue must not be called for a stale event") + return "", nil + } + + c.applySecretUpdatedPayload(payload, slog.Default(), fetchValue) + + assert.Empty(t, syncer.upserted, "a stale update must not be applied") + cached, _ := c.secretRevisionCache.Load("openai-key") + assert.Equal(t, int64(10), cached, "the cached revision must not regress") +} + +func TestApplySecretUpdatedPayload_NewerRevision_AppliedAndRevisionAdvanced(t *testing.T) { + syncer := newMockSecretSyncer() + c := stubClient(syncer) + c.secretRevisionCache.Store("openai-key", int64(5)) + + payload := SecretUpdatedEventPayload{Handle: "openai-key", Hash: "hmac-sha256:new", Revision: 10} + fetchValue := func(handle string) (string, error) { return "sk-rotated", nil } + + c.applySecretUpdatedPayload(payload, slog.Default(), fetchValue) + + assert.Equal(t, "sk-rotated", syncer.upserted["openai-key"]) + cached, _ := c.secretRevisionCache.Load("openai-key") + assert.Equal(t, int64(10), cached) +} + +func TestApplySecretUpdatedPayload_EqualRevision_StillApplied(t *testing.T) { + // A redelivery of the exact same event (e.g. an at-least-once retry from the + // EventHub) must still be applied — only strictly older revisions are stale. + syncer := newMockSecretSyncer() + c := stubClient(syncer) + c.secretRevisionCache.Store("openai-key", int64(10)) + + payload := SecretUpdatedEventPayload{Handle: "openai-key", Hash: "hmac-sha256:new", Revision: 10} + fetchValue := func(handle string) (string, error) { return "sk-rotated", nil } + + c.applySecretUpdatedPayload(payload, slog.Default(), fetchValue) + + assert.Equal(t, "sk-rotated", syncer.upserted["openai-key"], "redelivery of the same revision must still apply") +} + +func TestHandleSecretDeletedEvent_StaleRevision_NotEvicted(t *testing.T) { + syncer := newMockSecretSyncer() + syncer.upserted["old-key"] = "sk-active" + c := stubClient(syncer) + populateCache(c, map[string]string{"old-key": "hmac-sha256:active"}) + c.secretRevisionCache.Store("old-key", int64(10)) + + event := map[string]interface{}{ + "type": "secret.deleted", + "correlationId": "corr-stale", + "payload": map[string]interface{}{"handle": "old-key", "revision": float64(5)}, + } + + c.handleSecretDeletedEvent(event) + + assert.Empty(t, syncer.deleted, "a stale deletion must not evict") + assert.Contains(t, syncer.upserted, "old-key") + _, ok := c.secretHashCache.Load("old-key") + assert.True(t, ok, "hash cache must be left intact for a stale deletion") +} + +// TestSecretLifecycle_DeleteThenRecreate_StaleDeleteRedelivery_DoesNotEvict covers +// the exact sequence a reviewer flagged as unprotected: update (rev 1) establishes the +// secret, delete (rev 2) evicts it, the same handle is reused by a freshly created +// secret (rev 3), and then the rev-2 deletion is redelivered late (a realistic outcome +// of common/eventhub's poll-based, at-least-once delivery with no cross-replica +// ordering guarantee). The redelivered deletion must be rejected as stale rather than +// evicting the new secret. +func TestSecretLifecycle_DeleteThenRecreate_StaleDeleteRedelivery_DoesNotEvict(t *testing.T) { + syncer := newMockSecretSyncer() + c := stubClient(syncer) + + // rev 1: initial update establishes the secret. + updatePayload := func(revision int64, value string) SecretUpdatedEventPayload { + return SecretUpdatedEventPayload{Handle: "openai-key", DisplayName: "OpenAI Key", Hash: "hmac-sha256:" + value, Revision: revision} + } + fetchValue := func(value string) func(string) (string, error) { + return func(string) (string, error) { return value, nil } + } + + c.applySecretUpdatedPayload(updatePayload(1, "v1"), slog.Default(), fetchValue("sk-v1")) + assert.Equal(t, "sk-v1", syncer.upserted["openai-key"]) + + // rev 2: deletion evicts the secret. + deleteEvent := func(revision int) map[string]interface{} { + return map[string]interface{}{ + "type": "secret.deleted", + "correlationId": "corr-delete", + "payload": map[string]interface{}{"handle": "openai-key", "revision": float64(revision)}, + } + } + c.handleSecretDeletedEvent(deleteEvent(2)) + assert.NotContains(t, syncer.upserted, "openai-key", "secret must be evicted after deletion") + + // rev 3: the handle is reused by a freshly created secret. + c.applySecretUpdatedPayload(updatePayload(3, "v3"), slog.Default(), fetchValue("sk-v3")) + assert.Equal(t, "sk-v3", syncer.upserted["openai-key"], "the new secret under the reused handle must be applied") + + // The rev-2 deletion is redelivered (late retry / at-least-once redelivery). It + // must be recognized as stale relative to rev 3 and must NOT re-evict. + c.handleSecretDeletedEvent(deleteEvent(2)) + assert.Equal(t, "sk-v3", syncer.upserted["openai-key"], "a stale, redelivered deletion must not evict the newly created secret") + + cached, ok := c.secretRevisionCache.Load("openai-key") + assert.True(t, ok) + assert.Equal(t, int64(3), cached, "cached revision must remain at the new secret's revision") +} + +// --------------------------------------------------------------------------- +// Revision precision through the real WebSocket JSON decode (handleMessage) +// +// These go through client.handleMessage with raw JSON bytes rather than a +// hand-built map[string]interface{}, because the precision bug they guard +// against lives specifically in that decode step: handleMessage used to parse +// the message with plain json.Unmarshal into map[string]interface{}, which +// decodes JSON numbers as float64. float64 has ~53 bits of integer precision, +// but a UnixNano revision is ~60 bits, so two revisions within ~256ns of each +// other at that magnitude decoded to the identical value — verified empirically +// before the fix. A test built from Go int64 literals instead of raw JSON text +// would never exercise that decode path and would pass whether or not the bug +// was present. +// --------------------------------------------------------------------------- + +func deletedMessage(handle string, revision int64) []byte { + return []byte(fmt.Sprintf( + `{"type":"secret.deleted","correlationId":"corr-precision","payload":{"handle":%q,"revision":%d}}`, + handle, revision, + )) +} + +func TestHandleMessage_SecretDeleted_AdjacentRevisions_ReorderedStaleRejected(t *testing.T) { + syncer := newMockSecretSyncer() + syncer.upserted["openai-key"] = "sk-active" + c := stubClient(syncer) + populateCache(c, map[string]string{"openai-key": "hmac-sha256:active"}) + + // Realistic UnixNano magnitude (~10^18). newRevision and staleRevision differ + // by 100ns — inside the ~256ns window that collided under the float64 bug. + const newRevision int64 = 1735900000123456700 + const staleRevision int64 = newRevision - 100 + + c.handleMessage(websocket.TextMessage, deletedMessage("openai-key", newRevision)) + assert.Equal(t, []string{"openai-key"}, syncer.deleted, "the newer deletion must evict") + + // The older, adjacent revision arrives late (reordered/redelivered). It must + // be rejected as stale, not re-applied. + c.handleMessage(websocket.TextMessage, deletedMessage("openai-key", staleRevision)) + assert.Equal(t, []string{"openai-key"}, syncer.deleted, + "a stale reordered deletion adjacent in time to the last-applied one must not evict again") + + cached, ok := c.secretRevisionCache.Load("openai-key") + assert.True(t, ok) + assert.Equal(t, newRevision, cached, "cached revision must remain exactly the newer value, not a float64-rounded approximation") +} + +func TestHandleMessage_SecretDeleted_EqualRevisionRedelivery_StillApplied(t *testing.T) { + syncer := newMockSecretSyncer() + syncer.upserted["openai-key"] = "sk-active" + c := stubClient(syncer) + populateCache(c, map[string]string{"openai-key": "hmac-sha256:active"}) + + const revision int64 = 1735900000123456700 + + c.handleMessage(websocket.TextMessage, deletedMessage("openai-key", revision)) + c.handleMessage(websocket.TextMessage, deletedMessage("openai-key", revision)) + + assert.Equal(t, []string{"openai-key", "openai-key"}, syncer.deleted, + "an exact redelivery of the same revision must still be applied (idempotent), not rejected as stale") +} + +// --------------------------------------------------------------------------- +// Poll-based eviction (evictSecretsNotIn, and its wiring into +// syncSecretsBulk / syncSecretsIncremental) +// +// A gateway that is disconnected at the moment a secret is deleted only learns +// about it via the live secret.deleted event if it's connected when the event is +// broadcast. evictSecretsNotIn is the poll-based recovery path: on the next +// reconnect/poll it diffs the Platform API response against secretHashCache and +// evicts anything no longer ACTIVE, using the same secretSyncer.Delete + +// secretHashCache.Delete calls handleSecretDeletedEvent uses for the live path. +// --------------------------------------------------------------------------- + +func TestEvictSecretsNotIn_HandleMissingFromActiveSet_Evicted(t *testing.T) { + syncer := newMockSecretSyncer() + c := stubClient(syncer) + populateCache(c, map[string]string{"gone-handle": "hmac-sha256:old"}) + + evicted := c.evictSecretsNotIn(map[string]struct{}{}) + + assert.Equal(t, 1, evicted) + assert.Equal(t, []string{"gone-handle"}, syncer.deleted) + _, ok := c.secretHashCache.Load("gone-handle") + assert.False(t, ok, "evicted handle must be removed from secretHashCache") +} + +func TestEvictSecretsNotIn_HandleFlippedNonActive_Evicted(t *testing.T) { + // A handle still present in the Platform API response but no longer ACTIVE + // (e.g. DEPRECATED) is represented the same way as a missing handle: it's + // simply absent from activeHandles, since the caller only adds ACTIVE handles. + syncer := newMockSecretSyncer() + c := stubClient(syncer) + populateCache(c, map[string]string{"deprecated-handle": "hmac-sha256:x"}) + + evicted := c.evictSecretsNotIn(map[string]struct{}{}) + + assert.Equal(t, 1, evicted) + assert.True(t, syncer.wasDeleted("deprecated-handle")) + _, ok := c.secretHashCache.Load("deprecated-handle") + assert.False(t, ok) +} + +func TestEvictSecretsNotIn_UnrelatedActiveHandle_LeftAlone(t *testing.T) { + syncer := newMockSecretSyncer() + c := stubClient(syncer) + populateCache(c, map[string]string{"stable-handle": "hmac-sha256:stable"}) + + evicted := c.evictSecretsNotIn(map[string]struct{}{"stable-handle": {}}) + + assert.Equal(t, 0, evicted) + assert.Empty(t, syncer.deleted) + cached, ok := c.secretHashCache.Load("stable-handle") + require.True(t, ok, "unrelated ACTIVE, unchanged handle must remain cached") + assert.Equal(t, "hmac-sha256:stable", cached) +} + +func TestEvictSecretsNotIn_MixedSet(t *testing.T) { + syncer := newMockSecretSyncer() + c := stubClient(syncer) + populateCache(c, map[string]string{ + "gone-handle": "hmac-sha256:old", + "deprecated-handle": "hmac-sha256:x", + "stable-handle": "hmac-sha256:stable", + }) + + evicted := c.evictSecretsNotIn(map[string]struct{}{"stable-handle": {}}) + + assert.Equal(t, 2, evicted) + assert.True(t, syncer.wasDeleted("gone-handle")) + assert.True(t, syncer.wasDeleted("deprecated-handle")) + assert.False(t, syncer.wasDeleted("stable-handle")) + + _, stableOk := c.secretHashCache.Load("stable-handle") + assert.True(t, stableOk) +} + +func TestEvictSecretsNotIn_DeleteError_HashCacheNotCleared(t *testing.T) { + syncer := newMockSecretSyncer() + syncer.deleteErr = errors.New("storage locked") + c := stubClient(syncer) + populateCache(c, map[string]string{"gone-handle": "hmac-sha256:old"}) + + evicted := c.evictSecretsNotIn(map[string]struct{}{}) + + assert.Equal(t, 1, evicted, "the handle is still counted as stale even though eviction failed") + _, ok := c.secretHashCache.Load("gone-handle") + assert.True(t, ok, "hash cache must be left intact so a later poll retries the eviction") +} + +// wasDeleted reports whether Delete was called for handle, for readability in +// the mixed-set assertions above. +func (m *mockSecretSyncer) wasDeleted(handle string) bool { + for _, h := range m.deleted { + if h == handle { + return true + } + } + return false +} + +// --- End-to-end coverage through the real HTTP-backed sync methods --- + +// platformSecretJSON mirrors PlatformSecretMeta's wire shape for building test +// server responses. +type platformSecretJSON struct { + ID string `json:"uuid"` + Handle string `json:"handle"` + DisplayName string `json:"name"` + Hash string `json:"hash"` + Status string `json:"status"` + Value *string `json:"value,omitempty"` +} + +// newSecretSyncHTTPClient spins up an httptest TLS server serving +// GET /api/internal/v1/secrets (and, if valueHandler is non-nil, +// GET /api/internal/v1/secrets/{handle}/value) and returns a *Client wired to +// it via a real *utils.APIUtilsService, with a mockSecretSyncer installed. +func newSecretSyncHTTPClient(t *testing.T, listHandler http.HandlerFunc, valueHandler http.HandlerFunc) (*Client, *mockSecretSyncer) { + t.Helper() + + mux := http.NewServeMux() + mux.HandleFunc("/api/internal/v1/secrets", listHandler) + if valueHandler != nil { + mux.HandleFunc("/api/internal/v1/secrets/", func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/value") { + valueHandler(w, r) + return + } + http.NotFound(w, r) + }) + } + + server := httptest.NewTLSServer(mux) + t.Cleanup(server.Close) + + syncer := newMockSecretSyncer() + c := stubClient(syncer) + c.apiUtilsService = utils.NewAPIUtilsService(utils.PlatformAPIConfig{ + BaseURL: server.URL + "/api/internal/v1", + InsecureSkipVerify: true, + }, slog.Default()) + + return c, syncer +} + +func writeSecretsList(t *testing.T, w http.ResponseWriter, secrets []platformSecretJSON) { + t.Helper() + w.Header().Set("Content-Type", "application/json") + require.NoError(t, json.NewEncoder(w).Encode(map[string]any{"list": secrets, "count": len(secrets)})) +} + +// TestSyncSecretsIncremental_EndToEnd_EvictsHandleMissingFromResponse proves the +// real syncSecretsIncremental wiring — not just evictSecretsNotIn in isolation — +// evicts a handle that was cached ACTIVE previously but has since been +// permanently deleted (absent from the poll response entirely). +func TestSyncSecretsIncremental_EndToEnd_EvictsHandleMissingFromResponse(t *testing.T) { + c, syncer := newSecretSyncHTTPClient(t, func(w http.ResponseWriter, r *http.Request) { + writeSecretsList(t, w, []platformSecretJSON{}) + }, nil) + populateCache(c, map[string]string{"gone-handle": "hmac-sha256:old"}) + + c.syncSecretsIncremental() + + assert.True(t, syncer.wasDeleted("gone-handle")) + _, ok := c.secretHashCache.Load("gone-handle") + assert.False(t, ok) +} + +// TestSyncSecretsIncremental_EndToEnd_EvictsHandleFlippedToDeprecated proves a +// handle still present in the response but flipped to DEPRECATED is evicted. +func TestSyncSecretsIncremental_EndToEnd_EvictsHandleFlippedToDeprecated(t *testing.T) { + c, syncer := newSecretSyncHTTPClient(t, func(w http.ResponseWriter, r *http.Request) { + writeSecretsList(t, w, []platformSecretJSON{ + {ID: "uuid-1", Handle: "deprecated-handle", Hash: "hmac-sha256:x", Status: "DEPRECATED"}, + }) + }, nil) + populateCache(c, map[string]string{"deprecated-handle": "hmac-sha256:x"}) + + c.syncSecretsIncremental() + + assert.True(t, syncer.wasDeleted("deprecated-handle")) + _, ok := c.secretHashCache.Load("deprecated-handle") + assert.False(t, ok) +} + +// TestSyncSecretsIncremental_EndToEnd_UnrelatedActiveUnchangedHandleLeftAlone +// proves a cached handle that's still ACTIVE with an unchanged hash is neither +// evicted nor re-upserted. +func TestSyncSecretsIncremental_EndToEnd_UnrelatedActiveUnchangedHandleLeftAlone(t *testing.T) { + c, syncer := newSecretSyncHTTPClient(t, func(w http.ResponseWriter, r *http.Request) { + writeSecretsList(t, w, []platformSecretJSON{ + {ID: "uuid-2", Handle: "stable-handle", Hash: "hmac-sha256:stable", Status: "ACTIVE"}, + }) + }, nil) + populateCache(c, map[string]string{"stable-handle": "hmac-sha256:stable"}) + + c.syncSecretsIncremental() + + assert.False(t, syncer.wasDeleted("stable-handle")) + assert.NotContains(t, syncer.upserted, "stable-handle", "unchanged hash must be skipped, not re-upserted") + cached, ok := c.secretHashCache.Load("stable-handle") + require.True(t, ok) + assert.Equal(t, "hmac-sha256:stable", cached) +} + +// TestSyncSecretsIncremental_EndToEnd_MixedBatch exercises deletion, deprecation, +// an untouched ACTIVE handle, and a changed ACTIVE handle together in one poll. +func TestSyncSecretsIncremental_EndToEnd_MixedBatch(t *testing.T) { + c, syncer := newSecretSyncHTTPClient(t, + func(w http.ResponseWriter, r *http.Request) { + writeSecretsList(t, w, []platformSecretJSON{ + {ID: "uuid-2", Handle: "stable-handle", Hash: "hmac-sha256:stable", Status: "ACTIVE"}, + {ID: "uuid-3", Handle: "deprecated-handle", Hash: "hmac-sha256:x", Status: "DEPRECATED"}, + {ID: "uuid-4", Handle: "changed-handle", Hash: "hmac-sha256:new", Status: "ACTIVE"}, + }) + }, + func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]string{"value": "new-plaintext"}) + }, + ) + populateCache(c, map[string]string{ + "stable-handle": "hmac-sha256:stable", + "deprecated-handle": "hmac-sha256:x", + "gone-handle": "hmac-sha256:old", // absent from the response entirely + "changed-handle": "hmac-sha256:old", + }) + + c.syncSecretsIncremental() + + assert.True(t, syncer.wasDeleted("gone-handle")) + assert.True(t, syncer.wasDeleted("deprecated-handle")) + assert.False(t, syncer.wasDeleted("stable-handle")) + assert.False(t, syncer.wasDeleted("changed-handle")) + + _, goneOk := c.secretHashCache.Load("gone-handle") + assert.False(t, goneOk) + _, depOk := c.secretHashCache.Load("deprecated-handle") + assert.False(t, depOk) + + changedCached, ok := c.secretHashCache.Load("changed-handle") + require.True(t, ok) + assert.Equal(t, "hmac-sha256:new", changedCached) + assert.Equal(t, "new-plaintext", syncer.upserted["changed-handle"]) +} + +// TestSyncSecretsBulk_EndToEnd_EvictsPreExistingCacheNotInResponse covers the +// (mostly defensive) eviction path in the bulk/startup sync: any handle already +// in secretHashCache before the bulk fetch runs that the Platform API no longer +// returns as ACTIVE gets evicted too, for consistency with the incremental path. +func TestSyncSecretsBulk_EndToEnd_EvictsPreExistingCacheNotInResponse(t *testing.T) { + value := "plaintext-value" + c, syncer := newSecretSyncHTTPClient(t, func(w http.ResponseWriter, r *http.Request) { + writeSecretsList(t, w, []platformSecretJSON{ + {ID: "uuid-1", Handle: "active-handle", Hash: "hmac-sha256:1", Status: "ACTIVE", Value: &value}, + }) + }, nil) + populateCache(c, map[string]string{"stale-handle": "hmac-sha256:old"}) + + c.syncSecretsBulk() + + assert.True(t, syncer.wasDeleted("stale-handle")) + _, staleOk := c.secretHashCache.Load("stale-handle") + assert.False(t, staleOk) + + activeCached, ok := c.secretHashCache.Load("active-handle") + require.True(t, ok) + assert.Equal(t, "hmac-sha256:1", activeCached) +} diff --git a/platform-api/internal/constants/constants.go b/platform-api/internal/constants/constants.go index 00330f75ae..04a7cd40d0 100644 --- a/platform-api/internal/constants/constants.go +++ b/platform-api/internal/constants/constants.go @@ -24,6 +24,23 @@ import "regexp" // ref-extraction (repository) and ref-validation (service) always match the same set. var SecretPlaceholderRe = regexp.MustCompile(`\{\{\s*secret\s+\\?"([^"\\]+)\\?"\s*\}\}`) +// SecretHandlePattern mirrors the AI Workspace UI's client-side slug pattern +// (CreateSecret.tsx HANDLE_PATTERN) — lowercase letters, digits, and single +// hyphens, no leading/trailing/doubled hyphens. Enforced server-side so a +// non-UI caller (curl, future CLI) cannot create a handle that the UI would +// never generate — in particular, one containing "/" would be permanently +// unreachable via GET/PUT/DELETE /secrets/{secretId} (the router treats "/" +// as a path-segment boundary), and one containing quotes or "{{"/"}}" could +// interfere with SecretPlaceholderRe matching. +var SecretHandlePattern = regexp.MustCompile(`^[a-z0-9]+(-[a-z0-9]+)*$`) + +// SecretHandleMaxLength matches the `handle VARCHAR(40)` column in the secrets +// table. SQLite does not enforce +// VARCHAR length limits, so this must be checked in application code — without +// it, a handle exceeding this length is silently accepted on SQLite but would +// raise a raw, unfriendly DB constraint error on PostgreSQL. +const SecretHandleMaxLength = 40 + // ValidLifecycleStates Valid lifecycle states var ValidLifecycleStates = map[string]bool{ "STAGED": true, diff --git a/platform-api/internal/dto/api.go b/platform-api/internal/dto/api.go index 77f56366d3..ccea01bf28 100644 --- a/platform-api/internal/dto/api.go +++ b/platform-api/internal/dto/api.go @@ -21,6 +21,7 @@ import ( "time" "github.com/wso2/api-platform/platform-api/api" + "github.com/wso2/api-platform/platform-api/internal/model" ) // API represents an API entity in the platform @@ -156,8 +157,9 @@ type UpstreamYAML struct { // UpstreamTarget represents a single upstream target (url or ref) type UpstreamTarget struct { - URL string `yaml:"url,omitempty"` - Ref string `yaml:"ref,omitempty"` + URL string `yaml:"url,omitempty"` + Ref string `yaml:"ref,omitempty"` + Auth *model.UpstreamAuth `yaml:"auth,omitempty"` } // APIListResponse represents a paginated list of APIs (constitution-compliant) @@ -166,4 +168,3 @@ type APIListResponse struct { List []*API `json:"list" yaml:"list"` // Array of API objects Pagination Pagination `json:"pagination" yaml:"pagination"` // Pagination metadata } - diff --git a/platform-api/internal/dto/secret.go b/platform-api/internal/dto/secret.go index 68fbd99117..01fc0b04ec 100644 --- a/platform-api/internal/dto/secret.go +++ b/platform-api/internal/dto/secret.go @@ -31,10 +31,12 @@ type CreateSecretRequest struct { // UpdateSecretRequest is the request body for PUT /api/v0.9/secrets/{secretId}. // Accepts multipart/form-data to support file-based secret values in future. +// Value is optional: an empty Value updates only DisplayName/Description without +// rotating the underlying credential or reactivating a deprecated secret. type UpdateSecretRequest struct { DisplayName string `form:"displayName"` Description string `form:"description"` - Value string `form:"value" binding:"required"` + Value string `form:"value"` } // SecretResponse is returned on POST and PUT. @@ -102,3 +104,9 @@ type SecretReferenceDTO struct { type SecretInUseDetails struct { References []SecretReferenceDTO `json:"references"` } + +// SecretUsagesResponse is returned by GET /api/v0.9/secrets/{secretId}/usages — +// the resources that currently reference the secret. +type SecretUsagesResponse struct { + References []SecretReferenceDTO `json:"references"` +} diff --git a/platform-api/internal/handler/secret.go b/platform-api/internal/handler/secret.go index 12cd9ab70a..c7624d7e42 100644 --- a/platform-api/internal/handler/secret.go +++ b/platform-api/internal/handler/secret.go @@ -46,6 +46,7 @@ func (h *SecretHandler) RegisterRoutes(mux *http.ServeMux) { mux.HandleFunc("POST "+constants.APIBasePath+"/secrets", middleware.MapErrors(h.slogger, h.CreateSecret)) mux.HandleFunc("GET "+constants.APIBasePath+"/secrets", middleware.MapErrors(h.slogger, h.ListSecrets)) mux.HandleFunc("GET "+constants.APIBasePath+"/secrets/{secretId}", middleware.MapErrors(h.slogger, h.GetSecret)) + mux.HandleFunc("GET "+constants.APIBasePath+"/secrets/{secretId}/usages", middleware.MapErrors(h.slogger, h.GetSecretUsages)) mux.HandleFunc("PUT "+constants.APIBasePath+"/secrets/{secretId}", middleware.MapErrors(h.slogger, h.UpdateSecret)) mux.HandleFunc("DELETE "+constants.APIBasePath+"/secrets/{secretId}", middleware.MapErrors(h.slogger, h.DeleteSecret)) } @@ -139,6 +140,27 @@ func (h *SecretHandler) GetSecret(w http.ResponseWriter, r *http.Request) error return nil } +func (h *SecretHandler) GetSecretUsages(w http.ResponseWriter, r *http.Request) error { + orgID, ok := middleware.GetOrganizationFromRequest(r) + if !ok { + return apperror.Unauthorized.New(). + WithLogMessage("organization claim not found in token") + } + + handle := r.PathValue("secretId") + if handle == "" { + return apperror.ValidationFailed.New("Secret name is required") + } + + refs, err := h.secretService.GetReferences(orgID, handle) + if err != nil { + return serviceError(err, "failed to get secret usages") + } + + httputil.WriteJSON(w, http.StatusOK, dto.SecretUsagesResponse{References: refs}) + return nil +} + func (h *SecretHandler) UpdateSecret(w http.ResponseWriter, r *http.Request) error { orgID, ok := middleware.GetOrganizationFromRequest(r) if !ok { @@ -164,9 +186,6 @@ func (h *SecretHandler) UpdateSecret(w http.ResponseWriter, r *http.Request) err Description: r.FormValue("description"), Value: r.FormValue("value"), } - if req.Value == "" { - return apperror.ValidationFailed.New("value is required") - } resp, err := h.secretService.Update(orgID, handle, userID, &req) if err != nil { diff --git a/platform-api/internal/handler/secret_integration_test.go b/platform-api/internal/handler/secret_integration_test.go index 8bed140fec..5ae2e9aea7 100644 --- a/platform-api/internal/handler/secret_integration_test.go +++ b/platform-api/internal/handler/secret_integration_test.go @@ -21,6 +21,7 @@ import ( "bytes" "database/sql" "encoding/json" + "errors" "fmt" "log/slog" "mime/multipart" @@ -355,7 +356,11 @@ func TestSecretHandler_Update_200(t *testing.T) { } // TC-IT-10b: PUT on a DEPRECATED secret reactivates it (status → ACTIVE) and re-encrypts the value. -func TestSecretHandler_Update_ReactivatesDeprecatedSecret(t *testing.T) { +// A deleted-and-unreferenced secret is now permanently removed (see +// TestSecretHandler_Delete_HardDeletesRow), so there is no longer a row left to +// reactivate via rotation: a PUT against a deleted handle 404s, and a fresh POST +// with the same handle is free to create a brand-new secret. +func TestSecretHandler_Update_DeletedHandleNotFound(t *testing.T) { tmpDir := t.TempDir() sqlDB, err := sql.Open("sqlite3", filepath.Join(tmpDir, "test-reactivate.db")) if err != nil { @@ -379,7 +384,7 @@ func TestSecretHandler_Update_ReactivatesDeprecatedSecret(t *testing.T) { NewSecretHandler(svc, identityService, slog.Default()).RegisterRoutes(mux) r := middleware.NewTestContextMiddleware(mux) - // Create then soft-delete (deprecate) the secret. + // Create then delete the secret. body, ct := multipartForm(map[string]string{"id": "react-key", "displayName": "React Key", "value": "old-val"}) req, _ := http.NewRequest(http.MethodPost, "/api/v0.9/secrets", body) req.Header.Set("Content-Type", ct) @@ -400,14 +405,14 @@ func TestSecretHandler_Update_ReactivatesDeprecatedSecret(t *testing.T) { t.Fatalf("delete: expected 204, got %d", wDel.Code) } - // Confirm it is DEPRECATED before rotation. - var statusBefore string - sqlDB.QueryRow(`SELECT status FROM secrets WHERE handle = 'react-key'`).Scan(&statusBefore) - if statusBefore != "DEPRECATED" { - t.Fatalf("expected DEPRECATED before rotation, got %s", statusBefore) + // Confirm the row is gone entirely. + var count int + sqlDB.QueryRow(`SELECT COUNT(*) FROM secrets WHERE handle = 'react-key'`).Scan(&count) + if count != 0 { + t.Fatalf("expected row to be gone after delete, found %d", count) } - // Rotate — PUT should reactivate. + // Rotate — PUT against the now-nonexistent handle must 404. putBody, putCT := multipartForm(map[string]string{"value": "new-val"}) putReq, _ := http.NewRequest(http.MethodPut, "/api/v0.9/secrets/react-key", putBody) putReq.Header.Set("Content-Type", putCT) @@ -415,23 +420,28 @@ func TestSecretHandler_Update_ReactivatesDeprecatedSecret(t *testing.T) { putReq.Header.Set("X-Test-User", "alice") wPut := httptest.NewRecorder() r.ServeHTTP(wPut, putReq) - if wPut.Code != http.StatusOK { - t.Fatalf("rotate: expected 200, got %d: %s", wPut.Code, wPut.Body.String()) + if wPut.Code != http.StatusNotFound { + t.Fatalf("rotate: expected 404, got %d: %s", wPut.Code, wPut.Body.String()) } - // Status must be ACTIVE and value must decrypt to the new plaintext. - var statusAfter string - sqlDB.QueryRow(`SELECT status FROM secrets WHERE handle = 'react-key'`).Scan(&statusAfter) - if statusAfter != "ACTIVE" { - t.Errorf("expected ACTIVE after rotation, got %s", statusAfter) + // A fresh create with the same handle must succeed — the handle is fully free. + recreateBody, recreateCT := multipartForm(map[string]string{"id": "react-key", "displayName": "React Key", "value": "brand-new-val"}) + recreateReq, _ := http.NewRequest(http.MethodPost, "/api/v0.9/secrets", recreateBody) + recreateReq.Header.Set("Content-Type", recreateCT) + recreateReq.Header.Set("X-Test-Org", "org-react-it") + recreateReq.Header.Set("X-Test-User", "alice") + wRecreate := httptest.NewRecorder() + r.ServeHTTP(wRecreate, recreateReq) + if wRecreate.Code != http.StatusCreated { + t.Fatalf("recreate: expected 201, got %d: %s", wRecreate.Code, wRecreate.Body.String()) } plaintext, err := svc.Decrypt("org-react-it", "react-key") if err != nil { - t.Fatalf("Decrypt after reactivation: %v", err) + t.Fatalf("Decrypt after recreate: %v", err) } - if plaintext != "new-val" { - t.Errorf("expected plaintext=new-val, got %s", plaintext) + if plaintext != "brand-new-val" { + t.Errorf("expected plaintext=brand-new-val, got %s", plaintext) } } @@ -643,11 +653,10 @@ func TestSecretHandler_Create_ValueNotInListResponse(t *testing.T) { } } -// TC-60: DELETE soft-deletes — status becomes DEPRECATED, physical row is retained. -// Verified by: (a) 204 on first delete, (b) Exists() returns false (ACTIVE filter), -// (c) the DB row still exists with status=DEPRECATED (checked via direct SQL), -// (d) Decrypt returns "secret is deprecated" — not "not found". -func TestSecretHandler_Delete_SoftDeletesRow(t *testing.T) { +// TC-60: DELETE permanently removes an unreferenced secret. Verified by: +// (a) 204 on first delete, (b) Exists() returns false, (c) the DB row is gone +// entirely (checked via direct SQL), (d) Decrypt returns "not found". +func TestSecretHandler_Delete_HardDeletesRow(t *testing.T) { // Build isolated stack so we can access the DB and service directly. tmpDir := t.TempDir() dbPath := filepath.Join(tmpDir, "test-sd.db") @@ -696,40 +705,37 @@ func TestSecretHandler_Delete_SoftDeletesRow(t *testing.T) { t.Fatalf("delete: expected 204, got %d", wDel.Code) } - // (b) Exists() returns false — secret no longer ACTIVE + // (b) Exists() returns false — secret is gone exists, err := repo.Exists("org-sd-it", "soft-del-key") if err != nil { t.Fatalf("Exists: %v", err) } if exists { - t.Error("Exists should return false after soft-delete") + t.Error("Exists should return false after delete") } - // (c) Physical row still present with status=DEPRECATED + // (c) Physical row is gone entirely var status string err = sqlDB.QueryRow( `SELECT status FROM secrets WHERE organization_uuid = ? AND handle = ?`, "org-sd-it", "soft-del-key", ).Scan(&status) - if err != nil { - t.Fatalf("DB row missing after soft-delete: %v", err) - } - if status != "DEPRECATED" { - t.Errorf("expected status DEPRECATED, got %q", status) + if !errors.Is(err, sql.ErrNoRows) { + t.Fatalf("expected row to be gone after delete, got err=%v status=%q", err, status) } - // (d) Decrypt returns "secret is deprecated", not "not found" + // (d) Decrypt returns "not found", not a value _, decryptErr := svc.Decrypt("org-sd-it", "soft-del-key") - if decryptErr == nil { - t.Fatal("expected error decrypting DEPRECATED secret") - } - if decryptErr.Error() != "secret is deprecated" { - t.Errorf("expected 'secret is deprecated', got %q", decryptErr.Error()) + if !apperror.SecretNotFound.Is(decryptErr) { + t.Errorf("expected SecretNotFound, got %v", decryptErr) } } // TC-61: DEPRECATED secret — Decrypt returns error (platform API contract the GW -// controller relies on to skip /value calls for DEPRECATED items). +// controller relies on to skip /value calls for DEPRECATED items). A DEPRECATED +// row is no longer reachable via the Delete flow (which now hard-deletes an +// unreferenced secret) — this seeds one directly via SQL to exercise Decrypt's +// defensive status check on its own. func TestSecretService_Decrypt_DeprecatedSecretReturnsError(t *testing.T) { tmpDir := t.TempDir() dbPath := filepath.Join(tmpDir, "test-dep.db") @@ -750,7 +756,6 @@ func TestSecretService_Decrypt_DeprecatedSecretReturnsError(t *testing.T) { repo := repository.NewSecretRepo(db) svc := service.NewSecretService(repo, v, service.NewIdentityService(repository.NewUserIdentityMappingRepo(db))) - // Create and then soft-delete a secret _, err = svc.Create("org-dep-it", "alice", &dto.CreateSecretRequest{ Handle: "dep-secret", Value: "plaintext", @@ -758,8 +763,8 @@ func TestSecretService_Decrypt_DeprecatedSecretReturnsError(t *testing.T) { if err != nil { t.Fatalf("create: %v", err) } - if err = svc.Delete("org-dep-it", "dep-secret", "alice"); err != nil { - t.Fatalf("delete: %v", err) + if _, err := sqlDB.Exec(`UPDATE secrets SET status = 'DEPRECATED' WHERE handle = 'dep-secret'`); err != nil { + t.Fatalf("deprecate: %v", err) } // Decrypt must return an error for DEPRECATED secret — not the plaintext diff --git a/platform-api/internal/model/secret.go b/platform-api/internal/model/secret.go index 77eb601ed6..c847fefae8 100644 --- a/platform-api/internal/model/secret.go +++ b/platform-api/internal/model/secret.go @@ -68,3 +68,38 @@ type SecretReference struct { Handle string `json:"handle"` Name string `json:"name"` } + +// SecretUpdatedEvent is broadcast to every gateway in the organization when a secret's +// value is rotated. Hash is the HMAC-SHA256 change-detection digest (see hashSecret) — +// safe to broadcast, since it never permits recovering the plaintext value. The +// plaintext itself is deliberately never part of this payload: the EventHub persists +// events to the shared DB, so gateways instead pull the fresh value over the +// authenticated internal secret-value endpoint once they receive this notification. +// +// Revision is UnixNano() of the secret's updated_at at the moment this event was +// raised (set by SecretService.Update/Delete from the same value just committed to +// the DB). It is not a true per-row incrementing counter — that would need a schema +// migration across three DB engines — but time.Now() on a single writer process is +// monotonically non-decreasing across successive calls, which is all a gateway needs +// to detect an event that arrived out of order relative to one it already applied for +// the same handle. See Client.secretRevisionCache in gateway-controller. +type SecretUpdatedEvent struct { + Handle string `json:"handle"` + DisplayName string `json:"name"` + Hash string `json:"hash"` + Revision int64 `json:"revision"` +} + +// SecretDeletedEvent is broadcast to every gateway in the organization when a +// secret is permanently deleted. Deletion only succeeds once no artifact — current +// config or any deployed snapshot, on any gateway — references the handle, so every +// gateway can safely evict its local copy on receipt. +// +// Revision — see SecretUpdatedEvent.Revision. A gateway must keep comparing against +// it even after evicting the secret locally, so a deletion event that arrives late +// (after the same handle has already been reused by a subsequent create) is not +// applied and does not evict the newly created secret. +type SecretDeletedEvent struct { + Handle string `json:"handle"` + Revision int64 `json:"revision"` +} diff --git a/platform-api/internal/repository/api.go b/platform-api/internal/repository/api.go index 115179d061..02f15564cb 100644 --- a/platform-api/internal/repository/api.go +++ b/platform-api/internal/repository/api.go @@ -441,18 +441,25 @@ func (r *APIRepo) UpdateAPI(api *model.API) error { return tx.Commit() } -// DeleteAPI removes an API and all its configurations -func (r *APIRepo) DeleteAPI(apiUUID, orgUUID string) error { +// DeleteAPI removes an API and all its configurations, returning the secret handles +// it referenced (current config and any deployed snapshots) so the caller can check +// each for orphan cleanup now that this artifact no longer holds a reference to them. +func (r *APIRepo) DeleteAPI(apiUUID, orgUUID string) ([]string, error) { // Start transaction for atomicity tx, err := r.db.Begin() if err != nil { - return err + return nil, err } defer tx.Rollback() + secretHandles, err := secretHandlesForArtifact(tx, r.db, apiUUID) + if err != nil { + return nil, err + } + // Delete gateway associations if _, err := tx.Exec(r.db.Rebind(`DELETE FROM artifact_gateway_mappings WHERE artifact_uuid = ? AND organization_uuid = ?`), apiUUID, orgUUID); err != nil { - return err + return nil, err } // Delete in order of dependencies (children first, parent last) @@ -468,21 +475,24 @@ func (r *APIRepo) DeleteAPI(apiUUID, orgUUID string) error { switch i { case 0: if _, err := tx.Exec(r.db.Rebind(query), apiUUID, orgUUID); err != nil { - return err + return nil, err } default: if _, err := tx.Exec(r.db.Rebind(query), apiUUID); err != nil { - return err + return nil, err } } } // Delete from artifacts table using artifactRepo if err := r.artifactRepo.Delete(tx, apiUUID); err != nil { - return err + return nil, err } - return tx.Commit() + if err := tx.Commit(); err != nil { + return nil, err + } + return secretHandles, nil } // CheckAPIExistsByHandleInOrganization checks if an API with the given handle exists within a specific organization diff --git a/platform-api/internal/repository/api_test.go b/platform-api/internal/repository/api_test.go index 2aba1fa85b..86a4188c79 100644 --- a/platform-api/internal/repository/api_test.go +++ b/platform-api/internal/repository/api_test.go @@ -86,7 +86,7 @@ func TestAPIRepo_CreateAndRead(t *testing.T) { t.Fatalf("CreateAPI failed: %v", err) } defer func() { - if err := repo.DeleteAPI(api.ID, orgUUID); err != nil { + if _, err := repo.DeleteAPI(api.ID, orgUUID); err != nil { t.Errorf("DeleteAPI cleanup failed: %v", err) } }() @@ -152,7 +152,7 @@ func TestAPIRepo_CreateAPI_SetsUpdatedBy(t *testing.T) { t.Fatalf("CreateAPI failed: %v", err) } defer func() { - if err := repo.DeleteAPI(api.ID, orgUUID); err != nil { + if _, err := repo.DeleteAPI(api.ID, orgUUID); err != nil { t.Errorf("DeleteAPI cleanup failed: %v", err) } }() @@ -282,7 +282,7 @@ func TestAPIRepo_Update(t *testing.T) { t.Fatalf("CreateAPI failed: %v", err) } defer func() { - if err := repo.DeleteAPI(api.ID, orgUUID); err != nil { + if _, err := repo.DeleteAPI(api.ID, orgUUID); err != nil { t.Errorf("DeleteAPI cleanup failed: %v", err) } }() @@ -353,7 +353,7 @@ func TestAPIRepo_Delete(t *testing.T) { t.Fatalf("CreateAPI failed: %v", err) } - if err := repo.DeleteAPI(api.ID, orgUUID); err != nil { + if _, err := repo.DeleteAPI(api.ID, orgUUID); err != nil { t.Fatalf("DeleteAPI failed: %v", err) } @@ -434,7 +434,7 @@ func TestAPIRepo_DeleteAPIRemovesCustomPolicyUsagesWithoutForeignKeys(t *testing t.Fatalf("DeleteCustomPolicyIfUnused() before API delete = nil error, want PolicyInUse") } - if err := repo.DeleteAPI(api.ID, orgUUID); err != nil { + if _, err := repo.DeleteAPI(api.ID, orgUUID); err != nil { t.Fatalf("DeleteAPI failed: %v", err) } @@ -481,7 +481,7 @@ func TestAPIRepo_CheckAPIExistsByNameAndVersionInOrganization(t *testing.T) { t.Fatalf("CreateAPI failed: %v", err) } defer func() { - if err := repo.DeleteAPI(api.ID, orgUUID); err != nil { + if _, err := repo.DeleteAPI(api.ID, orgUUID); err != nil { t.Errorf("DeleteAPI cleanup failed: %v", err) } }() @@ -541,7 +541,7 @@ func TestAPIRepo_CheckAPIExistsByHandleInOrganization(t *testing.T) { t.Fatalf("CreateAPI failed: %v", err) } defer func() { - if err := repo.DeleteAPI(api.ID, orgUUID); err != nil { + if _, err := repo.DeleteAPI(api.ID, orgUUID); err != nil { t.Errorf("DeleteAPI cleanup failed: %v", err) } }() @@ -601,7 +601,7 @@ func TestAPIRepo_CreateSetsArtifactKind(t *testing.T) { t.Fatalf("CreateAPI failed: %v", err) } t.Cleanup(func() { - if err := repo.DeleteAPI(api.ID, orgUUID); err != nil { + if _, err := repo.DeleteAPI(api.ID, orgUUID); err != nil { t.Errorf("DeleteAPI cleanup failed: %v", err) } }) @@ -697,7 +697,7 @@ func TestAPIRepo_CreateAndRead_FullConfiguration(t *testing.T) { t.Fatalf("CreateAPI failed: %v", err) } defer func() { - if err := repo.DeleteAPI(api.ID, orgUUID); err != nil { + if _, err := repo.DeleteAPI(api.ID, orgUUID); err != nil { t.Errorf("DeleteAPI cleanup failed: %v", err) } }() diff --git a/platform-api/internal/repository/artifact.go b/platform-api/internal/repository/artifact.go index f4b06e4bb3..d736026583 100644 --- a/platform-api/internal/repository/artifact.go +++ b/platform-api/internal/repository/artifact.go @@ -66,6 +66,9 @@ func (r *ArtifactRepo) Delete(tx *sql.Tx, uuid string) error { if err := deleteCustomPolicyUsagesTx(tx, r.db, uuid); err != nil { return err } + if err := deleteArtifactSecretRefs(tx, r.db, uuid); err != nil { + return err + } query := `DELETE FROM artifacts WHERE uuid = ?` result, err := tx.Exec(r.db.Rebind(query), uuid) if err != nil { diff --git a/platform-api/internal/repository/artifact_refs.go b/platform-api/internal/repository/artifact_refs.go index 536eedf20a..1a740b72e1 100644 --- a/platform-api/internal/repository/artifact_refs.go +++ b/platform-api/internal/repository/artifact_refs.go @@ -86,3 +86,36 @@ func upsertDeploymentSecretRefs(tx *sql.Tx, db *database.DB, orgID, artifactUUID } return nil } + +// secretHandlesForArtifact returns every distinct secret handle referenced by +// artifactUUID — both its current-config ref (gateway_id='') and any deployed-gateway +// refs — so a caller can check each for orphan cleanup after the artifact is deleted. +func secretHandlesForArtifact(tx *sql.Tx, db *database.DB, artifactUUID string) ([]string, error) { + rows, err := tx.Query(db.Rebind(` + SELECT DISTINCT secret_handle FROM artifact_secret_refs WHERE artifact_uuid = ? + `), artifactUUID) + if err != nil { + return nil, err + } + defer rows.Close() + + var handles []string + for rows.Next() { + var handle string + if err := rows.Scan(&handle); err != nil { + return nil, err + } + handles = append(handles, handle) + } + return handles, rows.Err() +} + +// deleteArtifactSecretRefs removes every artifact_secret_refs row for artifactUUID. +// Explicit delete, not relying on the FK cascade — SQL Server declares this table's +// artifact_uuid FK as ON DELETE NO ACTION (unlike Postgres/SQLite's ON DELETE CASCADE), +// so without this the artifact delete would fail there once any ref rows exist. Mirrors +// ArtifactRepo.Delete's existing deleteCustomPolicyUsagesTx call for the same reason. +func deleteArtifactSecretRefs(tx *sql.Tx, db *database.DB, artifactUUID string) error { + _, err := tx.Exec(db.Rebind(`DELETE FROM artifact_secret_refs WHERE artifact_uuid = ?`), artifactUUID) + return err +} diff --git a/platform-api/internal/repository/interfaces.go b/platform-api/internal/repository/interfaces.go index 13354e371d..f474722952 100644 --- a/platform-api/internal/repository/interfaces.go +++ b/platform-api/internal/repository/interfaces.go @@ -111,7 +111,10 @@ type APIRepository interface { CountAPIsByOrganizationUUID(orgUUID, projectUUID, search string) (int, error) GetAPIsByGatewayUUID(gatewayUUID, orgUUID string) ([]*model.API, error) UpdateAPI(api *model.API) error - DeleteAPI(apiUUID, orgUUID string) error + // DeleteAPI removes the API and returns the secret handles it referenced, so the + // caller can check each for orphan cleanup now that this artifact no longer holds + // a reference to them. + DeleteAPI(apiUUID, orgUUID string) ([]string, error) // API-Gateway association methods GetAPIGatewaysWithDetails(apiUUID, orgUUID string) ([]*model.APIGatewayWithDetails, error) @@ -254,7 +257,10 @@ type LLMProviderRepository interface { Count(orgUUID string) (int, error) Update(p *model.LLMProvider) error UpdateWithCustomPolicyUsages(p *model.LLMProvider, policyUUIDs []string) error - Delete(providerID, orgUUID string) error + // Delete removes the provider and returns the secret handles it referenced, so the + // caller can check each for orphan cleanup now that this artifact no longer holds + // a reference to them. + Delete(providerID, orgUUID string) ([]string, error) Exists(providerID, orgUUID string) (bool, error) // EnsureGatewayAssociation creates a gateway association for the provider if one // does not already exist and resolves the metadata to use for the deployment. @@ -287,7 +293,10 @@ type LLMProxyRepository interface { CountByProject(orgUUID, projectUUID string) (int, error) CountByProvider(orgUUID, providerID string) (int, error) Update(p *model.LLMProxy) error - Delete(proxyID, orgUUID string) error + // Delete removes the proxy and returns the secret handles it referenced, so the + // caller can check each for orphan cleanup now that this artifact no longer holds + // a reference to them. + Delete(proxyID, orgUUID string) ([]string, error) Exists(proxyID, orgUUID string) (bool, error) // EnsureGatewayAssociation creates a gateway association for the proxy if one does // not already exist and resolves the metadata to use for the deployment. @@ -304,7 +313,10 @@ type MCPProxyRepository interface { Count(orgUUID string) (int, error) CountByProject(orgUUID, projectUUID string) (int, error) Update(p *model.MCPProxy) error - Delete(handle, orgUUID string) error + // Delete removes the proxy and returns the secret handles it referenced, so the + // caller can check each for orphan cleanup now that this artifact no longer holds + // a reference to them. + Delete(handle, orgUUID string) ([]string, error) Exists(handle, orgUUID string) (bool, error) EnsureGatewayAssociation(proxyUUID, gatewayUUID, orgUUID, createdBy, deployMetadata string, metadataProvided bool) (string, error) } @@ -352,7 +364,7 @@ type SecretRepository interface { ListByHandles(orgID string, handles []string, updatedAfter *time.Time) ([]*model.Secret, error) Count(orgID string) (int, error) Update(s *model.Secret) error - FindRefsAndSoftDelete(orgID, handle, updatedBy string) ([]model.SecretReference, error) + FindRefsAndDelete(orgID, handle string) ([]model.SecretReference, error) FindRefs(orgID, handle string) ([]model.SecretReference, error) Exists(orgID, handle string) (bool, error) } diff --git a/platform-api/internal/repository/llm.go b/platform-api/internal/repository/llm.go index fb1809bbc1..0519a6d417 100644 --- a/platform-api/internal/repository/llm.go +++ b/platform-api/internal/repository/llm.go @@ -1178,10 +1178,13 @@ func (r *LLMProviderRepo) update(p *model.LLMProvider, policyUUIDs []string, rec return nil } -func (r *LLMProviderRepo) Delete(providerID, orgUUID string) error { +// Delete removes the LLM provider and returns the secret handles it referenced +// (current config and any deployed snapshots), so the caller can check each for +// orphan cleanup now that this artifact no longer holds a reference to them. +func (r *LLMProviderRepo) Delete(providerID, orgUUID string) ([]string, error) { tx, err := r.db.Begin() if err != nil { - return err + return nil, err } defer tx.Rollback() @@ -1193,25 +1196,30 @@ func (r *LLMProviderRepo) Delete(providerID, orgUUID string) error { err = tx.QueryRow(r.db.Rebind(query), providerID, orgUUID).Scan(&providerUUID) if err != nil { if errors.Is(err, sql.ErrNoRows) { - return sql.ErrNoRows + return nil, sql.ErrNoRows } - return err + return nil, err + } + + secretHandles, err := secretHandlesForArtifact(tx, r.db, providerUUID) + if err != nil { + return nil, err } // Delete from llm_providers first, then artifacts _, err = tx.Exec(r.db.Rebind(`DELETE FROM llm_providers WHERE uuid = ?`), providerUUID) if err != nil { - return err + return nil, err } if err := r.artifactRepo.Delete(tx, providerUUID); err != nil { - return err + return nil, err } if err := tx.Commit(); err != nil { - return err + return nil, err } - return nil + return secretHandles, nil } func (r *LLMProviderRepo) Exists(providerID, orgUUID string) (bool, error) { @@ -1600,10 +1608,13 @@ func (r *LLMProxyRepo) EnsureGatewayAssociation(proxyUUID, gatewayUUID, orgUUID, return ensureArtifactGatewayAssociation(r.db, proxyUUID, gatewayUUID, orgUUID, createdBy, deployMetadata, metadataProvided) } -func (r *LLMProxyRepo) Delete(proxyID, orgUUID string) error { +// Delete removes the LLM proxy and returns the secret handles it referenced +// (current config and any deployed snapshots), so the caller can check each for +// orphan cleanup now that this artifact no longer holds a reference to them. +func (r *LLMProxyRepo) Delete(proxyID, orgUUID string) ([]string, error) { tx, err := r.db.Begin() if err != nil { - return err + return nil, err } defer tx.Rollback() @@ -1615,25 +1626,30 @@ func (r *LLMProxyRepo) Delete(proxyID, orgUUID string) error { err = tx.QueryRow(r.db.Rebind(query), proxyID, orgUUID).Scan(&proxyUUID) if err != nil { if errors.Is(err, sql.ErrNoRows) { - return sql.ErrNoRows + return nil, sql.ErrNoRows } - return err + return nil, err + } + + secretHandles, err := secretHandlesForArtifact(tx, r.db, proxyUUID) + if err != nil { + return nil, err } // Delete from llm_proxies first, then artifacts using artifactRepo _, err = tx.Exec(r.db.Rebind(`DELETE FROM llm_proxies WHERE uuid = ?`), proxyUUID) if err != nil { - return err + return nil, err } if err := r.artifactRepo.Delete(tx, proxyUUID); err != nil { - return err + return nil, err } if err := tx.Commit(); err != nil { - return err + return nil, err } - return nil + return secretHandles, nil } func (r *LLMProxyRepo) Exists(proxyID, orgUUID string) (bool, error) { diff --git a/platform-api/internal/repository/llm_test.go b/platform-api/internal/repository/llm_test.go index db8e92b1c1..72e2281ef8 100644 --- a/platform-api/internal/repository/llm_test.go +++ b/platform-api/internal/repository/llm_test.go @@ -149,7 +149,7 @@ func TestLLMProviderRepoDeleteRemovesCustomPolicyUsagesWithoutForeignKeys(t *tes t.Fatalf("DeleteCustomPolicyIfUnused() before provider delete = nil error, want PolicyInUse") } - if err := providerRepo.Delete(provider.ID, orgUUID); err != nil { + if _, err := providerRepo.Delete(provider.ID, orgUUID); err != nil { t.Fatalf("delete provider: %v", err) } diff --git a/platform-api/internal/repository/mcp.go b/platform-api/internal/repository/mcp.go index 2991c9f22f..b0507f60d4 100644 --- a/platform-api/internal/repository/mcp.go +++ b/platform-api/internal/repository/mcp.go @@ -377,11 +377,14 @@ func (r *MCPProxyRepo) EnsureGatewayAssociation(proxyUUID, gatewayUUID, orgUUID, return ensureArtifactGatewayAssociation(r.db, proxyUUID, gatewayUUID, orgUUID, createdBy, deployMetadata, metadataProvided) } -// Delete deletes an MCP proxy by its handle and organization UUID -func (r *MCPProxyRepo) Delete(handle, orgUUID string) error { +// Delete deletes an MCP proxy by its handle and organization UUID, returning the +// secret handles it referenced (current config and any deployed snapshots) so the +// caller can check each for orphan cleanup now that this artifact no longer holds +// a reference to them. +func (r *MCPProxyRepo) Delete(handle, orgUUID string) ([]string, error) { tx, err := r.db.Begin() if err != nil { - return err + return nil, err } defer tx.Rollback() @@ -393,25 +396,30 @@ func (r *MCPProxyRepo) Delete(handle, orgUUID string) error { err = tx.QueryRow(r.db.Rebind(query), handle, orgUUID).Scan(&proxyUUID) if err != nil { if errors.Is(err, sql.ErrNoRows) { - return sql.ErrNoRows + return nil, sql.ErrNoRows } - return err + return nil, err + } + + secretHandles, err := secretHandlesForArtifact(tx, r.db, proxyUUID) + if err != nil { + return nil, err } // Delete from mcp_proxies first, then artifacts using artifactRepo _, err = tx.Exec(r.db.Rebind(`DELETE FROM mcp_proxies WHERE uuid = ?`), proxyUUID) if err != nil { - return err + return nil, err } if err := r.artifactRepo.Delete(tx, proxyUUID); err != nil { - return err + return nil, err } if err := tx.Commit(); err != nil { - return err + return nil, err } - return nil + return secretHandles, nil } // Exists checks if an MCP proxy exists by its handle and organization UUID diff --git a/platform-api/internal/repository/secret.go b/platform-api/internal/repository/secret.go index ba4ef8b939..0813d377be 100644 --- a/platform-api/internal/repository/secret.go +++ b/platform-api/internal/repository/secret.go @@ -230,10 +230,10 @@ func (r *SecretRepo) Update(s *model.Secret) error { return nil } -// FindRefsAndSoftDelete checks for active artifact references and deprecates the +// FindRefsAndDelete checks for active artifact references and permanently deletes the // secret in a single transaction, eliminating the TOCTOU window. -// Returns the references without deprecating if any are found. -func (r *SecretRepo) FindRefsAndSoftDelete(orgID, handle, updatedBy string) ([]model.SecretReference, error) { +// Returns the references without deleting if any are found. +func (r *SecretRepo) FindRefsAndDelete(orgID, handle string) ([]model.SecretReference, error) { tx, err := r.db.Begin() if err != nil { return nil, fmt.Errorf("failed to begin transaction: %w", err) @@ -241,9 +241,18 @@ func (r *SecretRepo) FindRefsAndSoftDelete(orgID, handle, updatedBy string) ([]m defer tx.Rollback() //nolint:errcheck var lockQuery string - if r.db.Driver() == "postgres" || r.db.Driver() == "postgresql" { + switch r.db.Driver() { + case "postgres", "postgresql": lockQuery = `SELECT uuid FROM secrets WHERE organization_uuid = $1 AND handle = $2 LIMIT 1 FOR UPDATE` - } else { + case database.DriverSQLServer: + // T-SQL has no LIMIT clause — SELECT TOP (1) is the equivalent (fixing an + // invalid-syntax error previously surfaced to callers as a generic 500). + // WITH (UPDLOCK, ROWLOCK) is T-SQL's counterpart to Postgres's FOR UPDATE: + // without it, a plain SELECT takes no lock held for the transaction's + // lifetime, so two concurrent deletes/updates on the same secret would not + // serialize against each other on SQL Server the way they do on Postgres. + lockQuery = r.db.Rebind(`SELECT TOP (1) uuid FROM secrets WITH (UPDLOCK, ROWLOCK) WHERE organization_uuid = ? AND handle = ?`) + default: lockQuery = r.db.Rebind(`SELECT uuid FROM secrets WHERE organization_uuid = ? AND handle = ? LIMIT 1`) } var lockedID string @@ -289,14 +298,12 @@ func (r *SecretRepo) FindRefsAndSoftDelete(orgID, handle, updatedBy string) ([]m return refs, nil } - deleteQuery := r.db.Rebind(` - UPDATE secrets - SET status = 'DEPRECATED', updated_at = ?, updated_by = ? - WHERE organization_uuid = ? AND handle = ? - `) - result, err := tx.Exec(deleteQuery, time.Now().UTC(), updatedBy, orgID, handle) + // secret_scopes cascades from secrets.uuid on every dialect; artifact_secret_refs + // has no rows for this handle at this point (the zero-refs check above just passed). + deleteQuery := r.db.Rebind(`DELETE FROM secrets WHERE organization_uuid = ? AND handle = ?`) + result, err := tx.Exec(deleteQuery, orgID, handle) if err != nil { - return nil, fmt.Errorf("failed to deprecate secret: %w", err) + return nil, fmt.Errorf("failed to delete secret: %w", err) } affected, err := result.RowsAffected() if err != nil { diff --git a/platform-api/internal/repository/secret_test.go b/platform-api/internal/repository/secret_test.go index f69831fb3d..a1c3e9ba64 100644 --- a/platform-api/internal/repository/secret_test.go +++ b/platform-api/internal/repository/secret_test.go @@ -184,7 +184,7 @@ func TestSecretRepo_Update(t *testing.T) { } } -func TestSecretRepo_SoftDelete(t *testing.T) { +func TestSecretRepo_HardDelete(t *testing.T) { db, cleanup := setupTestDB(t) t.Cleanup(cleanup) @@ -204,21 +204,25 @@ func TestSecretRepo_SoftDelete(t *testing.T) { t.Fatalf("Create: %v", err) } - refs, err := repo.FindRefsAndSoftDelete(orgID, "deletable", "admin") + refs, err := repo.FindRefsAndDelete(orgID, "deletable") if err != nil { - t.Fatalf("FindRefsAndSoftDelete: %v", err) + t.Fatalf("FindRefsAndDelete: %v", err) } if len(refs) > 0 { t.Fatalf("expected no refs blocking delete, got %v", refs) } - // Exists should return false after soft-delete (status=DEPRECATED) + // The row must be gone entirely, not merely deprecated. + if _, err := repo.GetByHandle(orgID, "deletable"); !apperror.SecretNotFound.Is(err) { + t.Errorf("GetByHandle after delete: got %v, want SecretNotFound", err) + } + exists, err := repo.Exists(orgID, "deletable") if err != nil { t.Fatalf("Exists: %v", err) } if exists { - t.Error("expected secret to be inactive after soft-delete") + t.Error("expected secret to be gone after delete") } } @@ -862,11 +866,11 @@ func TestSecretRepo_Create_UniqueConstraint_409(t *testing.T) { } } -// TestSecretRepo_FindRefsAndSoftDelete_Transactional verifies the transactional -// behaviour of FindRefsAndSoftDelete (scenario 87): -// - When refs exist the secret is NOT deprecated and the refs are returned. -// - After refs are removed a second call DOES deprecate the secret. -func TestSecretRepo_FindRefsAndSoftDelete_Transactional(t *testing.T) { +// TestSecretRepo_FindRefsAndDelete_Transactional verifies the transactional +// behaviour of FindRefsAndDelete (scenario 87): +// - When refs exist the secret is NOT deleted and the refs are returned. +// - After refs are removed a second call DOES permanently delete the secret. +func TestSecretRepo_FindRefsAndDelete_Transactional(t *testing.T) { db, cleanup := setupTestDB(t) t.Cleanup(cleanup) @@ -900,10 +904,10 @@ func TestSecretRepo_FindRefsAndSoftDelete_Transactional(t *testing.T) { t.Fatalf("insert ref: %v", err) } - // (a) With refs present: FindRefsAndSoftDelete must return refs and must NOT deprecate. - refs, err := repo.FindRefsAndSoftDelete(orgID, "txn-secret", "admin") + // (a) With refs present: FindRefsAndDelete must return refs and must NOT delete. + refs, err := repo.FindRefsAndDelete(orgID, "txn-secret") if err != nil { - t.Fatalf("FindRefsAndSoftDelete (with refs): %v", err) + t.Fatalf("FindRefsAndDelete (with refs): %v", err) } if len(refs) == 0 { t.Fatal("expected refs to block deletion, got none") @@ -918,27 +922,31 @@ func TestSecretRepo_FindRefsAndSoftDelete_Transactional(t *testing.T) { t.Errorf("secret should still be ACTIVE, got %q", got.Status) } - // (b) Remove the ref, then FindRefsAndSoftDelete must deprecate the secret. + // (b) Remove the ref, then FindRefsAndDelete must permanently delete the secret. _, err = db.Exec(`DELETE FROM artifact_secret_refs WHERE organization_uuid = ? AND artifact_uuid = 'art-txn-001'`, orgID) if err != nil { t.Fatalf("delete ref: %v", err) } - refs, err = repo.FindRefsAndSoftDelete(orgID, "txn-secret", "admin") + refs, err = repo.FindRefsAndDelete(orgID, "txn-secret") if err != nil { - t.Fatalf("FindRefsAndSoftDelete (no refs): %v", err) + t.Fatalf("FindRefsAndDelete (no refs): %v", err) } if len(refs) != 0 { t.Errorf("expected no refs after removal, got %d", len(refs)) } - // Secret must now be DEPRECATED (active Exists returns false). + // The row must be gone entirely, not merely deprecated. + if _, err := repo.GetByHandle(orgID, "txn-secret"); !apperror.SecretNotFound.Is(err) { + t.Errorf("GetByHandle after delete: got %v, want SecretNotFound", err) + } + exists, err := repo.Exists(orgID, "txn-secret") if err != nil { t.Fatalf("Exists: %v", err) } if exists { - t.Error("expected secret to be DEPRECATED (inactive) after successful soft-delete") + t.Error("expected secret to be gone after successful delete") } } diff --git a/platform-api/internal/server/scope_route_coverage_test.go b/platform-api/internal/server/scope_route_coverage_test.go index 0056cdbec7..6e9d5e67e6 100644 --- a/platform-api/internal/server/scope_route_coverage_test.go +++ b/platform-api/internal/server/scope_route_coverage_test.go @@ -130,6 +130,7 @@ func TestSecretsRoutesAreRegisteredOnTheBasePath(t *testing.T) { {http.MethodGet, constants.APIBasePath + "/secrets"}, {http.MethodPost, constants.APIBasePath + "/secrets"}, {http.MethodGet, constants.APIBasePath + "/secrets/s-1"}, + {http.MethodGet, constants.APIBasePath + "/secrets/s-1/usages"}, {http.MethodPut, constants.APIBasePath + "/secrets/s-1"}, {http.MethodDelete, constants.APIBasePath + "/secrets/s-1"}, } { diff --git a/platform-api/internal/server/server.go b/platform-api/internal/server/server.go index fd376a7f13..4d9dee1a13 100644 --- a/platform-api/internal/server/server.go +++ b/platform-api/internal/server/server.go @@ -322,7 +322,8 @@ func StartPlatformAPIServer(cfg *config.Server, slogger *slog.Logger, if vaultErr != nil { return nil, fmt.Errorf("failed to initialize secret vault: %w", vaultErr) } - secretService := service.NewSecretService(secretRepo, secretVault, identityService) + secretService := service.NewSecretService(secretRepo, secretVault, identityService). + WithGatewayBroadcast(gatewayRepo, gatewayEventsService) // Initialize handlers orgHandler := handler.NewOrganizationHandler(orgService, identityService, cfg.Auth.Authorization.Mode, slogger) diff --git a/platform-api/internal/service/api.go b/platform-api/internal/service/api.go index 46696c3ee1..5bade0ed51 100644 --- a/platform-api/internal/service/api.go +++ b/platform-api/internal/service/api.go @@ -473,9 +473,13 @@ func (s *APIService) DeleteAPI(apiUUID, orgUUID, deletedBy string) error { } // Delete API from repository (this also deletes associations) - if err := s.apiRepo.DeleteAPI(apiUUID, orgUUID); err != nil { + referencedSecrets, err := s.apiRepo.DeleteAPI(apiUUID, orgUUID) + if err != nil { return fmt.Errorf("failed to delete api: %w", err) } + if s.secretService != nil { + s.secretService.CleanupOrphanedSecrets(orgUUID, referencedSecrets, deletedBy) + } _ = s.auditRepo.Record("DELETE", apiUUID, "rest_api", orgUUID, deletedBy) diff --git a/platform-api/internal/service/api_secret_integration_test.go b/platform-api/internal/service/api_secret_integration_test.go index 7d337cd078..3de2c4e707 100644 --- a/platform-api/internal/service/api_secret_integration_test.go +++ b/platform-api/internal/service/api_secret_integration_test.go @@ -215,12 +215,10 @@ func TestAPIServiceSecretLifecycle_Integration(t *testing.T) { t.Fatalf("UpdateAPI (rotate) failed: %v", err) } - secretA, err := secretSvc.Get(apiSecretITOrgUUID, "it-secret-a") - if err != nil { - t.Fatalf("failed to fetch secret A after rotation: %v", err) - } - if secretA.Status != string(model.SecretStatusDeprecated) { - t.Errorf("expected secret A to be deprecated after rotation, got status=%q", secretA.Status) + // Secret A is no longer referenced by anything after rotation, so rotation + // cleanup permanently deletes it rather than merely deprecating it. + if _, err := secretSvc.Get(apiSecretITOrgUUID, "it-secret-a"); !apperror.SecretNotFound.Is(err) { + t.Errorf("expected secret A to be permanently deleted after rotation, got: %v", err) } secretB, err := secretSvc.Get(apiSecretITOrgUUID, "it-secret-b") if err != nil { @@ -229,10 +227,6 @@ func TestAPIServiceSecretLifecycle_Integration(t *testing.T) { if secretB.Status != string(model.SecretStatusActive) { t.Errorf("expected secret B to remain active after rotation, got status=%q", secretB.Status) } - // Secret A is no longer referenced, so it can now be hard-deleted. - if err := secretSvc.Delete(apiSecretITOrgUUID, "it-secret-a", "alice"); err != nil { - t.Errorf("expected secret A to be deletable after rotation freed it, got: %v", err) - } // --- Validation: a placeholder in upstream.auth that doesn't resolve is rejected --- badUpstreamReq := &api.RESTAPI{ @@ -272,3 +266,51 @@ func TestAPIServiceSecretLifecycle_Integration(t *testing.T) { t.Errorf("expected a validation error for missing secret ref, got: %v", err) } } + +// TestAPIServiceDeleteAPI_CleansUpOrphanedSecret_Integration proves deleting a +// REST API permanently removes a secret it solely referenced, against a real +// DB exercising the actual artifact_secret_refs tracking. +func TestAPIServiceDeleteAPI_CleansUpOrphanedSecret_Integration(t *testing.T) { + apiSvc, secretSvc, cleanup := setupAPISecretTestEnv(t) + defer cleanup() + + createTestSecret(t, secretSvc, apiSecretITOrgUUID, "del-solo-secret", "sk-solo-token") + + createReq := &api.CreateRESTAPIRequest{ + DisplayName: "IT REST API To Delete", + Context: "/it-rest-api-delete", + Version: "v1", + ProjectId: "api-secret-it-proj", + Upstream: api.Upstream{ + Main: api.UpstreamDefinition{ + Url: utils.StringPtrIfNotEmpty("https://backend.internal/api"), + Auth: &api.UpstreamAuth{ + Type: upstreamAuthTypePtr("bearer"), + Header: ptr("Authorization"), + Value: ptr(`{{ secret "del-solo-secret" }}`), + }, + }, + }, + } + created, err := apiSvc.CreateAPI(createReq, apiSecretITOrgUUID, "alice") + if err != nil { + t.Fatalf("CreateAPI failed: %v", err) + } + apiUUID := created.Id + if apiUUID == nil || *apiUUID == "" { + t.Fatal("expected CreateAPI to return an id") + } + + // Sanity check: the secret is blocked from direct deletion while the API references it. + if err := secretSvc.Delete(apiSecretITOrgUUID, "del-solo-secret", "alice"); err == nil { + t.Fatal("expected secret deletion to be blocked while the API references it") + } + + if err := apiSvc.DeleteAPIByHandle(*apiUUID, apiSecretITOrgUUID, "alice"); err != nil { + t.Fatalf("DeleteAPIByHandle failed: %v", err) + } + + if _, err := secretSvc.Get(apiSecretITOrgUUID, "del-solo-secret"); !apperror.SecretNotFound.Is(err) { + t.Errorf("expected orphaned secret to be permanently deleted after API deletion, got: %v", err) + } +} diff --git a/platform-api/internal/service/api_test.go b/platform-api/internal/service/api_test.go index e74b70e333..75da950f38 100644 --- a/platform-api/internal/service/api_test.go +++ b/platform-api/internal/service/api_test.go @@ -755,8 +755,8 @@ func TestAPIServiceUpdate_CleansUpRotatedSecret(t *testing.T) { if err != nil { t.Fatalf("expected no error, got: %v", err) } - if secretRepo.secrets["old-handle"].Status != model.SecretStatusDeprecated { - t.Errorf("expected old secret to be deprecated, got status=%v", secretRepo.secrets["old-handle"].Status) + if _, ok := secretRepo.secrets["old-handle"]; ok { + t.Error("expected old secret to be permanently deleted after rotation cleanup") } } diff --git a/platform-api/internal/service/gateway_events.go b/platform-api/internal/service/gateway_events.go index 095bb0aea8..d9ad7551fb 100644 --- a/platform-api/internal/service/gateway_events.go +++ b/platform-api/internal/service/gateway_events.go @@ -79,6 +79,15 @@ const ( EventTypeSubscriptionPlanCreated = "subscriptionPlan.created" EventTypeSubscriptionPlanUpdated = "subscriptionPlan.updated" EventTypeSubscriptionPlanDeleted = "subscriptionPlan.deleted" + + // EventTypeSecretUpdated fires on rotation (PUT /secrets/:handle). There is no + // EventTypeSecretCreated: a freshly created secret is not yet referenced by any + // deployed artifact, so no connected gateway has anything to refresh — the first + // time a gateway needs it, the artifact-deploy path's syncSecretRefsFromYAML picks + // it up already. EventTypeSecretDeleted fires on delete, which permanently + // removes the secret once no artifact references the handle. + EventTypeSecretUpdated = "secret.updated" + EventTypeSecretDeleted = "secret.deleted" ) // GatewayEventsService handles broadcasting events to connected gateways via EventHub. @@ -256,6 +265,16 @@ func (s *GatewayEventsService) BroadcastSubscriptionPlanDeletedEvent(gatewayID s return s.broadcastEvent(gatewayID, EventTypeSubscriptionPlanDeleted, event) } +// BroadcastSecretUpdatedEvent sends a secret.updated event to target gateway. +func (s *GatewayEventsService) BroadcastSecretUpdatedEvent(gatewayID string, event *model.SecretUpdatedEvent) error { + return s.broadcastEvent(gatewayID, EventTypeSecretUpdated, event) +} + +// BroadcastSecretDeletedEvent sends a secret.deleted event to target gateway. +func (s *GatewayEventsService) BroadcastSecretDeletedEvent(gatewayID string, event *model.SecretDeletedEvent) error { + return s.broadcastEvent(gatewayID, EventTypeSecretDeleted, event) +} + // broadcastEvent is the generic helper for broadcasting gateway events without a userId. func (s *GatewayEventsService) broadcastEvent(gatewayID, eventType string, payload interface{}) error { return s.broadcastEventWithUserID(gatewayID, "", eventType, payload) diff --git a/platform-api/internal/service/llm.go b/platform-api/internal/service/llm.go index eece1699a3..2759e19e72 100644 --- a/platform-api/internal/service/llm.go +++ b/platform-api/internal/service/llm.go @@ -1320,12 +1320,16 @@ func (s *LLMProviderService) Delete(orgUUID, handle, deletedBy string) error { } } - if err := s.repo.Delete(handle, orgUUID); err != nil { + referencedSecrets, err := s.repo.Delete(handle, orgUUID) + if err != nil { if errors.Is(err, sql.ErrNoRows) { return apperror.LLMProviderNotFound.New() } return fmt.Errorf("failed to delete provider: %w", err) } + if s.secretService != nil { + s.secretService.CleanupOrphanedSecrets(orgUUID, referencedSecrets, deletedBy) + } _ = s.auditRepo.Record("DELETE", provider.UUID, "llm_provider", orgUUID, deletedBy) @@ -1918,12 +1922,16 @@ func (s *LLMProxyService) Delete(orgUUID, handle, deletedBy string) error { } } - if err := s.repo.Delete(handle, orgUUID); err != nil { + referencedSecrets, err := s.repo.Delete(handle, orgUUID) + if err != nil { if errors.Is(err, sql.ErrNoRows) { return apperror.LLMProxyNotFound.New() } return fmt.Errorf("failed to delete proxy: %w", err) } + if s.secretService != nil { + s.secretService.CleanupOrphanedSecrets(orgUUID, referencedSecrets, deletedBy) + } _ = s.auditRepo.Record("DELETE", proxy.UUID, "llm_proxy", orgUUID, deletedBy) // Send deletion events to all gateways in the organization diff --git a/platform-api/internal/service/llm_test.go b/platform-api/internal/service/llm_test.go index e28bc25b0a..d2a45791e4 100644 --- a/platform-api/internal/service/llm_test.go +++ b/platform-api/internal/service/llm_test.go @@ -1012,6 +1012,7 @@ type mockLLMProviderRepo struct { existsResult bool countResult int getByIDFunc func(providerID, orgUUID string) (*model.LLMProvider, error) + deleteFunc func(providerID, orgUUID string) ([]string, error) createCalled bool created *model.LLMProvider updated *model.LLMProvider @@ -1047,6 +1048,13 @@ func (m *mockLLMProviderRepo) Update(p *model.LLMProvider) error { return nil } +func (m *mockLLMProviderRepo) Delete(providerID, orgUUID string) ([]string, error) { + if m.deleteFunc != nil { + return m.deleteFunc(providerID, orgUUID) + } + return nil, nil +} + func (m *mockLLMProviderRepo) UpdateWithCustomPolicyUsages(p *model.LLMProvider, _ []string) error { return m.Update(p) } @@ -1088,6 +1096,7 @@ type mockLLMProxyRepo struct { listByProviderItems []*model.LLMProxy lastListProviderUUID string getByIDFunc func(proxyID, orgUUID string) (*model.LLMProxy, error) + deleteFunc func(proxyID, orgUUID string) ([]string, error) created *model.LLMProxy updated *model.LLMProxy } @@ -1117,6 +1126,13 @@ func (m *mockLLMProxyRepo) Update(p *model.LLMProxy) error { return nil } +func (m *mockLLMProxyRepo) Delete(proxyID, orgUUID string) ([]string, error) { + if m.deleteFunc != nil { + return m.deleteFunc(proxyID, orgUUID) + } + return nil, nil +} + func (m *mockLLMProxyRepo) ListByProvider(orgUUID, providerUUID string, limit, offset int) ([]*model.LLMProxy, error) { m.lastListProviderUUID = providerUUID return m.listByProviderItems, nil @@ -1784,14 +1800,72 @@ func TestLLMProviderServiceUpdate_CleansUpRotatedSecret(t *testing.T) { if _, err := service.Update("org-1", "provider-1", "alice", request); err != nil { t.Fatalf("expected no error, got: %v", err) } - if secretRepo.secrets["old-handle"].Status != model.SecretStatusDeprecated { - t.Fatalf("expected old secret to be deprecated, got status=%v", secretRepo.secrets["old-handle"].Status) + if _, ok := secretRepo.secrets["old-handle"]; ok { + t.Fatal("expected old secret to be permanently deleted after rotation cleanup") } if secretRepo.secrets["new-handle"].Status != model.SecretStatusActive { t.Fatalf("expected new secret to remain active, got status=%v", secretRepo.secrets["new-handle"].Status) } } +// TestLLMProviderServiceDelete_CleansUpOrphanedSecret proves deleting a provider +// permanently removes a secret it referenced, once the provider (and thus the +// artifact_secret_refs row backing that reference) is gone and nothing else +// references the handle. +func TestLLMProviderServiceDelete_CleansUpOrphanedSecret(t *testing.T) { + providerRepo := &mockLLMProviderRepo{ + getByIDFunc: func(providerID, orgUUID string) (*model.LLMProvider, error) { + return &model.LLMProvider{UUID: "prov-uuid", ID: providerID}, nil + }, + deleteFunc: func(providerID, orgUUID string) ([]string, error) { + return []string{"solo-handle"}, nil + }, + } + secretRepo := newMockRepo() + secretRepo.secrets["solo-handle"] = &model.Secret{Handle: "solo-handle", Status: model.SecretStatusActive} + secretService := NewSecretService(secretRepo, &mockVault{}, newTestIdentityService()) + + service := NewLLMProviderService(providerRepo, nil, nil, nil, nil, nil, nil, slog.Default(), &noopAuditRepo{}, &config.Server{}, newTestIdentityService()) + service.SetSecretService(secretService) + + if err := service.Delete("org-1", "provider-1", "alice"); err != nil { + t.Fatalf("expected no error, got: %v", err) + } + if _, ok := secretRepo.secrets["solo-handle"]; ok { + t.Error("expected orphaned secret to be permanently deleted after provider deletion") + } +} + +// TestLLMProviderServiceDelete_DoesNotCleanUpSecretStillReferencedElsewhere proves +// the orphan cleanup is reference-checked, not an unconditional delete of every +// handle the provider happened to use. +func TestLLMProviderServiceDelete_DoesNotCleanUpSecretStillReferencedElsewhere(t *testing.T) { + providerRepo := &mockLLMProviderRepo{ + getByIDFunc: func(providerID, orgUUID string) (*model.LLMProvider, error) { + return &model.LLMProvider{UUID: "prov-uuid", ID: providerID}, nil + }, + deleteFunc: func(providerID, orgUUID string) ([]string, error) { + return []string{"shared-handle"}, nil + }, + } + secretRepo := newMockRepo() + secretRepo.secrets["shared-handle"] = &model.Secret{Handle: "shared-handle", Status: model.SecretStatusActive} + secretRepo.findRefsFn = func(orgID, handle string) ([]model.SecretReference, error) { + return []model.SecretReference{{Handle: "other-proxy", Name: "Other Proxy", Type: "LlmProxy"}}, nil + } + secretService := NewSecretService(secretRepo, &mockVault{}, newTestIdentityService()) + + service := NewLLMProviderService(providerRepo, nil, nil, nil, nil, nil, nil, slog.Default(), &noopAuditRepo{}, &config.Server{}, newTestIdentityService()) + service.SetSecretService(secretService) + + if err := service.Delete("org-1", "provider-1", "alice"); err != nil { + t.Fatalf("expected no error, got: %v", err) + } + if _, ok := secretRepo.secrets["shared-handle"]; !ok { + t.Error("expected secret still referenced elsewhere to survive provider deletion") + } +} + func TestLLMProxyServiceUpdate_CleansUpRotatedSecret(t *testing.T) { now := time.Now() proxyRepo := &mockLLMProxyRepo{} @@ -1842,8 +1916,8 @@ func TestLLMProxyServiceUpdate_CleansUpRotatedSecret(t *testing.T) { if err != nil { t.Fatalf("expected no error, got: %v", err) } - if secretRepo.secrets["old-handle"].Status != model.SecretStatusDeprecated { - t.Fatalf("expected old secret to be deprecated, got status=%v", secretRepo.secrets["old-handle"].Status) + if _, ok := secretRepo.secrets["old-handle"]; ok { + t.Fatal("expected old secret to be permanently deleted after rotation cleanup") } } diff --git a/platform-api/internal/service/mcp.go b/platform-api/internal/service/mcp.go index dd498fb6a2..aae0a687fb 100644 --- a/platform-api/internal/service/mcp.go +++ b/platform-api/internal/service/mcp.go @@ -522,12 +522,16 @@ func (s *MCPProxyService) Delete(orgUUID, handle, deletedBy string) error { } } - if err := s.repo.Delete(handle, orgUUID); err != nil { + referencedSecrets, err := s.repo.Delete(handle, orgUUID) + if err != nil { if errors.Is(err, sql.ErrNoRows) { return apperror.MCPProxyNotFound.Wrap(err) } return fmt.Errorf("failed to delete MCP proxy: %w", err) } + if s.secretService != nil { + s.secretService.CleanupOrphanedSecrets(orgUUID, referencedSecrets, deletedBy) + } _ = s.auditRepo.Record("DELETE", mcpProxy.UUID, "mcp_proxy", orgUUID, deletedBy) // Send deletion events to all gateways in the organization diff --git a/platform-api/internal/service/mcp_secret_integration_test.go b/platform-api/internal/service/mcp_secret_integration_test.go index 98fd8bd980..8a55996aef 100644 --- a/platform-api/internal/service/mcp_secret_integration_test.go +++ b/platform-api/internal/service/mcp_secret_integration_test.go @@ -32,6 +32,7 @@ import ( "github.com/wso2/api-platform/platform-api/api" "github.com/wso2/api-platform/platform-api/config" + "github.com/wso2/api-platform/platform-api/internal/apperror" "github.com/wso2/api-platform/platform-api/internal/database" "github.com/wso2/api-platform/platform-api/internal/model" "github.com/wso2/api-platform/platform-api/internal/repository" @@ -123,12 +124,10 @@ func TestMCPProxyServiceUpdate_CleansUpRotatedSecret_Integration(t *testing.T) { t.Fatalf("Update (rotate) failed: %v", err) } - secretA, err := secretSvc.Get(mcpSecretITOrgUUID, "mcp-it-secret-a") - if err != nil { - t.Fatalf("failed to fetch secret A after rotation: %v", err) - } - if secretA.Status != string(model.SecretStatusDeprecated) { - t.Errorf("expected secret A to be deprecated after rotation, got status=%q", secretA.Status) + // Secret A is no longer referenced by anything after rotation, so rotation + // cleanup permanently deletes it rather than merely deprecating it. + if _, err := secretSvc.Get(mcpSecretITOrgUUID, "mcp-it-secret-a"); !apperror.SecretNotFound.Is(err) { + t.Errorf("expected secret A to be permanently deleted after rotation, got: %v", err) } secretB, err := secretSvc.Get(mcpSecretITOrgUUID, "mcp-it-secret-b") if err != nil { @@ -138,3 +137,77 @@ func TestMCPProxyServiceUpdate_CleansUpRotatedSecret_Integration(t *testing.T) { t.Errorf("expected secret B to remain active after rotation, got status=%q", secretB.Status) } } + +// TestMCPProxyServiceDelete_CleansUpOrphanedSecret_Integration proves deleting +// an MCP proxy permanently removes a secret it solely referenced, against a +// real DB exercising the actual artifact_secret_refs tracking. +func TestMCPProxyServiceDelete_CleansUpOrphanedSecret_Integration(t *testing.T) { + tmpDir := t.TempDir() + dbPath := filepath.Join(tmpDir, "mcp-secret-delete-it.db") + sqlDB, err := sql.Open("sqlite3", dbPath) + if err != nil { + t.Fatalf("open db: %v", err) + } + defer sqlDB.Close() + sqlDB.Exec("PRAGMA foreign_keys = ON") + db := &database.DB{DB: sqlDB} + + schemaPath := filepath.Join("..", "database", "schema.sqlite.sql") + schema, err := os.ReadFile(schemaPath) + if err != nil { + t.Fatalf("read schema: %v", err) + } + if _, err = db.Exec(string(schema)); err != nil { + t.Fatalf("apply schema: %v", err) + } + const orgUUID = "org-mcp-secret-delete-it" + if _, err = db.Exec(`INSERT INTO organizations (uuid, handle, display_name, region, idp_organization_ref_uuid, created_at, updated_at) + VALUES (?, 'mcp-secret-delete-it-org', 'MCP Secret Delete IT Org', 'default', 'idp-ref', datetime('now'), datetime('now'))`, orgUUID); err != nil { + t.Fatalf("insert org: %v", err) + } + + v, err := vault.NewInHouseVault([]byte("12345678901234567890123456789012")) + if err != nil { + t.Fatalf("create vault: %v", err) + } + identity := NewIdentityService(repository.NewUserIdentityMappingRepo(db)) + secretRepo := repository.NewSecretRepo(db) + secretSvc := NewSecretService(secretRepo, v, identity) + + createTestSecret(t, secretSvc, orgUUID, "mcp-del-solo-secret", "sk-mcp-solo-token") + + mcpRepo := repository.NewMCPProxyRepo(db) + mcpSvc := NewMCPProxyService(mcpRepo, nil, nil, nil, nil, slog.Default(), repository.NewAuditRepo(db), &config.Server{}, identity) + mcpSvc.WithSecretService(secretSvc) + + created, err := mcpSvc.Create(orgUUID, "alice", &api.MCPProxy{ + DisplayName: "IT MCP Proxy To Delete", + Version: "v1.0", + Upstream: api.Upstream{ + Main: api.UpstreamDefinition{ + Url: utils.StringPtrIfNotEmpty("https://mcp-backend.internal"), + Auth: &api.UpstreamAuth{ + Type: upstreamAuthTypePtr("bearer"), + Header: ptr("Authorization"), + Value: ptr(`{{ secret "mcp-del-solo-secret" }}`), + }, + }, + }, + }) + if err != nil { + t.Fatalf("Create failed: %v", err) + } + + // Sanity check: the secret is blocked from direct deletion while the proxy references it. + if err := secretSvc.Delete(orgUUID, "mcp-del-solo-secret", "alice"); err == nil { + t.Fatal("expected secret deletion to be blocked while the proxy references it") + } + + if err := mcpSvc.Delete(orgUUID, *created.Id, "alice"); err != nil { + t.Fatalf("Delete failed: %v", err) + } + + if _, err := secretSvc.Get(orgUUID, "mcp-del-solo-secret"); !apperror.SecretNotFound.Is(err) { + t.Errorf("expected orphaned secret to be permanently deleted after proxy deletion, got: %v", err) + } +} diff --git a/platform-api/internal/service/mcp_secret_resolution_test.go b/platform-api/internal/service/mcp_secret_resolution_test.go new file mode 100644 index 0000000000..94c4ca319c --- /dev/null +++ b/platform-api/internal/service/mcp_secret_resolution_test.go @@ -0,0 +1,128 @@ +/* + * Copyright (c) 2026, WSO2 LLC. (http://www.wso2.org) All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package service + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" + "github.com/wso2/api-platform/platform-api/api" + "github.com/wso2/api-platform/platform-api/config" + "github.com/wso2/api-platform/platform-api/internal/dto" + "github.com/wso2/api-platform/platform-api/internal/model" +) + +// TestFetchServerInfo_ProxyIdRefetch_ResolvesSecretHandle verifies the exact bug +// fixed in FetchServerInfo's proxyId branch: the stored upstream auth value is a +// {{ secret "handle" }} placeholder, not the plaintext credential, and it must be +// resolved through the secret store before being sent to the target MCP server as +// the actual header value — never sent to the upstream as the literal placeholder +// text, and never appear anywhere in the response returned to the caller. +func TestFetchServerInfo_ProxyIdRefetch_ResolvesSecretHandle(t *testing.T) { + const orgID = "org-1" + const plaintextCredential = "Bearer super-secret-live-token" + + // Real SecretService over the lightweight mock vault/repo already used by + // secret_service_test.go, so Create/Decrypt round-trip through the same + // encrypt/decrypt code path FetchServerInfo itself calls. + vault := &mockVault{} + secretRepo := newMockRepo() + secretService := NewSecretService(secretRepo, vault, newTestIdentityService()) + + created, err := secretService.Create(orgID, "tester", &dto.CreateSecretRequest{ + Handle: "upstream-auth-handle", + DisplayName: "Upstream auth", + Value: plaintextCredential, + Type: model.SecretTypeGeneric, + }) + require.NoError(t, err) + require.Equal(t, "upstream-auth-handle", created.Handle) + + var receivedAuthHeader string + mockMCPServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodHead { + w.WriteHeader(http.StatusOK) + return + } + + var body struct { + Method string `json:"method"` + ID *int `json:"id"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + + // Captured on every JSON-RPC call — initialize is the first one that + // carries the custom auth header, so this is where the leak (or lack of + // one) would show up. + if h := r.Header.Get("Authorization"); h != "" { + receivedAuthHeader = h + } + + w.Header().Set("Content-Type", "application/json") + switch body.Method { + case "initialize": + _, _ = w.Write([]byte(`{"jsonrpc":"2.0","id":1,"result":{"protocolVersion":"2025-06-18","capabilities":{},"serverInfo":{"name":"test","version":"1.0.0"}}}`)) + case "notifications/initialized": + w.WriteHeader(http.StatusAccepted) + case "tools/list": + _, _ = w.Write([]byte(`{"jsonrpc":"2.0","id":2,"result":{"tools":[]}}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer mockMCPServer.Close() + + repo := &mockMCPProxyRepository{getByHandleResult: &model.MCPProxy{ + Handle: "mcp-proxy-1", + Configuration: model.MCPProxyConfiguration{ + Upstream: model.UpstreamConfig{ + Main: &model.UpstreamEndpoint{ + URL: mockMCPServer.URL, + Auth: &model.UpstreamAuth{ + Type: "header", + Header: "Authorization", + // The stored value is always a placeholder, never plaintext. + Value: `{{ secret "upstream-auth-handle" }}`, + }, + }, + }, + }, + }} + + mcpService := NewMCPProxyService(repo, nil, nil, nil, nil, newTestLogger(), &noopAuditRepo{}, &config.Server{}, newTestIdentityService()) + mcpService.WithSecretService(secretService) + + proxyID := "mcp-proxy-1" + resp, err := mcpService.FetchServerInfo(orgID, &api.MCPServerInfoFetchRequest{ProxyId: &proxyID}) + require.NoError(t, err) + require.NotNil(t, resp) + + require.Equal(t, plaintextCredential, receivedAuthHeader, + "the upstream MCP server must receive the resolved plaintext credential, not the raw secret placeholder") + require.NotContains(t, receivedAuthHeader, "{{ secret", + "the placeholder text must never be sent to the upstream server as-is") + + // The resolved plaintext must never appear anywhere in the client-facing response. + responseJSON, err := json.Marshal(resp) + require.NoError(t, err) + require.NotContains(t, string(responseJSON), plaintextCredential, + "the resolved credential must never be echoed back in the fetch-server-info response") +} diff --git a/platform-api/internal/service/secret_service.go b/platform-api/internal/service/secret_service.go index 89691e3678..7fe0a9cb1d 100644 --- a/platform-api/internal/service/secret_service.go +++ b/platform-api/internal/service/secret_service.go @@ -44,13 +44,28 @@ func (e *SecretInUseError) Error() string { } type SecretService struct { - repo repository.SecretRepository - vault vault.SecretVault - identity *IdentityService + repo repository.SecretRepository + vault vault.SecretVault + identity *IdentityService + gatewayRepo repository.GatewayRepository + gatewayEvents *GatewayEventsService + slogger *slog.Logger } func NewSecretService(repo repository.SecretRepository, v vault.SecretVault, identity *IdentityService) *SecretService { - return &SecretService{repo: repo, vault: v, identity: identity} + return &SecretService{repo: repo, vault: v, identity: identity, slogger: slog.Default()} +} + +// WithGatewayBroadcast enables best-effort secret.updated/secret.deleted +// notifications to every gateway in the organization on rotate/delete (see +// broadcastSecretEvent). Optional: a SecretService built without this call simply +// skips broadcasting — kept as a post-construction setter rather than a constructor +// parameter so the many existing unit tests that build a bare SecretService don't +// all need updating for a purely additive, best-effort side channel. +func (s *SecretService) WithGatewayBroadcast(gatewayRepo repository.GatewayRepository, gatewayEvents *GatewayEventsService) *SecretService { + s.gatewayRepo = gatewayRepo + s.gatewayEvents = gatewayEvents + return s } // toSecretResponse converts secret via secretToResponse and resolves its @@ -89,6 +104,10 @@ func (s *SecretService) toSecretSummary(secret *model.Secret) (*dto.SecretSummar } func (s *SecretService) Create(orgID, createdBy string, req *dto.CreateSecretRequest) (*dto.SecretResponse, error) { + if err := validateSecretHandle(req.Handle); err != nil { + return nil, err + } + secretType := req.Type if secretType == "" { secretType = model.SecretTypeGeneric @@ -178,41 +197,146 @@ func (s *SecretService) Update(orgID, handle, updatedBy string, req *dto.UpdateS return nil, err } - ciphertext, err := s.vault.Encrypt(context.Background(), req.Value) - if err != nil { - return nil, fmt.Errorf("failed to encrypt secret: %w", err) - } - if req.DisplayName != "" { existing.DisplayName = req.DisplayName } if req.Description != "" { existing.Description = req.Description } - existing.Ciphertext = ciphertext - existing.Hash = hashSecret(s.vault.HashKey(), req.Value) + // Value is optional: a metadata-only edit (no value) must not touch the + // ciphertext/hash or reactivate a deprecated secret — only an explicit + // rotation is an intent to put the secret back into service. + if req.Value != "" { + ciphertext, err := s.vault.Encrypt(context.Background(), req.Value) + if err != nil { + return nil, fmt.Errorf("failed to encrypt secret: %w", err) + } + existing.Ciphertext = ciphertext + existing.Hash = hashSecret(s.vault.HashKey(), req.Value) + existing.Status = model.SecretStatusActive + } existing.UpdatedBy = updatedBy - // Rotation is an explicit intent to put the secret back into service. - existing.Status = model.SecretStatusActive if err := s.repo.Update(existing); err != nil { return nil, fmt.Errorf("failed to update secret: %w", err) } - return s.toSecretResponse(existing) + // Build the response before broadcasting: toSecretResponse can still fail + // (identity-mapping lookups), and the caller must not be told the rotation + // failed after gateways have already been notified of it. If this errors, + // the DB commit above still stands — a retry will simply broadcast then. + resp, err := s.toSecretResponse(existing) + if err != nil { + return nil, err + } + + s.broadcastSecretEvent(orgID, "updated", &model.SecretUpdatedEvent{ + Handle: existing.Handle, + DisplayName: existing.DisplayName, + Hash: existing.Hash, + // existing.UpdatedAt was just set in-place by s.repo.Update above — see + // model.SecretUpdatedEvent.Revision for why UnixNano() is a safe ordering token. + Revision: existing.UpdatedAt.UnixNano(), + }) + + return resp, nil +} + +// GetReferences returns the resources that currently reference handle, so a +// caller can show why a secret is in use before attempting a delete. Existence +// is checked first so an unknown handle reports SecretNotFound rather than an +// empty usages list indistinguishable from "not referenced". +func (s *SecretService) GetReferences(orgID, handle string) ([]dto.SecretReferenceDTO, error) { + if _, err := s.repo.GetByHandle(orgID, handle); err != nil { + return nil, err + } + + refs, err := s.repo.FindRefs(orgID, handle) + if err != nil { + return nil, fmt.Errorf("failed to find secret references: %w", err) + } + + result := make([]dto.SecretReferenceDTO, 0, len(refs)) + for _, ref := range refs { + result = append(result, dto.SecretReferenceDTO{Type: ref.Type, Handle: ref.Handle, Name: ref.Name}) + } + return result, nil } func (s *SecretService) Delete(orgID, handle, updatedBy string) error { - refs, err := s.repo.FindRefsAndSoftDelete(orgID, handle, updatedBy) + // Captured before the delete so the broadcast Revision reflects the same + // moment as the DB write. + deletedAt := time.Now().UTC() + + refs, err := s.repo.FindRefsAndDelete(orgID, handle) if err != nil { return fmt.Errorf("failed to delete secret: %w", err) } if len(refs) > 0 { return &SecretInUseError{References: refs} } + + s.broadcastSecretEvent(orgID, "deleted", &model.SecretDeletedEvent{ + Handle: handle, + Revision: deletedAt.UnixNano(), + }) + return nil } +// CleanupOrphanedSecrets permanently deletes any of the given handles that are no +// longer referenced by anything, once the resource that used to hold one of these +// references (an LLM provider/proxy or MCP proxy) has already been deleted. Best-effort: +// a handle still referenced by some other resource, already gone, or any other error is +// logged and skipped rather than failing the caller's delete — the resource is already +// durably deleted by the time this runs. +func (s *SecretService) CleanupOrphanedSecrets(orgID string, handles []string, deletedBy string) { + for _, handle := range handles { + if err := s.Delete(orgID, handle, deletedBy); err != nil { + var inUseErr *SecretInUseError + if errors.As(err, &inUseErr) || apperror.SecretNotFound.Is(err) { + continue // still referenced elsewhere, or already gone — expected, not a failure + } + s.slogger.Warn("failed to clean up orphaned secret after resource deletion", + "handle", handle, "orgId", orgID, "err", err) + } + } +} + +// broadcastSecretEvent sends a secret.* event to every gateway in the organization. +// Best-effort: a load or delivery failure is logged and swallowed rather than failing +// the rotate/delete call — the change is already durably committed, and a gateway that +// misses the push still catches up via its own poll-based incremental sync (see +// docs/specs/secrets-management.md §6.5-6.7). Also a safe no-op when WithGatewayBroadcast +// was never called (e.g. in unit tests). +func (s *SecretService) broadcastSecretEvent(orgUUID, action string, payload interface{}) { + if s.gatewayEvents == nil || s.gatewayRepo == nil { + return + } + gateways, err := s.gatewayRepo.GetByOrganizationID(orgUUID) + if err != nil { + s.slogger.Warn("Failed to load gateways for secret broadcast", + "orgId", orgUUID, "action", action, "error", err) + return + } + for _, gw := range gateways { + if gw == nil || gw.ID == "" { + continue + } + var broadcastErr error + switch action { + case "updated": + broadcastErr = s.gatewayEvents.BroadcastSecretUpdatedEvent(gw.ID, payload.(*model.SecretUpdatedEvent)) + case "deleted": + broadcastErr = s.gatewayEvents.BroadcastSecretDeletedEvent(gw.ID, payload.(*model.SecretDeletedEvent)) + } + if broadcastErr != nil { + s.slogger.Warn("Failed to broadcast secret event", + "gatewayId", gw.ID, "action", action, "error", broadcastErr) + } + } +} + // extractSecretHandle returns the handle embedded in a {{ secret "handle" }} // placeholder, or "" if value is empty, plaintext, or otherwise not a placeholder. func extractSecretHandle(value string) string { @@ -269,7 +393,9 @@ func (s *SecretService) cleanupRotatedSecret(orgUUID, oldValue, newValue, update } // ValidateSecretRefs checks that every {{ secret "handle" }} placeholder in configText -// resolves to an active org-scoped secret. +// resolves to an active org-scoped secret. Missing and deprecated handles are reported +// separately (§5.6: a deprecated secret exists but cannot be referenced by new/updated +// resources) so the caller isn't told a real, existing-but-retired handle "does not exist". func (s *SecretService) ValidateSecretRefs(orgID, configText string) error { matches := constants.SecretPlaceholderRe.FindAllStringSubmatch(configText, -1) if len(matches) == 0 { @@ -278,6 +404,7 @@ func (s *SecretService) ValidateSecretRefs(orgID, configText string) error { seen := make(map[string]struct{}) var missing []string + var deprecated []string for _, m := range matches { handle := m[1] @@ -286,18 +413,50 @@ func (s *SecretService) ValidateSecretRefs(orgID, configText string) error { } seen[handle] = struct{}{} - found, err := s.repo.Exists(orgID, handle) + secret, err := s.repo.GetByHandle(orgID, handle) if err != nil { + if apperror.SecretNotFound.Is(err) { + missing = append(missing, handle) + continue + } return fmt.Errorf("failed to check existence of secret %q: %w", handle, err) } - if !found { - missing = append(missing, handle) + if secret.Status == model.SecretStatusDeprecated { + deprecated = append(deprecated, handle) } } + if len(missing) == 0 && len(deprecated) == 0 { + return nil + } + + var parts []string if len(missing) > 0 { + parts = append(parts, fmt.Sprintf("do not exist: %s", strings.Join(missing, ", "))) + } + if len(deprecated) > 0 { + parts = append(parts, fmt.Sprintf("are deprecated and cannot be referenced by new or updated resources: %s", strings.Join(deprecated, ", "))) + } + return apperror.ValidationFailed.New(fmt.Sprintf( + "The following referenced secrets %s.", strings.Join(parts, "; "))) +} + +// validateSecretHandle enforces the same handle shape the AI Workspace UI generates +// (constants.SecretHandlePattern) and the DB column's length limit +// (constants.SecretHandleMaxLength), so a non-UI caller cannot create a secret the +// UI itself could never produce — see constants.SecretHandlePattern's doc comment +// for why this matters (unreachable-via-router handles, placeholder-regex interference). +func validateSecretHandle(handle string) error { + if handle == "" { + return apperror.ValidationFailed.New("A secret handle is required.") + } + if len(handle) > constants.SecretHandleMaxLength { return apperror.ValidationFailed.New(fmt.Sprintf( - "The following referenced secrets do not exist: %s.", strings.Join(missing, ", "))) + "Secret handle must not exceed %d characters.", constants.SecretHandleMaxLength)) + } + if !constants.SecretHandlePattern.MatchString(handle) { + return apperror.ValidationFailed.New( + "Secret handle may only contain lowercase letters, numbers, and single hyphens (no leading, trailing, or doubled hyphens).") } return nil } diff --git a/platform-api/internal/service/secret_service_broadcast_test.go b/platform-api/internal/service/secret_service_broadcast_test.go new file mode 100644 index 0000000000..3308379700 --- /dev/null +++ b/platform-api/internal/service/secret_service_broadcast_test.go @@ -0,0 +1,352 @@ +/* + * Copyright (c) 2026, WSO2 LLC. (http://www.wso2.org) All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package service + +// Covers the platform-api-side publish path left uncovered when secret.updated / +// secret.deleted broadcasting was added (see docs/specs/secrets-management-test-scenarios.csv +// rows 121-122, previously marked GAP): SecretService.Update / Delete calling +// broadcastSecretEvent -> GatewayEventsService -> EventHub once WithGatewayBroadcast is +// wired. The receiving side (gateway-controller's handleSecretUpdatedEvent / +// handleSecretDeletedEvent) already has full coverage in +// gateway/gateway-controller/pkg/controlplane/sync_secrets_test.go (rows 124-132). + +import ( + "encoding/json" + "errors" + "log/slog" + "testing" + + "github.com/wso2/api-platform/common/eventhub" + "github.com/wso2/api-platform/platform-api/internal/dto" + "github.com/wso2/api-platform/platform-api/internal/model" + "github.com/wso2/api-platform/platform-api/internal/repository" +) + +// ---- fake EventHub ----------------------------------------------------------- + +// fakeEventHub is a minimal eventhub.EventHub double that records every published +// event per gateway. Only PublishEvent is exercised by GatewayEventsService's +// broadcastEvent path; the rest satisfy the interface as no-ops. +type fakeEventHub struct { + publishFn func(gatewayID string, event eventhub.Event) error + published map[string][]eventhub.Event +} + +func newFakeEventHub() *fakeEventHub { + return &fakeEventHub{published: make(map[string][]eventhub.Event)} +} + +func (f *fakeEventHub) Initialize() error { return nil } +func (f *fakeEventHub) RegisterGateway(_ string) error { return nil } +func (f *fakeEventHub) UnsubscribeAll(_ string) error { return nil } +func (f *fakeEventHub) CleanUpEvents() error { return nil } +func (f *fakeEventHub) Close() error { return nil } +func (f *fakeEventHub) Subscribe(_ string) (<-chan eventhub.Event, error) { + return nil, nil +} +func (f *fakeEventHub) Unsubscribe(_ string, _ <-chan eventhub.Event) error { + return nil +} + +func (f *fakeEventHub) PublishEvent(gatewayID string, event eventhub.Event) error { + if f.publishFn != nil { + if err := f.publishFn(gatewayID, event); err != nil { + return err + } + } + f.published[gatewayID] = append(f.published[gatewayID], event) + return nil +} + +// decodedPayload unmarshals the dto.GatewayEventDTO carried in a hub event's +// EventData, returning the event's Type string and its raw Payload for a +// caller-supplied struct to further unmarshal. +func decodedPayload(t *testing.T, evt eventhub.Event) (string, json.RawMessage) { + t.Helper() + var wrapper struct { + Type string `json:"type"` + Payload json.RawMessage `json:"payload"` + } + if err := json.Unmarshal([]byte(evt.EventData), &wrapper); err != nil { + t.Fatalf("failed to unmarshal event data: %v", err) + } + return wrapper.Type, wrapper.Payload +} + +// ---- fake gateway repo -------------------------------------------------------- + +// mockGatewayRepoForBroadcast embeds the full GatewayRepository interface (nil by +// default for everything) and overrides only GetByOrganizationID, the one method +// broadcastSecretEvent actually calls. +type mockGatewayRepoForBroadcast struct { + repository.GatewayRepository + getByOrgFn func(orgID string) ([]*model.Gateway, error) +} + +func (m *mockGatewayRepoForBroadcast) GetByOrganizationID(orgID string) ([]*model.Gateway, error) { + return m.getByOrgFn(orgID) +} + +// ---- helpers ------------------------------------------------------------------- + +func newBroadcastWiredSecretService(repo *mockSecretRepo, gwRepo repository.GatewayRepository, hub eventhub.EventHub) *SecretService { + events := NewGatewayEventsService(hub, nil, slog.Default()) + return NewSecretService(repo, &mockVault{}, newTestIdentityService()). + WithGatewayBroadcast(gwRepo, events) +} + +func twoGateways() *mockGatewayRepoForBroadcast { + return &mockGatewayRepoForBroadcast{ + getByOrgFn: func(orgID string) ([]*model.Gateway, error) { + return []*model.Gateway{{ID: "gw-a"}, {ID: "gw-b"}}, nil + }, + } +} + +// ---- Update -> secret.updated -------------------------------------------------- + +func TestSecretService_Update_BroadcastsSecretUpdatedEventToAllGateways(t *testing.T) { + repo := newMockRepo() + repo.secrets["openai-key"] = &model.Secret{Handle: "openai-key", DisplayName: "OpenAI Key", Status: model.SecretStatusActive} + + hub := newFakeEventHub() + svc := newBroadcastWiredSecretService(repo, twoGateways(), hub) + + if _, err := svc.Update("org1", "openai-key", "alice", &dto.UpdateSecretRequest{Value: "sk-rotated"}); err != nil { + t.Fatalf("Update: %v", err) + } + + for _, gw := range []string{"gw-a", "gw-b"} { + events := hub.published[gw] + if len(events) != 1 { + t.Fatalf("gateway %s: got %d published events, want 1", gw, len(events)) + } + eventType, payload := decodedPayload(t, events[0]) + if eventType != EventTypeSecretUpdated { + t.Errorf("gateway %s: event type = %q, want %q", gw, eventType, EventTypeSecretUpdated) + } + var updated model.SecretUpdatedEvent + if err := json.Unmarshal(payload, &updated); err != nil { + t.Fatalf("unmarshal payload: %v", err) + } + if updated.Handle != "openai-key" { + t.Errorf("gateway %s: payload handle = %q, want %q", gw, updated.Handle, "openai-key") + } + if updated.Hash != repo.secrets["openai-key"].Hash { + t.Errorf("gateway %s: payload hash = %q, want the freshly rotated hash %q", gw, updated.Hash, repo.secrets["openai-key"].Hash) + } + wantAction := "UPDATE" + if events[0].Action != wantAction { + t.Errorf("gateway %s: hub action = %q, want %q", gw, events[0].Action, wantAction) + } + } +} + +// ---- Delete -> secret.deleted ------------------------------------------------ + +func TestSecretService_Delete_BroadcastsSecretDeletedEventToAllGateways(t *testing.T) { + repo := newMockRepo() + repo.secrets["unused-key"] = &model.Secret{Handle: "unused-key", Status: model.SecretStatusActive} + + hub := newFakeEventHub() + svc := newBroadcastWiredSecretService(repo, twoGateways(), hub) + + if err := svc.Delete("org1", "unused-key", "alice"); err != nil { + t.Fatalf("Delete: %v", err) + } + + for _, gw := range []string{"gw-a", "gw-b"} { + events := hub.published[gw] + if len(events) != 1 { + t.Fatalf("gateway %s: got %d published events, want 1", gw, len(events)) + } + eventType, payload := decodedPayload(t, events[0]) + if eventType != EventTypeSecretDeleted { + t.Errorf("gateway %s: event type = %q, want %q", gw, eventType, EventTypeSecretDeleted) + } + var deleted model.SecretDeletedEvent + if err := json.Unmarshal(payload, &deleted); err != nil { + t.Fatalf("unmarshal payload: %v", err) + } + if deleted.Handle != "unused-key" { + t.Errorf("gateway %s: payload handle = %q, want %q", gw, deleted.Handle, "unused-key") + } + wantAction := "DELETE" + if events[0].Action != wantAction { + t.Errorf("gateway %s: hub action = %q, want %q", gw, events[0].Action, wantAction) + } + } +} + +// TestSecretService_Delete_BlockedWhenInUse_DoesNotBroadcast confirms the 409 +// in-use path returns before broadcastSecretEvent is ever reached — a secret that +// was not actually deleted must not tell any gateway to evict it. +func TestSecretService_Delete_BlockedWhenInUse_DoesNotBroadcast(t *testing.T) { + repo := newMockRepo() + repo.secrets["in-use"] = &model.Secret{Handle: "in-use", Status: model.SecretStatusActive} + repo.findRefsFn = func(orgID, handle string) ([]model.SecretReference, error) { + return []model.SecretReference{{Handle: "my-api", Name: "My API", Type: "RestApi"}}, nil + } + + hub := newFakeEventHub() + svc := newBroadcastWiredSecretService(repo, twoGateways(), hub) + + err := svc.Delete("org1", "in-use", "alice") + var inUseErr *SecretInUseError + if !errors.As(err, &inUseErr) { + t.Fatalf("expected SecretInUseError, got %T: %v", err, err) + } + + if total := len(hub.published["gw-a"]) + len(hub.published["gw-b"]); total != 0 { + t.Errorf("expected no events published when delete is blocked, got %d", total) + } +} + +// ---- Best-effort: broadcast failures never fail the caller's request ----------- + +func TestSecretService_Update_GatewayRepoError_DoesNotFailUpdate(t *testing.T) { + repo := newMockRepo() + repo.secrets["k1"] = &model.Secret{Handle: "k1", Status: model.SecretStatusActive} + + gwRepo := &mockGatewayRepoForBroadcast{ + getByOrgFn: func(orgID string) ([]*model.Gateway, error) { + return nil, errors.New("db unavailable") + }, + } + svc := newBroadcastWiredSecretService(repo, gwRepo, newFakeEventHub()) + + if _, err := svc.Update("org1", "k1", "alice", &dto.UpdateSecretRequest{Value: "new-val"}); err != nil { + t.Fatalf("Update must succeed even when loading gateways for broadcast fails, got: %v", err) + } + if repo.secrets["k1"].Status != model.SecretStatusActive { + t.Error("secret's own state must be unaffected by a broadcast-side failure") + } +} + +func TestSecretService_Update_PublishEventError_DoesNotFailUpdate_OtherGatewayStillNotified(t *testing.T) { + repo := newMockRepo() + repo.secrets["k1"] = &model.Secret{Handle: "k1", Status: model.SecretStatusActive} + + hub := newFakeEventHub() + hub.publishFn = func(gatewayID string, _ eventhub.Event) error { + if gatewayID == "gw-a" { + return errors.New("websocket delivery failed") + } + return nil + } + svc := newBroadcastWiredSecretService(repo, twoGateways(), hub) + + if _, err := svc.Update("org1", "k1", "alice", &dto.UpdateSecretRequest{Value: "new-val"}); err != nil { + t.Fatalf("Update must succeed even when one gateway's publish fails, got: %v", err) + } + if len(hub.published["gw-a"]) != 0 { + t.Errorf("gw-a's failed publish must not be recorded as delivered, got %d", len(hub.published["gw-a"])) + } + if len(hub.published["gw-b"]) != 1 { + t.Errorf("gw-b must still receive its event independently of gw-a's failure, got %d", len(hub.published["gw-b"])) + } +} + +// TestSecretService_Update_SkipsNilOrEmptyIDGateways confirms a malformed gateway +// entry (nil, or missing its ID) is skipped rather than passed to the event hub with +// an empty gateway ID. +func TestSecretService_Update_SkipsNilOrEmptyIDGateways(t *testing.T) { + repo := newMockRepo() + repo.secrets["k1"] = &model.Secret{Handle: "k1", Status: model.SecretStatusActive} + + hub := newFakeEventHub() + gwRepo := &mockGatewayRepoForBroadcast{ + getByOrgFn: func(orgID string) ([]*model.Gateway, error) { + return []*model.Gateway{nil, {ID: ""}, {ID: "gw-a"}}, nil + }, + } + svc := newBroadcastWiredSecretService(repo, gwRepo, hub) + + if _, err := svc.Update("org1", "k1", "alice", &dto.UpdateSecretRequest{Value: "new-val"}); err != nil { + t.Fatalf("Update: %v", err) + } + if len(hub.published[""]) != 0 { + t.Errorf("no event should be published under an empty gateway ID, got %d", len(hub.published[""])) + } + if len(hub.published["gw-a"]) != 1 { + t.Errorf("the one well-formed gateway must still receive its event, got %d", len(hub.published["gw-a"])) + } +} + +// ---- Update: broadcast must not fire before the response is confirmed --------- + +// failingIdentityRepo fails GetSubByUUID for a specific uuid, letting a test force +// SecretService.toSecretResponse to fail after the DB commit but before broadcast. +type failingIdentityRepo struct { + failFor string +} + +func (r failingIdentityRepo) GetOrCreateUUID(identity string) (string, error) { + return identity, nil +} + +func (r failingIdentityRepo) GetSubByUUID(uuid string) (string, bool, error) { + if uuid == r.failFor { + return "", false, errors.New("identity lookup unavailable") + } + return uuid, true, nil +} + +func (r failingIdentityRepo) GetSubsByUUIDs(uuids []string) (map[string]string, error) { + result := make(map[string]string, len(uuids)) + for _, id := range uuids { + if id == r.failFor { + return nil, errors.New("identity lookup unavailable") + } + if id != "" { + result[id] = id + } + } + return result, nil +} + +// TestSecretService_Update_ResponseBuildFailure_DoesNotBroadcast guards the +// ordering fix in Update(): broadcastSecretEvent must run only after +// toSecretResponse has succeeded, so an identity-lookup failure can't report +// "rotation failed" to the caller after gateways have already been told about it. +func TestSecretService_Update_ResponseBuildFailure_DoesNotBroadcast(t *testing.T) { + repo := newMockRepo() + repo.secrets["k1"] = &model.Secret{Handle: "k1", Status: model.SecretStatusActive, UpdatedBy: "alice"} + + hub := newFakeEventHub() + events := NewGatewayEventsService(hub, nil, slog.Default()) + identity := NewIdentityService(failingIdentityRepo{failFor: "alice"}) + svc := NewSecretService(repo, &mockVault{}, identity).WithGatewayBroadcast(twoGateways(), events) + + _, err := svc.Update("org1", "k1", "alice", &dto.UpdateSecretRequest{Value: "new-val"}) + if err == nil { + t.Fatal("expected Update to fail when the response cannot be built") + } + + // The DB commit already happened — the caller error must not mean the rotation + // didn't happen, only that we couldn't confirm it back to them synchronously. + if repo.secrets["k1"].Hash == "" { + t.Error("expected the DB row to still reflect the rotation despite the response-build failure") + } + + // The whole point of the ordering fix: no gateway should have been told about a + // rotation that was reported back to the caller as a failure. + if total := len(hub.published["gw-a"]) + len(hub.published["gw-b"]); total != 0 { + t.Errorf("expected no broadcast when toSecretResponse fails, got %d events", total) + } +} diff --git a/platform-api/internal/service/secret_service_test.go b/platform-api/internal/service/secret_service_test.go index 152a63793a..0e0943121f 100644 --- a/platform-api/internal/service/secret_service_test.go +++ b/platform-api/internal/service/secret_service_test.go @@ -20,6 +20,7 @@ package service import ( "context" "errors" + "strings" "testing" "time" @@ -64,14 +65,14 @@ type mockSecretRepo struct { secrets map[string]*model.Secret - createFn func(*model.Secret) error - existsFn func(orgID, handle string) (bool, error) - getByHandleFn func(orgID, handle string) (*model.Secret, error) - updateFn func(*model.Secret) error - findRefsAndSoftDeleteFn func(orgID, handle, by string) ([]model.SecretReference, error) - findRefsFn func(orgID, handle string) ([]model.SecretReference, error) - listFn func(orgID string, limit, offset int, after *time.Time) ([]*model.Secret, error) - countFn func(orgID string) (int, error) + createFn func(*model.Secret) error + existsFn func(orgID, handle string) (bool, error) + getByHandleFn func(orgID, handle string) (*model.Secret, error) + updateFn func(*model.Secret) error + findRefsAndDeleteFn func(orgID, handle string) ([]model.SecretReference, error) + findRefsFn func(orgID, handle string) ([]model.SecretReference, error) + listFn func(orgID string, limit, offset int, after *time.Time) ([]*model.Secret, error) + countFn func(orgID string) (int, error) } func newMockRepo() *mockSecretRepo { @@ -122,9 +123,9 @@ func (m *mockSecretRepo) Update(s *model.Secret) error { return nil } -func (m *mockSecretRepo) FindRefsAndSoftDelete(orgID, handle, by string) ([]model.SecretReference, error) { - if m.findRefsAndSoftDeleteFn != nil { - return m.findRefsAndSoftDeleteFn(orgID, handle, by) +func (m *mockSecretRepo) FindRefsAndDelete(orgID, handle string) ([]model.SecretReference, error) { + if m.findRefsAndDeleteFn != nil { + return m.findRefsAndDeleteFn(orgID, handle) } if m.findRefsFn != nil { refs, err := m.findRefsFn(orgID, handle) @@ -132,11 +133,10 @@ func (m *mockSecretRepo) FindRefsAndSoftDelete(orgID, handle, by string) ([]mode return refs, err } } - s, ok := m.secrets[handle] - if !ok { + if _, ok := m.secrets[handle]; !ok { return nil, apperror.SecretNotFound.New() } - s.Status = model.SecretStatusDeprecated + delete(m.secrets, handle) return nil, nil } @@ -276,6 +276,64 @@ func TestSecretService_Create_InvalidType_ReturnsError(t *testing.T) { } } +func TestSecretService_Create_InvalidHandle_ReturnsError(t *testing.T) { + cases := []struct { + name string + handle string + }{ + {"empty", ""}, + {"contains space", "bad handle"}, + {"contains slash", "foo/bar"}, + {"uppercase", "Bad-Handle"}, + {"leading hyphen", "-bad"}, + {"trailing hyphen", "bad-"}, + {"doubled hyphen", "bad--handle"}, + {"special characters", "bad!@#$%^&*()"}, + {"exceeds max length", strings.Repeat("a", 41)}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + repo := newMockRepo() + svc := NewSecretService(repo, &mockVault{}, newTestIdentityService()) + + _, err := svc.Create("org1", "alice", &dto.CreateSecretRequest{ + Handle: tc.handle, DisplayName: "Test", Value: "v", + }) + if err == nil { + t.Fatalf("expected validation error for handle %q, got nil", tc.handle) + } + if !apperror.ValidationFailed.Is(err) { + t.Errorf("expected VALIDATION_FAILED for handle %q, got: %v", tc.handle, err) + } + if _, exists := repo.secrets[tc.handle]; exists { + t.Errorf("invalid handle %q must not reach repo.Create", tc.handle) + } + }) + } +} + +// TestSecretService_Create_ValidHandle_Accepted guards against the handle +// pattern/length check (added after a handle containing "/" was found to be +// creatable but then permanently unreachable via GET/PUT/DELETE — the router +// treats "/" as a path-segment boundary) becoming too strict for handles the +// UI actually generates. +func TestSecretService_Create_ValidHandle_Accepted(t *testing.T) { + cases := []string{"openai-key", "a", "a1-b2-c3", strings.Repeat("a", 40)} + for _, handle := range cases { + t.Run(handle, func(t *testing.T) { + repo := newMockRepo() + svc := NewSecretService(repo, &mockVault{}, newTestIdentityService()) + + if _, err := svc.Create("org1", "alice", &dto.CreateSecretRequest{ + Handle: handle, DisplayName: "Test", Value: "v", + }); err != nil { + t.Fatalf("unexpected error for valid handle %q: %v", handle, err) + } + }) + } +} + // ---- List tests ------------------------------------------------------------- func TestSecretService_List_ReturnsPagination(t *testing.T) { @@ -347,6 +405,51 @@ func TestSecretService_Get_ReturnsSecret(t *testing.T) { } } +// ---- GetReferences tests ----------------------------------------------------- + +func TestSecretService_GetReferences_NotFound(t *testing.T) { + repo := newMockRepo() + svc := NewSecretService(repo, &mockVault{}, newTestIdentityService()) + + _, err := svc.GetReferences("org1", "missing") + if !apperror.SecretNotFound.Is(err) { + t.Errorf("expected ErrSecretNotFound, got %v", err) + } +} + +func TestSecretService_GetReferences_ReturnsReferences(t *testing.T) { + repo := newMockRepo() + repo.secrets["s1"] = &model.Secret{Handle: "s1", Status: model.SecretStatusActive} + repo.findRefsFn = func(orgID, handle string) ([]model.SecretReference, error) { + return []model.SecretReference{ + {Handle: "my-api", Name: "My API", Type: "RestApi"}, + }, nil + } + + svc := NewSecretService(repo, &mockVault{}, newTestIdentityService()) + refs, err := svc.GetReferences("org1", "s1") + if err != nil { + t.Fatalf("GetReferences: %v", err) + } + if len(refs) != 1 || refs[0].Handle != "my-api" || refs[0].Type != "RestApi" { + t.Errorf("refs = %+v, want a single RestApi reference for my-api", refs) + } +} + +func TestSecretService_GetReferences_NoUsages_ReturnsEmpty(t *testing.T) { + repo := newMockRepo() + repo.secrets["s1"] = &model.Secret{Handle: "s1", Status: model.SecretStatusActive} + + svc := NewSecretService(repo, &mockVault{}, newTestIdentityService()) + refs, err := svc.GetReferences("org1", "s1") + if err != nil { + t.Fatalf("GetReferences: %v", err) + } + if len(refs) != 0 { + t.Errorf("refs = %+v, want empty", refs) + } +} + // ---- Update tests ----------------------------------------------------------- func TestSecretService_Update_NotFound(t *testing.T) { @@ -376,6 +479,38 @@ func TestSecretService_Update_EncryptsNewValue(t *testing.T) { } } +func TestSecretService_Update_NoValue_UpdatesMetadataOnly_DoesNotReencryptOrReactivate(t *testing.T) { + repo := newMockRepo() + repo.secrets["upd"] = &model.Secret{ + Handle: "upd", + DisplayName: "Old Name", + Ciphertext: []byte("cipher:original"), + Hash: "hmac-sha256:original", + Status: model.SecretStatusDeprecated, + } + svc := NewSecretService(repo, &mockVault{}, newTestIdentityService()) + + _, err := svc.Update("org1", "upd", "bob", &dto.UpdateSecretRequest{DisplayName: "New Name"}) + if err != nil { + t.Fatalf("Update: %v", err) + } + + got := repo.secrets["upd"] + if got.DisplayName != "New Name" { + t.Errorf("DisplayName = %q, want %q", got.DisplayName, "New Name") + } + if string(got.Ciphertext) != "cipher:original" { + t.Errorf("Ciphertext changed on a metadata-only update: got %q", got.Ciphertext) + } + if got.Hash != "hmac-sha256:original" { + t.Errorf("Hash changed on a metadata-only update: got %q", got.Hash) + } + if got.Status != model.SecretStatusDeprecated { + t.Errorf("Status = %q, want unchanged %q — a metadata-only update must not reactivate a deprecated secret", + got.Status, model.SecretStatusDeprecated) + } +} + // ---- Delete tests ----------------------------------------------------------- func TestSecretService_Delete_BlockedWhenInUse(t *testing.T) { @@ -427,8 +562,8 @@ func TestSecretService_Delete_SucceedsWhenNotInUse(t *testing.T) { if err := svc.Delete("org1", "unused", "alice"); err != nil { t.Errorf("Delete: %v", err) } - if repo.secrets["unused"].Status != model.SecretStatusDeprecated { - t.Error("expected status to be DEPRECATED after deletion") + if _, ok := repo.secrets["unused"]; ok { + t.Error("expected secret to be permanently removed after deletion") } } @@ -509,9 +644,9 @@ func TestSecretService_ValidateSecretRefs_JSONEscapedForm_Missing(t *testing.T) func TestSecretService_ValidateSecretRefs_DeduplicatesHandles(t *testing.T) { callCount := 0 repo := newMockRepo() - repo.existsFn = func(orgID, handle string) (bool, error) { + repo.getByHandleFn = func(orgID, handle string) (*model.Secret, error) { callCount++ - return true, nil + return &model.Secret{Handle: handle, Status: model.SecretStatusActive}, nil } svc := NewSecretService(repo, &mockVault{}, newTestIdentityService()) @@ -520,7 +655,46 @@ func TestSecretService_ValidateSecretRefs_DeduplicatesHandles(t *testing.T) { t.Errorf("unexpected error: %v", err) } if callCount != 1 { - t.Errorf("Exists called %d times, want 1 (should deduplicate)", callCount) + t.Errorf("GetByHandle called %d times, want 1 (should deduplicate)", callCount) + } +} + +// TestSecretService_ValidateSecretRefs_DeprecatedHandle_DistinctMessage guards +// against a deprecated (existing, retired) secret being reported the same way +// as a genuinely missing one — see §5.6: a deprecated secret still exists and +// cannot be referenced by new/updated resources, which is a different failure +// mode than a typo'd or never-created handle. +func TestSecretService_ValidateSecretRefs_DeprecatedHandle_DistinctMessage(t *testing.T) { + repo := newMockRepo() + repo.secrets["retired-key"] = &model.Secret{Handle: "retired-key", Status: model.SecretStatusDeprecated} + + svc := NewSecretService(repo, &mockVault{}, newTestIdentityService()) + err := svc.ValidateSecretRefs("org1", `{{ secret "retired-key" }}`) + if err == nil { + t.Fatal("expected error for a deprecated secret reference") + } + if !apperror.ValidationFailed.Is(err) { + t.Errorf("expected VALIDATION_FAILED, got: %v", err) + } + if strings.Contains(err.Error(), "do not exist") { + t.Errorf("a deprecated (existing) secret must not be reported as nonexistent: %v", err) + } + if !strings.Contains(err.Error(), "deprecated") || !strings.Contains(err.Error(), "retired-key") { + t.Errorf("expected message to call out the deprecated handle by name, got: %v", err) + } +} + +func TestSecretService_ValidateSecretRefs_MissingAndDeprecated_BothReported(t *testing.T) { + repo := newMockRepo() + repo.secrets["retired-key"] = &model.Secret{Handle: "retired-key", Status: model.SecretStatusDeprecated} + + svc := NewSecretService(repo, &mockVault{}, newTestIdentityService()) + err := svc.ValidateSecretRefs("org1", `{{ secret "retired-key" }} {{ secret "ghost-key" }}`) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "retired-key") || !strings.Contains(err.Error(), "ghost-key") { + t.Errorf("expected both handles named in the message, got: %v", err) } } diff --git a/platform-api/internal/utils/api.go b/platform-api/internal/utils/api.go index 3c14ce1982..988f833b05 100644 --- a/platform-api/internal/utils/api.go +++ b/platform-api/internal/utils/api.go @@ -498,6 +498,9 @@ func (u *APIUtil) BuildAPIDeploymentYAML(apiModel *model.API) (*dto.APIDeploymen if apiModel.Configuration.Upstream.Main.Ref != "" { upstreamYAML.Main.Ref = apiModel.Configuration.Upstream.Main.Ref } + if apiModel.Configuration.Upstream.Main.Auth != nil { + upstreamYAML.Main.Auth = apiModel.Configuration.Upstream.Main.Auth + } } if apiModel.Configuration.Upstream.Sandbox != nil { upstreamYAML.Sandbox = &dto.UpstreamTarget{} @@ -507,6 +510,9 @@ func (u *APIUtil) BuildAPIDeploymentYAML(apiModel *model.API) (*dto.APIDeploymen if apiModel.Configuration.Upstream.Sandbox.Ref != "" { upstreamYAML.Sandbox.Ref = apiModel.Configuration.Upstream.Sandbox.Ref } + if apiModel.Configuration.Upstream.Sandbox.Auth != nil { + upstreamYAML.Sandbox.Auth = apiModel.Configuration.Upstream.Sandbox.Auth + } } } diff --git a/platform-api/internal/utils/api_test.go b/platform-api/internal/utils/api_test.go index 823e1990ac..0ca9cda0b0 100644 --- a/platform-api/internal/utils/api_test.go +++ b/platform-api/internal/utils/api_test.go @@ -19,6 +19,7 @@ package utils import ( "reflect" + "strings" "testing" "gopkg.in/yaml.v3" @@ -654,6 +655,81 @@ func TestBuildAPIDeploymentYAML(t *testing.T) { } } +// TestBuildAPIDeploymentYAML_IncludesUpstreamAuth guards against upstream.main/sandbox.auth +// being silently dropped from the deployment YAML sent to the gateway-controller. A REST +// API's auth (including a {{ secret "handle" }} placeholder) is stored and redacted/validated +// correctly end-to-end, but until this test was added nothing verified it actually reached the +// deployment YAML the gateway-controller resolves placeholders from and renders into Envoy +// config — the field was simply missing from dto.UpstreamTarget. +func TestBuildAPIDeploymentYAML_IncludesUpstreamAuth(t *testing.T) { + util := &APIUtil{} + + ctx := "/test" + apiModel := &model.API{ + Name: "Test API", + Handle: "test-api-handle", + Version: "v1.0", + Kind: constants.RestApi, + Configuration: model.RestAPIConfig{ + Context: &ctx, + Upstream: model.UpstreamConfig{ + Main: &model.UpstreamEndpoint{ + URL: "http://backend:8080", + Auth: &model.UpstreamAuth{ + Type: "api-key", + Header: "Authorization", + Value: `{{ secret "main-handle" }}`, + }, + }, + Sandbox: &model.UpstreamEndpoint{ + URL: "http://sandbox-backend:8080", + Auth: &model.UpstreamAuth{ + Type: "api-key", + Header: "Authorization", + Value: `{{ secret "sandbox-handle" }}`, + }, + }, + }, + }, + ProjectID: "proj-123", + } + + deploymentStruct, err := util.BuildAPIDeploymentYAML(apiModel) + if err != nil { + t.Fatalf("BuildAPIDeploymentYAML() error = %v", err) + } + + main := deploymentStruct.Spec.Upstream.Main + if main == nil || main.Auth == nil { + t.Fatal("expected upstream.main.auth to be set") + } + if main.Auth.Type != "api-key" || main.Auth.Header != "Authorization" || main.Auth.Value != `{{ secret "main-handle" }}` { + t.Errorf("Main.Auth = %+v", main.Auth) + } + + sandbox := deploymentStruct.Spec.Upstream.Sandbox + if sandbox == nil || sandbox.Auth == nil { + t.Fatal("expected upstream.sandbox.auth to be set") + } + if sandbox.Auth.Type != "api-key" || sandbox.Auth.Header != "Authorization" || sandbox.Auth.Value != `{{ secret "sandbox-handle" }}` { + t.Errorf("Sandbox.Auth = %+v", sandbox.Auth) + } + + // The whole point: the raw secret placeholder must survive into the marshalled YAML + // text, since the gateway-controller's syncSecretRefsFromYAML finds it via a regex + // over this exact byte stream (see gateway-controller/pkg/constants.SecretPlaceholderRe). + yamlBytes, err := yaml.Marshal(deploymentStruct) + if err != nil { + t.Fatalf("failed to marshal struct: %v", err) + } + yamlText := string(yamlBytes) + for _, want := range []string{`{{ secret "main-handle" }}`, `{{ secret "sandbox-handle" }}`} { + if !strings.Contains(yamlText, want) { + t.Errorf("deployment YAML missing placeholder %q; got:\n%s", want, yamlText) + } + } +} + // TestUpstreamConfigModelToAPI_RedactsAuthValue proves ModelToRESTAPI never // exposes the real upstream auth credential in a read response — matching the // LLM Provider/Proxy and MCP Proxy mappers' redaction behaviour. diff --git a/platform-api/resources/openapi.yaml b/platform-api/resources/openapi.yaml index 46f76544e1..3d714bd1af 100644 --- a/platform-api/resources/openapi.yaml +++ b/platform-api/resources/openapi.yaml @@ -4716,6 +4716,42 @@ paths: '500': $ref: '#/components/responses/InternalServerError' + /secrets/{secretId}/usages: + get: + summary: List a secret's usages + description: | + Returns the resources (REST APIs, LLM providers, LLM proxies, MCP proxies) that + currently reference this secret via a `{{ secret "handle" }}` placeholder. + operationId: getSecretUsages + security: + - OAuth2Security: + - ap:secret:read + - ap:secret:manage + tags: + - Secrets + parameters: + - name: secretId + in: path + required: true + description: The secret handle + schema: + type: string + responses: + '200': + description: Resources that reference this secret. + content: + application/json: + schema: + $ref: '#/components/schemas/SecretUsagesResponse' + '401': + $ref: '#/components/responses/Unauthorized' + '404': + $ref: '#/components/responses/NotFound' + '503': + $ref: '#/components/responses/ServiceUnavailable' + '500': + $ref: '#/components/responses/InternalServerError' + components: headers: Location: @@ -8614,6 +8650,31 @@ components: pagination: $ref: '#/components/schemas/Pagination' + SecretReference: + type: object + description: Identifies a resource that references a secret. + properties: + type: + type: string + description: The referencing artifact's kind. + enum: [RestApi, LlmProvider, LlmProxy, Mcp] + handle: + type: string + description: The referencing resource's handle. + name: + type: string + description: The referencing resource's display name. + + SecretUsagesResponse: + type: object + required: + - references + properties: + references: + type: array + items: + $ref: '#/components/schemas/SecretReference' + GatewayTokenListResponse: type: object required: diff --git a/portals/ai-workspace/cypress/e2e/001-providers/002-provider-secret-management.cy.js b/portals/ai-workspace/cypress/e2e/001-providers/002-provider-secret-management.cy.js index 441ff8c384..e603aa1dbc 100644 --- a/portals/ai-workspace/cypress/e2e/001-providers/002-provider-secret-management.cy.js +++ b/portals/ai-workspace/cypress/e2e/001-providers/002-provider-secret-management.cy.js @@ -42,14 +42,6 @@ import { appPathPattern } from '../../support/appPath'; // Shared helpers // --------------------------------------------------------------------------- -function toSlug(value) { - return value - .toLowerCase() - .trim() - .replace(/[^a-z0-9]+/g, '-') - .replace(/^-+|-+$/g, ''); -} - function navigateToAddProvider() { cy.get('[data-cyid="nav-service-provider"]', { timeout: 30000 }) .should('be.visible') @@ -81,6 +73,12 @@ describe('AI Workspace — LLM provider secret management (create flow)', () => const suffix = Date.now().toString().slice(-8); const orgHandle = Cypress.env('ORG_HANDLE'); const providerName = `E2E Secret Provider ${suffix}`; + // Dedicated, compact base for pre-created placeholder secret handles — kept + // independent of providerName's length so it always stays well within the + // platform-api's 40-character secret handle limit regardless of the display + // name above (a slug of providerName alone can already exceed 40 chars once + // a suffix like "-tc63-api-key" is appended). + const secretHandleBase = `e2e-${suffix}`; let authToken = ''; let organizationId = ''; @@ -172,7 +170,7 @@ describe('AI Workspace — LLM provider secret management (create flow)', () => // TC-58: Re-save provider already using a placeholder → POST /secrets NOT called // ------------------------------------------------------------------------- it('TC-58: does not create a duplicate secret when the auth value is already a placeholder', () => { - const existingHandle = `${toSlug(providerName)}-api-key`; + const existingHandle = `${secretHandleBase}-api-key`; // Pre-create the secret via API so there's a real secret backing the placeholder. cy.request({ method: 'POST', @@ -267,7 +265,7 @@ describe('AI Workspace — LLM provider secret management (create flow)', () => // in the org (encryption proof, not scoped to a single just-created secret). // ------------------------------------------------------------------------- it('TC-63: GET /secrets never exposes plaintext value for any secret in the org', () => { - const handle = `${toSlug(providerName)}-tc63-api-key`; + const handle = `${secretHandleBase}-tc63-api-key`; cy.request({ method: 'POST', @@ -318,6 +316,8 @@ describe('AI Workspace — LLM provider secret management (update flow)', () => const suffix = Date.now().toString().slice(-8); const orgHandle = Cypress.env('ORG_HANDLE'); const providerName = `E2E Secret Update Provider ${suffix}`; + // See secretHandleBase in the create-flow describe block above — same reasoning. + const secretHandleBase = `e2e-${suffix}`; const INITIAL_KEY = `sk-update-initial-${suffix}`; const UPDATED_KEY = `sk-update-new-${suffix}`; @@ -450,7 +450,7 @@ describe('AI Workspace — LLM provider secret management (update flow)', () => // TC-61: Edit credential by typing an explicit placeholder → POST /secrets NOT called // ------------------------------------------------------------------------- it('TC-61: typing a placeholder value skips secret creation and sends the placeholder directly', () => { - const explicitHandle = `${toSlug(providerName)}-api-key`; + const explicitHandle = `${secretHandleBase}-api-key`; // Pre-create the secret so the platform-api accepts the placeholder in the PUT. // cy.request() bypasses cy.intercept(), so this won't affect secretCallCount below. diff --git a/portals/ai-workspace/cypress/e2e/001-providers/003-llm-proxy-secret-management.cy.js b/portals/ai-workspace/cypress/e2e/001-providers/003-llm-proxy-secret-management.cy.js index 28c01a0f4d..712bf0d60a 100644 --- a/portals/ai-workspace/cypress/e2e/001-providers/003-llm-proxy-secret-management.cy.js +++ b/portals/ai-workspace/cypress/e2e/001-providers/003-llm-proxy-secret-management.cy.js @@ -45,14 +45,6 @@ // Shared helpers // --------------------------------------------------------------------------- -function toSlug(value) { - return value - .toLowerCase() - .trim() - .replace(/[^a-z0-9]+/g, '-') - .replace(/^-+|-+$/g, ''); -} - function loginAndFetchAuthContext(setAuthToken, setOrganizationId) { cy.login(); cy.request({ @@ -128,6 +120,12 @@ describe('AI Workspace — LLM proxy secret management (create flow)', () => { const projectName = `E2E Proxy Secret Project ${suffix}`; const providerName = `E2E Proxy Secret Provider ${suffix}`; const proxyName = `E2E Proxy Secret Proxy ${suffix}`; + // Dedicated, compact base for pre-created placeholder secret handles — kept + // independent of proxyName's length so it always stays well within the + // platform-api's 40-character secret handle limit regardless of the display + // name above (a slug of proxyName alone can already exceed 40 chars once a + // suffix like "-provider-api-key" is appended). + const secretHandleBase = `e2e-${suffix}`; let authToken = ''; let organizationId = ''; @@ -222,7 +220,7 @@ describe('AI Workspace — LLM proxy secret management (create flow)', () => { // TC-2: Re-save proxy already using a placeholder → POST /secrets NOT called // ------------------------------------------------------------------------- it('TC-2: does not create a duplicate secret when the API key is already a placeholder', () => { - const existingHandle = `${toSlug(proxyName)}-provider-api-key`; + const existingHandle = `${secretHandleBase}-provider-api-key`; cy.request({ method: 'POST', url: '/proxy/api/v0.9/secrets', @@ -310,6 +308,8 @@ describe('AI Workspace — LLM proxy secret management (update flow)', () => { const projectName = `E2E Proxy Update Project ${suffix}`; const providerName = `E2E Proxy Update Provider ${suffix}`; const proxyName = `E2E Proxy Update Proxy ${suffix}`; + // See secretHandleBase in the create-flow describe block above — same reasoning. + const secretHandleBase = `e2e-${suffix}`; const INITIAL_KEY = `sk-proxy-update-initial-${suffix}`; let authToken = ''; @@ -444,7 +444,7 @@ describe('AI Workspace — LLM proxy secret management (update flow)', () => { // TC-5: Edit API key by typing an explicit placeholder → POST /secrets NOT called // ------------------------------------------------------------------------- it('TC-5: typing a placeholder value skips secret creation and sends the placeholder directly', () => { - const explicitHandle = `${toSlug(proxyName)}-provider-api-key`; + const explicitHandle = `${secretHandleBase}-provider-api-key`; cy.request({ method: 'POST', diff --git a/portals/ai-workspace/src/App.tsx b/portals/ai-workspace/src/App.tsx index 048ad54ca0..454af7600f 100644 --- a/portals/ai-workspace/src/App.tsx +++ b/portals/ai-workspace/src/App.tsx @@ -20,12 +20,13 @@ import { Routes, Route, Navigate, + Outlet, useLocation, useNavigate, } from 'react-router-dom'; import AutoLoginPage from './pages/login/AutoLoginPage'; import AppShellMain from './pages/appShell/appShellMain'; -import { AppShellProvider } from './contexts/AppShellContext'; +import { AppShellProvider, useAppShell } from './contexts/AppShellContext'; import { RoleProvider } from './contexts/RoleContext'; import PageErrorBoundary from './Components/common/PageErrorBoundary'; import { AIWorkspaceSnackbarProvider } from './contexts/AIWorkspaceSnackbarContext'; @@ -71,8 +72,12 @@ import CustomPoliciesList from './pages/appShell/appShellPages/gateways/CustomPo import OrgRegisterPage from './pages/register/OrgRegisterPage'; import Insights from './pages/appShell/appShellPages/insights/Main'; import QuickStart from './pages/appShell/appShellPages/quickStart/Main'; -import Settings, { SettingsIndexRedirect } from './pages/appShell/appShellPages/settings/Main'; +import Settings, { SettingsIndexRedirect, resolveSettingsFallbackPath } from './pages/appShell/appShellPages/settings/Main'; import ProviderTemplatesList from './pages/appShell/appShellPages/providerTemplate/ProviderTemplatesList'; +import SecretsList from './pages/appShell/appShellPages/secret/SecretsList'; +import CreateSecret from './pages/appShell/appShellPages/secret/CreateSecret'; +import SecretOverview from './pages/appShell/appShellPages/secret/SecretOverview'; +import RotateSecret from './pages/appShell/appShellPages/secret/RotateSecret'; import ExternalServersList from './pages/appShell/appShellPages/externalServers/ExternalServersList'; import ExternalServersNew from './pages/appShell/appShellPages/externalServers/ExternalServersNew'; import ExternalServersOverview from './pages/appShell/appShellPages/externalServers/ExternalServersOverview'; @@ -83,6 +88,8 @@ import { LLMProvidersProvider } from './contexts/llmProvider'; import React, { useRef, useState } from 'react'; import { ChoreoUserProvider } from './contexts/ChoreoUserContext'; import { useAppAuth } from './contexts/AppAuthContext'; +import { SCOPES } from './auth/permissions'; +import { buildOrgPath } from './utils/projectRouting'; import { Box, Button, Stack, Typography } from '@wso2/oxygen-ui'; import OoopsImage from './assets/images/Ooops.svg'; @@ -125,6 +132,34 @@ function ProtectedRoute({ children }: { children: React.ReactNode }) { return <>{children}; } +/** + * Layout-route guard: denies the wrapped route branch (rendered via ) + * unless the caller holds `scope`, redirecting to a settings section they can + * actually access rather than a hardcoded path. Unlike ProtectedRoute + * (authentication only), this enforces per-route authorization — needed so a + * direct navigation to a URL like /settings/secrets can't render the page for a + * caller who lacks the corresponding scope. + */ +function RequireScope({ scope }: { scope: string }) { + const { hasPermission, isLoading } = useAppAuth(); + const { currentOrganization } = useAppShell(); + + // A hard navigation/refresh straight to a guarded URL mounts this before the + // BFF session fetch resolves — hasPermission() reads as false for everything + // on that first render. Wait for it to settle before deciding, otherwise a + // legitimately-scoped caller gets bounced by this one-shot redirect before + // their scopes have even loaded. + if (isLoading) { + return null; + } + + if (!hasPermission(scope)) { + return ; + } + + return ; +} + // The OIDC ?code= callback is now handled server-side by the BFF at // /api/auth/callback (which sets the session cookie and 302s back into the app). // This SPA route only catches stray hits to the legacy /signin path and bounces @@ -590,6 +625,40 @@ export default function App() { } /> + }> + + + + } + /> + + + + } + /> + + + + } + /> + + + + } + /> + }> diff --git a/portals/ai-workspace/src/Components/common/SecretValueField.tsx b/portals/ai-workspace/src/Components/common/SecretValueField.tsx new file mode 100644 index 0000000000..741b3b010d --- /dev/null +++ b/portals/ai-workspace/src/Components/common/SecretValueField.tsx @@ -0,0 +1,197 @@ +/* + * Copyright (c) 2026, WSO2 LLC. (https://www.wso2.com). + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React, { useEffect, useState } from 'react'; +import { Autocomplete, Box, CircularProgress, IconButton, TextField, Typography } from '@wso2/oxygen-ui'; +import { Eye, EyeOff, KeyRound } from '@wso2/oxygen-ui-icons-react'; +import { buildSecretPlaceholder, extractSecretHandle, listSecrets, type SecretMetadata } from '../../apis/secretApis'; + +type SecretOptionOrText = SecretMetadata | string; + +export interface SecretValueFieldProps { + value: string; + onChange: (value: string) => void; + onFocus?: React.FocusEventHandler; + onBlur?: React.FocusEventHandler; + disabled?: boolean; + placeholder?: string; + required?: boolean; + error?: boolean; + helperText?: React.ReactNode; + size?: 'small' | 'medium'; + 'data-cyid'?: string; + 'data-testid'?: string; +} + +function getOptionLabel(option: SecretOptionOrText): string { + return typeof option === 'string' ? option : option.displayName; +} + +/** + * Combobox for an upstream credential's Value field — same free-entry-plus-picker + * affordance as GitHub's label picker, applied to secrets: existing secrets are + * offered in a dropdown (selecting one substitutes {{ secret "handle" }}), or the + * user can type a brand-new plaintext value directly (encrypted into a new secret + * by the caller on submit, same as before) or a {{ secret "handle" }} placeholder + * by hand, which is passed through unchanged. + */ +export default function SecretValueField({ + value, + onChange, + onFocus, + onBlur, + disabled, + placeholder, + required, + error, + helperText, + size, + 'data-cyid': dataCyId, + 'data-testid': dataTestId, +}: SecretValueFieldProps): React.JSX.Element { + const [options, setOptions] = useState([]); + const [isLoadingOptions, setIsLoadingOptions] = useState(true); + const [showValue, setShowValue] = useState(false); + const [isInputFocused, setIsInputFocused] = useState(false); + + useEffect(() => { + let isMounted = true; + (async () => { + try { + const response = await listSecrets({ limit: 100 }); + if (!isMounted) return; + setOptions((response.list ?? []).filter((s) => s.status === 'ACTIVE' && s.type === 'GENERIC')); + } catch { + // Best-effort: a dropdown that fails to load still lets the user type a + // value directly — this field never blocks on the existing-secrets list. + } finally { + if (isMounted) setIsLoadingOptions(false); + } + })(); + return () => { + isMounted = false; + }; + }, []); + + const selectedHandle = extractSecretHandle(value); + const selectedOption = selectedHandle ? (options.find((s) => s.id === selectedHandle) ?? null) : null; + const autocompleteValue: SecretOptionOrText = selectedOption ?? value; + + // Pre-filtered rather than left to MUI's own filterOptions, so `open` below can + // stay false when nothing matches — an open-but-empty popup would otherwise sit + // on top of (and swallow clicks meant for) whatever follows this field on the page. + const query = value.trim().toLowerCase(); + const visibleOptions = selectedOption + ? [] + : query + ? options.filter((opt) => `${opt.displayName} ${opt.id}`.toLowerCase().includes(query)) + : options; + const isOpen = isInputFocused && visibleOptions.length > 0; + + return ( + + freeSolo + fullWidth + size={size} + disabled={disabled} + open={isOpen} + options={visibleOptions} + filterOptions={(opts) => opts} + loading={isLoadingOptions} + value={autocompleteValue} + getOptionLabel={getOptionLabel} + isOptionEqualToValue={(option, val) => typeof val !== 'string' && typeof option !== 'string' && option.id === val.id} + onChange={(_event, newValue) => { + if (newValue === null) { + onChange(''); + } else if (typeof newValue === 'string') { + onChange(newValue); + } else { + onChange(buildSecretPlaceholder(newValue.id)); + } + }} + onInputChange={(_event, newInputValue, reason) => { + // 'reset' fires on selection/programmatic value changes — already handled by onChange. + if (reason === 'input') { + onChange(newInputValue); + } + }} + renderOption={(props, option) => { + if (typeof option === 'string') return null; + return ( + + + + + {option.displayName} + + + {option.id} + + + + ); + }} + renderInput={(params) => ( + ) => { + params.inputProps.onFocus?.(event); + setIsInputFocused(true); + onFocus?.(event); + }, + onBlur: (event: React.FocusEvent) => { + params.inputProps.onBlur?.(event); + setIsInputFocused(false); + onBlur?.(event); + }, + }, + input: { + ...params.InputProps, + endAdornment: ( + <> + {isLoadingOptions ? : null} + setShowValue((prev) => !prev)} + aria-label={showValue ? 'Hide value' : 'Show value'} + tabIndex={-1} + > + {showValue ? : } + + {params.InputProps.endAdornment} + + ), + }, + }} + /> + )} + /> + ); +} diff --git a/portals/ai-workspace/src/apis/secretApis.ts b/portals/ai-workspace/src/apis/secretApis.ts index 7995a081ba..454e038a16 100644 --- a/portals/ai-workspace/src/apis/secretApis.ts +++ b/portals/ai-workspace/src/apis/secretApis.ts @@ -47,6 +47,7 @@ export interface SecretMetadata { type: SecretType; provider: string; status: SecretStatus; + createdBy?: string; createdAt: string; updatedAt: string; } @@ -120,6 +121,15 @@ export async function getSecret(handle: string): Promise { return get(`/secrets/${handle}`); } +/** + * Returns the resources (REST APIs, LLM providers, LLM proxies, MCP proxies) that + * currently reference this secret via a {{ secret "handle" }} placeholder. + */ +export async function getSecretUsages(handle: string): Promise { + const response = await get<{ references: SecretReference[] }>(`/secrets/${handle}/usages`); + return response.references ?? []; +} + /** * Rotates a secret's value. All {{ secret "handle" }} references remain valid — * the gateway picks up the new value on its next sync cycle. @@ -131,7 +141,7 @@ export async function updateSecret( ): Promise { const form = new FormData(); form.append('value', request.value); - if (request.name) form.append('name', request.name); + if (request.name) form.append('displayName', request.name); if (request.description) form.append('description', request.description); return putForm(`/secrets/${handle}`, form); } diff --git a/portals/ai-workspace/src/clients/choreoApiClient.ts b/portals/ai-workspace/src/clients/choreoApiClient.ts index a6d63667a8..f755387024 100644 --- a/portals/ai-workspace/src/clients/choreoApiClient.ts +++ b/portals/ai-workspace/src/clients/choreoApiClient.ts @@ -75,6 +75,9 @@ const buildUrl = ( const full = path.startsWith('http') ? path : `${baseUrl}${path}`; if (!params || Object.keys(params).length === 0) return full; + // `full` may be a relative path (the default PLATFORM_API_BASE_URL is a same-origin + // BFF proxy path, not an absolute URL) — the URL constructor requires a base in that + // case, or it throws "Failed to construct 'URL': Invalid URL". const url = new URL(full, window.location.origin); for (const [k, v] of Object.entries(params)) { if (v !== undefined && v !== null) url.searchParams.append(k, String(v)); diff --git a/portals/ai-workspace/src/hooks/useIsMounted.ts b/portals/ai-workspace/src/hooks/useIsMounted.ts new file mode 100644 index 0000000000..9b6c650870 --- /dev/null +++ b/portals/ai-workspace/src/hooks/useIsMounted.ts @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2026, WSO2 LLC. (https://www.wso2.com). + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { useCallback, useEffect, useRef } from 'react'; + +/** + * Returns a stable function that reports whether the calling component is + * still mounted. Use it to guard state updates after an `await` — e.g. an API + * call whose response resolves after the user has already navigated away — + * which would otherwise trigger a "set state on an unmounted component" + * warning (and, for update-then-navigate flows, a redundant navigate() call). + */ +export default function useIsMounted(): () => boolean { + const mountedRef = useRef(true); + + useEffect(() => { + mountedRef.current = true; + return () => { + mountedRef.current = false; + }; + }, []); + + return useCallback(() => mountedRef.current, []); +} diff --git a/portals/ai-workspace/src/pages/appShell/appShellPages/externalServers/ExternalServersNew.tsx b/portals/ai-workspace/src/pages/appShell/appShellPages/externalServers/ExternalServersNew.tsx index 66bb22a2bc..fe5e468990 100644 --- a/portals/ai-workspace/src/pages/appShell/appShellPages/externalServers/ExternalServersNew.tsx +++ b/portals/ai-workspace/src/pages/appShell/appShellPages/externalServers/ExternalServersNew.tsx @@ -29,7 +29,6 @@ import { FormControl, FormLabel, Grid, - IconButton, InputAdornment, PageContent, PageTitle, @@ -41,8 +40,6 @@ import { import { ChevronDown, ChevronLeft, - Eye, - EyeOff, HelpCircle, } from '@wso2/oxygen-ui-icons-react'; import { FormattedMessage, useIntl } from 'react-intl'; @@ -67,6 +64,7 @@ import ExternalServersCreateForm from './ExternalServersCreateForm'; import ExternalServersValidationDetails from './ExternalServersValidationDetails'; import type { EndpointValidationResponse } from './externalServersValidationTypes'; import { getErrorMessage, getFieldErrors } from '../../../../utils/apiError'; +import SecretValueField from '../../../../Components/common/SecretValueField'; // Backend field names (from CreateMCPServerRequest) mapped onto this form's state keys. // "displayName" maps to the server name field; the rest match one-to-one. @@ -144,7 +142,6 @@ export default function ExternalServersNew(): JSX.Element { const [isCreateStep, setIsCreateStep] = useState(false); const [authHeaderName, setAuthHeaderName] = useState(''); const [authHeaderValue, setAuthHeaderValue] = useState(''); - const [showAuthHeaderValue, setShowAuthHeaderValue] = useState(false); const [serverName, setServerName] = useState(''); const [serverVersion, setServerVersion] = useState(''); const [serverDescription, setServerDescription] = useState(''); @@ -538,44 +535,13 @@ export default function ExternalServersNew(): JSX.Element { defaultMessage="Value" /> - - setAuthHeaderValue(event.target.value) - } - slotProps={{ - input: { - endAdornment: ( - - - setShowAuthHeaderValue( - (prev) => !prev - ) - } - aria-label={ - showAuthHeaderValue - ? 'Hide header value' - : 'Show header value' - } - > - {showAuthHeaderValue ? ( - - ) : ( - - )} - - - ), - }, - }} + onChange={setAuthHeaderValue} /> diff --git a/portals/ai-workspace/src/pages/appShell/appShellPages/externalServers/ExternalServersOverview.tsx b/portals/ai-workspace/src/pages/appShell/appShellPages/externalServers/ExternalServersOverview.tsx index 5d0f6d91e2..0928686b43 100644 --- a/portals/ai-workspace/src/pages/appShell/appShellPages/externalServers/ExternalServersOverview.tsx +++ b/portals/ai-workspace/src/pages/appShell/appShellPages/externalServers/ExternalServersOverview.tsx @@ -58,8 +58,6 @@ import { Clock, Copy, Edit, - Eye, - EyeOff, Trash2, } from '@wso2/oxygen-ui-icons-react'; import { FormattedMessage } from 'react-intl'; @@ -110,6 +108,7 @@ import { } from '../../../../utils/artifactDeletion'; import { useAppAuth } from '../../../../contexts/AppAuthContext'; import { NO_PERMISSION_TOOLTIP, SCOPES } from '../../../../auth/permissions'; +import SecretValueField from '../../../../Components/common/SecretValueField'; function getInitials(name: string): string { const words = name.trim().split(/\s+/); @@ -261,7 +260,6 @@ export default function ExternalServersOverview(): JSX.Element { const [endpointUrl, setEndpointUrl] = useState(''); const [authHeaderName, setAuthHeaderName] = useState(''); const [authHeaderValue, setAuthHeaderValue] = useState(''); - const [showAuthHeaderValue, setShowAuthHeaderValue] = useState(false); const [isCredentialMasked, setIsCredentialMasked] = useState(false); const [hasCredentialChanged, setHasCredentialChanged] = useState(false); const [isRefetching, setIsRefetching] = useState(false); @@ -1306,9 +1304,7 @@ export default function ExternalServersOverview(): JSX.Element { Value - { @@ -1318,38 +1314,11 @@ export default function ExternalServersOverview(): JSX.Element { setHasCredentialChanged(false); } }} - onChange={(event) => { - setAuthHeaderValue(event.target.value); + onChange={(nextValue) => { + setAuthHeaderValue(nextValue); setHasCredentialChanged(true); }} - slotProps={{ - htmlInput: { - 'data-testid': 'backend-connection-auth-value', - }, - input: { - endAdornment: ( - - - setShowAuthHeaderValue((prev) => !prev) - } - aria-label={ - showAuthHeaderValue - ? 'Hide header value' - : 'Show header value' - } - > - {showAuthHeaderValue ? ( - - ) : ( - - )} - - - ), - }, - }} + data-testid="backend-connection-auth-value" /> diff --git a/portals/ai-workspace/src/pages/appShell/appShellPages/proxies/LLMProxyNew.tsx b/portals/ai-workspace/src/pages/appShell/appShellPages/proxies/LLMProxyNew.tsx index d04cff06a4..54eefcd639 100644 --- a/portals/ai-workspace/src/pages/appShell/appShellPages/proxies/LLMProxyNew.tsx +++ b/portals/ai-workspace/src/pages/appShell/appShellPages/proxies/LLMProxyNew.tsx @@ -72,6 +72,7 @@ import { logger } from '../../../../utils/logger'; import { getErrorMessage, getFieldErrors } from '../../../../utils/apiError'; import { useAppAuth } from '../../../../contexts/AppAuthContext'; import { NO_PERMISSION_TOOLTIP, SCOPES } from '../../../../auth/permissions'; +import SecretValueField from '../../../../Components/common/SecretValueField'; type FormState = { name: string; @@ -806,18 +807,14 @@ function LLMProxyNewContent({ gap: 1, }} > - - setManualApiKeyValue(event.target.value) - } + onChange={(nextValue) => setManualApiKeyValue(nextValue)} data-cyid="proxy-api-key-input" /> {isManualKeyReady && ( diff --git a/portals/ai-workspace/src/pages/appShell/appShellPages/proxies/LLMProxyProviderTab.tsx b/portals/ai-workspace/src/pages/appShell/appShellPages/proxies/LLMProxyProviderTab.tsx index b121a3ee92..054b11d39b 100644 --- a/portals/ai-workspace/src/pages/appShell/appShellPages/proxies/LLMProxyProviderTab.tsx +++ b/portals/ai-workspace/src/pages/appShell/appShellPages/proxies/LLMProxyProviderTab.tsx @@ -24,7 +24,6 @@ import { MenuItem, Select, Stack, - TextField, Typography, } from '@wso2/oxygen-ui'; import { useLLMProviders } from '../../../../contexts/llmProvider'; @@ -36,6 +35,7 @@ import { logger } from '../../../../utils/logger'; import useAIWorkspaceSnackbar from '../../../../hooks/aiWorkspaceSnackbar'; import type { LLMProvider, ProxyApiKeySecurity } from '../../../../utils/types'; import { FormattedMessage } from 'react-intl'; +import SecretValueField from '../../../../Components/common/SecretValueField'; /** * Provider tab – lets the user select / change the LLM Service Provider @@ -269,13 +269,11 @@ export default function LLMProxyProviderTab() { defaultMessage={'API Key'} /> - handleApiKeyChange(e.target.value)} - fullWidth + onChange={handleApiKeyChange} /> diff --git a/portals/ai-workspace/src/pages/appShell/appShellPages/secret/CreateSecret.tsx b/portals/ai-workspace/src/pages/appShell/appShellPages/secret/CreateSecret.tsx new file mode 100644 index 0000000000..02079b25af --- /dev/null +++ b/portals/ai-workspace/src/pages/appShell/appShellPages/secret/CreateSecret.tsx @@ -0,0 +1,283 @@ +/* + * Copyright (c) 2026, WSO2 LLC. (https://www.wso2.com). + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React, { useState } from 'react'; +import { Link as RouterLink, useNavigate } from 'react-router-dom'; +import { + Box, + Button, + Chip, + FormControl, + FormLabel, + Grid, + IconButton, + InputAdornment, + PageContent, + PageTitle, + Stack, + TextField, + Typography, +} from '@wso2/oxygen-ui'; +import { ChevronLeft, Eye, EyeOff } from '@wso2/oxygen-ui-icons-react'; +import { FormattedMessage } from 'react-intl'; +import { createSecret, type SecretType } from '../../../../apis/secretApis'; +import { useAppShell } from '../../../../contexts/AppShellContext'; +import { buildOrgPath } from '../../../../utils/projectRouting'; +import useAIWorkspaceSnackbar from '../../../../hooks/aiWorkspaceSnackbar'; +import useIsMounted from '../../../../hooks/useIsMounted'; +import { getErrorMessage } from '../../../../utils/apiError'; + +const MAX_NAME_LENGTH = 120; +const MAX_DESCRIPTION_LENGTH = 300; +const MAX_HANDLE_LENGTH = 40; // matches the `handle VARCHAR(40)` column enforced server-side +const HANDLE_PATTERN = /^[a-z0-9]+(-[a-z0-9]+)*$/; + +function toHandle(value: string): string { + return value + .toLowerCase() + .trim() + .replace(/[^a-z0-9]+/g, '-') + .replace(/^-+|-+$/g, ''); +} + +export default function CreateSecret(): React.JSX.Element { + const navigate = useNavigate(); + const { currentOrganization } = useAppShell(); + const showSnackbar = useAIWorkspaceSnackbar(); + const isMounted = useIsMounted(); + + const [displayName, setDisplayName] = useState(''); + const [handle, setHandle] = useState(''); + const [handleTouched, setHandleTouched] = useState(false); + const [description, setDescription] = useState(''); + const [value, setValue] = useState(''); + const [valueVisible, setValueVisible] = useState(false); + // Fixed until certificate secrets are implemented — see the Type field below. + const type: SecretType = 'GENERIC'; + const [nameTouched, setNameTouched] = useState(false); + const [valueTouched, setValueTouched] = useState(false); + const [isSubmitting, setIsSubmitting] = useState(false); + + const listPath = buildOrgPath(currentOrganization, '/settings/secrets'); + + const handleNameChange = (nextName: string) => { + setDisplayName(nextName); + if (!handleTouched) setHandle(toHandle(nextName)); + }; + + const isNameValid = displayName.trim().length > 0 && displayName.length <= MAX_NAME_LENGTH; + const isDescriptionValid = description.length <= MAX_DESCRIPTION_LENGTH; + const isHandleValid = HANDLE_PATTERN.test(handle) && handle.length <= MAX_HANDLE_LENGTH; + const isValueValid = value.trim().length > 0; + const isFormValid = isNameValid && isHandleValid && isDescriptionValid && isValueValid; + + const handleSubmit = async (event?: React.FormEvent) => { + if (event) event.preventDefault(); + if (!isFormValid || isSubmitting) return; + + setIsSubmitting(true); + try { + const secret = await createSecret({ + id: handle, + displayName: displayName.trim(), + description: description.trim() || undefined, + value, + type, + }); + if (!isMounted()) return; + showSnackbar('Secret created successfully.', 'success'); + navigate(`${listPath}/${secret.id}`); + } catch (err) { + if (!isMounted()) return; + showSnackbar(getErrorMessage(err, 'Failed to create secret.'), 'error'); + } finally { + if (isMounted()) setIsSubmitting(false); + } + }; + + return ( + + + + + + + + + + + + + + + + + Type + + + + Certificate secrets are coming soon. + + + + + + + + Display Name + handleNameChange(e.target.value)} + onBlur={() => setNameTouched(true)} + placeholder="e.g. WSO2 OpenAI API Key" + error={nameTouched && !isNameValid} + helperText={ + nameTouched && displayName.trim().length === 0 + ? 'Display name is required.' + : displayName.length > MAX_NAME_LENGTH + ? `Must not exceed ${MAX_NAME_LENGTH} characters.` + : '' + } + data-cyid="secret-name-input" + /> + + + + + + Handle + { + setHandleTouched(true); + setHandle(e.target.value.toLowerCase()); + }} + placeholder="wso2-openai-key" + slotProps={{ + input: { style: { fontFamily: 'monospace' } }, + htmlInput: { maxLength: MAX_HANDLE_LENGTH }, + }} + error={handle.length > 0 && !isHandleValid} + helperText={ + handle.length > 0 && !isHandleValid + ? 'Lowercase letters, numbers, and single hyphens only.' + : 'Immutable after creation. Referenced as {{ secret "' + (handle || 'handle') + '" }}.' + } + data-cyid="secret-handle-input" + /> + + + + + + Description + setDescription(e.target.value)} + placeholder="e.g. Gemini 1.5 Pro — production project" + error={!isDescriptionValid} + helperText={ + !isDescriptionValid + ? `Must not exceed ${MAX_DESCRIPTION_LENGTH} characters (${description.length}/${MAX_DESCRIPTION_LENGTH}).` + : '' + } + /> + + + + {/* TODO(certificate-secrets): once implemented, branch on `type` here — + a file-upload control for CERTIFICATE, this text field for GENERIC. */} + + + Value + setValue(e.target.value)} + onBlur={() => setValueTouched(true)} + placeholder="Paste your credential here" + autoComplete="new-password" + error={valueTouched && !isValueValid} + helperText={valueTouched && !isValueValid ? 'A value is required.' : ''} + slotProps={{ + input: { + endAdornment: ( + + setValueVisible((v) => !v)} + aria-label={valueVisible ? 'Hide value' : 'Show value'} + > + {valueVisible ? : } + + + ), + }, + }} + data-cyid="secret-value-input" + /> + + + + + + + + + + + + ); +} diff --git a/portals/ai-workspace/src/pages/appShell/appShellPages/secret/DeleteSecretDialog.tsx b/portals/ai-workspace/src/pages/appShell/appShellPages/secret/DeleteSecretDialog.tsx new file mode 100644 index 0000000000..b021ca445b --- /dev/null +++ b/portals/ai-workspace/src/pages/appShell/appShellPages/secret/DeleteSecretDialog.tsx @@ -0,0 +1,144 @@ +/* + * Copyright (c) 2026, WSO2 LLC. (https://www.wso2.com). + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React, { useEffect, useState } from 'react'; +import { + Box, + Button, + Chip, + Dialog, + DialogActions, + DialogContent, + DialogContentText, + DialogTitle, + Stack, + Typography, +} from '@wso2/oxygen-ui'; +import { deleteSecret, SecretConflictError, type SecretMetadata, type SecretReference } from '../../../../apis/secretApis'; +import useAIWorkspaceSnackbar from '../../../../hooks/aiWorkspaceSnackbar'; +import useIsMounted from '../../../../hooks/useIsMounted'; +import { getErrorMessage } from '../../../../utils/apiError'; + +interface DeleteSecretDialogProps { + secret: SecretMetadata | null; + onClose: () => void; + /** Called after the secret has actually been permanently deleted on the server. */ + onDeleted: (handle: string) => void; +} + +/** + * Deleting a secret permanently removes it on the backend and is blocked with a + * 409 if the secret is still referenced by another resource. This dialog handles + * both outcomes: a plain confirmation, or — once blocked — the list of + * referencing resources returned on the conflict. + */ +export default function DeleteSecretDialog({ secret, onClose, onDeleted }: DeleteSecretDialogProps): React.JSX.Element { + const showSnackbar = useAIWorkspaceSnackbar(); + const isMounted = useIsMounted(); + const [isDeleting, setIsDeleting] = useState(false); + const [conflictRefs, setConflictRefs] = useState(null); + + useEffect(() => { + setConflictRefs(null); + }, [secret?.id]); + + const handleConfirm = async () => { + if (!secret) return; + setIsDeleting(true); + try { + await deleteSecret(secret.id); + if (!isMounted()) return; + showSnackbar(`"${secret.displayName}" was deleted.`, 'success'); + onDeleted(secret.id); + onClose(); + } catch (error) { + if (!isMounted()) return; + if (error instanceof SecretConflictError) { + setConflictRefs(error.conflict.references); + } else { + showSnackbar(getErrorMessage(error, 'Failed to delete secret.'), 'error'); + onClose(); + } + } finally { + if (isMounted()) setIsDeleting(false); + } + }; + + return ( + + {conflictRefs ? ( + <> + Can't delete this secret + + + {secret?.displayName} is still referenced by {conflictRefs.length}{' '} + {conflictRefs.length === 1 ? 'resource' : 'resources'}. Remove the reference from each one below before + deleting it again. + + + {conflictRefs.map((ref) => ( + + + + + {ref.name} + + + + ))} + + + + + + + ) : ( + <> + Delete secret + + + Delete {secret?.displayName}? This permanently deletes the secret and cannot be + undone. Deletion is blocked while any resource still references it. + + + + + + + + )} + + ); +} diff --git a/portals/ai-workspace/src/pages/appShell/appShellPages/secret/RotateSecret.tsx b/portals/ai-workspace/src/pages/appShell/appShellPages/secret/RotateSecret.tsx new file mode 100644 index 0000000000..7acfc0780d --- /dev/null +++ b/portals/ai-workspace/src/pages/appShell/appShellPages/secret/RotateSecret.tsx @@ -0,0 +1,236 @@ +/* + * Copyright (c) 2026, WSO2 LLC. (https://www.wso2.com). + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React, { useEffect, useRef, useState } from 'react'; +import { Link as RouterLink, useNavigate, useParams } from 'react-router-dom'; +import { + Alert, + Box, + Button, + FormControl, + FormLabel, + Grid, + IconButton, + InputAdornment, + PageContent, + PageTitle, + Skeleton, + Stack, + TextField, +} from '@wso2/oxygen-ui'; +import { ChevronLeft, Eye, EyeOff } from '@wso2/oxygen-ui-icons-react'; +import { FormattedMessage } from 'react-intl'; +import { getSecret, updateSecret, type SecretMetadata } from '../../../../apis/secretApis'; +import { useAppShell } from '../../../../contexts/AppShellContext'; +import { buildOrgPath } from '../../../../utils/projectRouting'; +import useAIWorkspaceSnackbar from '../../../../hooks/aiWorkspaceSnackbar'; +import useIsMounted from '../../../../hooks/useIsMounted'; +import { getErrorMessage } from '../../../../utils/apiError'; +import ErrorAlert from '../../../../Components/common/ErrorAlert'; + +export default function RotateSecret(): React.JSX.Element { + const { handle } = useParams<{ handle: string }>(); + const navigate = useNavigate(); + const { currentOrganization } = useAppShell(); + const showSnackbar = useAIWorkspaceSnackbar(); + const isMounted = useIsMounted(); + + const [secret, setSecret] = useState(null); + const [isLoading, setIsLoading] = useState(true); + const [loadError, setLoadError] = useState(null); + + const [value, setValue] = useState(''); + const [valueVisible, setValueVisible] = useState(false); + const [displayName, setDisplayName] = useState(''); + const [description, setDescription] = useState(''); + const [isSubmitting, setIsSubmitting] = useState(false); + + const listPath = buildOrgPath(currentOrganization, '/settings/secrets'); + const overviewPath = handle ? `${listPath}/${handle}` : listPath; + + // Guards against out-of-order responses: if the handle changes while a fetch for + // the previous handle is still in flight, that response must not overwrite the + // form fields now shown for the newly-active handle — otherwise a submit could + // send the previous secret's display name/description to the current handle. + const requestIdRef = useRef(0); + + const fetchSecret = async () => { + if (!handle) return; + const requestId = ++requestIdRef.current; + try { + setIsLoading(true); + setLoadError(null); + const response = await getSecret(handle); + if (!isMounted() || requestIdRef.current !== requestId) return; // superseded or unmounted + setSecret(response); + setDisplayName(response.displayName); + setDescription(response.description ?? ''); + } catch (err) { + if (!isMounted() || requestIdRef.current !== requestId) return; + setLoadError(err instanceof Error ? err : new Error('Failed to load secret.')); + } finally { + if (isMounted() && requestIdRef.current === requestId) setIsLoading(false); + } + }; + + useEffect(() => { + void fetchSecret(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [handle]); + + const handleSubmit = async (event?: React.FormEvent) => { + if (event) event.preventDefault(); + if (!handle || isSubmitting) return; + + setIsSubmitting(true); + try { + await updateSecret(handle, { + value: value.trim(), + name: displayName.trim() || undefined, + description: description.trim() || undefined, + }); + if (!isMounted()) return; + showSnackbar('Secret updated successfully.', 'success'); + navigate(overviewPath); + } catch (err) { + if (!isMounted()) return; + showSnackbar(getErrorMessage(err, 'Failed to update secret.'), 'error'); + } finally { + if (isMounted()) setIsSubmitting(false); + } + }; + + return ( + + + + {isLoading ? ( + + + + + ) : loadError || !secret ? ( + + + + ) : ( + <> + + + + + + + + + + + This will get automatically synced to the gateway. + + + + + + + Display Name + setDisplayName(e.target.value)} /> + + + + + + Handle + + + + + + + Description + setDescription(e.target.value)} + placeholder="Reason for rotation, expiry info…" + /> + + + + + + Value + setValue(e.target.value)} + placeholder="Paste the new credential" + autoComplete="new-password" + helperText="Leave blank to keep the current value. Providing a new value reactivates a deprecated secret." + slotProps={{ + input: { + endAdornment: ( + + setValueVisible((v) => !v)} + aria-label={valueVisible ? 'Hide value' : 'Show value'} + > + {valueVisible ? : } + + + ), + }, + }} + data-cyid="rotate-secret-value-input" + /> + + + + + + + + + + + + )} + + ); +} diff --git a/portals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretOverview.tsx b/portals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretOverview.tsx new file mode 100644 index 0000000000..8bf15bfdf2 --- /dev/null +++ b/portals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretOverview.tsx @@ -0,0 +1,366 @@ +/* + * Copyright (c) 2026, WSO2 LLC. (https://www.wso2.com). + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React, { useEffect, useRef, useState } from 'react'; +import { Link as RouterLink, useNavigate, useParams } from 'react-router-dom'; +import { Avatar, Box, Button, Card, Chip, Divider, IconButton, PageContent, Skeleton, Stack, Typography } from '@wso2/oxygen-ui'; +import { ChevronLeft, Clock, Copy, KeyRound, RotateCw, Trash2 } from '@wso2/oxygen-ui-icons-react'; +import { getSecret, getSecretUsages, type SecretMetadata, type SecretReference } from '../../../../apis/secretApis'; +import { useAppShell } from '../../../../contexts/AppShellContext'; +import { buildOrgPath } from '../../../../utils/projectRouting'; +import useAIWorkspaceSnackbar from '../../../../hooks/aiWorkspaceSnackbar'; +import ErrorAlert from '../../../../Components/common/ErrorAlert'; +import DeleteSecretDialog from './DeleteSecretDialog'; +import NoData from '../../../../assets/images/NoData.svg'; + +// Maps the backend's artifact-type identifiers to display labels for the +// Usages section below. +const REFERENCE_TYPE_LABELS: Record = { + RestApi: 'REST API', + LlmProvider: 'LLM Provider', + LlmProxy: 'LLM Proxy', + Mcp: 'MCP Proxy', +}; + +function formatReferenceType(type: string): string { + return REFERENCE_TYPE_LABELS[type] ?? type; +} + +// Matches the relative-time convention used elsewhere in the app (e.g. +// contexts/llmProvider/LLMProviderContext.tsx's formatRelativeTime). +function formatRelativeTime(value?: string): string { + if (!value) return 'Unknown'; + const date = new Date(value); + if (Number.isNaN(date.getTime())) return 'Unknown'; + + const diffSeconds = Math.abs(Date.now() - date.getTime()) / 1000; + if (diffSeconds < 45) return 'Just now'; + if (diffSeconds < 90) return '1 minute ago'; + + const diffMinutes = diffSeconds / 60; + if (diffMinutes < 45) return `${Math.round(diffMinutes)} minutes ago`; + if (diffMinutes < 90) return '1 hour ago'; + + const diffHours = diffMinutes / 60; + if (diffHours < 22) return `${Math.round(diffHours)} hours ago`; + if (diffHours < 36) return '1 day ago'; + + const diffDays = diffHours / 24; + if (diffDays < 26) return `${Math.round(diffDays)} days ago`; + if (diffDays < 45) return '1 month ago'; + + const diffMonths = diffDays / 30; + if (diffMonths < 11) return `${Math.round(diffMonths)} months ago`; + const diffYears = diffDays / 365; + return `${Math.round(diffYears)} year${Math.round(diffYears) === 1 ? '' : 's'} ago`; +} + +function KeyValueRow({ label, children }: { label: string; children: React.ReactNode }): React.JSX.Element { + return ( + + + {label} + + {children} + + ); +} + +export default function SecretOverview(): React.JSX.Element { + const { handle } = useParams<{ handle: string }>(); + const navigate = useNavigate(); + const { currentOrganization } = useAppShell(); + const showSnackbar = useAIWorkspaceSnackbar(); + + const [secret, setSecret] = useState(null); + const [isLoading, setIsLoading] = useState(true); + const [error, setError] = useState(null); + const [deleteTarget, setDeleteTarget] = useState(null); + + const [usages, setUsages] = useState([]); + const [usagesLoading, setUsagesLoading] = useState(true); + const [usagesError, setUsagesError] = useState(null); + + const listPath = buildOrgPath(currentOrganization, '/settings/secrets'); + + const handleCopyHandle = async (value: string) => { + await navigator.clipboard.writeText(value); + showSnackbar('Handle copied to clipboard.', 'success'); + }; + + // Guards against out-of-order responses: if the handle changes while a fetch for + // the previous handle is still in flight, that response must not overwrite the + // metadata now on screen for the newly-active handle. + const requestIdRef = useRef(0); + + const fetchSecret = async () => { + if (!handle) return; + const requestId = ++requestIdRef.current; + try { + setIsLoading(true); + setError(null); + const response = await getSecret(handle); + if (requestIdRef.current !== requestId) return; // superseded by a newer request + setSecret(response); + } catch (err) { + if (requestIdRef.current !== requestId) return; + setError(err instanceof Error ? err : new Error('Failed to load secret.')); + } finally { + if (requestIdRef.current === requestId) setIsLoading(false); + } + }; + + useEffect(() => { + void fetchSecret(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [handle]); + + // Separate request-id guard from fetchSecret's: the two fetches are independent + // and must not clobber each other's staleness tracking. + const usagesRequestIdRef = useRef(0); + + const fetchUsages = async () => { + if (!handle) return; + const requestId = ++usagesRequestIdRef.current; + try { + setUsagesLoading(true); + setUsagesError(null); + const response = await getSecretUsages(handle); + if (usagesRequestIdRef.current !== requestId) return; + setUsages(response); + } catch (err) { + if (usagesRequestIdRef.current !== requestId) return; + setUsagesError(err instanceof Error ? err : new Error('Failed to load usages.')); + } finally { + if (usagesRequestIdRef.current === requestId) setUsagesLoading(false); + } + }; + + useEffect(() => { + void fetchUsages(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [handle]); + + return ( + + + + {isLoading ? ( + + + + + + ) : error || !secret ? ( + + + + ) : ( + + {/* Header card */} + + + + + + + + + + {secret.displayName} + + + + {secret.description && ( + + {secret.description} + + )} + + + Last updated : + + + + {formatRelativeTime(secret.updatedAt)} + + + {secret.createdBy && ( + + Created by: {secret.createdBy} + + )} + + + + + + + setDeleteTarget(secret)} + aria-label={`Delete ${secret.displayName}`} + data-cyid="delete-secret-button" + > + + + + + + + + + + + + {secret.id} + void handleCopyHandle(secret.id)} + aria-label="Copy handle" + sx={{ flexShrink: 0 }} + > + + + + + + + •••••••••••• + + write-once — never returned after creation + + + + + + + {/* Usages */} + + + + Usages + + {usagesLoading ? ( + + + + + ) : usagesError ? ( + + ) : usages.length === 0 ? ( + + + + No resources currently reference this secret. + + + ) : ( + + {usages.map((ref) => ( + + + {ref.name || ref.handle} + + + + ))} + + )} + + + + )} + + setDeleteTarget(null)} + onDeleted={() => navigate(listPath)} + /> + + ); +} diff --git a/portals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretsList.tsx b/portals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretsList.tsx new file mode 100644 index 0000000000..bb6e7bd9a3 --- /dev/null +++ b/portals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretsList.tsx @@ -0,0 +1,332 @@ +/* + * Copyright (c) 2026, WSO2 LLC. (https://www.wso2.com). + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React, { useEffect, useMemo, useRef, useState } from 'react'; +import { Link as RouterLink, useNavigate } from 'react-router-dom'; +import { + Box, + Button, + Card, + Chip, + IconButton, + InputAdornment, + PageContent, + PageTitle, + Skeleton, + Table, + TableBody, + TableCell, + TableContainer, + TableHead, + TablePagination, + TableRow, + TextField, + Typography, +} from '@wso2/oxygen-ui'; +import { KeyRound, Plus, Search, Trash2 } from '@wso2/oxygen-ui-icons-react'; +import { FormattedMessage } from 'react-intl'; +import { listSecrets, type SecretMetadata } from '../../../../apis/secretApis'; +import { useAppShell } from '../../../../contexts/AppShellContext'; +import { buildOrgPath } from '../../../../utils/projectRouting'; +import { getErrorMessage } from '../../../../utils/apiError'; +import ErrorAlert from '../../../../Components/common/ErrorAlert'; +import DeleteSecretDialog from './DeleteSecretDialog'; + +const ROWS_PER_PAGE_OPTIONS = [10, 25, 50]; + +function formatDate(value?: string): string { + if (!value) return '—'; + const date = new Date(value); + if (Number.isNaN(date.getTime())) return '—'; + return date.toLocaleDateString(undefined, { year: 'numeric', month: 'short', day: 'numeric' }); +} + +export default function SecretsList(): React.JSX.Element { + const navigate = useNavigate(); + const { currentOrganization } = useAppShell(); + + const [secrets, setSecrets] = useState([]); + const [isLoading, setIsLoading] = useState(true); + const [error, setError] = useState(null); + const [searchQuery, setSearchQuery] = useState(''); + const [deleteTarget, setDeleteTarget] = useState(null); + const [page, setPage] = useState(0); + const [rowsPerPage, setRowsPerPage] = useState(ROWS_PER_PAGE_OPTIONS[0]); + + const organizationId = currentOrganization?.uuid ?? ''; + const basePath = buildOrgPath(currentOrganization, '/settings/secrets'); + + // Guards against out-of-order responses (e.g. two concurrent fetches from a + // double-click on Retry, or — defensively — an organization change): a response + // for a superseded request must not replace the list a more recent request + // already populated. Each mount gets its own counter, since OrgShell remounts + // this whole page (via a key on the organization id) on every organization switch. + const requestIdRef = useRef(0); + + const fetchSecrets = async () => { + const requestId = ++requestIdRef.current; + try { + setIsLoading(true); + setError(null); + const response = await listSecrets({ limit: 100 }); + if (requestIdRef.current !== requestId) return; // superseded by a newer request + setSecrets(response.list ?? []); + } catch (err) { + if (requestIdRef.current !== requestId) return; + setError(err instanceof Error ? err : new Error(getErrorMessage(err, 'Failed to load secrets.'))); + } finally { + if (requestIdRef.current === requestId) setIsLoading(false); + } + }; + + useEffect(() => { + if (!organizationId) return; + void fetchSecrets(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [organizationId]); + + const filteredSecrets = useMemo(() => { + const query = searchQuery.trim().toLowerCase(); + if (!query) return secrets; + return secrets.filter((secret) => + [secret.displayName, secret.id, secret.description].filter(Boolean).join(' ').toLowerCase().includes(query) + ); + }, [searchQuery, secrets]); + + useEffect(() => { + setPage(0); + }, [rowsPerPage, searchQuery]); + + // Clamps to the last valid page when the list shrinks out from under the current + // page (e.g. deleting the only secret on the last page), instead of rendering blank. + const pageCount = Math.max(1, Math.ceil(filteredSecrets.length / rowsPerPage)); + const safePage = Math.min(page, pageCount - 1); + + const pageSecrets = useMemo( + () => filteredSecrets.slice(safePage * rowsPerPage, safePage * rowsPerPage + rowsPerPage), + [filteredSecrets, safePage, rowsPerPage] + ); + + return ( + + + + + + + + + + + + {secrets.length > 0 && ( + + )} + + + {error && !isLoading && ( + + + + )} + + {isLoading ? ( + + + + + + Name + Handle + Type + Last updated + Actions + + + + {[...Array(3)].map((_, index) => ( + + + + + + + + ))} + +
+ + + ) : !error && secrets.length === 0 ? ( + + + + + + + + + + + + + ) : !error ? ( + <> + + setSearchQuery(event.target.value)} + slotProps={{ + input: { + startAdornment: ( + + + + ), + }, + }} + /> + + + + + + + Name + Handle + Type + Last updated + Actions + + + + {filteredSecrets.length === 0 ? ( + + + + No secrets match your search. + + + + ) : ( + pageSecrets.map((secret) => ( + navigate(`${basePath}/${secret.id}`)} + onKeyDown={(event) => { + if (event.key === 'Enter' || event.key === ' ') { + event.preventDefault(); + navigate(`${basePath}/${secret.id}`); + } + }} + aria-label={`View ${secret.displayName}`} + sx={{ cursor: 'pointer' }} + data-cyid={`secret-row-${secret.id}`} + > + {secret.displayName} + + + {secret.id} + + + + + + {formatDate(secret.updatedAt)} + + { + event.stopPropagation(); + setDeleteTarget(secret); + }} + aria-label={`Delete ${secret.displayName}`} + data-cyid={`delete-secret-${secret.id}`} + > + + + + + )) + )} + +
+ + {filteredSecrets.length > 0 && ( + setPage(nextPage)} + rowsPerPage={rowsPerPage} + onRowsPerPageChange={(event) => setRowsPerPage(parseInt(event.target.value, 10))} + rowsPerPageOptions={ROWS_PER_PAGE_OPTIONS} + /> + )} + + + ) : null} + + setDeleteTarget(null)} + onDeleted={(handle) => setSecrets((prev) => prev.filter((s) => s.id !== handle))} + /> + + ); +} diff --git a/portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/AddNewProvider/ProviderTemplateFormFields.tsx b/portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/AddNewProvider/ProviderTemplateFormFields.tsx index cfa7737d32..1af762d408 100644 --- a/portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/AddNewProvider/ProviderTemplateFormFields.tsx +++ b/portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/AddNewProvider/ProviderTemplateFormFields.tsx @@ -26,8 +26,6 @@ import { FormHelperText, FormLabel, Grid, - IconButton, - InputAdornment, MenuItem, Select, Stack, @@ -36,16 +34,14 @@ import { Box, CircularProgress, } from '@wso2/oxygen-ui'; -import { Eye, EyeOff } from '@wso2/oxygen-ui-icons-react'; import type { ProviderTemplate } from '../../../../../utils/types'; import type { FormState } from './serviceProviderTypes'; import { FormattedMessage } from 'react-intl'; +import SecretValueField from '../../../../../Components/common/SecretValueField'; type ProviderTemplateFormFieldsProps = { formState: FormState; setFormState: React.Dispatch>; - showCredential: boolean; - setShowCredential: React.Dispatch>; template?: ProviderTemplate | null; isLoading: boolean; error: Error | null; @@ -71,8 +67,6 @@ const buildAutoContext = (name: string): string => { export default function ProviderTemplateFormFields({ formState, setFormState, - showCredential, - setShowCredential, template, isLoading, error, @@ -349,44 +343,16 @@ export default function ProviderTemplateFormFields({ defaultMessage={'API Key'} /> - + onChange={(nextValue) => setFormState((prev) => ({ ...prev, - upstreamAuthValue: e.target.value, + upstreamAuthValue: nextValue, })) } - type={showCredential ? 'text' : 'password'} data-cyid="provider-api-key-input" - slotProps={{ - input: { - endAdornment: ( - - setShowCredential((prev) => !prev)} - aria-label={ - showCredential ? 'Hide credentials' : 'Show credentials' - } - > - {showCredential ? ( - - ) : ( - - )} - - - ), - }, - }} placeholder="Enter your API key or token (optional)" - // helperText={ - // template?.metadata?.auth?.valuePrefix - // ? `Will be prefixed with: ${template.metadata.auth.valuePrefix}` - // : 'Your authentication credential for the upstream provider' - // } /> diff --git a/portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderConnectionTab.tsx b/portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderConnectionTab.tsx index 3267bfe930..c0db22a5f1 100644 --- a/portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderConnectionTab.tsx +++ b/portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderConnectionTab.tsx @@ -21,14 +21,11 @@ import { FormControl, FormHelperText, FormLabel, - IconButton, - InputAdornment, MenuItem, Select, Stack, TextField, } from '@wso2/oxygen-ui'; -import { Eye, EyeOff } from '@wso2/oxygen-ui-icons-react'; import { useLLMProvider } from '../../../../contexts/llmProvider'; import { useAppShell } from '../../../../contexts/AppShellContext'; import { PLATFORM_API_BASE_URL } from '../../../../paths'; @@ -37,6 +34,7 @@ import * as providerTemplateApis from '../../../../apis/providerTemplateApis'; import type { ProviderTemplate } from '../../../../utils/types'; import { logger } from '../../../../utils/logger'; import { isValidHttpUrl } from '../../../../utils/providerTemplateFields'; +import SecretValueField from '../../../../Components/common/SecretValueField'; const MASKED_CREDENTIAL_VALUE = '******'; @@ -51,7 +49,6 @@ export default function ServiceProviderConnectionTab() { const [credentialValue, setCredentialValue] = useState(''); const [isCredentialMasked, setIsCredentialMasked] = useState(false); const [hasCredentialChanged, setHasCredentialChanged] = useState(false); - const [showCredential, setShowCredential] = useState(false); const [providerTemplate, setProviderTemplate] = useState(null); const showSnackbar = useAIWorkspaceSnackbar(); @@ -382,9 +379,8 @@ export default function ServiceProviderConnectionTab() { Credentials - { @@ -394,8 +390,7 @@ export default function ServiceProviderConnectionTab() { setHasCredentialChanged(false); } }} - onChange={(e) => { - const nextValue = e.target.value; + onChange={(nextValue) => { setCredentialValue(nextValue); setHasCredentialChanged(true); if (isDraftMode && !isCredentialMasked) { @@ -411,30 +406,6 @@ export default function ServiceProviderConnectionTab() { void handleUpdateCredential(); } }} - slotProps={{ - input: { - endAdornment: ( - - setShowCredential((prev) => !prev)} - aria-label={ - showCredential - ? 'Hide credentials' - : 'Show credentials' - } - > - {showCredential ? ( - - ) : ( - - )} - - - ), - }, - }} /> diff --git a/portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsx b/portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsx index 754348780f..0d8f1d5eda 100644 --- a/portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsx +++ b/portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsx @@ -77,8 +77,6 @@ const FIELD_NAME_MAP: Partial> = { type TemplateBasedFormFieldsContainerProps = { formState: FormState; setFormState: React.Dispatch>; - showCredential: boolean; - setShowCredential: React.Dispatch>; setOpenapiSpec: React.Dispatch>; fieldErrors: Partial>; }; @@ -86,8 +84,6 @@ type TemplateBasedFormFieldsContainerProps = { function TemplateBasedFormFieldsContainer({ formState, setFormState, - showCredential, - setShowCredential, setOpenapiSpec, fieldErrors, }: TemplateBasedFormFieldsContainerProps) { @@ -143,8 +139,6 @@ function TemplateBasedFormFieldsContainer({ >>({}); - const [showCredential, setShowCredential] = useState(false); const [guardrails, setGuardrails] = useState([]); const [guardrailDrawerOpen, setGuardrailDrawerOpen] = useState(false); const [selectedGuardrail, setSelectedGuardrail] = useState( @@ -581,8 +574,6 @@ export default function ServiceProviderNew() { diff --git a/portals/ai-workspace/src/pages/appShell/appShellPages/settings/Main.tsx b/portals/ai-workspace/src/pages/appShell/appShellPages/settings/Main.tsx index ac9ab80ce4..5a676a10c3 100644 --- a/portals/ai-workspace/src/pages/appShell/appShellPages/settings/Main.tsx +++ b/portals/ai-workspace/src/pages/appShell/appShellPages/settings/Main.tsx @@ -35,7 +35,7 @@ import { PageTitle, Stack, } from '@wso2/oxygen-ui'; -import { LayoutTemplate, ShieldCheck } from '@wso2/oxygen-ui-icons-react'; +import { KeyRound, LayoutTemplate, ShieldCheck } from '@wso2/oxygen-ui-icons-react'; import { FormattedMessage } from 'react-intl'; import { useAppShell } from '../../../../contexts/AppShellContext'; import { useAppAuth } from '../../../../contexts/AppAuthContext'; @@ -68,18 +68,40 @@ const NAV_ITEMS: NavItem[] = [ path: '/settings/custom-policies', scope: SCOPES.GATEWAY_CUSTOM_POLICY_READ, }, + { + key: 'secrets', + label: 'Secrets', + icon: , + path: '/settings/secrets', + scope: SCOPES.SECRET_READ, + }, ]; export default function Settings() { const navigate = useNavigate(); const location = useLocation(); const { currentOrganization } = useAppShell(); - const { hasPermission } = useAppAuth(); + const { hasPermission, isLoading: isAuthLoading } = useAppAuth(); + + // A hard navigation/refresh straight to a nested settings URL mounts this + // component before the BFF session fetch resolves. ProtectedRoute already + // blocks rendering on isLoading for the top-level authenticated/not branch, + // but scope data can still be mid-flight on the very first render here — do + // not let a not-yet-loaded hasPermission()===false for everything read as + // "no access" and fire the one-shot redirects below. + if (isAuthLoading) { + return null; + } const visibleNavItems = NAV_ITEMS.filter((item) => hasPermission(item.scope)); - const selectedKey = visibleNavItems.find((item) => - location.pathname.includes(item.path) - )?.key; + + // Match against the full NAV_ITEMS list, not just visibleNavItems: a caller who + // navigates directly to a settings URL they lack the scope for must be denied + // access below, not merely have the sidebar fall back to highlighting a + // different item while the actual (unauthorized) page still renders via + // . + const matchedItem = NAV_ITEMS.find((item) => location.pathname.includes(item.path)); + const selectedKey = matchedItem?.key ?? visibleNavItems[0]?.key; // Settings requires at least one visible section; send others to org home. if (visibleNavItems.length === 0) { @@ -91,6 +113,18 @@ export default function Settings() { ); } + // The active route matched a known settings section the caller lacks the scope + // for (e.g. direct navigation to /settings/secrets without SECRET_READ) — + // redirect to a section they can actually access instead of rendering it. + if (matchedItem && !hasPermission(matchedItem.scope)) { + return ( + + ); + } + return ( {/* Persistent left sub-nav */} @@ -138,11 +172,25 @@ export default function Settings() { ); } +// resolveSettingsFallbackPath picks the settings-relative destination a caller with +// `hasPermission` should land on: the first section they hold the scope for, or the +// org home page if they hold none of them. Shared by SettingsIndexRedirect and any +// route guard (see App.tsx's RequireScope) that needs to bounce an unauthorized +// caller somewhere useful instead of a hardcoded path. +export function resolveSettingsFallbackPath(hasPermission: (scope: string) => boolean): string { + const firstVisibleItem = NAV_ITEMS.find((item) => hasPermission(item.scope)); + return firstVisibleItem ? firstVisibleItem.path : '/home'; +} + export function SettingsIndexRedirect() { const { currentOrganization } = useAppShell(); - const { hasPermission } = useAppAuth(); - const firstVisibleItem = NAV_ITEMS.find((item) => hasPermission(item.scope)); - const target = firstVisibleItem ? firstVisibleItem.path : '/home'; + const { hasPermission, isLoading: isAuthLoading } = useAppAuth(); + + // Same one-shot-redirect-before-scopes-load hazard as Settings()/RequireScope + // above — wait for the session fetch to settle before picking a destination. + if (isAuthLoading) { + return null; + } - return ; + return ; } diff --git a/tests/integration-e2e/README.md b/tests/integration-e2e/README.md index d0456ea0b1..27c52f66de 100644 --- a/tests/integration-e2e/README.md +++ b/tests/integration-e2e/README.md @@ -92,9 +92,10 @@ Or via make (from `platform-api/`): `make e2e`, `make e2e-all-dbs`. each run, so a rotating secret works either way. - `E2E_TAGS=@smoke` runs a tag subset (other tags: `@secured`, `@multigateway`, `@devportal`, `@lifecycle` for the credential-lifecycle scenario — run it alone - with `E2E_TAGS="@devportal && @lifecycle"` —, and the on-demand secret fetch + with `E2E_TAGS="@devportal && @lifecycle"` —, the on-demand secret fetch scenarios `@llm_provider`, `@llm_proxy`, `@mcp_proxy`, `@rest_api_secret` and - `@policy_secret`). The `@multigateway` and `@devportal` scenarios run only on the + `@policy_secret`, and `@secret_lifecycle` for the secret rotation/deletion push-event + scenarios). The `@multigateway` and `@devportal` scenarios run only on the postgres stack (the only one wired with a second gateway and the developer portal) and are otherwise skipped automatically. - `PA_HOST_PORT` / `GW_HTTP_PORT` / `GW2_HTTP_PORT` / `AP_HOST_PORT` override the @@ -259,6 +260,27 @@ resolution keeps the artifact from ever rendering, so it never shows up). against the single matching registered version in `gateway/build-manifest.yaml` and rejects a full semver string like `"v1.1.0"`. +### Secret rotation/deletion push-event scenarios + +The scenarios above cover secret resolution *at deploy time*. `secret_lifecycle.feature` +(`@secret_lifecycle`) covers the opposite direction: a secret that a gateway is +**already** resolving gets rotated or deleted while that gateway stays connected, with +no controller restart and no new deployment. platform-api pushes a +`secret.updated`/`secret.deleted` WebSocket event to every gateway in the org +(`SecretService.broadcastSecretEvent`); the controller's `handleSecretUpdatedEvent` / +`handleSecretDeletedEvent` (`pkg/controlplane/client.go`) react immediately instead of +waiting for the next reconnect's incremental sync. Both scenarios build on the same +secret-backed REST API fixture as `rest_api_secret.feature` (shared Given steps), then +poll the gateway-controller's own `GET /api/management/v1/secrets/:handle` — which +returns the gateway's locally decrypted value, not platform-api's — to observe the +effect directly: + +12. **Rotation** — `PUT /secrets/:handle` with a new value. Asserts the gateway's local + copy reaches the rotated value. +13. **Deletion** — `PUT /rest-apis/:id` swaps the upstream auth to a brand-new secret, + leaving the original referenced by nothing; platform-api's `cleanupRotatedSecret` + deprecates it automatically. Asserts the gateway's local copy is evicted (404). + ## Status — passing on all three databases The full live-traffic scenario passes on **SQLite, PostgreSQL and SQL Server** diff --git a/tests/integration-e2e/features/secret_lifecycle.feature b/tests/integration-e2e/features/secret_lifecycle.feature new file mode 100644 index 0000000000..bc6da23875 --- /dev/null +++ b/tests/integration-e2e/features/secret_lifecycle.feature @@ -0,0 +1,26 @@ +@secret_lifecycle +Feature: Live secret rotation and deletion push events to a connected gateway + As an API platform operator + I want a connected gateway to pick up a secret rotation or deletion immediately + So that credential changes take effect without waiting for the gateway's next + reconnect-triggered poll, and without restarting the gateway-controller. + + Background: + Given the platform-api control plane and gateway data plane are running + And I am authenticated to platform-api + + Scenario: Rotating a secret pushes the new value to an already-connected gateway + Given a secret containing a REST API upstream credential + And a REST API whose upstream auth references the secret + And I deploy the secret-backed REST API to the gateway + And the gateway has the secret-backed REST API configured + When I rotate the secret to a new value + Then the gateway's local copy of the secret has the rotated value + + Scenario: Deleting a secret evicts it from an already-connected gateway + Given a secret containing a REST API upstream credential + And a REST API whose upstream auth references the secret + And I deploy the secret-backed REST API to the gateway + And the gateway has the secret-backed REST API configured + When I update the REST API to reference a different secret instead + Then the gateway evicts the original secret from its local store diff --git a/tests/integration-e2e/secret_helpers_test.go b/tests/integration-e2e/secret_helpers_test.go index 7bb8feebb8..3d12431b99 100644 --- a/tests/integration-e2e/secret_helpers_test.go +++ b/tests/integration-e2e/secret_helpers_test.go @@ -24,16 +24,43 @@ package e2e // secret-backed artifact, then polls the gateway-controller's management API // until the artifact appears — confirming the controller resolved the // {{ secret "..." }} reference at deploy time. +// +// rotateSecret / waitGatewaySecretValue / waitGatewaySecretGone below back a +// different scenario family — secret_lifecycle.feature — which exercises the +// live secret.updated/secret.deleted WebSocket push path (handleSecretUpdatedEvent / +// handleSecretDeletedEvent) against an already-connected gateway, rather than the +// on-demand fetch that happens at artifact-deploy time. import ( "bytes" + "encoding/json" "fmt" "io" "mime/multipart" "net/http" + "os" + "strconv" "time" ) +// defaultMaxSecretHelperRespBytes bounds how much of a platform-api response body +// these secret helpers will buffer into memory — a safety ceiling against a +// misbehaving server returning an unbounded stream, not an expected size. +// Override via E2E_MAX_RESP_BYTES for scenarios that legitimately need more. +const defaultMaxSecretHelperRespBytes = 10 << 20 // 10 MiB + +// maxSecretHelperRespBytes returns the configured response-body size ceiling +// (E2E_MAX_RESP_BYTES), falling back to defaultMaxSecretHelperRespBytes when unset +// or invalid. +func maxSecretHelperRespBytes() int64 { + if v := os.Getenv("E2E_MAX_RESP_BYTES"); v != "" { + if n, err := strconv.ParseInt(v, 10, 64); err == nil && n > 0 { + return n + } + } + return defaultMaxSecretHelperRespBytes +} + // createSecret creates a GENERIC secret in platform-api via multipart/form-data // and returns its handle. func createSecret(displayName, value string) (string, error) { @@ -65,7 +92,7 @@ func createSecret(displayName, value string) (string, error) { return "", err } defer resp.Body.Close() - body, _ := io.ReadAll(resp.Body) + body, _ := io.ReadAll(io.LimitReader(resp.Body, maxSecretHelperRespBytes())) if resp.StatusCode >= 300 { return "", fmt.Errorf("create secret failed (%d): %s", resp.StatusCode, body) } @@ -127,3 +154,137 @@ func waitGatewayResource(resourcePath string, timeout time.Duration) error { return fmt.Errorf("gateway did not configure resource %q within timeout: last status %d", resourcePath, lastStatus) } + +// rotateSecret rotates an existing secret's value via PUT /secrets/:handle +// (multipart/form-data) — the same call the AI Workspace UI's "Rotate secret" +// action makes. platform-api broadcasts a secret.updated event to every gateway +// in the org as part of this call. +func rotateSecret(handle, newValue string) error { + buf := &bytes.Buffer{} + mw := multipart.NewWriter(buf) + if err := mw.WriteField("value", newValue); err != nil { + return err + } + mw.Close() + + req, err := http.NewRequest(http.MethodPut, platformAPI+platformAPIBase+"/secrets/"+handle, buf) + if err != nil { + return err + } + req.Header.Set("Content-Type", mw.FormDataContentType()) + req.Header.Set("Authorization", "Bearer "+suite.token) + + resp, err := httpClient.Do(req) + if err != nil { + return err + } + defer resp.Body.Close() + body, _ := io.ReadAll(io.LimitReader(resp.Body, maxSecretHelperRespBytes())) + if resp.StatusCode >= 300 { + return fmt.Errorf("rotate secret failed (%d): %s", resp.StatusCode, body) + } + return nil +} + +// deleteSecret permanently deletes a secret via DELETE /secrets/:handle — the +// same call the AI Workspace UI's "Delete secret" action makes. platform-api broadcasts +// a secret.deleted event to every gateway in the org once the delete actually +// succeeds (it 409s instead if the handle is still referenced by any artifact, current +// config or deployed snapshot, on any gateway). +func deleteSecret(handle string) error { + req, err := http.NewRequest(http.MethodDelete, platformAPI+platformAPIBase+"/secrets/"+handle, nil) + if err != nil { + return err + } + req.Header.Set("Authorization", "Bearer "+suite.token) + + resp, err := httpClient.Do(req) + if err != nil { + return err + } + defer resp.Body.Close() + body, _ := io.ReadAll(io.LimitReader(resp.Body, maxSecretHelperRespBytes())) + if resp.StatusCode >= 300 { + return fmt.Errorf("delete secret failed (%d): %s", resp.StatusCode, body) + } + return nil +} + +// gatewaySecretValue extracts spec.value from a gateway-controller GetSecret +// response body (GET /api/management/v1/secrets/:handle — see +// buildSecretResourceResponse in the gateway-controller source). +func gatewaySecretValue(body []byte) string { + var r struct { + Spec struct { + Value string `json:"value"` + } `json:"spec"` + } + if json.Unmarshal(body, &r) != nil { + return "" + } + return r.Spec.Value +} + +// waitGatewaySecretValue polls the gateway-controller's own secret store +// (GET /api/management/v1/secrets/:handle, basic auth admin:admin) until its +// decrypted value equals expectedValue or timeout expires. Confirms a +// secret.updated push event caused an immediate re-fetch — not merely that the +// gateway will eventually catch up on its next reconnect-triggered poll. +func waitGatewaySecretValue(handle, expectedValue string, timeout time.Duration) error { + url := gwMgmtAPI + "/api/management/v1/secrets/" + handle + deadline := time.Now().Add(timeout) + var lastValue string + for time.Now().Before(deadline) { + req, err := http.NewRequest(http.MethodGet, url, nil) + if err != nil { + return err + } + req.SetBasicAuth("admin", "admin") + resp, err := httpClient.Do(req) + if err != nil { + time.Sleep(2 * time.Second) + continue + } + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + if resp.StatusCode == http.StatusOK { + lastValue = gatewaySecretValue(body) + if lastValue == expectedValue { + return nil + } + } + time.Sleep(2 * time.Second) + } + return fmt.Errorf("gateway secret %q did not reach the rotated value within timeout (last seen: %q)", + handle, lastValue) +} + +// waitGatewaySecretGone polls the gateway-controller's own secret store until +// GET /api/management/v1/secrets/:handle returns 404 or timeout expires. +// Confirms a secret.deleted push event caused the gateway to evict its local +// copy of a secret that is no longer referenced by any artifact. +func waitGatewaySecretGone(handle string, timeout time.Duration) error { + url := gwMgmtAPI + "/api/management/v1/secrets/" + handle + deadline := time.Now().Add(timeout) + var lastStatus int + for time.Now().Before(deadline) { + req, err := http.NewRequest(http.MethodGet, url, nil) + if err != nil { + return err + } + req.SetBasicAuth("admin", "admin") + resp, err := httpClient.Do(req) + if err != nil { + time.Sleep(2 * time.Second) + continue + } + io.Copy(io.Discard, resp.Body) //nolint:errcheck + resp.Body.Close() + lastStatus = resp.StatusCode + if lastStatus == http.StatusNotFound { + return nil + } + time.Sleep(2 * time.Second) + } + return fmt.Errorf("gateway did not evict secret %q within timeout: last status %d", handle, lastStatus) +} diff --git a/tests/integration-e2e/secret_lifecycle_steps_test.go b/tests/integration-e2e/secret_lifecycle_steps_test.go new file mode 100644 index 0000000000..a13383c3bb --- /dev/null +++ b/tests/integration-e2e/secret_lifecycle_steps_test.go @@ -0,0 +1,130 @@ +/* + * Copyright (c) 2026, WSO2 LLC. (https://www.wso2.com). + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package e2e + +// Steps for secret_lifecycle.feature — exercises the LIVE push-event path for an +// already-referenced, already-connected secret, as opposed to rest_api_secret.feature +// (and siblings) which exercise the on-demand fetch that happens at artifact-deploy +// time. The Background/Given steps here are shared with rest_api_secret.feature +// (aSecretForRestAPI, aRestAPIReferencingSecret, deploySecretBackedRestAPI, +// gatewayHasSecretBackedRestAPIConfigured in rest_api_secret_steps_test.go) — this +// feature picks up from an already-deployed, already-resolved secret-backed REST API +// and then rotates or replaces the secret: +// +// 1. Rotation: PUT /secrets/:handle with a new value (rotateSecret in +// secret_helpers_test.go). platform-api broadcasts secret.updated to every +// gateway in the org; the already-connected controller's handleSecretUpdatedEvent +// re-fetches the plaintext and upserts it, without a restart. The assertion polls +// the gateway-controller's own GET /api/management/v1/secrets/:handle until its +// decrypted value matches the rotated one. +// 2. Deletion: PUT /rest-apis/:id to swap the upstream auth to a brand-new secret, +// then redeploy (without this, the gateway's already-deployed snapshot still +// references the original handle — artifact_secret_refs keeps a gateway-scoped +// row for it independently of the artifact's current config — so the explicit +// DELETE below would 409 as still-referenced). Once the redeploy clears that +// gateway-scoped row, the original secret is referenced by nothing and +// DELETE /secrets/:handle succeeds, broadcasting secret.deleted; the +// controller's handleSecretDeletedEvent evicts its local copy. The assertion +// polls the same management endpoint until it 404s. + +import ( + "fmt" + "net/http" + "strings" +) + +// iRotateTheSecretToANewValue rotates w.restAPISecretHandle to a fresh value, +// exercising the live secret.updated push-event path. +func (w *world) iRotateTheSecretToANewValue() error { + if w.restAPISecretHandle == "" { + return fmt.Errorf("no secret handle — run the REST API secret background steps first") + } + w.restAPISecretRotatedValue = "e2e-test-restapi-rotated-" + randHex() + return rotateSecret(w.restAPISecretHandle, w.restAPISecretRotatedValue) +} + +// theGatewaysLocalSecretHasTheRotatedValue polls the gateway-controller's own +// secret store until it reflects the rotated value, confirming secret.updated +// triggered an immediate re-fetch rather than waiting for the next reconnect. +func (w *world) theGatewaysLocalSecretHasTheRotatedValue() error { + if w.restAPISecretRotatedValue == "" { + return fmt.Errorf("no rotated value recorded — run 'I rotate the secret to a new value' first") + } + return waitGatewaySecretValue(w.restAPISecretHandle, w.restAPISecretRotatedValue, pollTimeout) +} + +// iUpdateTheRestAPIToReferenceADifferentSecret creates a second secret, updates the +// REST API's upstream auth to reference it instead of the original one, redeploys so +// the gateway's deployed snapshot stops referencing the original handle too, then +// explicitly deletes the now-fully-unreferenced original secret. +func (w *world) iUpdateTheRestAPIToReferenceADifferentSecret() error { + if w.restAPISecretApiID == "" { + return fmt.Errorf("no REST API — run the REST API secret background steps first") + } + + replacementHandle, err := createSecret("E2E REST API Replacement Credential", "e2e-test-restapi-value-"+randHex()) + if err != nil { + return err + } + + // Reconstruct the exact displayName aRestAPIReferencingSecret used, so this + // full-replace PUT doesn't incidentally change anything but the auth value. + suffix := strings.TrimPrefix(w.restAPISecretContext, "/e2e-secret-") + displayName := "e2e-secret-api-" + suffix + + st, body, err := apiCall(http.MethodPut, "/rest-apis/"+w.restAPISecretApiID, suite.token, map[string]any{ + "displayName": displayName, + "context": w.restAPISecretContext, + "version": "v1", + "projectId": suite.projectID, + "upstream": map[string]any{ + "main": map[string]any{ + "url": "http://sample-backend:9080", + "auth": map[string]any{ + "type": "api-key", + "header": "Authorization", + "value": `{{ secret "` + replacementHandle + `" }}`, + }, + }, + }, + }) + if err != nil { + return err + } + if st >= 300 { + return fmt.Errorf("update REST API to swap secret failed (%d): %s", st, body) + } + + // Redeploy the updated config so the gateway's deployed snapshot (and therefore + // its gateway-scoped artifact_secret_refs row) picks up the replacement handle, + // freeing the original one from every reference — current config and deployed + // snapshot alike — before we try to delete it. + if _, err := deployRestAPIWithoutRestart(w.restAPISecretApiID, suite.gw1ID); err != nil { + return fmt.Errorf("redeploy after secret swap failed: %w", err) + } + + return deleteSecret(w.restAPISecretHandle) +} + +// theGatewayEvictsTheOriginalSecretFromItsLocalStore polls the gateway-controller's +// secret store until the original (now-unreferenced, permanently deleted) secret is +// gone, confirming the secret.deleted push event triggered eviction. +func (w *world) theGatewayEvictsTheOriginalSecretFromItsLocalStore() error { + return waitGatewaySecretGone(w.restAPISecretHandle, pollTimeout) +} diff --git a/tests/integration-e2e/steps_test.go b/tests/integration-e2e/steps_test.go index d1cf096e96..e5e89d5fdf 100644 --- a/tests/integration-e2e/steps_test.go +++ b/tests/integration-e2e/steps_test.go @@ -77,6 +77,11 @@ type world struct { policySecretApiID string // id of the created REST API policySecretContext string // e.g. /e2e-policy-ab12cd34 policySecretDepID string // deploymentId returned when the API is deployed + + // Secret rotation/deletion push-event scenario state (see + // secret_lifecycle_steps_test.go). Reuses the restAPISecret* fields above, + // populated by the shared rest_api_secret.feature background steps. + restAPISecretRotatedValue string // the new plaintext value after rotation } // initializeScenario is invoked by godog for each scenario; it binds a fresh @@ -138,6 +143,13 @@ func initializeScenario(sc *godog.ScenarioContext) { sc.Step(`^a REST API with a set-headers policy referencing the secret$`, w.aRestAPIWithPolicyReferencingSecret) sc.Step(`^I deploy the policy-secret REST API to the gateway$`, w.deployPolicySecretRestAPI) sc.Step(`^the gateway has the policy-secret REST API configured$`, w.gatewayHasPolicySecretRestAPIConfigured) + + // Secret rotation/deletion push-event steps (secret_lifecycle.feature). The + // Given steps are shared with rest_api_secret.feature (registered above). + sc.Step(`^I rotate the secret to a new value$`, w.iRotateTheSecretToANewValue) + sc.Step(`^the gateway's local copy of the secret has the rotated value$`, w.theGatewaysLocalSecretHasTheRotatedValue) + sc.Step(`^I update the REST API to reference a different secret instead$`, w.iUpdateTheRestAPIToReferenceADifferentSecret) + sc.Step(`^the gateway evicts the original secret from its local store$`, w.theGatewayEvictsTheOriginalSecretFromItsLocalStore) } // --- Background steps ------------------------------------------------------