From d30962bf6651755cf0d1c78b0328bc17eb0ab895 Mon Sep 17 00:00:00 2001 From: Konstantin Wohlwend Date: Mon, 25 May 2026 17:50:09 -0700 Subject: [PATCH] Fix GH tokens refresh & devtool tabs --- .claude/CLAUDE-KNOWLEDGE.md | 3 + .../access-token/crud.tsx | 1 + .../[provider_id]/access-token/crud.tsx | 1 + .../access-token-helpers.tsx | 22 ++- apps/backend/src/oauth/providers/base.tsx | 19 ++- apps/backend/src/oauth/providers/github.tsx | 11 +- .../new-project/page-client-parts/content.tsx | 2 + .../api/v1/connected-accounts.test.ts | 151 ++++++++++-------- packages/stack-shared/src/utils/urls.tsx | 2 + .../template/src/dev-tool/dev-tool-core.ts | 23 ++- .../apps/implementations/client-app-impl.ts | 4 +- .../apps/implementations/server-app-impl.ts | 4 +- 12 files changed, 154 insertions(+), 89 deletions(-) diff --git a/.claude/CLAUDE-KNOWLEDGE.md b/.claude/CLAUDE-KNOWLEDGE.md index cc2deb0c8..a0788788d 100644 --- a/.claude/CLAUDE-KNOWLEDGE.md +++ b/.claude/CLAUDE-KNOWLEDGE.md @@ -2,6 +2,9 @@ This file contains knowledge learned while working on the codebase in Q&A format. +## Q: How are connected-account OAuth tokens stored and refreshed? +A: Connected accounts live in `ProjectUserOAuthAccount`. Stored refresh tokens are in `OAuthToken` (`oauthAccountId`, `scopes`, `isValid`), and cached access tokens are in `OAuthAccessToken` (`expiresAt`, `scopes`, `isValid`). `retrieveOrRefreshAccessToken` first uses an unexpired valid access token with matching scopes; otherwise it looks for valid refresh tokens with matching scopes and invalidates only those that the provider explicitly rejects. + ## Q: Which Hexclave rename compatibility layers should be avoided in PR #1475 follow-ups? A: Do not keep backwards compatibility for the MCP tool name, cross-domain auth query parameter names, `NEXT_PUBLIC_STACK_PORT_PREFIX`, or a parallel `hexclaveAppInternalsSymbol`. For refresh/access cookies, read both legacy Stack and new Hexclave cookie names, but only write the canonical Hexclave cookies. diff --git a/apps/backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/[provider_account_id]/access-token/crud.tsx b/apps/backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/[provider_account_id]/access-token/crud.tsx index f664d10cf..80381a499 100644 --- a/apps/backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/[provider_account_id]/access-token/crud.tsx +++ b/apps/backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/[provider_account_id]/access-token/crud.tsx @@ -61,6 +61,7 @@ export const connectedAccountAccessTokenByAccountCrudHandlers = createLazyProxy( return await retrieveOrRefreshAccessToken({ prisma, providerInstance, + providerId: params.provider_id, tenancyId: auth.tenancy.id, oauthAccountIds: [oauthAccount.id], scope: data.scope, diff --git a/apps/backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/access-token/crud.tsx b/apps/backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/access-token/crud.tsx index abffc593b..9403032c8 100644 --- a/apps/backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/access-token/crud.tsx +++ b/apps/backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/access-token/crud.tsx @@ -56,6 +56,7 @@ export const connectedAccountAccessTokenCrudHandlers = createLazyProxy(() => cre return await retrieveOrRefreshAccessToken({ prisma, providerInstance, + providerId: params.provider_id, tenancyId: auth.tenancy.id, oauthAccountIds: oauthAccounts.map(a => a.id), scope: data.scope, diff --git a/apps/backend/src/app/api/latest/connected-accounts/access-token-helpers.tsx b/apps/backend/src/app/api/latest/connected-accounts/access-token-helpers.tsx index 9286f1baa..156b7e990 100644 --- a/apps/backend/src/app/api/latest/connected-accounts/access-token-helpers.tsx +++ b/apps/backend/src/app/api/latest/connected-accounts/access-token-helpers.tsx @@ -94,15 +94,20 @@ import.meta.vitest?.describe("isSharedAccessTokenBlocked", () => { export async function retrieveOrRefreshAccessToken(options: { prisma: Awaited>, providerInstance: OAuthBaseProvider, + providerId: string, tenancyId: string, oauthAccountIds: string[], scope: string | undefined, errorContext: Record, }): Promise<{ access_token: string }> { - const { prisma, providerInstance, tenancyId, oauthAccountIds, scope, errorContext } = options; + const { prisma, providerInstance, providerId, tenancyId, oauthAccountIds, scope, errorContext } = options; const accountIdFilter = oauthAccountIds.length === 1 ? { oauthAccountId: oauthAccountIds[0] } : { oauthAccountId: { in: oauthAccountIds } }; + const reauthorizeDetails = "The stored OAuth refresh token is missing, expired, revoked, or no longer accepted by the OAuth provider. The user needs to re-authorize this connected account."; + const requiredScopeDetails = scope + ? `The OAuth connection does not have a usable refresh token with the required scope (${scope}). The user needs to re-authorize this connected account with the requested scope.` + : "The OAuth connection does not have a usable refresh token for this connected account. The user needs to re-authorize this connected account."; // ====================== retrieve access token if it exists ====================== const accessTokens = await prisma.oAuthAccessToken.findMany({ @@ -142,10 +147,15 @@ export async function retrieveOrRefreshAccessToken(options: { return extractScopes(scope || "").every((s) => t.scopes.includes(s)); }); - if (filteredRefreshTokens.length === 0) { - throw new KnownErrors.OAuthConnectionDoesNotHaveRequiredScope(); + if (refreshTokens.length === 0) { + throw new KnownErrors.OAuthAccessTokenNotAvailable(providerId, reauthorizeDetails); } + if (filteredRefreshTokens.length === 0) { + throw new KnownErrors.OAuthAccessTokenNotAvailable(providerId, requiredScopeDetails); + } + + let invalidatedRefreshTokenDuringAttempt = false; for (const token of filteredRefreshTokens) { const tokenSetResult = await providerInstance.getAccessToken({ refreshToken: token.refreshToken, @@ -162,6 +172,7 @@ export async function retrieveOrRefreshAccessToken(options: { where: { id: token.id }, data: { isValid: false }, }); + invalidatedRefreshTokenDuringAttempt = true; continue; } case "temporarily-unavailable": { @@ -247,5 +258,8 @@ export async function retrieveOrRefreshAccessToken(options: { } } - throw new KnownErrors.OAuthConnectionDoesNotHaveRequiredScope(); + throw new KnownErrors.OAuthAccessTokenNotAvailable( + providerId, + invalidatedRefreshTokenDuringAttempt ? reauthorizeDetails : requiredScopeDetails, + ); } diff --git a/apps/backend/src/oauth/providers/base.tsx b/apps/backend/src/oauth/providers/base.tsx index 17d4e2aa3..c0a1e7038 100644 --- a/apps/backend/src/oauth/providers/base.tsx +++ b/apps/backend/src/oauth/providers/base.tsx @@ -217,7 +217,13 @@ export function getOAuthAccessTokenRefreshError(error: unknown, options: { return { type: "unexpected", cause: error, ...metadata }; } -function processTokenSet(providerName: string, tokenSet: OIDCTokenSet, defaultAccessTokenExpiresInMillis?: number): TokenSet { +type DefaultAccessTokenExpiresInMillis = number | ((tokenSet: OIDCTokenSet) => number | undefined); + +function getDefaultAccessTokenExpiresInMillis(tokenSet: OIDCTokenSet, defaultAccessTokenExpiresInMillis?: DefaultAccessTokenExpiresInMillis): number | undefined { + return typeof defaultAccessTokenExpiresInMillis === "function" ? defaultAccessTokenExpiresInMillis(tokenSet) : defaultAccessTokenExpiresInMillis; +} + +function processTokenSet(providerName: string, tokenSet: OIDCTokenSet, defaultAccessTokenExpiresInMillis?: DefaultAccessTokenExpiresInMillis): TokenSet { if (!tokenSet.access_token) { throw new HexclaveAssertionError(`No access token received from ${providerName}.`, { tokenSet, providerName }); } @@ -225,8 +231,9 @@ function processTokenSet(providerName: string, tokenSet: OIDCTokenSet, defaultAc // if expires_in or expires_at provided, use that // otherwise, if defaultAccessTokenExpiresInMillis provided, use that // otherwise, use 1h, and log an error + const defaultExpiresInMillis = getDefaultAccessTokenExpiresInMillis(tokenSet, defaultAccessTokenExpiresInMillis); - if (!tokenSet.expires_in && !tokenSet.expires_at && !defaultAccessTokenExpiresInMillis) { + if (!tokenSet.expires_in && !tokenSet.expires_at && !defaultExpiresInMillis) { captureError("processTokenSet", new HexclaveAssertionError(`No expires_in or expires_at received from OAuth provider ${providerName}. Falling back to 1h`, { tokenSetKeys: Object.keys(tokenSet) })); } @@ -237,8 +244,8 @@ function processTokenSet(providerName: string, tokenSet: OIDCTokenSet, defaultAc accessTokenExpiredAt: tokenSet.expires_in ? new Date(Date.now() + tokenSet.expires_in * 1000) : tokenSet.expires_at ? new Date(tokenSet.expires_at * 1000) : - defaultAccessTokenExpiresInMillis ? - new Date(Date.now() + defaultAccessTokenExpiresInMillis) : + defaultExpiresInMillis ? + new Date(Date.now() + defaultExpiresInMillis) : new Date(Date.now() + 3600 * 1000), }; } @@ -249,7 +256,7 @@ export abstract class OAuthBaseProvider { public readonly scope: string, public readonly redirectUri: string, public readonly authorizationExtraParams?: Record, - public readonly defaultAccessTokenExpiresInMillis?: number, + public readonly defaultAccessTokenExpiresInMillis?: DefaultAccessTokenExpiresInMillis, public readonly noPKCE?: boolean, public readonly openid?: boolean, public readonly alternativeIssuers?: string[], @@ -262,7 +269,7 @@ export abstract class OAuthBaseProvider { redirectUri: string, baseScope: string, authorizationExtraParams?: Record, - defaultAccessTokenExpiresInMillis?: number, + defaultAccessTokenExpiresInMillis?: DefaultAccessTokenExpiresInMillis, tokenEndpointAuthMethod?: "client_secret_post" | "client_secret_basic", noPKCE?: boolean, alternativeIssuers?: string[], diff --git a/apps/backend/src/oauth/providers/github.tsx b/apps/backend/src/oauth/providers/github.tsx index 69c550db6..fbdd499bf 100644 --- a/apps/backend/src/oauth/providers/github.tsx +++ b/apps/backend/src/oauth/providers/github.tsx @@ -23,10 +23,15 @@ export class GithubProvider extends OAuthBaseProvider { userinfoEndpoint: "https://api.github.com/user", redirectUri: getEnvVariable("NEXT_PUBLIC_STACK_API_URL") + "/api/v1/auth/oauth/callback/github", baseScope: "user:email", - // GitHub token does not expire except for lack of use in a year - // We set a default of 1 year + // GitHub can return either non-expiring OAuth-App-style access tokens, or + // expiring user tokens with refresh tokens. If GitHub gives us expires_in, + // the base provider uses that real value. This fallback is only for older + // responses without explicit expiry: refresh-token responses should be + // treated as short-lived, while access-token-only responses are long-lived + // and are still checked against /user before being returned. + // https://docs.github.com/en/apps/creating-github-apps/authenticating-with-a-github-app/refreshing-user-access-tokens // https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/token-expiration-and-revocation#user-token-expired-due-to-github-app-configuration - defaultAccessTokenExpiresInMillis: 1000 * 60 * 60 * 8, // 8 hours + defaultAccessTokenExpiresInMillis: (tokenSet) => tokenSet.refresh_token ? 1000 * 60 * 60 * 8 : 1000 * 60 * 60 * 24 * 365, ...options, })); } diff --git a/apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/content.tsx b/apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/content.tsx index 4f975f9ab..76da09258 100644 --- a/apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/content.tsx +++ b/apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/content.tsx @@ -76,6 +76,7 @@ function PageClientInner() { const [projectOnboardingStates, setProjectOnboardingStates] = useState>(new Map()); const [loadingStatuses, setLoadingStatuses] = useState(true); const [projectName, setProjectName] = useState(displayNameFromSearch ?? ""); + const hasProjectName = projectName.trim().length > 0; const [selectedTeamId, setSelectedTeamId] = useState(null); const [creatingTeam, setCreatingTeam] = useState(false); const [creatingProject, setCreatingProject] = useState(false); @@ -376,6 +377,7 @@ function PageClientInner() { { if (!beginPendingAction(creatingProjectRef, setCreatingProject)) { diff --git a/apps/e2e/tests/backend/endpoints/api/v1/connected-accounts.test.ts b/apps/e2e/tests/backend/endpoints/api/v1/connected-accounts.test.ts index 0f1823f2c..c13380b3d 100644 --- a/apps/e2e/tests/backend/endpoints/api/v1/connected-accounts.test.ts +++ b/apps/e2e/tests/backend/endpoints/api/v1/connected-accounts.test.ts @@ -3,6 +3,12 @@ import { localhostUrl } from "../../../../helpers/ports"; import { Auth, backendContext, createMailbox, niceBackendFetch } from "../../../backend-helpers"; const mockOAuthUrl = (path: string) => localhostUrl("14", path); +const reauthorizeAccessTokenDetails = "The stored OAuth refresh token is missing, expired, revoked, or no longer accepted by the OAuth provider. The user needs to re-authorize this connected account."; +const spotifyAccessTokenNotAvailableError = `Failed to retrieve an OAuth access token for the connected account (provider: spotify). ${reauthorizeAccessTokenDetails}`; + +function missingScopeDetails(scope: string) { + return `The OAuth connection does not have a usable refresh token with the required scope (${scope}). The user needs to re-authorize this connected account with the requested scope.`; +} it("should use the connected account access token to access the userinfo endpoint of the oauth provider", async ({ expect }) => { await Auth.OAuth.signIn(); @@ -122,6 +128,28 @@ it("should refresh the connected account access token when it is revoked from th `); }); +it("should return a scope-specific error when connected account tokens do not have the requested scope", async ({ expect }) => { + await Auth.OAuth.signIn(); + + const scope = "missing-scope"; + const details = missingScopeDetails(scope); + const response = await niceBackendFetch("/api/v1/connected-accounts/me/spotify/access-token", { + accessType: "client", + method: "POST", + body: { scope }, + }); + + expect(response.status).toBe(400); + expect(response.body).toEqual({ + code: "OAUTH_ACCESS_TOKEN_NOT_AVAILABLE", + details: { + provider: "spotify", + details, + }, + error: `Failed to retrieve an OAuth access token for the connected account (provider: spotify). ${details}`, + }); +}); + it("should prompt the user to re-authorize the connected account when the refresh token is revoked from the oauth provider", async ({ expect }) => { await Auth.OAuth.signIn(); @@ -175,19 +203,15 @@ it("should prompt the user to re-authorize the connected account when the refres scope: "openid", }, }); - expect(response5).toMatchInlineSnapshot(` - NiceResponse { - "status": 400, - "body": { - "code": "OAUTH_CONNECTION_DOES_NOT_HAVE_REQUIRED_SCOPE", - "error": "The OAuth connection does not have the required scope.", - }, - "headers": Headers { - "x-stack-known-error": "OAUTH_CONNECTION_DOES_NOT_HAVE_REQUIRED_SCOPE", -