Skip to content

Commit

Permalink
[PM-4816] Create shared LoginApprovalComponent (#11982)
Browse files Browse the repository at this point in the history
* Stub out dialog

* Genericize LoginApprovalComponent

* update ipc mocks

* Remove changes to account component

* Remove changes to account component

* Remove debug

* Remove test component

* Remove added translations

* Fix failing test

* Run lint and prettier

* Rename LoginApprovalServiceAbstraction to LoginApprovalComponentServiceAbstraction

* Add back missing "isVisible" check before calling loginRequest

* Rename classes to contain "Component" in the name

* Add missing space between "login attempt" and fingerprint phrase

* Require email
  • Loading branch information
alec-livefront authored Nov 22, 2024
1 parent 13d4b6f commit 02ea368
Show file tree
Hide file tree
Showing 12 changed files with 307 additions and 12 deletions.
3 changes: 1 addition & 2 deletions apps/desktop/src/app/app.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
import { CollectionService } from "@bitwarden/admin-console/common";
import { ModalRef } from "@bitwarden/angular/components/modal/modal.ref";
import { ModalService } from "@bitwarden/angular/services/modal.service";
import { FingerprintDialogComponent } from "@bitwarden/auth/angular";
import { FingerprintDialogComponent, LoginApprovalComponent } from "@bitwarden/auth/angular";
import { LogoutReason } from "@bitwarden/auth/common";
import { EventUploadService } from "@bitwarden/common/abstractions/event/event-upload.service";
import { NotificationsService } from "@bitwarden/common/abstractions/notifications.service";
Expand Down Expand Up @@ -67,7 +67,6 @@ import { PasswordGenerationServiceAbstraction } from "@bitwarden/generator-legac
import { KeyService, BiometricStateService } from "@bitwarden/key-management";

import { DeleteAccountComponent } from "../auth/delete-account.component";
import { LoginApprovalComponent } from "../auth/login/login-approval.component";
import { MenuAccount, MenuUpdateRequest } from "../main/menu/menu.updater";
import { flagEnabled } from "../platform/flags";
import { PremiumComponent } from "../vault/app/accounts/premium.component";
Expand Down
7 changes: 7 additions & 0 deletions apps/desktop/src/app/services/services.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
} from "@bitwarden/auth/angular";
import {
InternalUserDecryptionOptionsServiceAbstraction,
LoginApprovalComponentServiceAbstraction,
LoginEmailService,
PinServiceAbstraction,
} from "@bitwarden/auth/common";
Expand Down Expand Up @@ -87,6 +88,7 @@ import {
BiometricsService,
} from "@bitwarden/key-management";

import { DesktopLoginApprovalComponentService } from "../../auth/login/desktop-login-approval-component.service";
import { DesktopLoginComponentService } from "../../auth/login/desktop-login-component.service";
import { DesktopAutofillSettingsService } from "../../autofill/services/desktop-autofill-settings.service";
import { ElectronBiometricsService } from "../../key-management/biometrics/electron-biometrics.service";
Expand Down Expand Up @@ -349,6 +351,11 @@ const safeProviders: SafeProvider[] = [
useClass: LoginEmailService,
deps: [AccountService, AuthService, StateProvider],
}),
safeProvider({
provide: LoginApprovalComponentServiceAbstraction,
useClass: DesktopLoginApprovalComponentService,
deps: [I18nServiceAbstraction],
}),
];

