DEV Community: Iqbal120708

A Small Refresh Bug That Uncovered Security Gaps in My OTP System

Iqbal120708 — Sat, 25 Jul 2026 05:42:17 +0000

The Bug That Started It All

It began as a small bug I found myself while testing: OTP verification kept failing whenever I refreshed the page, opened a new tab, or copied the link to open on another device. Easy to reproduce, and my first instinct was to blame the frontend — check the state management, fix it, done.

I was wrong. What looked like a 10-minute bug forced me to redesign how my entire OTP system stored data — and that redesign process is what uncovered a gap I had no idea was there.

Digging In: The Root Cause Was in the Frontend

The immediate cause was easy to spot. I was passing the email to the verification page like this:

navigate('/verify-otp', { state: { email: formData.email } });

React Router's state only lives in the browser's memory. The moment the page refreshes, or the user opens the link in a new tab, that data is gone — location.state just comes back undefined.

The fix seemed obvious: stop relying on in-memory state, use something that survives a refresh. But before jumping to a solution, I got curious about something else — how do bigger platforms handle this? If I open a verification link from Gmail on my phone, days after requesting it from my laptop, how does that still work?

The answer changed how I thought about the whole problem: don't store the source of truth on the client. The client should only hold a reference — a token — while the actual data, validation, and expiry logic stay on the server.

That single realization is what turned a small frontend fix into a full redesign of my OTP system.

Taking Apart the Old Design

The existing OTP model looked complete enough — it stored the code, creation time, expiry time, and usage status, all in the database:

class OTPVerifications(models.Model):
    user = models.ForeignKey(get_user_model(), on_delete=models.CASCADE)
    otp = models.CharField(max_length=6)
    created_at = models.DateTimeField()
    expired_at = models.DateTimeField()
    is_used = models.BooleanField(default=False)

The validation looked reasonable too: match the code, check whether it falls within the created_at to expired_at window, mark is_used=True on success. This is a common pattern, and at a glance nothing looked wrong.

But once I started tearing it apart to move toward a token-based approach, three problems surfaced one after another:

The verify endpoint had no meaningful rate limiting. A 6-digit OTP is only 1 million combinations — without attempt limits, it's brute-forceable.
Error messages leaked information. "User not found" (404) versus "Invalid OTP code" (400) was enough for anyone to figure out which emails were registered in the system.
The generate-OTP endpoint had no protection at all. Nothing stopped it from being called over and over.

That third point is the one I initially brushed off — until I realized it was actually the most exposed part of the whole system.

Why the Generate Endpoint Was the Bigger Risk

It's easy to assume the verify endpoint is where the real danger is — after all, that's where someone could brute-force the code. But the generate-OTP endpoint turned out to be the easier target, because exploiting it doesn't require guessing anything. Just call it, repeatedly.

A few consequences became clear once I thought it through:

Cost attack. If OTPs go out over SMS, every request has a price. Spamming thousands of requests against one number can run up a real bill on the SMS gateway — without the attacker ever needing account access.
Denial of service against the victim. The person whose email or number gets targeted ends up flooded with OTP notifications, which is disruptive on its own and can even mask other suspicious activity happening on their account at the same time.
Resource exhaustion. If generating an OTP is a heavier operation — a DB query, a synchronous email send — spamming it can slow down the rest of the server too.

This is the part that's easy to overlook when securing an OTP system: people naturally lock down the verify step and forget that the request step needs its own protection just as much.

The Redesign: Splitting Responsibility Between Redis and the Database

The turning point was drawing a clear line between two storage layers: Redis holds everything short-lived and speed-sensitive, while the database gets simplified down to a historical log.

Redis stores the OTP session with a TTL for the session as a whole — no cron job or extra query needed to clean up stale data, Redis just deletes it once time is up. But since the OTP code itself has a shorter lifespan than the session (5 minutes versus a 30-minute session), checking whether the code is still valid stays manual in the application logic, rather than being fully offloaded to Redis:

otp_code = f"{secrets.randbelow(1000000):06d}"
r.setex(f"otp:{token}", 1800, json.dumps({
    "email": email,
    "otp": otp_code,
    "otp_created_at": time.time(),
}))

if time.time() - data["otp_created_at"] > 300:
    return res_error("OTP code has expired, please request a new one.", status.HTTP_400_BAD_REQUEST)

Redis handles expiry at the session level — when the whole token should die — while the OTP code's own expiry stays as application logic, so a user can still resend within the longer session window without an old, expired code remaining valid.

The database side, meanwhile, got drastically simplified. The expired_at and is_used fields are gone, and the plaintext otp field became a hash — because its only job now is recording history, not validating anything:

class OTPVerifications(models.Model):
    user = models.ForeignKey(get_user_model(), on_delete=models.CASCADE)
    otp_hash = models.CharField(max_length=64)
    created_at = models.DateTimeField(auto_now_add=True)
    used_at = models.DateTimeField(null=True, blank=True)

The change looks small on paper, but the effect is significant: validation got faster (no database query needed at the verification step), and the database stopped being a point of failure for expiry bugs — that responsibility now lives entirely in Redis and the application logic around it.

From Two Keys to One: Simplifying the Token/Session Design

Once Redis was set as the storage layer for OTPs, the next question came up: how do I let a user request a new code (resend) without forcing them to register from scratch, while still keeping a clear deadline for when the session actually has to die?

My first attempt looked reasonable on paper: split the OTP code and the session into two separate Redis keys — one for the active code (otp:{token}, short TTL), another for the overall session (otp_session:{token}, longer TTL).

r.setex(f"otp:{token}", 300, json.dumps({"email": email, "otp": otp_code}))
r.setex(f"otp_session:{token}", 1800, email)

But thinking it through further, this actually introduced new problems: the email ended up duplicated across two places, two TTLs had to stay in sync, and if one key disappeared before the other — say, from a bug or a timing delay — the state became ambiguous: the session alive but the OTP gone, or the other way around.

The solution turned out to be much simpler than I expected: one key, with a TTL long enough for the session, and the OTP code's own expiry checked manually in the application logic (as covered in the previous section). When a user resends, the value gets updated but the session TTL doesn't reset — using the KEEPTTL option Redis provides for exactly this kind of case:

