@@ -38,6 +38,7 @@ const exchangeWebFlowCodeMock = exchangeWebFlowCode as unknown as MockedFunction
3838describe ( 'renderer/utils/forges/github/flows.ts' , ( ) => {
3939 vi . spyOn ( logger , 'rendererLogInfo' ) . mockImplementation ( vi . fn ( ) ) ;
4040 const rendererLogErrorSpy = vi . spyOn ( logger , 'rendererLogError' ) . mockImplementation ( vi . fn ( ) ) ;
41+ const rendererLogWarnSpy = vi . spyOn ( logger , 'rendererLogWarn' ) . mockImplementation ( vi . fn ( ) ) ;
4142 const openExternalLinkSpy = vi . spyOn ( comms , 'openExternalLink' ) . mockImplementation ( vi . fn ( ) ) ;
4243
4344 beforeEach ( ( ) => {
@@ -150,11 +151,12 @@ describe('renderer/utils/forges/github/flows.ts', () => {
150151 } ;
151152
152153 it ( 'should call performGitHubWebOAuth using custom oauth app - success oauth flow' , async ( ) => {
154+ let deliverCallback : ( callbackUrl : string ) => void = ( ) => { } ;
153155 window . gitify . onAuthCallback = vi . fn ( ) . mockImplementation ( ( callback ) => {
154- callback ( 'gitify://oauth?code=123-456' ) ;
156+ deliverCallback = callback ;
155157 } ) ;
156158
157- const res = await flows . performGitHubWebOAuth ( {
159+ const resultPromise = flows . performGitHubWebOAuth ( {
158160 clientId : 'BYO_CLIENT_ID' as ClientID ,
159161 clientSecret : 'BYO_CLIENT_SECRET' as ClientSecret ,
160162 hostname : 'my.git.com' as Hostname ,
@@ -167,21 +169,106 @@ describe('renderer/utils/forges/github/flows.ts', () => {
167169 ) ,
168170 ) ;
169171
172+ // The authorization URL must carry a per-attempt state, and the login
173+ // only completes when the callback echoes that same value back.
174+ const authorizeUrl = new URL ( openExternalLinkSpy . mock . calls [ 0 ] [ 0 ] as string ) ;
175+ const state = authorizeUrl . searchParams . get ( 'state' ) ;
176+ expect ( state ) . toBeTruthy ( ) ;
177+
178+ deliverCallback ( `gitify://oauth?code=123-456&state=${ state } ` ) ;
179+
180+ const res = await resultPromise ;
181+
170182 expect ( window . gitify . onAuthCallback ) . toHaveBeenCalledTimes ( 1 ) ;
171183 expect ( window . gitify . onAuthCallback ) . toHaveBeenCalledWith ( expect . any ( Function ) ) ;
172184
173185 expect ( res . authMethod ) . toBe ( 'OAuth App' ) ;
174186 expect ( res . authCode ) . toBe ( '123-456' ) ;
175187 } ) ;
176188
189+ it ( 'does not resolve when the callback state does not match the generated state' , async ( ) => {
190+ let deliverCallback : ( callbackUrl : string ) => void = ( ) => { } ;
191+ window . gitify . onAuthCallback = vi . fn ( ) . mockImplementation ( ( callback ) => {
192+ deliverCallback = callback ;
193+ } ) ;
194+
195+ let resolved = false ;
196+ const resultPromise = flows . performGitHubWebOAuth ( webAuthOptions ) . then ( ( res ) => {
197+ resolved = true ;
198+ return res ;
199+ } ) ;
200+
201+ const authorizeUrl = new URL ( openExternalLinkSpy . mock . calls [ 0 ] [ 0 ] as string ) ;
202+ const state = authorizeUrl . searchParams . get ( 'state' ) ;
203+
204+ deliverCallback ( 'gitify://oauth?code=attacker-code&state=not-the-real-state' ) ;
205+ deliverCallback ( 'gitify://oauth?code=attacker-code-no-state' ) ;
206+
207+ // Give any pending microtasks a chance to run, then confirm neither
208+ // injected callback settled the login.
209+ await Promise . resolve ( ) ;
210+ await Promise . resolve ( ) ;
211+ expect ( resolved ) . toBe ( false ) ;
212+ expect ( rendererLogWarnSpy ) . toHaveBeenCalledTimes ( 2 ) ;
213+
214+ // The still-pending attempt completes once the matching state arrives.
215+ deliverCallback ( `gitify://oauth?code=the-real-code&state=${ state } ` ) ;
216+ const res = await resultPromise ;
217+ expect ( res . authCode ) . toBe ( 'the-real-code' ) ;
218+ } ) ;
219+
220+ it ( 'ignores an unparseable callback URL instead of tearing down the pending login' , async ( ) => {
221+ let deliverCallback : ( callbackUrl : string ) => void = ( ) => { } ;
222+ window . gitify . onAuthCallback = vi . fn ( ) . mockImplementation ( ( callback ) => {
223+ deliverCallback = callback ;
224+ } ) ;
225+
226+ let settled = false ;
227+ const resultPromise = flows . performGitHubWebOAuth ( webAuthOptions ) . then (
228+ ( res ) => {
229+ settled = true ;
230+ return res ;
231+ } ,
232+ ( err : unknown ) => {
233+ settled = true ;
234+ throw err ;
235+ } ,
236+ ) ;
237+
238+ // `gitify://[` passes the protocol handler's `gitify://` prefix check but
239+ // is not a parseable URL, so it must be ignored rather than rejecting the
240+ // login before the state is ever compared.
241+ deliverCallback ( 'gitify://[' ) ;
242+
243+ await Promise . resolve ( ) ;
244+ await Promise . resolve ( ) ;
245+ expect ( settled ) . toBe ( false ) ;
246+ expect ( rendererLogWarnSpy ) . toHaveBeenCalledTimes ( 1 ) ;
247+
248+ // Settle the still-pending attempt so no listener is left dangling.
249+ const authorizeUrl = new URL ( openExternalLinkSpy . mock . calls [ 0 ] [ 0 ] as string ) ;
250+ const state = authorizeUrl . searchParams . get ( 'state' ) ;
251+ deliverCallback ( `gitify://oauth?code=real-code&state=${ state } ` ) ;
252+
253+ await expect ( resultPromise ) . resolves . toMatchObject ( { authCode : 'real-code' } ) ;
254+ } ) ;
255+
177256 it ( 'should call performGitHubWebOAuth - failure' , async ( ) => {
257+ let deliverCallback : ( callbackUrl : string ) => void = ( ) => { } ;
178258 window . gitify . onAuthCallback = vi . fn ( ) . mockImplementation ( ( callback ) => {
179- callback (
180- 'gitify://auth?error=invalid_request&error_description=The+redirect_uri+is+missing+or+invalid.&error_uri=https://docs.github.com/en/developers/apps/troubleshooting-oauth-errors' ,
181- ) ;
259+ deliverCallback = callback ;
182260 } ) ;
183261
184- await expect ( async ( ) => await flows . performGitHubWebOAuth ( webAuthOptions ) ) . rejects . toEqual (
262+ const resultPromise = flows . performGitHubWebOAuth ( webAuthOptions ) ;
263+
264+ const authorizeUrl = new URL ( openExternalLinkSpy . mock . calls [ 0 ] [ 0 ] as string ) ;
265+ const state = authorizeUrl . searchParams . get ( 'state' ) ;
266+
267+ deliverCallback (
268+ `gitify://oauth?state=${ state } &error=invalid_request&error_description=The+redirect_uri+is+missing+or+invalid.&error_uri=https://docs.github.com/en/developers/apps/troubleshooting-oauth-errors` ,
269+ ) ;
270+
271+ await expect ( resultPromise ) . rejects . toEqual (
185272 new Error (
186273 "Oops! Something went wrong and we couldn't log you in using GitHub. Please try again. Reason: The redirect_uri is missing or invalid. Docs: https://docs.github.com/en/developers/apps/troubleshooting-oauth-errors" ,
187274 ) ,
@@ -198,6 +285,40 @@ describe('renderer/utils/forges/github/flows.ts', () => {
198285 expect ( window . gitify . onAuthCallback ) . toHaveBeenCalledWith ( expect . any ( Function ) ) ;
199286 } ) ;
200287
288+ it ( 'ignores a callback error whose state does not match the attempt' , async ( ) => {
289+ let deliverCallback : ( callbackUrl : string ) => void = ( ) => { } ;
290+ window . gitify . onAuthCallback = vi . fn ( ) . mockImplementation ( ( callback ) => {
291+ deliverCallback = callback ;
292+ } ) ;
293+
294+ let settled = false ;
295+ const resultPromise = flows . performGitHubWebOAuth ( webAuthOptions ) . then (
296+ ( res ) => {
297+ settled = true ;
298+ return res ;
299+ } ,
300+ ( err : unknown ) => {
301+ settled = true ;
302+ throw err ;
303+ } ,
304+ ) ;
305+
306+ deliverCallback ( 'gitify://oauth?error=access_denied' ) ;
307+ deliverCallback ( 'gitify://oauth?error=access_denied&state=not-the-real-state' ) ;
308+
309+ await Promise . resolve ( ) ;
310+ await Promise . resolve ( ) ;
311+ expect ( settled ) . toBe ( false ) ;
312+ expect ( rendererLogWarnSpy ) . toHaveBeenCalledTimes ( 2 ) ;
313+
314+ // Settle the still-pending attempt so no listener is left dangling.
315+ const authorizeUrl = new URL ( openExternalLinkSpy . mock . calls [ 0 ] [ 0 ] as string ) ;
316+ const state = authorizeUrl . searchParams . get ( 'state' ) ;
317+ deliverCallback ( `gitify://oauth?code=real-code&state=${ state } ` ) ;
318+
319+ await expect ( resultPromise ) . resolves . toMatchObject ( { authCode : 'real-code' } ) ;
320+ } ) ;
321+
201322 describe ( 'exchangeAuthCodeForAccessToken' , ( ) => {
202323 const authCode = '123-456' as AuthCode ;
203324
0 commit comments