From bb1946afb11b7a5ec589d844c67b31ee54574512 Mon Sep 17 00:00:00 2001 From: Smart Mekiliuwa Date: Wed, 15 Apr 2026 10:52:21 +0100 Subject: [PATCH] fix(billing): block empty owner email during billing sync Prevent Convoy from creating billing organisations with blank owner email by short-circuiting sync on empty values and returning a clear 422 when owner email cannot be resolved in billing bootstrap paths. --- api/handlers/billing.go | 4 +++- services/create_organisation.go | 10 +++++++++- services/create_organisation_test.go | 18 ++++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/api/handlers/billing.go b/api/handlers/billing.go index feb3f84c19..540de279c1 100644 --- a/api/handlers/billing.go +++ b/api/handlers/billing.go @@ -23,6 +23,7 @@ import ( ) var ErrHostRequiredForBilling = errors.New("organisation host (assigned domain) is required for billing. Please set the assigned domain in the configuration") +var ErrOwnerEmailRequiredForBilling = errors.New("organisation owner email is required for billing") type BillingHandler struct { *Handler @@ -58,7 +59,8 @@ func (h *BillingHandler) ensureOrganisationInBilling(w http.ResponseWriter, r *h ownerEmail := h.getOwnerEmail(r.Context(), orgID) if ownerEmail == "" { - h.A.Logger.Warnf("Failed to fetch owner email for organisation %s, using empty billing_email", orgID) + _ = render.Render(w, r, util.NewErrorResponse(ErrOwnerEmailRequiredForBilling.Error(), http.StatusUnprocessableEntity)) + return true } orgData := billing.BillingOrganisation{ diff --git a/services/create_organisation.go b/services/create_organisation.go index af37df3c00..0795ce4390 100644 --- a/services/create_organisation.go +++ b/services/create_organisation.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strings" "time" "github.com/dchest/uniuri" @@ -122,10 +123,17 @@ func RunBillingOrganisationSync( if logger == nil { panic("RunBillingOrganisationSync: logger is required and must not be nil") } + + trimmedEmail := strings.TrimSpace(userEmail) + if trimmedEmail == "" { + logger.Error("create_organisation: owner email is empty, skipping billing sync", "org_id", org.UID) + return + } + orgData := billing.BillingOrganisation{ Name: org.Name, ExternalID: org.UID, - BillingEmail: userEmail, + BillingEmail: trimmedEmail, Host: billingHost, } resp, createErr := billingClient.CreateOrganisation(ctx, orgData) diff --git a/services/create_organisation_test.go b/services/create_organisation_test.go index 4b0e45f7fa..03fb8fe3b7 100644 --- a/services/create_organisation_test.go +++ b/services/create_organisation_test.go @@ -17,6 +17,14 @@ import ( log "github.com/frain-dev/convoy/pkg/logger" ) +type panicOnCreateBillingClient struct { + *billing.MockBillingClient +} + +func (panicOnCreateBillingClient) CreateOrganisation(context.Context, billing.BillingOrganisation) (*billing.Response[billing.BillingOrganisation], error) { + panic("CreateOrganisation must not be called when owner email is empty") +} + func provideCreateOrganisationService(ctrl *gomock.Controller, newOrg *datastore.OrganisationRequest, user *datastore.User) *CreateOrganisationService { return &CreateOrganisationService{ OrgRepo: mocks.NewMockOrganisationRepository(ctrl), @@ -215,4 +223,14 @@ func TestRunBillingOrganisationSync(t *testing.T) { RunBillingOrganisationSync(ctx, mockBilling, org, cfg, userEmail, billingHost, mockOrgRepo, log.New("convoy", log.LevelInfo)) }) + + t.Run("skips_billing_sync_when_owner_email_is_empty", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockOrgRepo := mocks.NewMockOrganisationRepository(ctrl) + mockBilling := panicOnCreateBillingClient{MockBillingClient: &billing.MockBillingClient{}} + + RunBillingOrganisationSync(ctx, &mockBilling, org, cfg, "", billingHost, mockOrgRepo, log.New("convoy", log.LevelInfo)) + }) }