From 7c5afb7a413dea1a445c8bdf49441c9fb65e67ba Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Wed, 19 Jun 2024 10:51:51 +0200 Subject: [PATCH] test: add error logs and avoid consent-related flakyness --- .github/workflows/retry.yml | 2 +- tap/end2end-client-credentials.ts | 41 ++++++++-------------- tap/end2end-device-code.ts | 40 +++++++++++----------- tap/end2end.ts | 57 +++++++++++-------------------- tap/helper.ts | 28 +++++++++++++++ tap/server.mjs | 52 +++++++++++++++++++++++----- 6 files changed, 126 insertions(+), 94 deletions(-) diff --git a/.github/workflows/retry.yml b/.github/workflows/retry.yml index 07935bce..644b9e3d 100644 --- a/.github/workflows/retry.yml +++ b/.github/workflows/retry.yml @@ -11,7 +11,7 @@ on: jobs: retry: runs-on: ubuntu-latest - if: ${{ github.event.workflow_run.conclusion == 'failure' && github.event.workflow_run.run_attempt <= 2 }} + if: ${{ github.event.workflow_run.conclusion == 'failure' && github.event.workflow_run.run_attempt == 1 }} steps: - run: gh api -XPOST ${{ github.event.workflow_run.rerun_url }}-failed-jobs env: diff --git a/tap/end2end-client-credentials.ts b/tap/end2end-client-credentials.ts index 8f8b7f84..e306794f 100644 --- a/tap/end2end-client-credentials.ts +++ b/tap/end2end-client-credentials.ts @@ -1,7 +1,13 @@ import type QUnit from 'qunit' import * as jose from 'jose' -import { isDpopNonceError, setup } from './helper.js' +import { + assertNoWwwAuthenticateChallenges, + isDpopNonceError, + assertNotOAuth2Error, + setup, + unexpectedAuthorizationServerError, +} from './helper.js' import * as lib from '../src/index.js' import { keys } from './keys.js' @@ -98,10 +104,7 @@ export default (QUnit: QUnit) => { }) let response = await clientCredentialsGrantRequest() - if (lib.parseWwwAuthenticateChallenges(response)) { - t.ok(0) - throw new Error() - } + assertNoWwwAuthenticateChallenges(response) const processClientCredentialsResponse = () => lib.processClientCredentialsResponse(as, client, response) @@ -110,13 +113,9 @@ export default (QUnit: QUnit) => { if (isDpopNonceError(result)) { response = await clientCredentialsGrantRequest() result = await processClientCredentialsResponse() - if (lib.isOAuth2Error(result)) { - t.ok(0) - throw new Error() - } + if (!assertNotOAuth2Error(result)) return } else { - t.ok(0) - throw new Error() + throw unexpectedAuthorizationServerError(result) } } @@ -133,17 +132,11 @@ export default (QUnit: QUnit) => { t.ok(JSON.parse(clone)) } - if (lib.parseWwwAuthenticateChallenges(response)) { - t.ok(0) - throw new Error() - } + assertNoWwwAuthenticateChallenges(response) const result = await lib.processIntrospectionResponse(as, client, response) - if (lib.isOAuth2Error(result)) { - t.ok(0) - throw new Error() - } + if (!assertNotOAuth2Error(result)) return t.propContains(result, { active: true, @@ -156,16 +149,10 @@ export default (QUnit: QUnit) => { { let response = await lib.revocationRequest(as, client, access_token, authenticated) - if (lib.parseWwwAuthenticateChallenges(response)) { - t.ok(0) - throw new Error() - } + assertNoWwwAuthenticateChallenges(response) const result = await lib.processRevocationResponse(response) - if (lib.isOAuth2Error(result)) { - t.ok(0) - throw new Error() - } + if (!assertNotOAuth2Error(result)) return } } diff --git a/tap/end2end-device-code.ts b/tap/end2end-device-code.ts index a06921b2..6961f6cf 100644 --- a/tap/end2end-device-code.ts +++ b/tap/end2end-device-code.ts @@ -1,6 +1,12 @@ import type QUnit from 'qunit' -import { isDpopNonceError, setup } from './helper.js' +import { + assertNotOAuth2Error, + assertNoWwwAuthenticateChallenges, + isDpopNonceError, + setup, + unexpectedAuthorizationServerError, +} from './helper.js' import * as lib from '../src/index.js' import { keys } from './keys.js' import * as jose from 'jose' @@ -44,16 +50,10 @@ export default (QUnit: QUnit) => { params.set('scope', 'api:write') let response = await lib.deviceAuthorizationRequest(as, client, params) - if (lib.parseWwwAuthenticateChallenges(response)) { - t.ok(0) - throw new Error() - } + assertNoWwwAuthenticateChallenges(response) let result = await lib.processDeviceAuthorizationResponse(as, client, response) - if (lib.isOAuth2Error(result)) { - t.ok(0) - throw new Error() - } + if (!assertNotOAuth2Error(result)) return const { verification_uri_complete, device_code } = result await fetch('http://localhost:3000/drive', { @@ -68,24 +68,26 @@ export default (QUnit: QUnit) => { lib.deviceCodeGrantRequest(as, client, device_code, { DPoP }) let response = await deviceCodeGrantRequest() - if (lib.parseWwwAuthenticateChallenges(response)) { - t.ok(0) - throw new Error() - } + assertNoWwwAuthenticateChallenges(response) const processDeviceCodeResponse = () => lib.processDeviceCodeResponse(as, client, response) let result = await processDeviceCodeResponse() + let i = 0 + while (lib.isOAuth2Error(result) && result.error === 'authorization_pending') { + await new Promise((resolve) => setTimeout(resolve, 100)) + response = await deviceCodeGrantRequest() + result = await processDeviceCodeResponse() + i++ + if (i === 5) break + } + if (lib.isOAuth2Error(result)) { if (isDpopNonceError(result)) { response = await deviceCodeGrantRequest() result = await processDeviceCodeResponse() - if (lib.isOAuth2Error(result)) { - t.ok(0) - throw new Error() - } + if (!assertNotOAuth2Error(result)) return } else { - t.ok(0) - throw new Error() + throw unexpectedAuthorizationServerError(result) } } diff --git a/tap/end2end.ts b/tap/end2end.ts index 0bcc2838..6e64418d 100644 --- a/tap/end2end.ts +++ b/tap/end2end.ts @@ -1,7 +1,13 @@ import type QUnit from 'qunit' import * as jose from 'jose' -import { isDpopNonceError, setup } from './helper.js' +import { + assertNoWwwAuthenticateChallenges, + isDpopNonceError, + assertNotOAuth2Error, + setup, + unexpectedAuthorizationServerError, +} from './helper.js' import * as lib from '../src/index.js' import { keys } from './keys.js' @@ -127,10 +133,7 @@ export default (QUnit: QUnit) => { const pushedAuthorizationRequest = () => lib.pushedAuthorizationRequest(as, client, params, { DPoP }) let response = await pushedAuthorizationRequest() - if (lib.parseWwwAuthenticateChallenges(response)) { - t.ok(0) - throw new Error() - } + assertNoWwwAuthenticateChallenges(response) const processPushedAuthorizationResponse = () => lib.processPushedAuthorizationResponse(as, client, response) @@ -138,18 +141,11 @@ export default (QUnit: QUnit) => { if (lib.isOAuth2Error(result)) { if (isDpopNonceError(result)) { response = await pushedAuthorizationRequest() - if (lib.parseWwwAuthenticateChallenges(response)) { - t.ok(0) - throw new Error() - } + assertNoWwwAuthenticateChallenges(response) result = await processPushedAuthorizationResponse() - if (lib.isOAuth2Error(result)) { - t.ok(0) - throw new Error() - } + if (!assertNotOAuth2Error(result)) return } else { - t.ok(0) - throw new Error() + throw unexpectedAuthorizationServerError(result) } } for (const param of [...params.keys()]) { @@ -183,10 +179,7 @@ export default (QUnit: QUnit) => { currentUrl, lib.expectNoState, ) - if (lib.isOAuth2Error(callbackParams)) { - t.ok(0) - throw new Error() - } + if (!assertNotOAuth2Error(callbackParams)) return { const authorizationCodeGrantRequest = () => @@ -200,10 +193,7 @@ export default (QUnit: QUnit) => { ) let response = await authorizationCodeGrantRequest() - if (lib.parseWwwAuthenticateChallenges(response)) { - t.ok(0) - throw new Error() - } + assertNoWwwAuthenticateChallenges(response) const processAuthorizationCodeOpenIDResponse = () => lib.processAuthorizationCodeOpenIDResponse(as, client, response) @@ -212,20 +202,16 @@ export default (QUnit: QUnit) => { if (isDpopNonceError(result)) { response = await authorizationCodeGrantRequest() result = await processAuthorizationCodeOpenIDResponse() - if (lib.isOAuth2Error(result)) { - t.ok(0) - throw new Error() - } + if (!assertNotOAuth2Error(result)) return } else { - t.ok(0) - throw new Error() + throw unexpectedAuthorizationServerError(result) } } const { access_token, refresh_token, token_type } = result t.equal(token_type, dpop ? 'dpop' : 'bearer') if (!refresh_token) { - t.ok(0) + t.ok(0, 'expected a refresh token to be returned') throw new Error() } const { sub } = lib.getValidatedIdTokenClaims(result) @@ -239,8 +225,7 @@ export default (QUnit: QUnit) => { if (isDpopNonceError(challenges)) { response = await userInfoRequest() } else { - t.ok(0) - throw new Error() + throw unexpectedAuthorizationServerError(challenges) } } @@ -264,16 +249,12 @@ export default (QUnit: QUnit) => { if (isDpopNonceError(challenges)) { response = await refreshTokenGrantRequest() } else { - t.ok(0) - throw new Error() + throw unexpectedAuthorizationServerError(challenges) } } const result = await lib.processRefreshTokenResponse(as, client, response) - if (lib.isOAuth2Error(result)) { - t.ok(0) - throw new Error() - } + if (!assertNotOAuth2Error(result)) return } } diff --git a/tap/helper.ts b/tap/helper.ts index 354f956b..946e1268 100644 --- a/tap/helper.ts +++ b/tap/helper.ts @@ -16,6 +16,34 @@ export function isDpopNonceError(input: lib.OAuth2Error | lib.WWWAuthenticateCha } } +export function unexpectedAuthorizationServerError( + input: lib.WWWAuthenticateChallenge[] | lib.OAuth2Error, +) { + let msg: string + if ('error' in input) { + msg = `${input.error}: ${input.error_description}` + } else { + msg = `${input[0].parameters.error}: ${input[0].parameters.error_description}` + } + return new Error(msg, { cause: input }) +} + +export function assertNotOAuth2Error( + input: Parameters[0], +): input is Exclude[0], lib.OAuth2Error> { + if (lib.isOAuth2Error(input)) { + throw unexpectedAuthorizationServerError(input) + } + return true +} + +export function assertNoWwwAuthenticateChallenges(response: Response) { + let challenges: lib.WWWAuthenticateChallenge[] | undefined + if ((challenges = lib.parseWwwAuthenticateChallenges(response))) { + throw unexpectedAuthorizationServerError(challenges) + } +} + export async function setup( alg: lib.JWSAlgorithm, kp: CryptoKeyPair, diff --git a/tap/server.mjs b/tap/server.mjs index 8fbf09eb..6aa5041b 100644 --- a/tap/server.mjs +++ b/tap/server.mjs @@ -85,6 +85,10 @@ provider.use(async (ctx, next) => { encoding: ctx.charset, }) const params = new URLSearchParams(body.toString()) + const target = params.get('goto') + + console.log('\n\n=====') + console.log('starting user interaction on', target) browser = await puppeteer.launch({ executablePath: getChromePath(), @@ -95,21 +99,48 @@ provider.use(async (ctx, next) => { const page = await browser.newPage() await Promise.all([ - page.goto(params.get('goto')), + page.goto(target), page.waitForSelector(s), - page.waitForNetworkIdle(), + page.waitForNetworkIdle({ idleTime: 100 }), ]) - if ((await page.title()) === 'Device Login Confirmation') { - await Promise.all([page.click(s), page.waitForSelector(s), page.waitForNetworkIdle()]) + let pending = true + while (pending) { + let title + try { + title = await page.title() + } catch { + continue + } + switch (title) { + case 'Device Login Confirmation': + await Promise.all([ + page.click(s), + page.waitForFunction('document.title === "Sign-in"'), + page.waitForNetworkIdle({ idleTime: 100 }), + ]) + break + case 'Sign-in': + await page.type('[name="login"]', 'user') + await page.type('[name="password"]', 'pass') + await Promise.all([ + page.click(s), + page.waitForFunction('document.title === "Consent"'), + page.waitForNetworkIdle({ idleTime: 100 }), + ]) + break + case 'Consent': + await Promise.all([page.click(s), page.waitForNetworkIdle({ idleTime: 100 })]) + pending = false + break + default: + throw new Error(title) + } } - await page.type('[name="login"]', 'user') - await page.type('[name="password"]', 'pass') - await Promise.all([page.click(s), page.waitForSelector(s), page.waitForNetworkIdle()]) - await Promise.all([page.click(s), page.waitForNetworkIdle(s)]) - ctx.body = page.url() + console.log('done on title:', await page.title(), 'url:', ctx.body) + console.log('=====\n\n') } finally { await browser?.close() } @@ -117,6 +148,9 @@ provider.use(async (ctx, next) => { ctx.body = ctx.URL.href } else { await next() + if (typeof ctx.body === 'string' && ctx.body.includes('Continue')) { + ctx.body = ctx.body.replace('Sign-in', 'Consent') + } } })