Skip to content

Commit 72b22e8

Browse files
committed
fix: bind desktop login approval to the account and settle codes stash-aware
Approval is now recorded as the approving account's uid and re-verified after the dialog resolves, so an account change mid-dialog or between a transient failure and its retry can never redeem on a stale approval. Settlement clears the preserved-query stash only when it still holds the settling code, and the redemption runner drains the stash after each pass, so a newer code stashed mid-flight survives and is processed immediately.
1 parent 7966c9e commit 72b22e8

2 files changed

Lines changed: 179 additions & 31 deletions

File tree

src/platform/cloud/onboarding/desktopLoginRedemption.test.ts

Lines changed: 117 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ import { createMemoryHistory, createRouter } from 'vue-router'
88
* The code lives only in the preserved-query stash (the tracker strips it
99
* from the URL at capture time); redemption fires from router.afterEach and
1010
* an auth watcher, so every test drives a real in-memory router:
11-
* - Redemption requires explicit user approval, asked once per code
11+
* - Redemption requires explicit user approval, asked once per code and
12+
* account: an account change after approval (mid-dialog or before a retry)
13+
* must never redeem on the stale approval
1214
* - A valid code with an authenticated user is redeemed exactly once
1315
* - A session appearing without any navigation redeems via the auth watcher
1416
* - Terminal backend responses (400/403/404/409/410) drop the code with an
@@ -17,6 +19,8 @@ import { createMemoryHistory, createRouter } from 'vue-router'
1719
* retry; once the per-code attempt budget is spent the code is dropped
1820
* with an error toast
1921
* - Each distinct code gets its own approval and attempt budget
22+
* - A newer code stashed while an older one is in flight survives the older
23+
* code's settlement and is processed right after it
2024
* - Unauthenticated sessions keep the stash for a later trigger
2125
*
2226
* The fake clock (installed for every test) keeps the retry timers scheduled
@@ -535,6 +539,118 @@ describe('installDesktopLoginRedemption', () => {
535539
expect(stashedCode()).toBeUndefined()
536540
})
537541

