Skip to content

Commit

Permalink
Remove screen to select authentication method: passkey or pin (#2687)
Browse files Browse the repository at this point in the history 
* Remove screen to choose passkey or pin

* Fix e2e test

* Fix lint

* Disable VC with pin

* Fix dev-build test
  • Loading branch information
@lmuntaner
lmuntaner authored Nov 7, 2024
1 parent 34df5ae commit a559ccf
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 127 deletions.
11 changes: 6 additions & 5 deletions demos/using-dev-build/specs/auth.e2e.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@ describe("authentication", () => {
await browser.$("#registerButton").click();

// Construct Identity (no-op)
const constructIdentity = await browser.$(
'[data-action="construct-identity"]'
);
await constructIdentity.waitForExist();
await constructIdentity.click();
// TODO: GIX-3138 Clean up after release
// const constructIdentity = await browser.$(
// '[data-action="construct-identity"]'
// );
// await constructIdentity.waitForExist();
// await constructIdentity.click();

await browser.$("h1").waitForExist();
const title = await browser.$("h1");
Expand Down
104 changes: 19 additions & 85 deletions src/frontend/src/flows/register/index.ts
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,10 @@
import { AuthnMethodData } from "$generated/internet_identity_types";
import { withLoader } from "$src/components/loader";
import {
PinIdentityMaterial,
constructPinIdentity,
} from "$src/crypto/pinIdentity";
import { tempKeyWarningBox } from "$src/flows/manage/tempKeys";
import { PinIdentityMaterial } from "$src/crypto/pinIdentity";
import { idbStorePinIdentityMaterial } from "$src/flows/pin/idb";
import { setPinFlow } from "$src/flows/pin/setPin";
import { registerDisabled } from "$src/flows/registerDisabled";
import { I18n } from "$src/i18n";
import { setAnchorUsed } from "$src/storage";
import {
passkeyAuthnMethodData,
pinAuthnMethodData,
} from "$src/utils/authnMethodData";
import { passkeyAuthnMethodData } from "$src/utils/authnMethodData";
import {
AlreadyInProgress,
ApiError,
Expand All @@ -32,7 +23,6 @@ import {
import { SignIdentity } from "@dfinity/agent";
import { ECDSAKeyIdentity } from "@dfinity/identity";
import { nonNullish } from "@dfinity/utils";
import { TemplateResult } from "lit-html";
import type { UAParser } from "ua-parser-js";
import { precomputeFirst, promptCaptcha } from "./captcha";
import { displayUserNumberWarmup } from "./finish";
Expand All @@ -43,9 +33,9 @@ export const registerFlow = async ({
identityRegistrationStart,
checkCaptcha,
identityRegistrationFinish,
storePinIdentity,
storePinIdentity: _storePinIdentity,
registrationAllowed,
pinAllowed,
pinAllowed: _pinAllowed,
uaParser,
}: {
identityRegistrationStart: () => Promise<
Expand Down Expand Up @@ -108,78 +98,24 @@ export const registerFlow = async ({
const flowStart = precomputeFirst(() => identityRegistrationStart());

const displayUserNumber = displayUserNumberWarmup();
const savePasskeyResult = await savePasskeyOrPin({
pinAllowed: await pinAllowed(),
});
if (savePasskeyResult === "canceled") {
return "canceled";
}
const result_ = await (async () => {
if (savePasskeyResult === "pin") {
const pinResult = await setPinFlow();
if (pinResult.tag === "canceled") {
return "canceled";
}

pinResult.tag satisfies "ok";

// XXX: this withLoader could be replaced with one that indicates what's happening (like the
// "Hang tight, ..." spinner)
const { identity, pinIdentityMaterial } = await withLoader(() =>
constructPinIdentity(pinResult)
);
const alias = await inferPinAlias({
userAgent: navigator.userAgent,
uaParser,
});
return {
identity,
authnMethodData: pinAuthnMethodData({
alias,
pubKey: identity.getPublicKey().toDer(),
}),
finalizeIdentity: (userNumber: bigint) =>
storePinIdentity({ userNumber, pinIdentityMaterial }),
finishSlot: tempKeyWarningBox({ i18n: new I18n() }),
authnMethod: "pin" as const,
};
} else {
const identity = savePasskeyResult;
const alias = await inferPasskeyAlias({
authenticatorType: identity.getAuthenticatorAttachment(),
userAgent: navigator.userAgent,
uaParser,
});
return {
identity,
authnMethodData: passkeyAuthnMethodData({
alias,
pubKey: identity.getPublicKey().toDer(),
credentialId: identity.rawId,
authenticatorAttachment: identity.getAuthenticatorAttachment(),
}),
authnMethod: "passkey" as const,
};
}
})();

if (result_ === "canceled") {
const identity = await savePasskeyOrPin();
if (identity === undefined) {
// TODO: Return something meaningful if getting the identity fails
return "canceled";
}
const alias = await inferPasskeyAlias({
authenticatorType: identity.getAuthenticatorAttachment(),
userAgent: navigator.userAgent,
uaParser,
});

const {
identity,
authnMethodData,
finalizeIdentity,
finishSlot,
authnMethod,
}: {
identity: SignIdentity;
authnMethodData: AuthnMethodData;
finalizeIdentity?: (userNumber: bigint) => Promise<void>;
finishSlot?: TemplateResult;
authnMethod: "pin" | "passkey";
} = result_;
const authnMethodData = passkeyAuthnMethodData({
alias,
pubKey: identity.getPublicKey().toDer(),
credentialId: identity.rawId,
authenticatorAttachment: identity.getAuthenticatorAttachment(),
});
const authnMethod = "passkey" as const;

const startResult = await flowStart();
if (startResult.kind !== "registrationFlowStepSuccess") {
Expand Down Expand Up @@ -214,7 +150,6 @@ export const registerFlow = async ({
result.kind satisfies "loginSuccess";

const userNumber = result.userNumber;
await finalizeIdentity?.(userNumber);
// We don't want to nudge the user with the recovery phrase warning page
// right after they've created their anchor.
result.connection.updateIdentityMetadata({
Expand All @@ -227,7 +162,6 @@ export const registerFlow = async ({
);
await displayUserNumber({
userNumber,
marketingIntroSlot: finishSlot,
});
return { ...result, authnMethod };
};
Expand Down
30 changes: 9 additions & 21 deletions src/frontend/src/flows/register/passkey.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,27 +97,15 @@ const savePasskeyTemplate = ({
export const savePasskeyPage = renderPage(savePasskeyTemplate);

// Prompt the user to create a WebAuthn identity or a PIN identity (if allowed)
export const savePasskeyOrPin = ({
pinAllowed,
}: {
pinAllowed: boolean;
}): Promise<IIWebAuthnIdentity | "pin" | "canceled"> => {
return new Promise((resolve) =>
savePasskeyPage({
i18n: new I18n(),
cancel: () => resolve("canceled"),
scrollToTop: true,
constructPasskey: async () => {
try {
const identity = await withLoader(() => constructIdentity({}));
resolve(identity);
} catch (e) {
toast.error(errorMessage(e));
}
},
constructPin: pinAllowed ? () => resolve("pin") : undefined,
})
);
export const savePasskeyOrPin = async (): Promise<
IIWebAuthnIdentity | undefined
> => {
try {
return await withLoader(() => constructIdentity({}));
} catch (e) {
toast.error(errorMessage(e));
return undefined;
}
};

// Return an appropriate error message depending on the (inferred) type of WebAuthn error
Expand Down
5 changes: 3 additions & 2 deletions src/frontend/src/test-e2e/flows.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ import {
export const FLOWS = {
register: async function (browser: WebdriverIO.Browser): Promise<string> {
const registerView = new RegisterView(browser);
await registerView.waitForDisplay();
await registerView.create();
// TODO: GIX-3138 Clean up after release
// await registerView.waitForDisplay();
// await registerView.create();
if (CAPTCHA_ENABLED) {
await registerView.waitForRegisterConfirm();
await registerView.confirmRegisterConfirm();
Expand Down
16 changes: 9 additions & 7 deletions src/frontend/src/test-e2e/pinAuth.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ import {

const DEFAULT_PIN_DEVICE_NAME = "Chrome on Mac OS";

test("PIN registration not enabled on non-Apple device", async () => {
// TODO: GIX-3138 Clean up after release
// TODO: Test login with PIN only GIX-3139
test.skip("PIN registration not enabled on non-Apple device", async () => {
await runInBrowser(async (browser: WebdriverIO.Browser) => {
await browser.url(II_URL);
const welcomeView = new WelcomeView(browser);
Expand All @@ -38,7 +40,7 @@ test("PIN registration not enabled on non-Apple device", async () => {
// The PIN auth feature is only enabled for Apple specific user agents, so tests set the user
// agent to chrome on macOS

test("Register and Log in with PIN identity", async () => {
test.skip("Register and Log in with PIN identity", async () => {
await runInBrowser(async (browser: WebdriverIO.Browser) => {
const pin = "123456";

Expand All @@ -53,7 +55,7 @@ test("Register and Log in with PIN identity", async () => {
}, APPLE_USER_AGENT);
}, 300_000);

test("Register with PIN and login without prefilled identity number", async () => {
test.skip("Register with PIN and login without prefilled identity number", async () => {
await runInBrowser(async (browser: WebdriverIO.Browser) => {
const pin = "123456";
await browser.url(II_URL);
Expand All @@ -72,7 +74,7 @@ test("Register with PIN and login without prefilled identity number", async () =
}, APPLE_USER_AGENT);
}, 300_000);

test("Register and log in with PIN identity, retry on wrong PIN", async () => {
test.skip("Register and log in with PIN identity, retry on wrong PIN", async () => {
await runInBrowser(async (browser: WebdriverIO.Browser) => {
const pin = "123456";
const wrongPin = "456321";
Expand All @@ -98,7 +100,7 @@ test("Register and log in with PIN identity, retry on wrong PIN", async () => {
}, APPLE_USER_AGENT);
}, 300_000);

test("Should not prompt for PIN after deleting temp key", async () => {
test.skip("Should not prompt for PIN after deleting temp key", async () => {
await runInBrowser(async (browser: WebdriverIO.Browser) => {
const pin = "123456";
await addVirtualAuthenticator(browser);
Expand All @@ -121,7 +123,7 @@ test("Should not prompt for PIN after deleting temp key", async () => {
}, APPLE_USER_AGENT);
}, 300_000);

test("Log into client application using PIN registration flow", async () => {
test.skip("Log into client application using PIN registration flow", async () => {
await runInBrowser(async (browser: WebdriverIO.Browser) => {
const pin = "123456";

Expand All @@ -142,7 +144,7 @@ test("Log into client application using PIN registration flow", async () => {
}, APPLE_USER_AGENT);
}, 300_000);

test("Register with PIN then log into client application", async () => {
test.skip("Register with PIN then log into client application", async () => {
await runInBrowser(async (browser: WebdriverIO.Browser) => {
const pin = "123456";

Expand Down
5 changes: 3 additions & 2 deletions src/frontend/src/test-e2e/pinAuthDisabled.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import { FLOWS } from "./flows";
import { runInBrowser, switchToPopup } from "./util";
import { AuthenticateView, DemoAppView, RegisterView } from "./views";

test("Cannot register with PIN if dapp disallows PIN", async () => {
// TODO: GIX-3138 Clean up after release
test.skip("Cannot register with PIN if dapp disallows PIN", async () => {
await runInBrowser(async (browser: WebdriverIO.Browser) => {
const demoAppView = new DemoAppView(browser);
await demoAppView.open(TEST_APP_NICE_URL, II_URL);
Expand All @@ -21,7 +22,7 @@ test("Cannot register with PIN if dapp disallows PIN", async () => {
}, APPLE_USER_AGENT);
}, 300_000);

test("Cannot auth with PIN if dapp disallows PIN", async () => {
test.skip("Cannot auth with PIN if dapp disallows PIN", async () => {
await runInBrowser(async (browser: WebdriverIO.Browser) => {
const pin = "123456";

Expand Down
11 changes: 6 additions & 5 deletions src/frontend/src/test-e2e/verifiableCredentials/index.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,12 @@ const testConfigs: Array<{
issuer: ISSUER_APP_URL_LEGACY,
authType: "webauthn",
},
{
relyingParty: TEST_APP_CANONICAL_URL,
issuer: ISSUER_APP_URL,
authType: "pin",
},
// TODO: Renable with PIN GIX-3139
// {
// relyingParty: TEST_APP_CANONICAL_URL,
// issuer: ISSUER_APP_URL,
// authType: "pin",
// },
];

testConfigs.forEach(({ relyingParty, issuer, authType }) => {
Expand Down
3 changes: 3 additions & 0 deletions src/frontend/src/test-e2e/views.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ export class RenameView extends View {
}

export class RegisterView extends View {
// View: Passkey or PIN registration
// TODO: GIX-3138 Clean up after release
// At the moment it's used only in skipped tests.
async waitForDisplay(): Promise<void> {
await this.browser
.$('[data-action="construct-identity"]')
Expand Down

0 comments on commit a559ccf

Please sign in to comment.