Skip to content

Commit d69773c

Browse files
committed
Retry OAuth refreshes
1 parent 616d805 commit d69773c

6 files changed

Lines changed: 307 additions & 53 deletions

File tree

.claude/CLAUDE-KNOWLEDGE.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,9 @@ A: Check the error location:
217217
- Callback endpoint (400 error) - Validation failed during callback
218218
- Token endpoint (400 error) - Validation failed during token exchange
219219

220+
### Q: How should connected-account OAuth access-token refresh errors be classified?
221+
A: In `apps/backend/src/oauth/providers/base.tsx`, invalid/revoked refresh-token provider errors such as `invalid_grant` return `Result.error({ type: "invalid-refresh-token", ... })` so `access-token-helpers.tsx` can invalidate that stored refresh token and try another. Transient provider/network failures such as openid-client `RPError: outgoing request timed out after 3500ms` return `Result.error({ type: "temporarily-unavailable", cause })`; the connected-account helper converts that to `OAuthProviderTemporarilyUnavailable` without invalidating the refresh token. Expected refresh outcomes should be represented in the provider return type instead of thrown as known/status errors. Refresh requests use a 6s openid-client HTTP timeout and retry transient failures once. If a retry sees `invalid_grant` after an ambiguous transient failure, keep treating it as temporarily unavailable rather than invalidating the refresh token, because the first request may have reached the provider and rotated the token before our client timed out. Sentry should capture non-revocation refresh issues (temporary provider failure, invalid client, unexpected) with provider id/class, attempts, retry count, ambiguity state, final cause, and all provider errors seen during attempts; normal revoked/expired refresh tokens should not be reported.
222+
220223
## Git and Development Workflow
221224

222225
### Q: How should you format git commit messages in this project?

apps/backend/src/app/api/latest/connected-accounts/access-token-helpers.tsx

Lines changed: 92 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,37 @@
1-
import { OAuthBaseProvider, TokenSet } from "@/oauth/providers/base";
1+
import { OAuthBaseProvider } from "@/oauth/providers/base";
2+
import type { OAuthAccessTokenRefreshError } from "@/oauth/providers/base";
23
import { getPrismaClientForTenancy } from "@/prisma-client";
34
import { KnownErrors } from "@stackframe/stack-shared";
45
import { getEnvVariable } from "@stackframe/stack-shared/dist/utils/env";
5-
import { StackAssertionError, captureError } from "@stackframe/stack-shared/dist/utils/errors";
6-
import { Result } from "@stackframe/stack-shared/dist/utils/results";
6+
import { StackAssertionError, StatusError, captureError } from "@stackframe/stack-shared/dist/utils/errors";
77
import { extractScopes } from "@stackframe/stack-shared/dist/utils/strings";
88