data.update({"otp": new_otp_code, "otp_created_at": time.time()})
r.set(f"otp:{token}", json.dumps(data), keepttl=True)

With this, the OTP code changes every time it's resent, but the session clock keeps running from the original starting point — a user who resends at minute 25 still runs out of session at minute 30, not minute 55. One key, one source of truth, no risk of two pieces of data drifting out of sync.

The Bug That Slipped Through: A Daily Limit That Didn't Actually Limit Anything

With the token and session solid, I moved on to something that clearly needed protecting: limiting how many times an OTP could be requested in a day. The logic looked sound — a per-email counter that resets every 24 hours:

daily_key = f"otp_daily:{email}"
count = r.incr(daily_key)
if count == 1:
    r.expire(daily_key, 86400)
if count > 10:
    return res_error("Daily OTP request limit reached.", status.HTTP_429_TOO_MANY_REQUESTS)

This sat inside the resend endpoint, right after the token validation. It looked complete — until I tried walking through the attack scenario myself from the start.

The problem was in the order of checks. Before that daily-limit code could ever run, the request first validated whether the token was still alive in Redis:

raw = r.get(f"otp:{token}")
if not raw:
    return res_error("Session expired, please register again.", status.HTTP_400_BAD_REQUEST)

Once the token expired (30 minutes in), the request stopped right there — the daily limit below it never got a chance to run. A user, or an attacker, who found their session dead could just register again to get a fresh token. And since the register endpoint never checked otp_daily:{email} at all, whatever count had built up on the resend side meant nothing — every re-registration was effectively a brand-new allowance.

I had considered this feature done, until I sat down and ran the numbers myself: an attacker could just re-register every 30 minutes, and the daily limit I thought was protecting the system never actually stopped anything.

The Final Decision: Basing Limits on Real Risk, Not Assumptions

The obvious fix was to move the daily-limit check so it runs at every point an OTP gets generated — both register and resend — instead of just one endpoint. But before adding another layer of protection, I stopped and asked a more basic question: how big is the risk I'm actually trying to close?

I worked through the most aggressive attack scenario possible under the existing design: a 5-per-3-hours register limit, a 60-second resend cooldown, and a 5-failed-attempt lockout on verification.

The key detail that made this manageable: the attempt counter is keyed by token, not by OTP code. So even if an attacker resends to get a fresh code, the failed-attempt count doesn't reset — it stays tied to the session for as long as that token lives:

attempt_key = f"otp_attempt:{token}"
attempts = int(r.get(attempt_key) or 0)
if attempts >= 5:
    r.delete(f"otp:{token}")
    return res_error("Too many failed attempts. Please request a new OTP.", status.HTTP_429_TOO_MANY_REQUESTS)

That means resending doesn't buy an attacker a fresh set of guesses — it only gets them a new code to guess, still under the same shrinking budget of attempts:

5 registrations (limit) × 5 verify attempts per session = 25 total verify attempts

25 attempts against a 6-digit code — 1 million possible combinations — puts the odds of a successful guess at roughly 0.0025%. That's low enough that the 5-attempt lockout, combined with the register limit, is enough on its own. No extra protection needed there.

As for spam volume: with a 60-second cooldown and a 30-minute session, a single session allows at most ~30 resends. Across 5 registrations, that's up to 150 emails sent within a 3-hour window. If OTPs go out over email, that's a low, acceptable cost. If they go out over SMS, the math changes — the cooldown would need to stretch to a few minutes instead of 60 seconds to keep the cost down.

The conclusion: the separate daily limit on resend didn't need patching — it needed to be removed. The protections already in place elsewhere — the register limit, the resend cooldown, the verify lockout — were enough to cover the real risk, without adding another layer that turned out, as I'd just found, to introduce a new gap of its own.

Testing: Why the Backend and Frontend Needed Different Levels of Rigor

Once the backend felt solid, I asked myself: does the frontend need to be tested this thoroughly too? My OTP verification page in React really only does two things — read the token from the query param, and submit the form to the backend. There are no granular unit tests per function, a stark contrast to the backend, which I tested down to lockout scenarios, cooldowns, and other.

It turns out this isn't a gap — it reflects where the real risk actually sits in each layer. The backend is the source of truth: expiry, brute-force protection, atomic counters, all logic that, if wrong, can quietly become a security hole or corrupted data with nobody noticing until it's exploited. The frontend, on the other hand, is mostly presentational — take input, send it to the backend, display the result. If something breaks there, it's usually visible right away (a broken form, a visual bug), and easy to fix once spotted.

This also lines up with what I've generally read about testing practices across many projects: the backend gets detailed unit tests for business logic and validation, while the frontend leans more toward high-level integration tests — "user fills the form, clicks submit, gets redirected to the right page" — rather than unit tests for every small function. This isn't about the frontend mattering less; it's about putting testing effort where it actually needs the guarantee.

What I Took Away From This

Looking back, this all started from a bug that seemed minor — an email disappearing on page refresh. If I had just patched that on the frontend (say, storing the email in sessionStorage) and stopped there, my OTP system would have kept running fine — but every gap I found along the way would still be sitting there, unnoticed until someone eventually exploited it.

What I learned wasn't just about Redis, tokens, or rate limiting as techniques. What mattered more was the habit of pausing to ask "why is this happening" instead of jumping straight to the fastest fix. That small refresh bug forced me to walk back through every assumption I'd taken for granted — and some of them, like the daily limit that turned out to be easy to bypass, would never have surfaced if I hadn't ended up redesigning things from scratch.

What I learned: security isn't about adding as much protection as possible. It's more about calculating the real risk — who might attack, how costly it actually is, and how likely that scenario really is. Sometimes that means adding a new layer. But sometimes, like the daily limit I ended up removing, it means realizing that protection which looked safe was really just adding complexity without any real benefit.

The full code for this system is available in the project repos:

Backend (Django + DRF + Redis): github.com/Iqbal120708/url-shortener
Frontend (React): github.com/Iqbal120708/url-shortener-web

