1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-16 16:23:44 +00:00

[PM-14419] At-risk passwords change password service (#13279)

* [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
This commit is contained in:
Shane Melton
2025-02-13 10:58:44 -08:00
committed by GitHub
parent a0c38543ac
commit c67e6df839
7 changed files with 319 additions and 15 deletions

View File

@@ -59,11 +59,20 @@
type="button" type="button"
bitBadge bitBadge
variant="primary" variant="primary"
appStopProp
(click)="launchChangePassword(cipher)" (click)="launchChangePassword(cipher)"
[title]="'changeButtonTitle' | i18n: cipher.name" [title]="'changeButtonTitle' | i18n: cipher.name"
[attr.aria-label]="'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> </button>
</bit-item-action> </bit-item-action>
</bit-item> </bit-item>

View File

@@ -18,6 +18,8 @@ import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.servi
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { ToastService } from "@bitwarden/components"; import { ToastService } from "@bitwarden/components";
import { import {
ChangeLoginPasswordService,
DefaultChangeLoginPasswordService,
PasswordRepromptService, PasswordRepromptService,
SecurityTask, SecurityTask,
SecurityTaskType, SecurityTaskType,
@@ -70,6 +72,7 @@ describe("AtRiskPasswordsComponent", () => {
const setInlineMenuVisibility = jest.fn(); const setInlineMenuVisibility = jest.fn();
const mockToastService = mock<ToastService>(); const mockToastService = mock<ToastService>();
const mockAtRiskPasswordPageService = mock<AtRiskPasswordPageService>(); const mockAtRiskPasswordPageService = mock<AtRiskPasswordPageService>();
const mockChangeLoginPasswordService = mock<ChangeLoginPasswordService>();
beforeEach(async () => { beforeEach(async () => {
mockTasks$ = new BehaviorSubject<SecurityTask[]>([ mockTasks$ = new BehaviorSubject<SecurityTask[]>([
@@ -156,12 +159,16 @@ describe("AtRiskPasswordsComponent", () => {
.overrideComponent(AtRiskPasswordsComponent, { .overrideComponent(AtRiskPasswordsComponent, {
remove: { remove: {
imports: [PopupHeaderComponent, PopupPageComponent], imports: [PopupHeaderComponent, PopupPageComponent],
providers: [AtRiskPasswordPageService], providers: [
AtRiskPasswordPageService,
{ provide: ChangeLoginPasswordService, useClass: DefaultChangeLoginPasswordService },
],
}, },
add: { add: {
imports: [MockPopupHeaderComponent, MockPopupPageComponent], imports: [MockPopupHeaderComponent, MockPopupPageComponent],
providers: [ providers: [
{ provide: AtRiskPasswordPageService, useValue: mockAtRiskPasswordPageService }, { provide: AtRiskPasswordPageService, useValue: mockAtRiskPasswordPageService },
{ provide: ChangeLoginPasswordService, useValue: mockChangeLoginPasswordService },
], ],
}, },
}) })

View File

@@ -1,5 +1,5 @@
import { CommonModule } from "@angular/common"; import { CommonModule } from "@angular/common";
import { Component, inject } from "@angular/core"; import { Component, inject, signal } from "@angular/core";
import { Router } from "@angular/router"; import { Router } from "@angular/router";
import { combineLatest, firstValueFrom, map, of, shareReplay, startWith, switchMap } from "rxjs"; import { combineLatest, firstValueFrom, map, of, shareReplay, startWith, switchMap } from "rxjs";
@@ -16,7 +16,7 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { import {
BadgeComponent, BadgeModule,
ButtonModule, ButtonModule,
CalloutModule, CalloutModule,
ItemModule, ItemModule,
@@ -24,6 +24,8 @@ import {
TypographyModule, TypographyModule,
} from "@bitwarden/components"; } from "@bitwarden/components";
import { import {
ChangeLoginPasswordService,
DefaultChangeLoginPasswordService,
filterOutNullish, filterOutNullish,
PasswordRepromptService, PasswordRepromptService,
SecurityTaskType, SecurityTaskType,
@@ -37,8 +39,6 @@ import { PopupPageComponent } from "../../../../platform/popup/layout/popup-page
import { AtRiskPasswordPageService } from "./at-risk-password-page.service"; import { AtRiskPasswordPageService } from "./at-risk-password-page.service";
@Component({ @Component({
selector: "vault-at-risk-passwords",
standalone: true,
imports: [ imports: [
PopupPageComponent, PopupPageComponent,
PopupHeaderComponent, PopupHeaderComponent,
@@ -46,12 +46,17 @@ import { AtRiskPasswordPageService } from "./at-risk-password-page.service";
ItemModule, ItemModule,
CommonModule, CommonModule,
JslibModule, JslibModule,
BadgeComponent,
TypographyModule, TypographyModule,
CalloutModule, CalloutModule,
ButtonModule, ButtonModule,
BadgeModule,
], ],
providers: [AtRiskPasswordPageService], providers: [
AtRiskPasswordPageService,
{ provide: ChangeLoginPasswordService, useClass: DefaultChangeLoginPasswordService },
],
selector: "vault-at-risk-passwords",
standalone: true,
templateUrl: "./at-risk-passwords.component.html", templateUrl: "./at-risk-passwords.component.html",
}) })
export class AtRiskPasswordsComponent { export class AtRiskPasswordsComponent {
@@ -60,12 +65,20 @@ export class AtRiskPasswordsComponent {
private cipherService = inject(CipherService); private cipherService = inject(CipherService);
private i18nService = inject(I18nService); private i18nService = inject(I18nService);
private accountService = inject(AccountService); private accountService = inject(AccountService);
private platformUtilsService = inject(PlatformUtilsService);
private passwordRepromptService = inject(PasswordRepromptService); private passwordRepromptService = inject(PasswordRepromptService);
private router = inject(Router); private router = inject(Router);
private autofillSettingsService = inject(AutofillSettingsServiceAbstraction); private autofillSettingsService = inject(AutofillSettingsServiceAbstraction);
private toastService = inject(ToastService); private toastService = inject(ToastService);
private atRiskPasswordPageService = inject(AtRiskPasswordPageService); 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( private activeUserData$ = this.accountService.activeAccount$.pipe(
filterOutNullish(), filterOutNullish(),
@@ -138,12 +151,6 @@ export class AtRiskPasswordsComponent {
}); });
} }
async launchChangePassword(cipher: CipherView) {
if (cipher.login?.uri) {
this.platformUtilsService.launchUri(cipher.login.uri);
}
}
async activateInlineAutofillMenuVisibility() { async activateInlineAutofillMenuVisibility() {
await this.autofillSettingsService.setInlineMenuVisibility( await this.autofillSettingsService.setInlineMenuVisibility(
AutofillOverlayVisibility.OnButtonClick, AutofillOverlayVisibility.OnButtonClick,
@@ -159,4 +166,19 @@ export class AtRiskPasswordsComponent {
const { userId } = await firstValueFrom(this.activeUserData$); const { userId } = await firstValueFrom(this.activeUserData$);
await this.atRiskPasswordPageService.dismissCallout(userId); 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);
}
};
} }

View File

@@ -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>;
}

View File

@@ -25,3 +25,6 @@ export * from "./components/add-edit-folder-dialog/add-edit-folder-dialog.compon
export * as VaultIcons from "./icons"; export * as VaultIcons from "./icons";
export * from "./tasks"; export * from "./tasks";
export * from "./abstractions/change-login-password.service";
export * from "./services/default-change-login-password.service";

View File

@@ -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");
});
});

View File

@@ -0,0 +1,96 @@
import { Injectable } from "@angular/core";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { CipherType } from "@bitwarden/common/vault/enums";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { ChangeLoginPasswordService } from "../abstractions/change-login-password.service";
@Injectable()
export class DefaultChangeLoginPasswordService implements ChangeLoginPasswordService {
constructor(private apiService: ApiService) {}
/**
* @inheritDoc
*/
async getChangePasswordUrl(cipher: CipherView): Promise<string | null> {
// Ensure we have a cipher with at least one URI
if (cipher.type !== CipherType.Login || cipher.login == null || !cipher.login.hasUris) {
return null;
}
// Find the first valid URL that is an HTTP or HTTPS URL
const url = cipher.login.uris
.map((m) => Utils.getUrl(m.uri))
.find((m) => m != null && (m.protocol === "http:" || m.protocol === "https:"));
if (url == null) {
return null;
}
const [reliable, wellKnownChangeUrl] = await Promise.all([
this.hasReliableHttpStatusCode(url.origin),
this.getWellKnownChangePasswordUrl(url.origin),
]);
if (!reliable || wellKnownChangeUrl == null) {
return cipher.login.uri;
}
return wellKnownChangeUrl;
}
/**
* Checks if the server returns a non-200 status code for a resource that should not exist.
* See https://w3c.github.io/webappsec-change-password-url/response-code-reliability.html#semantics
* @param urlOrigin The origin of the URL to check
*/
private async hasReliableHttpStatusCode(urlOrigin: string): Promise<boolean> {
try {
const url = new URL(
"./.well-known/resource-that-should-not-exist-whose-status-code-should-not-be-200",
urlOrigin,
);
const request = new Request(url, {
method: "GET",
mode: "same-origin",
credentials: "omit",
cache: "no-store",
redirect: "follow",
});
const response = await this.apiService.nativeFetch(request);
return !response.ok;
} catch {
return false;
}
}
/**
* Builds a well-known change password URL for the given origin. Attempts to fetch the URL to ensure a valid response
* is returned. Returns null if the request throws or the response is not 200 OK.
* See https://w3c.github.io/webappsec-change-password-url/
* @param urlOrigin The origin of the URL to check
*/
private async getWellKnownChangePasswordUrl(urlOrigin: string): Promise<string | null> {
try {
const url = new URL("./.well-known/change-password", urlOrigin);
const request = new Request(url, {
method: "GET",
mode: "same-origin",
credentials: "omit",
cache: "no-store",
redirect: "follow",
});
const response = await this.apiService.nativeFetch(request);
return response.ok ? url.toString() : null;
} catch {
return null;
}
}
}