From eec0d485688a7db3bd545243d0e9846006b8b1c0 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 16 Sep 2026 16:58:58 -0400 Subject: [PATCH] fix(cloud-auth): do not relink an in-flight sign-in after sign-out Signing out now invalidates outstanding PKCE attempts in main and the renderer so a later browser tab cannot restore the session. --- ...file-cloud-service-connect-overlap.test.ts | 91 ++++++++++++++++--- .../orca-profiles/profile-cloud-service.ts | 8 ++ ...files-auth-actions-connect-overlap.test.ts | 59 +++++++++++- .../slices/orca-profiles-auth-actions.ts | 2 + 4 files changed, 145 insertions(+), 15 deletions(-) diff --git a/src/main/orca-profiles/profile-cloud-service-connect-overlap.test.ts b/src/main/orca-profiles/profile-cloud-service-connect-overlap.test.ts index 3321fa06877..bc7e4f74d66 100644 --- a/src/main/orca-profiles/profile-cloud-service-connect-overlap.test.ts +++ b/src/main/orca-profiles/profile-cloud-service-connect-overlap.test.ts @@ -8,17 +8,21 @@ import type { OrcaProfileCloudSummary } from '../../shared/orca-profiles' -const { beginOrcaCloudPkceFlowMock, exchangeOrcaCloudAuthCodeMock, safeStorageMock } = vi.hoisted( - () => ({ - beginOrcaCloudPkceFlowMock: vi.fn(), - exchangeOrcaCloudAuthCodeMock: vi.fn(), - safeStorageMock: { - decryptString: vi.fn((value: Buffer) => value.toString('utf-8')), - encryptString: vi.fn((value: string) => Buffer.from(value, 'utf-8')), - isEncryptionAvailable: vi.fn(() => true) - } - }) -) +const { + beginOrcaCloudPkceFlowMock, + exchangeOrcaCloudAuthCodeMock, + revokeOrcaCloudSessionMock, + safeStorageMock +} = vi.hoisted(() => ({ + beginOrcaCloudPkceFlowMock: vi.fn(), + exchangeOrcaCloudAuthCodeMock: vi.fn(), + revokeOrcaCloudSessionMock: vi.fn(), + safeStorageMock: { + decryptString: vi.fn((value: Buffer) => value.toString('utf-8')), + encryptString: vi.fn((value: string) => Buffer.from(value, 'utf-8')), + isEncryptionAvailable: vi.fn(() => true) + } +})) let userDataPath = '' @@ -36,11 +40,15 @@ vi.mock('./profile-cloud-pkce', () => ({ vi.mock('./profile-cloud-client', () => ({ createOrcaCloudProfile: vi.fn(), exchangeOrcaCloudAuthCode: exchangeOrcaCloudAuthCodeMock, - revokeOrcaCloudSession: vi.fn(), + revokeOrcaCloudSession: revokeOrcaCloudSessionMock, selectOrcaCloudOrg: vi.fn() })) -import { connectCurrentOrcaProfile, getCurrentOrcaProfileAuthStatus } from './profile-cloud-service' +import { + connectCurrentOrcaProfile, + getCurrentOrcaProfileAuthStatus, + signOutCurrentOrcaProfile +} from './profile-cloud-service' const earlierCloud: OrcaProfileCloudSummary = { cloudProfileId: 'cloud-profile-1', @@ -69,6 +77,8 @@ describe('Orca cloud overlapping connect', () => { userDataPath = mkdtempSync(join(tmpdir(), 'orca-cloud-connect-overlap-')) beginOrcaCloudPkceFlowMock.mockReset() exchangeOrcaCloudAuthCodeMock.mockReset() + revokeOrcaCloudSessionMock.mockReset() + revokeOrcaCloudSessionMock.mockResolvedValue(undefined) safeStorageMock.decryptString.mockReset() safeStorageMock.encryptString.mockReset() safeStorageMock.isEncryptionAvailable.mockReset() @@ -223,4 +233,59 @@ describe('Orca cloud overlapping connect', () => { await expect(earlier).resolves.toMatchObject({ status: 'cancelled' }) expect(getCurrentOrcaProfileAuthStatus(userDataPath).cloud?.email).toBe('ada@example.com') }) + + it('does not relink an in-flight later wait after sign-out', async () => { + type PkceCode = { + code: string + codeVerifier: string + nonce: string + redirectUri: string + state: string + } + let finishEarlierPkce!: (value: PkceCode) => void + let finishLaterPkce!: (value: PkceCode) => void + beginOrcaCloudPkceFlowMock + .mockReturnValueOnce( + new Promise((resolve) => { + finishEarlierPkce = resolve + }) + ) + .mockReturnValueOnce( + new Promise((resolve) => { + finishLaterPkce = resolve + }) + ) + exchangeOrcaCloudAuthCodeMock.mockImplementation(async (_config, args) => ({ + accessToken: 'access-token', + refreshToken: 'refresh-token', + expiresAt: Date.now() + 3_600_000, + cloud: args.code === 'later-code' ? laterCloud : earlierCloud, + organizations, + capabilities + })) + + const earlier = connectCurrentOrcaProfile(userDataPath) + const later = connectCurrentOrcaProfile(userDataPath) + finishEarlierPkce({ + code: 'earlier-code', + codeVerifier: 'earlier-verifier', + nonce: 'earlier-nonce', + redirectUri: 'http://127.0.0.1:4100/auth/callback', + state: 'earlier-state' + }) + await expect(earlier).resolves.toMatchObject({ status: 'connected' }) + await expect(signOutCurrentOrcaProfile(userDataPath)).resolves.toMatchObject({ + status: 'signed-out' + }) + finishLaterPkce({ + code: 'later-code', + codeVerifier: 'later-verifier', + nonce: 'later-nonce', + redirectUri: 'http://127.0.0.1:4101/auth/callback', + state: 'later-state' + }) + await expect(later).resolves.toMatchObject({ status: 'cancelled' }) + expect(exchangeOrcaCloudAuthCodeMock).toHaveBeenCalledTimes(1) + expect(getCurrentOrcaProfileAuthStatus(userDataPath)).toMatchObject({ state: 'local' }) + }) }) diff --git a/src/main/orca-profiles/profile-cloud-service.ts b/src/main/orca-profiles/profile-cloud-service.ts index 66354b567b1..8c842560bda 100644 --- a/src/main/orca-profiles/profile-cloud-service.ts +++ b/src/main/orca-profiles/profile-cloud-service.ts @@ -39,6 +39,11 @@ export { refreshCurrentOrcaProfileAuth } from './profile-cloud-capability-refres let nextCloudConnectAttempt = 0 let linkedCloudConnectAttempt = 0 +function invalidateOutstandingCloudConnectAttempts(): void { + nextCloudConnectAttempt += 1 + linkedCloudConnectAttempt = nextCloudConnectAttempt +} + function isUserCancelledAuthError(message: string): boolean { return message === 'orca_cloud_auth_timeout' || message === 'orca_cloud_auth_denied' } @@ -123,6 +128,9 @@ export async function connectCurrentOrcaProfile( export async function signOutCurrentOrcaProfile( userDataPath: string ): Promise { + // Why: a Sign in click still waiting in the browser must not relink after + // the user explicitly signed out. + invalidateOutstandingCloudConnectAttempts() const active = ensureActiveOrcaProfile(userDataPath) const configState = getOrcaCloudAuthConfig() const session = readOrcaCloudSession(active.profile.id, userDataPath) diff --git a/src/renderer/src/store/slices/orca-profiles-auth-actions-connect-overlap.test.ts b/src/renderer/src/store/slices/orca-profiles-auth-actions-connect-overlap.test.ts index b54d52892c3..2d455ee244c 100644 --- a/src/renderer/src/store/slices/orca-profiles-auth-actions-connect-overlap.test.ts +++ b/src/renderer/src/store/slices/orca-profiles-auth-actions-connect-overlap.test.ts @@ -2,7 +2,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { ConnectCurrentOrcaProfileResult, OrcaProfileAuthStatus, - OrcaProfileListState + OrcaProfileListState, + SignOutCurrentOrcaProfileResult } from '../../../../shared/orca-profiles' import { createTestStore } from './store-test-helpers' @@ -53,7 +54,8 @@ const connectedAuthStatus: OrcaProfileAuthStatus = { } const orcaProfilesApi = { - connectCurrent: vi.fn() + connectCurrent: vi.fn(), + signOutCurrent: vi.fn() } describe('orca profile overlapping connect actions', () => { @@ -104,4 +106,57 @@ describe('orca profile overlapping connect actions', () => { expect(store.getState().orcaProfileAuthStatus).toEqual(laterAuthStatus) expect(store.getState().orcaProfiles).toEqual(laterConnected.profiles) }) + + it('ignores an in-flight later connect after sign-out', async () => { + const signedOutAuth: OrcaProfileAuthStatus = { + activeProfileId: 'local-default', + configured: true, + state: 'local', + persistence: 'none' + } + const signedOut: SignOutCurrentOrcaProfileResult = { + status: 'signed-out', + auth: signedOutAuth, + activeProfileId: 'local-default', + profiles: listState.profiles + } + const earlierConnected: ConnectCurrentOrcaProfileResult = { + status: 'connected', + auth: connectedAuthStatus, + activeProfileId: 'local-default', + profiles: [{ ...listState.profiles[0], kind: 'cloud-linked', cloud: connectedCloud }] + } + const laterConnected: ConnectCurrentOrcaProfileResult = { + status: 'connected', + auth: { + ...connectedAuthStatus, + cloud: { ...connectedCloud, userId: 'user-2', email: 'ada@example.com' } + }, + activeProfileId: 'local-default', + profiles: [ + { + ...listState.profiles[0], + kind: 'cloud-linked', + cloud: { ...connectedCloud, userId: 'user-2', email: 'ada@example.com' } + } + ] + } + let finishLater!: (value: ConnectCurrentOrcaProfileResult) => void + orcaProfilesApi.connectCurrent.mockResolvedValueOnce(earlierConnected).mockReturnValueOnce( + new Promise((resolve) => { + finishLater = resolve + }) + ) + orcaProfilesApi.signOutCurrent.mockResolvedValue(signedOut) + const store = createTestStore() + + const earlier = store.getState().connectCurrentOrcaProfile() + const later = store.getState().connectCurrentOrcaProfile() + await expect(earlier).resolves.toEqual(earlierConnected) + await expect(store.getState().signOutCurrentOrcaProfile()).resolves.toEqual(signedOut) + finishLater(laterConnected) + await expect(later).resolves.toEqual(laterConnected) + expect(store.getState().orcaProfileAuthStatus).toEqual(signedOutAuth) + expect(store.getState().orcaProfiles).toEqual(listState.profiles) + }) }) diff --git a/src/renderer/src/store/slices/orca-profiles-auth-actions.ts b/src/renderer/src/store/slices/orca-profiles-auth-actions.ts index 93218c87645..88e568d687c 100644 --- a/src/renderer/src/store/slices/orca-profiles-auth-actions.ts +++ b/src/renderer/src/store/slices/orca-profiles-auth-actions.ts @@ -180,6 +180,8 @@ export const createOrcaProfilesAuthActions: StateCreator< }, signOutCurrentOrcaProfile: async () => { + nextConnectAttempt += 1 + appliedConnectAttempt = nextConnectAttempt try { const result = await window.api.orcaProfiles.signOutCurrent() set({