Behind RefundRequest: Designing Item-Level Refunds for a Single-Vendor E-commerce App

Iqbal120708 — Sat, 18 Jul 2026 06:20:31 +0000

Introduction

When I built the refund feature for this single-vendor e-commerce project, I ran into a problem that looked simple on the surface but turned out to have a fairly deep root cause: if an order contained more than one product, and a customer only wanted to refund one of them, the system I'd originally designed would end up refunding every other product in that same order too.

The reason was straightforward — refund status originally lived only at the Order level, not per product. So the moment an order's status changed to "refunded," every item inside it got dragged along, even though the customer had only asked to refund one item.

This article walks through how I got to the final design: why RefundRequest became a separate model pointing to OrderItem (rather than just a status field on Order), why refunds are sent manually to the customer's bank account/e-wallet instead of automatically through the Midtrans Refund API, how I handled a race condition between a webhook and an admin action that can realistically happen almost simultaneously, and why the actual refund execution step (complete()) has to pass 4 guards before it runs.

Some approaches I tried and then rejected myself are included here too — I think that's actually the most honest way to show how the thinking happened, not just the end result.

Why `RefundRequest` Is a FK to `OrderItem`, Not a Status on `Order`

My first attempt at fixing this problem was the solution that looked "correct" on paper: move all refund/cancel status from Order to OrderItem. If the status lived at the item level, the "refunding 1 product cascades into refunding all of them" problem would just disappear.

But once I actually started, this solution meant a large-scale refactor — methods in the Midtrans service, the Order model, and the serializer would all need to change to follow the status move to OrderItem. Meanwhile, Order.Status.CANCELED and Order.PaymentStatus.REFUNDED weren't actually being used anywhere yet at that point. I decided not to move the entire status set — instead, I dropped those two fields and created a new model, RefundRequest, to handle the status on its own. On OrderItem, I added two properties, is_refunded and has_active_refund, to read refund state without needing an extra status field on Order.

The same pattern showed up again when I considered Order.reduced_stock. I initially planned to move this field to OrderItem too, along with a rewrite of reduce_product_stock/restore_product_stock and their tests. I dropped it for the same reason: the migration was too large, and the RefundRequest.status I'd just built was already enough of a guard. reduced_stock stayed a one-time boolean on Order, never toggled back by a refund or cancel. I accepted the trade-off consciously: once a partial refund happens, this field no longer means "the order's entire stock is still fully held" — it's only valid as a guard in three specific places (reduce_stock(), reverse_stock(), release_reservation()), and shouldn't be read as an accurate stock indicator anywhere else.

The final design for the refund model itself: RefundRequest with a ForeignKey to OrderItem — not a OneToOneField. The reason for a plain FK: it preserves history if a customer requests a refund, gets rejected, then requests again for the same item.

