Skip to content

Commit

Permalink
Clean up cancelled RP IDs if no filtered credentials
Browse files Browse the repository at this point in the history
  • Loading branch information
lmuntaner committed Jan 28, 2025
1 parent f53b8eb commit 392f211
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 24 deletions.
82 changes: 70 additions & 12 deletions src/frontend/src/utils/iiConnection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 () => {
Expand Down
37 changes: 25 additions & 12 deletions src/frontend/src/utils/iiConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -409,26 +413,35 @@ export class Connection {
credentials: CredentialData[],
skipCancelledRpIdsStorage = false
): Promise<LoginSuccess | WebAuthnFailed | PossiblyWrongRPID | AuthFail> => {
let cancelledRpIds: Set<string | undefined>;
// Get cancelled rpids for the user from local storage.
const { cancelledRpIds, lastShownAddCurrentDevicePage } =
skipCancelledRpIdsStorage
? {
cancelledRpIds: new Set<string>(),
lastShownAddCurrentDevicePage: undefined,
}
: await getCancelledRpIds({
userNumber,
origin: window.location.origin,
});
const persistedData = skipCancelledRpIdsStorage
? {
cancelledRpIds: new Set<string>(),
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<string | undefined>();
filteredCredentials = credentials;
}
const rpId = dynamicRPIdEnabled
? findWebAuthnRpId(currentOrigin, filteredCredentials, relatedDomains())
: undefined;
Expand Down

0 comments on commit 392f211

Please sign in to comment.