mirror of
https://github.com/stack-auth/stack.git
synced 2026-06-04 21:04:37 +08:00
Longer refresh token expiries for OAuth providers that don't return one
This commit is contained in:
parent
bef9452c95
commit
fae8d2dfab
@ -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.
|
||||
|
||||
@ -0,0 +1 @@
|
||||
ALTER TABLE "OAuthAccessToken" ALTER COLUMN "expiresAt" DROP NOT NULL;
|
||||
@ -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<ReturnType<typeof preMigration>>) => {
|
||||
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();
|
||||
};
|
||||
@ -621,7 +621,7 @@ model OAuthAccessToken {
|
||||
|
||||
accessToken String
|
||||
scopes String[]
|
||||
expiresAt DateTime
|
||||
expiresAt DateTime?
|
||||
isValid Boolean @default(true)
|
||||
}
|
||||
|
||||
|
||||
@ -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,
|
||||
},
|
||||
});
|
||||
|
||||
@ -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),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@ -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,
|
||||
}));
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user