Skip to content

test: Replace dummy auth with virtual authenticator in e2e#3688

Merged
aterga merged 34 commits intomainfrom
sea-snake/replace-dummy-auth-with-virtual-authenticator
Mar 23, 2026
Merged

test: Replace dummy auth with virtual authenticator in e2e#3688
aterga merged 34 commits intomainfrom
sea-snake/replace-dummy-auth-with-virtual-authenticator

Conversation

@sea-snake
Copy link
Copy Markdown
Contributor

@sea-snake sea-snake commented Mar 17, 2026

Replace dummy auth with virtual authenticator in e2e, this change guarantees that passkey authentication will also work on production when pipelines pass.

Changes

  • Replace dummyAuth with addVirtualAuthenticator in all e2e tests.
  • Rewrite identity fixture to use virtual authenticator instead of dummy auth.
  • Add automatic created credential tracking to identity fixture so that credentials are captured immediately after their creation, enabling e2e flows where an identity has multiple passkeys.
  • Move actor creation out of identity fixture into utils.
  • Remove animated transition from AuthPanel since this animation was barely visible at the cost of making the tests flaky. (This vertical animation was shown when identities are switched, since that now automatically authorizes, the animation is redundant).
  • Remove animated transition from authorize layout since this animation was barely visible at the cost of making the tests flaky. (This horizontal animation was shown when moving between access method (new user) and continue screen).
  • Fix managePage.assertVisible() method, make it assert more explicitly that you're on the homepage.
  • Delete migration.spec.ts, was already replaced by upgrade.spec.ts but apparently wasn't removed yet (all tests were already marked as skipped).

Copilot AI review requested due to automatic review settings March 18, 2026 11:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Playwright E2E suite to stop relying on “dummy auth” prompts and instead use WebAuthn virtual authenticators + credential syncing, aligning the tests with passkey-based flows and simplifying canister init configuration used in CI.

Changes:

  • Added a removeVirtualAuthenticator utility and updated tests to consistently use virtual authenticators for passkey flows.
  • Refactored identity/test fixtures to track real WebAuthn credentials and keep them in sync when new passkeys are created during tests.
  • Updated CI canister install arguments to remove dummy_auth init config.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/frontend/tests/e2e-playwright/utils.ts Adds removeVirtualAuthenticator helper for CDP WebAuthn cleanup.
src/frontend/tests/e2e-playwright/routes/upgrade.spec.ts Migrates upgrade E2E to virtual authenticator credentials and adds cleanup.
src/frontend/tests/e2e-playwright/routes/manage/index.spec.ts Refactors dashboard navigation tests to use shared fixtures + authenticator cleanup.
src/frontend/tests/e2e-playwright/routes/manage/access.spec.ts Updates access-method tests to add passkeys via real WebAuthn flows and legacy key creation via actor.
src/frontend/tests/e2e-playwright/routes/index.spec.ts Migrates “first visit” and “last used identities” flows away from dummy auth to virtual authenticators.
src/frontend/tests/e2e-playwright/routes/authorize/postMessages.spec.ts Uses fixture identities + passkey sign-in for postMessage authorization test.
src/frontend/tests/e2e-playwright/routes/authorize/migration.spec.ts Removes obsolete skipped legacy migration test.
src/frontend/tests/e2e-playwright/routes/authorize/legacy.spec.ts Updates legacy-domain authorize flow to create identities via passkeys.
src/frontend/tests/e2e-playwright/routes/authorize/index.spec.ts Migrates authorize flows (existing/new passkey + “another device”) to fixtures + authenticators.
src/frontend/tests/e2e-playwright/routes/authorize/delegationTtl.spec.ts Switches delegation TTL tests from dummy auth prompts to virtual authenticators.
src/frontend/tests/e2e-playwright/routes/authorize/continue.spec.ts Refactors continue-flow tests to use shared identity fixtures and IdentityWizard.
src/frontend/tests/e2e-playwright/routes/authorize/alternativeOrigins.spec.ts Migrates alternative-origins tests to passkey authenticators + fixture identities.
src/frontend/tests/e2e-playwright/routes/authorize/account.spec.ts Updates multi-account authorization tests to use fixture identities + passkey sign-in.
src/frontend/tests/e2e-playwright/fixtures/managePage.ts Tightens dashboard URL assertion to /manage only.
src/frontend/tests/e2e-playwright/fixtures/manageAccessPage.ts Removes dummy-auth callback requirement for passkey-add dialog helper.
src/frontend/tests/e2e-playwright/fixtures/identity.ts Introduces credential tracking + navigator.credentials.create hook to sync new passkeys into fixture state.
.github/workflows/canister-tests.yml Removes dummy_auth from canister install arguments in CI.
Comments suppressed due to low confidence (1)

