-
Notifications
You must be signed in to change notification settings - Fork 3
test: add-virtual-authenticator-tests #467
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| // Project tasks configuration. See https://zed.dev/docs/tasks for documentation. | ||
| [ | ||
| { | ||
| "label": "nx build", | ||
| "command": "pnpm", | ||
| "args": ["nx", "affected", "-t build", "--parallel=3"], | ||
| "cwd": "$ZED_WORKTREE_ROOT", | ||
| "use_new_terminal": false, | ||
| "allow_concurrent_runs": true, | ||
| "reveal": "no_focus" | ||
| }, | ||
| { | ||
| "label": "nx lint", | ||
| "command": "pnpm", | ||
| "args": ["nx", "affected", "-t lint", "--parallel=3"], | ||
| "cwd": "$ZED_WORKTREE_ROOT", | ||
| "use_new_terminal": false, | ||
| "allow_concurrent_runs": true, | ||
| "reveal": "no_focus" | ||
| }, | ||
| { | ||
| "label": "nx test", | ||
| "command": "pnpm", | ||
| "args": ["nx", "affected", "-t test", "--parallel=3"], | ||
| "cwd": "$ZED_WORKTREE_ROOT", | ||
| "use_new_terminal": false, | ||
| "allow_concurrent_runs": true, | ||
| "reveal": "no_focus" | ||
| }, | ||
| { | ||
| "label": "nx watch davinci-watch", | ||
| "command": "pnpm", | ||
| "args": ["watch", "@forgerock/davinci-app"], | ||
| "cwd": "$ZED_WORKTREE_ROOT", | ||
| "use_new_terminal": true, | ||
| "allow_concurrent_runs": true, | ||
| "reveal": "always" | ||
| }, | ||
| { | ||
| "label": "nx watch oidc-e2e", | ||
| "command": "pnpm", | ||
| "args": ["watch", "@forgerock/oidc-app"], | ||
| "cwd": "$ZED_WORKTREE_ROOT", | ||
| "use_new_terminal": true, | ||
| "allow_concurrent_runs": true, | ||
| "reveal": "no_focus" | ||
| }, | ||
| { | ||
| "label": "nx watch protect-watch", | ||
| "command": "pnpm", | ||
| "args": ["watch", "@forgerock/protect-app"], | ||
| "cwd": "$ZED_WORKTREE_ROOT", | ||
| "use_new_terminal": true, | ||
| "allow_concurrent_runs": true, | ||
| "reveal": "no_focus" | ||
| }, | ||
| { | ||
| "label": "nx typecheck repo", | ||
| "command": "pnpm", | ||
| "args": ["nx", "affected -t typecheck"], | ||
| "cwd": "$ZED_WORKTREE_ROOT", | ||
|
Comment on lines
+58
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Split Nx CLI flags into separate args.
🤖 Prompt for AI Agents |
||
| "reveal": "no_focus" | ||
| }, | ||
| { | ||
| "label": "oidc e2e ui", | ||
| "command": "pnpm", | ||
| "args": ["nx", "e2e", "@forgerock/oidc-suites", "--ui", "--skipNxCache"], | ||
| "cwd": "$ZED_WORKTREE_ROOT" | ||
| }, | ||
| { | ||
| "label": "protect-app e2e ui", | ||
| "command": "pnpm", | ||
| "args": ["nx", "e2e", "@forgerock/protect-suites", "--ui", "--skipNxCache"], | ||
| "cwd": "$ZED_WORKTREE_ROOT", | ||
| "reveal": "no_focus" | ||
| }, | ||
| { | ||
| "label": "local release repo", | ||
| "command": "pnpm", | ||
| "args": ["release-local"], | ||
| "cwd": "$ZED_WORKTREE_ROOT", | ||
| "reveal": "no_focus" | ||
| }, | ||
| { | ||
| "label": "journey e2e ui", | ||
| "command": "pnpm", | ||
| "args": ["nx", "e2e", "@forgerock/journey-suites", "--ui", "--skipNxCache"], | ||
| "cwd": "$ZED_WORKTREE_ROOT" | ||
| } | ||
| ] | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,31 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { JourneyStep } from '@forgerock/journey-client/types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { WebAuthn, WebAuthnStepType } from '@forgerock/journey-client/webauthn'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function webauthnComponent(journeyEl: HTMLDivElement, step: JourneyStep, idx: number) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const container = document.createElement('div'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| container.id = `webauthn-container-${idx}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const info = document.createElement('p'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| info.innerText = 'Please complete the WebAuthn challenge using your authenticator.'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| container.appendChild(info); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| journeyEl.appendChild(container); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const webAuthnStepType = WebAuthn.getWebAuthnStepType(step); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async function handleWebAuthn() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (webAuthnStepType === WebAuthnStepType.Authentication) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log('trying authentication'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await WebAuthn.authenticate(step); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log('trying registration'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (WebAuthnStepType.Registration === webAuthnStepType) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await WebAuthn.register(step); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Promise.resolve(undefined); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.error('WebAuthn error:', error); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+14
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix misplaced console.log statement. Line 19 contains Apply this diff to fix the logging: async function handleWebAuthn() {
try {
if (webAuthnStepType === WebAuthnStepType.Authentication) {
console.log('trying authentication');
await WebAuthn.authenticate(step);
- console.log('trying registration');
} else if (WebAuthnStepType.Registration === webAuthnStepType) {
+ console.log('trying registration');
await WebAuthn.register(step);
} else {
return Promise.resolve(undefined);
}
} catch (error) {
console.error('WebAuthn error:', error);
}
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return handleWebAuthn(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,19 +9,22 @@ import './style.css'; | |
| import { journey } from '@forgerock/journey-client'; | ||
|
|
||
| import type { | ||
| RequestMiddleware, | ||
| NameCallback, | ||
| PasswordCallback, | ||
| RequestMiddleware, | ||
| } from '@forgerock/journey-client/types'; | ||
|
|
||
| import textComponent from './components/text.js'; | ||
| import passwordComponent from './components/password.js'; | ||
| import textComponent from './components/text.js'; | ||
| import { serverConfigs } from './server-configs.js'; | ||
| import { webauthnComponent } from './components/webauthn.js'; | ||
| import { WebAuthn, WebAuthnStepType } from '@forgerock/journey-client/webauthn'; | ||
|
|
||
| const qs = window.location.search; | ||
| const searchParams = new URLSearchParams(qs); | ||
|
|
||
| const config = serverConfigs[searchParams.get('clientId') || 'basic']; | ||
| const journeyName = searchParams.get('clientId') || 'UsernamePassword'; | ||
| const config = serverConfigs[journeyName]; | ||
|
|
||
| const requestMiddleware: RequestMiddleware[] = [ | ||
| (req, action, next) => { | ||
|
|
@@ -48,7 +51,7 @@ const requestMiddleware: RequestMiddleware[] = [ | |
| const formEl = document.getElementById('form') as HTMLFormElement; | ||
| const journeyEl = document.getElementById('journey') as HTMLDivElement; | ||
|
|
||
| let step = await journeyClient.start(); | ||
| let step = await journeyClient.start({ ...config, journey: journeyName }); | ||
|
|
||
| function renderComplete() { | ||
| if (step?.type !== 'LoginSuccess') { | ||
|
|
@@ -103,9 +106,30 @@ const requestMiddleware: RequestMiddleware[] = [ | |
| header.innerText = formName || ''; | ||
| journeyEl.appendChild(header); | ||
|
|
||
| const callbacks = step.callbacks; | ||
| const webAuthnStep = WebAuthn.getWebAuthnStepType(step); | ||
|
|
||
| if ( | ||
| webAuthnStep === WebAuthnStepType.Authentication || | ||
| webAuthnStep === WebAuthnStepType.Registration | ||
| ) { | ||
| await webauthnComponent(journeyEl, step, 0); | ||
| step = await journeyClient.next(step); | ||
| if (step?.type === 'Step') { | ||
| await renderForm(); | ||
| } else if (step?.type === 'LoginSuccess') { | ||
| console.log('Basic login successful'); | ||
| renderComplete(); | ||
| } else if (step?.type === 'LoginFailure') { | ||
| renderForm(); | ||
| renderError(); | ||
| } else { | ||
| console.error('Unknown node status', step); | ||
| } | ||
|
Comment on lines
+117
to
+127
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle When 🤖 Prompt for AI Agents |
||
| return; // prevent the rest of the function from running | ||
| } | ||
|
|
||
| callbacks.forEach((callback, idx) => { | ||
| const callbacks = step.callbacks; | ||
| callbacks.forEach(async (callback, idx) => { | ||
| if (callback.getType() === 'NameCallback') { | ||
| const cb = callback as NameCallback; | ||
| textComponent( | ||
|
|
@@ -146,7 +170,7 @@ const requestMiddleware: RequestMiddleware[] = [ | |
| * Recursively render the form with the new state | ||
| */ | ||
| if (step?.type === 'Step') { | ||
| renderForm(); | ||
| await renderForm(); | ||
| } else if (step?.type === 'LoginSuccess') { | ||
| console.log('Basic login successful'); | ||
| renderComplete(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,37 @@ | |
| import type { JourneyClientConfig } from '@forgerock/journey-client/types'; | ||
|
|
||
| export const serverConfigs: Record<string, JourneyClientConfig> = { | ||
| basic: { | ||
| UsernamePassword: { | ||
| serverConfig: { | ||
| baseUrl: 'https://openam-sdks.forgeblocks.com/am/', | ||
| }, | ||
| realmPath: '/alpha', | ||
| }, | ||
| ['TEST_WebAuthn-Registration']: { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can think about how to actually do this if the objects the same every time.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I wasn't sure exactly how we wanted to handle different configs. I ended up not using it at all in all the tests I moved over from the legacy repo. |
||
| serverConfig: { | ||
| baseUrl: 'https://openam-sdks.forgeblocks.com/am/', | ||
| }, | ||
| realmPath: '/alpha', | ||
| }, | ||
| ['TEST_WebAuthnAuthentication_UsernamePassword']: { | ||
| serverConfig: { | ||
| baseUrl: 'https://openam-sdks.forgeblocks.com/am/', | ||
| }, | ||
| realmPath: '/alpha', | ||
| }, | ||
| ['TEST_WebAuthnAuthentication']: { | ||
| serverConfig: { | ||
| baseUrl: 'https://openam-sdks.forgeblocks.com/am/', | ||
| }, | ||
| realmPath: '/alpha', | ||
| }, | ||
| ['TEST_WebAuthn-Registration-UsernameToDevice']: { | ||
| serverConfig: { | ||
| baseUrl: 'https://openam-sdks.forgeblocks.com/am/', | ||
| }, | ||
| realmPath: '/alpha', | ||
| }, | ||
| ['TEST_WebAuthnAuthentication_Usernameless']: { | ||
| serverConfig: { | ||
| baseUrl: 'https://openam-sdks.forgeblocks.com/am/', | ||
| }, | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,119 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { test, expect } from '@playwright/test'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { CDPSession } from 'playwright'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { asyncEvents } from './utils/async-events.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { password, username } from './utils/demo-user.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| test.use({ browserName: 'chromium' }); // ensure CDP/WebAuthn is available | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are Virtual Authenticators only viable on Chromium?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes |
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| test('Register and authenticate with webauthn device (username + password journey)', async ({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| page, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| context, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let cdp: CDPSession | undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let authenticatorId: string | undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let webauthnEnabled = false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let recordedCredentialIds: string[] = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| await test.step('Configure virtual authenticator', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| cdp = await context.newCDPSession(page); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await cdp.send('WebAuthn.enable'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| webauthnEnabled = true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // A "platform" authenticator (aka internal) with UV+RK enabled is the usual default for passkeys. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const response = await cdp.send('WebAuthn.addVirtualAuthenticator', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| options: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| protocol: 'ctap2', | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| transport: 'internal', // platform authenticator | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| hasResidentKey: true, // allow discoverable credentials (passkeys) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| hasUserVerification: true, // device supports UV | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| isUserVerified: true, // simulate successful UV (PIN/biometric) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| automaticPresenceSimulation: true, // auto "touch"/presence | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| authenticatorId = response.authenticatorId; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+12
to
+34
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be good to have a Friday conversation about this CDP config stuff. |
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { navigate } = asyncEvents(page); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // First, register a credential we can use later | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await test.step('Navigate to registration journey', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like a new pattern (e.g. |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| await navigate('/?clientId=TEST_WebAuthn-Registration'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await expect(page).toHaveURL('http://localhost:5829/?clientId=TEST_WebAuthn-Registration'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| await test.step('Complete primary credentials', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await page.getByLabel('User Name').fill(username); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await page.getByLabel('Password').fill(password); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await page.getByRole('button', { name: 'Submit' }).click(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| await test.step('Register WebAuthn credential', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test says 'Register WebAuthn credential', but it doesn't do anything other than expect the Logout button to be present. Am I missing something?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I think this is from my debugging leftover. was going to clean up once I fix the tests with the node solution |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // With the virtual authenticator present and presence auto-simulated, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // registration will complete without any OS prompts. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await expect(page.getByRole('button', { name: 'Logout' })).toBeVisible(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to avoid testing the test app's UI logic (e.g. using the presence of the Logout button to indirectly assert success), and focus more on testing outcomes of SDK functionality.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we do this in almost every test suite, is this something you're suggesting we no longer do?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I noticed this when I was refactoring the OIDC tests, and removed it from them. I just wanted to at least not continue to do it, and refactor existing tests when we can. |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| await test.step('Capture virtual authenticator credentials', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!cdp || !authenticatorId) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error('Expected CDP session and authenticator to be configured'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { credentials } = await cdp.send('WebAuthn.getCredentials', { authenticatorId }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| recordedCredentialIds = (credentials || []).map((item) => item.credentialId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(recordedCredentialIds.length).toBeGreaterThan(0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+57
to
+64
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this block testing in relation to the SDK?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's testing that the credentials were actually stored. so that the sdk stored the credential itself.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The SDK stores credentials?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe I could have been more clear, but i just meant that the SDK creates the credential and it exists. The sdk calls create. This verifies that we are in fact creating the credentials and they are stored (which is on the device it was created)
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| await test.step('Logout after registration', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await page.getByRole('button', { name: 'Logout' }).click(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await expect(page.getByRole('button', { name: 'Submit' })).toBeVisible(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Now authenticate via Username -> Password -> WebAuthn | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await test.step('Navigate to username+password authentication journey', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await navigate('/?clientId=TEST_WebAuthnAuthentication_UsernamePassword'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await expect(page).toHaveURL( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'http://localhost:5829/?clientId=TEST_WebAuthnAuthentication_UsernamePassword', | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| await test.step('Complete username credential', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await page.getByLabel('User Name').fill(username); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await page.getByRole('button', { name: 'Submit' }).click(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| await test.step('Complete password credential', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await page.getByLabel('Password').fill(password); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await page.getByRole('button', { name: 'Submit' }).click(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| await test.step('Authenticate WebAuthn credential', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!cdp || !authenticatorId) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error('Expected CDP session and authenticator to be configured'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Server has UV set to Discouraged; providing UV is fine. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await cdp.send('WebAuthn.setUserVerified', { authenticatorId, isUserVerified: true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // With the virtual authenticator present and presence auto-simulated, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // authentication will complete without any OS prompts. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await expect(page.getByRole('button', { name: 'Logout' })).toBeVisible(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| await test.step('Logout after authentication', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await page.getByRole('button', { name: 'Logout' }).click(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await expect(page.getByRole('button', { name: 'Submit' })).toBeVisible(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } finally { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!cdp) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| const activeCdp = cdp; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await test.step('Remove virtual authenticator', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (authenticatorId) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await activeCdp.send('WebAuthn.removeVirtualAuthenticator', { authenticatorId }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (webauthnEnabled) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await activeCdp.send('WebAuthn.disable'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+105
to
+118
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not return from the finally cleanup. Returning inside the Apply this diff to keep the cleanup while avoiding the unsafe } finally {
- if (!cdp) {
- return;
- }
-
- const activeCdp = cdp;
- await test.step('Remove virtual authenticator', async () => {
- if (authenticatorId) {
- await activeCdp.send('WebAuthn.removeVirtualAuthenticator', { authenticatorId });
- }
- if (webauthnEnabled) {
- await activeCdp.send('WebAuthn.disable');
- }
- });
+ const activeCdp = cdp;
+ if (activeCdp) {
+ await test.step('Remove virtual authenticator', async () => {
+ if (authenticatorId) {
+ await activeCdp.send('WebAuthn.removeVirtualAuthenticator', { authenticatorId });
+ }
+ if (webauthnEnabled) {
+ await activeCdp.send('WebAuthn.disable');
+ }
+ });
+ }
}📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (2.1.2)[error] 106-106: Unsafe usage of 'return'. 'return' in 'finally' overwrites the control flow statements inside 'try' and 'catch'. (lint/correctness/noUnsafeFinally) 🪛 ESLint[error] 106-106: Unsafe usage of ReturnStatement. (no-unsafe-finally) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are my zed tasks, would be nice to keep cause i dont have to rercreate them every time, should be able to use in vscode pretty closely if you'd like
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we'd want to commit this. We can ignore this, but also not remove it when we do a clean. The same as we do with our
.envfiles and such.