Isolate wallet secrets, add withdrawal idempotency, validate CORS origins

API.md

@@ -302,6 +302,12 @@ Auth required. Debits the merchant's balance and initiates a Nigerian bank payou
 
 `200` → a withdrawal object with `status`, `provider`, `provider_reference`.
 
+**Optional `Idempotency-Key` header.** Send a client-generated key (a UUID is fine) to make a retried request safe: resubmitting the same key returns the original withdrawal instead of creating a second one and debiting the balance twice. Without it, pressing "Withdraw" twice due to a slow network response can create two separate withdrawals. Max 255 characters; a header present but blank is treated the same as omitting it.
+
+```
+Idempotency-Key: 5b1c9e0a-2f3d-4b7a-9c1e-8a2d6f0b1c3d
+```
+
 Validation errors (`400`): `"insufficient available balance"`, `"withdrawals are only supported for the cNGN asset"`, `"amount_stroops must be a whole number of kobo"`, `"positive amount_stroops, bank_code, and a 10-digit account_number are required"`.
 
 > **Payouts do not currently complete.** The Paystack integration is real and correct, but Aframp's Paystack balance is unfunded, so live calls return `502` with *"Your balance is not enough to fulfil this request."* On failure the balance is **automatically refunded** and the withdrawal is recorded with `status: "failed"` and a `failure_reason` — no money or ledger record is lost. Treat `502` as "try later," not as data loss. Paystack's own minimum transfer is ₦50 = `500000000` stroops.

migrations/0007_withdrawal_idempotency_key.sql

