diff --git a/src/renderer/context/App.test.tsx b/src/renderer/context/App.test.tsx index a13bc522d..258c6ac70 100644 --- a/src/renderer/context/App.test.tsx +++ b/src/renderer/context/App.test.tsx @@ -13,9 +13,9 @@ import { useNotifications } from '../hooks/useNotifications'; import type { AuthState, ClientID, ClientSecret, SettingsState, Token } from '../types'; import type { DeviceFlowSession } from '../utils/auth/types'; -import * as authFlows from '../utils/auth/flows'; import * as authUtils from '../utils/auth/utils'; import * as storage from '../utils/core/storage'; +import { getAdapter } from '../utils/forges/registry'; import * as notifications from '../utils/notifications/notifications'; import * as comms from '../utils/system/comms'; import * as tray from '../utils/system/tray'; @@ -212,39 +212,41 @@ describe('renderer/context/App.tsx', () => { } as unknown as AuthState); const removeAccountSpy = vi.spyOn(authUtils, 'removeAccount'); - it('loginWithDeviceFlowStart calls startGitHubDeviceFlow', async () => { - const startGitHubDeviceFlowSpy = vi - .spyOn(authFlows, 'startGitHubDeviceFlow') - .mockImplementation(vi.fn()); + it('loginWithDeviceFlowStart delegates to the forge adapter', async () => { + const adapter = getAdapter('github'); + const startSpy = vi.spyOn(adapter, 'startDeviceFlow').mockImplementation(vi.fn()); const getContext = renderWithContext(); await act(async () => { - getContext().loginWithDeviceFlowStart(); + getContext().loginWithDeviceFlowStart('github'); }); - expect(startGitHubDeviceFlowSpy).toHaveBeenCalled(); + expect(startSpy).toHaveBeenCalled(); }); - it('loginWithDeviceFlowPoll calls pollGitHubDeviceFlow', async () => { - const pollGitHubDeviceFlowSpy = vi - .spyOn(authFlows, 'pollGitHubDeviceFlow') - .mockImplementation(vi.fn()); + it('loginWithDeviceFlowPoll delegates to the forge adapter', async () => { + const adapter = getAdapter('github'); + const pollSpy = vi.spyOn(adapter, 'pollDeviceFlow').mockImplementation(vi.fn()); const getContext = renderWithContext(); await act(async () => { - getContext().loginWithDeviceFlowPoll('session' as unknown as DeviceFlowSession); + getContext().loginWithDeviceFlowPoll('github', 'session' as unknown as DeviceFlowSession); }); - expect(pollGitHubDeviceFlowSpy).toHaveBeenCalledWith('session'); + expect(pollSpy).toHaveBeenCalledWith('session'); }); it('loginWithDeviceFlowComplete calls addAccount', async () => { const getContext = renderWithContext(); await act(async () => { - await getContext().loginWithDeviceFlowComplete('token' as Token, Constants.GITHUB_HOSTNAME); + await getContext().loginWithDeviceFlowComplete( + 'github', + 'token' as Token, + Constants.GITHUB_HOSTNAME, + ); }); expect(addAccountSpy).toHaveBeenCalledWith( @@ -256,20 +258,61 @@ describe('renderer/context/App.tsx', () => { ); }); - it('loginWithOAuthApp calls performGitHubWebOAuth', async () => { - const performGitHubWebOAuthSpy = vi.spyOn(authFlows, 'performGitHubWebOAuth'); + it('loginWithOAuthApp delegates to the forge adapter', async () => { + const adapter = getAdapter('github'); + const oauthSpy = vi.spyOn(adapter, 'performWebOAuth'); const getContext = renderWithContext(); await act(async () => { - getContext().loginWithOAuthApp({ + getContext().loginWithOAuthApp('github', { clientId: 'id' as ClientID, clientSecret: 'secret' as ClientSecret, hostname: Constants.GITHUB_HOSTNAME, }); }); - expect(performGitHubWebOAuthSpy).toHaveBeenCalled(); + expect(oauthSpy).toHaveBeenCalled(); + }); + + it('loginWithDeviceFlowStart throws when the forge does not support device flow', async () => { + const getContext = renderWithContext(); + + await expect(getContext().loginWithDeviceFlowStart('gitea')).rejects.toThrow( + /Device flow is not supported for forge "gitea"/, + ); + }); + + it('loginWithDeviceFlowPoll throws when the forge does not support device flow', async () => { + const getContext = renderWithContext(); + + await expect( + getContext().loginWithDeviceFlowPoll('gitea', {} as DeviceFlowSession), + ).rejects.toThrow(/Device flow is not supported for forge "gitea"/); + }); + + it('loginWithDeviceFlowComplete throws when the forge declares no auth method', async () => { + const getContext = renderWithContext(); + + await expect( + getContext().loginWithDeviceFlowComplete( + 'gitea', + 'token' as Token, + Constants.GITHUB_HOSTNAME, + ), + ).rejects.toThrow(/did not declare a device-flow auth method/); + }); + + it('loginWithOAuthApp throws when the forge does not support OAuth app login', async () => { + const getContext = renderWithContext(); + + await expect( + getContext().loginWithOAuthApp('gitea', { + clientId: 'id' as ClientID, + clientSecret: 'secret' as ClientSecret, + hostname: Constants.GITHUB_HOSTNAME, + }), + ).rejects.toThrow(/OAuth app login is not supported for forge "gitea"/); }); it('logoutFromAccount calls removeAccountNotifications, removeAccount', async () => { diff --git a/src/renderer/context/App.tsx b/src/renderer/context/App.tsx index bc5b7be67..703bab8d3 100644 --- a/src/renderer/context/App.tsx +++ b/src/renderer/context/App.tsx @@ -37,12 +37,6 @@ import type { LoginPersonalAccessTokenOptions, } from '../utils/auth/types'; -import { - exchangeAuthCodeForAccessToken, - performGitHubWebOAuth, - pollGitHubDeviceFlow, - startGitHubDeviceFlow, -} from '../utils/auth/flows'; import { addAccount, getAccountUUID, @@ -105,10 +99,14 @@ function migrateLegacyAuthState(auth: AuthState): AuthState { export interface AppContextState { auth: AuthState; isLoggedIn: boolean; - loginWithDeviceFlowStart: (hostname?: Hostname, scopes?: string[]) => Promise; - loginWithDeviceFlowPoll: (session: DeviceFlowSession) => Promise; - loginWithDeviceFlowComplete: (token: Token, hostname: Hostname) => Promise; - loginWithOAuthApp: (data: LoginOAuthWebOptions) => Promise; + loginWithDeviceFlowStart: ( + forge: Forge, + hostname?: Hostname, + scopes?: string[], + ) => Promise; + loginWithDeviceFlowPoll: (forge: Forge, session: DeviceFlowSession) => Promise; + loginWithDeviceFlowComplete: (forge: Forge, token: Token, hostname: Hostname) => Promise; + loginWithOAuthApp: (forge: Forge, data: LoginOAuthWebOptions) => Promise; loginWithPersonalAccessToken: (data: LoginPersonalAccessTokenOptions) => Promise; logoutFromAccount: (account: Account) => Promise; @@ -439,40 +437,49 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { }, [auth]); /** - * Login to GitHub Gitify OAuth App. - * - * Initiate device flow session. + * Initiate an OAuth device-flow session for the given forge. */ const loginWithDeviceFlowStart = useCallback( - async (hostname?: Hostname, scopes?: string[]) => await startGitHubDeviceFlow(hostname, scopes), + async (forge: Forge, hostname?: Hostname, scopes?: string[]) => { + const adapter = getAdapter(forge); + if (!adapter.startDeviceFlow) { + throw new Error(`Device flow is not supported for forge "${forge}".`); + } + return await adapter.startDeviceFlow(hostname, scopes); + }, [], ); /** - * Login to GitHub Gitify OAuth App. - * - * Poll for device flow session. + * Poll for completion of an OAuth device-flow session. */ - const loginWithDeviceFlowPoll = useCallback( - async (session: DeviceFlowSession) => await pollGitHubDeviceFlow(session), - [], - ); + const loginWithDeviceFlowPoll = useCallback(async (forge: Forge, session: DeviceFlowSession) => { + const adapter = getAdapter(forge); + if (!adapter.pollDeviceFlow) { + throw new Error(`Device flow is not supported for forge "${forge}".`); + } + return await adapter.pollDeviceFlow(session); + }, []); /** - * Login to GitHub Gitify OAuth App. - * - * Finalize device flow session. + * Finalise an OAuth device-flow session by recording the account. */ const loginWithDeviceFlowComplete = useCallback( - async (token: Token, hostname: Hostname) => { + async (forge: Forge, token: Token, hostname: Hostname) => { + const adapter = getAdapter(forge); + const method = adapter.deviceFlowAuthMethod; + if (!method) { + throw new Error(`Forge "${forge}" did not declare a device-flow auth method.`); + } + const existingAccount = auth.accounts.find( - (a) => a.hostname === hostname && a.method === 'GitHub App', + (a) => a.hostname === hostname && a.method === method, ); if (existingAccount) { await removeAccountNotifications(existingAccount); } - const updatedAuth = await addAccount(auth, 'GitHub App', token, hostname, 'github'); + const updatedAuth = await addAccount(auth, method, token, hostname, forge); persistAuth(updatedAuth); await fetchNotifications({ auth: updatedAuth, settings }); @@ -481,12 +488,17 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { ); /** - * Login with custom GitHub OAuth App. + * Login with a custom OAuth app on the given forge. */ const loginWithOAuthApp = useCallback( - async (data: LoginOAuthWebOptions) => { - const { authOptions, authCode } = await performGitHubWebOAuth(data); - const token = await exchangeAuthCodeForAccessToken(authCode, authOptions); + async (forge: Forge, data: LoginOAuthWebOptions) => { + const adapter = getAdapter(forge); + if (!adapter.performWebOAuth || !adapter.exchangeAuthCodeForToken) { + throw new Error(`OAuth app login is not supported for forge "${forge}".`); + } + + const { authOptions, authCode } = await adapter.performWebOAuth(data); + const token = await adapter.exchangeAuthCodeForToken(authCode, authOptions); const existingAccount = auth.accounts.find( (a) => a.hostname === authOptions.hostname && a.method === 'OAuth App', @@ -495,13 +507,7 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { await removeAccountNotifications(existingAccount); } - const updatedAuth = await addAccount( - auth, - 'OAuth App', - token, - authOptions.hostname, - 'github', - ); + const updatedAuth = await addAccount(auth, 'OAuth App', token, authOptions.hostname, forge); persistAuth(updatedAuth); await fetchNotifications({ auth: updatedAuth, settings }); diff --git a/src/renderer/routes/AccountScopes.test.tsx b/src/renderer/routes/AccountScopes.test.tsx index c2d6e761d..e96e13181 100644 --- a/src/renderer/routes/AccountScopes.test.tsx +++ b/src/renderer/routes/AccountScopes.test.tsx @@ -151,9 +151,9 @@ describe('renderer/routes/AccountScopes.tsx', () => { expect(screen.getByTestId('account-scopes-no-scopes')).toBeInTheDocument(); }); - it('should open developer settings when manage link is clicked', async () => { - const openDeveloperSettingsSpy = vi - .spyOn(links, 'openDeveloperSettings') + it('should open account settings when manage link is clicked', async () => { + const openAccountSettingsSpy = vi + .spyOn(links, 'openAccountSettings') .mockImplementation(vi.fn()); await act(async () => { @@ -162,7 +162,7 @@ describe('renderer/routes/AccountScopes.tsx', () => { await userEvent.click(screen.getByTestId('account-scopes-manage-link')); - expect(openDeveloperSettingsSpy).toHaveBeenCalledTimes(1); + expect(openAccountSettingsSpy).toHaveBeenCalledTimes(1); }); it('should go back when pressing the back icon', async () => { diff --git a/src/renderer/routes/AccountScopes.tsx b/src/renderer/routes/AccountScopes.tsx index 585b02c34..8edf6e08b 100644 --- a/src/renderer/routes/AccountScopes.tsx +++ b/src/renderer/routes/AccountScopes.tsx @@ -20,7 +20,7 @@ import { getRequiredScopeNames, hasRequiredScopes, } from '../utils/auth/scopes'; -import { openDeveloperSettings } from '../utils/system/links'; +import { openAccountSettings } from '../utils/system/links'; interface LocationState { account: Account; @@ -186,7 +186,7 @@ export const AccountScopesRoute: FC = () => { diff --git a/src/renderer/routes/Accounts.test.tsx b/src/renderer/routes/Accounts.test.tsx index c0d58edba..b82a1f14b 100644 --- a/src/renderer/routes/Accounts.test.tsx +++ b/src/renderer/routes/Accounts.test.tsx @@ -213,6 +213,7 @@ describe('renderer/routes/Accounts.tsx', () => { expect(navigateMock).toHaveBeenCalledTimes(1); expect(navigateMock).toHaveBeenCalledWith('/login-device-flow', { replace: true, + state: { forge: 'github' }, }); }); @@ -245,6 +246,7 @@ describe('renderer/routes/Accounts.tsx', () => { expect(navigateMock).toHaveBeenCalledTimes(1); expect(navigateMock).toHaveBeenCalledWith('/login-oauth-app', { replace: true, + state: { forge: 'github' }, }); }); diff --git a/src/renderer/routes/Accounts.tsx b/src/renderer/routes/Accounts.tsx index 61f137d3c..9c2bb9149 100644 --- a/src/renderer/routes/Accounts.tsx +++ b/src/renderer/routes/Accounts.tsx @@ -33,7 +33,7 @@ import { Errors } from '../utils/core/errors'; import { toError } from '../utils/core/logger'; import { saveState } from '../utils/core/storage'; import { getAdapter } from '../utils/forges/registry'; -import { openAccountProfile, openDeveloperSettings, openHost } from '../utils/system/links'; +import { openAccountProfile, openAccountSettings, openHost } from '../utils/system/links'; import { getAuthMethodIcon, getPlatformIcon } from '../utils/ui/icons'; export const AccountsRoute: FC = () => { @@ -92,7 +92,10 @@ export const AccountsRoute: FC = () => { }; const loginWithGitHub = async () => { - return navigate('/login-device-flow', { replace: true }); + return navigate('/login-device-flow', { + replace: true, + state: { forge: 'github' as const }, + }); }; const loginWithPersonalAccessToken = () => { @@ -107,7 +110,10 @@ export const AccountsRoute: FC = () => { }; const loginWithOAuthApp = () => { - return navigate('/login-oauth-app', { replace: true }); + return navigate('/login-oauth-app', { + replace: true, + state: { forge: 'github' as const }, + }); }; const getAccountError = (account: Account) => { @@ -203,7 +209,7 @@ export const AccountsRoute: FC = () => { data-testid="account-developer-settings" direction="horizontal" gap="condensed" - onClick={() => openDeveloperSettings(account)} + onClick={() => openAccountSettings(account)} title="Open developer settings ↗" > {AuthMethodIcon && } @@ -217,7 +223,7 @@ export const AccountsRoute: FC = () => { data-testid="account-bad-credentials" direction="horizontal" gap="condensed" - onClick={() => openDeveloperSettings(account)} + onClick={() => openAccountSettings(account)} title="Open developer settings ↗" > diff --git a/src/renderer/routes/Login.test.tsx b/src/renderer/routes/Login.test.tsx index 447d3669f..922be6775 100644 --- a/src/renderer/routes/Login.test.tsx +++ b/src/renderer/routes/Login.test.tsx @@ -35,7 +35,9 @@ describe('renderer/routes/Login.tsx', () => { await userEvent.click(screen.getByTestId('login-github')); expect(navigateMock).toHaveBeenCalledTimes(1); - expect(navigateMock).toHaveBeenCalledWith('/login-device-flow'); + expect(navigateMock).toHaveBeenCalledWith('/login-device-flow', { + state: { forge: 'github' }, + }); }); it('should navigate to login with personal access token', async () => { @@ -53,7 +55,9 @@ describe('renderer/routes/Login.tsx', () => { await userEvent.click(screen.getByTestId('login-oauth-app')); expect(navigateMock).toHaveBeenCalledTimes(1); - expect(navigateMock).toHaveBeenCalledWith('/login-oauth-app'); + expect(navigateMock).toHaveBeenCalledWith('/login-oauth-app', { + state: { forge: 'github' }, + }); }); it('should navigate to login with Gitea personal access token', async () => { diff --git a/src/renderer/routes/LoginWithDeviceFlow.test.tsx b/src/renderer/routes/LoginWithDeviceFlow.test.tsx index 7da6185b0..48f8b0a71 100644 --- a/src/renderer/routes/LoginWithDeviceFlow.test.tsx +++ b/src/renderer/routes/LoginWithDeviceFlow.test.tsx @@ -50,7 +50,7 @@ describe('renderer/routes/LoginWithDeviceFlow.tsx', () => { await userEvent.click(screen.getByTestId('device-scope-public')); - expect(loginWithDeviceFlowStartMock).toHaveBeenCalledWith(undefined, [ + expect(loginWithDeviceFlowStartMock).toHaveBeenCalledWith('github', undefined, [ 'notifications', 'read:user', 'public_repo', @@ -81,7 +81,7 @@ describe('renderer/routes/LoginWithDeviceFlow.tsx', () => { await userEvent.click(screen.getByTestId('device-scope-full')); - expect(loginWithDeviceFlowStartMock).toHaveBeenCalledWith(undefined, [ + expect(loginWithDeviceFlowStartMock).toHaveBeenCalledWith('github', undefined, [ 'notifications', 'read:user', 'repo', diff --git a/src/renderer/routes/LoginWithDeviceFlow.tsx b/src/renderer/routes/LoginWithDeviceFlow.tsx index 5ee94878a..790040016 100644 --- a/src/renderer/routes/LoginWithDeviceFlow.tsx +++ b/src/renderer/routes/LoginWithDeviceFlow.tsx @@ -13,16 +13,17 @@ import { Page } from '../components/layout/Page'; import { Footer } from '../components/primitives/Footer'; import { Header } from '../components/primitives/Header'; -import type { Account, Link } from '../types'; +import type { Account, Forge, Link } from '../types'; import type { DeviceFlowSession } from '../utils/auth/types'; import { getAlternateScopeNames, getRecommendedScopeNames } from '../utils/auth/scopes'; import { rendererLogError, toError } from '../utils/core/logger'; import { copyToClipboard, openExternalLink } from '../utils/system/comms'; -import { openDeveloperSettings } from '../utils/system/links'; +import { openAccountSettings } from '../utils/system/links'; interface LocationState { account?: Account; + forge?: Forge; } type ScopeChoice = 'public' | 'full'; @@ -30,7 +31,12 @@ type ScopeChoice = 'public' | 'full'; export const LoginWithDeviceFlowRoute: FC = () => { const navigate = useNavigate(); const location = useLocation(); - const { account: reAuthAccount } = (location.state ?? {}) as LocationState; + const { account: reAuthAccount, forge: routeForge } = (location.state ?? {}) as LocationState; + // Forge is supplied either by the login-method descriptor's state (new + // login) or via the re-authenticating account (re-auth). Either path lands + // here through a registered adapter, so a missing forge would indicate a + // mis-registered route — fall back to 'github' to preserve existing UX. + const forge: Forge = routeForge ?? reAuthAccount?.forge ?? 'github'; const { loginWithDeviceFlowStart, loginWithDeviceFlowPoll, loginWithDeviceFlowComplete } = useAppContext(); @@ -47,7 +53,7 @@ export const LoginWithDeviceFlowRoute: FC = () => { const scopes = scopeChoice === 'public' ? getAlternateScopeNames() : getRecommendedScopeNames(); - const newSession = await loginWithDeviceFlowStart(reAuthAccount?.hostname, scopes); + const newSession = await loginWithDeviceFlowStart(forge, reAuthAccount?.hostname, scopes); setSession(newSession); // Auto-copy the user code to clipboard @@ -64,7 +70,7 @@ export const LoginWithDeviceFlowRoute: FC = () => { if (scopeChoice) { initializeDeviceFlow(); } - }, [loginWithDeviceFlowStart, reAuthAccount, scopeChoice]); + }, [loginWithDeviceFlowStart, reAuthAccount, scopeChoice, forge]); // Poll for device flow completion useEffect(() => { @@ -81,10 +87,10 @@ export const LoginWithDeviceFlowRoute: FC = () => { try { while (isActive && Date.now() < session.expiresAt) { - const token = await loginWithDeviceFlowPoll(session); + const token = await loginWithDeviceFlowPoll(forge, session); if (token && isActive) { - await loginWithDeviceFlowComplete(token, session.hostname); + await loginWithDeviceFlowComplete(forge, token, session.hostname); navigate('/'); return; } @@ -118,7 +124,7 @@ export const LoginWithDeviceFlowRoute: FC = () => { } }; // oxlint-disable-next-line react/exhaustive-deps -- navigate is stable - }, [session, loginWithDeviceFlowPoll, loginWithDeviceFlowComplete]); + }, [session, loginWithDeviceFlowPoll, loginWithDeviceFlowComplete, forge]); const handleCopyUserCode = useCallback(async () => { if (session?.userCode) { @@ -229,7 +235,7 @@ export const LoginWithDeviceFlowRoute: FC = () => {