diff --git a/internal/api/handler_registrations.go b/internal/api/handler_registrations.go index b101d4cda..8a3231226 100644 --- a/internal/api/handler_registrations.go +++ b/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 } diff --git a/internal/api/handler_registrations_credentials.go b/internal/api/handler_registrations_credentials.go new file mode 100644 index 000000000..a9bb24ebc --- /dev/null +++ b/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) + } +} diff --git a/internal/api/handler_registrations_credentials_test.go b/internal/api/handler_registrations_credentials_test.go new file mode 100644 index 000000000..567bdb085 --- /dev/null +++ b/internal/api/handler_registrations_credentials_test.go @@ -0,0 +1,222 @@ +package api + +// Tests for the credential that may come with POST /api/register: it is +// checked when submitted and again when the registration is approved, with +// the same rules as POST /api/accounts/:id/credentials. + +import ( + "context" + "encoding/json" + "testing" + + "github.com/aws/aws-lambda-go/events" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" +) + +const regCredRegID = "6f1c2a8e-3b4d-4e5f-8a9b-0c1d2e3f4a5b" + +func mustJSON(t *testing.T, v any) string { + t.Helper() + b, err := json.Marshal(v) + require.NoError(t, err) + return string(b) +} + +// registrationBody builds a POST /api/register body; credential fields are +// left out when empty. +func registrationBody(t *testing.T, provider, credType, credPayload string) string { + t.Helper() + body := map[string]any{ + "provider": provider, "external_id": "test-project", + "account_name": "Test", "contact_email": "owner@example.com", + } + if credType != "" { + body["credential_type"] = credType + } + if credPayload != "" { + body["credential_payload"] = credPayload + } + return mustJSON(t, body) +} + +// submitWithCredential runs submitRegistration with a store that records the +// created registration. It returns the stored registration (nil when none +// was created) and the handler error. +func submitWithCredential(t *testing.T, h *Handler, body string) (*config.AccountRegistration, error) { + t.Helper() + ctx := context.Background() + store := new(MockConfigStore) + var created *config.AccountRegistration + store.On("CreateAccountRegistration", ctx, mock.Anything).Run(func(args mock.Arguments) { + created = args.Get(1).(*config.AccountRegistration) + }).Return(nil).Maybe() + h.config = store + _, err := h.submitRegistration(ctx, &events.LambdaFunctionURLRequest{Body: body}) + return created, err +} + +func TestSubmitRegistration_WithoutCredentialIsAccepted(t *testing.T) { + reg, err := submitWithCredential(t, &Handler{credStore: &MockCredentialStore{}}, registrationBody(t, "gcp", "", "")) + require.NoError(t, err) + require.NotNil(t, reg) + assert.Empty(t, reg.RegCredentialType) + assert.Empty(t, reg.RegCredentialPayload) +} + +func TestSubmitRegistration_AcceptsValidCredential(t *testing.T) { + cases := map[string]struct{ provider, credType, payload string }{ + "gcp wif from gcloud": {"gcp", "gcp_workload_identity_config", mustJSON(t, gcloudWIFConfig())}, + "gcp wif with file source": {"gcp", "gcp_workload_identity_config", mustJSON(t, gcloudWIFConfigWith( + "credential_source", map[string]any{"file": "/var/run/secrets/azure/tokens/azure-identity-token"}))}, + "aws access keys": {"aws", "aws_access_keys", `{"access_key_id":"AKIAEXAMPLE","secret_access_key":"placeholder"}`}, + "azure secret": {"azure", "azure_client_secret", `{"client_secret":"placeholder"}`}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + reg, err := submitWithCredential(t, &Handler{credStore: &MockCredentialStore{}}, + registrationBody(t, tc.provider, tc.credType, tc.payload)) + require.NoError(t, err) + require.NotNil(t, reg) + assert.Equal(t, tc.credType, reg.RegCredentialType) + // MockCredentialStore "encrypts" to the plaintext. + assert.JSONEq(t, tc.payload, reg.RegCredentialPayload) + }) + } +} + +func TestSubmitRegistration_RejectsInvalidCredential(t *testing.T) { + wif := "gcp_workload_identity_config" + cases := map[string]struct{ provider, credType, payload, msg string }{ + "type without payload": {"gcp", wif, "", "must be provided together"}, + "payload without type": {"gcp", "", mustJSON(t, gcloudWIFConfig()), "must be provided together"}, + "unknown type": {"gcp", "gcp_oauth_client", `{"a":"b"}`, "credential_type must be one of"}, + "gcp type for aws account": {"aws", wif, mustJSON(t, gcloudWIFConfig()), `cannot be used with provider "aws"`}, + "aws type for gcp account": {"gcp", "aws_access_keys", `{"access_key_id":"a","secret_access_key":"b"}`, `cannot be used with provider "gcp"`}, + "azure type for aws account": {"aws", "azure_client_secret", `{"client_secret":"x"}`, `cannot be used with provider "aws"`}, + "junk payload": {"gcp", wif, "not json at all", "must be a JSON object"}, + "truncated payload": {"gcp", wif, `{"type":"external_account"`, "must be a JSON object"}, + "array payload": {"gcp", wif, `["external_account"]`, "must be a JSON object"}, + "null payload": {"gcp", wif, "null", "must be a JSON object"}, + "string payload": {"gcp", wif, `"external_account"`, "must be a JSON object"}, + "empty object": {"gcp", wif, "{}", "must not be empty"}, + "missing token_url": {"gcp", wif, mustJSON(t, gcloudWIFConfigWith("token_url", "")), `"token_url" must be a non-empty string`}, + "unknown key": {"gcp", wif, mustJSON(t, gcloudWIFConfigWith("token_info_url", "https://sts.googleapis.com/v1/introspect")), `unknown key "token_info_url"`}, + "wrong wif type": {"gcp", wif, mustJSON(t, gcloudWIFConfigWith("type", "service_account")), `type="external_account"`}, + "non-google token_url": {"gcp", wif, mustJSON(t, gcloudWIFConfigWith("token_url", "https://token.example.net/v1/token")), "token_url must be an https URL on sts.googleapis.com"}, + "http token_url": {"gcp", wif, mustJSON(t, gcloudWIFConfigWith("token_url", "http://sts.googleapis.com/v1/token")), "token_url must be an https URL"}, + "non-google impersonation": {"gcp", wif, mustJSON(t, gcloudWIFConfigWith("service_account_impersonation_url", + "https://iamcredentials.googleapis.com.example.net/v1/x")), "service_account_impersonation_url must be an https URL"}, + "service account missing key": {"gcp", "gcp_service_account", `{"type":"service_account","project_id":"p","client_email":"e"}`, `missing required key "private_key"`}, + "aws keys missing secret": {"aws", "aws_access_keys", `{"access_key_id":"a"}`, `missing required key "secret_access_key"`}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + reg, err := submitWithCredential(t, &Handler{credStore: &MockCredentialStore{}}, + registrationBody(t, tc.provider, tc.credType, tc.payload)) + require400(t, err, tc.msg) + assert.Nil(t, reg, "an invalid credential must not create a registration") + }) + } +} + +// Without a credential store nothing can be kept, so neither field is +// recorded on the registration (the payload was already dropped before). +func TestSubmitRegistration_NoCredentialStoreRecordsNoCredential(t *testing.T) { + reg, err := submitWithCredential(t, &Handler{}, + registrationBody(t, "gcp", "gcp_workload_identity_config", mustJSON(t, gcloudWIFConfig()))) + require.NoError(t, err) + require.NotNil(t, reg) + assert.Empty(t, reg.RegCredentialType) + assert.Empty(t, reg.RegCredentialPayload) +} + +// recordingCredStore records SaveCredential calls. +type recordingCredStore struct { + MockCredentialStore + saved map[string][]byte +} + +func (r *recordingCredStore) SaveCredential(_ context.Context, accountID, credType string, payload []byte) error { + if r.saved == nil { + r.saved = map[string][]byte{} + } + r.saved[accountID+"/"+credType] = payload + return nil +} + +const gcpApprovalBody = `{"name":"Test","provider":"gcp","external_id":"test-project",` + + `"gcp_auth_mode":"workload_identity_federation","gcp_client_email":"` + regCredSAEmail + `"}` + +// approveWithCredential approves a pending registration whose stored +// credential is credType/payload (MockCredentialStore "decrypts" to the +// stored text). +func approveWithCredential(t *testing.T, credType, payload, approvalBody string) (*MockConfigStore, *recordingCredStore, any, error) { + t.Helper() + ctx := context.Background() + store := new(MockConfigStore) + store.On("GetAccountRegistration", mock.Anything, regCredRegID).Return(&config.AccountRegistration{ + ID: regCredRegID, Status: "pending", Provider: "gcp", ExternalID: "test-project", + RegCredentialType: credType, RegCredentialPayload: payload, + }, nil) + store.On("TransitionRegistrationStatus", mock.Anything, mock.Anything, "pending", mock.Anything).Return(nil).Maybe() + store.On("UpdateAccountRegistration", mock.Anything, mock.Anything).Return(nil).Maybe() + mockAuth := new(MockAuthService) + setupAdminAuth(ctx, mockAuth) + creds := &recordingCredStore{} + h := &Handler{auth: mockAuth, config: store, credStore: creds} + result, err := h.approveRegistration(ctx, &events.LambdaFunctionURLRequest{Body: approvalBody}, regCredRegID) + return store, creds, result, err +} + +func TestApproveRegistration_SavesCheckedCredential(t *testing.T) { + payload := mustJSON(t, gcloudWIFConfig()) + store, creds, result, err := approveWithCredential(t, "gcp_workload_identity_config", payload, gcpApprovalBody) + require.NoError(t, err) + account, ok := result.(*config.CloudAccount) + require.True(t, ok) + assert.True(t, account.Enabled) + store.AssertCalled(t, "CreateCloudAccount", mock.Anything, mock.Anything) + require.Contains(t, creds.saved, account.ID+"/gcp_workload_identity_config") + assert.JSONEq(t, payload, string(creds.saved[account.ID+"/gcp_workload_identity_config"])) +} + +func TestApproveRegistration_WithoutCredentialSavesNothing(t *testing.T) { + _, creds, _, err := approveWithCredential(t, "", "", gcpApprovalBody) + require.NoError(t, err) + assert.Empty(t, creds.saved) +} + +// A registration stored before submit-time checks existed can still hold a +// credential that fails them. Approval refuses it with a 400 and leaves the +// registration pending: nothing is transitioned, created or saved. +func TestApproveRegistration_RefusesInvalidStoredCredential(t *testing.T) { + wif := "gcp_workload_identity_config" + cases := map[string]struct{ credType, payload, body, msg string }{ + "non-google token_url": {wif, mustJSON(t, gcloudWIFConfigWith("token_url", "https://token.example.net/v1/token")), + gcpApprovalBody, "token_url must be an https URL on sts.googleapis.com"}, + "non-google impersonation url": {wif, mustJSON(t, gcloudWIFConfigWith("service_account_impersonation_url", "https://my-iamcredentials.googleapis.com/v1/x")), + gcpApprovalBody, "service_account_impersonation_url must be an https URL"}, + "junk payload": {wif, "not json", gcpApprovalBody, "must be a JSON object"}, + "payload without type": {"", mustJSON(t, gcloudWIFConfig()), gcpApprovalBody, "must be provided together"}, + "unknown type": {"gcp_oauth_client", `{"a":"b"}`, gcpApprovalBody, "credential_type must be one of"}, + "wrong type in wif slot": {wif, `{"type":"service_account","project_id":"p","private_key":"k","client_email":"e"}`, gcpApprovalBody, "unknown key"}, + "type does not match approved provider": {wif, mustJSON(t, gcloudWIFConfig()), + `{"name":"Test","provider":"azure","external_id":"00000000-0000-0000-0000-000000000000","azure_auth_mode":"managed_identity"}`, + `cannot be used with provider "azure"`}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + store, creds, result, err := approveWithCredential(t, tc.credType, tc.payload, tc.body) + assert.Nil(t, result) + require400(t, err, "the credentials submitted with this registration are not valid") + require400(t, err, tc.msg) + store.AssertNotCalled(t, "TransitionRegistrationStatus", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + store.AssertNotCalled(t, "CreateCloudAccount", mock.Anything, mock.Anything) + assert.Empty(t, creds.saved) + }) + } +} diff --git a/internal/api/validation.go b/internal/api/validation.go index dd6575dbb..20476d47f 100644 --- a/internal/api/validation.go +++ b/internal/api/validation.go @@ -15,6 +15,7 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/LeanerCloud/cloud-commitments-platform/internal/credentials" ) // Security constants. @@ -286,6 +287,29 @@ func validateGCPWIFPayload(payload map[string]interface{}) error { return err } } + return validateGCPWIFEndpointKeys(payload) +} + +// validateGCPWIFEndpointKeys checks the config's token exchange and +// service-account impersonation URLs with credentials.ValidateGCPWIFEndpoints, +// the same check the resolver runs before it uses a stored config. Runs after +// the required-key checks, so token_url is a non-empty string here. +func validateGCPWIFEndpointKeys(payload map[string]interface{}) error { + tokenURL, ok := payload["token_url"].(string) + if !ok { + return NewClientError(400, "credentials payload key \"token_url\" must be a non-empty string") + } + impersonationURL := "" + if v, ok := payload["service_account_impersonation_url"]; ok { + s, isStr := v.(string) + if !isStr || strings.TrimSpace(s) == "" { + return NewClientError(400, "credentials payload key \"service_account_impersonation_url\" must be a non-empty string") + } + impersonationURL = s + } + if err := credentials.ValidateGCPWIFEndpoints(tokenURL, impersonationURL); err != nil { + return NewClientError(400, "credentials payload for gcp_workload_identity_config: "+err.Error()) + } return nil } diff --git a/internal/api/validation_gcpwif_test.go b/internal/api/validation_gcpwif_test.go new file mode 100644 index 000000000..61d65870a --- /dev/null +++ b/internal/api/validation_gcpwif_test.go @@ -0,0 +1,76 @@ +package api + +// Tests for the endpoint URLs validateCredentialPayload accepts in a +// gcp_workload_identity_config payload. + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const regCredSAEmail = "cudly@test-project.iam.gserviceaccount.com" + +// gcloudWIFConfig is the external_account config the gcp-target IaC +// (iac/federation/gcp-target/terraform/registration.tf) and gcloud generate +// for an AWS-hosted deployment. +func gcloudWIFConfig() map[string]any { + return map[string]any{ + "type": "external_account", + "audience": "//iam.googleapis.com/projects/1/locations/global/workloadIdentityPools/cudly/providers/cudly", + "subject_token_type": "urn:ietf:params:aws:token-type:aws4_request", + "service_account_impersonation_url": "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/" + regCredSAEmail + ":generateAccessToken", + "token_url": "https://sts.googleapis.com/v1/token", + "credential_source": map[string]any{ + "environment_id": "aws1", + "region_url": "http://169.254.169.254/latest/meta-data/placement/availability-zone", + "url": "http://169.254.169.254/latest/meta-data/iam/security-credentials", + "regional_cred_verification_url": "https://sts.{region}.amazonaws.com?Action=GetCallerIdentity&Version=2011-06-15", + }, + } +} + +// gcloudWIFConfigWith returns gcloudWIFConfig with key set to value. +func gcloudWIFConfigWith(key string, value any) map[string]any { + cfg := gcloudWIFConfig() + cfg[key] = value + return cfg +} + +func require400(t *testing.T, err error, msg string) { + t.Helper() + ce, ok := IsClientError(err) + require.True(t, ok, "want a client error, got %v", err) + assert.Equal(t, 400, ce.code) + assert.Contains(t, ce.message, msg) +} + +func TestValidateCredentialPayload_GCPWIFEndpoints(t *testing.T) { + require.NoError(t, validateCredentialPayload("gcp_workload_identity_config", gcloudWIFConfig())) + + noImpersonation := gcloudWIFConfig() + delete(noImpersonation, "service_account_impersonation_url") + require.NoError(t, validateCredentialPayload("gcp_workload_identity_config", noImpersonation)) + + cases := map[string]struct { + key string + value any + msg string + }{ + "token_url look-alike suffix": {"token_url", "https://sts.googleapis.com.example.net/v1/token", "token_url must be an https URL on sts.googleapis.com"}, + "token_url look-alike prefix": {"token_url", "https://my-sts.googleapis.com/v1/token", "token_url must be an https URL on sts.googleapis.com"}, + "token_url userinfo": {"token_url", "https://sts.googleapis.com@token.example.net/v1/token", "token_url must be an https URL"}, + "token_url port": {"token_url", "https://sts.googleapis.com:8443/v1/token", "token_url must be an https URL"}, + "token_url http": {"token_url", "http://sts.googleapis.com/v1/token", "token_url must be an https URL"}, + "impersonation url other host": {"service_account_impersonation_url", "https://token.example.net/v1/x", "service_account_impersonation_url must be an https URL"}, + "impersonation url not a string": {"service_account_impersonation_url", 42, `"service_account_impersonation_url" must be a non-empty string`}, + "impersonation url empty": {"service_account_impersonation_url", " ", `"service_account_impersonation_url" must be a non-empty string`}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + err := validateCredentialPayload("gcp_workload_identity_config", gcloudWIFConfigWith(tc.key, tc.value)) + require400(t, err, tc.msg) + }) + } +} diff --git a/internal/credentials/gcp_endpoints.go b/internal/credentials/gcp_endpoints.go new file mode 100644 index 000000000..847958be3 --- /dev/null +++ b/internal/credentials/gcp_endpoints.go @@ -0,0 +1,81 @@ +package credentials + +import ( + "encoding/json" + "fmt" + "net/url" +) + +// Hosts a GCP external_account (workload identity federation) config may name +// for its token exchange and service-account impersonation endpoints. These +// are the values gcloud writes into a generated credential config, and the +// same endpoints BuildGCPFederatedCredential uses. +const ( + gcpSTSHost = "sts.googleapis.com" + gcpIAMCredentialsHost = "iamcredentials.googleapis.com" +) + +// gcpExternalAccountType is the "type" of a GCP external_account credential +// config. +const gcpExternalAccountType = "external_account" + +// ValidateGCPWIFEndpoints checks the endpoint URLs of a GCP external_account +// credential config. token_url is required and must be an https URL whose +// host is exactly sts.googleapis.com. service_account_impersonation_url is +// optional; when set it must be an https URL whose host is exactly +// iamcredentials.googleapis.com. Neither URL may carry user info or an +// explicit port. The returned error names the field and the expected form +// only, never the submitted value. +func ValidateGCPWIFEndpoints(tokenURL, impersonationURL string) error { + if tokenURL == "" { + return fmt.Errorf("token_url is required") + } + if err := checkGCPEndpoint("token_url", tokenURL, gcpSTSHost); err != nil { + return err + } + if impersonationURL == "" { + return nil + } + return checkGCPEndpoint("service_account_impersonation_url", impersonationURL, gcpIAMCredentialsHost) +} + +// checkGCPEndpoint requires raw to parse as an absolute https URL whose +// authority is exactly host: no user info, no port, no other host. +func checkGCPEndpoint(field, raw, host string) error { + u, err := url.Parse(raw) + if err != nil || u.Scheme != "https" || u.User != nil || u.Host != host { + return fmt.Errorf("%s must be an https URL on %s with no user info or port", field, host) + } + return nil +} + +// storedGCPExternalAccount holds the fields of a stored GCP credential JSON +// that ValidateGCPWIFEndpoints checks. The JSON tags match the ones +// golang.org/x/oauth2/google decodes the same file with, and decoding into a +// struct (not a map) matches keys the same way, so both read the same values. +type storedGCPExternalAccount struct { + Type string `json:"type"` + TokenURL string `json:"token_url"` + ServiceAccountImpersonationURL string `json:"service_account_impersonation_url"` +} + +// checkStoredGCPEndpoints checks a stored GCP credential JSON before a token +// source is built from it: when its type is external_account, its endpoints +// must pass ValidateGCPWIFEndpoints. Other types are left to the parser. +// Empty input passes so the caller can report the missing credential itself. +func checkStoredGCPEndpoints(accountID string, raw []byte) error { + if len(raw) == 0 { + return nil + } + var cfg storedGCPExternalAccount + if err := json.Unmarshal(raw, &cfg); err != nil { + return fmt.Errorf("credentials: parse gcp credentials for account %s: %w", accountID, err) + } + if cfg.Type != gcpExternalAccountType { + return nil + } + if err := ValidateGCPWIFEndpoints(cfg.TokenURL, cfg.ServiceAccountImpersonationURL); err != nil { + return fmt.Errorf("credentials: stored gcp workload identity config for account %s is not usable: %w; save the config again", accountID, err) + } + return nil +} diff --git a/internal/credentials/gcp_endpoints_test.go b/internal/credentials/gcp_endpoints_test.go new file mode 100644 index 000000000..b91415c81 --- /dev/null +++ b/internal/credentials/gcp_endpoints_test.go @@ -0,0 +1,209 @@ +package credentials + +import ( + "context" + "encoding/json" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + testSTSURL = "https://sts.googleapis.com/v1/token" + testImpersonationURL = "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/sa@test-project.iam.gserviceaccount.com:generateAccessToken" +) + +// endpointCase is a token_url or service_account_impersonation_url value +// that ValidateGCPWIFEndpoints must refuse. +type endpointCase struct{ name, url string } + +// badTokenURLs are token_url values that are not exactly +// https://sts.googleapis.com/...: other schemes, look-alike hosts, user info +// and explicit ports. +var badTokenURLs = []endpointCase{ + {"http scheme", "http://sts.googleapis.com/v1/token"}, + {"other scheme", "ftp://sts.googleapis.com/v1/token"}, + {"no scheme", "sts.googleapis.com/v1/token"}, + {"scheme-relative", "//sts.googleapis.com/v1/token"}, + {"suffix look-alike", "https://sts.googleapis.com.example.net/v1/token"}, + {"prefix look-alike", "https://my-sts.googleapis.com/v1/token"}, + {"other google host", "https://iamcredentials.googleapis.com/v1/token"}, + {"other host", "https://token.example.net/v1/token"}, + {"trailing dot", "https://sts.googleapis.com./v1/token"}, + {"userinfo before host", "https://sts.googleapis.com@token.example.net/v1/token"}, + {"userinfo on google host", "https://user:pass@sts.googleapis.com/v1/token"}, + {"explicit default port", "https://sts.googleapis.com:443/v1/token"}, + {"explicit other port", "https://sts.googleapis.com:8443/v1/token"}, + {"empty port", "https://sts.googleapis.com:/v1/token"}, + {"ip literal", "https://127.0.0.1/v1/token"}, + {"ipv6 literal", "https://[::1]/v1/token"}, + {"uppercase host", "https://STS.GOOGLEAPIS.COM/v1/token"}, + {"unparsable", "https://sts.googleapis.com/%zz"}, +} + +var badImpersonationURLs = []endpointCase{ + {"http scheme", "http://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/sa:generateAccessToken"}, + {"suffix look-alike", "https://iamcredentials.googleapis.com.example.net/v1/x"}, + {"prefix look-alike", "https://my-iamcredentials.googleapis.com/v1/x"}, + {"sts host", "https://sts.googleapis.com/v1/x"}, + {"userinfo before host", "https://iamcredentials.googleapis.com@token.example.net/v1/x"}, + {"explicit port", "https://iamcredentials.googleapis.com:443/v1/x"}, +} + +func TestValidateGCPWIFEndpoints_AcceptsGoogleEndpoints(t *testing.T) { + require.NoError(t, ValidateGCPWIFEndpoints(testSTSURL, testImpersonationURL)) + require.NoError(t, ValidateGCPWIFEndpoints(testSTSURL, ""), "impersonation URL is optional") + require.NoError(t, ValidateGCPWIFEndpoints("https://sts.googleapis.com/v1/token?x=1", "")) +} + +func TestValidateGCPWIFEndpoints_RequiresTokenURL(t *testing.T) { + err := ValidateGCPWIFEndpoints("", testImpersonationURL) + require.Error(t, err) + assert.Contains(t, err.Error(), "token_url is required") +} + +func TestValidateGCPWIFEndpoints_RejectsBadTokenURL(t *testing.T) { + for _, tc := range badTokenURLs { + t.Run(tc.name, func(t *testing.T) { + err := ValidateGCPWIFEndpoints(tc.url, testImpersonationURL) + require.Error(t, err) + assert.Contains(t, err.Error(), "token_url must be an https URL on sts.googleapis.com") + }) + } +} + +func TestValidateGCPWIFEndpoints_RejectsBadImpersonationURL(t *testing.T) { + for _, tc := range badImpersonationURLs { + t.Run(tc.name, func(t *testing.T) { + err := ValidateGCPWIFEndpoints(testSTSURL, tc.url) + require.Error(t, err) + assert.Contains(t, err.Error(), "service_account_impersonation_url must be an https URL on iamcredentials.googleapis.com") + }) + } +} + +// wifConfigJSON returns a gcloud-style external_account config (the shape +// `gcloud iam workload-identity-pools create-cred-config` writes) with the +// given endpoints. An empty impersonation URL leaves the key out. +func wifConfigJSON(t *testing.T, tokenURL, impersonationURL string) []byte { + t.Helper() + cfg := map[string]any{ + "universe_domain": "googleapis.com", + "type": "external_account", + "audience": "//iam.googleapis.com/projects/1/locations/global/workloadIdentityPools/pool/providers/provider", + "subject_token_type": "urn:ietf:params:oauth:token-type:jwt", + "token_url": tokenURL, + "credential_source": map[string]any{ + "file": "/var/run/secrets/azure/tokens/azure-identity-token", + "format": map[string]any{"type": "text"}, + }, + } + if impersonationURL != "" { + cfg["service_account_impersonation_url"] = impersonationURL + } + raw, err := json.Marshal(cfg) + require.NoError(t, err) + return raw +} + +// loadStoredWIF resolves a WIF-mode account whose stored config is raw. With +// federated set, the options and audience would allow the secret-free path, +// which a stored config takes priority over. +func loadStoredWIF(t *testing.T, raw []byte, federated bool) error { + t.Helper() + store := newMockStore() + store.data["acct1/"+CredTypeGCPWIFConfig] = raw + account := &config.CloudAccount{ID: "acct1", GCPAuthMode: "workload_identity_federation"} + opts := GCPResolveOptions{} + if federated { + account.GCPWIFAudience = "//iam.googleapis.com/projects/1/locations/global/workloadIdentityPools/pool/providers/provider" + opts = GCPResolveOptions{Signer: stubOIDCSigner{}, IssuerURL: "https://issuer.example.com"} + } + src, err := ResolveGCPTokenSourceWithOpts(context.Background(), account, store, opts) + if err == nil { + require.NotNil(t, src) + } else { + require.Nil(t, src) + } + return err +} + +func TestLoadStoredGCPTokenSource_WIF_AcceptsGcloudConfig(t *testing.T) { + require.NoError(t, loadStoredWIF(t, wifConfigJSON(t, testSTSURL, testImpersonationURL), false)) + require.NoError(t, loadStoredWIF(t, wifConfigJSON(t, testSTSURL, ""), false)) + require.NoError(t, loadStoredWIF(t, wifConfigJSON(t, testSTSURL, testImpersonationURL), true)) +} + +func TestLoadStoredGCPTokenSource_WIF_RejectsBadEndpoints(t *testing.T) { + type stored struct{ name, tokenURL, impURL string } + cases := make([]stored, 0, len(badTokenURLs)+len(badImpersonationURLs)+1) + for _, tc := range badTokenURLs { + cases = append(cases, stored{"token_url/" + tc.name, tc.url, testImpersonationURL}) + } + for _, tc := range badImpersonationURLs { + cases = append(cases, stored{"impersonation/" + tc.name, testSTSURL, tc.url}) + } + cases = append(cases, stored{"token_url/missing", "", testImpersonationURL}) + for _, tc := range cases { + for _, federated := range []bool{false, true} { + t.Run(tc.name, func(t *testing.T) { + err := loadStoredWIF(t, wifConfigJSON(t, tc.tokenURL, tc.impURL), federated) + require.Error(t, err) + assert.Contains(t, err.Error(), "stored gcp workload identity config for account acct1 is not usable") + assert.NotContains(t, err.Error(), "example.net", "the error must not echo the stored URL") + }) + } + } +} + +// Decoding matches x/oauth2: JSON keys match struct tags case-insensitively +// and the last duplicate wins, so the checked value is the one it would use. +func TestLoadStoredGCPTokenSource_WIF_ChecksTheValueTheParserUses(t *testing.T) { + cases := map[string]string{ + "case variant key": `{"type":"external_account","audience":"a","subject_token_type":"s",` + + `"token_url":"` + testSTSURL + `","Token_URL":"https://token.example.net/v1/token","credential_source":{"file":"x"}}`, + "duplicate key": `{"type":"external_account","audience":"a","subject_token_type":"s",` + + `"token_url":"` + testSTSURL + `","token_url":"https://token.example.net/v1/token","credential_source":{"file":"x"}}`, + } + for name, raw := range cases { + t.Run(name, func(t *testing.T) { + err := loadStoredWIF(t, []byte(raw), false) + require.Error(t, err) + assert.Contains(t, err.Error(), "is not usable") + }) + } +} + +func TestLoadStoredGCPTokenSource_WIF_MalformedFailsWithoutPanic(t *testing.T) { + for name, raw := range map[string]string{ + "not json": "not valid json", + "token_url not text": `{"type":"external_account","token_url":7}`, + "type not text": `{"type":["external_account"]}`, + "top level not object": `["external_account"]`, + } { + t.Run(name, func(t *testing.T) { + err := loadStoredWIF(t, []byte(raw), false) + require.Error(t, err) + assert.Contains(t, err.Error(), "parse gcp credentials") + }) + } +} + +// The service-account slot never needs an external_account config; if one +// is stored there it gets the same endpoint check before it is used. +func TestLoadStoredGCPTokenSource_ServiceAccountSlot_ChecksExternalAccountEndpoints(t *testing.T) { + store := newMockStore() + store.data["acct1/"+CredTypeGCPServiceAccount] = wifConfigJSON(t, "https://token.example.net/v1/token", "") + account := &config.CloudAccount{ID: "acct1", GCPAuthMode: "service_account_key"} + _, err := ResolveGCPTokenSource(context.Background(), account, store) + require.Error(t, err) + assert.Contains(t, err.Error(), "is not usable") +} + +func TestLoadStoredGCPTokenSource_WIF_EmptyStillReportsMissing(t *testing.T) { + err := loadStoredWIF(t, nil, false) + require.Error(t, err) + assert.Contains(t, err.Error(), "no gcp credentials stored") +} diff --git a/internal/credentials/resolver.go b/internal/credentials/resolver.go index d2863c3b9..865364172 100644 --- a/internal/credentials/resolver.go +++ b/internal/credentials/resolver.go @@ -471,6 +471,9 @@ func loadStoredGCPTokenSource( if err != nil { return nil, fmt.Errorf("credentials: load gcp credentials for account %s: %w", account.ID, err) } + if checkErr := checkStoredGCPEndpoints(account.ID, raw); checkErr != nil { + return nil, checkErr + } if raw == nil { return nil, fmt.Errorf("credentials: no gcp credentials stored for account %s", account.ID) }