Skip to content

Commit

Permalink
fix: pass signal to webAuthCheckLogin timer
Browse files Browse the repository at this point in the history
  • Loading branch information
lukekarrys committed May 2, 2024
1 parent d6f6ebe commit 53db633
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 83 deletions.
19 changes: 14 additions & 5 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,18 @@ const webAuth = async (opener, opts, body) => {

const webAuthOpener = async (opener, loginUrl, doneUrl, opts) => {
const abortController = new AbortController()
const { signal } = abortController
try {
log.verbose('web auth', 'opening url pair')
const [, authResult] = await Promise.all([
opener(loginUrl, { signal: abortController.signal }),
webAuthCheckLogin(doneUrl, { ...opts, cache: false }).then((r) => {
opener(loginUrl, { signal }).catch((err) => {
if (err.name === 'AbortError') {
abortController.abort()
return
}
throw err
}),
webAuthCheckLogin(doneUrl, { ...opts, cache: false }, { signal }).then((r) => {
log.verbose('web auth', 'done-check finished')
abortController.abort()
return r
Expand All @@ -96,7 +103,9 @@ const webAuthOpener = async (opener, loginUrl, doneUrl, opts) => {
}
}

const webAuthCheckLogin = async (doneUrl, opts) => {
const webAuthCheckLogin = async (doneUrl, opts, { signal } = {}) => {
signal?.throwIfAborted()

const res = await fetch(doneUrl, opts)
const content = await res.json()

Expand All @@ -110,9 +119,9 @@ const webAuthCheckLogin = async (doneUrl, opts) => {
if (res.status === 202) {
const retry = +res.headers.get('retry-after') * 1000
if (retry > 0) {
await timers.setTimeout(retry)
await timers.setTimeout(retry, null, { ref: false, signal })
}
return webAuthCheckLogin(doneUrl, opts)
return webAuthCheckLogin(doneUrl, opts, { signal })
}

throw new WebLoginInvalidResponse('GET', res, content)
Expand Down
75 changes: 0 additions & 75 deletions tap-snapshots/test-login.js-TAP.test.js

This file was deleted.

11 changes: 10 additions & 1 deletion tap-snapshots/test/login.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ exports[`test/login.js TAP cleanup > got expected requests 1`] = `
[
"POST /weblogin/-/v1/login",
"GET /weblogin/-/v1/login/blerg",
"GET /weblogin/-/v1/login/blerg",
"POST /weblogin/-/v1/login",
"GET /weblogin/-/v1/login/blerg",
"POST /weblogin/-/v1/login",
Expand Down Expand Up @@ -52,6 +53,14 @@ exports[`test/login.js TAP cleanup > got expected requests 1`] = `
"GET /retry-again/-/v1/login/blerg",
"POST /retry-again/-/v1/login",
"GET /retry-again/-/v1/login/blerg",
"GET /retry-again/-/v1/login/blerg"
"GET /retry-again/-/v1/login/blerg",
"POST /retry-long-time/-/v1/login",
"GET /retry-long-time/-/v1/login/loooooong",
"POST /retry-long-time/-/v1/login",
"GET /retry-long-time/-/v1/login/loooooong",
"POST /retry-long-time/-/v1/login",
"GET /retry-long-time/-/v1/login/loooooong",
"POST /retry-long-time/-/v1/login",
"GET /retry-long-time/-/v1/login/loooooong"
]
`
2 changes: 1 addition & 1 deletion test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ test('login fallback to couch when web login fails cancels opener promise', t =>
.reply(404, { error: 'Not found' })

let cancelled = false
const opener = (url, { signal }) => {
const opener = async (url, { signal }) => {
t.equal(url, loginUrl)
signal.addEventListener('abort', () => {
cancelled = true
Expand Down
46 changes: 45 additions & 1 deletion test/login.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const t = require('tap')
const profile = require('..')
const http = require('http')
const timers = require('timers/promises')
const PORT = +process.env.PORT || 13445

const reg = 'http://localhost:' + PORT
Expand Down Expand Up @@ -132,6 +133,17 @@ const server = http.createServer((q, s) => {
return respond(s, 200, { token: 'blerg' })
}

case '/retry-long-time/-/v1/login':
return respond(s, 200, {
loginUrl: 'http://www.example.com/loooooong',
doneUrl: reg + '/retry-long-time/-/v1/login/loooooong',
})

case '/retry-long-time/-/v1/login/loooooong': {
s.setHeader('retry-after', 60 * 1000)
return respond(s, 202, {})
}

case '/invalid-login/-/v1/login':
return respond(s, 200, { salt: 'im helping' })

Expand Down Expand Up @@ -165,7 +177,7 @@ t.test('start server', t => server.listen(PORT, t.end))

t.test('login web', t => {
let calledOpener = false
const opener = () => new Promise(resolve => {
const opener = async () => new Promise(resolve => {
calledOpener = true
resolve()
})
Expand All @@ -180,6 +192,12 @@ t.test('login web', t => {
})
})

t.test('webAuthCheckLogin', async t => {
await t.resolveMatch(profile.webAuthCheckLogin(reg + '/weblogin/-/v1/login/blerg', {
registry: reg + '/weblogin/',
}), { token: 'blerg' })
})

t.test('adduser web', t => {
let calledOpener = false
const opener = () => new Promise(resolve => {
Expand Down Expand Up @@ -517,6 +535,32 @@ t.test('no retry-after 202 response', t => {
t.end()
})

t.test('opener error with long 202 response', t => {
const registry = reg + '/retry-long-time/'

const err = new Error('opener error')
const throwOpener = async () => {
await timers.setTimeout(100)
throw err
}
const abortOpener = async () => {
await timers.setTimeout(100, null, { signal: AbortSignal.timeout(10) })
}
const prompter = () => {
throw new Error('should not do this')
}

t.test('loginWeb', async t => {
await t.rejects(profile.loginWeb(throwOpener, { registry }), err)
await t.rejects(profile.loginWeb(abortOpener, { registry }), { name: 'AbortError' })
})
t.test('login', async t => {
await t.rejects(profile.login(throwOpener, prompter, { registry }), err)
await t.rejects(profile.login(abortOpener, prompter, { registry }), { name: 'AbortError' })
})
t.end()
})

t.test('cleanup', t => {
// NOTE: snapshot paths are not platform-independent
process.platform !== 'win32' &&
Expand Down

0 comments on commit 53db633

Please sign in to comment.