src/frontend/tests/e2e-playwright/routes/manage/access.spec.ts:45

  • Mutating identities[0].credentials here does not update the already-installed virtual authenticator on page, and signInWithIdentity doesn't consult identity.credentials when authenticating. As a result, this re-login may still use the new passkey, so the test doesn't actually verify that the original passkey still works. To make this assertion meaningful, recreate/replace the virtual authenticator (e.g., remove the current authenticator and call addAuthenticatorForIdentity after setting identity.credentials to the single desired credential).
    // Verify we can still sign in with the existing passkey
    identities[0].credentials = [existingCredential];
    await managePage.signOut();
    await manageAccessPage.goto();
    await signInWithIdentity(page, identities[0].identityNumber);
    await manageAccessPage.assertVisible();

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/frontend/tests/e2e-playwright/routes/authorize/legacy.spec.ts Outdated
Comment thread src/frontend/tests/e2e-playwright/routes/manage/access.spec.ts
Comment thread src/frontend/tests/e2e-playwright/routes/index.spec.ts Outdated
Comment thread src/frontend/tests/e2e-playwright/routes/authorize/index.spec.ts Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Playwright e2e suite to use Chrome’s WebAuthn virtual authenticator instead of dummyAuth, aligning test authentication with production passkey behavior and reducing UI-driven flakiness by removing barely-visible route/identity-switch animations.

Changes:

  • Replace dummyAuth usage with WebAuthn virtual authenticator helpers across e2e tests and fixtures.
  • Rewrite the identity fixture to track and manage multiple credentials per identity (including automatic capture after passkey creation).
  • Reduce test flakiness by removing UI transitions and tightening managePage.assertVisible() to assert the homepage more explicitly.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/frontend/tests/e2e-playwright/utils.ts Adds virtual authenticator management helpers and a WebAuthn-backed CredentialIdentity plus actor creation via credential.
src/frontend/tests/e2e-playwright/routes/upgrade.spec.ts Migrates upgrade flow test to virtual authenticator + actor authenticated by a real credential.
src/frontend/tests/e2e-playwright/routes/recovery.spec.ts Switches actor usage to credential-backed actor creation.
src/frontend/tests/e2e-playwright/routes/manage/recovery.spec.ts Switches actor usage to credential-backed actor creation.
src/frontend/tests/e2e-playwright/routes/manage/index.spec.ts Refactors dashboard/navigation tests to use fixture identities and virtual authenticator helpers.
src/frontend/tests/e2e-playwright/routes/manage/access.spec.ts Updates access-method tests for multiple passkeys; adds actor-based bulk passkey creation to avoid rate limits.
src/frontend/tests/e2e-playwright/routes/index.spec.ts Refactors first-visit flows to virtual authenticator + identity fixtures; adds context cleanup.
src/frontend/tests/e2e-playwright/routes/authorize/postMessages.spec.ts Uses fixture identity sign-in instead of dummy auth.
src/frontend/tests/e2e-playwright/routes/authorize/migration.spec.ts Deletes obsolete skipped migration test file.
src/frontend/tests/e2e-playwright/routes/authorize/legacy.spec.ts Ensures authenticator is added on the correct popup page.
src/frontend/tests/e2e-playwright/routes/authorize/index.spec.ts Refactors authorize flows (including “other device”) to use fixtures + virtual authenticators with cleanup.
src/frontend/tests/e2e-playwright/routes/authorize/delegationTtl.spec.ts Uses virtual authenticator for delegation TTL tests.
src/frontend/tests/e2e-playwright/routes/authorize/continue.spec.ts Refactors continue-flow tests around fixture identities and removes dummy auth usage.
src/frontend/tests/e2e-playwright/routes/authorize/alternativeOrigins.spec.ts Refactors origin-compat tests to sign in via fixtures/virtual authenticator.
src/frontend/tests/e2e-playwright/routes/authorize/account.spec.ts Refactors multi-account authorize tests to use fixture identities + authenticators.
src/frontend/tests/e2e-playwright/fixtures/managePage.ts Makes assertVisible() explicitly assert the /manage homepage URL.
src/frontend/tests/e2e-playwright/fixtures/manageAccessPage.ts Removes dummy-auth plumbing from the “add passkey” dialog helper.
src/frontend/tests/e2e-playwright/fixtures/identity.ts Replaces dummy-auth-based identity creation with credential-backed identities and credential tracking/sync.
src/frontend/src/routes/(new-styling)/(resuming-channel)/resume-openid-authorize/+page.svelte Updates condition for installing attributes listener (dummy auth → fetch-root-key flag).
src/frontend/src/routes/(new-styling)/(channel)/authorize/(panel)/+layout.svelte Removes identity-switch animation wrapper to reduce test flakiness.
src/frontend/src/lib/components/layout/AuthPanel.svelte Removes navigation fly-transition to reduce test flakiness.
.github/workflows/canister-tests.yml Updates canister install args to enable fetch_root_key instead of dummy auth.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/frontend/tests/e2e-playwright/fixtures/identity.ts Outdated
Comment thread src/frontend/tests/e2e-playwright/routes/manage/access.spec.ts
Comment thread src/frontend/tests/e2e-playwright/routes/manage/access.spec.ts
Comment thread .github/workflows/canister-tests.yml Outdated
@sea-snake sea-snake requested a review from aterga March 23, 2026 14:58
Copy link
Copy Markdown
Collaborator