(Snippet simplified to focus on what's discussed here — other fields like amount, status, and the destination account are covered in the next section.)

class RefundRequest(BaseModel):
    class Reason(models.TextChoices):
        CUSTOMER_CANCEL = "customer_cancel", "Cancelled by Customer"
        OUT_OF_STOCK = "out_of_stock", "Out of Stock"
        RETURN = "return", "Post-Shipment Return"
        OTHER = "other", "Other"

    order_item = models.ForeignKey(
        OrderItem,
        on_delete=models.PROTECT,
        related_name="refund_requests",
    )

    reason = models.CharField(
        max_length=30,
        choices=Reason.choices,
        help_text=(
            "Auto-set to CUSTOMER_CANCEL/RETURN when the customer submits it. "
            "Admin can edit manually (e.g. to OUT_OF_STOCK) before status is COMPLETED."
        ),
    )

    @property
    def order(self):
        return self.order_item.order

The "only 1 active request per item" rule isn't enforced through a database constraint — a constraint would need to account for status, which can change (REQUESTED/APPROVED are active, REJECTED/COMPLETED are not). I put it at the serializer level instead:

def validate_order_item(self, order_item):
    request = self.context["request"]
    if order_item.order.user_id != request.user.id:
        raise serializers.ValidationError("This item doesn't belong to your order.")
    if order_item.has_active_refund:
        raise serializers.ValidationError("There's already an active refund request for this item.")
    return order_item

There's one small detail worth pointing out from the model above. I initially added order as a direct ForeignKey for faster queries, then removed it — order is already reachable through order_item.order, and Django doesn't guarantee those two fields would stay consistent with each other. I replaced it with @property order, so there's only one source of truth.

The more important decision here: I merged order cancellation and refund into the same model, distinguished by the reason field above (CUSTOMER_CANCEL vs RETURN vs others) rather than a separate model or flow. I initially assumed cancellation needed its own flow (its own canceled_at, and so on). But on reflection, cancellation is really just a special case of refund — the amount is always the full price, and it happens before the item ships. This is the part I think matters most here: recognizing that two things that look different are actually the same domain, before building two separate systems for the same thing.

Why Refunds Are Sent Manually, Not Automatically Through the Midtrans API

The most basic reason refunds in this system aren't automated through the Midtrans Refund API: not every payment method this project uses supports it. bank_transfer, cstore, and echannel have no automatic refund path in Midtrans at all — if I forced an API refund integration, some payment methods would still need to be handled manually anyway, meaning two different paths for the same problem. I chose one manual path for every method instead, for consistency.

As a consequence, I needed somewhere to store the refund destination. My first plan was just bank_name/account_number — a quiet assumption that every customer has a bank account. That assumption was wrong; a lot of customers only use e-wallets. I revised it to be generic:

class DestinationType(models.TextChoices):
    BANK = "bank", "Bank Transfer"
    EWALLET = "ewallet", "E-Wallet"

class Provider(models.TextChoices):
    BCA = "bca", "BCA"
    MANDIRI = "mandiri", "Mandiri"
    BNI = "bni", "BNI"
    BRI = "bri", "BRI"
    PERMATA = "permata", "Permata"
    CIMB = "cimb", "CIMB Niaga"
    GOPAY = "gopay", "GoPay"
    SHOPEEPAY = "shopeepay", "ShopeePay"
    DANA = "dana", "DANA"
    OVO = "ovo", "OVO"

BANK_PROVIDERS = {"bca", "mandiri", "bni", "bri", "permata", "cimb"}
EWALLET_PROVIDERS = {"gopay", "shopeepay", "dana", "ovo"}

destination_provider uses choices instead of free text — so the customer picks from a list rather than typing it manually, which is prone to typos. Rendering the dropdown/search is the frontend's job; the backend just exposes the valid options.

One validation I put explicitly in the serializer: destination_provider has to be consistent with destination_type — bank providers can only pair with BANK, e-wallet providers can only pair with EWALLET. Without this check, inconsistent data (say, type=BANK with provider=GOPAY) could slip into the database and only get caught when an admin tries to actually send the transfer.

account_holder_name is stored separately too — not for automated validation, but so an admin can match the recorded name against the name shown in their transfer app before actually sending money, to prevent transferring to the wrong person.

One thing I decided deliberately: refunds in this system always go to a manual bank account/e-wallet, never automatically back to the original payment method (unlike large marketplaces that can refund straight to their own internal balance/wallet). Two reasons: this system has no internal balance/wallet, and the Midtrans Refund API — which does support automatic refunds for gopay/shopeepay/qris — was deliberately left unintegrated for now. Same principle as the previous section: don't add complexity for a case that isn't clearly needed at this project's scale.

Race Condition: How the Webhook Path and the Admin Path Stay in Sync

There's a real scenario I had to handle: a Midtrans reversal webhook (a payment that was PAID flipping to FAILED) and an admin completing a refund can happen almost at the same time on the same order — a realistic 1-5 minute window. If both paths read and write stock data without knowing about each other, reserved_stock/stock could get adjusted twice for the same item.

These two paths don't share one single function outright — they stay in sync through row-level locks on the database rows they both touch.

The webhook path (reverse_stock() case) uses the get_unrefunded_items() helper to exclude items that already have a COMPLETED RefundRequest:

def get_unrefunded_items(order):
    refunded_item_ids = RefundRequest.objects.filter(
        order_item__order=order,
        status=RefundRequest.Status.COMPLETED,
    ).values_list("order_item_id", flat=True)
    return order.items.select_for_update().exclude(id__in=refunded_item_ids)

reverse_stock() — called when a previously successful payment gets reversed by Midtrans (paid → failed), and only runs if reduced_stock=True:

def reverse_stock(self):
    if not self.order.reduced_stock:
        return

    if self.order.status in [Order.Status.SHIPPED, Order.Status.DELIVERED]:
        logger_error.critical(
            "Payment reversal but order already shipped/delivered - "
            "NOT auto-restocking, needs manual review",
            extra={
                "event_type": "transaction",
                "order_id": self.order.order_id,
                "order_status": self.order.status,
            },
        )
        return

    try:
        items_to_restore = get_unrefunded_items(self.order)
        restore_product_stock(items_to_restore)
        self.order.reduced_stock = False
        self.order.save(update_fields=["reduced_stock"])
        logger.warning(
            "Stock restored due to payment reversal (paid -> failed)",
            extra={"event_type": "transaction", "order_id": self.order.order_id},
        )
    except Exception:
        logger_error.exception(
            "Failed to restore stock after reversal",
            extra={"event_type": "transaction", "order_id": self.order.order_id},
        )
        raise

The admin path, complete(), doesn't call get_unrefunded_items() at all. It operates on a single item, locking OrderItem, Order, and Product directly:

def complete(self):
    refund_request = self.refund_request

    with transaction.atomic():
        order_item = OrderItem.objects.select_for_update().get(
            pk=refund_request.order_item_id
        )
        order = Order.objects.select_for_update().get(pk=order_item.order_id)

        if refund_request.status != RefundRequest.Status.APPROVED:
            raise ValidationError(
                "Only requests with status APPROVED can be completed."
            )

        valid_statuses = [
            Order.Status.PENDING, Order.Status.PROCESSING,
            Order.Status.SHIPPED, Order.Status.DELIVERED,
        ]
        if order.status not in valid_statuses:
            raise ValidationError(
                f"Order status is {order.status}, refund cancelled."
            )

        if order.payment_status == Order.PaymentStatus.FAILED:
            raise ValidationError(
                "Payment failed, the item was never shipped -- "
                "there's nothing to refund."
            )

        if order.payment_status == Order.PaymentStatus.PAID and not order.reduced_stock:
            logger_error.critical(
                "Refund blocked - order is PAID but reduced_stock is False, needs manual review",
                extra={
                    "event_type": "refund",
                    "order_id": order.order_id,
                    "refund_request_id": refund_request.id,
                    "payment_status": order.payment_status,
                    "reduced_stock": order.reduced_stock,
                },
            )
            send_refund_anomaly_email.delay(order.id, refund_request.id)
            raise ValidationError(
                "Order is in an anomalous state (paid but stock not yet processed) -- "
                "needs admin review before the refund can proceed."
            )

        refund_request.status = RefundRequest.Status.COMPLETED
        refund_request.completed_at = timezone.now()
        refund_request.save(update_fields=["status", "completed_at"])

        product = Product.objects.select_for_update().get(pk=order_item.product_id)
        if order.reduced_stock:
            product.stock += order_item.qty
        else:
            product.reserved_stock -= order_item.qty
        product.save()

    send_refund_status_email.delay(refund_request.id)

Locking OrderItem in complete() looks odd at first — no OrderItem field is ever written inside this function. But its purpose isn't to protect OrderItem's own data — it's a synchronization point with get_unrefunded_items(), which also locks the same OrderItem rows.

Concretely, here's the sequence this prevents. Order X has 2 items, A and B, with reduced_stock=True. An admin just approved a refund for item A. Almost simultaneously, Midtrans sends a reversal webhook for the same order X.

complete() locks the OrderItem A row first (select_for_update()) and starts processing.
reverse_stock() runs almost at the same time, calling get_unrefunded_items(). Notice the order inside this helper: the RefundRequest.objects.filter(...) line has no lock, and Django evaluates that query lazily — it's only actually executed against the database when .exclude() on the next line gets evaluated. That .exclude() line is the one that needs select_for_update() on OrderItem, and since item A is already locked by complete(), the entire query — including the RefundRequest query that only runs later — gets held up too.
complete() finishes all its guards, saves refund_request.status = COMPLETED, adds to product.stock for item A, then commits — releasing the lock on item A.
get_unrefunded_items() can now proceed, acquires the lock, and its RefundRequest query finally executes — reading the now-updated data: RefundRequest for item A is COMPLETED, so get_unrefunded_items() automatically excludes it from the result.
restore_product_stock only runs for item B. Item A doesn't get its stock added back a second time.

If complete() didn't also lock OrderItem, step 2 wouldn't wait — get_unrefunded_items() could read stale data (status still APPROVED, not yet COMPLETED) before complete() finishes, and would still process a restore for item A even though it's being refunded in another transaction. The lock isn't about protecting a field that changes — it's about forcing the second transaction to delay reading its data until the first, overlapping transaction is done.

release_reservation() uses the same get_unrefunded_items(), so it gets the same lock protection against complete() — but it handles a different status transition (pending → failed for abandoned checkouts, not a reversal of a successful payment). The function itself doesn't re-check whether it's actually safe to run — it relies on its caller (the view receiving the webhook) to confirm this is genuinely a new transition, not a reversal or a duplicate webhook, before calling it. I won't go further into this here since it doesn't add anything to the race condition argument above.

4 Guards Before a Refund Actually Executes

The complete() code already appeared in full in the previous section to explain the locking — this section focuses on the 4 guards inside it, why each one exists, and why they're ordered the way they are.

Guard 1 — status must be APPROVED

if refund_request.status != RefundRequest.Status.APPROVED:
    raise ValidationError(
        "Only requests with status APPROVED can be completed."
    )

This prevents double execution. approve()/reject() only change status and a timestamp — they never touch stock at all. complete() is the only place that actually executes the stock restore. Without this guard, calling complete() twice for the same request (say, from a retry or a double-click in the admin) could add stock back twice for the same refund.

Guard 2 — order status must be one of the valid statuses

valid_statuses = [
    Order.Status.PENDING, Order.Status.PROCESSING,
    Order.Status.SHIPPED, Order.Status.DELIVERED,
]
if order.status not in valid_statuses:
    raise ValidationError(
        f"Order status is {order.status}, refund cancelled."
    )

This is a general fence — an order whose status falls outside this list (for example, already CANCELLED through another path) shouldn't have its refund processed further.

Guard 3 — payment must not be FAILED

if order.payment_status == Order.PaymentStatus.FAILED:
    raise ValidationError(
        "Payment failed, the item was never shipped -- "
        "there's nothing to refund."
    )

If the payment failed, there's no money to give back — the item was never shipped in the first place. This guard prevents a refund from being processed for a transaction that logically never happened.

Guard 4 — the anomalous combination: PAID but stock never processed

if order.payment_status == Order.PaymentStatus.PAID and not order.reduced_stock:
    logger_error.critical(
        "Refund blocked - order is PAID but reduced_stock is False, needs manual review",
        extra={
            "event_type": "refund",
            "order_id": order.order_id,
            "refund_request_id": refund_request.id,
            "payment_status": order.payment_status,
            "reduced_stock": order.reduced_stock,
        },
    )
    send_refund_anomaly_email.delay(order.id, refund_request.id)
    raise ValidationError(
        "Order is in an anomalous state (paid but stock not yet processed) -- "
        "needs admin review before the refund can proceed."
    )

This guard is different from the previous three — it's not checking "is this allowed," it's checking for a state combination that shouldn't be possible if everything upstream worked correctly: the order was paid, but its stock was never reduced. If this happens, some other step in the system got skipped.

The response also differs from the first three guards. Those three just raise ValidationError — rejected, done, the admin sees the error message and knows why. This guard adds two more things: logger_error.critical (not a warning or info level) and send_refund_anomaly_email.delay(...), which emails the store. The reasoning: an anomalous state like this isn't enough to just sit in a log that no one may ever open. The admin needs to know actively, not just have it recorded passively, because this signals a bug or some other failed process somewhere unexpected — not just "the user entered something wrong," like the previous three guards.

The order of these four guards isn't arbitrary either — from the cheapest to check (a single status field) to the most expensive in meaning (a system anomaly that needs an active notification). The cheapest guard runs first so common cases (a retry, an order that isn't eligible yet) fail fast without triggering the critical log and email meant only for genuinely abnormal cases.

