fix(auth): preserve returnToPath across Google/Microsoft SSO redirects (#20537)
## 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 `<workspace>/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.
This commit is contained in:
@@ -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');
|
||||
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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<string> {
|
||||
@@ -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({
|
||||
|
||||
@@ -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);
|
||||
|
||||
+3
@@ -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);
|
||||
|
||||
@@ -9,4 +9,5 @@ export type SocialSSOState = {
|
||||
workspacePersonalInviteToken?: string;
|
||||
action?: SocialSSOSignInUpActionType;
|
||||
locale?: keyof typeof APP_LOCALES;
|
||||
returnToPath?: string;
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user