fix(api): validate registration credentials and GCP WIF endpoint URLs

internal/api/handler_registrations.go

@@ -93,14 +93,17 @@ func (h *Handler) submitRegistration(ctx context.Context, req *events.LambdaFunc
 		GCPClientEmail:      body.GCPClientEmail,
 		GCPAuthMode:         body.GCPAuthMode,
 		GCPWIFAudience:      body.GCPWIFAudience,
-		RegCredentialType:   body.CredentialType,
 	}
 
 	encrypted, encErr := h.encryptRegistrationCredential(body.CredentialPayload)
 	if encErr != nil {
 		return nil, fmt.Errorf("registrations: encrypt credential: %w", encErr)
 	}
-	reg.RegCredentialPayload = encrypted
+	if encrypted != "" {
+		// Without a credential store nothing is kept, so record no type either.
+		reg.RegCredentialType = body.CredentialType
+		reg.RegCredentialPayload = encrypted
+	}
 
 	if err := h.insertRegistration(ctx, reg); err != nil {
 		return nil, err
@@ -233,22 +236,6 @@ func (h *Handler) setReviewMetadata(ctx context.Context, reg *config.AccountRegi
 	}
 }
 
-// storeRegistrationCredentials decrypts and stores credentials from the registration
-// record into the account_credentials table. Errors are logged, not propagated.
-func (h *Handler) storeRegistrationCredentials(ctx context.Context, reg *config.AccountRegistration, accountID string) {
-	if reg.RegCredentialPayload == "" || reg.RegCredentialType == "" || h.credStore == nil {
-		return
-	}
-	plaintext, err := h.credStore.DecryptPayload(reg.RegCredentialPayload)
-	if err != nil {
-		logging.Warnf("registration %s: failed to decrypt credential payload: %v", reg.ID, err)
-		return
-	}
-	if err := h.credStore.SaveCredential(ctx, accountID, reg.RegCredentialType, plaintext); err != nil {
-		logging.Warnf("registration %s: failed to store credential: %v", reg.ID, err)
-	}
-}
-
 // encryptRegistrationCredential encrypts the credential payload for storage in the
 // registration table. Returns empty string if no credential is provided.
 func (h *Handler) encryptRegistrationCredential(payload string) (string, error) {
@@ -278,15 +265,9 @@ func (h *Handler) approveRegistration(ctx context.Context, httpReq *events.Lambd
 		return nil, err
 	}
 
-	var acctReq CloudAccountRequest
-	if err := json.Unmarshal([]byte(httpReq.Body), &acctReq); err != nil {
-		return nil, NewClientError(400, "invalid account request body")
-	}
-	if err := validateCloudAccountRequest(acctReq); err != nil {
-		return nil, err
-	}
-	// Before the transition, so a rejected config leaves the registration pending.
-	if err := h.requireHostForEmptyRoleARN(ctx, "(registration "+reg.ID+")", &acctReq); err != nil {
+	// Before the transition, so a rejected request leaves the registration pending.
+	acctReq, regCred, err := h.parseApprovalRequest(ctx, reg, httpReq)
+	if err != nil {
 		return nil, err
 	}
 
@@ -312,7 +293,7 @@ func (h *Handler) approveRegistration(ctx context.Context, httpReq *events.Lambd
 	// KMS-backed federated. Without this, every federated-path
 	// approval leaves the account disabled and the operator has to
 	// follow up with a manual PUT enabled=true.
-	account.Enabled = reg.RegCredentialPayload != "" || accountHasCredentialFreePath(account)
+	account.Enabled = regCred != nil || accountHasCredentialFreePath(account)
 	account.CreatedAt = now
 	account.UpdatedAt = now
 
@@ -321,7 +302,7 @@ func (h *Handler) approveRegistration(ctx context.Context, httpReq *events.Lambd
 	}
 
 	// Auto-store credentials if the registration included them.
-	h.storeRegistrationCredentials(ctx, reg, account.ID)
+	h.storeRegistrationCredentials(ctx, reg, account.ID, regCred)
 
 	// Link the cloud account and wipe the credential payload from the registration record.
 	reg.CloudAccountID = &account.ID
@@ -338,6 +319,25 @@ func (h *Handler) approveRegistration(ctx context.Context, httpReq *events.Lambd
 	return account, nil
 }
 
+// parseApprovalRequest reads and checks an approval body for reg: the
+// CloudAccountRequest itself, the host-account rule for an empty role ARN, and
+// the credential stored with the registration (see registrationCredential).
+// It returns the request and the checked credential, nil when there is none.
+func (h *Handler) parseApprovalRequest(ctx context.Context, reg *config.AccountRegistration, httpReq *events.LambdaFunctionURLRequest) (CloudAccountRequest, []byte, error) {
+	var acctReq CloudAccountRequest
+	if err := json.Unmarshal([]byte(httpReq.Body), &acctReq); err != nil {
+		return acctReq, nil, NewClientError(400, "invalid account request body")
+	}
+	if err := validateCloudAccountRequest(acctReq); err != nil {
+		return acctReq, nil, err
+	}
+	if err := h.requireHostForEmptyRoleARN(ctx, "(registration "+reg.ID+")", &acctReq); err != nil {
+		return acctReq, nil, err
+	}
+	regCred, err := h.registrationCredential(reg, acctReq.Provider)
+	return acctReq, regCred, err
+}
+
 // accountHasCredentialFreePath returns true when the account's auth
 // mode resolves a credential without any stored secret — either via
 // ambient instance credentials (role assumption, managed identity,
@@ -432,7 +432,9 @@ func (h *Handler) deleteRegistration(ctx context.Context, id string) (any, error
 const maxAccountNameLen = 256
 
 // validateRegistrationRequest checks required fields for a registration
-// submission. It also normalizes account_name in place: CR/LF characters are
+// submission, and the optional credential with parseRegistrationCredential,
+// replacing credential_payload in place with the checked encoding. It also
+// normalizes account_name in place: CR/LF characters are
 // stripped (defense-in-depth against email header injection at the data
 // source, complementing sanitizeHeader on the email subject path) and the
 // result is length-capped.
@@ -464,6 +466,11 @@ func validateRegistrationRequest(req *RegistrationRequest) error {
 	if !strings.Contains(req.ContactEmail, "@") || len(req.ContactEmail) < 5 {
 		return NewClientError(400, "contact_email must be a valid email address")
 	}
+	credential, err := parseRegistrationCredential(req.Provider, req.CredentialType, req.CredentialPayload)
+	if err != nil {
+		return err
+	}
+	req.CredentialPayload = string(credential)
 	return nil
 }
 

internal/api/handler_registrations_credentials.go

@@ -0,0 +1,102 @@
+package api
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/logging"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+)
+
+// credentialTypeProviders maps each credential_type to the provider whose
+// accounts use it.
+var credentialTypeProviders = map[string]string{
+	"aws_access_keys":              "aws",
+	"azure_client_secret":          "azure",
+	"gcp_service_account":          "gcp",
+	"gcp_workload_identity_config": "gcp",
+}
+
+// parseRegistrationCredential checks the optional credential_type and
+// credential_payload of a registration for provider. With both empty there is
+// no credential and it returns (nil, nil). Otherwise both are required, the
+// type must be one POST /api/accounts/:id/credentials accepts and must belong
+// to provider, and the payload must be a JSON object that passes
+// validateCredentialPayload, the same shape check that endpoint runs. It
+// returns the payload re-encoded from the checked object, so the bytes that
+// are stored are exactly the ones that were checked.
+func parseRegistrationCredential(provider, credType, payload string) ([]byte, error) {
+	if credType == "" && payload == "" {
+		return nil, nil
+	}
+	if err := checkRegistrationCredentialType(provider, credType, payload); err != nil {
+		return nil, err
+	}
+	var fields map[string]interface{}
+	if err := json.Unmarshal([]byte(payload), &fields); err != nil || fields == nil {
+		return nil, NewClientError(400, "credential_payload must be a JSON object")
+	}
+	if err := validateCredentialPayload(credType, fields); err != nil {
+		return nil, err
+	}
+	canonical, err := json.Marshal(fields)
+	if err != nil {
+		return nil, NewClientError(400, "credential_payload must be a JSON object")
+	}
+	return canonical, nil
+}
+
+// checkRegistrationCredentialType requires credential_type and
+// credential_payload together, and a credential_type that is known and
+// belongs to provider.
+func checkRegistrationCredentialType(provider, credType, payload string) error {
+	if credType == "" || payload == "" {
+		return NewClientError(400, "credential_type and credential_payload must be provided together")
+	}
+	if !validCredentialTypes[credType] {
+		return NewClientError(400, "credential_type must be one of: aws_access_keys, azure_client_secret, gcp_service_account, gcp_workload_identity_config")
+	}
+	if credentialTypeProviders[credType] != provider {
+		return NewClientError(400, fmt.Sprintf("credential_type %q cannot be used with provider %q", credType, provider))
+	}
+	return nil
+}
+
+// registrationCredential decrypts the credential stored with a pending
+// registration and checks it again with parseRegistrationCredential against
+// the provider of the account being approved. It runs before the
+// registration leaves "pending", so a credential that fails the check stops
+// the approval with a 400 the admin sees, and nothing is created or saved.
+// Returns (nil, nil) when the registration carries no credential.
+func (h *Handler) registrationCredential(reg *config.AccountRegistration, provider string) ([]byte, error) {
+	if reg.RegCredentialPayload == "" {
+		return nil, nil
+	}
+	if h.credStore == nil {
+		return nil, fmt.Errorf("registrations: credential store not configured")
+	}
+	plaintext, err := h.credStore.DecryptPayload(reg.RegCredentialPayload)
+	if err != nil {
+		return nil, fmt.Errorf("registrations: decrypt credential: %w", err)
+	}
+	canonical, err := parseRegistrationCredential(provider, reg.RegCredentialType, string(plaintext))
+	if err != nil {
+		return nil, NewClientError(400, "the credentials submitted with this registration are not valid ("+err.Error()+
+			"); reject the registration and ask the registrant to submit it again")
+	}
+	return canonical, nil
+}
+
+// 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) {
+	if payload == nil || h.credStore == nil {
+		return
+	}
+	if err := h.credStore.SaveCredential(ctx, accountID, reg.RegCredentialType, payload); err != nil {
+		logging.Warnf("registration %s: failed to store credential: %v", reg.ID, err)
+	}
+}