Stop leaking the refresh token in the social SSO redirect URL (#23061)
The Google/Microsoft callback for a sign-in with no target workspace
redirected to `/sign-in-up?tokenPair={...}`, putting a 60-day refresh
token in a query string. Those persist in browser history, `Referer`
headers and access logs.
It now carries a single-use, 5-minute opaque token in the URL fragment,
which the frontend exchanges over POST. Browsers never send the fragment
on the wire, so the token stays out of access logs, proxies and
`Referer` headers entirely. Redemption claims the row with a `DELETE`
guarded on `revokedAt`/`deletedAt` being null, so concurrent requests
cannot each mint a refresh token and a revoked token cannot redeem.
Enterprise SSO (OIDC/SAML) already used a POST exchange and is
unchanged.
```mermaid
sequenceDiagram
participant Browser
participant Server
participant DB
Note over Browser,Server: before, the redirect carried access + 60-day refresh in ?tokenPair
Browser->>Server: GET /auth/google/redirect
Server->>DB: store sha256(token), expires in 5 min
Server-->>Browser: 302 /sign-in-up#ssoExchangeToken=opaque
Note over Browser: fragment never sent back to any server
Browser->>Server: POST getAuthTokensFromSSOExchangeToken
Server->>DB: guarded DELETE, single-use claim
Server-->>Browser: access + refresh token, in the response body
```
Since the token is single-use, the refresh token is minted at redemption
instead of at callback, so an abandoned redirect leaves an inert expired
hash rather than a live credential.
Redemption lives in its own `SignInUpSSOExchangeTokenEffect` +
`useRedeemSSOExchangeToken`, mirroring the existing
`VerifyLoginTokenEffect` + `useVerifyLogin` pair, so
`SignInUpGlobalScopeFormEffect` only loses the vulnerable branch. Like
`useVerifyLogin`, the hook clears any stale token pair before
exchanging. The effect reads `window.location.hash` live and strips it
synchronously, which doubles as the StrictMode double-invocation latch.
Remaining exposure is the browser itself (history until the synchronous
strip, client-side scripts), same as any fragment-based OAuth response.
`loginToken` on the workspace-targeted branch still travels as
`/verify?loginToken=` and is replayable for 15 minutes; moving it to the
fragment too is a separate change.
A fast instance command adds a unique partial index on `("type",
"value")` for live SSO exchange tokens, so redemption is an index lookup
instead of a full scan of the shared token table and at most one row can
ever match.
This commit is contained in:
+11
@@ -0,0 +1,11 @@
|
||||
import { gql } from '@apollo/client';
|
||||
|
||||
export const GET_AUTH_TOKENS_FROM_SSO_EXCHANGE_TOKEN = gql`
|
||||
mutation getAuthTokensFromSSOExchangeToken($ssoExchangeToken: String!) {
|
||||
getAuthTokensFromSSOExchangeToken(ssoExchangeToken: $ssoExchangeToken) {
|
||||
tokens {
|
||||
...AuthTokenPairFragment
|
||||
}
|
||||
}
|
||||
}
|
||||
`;
|
||||
+140
@@ -0,0 +1,140 @@
|
||||
import { renderHook } from '@testing-library/react';
|
||||
import { Provider as JotaiProvider } from 'jotai';
|
||||
|
||||
import { isAppEffectRedirectEnabledState } from '@/app/states/isAppEffectRedirectEnabledState';
|
||||
import { useRedeemSSOExchangeToken } from '@/auth/hooks/useRedeemSSOExchangeToken';
|
||||
import { tokenPairState } from '@/auth/states/tokenPairState';
|
||||
import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar';
|
||||
import {
|
||||
jotaiStore,
|
||||
resetJotaiStore,
|
||||
} from '@/ui/utilities/state/jotai/jotaiStore';
|
||||
|
||||
const mockGetAuthTokensFromSSOExchangeToken = jest.fn();
|
||||
|
||||
jest.mock('@apollo/client/react', () => ({
|
||||
...jest.requireActual('@apollo/client/react'),
|
||||
useMutation: () => [mockGetAuthTokensFromSSOExchangeToken],
|
||||
}));
|
||||
|
||||
jest.mock('@/ui/feedback/snack-bar-manager/hooks/useSnackBar', () => ({
|
||||
useSnackBar: jest.fn(),
|
||||
}));
|
||||
|
||||
const renderHooks = () => {
|
||||
const { result } = renderHook(() => useRedeemSSOExchangeToken(), {
|
||||
wrapper: ({ children }) => JotaiProvider({ store: jotaiStore, children }),
|
||||
});
|
||||
|
||||
return { result };
|
||||
};
|
||||
|
||||
const staleTokenPair = {
|
||||
accessOrWorkspaceAgnosticToken: {
|
||||
token: 'stale-access-token',
|
||||
expiresAt: '2020-01-01T00:00:00.000Z',
|
||||
},
|
||||
refreshToken: {
|
||||
token: 'stale-refresh-token',
|
||||
expiresAt: '2020-01-01T00:00:00.000Z',
|
||||
},
|
||||
};
|
||||
|
||||
const freshTokenPair = {
|
||||
accessOrWorkspaceAgnosticToken: {
|
||||
token: 'fresh-access-token',
|
||||
expiresAt: '2100-01-01T00:00:00.000Z',
|
||||
},
|
||||
refreshToken: {
|
||||
token: 'fresh-refresh-token',
|
||||
expiresAt: '2100-01-01T00:00:00.000Z',
|
||||
},
|
||||
};
|
||||
|
||||
describe('useRedeemSSOExchangeToken', () => {
|
||||
const mockEnqueueErrorSnackBar = jest.fn();
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
localStorage.clear();
|
||||
resetJotaiStore();
|
||||
|
||||
(useSnackBar as jest.Mock).mockReturnValue({
|
||||
enqueueErrorSnackBar: mockEnqueueErrorSnackBar,
|
||||
});
|
||||
|
||||
mockGetAuthTokensFromSSOExchangeToken.mockResolvedValue({
|
||||
data: {
|
||||
getAuthTokensFromSSOExchangeToken: { tokens: freshTokenPair },
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('should store the redeemed token pair', async () => {
|
||||
const { result } = renderHooks();
|
||||
|
||||
await result.current.redeemSSOExchangeToken('sso-exchange-token');
|
||||
|
||||
expect(mockGetAuthTokensFromSSOExchangeToken).toHaveBeenCalledWith({
|
||||
variables: { ssoExchangeToken: 'sso-exchange-token' },
|
||||
});
|
||||
expect(jotaiStore.get(tokenPairState.atom)).toEqual(freshTokenPair);
|
||||
});
|
||||
|
||||
it('should clear the existing token pair before exchanging', async () => {
|
||||
jotaiStore.set(tokenPairState.atom, staleTokenPair);
|
||||
|
||||
const tokenPairsAtExchangeTime: unknown[] = [];
|
||||
|
||||
mockGetAuthTokensFromSSOExchangeToken.mockImplementation(() => {
|
||||
tokenPairsAtExchangeTime.push(jotaiStore.get(tokenPairState.atom));
|
||||
|
||||
return Promise.resolve({
|
||||
data: { getAuthTokensFromSSOExchangeToken: { tokens: freshTokenPair } },
|
||||
});
|
||||
});
|
||||
|
||||
const { result } = renderHooks();
|
||||
|
||||
await result.current.redeemSSOExchangeToken('sso-exchange-token');
|
||||
|
||||
expect(tokenPairsAtExchangeTime).toEqual([null]);
|
||||
});
|
||||
|
||||
it('should disable the redirect effect while exchanging and restore it after', async () => {
|
||||
const redirectFlagsAtExchangeTime: unknown[] = [];
|
||||
|
||||
mockGetAuthTokensFromSSOExchangeToken.mockImplementation(() => {
|
||||
redirectFlagsAtExchangeTime.push(
|
||||
jotaiStore.get(isAppEffectRedirectEnabledState.atom),
|
||||
);
|
||||
|
||||
return Promise.resolve({
|
||||
data: { getAuthTokensFromSSOExchangeToken: { tokens: freshTokenPair } },
|
||||
});
|
||||
});
|
||||
|
||||
const { result } = renderHooks();
|
||||
|
||||
await result.current.redeemSSOExchangeToken('sso-exchange-token');
|
||||
|
||||
expect(redirectFlagsAtExchangeTime).toEqual([false]);
|
||||
expect(jotaiStore.get(isAppEffectRedirectEnabledState.atom)).toBe(true);
|
||||
});
|
||||
|
||||
it('should snackbar and leave no token pair when redemption fails', async () => {
|
||||
mockGetAuthTokensFromSSOExchangeToken.mockRejectedValueOnce(
|
||||
new Error('Invalid SSO exchange token'),
|
||||
);
|
||||
|
||||
const { result } = renderHooks();
|
||||
|
||||
await result.current.redeemSSOExchangeToken('sso-exchange-token');
|
||||
|
||||
expect(mockEnqueueErrorSnackBar).toHaveBeenCalledWith({
|
||||
message: 'Invalid SSO exchange token',
|
||||
});
|
||||
expect(jotaiStore.get(tokenPairState.atom)).toBeNull();
|
||||
expect(jotaiStore.get(isAppEffectRedirectEnabledState.atom)).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -638,7 +638,6 @@ export const useAuth = () => {
|
||||
signInWithCredentials: handleCredentialsSignIn,
|
||||
signInWithGoogle: handleGoogleLogin,
|
||||
signInWithMicrosoft: handleMicrosoftLogin,
|
||||
setAuthTokens: handleSetAuthTokens,
|
||||
getAuthTokensFromOTP: handleGetAuthTokensFromOTP,
|
||||
navigateAfterMultiWorkspaceSignInUp,
|
||||
};
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
import { isAppEffectRedirectEnabledState } from '@/app/states/isAppEffectRedirectEnabledState';
|
||||
import { tokenPairState } from '@/auth/states/tokenPairState';
|
||||
import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar';
|
||||
import { useSetAtomState } from '@/ui/utilities/state/jotai/hooks/useSetAtomState';
|
||||
import { CombinedGraphQLErrors } from '@apollo/client/errors';
|
||||
import { useMutation } from '@apollo/client/react';
|
||||
import { useCallback } from 'react';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { GetAuthTokensFromSsoExchangeTokenDocument } from '~/generated-metadata/graphql';
|
||||
|
||||
export const useRedeemSSOExchangeToken = () => {
|
||||
const { enqueueErrorSnackBar } = useSnackBar();
|
||||
const setTokenPair = useSetAtomState(tokenPairState);
|
||||
const setIsAppEffectRedirectEnabled = useSetAtomState(
|
||||
isAppEffectRedirectEnabledState,
|
||||
);
|
||||
const [getAuthTokensFromSSOExchangeToken] = useMutation(
|
||||
GetAuthTokensFromSsoExchangeTokenDocument,
|
||||
);
|
||||
|
||||
const redeemSSOExchangeToken = useCallback(
|
||||
async (ssoExchangeToken: string) => {
|
||||
// Keeps PageChangeEffect from consuming returnToPath mid token swap, and
|
||||
// drops any stale pair so the resume waits for the one being redeemed
|
||||
setIsAppEffectRedirectEnabled(false);
|
||||
setTokenPair(null);
|
||||
|
||||
try {
|
||||
const { data } = await getAuthTokensFromSSOExchangeToken({
|
||||
variables: { ssoExchangeToken },
|
||||
});
|
||||
|
||||
if (!isDefined(data?.getAuthTokensFromSSOExchangeToken)) {
|
||||
throw new Error('No getAuthTokensFromSSOExchangeToken result');
|
||||
}
|
||||
|
||||
setTokenPair(data.getAuthTokensFromSSOExchangeToken.tokens);
|
||||
} catch (error: unknown) {
|
||||
enqueueErrorSnackBar(
|
||||
CombinedGraphQLErrors.is(error)
|
||||
? { apolloError: error }
|
||||
: { message: error instanceof Error ? error.message : undefined },
|
||||
);
|
||||
} finally {
|
||||
setIsAppEffectRedirectEnabled(true);
|
||||
}
|
||||
},
|
||||
[
|
||||
getAuthTokensFromSSOExchangeToken,
|
||||
setTokenPair,
|
||||
setIsAppEffectRedirectEnabled,
|
||||
enqueueErrorSnackBar,
|
||||
],
|
||||
);
|
||||
|
||||
return { redeemSSOExchangeToken };
|
||||
};
|
||||
+1
-16
@@ -7,13 +7,10 @@ import {
|
||||
import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue';
|
||||
import { useLoadCurrentUser } from '@/users/hooks/useLoadCurrentUser';
|
||||
import { useEffect } from 'react';
|
||||
import { useSearchParams } from 'react-router-dom';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
export const SignInUpGlobalScopeFormEffect = () => {
|
||||
const signInUpStep = useAtomStateValue(signInUpStepState);
|
||||
const [searchParams, setSearchParams] = useSearchParams();
|
||||
const { setAuthTokens, navigateAfterMultiWorkspaceSignInUp } = useAuth();
|
||||
const { navigateAfterMultiWorkspaceSignInUp } = useAuth();
|
||||
const { loadCurrentUser } = useLoadCurrentUser();
|
||||
const hasAccessTokenPair = useHasAccessTokenPair();
|
||||
|
||||
@@ -26,24 +23,12 @@ export const SignInUpGlobalScopeFormEffect = () => {
|
||||
);
|
||||
};
|
||||
|
||||
const tokenPairFromUrl = searchParams.get('tokenPair');
|
||||
if (isDefined(tokenPairFromUrl)) {
|
||||
setAuthTokens(JSON.parse(tokenPairFromUrl));
|
||||
searchParams.delete('tokenPair');
|
||||
setSearchParams(searchParams);
|
||||
void resumeOnCentralDomain();
|
||||
return;
|
||||
}
|
||||
|
||||
if (signInUpStep !== SignInUpStep.Init) return;
|
||||
if (!hasAccessTokenPair) return;
|
||||
|
||||
void resumeOnCentralDomain();
|
||||
}, [
|
||||
searchParams,
|
||||
setSearchParams,
|
||||
loadCurrentUser,
|
||||
setAuthTokens,
|
||||
signInUpStep,
|
||||
hasAccessTokenPair,
|
||||
navigateAfterMultiWorkspaceSignInUp,
|
||||
|
||||
+30
@@ -0,0 +1,30 @@
|
||||
import { useRedeemSSOExchangeToken } from '@/auth/hooks/useRedeemSSOExchangeToken';
|
||||
import { useEffect } from 'react';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
export const SignInUpSSOExchangeTokenEffect = () => {
|
||||
const { redeemSSOExchangeToken } = useRedeemSSOExchangeToken();
|
||||
|
||||
useEffect(() => {
|
||||
const ssoExchangeToken = new URLSearchParams(
|
||||
window.location.hash.substring(1),
|
||||
).get('ssoExchangeToken');
|
||||
|
||||
if (!isDefined(ssoExchangeToken)) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Stripping synchronously through window.history rather than the router
|
||||
// (whose data-router navigations defer the replace) latches re-invoked and
|
||||
// remounted effects out: they re-read window.location and find no token
|
||||
window.history.replaceState(
|
||||
window.history.state,
|
||||
'',
|
||||
window.location.pathname + window.location.search,
|
||||
);
|
||||
|
||||
void redeemSSOExchangeToken(ssoExchangeToken);
|
||||
}, [redeemSSOExchangeToken]);
|
||||
|
||||
return <></>;
|
||||
};
|
||||
+73
@@ -0,0 +1,73 @@
|
||||
import { render, screen, waitFor } from '@testing-library/react';
|
||||
import { StrictMode } from 'react';
|
||||
import { BrowserRouter, useSearchParams } from 'react-router-dom';
|
||||
|
||||
import { SignInUpSSOExchangeTokenEffect } from '@/auth/sign-in-up/components/internal/SignInUpSSOExchangeTokenEffect';
|
||||
|
||||
const redeemSSOExchangeTokenMock = jest.fn();
|
||||
|
||||
jest.mock('@/auth/hooks/useRedeemSSOExchangeToken', () => ({
|
||||
useRedeemSSOExchangeToken: () => ({
|
||||
redeemSSOExchangeToken: redeemSSOExchangeTokenMock,
|
||||
}),
|
||||
}));
|
||||
|
||||
const SearchParamsProbe = () => {
|
||||
const [searchParams] = useSearchParams();
|
||||
|
||||
return <div data-testid="search-params">{searchParams.toString()}</div>;
|
||||
};
|
||||
|
||||
// BrowserRouter because the effect reads and strips window.location, which
|
||||
// MemoryRouter never touches
|
||||
const renderEffect = (initialUrl: string) => {
|
||||
window.history.replaceState(null, '', initialUrl);
|
||||
|
||||
return render(
|
||||
<StrictMode>
|
||||
<BrowserRouter>
|
||||
<SignInUpSSOExchangeTokenEffect />
|
||||
<SearchParamsProbe />
|
||||
</BrowserRouter>
|
||||
</StrictMode>,
|
||||
);
|
||||
};
|
||||
|
||||
const getSearchParams = () => screen.getByTestId('search-params').textContent;
|
||||
|
||||
describe('SignInUpSSOExchangeTokenEffect', () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
window.history.replaceState(null, '', '/');
|
||||
});
|
||||
|
||||
it('redeems the single use token at most once', async () => {
|
||||
renderEffect('/sign-in-up#ssoExchangeToken=sso-exchange-token');
|
||||
|
||||
await waitFor(() => {
|
||||
expect(redeemSSOExchangeTokenMock).toHaveBeenCalledWith(
|
||||
'sso-exchange-token',
|
||||
);
|
||||
});
|
||||
expect(redeemSSOExchangeTokenMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('strips the token from the url while keeping returnToPath', async () => {
|
||||
renderEffect(
|
||||
'/sign-in-up?returnToPath=%2Fsettings%2Fprofile#ssoExchangeToken=sso-exchange-token',
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(window.location.hash).toBe('');
|
||||
});
|
||||
expect(getSearchParams()).toBe('returnToPath=%2Fsettings%2Fprofile');
|
||||
expect(redeemSSOExchangeTokenMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('does nothing when the url carries no token', () => {
|
||||
renderEffect('/sign-in-up');
|
||||
|
||||
expect(redeemSSOExchangeTokenMock).not.toHaveBeenCalled();
|
||||
expect(getSearchParams()).toBe('');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user