@@ -0,0 +1,13 @@
+-- Backs the Idempotency-Key header on POST /withdraw: a merchant retrying a
+-- slow/ambiguous request supplies the same key, and the second request must
+-- return the original withdrawal rather than create a second one. NULL for
+-- requests sent without the header (idempotency is opt-in, matching the
+-- header's optional status in the API).
+ALTER TABLE withdrawals ADD COLUMN idempotency_key TEXT;
+
+-- Scoped per merchant: the header is a client-generated ID with no global
+-- uniqueness guarantee across merchants, and merchant_id is part of every
+-- other withdrawal lookup already.
+CREATE UNIQUE INDEX withdrawals_merchant_idempotency_key_idx
+  ON withdrawals (merchant_id, idempotency_key)
+  WHERE idempotency_key IS NOT NULL;

openapi.yaml

@@ -273,6 +273,14 @@ paths:
         complete — Aframp's Paystack balance is unfunded, so live calls return 502.
         On failure the balance is automatically refunded and the withdrawal is
         recorded with status "failed" plus a failure_reason; nothing is lost.
+      parameters:
+        - name: Idempotency-Key
+          in: header
+          required: false
+          schema: { type: string, maxLength: 255 }
+          description: >
+            Optional client-generated key. Resubmitting the same key returns the
+            original withdrawal instead of creating and debiting a second one.
       requestBody:
         required: true
         content:

src/api/withdrawals.rs

@@ -1,14 +1,42 @@
 use axum::extract::{Query, State};
+use axum::http::HeaderMap;
 use axum::Json;
 use serde::Deserialize;
 
 use crate::auth::extractor::AuthUser;
 use crate::error::{bad_gateway, bad_request, bad_request_field, internal, ApiResult};
 use crate::models::{CreateWithdrawalRequest, NewWithdrawal, Withdrawal};
 use crate::services::withdrawals::{self, WithdrawalError};
-use crate::validation::{is_valid_account_number, is_valid_bank_code};
+use crate::validation::{is_valid_account_number, is_valid_bank_code, MAX_IDEMPOTENCY_KEY_LEN};
 use crate::AppState;
 
+/// Reads and validates the `Idempotency-Key` header. `Ok(None)` means the
+/// client didn't send one (idempotency is opt-in). A header present but not
+/// valid UTF-8 or over length is a client error, not silently ignored —
+/// silently ignoring it would make the client believe idempotency is active
+/// when it isn't.
+fn idempotency_key(
+    headers: &HeaderMap,
+) -> Result<Option<String>, (axum::http::StatusCode, Json<crate::error::ApiError>)> {
+    let Some(value) = headers.get("idempotency-key") else {
+        return Ok(None);
+    };
+    let key = value
+        .to_str()
+        .map_err(|_| bad_request_field("idempotency-key", "must be a valid UTF-8 header value"))?
+        .trim();
+    if key.is_empty() {
+        return Ok(None);
+    }
+    if key.len() > MAX_IDEMPOTENCY_KEY_LEN {
+        return Err(bad_request_field(
+            "idempotency-key",
+            "must be at most 255 characters",
+        ));
+    }
+    Ok(Some(key.to_string()))
+}
+
 #[derive(Deserialize)]
 pub struct ListParams {
     pub limit: Option<i64>,
@@ -17,11 +45,13 @@ pub struct ListParams {
 pub async fn create(
     State(state): State<AppState>,
     auth: AuthUser,
+    headers: HeaderMap,
     Json(req): Json<CreateWithdrawalRequest>,
 ) -> ApiResult<Json<Withdrawal>> {
     let merchant_id = auth
         .merchant_id
         .ok_or_else(|| bad_request("no merchant associated with this account"))?;
+    let idempotency_key = idempotency_key(&headers)?;
     if req.amount_stroops <= 0 {
         return Err(bad_request_field(
             "amount_stroops",
@@ -46,6 +76,7 @@ pub async fn create(
             asset: req.asset.unwrap_or_else(|| "cNGN".into()),
             bank_code: req.bank_code,
             account_number: req.account_number,
+            idempotency_key,
         },
     )
     .await

src/auth/extractor.rs

@@ -28,9 +28,9 @@ impl FromRequestParts<AppState> for AuthUser {
             .and_then(|v| v.to_str().ok())
             .and_then(|v| v.strip_prefix("Bearer "))
             .or_else(|| cookie::from_headers(&parts.headers))
-            .ok_or_else(|| (StatusCode::UNAUTHORIZED, Json(ApiError { error: "missing session cookie or bearer token".into() })))?;
+            .ok_or_else(|| (StatusCode::UNAUTHORIZED, Json(ApiError { error: "missing session cookie or bearer token".into(), field: None })))?;
         let claims = jwt::verify(&state.jwt_secret, token)
-            .map_err(|_| (StatusCode::UNAUTHORIZED, Json(ApiError { error: "invalid or expired token".into() })))?;
+            .map_err(|_| (StatusCode::UNAUTHORIZED, Json(ApiError { error: "invalid or expired token".into(), field: None })))?;
         Ok(AuthUser {
             user_id: claims.sub,
             merchant_id: claims.merchant_id,

src/config.rs

@@ -93,12 +93,10 @@ impl AppConfig {
                 .unwrap_or(60),
             wallet_encryption_key: SecretString::new(env("WALLET_ENCRYPTION_KEY")?),
             paystack_secret_key: SecretString::new(env("PAYSTACK_SECRET_KEY")?),
-            cors_allowed_origins: std::env::var("CORS_ALLOWED_ORIGINS")
-                .unwrap_or_else(|_| "http://localhost:3001".into())
-                .split(',')
-                .map(|origin| origin.trim().to_string())
-                .filter(|origin| !origin.is_empty())
-                .collect(),
+            cors_allowed_origins: parse_cors_origins(
+                &std::env::var("CORS_ALLOWED_ORIGINS")
+                    .unwrap_or_else(|_| "http://localhost:3001".into()),
+            )?,
             cookie: CookieConfig {
                 secure: cookie_secure,
                 same_site: cookie_same_site,
@@ -107,6 +105,63 @@ impl AppConfig {
     }
 }
 
+/// Parses `CORS_ALLOWED_ORIGINS` into a list of validated origins, failing
+/// fast with a clear message rather than letting a malformed value surface
+/// later as an opaque panic from `HeaderValue` parsing in `main.rs`, or
+/// silently reach the CORS layer as a value it doesn't handle the way the
+/// operator expects.
+fn parse_cors_origins(raw: &str) -> Result<Vec<String>, String> {
+    raw.split(',')
+        .map(|origin| origin.trim())
+        .filter(|origin| !origin.is_empty())
+        .map(|origin| {
+            validate_origin(origin)?;
+            Ok(origin.to_string())
+        })
+        .collect()
+}
+
+/// An "origin" is scheme + host [+ port] only — no path, query, fragment, or
+/// userinfo. `http::Uri` already gives us a real URL parser without pulling
+/// in a new dependency (`http` is already required by `axum`).
+fn validate_origin(origin: &str) -> Result<(), String> {
+    if origin == "*" {
+        return Err(
+            "CORS_ALLOWED_ORIGINS: wildcard `*` is not allowed — this API sends credentials \
+             (the session cookie), and browsers reject a wildcard origin on a credentialed \
+             request anyway. List each allowed origin explicitly."
+                .into(),
+        );
+    }
+
+    let uri: http::Uri = origin
+        .parse()
+        .map_err(|_| format!("CORS_ALLOWED_ORIGINS: `{origin}` is not a valid URL"))?;
+
+    let scheme = uri.scheme_str().ok_or_else(|| {
+        format!("CORS_ALLOWED_ORIGINS: `{origin}` must include a scheme (http:// or https://)")
+    })?;
+    if scheme != "http" && scheme != "https" {
+        return Err(format!(
+            "CORS_ALLOWED_ORIGINS: `{origin}` scheme must be http or https, got `{scheme}`"
+        ));
+    }
+    if uri.host().is_none() {
+        return Err(format!("CORS_ALLOWED_ORIGINS: `{origin}` must include a host"));
+    }
+    if !matches!(uri.path(), "" | "/") {
+        return Err(format!(
+            "CORS_ALLOWED_ORIGINS: `{origin}` must not include a path — an origin is scheme + host + port only"
+        ));
+    }
+    if uri.query().is_some() {
+        return Err(format!(
+            "CORS_ALLOWED_ORIGINS: `{origin}` must not include a query string"
+        ));
+    }
+    Ok(())
+}
+
 fn env(name: &str) -> Result<String, String> {
     std::env::var(name).map_err(|_| format!("{name} is required"))
 }

src/models/mod.rs

@@ -13,5 +13,5 @@ pub use merchant::{Merchant, NewMerchant};
 pub use payment::{NewPayment, Payment, UpdatePaymentStatus};
 pub use payment_request::{CreatePaymentRequestRequest, PaymentRequest};
 pub use user::{AuthResponse, LoginRequest, NewUser, SignupRequest, User};
-pub use wallet::{CreateWalletRequest, NewWallet, Wallet};
+pub use wallet::{CreateWalletRequest, NewWallet, Wallet, WalletSecretRow};
 pub use withdrawal::{CreateWithdrawalRequest, NewWithdrawal, Withdrawal};

src/models/wallet.rs

@@ -3,6 +3,17 @@ use serde::{Deserialize, Serialize};
 use sqlx::FromRow;
 use uuid::Uuid;
 
+/// The API- and JSON-facing view of a wallet.
+///
+/// **Never add `secret_key_encrypted` to this struct.** It derives
+/// `Serialize`, so any field on it can end up in an HTTP response the
+/// moment a handler wraps it in `Json(..)` — that's the whole reason this
+/// struct exists as the *only* row shape used by wallet-returning API
+/// queries. The encrypted Stellar seed lives in the `wallets` table but is
+/// loaded only through [`WalletSecretRow`] / `services::wallets::wallet_secret_by_id`,
+/// which nothing outside the blockchain module calls. If a feature needs
+/// the encrypted secret (e.g. a future sweep-wallet operation), extend that
+/// path — not this one.
 #[derive(Debug, Clone, Serialize, FromRow)]
 pub struct Wallet {
     pub id: Uuid,
@@ -12,6 +23,20 @@ pub struct Wallet {
     pub created_at: DateTime<Utc>,
 }
 
+/// Internal row shape carrying the encrypted secret seed. Deliberately does
+/// **not** derive `Serialize` — the compiler rejects `Json(wallet_secret)`
+/// outright rather than relying on every future caller remembering not to
+/// serialize it. Used only by the blockchain module to decrypt a signing key
+/// for outbound operations; never returned from an API handler.
+#[derive(Debug, Clone, FromRow)]
+pub struct WalletSecretRow {
+    pub id: Uuid,
+    pub merchant_id: Uuid,
+    pub address: String,
+    pub network: String,
+    pub secret_key_encrypted: String,
+}
+
 #[derive(Debug, Clone)]
 pub struct NewWallet {
     pub merchant_id: Uuid,

src/models/withdrawal.rs

@@ -34,4 +34,7 @@ pub struct NewWithdrawal {
     pub asset: String,
     pub bank_code: String,
     pub account_number: String,
+    /// From the client's `Idempotency-Key` header, if sent. `None` means the
+    /// request opted out of idempotency and always creates a new withdrawal.
+    pub idempotency_key: Option<String>,
 }

src/services/wallets.rs

@@ -2,7 +2,7 @@ use sqlx::PgPool;
 use uuid::Uuid;
 
 use crate::blockchain::{keypair, wallet_crypto};
-use crate::models::{NewWallet, Wallet};
+use crate::models::{NewWallet, Wallet, WalletSecretRow};
 
 #[derive(Debug, thiserror::Error)]
 pub enum CreateWalletError {
@@ -83,3 +83,21 @@ pub async fn wallet_by_address(db: &PgPool, address: &str) -> Result<Option<Wall
     .fetch_optional(db)
     .await
 }
+
+/// Loads a wallet's row including its encrypted secret seed. This is the
+/// *only* function in the codebase that should select
+/// `secret_key_encrypted` — everything else uses the [`Wallet`] shape,
+/// which cannot carry it because that struct has no such field. Intended
+/// for the blockchain module (signing an outbound transaction); an API
+/// handler must never call this.
+pub async fn wallet_secret_by_id(
+    db: &PgPool,
+    id: Uuid,
+) -> Result<Option<WalletSecretRow>, sqlx::Error> {
+    sqlx::query_as::<_, WalletSecretRow>(
+        "SELECT id, merchant_id, address, network, secret_key_encrypted FROM wallets WHERE id = $1",
+    )
+    .bind(id)
+    .fetch_optional(db)
+    .await
+}

src/services/withdrawals.rs

@@ -22,6 +22,9 @@ pub enum WithdrawalError {
     Database(#[from] sqlx::Error),
 }
 
+/// Postgres error code for a unique-constraint violation.
+const UNIQUE_VIOLATION: &str = "23505";
+
 pub async fn create_withdrawal(
     db: &PgPool,
     provider: &dyn PaymentProvider,
@@ -33,6 +36,18 @@ pub async fn create_withdrawal(
     if withdrawal.amount_stroops % STROOPS_PER_KOBO != 0 {
         return Err(WithdrawalError::InvalidAmountPrecision);
     }
+
+    // A resubmission with a key already used by this merchant returns the
+    // original withdrawal untouched — no new debit, no new Paystack call.
+    // This check-then-insert has a race (two concurrent requests with the
+    // same fresh key can both pass it), which the UNIQUE_VIOLATION handling
+    // below closes: the loser of that race re-reads instead of erroring.
+    if let Some(key) = &withdrawal.idempotency_key {
+        if let Some(existing) = withdrawal_by_idempotency_key(db, withdrawal.merchant_id, key).await? {
+            return Ok(existing);
+        }
+    }
+
     let amount_kobo = withdrawal.amount_stroops / STROOPS_PER_KOBO;
 
     let mut tx = db.begin().await?;
@@ -54,11 +69,11 @@ pub async fn create_withdrawal(
         return Err(WithdrawalError::InsufficientBalance);
     }
 
-    let w = sqlx::query_as::<_, Withdrawal>(
+    let inserted = sqlx::query_as::<_, Withdrawal>(
         "INSERT INTO withdrawals (
-             merchant_id, amount_stroops, asset, status, bank_code, account_number
+             merchant_id, amount_stroops, asset, status, bank_code, account_number, idempotency_key
          )
-         VALUES ($1, $2, $3, 'pending', $4, $5)
+         VALUES ($1, $2, $3, 'pending', $4, $5, $6)
          RETURNING id, merchant_id, amount_stroops, asset, status, provider,
                    provider_reference, bank_code, account_number, failure_reason,
                    created_at, updated_at",
@@ -68,8 +83,27 @@ pub async fn create_withdrawal(
     .bind(&withdrawal.asset)
     .bind(&withdrawal.bank_code)
     .bind(&withdrawal.account_number)
+    .bind(&withdrawal.idempotency_key)
     .fetch_one(&mut *tx)
-    .await?;
+    .await;
+
+    let w = match inserted {
+        Ok(w) => w,
+        Err(sqlx::Error::Database(db_err)) if db_err.code().as_deref() == Some(UNIQUE_VIOLATION) => {
+            // Lost the race described above: another request with the same
+            // key committed first. Undo this request's debit — the other
+            // request's withdrawal is the one of record — and return it.
+            tx.rollback().await?;
+            let key = withdrawal.idempotency_key.as_deref().unwrap_or_default();
+            return withdrawal_by_idempotency_key(db, withdrawal.merchant_id, key)
+                .await?
+                .ok_or(WithdrawalError::Database(sqlx::Error::Database(db_err)));
+        }
+        Err(other) => {
+            tx.rollback().await?;
+            return Err(WithdrawalError::Database(other));
+        }
+    };
 
     // Commit the debit + pending row before ever calling out to Paystack. This
     // guarantees a durable record that the withdrawal was attempted regardless
@@ -141,6 +175,24 @@ pub async fn create_withdrawal(
     }
 }
 
+async fn withdrawal_by_idempotency_key(
+    db: &PgPool,
+    merchant_id: Uuid,
+    key: &str,
+) -> Result<Option<Withdrawal>, sqlx::Error> {
+    sqlx::query_as::<_, Withdrawal>(
+        "SELECT id, merchant_id, amount_stroops, asset, status, provider,
+                provider_reference, bank_code, account_number, failure_reason,
+                created_at, updated_at
+           FROM withdrawals
+          WHERE merchant_id = $1 AND idempotency_key = $2",
+    )
+    .bind(merchant_id)
+    .bind(key)
+    .fetch_optional(db)
+    .await
+}
+
 pub async fn withdrawals_by_merchant(
     db: &PgPool,
     merchant_id: Uuid,