From 392f2112d1148bb71fee59f2d0d578cd7e46b06f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lloren=C3=A7?= Date: Tue, 28 Jan 2025 16:28:02 +0100 Subject: [PATCH] Clean up cancelled RP IDs if no filtered credentials --- src/frontend/src/utils/iiConnection.test.ts | 82 ++++++++++++++++++--- src/frontend/src/utils/iiConnection.ts | 37 +++++++--- 2 files changed, 95 insertions(+), 24 deletions(-) diff --git a/src/frontend/src/utils/iiConnection.test.ts b/src/frontend/src/utils/iiConnection.test.ts index 8ae6bc626a..bc89a197c6 100644 --- a/src/frontend/src/utils/iiConnection.test.ts +++ b/src/frontend/src/utils/iiConnection.test.ts @@ -241,6 +241,69 @@ describe("Connection.login", () => { } }); + it("connection cleans up cancelled RP IDs if no credentials when user cancels in a valid RP ID", async () => { + // This one would fail because it's not the device the user is using at the moment. + const currentOriginDevice: DeviceData = createMockDevice(currentOrigin); + const currentOriginCredentialData = + convertToValidCredentialData(currentOriginDevice); + const currentDevice: DeviceData = createMockDevice(); + const currentDeviceCredentialData = + convertToValidCredentialData(currentDevice); + const mockActor = { + identity_info: vi.fn().mockResolvedValue({ Ok: { metadata: [] } }), + lookup: vi.fn().mockResolvedValue([currentOriginDevice, currentDevice]), + } as unknown as ActorSubclass<_SERVICE>; + const connection = new Connection("aaaaa-aa", mockActor); + + // First try. This is the right RP ID, but the user cancelled manually + failSign = true; + const firstLoginResult = await connection.login(BigInt(12345)); + + expect(firstLoginResult.kind).toBe("possiblyWrongRPID"); + expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledTimes(1); + expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledWith( + expect.arrayContaining([ + currentOriginCredentialData, + currentDeviceCredentialData, + ]), + undefined + ); + + failSign = true; + const secondLoginResult = await connection.login(BigInt(12345)); + + expect(secondLoginResult.kind).toBe("possiblyWrongRPID"); + expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenNthCalledWith( + 2, + expect.arrayContaining([ + currentOriginCredentialData, + currentDeviceCredentialData, + ]), + "identity.ic0.app" + ); + + failSign = false; + const thirdLoginResult = await connection.login(BigInt(12345)); + + expect(thirdLoginResult.kind).toBe("loginSuccess"); + if (thirdLoginResult.kind === "loginSuccess") { + expect(thirdLoginResult.showAddCurrentDevice).toBe(false); + expect(thirdLoginResult.connection).toBeInstanceOf( + AuthenticatedConnection + ); + expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledTimes(3); + expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenNthCalledWith( + 3, + expect.arrayContaining([ + currentDeviceCredentialData, + currentDeviceCredentialData, + ]), + // The same RP ID as in the first login + undefined + ); + } + }); + // Test that the cancelled RP IDs are persisted across browser refrheses it("connection excludes rpId when user cancels after new Conection is created", async () => { // This one would fail because it's not the device the user is using at the moment. @@ -452,19 +515,14 @@ describe("Connection.login", () => { 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." - ) + const loginResult = await connection.fromWebauthnCredentials( + userNumber, + [credentialDataFromCurrentDomain], + !skipCancelledRpIdsStorage ); + + // The user cancelled, but we can't know why. + expect(loginResult.kind).toBe("webAuthnFailed"); }); it("doesn't persist the cancelled RP ID if skipCancelledRpIdsStorage is true", async () => { diff --git a/src/frontend/src/utils/iiConnection.ts b/src/frontend/src/utils/iiConnection.ts index 0afd8bc005..4b80b908cc 100644 --- a/src/frontend/src/utils/iiConnection.ts +++ b/src/frontend/src/utils/iiConnection.ts @@ -37,7 +37,11 @@ import { IdentityMetadata, IdentityMetadataRepository, } from "$src/repositories/identityMetadata"; -import { addAnchorCancelledRpId, getCancelledRpIds } from "$src/storage"; +import { + addAnchorCancelledRpId, + cleanUpRpIdMapper, + getCancelledRpIds, +} from "$src/storage"; import { CanisterError, diagnosticInfo, @@ -409,26 +413,35 @@ export class Connection { credentials: CredentialData[], skipCancelledRpIdsStorage = false ): Promise => { + let cancelledRpIds: Set; // Get cancelled rpids for the user from local storage. - const { cancelledRpIds, lastShownAddCurrentDevicePage } = - skipCancelledRpIdsStorage - ? { - cancelledRpIds: new Set(), - lastShownAddCurrentDevicePage: undefined, - } - : await getCancelledRpIds({ - userNumber, - origin: window.location.origin, - }); + const persistedData = skipCancelledRpIdsStorage + ? { + cancelledRpIds: new Set(), + lastShownAddCurrentDevicePage: undefined, + } + : await getCancelledRpIds({ + userNumber, + origin: window.location.origin, + }); + cancelledRpIds = persistedData.cancelledRpIds; + const lastShownAddCurrentDevicePage = + persistedData.lastShownAddCurrentDevicePage; const currentOrigin = window.location.origin; const dynamicRPIdEnabled = DOMAIN_COMPATIBILITY.isEnabled() && supportsWebauthRoR(window.navigator.userAgent); - const filteredCredentials = excludeCredentialsFromOrigins( + let filteredCredentials = excludeCredentialsFromOrigins( credentials, cancelledRpIds, currentOrigin ); + // It probably means that the user cancelled a valid RP ID manually + if (filteredCredentials.length === 0) { + await cleanUpRpIdMapper(userNumber); + cancelledRpIds = new Set(); + filteredCredentials = credentials; + } const rpId = dynamicRPIdEnabled ? findWebAuthnRpId(currentOrigin, filteredCredentials, relatedDomains()) : undefined;