diff --git a/.claude/CLAUDE-KNOWLEDGE.md b/.claude/CLAUDE-KNOWLEDGE.md index cc2deb0c8..b3f8b6805 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`). A null `OAuthAccessToken.expiresAt` means the OAuth provider did not supply an access-token expiry; `retrieveOrRefreshAccessToken` treats null-expiry tokens as candidates and still calls the provider-specific validity check before returning them. If no usable access token exists, 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/prisma/migrations/20260526060000_nullable_oauth_access_token_expires_at/migration.sql b/apps/backend/prisma/migrations/20260526060000_nullable_oauth_access_token_expires_at/migration.sql new file mode 100644 index 000000000..a2aac294f --- /dev/null +++ b/apps/backend/prisma/migrations/20260526060000_nullable_oauth_access_token_expires_at/migration.sql @@ -0,0 +1 @@ +ALTER TABLE "OAuthAccessToken" ALTER COLUMN "expiresAt" DROP NOT NULL; diff --git a/apps/backend/prisma/migrations/20260526060000_nullable_oauth_access_token_expires_at/tests/nullable-expires-at.ts b/apps/backend/prisma/migrations/20260526060000_nullable_oauth_access_token_expires_at/tests/nullable-expires-at.ts new file mode 100644 index 000000000..c127b775d --- /dev/null +++ b/apps/backend/prisma/migrations/20260526060000_nullable_oauth_access_token_expires_at/tests/nullable-expires-at.ts @@ -0,0 +1,86 @@ +import { randomUUID } from "crypto"; +import type { Sql } from "postgres"; +import { expect } from "vitest"; + +export const preMigration = async (sql: Sql) => { + const projectId = `test-${randomUUID()}`; + const tenancyId = randomUUID(); + const projectUserId = randomUUID(); + const oauthAccountId = randomUUID(); + + await sql` + INSERT INTO "Project" ("id", "createdAt", "updatedAt", "displayName", "description", "isProductionMode") + VALUES (${projectId}, NOW(), NOW(), 'Test', '', false) + `; + await sql` + INSERT INTO "Tenancy" ("id", "createdAt", "updatedAt", "projectId", "branchId", "hasNoOrganization") + VALUES (${tenancyId}::uuid, NOW(), NOW(), ${projectId}, 'main', 'TRUE'::"BooleanTrue") + `; + await sql` + INSERT INTO "ProjectUser" ("projectUserId", "tenancyId", "mirroredProjectId", "mirroredBranchId", "createdAt", "updatedAt", "lastActiveAt") + VALUES (${projectUserId}::uuid, ${tenancyId}::uuid, ${projectId}, 'main', NOW(), NOW(), NOW()) + `; + await sql` + INSERT INTO "ProjectUserOAuthAccount" ( + "id", + "tenancyId", + "projectUserId", + "configOAuthProviderId", + "providerAccountId", + "createdAt", + "updatedAt" + ) + VALUES ( + ${oauthAccountId}::uuid, + ${tenancyId}::uuid, + ${projectUserId}::uuid, + 'github', + 'github-account', + NOW(), + NOW() + ) + `; + + return { tenancyId, oauthAccountId }; +}; + +export const postMigration = async (sql: Sql, ctx: Awaited>) => { + const columnRows = await sql` + SELECT is_nullable, data_type + FROM information_schema.columns + WHERE table_name = 'OAuthAccessToken' + AND column_name = 'expiresAt' + `; + expect(columnRows).toHaveLength(1); + expect(columnRows[0].is_nullable).toBe("YES"); + expect(columnRows[0].data_type).toBe("timestamp without time zone"); + + await sql` + INSERT INTO "OAuthAccessToken" ( + "id", + "tenancyId", + "oauthAccountId", + "accessToken", + "scopes", + "expiresAt" + ) + VALUES ( + ${randomUUID()}::uuid, + ${ctx.tenancyId}::uuid, + ${ctx.oauthAccountId}::uuid, + 'github-access-token-without-expiry', + ARRAY['user:email']::text[], + NULL + ) + `; + + const tokenRows = await sql` + SELECT "expiresAt" + FROM "OAuthAccessToken" + WHERE "tenancyId" = ${ctx.tenancyId}::uuid + AND "oauthAccountId" = ${ctx.oauthAccountId}::uuid + AND "accessToken" = 'github-access-token-without-expiry' + `; + expect(tokenRows).toHaveLength(1); + expect(tokenRows[0].expiresAt).toBeNull(); +}; diff --git a/apps/backend/prisma/schema.prisma b/apps/backend/prisma/schema.prisma index e76edcadc..1b4d8544f 100644 --- a/apps/backend/prisma/schema.prisma +++ b/apps/backend/prisma/schema.prisma @@ -621,7 +621,7 @@ model OAuthAccessToken { accessToken String scopes String[] - expiresAt DateTime + expiresAt DateTime? isValid Boolean @default(true) } 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 c53fa5f75..ee8891c4c 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 dee2aa98d..d167c15b5 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 47b38aa15..87b543c84 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,24 +94,30 @@ 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({ where: { tenancyId, ...accountIdFilter, - expiresAt: { - gt: new Date(Date.now() + 5 * 60 * 1000), - }, + OR: [ + { expiresAt: null }, + { expiresAt: { gt: new Date(Date.now() + 5 * 60 * 1000) } }, + ], isValid: true, }, }); @@ -142,10 +148,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 +173,7 @@ export async function retrieveOrRefreshAccessToken(options: { where: { id: token.id }, data: { isValid: false }, }); + invalidatedRefreshTokenDuringAttempt = true; continue; } case "temporarily-unavailable": { @@ -247,5 +259,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 fe0bc5345..5b6d46d98 100644 --- a/apps/backend/src/oauth/providers/base.tsx +++ b/apps/backend/src/oauth/providers/base.tsx @@ -37,7 +37,7 @@ custom.setHttpOptionsDefaults({ export type TokenSet = { accessToken: string, refreshToken?: string, - accessTokenExpiredAt: Date, + accessTokenExpiredAt: Date | null, idToken?: string, }; @@ -217,16 +217,20 @@ export function getOAuthAccessTokenRefreshError(error: unknown, options: { return { type: "unexpected", cause: error, ...metadata }; } -function processTokenSet(providerName: string, tokenSet: OIDCTokenSet, defaultAccessTokenExpiresInMillis?: number): TokenSet { +type DefaultAccessTokenExpiresInMillis = number | null | ((tokenSet: OIDCTokenSet) => number | null | undefined); + +function processTokenSet(providerName: string, tokenSet: OIDCTokenSet, defaultAccessTokenExpiresInMillis?: DefaultAccessTokenExpiresInMillis): TokenSet { if (!tokenSet.access_token) { throw new HexclaveAssertionError(`No access token received from ${providerName}.`, { tokenSet, providerName }); } - // if expires_in or expires_at provided, use that - // otherwise, if defaultAccessTokenExpiresInMillis provided, use that - // otherwise, use 1h, and log an error + // Use provider-supplied expiry first. If the provider omits expiry, a provider + // can supply a fallback duration, return null to explicitly model + // "non-expiring/unknown expiry", or leave it undefined to use the generic + // one-hour fallback and capture telemetry. + const defaultExpiresInMillis = typeof defaultAccessTokenExpiresInMillis === "function" ? defaultAccessTokenExpiresInMillis(tokenSet) : defaultAccessTokenExpiresInMillis; - if (!tokenSet.expires_in && !tokenSet.expires_at && !defaultAccessTokenExpiresInMillis) { + if (tokenSet.expires_in == null && tokenSet.expires_at == null && defaultExpiresInMillis === undefined) { captureError("processTokenSet", new HexclaveAssertionError(`No expires_in or expires_at received from OAuth provider ${providerName}. Falling back to 1h`, { tokenSetKeys: Object.keys(tokenSet) })); } @@ -234,12 +238,13 @@ function processTokenSet(providerName: string, tokenSet: OIDCTokenSet, defaultAc idToken: tokenSet.id_token, accessToken: tokenSet.access_token, refreshToken: tokenSet.refresh_token, - accessTokenExpiredAt: tokenSet.expires_in ? + accessTokenExpiredAt: tokenSet.expires_in != null ? new Date(Date.now() + tokenSet.expires_in * 1000) : - tokenSet.expires_at ? new Date(tokenSet.expires_at * 1000) : - defaultAccessTokenExpiresInMillis ? - new Date(Date.now() + defaultAccessTokenExpiresInMillis) : - new Date(Date.now() + 3600 * 1000), + tokenSet.expires_at != null ? new Date(tokenSet.expires_at * 1000) : + defaultExpiresInMillis === null ? null : + defaultExpiresInMillis !== undefined ? + new Date(Date.now() + defaultExpiresInMillis) : + new Date(Date.now() + 3600 * 1000), }; } @@ -249,7 +254,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 +267,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 89defba65..ee9cb82f6 100644 --- a/apps/backend/src/oauth/providers/github.tsx +++ b/apps/backend/src/oauth/providers/github.tsx @@ -23,10 +23,17 @@ 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. Access-token-only responses are effectively + // non-expiring OAuth App tokens, so store NULL to mean "the provider did + // not supply an expiry"; they 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 : null, ...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 485dea7ff..68f6e6f68 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", -