@NgModule({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import { TestBed } from "@angular/core/testing";
import { mock, MockProxy } from "jest-mock-extended";
import { Subject } from "rxjs";

import { LoginApprovalComponent } from "@bitwarden/auth/angular";
import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/platform/abstractions/i18n.service";

import { DesktopLoginApprovalComponentService } from "./desktop-login-approval-component.service";

describe("DesktopLoginApprovalComponentService", () => {
let service: DesktopLoginApprovalComponentService;
let i18nService: MockProxy<I18nServiceAbstraction>;
let originalIpc: any;

beforeEach(() => {
originalIpc = (global as any).ipc;
(global as any).ipc = {
auth: {
loginRequest: jest.fn(),
},
platform: {
isWindowVisible: jest.fn(),
},
};

i18nService = mock<I18nServiceAbstraction>({
t: jest.fn(),
userSetLocale$: new Subject<string>(),
locale$: new Subject<string>(),
});

TestBed.configureTestingModule({
providers: [
DesktopLoginApprovalComponentService,
{ provide: I18nServiceAbstraction, useValue: i18nService },
],
});

service = TestBed.inject(DesktopLoginApprovalComponentService);
});

afterEach(() => {
jest.clearAllMocks();
(global as any).ipc = originalIpc;
});

it("is created successfully", () => {
expect(service).toBeTruthy();
});

it("calls ipc.auth.loginRequest with correct parameters when window is not visible", async () => {
const title = "Log in requested";
const email = "test@bitwarden.com";
const message = `Confirm login attempt for ${email}`;
const closeText = "Close";

const loginApprovalComponent = { email } as LoginApprovalComponent;
i18nService.t.mockImplementation((key: string) => {
switch (key) {
case "logInRequested":
return title;
case "confirmLoginAtemptForMail":
return message;
case "close":
return closeText;
default:
return "";
}
});

jest.spyOn(ipc.platform, "isWindowVisible").mockResolvedValue(false);
jest.spyOn(ipc.auth, "loginRequest").mockResolvedValue();

await service.showLoginRequestedAlertIfWindowNotVisible(loginApprovalComponent.email);

expect(ipc.auth.loginRequest).toHaveBeenCalledWith(title, message, closeText);
});

it("does not call ipc.auth.loginRequest when window is visible", async () => {
const loginApprovalComponent = { email: "test@bitwarden.com" } as LoginApprovalComponent;

jest.spyOn(ipc.platform, "isWindowVisible").mockResolvedValue(true);
jest.spyOn(ipc.auth, "loginRequest");

await service.showLoginRequestedAlertIfWindowNotVisible(loginApprovalComponent.email);

expect(ipc.auth.loginRequest).not.toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { Injectable } from "@angular/core";

import { DefaultLoginApprovalComponentService } from "@bitwarden/auth/angular";
import { LoginApprovalComponentServiceAbstraction } from "@bitwarden/auth/common";
import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/platform/abstractions/i18n.service";

@Injectable()
export class DesktopLoginApprovalComponentService
extends DefaultLoginApprovalComponentService
implements LoginApprovalComponentServiceAbstraction
{
constructor(private i18nService: I18nServiceAbstraction) {
super();
}

async showLoginRequestedAlertIfWindowNotVisible(email: string): Promise<void> {
const isVisible = await ipc.platform.isWindowVisible();
if (!isVisible) {
await ipc.auth.loginRequest(
this.i18nService.t("logInRequested"),
this.i18nService.t("confirmLoginAtemptForMail", email),
this.i18nService.t("close"),
);
}
}
}
4 changes: 4 additions & 0 deletions libs/auth/src/angular/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,7 @@ export * from "./vault-timeout-input/vault-timeout-input.component";

// self hosted environment configuration dialog
export * from "./self-hosted-env-config-dialog/self-hosted-env-config-dialog.component";

// login approval
export * from "./login-approval/login-approval.component";
export * from "./login-approval/default-login-approval-component.service";
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { TestBed } from "@angular/core/testing";

import { DefaultLoginApprovalComponentService } from "./default-login-approval-component.service";
import { LoginApprovalComponent } from "./login-approval.component";

describe("DefaultLoginApprovalComponentService", () => {
let service: DefaultLoginApprovalComponentService;

beforeEach(() => {
TestBed.configureTestingModule({
providers: [DefaultLoginApprovalComponentService],
});

service = TestBed.inject(DefaultLoginApprovalComponentService);
});

it("is created successfully", () => {
expect(service).toBeTruthy();
});

it("has showLoginRequestedAlertIfWindowNotVisible method that is a no-op", async () => {
const loginApprovalComponent = {} as LoginApprovalComponent;
await service.showLoginRequestedAlertIfWindowNotVisible(loginApprovalComponent.email);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { LoginApprovalComponentServiceAbstraction } from "../../common/abstractions/login-approval-component.service.abstraction";

/**
* Default implementation of the LoginApprovalComponentServiceAbstraction.
*/
export class DefaultLoginApprovalComponentService
implements LoginApprovalComponentServiceAbstraction
{
/**
* No-op implementation of the showLoginRequestedAlertIfWindowNotVisible method.
* @returns
*/
async showLoginRequestedAlertIfWindowNotVisible(email?: string): Promise<void> {
return;
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<bit-dialog>
<span bitDialogTitle>{{ "areYouTryingtoLogin" | i18n }}</span>
<ng-container bitDialogContent>
<h4>{{ "logInAttemptBy" | i18n: email }}</h4>
<h4 class="tw-mb-3">{{ "logInAttemptBy" | i18n: email }}</h4>
<div>
<b>{{ "fingerprintPhraseHeader" | i18n }}</b>
<p class="tw-text-code">{{ fingerprintPhrase }}</p>
Expand Down
122 changes: 122 additions & 0 deletions libs/auth/src/angular/login-approval/login-approval.component.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
import { DialogRef, DIALOG_DATA } from "@angular/cdk/dialog";
import { ComponentFixture, TestBed } from "@angular/core/testing";
import { mock, MockProxy } from "jest-mock-extended";
import { of } from "rxjs";

import {
AuthRequestServiceAbstraction,
LoginApprovalComponentServiceAbstraction,
} from "@bitwarden/auth/common";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response";
import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { UserId } from "@bitwarden/common/types/guid";
import { ToastService } from "@bitwarden/components";
import { KeyService } from "@bitwarden/key-management";

import { LoginApprovalComponent } from "./login-approval.component";

describe("LoginApprovalComponent", () => {
let component: LoginApprovalComponent;
let fixture: ComponentFixture<LoginApprovalComponent>;

let authRequestService: MockProxy<AuthRequestServiceAbstraction>;
let accountService: MockProxy<AccountService>;
let apiService: MockProxy<ApiService>;
let i18nService: MockProxy<I18nService>;
let dialogRef: MockProxy<DialogRef>;
let toastService: MockProxy<ToastService>;

const testNotificationId = "test-notification-id";
const testEmail = "test@bitwarden.com";
const testPublicKey = "test-public-key";

beforeEach(async () => {
authRequestService = mock<AuthRequestServiceAbstraction>();
accountService = mock<AccountService>();
apiService = mock<ApiService>();
i18nService = mock<I18nService>();
dialogRef = mock<DialogRef>();
toastService = mock<ToastService>();

accountService.activeAccount$ = of({
email: testEmail,
id: "test-user-id" as UserId,
emailVerified: true,
name: null,
});

await TestBed.configureTestingModule({
imports: [LoginApprovalComponent],
providers: [
{ provide: DIALOG_DATA, useValue: { notificationId: testNotificationId } },
{ provide: AuthRequestServiceAbstraction, useValue: authRequestService },
{ provide: AccountService, useValue: accountService },
{ provide: PlatformUtilsService, useValue: mock<PlatformUtilsService>() },
{ provide: I18nService, useValue: i18nService },
{ provide: ApiService, useValue: apiService },
{ provide: AppIdService, useValue: mock<AppIdService>() },
{ provide: KeyService, useValue: mock<KeyService>() },
{ provide: DialogRef, useValue: dialogRef },
{ provide: ToastService, useValue: toastService },
{
provide: LoginApprovalComponentServiceAbstraction,
useValue: mock<LoginApprovalComponentServiceAbstraction>(),
},
],
}).compileComponents();

fixture = TestBed.createComponent(LoginApprovalComponent);
component = fixture.componentInstance;
});

it("creates successfully", () => {
expect(component).toBeTruthy();
});

describe("ngOnInit", () => {
beforeEach(() => {
apiService.getAuthRequest.mockResolvedValue({
publicKey: testPublicKey,
creationDate: new Date().toISOString(),
} as AuthRequestResponse);
authRequestService.getFingerprintPhrase.mockResolvedValue("test-phrase");
});

it("retrieves and sets auth request data", async () => {
await component.ngOnInit();

expect(apiService.getAuthRequest).toHaveBeenCalledWith(testNotificationId);
expect(component.email).toBe(testEmail);
expect(component.fingerprintPhrase).toBeDefined();
});

it("updates time text initially", async () => {
i18nService.t.mockReturnValue("justNow");

await component.ngOnInit();
expect(component.requestTimeText).toBe("justNow");
});
});

describe("denyLogin", () => {
it("denies auth request and shows info toast", async () => {
const response = { requestApproved: false } as AuthRequestResponse;
apiService.getAuthRequest.mockResolvedValue(response);
authRequestService.approveOrDenyAuthRequest.mockResolvedValue(response);
i18nService.t.mockReturnValue("denied message");

await component.denyLogin();

expect(authRequestService.approveOrDenyAuthRequest).toHaveBeenCalledWith(false, response);
expect(toastService.showToast).toHaveBeenCalledWith({
variant: "info",
title: null,
message: "denied message",
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import { Component, OnInit, OnDestroy, Inject } from "@angular/core";
import { Subject, firstValueFrom, map } from "rxjs";

import { JslibModule } from "@bitwarden/angular/jslib.module";
import { AuthRequestServiceAbstraction } from "@bitwarden/auth/common";
import {
AuthRequestServiceAbstraction,
LoginApprovalComponentServiceAbstraction as LoginApprovalComponentService,
} from "@bitwarden/auth/common";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response";
Expand Down Expand Up @@ -56,6 +59,7 @@ export class LoginApprovalComponent implements OnInit, OnDestroy {
protected keyService: KeyService,
private dialogRef: DialogRef,
private toastService: ToastService,
private loginApprovalComponentService: LoginApprovalComponentService,
) {
this.notificationId = params.notificationId;
}
Expand Down Expand Up @@ -89,14 +93,7 @@ export class LoginApprovalComponent implements OnInit, OnDestroy {
this.updateTimeText();
}, RequestTimeUpdate);

const isVisible = await ipc.platform.isWindowVisible();
if (!isVisible) {
await ipc.auth.loginRequest(
this.i18nService.t("logInRequested"),
this.i18nService.t("confirmLoginAtemptForMail", this.email),
this.i18nService.t("close"),
);
}
this.loginApprovalComponentService.showLoginRequestedAlertIfWindowNotVisible(this.email);
}
}

Expand Down
1 change: 1 addition & 0 deletions libs/auth/src/common/abstractions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ export * from "./login-email.service";
export * from "./login-strategy.service";
export * from "./user-decryption-options.service.abstraction";
export * from "./auth-request.service.abstraction";
export * from "./login-approval-component.service.abstraction";
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/**
* Abstraction for the LoginApprovalComponent service.
*/
export abstract class LoginApprovalComponentServiceAbstraction {
/**
* Shows a login requested alert if the window is not visible.
*/
abstract showLoginRequestedAlertIfWindowNotVisible: (email?: string) => Promise<void>;
}

0 comments on commit 02ea368

Please sign in to comment.