9+
function captureOAuthAccessTokenRefreshIssue(options: {
10+
location: string,
11+
message: string,
12+
providerInstance: OAuthBaseProvider,
13+
errorContext: Record<string, unknown>,
14+
refreshError: Exclude<OAuthAccessTokenRefreshError, { type: "invalid-refresh-token" }>,
15+
}) {
16+
const providerId = typeof options.errorContext.providerId === "string" ? options.errorContext.providerId : "unknown";
17+
const providerClass = options.providerInstance.constructor.name;
18+
captureError(options.location, new StackAssertionError(
19+
`${options.message} (providerId: ${providerId}, providerClass: ${providerClass}, attempts: ${options.refreshError.attempts}, retries: ${options.refreshError.retryCount})`,
20+
{
21+
cause: options.refreshError.cause,
22+
...options.errorContext,
23+
providerId,
24+
providerClass,
25+
refreshErrorType: options.refreshError.type,
26+
attempts: options.refreshError.attempts,
27+
retryCount: options.refreshError.retryCount,
28+
sawAmbiguousRefreshAttempt: options.refreshError.sawAmbiguousRefreshAttempt,
29+
error: options.refreshError.cause,
30+
causes: options.refreshError.causes,
31+
},
32+
));
33+
}
34+
935
/**
1036
* Access tokens minted under Stack Auth's shared OAuth apps must not be handed
1137
* to clients — they carry Stack Auth's brand at the provider. Only allowed when
@@ -122,27 +148,71 @@ export async function retrieveOrRefreshAccessToken(options: {
122148
}
123149

124150
for (const token of filteredRefreshTokens) {
125-
let tokenSetResult: Result<TokenSet, string>;
126-
try {
127-
tokenSetResult = await providerInstance.getAccessToken({
128-
refreshToken: token.refreshToken,
129-
scope,
130-
});
131-
} catch (error) {
132-
captureError('oauth-access-token-refresh-unexpected-error', new StackAssertionError('Unexpected error refreshing access token — this may indicate a bug or misconfiguration', {
133-
error,
134-
...errorContext,
135-
}));
136-
137-
tokenSetResult = Result.error("Unexpected error refreshing access token");
138-
}
151+
const tokenSetResult = await providerInstance.getAccessToken({
152+
refreshToken: token.refreshToken,
153+
scope,
154+
});
139155

140156
if (tokenSetResult.status === "error") {
141-
await prisma.oAuthToken.update({
142-
where: { id: token.id },
143-
data: { isValid: false },
144-
});
145-
continue;
157+
switch (tokenSetResult.error.type) {
158+
case "invalid-refresh-token": {
159+
// Only this outcome proves that the stored refresh token should no
160+
// longer be used. Provider timeouts and retry ambiguity must not
161+
// invalidate the connection; the user usually cannot fix those.
162+
await prisma.oAuthToken.update({
163+
where: { id: token.id },
164+
data: { isValid: false },
165+
});
166+
continue;
167+
}
168+
case "temporarily-unavailable": {
169+
// The customer should retry later. Do not mark the OAuth connection as
170+
// broken or ask the user to reconnect based on a transient provider
171+
// failure.
172+
captureOAuthAccessTokenRefreshIssue({
173+
location: "oauth-access-token-refresh-provider-temporarily-unavailable",
174+
message: "OAuth provider temporarily unavailable during access token refresh",
175+
providerInstance,
176+
errorContext,
177+
refreshError: tokenSetResult.error,
178+
});
179+
throw new KnownErrors.OAuthProviderTemporarilyUnavailable();
180+
}
181+
case "invalid-client": {
182+
captureOAuthAccessTokenRefreshIssue({
183+
location: "oauth-access-token-refresh-invalid-client",
184+
message: "OAuth provider rejected configured client during access token refresh",
185+
providerInstance,
186+
errorContext,
187+
refreshError: tokenSetResult.error,
188+
});
189+
throw new StatusError(400, `Invalid client credentials for this OAuth provider. Please ensure the configuration in the Stack Auth dashboard is correct.`);
190+
}
191+
case "unexpected": {
192+
captureOAuthAccessTokenRefreshIssue({
193+
location: "oauth-access-token-refresh-unexpected-error",
194+
message: "Unexpected OAuth provider error during access token refresh",
195+
providerInstance,
196+
errorContext,
197+
refreshError: tokenSetResult.error,
198+
});
199+
const assertionError = new StackAssertionError('Unexpected error refreshing access token — this may indicate a bug or misconfiguration', {
200+
error: tokenSetResult.error.cause,
201+
providerClass: providerInstance.constructor.name,
202+
refreshErrorType: tokenSetResult.error.type,
203+
attempts: tokenSetResult.error.attempts,
204+
retryCount: tokenSetResult.error.retryCount,
205+
sawAmbiguousRefreshAttempt: tokenSetResult.error.sawAmbiguousRefreshAttempt,
206+
causes: tokenSetResult.error.causes,
207+
...errorContext,
208+
});
209+
throw assertionError;
210+
}
211+
default: {
212+
const _: never = tokenSetResult.error;
213+
throw new StackAssertionError("Unhandled OAuth access token refresh error", { error: _ });
214+
}
215+
}
146216
}
147217

148218
const tokenSet = tokenSetResult.data;

apps/backend/src/oauth/providers/base.test.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it } from "vitest";
2-
import { isRetryableOAuthUserInfoError } from "./base";
2+
import { getOAuthAccessTokenRefreshError, getOAuthAccessTokenRefreshErrorDisposition, isRetryableOAuthUserInfoError } from "./base";
33

44
describe("isRetryableOAuthUserInfoError", () => {
55
it("returns true for openid-client timeout errors", () => {
@@ -52,3 +52,52 @@ describe("isRetryableOAuthUserInfoError", () => {
5252
})).toBe(true);
5353
});
5454
});
55+
56+
describe("getOAuthAccessTokenRefreshErrorDisposition", () => {
57+
it("treats openid-client refresh timeouts as temporarily unavailable", () => {
58+
expect(getOAuthAccessTokenRefreshErrorDisposition({
59+
name: "RPError",
60+
message: "outgoing request timed out after 3500ms",
61+
})).toEqual({ type: "temporarily-unavailable" });
62+
});
63+
64+
it("treats invalid_grant refresh failures as invalid refresh tokens", () => {
65+
expect(getOAuthAccessTokenRefreshErrorDisposition({
66+
error: "invalid_grant",
67+
})).toEqual({
68+
type: "invalid-refresh-token",
69+
message: "Refresh token is invalid or expired",
70+
});
71+
});
72+
73+
it("recognizes nested OAuth provider error codes", () => {
74+
expect(getOAuthAccessTokenRefreshErrorDisposition({
75+
error: {
76+
error: "invalid_grant",
77+
},
78+
})).toEqual({
79+
type: "invalid-refresh-token",
80+
message: "Refresh token is invalid or expired",
81+
});
82+
});
83+
});
84+
85+
describe("getOAuthAccessTokenRefreshError", () => {
86+
it("does not treat invalid_grant after an ambiguous refresh attempt as a revoked token", () => {
87+
const providerError = {
88+
error: "invalid_grant",
89+
};
90+
expect(getOAuthAccessTokenRefreshError(providerError, {
91+
sawAmbiguousRefreshAttempt: true,
92+
attempts: 2,
93+
causes: [{ name: "RPError" }, providerError],
94+
})).toEqual({
95+
type: "temporarily-unavailable",
96+
cause: providerError,
97+
attempts: 2,
98+
retryCount: 1,
99+
sawAmbiguousRefreshAttempt: true,
100+
causes: [{ name: "RPError" }, providerError],
101+
});
102+
});
103+
});

0 commit comments

Comments
 (0)