Closing Thoughts

Looking back, almost every major decision in this feature follows the same pattern: start with a solution that looks more "complete" on paper — a full status migration to the item level, separate destination fields per bank, an automatic refund API integration — then walk it back once it becomes clear the cost of that change doesn't match the actual problem at hand. RefundRequest as a new model, reduced_stock staying on Order, manual refunds to a bank account/e-wallet — all of these are smaller versions of the original plan, not the original plan itself.

The principle I kept coming back to: only add complexity when there's actual evidence it's needed. Partial-quantity refunds, a Midtrans Refund API integration, a separate Django app for refunds — I rejected all of these, not because they could never be useful, but because there was no evidence they were needed at this single-vendor scale yet. If that need shows up later, having RefundRequest as its own model makes that change easier to make without tearing apart Order.

The full code is on GitHub. If you have questions, or think part of this design could've been approached differently, I'd be glad to discuss it in the comments.

Preventing Overselling with Stock Reservation and select_for_update in Django

Iqbal120708 — Thu, 09 Jul 2026 07:59:51 +0000

Picture two users checking out the same product, with only one unit left in stock, at nearly the same moment. Without special handling, both requests can pass stock validation and both orders get created — even though only one item actually exists. This isn't a rare edge case; on a system with reasonable traffic, this kind of race condition can happen daily.

This article covers how I handled this in the checkout endpoint of my single-vendor-ecommerce project: not by decrementing stock immediately at checkout, but through a database-level locked reservation mechanism.

Why Reservation, Not Immediate Stock Deduction

