fix(registrations): clear the stored credential once a registration is decided

internal/api/handler_registrations.go

@@ -307,7 +307,13 @@ func (h *Handler) approveRegistration(ctx context.Context, httpReq *events.Lambd
 	}
 
 	// Auto-store credentials if the registration included them.
-	h.storeRegistrationCredentials(ctx, reg, account.ID, regCred)
+	// The registration's copy is dropped only after the save succeeds; until
+	// then it is the only copy. The account exists, so a failed clear is logged.
+	if h.storeRegistrationCredentials(ctx, reg, account.ID, regCred) {
+		if err := h.config.ClearRegistrationCredentialPayload(ctx, reg.ID); err != nil {
+			logging.Errorf("registration %s: failed to clear stored credential payload: %v", reg.ID, err)
+		}
+	}
 
 	h.notifyRegistrant(reg, email.RegistrationDecisionData{
 		AccountName: reg.AccountName, Provider: reg.Provider,

internal/api/handler_registrations_credentials.go

@@ -91,12 +91,14 @@ func (h *Handler) registrationCredential(reg *config.AccountRegistration, provid
 // storeRegistrationCredentials saves a registration credential that
 // registrationCredential has checked into the account_credentials table.
 // The account already exists at this point, so a store error is logged, not
-// returned.
-func (h *Handler) storeRegistrationCredentials(ctx context.Context, reg *config.AccountRegistration, accountID string, payload []byte) {
+// returned. It reports whether the credential was saved.
+func (h *Handler) storeRegistrationCredentials(ctx context.Context, reg *config.AccountRegistration, accountID string, payload []byte) bool {
 	if payload == nil || h.credStore == nil {
-		return
+		return false
 	}
 	if err := h.credStore.SaveCredential(ctx, accountID, reg.RegCredentialType, payload); err != nil {
-		logging.Warnf("registration %s: failed to store credential: %v", reg.ID, err)
+		logging.Errorf("registration %s: failed to store credential: %v", reg.ID, err)
+		return false
 	}
+	return true
 }

internal/api/handler_registrations_credentials_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/credentials"
 )
 
 const regCredRegID = "6f1c2a8e-3b4d-4e5f-8a9b-0c1d2e3f4a5b"
@@ -166,6 +167,7 @@ func approveWithCredential(t *testing.T, credType, payload, approvalBody string)
 	store.On("ApproveAccountRegistration", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Maybe()
 	store.On("TransitionRegistrationStatus", mock.Anything, mock.Anything, "pending", mock.Anything).Return(nil).Maybe()
 	store.On("UpdateAccountRegistration", mock.Anything, mock.Anything).Return(nil).Maybe()
+	store.On("ClearRegistrationCredentialPayload", mock.Anything, mock.Anything).Return(nil).Maybe()
 	// The router enforces the approve permission; called directly with no
 	// bearer token, approveRegistration resolves no session, so the auth mock
 	// carries no expectations (an unexpected auth call would panic).
@@ -227,3 +229,41 @@ func TestApproveRegistration_RefusesInvalidStoredCredential(t *testing.T) {
 		})
 	}
 }
+
+// failingCredStore fails every SaveCredential call.
+type failingCredStore struct{ MockCredentialStore }
+
+func (failingCredStore) SaveCredential(context.Context, string, string, []byte) error {
+	return assert.AnError
+}
+
+func approveWithCredStore(t *testing.T, creds credentials.CredentialStore, payload string) *MockConfigStore {
+	t.Helper()
+	store := new(MockConfigStore)
+	store.On("GetAccountRegistration", mock.Anything, regCredRegID).Return(&config.AccountRegistration{
+		ID: regCredRegID, Status: "pending", Provider: "gcp", ExternalID: "test-project",
+		RegCredentialType: "gcp_workload_identity_config", RegCredentialPayload: payload,
+	}, nil)
+	store.On("ApproveAccountRegistration", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
+	store.On("ClearRegistrationCredentialPayload", mock.Anything, regCredRegID).Return(nil).Maybe()
+	h := &Handler{auth: new(MockAuthService), config: store, credStore: creds}
+	_, err := h.approveRegistration(context.Background(), &events.LambdaFunctionURLRequest{Body: gcpApprovalBody}, regCredRegID)
+	require.NoError(t, err)
+	return store
+}
+
+func TestApproveRegistration_ClearsPayloadAfterSave(t *testing.T) {
+	store := approveWithCredStore(t, &recordingCredStore{}, mustJSON(t, gcloudWIFConfig()))
+	store.AssertNumberOfCalls(t, "ClearRegistrationCredentialPayload", 1)
+	store.AssertCalled(t, "ClearRegistrationCredentialPayload", mock.Anything, regCredRegID)
+}
+
+func TestApproveRegistration_KeepsPayloadWhenSaveFails(t *testing.T) {
+	store := approveWithCredStore(t, &failingCredStore{}, mustJSON(t, gcloudWIFConfig()))
+	store.AssertNotCalled(t, "ClearRegistrationCredentialPayload", mock.Anything, mock.Anything)
+}
+
+func TestApproveRegistration_KeepsPayloadWhenNothingSaved(t *testing.T) {
+	store := approveWithCredStore(t, &recordingCredStore{}, "")
+	store.AssertNotCalled(t, "ClearRegistrationCredentialPayload", mock.Anything, mock.Anything)
+}

internal/config/interfaces.go

@@ -780,6 +780,9 @@ type StoreInterface interface {
 	// TransitionRegistrationStatus atomically updates a registration's workflow fields.
 	// actor is the UUID of the reviewer (nil for system-initiated transitions).
 	TransitionRegistrationStatus(ctx context.Context, reg *AccountRegistration, fromStatus string, actor *string) error
+	// ClearRegistrationCredentialPayload removes the encrypted credential kept
+	// with a registration. A missing id is not an error.
+	ClearRegistrationCredentialPayload(ctx context.Context, registrationID string) error
 	// ApproveAccountRegistration atomically approves a pending registration,
 	// creates its cloud account and links them. Returns ErrRegistrationConflict
 	// when the registration is no longer pending; on any error nothing is saved.

internal/config/store_postgres_registrations.go

@@ -219,6 +219,7 @@ func (s *PostgresStore) TransitionRegistrationStatus(ctx context.Context, reg *A
 			reviewed_by      = $5,
 			reviewed_at      = $6,
 			updated_at       = $7,
+			reg_credential_payload = CASE WHEN $10 THEN NULL ELSE reg_credential_payload END,
 			transitioned_by  = $9,
 			transitioned_at  = NOW()
 		WHERE id = $1 AND status = $8
@@ -239,6 +240,7 @@ func (s *PostgresStore) TransitionRegistrationStatus(ctx context.Context, reg *A
 		reg.UpdatedAt,
 		fromStatus,
 		actor,
+		reg.Status == "rejected",
 	)
 	if err != nil {
 		return fmt.Errorf("failed to transition account registration status: %w", err)
@@ -249,6 +251,17 @@ func (s *PostgresStore) TransitionRegistrationStatus(ctx context.Context, reg *A
 	return nil
 }
 
+// ClearRegistrationCredentialPayload removes the encrypted credential kept with
+// a registration once it has been saved elsewhere. A missing id affects no rows
+// and is not an error.
+func (s *PostgresStore) ClearRegistrationCredentialPayload(ctx context.Context, registrationID string) error {
+	_, err := s.db.Exec(ctx, `UPDATE account_registrations SET reg_credential_payload = NULL WHERE id = $1`, registrationID)
+	if err != nil {
+		return fmt.Errorf("failed to clear registration credential payload: %w", err)
+	}
+	return nil
+}
+
 // ApproveAccountRegistration moves a pending registration to approved, creates
 // its cloud account and links the two, all in one transaction. If any step
 // fails nothing is written, so a registration is never left approved with no

internal/config/store_postgres_registrations_integration_test.go

@@ -65,3 +65,56 @@ func TestCreateAccountRegistration_UniquenessPostgres(t *testing.T) {
 	require.NoError(t, err)
 	require.NoError(t, store.CreateAccountRegistration(ctx, newPendingRegistration("tok-137-d", "111111111111")))
 }
+
+func registrationPayload(t *testing.T, store *PostgresStore, id string) *string {
+	t.Helper()
+	var payload *string
+	require.NoError(t, store.db.QueryRow(context.Background(),
+		`SELECT reg_credential_payload FROM account_registrations WHERE id = $1`, id).Scan(&payload))
+	return payload
+}
+
+func newRegistrationWithPayload(token, externalID string) *AccountRegistration {
+	reg := newPendingRegistration(token, externalID)
+	reg.RegCredentialType = "gcp_workload_identity_config"
+	reg.RegCredentialPayload = "encrypted-blob"
+	return reg
+}
+
+func TestRegistrationCredentialPayload_ClearedAfterApprovalPostgres(t *testing.T) {
+	ctx := context.Background()
+	container := testhelpers.RequireMigratedDatabase(ctx, t, getTestMigrationsPath())
+	store := NewPostgresStore(container.DB)
+
+	reg := newRegistrationWithPayload("tok-clear-a", "111111111111")
+	require.NoError(t, store.CreateAccountRegistration(ctx, reg))
+	require.NotNil(t, registrationPayload(t, store, reg.ID))
+
+	account := &CloudAccount{Name: "Prod", Provider: "aws", ExternalID: "111111111111", Enabled: true}
+	require.NoError(t, store.ApproveAccountRegistration(ctx, reg, account, nil))
+	assert.NotNil(t, registrationPayload(t, store, reg.ID), "approval alone keeps the payload")
+
+	require.NoError(t, store.ClearRegistrationCredentialPayload(ctx, reg.ID))
+	assert.Nil(t, registrationPayload(t, store, reg.ID))
+}
+
+func TestRegistrationCredentialPayload_ClearedOnRejectionPostgres(t *testing.T) {
+	ctx := context.Background()
+	container := testhelpers.RequireMigratedDatabase(ctx, t, getTestMigrationsPath())
+	store := NewPostgresStore(container.DB)
+
+	reg := newRegistrationWithPayload("tok-clear-b", "222222222222")
+	require.NoError(t, store.CreateAccountRegistration(ctx, reg))
+
+	reg.Status = "rejected"
+	require.NoError(t, store.TransitionRegistrationStatus(ctx, reg, "pending", nil))
+	assert.Nil(t, registrationPayload(t, store, reg.ID))
+}
+
+func TestClearRegistrationCredentialPayload_MissingIDIsNoOpPostgres(t *testing.T) {
+	ctx := context.Background()
+	container := testhelpers.RequireMigratedDatabase(ctx, t, getTestMigrationsPath())
+	store := NewPostgresStore(container.DB)
+
+	require.NoError(t, store.ClearRegistrationCredentialPayload(ctx, "00000000-0000-0000-0000-000000000000"))
+}

internal/mocks/stores.go

@@ -2144,6 +2144,12 @@ func (m *MockConfigStore) TransitionRegistrationStatus(ctx context.Context, reg
 	return args.Error(0)
 }
 
+func (m *MockConfigStore) ClearRegistrationCredentialPayload(ctx context.Context, registrationID string) error {
+	m.record("ClearRegistrationCredentialPayload", ctx, registrationID)
+	args := m.Called(ctx, registrationID)
+	return args.Error(0)
+}
+
 func (m *MockConfigStore) ApproveAccountRegistration(ctx context.Context, reg *config.AccountRegistration, account *config.CloudAccount, actor *string) error {
 	m.record("ApproveAccountRegistration", ctx, reg, account, actor)
 	args := m.Called(ctx, reg, account, actor)