542+
it('re-asks for approval when the account changes after approval and redeems with the new account token', async () => {
543+
const { trigger, seedStash, stashedCode } = await setup()
544+
seedStash(VALID_CODE)
545+
mockFetch
546+
.mockResolvedValueOnce(new Response(null, { status: 500 }))
547+
.mockResolvedValueOnce(okResponse())
548+
549+
// user-1 approves; the redeem fails transiently, keeping the code stashed.
550+
await trigger()
551+
expect(mockConfirm).toHaveBeenCalledTimes(1)
552+
expect(mockFetch).toHaveBeenCalledTimes(1)
553+
expect(stashedCode()).toBe(VALID_CODE)
554+
555+
// The session changes to user-2 before the retry: user-1's approval must
556+
// not authorize redeeming with user-2's token.
557+
mockAuthStore.currentUser = {
558+
uid: 'user-2',
559+
getIdToken: vi.fn().mockResolvedValue('second-user-token')
560+
}
561+
562+
await vi.waitFor(() => expect(mockConfirm).toHaveBeenCalledTimes(2))
563+
await vi.waitFor(() => expect(mockFetch).toHaveBeenCalledTimes(2))
564+
expect(mockFetch).toHaveBeenLastCalledWith(
565+
REDEEM_URL,
566+
expect.objectContaining({
567+
headers: expect.objectContaining({
568+
Authorization: 'Bearer second-user-token'
569+
})
570+
})
571+
)
572+
expect(stashedCode()).toBeUndefined()
573+
})
574+
575+
it('does not redeem when the session changes while the approval dialog is open', async () => {
576+
const { trigger, seedStash, stashedCode } = await setup()
577+
seedStash(VALID_CODE)
578+
let approve!: (value: boolean) => void
579+
mockConfirm.mockReturnValueOnce(
580+
new Promise<boolean>((resolve) => {
581+
approve = resolve
582+
})
583+
)
584+
mockFetch.mockResolvedValue(okResponse())
585+
586+
await trigger()
587+
await vi.waitFor(() => expect(mockConfirm).toHaveBeenCalledTimes(1))
588+
589+
// The session swaps to user-2 while user-1's dialog is still open; the
590+
// approval that resolves now is ambiguous and must not redeem.
591+
const secondUser = {
592+
uid: 'user-2',
593+
getIdToken: vi.fn().mockResolvedValue('second-user-token')
594+
}
595+
mockAuthStore.currentUser = secondUser
596+
await flushRedemption()
597+
approve(true)
598+
await flushRedemption()
599+
600+
expect(mockFetch).not.toHaveBeenCalled()
601+
expect(stashedCode()).toBe(VALID_CODE)
602+
603+
// The next trigger re-prompts under user-2 and redeems with its token.
604+
await trigger()
605+
606+
expect(mockConfirm).toHaveBeenCalledTimes(2)
607+
expect(mockFetch).toHaveBeenCalledTimes(1)
608+
expect(mockFetch).toHaveBeenCalledWith(
609+
REDEEM_URL,
610+
expect.objectContaining({
611+
headers: expect.objectContaining({
612+
Authorization: 'Bearer second-user-token'
613+
})
614+
})
615+
)
616+
expect(stashedCode()).toBeUndefined()
617+
})
618+
619+
it.for([
620+
['succeeds', () => okResponse()],
621+
['fails terminally', () => new Response(null, { status: 404 })]
622+
] as const)(
623+
'processes a newer code stashed mid-flight after the older redemption %s',
624+
async ([_label, firstResponse]) => {
625+
const { trigger, seedStash, stashedCode } = await setup()
626+
seedStash(VALID_CODE)
627+
let resolveFirstFetch!: (response: Response) => void
628+
mockFetch.mockReturnValueOnce(
629+
new Promise<Response>((resolve) => {
630+
resolveFirstFetch = resolve
631+
})
632+
)
633+
634+
await trigger()
635+
await vi.waitFor(() => expect(mockFetch).toHaveBeenCalledTimes(1))
636+
637+
// A second code arrives while the first redemption is still in flight;
638+
// settling the first must not clear it, and it must be processed
639+
// without waiting for another navigation.
640+
seedStash(SECOND_CODE)
641+
mockFetch.mockResolvedValue(okResponse())
642+
resolveFirstFetch(firstResponse())
643+
644+
await vi.waitFor(() => expect(mockFetch).toHaveBeenCalledTimes(2))
645+
expect(mockConfirm).toHaveBeenCalledTimes(2)
646+
expect(mockFetch).toHaveBeenLastCalledWith(
647+
REDEEM_URL,
648+
expect.objectContaining({ body: JSON.stringify({ code: SECOND_CODE }) })
649+
)
650+
expect(stashedCode()).toBeUndefined()
651+
}
652+
)
653+
538654
it('coalesces concurrent triggers into one dialog and one request', async () => {
539655
const { router, seedStash } = await setup()
540656
seedStash(VALID_CODE)

src/platform/cloud/onboarding/desktopLoginRedemption.ts

Lines changed: 62 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -36,33 +36,42 @@ const REDEEM_TIMEOUT_MS = 10_000
3636

3737
interface CodeRedemptionState {
3838
attempts: number
39-
approved: boolean
39+
approvedUserUid: string | null
4040
settled: boolean
4141
}
4242

4343
// Keyed by code so a different code arriving later gets its own approval and
4444
// attempt budget, while retries of the same code reuse both.
4545
const codeStates = new Map<string, CodeRedemptionState>()
4646

47-
// Coalesces concurrent triggers (afterEach can burst) into one redemption.
48-
let inFlight: Promise<void> | null = null
47+
// Coalesces concurrent triggers (afterEach can burst) into one drain.
48+
let draining = false
4949

5050
let authWatcherInstalled = false
5151

5252
function getCodeState(code: string): CodeRedemptionState {
5353
const existing = codeStates.get(code)
5454
if (existing) return existing
55-
const fresh = { attempts: 0, approved: false, settled: false }
55+
const fresh = { attempts: 0, approvedUserUid: null, settled: false }
5656
codeStates.set(code, fresh)
5757
return fresh
5858
}
5959

60-
function settle(state: CodeRedemptionState): void {
60+
// A newer code can be stashed while an older one is mid-redemption; settling
61+
// the older one must not wipe it.
62+
function clearStashIfHolds(code: string): void {
63+
if (getPreservedQueryParam(NAMESPACE, DESKTOP_LOGIN_CODE_KEY) === code) {
64+
clearPreservedQuery(NAMESPACE)
65+
}
66+
}
67+
68+
function settle(code: string, state: CodeRedemptionState): void {
6169
state.settled = true
62-
clearPreservedQuery(NAMESPACE)
70+
clearStashIfHolds(code)
6371
}
6472

6573
function handleTransientFailure(
74+
code: string,
6675
state: CodeRedemptionState,
6776
reason: string
6877
): void {
@@ -78,7 +87,7 @@ function handleTransientFailure(
7887
// The budget is spent on a sign-in the user explicitly approved: drop the
7988
// stash so reloads never retry a dead code, and tell the user instead of
8089
// failing silently.
81-
settle(state)
90+
settle(code, state)
8291
useToastStore().add({
8392
severity: 'error',
8493
summary: t('desktopLogin.failedSummary'),
@@ -89,23 +98,28 @@ function handleTransientFailure(
8998

9099
// A leaked link carrying a code must not silently bind the victim's session
91100
// to an attacker's desktop app: redemption requires explicit approval.
92-
// Approval is per code — retries of the same code reuse it, but a different
93-
// code arriving later must be approved on its own.
94-
async function confirmRedemption(state: CodeRedemptionState): Promise<boolean> {
95-
if (state.approved) return true
101+
// Approval is per code *and* account — retries of the same code under the
102+
// same account reuse it, but a different code, or the same code under a
103+
// different account, must be approved on its own.
104+
async function confirmRedemption(
105+
state: CodeRedemptionState,
106+
uid: string
107+
): Promise<boolean> {
108+
if (state.approvedUserUid === uid) return true
96109
const confirmed = await useDialogService().confirm({
97110
title: t('desktopLogin.confirmSummary'),
98111
message: t('desktopLogin.confirmMessage')
99112
})
100-
state.approved = confirmed === true
101-
return state.approved
113+
if (confirmed !== true) return false
114+
state.approvedUserUid = uid
115+
return true
102116
}
103117

104118
async function redeemCode(code: string): Promise<void> {
105119
const state = getCodeState(code)
106120
if (state.settled) {
107121
// A later navigation can re-capture an already-settled code; drop it.
108-
clearPreservedQuery(NAMESPACE)
122+
clearStashIfHolds(code)
109123
return
110124
}
111125

@@ -114,22 +128,29 @@ async function redeemCode(code: string): Promise<void> {
114128
const user = useAuthStore().currentUser
115129
if (!user) return
116130

117-
if (!(await confirmRedemption(state))) {
131+
if (!(await confirmRedemption(state, user.uid))) {
118132
// Declined/dismissed: drop the code without an error.
119-
settle(state)
133+
settle(code, state)
120134
return
121135
}
122136

137+
// The session can change while the dialog is open or between a transient
138+
// failure and its retry: approval binds the code to one account, so redeem
139+
// only while that exact account is still active. On mismatch the code stays
140+
// stashed and the next trigger re-prompts under the now-current account.
141+
const approvedUser = useAuthStore().currentUser
142+
if (!approvedUser || approvedUser.uid !== state.approvedUserUid) return
143+
123144
state.attempts++
124145

125146
// Fetch the token straight from the Firebase user: authStore.getIdToken()
126147
// surfaces failures through a modal error dialog, which this background
127148
// flow must not trigger.
128149
let idToken: string
129150
try {
130-
idToken = await user.getIdToken()
151+
idToken = await approvedUser.getIdToken()
131152
} catch {
132-
handleTransientFailure(state, 'could not get id token')
153+
handleTransientFailure(code, state, 'could not get id token')
133154
return
134155
}
135156

@@ -148,6 +169,7 @@ async function redeemCode(code: string): Promise<void> {
148169
})
149170
} catch (error) {
150171
handleTransientFailure(
172+
code,
151173
state,
152174
error instanceof Error && error.name === 'TimeoutError'
153175
? 'request timed out'
@@ -157,7 +179,7 @@ async function redeemCode(code: string): Promise<void> {
157179
}
158180

159181
if (response.ok) {
160-
settle(state)
182+
settle(code, state)
161183
useToastStore().add({
162184
severity: 'success',
163185
summary: t('desktopLogin.successSummary'),
@@ -168,7 +190,7 @@ async function redeemCode(code: string): Promise<void> {
168190
}
169191

170192
if (TERMINAL_REDEEM_STATUSES.has(response.status)) {
171-
settle(state)
193+
settle(code, state)
172194
useToastStore().add({
173195
severity: 'error',
174196
summary: t('desktopLogin.expiredSummary'),
@@ -178,25 +200,35 @@ async function redeemCode(code: string): Promise<void> {
178200
return
179201
}
180202

181-
handleTransientFailure(state, `status ${response.status}`)
203+
handleTransientFailure(code, state, `status ${response.status}`)
182204
}
183205

184206
async function redeemPendingDesktopLoginCode(): Promise<void> {
185207
// Never rejects: the triggers fire-and-forget this and must not be derailed
186208
// by a redemption failure.
209+
if (draining) return
210+
draining = true
187211
try {
188-
const code = getPreservedQueryParam(NAMESPACE, DESKTOP_LOGIN_CODE_KEY)
189-
if (!code) return
190-
if (!DESKTOP_LOGIN_CODE_PATTERN.test(code)) {
191-
clearPreservedQuery(NAMESPACE)
192-
return
212+
// A newer code can replace the stashed one while an older redemption is
213+
// mid-flight (its trigger lands here while draining and returns), so after
214+
// each pass process whatever the stash holds now. A pass that leaves its
215+
// own code stashed — a transient failure awaiting its retry timer, or an
216+
// account mismatch awaiting the next trigger — ends the drain.
217+
let processedCode: string | undefined
218+
for (;;) {
219+
const code = getPreservedQueryParam(NAMESPACE, DESKTOP_LOGIN_CODE_KEY)
220+
if (!code || code === processedCode) return
221+
if (!DESKTOP_LOGIN_CODE_PATTERN.test(code)) {
222+
clearPreservedQuery(NAMESPACE)
223+
return
224+
}
225+
processedCode = code
226+
await redeemCode(code)
193227
}
194-
inFlight ??= redeemCode(code).finally(() => {
195-
inFlight = null
196-
})
197-
await inFlight
198228
} catch (error) {
199229
console.error('[DesktopLoginRedemption] Redemption failed:', error)
230+
} finally {
231+
draining = false
200232
}
201233
}
202234

0 commit comments

Comments
 (0)