Skip to content

Commit

Permalink
[PM-14419] At-risk passwords change password service (#13279)
Browse files Browse the repository at this point in the history
* [PM-14419] Introduce the change-login-password service and its default implementation

* [PM-14419] Use the change login password service on the at-risk passwords page

* [PM-14419] Add unit tests

* [PM-14419] Use existing fixed test environment

* [PM-14419] Add mock implementation for ChangeLoginPasswordService in at-risk passwords tests

* [PM-14419] Linter
  • Loading branch information
shane-melton authored Feb 13, 2025
1 parent a0c3854 commit c67e6df
Show file tree
Hide file tree
Showing 7 changed files with 319 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,20 @@
type="button"
bitBadge
variant="primary"
appStopProp
(click)="launchChangePassword(cipher)"
[title]="'changeButtonTitle' | i18n: cipher.name"
[attr.aria-label]="'changeButtonTitle' | i18n: cipher.name"
[disabled]="launchingCipher() == cipher"
>
{{ "change" | i18n }}
<ng-container *ngIf="launchingCipher() != cipher">
{{ "change" | i18n }}
</ng-container>
<i
*ngIf="launchingCipher() == cipher"
class="bwi bwi-spinner bwi-spin"
aria-hidden="true"
></i>
</button>
</bit-item-action>
</bit-item>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.servi
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { ToastService } from "@bitwarden/components";
import {
ChangeLoginPasswordService,
DefaultChangeLoginPasswordService,
PasswordRepromptService,
SecurityTask,
SecurityTaskType,
Expand Down Expand Up @@ -70,6 +72,7 @@ describe("AtRiskPasswordsComponent", () => {
const setInlineMenuVisibility = jest.fn();
const mockToastService = mock<ToastService>();
const mockAtRiskPasswordPageService = mock<AtRiskPasswordPageService>();
const mockChangeLoginPasswordService = mock<ChangeLoginPasswordService>();

beforeEach(async () => {
mockTasks$ = new BehaviorSubject<SecurityTask[]>([
Expand Down Expand Up @@ -156,12 +159,16 @@ describe("AtRiskPasswordsComponent", () => {
.overrideComponent(AtRiskPasswordsComponent, {
remove: {
imports: [PopupHeaderComponent, PopupPageComponent],
providers: [AtRiskPasswordPageService],
providers: [
AtRiskPasswordPageService,
{ provide: ChangeLoginPasswordService, useClass: DefaultChangeLoginPasswordService },
],
},
add: {
imports: [MockPopupHeaderComponent, MockPopupPageComponent],
providers: [
{ provide: AtRiskPasswordPageService, useValue: mockAtRiskPasswordPageService },
{ provide: ChangeLoginPasswordService, useValue: mockChangeLoginPasswordService },
],
},
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { CommonModule } from "@angular/common";
import { Component, inject } from "@angular/core";
import { Component, inject, signal } from "@angular/core";
import { Router } from "@angular/router";
import { combineLatest, firstValueFrom, map, of, shareReplay, startWith, switchMap } from "rxjs";

Expand All @@ -16,14 +16,16 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import {
BadgeComponent,
BadgeModule,
ButtonModule,
CalloutModule,
ItemModule,
ToastService,
TypographyModule,
} from "@bitwarden/components";
import {
ChangeLoginPasswordService,
DefaultChangeLoginPasswordService,
filterOutNullish,
PasswordRepromptService,
SecurityTaskType,
Expand All @@ -37,21 +39,24 @@ import { PopupPageComponent } from "../../../../platform/popup/layout/popup-page
import { AtRiskPasswordPageService } from "./at-risk-password-page.service";

@Component({
selector: "vault-at-risk-passwords",
standalone: true,
imports: [
PopupPageComponent,
PopupHeaderComponent,
PopOutComponent,
ItemModule,
CommonModule,
JslibModule,
BadgeComponent,
TypographyModule,
CalloutModule,
ButtonModule,
BadgeModule,
],
providers: [AtRiskPasswordPageService],
providers: [
AtRiskPasswordPageService,
{ provide: ChangeLoginPasswordService, useClass: DefaultChangeLoginPasswordService },
],
selector: "vault-at-risk-passwords",
standalone: true,
templateUrl: "./at-risk-passwords.component.html",
})
export class AtRiskPasswordsComponent {
Expand All @@ -60,12 +65,20 @@ export class AtRiskPasswordsComponent {
private cipherService = inject(CipherService);
private i18nService = inject(I18nService);
private accountService = inject(AccountService);
private platformUtilsService = inject(PlatformUtilsService);
private passwordRepromptService = inject(PasswordRepromptService);
private router = inject(Router);
private autofillSettingsService = inject(AutofillSettingsServiceAbstraction);
private toastService = inject(ToastService);
private atRiskPasswordPageService = inject(AtRiskPasswordPageService);
private changeLoginPasswordService = inject(ChangeLoginPasswordService);
private platformUtilsService = inject(PlatformUtilsService);

/**
* The cipher that is currently being launched. Used to show a loading spinner on the badge button.
* The UI utilize a bitBadge which does not support async actions (like bitButton does).
* @protected
*/
protected launchingCipher = signal<CipherView | null>(null);

private activeUserData$ = this.accountService.activeAccount$.pipe(
filterOutNullish(),
Expand Down Expand Up @@ -138,12 +151,6 @@ export class AtRiskPasswordsComponent {
});
}

async launchChangePassword(cipher: CipherView) {
if (cipher.login?.uri) {
this.platformUtilsService.launchUri(cipher.login.uri);
}
}

async activateInlineAutofillMenuVisibility() {
await this.autofillSettingsService.setInlineMenuVisibility(
AutofillOverlayVisibility.OnButtonClick,
Expand All @@ -159,4 +166,19 @@ export class AtRiskPasswordsComponent {
const { userId } = await firstValueFrom(this.activeUserData$);
await this.atRiskPasswordPageService.dismissCallout(userId);
}

launchChangePassword = async (cipher: CipherView) => {
try {
this.launchingCipher.set(cipher);
const url = await this.changeLoginPasswordService.getChangePasswordUrl(cipher);

if (url == null) {
return;
}

this.platformUtilsService.launchUri(url);
} finally {
this.launchingCipher.set(null);
}
};
}
10 changes: 10 additions & 0 deletions libs/vault/src/abstractions/change-login-password.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";

export abstract class ChangeLoginPasswordService {
/**
* Attempts to find a well-known change password URL for the given cipher. Only works for Login ciphers with at
* least one http/https URL. If no well-known change password URL is found, the first URL is returned.
* Non-Login ciphers and Logins with no valid http/https URLs return null.
*/
abstract getChangePasswordUrl(cipher: CipherView): Promise<string | null>;
}
3 changes: 3 additions & 0 deletions libs/vault/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,6 @@ export * from "./components/add-edit-folder-dialog/add-edit-folder-dialog.compon
export * as VaultIcons from "./icons";

export * from "./tasks";

export * from "./abstractions/change-login-password.service";
export * from "./services/default-change-login-password.service";
157 changes: 157 additions & 0 deletions libs/vault/src/services/default-change-login-password.service.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
/**
* Jest needs to run in custom environment to mock Request/Response objects
* @jest-environment ../../libs/shared/test.environment.ts
*/

import { mock } from "jest-mock-extended";

import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { CipherType } from "@bitwarden/common/vault/enums";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { LoginUriView } from "@bitwarden/common/vault/models/view/login-uri.view";
import { LoginView } from "@bitwarden/common/vault/models/view/login.view";

import { DefaultChangeLoginPasswordService } from "./default-change-login-password.service";

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

let mockShouldNotExistResponse: Response;
let mockWellKnownResponse: Response;

const mockApiService = mock<ApiService>();

beforeEach(() => {
mockApiService.nativeFetch.mockClear();

// Default responses to success state
mockShouldNotExistResponse = new Response("Not Found", { status: 404 });
mockWellKnownResponse = new Response("OK", { status: 200 });

mockApiService.nativeFetch.mockImplementation((request) => {
if (
request.url.endsWith("resource-that-should-not-exist-whose-status-code-should-not-be-200")
) {
return Promise.resolve(mockShouldNotExistResponse);
}

if (request.url.endsWith(".well-known/change-password")) {
return Promise.resolve(mockWellKnownResponse);
}

throw new Error("Unexpected request");
});
service = new DefaultChangeLoginPasswordService(mockApiService);
});

it("should return null for non-login ciphers", async () => {
const cipher = {
type: CipherType.Card,
} as CipherView;

const url = await service.getChangePasswordUrl(cipher);

expect(url).toBeNull();
});

it("should return null for logins with no URIs", async () => {
const cipher = {
type: CipherType.Login,
login: Object.assign(new LoginView(), { uris: [] as LoginUriView[] }),
} as CipherView;

const url = await service.getChangePasswordUrl(cipher);

expect(url).toBeNull();
});

it("should return null for logins with no valid HTTP/HTTPS URIs", async () => {
const cipher = {
type: CipherType.Login,
login: Object.assign(new LoginView(), {
uris: [{ uri: "ftp://example.com" }],
}),
} as CipherView;

const url = await service.getChangePasswordUrl(cipher);

expect(url).toBeNull();
});

it("should check the origin for a reliable status code", async () => {
const cipher = {
type: CipherType.Login,
login: Object.assign(new LoginView(), {
uris: [{ uri: "https://example.com" }],
}),
} as CipherView;

await service.getChangePasswordUrl(cipher);

expect(mockApiService.nativeFetch).toHaveBeenCalledWith(
expect.objectContaining({
url: "https://example.com/.well-known/resource-that-should-not-exist-whose-status-code-should-not-be-200",
}),
);
});

it("should attempt to fetch the well-known change password URL", async () => {
const cipher = {
type: CipherType.Login,
login: Object.assign(new LoginView(), {
uris: [{ uri: "https://example.com" }],
}),
} as CipherView;

await service.getChangePasswordUrl(cipher);

expect(mockApiService.nativeFetch).toHaveBeenCalledWith(
expect.objectContaining({
url: "https://example.com/.well-known/change-password",
}),
);
});

it("should return the well-known change password URL when successful at verifying the response", async () => {
const cipher = {
type: CipherType.Login,
login: Object.assign(new LoginView(), {
uris: [{ uri: "https://example.com" }],
}),
} as CipherView;

const url = await service.getChangePasswordUrl(cipher);

expect(url).toBe("https://example.com/.well-known/change-password");
});

it("should return the original URI when unable to verify the response", async () => {
mockShouldNotExistResponse = new Response("Ok", { status: 200 });

const cipher = {
type: CipherType.Login,
login: Object.assign(new LoginView(), {
uris: [{ uri: "https://example.com" }],
}),
} as CipherView;

const url = await service.getChangePasswordUrl(cipher);

expect(url).toBe("https://example.com");
});

it("should return the original URI when the well-known URL is not found", async () => {
mockWellKnownResponse = new Response("Not Found", { status: 404 });

const cipher = {
type: CipherType.Login,
login: Object.assign(new LoginView(), {
uris: [{ uri: "https://example.com" }],
}),
} as CipherView;

const url = await service.getChangePasswordUrl(cipher);

expect(url).toBe("https://example.com");
});
});
Loading

0 comments on commit c67e6df

Please sign in to comment.