vaultwarden
/
bitwarden-estensione-browser
mirror of https://github.com/bitwarden/browser
1
0
Fork 0
Code Issues 1 Releases Wiki Activity

[PM-4577] Enhance passkey user verification to use configured unlock methods (#8746) 

* initial commit

* fixed issue with clearing search index state

* clear user index before account is totally cleaned up

* added logout clear on option

* removed redundant clear index from logout

* Implemented user verification logic for the different use cases, added functions to pompt for user to set pin

* added missing await and removed else if conditionals

* fixed no return after user sets pin

* added comment to further explain user verification when user is coming from lock screen

* [PM-7836] UV not properly used when creating an item from [+] or Save passkey as new item (#8993)

* added user verification using the save new login button and + button

* removed commented out code

* [PM-7808][PM-7848] UV Preferred/Required, Item has MP reprompt, user without MP incorrectly bypasses UV and When UV = discouraged, cannot save passkey to item using [+] button (#9015)
This commit is contained in:
SmithThe4th 2024-05-09 14:18:02 -04:00 committed by GitHub
parent 9eef1f0953
commit acc4251372
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 398 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
apps/browser/src/auth/guards/fido2-auth.guard.ts
View File

@ -26,7 +26,9 @@ export const fido2AuthGuard: CanActivateFn = async (
const authStatus = await authService.getAuthStatus();
if (authStatus === AuthenticationStatus.Locked) {
routerService.setPreviousUrl(state.url);
// Appending fromLock=true to the query params to indicate that the user is being redirected from the lock screen, this is used for user verification.
const previousUrl = `${state.url}&fromLock=true`;
routerService.setPreviousUrl(previousUrl);
return router.createUrlTree(["/lock"], { queryParams: route.queryParams });
}

2
apps/browser/src/popup/app.module.ts
View File

@ -14,6 +14,7 @@ import { EnvironmentSelectorComponent } from "@bitwarden/angular/auth/components
import { JslibModule } from "@bitwarden/angular/jslib.module";
import { ColorPasswordCountPipe } from "@bitwarden/angular/pipes/color-password-count.pipe";
import { ColorPasswordPipe } from "@bitwarden/angular/pipes/color-password.pipe";
import { UserVerificationDialogComponent } from "@bitwarden/auth/angular";
import { AvatarModule, ButtonModule, ToastModule } from "@bitwarden/components";
import { ExportScopeCalloutComponent } from "@bitwarden/vault-export-ui";
@ -121,6 +122,7 @@ import "../platform/popup/locales";
PopupTabNavigationComponent,
PopupFooterComponent,
PopupHeaderComponent,
UserVerificationDialogComponent,
],
declarations: [
ActionButtonsComponent,

7
apps/browser/src/popup/services/services.module.ts
View File

@ -87,6 +87,7 @@ import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.serv
import { TotpService as TotpServiceAbstraction } from "@bitwarden/common/vault/abstractions/totp.service";
import { TotpService } from "@bitwarden/common/vault/services/totp.service";
import { DialogService, ToastService } from "@bitwarden/components";
import { PasswordRepromptService } from "@bitwarden/vault";
import { UnauthGuardService } from "../../auth/popup/services";
import { AutofillService as AutofillServiceAbstraction } from "../../autofill/services/abstractions/autofill.service";
@ -117,6 +118,7 @@ import { ForegroundMemoryStorageService } from "../../platform/storage/foregroun
import { fromChromeRuntimeMessaging } from "../../platform/utils/from-chrome-runtime-messaging";
import { BrowserSendStateService } from "../../tools/popup/services/browser-send-state.service";
import { FilePopoutUtilsService } from "../../tools/popup/services/file-popout-utils.service";
import { Fido2UserVerificationService } from "../../vault/services/fido2-user-verification.service";
import { VaultBrowserStateService } from "../../vault/services/vault-browser-state.service";
import { VaultFilterService } from "../../vault/services/vault-filter.service";
@ -600,6 +602,11 @@ const safeProviders: SafeProvider[] = [
provide: CLIENT_TYPE,
useValue: ClientType.Browser,
}),
safeProvider({
provide: Fido2UserVerificationService,
useClass: Fido2UserVerificationService,
deps: [PasswordRepromptService, UserVerificationService, DialogService],
}),
];
@NgModule({

47
apps/browser/src/vault/popup/components/fido2/fido2.component.ts
View File

@ -27,13 +27,13 @@ import { LoginUriView } from "@bitwarden/common/vault/models/view/login-uri.view
import { LoginView } from "@bitwarden/common/vault/models/view/login.view";
import { SecureNoteView } from "@bitwarden/common/vault/models/view/secure-note.view";
import { DialogService } from "@bitwarden/components";
import { PasswordRepromptService } from "@bitwarden/vault";
import { ZonedMessageListenerService } from "../../../../platform/browser/zoned-message-listener.service";
import {
BrowserFido2Message,
BrowserFido2UserInterfaceSession,
} from "../../../fido2/browser-fido2-user-interface.service";
import { Fido2UserVerificationService } from "../../../services/fido2-user-verification.service";
import { VaultPopoutType } from "../../utils/vault-popout-window";
interface ViewData {
@ -59,6 +59,7 @@ export class Fido2Component implements OnInit, OnDestroy {
protected data$: Observable<ViewData>;
protected sessionId?: string;
protected senderTabId?: string;
protected fromLock?: boolean;
protected ciphers?: CipherView[] = [];
protected displayedCiphers?: CipherView[] = [];
protected loading = false;
@ -71,13 +72,13 @@ export class Fido2Component implements OnInit, OnDestroy {
private router: Router,
private activatedRoute: ActivatedRoute,
private cipherService: CipherService,
private passwordRepromptService: PasswordRepromptService,
private platformUtilsService: PlatformUtilsService,
private domainSettingsService: DomainSettingsService,
private searchService: SearchService,
private logService: LogService,
private dialogService: DialogService,
private browserMessagingApi: ZonedMessageListenerService,
private fido2UserVerificationService: Fido2UserVerificationService,
) {}
ngOnInit() {
@ -89,6 +90,7 @@ export class Fido2Component implements OnInit, OnDestroy {
sessionId: queryParamMap.get("sessionId"),
senderTabId: queryParamMap.get("senderTabId"),
senderUrl: queryParamMap.get("senderUrl"),
fromLock: queryParamMap.get("fromLock"),
})),
);
@ -101,6 +103,7 @@ export class Fido2Component implements OnInit, OnDestroy {
this.sessionId = queryParams.sessionId;
this.senderTabId = queryParams.senderTabId;
this.url = queryParams.senderUrl;
this.fromLock = queryParams.fromLock === "true";
// For a 'NewSessionCreatedRequest', abort if it doesn't belong to the current session.
if (
message.type === "NewSessionCreatedRequest" &&
@ -210,7 +213,11 @@ export class Fido2Component implements OnInit, OnDestroy {
protected async submit() {
const data = this.message$.value;
if (data?.type === "PickCredentialRequest") {
const userVerified = await this.handleUserVerification(data.userVerification, this.cipher);
const userVerified = await this.fido2UserVerificationService.handleUserVerification(
data.userVerification,
this.cipher,
this.fromLock,
);
this.send({
sessionId: this.sessionId,
@ -231,7 +238,11 @@ export class Fido2Component implements OnInit, OnDestroy {
}
}
const userVerified = await this.handleUserVerification(data.userVerification, this.cipher);
const userVerified = await this.fido2UserVerificationService.handleUserVerification(
data.userVerification,
this.cipher,
this.fromLock,
);
this.send({
sessionId: this.sessionId,
@ -248,14 +259,21 @@ export class Fido2Component implements OnInit, OnDestroy {
const data = this.message$.value;
if (data?.type === "ConfirmNewCredentialRequest") {
const name = data.credentialName || data.rpId;
await this.createNewCipher(name);
const userVerified = await this.fido2UserVerificationService.handleUserVerification(
data.userVerification,
this.cipher,
this.fromLock,
);
if (!data.userVerification || userVerified) {
await this.createNewCipher(name);
}
// We are bypassing user verification pending implementation of PIN and biometric support.
this.send({
sessionId: this.sessionId,
cipherId: this.cipher?.id,
type: "ConfirmNewCredentialResponse",
userVerified: data.userVerification,
userVerified,
});
}
@ -304,6 +322,7 @@ export class Fido2Component implements OnInit, OnDestroy {
uilocation: "popout",
senderTabId: this.senderTabId,
sessionId: this.sessionId,
fromLock: this.fromLock,
userVerification: data.userVerification,
singleActionPopout: `${VaultPopoutType.fido2Popout}_${this.sessionId}`,
},
@ -374,20 +393,6 @@ export class Fido2Component implements OnInit, OnDestroy {
}
}
private async handleUserVerification(
userVerificationRequested: boolean,
cipher: CipherView,
): Promise<boolean> {
const masterPasswordRepromptRequired = cipher && cipher.reprompt !== 0;
if (masterPasswordRepromptRequired) {
return await this.passwordRepromptService.showPasswordPrompt();
}
// We are bypassing user verification pending implementation of PIN and biometric support.
return userVerificationRequested;
}
private send(msg: BrowserFido2Message) {
BrowserFido2UserInterfaceSession.sendMessage({
sessionId: this.sessionId,

20
apps/browser/src/vault/popup/components/vault/add-edit.component.ts
View File

@ -30,6 +30,7 @@ import { BrowserApi } from "../../../../platform/browser/browser-api";
import BrowserPopupUtils from "../../../../platform/popup/browser-popup-utils";
import { PopupCloseWarningService } from "../../../../popup/services/popup-close-warning.service";
import { BrowserFido2UserInterfaceSession } from "../../../fido2/browser-fido2-user-interface.service";
import { Fido2UserVerificationService } from "../../../services/fido2-user-verification.service";
import { fido2PopoutSessionData$ } from "../../utils/fido2-popout-session-data";
import { closeAddEditVaultItemPopout, VaultPopoutType } from "../../utils/vault-popout-window";
@ -69,6 +70,7 @@ export class AddEditComponent extends BaseAddEditComponent {
dialogService: DialogService,
datePipe: DatePipe,
configService: ConfigService,
private fido2UserVerificationService: Fido2UserVerificationService,
) {
super(
cipherService,
@ -168,11 +170,17 @@ export class AddEditComponent extends BaseAddEditComponent {
async submit(): Promise<boolean> {
const fido2SessionData = await firstValueFrom(this.fido2PopoutSessionData$);
const { isFido2Session, sessionId, userVerification } = fido2SessionData;
const { isFido2Session, sessionId, userVerification, fromLock } = fido2SessionData;
const inFido2PopoutWindow = BrowserPopupUtils.inPopout(window) && isFido2Session;
if (
inFido2PopoutWindow &&
!(await this.handleFido2UserVerification(sessionId, userVerification))
userVerification &&
!(await this.fido2UserVerificationService.handleUserVerification(
userVerification,
this.cipher,
fromLock,
))
) {
return false;
}
@ -327,14 +335,6 @@ export class AddEditComponent extends BaseAddEditComponent {
}, 200);
}
private async handleFido2UserVerification(
sessionId: string,
userVerification: boolean,
): Promise<boolean> {
// We are bypassing user verification pending implementation of PIN and biometric support.
return true;
}
repromptChanged() {
super.repromptChanged();

1
apps/browser/src/vault/popup/utils/fido2-popout-session-data.ts
View File

@ -16,6 +16,7 @@ export function fido2PopoutSessionData$() {
fallbackSupported: queryParams.fallbackSupported === "true",
userVerification: queryParams.userVerification === "true",
senderUrl: queryParams.senderUrl as string,
fromLock: queryParams.fromLock === "true",
})),
);
}

248
apps/browser/src/vault/services/fido2-user-verification.service.spec.ts Normal file
View File

@ -0,0 +1,248 @@
import { MockProxy, mock } from "jest-mock-extended";
import { UserVerificationDialogComponent } from "@bitwarden/auth/angular";
import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { CipherRepromptType, CipherType } from "@bitwarden/common/vault/enums";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { DialogService } from "@bitwarden/components";
import { PasswordRepromptService } from "@bitwarden/vault";
import { SetPinComponent } from "./../../auth/popup/components/set-pin.component";
import { Fido2UserVerificationService } from "./fido2-user-verification.service";
jest.mock("@bitwarden/auth/angular", () => ({
UserVerificationDialogComponent: {
open: jest.fn().mockResolvedValue({ userAction: "confirm", verificationSuccess: true }),
},
}));
jest.mock("../../auth/popup/components/set-pin.component", () => {
return {
SetPinComponent: {
open: jest.fn(),
},
};
});
describe("Fido2UserVerificationService", () => {
let fido2UserVerificationService: Fido2UserVerificationService;
let passwordRepromptService: MockProxy<PasswordRepromptService>;
let userVerificationService: MockProxy<UserVerificationService>;
let dialogService: MockProxy<DialogService>;
let cipher: CipherView;
beforeEach(() => {
passwordRepromptService = mock<PasswordRepromptService>();
userVerificationService = mock<UserVerificationService>();
dialogService = mock<DialogService>();
cipher = createCipherView();
fido2UserVerificationService = new Fido2UserVerificationService(
passwordRepromptService,
userVerificationService,
dialogService,
);
(UserVerificationDialogComponent.open as jest.Mock).mockResolvedValue({
userAction: "confirm",
verificationSuccess: true,
});
});
describe("handleUserVerification", () => {
describe("user verification requested is true", () => {
it("should return true if user is redirected from lock screen and master password reprompt is not required", async () => {
const result = await fido2UserVerificationService.handleUserVerification(
true,
cipher,
true,
);
expect(result).toBe(true);
});
it("should call master password reprompt dialog if user is redirected from lock screen, has master password and master password reprompt is required", async () => {
cipher.reprompt = CipherRepromptType.Password;
userVerificationService.hasMasterPassword.mockResolvedValue(true);
passwordRepromptService.showPasswordPrompt.mockResolvedValue(true);
const result = await fido2UserVerificationService.handleUserVerification(
true,
cipher,
true,
);
expect(passwordRepromptService.showPasswordPrompt).toHaveBeenCalled();
expect(result).toBe(true);
});
it("should call user verification dialog if user is redirected from lock screen, does not have a master password and master password reprompt is required", async () => {
cipher.reprompt = CipherRepromptType.Password;
userVerificationService.hasMasterPassword.mockResolvedValue(false);
const result = await fido2UserVerificationService.handleUserVerification(
true,
cipher,
true,
);
expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
clientSideOnlyVerification: true,
});
expect(result).toBe(true);
});
it("should call user verification dialog if user is not redirected from lock screen, does not have a master password and master password reprompt is required", async () => {
cipher.reprompt = CipherRepromptType.Password;
userVerificationService.hasMasterPassword.mockResolvedValue(false);
const result = await fido2UserVerificationService.handleUserVerification(
true,
cipher,
false,
);
expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
clientSideOnlyVerification: true,
});
expect(result).toBe(true);
});
it("should call master password reprompt dialog if user is not redirected from lock screen, has a master password and master password reprompt is required", async () => {
cipher.reprompt = CipherRepromptType.Password;
userVerificationService.hasMasterPassword.mockResolvedValue(false);
passwordRepromptService.showPasswordPrompt.mockResolvedValue(true);
const result = await fido2UserVerificationService.handleUserVerification(
true,
cipher,
false,
);
expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
clientSideOnlyVerification: true,
});
expect(result).toBe(true);
});
it("should call user verification dialog if user is not redirected from lock screen and no master password reprompt is required", async () => {
const result = await fido2UserVerificationService.handleUserVerification(
true,
cipher,
false,
);
expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
clientSideOnlyVerification: true,
});
expect(result).toBe(true);
});
it("should prompt user to set pin if user has no verification method", async () => {
(UserVerificationDialogComponent.open as jest.Mock).mockResolvedValue({
userAction: "confirm",
verificationSuccess: false,
noAvailableClientVerificationMethods: true,
});
await fido2UserVerificationService.handleUserVerification(true, cipher, false);
expect(SetPinComponent.open).toHaveBeenCalledWith(dialogService);
});
});
describe("user verification requested is false", () => {
it("should return false if user is redirected from lock screen and master password reprompt is not required", async () => {
const result = await fido2UserVerificationService.handleUserVerification(
false,
cipher,
true,
);
expect(result).toBe(false);
});
it("should return false if user is not redirected from lock screen and master password reprompt is not required", async () => {
const result = await fido2UserVerificationService.handleUserVerification(
false,
cipher,
false,
);
expect(result).toBe(false);
});
it("should call master password reprompt dialog if user is redirected from lock screen, has master password and master password reprompt is required", async () => {
cipher.reprompt = CipherRepromptType.Password;
userVerificationService.hasMasterPassword.mockResolvedValue(true);
passwordRepromptService.showPasswordPrompt.mockResolvedValue(true);
const result = await fido2UserVerificationService.handleUserVerification(
false,
cipher,
true,
);
expect(result).toBe(true);
expect(passwordRepromptService.showPasswordPrompt).toHaveBeenCalled();
});
it("should call user verification dialog if user is redirected from lock screen, does not have a master password and master password reprompt is required", async () => {
cipher.reprompt = CipherRepromptType.Password;
userVerificationService.hasMasterPassword.mockResolvedValue(false);
const result = await fido2UserVerificationService.handleUserVerification(
false,
cipher,
true,
);
expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
clientSideOnlyVerification: true,
});
expect(result).toBe(true);
});
it("should call user verification dialog if user is not redirected from lock screen, does not have a master password and master password reprompt is required", async () => {
cipher.reprompt = CipherRepromptType.Password;
userVerificationService.hasMasterPassword.mockResolvedValue(false);
const result = await fido2UserVerificationService.handleUserVerification(
false,
cipher,
false,
);
expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
clientSideOnlyVerification: true,
});
expect(result).toBe(true);
});
it("should call master password reprompt dialog if user is not redirected from lock screen, has a master password and master password reprompt is required", async () => {
cipher.reprompt = CipherRepromptType.Password;
userVerificationService.hasMasterPassword.mockResolvedValue(false);
passwordRepromptService.showPasswordPrompt.mockResolvedValue(true);
const result = await fido2UserVerificationService.handleUserVerification(
false,
cipher,
false,
);
expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
clientSideOnlyVerification: true,
});
expect(result).toBe(true);
});
});
});
});
function createCipherView() {
const cipher = new CipherView();
cipher.id = Utils.newGuid();
cipher.type = CipherType.Login;
cipher.reprompt = CipherRepromptType.None;
return cipher;
}