The simplest way to prevent overselling is to decrement stock immediately when a user starts checkout. But this has a problem: checkout doesn't always end in a completed payment. Users can abandon the flow, sessions can expire, or payments can fail. If stock is already reduced at checkout time, the system has to constantly restore stock for every failure case — and each restoration is itself a place where miscounts or missed rollbacks can happen.

The approach I used separates two numbers: stock (the actual quantity on hand) and reserved_stock (the quantity currently "held" by an in-progress checkout). During checkout, only reserved_stock changes. The real stock only decreases once payment is confirmed through the Midtrans webhook.

With this separation, availability is calculated as stock - reserved_stock, not stock alone. This means items held by another in-progress checkout automatically become invisible as available stock to other users, without ever touching the real stock number until the transaction is actually finalized.

Implementation: Locking in `_validate_and_reserve_stock`

Here's the code from checkout.py that handles stock validation and reservation:

def _validate_and_reserve_stock(self, carts):
    product_ids = carts.values_list("product__id", flat=True)
    products = (
        Product.objects.select_for_update()
        .filter(id__in=product_ids)
        .order_by("id")
    )

    products_map = {product.id: product for product in products}

    for cart in carts:
        product = products_map[cart.product.id]
        available_stock = product.stock - product.reserved_stock

        if available_stock < cart.qty:
            raise serializers.ValidationError(
                {"detail": f"Stok {product.name} tidak cukup"}
            )

        product.reserved_stock += cart.qty

    Product.objects.bulk_update(products, ["reserved_stock"])

There are three things I made sure of here:

select_for_update() locks the relevant product rows at the database level. Until this transaction finishes, any other checkout request touching the same product has to wait — it can't read stale data and slip past stock validation.
.order_by("id") guarantees a consistent lock acquisition order. If two checkouts both reserve products A and B but in different order, without a consistent order_by, they could end up waiting on each other's locks — a deadlock.
A single bulk_update at the end, instead of calling save() per product inside the loop, so the write to the database happens as one query instead of N separate ones.

Availability is calculated from product.stock - product.reserved_stock, consistent with the separation covered in the previous section. If stock isn't sufficient, the request is rejected with a ValidationError before a single row is modified.

Why It's Wrapped in a Single `transaction.atomic()`

Stock reservation doesn't stand alone — it's part of a larger flow in CheckoutService.execute():

def execute(self):
    with transaction.atomic():
        checkout = CheckoutSession.objects.create(
            user=self.user,
            cart_ids=self.cart_ids,
            destination=self.destination,
            store=self.store,
            expires_at=timezone.now() + timedelta(minutes=10),
        )

        carts = get_valid_carts(self.user, checkout.cart_ids)

        self._validate_and_reserve_stock(carts)

        service = OrderService(checkout, carts)
        order = service.execute()
        checkout.order = order
        checkout.save(update_fields=["order"])

        return checkout

Notice that _validate_and_reserve_stock runs before OrderService.execute() finishes creating the Order and OrderItem records. That means reserved_stock is already incremented before there's any guarantee the order itself will be created successfully.

At first glance this looks risky — what happens if OrderService.execute() fails midway? The answer is in the transaction.atomic() scope wrapping everything. Django doesn't commit anything to the database until this block completes without error. If OrderService.execute() raises any exception, every change inside the block — including the reserved_stock increment from bulk_update — gets rolled back together.

In other words, the step ordering doesn't create a gap, because the unit of consistency isn't per-step, it's per-transaction. Stock reservation and order creation succeed or fail as a single unit.

Known Limitations

Locking with select_for_update() solves the race condition at checkout creation, but there's one thing it doesn't solve: releasing the reservation when a checkout is abandoned.

Every CheckoutSession has an expires_at, set to 10 minutes from creation. If a user starts checkout but never proceeds to payment — whether they change their mind, close the app, or lose connection — the reserved_stock incremented for that checkout stays held even after the session expires. There's currently no process that automatically detects expired checkouts and releases their reserved_stock.

The impact: if enough checkouts get abandoned on a product with limited stock, available_stock (stock - reserved_stock) can end up lower than what's actually available, even though no payment is in progress for those reservations.

This isn't a flaw in the locking mechanism — the locking itself solves exactly what it was designed to solve. It's a separate gap: the reservation lifecycle doesn't yet have an automatic cleanup process. The next step I'm planning is a scheduled task (Celery beat is the likely candidate) that periodically scans for CheckoutSession records past their expires_at without a paid order, and releases their associated reserved_stock.

End-to-End Flow

Here's the complete flow of CheckoutView.post(), from incoming request to returned response — including the points where the process can fail and how rollback works inside transaction.atomic()

Conclusion

Handling race conditions in checkout isn't just about adding select_for_update() in one place. What matters more is identifying the right unit of consistency: in this case, stock reservation and order creation need to succeed or fail together, not as two separate operations.

Locking solves the stale-read problem when checkouts happen concurrently. The right transaction.atomic() scope makes sure the step ordering doesn't open a new gap. But neither of those automatically solves the full lifecycle of a reservation — including when it should be released if it's never used. Recognizing that boundary, and knowing what the next step is, matters just as much as writing the locking itself.

Debugging a Payment Webhook: How I Caught a Silent Failure That Would Have Blocked Every Non-Card Transaction

Iqbal120708 — Mon, 06 Jul 2026 08:38:23 +0000

While building the payment flow for a single-vendor e-commerce portfolio project, I wrote a Midtrans webhook handler that looked correct on the first pass. It parsed the payload, checked the signature, updated the order status, and adjusted stock. It would have passed tests for the happy path, and I initially considered it done.

Then I went back through Midtrans' official documentation on transaction statuses and fraud detection, and found a bug that would have silently failed transactions paid through QRIS, bank transfer, or other non-card methods — which, for most Indonesian e-commerce stores, make up the majority of traffic.

That bug turned out to be the first of several problems that only surface once you take webhooks seriously: signature validation edge cases, why webhooks retry and what that quietly breaks, a real reversal case most tutorials never mention, and a subtle bug in reservation-release logic that took me three attempts to get right. This post walks through all of them.

Trusting a Public Endpoint

