Skip to content

Commit

Permalink
Fix dead-end in device add flow (#2338)
Browse files Browse the repository at this point in the history
This PR fixes a dead end where users would end up stuck after adding a
new passkey to an identity from the sign-in screen.

Note: This PR does not yet include the new confirmation screens as per
figma. This will be done separately.
  • Loading branch information
frederikrothenberger committed Mar 7, 2024
1 parent 7f542b7 commit 736392d
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 16 deletions.
32 changes: 17 additions & 15 deletions src/frontend/src/components/authenticateBox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@ export const authenticateBox = async ({
for (;;) {
const result = await promptAuth();

// If the user canceled, we retry
// If the user canceled or just added a device, we retry
if ("tag" in result) {
result satisfies { tag: "canceled" };
result satisfies { tag: "canceled" | "deviceAdded" };
continue;
}

Expand Down Expand Up @@ -158,7 +158,9 @@ export const authenticateBoxFlow = async <T, I>({
}: {
i18n: I18n;
templates: AuthnTemplates;
addDevice: (userNumber?: bigint) => Promise<{ alias: string }>;
addDevice: (
userNumber?: bigint
) => Promise<{ alias: string; userNumber: bigint }>;
loginPasskey: (
userNumber: bigint
) => Promise<
Expand Down Expand Up @@ -192,6 +194,7 @@ export const authenticateBoxFlow = async <T, I>({
})
| FlowError
| { tag: "canceled" }
| { tag: "deviceAdded" }
> => {
const pages = authnScreens(i18n, { ...templates });

Expand Down Expand Up @@ -243,20 +246,18 @@ export const authenticateBoxFlow = async <T, I>({
})
| FlowError
| { tag: "canceled" }
| { tag: "deviceAdded" }
> => {
const result = await pages.useExisting();
if (result.tag === "submit") {
return doLogin({ userNumber: result.userNumber });
}

if (result.tag === "add_device") {
const _ = await addDevice(result.userNumber);
// XXX: we don't currently do anything with the result from adding a device and
// we let the flow hang.
const hang = await new Promise<never>((_) => {
/* hang forever */
});
return hang;
const { userNumber } = await addDevice(result.userNumber);
// The user number now has a passkey associated and hence should be remembered
await setAnchorUsed(userNumber);
return { tag: "deviceAdded" } as const;
}

if (result.tag === "register") {
Expand Down Expand Up @@ -654,7 +655,7 @@ const loginPinIdentityMaterial = ({
const asNewDevice = async (
connection: Connection,
prefilledUserNumber?: bigint
) => {
): Promise<{ alias: string; userNumber: bigint }> => {
// Prompt the user for an anchor and provide additional information about the flow.
// If the user number is already known, it is prefilled in the screen.
const promptUserNumberWithInfo = async (prefilledUserNumber?: bigint) => {
Expand All @@ -672,10 +673,11 @@ const asNewDevice = async (
return result;
};

return registerTentativeDevice(
await promptUserNumberWithInfo(prefilledUserNumber),
connection
);
const userNumber = await promptUserNumberWithInfo(prefilledUserNumber);
return {
userNumber,
...(await registerTentativeDevice(userNumber, connection)),
};
};

// Helper to convert PIN identity material to a Der public-key
Expand Down
9 changes: 9 additions & 0 deletions src/frontend/src/test-e2e/addDevice.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
AddIdentityAnchorView,
AddRemoteDeviceInstructionsView,
AddRemoteDeviceVerificationCodeView,
AuthenticateView,
MainView,
NotInRegistrationModeView,
VerifyRemoteDeviceView,
Expand Down Expand Up @@ -151,6 +152,14 @@ test("Add remote device starting on new device", async () => {
const addDeviceSuccessView = new AddDeviceSuccessView(browser);
await addDeviceSuccessView.waitForDisplay();
await addDeviceSuccessView.continue();

// browser 2 again
// make sure the browser now shows the sign-in screen with the user number
// pre-filled
await focusBrowser(browser2);
const authView = new AuthenticateView(browser2);
await authView.waitForDisplay();
await authView.expectAnchor(userNumber);
});

await mainView.waitForDisplay();
Expand Down
4 changes: 4 additions & 0 deletions src/frontend/src/test-e2e/views.ts
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,10 @@ export class AuthenticateView extends View {
await this.browser.$(`[data-anchor-id="${anchor}"]`).click();
}

async expectAnchor(anchor: string): Promise<void> {
await this.browser.$(`[data-anchor-id="${anchor}"]`).waitForDisplayed();
}

async expectAnchorInputField(): Promise<void> {
await this.browser
.$('[data-role="anchor-input"]')
Expand Down
5 changes: 4 additions & 1 deletion src/showcase/src/flows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ export const iiFlows: Record<string, () => void> = {
templates: manageTemplates,
addDevice: () => {
toast.info(html`Added device`);
return Promise.resolve({ alias: "My Device" });
return Promise.resolve({
alias: "My Device",
userNumber: BigInt(1234),
});
},
loginPasskey: async () => {
await new Promise((resolve) => setTimeout(resolve, 2000));
Expand Down

0 comments on commit 736392d

Please sign in to comment.