fix(security): RQ-3893 stop reading and writing appSumoCodes from the browser - #115
fix(security): RQ-3893 stop reading and writing appSumoCodes from the browser#115rohitneharabrowserstack wants to merge 1 commit into
Conversation
… browser
This component performed both halves of AppSumo redemption client-side: it read
`appSumoCodes/{code}` to validate a typed code, then wrote `redeemed: true` in a
batch after the callable returned. That is the only reason the Firestore rules
granted every authenticated user read+update on the collection — which let any
account enumerate licence codes and clear `redeemed` on a paying customer's.
requestly/requestly-cloud#880 closes the collection and moves both operations to
the server. Those rules cannot deploy until this ships: with the collection
locked, the read fails so no code ever validates and the submit button never
enables, and the redeem write fails silently (it was un-awaited with no catch),
leaving codes reusable while the user sees a success toast.
- verifyCode calls the new `subscription-validateAppSumoCodes` callable, which
returns a verdict only for codes the caller supplied rather than exposing the
collection to a wildcard read (which also grants `list`).
- redeemSubmittedCodes is deleted; the callable now marks codes redeemed in the
same transaction that grants the tier.
- An unsuccessful response that is not `max_limit_reached` no longer falls into
the success branch and reports "unlocked" for a redemption that never happened.
The workspace picker already restricted itself to workspaces where the caller is
an admin, so it needs no change to match the callable's new admin check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Type-check verified
A set diff of the two error lists is empty — this change introduces no new type errors. Nothing in the output mentions Two caveats, so the numbers are not read as more than they are:
Getting this to run at all needed Behaviour notes for review
|
|
The coupling runs in both directions:
Safe order is functions → client → rules:
Both PR descriptions have been updated. Since steps 1 and 3 live in the same PR yet must not deploy together, #880 is probably worth splitting (functions in one PR, |
|
Superseded by #116. AppSumo is being retired, so the redemption screen is deleted rather than rewired through server-side callables — see #116 and requestly/requestly-cloud#882. This PR is still correct as written, but it is now unnecessary work: it exists to keep a screen functioning that is being removed, and it carries a deploy-ordering dependency on the new callables that #116 does not have. Recommend closing. |
Client half of RQ-3893. Prerequisite for requestly/requestly-cloud#880 — those Firestore rules cannot deploy until this ships.
Why this is needed
requestly-cloud#880 closes an escalation path: the
appSumoCodesFirestore rules granted every authenticated userread, update, so any account could enumerate licence codes and clearredeemedon a paying customer's. The fix locks the collection toif false.This component is the reason those permissions existed. It performed both halves of redemption in the browser:
Deploying the rules against this code would have broken redemption outright:
verifiedis never set, soisAllCodeCheckPassedstays false andonSubmitthrows "Please fill all the fields correctly" — nobody could redeem at all.redeemSubmittedCodes()was called un-awaited inside.then()with nocatch, it failed silently — the user would see the success toast while the code stayed unredeemed and infinitely reusable.Changes
Validation moves to a callable
verifyCodenow callssubscription-validateAppSumoCodes(added in requestly-cloud#880) instead of reading Firestore. The callable returns a verdict only for codes the caller supplied, so it is not an enumeration oracle — the previous wildcardallow readalso grantedlist, which is what made the whole table readable.The user-facing messages are unchanged: "Invalid code" for a code that does not exist, "Code already redeemed" for one already used, plus a new "Could not verify this code, please try again" when the call itself fails (previously an exception here left the input silently unverified).
Client-side redemption removed
redeemSubmittedCodes()is deleted. The server now marks codes redeemed inside the same transaction that grants the tier, which also closes the race the client version had between readingredeemedand setting it.Failed redemptions no longer report success
if (!response?.data?.success && response?.data?.error === "max_limit_reached") { setShowMaxCodesExeceededError(true); + } else if (!response?.data?.success) { + toast.error(response?.data?.message || "Could not redeem these codes, please try again", 10); } else {Any unsuccessful response that was not
max_limit_reachedpreviously fell into the success branch and showed "Lifetime access … unlocked" for a redemption that never happened. The outercatchnow surfaces a toast as well, instead of onlyconsole.error.The component no longer imports
firebase/firestoreat all.Not changed
The workspace picker already restricts to workspaces where the caller is an admin (
AppSumoWorkspaceDropdown,members[uid].role === "admin"), so it is consistent with the new admin check on the callable. No change needed there.Deploy order
Corrected 2026-08-26. An earlier revision of this section said to deploy this PR first. That is wrong: this PR calls
subscription-validateAppSumoCodes, so shipping it before that callable exists makeshttpsCallablereject withfunctions/not-found, the input never verifies, and the submit button never enables — the same breakage, just from the other side.The safe order is:
subscription-validateAppSumoCodesand the reworkedsubscription-updateTeamSubscriptionForAppSumo). Additive — the currently-deployed client does not call them.appSumoCodes.Deploying this PR before step 1, or the rules before step 2, breaks AppSumo redemption.