The Midtrans webhook endpoint has no authentication in the usual sense — it's AllowAny, reachable by anyone on the internet. That's by design: Midtrans needs to be able to POST to it without a login flow. But it means the handler itself has to verify that the request actually came from Midtrans, not from someone who found the URL and decided to send a fake "payment successful" notification.

Midtrans solves this with a signature: signature_key is generated by appending order_id, status_code, gross_amount, and ServerKey into a string, then hashing it with SHA-512. The server key is a shared secret only Midtrans and the merchant know, so nobody else can build this signature hash.

def validate_signature(self, payload):
    try:
        self.payload = json.loads(payload)
    except json.JSONDecodeError:
        raise InvalidMidtransPayload("Invalid JSON payload")

    order_id = str(self.payload.get("order_id"))
    status = str(self.payload.get("status_code"))
    gross_amount = str(self.payload.get("gross_amount"))
    signature = self.payload.get("signature_key")

    raw = f"{order_id}{status}{gross_amount}{settings.MIDTRANS_SERVER_KEY}"
    expected_signature = hashlib.sha512(raw.encode()).hexdigest()

    if signature != expected_signature:
        raise InvalidMidtransSignature("Invalid Midtrans signature")
    return self.payload

If the JSON body is malformed, or the signature doesn't match, the request is rejected before touching the database at all — the order lookup and status update only happen after this check passes.

Bug #1: The fraud_status Trap

My first version of the status logic looked like this:

if transaction_status in ["settlement", "capture"]:
    if fraud_status == "accept":
        self.order.payment_status = "paid"

This passed every test I initially wrote, because those tests used card-like payloads with fraud_status: "accept". It looked done.

The problem surfaced once I checked how Midtrans' Fraud Detection System actually applies. Whitelisting for FDS can only be done for card transactions and does not apply to other payment methods which is a strong signal that FDS, and by extension fraud_status, is a concept tied specifically to card payments. QRIS, bank transfer, and other non-card methods in this store's enabled_payments list aren't evaluated by it the same way. In practice, that means fraud_status can arrive as None for those methods — and my original check silently rejected every one of them, because None == "accept" is False.

The fix:

if transaction_status in ["settlement", "capture"]:
    new_status = "paid" if fraud_status in (None, "accept") else "failed"

Nothing crashed. No error was logged. Orders paid through the store's most common payment methods would have just... never turned into paid, silently sitting in pending forever. That's the kind of bug that doesn't show up until someone complains their payment "went through" but the order never updated — which is a much worse way to find out than reading the docs.

Idempotency — Why Webhooks Retry, and Why That Breaks Naive Code

Webhooks aren't a one-shot delivery. In extremely rare cases, Midtrans may send multiple notifications for the same transaction event, and it should not cause duplicate entries at the merchant's end. If your server is slow to respond, times out, or returns anything other than a clean success, Midtrans will try again.

That single fact breaks a lot of naive webhook code. If reduce_stock() runs every time a "paid" notification arrives, without checking whether it already ran, a retried webhook will deduct stock twice for the same order — once because the customer actually bought two items, and once again because Midtrans wasn't sure the first notification got through.

The fix is a guard based on state, not on trusting each webhook to only arrive once:

def reduce_stock(self):
    try:
        if not self.order.reduced_stock:
            reduce_product_stock(self.order.items.all())
            self.order.reduced_stock = True
            self.order.save(update_fields=["reduced_stock"])
    except Exception:
        logger_error.exception(
            "Failed to reserve stock",
            extra={"event_type": "transaction", "order_id": self.order.order_id},
        )
        raise

reduced_stock is a boolean on the order itself, not a separate idempotency-key table. The first webhook flips it to True and deducts stock. Every subsequent webhook for the same order — retry or otherwise — sees True and does nothing. Midtrans' own recommendation is to use order_id as the key to track entries and implement notifications in an idempotent way, and this is the simplest version of that: reuse a field that already has to exist, instead of adding new infrastructure just to deduplicate.

The exception handling matters here too. If reduce_product_stock() fails — say, a database hiccup — the exception is logged and re-raised, not swallowed. That makes the view return a 500, which tells Midtrans to retry. The retry mechanism Midtrans already provides becomes the recovery mechanism, as long as the operation stays idempotent.

The Reversal Case Nobody Talks About

Most webhook tutorials treat settlement as a terminal state: the money is in, the order is paid, done. Midtrans' own documentation says otherwise for a specific set of payment methods: for Permata Bank Transfer, Mandiri Bill Payment, and Indomaret, in rare cases settlement can later change into deny — known as Reversal — usually within 1-5 minutes, caused by the payment provider's network. When that happens, the transaction fund is reversed back to the customer, and no fund is received on the merchant side.

That means payment_status can't just move
forward through pending → paid and stop there. It needs a rule for when paid is allowed to become failed again — and, just as importantly, a rule for when it isn't.

def change_payment_status_order(self):
    transaction_status = self.payload.get("transaction_status")
    fraud_status = self.payload.get("fraud_status")
    old_status = self.order.payment_status

    if transaction_status in ["settlement", "capture"]:
        new_status = "paid" if fraud_status in (None, "accept") else "failed"
    elif transaction_status in ["deny", "cancel", "expire"]:
        new_status = "failed"
    else:
        self.old_status = old_status
        self.new_status = old_status
        return old_status == "paid"

    if old_status == "failed" and new_status == "paid":
        logger_error.warning(
            "Order sudah failed, transaksi mencoba jadi paid - diabaikan",
            extra={
                "event_type": "transaction",
                "order_id": self.order.order_id,
                "transaction_status": transaction_status,
            },
        )
        self.old_status = old_status
        self.new_status = old_status
        return False

    if old_status != new_status:
        self.order.payment_status = new_status
        self.order.save(update_fields=["payment_status"])

    self.old_status = old_status
    self.new_status = new_status
    return new_status == "paid"