@aterga aterga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but some more feedback from SuperGrok (may have false positives)

  • Credential mutation — potential bug? In access.spec.ts (and possibly elsewhere) there’s identities[0].credentials.push(...).
    This only mutates the JS array — it does not update the Chrome Virtual Authenticator.
    Suggestion: add a small helper (syncCredentialsToAuthenticator or recreate the authenticator) so the test stays in sync with real WebAuthn behaviour.
  • Authenticator cleanup Make sure removeVirtualAuthenticator is called in afterEach / afterAll so no leftover state leaks between tests (especially important in parallel CI runs). Add a comment if it’s already there.
  • Minor polish
    • Add 1–2 lines of comments explaining the new credential-tracking logic in identity.ts
    • The void on the listener in resume-openid-authorize is fine, but consider adding a fire-and-forget comment or tiny error handling.

@sea-snake
Copy link
Copy Markdown
Contributor Author

sea-snake commented Mar 23, 2026

Credential mutation — potential bug? In access.spec.ts (and possibly elsewhere) there’s identities[0].credentials.push(...).
This only mutates the JS array — it does not update the Chrome Virtual Authenticator.
Suggestion: add a small helper (syncCredentialsToAuthenticator or recreate the authenticator) so the test stays in sync with real WebAuthn behaviour.

False positive, it's likely grok was looking at older changes (maybe due to prior merge conflict) since this is no longer the case, instead it uses a setCredentialsForIdentity method now that keeps things in sync.

Authenticator cleanup Make sure removeVirtualAuthenticator is called in afterEach / afterAll so no leftover state leaks between tests (especially important in parallel CI runs). Add a comment if it’s already there.

Authenticators are bound to their page, not browser so they can't leak between tests. The identity fixture explicity listens for page navigations and their URL so they're always available on II pages.

Add 1–2 lines of comments explaining the new credential-tracking logic in identity.ts

The current ts doc comments on the relevant methods and inline comments should suffice.

The void on the listener in resume-openid-authorize is fine, but consider adding a fire-and-forget comment or tiny error handling.

This error handling was also brought up previously, it's outside the scope of this PR though.

@aterga aterga enabled auto-merge March 23, 2026 17:21
@aterga aterga added this pull request to the merge queue Mar 23, 2026
Merged via the queue into main with commit 25242c0 Mar 23, 2026
50 of 51 checks passed
@aterga aterga deleted the sea-snake/replace-dummy-auth-with-virtual-authenticator branch March 23, 2026 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants