diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index a7cee53e08..50c629e87f 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -1641,6 +1641,9 @@ "selfHostedEnvFormInvalid": { "message": "You must add either the base Server URL or at least one custom environment." }, + "selfHostedEnvMustUseHttps": { + "message": "URLs must use HTTPS." + }, "customEnvironment": { "message": "Custom environment" }, diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 8170c2a65a..561ad5e9c9 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -1215,7 +1215,7 @@ export default class MainBackground { logoutCallback, this.messagingService, this.accountService, - new SignalRConnectionService(this.apiService, this.logService), + new SignalRConnectionService(this.apiService, this.logService, this.platformUtilsService), this.authService, this.webPushConnectionService, this.authRequestAnsweringService, diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index 605fefb03f..70242b6674 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -1035,6 +1035,9 @@ "selfHostedEnvFormInvalid": { "message": "You must add either the base Server URL or at least one custom environment." }, + "selfHostedEnvMustUseHttps": { + "message": "URLs must use HTTPS." + }, "customEnvironment": { "message": "Custom environment" }, diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 7dd0d99a4e..2e0fd99bbc 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -7132,6 +7132,9 @@ "selfHostedEnvFormInvalid": { "message": "You must add either the base Server URL or at least one custom environment." }, + "selfHostedEnvMustUseHttps": { + "message": "URLs must use HTTPS." + }, "apiUrl": { "message": "API server URL" }, diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 94b9f6240a..853877da81 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -968,7 +968,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: SignalRConnectionService, useClass: SignalRConnectionService, - deps: [ApiServiceAbstraction, LogService], + deps: [ApiServiceAbstraction, LogService, PlatformUtilsServiceAbstraction], }), safeProvider({ provide: WebPushConnectionService, @@ -1223,7 +1223,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: AnonymousHubServiceAbstraction, useClass: AnonymousHubService, - deps: [EnvironmentService, AuthRequestServiceAbstraction], + deps: [EnvironmentService, AuthRequestServiceAbstraction, PlatformUtilsServiceAbstraction], }), safeProvider({ provide: ValidationServiceAbstraction, diff --git a/libs/auth/src/angular/self-hosted-env-config-dialog/self-hosted-env-config-dialog.component.ts b/libs/auth/src/angular/self-hosted-env-config-dialog/self-hosted-env-config-dialog.component.ts index 40fdfb8c17..6fb40179af 100644 --- a/libs/auth/src/angular/self-hosted-env-config-dialog/self-hosted-env-config-dialog.component.ts +++ b/libs/auth/src/angular/self-hosted-env-config-dialog/self-hosted-env-config-dialog.component.ts @@ -1,5 +1,5 @@ import { CommonModule } from "@angular/common"; -import { Component, OnDestroy, OnInit } from "@angular/core"; +import { Component, inject, OnDestroy, OnInit } from "@angular/core"; import { AbstractControl, FormBuilder, @@ -16,6 +16,8 @@ import { EnvironmentService, Region, } from "@bitwarden/common/platform/abstractions/environment.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports import { @@ -51,6 +53,25 @@ function selfHostedEnvSettingsFormValidator(): ValidatorFn { }; } +function onlyHttpsValidator(): ValidatorFn { + const i18nService = inject(I18nService); + const platformUtilsService = inject(PlatformUtilsService); + + return (control: AbstractControl): ValidationErrors | null => { + const url = control.value as string; + + if (url && !url.startsWith("https://") && !platformUtilsService.isDev()) { + return { + onlyHttpsAllowed: { + message: i18nService.t("selfHostedEnvMustUseHttps"), + }, + }; // invalid + } + + return null; // valid + }; +} + /** * Dialog for configuring self-hosted environment settings. */ @@ -89,12 +110,12 @@ export class SelfHostedEnvConfigDialogComponent implements OnInit, OnDestroy { formGroup = this.formBuilder.group( { - baseUrl: [""], - webVaultUrl: [""], - apiUrl: [""], - identityUrl: [""], - iconsUrl: [""], - notificationsUrl: [""], + baseUrl: ["", [onlyHttpsValidator()]], + webVaultUrl: ["", [onlyHttpsValidator()]], + apiUrl: ["", [onlyHttpsValidator()]], + identityUrl: ["", [onlyHttpsValidator()]], + iconsUrl: ["", [onlyHttpsValidator()]], + notificationsUrl: ["", [onlyHttpsValidator()]], }, { validators: selfHostedEnvSettingsFormValidator() }, ); @@ -162,10 +183,11 @@ export class SelfHostedEnvConfigDialogComponent implements OnInit, OnDestroy { }); } submit = async () => { + this.formGroup.markAllAsTouched(); this.showErrorSummary = false; if (this.formGroup.invalid) { - this.showErrorSummary = true; + this.showErrorSummary = Boolean(this.formGroup.errors?.["atLeastOneUrlIsRequired"]); return; } diff --git a/libs/common/src/auth/services/anonymous-hub.service.ts b/libs/common/src/auth/services/anonymous-hub.service.ts index 3900dd53ee..561cddb537 100644 --- a/libs/common/src/auth/services/anonymous-hub.service.ts +++ b/libs/common/src/auth/services/anonymous-hub.service.ts @@ -18,6 +18,8 @@ import { NotificationResponse, } from "../../models/response/notification.response"; import { EnvironmentService } from "../../platform/abstractions/environment.service"; +import { PlatformUtilsService } from "../../platform/abstractions/platform-utils.service"; +import { InsecureUrlNotAllowedError } from "../../services/api-errors"; import { AnonymousHubService as AnonymousHubServiceAbstraction } from "../abstractions/anonymous-hub.service"; export class AnonymousHubService implements AnonymousHubServiceAbstraction { @@ -27,10 +29,14 @@ export class AnonymousHubService implements AnonymousHubServiceAbstraction { constructor( private environmentService: EnvironmentService, private authRequestService: AuthRequestServiceAbstraction, + private platformUtilsService: PlatformUtilsService, ) {} async createHubConnection(token: string) { this.url = (await firstValueFrom(this.environmentService.environment$)).getNotificationsUrl(); + if (!this.url.startsWith("https://") && !this.platformUtilsService.isDev()) { + throw new InsecureUrlNotAllowedError(); + } this.anonHubConnection = new HubConnectionBuilder() .withUrl(this.url + "/anonymous-hub?Token=" + token, { diff --git a/libs/common/src/platform/server-notifications/internal/signalr-connection.service.ts b/libs/common/src/platform/server-notifications/internal/signalr-connection.service.ts index 5998668f13..2a9e7fc714 100644 --- a/libs/common/src/platform/server-notifications/internal/signalr-connection.service.ts +++ b/libs/common/src/platform/server-notifications/internal/signalr-connection.service.ts @@ -10,8 +10,10 @@ import { Observable, Subscription } from "rxjs"; import { ApiService } from "../../../abstractions/api.service"; import { NotificationResponse } from "../../../models/response/notification.response"; +import { InsecureUrlNotAllowedError } from "../../../services/api-errors"; import { UserId } from "../../../types/guid"; import { LogService } from "../../abstractions/log.service"; +import { PlatformUtilsService } from "../../abstractions/platform-utils.service"; // 2 Minutes const MIN_RECONNECT_TIME = 2 * 60 * 1000; @@ -69,12 +71,17 @@ export class SignalRConnectionService { constructor( private readonly apiService: ApiService, private readonly logService: LogService, + private readonly platformUtilsService: PlatformUtilsService, private readonly hubConnectionBuilderFactory: () => HubConnectionBuilder = () => new HubConnectionBuilder(), private readonly timeoutManager: TimeoutManager = globalThis, ) {} connect$(userId: UserId, notificationsUrl: string) { + if (!notificationsUrl.startsWith("https://") && !this.platformUtilsService.isDev()) { + throw new InsecureUrlNotAllowedError(); + } + return new Observable((subsciber) => { const connection = this.hubConnectionBuilderFactory() .withUrl(notificationsUrl + "/hub", { diff --git a/libs/common/src/services/api-errors.ts b/libs/common/src/services/api-errors.ts new file mode 100644 index 0000000000..6dc9c8fce6 --- /dev/null +++ b/libs/common/src/services/api-errors.ts @@ -0,0 +1,9 @@ +export class InsecureUrlNotAllowedError extends Error { + constructor(url?: string) { + if (url === undefined) { + super("Insecure URL not allowed. All URLs must use HTTPS."); + } else { + super(`Insecure URL not allowed: ${url}. All URLs must use HTTPS.`); + } + } +} diff --git a/libs/common/src/services/api.service.spec.ts b/libs/common/src/services/api.service.spec.ts index 6d6e96de9e..1fb8f86697 100644 --- a/libs/common/src/services/api.service.spec.ts +++ b/libs/common/src/services/api.service.spec.ts @@ -20,6 +20,7 @@ import { Environment, EnvironmentService } from "../platform/abstractions/enviro import { LogService } from "../platform/abstractions/log.service"; import { PlatformUtilsService } from "../platform/abstractions/platform-utils.service"; +import { InsecureUrlNotAllowedError } from "./api-errors"; import { ApiService, HttpOperations } from "./api.service"; describe("ApiService", () => { @@ -411,4 +412,39 @@ describe("ApiService", () => { ).rejects.toMatchObject(error); }, ); + + it("throws error when trying to fetch an insecure URL", async () => { + environmentService.getEnvironment$.calledWith(testActiveUser).mockReturnValue( + of({ + getApiUrl: () => "http://example.com", + } satisfies Partial as Environment), + ); + + httpOperations.createRequest.mockImplementation((url, request) => { + return { + url: url, + cache: request.cache, + credentials: request.credentials, + method: request.method, + mode: request.mode, + signal: request.signal ?? undefined, + headers: new Headers(request.headers), + } satisfies Partial as unknown as Request; + }); + + const nativeFetch = jest.fn, [request: Request]>(); + nativeFetch.mockImplementation((request) => { + return Promise.resolve({ + ok: true, + status: 204, + headers: new Headers(), + } satisfies Partial as Response); + }); + sut.nativeFetch = nativeFetch; + + await expect( + async () => await sut.send("GET", "/something", null, true, true, null), + ).rejects.toThrow(InsecureUrlNotAllowedError); + expect(nativeFetch).not.toHaveBeenCalled(); + }); }); diff --git a/libs/common/src/services/api.service.ts b/libs/common/src/services/api.service.ts index b7f5f0ed00..8314e44e75 100644 --- a/libs/common/src/services/api.service.ts +++ b/libs/common/src/services/api.service.ts @@ -117,6 +117,8 @@ import { AttachmentResponse } from "../vault/models/response/attachment.response import { CipherResponse } from "../vault/models/response/cipher.response"; import { OptionalCipherResponse } from "../vault/models/response/optional-cipher.response"; +import { InsecureUrlNotAllowedError } from "./api-errors"; + export type HttpOperations = { createRequest: (url: string, request: RequestInit) => Request; }; @@ -1310,6 +1312,10 @@ export class ApiService implements ApiServiceAbstraction { } async fetch(request: Request): Promise { + if (!request.url.startsWith("https://") && !this.platformUtilsService.isDev()) { + throw new InsecureUrlNotAllowedError(); + } + if (request.method === "GET") { request.headers.set("Cache-Control", "no-store"); request.headers.set("Pragma", "no-cache");