From 12af3af3b3596c199a5ef9846cf64ef1012e045a Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Mon, 3 Aug 2026 14:53:30 +0100 Subject: [PATCH] Harden Square HTTP client and dev mock: status codes, token validation, deadline wire format Add StatusCode/Category/Field to squareAPIError and an IsNotFound helper so 400/401/404/429/5xx are distinguishable structurally instead of by substring. Validate cnon:/ccof: token prefixes in createPayment/createCardOnFile (PCI parity with the mock). Reject ccof charges without customer_id in the mock so dev parity catches the production bug. Emit Deadline as the RFC 3339 duration (PT5M) and correct the deprecated-comment. --- backend/internal/square/square_dev.go | 33 +++- backend/internal/square/square_dev_test.go | 84 ++++++-- backend/internal/square/square_http_client.go | 136 +++++++++++-- .../square/square_http_client_test.go | 183 ++++++++++++++++++ backend/internal/square/types.go | 2 +- 5 files changed, 399 insertions(+), 39 deletions(-) diff --git a/backend/internal/square/square_dev.go b/backend/internal/square/square_dev.go index a9a0f50..ccb4a81 100644 --- a/backend/internal/square/square_dev.go +++ b/backend/internal/square/square_dev.go @@ -6,8 +6,10 @@ import ( "context" "crussell/clock" "crypto/sha256" + "errors" "fmt" "log" + "net/http" "os" "strings" "sync" @@ -44,6 +46,10 @@ type MockClient struct { // prod-only pending-refund branch (normally only reachable against the // real Square API) can be exercised in dev/tests. ForceRefundPending bool + // FailCreateCheckout makes CreateCheckout return an error so the handler's + // post-insert CreateCheckout-failure path (marking the provisional + // terminal_checkouts row failed) can be exercised in dev/tests. + FailCreateCheckout bool } type devProdClient struct{} @@ -128,7 +134,19 @@ func (m *MockClient) CreatePayment(ctx context.Context, req CreatePaymentReq) (* // ccof:xxx card ID). Raw PANs are rejected exactly as Square would, so the // mock behaves identically to production (PCI-DSS parity). if !isTokenLike(req.SourceID) { - return nil, fmt.Errorf("invalid source_id: %q — use a card nonce (cnon:xxx) or card ID (ccof:xxx)", req.SourceID) + return nil, fmt.Errorf("invalid source_id: %s — use a card nonce (cnon:xxx) or card ID (ccof:xxx)", tokenPrefix(req.SourceID)) + } + // Square requires customer_id when charging a card-on-file (ccof:) token. + // The mock enforces the same rule so dev parity catches the production bug + // where a saved-card charge is sent without the customer's Square customer + // id (real Square rejects it with a 400 INVALID_REQUEST_ERROR). + if strings.HasPrefix(req.SourceID, "ccof:") && req.CustomerID == "" { + return nil, &squareAPIError{ + Code: "INVALID_REQUEST_ERROR", + Detail: "customer_id required for card-on-file source", + StatusCode: http.StatusBadRequest, + err: errors.New("square: customer_id required for card-on-file source"), + } } // Do NOT log the full source token — it is a single-use nonce (cnon:) or a // card reference (ccof:) that could be replayed. Log only its prefix and @@ -223,6 +241,9 @@ func (m *MockClient) CreatePayment(ctx context.Context, req CreatePaymentReq) (* } func (m *MockClient) CreateCheckout(ctx context.Context, req CreateCheckoutReq) (*CheckoutResult, error) { + if m.FailCreateCheckout { + return nil, fmt.Errorf("mock: checkout creation failed (simulated failure)") + } log.Printf("[SQUARE-MOCK] CreateCheckout: amount=%d, allowTipping=%v, reference=%s", req.Amount, req.AllowTipping, req.ReferenceID) now := clock.Now().UTC() @@ -237,7 +258,7 @@ func (m *MockClient) CreateCheckout(ctx context.Context, req CreateCheckoutReq) Note: req.Note, CreatedAt: now.Format(time.RFC3339), UpdatedAt: now.Format(time.RFC3339), - Deadline: now.Add(5 * time.Minute).Format(time.RFC3339), + Deadline: "PT5M", // deadline_duration wire format: RFC 3339 duration, not a timestamp } m.mu.Lock() @@ -429,7 +450,7 @@ func (m *MockClient) CreateCardOnFile(ctx context.Context, userID, cardToken, cu // ccof:xxx card ID). Raw PANs are rejected exactly as Square would, so the // mock behaves identically to production. if !isTokenLike(cardToken) { - return nil, fmt.Errorf("invalid source_id: %q — use a card nonce (cnon:xxx) or card ID (ccof:xxx)", cardToken) + return nil, fmt.Errorf("invalid source_id: %s — use a card nonce (cnon:xxx) or card ID (ccof:xxx)", tokenPrefix(cardToken)) } m.mu.Lock() @@ -520,12 +541,6 @@ func (m *MockClient) ListPaymentRefunds(ctx context.Context, paymentID string, b return out, nil } -// isTokenLike returns true for Square source_id tokens: cnon:xxx nonces and -// ccof:xxx card IDs. Raw PANs (all digits) are NOT token-like and are rejected. -func isTokenLike(s string) bool { - return strings.HasPrefix(s, "cnon:") || strings.HasPrefix(s, "ccof:") -} - // redactedEmail masks a customer email for dev logs (PII, S-2 convention): // only the first two characters of the local part plus the domain are shown, // e.g. "ja***@example.com". Malformed addresses fall back to "[redacted]". diff --git a/backend/internal/square/square_dev_test.go b/backend/internal/square/square_dev_test.go index 264cc98..17bd1c5 100644 --- a/backend/internal/square/square_dev_test.go +++ b/backend/internal/square/square_dev_test.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "log" + "net/http" "os" "strings" "sync" @@ -88,6 +89,28 @@ func TestDevClient_CreateCheckout_PendingThenCompleted(t *testing.T) { assert.NotEmpty(t, completed.EntryMethod) } +// TestDevClient_CreateCheckout_DeadlineDurationFormat verifies the mock emits +// Square's deadline_duration wire format — an RFC 3339 duration ("PT5M"), NOT +// an absolute RFC3339 timestamp — so dev parity matches the real API. +func TestDevClient_CreateCheckout_DeadlineDurationFormat(t *testing.T) { + client := NewDevClient().(*MockClient) + ctx := context.Background() + + result, err := client.CreateCheckout(ctx, CreateCheckoutReq{ + Amount: 5000, + Currency: "GBP", + IdempotencyKey: "checkout-deadline", + ReferenceID: "deadline-ref", + }) + require.NoError(t, err) + assert.Equal(t, "PT5M", result.Deadline, "deadline_duration must be an RFC 3339 duration, not a timestamp") + + // Round-trip through checkoutFromSquare: the wire value is copied through + // unchanged (it is not parsed/reformatted anywhere in the package). + res := checkoutFromSquare(&sqTerminalCheckout{Deadline: result.Deadline}) + assert.Equal(t, "PT5M", res.Deadline) +} + func TestDevClient_CreateCheckout_NoTip(t *testing.T) { client := NewDevClient().(*MockClient) @@ -872,20 +895,55 @@ func TestDevClient_CreatePayment_RejectsRawPAN(t *testing.T) { {"amex", "378282246310005"}, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result, err := client.CreatePayment(ctx, CreatePaymentReq{ - Amount: 5000, - Currency: "GBP", - SourceID: tt.pan, - IdempotencyKey: "raw-pan-" + tt.name, - ReferenceID: "booking-raw", + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := client.CreatePayment(ctx, CreatePaymentReq{ + Amount: 5000, + Currency: "GBP", + SourceID: tt.pan, + IdempotencyKey: "raw-pan-" + tt.name, + ReferenceID: "booking-raw", + }) + require.Error(t, err, "raw PAN must be rejected for production parity") + assert.Nil(t, result) + assert.Contains(t, err.Error(), "invalid source_id") }) - require.Error(t, err, "raw PAN must be rejected for production parity") - assert.Nil(t, result) - assert.Contains(t, err.Error(), "invalid source_id") - }) - } + } +} + +// TestDevClient_CreatePayment_CardOnFileRequiresCustomerID verifies the mock +// mirrors Square's real enforcement: charging a ccof: (card-on-file) token +// without a customer_id is rejected with a structured 400 INVALID_REQUEST_ERROR +// (this is the exact production bug the mock must catch in dev), while the same +// charge with a customer_id succeeds as ON_FILE. +func TestDevClient_CreatePayment_CardOnFileRequiresCustomerID(t *testing.T) { + client := NewDevClient().(*MockClient) + ctx := context.Background() + + _, err := client.CreatePayment(ctx, CreatePaymentReq{ + Amount: 5000, + Currency: "GBP", + SourceID: "ccof:mock_saved", + IdempotencyKey: "ccof-no-customer", + ReferenceID: "booking-ccof-no-customer", + }) + require.Error(t, err, "ccof charge without customer_id must be rejected") + assert.Equal(t, "INVALID_REQUEST_ERROR", ErrorCode(err)) + assert.Contains(t, ErrorDetail(err), "customer_id required") + assert.Equal(t, http.StatusBadRequest, ErrorStatusCode(err)) + + result, err := client.CreatePayment(ctx, CreatePaymentReq{ + Amount: 5000, + Currency: "GBP", + SourceID: "ccof:mock_saved", + IdempotencyKey: "ccof-with-customer", + ReferenceID: "booking-ccof-with-customer", + CustomerID: "cus_mock_1", + }) + require.NoError(t, err) + assert.Equal(t, "COMPLETED", result.Status) + assert.Equal(t, "ON_FILE", result.EntryMethod) + assert.Equal(t, "cus_mock_1", result.CustomerID) } func TestDevClient_RefundPayment_ForcePending(t *testing.T) { diff --git a/backend/internal/square/square_http_client.go b/backend/internal/square/square_http_client.go index 32b1cd3..a05e58e 100644 --- a/backend/internal/square/square_http_client.go +++ b/backend/internal/square/square_http_client.go @@ -96,7 +96,14 @@ func (c *httpClient) doJSON(ctx context.Context, method, path string, body, targ if json.Unmarshal(respBody, &errResp) == nil && len(errResp.Errors) > 0 { se := errResp.Errors[0] msg := fmt.Sprintf("square: %s %s: [%s/%s] %s (field: %s)", method, path, se.Category, se.Code, se.Detail, se.Field) - return &squareAPIError{Code: se.Code, Detail: se.Detail, err: errors.New(msg)} + return &squareAPIError{ + Code: se.Code, + Detail: se.Detail, + Category: se.Category, + Field: se.Field, + StatusCode: resp.StatusCode, + err: errors.New(msg), + } } return fmt.Errorf("square: %s %s: HTTP %d: %s", method, path, resp.StatusCode, string(respBody)) } @@ -228,8 +235,11 @@ type sqTerminalCheckout struct { ReferenceID string `json:"reference_id,omitempty"` Note string `json:"note,omitempty"` PaymentIDs []string `json:"payment_ids,omitempty"` - // Deadline (deadline_duration) is deprecated in the TerminalCheckout API — - // retained read-only for informational purposes; harmless when set. + // Deadline (deadline_duration) is a LIVE TerminalCheckout field: an RFC 3339 + // duration (e.g. "PT5M") telling the terminal how long the checkout stays + // active. Square defaults it to 5 minutes. It is NOT an absolute timestamp + // and NOT deprecated. Kept as a string because the app only copies it + // through to CheckoutResult.Deadline. Deadline string `json:"deadline_duration,omitempty"` CreatedAt string `json:"created_at"` UpdatedAt string `json:"updated_at"` @@ -339,11 +349,38 @@ func validSquareID(id string) bool { return true } +// isTokenLike returns true for Square source_id tokens: cnon:xxx nonces and +// ccof:xxx card IDs. Raw PANs (all digits) are NOT token-like and are rejected. +// This is the single source of truth for token validation, shared by the real +// HTTP client and the dev mock so PCI-DSS parity holds in both builds. +func isTokenLike(s string) bool { + return strings.HasPrefix(s, "cnon:") || strings.HasPrefix(s, "ccof:") +} + +// tokenPrefix returns a PCI-safe abbreviation of a card token for error and +// log messages: the first 8 characters plus an ellipsis and the total length. +// The full token (a raw PAN, single-use nonce, or card reference) must NEVER +// be echoed — handlers log these errors, so embedding the raw value would land +// client-submitted card data verbatim in server logs. +func tokenPrefix(s string) string { + if len(s) > 8 { + return fmt.Sprintf("%s... (len %d)", s[:8], len(s)) + } + return fmt.Sprintf("%s (len %d)", s, len(s)) +} + func createPaymentHTTP(ctx context.Context, req CreatePaymentReq) (*PaymentResult, error) { return createPaymentHTTPWithClient(ctx, req, newHTTPClient()) } func createPaymentHTTPWithClient(ctx context.Context, req CreatePaymentReq, hc *httpClient) (*PaymentResult, error) { + // PCI-DSS parity with the dev mock: reject raw PANs before they reach + // Square. source_id must be a cnon: nonce or ccof: card ID — anything else + // (e.g. a plain card number) is refused client-side so no card data is ever + // sent to the API in a non-token form. + if !isTokenLike(req.SourceID) { + return nil, fmt.Errorf("square: invalid card token %s — use a card nonce (cnon:xxx) or card ID (ccof:xxx)", tokenPrefix(req.SourceID)) + } body := sqCreatePaymentRequest{ SourceID: req.SourceID, IdempotencyKey: req.IdempotencyKey, @@ -452,12 +489,19 @@ func getPaymentHTTPWithClient(ctx context.Context, paymentID string, hc *httpCli } // squareAPIError wraps a formatted Square API error while exposing the -// structured Square error code so callers can classify definitive business -// rejections (e.g. ErrRefundDeclined) vs ambiguous transport/server errors. +// structured Square error code and the HTTP status code so callers can +// classify definitive business rejections (e.g. ErrRefundDeclined) vs +// ambiguous transport/server errors, and distinguish 400/401/429/500 +// structurally without parsing the message. Category/Field are captured from +// Square's error payload (the wire error carries them; they were previously +// dropped). type squareAPIError struct { - Code string - Detail string - err error + Code string + Detail string + Category string + Field string + StatusCode int + err error } func (e *squareAPIError) Error() string { return e.err.Error() } @@ -486,6 +530,54 @@ func ErrorDetail(err error) string { return "" } +// ErrorStatusCode returns the HTTP status code of the Square response carried +// by err when err (or any error it wraps) is a *squareAPIError, and 0 +// otherwise. Callers can distinguish 400/401/429/500 structurally instead of +// substring-matching "HTTP 400" etc. +func ErrorStatusCode(err error) int { + var sqErr *squareAPIError + if errors.As(err, &sqErr) { + return sqErr.StatusCode + } + return 0 +} + +// ErrorCategory returns the Square error Category carried by err when err (or +// any error it wraps) is a *squareAPIError, and "" otherwise. +func ErrorCategory(err error) string { + var sqErr *squareAPIError + if errors.As(err, &sqErr) { + return sqErr.Category + } + return "" +} + +// ErrorField returns the Square error Field carried by err when err (or any +// error it wraps) is a *squareAPIError, and "" otherwise. +func ErrorField(err error) string { + var sqErr *squareAPIError + if errors.As(err, &sqErr) { + return sqErr.Field + } + return "" +} + +// IsNotFound reports whether err is a Square "not found" condition: the +// structured NOT_FOUND error code, an HTTP 404 response status, or a plain +// non-JSON 404 body (doJSON's fallback error message embeds "HTTP 404"). +// Callers use this to treat already-completed/unknown Square resources as +// idempotent no-ops instead of substring-matching the error message. +func IsNotFound(err error) bool { + if err == nil { + return false + } + var sqErr *squareAPIError + if errors.As(err, &sqErr) { + return sqErr.Code == "NOT_FOUND" || sqErr.StatusCode == http.StatusNotFound + } + return strings.Contains(err.Error(), "HTTP 404") +} + // Definitive Square refund rejection codes — the refund was declined and can // never succeed, so retrying is pointless and the refund record should be // marked 'failed'. Anything else (transport errors, 5xx) is left ambiguous so @@ -561,6 +653,11 @@ func createCardOnFileHTTP(ctx context.Context, userID, cardToken, customerID str } func createCardOnFileHTTPWithClient(ctx context.Context, userID, cardToken, customerID string, hc *httpClient) (*CardOnFile, error) { + // PCI-DSS parity with the dev mock: source_id must be a cnon: nonce or + // ccof: card ID. A raw PAN is refused client-side before it reaches Square. + if !isTokenLike(cardToken) { + return nil, fmt.Errorf("square: invalid card token %s — use a card nonce (cnon:xxx) or card ID (ccof:xxx)", tokenPrefix(cardToken)) + } // Deterministic idempotency key derived from user + card (not time-based) // so that retries with the same details don't create duplicate cards. @@ -596,11 +693,13 @@ func getCardsOnFileHTTPWithClient(ctx context.Context, userID string, hc *httpCl // Filter by reference_id natively: Square's List Cards API supports the // reference_id query param, and cards are created with reference_id = the // local user ID. customer_id is not used for the filter because a user may - // have no provisioned Square customer. List Cards pages at 25 cards, so - // loop on the cursor to avoid silently truncating a large saved-card list - // (N-10). + // have no provisioned Square customer. List Cards has NO limit param and + // pages at 25 cards per page, so loop on the cursor to avoid silently + // truncating a large saved-card list (N-10). The 20-page guard therefore + // caps out at 500 cards before warning. var cards []CardOnFile path := "/v2/cards?reference_id=" + url.QueryEscape(userID) + truncated := false for page := 0; page < 20; page++ { var resp sqListCardsResponse if err := hc.doJSON(ctx, http.MethodGet, path, nil, &resp); err != nil { @@ -612,8 +711,17 @@ func getCardsOnFileHTTPWithClient(ctx context.Context, userID string, hc *httpCl if resp.Cursor == "" { break } + if page == 19 { + truncated = true + } path = "/v2/cards?reference_id=" + url.QueryEscape(userID) + "&cursor=" + url.QueryEscape(resp.Cursor) } + if truncated { + // 20 pages fetched and a cursor is still present — return what we + // collected rather than discarding partial results (mirrors the + // listRefunds 20-page guard's behavior). + log.Printf("[SQUARE] list cards for %s exceeded 20 pages (infinite-loop guard) — returning partial results: %d cards", userID, len(cards)) + } if cards == nil { cards = []CardOnFile{} } @@ -667,11 +775,7 @@ func cancelCheckoutHTTPWithClient(ctx context.Context, checkoutID string, hc *ht if err := hc.doJSON(ctx, http.MethodPost, "/v2/terminals/checkouts/"+checkoutID+"/cancel", nil, &resp); err != nil { // Square returns 404 / NOT_FOUND when the checkout is already // completed or canceled — that is a no-op, not a failure. - var sqErr *squareAPIError - if errors.As(err, &sqErr) && sqErr.Code == "NOT_FOUND" { - return nil - } - if strings.Contains(err.Error(), "HTTP 404") { + if IsNotFound(err) { return nil } return err diff --git a/backend/internal/square/square_http_client_test.go b/backend/internal/square/square_http_client_test.go index d73bcd1..d150076 100644 --- a/backend/internal/square/square_http_client_test.go +++ b/backend/internal/square/square_http_client_test.go @@ -106,6 +106,73 @@ func TestCreateCheckoutHTTP_DeviceOptionsWireShape(t *testing.T) { } } +// TestCreateCheckoutHTTP_DeadlineWireShape verifies the deadline_duration +// contract on BOTH sides of the wire. The terminal checkout REQUEST omits +// deadline_duration entirely (Square defaults it to 5 minutes; the struct has +// no request-side field), while the RESPONSE's deadline_duration ("PT5M") is +// parsed through checkoutFromSquare into CheckoutResult.Deadline — so callers +// see Square's live deadline instead of a stale default. +func TestCreateCheckoutHTTP_DeadlineWireShape(t *testing.T) { + var captured map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Errorf("expected POST, got %s", r.Method) + } + if err := json.NewDecoder(r.Body).Decode(&captured); err != nil { + t.Errorf("failed to decode request body: %v", err) + } + w.Header().Set("Content-Type", "application/json") + // The real API echoes the checkout with a LIVE deadline_duration. + _, _ = w.Write([]byte(`{"checkout":{"id":"chk_deadline","status":"PENDING","amount_money":{"amount":5000,"currency":"GBP"},"deadline_duration":"PT5M","created_at":"2026-07-31T00:00:00Z","updated_at":"2026-07-31T00:00:00Z"}}`)) + })) + defer srv.Close() + + hc := &httpClient{baseURL: srv.URL, token: "test-token", http: srv.Client()} + + res, err := createCheckoutHTTPWithClient(context.Background(), CreateCheckoutReq{ + Amount: 5000, + Currency: "GBP", + IdempotencyKey: "ik-1", + DeviceID: "dvc_test", + }, hc) + if err != nil { + t.Fatalf("createCheckoutHTTP failed: %v", err) + } + + // The request must NOT send deadline_duration (the code relies on Square's + // 5-minute default; there is no request-side deadline field). + checkout, ok := captured["checkout"].(map[string]any) + if !ok { + t.Fatalf("expected checkout object in body, got %v", captured) + } + if _, hasDeadline := checkout["deadline_duration"]; hasDeadline { + t.Errorf("expected request to omit deadline_duration (Square default applies), got %v", captured) + } + + // The response's deadline_duration is copied through to CheckoutResult. + if res.Deadline != "PT5M" { + t.Errorf("expected CheckoutResult.Deadline = PT5M, got %q", res.Deadline) + } +} + +func TestCheckoutFromSquare_Deadline(t *testing.T) { + res := checkoutFromSquare(&sqTerminalCheckout{ + ID: "chk_1", + Status: "PENDING", + AmountMoney: sqMoney{Amount: 5000, Currency: "GBP"}, + Deadline: "PT5M", + }) + if res.Deadline != "PT5M" { + t.Errorf("expected Deadline PT5M, got %q", res.Deadline) + } + + // No deadline in the wire response → empty Deadline (Square may omit it). + resEmpty := checkoutFromSquare(&sqTerminalCheckout{ID: "chk_2", Status: "PENDING"}) + if resEmpty.Deadline != "" { + t.Errorf("expected empty Deadline when wire omits deadline_duration, got %q", resEmpty.Deadline) + } +} + // TestDoJSON_ErrorParsing covers the doJSON error branches: structured Square // errors become *squareAPIError (with Code/Detail preserved), while non-JSON // error bodies fall back to a plain error (so refund classification treats the @@ -134,6 +201,28 @@ func TestDoJSON_ErrorParsing(t *testing.T) { if sqErr.Detail != "The refund was declined" { t.Errorf("expected detail, got %q", sqErr.Detail) } + // The HTTP status, category, and field must be captured from the wire + // error so callers can distinguish 400/401/429/500 structurally. + if sqErr.StatusCode != http.StatusBadRequest { + t.Errorf("expected StatusCode 400, got %d", sqErr.StatusCode) + } + if sqErr.Category != "PAYMENT_METHOD_ERROR" { + t.Errorf("expected Category PAYMENT_METHOD_ERROR, got %q", sqErr.Category) + } + if sqErr.Field != "payment_id" { + t.Errorf("expected Field payment_id, got %q", sqErr.Field) + } + // Exported accessors surface the same values through wrapped errors. + wrapped := fmt.Errorf("wrap: %w", err) + if got := ErrorStatusCode(wrapped); got != http.StatusBadRequest { + t.Errorf("expected ErrorStatusCode 400 through wrap, got %d", got) + } + if got := ErrorCategory(wrapped); got != "PAYMENT_METHOD_ERROR" { + t.Errorf("expected ErrorCategory PAYMENT_METHOD_ERROR through wrap, got %q", got) + } + if got := ErrorField(wrapped); got != "payment_id" { + t.Errorf("expected ErrorField payment_id through wrap, got %q", got) + } }) t.Run("non_json_error_body_is_plain_error", func(t *testing.T) { @@ -1053,3 +1142,97 @@ func TestErrorCode_ErrorDetail(t *testing.T) { } }) } + +// TestIsNotFound verifies the structured not-found check: the NOT_FOUND code, +// an HTTP 404 status, and a plain non-JSON 404 body all count; 400/500 errors +// and nil do not. +func TestIsNotFound(t *testing.T) { + notFound := &squareAPIError{Code: "NOT_FOUND", Detail: "nope", StatusCode: http.StatusNotFound, err: errors.New("square: boom")} + if !IsNotFound(notFound) { + t.Error("expected structured NOT_FOUND code to be IsNotFound") + } + status404 := &squareAPIError{Code: "OTHER_CODE", Detail: "nope", StatusCode: http.StatusNotFound, err: errors.New("square: boom")} + if !IsNotFound(status404) { + t.Error("expected HTTP 404 status to be IsNotFound regardless of code") + } + wrapped := fmt.Errorf("wrap: %w", notFound) + if !IsNotFound(wrapped) { + t.Error("expected IsNotFound to work through a wrapped error") + } + + badRequest := &squareAPIError{Code: "INVALID_VALUE", Detail: "bad", StatusCode: http.StatusBadRequest, err: errors.New("square: boom")} + if IsNotFound(badRequest) { + t.Error("expected HTTP 400 NOT to be IsNotFound") + } + if IsNotFound(errors.New("square: GET /v2/x: HTTP 500: boom")) { + t.Error("expected HTTP 500 message NOT to be IsNotFound") + } + if IsNotFound(nil) { + t.Error("expected nil NOT to be IsNotFound") + } + + // Plain non-JSON 404 body (doJSON's fallback error) still counts. + if !IsNotFound(errors.New("square: POST /v2/x: HTTP 404: plain body")) { + t.Error("expected plain HTTP 404 message to be IsNotFound") + } +} + +// TestCreatePaymentHTTP_RejectsRawPAN verifies the real HTTP client refuses a +// raw PAN before forwarding to Square (PCI-DSS parity with the dev mock). +func TestCreatePaymentHTTP_RejectsRawPAN(t *testing.T) { + hc := &httpClient{baseURL: "http://unused", token: "t", http: &http.Client{}} + _, err := createPaymentHTTPWithClient(context.Background(), CreatePaymentReq{ + Amount: 5000, Currency: "GBP", SourceID: "4111111111111111", IdempotencyKey: "ik-raw-pan", + }, hc) + if err == nil { + t.Fatal("expected raw PAN to be rejected before any HTTP call") + } + if !strings.Contains(err.Error(), "invalid card token") { + t.Errorf("expected 'invalid card token' error, got %q", err.Error()) + } +} + +// TestCreateCardOnFileHTTP_RejectsRawPAN verifies the real HTTP client refuses +// a raw PAN card token before forwarding to Square (PCI-DSS parity). +func TestCreateCardOnFileHTTP_RejectsRawPAN(t *testing.T) { + hc := &httpClient{baseURL: "http://unused", token: "t", http: &http.Client{}} + _, err := createCardOnFileHTTPWithClient(context.Background(), "user_1", "4111111111111111", "", hc) + if err == nil { + t.Fatal("expected raw PAN to be rejected before any HTTP call") + } + if !strings.Contains(err.Error(), "invalid card token") { + t.Errorf("expected 'invalid card token' error, got %q", err.Error()) + } +} + +// TestIsTokenLike covers the token validation single source of truth: cnon: +// nonces and ccof: card IDs are token-like, raw PANs and empty/malformed +// values are not. A bare "cnon:"/ccof:" prefix IS accepted — HasPrefix only +// checks the prefix and Square rejects empty payloads with its own 400. +func TestIsTokenLike(t *testing.T) { + valid := []string{ + "cnon:abc", + "cnon:test-card", + "ccof:abc", + "ccof:mock_card_123", + "cnon:", + "ccof:", + } + invalid := []string{ + "", + "4111111111111111", + "visa", + "cnon_abc", + "abc:cnon:", + } + for _, tok := range valid { + if !isTokenLike(tok) { + t.Errorf("expected %q to be token-like", tok) + } + } + for _, tok := range invalid { + if isTokenLike(tok) { + t.Errorf("expected %q to NOT be token-like", tok) + } + } +} diff --git a/backend/internal/square/types.go b/backend/internal/square/types.go index dc1950c..ad2be99 100644 --- a/backend/internal/square/types.go +++ b/backend/internal/square/types.go @@ -112,7 +112,7 @@ type CheckoutResult struct { PaymentIDs []string // payment ID(s) once completed CreatedAt string // ISO 8601 timestamp UpdatedAt string // ISO 8601 timestamp - Deadline string // ISO 8601 deadline duration + Deadline string // deadline_duration from Square: an RFC 3339 duration (e.g. "PT5M"), NOT an absolute timestamp; Square defaults it to 5 minutes } // CardOnFile maps to Square's Card object from the Cards API.