If you integrate with Fortnox, the Swedish accounting system, there is one OAuth detail that will bite you the first time real users click fast: a refresh token can be used exactly once. Fortnox rotates it on every refresh. Spend the same one twice and the second call gets invalid_grant.
That sounds harmless until you look at how a normal app behaves.
How it breaks
The access token lives for an hour. When it has expired, the next API call refreshes it first. The obvious code looks like this:
async function call(tenant, method, path, body) {
if (Date.now() > tenant.expires_at) {
const t = await refresh(tenant.refresh_token); // POST /oauth-v1/token
Object.assign(tenant, t);
await save(tenant);
}
return fetch(API + path, { method, headers: { Authorization: `Bearer ${tenant.access_token}` }, body });
}
Now a user opens the app after lunch. The app loads two screens at once, or the user presses "send all" on five receipts. Five calls start within a few milliseconds. All five read the same tenant row from the database, all five see an expired access token, and all five refresh with the same refresh token.
The first one wins and gets a new pair. The other four get invalid_grant. Depending on how you handle that error, the user is either told their Fortnox connection is broken and must be reconnected, or, worse, one of the losing calls saves an error state over the good tokens the winner just stored.
Nothing was actually wrong with the connection. The user did nothing wrong. It only happens under concurrency, so it never shows up when you test one request at a time.
The fix: one refresh per refresh token
The rule is simple: for any given refresh token, only one refresh may ever be in flight. Everybody else who finds the token expired waits for that refresh and uses its result.
In a single Node process, a Map of promises keyed by the refresh token is enough:
const refreshing = new Map();
function sharedRefresh(tenant, save) {
const key = tenant.refresh_token;
let p = refreshing.get(key);
if (!p) {
// Save inside the shared promise, so the new pair is in the database
// before anyone waiting on it reads the connection again.
p = refresh(key).then(async (t) => { await save({ ...tenant, ...t }); return t; });
refreshing.set(key, p);
p.then(
// Keep a successful result for a minute (see below).
() => setTimeout(() => { if (refreshing.get(key) === p) refreshing.delete(key); }, 60_000).unref?.(),
// Drop a failure at once, so the next call can try again.
() => { if (refreshing.get(key) === p) refreshing.delete(key); },
);
}
return p;
}
async function call(tenant, save, method, path, opts) {
if (Date.now() > tenant.expires_at) Object.assign(tenant, await sharedRefresh(tenant, save));
// ... the actual request
}
Three details matter more than they look.
1. Save before you resolve. If the promise resolved first and saved afterwards, a waiter could reload the tenant from the database in between and find the old refresh token. It would then start a second refresh with a spent token, which is exactly the bug again.
2. Keep the successful result for a while. A request that read the tenant row a moment before the new tokens were saved still carries the old refresh token. If the map entry were deleted the moment the refresh finished, that late request would find nothing in the map and start its own refresh with the spent token. Keeping the resolved promise for a minute means late readers of the old token still get the new pair.
3. Forget failures immediately. If Fortnox is down for a second, you don't want every call for the next minute to get the same cached rejection.
Two more things that are easy to miss
Refresh with the credentials that issued the token. If you have more than one Fortnox app (we have two: one for receipts, one for a point-of-sale integration), a token must be refreshed with the client ID and secret of the app that issued it. Refresh a token from app B with app A's credentials and you get a rejection that looks exactly like a revoked connection. Store which app a tenant belongs to next to the tokens.
This is per process. The Map works because our API runs as one Node process. With several instances behind a load balancer, you need the same idea with a shared lock, for example a row lock in Postgres or a Redis SET NX with a short TTL, and the other instances re-reading the tokens once the lock is released.
How to test it
The bug only exists under concurrency, so test it that way. Mock the token endpoint to be slow (100 ms is plenty) and to reject any refresh token it has already seen, expire the access token, then fire several calls at once:
await Promise.all([1, 2, 3, 4, 5].map(() => call(tenant, save, 'GET', '/companyinformation')));
// expect: exactly one POST to the token endpoint, five successful calls
Without the shared refresh, four of the five calls fail. With it, the token endpoint is hit once and all five succeed.
I found this while building KvittoFlow, a receipt and mileage app that books straight into Fortnox. A "send all" button is exactly the kind of feature that triggers it.
Top comments (0)