diff --git a/src/renderer/context/App.test.tsx b/src/renderer/context/App.test.tsx index 258c6ac70..7ad8420e6 100644 --- a/src/renderer/context/App.test.tsx +++ b/src/renderer/context/App.test.tsx @@ -214,7 +214,7 @@ describe('renderer/context/App.tsx', () => { it('loginWithDeviceFlowStart delegates to the forge adapter', async () => { const adapter = getAdapter('github'); - const startSpy = vi.spyOn(adapter, 'startDeviceFlow').mockImplementation(vi.fn()); + const startSpy = vi.spyOn(adapter.deviceFlow!, 'start').mockImplementation(vi.fn()); const getContext = renderWithContext(); @@ -227,7 +227,7 @@ describe('renderer/context/App.tsx', () => { it('loginWithDeviceFlowPoll delegates to the forge adapter', async () => { const adapter = getAdapter('github'); - const pollSpy = vi.spyOn(adapter, 'pollDeviceFlow').mockImplementation(vi.fn()); + const pollSpy = vi.spyOn(adapter.deviceFlow!, 'poll').mockImplementation(vi.fn()); const getContext = renderWithContext(); @@ -260,7 +260,7 @@ describe('renderer/context/App.tsx', () => { it('loginWithOAuthApp delegates to the forge adapter', async () => { const adapter = getAdapter('github'); - const oauthSpy = vi.spyOn(adapter, 'performWebOAuth'); + const oauthSpy = vi.spyOn(adapter.oauthWebApp!, 'performWebOAuth'); const getContext = renderWithContext(); @@ -291,7 +291,7 @@ describe('renderer/context/App.tsx', () => { ).rejects.toThrow(/Device flow is not supported for forge "gitea"/); }); - it('loginWithDeviceFlowComplete throws when the forge declares no auth method', async () => { + it('loginWithDeviceFlowComplete throws when the forge does not support device flow', async () => { const getContext = renderWithContext(); await expect( @@ -300,7 +300,7 @@ describe('renderer/context/App.tsx', () => { 'token' as Token, Constants.GITHUB_HOSTNAME, ), - ).rejects.toThrow(/did not declare a device-flow auth method/); + ).rejects.toThrow(/does not support device flow/); }); it('loginWithOAuthApp throws when the forge does not support OAuth app login', async () => { diff --git a/src/renderer/context/App.tsx b/src/renderer/context/App.tsx index 703bab8d3..8c13f0dd3 100644 --- a/src/renderer/context/App.tsx +++ b/src/renderer/context/App.tsx @@ -441,11 +441,11 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { */ const loginWithDeviceFlowStart = useCallback( async (forge: Forge, hostname?: Hostname, scopes?: string[]) => { - const adapter = getAdapter(forge); - if (!adapter.startDeviceFlow) { + const { deviceFlow } = getAdapter(forge); + if (!deviceFlow) { throw new Error(`Device flow is not supported for forge "${forge}".`); } - return await adapter.startDeviceFlow(hostname, scopes); + return await deviceFlow.start(hostname, scopes); }, [], ); @@ -454,11 +454,11 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { * Poll for completion of an OAuth device-flow session. */ const loginWithDeviceFlowPoll = useCallback(async (forge: Forge, session: DeviceFlowSession) => { - const adapter = getAdapter(forge); - if (!adapter.pollDeviceFlow) { + const { deviceFlow } = getAdapter(forge); + if (!deviceFlow) { throw new Error(`Device flow is not supported for forge "${forge}".`); } - return await adapter.pollDeviceFlow(session); + return await deviceFlow.poll(session); }, []); /** @@ -466,11 +466,11 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { */ const loginWithDeviceFlowComplete = useCallback( 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 { deviceFlow } = getAdapter(forge); + if (!deviceFlow) { + throw new Error(`Forge "${forge}" does not support device flow.`); } + const method = deviceFlow.authMethod; const existingAccount = auth.accounts.find( (a) => a.hostname === hostname && a.method === method, @@ -492,13 +492,13 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { */ const loginWithOAuthApp = useCallback( async (forge: Forge, data: LoginOAuthWebOptions) => { - const adapter = getAdapter(forge); - if (!adapter.performWebOAuth || !adapter.exchangeAuthCodeForToken) { + const { oauthWebApp } = getAdapter(forge); + if (!oauthWebApp) { 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 { authOptions, authCode } = await oauthWebApp.performWebOAuth(data); + const token = await oauthWebApp.exchangeAuthCodeForToken(authCode, authOptions); const existingAccount = auth.accounts.find( (a) => a.hostname === authOptions.hostname && a.method === 'OAuth App', diff --git a/src/renderer/routes/Accounts.tsx b/src/renderer/routes/Accounts.tsx index fa116d776..e1bdb37d8 100644 --- a/src/renderer/routes/Accounts.tsx +++ b/src/renderer/routes/Accounts.tsx @@ -243,7 +243,7 @@ export const AccountsRoute: FC = () => { variant={i === 0 ? 'primary' : 'default'} /> - {!hasBadCredentials && getAdapter(account).supportsOAuthScopes && ( + {!hasBadCredentials && getAdapter(account).oauthScopes && ( { - - - Note: to change previously granted permissions, revoke Gitify's access at{' '} - - , then re-authorize above. - - + {(() => { + const adapter = getAdapter(forge); + const revokeUrl = adapter.deviceFlow?.getRevokeAccessUrl( + (reAuthAccount?.hostname ?? Constants.GITHUB_HOSTNAME) as Hostname, + ); + if (!revokeUrl) { + return null; + } + const label = `${adapter.displayName} → Developer Settings`; + return ( + + + Note: to change previously granted permissions, revoke Gitify's access at{' '} + + , then re-authorize above. + + + ); + })()} ); diff --git a/src/renderer/routes/LoginWithOAuthApp.tsx b/src/renderer/routes/LoginWithOAuthApp.tsx index dd8a48952..9f217da1f 100644 --- a/src/renderer/routes/LoginWithOAuthApp.tsx +++ b/src/renderer/routes/LoginWithOAuthApp.tsx @@ -18,7 +18,7 @@ import type { LoginOAuthWebOptions } from '../utils/auth/types'; import { isValidHostname } from '../utils/auth/utils'; import { rendererLogError, toError } from '../utils/core/logger'; -import { getNewOAuthAppURL, isValidClientId, isValidToken } from '../utils/forges/github/auth'; +import { getAdapter } from '../utils/forges/registry'; import { openExternalLink } from '../utils/system/comms'; interface LocationState { @@ -37,8 +37,9 @@ interface IFormErrors { invalidCredentialsForHost?: string; } -export const validateForm = (values: IFormData): IFormErrors => { +export const validateForm = (values: IFormData, forge: Forge = 'github'): IFormErrors => { const errors: IFormErrors = {}; + const adapter = getAdapter(forge); if (!values.hostname) { errors.hostname = 'Hostname is required'; @@ -48,13 +49,13 @@ export const validateForm = (values: IFormData): IFormErrors => { if (!values.clientId) { errors.clientId = 'Client ID is required'; - } else if (!isValidClientId(values.clientId)) { + } else if (!adapter.oauthWebApp?.validateClientId(values.clientId)) { errors.clientId = 'Client ID format is invalid'; } if (!values.clientSecret) { errors.clientSecret = 'Client Secret is required'; - } else if (!isValidToken(values.clientSecret as unknown as Token)) { + } else if (!adapter.validateToken(values.clientSecret as unknown as Token)) { errors.clientSecret = 'Client Secret format is invalid'; } @@ -83,7 +84,7 @@ export const LoginWithOAuthAppRoute: FC = () => { const handleSubmit = async () => { setIsVerifyingCredentials(true); - const newErrors = validateForm(formData); + const newErrors = validateForm(formData, forge); setErrors(newErrors); @@ -166,7 +167,12 @@ export const LoginWithOAuthAppRoute: FC = () => { data-testid="login-create-oauth-app" disabled={!formData.hostname} leadingVisual={PersonIcon} - onClick={() => openExternalLink(getNewOAuthAppURL(formData.hostname))} + onClick={() => { + const url = getAdapter(forge).oauthWebApp?.getNewOAuthAppUrl(formData.hostname); + if (url) { + openExternalLink(url); + } + }} size="small" > Create new OAuth App diff --git a/src/renderer/utils/auth/scopes.ts b/src/renderer/utils/auth/scopes.ts index 9cf6d4418..8aa0d2f2e 100644 --- a/src/renderer/utils/auth/scopes.ts +++ b/src/renderer/utils/auth/scopes.ts @@ -8,12 +8,12 @@ import { getAdapter } from '../forges/registry'; * Return true if the account has all required OAuth scopes. * * Scope semantics live on the forge adapter — forges without an OAuth scope - * concept (e.g. Gitea) report `true` for every check. + * concept (e.g. Gitea) omit `oauthScopes`, and every check resolves to `true`. * * @param account - The account whose scopes to check. */ export function hasRequiredScopes(account: Account): boolean { - return getAdapter(account).hasRequiredScopes(account); + return getAdapter(account).oauthScopes?.hasRequired(account) ?? true; } /** @@ -22,7 +22,7 @@ export function hasRequiredScopes(account: Account): boolean { * @param account - The account whose scopes to check. */ export function hasRecommendedScopes(account: Account): boolean { - return getAdapter(account).hasRecommendedScopes(account); + return getAdapter(account).oauthScopes?.hasRecommended(account) ?? true; } /** @@ -31,7 +31,7 @@ export function hasRecommendedScopes(account: Account): boolean { * @param account - The account whose scopes to check. */ export function hasAlternateScopes(account: Account): boolean { - return getAdapter(account).hasAlternateScopes(account); + return getAdapter(account).oauthScopes?.hasAlternate(account) ?? true; } /** diff --git a/src/renderer/utils/forges/gitea/adapter.test.ts b/src/renderer/utils/forges/gitea/adapter.test.ts index 63f909a4c..a550a0676 100644 --- a/src/renderer/utils/forges/gitea/adapter.test.ts +++ b/src/renderer/utils/forges/gitea/adapter.test.ts @@ -167,11 +167,9 @@ describe('renderer/utils/forges/gitea/adapter.ts', () => { }); }); - describe('OAuth scope helpers', () => { - it('always reports every scope check as satisfied', () => { - expect(giteaAdapter.hasRequiredScopes(mockGiteaAccount)).toBe(true); - expect(giteaAdapter.hasRecommendedScopes(mockGiteaAccount)).toBe(true); - expect(giteaAdapter.hasAlternateScopes(mockGiteaAccount)).toBe(true); + describe('oauthScopes capability bundle', () => { + it('is omitted because Gitea has no OAuth scope concept', () => { + expect(giteaAdapter.oauthScopes).toBeUndefined(); }); }); diff --git a/src/renderer/utils/forges/gitea/adapter.ts b/src/renderer/utils/forges/gitea/adapter.ts index 6b67e8fe6..f29dfc2d2 100644 --- a/src/renderer/utils/forges/gitea/adapter.ts +++ b/src/renderer/utils/forges/gitea/adapter.ts @@ -108,8 +108,6 @@ export const giteaAdapter: ForgeAdapter = { // for those branches. getAuthMethodIcon: () => KeyIcon, - supportsOAuthScopes: false, - loginMethods: [ { testId: 'login-gitea-pat', @@ -120,9 +118,6 @@ export const giteaAdapter: ForgeAdapter = { }, ], - // Gitea has no GitHub-style OAuth scope concept; treat any token as fully - // scoped so the recommended/alternate UI prompts never surface for Gitea. - hasRequiredScopes: () => true, - hasRecommendedScopes: () => true, - hasAlternateScopes: () => true, + // Gitea has no GitHub-style OAuth scope concept, so `oauthScopes` is + // omitted. Callers skip scopes UI when this is undefined. }; diff --git a/src/renderer/utils/forges/github/adapter.test.ts b/src/renderer/utils/forges/github/adapter.test.ts index 5614b90d5..3fec6742f 100644 --- a/src/renderer/utils/forges/github/adapter.test.ts +++ b/src/renderer/utils/forges/github/adapter.test.ts @@ -47,11 +47,14 @@ describe('renderer/utils/forges/github/adapter.ts', () => { }); it('wires the device-flow and OAuth-app methods so the context can dispatch via the adapter', () => { - expect(githubAdapter.deviceFlowAuthMethod).toBe('GitHub App'); - expect(githubAdapter.startDeviceFlow).toBeDefined(); - expect(githubAdapter.pollDeviceFlow).toBeDefined(); - expect(githubAdapter.performWebOAuth).toBeDefined(); - expect(githubAdapter.exchangeAuthCodeForToken).toBeDefined(); + expect(githubAdapter.deviceFlow?.authMethod).toBe('GitHub App'); + expect(githubAdapter.deviceFlow?.start).toBeDefined(); + expect(githubAdapter.deviceFlow?.poll).toBeDefined(); + expect(githubAdapter.deviceFlow?.getRevokeAccessUrl).toBeDefined(); + expect(githubAdapter.oauthWebApp?.performWebOAuth).toBeDefined(); + expect(githubAdapter.oauthWebApp?.exchangeAuthCodeForToken).toBeDefined(); + expect(githubAdapter.oauthWebApp?.validateClientId).toBeDefined(); + expect(githubAdapter.oauthWebApp?.getNewOAuthAppUrl).toBeDefined(); }); }); @@ -166,33 +169,41 @@ describe('renderer/utils/forges/github/adapter.ts', () => { }); }); - describe('OAuth scope helpers', () => { + describe('oauthScopes capability bundle', () => { function withScopes(scopes: string[]) { return { ...mockGitHubCloudAccount, scopes }; } - it('hasRequiredScopes is true when notifications + read:user are present', () => { - expect(githubAdapter.hasRequiredScopes(withScopes(['notifications', 'read:user']))).toBe( - true, - ); + it('exposes the bundle (GitHub has an OAuth scope concept)', () => { + expect(githubAdapter.oauthScopes).toBeDefined(); }); - it('hasRequiredScopes is false when a required scope is missing', () => { - expect(githubAdapter.hasRequiredScopes(withScopes(['notifications']))).toBe(false); + it('hasRequired is true when notifications + read:user are present', () => { + expect( + githubAdapter.oauthScopes!.hasRequired(withScopes(['notifications', 'read:user'])), + ).toBe(true); }); - it('hasRecommendedScopes requires the full repo scope set', () => { + it('hasRequired is false when a required scope is missing', () => { + expect(githubAdapter.oauthScopes!.hasRequired(withScopes(['notifications']))).toBe(false); + }); + + it('hasRecommended requires the full repo scope set', () => { expect( - githubAdapter.hasRecommendedScopes(withScopes(['notifications', 'read:user', 'repo'])), + githubAdapter.oauthScopes!.hasRecommended( + withScopes(['notifications', 'read:user', 'repo']), + ), ).toBe(true); - expect(githubAdapter.hasRecommendedScopes(withScopes(['notifications', 'read:user']))).toBe( - false, - ); + expect( + githubAdapter.oauthScopes!.hasRecommended(withScopes(['notifications', 'read:user'])), + ).toBe(false); }); - it('hasAlternateScopes accepts public_repo as the legacy substitute', () => { + it('hasAlternate accepts public_repo as the legacy substitute', () => { expect( - githubAdapter.hasAlternateScopes(withScopes(['notifications', 'read:user', 'public_repo'])), + githubAdapter.oauthScopes!.hasAlternate( + withScopes(['notifications', 'read:user', 'public_repo']), + ), ).toBe(true); }); }); diff --git a/src/renderer/utils/forges/github/adapter.ts b/src/renderer/utils/forges/github/adapter.ts index c61a4c1eb..641bc5ba7 100644 --- a/src/renderer/utils/forges/github/adapter.ts +++ b/src/renderer/utils/forges/github/adapter.ts @@ -6,7 +6,14 @@ import type { Account, Link, RawGitifyNotification, SettingsState } from '../../ import type { AuthMethod } from '../../auth/types'; import type { ForgeAdapter, NotificationDisplayHelpers, RefreshAccountData } from '../types'; -import { extractHostVersion, getDeveloperSettingsURL, getNewTokenURL, isValidToken } from './auth'; +import { + extractHostVersion, + getDeveloperSettingsURL, + getNewOAuthAppURL, + getNewTokenURL, + isValidClientId, + isValidToken, +} from './auth'; import { githubCapabilities } from './capabilities'; import { fetchAuthenticatedUserDetails, @@ -102,8 +109,6 @@ export const githubAdapter: ForgeAdapter = { documentationUrl: Constants.GITHUB_DOCS.PAT_URL as Link, getAuthMethodIcon: githubAuthMethodIcon, - supportsOAuthScopes: true, - loginMethods: [ { testId: 'login-github', @@ -128,15 +133,26 @@ export const githubAdapter: ForgeAdapter = { }, ], - deviceFlowAuthMethod: 'GitHub App', - startDeviceFlow: startGitHubDeviceFlow, - pollDeviceFlow: pollGitHubDeviceFlow, - performWebOAuth: performGitHubWebOAuth, - exchangeAuthCodeForToken: exchangeAuthCodeForAccessToken, + deviceFlow: { + authMethod: 'GitHub App', + start: startGitHubDeviceFlow, + poll: pollGitHubDeviceFlow, + getRevokeAccessUrl: (hostname) => + getDeveloperSettingsURL({ hostname, method: 'GitHub App' } as Account), + }, + + oauthWebApp: { + performWebOAuth: performGitHubWebOAuth, + exchangeAuthCodeForToken: exchangeAuthCodeForAccessToken, + validateClientId: isValidClientId, + getNewOAuthAppUrl: getNewOAuthAppURL, + }, - hasRequiredScopes: (account) => accountHasScopes(account, 'REQUIRED'), - hasRecommendedScopes: (account) => accountHasScopes(account, 'RECOMMENDED'), - hasAlternateScopes: (account) => accountHasScopes(account, 'ALTERNATE'), + oauthScopes: { + hasRequired: (account) => accountHasScopes(account, 'REQUIRED'), + hasRecommended: (account) => accountHasScopes(account, 'RECOMMENDED'), + hasAlternate: (account) => accountHasScopes(account, 'ALTERNATE'), + }, }; function accountHasScopes( diff --git a/src/renderer/utils/forges/types.ts b/src/renderer/utils/forges/types.ts index 67002c70d..892ab7deb 100644 --- a/src/renderer/utils/forges/types.ts +++ b/src/renderer/utils/forges/types.ts @@ -185,39 +185,72 @@ export interface ForgeAdapter { // --- Auth flows --- // Optional because not every forge supports every flow. Gitea today is - // PAT-only and implements none of these. The orchestrator gates UI on the - // presence of these methods (and on `loginMethods` entries pointing at + // PAT-only and omits both bundles. The orchestrator gates UI on the + // presence of these bundles (and on `loginMethods` entries pointing at // `/login-device-flow` or `/login-oauth-app`). /** - * The `AuthMethod` string recorded on the account when a successful device - * flow login completes. Required when `startDeviceFlow` is provided. + * OAuth device-flow capability. Forges without device-flow support (e.g. + * Gitea) omit this bundle entirely — callers gate the device-flow UI on + * its presence. */ - deviceFlowAuthMethod?: AuthMethod; - /** Start an OAuth device-flow authorization session. */ - startDeviceFlow?(hostname?: Hostname, scopes?: string[]): Promise; - /** Poll for completion of a device-flow session; resolves to a token when granted. */ - pollDeviceFlow?(session: DeviceFlowSession): Promise; - /** Initiate a custom-OAuth-app web flow. */ - performWebOAuth?(options: LoginOAuthWebOptions): Promise; - /** Exchange an OAuth web-flow authorization code for an access token. */ - exchangeAuthCodeForToken?(authCode: AuthCode, options: LoginOAuthWebOptions): Promise; + deviceFlow?: DeviceFlowSupport; - // --- OAuth scopes --- - // Forges without an OAuth scope concept (e.g. Gitea) report `true` for - // every check, signalling "nothing to verify". + /** + * Custom-OAuth-app web flow. Forges without OAuth-app support (e.g. Gitea) + * omit this bundle entirely — callers gate the OAuth-app UI on its presence. + */ + oauthWebApp?: OAuthWebAppSupport; /** - * Whether this forge has an OAuth scope concept at all. Drives whether the - * UI surfaces a scopes affordance (account view scopes, scope health icon). - * Forges that return `false` here should have callers skip rendering any - * scopes UI rather than showing a meaningless "all granted" or empty state. + * OAuth scope checks. Forges with no scope concept (e.g. Gitea) omit this + * bundle entirely — callers should treat `oauthScopes === undefined` as + * "nothing to verify" and skip any scopes UI rather than render a + * meaningless "all granted" state. */ - readonly supportsOAuthScopes: boolean; + oauthScopes?: OAuthScopesSupport; +} + +/** + * OAuth scope-checking capability bundle. Present only on forges with an + * OAuth scope concept (GitHub today). + */ +export interface OAuthScopesSupport { /** Whether the account holds the minimum scopes Gitify needs to function. */ - hasRequiredScopes(account: Account): boolean; + hasRequired(account: Account): boolean; /** Whether the account holds the full recommended scope set. */ - hasRecommendedScopes(account: Account): boolean; + hasRecommended(account: Account): boolean; /** Whether the account holds the alternate (legacy) scope set. */ - hasAlternateScopes(account: Account): boolean; + hasAlternate(account: Account): boolean; +} + +/** + * Custom-OAuth-app web flow capability bundle. Present only on forges that + * support browser-redirect OAuth with user-supplied client credentials + * (GitHub today). + */ +export interface OAuthWebAppSupport { + /** Start the OAuth web flow and return the auth code once the user consents. */ + performWebOAuth(options: LoginOAuthWebOptions): Promise; + /** Exchange an OAuth web-flow authorization code for an access token. */ + exchangeAuthCodeForToken(authCode: AuthCode, options: LoginOAuthWebOptions): Promise; + /** Whether the supplied OAuth client ID matches the forge's expected format. */ + validateClientId(clientId: string): boolean; + /** URL the user visits to create a new OAuth app on the forge. */ + getNewOAuthAppUrl(hostname: Hostname): Link; +} + +/** + * OAuth device-flow capability bundle. Present only on forges that support + * browser-side device authorisation (GitHub today). + */ +export interface DeviceFlowSupport { + /** `AuthMethod` recorded on the account when a successful login completes. */ + authMethod: AuthMethod; + /** Start a device-flow authorization session. */ + start(hostname?: Hostname, scopes?: string[]): Promise; + /** Poll for completion; resolves to a token when granted. */ + poll(session: DeviceFlowSession): Promise; + /** URL the user visits to revoke Gitify's access on this forge. */ + getRevokeAccessUrl(hostname: Hostname): Link; }