diff --git a/src/frontend/src/flows/recovery/recoverWith/device.ts b/src/frontend/src/flows/recovery/recoverWith/device.ts index 2953d2602a..1764a52e91 100644 --- a/src/frontend/src/flows/recovery/recoverWith/device.ts +++ b/src/frontend/src/flows/recovery/recoverWith/device.ts @@ -115,5 +115,12 @@ const attemptRecovery = async ({ .map(convertToValidCredentialData) .filter(nonNullish); - return await connection.fromWebauthnCredentials(userNumber, credentialData); + // Cancelled RP IDs are based on the other credentials, we don't want them for the recovery device. + // If we had multiple recovery devices, we would need to handle this differently. + const skipCancelledRpIdsStorage = true; + return await connection.fromWebauthnCredentials( + userNumber, + credentialData, + skipCancelledRpIdsStorage + ); }; diff --git a/src/frontend/src/utils/iiConnection.test.ts b/src/frontend/src/utils/iiConnection.test.ts index 226fab0aaa..f1ef96ce00 100644 --- a/src/frontend/src/utils/iiConnection.test.ts +++ b/src/frontend/src/utils/iiConnection.test.ts @@ -8,7 +8,11 @@ import { IdentityMetadata, RECOVERY_PAGE_SHOW_TIMESTAMP_MILLIS, } from "$src/repositories/identityMetadata"; -import { setLastShownAddCurrentDevicePage } from "$src/storage"; +import { + addAnchorCancelledRpId, + getCancelledRpIds, + setLastShownAddCurrentDevicePage, +} from "$src/storage"; import { ActorSubclass, DerEncodedPublicKey, Signature } from "@dfinity/agent"; import { DelegationIdentity, WebAuthnIdentity } from "@dfinity/identity"; import { IDBFactory } from "fake-indexeddb"; @@ -394,6 +398,111 @@ describe("Connection.login", () => { ); } }); + + describe("Connection.fromWebauthnCredentials", () => { + const userNumber = BigInt(12345); + const deviceFromCurrentDomain: DeviceData = + createMockDevice(currentOrigin); + const credentialDataFromCurrentDomain = convertToValidCredentialData( + deviceFromCurrentDomain + ) as CredentialData; + const deviceAnotherDomain: DeviceData = createMockDevice( + "htts://identity.ic0.app" + ); + const credentialDataAnotherDomain = convertToValidCredentialData( + deviceAnotherDomain + ) as CredentialData; + const skipCancelledRpIdsStorage = true; + + it("doesn't use the cancelled RP ID if skipCancelledRpIdsStorage is true", async () => { + const cancelledRpId = new URL(currentOrigin).hostname; + await addAnchorCancelledRpId({ + userNumber, + origin: currentOrigin, + cancelledRpId, + }); + const connection = new Connection("aaaaa-aa", mockActor); + + const loginResult = await connection.fromWebauthnCredentials( + userNumber, + [credentialDataFromCurrentDomain], + skipCancelledRpIdsStorage + ); + + expect(loginResult.kind).toBe("loginSuccess"); + if (loginResult.kind === "loginSuccess") { + expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledTimes( + 1 + ); + expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledWith( + [credentialDataFromCurrentDomain], + // `undefined` means the current origin which is the one that was cancelled + undefined + ); + } + }); + + it("uses the cancelled RP ID if skipCancelledRpIdsStorage is false", async () => { + const cancelledRpId = new URL(currentOrigin).hostname; + await addAnchorCancelledRpId({ + userNumber, + origin: currentOrigin, + cancelledRpId, + }); + const connection = new Connection("aaaaa-aa", mockActor); + + failSign = true; + const call = () => + connection.fromWebauthnCredentials( + userNumber, + [credentialDataFromCurrentDomain], + !skipCancelledRpIdsStorage + ); + + // This is because the device is filtered out and then the `findWebAuthnRpId` doesn't receive any device. + await expect(call).rejects.toThrowError( + new Error( + "Not possible. Every registered user has at least one device." + ) + ); + }); + + it("doesn't persist the cancelled RP ID if skipCancelledRpIdsStorage is true", async () => { + const connection = new Connection("aaaaa-aa", mockActor); + + failSign = true; + const loginResult = await connection.fromWebauthnCredentials( + userNumber, + [credentialDataFromCurrentDomain, credentialDataAnotherDomain], + skipCancelledRpIdsStorage + ); + + expect(loginResult.kind).toBe("webAuthnFailed"); + const { cancelledRpIds } = await getCancelledRpIds({ + userNumber, + origin: currentOrigin, + }); + expect(cancelledRpIds).toEqual(new Set()); + }); + + it("doesn't persist the cancelled RP ID if skipCancelledRpIdsStorage is false", async () => { + const connection = new Connection("aaaaa-aa", mockActor); + + failSign = true; + const loginResult = await connection.fromWebauthnCredentials( + userNumber, + [credentialDataFromCurrentDomain, credentialDataAnotherDomain], + !skipCancelledRpIdsStorage + ); + + expect(loginResult.kind).toBe("possiblyWrongRPID"); + const { cancelledRpIds } = await getCancelledRpIds({ + userNumber, + origin: currentOrigin, + }); + expect(cancelledRpIds).toEqual(new Set([undefined])); + }); + }); }); describe("domains compatibility flag enabled and browser doesn't support", () => { diff --git a/src/frontend/src/utils/iiConnection.ts b/src/frontend/src/utils/iiConnection.ts index 0cfe842c3f..a1ac42a53a 100644 --- a/src/frontend/src/utils/iiConnection.ts +++ b/src/frontend/src/utils/iiConnection.ts @@ -402,14 +402,20 @@ export class Connection { fromWebauthnCredentials = async ( userNumber: bigint, - credentials: CredentialData[] + credentials: CredentialData[], + skipCancelledRpIdsStorage = false ): Promise => { // Get cancelled rpids for the user from local storage. const { cancelledRpIds, lastShownAddCurrentDevicePage } = - await getCancelledRpIds({ - userNumber, - origin: window.location.origin, - }); + skipCancelledRpIdsStorage + ? { + cancelledRpIds: new Set(), + lastShownAddCurrentDevicePage: undefined, + } + : await getCancelledRpIds({ + userNumber, + origin: window.location.origin, + }); const currentOrigin = window.location.origin; const dynamicRPIdEnabled = DOMAIN_COMPATIBILITY.isEnabled() && @@ -442,6 +448,7 @@ export class Connection { // We only want to cache cancelled rpids if there can be multiple rpids. if ( dynamicRPIdEnabled && + !skipCancelledRpIdsStorage && hasCredentialsFromMultipleOrigins(credentials) ) { try {