The rule ends up asymmetric on purpose: paid → failed is allowed (that's the reversal), but failed → paid is blocked. Midtrans never documents a transaction coming back to life after being denied, cancelled, or expired — so if that transition is ever requested, it's
treated as a stale or out-of-order notification, logged, and ignored rather than trusted.

Two Kinds of Stock

Before going into the explanation, it's worth being explicit about something the code relies on but never says out loud: this store tracks two separate stock numbers per product.

stock — the actual, physical count of items available.
reserved_stock — items temporarily held during checkout, before payment is confirmed one way or another.

A customer starting checkout doesn't reduce stock yet — it bumps reserved_stock, so the same item can't be oversold to someone else while this customer is still deciding whether to pay. stock itself only moves once the order's fate is actually known.

That distinction matters because a failed order can fail in two very different ways, and each one needs a different cleanup:

pending → paid — reduce_stock() runs, reducing both stock and reserved_stock.
pending → failed — release_reservation() runs, reducing only reserved_stock. stock is untouched, since it was never deducted in the first place.
paid → failed (reversal) — reverse_stock() runs, increasing stock back. reserved_stock is untouched, since it was already cleared when the order first became paid.

The code backs this up directly. reduce_product_stock(), called when an order becomes paid, touches both fields:

product.stock -= item.qty
product.reserved_stock -= item.qty
product.save(update_fields=["stock", "reserved_stock"])

restore_product_stock(), called on reversal, touches only stock:

product.stock += item.qty
product.save(update_fields=["stock"])

And release_reservation(), called when an order fails before ever reaching paid, touches only reserved_stock:

product.reserved_stock -= item.qty
product.save(update_fields=["reserved_stock"])

Without release_reservation(), an abandoned checkout — someone who starts paying and never finishes — would leave reserved_stock inflated forever. Nothing physically sold, but the store would slowly look more out of stock than it actually is.

Where it gets tricky

release_reservation() needs to run only when an order fails for the first time — not on a reversal, and not on a duplicate webhook. A single guard based on reduced_stock isn't enough, because reverse_stock() (called just before it) also modifies that same flag as a side effect, so it stops being a reliable signal for what's happening in this webhook.

The guard that actually holds checks both the status before and after this specific request:

webhook_midtrans.reverse_stock()
if webhook_midtrans.new_status == "failed" and webhook_midtrans.old_status not in ("paid", "failed"):
    webhook_midtrans.release_reservation()

This narrows the trigger down to exactly one case: an order moving into failed for the first time, ruling out both reversals and duplicate notifications.

Testing Strategy — Three Layers, Three Purposes

Once the logic settled, the next question was how to test it without either missing real bugs or drowning in redundant tests. I ended up with three layers, each proving something the others can't.

Unit tests — WebhookMidtrans methods tested in isolation, with MagicMock() standing in for the order, products, and database. These cover the decision logic itself: status transitions for every combination of transaction_status and fraud_status, the idempotency guards on reduce_stock() and reverse_stock(), and signature validation against malformed or mismatched payloads. No database, no HTTP — just whether the logic produces the right output for a given input.

def test_paid_to_failed_on_reversal(self):
    payload = {"transaction_status": "deny", "fraud_status": None}
    webhook = self._build_webhook(payload, current_status="paid")

    result = webhook.change_payment_status_order()

    self.assertFalse(result)
    self.assertEqual(webhook.order.payment_status, "failed")
    self.assertEqual(webhook.old_status, "paid")

Integration tests with mocks — MidtransWebhookView tested through the actual HTTP layer, but with WebhookMidtrans mocked entirely. These don't re-test the logic above; they test wiring — that the view calls reduce_stock() when is_paid is True and reverse_stock() otherwise, that each exception type from get_order() maps to the right status code, and that a failure in reduce_stock() or reverse_stock() returns a 500 rather than a misleading success.

End-to-end tests (TransactionTestCase, a real database) — the view, WebhookMidtrans, and the actual models, no mocking. This is the only layer that can prove things the other two can't: that transaction.atomic() really rolls back every write in a loop when one item fails partway through, that model relationships and field names actually line up, and that a real database failure doesn't leave stock partially updated.

def test_reduce_stock_failure_rolls_back_earlier_item_in_same_loop(self):
    self.product_b.stock = 1  # not enough for the requested qty
    self.product_b.save(update_fields=["stock"])

    order = self._make_order(payment_status="pending")
    body = self._signed_payload(order.order_id, "settlement", fraud_status=None)

    response = self._post(body)

    self.assertEqual(response.status_code, 500)
    self.product_a.refresh_from_db()
    self.assertEqual(self.product_a.stock, 10)  # rolled back, not silently deducted

That last test is the reason all three layers exist. A mock can tell you reduce_stock() was called. It can't tell you what happens to a second product in the same order when the third one fails partway through a loop, inside a transaction, against a real database. Only running it for real answers that.

A Known Gap I'm Leaving As-Is

There's a logging issue in the retry handling I haven't fixed yet.

When reduce_stock() or reverse_stock() fails, the handler logs a critical message telling whoever's on call to review the order manually, then returns a 500 so Midtrans retries. That's fine on its own. But if the retry keeps failing, each attempt logs another "needs manual review" message — five failed retries means five log entries for what's really one ongoing problem. And if the first attempt fails but a retry two seconds later succeeds, that one critical log still fired for nothing, even though the order resolved itself.

Neither case is dangerous — the order's actual state stays correct either way, this is purely a logging noise problem. Fixing it properly would mean tracking failure state per order instead of logging on every attempt — a small change, but one I haven't made yet.

Closing

None of these bugs were dramatic. Nothing crashed, no stack trace pointed at the problem, no error log screamed for attention. That's exactly what makes payment code different from most other backend work — a silent failure here doesn't mean a feature is broken, it means money moves and your records don't agree with it, or stock quietly drifts out of sync with what's actually on the shelf. The cost of getting it wrong shows up somewhere else, later, as a support ticket or a reconciliation headache.

Working through this taught me to treat "it passed the happy path" as the start of the review, not the end of it — and to actually read the payment provider's documentation for the cases it explicitly calls out as rare, instead of assuming those cases are rare enough to ignore.

The full implementation, including all three layers of tests, is available on success GitHub. If you're working on something similar or spot something I missed, I'd genuinely like to hear about it — reach out on LinkedIn or drop a comment below.