Skip to content

Commit 8a89dd5

Browse files
committed
Gate OAuth token refresh on credential-store writability
A vault write failure while persisting a refreshed OAuth token lost the rotated refresh token: the grant had already consumed the single-use stored token at the authorization server, so once the vault recovered the next refresh replayed the revoked token, got invalid_grant, and the connection demanded a re-auth over what was only a storage blip. Before consuming the refresh token, rewrite the stored value in place as a writability probe. A store outage now fails the resolve before the grant, leaving the stored token valid so the connection recovers with the store.
1 parent 7a17493 commit 8a89dd5

2 files changed

Lines changed: 34 additions & 20 deletions

File tree

e2e/cloud/vault-write-outage.test.ts

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,30 +7,31 @@
77
// 1. An OAuth connection's access token expires (per-client TTL of 1s via
88
// the emulator's DCR `access_token_ttl_seconds` extension, well inside
99
// the 60s refresh skew).
10-
// 2. A health check resolves the connection → triggers the refresh-token
11-
// grant. The grant SUCCEEDS and the authorization server ROTATES the
12-
// refresh token (single use — the emulator mirrors AuthKit).
13-
// 3. Persisting the new tokens does read → `PUT /vault/v1/kv/:id`. The PUT
14-
// fails (in prod: 409 version conflict, an OAuth-shaped 400, or an HTML
15-
// error page; here: the armed 400 with `error`/`error_description`, the
16-
// exact shape the WorkOS SDK maps to OauthException — Sentry issue -53).
17-
// 4. The write failure surfaces as `StorageError: WorkOS Vault secret write
10+
// 2. A health check resolves the connection → the refresh path runs. Vault
11+
// writes go through read → `PUT /vault/v1/kv/:id`; the PUT fails (in
12+
// prod: 409 version conflict, an OAuth-shaped 400, or an HTML error
13+
// page; here: the armed 400 with `error`/`error_description`, the exact
14+
// shape the WorkOS SDK maps to OauthException — Sentry issue -53).
15+
// 3. The write failure surfaces as `StorageError: WorkOS Vault secret write
1816
// failed` → the health endpoint answers a typed InternalError (the 500
19-
// Sentry records).
17+
// Sentry records). Crucially it must fail BEFORE the refresh-token grant
18+
// (the AS rotates the single-use token; consuming it with an unwritable
19+
// store loses the rotated copy forever).
2020
//
21-
// The second scenario pins the DAMAGE, not just the surface error: the vault
22-
// write failed AFTER the refresh token was consumed and rotated, so the new
23-
// refresh token is lost and the stored one is revoked. Once the vault
24-
// recovers, the connection must recover too — today it cannot (the next
25-
// refresh gets invalid_grant → the connection reads "expired" and demands a
26-
// re-auth). Red until the refresh/persist ordering is made crash-safe.
21+
// The second scenario pins the DAMAGE the surface error used to hide: when
22+
// the vault write failed AFTER the refresh token was consumed and rotated,
23+
// the rotated token was lost and the stored one already revoked — the next
24+
// refresh got invalid_grant and the connection demanded a re-auth over a
25+
// storage blip. The fix gates the grant on a proof-of-writability rewrite of
26+
// the stored refresh token, so a vault outage fails BEFORE the single-use
27+
// token is spent and the connection recovers with the vault.
2728
import { randomBytes } from "node:crypto";
2829

2930
import { expect } from "@effect/vitest";
3031
import { Effect } from "effect";
3132
import type { HttpApiClient } from "effect/unstable/httpapi";
3233
import { composePluginApi } from "@executor-js/api/server";
33-
import { connectEmulator, type EmulatorClient } from "@executor-js/emulate";
34+
import { connectEmulator } from "@executor-js/emulate";
3435
import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api";
3536
import {
3637
AuthTemplateSlug,
@@ -243,22 +244,24 @@ scenario(
243244
).toBe("InternalError");
244245

245246
// The proof this is the production mechanism and not an incidental
246-
// failure: the product performed the refresh grant and then hit the
247-
// injected fault on the vault update.
247+
// failure: the product hit the injected fault on the vault update leg.
248248
const ledger = yield* Effect.promise(() => workos.ledger.list(200));
249249
const faultedPut = ledger.find((entry) => entry.faulted === true && entry.method === "PUT");
250250
expect(faultedPut?.path, "the failure came from the injected vault-update fault").toMatch(
251251
/\/vault\/v1\/kv\//,
252252
);
253+
// The crash-safety contract: the write-gate fails BEFORE the refresh
254+
// grant, so the single-use refresh token is never consumed while the
255+
// store cannot persist its rotated successor.
253256
const refreshGrant = ledger.find(
254257
(entry) =>
255258
entry.path.endsWith("/oauth2/token") &&
256259
JSON.stringify(entry.request.body ?? "").includes("refresh_token"),
257260
);
258261
expect(
259262
refreshGrant,
260-
"the vault write that failed was persisting a completed refresh grant",
261-
).toBeDefined();
263+
"no refresh grant is issued while the vault cannot persist the rotated token",
264+
).toBeUndefined();
262265
}),
263266
Effect.gen(function* () {
264267
yield* Effect.promise(() => workos.faults.clear());

packages/core/sdk/src/executor.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1570,6 +1570,17 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
15701570
if (!refreshToken) {
15711571
return yield* reauth("Stored refresh token could not be resolved.");
15721572
}
1573+
// Prove the credential store is WRITABLE before consuming the
1574+
// single-use refresh token: the grant rotates it at the AS, so
1575+
// a persist failure after the grant loses the only copy of the
1576+
// rotated token and permanently invalidates the connection
1577+
// (the next refresh replays the revoked token → invalid_grant
1578+
// → forced re-auth). A no-op rewrite of the value just read
1579+
// fails fast during a store outage, leaving the stored token
1580+
// valid so the connection recovers when the store does.
1581+
if (provider.set) {
1582+
yield* provider.set(ProviderItemId.make(row.refresh_item_id), refreshToken);
1583+
}
15731584
return yield* refreshAccessToken({
15741585
tokenUrl,
15751586
clientId: String(clientRow.client_id),

0 commit comments

Comments
 (0)