diff --git a/.claude/CLAUDE-KNOWLEDGE.md b/.claude/CLAUDE-KNOWLEDGE.md index a0788788d..b3f8b6805 100644 --- a/.claude/CLAUDE-KNOWLEDGE.md +++ b/.claude/CLAUDE-KNOWLEDGE.md @@ -3,7 +3,7 @@ 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. +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/access-token-helpers.tsx b/apps/backend/src/app/api/latest/connected-accounts/access-token-helpers.tsx index 156b7e990..0d1d112bc 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 @@ -114,9 +114,10 @@ export async function retrieveOrRefreshAccessToken(options: { 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, }, }); diff --git a/apps/backend/src/oauth/providers/base.tsx b/apps/backend/src/oauth/providers/base.tsx index c0a1e7038..e50f32835 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,23 +217,20 @@ export function getOAuthAccessTokenRefreshError(error: unknown, options: { return { type: "unexpected", cause: error, ...metadata }; } -type DefaultAccessTokenExpiresInMillis = number | ((tokenSet: OIDCTokenSet) => number | undefined); - -function getDefaultAccessTokenExpiresInMillis(tokenSet: OIDCTokenSet, defaultAccessTokenExpiresInMillis?: DefaultAccessTokenExpiresInMillis): number | undefined { - return typeof defaultAccessTokenExpiresInMillis === "function" ? defaultAccessTokenExpiresInMillis(tokenSet) : defaultAccessTokenExpiresInMillis; -} +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 - const defaultExpiresInMillis = getDefaultAccessTokenExpiresInMillis(tokenSet, defaultAccessTokenExpiresInMillis); + // 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 && !defaultExpiresInMillis) { + 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) })); } @@ -241,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) : - defaultExpiresInMillis ? - new Date(Date.now() + defaultExpiresInMillis) : - 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), }; } diff --git a/apps/backend/src/oauth/providers/github.tsx b/apps/backend/src/oauth/providers/github.tsx index fbdd499bf..80a57d317 100644 --- a/apps/backend/src/oauth/providers/github.tsx +++ b/apps/backend/src/oauth/providers/github.tsx @@ -27,11 +27,13 @@ export class GithubProvider extends OAuthBaseProvider { // 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. + // 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: (tokenSet) => tokenSet.refresh_token ? 1000 * 60 * 60 * 8 : 1000 * 60 * 60 * 24 * 365, + defaultAccessTokenExpiresInMillis: (tokenSet) => tokenSet.refresh_token ? 1000 * 60 * 60 * 8 : null, ...options, })); }