Skip to content

Commit

Permalink
Retry with differen rp id
Browse files Browse the repository at this point in the history
  • Loading branch information
lmuntaner committed Dec 17, 2024
1 parent 5e77adc commit 9debab8
Show file tree
Hide file tree
Showing 4 changed files with 252 additions and 10 deletions.
106 changes: 106 additions & 0 deletions src/frontend/src/utils/findWebAuthnRpId.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { CredentialData } from "./credential-devices";
import {
BETA_DOMAINS,
PROD_DOMAINS,
excludeCredentialsFromOrigins,
findWebAuthnRpId,
} from "./findWebAuthnRpId";

Expand Down Expand Up @@ -165,3 +166,108 @@ describe("findWebAuthnRpId", () => {
);
});
});

describe("excludeCredentialsFromOrigins", () => {
const mockDeviceData = (origin?: string): CredentialData => ({
origin,
credentialId: new ArrayBuffer(1),
pubkey: new ArrayBuffer(1),
});

test("excludes credentials from specified origins", () => {
const credentials = [
mockDeviceData("https://identity.ic0.app"),
mockDeviceData("https://identity.internetcomputer.org"),
mockDeviceData("https://identity.icp0.io"),
];
const originsToExclude = new Set(["https://identity.ic0.app"]);
const currentOrigin = "https://identity.internetcomputer.org";

const result = excludeCredentialsFromOrigins(
credentials,
originsToExclude,
currentOrigin
);

expect(result).toHaveLength(2);
expect(result).toEqual([
mockDeviceData("https://identity.internetcomputer.org"),
mockDeviceData("https://identity.icp0.io"),
]);
});

test("treats undefined credential origins as DEFAULT_DOMAIN", () => {
const credentials = [
mockDeviceData(undefined), // Should be treated as DEFAULT_DOMAIN
mockDeviceData("https://identity.internetcomputer.org"),
];
const originsToExclude = new Set(["https://identity.ic0.app"]); // Should match DEFAULT_DOMAIN
const currentOrigin = "https://identity.internetcomputer.org";

const result = excludeCredentialsFromOrigins(
credentials,
originsToExclude,
currentOrigin
);

expect(result).toHaveLength(1);
expect(result).toEqual([
mockDeviceData("https://identity.internetcomputer.org"),
]);
});

test("treats undefined origins in exclusion set as currentOrigin", () => {
const credentials = [
mockDeviceData("https://identity.ic0.app"),
mockDeviceData("https://identity.internetcomputer.org"),
];
const originsToExclude = new Set([undefined]); // Should be treated as currentOrigin
const currentOrigin = "https://identity.internetcomputer.org";

const result = excludeCredentialsFromOrigins(
credentials,
originsToExclude,
currentOrigin
);

expect(result).toHaveLength(1);
expect(result).toEqual([mockDeviceData("https://identity.ic0.app")]);
});

test("returns empty array when all credentials are excluded", () => {
const credentials = [
mockDeviceData("https://identity.ic0.app"),
mockDeviceData("https://identity.internetcomputer.org"),
];
const originsToExclude = new Set([
"https://identity.ic0.app",
"https://identity.internetcomputer.org",
]);
const currentOrigin = "https://identity.ic0.app";

const result = excludeCredentialsFromOrigins(
credentials,
originsToExclude,
currentOrigin
);

expect(result).toHaveLength(0);
});

test("returns all credentials when no origins to exclude", () => {
const credentials = [
mockDeviceData("https://identity.ic0.app"),
mockDeviceData("https://identity.internetcomputer.org"),
];
const originsToExclude = new Set<string>();
const currentOrigin = "https://identity.ic0.app";

const result = excludeCredentialsFromOrigins(
credentials,
originsToExclude,
currentOrigin
);

expect(result).toEqual(credentials);
});
});
37 changes: 37 additions & 0 deletions src/frontend/src/utils/findWebAuthnRpId.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,43 @@ export const relatedDomains = (): string[] => {
return [];
};

export const hasCredentialsFromMultipleOrigins = (
credentials: CredentialData[]
): boolean =>
new Set(credentials.map(({ origin }) => origin ?? DEFAULT_DOMAIN)).size > 1;

