From b2a019979928277f55b2f44231ee87fba70593ff Mon Sep 17 00:00:00 2001 From: Konstantin Wohlwend Date: Fri, 3 Oct 2025 01:35:09 -0700 Subject: [PATCH] Make refresh tokens more robust when deleting users --- .../oauth/callback/[provider_id]/route.tsx | 24 +++- .../app/api/latest/auth/oauth/token/route.tsx | 2 +- .../auth/sessions/current/refresh/route.tsx | 15 ++- apps/backend/src/lib/tokens.tsx | 106 ++++++++++-------- apps/backend/src/oauth/model.tsx | 53 +++++---- .../endpoints/api/v1/auth/oauth/token.test.ts | 60 ++++++++++ .../v1/auth/sessions/current/refresh.test.ts | 35 ++++++ packages/stack-shared/src/utils/strings.tsx | 3 + 8 files changed, 215 insertions(+), 83 deletions(-) diff --git a/apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx b/apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx index c86891c01..03465bb14 100644 --- a/apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx +++ b/apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx @@ -354,7 +354,29 @@ const handler = createSmartRouteHandler({ throw new KnownErrors.SignUpNotEnabled(); } - const currentUser = projectUserId ? await usersCrudHandlers.adminRead({ tenancy, user_id: projectUserId }) : null; + // Set currentUser to the user that was signed in with the `token` access token during the /authorize request + let currentUser; + if (projectUserId) { + // note that it's possible that the user has been deleted, but the request is still done with a token that was issued before the user was deleted + // (or the user was deleted between the /authorize and /callback requests) + // hence, we catch the error and ignore if that's the case + try { + currentUser = await usersCrudHandlers.adminRead({ + tenancy, + user_id: projectUserId, + allowedErrorTypes: [KnownErrors.UserNotFound], + }); + } catch (error) { + if (KnownErrors.UserNotFound.isInstance(error)) { + currentUser = null; + } else { + throw error; + } + } + } else { + currentUser = null; + } + const newAccountBeforeAuthMethod = await createOrUpgradeAnonymousUser( tenancy, currentUser, diff --git a/apps/backend/src/app/api/latest/auth/oauth/token/route.tsx b/apps/backend/src/app/api/latest/auth/oauth/token/route.tsx index b1cc88bc7..ac7d61f4a 100644 --- a/apps/backend/src/app/api/latest/auth/oauth/token/route.tsx +++ b/apps/backend/src/app/api/latest/auth/oauth/token/route.tsx @@ -41,7 +41,7 @@ export const POST = createSmartRouteHandler({ oauthRequest, oauthResponse, { - // note the `accessTokenLifetime` won't have any effect here because we set it in the `generateAccessToken` function + // note the `accessTokenLifetime` won't have any effect here because we set it in the `generateAccessTokenFromRefreshTokenIfValid` function refreshTokenLifetime: 60 * 60 * 24 * 365, // 1 year alwaysIssueNewRefreshToken: false, // add token rotation later } diff --git a/apps/backend/src/app/api/latest/auth/sessions/current/refresh/route.tsx b/apps/backend/src/app/api/latest/auth/sessions/current/refresh/route.tsx index d23962d22..f7e2da99a 100644 --- a/apps/backend/src/app/api/latest/auth/sessions/current/refresh/route.tsx +++ b/apps/backend/src/app/api/latest/auth/sessions/current/refresh/route.tsx @@ -1,4 +1,4 @@ -import { generateAccessToken } from "@/lib/tokens"; +import { generateAccessTokenFromRefreshTokenIfValid } from "@/lib/tokens"; import { getPrismaClientForTenancy, globalPrismaClient } from "@/prisma-client"; import { createSmartRouteHandler } from "@/route-handlers/smart-route-handler"; import { KnownErrors } from "@stackframe/stack-shared"; @@ -37,16 +37,15 @@ export const POST = createSmartRouteHandler({ }, }); - if (!sessionObj || (sessionObj.expiresAt && sessionObj.expiresAt < new Date())) { + const accessToken = await generateAccessTokenFromRefreshTokenIfValid({ + tenancy, + refreshTokenObj: sessionObj, + }); + + if (!accessToken) { throw new KnownErrors.RefreshTokenNotFoundOrExpired(); } - const accessToken = await generateAccessToken({ - tenancy, - userId: sessionObj.projectUserId, - refreshTokenId: sessionObj.id, - }); - return { statusCode: 200, bodyType: "json", diff --git a/apps/backend/src/lib/tokens.tsx b/apps/backend/src/lib/tokens.tsx index 56061d845..92585bbec 100644 --- a/apps/backend/src/lib/tokens.tsx +++ b/apps/backend/src/lib/tokens.tsx @@ -1,11 +1,11 @@ import { usersCrudHandlers } from '@/app/api/latest/users/crud'; import { globalPrismaClient } from '@/prisma-client'; -import { Prisma } from '@prisma/client'; import { KnownErrors } from '@stackframe/stack-shared'; import { yupBoolean, yupNumber, yupObject, yupString } from "@stackframe/stack-shared/dist/schema-fields"; +import { AccessTokenPayload } from '@stackframe/stack-shared/dist/sessions'; import { generateSecureRandomString } from '@stackframe/stack-shared/dist/utils/crypto'; import { getEnvVariable } from '@stackframe/stack-shared/dist/utils/env'; -import { StackAssertionError, throwErr } from '@stackframe/stack-shared/dist/utils/errors'; +import { StackAssertionError } from '@stackframe/stack-shared/dist/utils/errors'; import { getPrivateJwks, getPublicJwkSet, signJWT, verifyJWT } from '@stackframe/stack-shared/dist/utils/jwt'; import { Result } from '@stackframe/stack-shared/dist/utils/results'; import { traceSpan } from '@stackframe/stack-shared/dist/utils/telemetry'; @@ -13,7 +13,6 @@ import * as jose from 'jose'; import { JOSEError, JWTExpired } from 'jose/errors'; import { SystemEventTypes, logEvent } from './events'; import { Tenancy } from './tenancies'; -import { AccessTokenPayload } from '@stackframe/stack-shared/dist/sessions'; export const authorizationHeaderSchema = yupString().matches(/^StackSession [^ ]+$/); @@ -116,25 +115,45 @@ export async function decodeAccessToken(accessToken: string, { allowAnonymous }: }); } -export async function generateAccessToken(options: { +export async function isRefreshTokenValid(options: { tenancy: Tenancy, - userId: string, - refreshTokenId: string, + refreshTokenObj: null | { + projectUserId: string, + id: string, + expiresAt: Date | null, + }, }) { + return !!await generateAccessTokenFromRefreshTokenIfValid(options); +} + +export async function generateAccessTokenFromRefreshTokenIfValid(options: { + tenancy: Tenancy, + refreshTokenObj: null | { + projectUserId: string, + id: string, + expiresAt: Date | null, + }, +}) { + if (!options.refreshTokenObj) { + return null; + } + + if (options.refreshTokenObj.expiresAt && options.refreshTokenObj.expiresAt < new Date()) { + return null; + } + let user; try { user = await usersCrudHandlers.adminRead({ tenancy: options.tenancy, - user_id: options.userId, + user_id: options.refreshTokenObj.projectUserId, allowedErrorTypes: [KnownErrors.UserNotFound], }); } catch (error) { if (error instanceof KnownErrors.UserNotFound) { - throw new StackAssertionError(`User not found in generateAccessToken. Was the user's account deleted?`, { - userId: options.userId, - refreshTokenId: options.refreshTokenId, - tenancy: options.tenancy, - }); + // The user was deleted — their refresh token still exists because we don't cascade deletes across source-of-truth/global tables. + // => refresh token is invalid + return null; } throw error; } @@ -144,17 +163,17 @@ export async function generateAccessToken(options: { { projectId: options.tenancy.project.id, branchId: options.tenancy.branchId, - userId: options.userId, - sessionId: options.refreshTokenId, + userId: options.refreshTokenObj.projectUserId, + sessionId: options.refreshTokenObj.id, isAnonymous: user.is_anonymous, } ); const payload: Omit = { - sub: options.userId, + sub: options.refreshTokenObj.projectUserId, project_id: options.tenancy.project.id, branch_id: options.tenancy.branchId, - refresh_token_id: options.refreshTokenId, + refresh_token_id: options.refreshTokenObj.id, role: 'authenticated', name: user.display_name, email: user.primary_email, @@ -171,44 +190,39 @@ export async function generateAccessToken(options: { }); } -export async function createAuthTokens(options: { +type CreateRefreshTokenOptions = { tenancy: Tenancy, projectUserId: string, expiresAt?: Date, isImpersonation?: boolean, -}) { +} + +export async function createRefreshTokenObj(options: CreateRefreshTokenOptions) { options.expiresAt ??= new Date(Date.now() + 1000 * 60 * 60 * 24 * 365); options.isImpersonation ??= false; const refreshToken = generateSecureRandomString(); - try { - const refreshTokenObj = await globalPrismaClient.projectUserRefreshToken.create({ - data: { - tenancyId: options.tenancy.id, - projectUserId: options.projectUserId, - refreshToken: refreshToken, - expiresAt: options.expiresAt, - isImpersonation: options.isImpersonation, - }, - }); + const refreshTokenObj = await globalPrismaClient.projectUserRefreshToken.create({ + data: { + tenancyId: options.tenancy.id, + projectUserId: options.projectUserId, + refreshToken: refreshToken, + expiresAt: options.expiresAt, + isImpersonation: options.isImpersonation, + }, + }); - const accessToken = await generateAccessToken({ - tenancy: options.tenancy, - userId: options.projectUserId, - refreshTokenId: refreshTokenObj.id, - }); - - - return { refreshToken, accessToken }; - - } catch (error) { - if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2003') { - throwErr(new Error( - `Auth token creation failed for tenancyId ${options.tenancy.id} and projectUserId ${options.projectUserId}: ${error.message}`, - { cause: error } - )); - } - throw error; - } + return refreshTokenObj; +} + +export async function createAuthTokens(options: CreateRefreshTokenOptions) { + const refreshTokenObj = await createRefreshTokenObj(options); + + const accessToken = await generateAccessTokenFromRefreshTokenIfValid({ + tenancy: options.tenancy, + refreshTokenObj: refreshTokenObj, + }); + + return { refreshToken: refreshTokenObj.refreshToken, accessToken }; } diff --git a/apps/backend/src/oauth/model.tsx b/apps/backend/src/oauth/model.tsx index af103ca38..fe6706a19 100644 --- a/apps/backend/src/oauth/model.tsx +++ b/apps/backend/src/oauth/model.tsx @@ -2,12 +2,11 @@ import { createMfaRequiredError } from "@/app/api/latest/auth/mfa/sign-in/verifi import { checkApiKeySet } from "@/lib/internal-api-keys"; import { validateRedirectUrl } from "@/lib/redirect-urls"; import { getSoleTenancyFromProjectBranch, getTenancy } from "@/lib/tenancies"; -import { decodeAccessToken, generateAccessToken } from "@/lib/tokens"; +import { createRefreshTokenObj, decodeAccessToken, generateAccessTokenFromRefreshTokenIfValid, isRefreshTokenValid } from "@/lib/tokens"; import { getPrismaClientForTenancy, globalPrismaClient } from "@/prisma-client"; import { AuthorizationCode, AuthorizationCodeModel, Client, Falsey, RefreshToken, Token, User } from "@node-oauth/oauth2-server"; import { PrismaClientKnownRequestError } from "@prisma/client/runtime/library"; import { KnownErrors } from "@stackframe/stack-shared"; -import { generateSecureRandomString } from "@stackframe/stack-shared/dist/utils/crypto"; import { captureError, throwErr } from "@stackframe/stack-shared/dist/utils/errors"; import { getProjectBranchFromClientId } from "."; @@ -98,35 +97,20 @@ export class OAuthModel implements AuthorizationCodeModel { assertScopeIsValid(scope); const tenancy = await getSoleTenancyFromProjectBranch(...getProjectBranchFromClientId(client.id)); - if (!user.refreshTokenId) { - // create new refresh token - const refreshToken = await this.generateRefreshToken(client, user, scope); - // save it in user, then we just access it in refresh - // HACK: This is a hack to ensure the refresh token is already there so we can associate the access token with it - const newRefreshToken = await globalPrismaClient.projectUserRefreshToken.create({ - data: { - refreshToken, - tenancyId: tenancy.id, - projectUserId: user.id, - expiresAt: new Date(), - }, - }); - user.refreshTokenId = newRefreshToken.id; - } + const refreshTokenObj = await this._getOrCreateRefreshTokenObj(client, user, scope); - return await generateAccessToken({ + return await generateAccessTokenFromRefreshTokenIfValid({ tenancy, - userId: user.id, - refreshTokenId: user.refreshTokenId ?? throwErr("Refresh token ID not found on OAuth user"), - }); + refreshTokenObj, + }) ?? throwErr("Get or create refresh token failed; returned refreshTokenObj that's invalid (or maybe it's an ultra-rare race condition and it became invalid in since the function call?)", { refreshTokenObj }); // TODO fix the ultra-rare race condition — although unless we're at gigascale this should basically never happen } - async generateRefreshToken(client: Client, user: User, scope: string[]): Promise { - assertScopeIsValid(scope); + async _getOrCreateRefreshTokenObj(client: Client, user: User, scope: string[]) { + const tenancy = await getSoleTenancyFromProjectBranch(...getProjectBranchFromClientId(client.id)); + // if refresh token already exists and is valid, return it if (user.refreshTokenId) { - const tenancy = await getSoleTenancyFromProjectBranch(...getProjectBranchFromClientId(client.id)); - const refreshToken = await globalPrismaClient.projectUserRefreshToken.findUniqueOrThrow({ + const refreshTokenObj = await globalPrismaClient.projectUserRefreshToken.findUnique({ where: { tenancyId_id: { tenancyId: tenancy.id, @@ -134,10 +118,25 @@ export class OAuthModel implements AuthorizationCodeModel { }, }, }); - return refreshToken.refreshToken; + if (refreshTokenObj && await isRefreshTokenValid({ tenancy, refreshTokenObj })) { + return refreshTokenObj; + } } - return generateSecureRandomString(); + // otherwise, create a new refresh token and set its ID on the user + const refreshTokenObj = await createRefreshTokenObj({ + tenancy, + projectUserId: user.id, + }); + user.refreshTokenId = refreshTokenObj.id; + return refreshTokenObj; + } + + async generateRefreshToken(client: Client, user: User, scope: string[]): Promise { + assertScopeIsValid(scope); + + const tokenObj = await this._getOrCreateRefreshTokenObj(client, user, scope); + return tokenObj.refreshToken; } async saveToken(token: Token, client: Client, user: User): Promise { diff --git a/apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/token.test.ts b/apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/token.test.ts index c0860a895..3abc6aeee 100644 --- a/apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/token.test.ts +++ b/apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/token.test.ts @@ -95,6 +95,66 @@ describe("with grant_type === 'authorization_code'", async () => { await Auth.expectToBeSignedIn(); }); + it("should sign in a user even when the same OAuth account has been used with a previous user that has been deleted since", async ({ expect }) => { + const response = await Auth.OAuth.signIn(); + expect(response.tokenResponse).toMatchInlineSnapshot(` + NiceResponse { + "status": 200, + "body": { + "access_token": , + "afterCallbackRedirectUrl": null, + "after_callback_redirect_url": null, + "expires_in": 3599, + "is_new_user": true, + "newUser": true, + "refresh_token": , + "scope": "legacy", + "token_type": "Bearer", + }, + "headers": Headers { + "pragma": "no-cache", +