Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create WebAuthn flows based on device order (recently used). #2883

Merged
merged 5 commits into from
Feb 26, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/frontend/src/utils/findWebAuthnFlows.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,4 +128,25 @@ describe("findWebAuthnFlows", () => {

expect(result).toEqual([{ useIframe: false, rpId: undefined }]);
});

it("should return flows in order of devices (recently used)", () => {
const result = findWebAuthnFlows({
supportsRor: true,
devices: [
createMockCredential(nonCurrentOrigin2),
createMockCredential(currentOrigin),
createMockCredential(currentOrigin),
createMockCredential(nonCurrentOrigin1),
createMockCredential(nonCurrentOrigin2),
],
currentOrigin: currentOrigin,
relatedOrigins: [currentOrigin, nonCurrentOrigin1, nonCurrentOrigin2],
});

expect(result).toEqual([
{ useIframe: true, rpId: nonCurrentOrigin2RpId },
{ useIframe: false, rpId: undefined },
{ useIframe: true, rpId: nonCurrentOrigin1RpId },
]);
});
});
69 changes: 27 additions & 42 deletions src/frontend/src/utils/findWebAuthnFlows.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { II_LEGACY_ORIGIN } from "$src/constants";
import { isNullish, nonNullish } from "@dfinity/utils";
import { CredentialData } from "./credential-devices";
import {
excludeCredentialsFromOrigins,
findWebAuthnRpId,
} from "./findWebAuthnRpId";

export type WebAuthnFlow = {
useIframe: boolean;
Expand All @@ -26,9 +24,7 @@ type Parameters = {
* - Which RP ID to use. This is used for the iframe or for Related Origin Requests.
*
* Logic:
* - To calculate the RP ID, we use the `findWebAuthnRpId` function.
* - Calculate the RP ID first with all the credentials.
* - For the subsequent RP IDs, the credentials' origin that matches the previous RP ID will be excluded.
* - To calculate the RP IDs, we look for all RP IDs within the devices
* - At the moment, we only use non-iframe if the RP ID matches the current origin. to avoid bad UX, if the RP ID doesn't match the current origin, the iframe will be used.
*
* @param {Parameters} params - The parameters to find the webauthn steps.
Expand All @@ -39,46 +35,35 @@ export const findWebAuthnFlows = ({
currentOrigin,
relatedOrigins,
}: Parameters): WebAuthnFlow[] => {
const steps: WebAuthnFlow[] = [];
let filteredCredentials = [...devices];
const rpIds = new Set<string | undefined>();
const currentRpId = new URL(currentOrigin).hostname;
const relatedRpIds = relatedOrigins.map(
(relatedOrigin) => new URL(relatedOrigin).hostname
);

while (filteredCredentials.length > 0) {
const rpId = findWebAuthnRpId(
currentOrigin,
filteredCredentials,
relatedOrigins
);
// The devices are expected to be ordered by recently used first
const deviceRpIds = [
...new Set(
devices
// Device origin to RP ID (hostname)
.map((device) =>
device.origin !== currentOrigin && currentOrigin !== II_LEGACY_ORIGIN
? new URL(device.origin ?? II_LEGACY_ORIGIN).hostname
: undefined
)
// Filter out RP IDs that are not within `relatedRpIds`
.filter((rpId) => isNullish(rpId) || relatedRpIds.includes(rpId))
),
];

// EXCEPTION: At the moment, to avoid bad UX, if the RP ID doesn't match the current origin, the iframe will be used.
// This is because it's hard to find out whether a user's credentials come from a third party password manager or not.
// The iframe workaround works for all users.
const useIframe =
rpId !== undefined && rpId !== new URL(currentOrigin).hostname;
// Create steps from `deviceRpIds`, currently that's one step per RP ID
const steps: WebAuthnFlow[] = deviceRpIds.map((rpId) => ({
rpId,
useIframe: nonNullish(rpId) && rpId !== currentRpId,
}));

const isRepeatedStep = steps.some(
(step) => step.rpId === rpId && step.useIframe === useIframe
);
// Exit when the flow is the same to avoid infinite loops.
if (isRepeatedStep) {
break;
}

steps.push({ useIframe, rpId });
rpIds.add(rpId);
filteredCredentials = excludeCredentialsFromOrigins(
filteredCredentials,
rpIds,
currentOrigin
);
}

// One that doesn't use any new domain.
const defaultStep = { useIframe: false, rpId: undefined };
// If there are no steps, add a default step.
if (steps.length === 0) {
steps.push(defaultStep);
steps.push({ useIframe: false, rpId: undefined });
}

return steps;
};
Loading
Loading