mirror of
https://github.com/bitwarden/browser
synced 2025-12-06 00:13:28 +00:00
[PM-21024] Use Server for Password Change URLs (#14912)
* migrate change login password service to use bitwarden server rather than fetch directly - avoids CSP entirely * add `HelpUsersUpdatePasswords` policy to policy type * add `HelpUsersUpdatePasswordsPolicy` components * allow list description override for policy description * add `HelpUsersUpdatePasswordsPolicy` when the feature flag is enabled * apply `HelpUsersUpdatePasswords` to everyone in an org * use policy to guard the well known password API * fix tests * refactor to use `policyAppliesToUser$` * remove policy work for change password - this was removed from scope * update copy for show favicon setting - it now handles both favicons and change password urls * remove favicon setting description - no longer needed * only call change password service when the setting is enabled * add popover for permitting cipher details * import permit popover directly into the settings component * replace `nativeFetch` with `fetch` * use string literal to construct URL rather than `URL` class - The `getIconsUrl` can return with an appended path which the new URL constructor will strip when passed as the base parameter * use string literal to construct URL rather than `URL` class instance (#16045) - The `getIconsUrl` can return with an appended path which the new URL constructor will strip when passed as the base parameter * [PM-24716] UI changes for Change URI work (#16043) * use platform service to launch the URI - this allows desktop to open a separate browser instance rather than use electron * fix spacing on web app * add bitLink for focus/hover states * remove spacing around links
This commit is contained in:
@@ -0,0 +1,22 @@
|
||||
<button
|
||||
type="button"
|
||||
class="tw-border-none tw-bg-transparent tw-text-primary-600 tw-p-0"
|
||||
[bitPopoverTriggerFor]="permitDetailsPopover"
|
||||
position="above-center"
|
||||
[appA11yTitle]="'aboutThisSetting' | i18n"
|
||||
bitLink
|
||||
>
|
||||
<i class="bwi bwi-question-circle"></i>
|
||||
</button>
|
||||
|
||||
<bit-popover [title]="'aboutThisSetting' | i18n" #permitDetailsPopover>
|
||||
<p>
|
||||
{{ "permitCipherDetailsDescription" | i18n }}
|
||||
</p>
|
||||
<div class="tw-flex tw-gap-1.5 tw-items-center">
|
||||
<a bitLink href="#" (click)="openLearnMore($event)" class="tw-flex">
|
||||
{{ "learnMore" | i18n }}
|
||||
</a>
|
||||
<i slot="end" class="bwi bwi-external-link tw-text-primary-600" aria-hidden="true"></i>
|
||||
</div>
|
||||
</bit-popover>
|
||||
@@ -0,0 +1,19 @@
|
||||
import { Component, inject } from "@angular/core";
|
||||
|
||||
import { JslibModule } from "@bitwarden/angular/jslib.module";
|
||||
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
|
||||
import { LinkModule, PopoverModule } from "@bitwarden/components";
|
||||
|
||||
@Component({
|
||||
selector: "vault-permit-cipher-details-popover",
|
||||
templateUrl: "./permit-cipher-details-popover.component.html",
|
||||
imports: [PopoverModule, JslibModule, LinkModule],
|
||||
})
|
||||
export class PermitCipherDetailsPopoverComponent {
|
||||
private platformUtilService = inject(PlatformUtilsService);
|
||||
|
||||
openLearnMore(e: Event) {
|
||||
e.preventDefault();
|
||||
this.platformUtilService.launchUri("https://bitwarden.com/help/website-icons/");
|
||||
}
|
||||
}
|
||||
@@ -20,6 +20,7 @@ export { openPasswordHistoryDialog } from "./components/password-history/passwor
|
||||
export * from "./components/add-edit-folder-dialog/add-edit-folder-dialog.component";
|
||||
export * from "./components/carousel";
|
||||
export * from "./components/new-cipher-menu/new-cipher-menu.component";
|
||||
export * from "./components/permit-cipher-details-popover/permit-cipher-details-popover.component";
|
||||
|
||||
export { DefaultSshImportPromptService } from "./services/default-ssh-import-prompt.service";
|
||||
export { SshImportPromptService } from "./services/ssh-import-prompt.service";
|
||||
|
||||
@@ -4,10 +4,14 @@
|
||||
*/
|
||||
|
||||
import { mock } from "jest-mock-extended";
|
||||
import { BehaviorSubject, of } from "rxjs";
|
||||
|
||||
import { ApiService } from "@bitwarden/common/abstractions/api.service";
|
||||
import { ClientType } from "@bitwarden/common/enums";
|
||||
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
|
||||
import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service";
|
||||
import {
|
||||
Environment,
|
||||
EnvironmentService,
|
||||
} from "@bitwarden/common/platform/abstractions/environment.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";
|
||||
@@ -18,37 +22,30 @@ import { DefaultChangeLoginPasswordService } from "./default-change-login-passwo
|
||||
describe("DefaultChangeLoginPasswordService", () => {
|
||||
let service: DefaultChangeLoginPasswordService;
|
||||
|
||||
let mockShouldNotExistResponse: Response;
|
||||
let mockWellKnownResponse: Response;
|
||||
|
||||
const getClientType = jest.fn(() => ClientType.Browser);
|
||||
|
||||
const mockApiService = mock<ApiService>();
|
||||
const platformUtilsService = mock<PlatformUtilsService>({
|
||||
getClientType,
|
||||
});
|
||||
const mockDomainSettingsService = mock<DomainSettingsService>();
|
||||
|
||||
const showFavicons$ = new BehaviorSubject<boolean>(true);
|
||||
|
||||
beforeEach(() => {
|
||||
mockApiService.nativeFetch.mockClear();
|
||||
mockApiService.fetch.mockClear();
|
||||
mockApiService.fetch.mockImplementation(() =>
|
||||
Promise.resolve({ ok: true, json: () => Promise.resolve({ uri: null }) } as Response),
|
||||
);
|
||||
|
||||
// Default responses to success state
|
||||
mockShouldNotExistResponse = new Response("Not Found", { status: 404 });
|
||||
mockWellKnownResponse = new Response("OK", { status: 200 });
|
||||
mockDomainSettingsService.showFavicons$ = showFavicons$;
|
||||
|
||||
mockApiService.nativeFetch.mockImplementation((request) => {
|
||||
if (
|
||||
request.url.endsWith("resource-that-should-not-exist-whose-status-code-should-not-be-200")
|
||||
) {
|
||||
return Promise.resolve(mockShouldNotExistResponse);
|
||||
}
|
||||
const mockEnvironmentService = {
|
||||
environment$: of({
|
||||
getIconsUrl: () => "https://icons.bitwarden.com",
|
||||
} as Environment),
|
||||
} as EnvironmentService;
|
||||
|
||||
if (request.url.endsWith(".well-known/change-password")) {
|
||||
return Promise.resolve(mockWellKnownResponse);
|
||||
}
|
||||
|
||||
throw new Error("Unexpected request");
|
||||
});
|
||||
service = new DefaultChangeLoginPasswordService(mockApiService, platformUtilsService);
|
||||
service = new DefaultChangeLoginPasswordService(
|
||||
mockApiService,
|
||||
mockEnvironmentService,
|
||||
mockDomainSettingsService,
|
||||
);
|
||||
});
|
||||
|
||||
it("should return null for non-login ciphers", async () => {
|
||||
@@ -85,7 +82,7 @@ describe("DefaultChangeLoginPasswordService", () => {
|
||||
expect(url).toBeNull();
|
||||
});
|
||||
|
||||
it("should check the origin for a reliable status code", async () => {
|
||||
it("should call the icons url endpoint", async () => {
|
||||
const cipher = {
|
||||
type: CipherType.Login,
|
||||
login: Object.assign(new LoginView(), {
|
||||
@@ -95,35 +92,42 @@ describe("DefaultChangeLoginPasswordService", () => {
|
||||
|
||||
await service.getChangePasswordUrl(cipher);
|
||||
|
||||
expect(mockApiService.nativeFetch).toHaveBeenCalledWith(
|
||||
expect(mockApiService.fetch).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
url: "https://example.com/.well-known/resource-that-should-not-exist-whose-status-code-should-not-be-200",
|
||||
url: "https://icons.bitwarden.com/change-password-uri?uri=https%3A%2F%2Fexample.com%2F",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("should attempt to fetch the well-known change password URL", async () => {
|
||||
it("should return the original URI when unable to verify the response", async () => {
|
||||
mockApiService.fetch.mockImplementation(() =>
|
||||
Promise.resolve({ ok: true, json: () => Promise.resolve({ uri: null }) } as Response),
|
||||
);
|
||||
|
||||
const cipher = {
|
||||
type: CipherType.Login,
|
||||
login: Object.assign(new LoginView(), {
|
||||
uris: [{ uri: "https://example.com" }],
|
||||
uris: [{ uri: "https://example.com/" }],
|
||||
}),
|
||||
} as CipherView;
|
||||
|
||||
await service.getChangePasswordUrl(cipher);
|
||||
const url = await service.getChangePasswordUrl(cipher);
|
||||
|
||||
expect(mockApiService.nativeFetch).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
url: "https://example.com/.well-known/change-password",
|
||||
}),
|
||||
);
|
||||
expect(url).toBe("https://example.com/");
|
||||
});
|
||||
|
||||
it("should return the well-known change password URL when successful at verifying the response", async () => {
|
||||
it("should return the well known change url from the response", async () => {
|
||||
mockApiService.fetch.mockImplementation(() => {
|
||||
return Promise.resolve({
|
||||
ok: true,
|
||||
json: () => Promise.resolve({ uri: "https://example.com/.well-known/change-password" }),
|
||||
} as Response);
|
||||
});
|
||||
|
||||
const cipher = {
|
||||
type: CipherType.Login,
|
||||
login: Object.assign(new LoginView(), {
|
||||
uris: [{ uri: "https://example.com" }],
|
||||
uris: [{ uri: "https://example.com/" }, { uri: "https://working.com/" }],
|
||||
}),
|
||||
} as CipherView;
|
||||
|
||||
@@ -132,49 +136,20 @@ describe("DefaultChangeLoginPasswordService", () => {
|
||||
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/");
|
||||
});
|
||||
|
||||
it("should try the next URI if the first one fails", async () => {
|
||||
mockApiService.nativeFetch.mockImplementation((request) => {
|
||||
if (
|
||||
request.url.endsWith("resource-that-should-not-exist-whose-status-code-should-not-be-200")
|
||||
) {
|
||||
return Promise.resolve(mockShouldNotExistResponse);
|
||||
mockApiService.fetch.mockImplementation((request) => {
|
||||
if (request.url.includes("no-wellknown.com")) {
|
||||
return Promise.resolve({
|
||||
ok: true,
|
||||
json: () => Promise.resolve({ uri: null }),
|
||||
} as Response);
|
||||
}
|
||||
|
||||
if (request.url.endsWith(".well-known/change-password")) {
|
||||
if (request.url.includes("working.com")) {
|
||||
return Promise.resolve(mockWellKnownResponse);
|
||||
}
|
||||
return Promise.resolve(new Response("Not Found", { status: 404 }));
|
||||
if (request.url.includes("working.com")) {
|
||||
return Promise.resolve({
|
||||
ok: true,
|
||||
json: () => Promise.resolve({ uri: "https://working.com/.well-known/change-password" }),
|
||||
} as Response);
|
||||
}
|
||||
|
||||
throw new Error("Unexpected request");
|
||||
@@ -192,19 +167,19 @@ describe("DefaultChangeLoginPasswordService", () => {
|
||||
expect(url).toBe("https://working.com/.well-known/change-password");
|
||||
});
|
||||
|
||||
it("should return the first URI when the client type is not browser", async () => {
|
||||
getClientType.mockReturnValue(ClientType.Web);
|
||||
it("returns the first URI when `showFavicons$` setting is disabled", async () => {
|
||||
showFavicons$.next(false);
|
||||
|
||||
const cipher = {
|
||||
type: CipherType.Login,
|
||||
login: Object.assign(new LoginView(), {
|
||||
uris: [{ uri: "https://example.com/" }, { uri: "https://example-2.com/" }],
|
||||
uris: [{ uri: "https://example.com/" }, { uri: "https://another.com/" }],
|
||||
}),
|
||||
} as CipherView;
|
||||
|
||||
const url = await service.getChangePasswordUrl(cipher);
|
||||
|
||||
expect(mockApiService.nativeFetch).not.toHaveBeenCalled();
|
||||
expect(url).toBe("https://example.com/");
|
||||
expect(mockApiService.fetch).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,9 +1,12 @@
|
||||
import { Injectable } from "@angular/core";
|
||||
import { firstValueFrom } from "rxjs";
|
||||
|
||||
import { ApiService } from "@bitwarden/common/abstractions/api.service";
|
||||
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
|
||||
import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service";
|
||||
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
|
||||
import { Utils } from "@bitwarden/common/platform/misc/utils";
|
||||
import { CipherType } from "@bitwarden/common/vault/enums";
|
||||
import { ChangePasswordUriResponse } from "@bitwarden/common/vault/models/response/change-password-uri.response";
|
||||
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
|
||||
|
||||
import { ChangeLoginPasswordService } from "../abstractions/change-login-password.service";
|
||||
@@ -12,7 +15,8 @@ import { ChangeLoginPasswordService } from "../abstractions/change-login-passwor
|
||||
export class DefaultChangeLoginPasswordService implements ChangeLoginPasswordService {
|
||||
constructor(
|
||||
private apiService: ApiService,
|
||||
private platformUtilsService: PlatformUtilsService,
|
||||
private environmentService: EnvironmentService,
|
||||
private domainSettingsService: DomainSettingsService,
|
||||
) {}
|
||||
|
||||
/**
|
||||
@@ -33,24 +37,19 @@ export class DefaultChangeLoginPasswordService implements ChangeLoginPasswordSer
|
||||
return null;
|
||||
}
|
||||
|
||||
// CSP policies on the web and desktop restrict the application from making
|
||||
// cross-origin requests, breaking the below .well-known URL checks.
|
||||
// For those platforms, this will short circuit and return the first URL.
|
||||
// PM-21024 will build a solution for the server side to handle this.
|
||||
if (this.platformUtilsService.getClientType() !== "browser") {
|
||||
const enableFaviconChangePassword = await firstValueFrom(
|
||||
this.domainSettingsService.showFavicons$,
|
||||
);
|
||||
|
||||
// When the setting is not enabled, return the first URL
|
||||
if (!enableFaviconChangePassword) {
|
||||
return urls[0].href;
|
||||
}
|
||||
|
||||
for (const url of urls) {
|
||||
const [reliable, wellKnownChangeUrl] = await Promise.all([
|
||||
this.hasReliableHttpStatusCode(url.origin),
|
||||
this.getWellKnownChangePasswordUrl(url.origin),
|
||||
]);
|
||||
const wellKnownChangeUrl = await this.fetchWellKnownChangePasswordUri(url.href);
|
||||
|
||||
// Some servers return a 200 OK for a resource that should not exist
|
||||
// Which means we cannot trust the well-known URL is valid, so we skip it
|
||||
// to avoid potentially sending users to a 404 page
|
||||
if (reliable && wellKnownChangeUrl != null) {
|
||||
if (wellKnownChangeUrl) {
|
||||
return wellKnownChangeUrl;
|
||||
}
|
||||
}
|
||||
@@ -60,55 +59,41 @@ export class DefaultChangeLoginPasswordService implements ChangeLoginPasswordSer
|
||||
}
|
||||
|
||||
/**
|
||||
* 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
|
||||
* Fetches the well-known change-password-uri for the given URL.
|
||||
* @returns The full URL to the change password page, or null if it could not be found.
|
||||
*/
|
||||
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,
|
||||
);
|
||||
private async fetchWellKnownChangePasswordUri(url: string): Promise<string | null> {
|
||||
const getChangePasswordUriRequest = await this.buildChangePasswordUriRequest(url);
|
||||
|
||||
const request = new Request(url, {
|
||||
method: "GET",
|
||||
mode: "same-origin",
|
||||
credentials: "omit",
|
||||
cache: "no-store",
|
||||
redirect: "follow",
|
||||
});
|
||||
const response = await this.apiService.fetch(getChangePasswordUriRequest);
|
||||
|
||||
const response = await this.apiService.nativeFetch(request);
|
||||
return !response.ok;
|
||||
} catch {
|
||||
return false;
|
||||
if (!response.ok) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const data = await response.json();
|
||||
|
||||
const { uri } = new ChangePasswordUriResponse(data);
|
||||
|
||||
return uri;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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
|
||||
* Construct the request for the change-password-uri endpoint.
|
||||
*/
|
||||
private async getWellKnownChangePasswordUrl(urlOrigin: string): Promise<string | null> {
|
||||
try {
|
||||
const url = new URL("./.well-known/change-password", urlOrigin);
|
||||
private async buildChangePasswordUriRequest(cipherUri: string): Promise<Request> {
|
||||
const searchParams = new URLSearchParams();
|
||||
searchParams.set("uri", cipherUri);
|
||||
|
||||
const request = new Request(url, {
|
||||
method: "GET",
|
||||
mode: "same-origin",
|
||||
credentials: "omit",
|
||||
cache: "no-store",
|
||||
redirect: "follow",
|
||||
});
|
||||
// The change-password-uri endpoint lives within the icons service
|
||||
// as it uses decrypted cipher data.
|
||||
const env = await firstValueFrom(this.environmentService.environment$);
|
||||
const iconsUrl = env.getIconsUrl();
|
||||
|
||||
const response = await this.apiService.nativeFetch(request);
|
||||
const url = new URL(`${iconsUrl}/change-password-uri?${searchParams.toString()}`);
|
||||
|
||||
return response.ok ? url.toString() : null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
return new Request(url, {
|
||||
method: "GET",
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user