-
Notifications
You must be signed in to change notification settings - Fork 142
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
Implement API to auto-select identity based on known principal #2563
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 |
---|---|---|
|
@@ -66,18 +66,20 @@ export const authenticateBox = async ({ | |
i18n, | ||
templates, | ||
allowPinAuthentication, | ||
autoSelectionIdentity, | ||
}: { | ||
connection: Connection; | ||
i18n: I18n; | ||
templates: AuthnTemplates; | ||
allowPinAuthentication: boolean; | ||
autoSelectionIdentity?: bigint; | ||
}): Promise<{ | ||
userNumber: bigint; | ||
connection: AuthenticatedConnection; | ||
newAnchor: boolean; | ||
authnMethod: "pin" | "passkey" | "recovery"; | ||
}> => { | ||
const promptAuth = () => | ||
const promptAuth = (autoSelectIdentity?: bigint) => | ||
authenticateBoxFlow<PinIdentityMaterial>({ | ||
i18n, | ||
templates, | ||
|
@@ -99,12 +101,13 @@ export const authenticateBox = async ({ | |
retrievePinIdentityMaterial: ({ userNumber }) => | ||
idbRetrievePinIdentityMaterial({ userNumber }), | ||
allowPinAuthentication, | ||
autoSelectIdentity, | ||
}); | ||
|
||
// Retry until user has successfully authenticated | ||
for (;;) { | ||
try { | ||
const result = await promptAuth(); | ||
const result = await promptAuth(autoSelectionIdentity); | ||
|
||
// If the user canceled or just added a device, we retry | ||
if ("tag" in result) { | ||
|
@@ -126,6 +129,9 @@ export const authenticateBox = async ({ | |
primaryButton: "Try again", | ||
}); | ||
} | ||
// clear out the auto-select so that after the first error / cancel | ||
// the identity number picker actually waits for input | ||
autoSelectionIdentity = undefined; | ||
} | ||
}; | ||
|
||
|
@@ -165,6 +171,7 @@ export const authenticateBoxFlow = async <I>({ | |
verifyPinValidity, | ||
retrievePinIdentityMaterial, | ||
allowPinAuthentication, | ||
autoSelectIdentity, | ||
}: { | ||
i18n: I18n; | ||
templates: AuthnTemplates; | ||
|
@@ -192,6 +199,7 @@ export const authenticateBoxFlow = async <I>({ | |
userNumber: bigint; | ||
}) => Promise<I | undefined>; | ||
allowPinAuthentication: boolean; | ||
autoSelectIdentity?: bigint; | ||
verifyPinValidity: (opts: { | ||
userNumber: bigint; | ||
pinIdentityMaterial: I; | ||
|
@@ -292,7 +300,10 @@ export const authenticateBoxFlow = async <I>({ | |
// we assume a new user and show the "firstTime" screen. | ||
const anchors = await getAnchors(); | ||
if (isNonEmptyArray(anchors)) { | ||
const result = await pages.pick({ anchors }); | ||
const result = await pages.pick({ | ||
anchors, | ||
autoSelect: autoSelectIdentity, | ||
}); | ||
|
||
if (result.tag === "pick") { | ||
return doLogin({ userNumber: result.userNumber }); | ||
|
@@ -565,16 +576,29 @@ export const authnScreens = (i18n: I18n, props: AuthnTemplates) => { | |
resolve({ tag: "recover", userNumber }), | ||
}) | ||
), | ||
pick: (pickProps: { anchors: NonEmptyArray<bigint> }) => | ||
pick: (pickProps: { | ||
anchors: NonEmptyArray<bigint>; | ||
autoSelect?: bigint; | ||
}) => | ||
new Promise< | ||
{ tag: "more_options" } | { tag: "pick"; userNumber: bigint } | ||
>((resolve) => | ||
>((resolve) => { | ||
// render page first so that when the identity is picked and the passkey | ||
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 it that we need to wait? Couldn't we show a spinner while we wait for it and then resolve? 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. Well, normally we wait for the user to chose. But if we have an a 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 could render something else if we want to improve it I guess. But definitely not a priority. |
||
// dialog pops up, the II page is not just blank. | ||
pages.pick({ | ||
...pickProps, | ||
onSubmit: (userNumber) => resolve({ tag: "pick", userNumber }), | ||
moreOptions: () => resolve({ tag: "more_options" }), | ||
}) | ||
), | ||
}); | ||
// If an existing autoSelect value is supplied immediately | ||
// resolve with the auto-selected identity number | ||
if ( | ||
nonNullish(pickProps.autoSelect) && | ||
pickProps.anchors.includes(pickProps.autoSelect) | ||
) { | ||
resolve({ tag: "pick", userNumber: pickProps.autoSelect }); | ||
} | ||
}), | ||
}; | ||
}; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
import { II_URL, TEST_APP_NICE_URL } from "$src/test-e2e/constants"; | ||
import { FLOWS } from "$src/test-e2e/flows"; | ||
import { | ||
addWebAuthnCredential, | ||
getWebAuthnCredentials, | ||
originToRelyingPartyId, | ||
runInBrowser, | ||
switchToPopup, | ||
} from "$src/test-e2e/util"; | ||
import { AuthenticateView, DemoAppView } from "$src/test-e2e/views"; | ||
import { expect } from "vitest"; | ||
|
||
test("Should prompt for passkey auth immediately when supplying known auto-select principal", async () => { | ||
await runInBrowser(async (browser: WebdriverIO.Browser) => { | ||
const { demoAppView, credentials } = await registerAndSignIn(browser); | ||
const exp1 = await browser.$("#expiration").getText(); | ||
|
||
// authenticate again, but this time _with_ a known principal | ||
const knownPrincipal = await demoAppView.getPrincipal(); | ||
await demoAppView.setAutoSelectionPrincipal(knownPrincipal); | ||
await demoAppView.signin(); | ||
|
||
// add credential previously registered to the new tab again | ||
const authenticatorId2 = await switchToPopup(browser); | ||
await addWebAuthnCredential( | ||
browser, | ||
authenticatorId2, | ||
credentials[0], | ||
originToRelyingPartyId(II_URL) | ||
); | ||
|
||
// Passkey interaction completes automatically with virtual authenticator | ||
await demoAppView.waitForAuthenticated(); | ||
|
||
// By having different expiry timestamps we know that the sign-in completed | ||
// twice successfully | ||
const exp2 = await browser.$("#expiration").getText(); | ||
expect(exp1).not.toBe(exp2); | ||
}); | ||
}, 300_000); | ||
|
||
test("Should require user interaction when supplying unknown auto-select principal", async () => { | ||
await runInBrowser(async (browser: WebdriverIO.Browser) => { | ||
const { demoAppView, credentials, userNumber } = await registerAndSignIn( | ||
browser | ||
); | ||
const exp1 = await browser.$("#expiration").getText(); | ||
|
||
// authenticate again, but this time with an unknown principal | ||
// we use a canister id here, because II will never issue a canister id to users, but it is a valid principal | ||
await demoAppView.setAutoSelectionPrincipal("rdmx6-jaaaa-aaaaa-aaadq-cai"); | ||
await demoAppView.signin(); | ||
|
||
// add credential previously registered to the new tab again | ||
const authenticatorId2 = await switchToPopup(browser); | ||
await addWebAuthnCredential( | ||
browser, | ||
authenticatorId2, | ||
credentials[0], | ||
originToRelyingPartyId(II_URL) | ||
); | ||
|
||
const authView = new AuthenticateView(browser); | ||
await authView.waitForDisplay(); | ||
// needs explicit identity selection | ||
await authView.pickAnchor(userNumber); | ||
|
||
// Passkey interaction completes automatically with virtual authenticator | ||
await demoAppView.waitForAuthenticated(); | ||
|
||
// By having different expiry timestamps we know that the sign-in completed | ||
// twice successfully | ||
const exp2 = await browser.$("#expiration").getText(); | ||
expect(exp1).not.toBe(exp2); | ||
}); | ||
}, 300_000); | ||
|
||
/** | ||
* Registers a user and signs in with the demo app. | ||
* @param browser browser to use. | ||
* @return - the demo app view to proceed with the test | ||
* - the identity number of the registered identity | ||
* - the webauthn credential that was created when registering the identity. | ||
*/ | ||
async function registerAndSignIn(browser: WebdriverIO.Browser) { | ||
const demoAppView = new DemoAppView(browser); | ||
await demoAppView.open(TEST_APP_NICE_URL, II_URL); | ||
await demoAppView.waitForDisplay(); | ||
expect(await demoAppView.getPrincipal()).toBe(""); | ||
await demoAppView.signin(); | ||
const authenticatorId1 = await switchToPopup(browser); | ||
const userNumber = await FLOWS.registerNewIdentityAuthenticateView(browser); | ||
const credentials = await getWebAuthnCredentials(browser, authenticatorId1); | ||
expect(credentials).toHaveLength(1); | ||
await demoAppView.waitForAuthenticated(); | ||
return { demoAppView, credentials, userNumber }; | ||
} |
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.
autoSelect
seems like a boolean type of parameter. How aboutselectedIdentity
?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.
As discussed, it is only automatically selected on the initial picker screen, but the user can still override that by cancelling the passkey interaction and picking something else.