/**
* Filters out credentials from specific origins.
*
* This function takes a list of credentials and removes any that match the provided origins.
* If a credential has no origin (undefined), it is treated as if it had the `DEFAULT_DOMAIN`.
* Two origins match if they have the same hostname (domain).
*
* @param credentials - List of credential devices to filter
* @param origins - Set of origins to exclude (undefined values are treated as `currentOrigin`)
* @param currentOrigin - The current origin to use when comparing against undefined origins
* @returns Filtered list of credentials, excluding those from the specified origins
*/
export const excludeCredentialsFromOrigins = (
credentials: CredentialData[],
origins: Set<string | undefined>,
currentOrigin: string
): CredentialData[] => {
if (origins.size === 0) {
return credentials;
}
// Change `undefined` to the current origin.
const originsToExclude = Array.from(origins).map(
(origin) => origin ?? currentOrigin
);
return credentials.filter(
(credential) =>
originsToExclude.filter((originToExclude) =>
sameDomain(credential.origin ?? DEFAULT_DOMAIN, originToExclude)
).length === 0
);
};

const sameDomain = (url1: string, url2: string): boolean =>
new URL(url1).hostname === new URL(url2).hostname;

Expand Down
82 changes: 80 additions & 2 deletions src/frontend/src/utils/iiConnection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,13 @@ const mockActor = {
lookup: vi.fn().mockResolvedValue([mockDevice]),
} as unknown as ActorSubclass<_SERVICE>;

const currentOrigin = "https://identity.internetcomputer.org";

beforeEach(() => {
infoResponse = undefined;
vi.clearAllMocks();
vi.stubGlobal("location", {
origin: "https://identity.internetcomputer.org",
origin: currentOrigin,
});
DOMAIN_COMPATIBILITY.reset();
});
Expand Down Expand Up @@ -112,6 +114,7 @@ test("commits changes on identity metadata", async () => {
});

