Use identity metadata to not show recovery warning page (#2523)

* Use identity metadata to not show recovery warning page * Remove generic * Fix minor issues * Add comment
dfinity · Jul 3, 2024 · 9a8bae0 · 9a8bae0
1 parent 61e6696
commit 9a8bae0
Show file tree

Hide file tree

Showing 15 changed files with 123 additions and 92 deletions.
diff --git a/src/frontend/src/components/authenticateBox.ts b/src/frontend/src/components/authenticateBox.ts
@@ -8,7 +8,11 @@ import { registerTentativeDevice } from "$src/flows/addDevice/welcomeView/regist
 import { idbRetrievePinIdentityMaterial } from "$src/flows/pin/idb";
 import { usePin } from "$src/flows/pin/usePin";
 import { useRecovery } from "$src/flows/recovery/useRecovery";
-import { getRegisterFlowOpts, registerFlow } from "$src/flows/register";
+import {
+  RegisterFlowOpts,
+  getRegisterFlowOpts,
+  registerFlow,
+} from "$src/flows/register";
 import { I18n } from "$src/i18n";
 import { getAnchors, setAnchorUsed } from "$src/storage";
 import {
@@ -83,7 +87,7 @@ export const authenticateBox = async ({
   authnMethod: "pin" | "passkey" | "recovery";
 }> => {
   const promptAuth = () =>
-    authenticateBoxFlow<AuthenticatedConnection, PinIdentityMaterial>({
+    authenticateBoxFlow<PinIdentityMaterial>({
       i18n,
       templates,
       addDevice: (userNumber) => asNewDevice(connection, userNumber),
@@ -159,7 +163,7 @@ const pinIdentityAuthenticatorValidity = async ({
 
 /** Authentication box component which authenticates a user
  * to II or to another dapp */
-export const authenticateBoxFlow = async <T, I>({
+export const authenticateBoxFlow = async <I>({
   i18n,
   templates,
   addDevice,
@@ -179,7 +183,7 @@ export const authenticateBoxFlow = async <T, I>({
   loginPasskey: (
     userNumber: bigint
   ) => Promise<
-    LoginSuccess<T> | AuthFail | WebAuthnFailed | UnknownUser | ApiError
+    LoginSuccess | AuthFail | WebAuthnFailed | UnknownUser | ApiError
   >;
   loginPinIdentityMaterial: ({
     userNumber,
@@ -189,8 +193,8 @@ export const authenticateBoxFlow = async <T, I>({
     userNumber: bigint;
     pin: string;
     pinIdentityMaterial: I;
-  }) => Promise<LoginSuccess<T> | BadPin>;
-  recover: () => Promise<LoginSuccess<T> | { tag: "canceled" }>;
+  }) => Promise<LoginSuccess | BadPin>;
+  recover: () => Promise<LoginSuccess | { tag: "canceled" }>;
   retrievePinIdentityMaterial: ({
     userNumber,
   }: {
@@ -201,9 +205,9 @@ export const authenticateBoxFlow = async <T, I>({
     userNumber: bigint;
     pinIdentityMaterial: I;
   }) => Promise<"valid" | "expired">;
-  registerFlowOpts: Parameters<typeof registerFlow<T>>[0];
+  registerFlowOpts: RegisterFlowOpts;
 }): Promise<
-  | (LoginSuccess<T> & {
+  | (LoginSuccess & {
       newAnchor: boolean;
       authnMethod: "pin" | "passkey" | "recovery";
     })
@@ -215,7 +219,7 @@ export const authenticateBoxFlow = async <T, I>({
 
   // The registration flow for a new identity
   const doRegister = async (): Promise<
-    | (LoginSuccess<T> & {
+    | (LoginSuccess & {
         newAnchor: true;
         authnMethod: "pin" | "passkey" | "recovery";
       })
@@ -225,7 +229,7 @@ export const authenticateBoxFlow = async <T, I>({
     | RegisterNoSpace
     | { tag: "canceled" }
   > => {
-    const result2 = await registerFlow<T>(registerFlowOpts);
+    const result2 = await registerFlow(registerFlowOpts);
 
     if (result2 === "canceled") {
       return { tag: "canceled" } as const;
@@ -235,7 +239,7 @@ export const authenticateBoxFlow = async <T, I>({
       return result2;
     }
 
-    result2 satisfies LoginSuccess<T>;
+    result2 satisfies LoginSuccess;
     return {
       newAnchor: true,
       ...result2,
@@ -255,7 +259,7 @@ export const authenticateBoxFlow = async <T, I>({
 
   // Prompt for an identity number
   const doPrompt = async (): Promise<
-    | (LoginSuccess<T> & {
+    | (LoginSuccess & {
         newAnchor: boolean;
         authnMethod: "pin" | "passkey" | "recovery";
       })
@@ -284,7 +288,7 @@ export const authenticateBoxFlow = async <T, I>({
       return { tag: "canceled" } as const;
     }
 
-    recoverResult satisfies LoginSuccess<T>;
+    recoverResult satisfies LoginSuccess;
     return {
       newAnchor:
         false /* If an anchor was recovered, then it's _not_ a new anchor */,
@@ -389,9 +393,11 @@ const clarifyError_ = <K extends FlowError["kind"]>(
 ): Omit<ErrorOptions, "primaryButton"> =>
   clarifyError[flowError.kind](flowError);
 
-export const handleLoginFlowResult = async <T, E>(
-  result: (LoginSuccess<T> & E) | FlowError
-): Promise<({ userNumber: bigint; connection: T } & E) | undefined> => {
+export const handleLoginFlowResult = async <E>(
+  result: (LoginSuccess & E) | FlowError
+): Promise<
+  ({ userNumber: bigint; connection: AuthenticatedConnection } & E) | undefined
+> => {
   if (result.kind === "loginSuccess") {
     await setAnchorUsed(result.userNumber);
     return result;
@@ -707,7 +713,7 @@ const pinIdentityToDerPubkey = async (
 };
 
 // Find and use a passkey, whether PIN or webauthn
-const useIdentityFlow = async <T, I>({
+const useIdentityFlow = async <I>({
   userNumber,
   allowPinAuthentication,
   retrievePinIdentityMaterial,
@@ -724,7 +730,7 @@ const useIdentityFlow = async <T, I>({
   loginPasskey: (
     userNumber: bigint
   ) => Promise<
-    LoginSuccess<T> | AuthFail | WebAuthnFailed | UnknownUser | ApiError
+    LoginSuccess | AuthFail | WebAuthnFailed | UnknownUser | ApiError
   >;
   allowPinAuthentication: boolean;
   verifyPinValidity: (opts: {
@@ -739,9 +745,9 @@ const useIdentityFlow = async <T, I>({
     userNumber: bigint;
     pin: string;
     pinIdentityMaterial: I;
-  }) => Promise<LoginSuccess<T> | BadPin>;
+  }) => Promise<LoginSuccess | BadPin>;
 }): Promise<
-  | (LoginSuccess<T> & {
+  | (LoginSuccess & {
       newAnchor: boolean;
       authnMethod: "pin" | "passkey" | "recovery";
     })
@@ -792,7 +798,7 @@ const useIdentityFlow = async <T, I>({
   }
 
   // Otherwise, attempt login with PIN
-  const result = await usePin<LoginSuccess<T> | BadPin>({
+  const result = await usePin<LoginSuccess | BadPin>({
     verifyPin: async (pin) => {
       const result = await loginPinIdentityMaterial({
         userNumber,
@@ -804,7 +810,7 @@ const useIdentityFlow = async <T, I>({
         return { ok: false, error: "Invalid PIN" };
       }
 
-      result satisfies LoginSuccess<T>;
+      result satisfies LoginSuccess;
       return { ok: true, value: result };
     },
   });
@@ -826,7 +832,7 @@ const useIdentityFlow = async <T, I>({
     return { ...pinResult };
   }
 
-  pinResult satisfies LoginSuccess<T>;
+  pinResult satisfies LoginSuccess;
 
   // We log in with an existing PIN anchor, meaning it is _not_ a new anchor
   return { newAnchor: false, authnMethod: "pin", ...pinResult };

diff --git a/src/frontend/src/flows/authorize/index.ts b/src/frontend/src/flows/authorize/index.ts
@@ -212,14 +212,17 @@ const authenticate = async (
   const derivationOrigin =
     authContext.authRequest.derivationOrigin ?? authContext.requestOrigin;
 
-  // TODO: Commit state
-  const result = await withLoader(() =>
-    fetchDelegation({
-      connection: authSuccess.connection,
-      derivationOrigin,
-      publicKey: authContext.authRequest.sessionPublicKey,
-      maxTimeToLive: authContext.authRequest.maxTimeToLive,
-    })
+  // Ignore the response of committing the metadata because it's not crucial.
+  const [result] = await withLoader(() =>
+    Promise.all([
+      fetchDelegation({
+        connection: authSuccess.connection,
+        derivationOrigin,
+        publicKey: authContext.authRequest.sessionPublicKey,
+        maxTimeToLive: authContext.authRequest.maxTimeToLive,
+      }),
+      authSuccess.connection.commitMetadata(),
+    ])
   );
 
   if ("error" in result) {

diff --git a/src/frontend/src/flows/manage/index.ts b/src/frontend/src/flows/manage/index.ts
@@ -250,8 +250,10 @@ export const renderManage = async ({
   for (;;) {
     let anchorInfo: IdentityAnchorInfo;
     try {
-      // TODO: commit state
-      anchorInfo = await withLoader(() => connection.getAnchorInfo());
+      // Ignore the `commitMetadata` response, it's not critical for the application.
+      [anchorInfo] = await withLoader(() =>
+        Promise.all([connection.getAnchorInfo(), connection.commitMetadata()])
+      );
     } catch (error: unknown) {
       await displayFailedToListDevices(
         error instanceof Error ? error : unknownError()
@@ -367,13 +369,6 @@ export const displayManage = (
         onAddDevice,
         addRecoveryPhrase,
         addRecoveryKey: async () => {
-          const confirmed = confirm(
-            "Add a Recovery Device\n\nUse a FIDO Security Key, like a YubiKey, as an additional recovery method."
-          );
-          if (!confirmed) {
-            // No resolve here because we don't need to reload the screen
-            return;
-          }
           await setupKey({ connection });
           resolve();
         },

diff --git a/src/frontend/src/flows/recovery/recoveryWizard.ts b/src/frontend/src/flows/recovery/recoveryWizard.ts
@@ -80,16 +80,31 @@ export const addPhrase = ({
   );
 };
 
+// TODO: Add e2e test https://dfinity.atlassian.net/browse/GIX-2600
 export const recoveryWizard = async (
   userNumber: bigint,
   connection: AuthenticatedConnection
 ): Promise<void> => {
   // Here, if the user doesn't have any recovery device, we prompt them to add
   // one.
-  const recoveries = await withLoader(() =>
-    connection.lookupRecovery(userNumber)
+  const [recoveries, identityMetadata] = await withLoader(() =>
+    Promise.all([
+      connection.lookupRecovery(userNumber),
+      connection.getIdentityMetadata(),
+    ])
   );
-  if (recoveries.length === 0) {
+
+  const ONE_WEEK_MILLIS = 7 * 24 * 60 * 60 * 1000;
+  const nowInMillis = Date.now();
+  const oneWeekAgoTimestamp = nowInMillis - ONE_WEEK_MILLIS;
+  const hasNotSeenRecoveryPageLastWeek =
+    (identityMetadata?.recoveryPageShownTimestampMillis ?? 0) <
+    oneWeekAgoTimestamp;
+  if (recoveries.length === 0 && hasNotSeenRecoveryPageLastWeek) {
+    // `await` here doesn't add any waiting time beacause we already got the metadata earlier.
+    await connection.updateIdentityMetadata({
+      recoveryPageShownTimestampMillis: nowInMillis,
+    });
     const doAdd = await addPhrase({ intent: "securityReminder" });
     if (doAdd !== "cancel") {
       doAdd satisfies "ok";

diff --git a/src/frontend/src/flows/register/index.ts b/src/frontend/src/flows/register/index.ts
@@ -20,7 +20,6 @@ import { authenticatorAttachmentToKeyType } from "$src/utils/authenticatorAttach
 import {
   ApiError,
   AuthFail,
-  AuthenticatedConnection,
   BadChallenge,
   Connection,
   IIWebAuthnIdentity,
@@ -37,7 +36,7 @@ import { displayUserNumberWarmup } from "./finish";
 import { savePasskeyOrPin } from "./passkey";
 
 /** Registration (anchor creation) flow for new users */
-export const registerFlow = async <T>({
+export const registerFlow = async ({
   createChallenge: createChallenge_,
   register,
   storePinIdentity,
@@ -53,7 +52,7 @@ export const registerFlow = async <T>({
     credentialId?: CredentialId;
     challengeResult: { chars: string; challenge: Challenge };
   }) => Promise<
-    LoginSuccess<T> | BadChallenge | ApiError | AuthFail | RegisterNoSpace
+    LoginSuccess | BadChallenge | ApiError | AuthFail | RegisterNoSpace
   >;
   storePinIdentity: (opts: {
     userNumber: bigint;
@@ -63,7 +62,7 @@ export const registerFlow = async <T>({
   pinAllowed: () => Promise<boolean>;
   uaParser: PreloadedUAParser;
 }): Promise<
-  | (LoginSuccess<T> & { authnMethod: "passkey" | "pin" })
+  | (LoginSuccess & { authnMethod: "passkey" | "pin" })
   | BadChallenge
   | ApiError
   | AuthFail
@@ -193,6 +192,13 @@ export const registerFlow = async <T>({
   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.
+  // The metadata starts to fetch when the connection is created.
+  // But it might not have finished yet, so we `await` for `updateIdentityMetadata` to also wait for it.
+  await result.connection.updateIdentityMetadata({
+    recoveryPageShownTimestampMillis: Date.now(),
+  });
   await setAnchorUsed(userNumber);
   await displayUserNumber({
     userNumber,
@@ -202,9 +208,7 @@ export const registerFlow = async <T>({
   return { ...result, authnMethod };
 };
 
-export type RegisterFlowOpts<T = AuthenticatedConnection> = Parameters<
-  typeof registerFlow<T>
->[0];
+export type RegisterFlowOpts = Parameters<typeof registerFlow>[0];
 
 export const getRegisterFlowOpts = ({
   connection,

diff --git a/src/frontend/src/repositories/identityMetadata.ts b/src/frontend/src/repositories/identityMetadata.ts
@@ -101,9 +101,9 @@ export class IdentityMetadataRepository {
     metadata: RawMetadataState
   ): metadata is MetadataMapV2 => {
     return (
-      this.rawMetadata !== "loading" &&
-      this.rawMetadata !== "error" &&
-      this.rawMetadata !== "not-loaded"
+      metadata !== "loading" &&
+      metadata !== "error" &&
+      metadata !== "not-loaded"
     );
   };
 

diff --git a/src/frontend/src/test-e2e/alternativeOrigin/endpointFormat.test.ts b/src/frontend/src/test-e2e/alternativeOrigin/endpointFormat.test.ts
@@ -21,7 +21,6 @@ test("Should not issue delegation when /.well-known/ii-alternative-origins has t
     const authenticatorId1 = await addVirtualAuthenticator(browser);
     await browser.url(II_URL);
     await FLOWS.registerNewIdentityWelcomeView(browser);
-    await FLOWS.addRecoveryMechanismSeedPhrase(browser);
     const credentials = await getWebAuthnCredentials(browser, authenticatorId1);
     expect(credentials).toHaveLength(1);
 
@@ -59,7 +58,6 @@ test("Should not follow redirect returned by /.well-known/ii-alternative-origins
     const authenticatorId1 = await addVirtualAuthenticator(browser);
     await browser.url(II_URL);
     await FLOWS.registerNewIdentityWelcomeView(browser);
-    await FLOWS.addRecoveryMechanismSeedPhrase(browser);
     const credentials = await getWebAuthnCredentials(browser, authenticatorId1);
     expect(credentials).toHaveLength(1);
 
@@ -97,7 +95,6 @@ test("Should fetch /.well-known/ii-alternative-origins using the non-raw url", a
     const authenticatorId1 = await addVirtualAuthenticator(browser);
     await browser.url(II_URL);
     const userNumber = await FLOWS.registerNewIdentityWelcomeView(browser);
-    await FLOWS.addRecoveryMechanismSeedPhrase(browser);
     const credentials = await getWebAuthnCredentials(browser, authenticatorId1);
     expect(credentials).toHaveLength(1);
 
@@ -142,7 +139,6 @@ test("Should allow arbitrary URL as derivation origin", async () => {
     const authenticatorId1 = await addVirtualAuthenticator(browser);
     await browser.url(II_URL);
     const userNumber = await FLOWS.registerNewIdentityWelcomeView(browser);
-    await FLOWS.addRecoveryMechanismSeedPhrase(browser);
     const credentials = await getWebAuthnCredentials(browser, authenticatorId1);
     expect(credentials).toHaveLength(1);
 

diff --git a/src/frontend/src/test-e2e/alternativeOrigin/ingressFormat.test.ts b/src/frontend/src/test-e2e/alternativeOrigin/ingressFormat.test.ts
@@ -20,7 +20,6 @@ test("Should not issue delegation when derivationOrigin is missing from /.well-k
     const authenticatorId1 = await addVirtualAuthenticator(browser);
     await browser.url(II_URL);
     const _userNumber = await FLOWS.registerNewIdentityWelcomeView(browser);
-    await FLOWS.addRecoveryMechanismSeedPhrase(browser);
     const credentials = await getWebAuthnCredentials(browser, authenticatorId1);
     expect(credentials).toHaveLength(1);