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.

Sign up for GitHub

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

[PM-3807] All passkeys as login ciphers - Minimal implementation to minimize blockers #6233

Merged
merged 7 commits into from
Sep 11, 2023
Merged

[PM-3807] All passkeys as login ciphers - Minimal implementation to minimize blockers #6233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
84c8173
[PM-3807] feat: remove non-discoverable from fido2 user interface class
coroiu Sep 8, 2023
119fe9b
[PM-3807] feat: merge fido2 component ui
coroiu Sep 8, 2023
7effcd4
[PM-3807] feat: return `cipherId` from user interface
coroiu Sep 8, 2023
d074b9a
[PM-3807] feat: merge credential creation logic in authenticator
coroiu Sep 8, 2023
0cf7641
[PM-3807] feat: merge credential assertion logic in authenticator
coroiu Sep 8, 2023
474cbfd
Merge branch 'EC-598-beeep-properly-store-passkeys-in-bitwarden' into…
gbubemismith Sep 8, 2023
95eef7a
Merge branch 'EC-598-beeep-properly-store-passkeys-in-bitwarden' into…
gbubemismith Sep 10, 2023
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
33 changes: 1 addition & 32 deletions apps/browser/src/services/fido2/browser-fido2-user-interface.service.ts
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be owned by a team? Removes the need for tech-leads to review this pr.

Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,6 @@ export type BrowserFido2Message = { sessionId: string } & (
}
| {
type: "ConfirmNewCredentialResponse";
userVerified: boolean;
}
| {
type: "ConfirmNewNonDiscoverableCredentialRequest";
credentialName: string;
userName: string;
userVerification: boolean;
fallbackSupported: boolean;
}
| {
type: "ConfirmNewNonDiscoverableCredentialResponse";
cipherId: string;
userVerified: boolean;
}
Expand Down Expand Up @@ -213,7 +202,7 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi
credentialName,
userName,
userVerification,
}: NewCredentialParams): Promise<{ confirmed: boolean; userVerified: boolean }> {
}: NewCredentialParams): Promise<{ cipherId: string; userVerified: boolean }> {
const data: BrowserFido2Message = {
type: "ConfirmNewCredentialRequest",
sessionId: this.sessionId,
Expand All @@ -226,26 +215,6 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi
await this.send(data);
const response = await this.receive("ConfirmNewCredentialResponse");

return { confirmed: true, userVerified: response.userVerified };
}

async confirmNewNonDiscoverableCredential({
credentialName,
userName,
userVerification,
}: NewCredentialParams): Promise<{ cipherId: string; userVerified: boolean }> {
const data: BrowserFido2Message = {
type: "ConfirmNewNonDiscoverableCredentialRequest",
sessionId: this.sessionId,
credentialName,
userName,
userVerification,
fallbackSupported: this.fallbackSupported,
};

await this.send(data);
const response = await this.receive("ConfirmNewNonDiscoverableCredentialResponse");

return { cipherId: response.cipherId, userVerified: response.userVerified };
}

Expand Down
11 changes: 1 addition & 10 deletions apps/browser/src/vault/popup/components/fido2/fido2.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<ng-container
*ngIf="
data.message.type == 'PickCredentialRequest' ||
data.message.type == 'ConfirmNewNonDiscoverableCredentialRequest'
data.message.type == 'ConfirmNewCredentialRequest'
"
>
A site is asking for authentication, please choose one of the following credentials to use:
Expand All @@ -24,15 +24,6 @@
</div>
</div>
</ng-container>
<ng-container *ngIf="data.message.type == 'ConfirmNewCredentialRequest'">
A site wants to create the following passkey in your vault
<div class="box list">
<div class="box-content">
<app-cipher-row [cipher]="ciphers[0]"></app-cipher-row>
</div>
</div>
<button type="button" class="btn btn-outline-secondary" (click)="confirm()">Create</button>
</ng-container>
<ng-container *ngIf="data.message.type == 'InformExcludedCredentialRequest'">
A passkey already exists in Bitwarden for this account
<div class="box list">
Expand Down
37 changes: 5 additions & 32 deletions apps/browser/src/vault/popup/components/fido2/fido2.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.servi
import { PasswordRepromptService } from "@bitwarden/common/vault/abstractions/password-reprompt.service";
import { CipherType } from "@bitwarden/common/vault/enums/cipher-type";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { Fido2KeyView } from "@bitwarden/common/vault/models/view/fido2-key.view";

import { BrowserApi } from "../../../../platform/browser/browser-api";
import {
Expand Down Expand Up @@ -80,23 +79,16 @@ export class Fido2Component implements OnInit, OnDestroy {
filter((message) => message != undefined),
concatMap(async (message) => {
if (message.type === "ConfirmNewCredentialRequest") {
const cipher = new CipherView();
cipher.name = message.credentialName;
cipher.type = CipherType.Fido2Key;
cipher.fido2Key = new Fido2KeyView();
cipher.fido2Key.userDisplayName = message.userName;
this.ciphers = [cipher];
this.ciphers = (await this.cipherService.getAllDecrypted()).filter(
(cipher) => cipher.type === CipherType.Login && !cipher.isDeleted
);
} else if (message.type === "PickCredentialRequest") {
this.ciphers = await Promise.all(
message.cipherIds.map(async (cipherId) => {
const cipher = await this.cipherService.get(cipherId);
return cipher.decrypt();
})
);
} else if (message.type === "ConfirmNewNonDiscoverableCredentialRequest") {
this.ciphers = (await this.cipherService.getAllDecrypted()).filter(
(cipher) => cipher.type === CipherType.Login && !cipher.isDeleted
);
} else if (message.type === "InformExcludedCredentialRequest") {
this.ciphers = await Promise.all(
message.existingCipherIds.map(async (cipherId) => {
Expand Down Expand Up @@ -140,7 +132,7 @@ export class Fido2Component implements OnInit, OnDestroy {
type: "PickCredentialResponse",
userVerified,
});
} else if (data?.type === "ConfirmNewNonDiscoverableCredentialRequest") {
} else if (data?.type === "ConfirmNewCredentialRequest") {
let userVerified = false;
if (data.userVerification) {
userVerified = await this.passwordRepromptService.showPasswordPrompt();
Expand All @@ -149,33 +141,14 @@ export class Fido2Component implements OnInit, OnDestroy {
this.send({
sessionId: this.sessionId,
cipherId: cipher.id,
type: "ConfirmNewNonDiscoverableCredentialResponse",
type: "ConfirmNewCredentialResponse",
userVerified,
});
}

this.loading = true;
}

async confirm() {
const data = this.message$.value;
if (data.type !== "ConfirmNewCredentialRequest") {
return;
}

let userVerified = false;
if (data.userVerification) {
userVerified = await this.passwordRepromptService.showPasswordPrompt();
}

this.send({
sessionId: this.sessionId,
type: "ConfirmNewCredentialResponse",
userVerified,
});
this.loading = true;
}

abort(fallback: boolean) {
this.unload(fallback);
window.close();
Expand Down
4 changes: 0 additions & 4 deletions libs/common/src/vault/abstractions/fido2/fido2-user-interface.service.abstraction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ export abstract class Fido2UserInterfaceSession {
confirmNewCredential: (
params: NewCredentialParams,
abortController?: AbortController
) => Promise<{ confirmed: boolean; userVerified: boolean }>;
confirmNewNonDiscoverableCredential: (
params: NewCredentialParams,
abortController?: AbortController
) => Promise<{ cipherId: string; userVerified: boolean }>;
informExcludedCredential: (
existingCipherIds: string[],
Expand Down
Loading