describe("Connection.login", () => {
let failSign = false;
beforeEach(() => {
vi.spyOn(MultiWebAuthnIdentity, "fromCredentials").mockImplementation(
() => {
Expand All @@ -133,6 +136,9 @@ describe("Connection.login", () => {
return new MockMultiWebAuthnIdentity(credentials, rpId);
}
override sign() {
if (failSign) {
throw new DOMException("Error test", "NotAllowedError");
}
this._actualIdentity = mockIdentity;
return Promise.resolve(new ArrayBuffer(0) as Signature);
}
Expand Down Expand Up @@ -189,7 +195,7 @@ describe("Connection.login", () => {
it("login returns authenticated connection without rpID if browser doesn't support it", async () => {
DOMAIN_COMPATIBILITY.set(true);
vi.stubGlobal("navigator", {
// Supports RoR
// Does NOT Supports RoR
userAgent:
"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:133.0) Gecko/20100101 Firefox/133.0",
});
Expand All @@ -207,4 +213,76 @@ describe("Connection.login", () => {
);
}
});

it("connection exludes rpId when user cancels", async () => {
// This one will fail because it's not the device the user is using at the moment.
const currentOriginDevice: DeviceData = {
alias: "mockDevice",
metadata: [],
origin: [currentOrigin],
protection: { protected: null },
pubkey: new Uint8Array(),
key_type: { platform: null },
purpose: { authentication: null },
credential_id: [],
};
const currentOriginCredentialData =
convertToCredentialData(currentOriginDevice);
const currentDevice: DeviceData = {
alias: "mockDevice",
metadata: [],
origin: [],
protection: { protected: null },
pubkey: new Uint8Array(),
key_type: { platform: null },
purpose: { authentication: null },
credential_id: [],
};
const currentDeviceCredentialData = convertToCredentialData(currentDevice);
const mockActor = {
identity_info: vi.fn().mockResolvedValue({ Ok: { metadata: [] } }),
lookup: vi.fn().mockResolvedValue([currentOriginDevice, currentDevice]),
} as unknown as ActorSubclass<_SERVICE>;
DOMAIN_COMPATIBILITY.set(true);
vi.stubGlobal("navigator", {
// Supports RoR
userAgent:
"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.0 Safari/605.1.15",
});
const connection = new Connection("aaaaa-aa", mockActor);

failSign = true;
const firstLoginResult = await connection.login(BigInt(12345));

expect(firstLoginResult.kind).toBe("webAuthnFailed");
expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledTimes(1);
expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledWith(
expect.arrayContaining([
currentOriginCredentialData,
currentDeviceCredentialData,
]),
undefined
);

failSign = false;
const secondLoginResult = await connection.login(BigInt(12345));

expect(secondLoginResult.kind).toBe("loginSuccess");
if (secondLoginResult.kind === "loginSuccess") {
expect(secondLoginResult.connection).toBeInstanceOf(
AuthenticatedConnection
);
expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledTimes(2);
expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenNthCalledWith(
2,
expect.arrayContaining([currentDeviceCredentialData]),
"identity.ic0.app"
);
expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenNthCalledWith(
2,
expect.not.arrayContaining([currentOriginCredentialData]),
"identity.ic0.app"
);
}
});
});
37 changes: 29 additions & 8 deletions src/frontend/src/utils/iiConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,12 @@ import {
import { Principal } from "@dfinity/principal";
import { isNullish, nonNullish } from "@dfinity/utils";
import { convertToCredentialData, CredentialData } from "./credential-devices";
import { findWebAuthnRpId, relatedDomains } from "./findWebAuthnRpId";
import {
excludeCredentialsFromOrigins,
findWebAuthnRpId,
hasCredentialsFromMultipleOrigins,
relatedDomains,
} from "./findWebAuthnRpId";
import { MultiWebAuthnIdentity } from "./multiWebAuthnIdentity";
import { isRecoveryDevice, RecoveryDevice } from "./recoveryDevice";
import { supportsWebauthRoR } from "./userAgent";
Expand Down Expand Up @@ -137,6 +142,12 @@ export interface IIWebAuthnIdentity extends SignIdentity {
}

export class Connection {
// The rpID is used to get the passkey from the browser's WebAuthn API.
// Using different rpIDs allows us to have compatibility across multiple domains.
// However, when one RP ID is used and the user cancels, it must be because the user is in a device
// registered in another domain. In this case, we must try the other rpID.
// Map<userNumber, Set<rpID>>
private _cancelledRpIds: Map<bigint, Set<string | undefined>> = new Map();
public constructor(
readonly canisterId: string,
// Used for testing purposes
Expand Down Expand Up @@ -376,15 +387,17 @@ export class Connection {
userNumber: bigint,
credentials: CredentialData[]
): Promise<LoginSuccess | WebAuthnFailed | AuthFail> => {
// TODO: Filter out the credentials from the used rpIDs.
const cancelledRpIds = this._cancelledRpIds.get(userNumber) ?? new Set();
const currentOrigin = window.location.origin;
const filteredCredentials = excludeCredentialsFromOrigins(
credentials,
cancelledRpIds,
currentOrigin
);
const rpId =
DOMAIN_COMPATIBILITY.isEnabled() &&
supportsWebauthRoR(window.navigator.userAgent)
? findWebAuthnRpId(
window.location.origin,
credentials,
relatedDomains()
)
? findWebAuthnRpId(currentOrigin, filteredCredentials, relatedDomains())
: undefined;

/* Recover the Identity (i.e. key pair) used when creating the anchor.
Expand All @@ -393,7 +406,7 @@ export class Connection {
*/
const identity = features.DUMMY_AUTH
? new DummyIdentity()
: MultiWebAuthnIdentity.fromCredentials(credentials, rpId);
: MultiWebAuthnIdentity.fromCredentials(filteredCredentials, rpId);
let delegationIdentity: DelegationIdentity;

// Here we expect a webauth exception if the user canceled the webauthn prompt (triggered by
Expand All @@ -402,6 +415,14 @@ export class Connection {
delegationIdentity = await this.requestFEDelegation(identity);
} catch (e: unknown) {
if (isWebAuthnCancel(e)) {
// We only want to cache cancelled rpids if there can be multiple rpids.
if (hasCredentialsFromMultipleOrigins(credentials)) {
if (this._cancelledRpIds.has(userNumber)) {
this._cancelledRpIds.get(userNumber)?.add(rpId);
} else {
this._cancelledRpIds.set(userNumber, new Set([rpId]));
}
}
return { kind: "webAuthnFailed" };
}

Expand Down

0 comments on commit 9debab8

Please sign in to comment.