From 49b966042080739de19ed94c36d0daa124ee78c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Wed, 13 May 2026 21:54:54 +0200 Subject: [PATCH] fix(auth): preserve returnToPath across Google/Microsoft SSO redirects (#20537) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes the consent-modal-not-reopening half of [#20535](https://github.com/twentyhq/twenty/issues/20535): when a signed-out user opens an OAuth `/authorize?...` URL (e.g. ChatGPT connecting to `api.twenty.com/mcp`) and signs in with **Google or Microsoft**, the original `/authorize` request was lost and the consent screen never reopened. ### Root cause `PageChangeEffect` already saves the deep link as `returnToPath` (Jotai atom) before navigating to `/welcome`. That atom is in-memory: it survives SPA navigation, and the cross-subdomain workspace hop is handled by `useBuildSearchParamsFromUrlSyncedStates` round-tripping the value through the URL. But the social-SSO path leaves `app.twenty.com` entirely — `app.twenty.com/welcome` → `api.twenty.com/auth/google` → Google → `api.twenty.com/auth/google/redirect` → frontend — so the atom is wiped. None of the existing code paths plumbed `returnToPath` through that hop: - `useAuth.buildRedirectUrl` packed `workspaceInviteHash`/`action`/etc. but not `returnToPath`. - `SocialSSOState` / the Google + Microsoft strategies didn't carry it through the OAuth `state` blob. - `signInUpWithSocialSSO` + `computeRedirectURI` didn't re-emit it on the redirect back to the frontend. The email path worked because all transitions stay on the default frontend domain, so the atom survives until `SignInUpGlobalScopeForm` bakes it into the workspace URL. ### What changed Plumb `returnToPath` through the SSO state the same way `workspaceInviteHash` and `action` already flow: - **Frontend** (`useAuth.buildRedirectUrl`): read `returnToPath` from the Jotai store and append it to `/auth/google` / `/auth/microsoft` when set and structurally valid. - **Server types** (`SocialSSOState`, `GoogleRequest['user']`, `MicrosoftRequest['user']`): add optional `returnToPath`. - **Strategies** (`google.auth.strategy.ts`, `microsoft.auth.strategy.ts`): include `returnToPath: req.query.returnToPath` in the JSON `state` and read it back in `validate`. - **auth.service.ts** (`signInUpWithSocialSSO`, `computeRedirectURI`): forward `returnToPath` on both branches — the multi-workspace redirect to `AppPath.SignInUp?tokenPair=...` and the single-workspace redirect to `/verify?loginToken=...`. Validated via a new `isValidReturnToPath` helper so a tampered query value can't become an open-redirect vector. After the round-trip, `useInitializeQueryParamState` rehydrates the atom from the URL and `usePageChangeEffectNavigateLocation` resolves it as the post-auth destination — same mechanism the email path already relied on. Out of scope: the OAuth `resource` parameter handling tracked in [#20296](https://github.com/twentyhq/twenty/issues/20296) is independent and not addressed here. ## Test plan - [x] `npx jest src/engine/core-modules/auth` (twenty-server) — 27 suites / 183 tests pass, including new `is-valid-return-to-path.util.spec.ts`. - [x] `npx jest src/modules/auth` (twenty-front) — 13 suites / 52 tests pass, including two new cases in `useAuth.test.tsx` covering the happy path and the protocol-relative open-redirect guard. - [x] `npx nx typecheck twenty-server` / `twenty-front` — clean. - [x] `npx oxlint` + `prettier --check` on touched files — clean. - [ ] Manual: signed-out user opens `https://app.twenty.com/authorize?client_id=...` → Continue with Google → completes Google → selects workspace → consent screen renders. - [ ] Manual: same flow, single workspace — lands on consent screen directly after Verify. - [ ] Manual: email path still works (regression). - [ ] Manual: tamper `returnToPath=//evil.com` on the `/auth/google` URL → server validation rejects, user lands at default home, not at `evil.com`. E2E note: existing `return-to-path.spec.ts` already covers deep links with query params through the email path. A mock OAuth provider would be needed to cover the SSO path end-to-end; unit coverage stands in for now. --- .../auth/hooks/__tests__/useAuth.test.tsx | 44 +++++++++++++++++++ .../src/modules/auth/hooks/useAuth.ts | 11 ++++- .../auth/services/auth.service.ts | 11 +++++ .../auth/strategies/google.auth.strategy.ts | 3 ++ .../strategies/microsoft.auth.strategy.ts | 3 ++ .../auth/types/social-sso-state.type.ts | 1 + 6 files changed, 72 insertions(+), 1 deletion(-) diff --git a/packages/twenty-front/src/modules/auth/hooks/__tests__/useAuth.test.tsx b/packages/twenty-front/src/modules/auth/hooks/__tests__/useAuth.test.tsx index 6477c08375..d7862eb45c 100644 --- a/packages/twenty-front/src/modules/auth/hooks/__tests__/useAuth.test.tsx +++ b/packages/twenty-front/src/modules/auth/hooks/__tests__/useAuth.test.tsx @@ -11,8 +11,10 @@ import { results, token, } from '@/auth/hooks/__mocks__/useAuth'; +import { returnToPathState } from '@/auth/states/returnToPathState'; import { SnackBarComponentInstanceContext } from '@/ui/feedback/snack-bar-manager/contexts/SnackBarComponentInstanceContext'; import { renderHook } from '@testing-library/react'; +import { getDefaultStore } from 'jotai'; const redirectSpy = jest.fn(); @@ -85,6 +87,7 @@ const renderHooks = () => { describe('useAuth', () => { beforeEach(() => { jest.clearAllMocks(); + getDefaultStore().set(returnToPathState.atom, ''); }); it('should return login token object', async () => { @@ -138,6 +141,47 @@ describe('useAuth', () => { ); }); + it('should forward returnToPath to /auth/google when set in state', async () => { + getDefaultStore().set( + returnToPathState.atom, + '/authorize?response_type=code&client_id=abc&state=xyz', + ); + + const { result } = renderHooks(); + + await act(async () => { + await result.current.signInWithGoogle({ + action: 'list-available-workspaces', + }); + }); + + const calledWithUrl = redirectSpy.mock.calls[0]?.[0] as string; + const parsed = new URL(calledWithUrl); + + expect(parsed.pathname).toBe('/auth/google'); + expect(parsed.searchParams.get('action')).toBe('list-available-workspaces'); + expect(parsed.searchParams.get('returnToPath')).toBe( + '/authorize?response_type=code&client_id=abc&state=xyz', + ); + }); + + it('should not forward an invalid (protocol-relative) returnToPath', async () => { + getDefaultStore().set(returnToPathState.atom, '//evil.example.com'); + + const { result } = renderHooks(); + + await act(async () => { + await result.current.signInWithGoogle({ + action: 'list-available-workspaces', + }); + }); + + const calledWithUrl = redirectSpy.mock.calls[0]?.[0] as string; + const parsed = new URL(calledWithUrl); + + expect(parsed.searchParams.has('returnToPath')).toBe(false); + }); + it('should handle sign-out', async () => { sessionStorage.setItem('lingering-key', 'should-be-cleared'); diff --git a/packages/twenty-front/src/modules/auth/hooks/useAuth.ts b/packages/twenty-front/src/modules/auth/hooks/useAuth.ts index a152a60be4..b23c2360fa 100644 --- a/packages/twenty-front/src/modules/auth/hooks/useAuth.ts +++ b/packages/twenty-front/src/modules/auth/hooks/useAuth.ts @@ -17,9 +17,12 @@ import { VerifyEmailAndGetWorkspaceAgnosticTokenDocument, } from '~/generated-metadata/graphql'; +import { returnToPathState } from '@/auth/states/returnToPathState'; import { tokenPairState } from '@/auth/states/tokenPairState'; import { clearSessionLocalStorageKeys } from '@/auth/utils/clearSessionLocalStorageKeys'; import { broadcastSignOutToOtherTabs } from '@/auth/utils/crossTabSignOut'; +import { isValidReturnToPath } from '@/auth/utils/isValidReturnToPath'; +import { isNonEmptyString } from '@sniptt/guards'; import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue'; import { useSetAtomState } from '@/ui/utilities/state/jotai/hooks/useSetAtomState'; @@ -524,9 +527,15 @@ export const useAuth = () => { url.searchParams.set('workspaceId', workspacePublicData.id); } + const returnToPath = store.get(returnToPathState.atom); + + if (isNonEmptyString(returnToPath) && isValidReturnToPath(returnToPath)) { + url.searchParams.set('returnToPath', returnToPath); + } + return url.toString(); }, - [workspacePublicData], + [workspacePublicData, store], ); const handleGoogleLogin = useCallback( diff --git a/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts b/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts index 14f4a10d0a..220a1cd8e6 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts @@ -10,6 +10,7 @@ import ms from 'ms'; import { PasswordUpdateNotifyEmail } from 'twenty-emails'; import { PermissionFlagType } from 'twenty-shared/constants'; import { AppPath, ConnectedAccountProvider } from 'twenty-shared/types'; +import { isNonEmptyString } from '@sniptt/guards'; import { assertIsDefinedOrThrow, isDefined } from 'twenty-shared/utils'; import { IsNull, Repository } from 'typeorm'; @@ -761,10 +762,12 @@ export class AuthService { loginToken, workspace, billingCheckoutSessionState, + returnToPath, }: { loginToken: string; workspace: WorkspaceDomainConfig; billingCheckoutSessionState?: string; + returnToPath?: string; }) { const url = this.workspaceDomainsService.buildWorkspaceURL({ workspace, @@ -772,6 +775,9 @@ export class AuthService { searchParams: { loginToken, ...(billingCheckoutSessionState ? { billingCheckoutSessionState } : {}), + ...(isNonEmptyString(returnToPath) && returnToPath.startsWith('/') + ? { returnToPath } + : {}), }, }); @@ -944,6 +950,7 @@ export class AuthService { billingCheckoutSessionState, action, locale, + returnToPath, }: MicrosoftRequest['user'] | GoogleRequest['user'], authProvider: AuthProviderEnum.Google | AuthProviderEnum.Microsoft, ): Promise { @@ -995,6 +1002,9 @@ export class AuthService { targetedTokenType: JwtTokenTypeEnum.WORKSPACE_AGNOSTIC, }), }), + ...(isNonEmptyString(returnToPath) && returnToPath.startsWith('/') + ? { returnToPath } + : {}), }, }); @@ -1066,6 +1076,7 @@ export class AuthService { loginToken: loginToken.token, workspace, billingCheckoutSessionState, + returnToPath, }); } catch (error) { return this.guardRedirectService.getRedirectErrorUrlAndCaptureExceptions({ diff --git a/packages/twenty-server/src/engine/core-modules/auth/strategies/google.auth.strategy.ts b/packages/twenty-server/src/engine/core-modules/auth/strategies/google.auth.strategy.ts index bcca948c32..890df81377 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/strategies/google.auth.strategy.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/strategies/google.auth.strategy.ts @@ -33,6 +33,7 @@ export type GoogleRequest = Omit< action: SocialSSOSignInUpActionType; workspaceId?: string; billingCheckoutSessionState?: string; + returnToPath?: string; }; }; @@ -59,6 +60,7 @@ export class GoogleStrategy extends PassportStrategy(Strategy, 'google') { workspacePersonalInviteToken: req.query.workspacePersonalInviteToken, action: req.query.action, locale: req.query.locale, + returnToPath: req.query.returnToPath, }), }; @@ -97,6 +99,7 @@ export class GoogleStrategy extends PassportStrategy(Strategy, 'google') { billingCheckoutSessionState: state?.billingCheckoutSessionState, action: state?.action ?? 'list-available-workspaces', locale: state?.locale, + returnToPath: state?.returnToPath, }; done(null, user); diff --git a/packages/twenty-server/src/engine/core-modules/auth/strategies/microsoft.auth.strategy.ts b/packages/twenty-server/src/engine/core-modules/auth/strategies/microsoft.auth.strategy.ts index 884a09429b..a32f5fb5cc 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/strategies/microsoft.auth.strategy.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/strategies/microsoft.auth.strategy.ts @@ -31,6 +31,7 @@ export type MicrosoftRequest = Omit< workspaceId?: string; billingCheckoutSessionState?: string; action: SocialSSOSignInUpActionType; + returnToPath?: string; }; }; @@ -58,6 +59,7 @@ export class MicrosoftStrategy extends PassportStrategy(Strategy, 'microsoft') { billingCheckoutSessionState: req.query.billingCheckoutSessionState, workspacePersonalInviteToken: req.query.workspacePersonalInviteToken, action: req.query.action, + returnToPath: req.query.returnToPath, oauthRetryCount: req.query.oauthRetryCount ? Number(req.query.oauthRetryCount) : undefined, @@ -95,6 +97,7 @@ export class MicrosoftStrategy extends PassportStrategy(Strategy, 'microsoft') { billingCheckoutSessionState: state?.billingCheckoutSessionState, locale: state?.locale, action: state?.action ?? 'list-available-workspaces', + returnToPath: state?.returnToPath, }; done(null, user); diff --git a/packages/twenty-server/src/engine/core-modules/auth/types/social-sso-state.type.ts b/packages/twenty-server/src/engine/core-modules/auth/types/social-sso-state.type.ts index efc375055e..0ba9bef42f 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/types/social-sso-state.type.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/types/social-sso-state.type.ts @@ -9,4 +9,5 @@ export type SocialSSOState = { workspacePersonalInviteToken?: string; action?: SocialSSOSignInUpActionType; locale?: keyof typeof APP_LOCALES; + returnToPath?: string; };