fix: handle unsupported providers in account reconnect flow
https://sonarly.com/issue/28291?type=bug Clicking "Reconnect" on a connected account with a non-standard provider value (not Google, Microsoft, or IMAP) throws an unhandled error because `getProviderUrl()` only supports two OAuth providers. Fix: packages/twenty-front/src/modules/settings/accounts/hooks/useTriggerProviderReconnect.ts 1. Added OAUTH_RECONNECTABLE_PROVIDERS constant — an explicit allowlist of providers that support OAuth reconnection (GOOGLE, MICROSOFT). 2. Exported isReconnectableProvider() helper — returns true for IMAP_SMTP_CALDAV or any OAuth-reconnectable provider. Used by UI components to decide whether to show the Reconnect button. 3. Added early-return guard before triggerApisOAuth call — if the provider is not in OAUTH_RECONNECTABLE_PROVIDERS, the function returns silently instead of falling through to getProviderUrl() which would throw. This is the gateway function for all reconnection flows. Previously, any provider not matching IMAP_SMTP_CALDAV fell through to triggerApisOAuth → getProviderUrl(), which only handles GOOGLE and MICROSOFT and throws for everything else. The allowlist guard prevents the throw for OIDC, SAML, and arbitrary varchar values like "EmailEngine". packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsRowDropdownMenu.tsx 1. Imported isReconnectableProvider from useTriggerProviderReconnect. 2. Added isReconnectableProvider(account.provider) check to the Reconnect MenuItem's render condition, alongside the existing account.authFailedAt check. Defense-in-depth: even though the hook now silently handles unsupported providers, the UI should not show a non-functional "Reconnect" button. Hiding it improves UX and prevents user confusion for accounts with providers like OIDC, SAML, or non-standard values. packages/twenty-front/src/modules/settings/accounts/hooks/__tests__/useTriggerProviderReconnect.test.tsx Added three new test cases under a "unsupported providers" describe block: 1. OIDC provider — verifies neither OAuth nor navigation is triggered. 2. SAML provider — same verification. 3. Unknown string value ('EmailEngine' cast as ConnectedAccountProvider) — same verification, directly reproducing the Sentry bug scenario. Also added an "error handling" describe block verifying that OAuth errors from supported providers still propagate correctly. Regression protection: these tests ensure the allowlist guard works for all known non-OAuth providers and for arbitrary varchar values that may exist in the database. The 'EmailEngine' cast test directly reproduces the production bug scenario.
This commit is contained in:
+15
-11
@@ -7,7 +7,10 @@ import {
|
||||
SettingsPath,
|
||||
} from 'twenty-shared/types';
|
||||
|
||||
import { useTriggerProviderReconnect } from '@/settings/accounts/hooks/useTriggerProviderReconnect';
|
||||
import {
|
||||
isReconnectableProvider,
|
||||
useTriggerProviderReconnect,
|
||||
} from '@/settings/accounts/hooks/useTriggerProviderReconnect';
|
||||
import { Dropdown } from '@/ui/layout/dropdown/components/Dropdown';
|
||||
import { DropdownContent } from '@/ui/layout/dropdown/components/DropdownContent';
|
||||
import { DropdownMenuItemsContainer } from '@/ui/layout/dropdown/components/DropdownMenuItemsContainer';
|
||||
@@ -121,16 +124,17 @@ export const SettingsAccountsRowDropdownMenu = ({
|
||||
closeDropdown(dropdownId);
|
||||
}}
|
||||
/>
|
||||
{account.authFailedAt && (
|
||||
<MenuItem
|
||||
LeftIcon={IconRefresh}
|
||||
text={t`Reconnect`}
|
||||
onClick={() => {
|
||||
triggerProviderReconnect(account.provider, account.id);
|
||||
closeDropdown(dropdownId);
|
||||
}}
|
||||
/>
|
||||
)}
|
||||
{account.authFailedAt &&
|
||||
isReconnectableProvider(account.provider) && (
|
||||
<MenuItem
|
||||
LeftIcon={IconRefresh}
|
||||
text={t`Reconnect`}
|
||||
onClick={() => {
|
||||
triggerProviderReconnect(account.provider, account.id);
|
||||
closeDropdown(dropdownId);
|
||||
}}
|
||||
/>
|
||||
)}
|
||||
<MenuItem
|
||||
accent="danger"
|
||||
LeftIcon={IconTrash}
|
||||
|
||||
+41
@@ -227,6 +227,47 @@ describe('useTriggerProviderReconnect', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('unsupported providers', () => {
|
||||
it('should not trigger OAuth for OIDC provider', async () => {
|
||||
const { result } = renderHook(() => useTriggerProviderReconnect());
|
||||
|
||||
await act(async () => {
|
||||
await result.current.triggerProviderReconnect(
|
||||
ConnectedAccountProvider.OIDC,
|
||||
);
|
||||
});
|
||||
|
||||
expect(mockTriggerApisOAuth).not.toHaveBeenCalled();
|
||||
expect(mockNavigate).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not trigger OAuth for SAML provider', async () => {
|
||||
const { result } = renderHook(() => useTriggerProviderReconnect());
|
||||
|
||||
await act(async () => {
|
||||
await result.current.triggerProviderReconnect(
|
||||
ConnectedAccountProvider.SAML,
|
||||
);
|
||||
});
|
||||
|
||||
expect(mockTriggerApisOAuth).not.toHaveBeenCalled();
|
||||
expect(mockNavigate).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not trigger OAuth for unknown provider values', async () => {
|
||||
const { result } = renderHook(() => useTriggerProviderReconnect());
|
||||
|
||||
await act(async () => {
|
||||
await result.current.triggerProviderReconnect(
|
||||
'EmailEngine' as ConnectedAccountProvider,
|
||||
);
|
||||
});
|
||||
|
||||
expect(mockTriggerApisOAuth).not.toHaveBeenCalled();
|
||||
expect(mockNavigate).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('error handling', () => {
|
||||
it('should handle triggerApisOAuth errors gracefully', async () => {
|
||||
const { result } = renderHook(() => useTriggerProviderReconnect());
|
||||
|
||||
+15
@@ -5,6 +5,17 @@ import { getSettingsPath } from 'twenty-shared/utils';
|
||||
import { useTriggerApisOAuth } from '@/settings/accounts/hooks/useTriggerApiOAuth';
|
||||
import { useNavigateSettings } from '~/hooks/useNavigateSettings';
|
||||
|
||||
const OAUTH_RECONNECTABLE_PROVIDERS: ConnectedAccountProvider[] = [
|
||||
ConnectedAccountProvider.GOOGLE,
|
||||
ConnectedAccountProvider.MICROSOFT,
|
||||
];
|
||||
|
||||
export const isReconnectableProvider = (
|
||||
provider: ConnectedAccountProvider,
|
||||
): boolean =>
|
||||
provider === ConnectedAccountProvider.IMAP_SMTP_CALDAV ||
|
||||
OAUTH_RECONNECTABLE_PROVIDERS.includes(provider);
|
||||
|
||||
export const useTriggerProviderReconnect = () => {
|
||||
const { triggerApisOAuth } = useTriggerApisOAuth();
|
||||
const navigate = useNavigateSettings();
|
||||
@@ -27,6 +38,10 @@ export const useTriggerProviderReconnect = () => {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!OAUTH_RECONNECTABLE_PROVIDERS.includes(provider)) {
|
||||
return;
|
||||
}
|
||||
|
||||
await triggerApisOAuth(provider, {
|
||||
...options,
|
||||
redirectLocation: getSettingsPath(SettingsPath.Accounts),
|
||||
|
||||
Reference in New Issue
Block a user