101
apps/browser/src/vault/services/fido2-user-verification.service.ts Normal file
View File

@ -0,0 +1,101 @@
import { firstValueFrom } from "rxjs";
import { UserVerificationDialogComponent } from "@bitwarden/auth/angular";
import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { DialogService } from "@bitwarden/components";
import { PasswordRepromptService } from "@bitwarden/vault";
import { SetPinComponent } from "../../auth/popup/components/set-pin.component";
export class Fido2UserVerificationService {
constructor(
private passwordRepromptService: PasswordRepromptService,
private userVerificationService: UserVerificationService,
private dialogService: DialogService,
) {}
/**
* Handles user verification for a user based on the cipher and user verification requested.
* @param userVerificationRequested Indicates if user verification is required or not.
* @param cipher Contains details about the cipher including master password reprompt.
* @param fromLock Indicates if the request is from the lock screen.
* @returns
*/
async handleUserVerification(
userVerificationRequested: boolean,
cipher: CipherView,
fromLock: boolean,
): Promise<boolean> {
const masterPasswordRepromptRequired = cipher && cipher.reprompt !== 0;
// If the request is from the lock screen, treat unlocking the vault as user verification,
// unless a master password reprompt is required.
if (fromLock) {
return masterPasswordRepromptRequired
? await this.handleMasterPasswordReprompt()
: userVerificationRequested;
}
if (masterPasswordRepromptRequired) {
return await this.handleMasterPasswordReprompt();
}
if (userVerificationRequested) {
return await this.showUserVerificationDialog();
}
return userVerificationRequested;
}
private async showMasterPasswordReprompt(): Promise<boolean> {
return await this.passwordRepromptService.showPasswordPrompt();
}
private async showUserVerificationDialog(): Promise<boolean> {
const result = await UserVerificationDialogComponent.open(this.dialogService, {
clientSideOnlyVerification: true,
});
if (result.userAction === "cancel") {
return;
}
// Handle unsuccessful verification attempts.
if (!result.verificationSuccess) {
// Check if no client-side verification methods are available.
if (result.noAvailableClientVerificationMethods) {
return await this.promptUserToSetPin();
}
return;
}
return result.verificationSuccess;
}
private async handleMasterPasswordReprompt(): Promise<boolean> {
const hasMasterPassword = await this.userVerificationService.hasMasterPassword();
// TDE users have no master password, so we need to use the UserVerification prompt
return hasMasterPassword
? await this.showMasterPasswordReprompt()
: await this.showUserVerificationDialog();
}
private async promptUserToSetPin() {
const dialogRef = SetPinComponent.open(this.dialogService);
if (!dialogRef) {
return;
}
const userHasPinSet = await firstValueFrom(dialogRef.closed);
if (!userHasPinSet) {
return;
}
// If the user has set a PIN, re-invoke the user verification dialog to complete the verification process.
return await this.showUserVerificationDialog();
}
}