fix: set user credentials in URL as Authorization header (#5884)

* Add functions to encode / decode base64

* Move URL validation function to utils package

* Improve URL validation and fail on startup

* Set user credentials in URL as Authorization header

* Improve monitoring endpoint validation errors

* Fix http client fallback tests

* Always remove username and password from URL

packages/api/src/utils/client/httpClient.ts

@@ -1,4 +1,4 @@
-import {ErrorAborted, Logger, TimeoutError} from "@lodestar/utils";
+import {ErrorAborted, Logger, TimeoutError, isValidHttpUrl, toBase64} from "@lodestar/utils";
 import {ReqGeneric, RouteDef} from "../index.js";
 import {ApiClientResponse, ApiClientSuccessResponse} from "../../interfaces.js";
 import {fetch, isFetchError} from "./fetch.js";
@@ -123,8 +123,15 @@ export class HttpClient implements IHttpClient {
 
     // opts.baseUrl is equivalent to `urls: [{baseUrl}]`
     // unshift opts.baseUrl to urls, without mutating opts.urls
-    for (const urlOrOpts of [...(baseUrl ? [baseUrl] : []), ...urls]) {
+    for (const [i, urlOrOpts] of [...(baseUrl ? [baseUrl] : []), ...urls].entries()) {
       const urlOpts: URLOpts = typeof urlOrOpts === "string" ? {baseUrl: urlOrOpts, ...allUrlOpts} : urlOrOpts;
+
+      if (!urlOpts.baseUrl) {
+        throw Error(`HttpClient.urls[${i}] is empty or undefined: ${urlOpts.baseUrl}`);
+      }
+      if (!isValidHttpUrl(urlOpts.baseUrl)) {
+        throw Error(`HttpClient.urls[${i}] must be a valid URL: ${urlOpts.baseUrl}`);
+      }
       // De-duplicate by baseUrl, having two baseUrls with different token or timeouts does not make sense
       if (!this.urlsOpts.some((opt) => opt.baseUrl === urlOpts.baseUrl)) {
         this.urlsOpts.push(urlOpts);
@@ -269,7 +276,7 @@ export class HttpClient implements IHttpClient {
     const timer = this.metrics?.requestTime.startTimer({routeId});
 
     try {
-      const url = urlJoin(baseUrl, opts.url) + (opts.query ? "?" + stringifyQuery(opts.query) : "");
+      const url = new URL(urlJoin(baseUrl, opts.url) + (opts.query ? "?" + stringifyQuery(opts.query) : ""));
 
       const headers =
         extraHeaders && opts.headers ? {...extraHeaders, ...opts.headers} : opts.headers || extraHeaders || {};
@@ -279,6 +286,14 @@ export class HttpClient implements IHttpClient {
       if (bearerToken && headers["Authorization"] === undefined) {
         headers["Authorization"] = `Bearer ${bearerToken}`;
       }
+      if (url.username || url.password) {
+        if (headers["Authorization"] === undefined) {
+          headers["Authorization"] = `Basic ${toBase64(`${url.username}:${url.password}`)}`;
+        }
+        // Remove the username and password from the URL
+        url.username = "";
+        url.password = "";
+      }
 
       this.logger?.debug("HttpClient request", {routeId});
 
@@ -291,7 +306,7 @@ export class HttpClient implements IHttpClient {
 
       if (!res.ok) {
         const errBody = await res.text();
-        throw new HttpError(`${res.statusText}: ${getErrorMessage(errBody)}`, res.status, url);
+        throw new HttpError(`${res.statusText}: ${getErrorMessage(errBody)}`, res.status, url.toString());
       }
 
       const streamTimer = this.metrics?.streamTime.startTimer({routeId});

packages/api/test/unit/client/httpClient.test.ts

@@ -135,6 +135,22 @@ describe("httpClient json client", () => {
     }
   });
 
+  it("should set user credentials in URL as Authorization header", async () => {
+    const {baseUrl} = await getServer({
+      ...testRoute,
+      handler: async (req) => {
+        expect(req.headers.authorization).to.equal("Basic dXNlcjpwYXNzd29yZA==");
+        return {};
+      },
+    });
+    const url = new URL(baseUrl);
+    url.username = "user";
+    url.password = "password";
+    const httpClient = new HttpClient({baseUrl: url.toString()});
+
+    await httpClient.json(testRoute);
+  });
+
   it("should handle aborting request with timeout", async () => {
     const {baseUrl} = await getServer({
       ...testRoute,

packages/api/test/unit/client/httpClientFallback.test.ts

@@ -8,7 +8,7 @@ describe("httpClient fallback", () => {
 
   // Using fetchSub instead of actually setting up servers because there are some strange
   // race conditions, where the server stub doesn't count the call in time before the test is over.
-  const fetchStub = Sinon.stub<[string], ReturnType<typeof fetch>>();
+  const fetchStub = Sinon.stub<[URL], ReturnType<typeof fetch>>();
 
   let httpClient: HttpClient;
 
@@ -21,10 +21,10 @@ describe("httpClient fallback", () => {
   const serverErrors = new Map<number, boolean>();
 
   // With baseURLs above find the server index associated with that URL
-  function getServerIndex(url: string): number {
-    const i = baseUrls.findIndex((baseUrl) => url.startsWith(baseUrl));
+  function getServerIndex(url: URL): number {
+    const i = baseUrls.findIndex((baseUrl) => url.toString().startsWith(baseUrl));
     if (i < 0) {
-      throw Error(`fetch called with unknown url ${url}`);
+      throw Error(`fetch called with unknown url ${url.toString()}`);
     }
     return i;
   }

packages/api/test/unit/client/httpClientOptions.test.ts

@@ -67,4 +67,20 @@ describe("HTTPClient options", () => {
       {baseUrl: baseUrl2, bearerToken: bearerToken2},
     ]);
   });
+
+  it("Throw if empty baseUrl", () => {
+    expect(() => new HttpClient({baseUrl: ""})).to.throw(Error);
+  });
+
+  it("Throw if invalid baseUrl", () => {
+    expect(() => new HttpClient({baseUrl: "invalid"})).to.throw(Error);
+  });
+
+  it("Throw if empty value in urls option", () => {
+    expect(() => new HttpClient({urls: [""]})).to.throw(Error);
+  });
+
+  it("Throw if invalid value in urls option", () => {
+    expect(() => new HttpClient({urls: ["invalid"]})).to.throw(Error);
+  });
 });

packages/beacon-node/src/eth1/provider/jsonRpcHttpClient.ts

@@ -1,5 +1,5 @@
 import {fetch} from "@lodestar/api";
-import {ErrorAborted, TimeoutError, retry} from "@lodestar/utils";
+import {ErrorAborted, TimeoutError, isValidHttpUrl, retry} from "@lodestar/utils";
 import {IGauge, IHistogram} from "../../metrics/interface.js";
 import {IJson, RpcPayload} from "../interface.js";
 import {encodeJwtToken} from "./jwt.js";
@@ -92,6 +92,9 @@ export class JsonRpcHttpClient implements IJsonRpcHttpClient {
       if (!url) {
         throw Error(`JsonRpcHttpClient.urls[${i}] is empty or undefined: ${url}`);
       }
+      if (!isValidHttpUrl(url)) {
+        throw Error(`JsonRpcHttpClient.urls[${i}] must be a valid URL: ${url}`);
+      }
     }
 
     this.jwtSecret = opts?.jwtSecret;
@@ -188,8 +191,6 @@ export class JsonRpcHttpClient implements IJsonRpcHttpClient {
    * Fetches JSON and throws detailed errors in case the HTTP request is not ok
    */
   private async fetchJsonOneUrl<R, T = unknown>(url: string, json: T, opts?: ReqOpts): Promise<R> {
-    if (!url) throw Error(`Empty or undefined JSON RPC HTTP client url: ${url}`);
-
     const controller = new AbortController();
     const timeout = setTimeout(() => controller.abort(), opts?.timeout ?? this.opts?.timeout ?? REQUEST_TIMEOUT);
 

packages/beacon-node/src/monitoring/service.ts

@@ -210,7 +210,7 @@ export class MonitoringService {
 
   private parseMonitoringEndpoint(endpoint: string): URL {
     if (!endpoint) {
-      throw new Error("Monitoring endpoint must be provided");
+      throw new Error(`Monitoring endpoint is empty or undefined: ${endpoint}`);
     }
 
     try {
@@ -226,7 +226,7 @@ export class MonitoringService {
 
       return url;
     } catch {
-      throw new Error("Monitoring endpoint must be a valid URL");
+      throw new Error(`Monitoring endpoint must be a valid URL: ${endpoint}`);
     }
   }
 }

packages/beacon-node/test/unit/monitoring/service.test.ts

@@ -58,14 +58,16 @@ describe("monitoring / service", () => {
     });
 
     it("should throw an error if monitoring endpoint is not provided", () => {
-      expect(() => new MonitoringService("beacon", {endpoint: ""}, {register, logger})).to.throw(
-        "Monitoring endpoint must be provided"
+      const endpoint = "";
+      expect(() => new MonitoringService("beacon", {endpoint}, {register, logger})).to.throw(
+        `Monitoring endpoint is empty or undefined: ${endpoint}`
       );
     });
 
     it("should throw an error if monitoring endpoint is not a valid URL", () => {
-      expect(() => new MonitoringService("beacon", {endpoint: "invalid"}, {register, logger})).to.throw(
-        "Monitoring endpoint must be a valid URL"
+      const endpoint = "invalid";
+      expect(() => new MonitoringService("beacon", {endpoint}, {register, logger})).to.throw(
+        `Monitoring endpoint must be a valid URL: ${endpoint}`
       );
     });
 

packages/cli/src/cmds/validator/keymanager/impl.ts

@@ -15,7 +15,8 @@ import {
 } from "@lodestar/api/keymanager";
 import {Interchange, SignerType, Validator} from "@lodestar/validator";
 import {ServerApi} from "@lodestar/api";
-import {getPubkeyHexFromKeystore, isValidatePubkeyHex, isValidHttpUrl} from "../../../util/format.js";
+import {isValidHttpUrl} from "@lodestar/utils";
+import {getPubkeyHexFromKeystore, isValidatePubkeyHex} from "../../../util/format.js";
 import {parseFeeRecipient} from "../../../util/index.js";
 import {DecryptKeystoresThreadPool} from "./decryptKeystores/index.js";
 import {IPersistedKeysBackend} from "./interface.js";

packages/cli/src/cmds/validator/signers/index.ts

@@ -4,9 +4,9 @@ import {deriveEth2ValidatorKeys, deriveKeyFromMnemonic} from "@chainsafe/bls-key
 import {toHexString} from "@chainsafe/ssz";
 import {interopSecretKey} from "@lodestar/state-transition";
 import {externalSignerGetKeys, Signer, SignerType} from "@lodestar/validator";
-import {LogLevel, Logger} from "@lodestar/utils";
+import {LogLevel, Logger, isValidHttpUrl} from "@lodestar/utils";
 import {defaultNetwork, GlobalArgs} from "../../../options/index.js";
-import {assertValidPubkeysHex, isValidHttpUrl, parseRange, YargsError} from "../../../util/index.js";
+import {assertValidPubkeysHex, parseRange, YargsError} from "../../../util/index.js";
 import {getAccountPaths} from "../paths.js";
 import {IValidatorCliArgs} from "../options.js";
 import {PersistedKeysBackend} from "../keymanager/persistedKeys.js";

packages/cli/src/util/format.ts

@@ -56,17 +56,6 @@ export function assertValidPubkeysHex(pubkeysHex: string[]): void {
   }
 }
 
-export function isValidHttpUrl(urlStr: string): boolean {
-  let url;
-  try {
-    url = new URL(urlStr);
-  } catch (_) {
-    return false;
-  }
-
-  return url.protocol === "http:" || url.protocol === "https:";
-}
-
 /**
  * Parses a file to get a list of bootnodes for a network.
  * Bootnodes file is expected to contain bootnode ENR's concatenated by newlines, or commas for

packages/utils/src/base64.ts

@@ -0,0 +1,9 @@
+const hasBufferFrom = typeof Buffer !== "undefined" && typeof Buffer.from === "function";
+
+export function toBase64(value: string): string {
+  return hasBufferFrom ? Buffer.from(value).toString("base64") : btoa(value);
+}
+
+export function fromBase64(value: string): string {
+  return hasBufferFrom ? Buffer.from(value, "base64").toString("utf8") : atob(value);
+}

packages/utils/src/index.ts

@@ -1,5 +1,6 @@
 export * from "./yaml/index.js";
 export * from "./assert.js";
+export * from "./base64.js";
 export * from "./bytes.js";
 export * from "./err.js";
 export * from "./errors.js";
@@ -14,6 +15,7 @@ export * from "./sleep.js";
 export * from "./sort.js";
 export * from "./timeout.js";
 export {RecursivePartial, bnToNum} from "./types.js";
+export * from "./validation.js";
 export * from "./verifyMerkleBranch.js";
 export * from "./promise.js";
 export * from "./waitFor.js";

packages/utils/src/validation.ts

@@ -0,0 +1,10 @@
+export function isValidHttpUrl(urlStr: string): boolean {
+  let url;
+  try {
+    url = new URL(urlStr);
+  } catch (_) {
+    return false;
+  }
+
+  return url.protocol === "http:" || url.protocol === "https:";
+}

packages/utils/test/unit/base64.test.ts

@@ -0,0 +1,15 @@
+import "../setup.js";
+import {expect} from "chai";
+import {toBase64, fromBase64} from "../../src/index.js";
+
+describe("toBase64", () => {
+  it("should encode UTF-8 string as base64 string", () => {
+    expect(toBase64("user:password")).to.be.equal("dXNlcjpwYXNzd29yZA==");
+  });
+});
+
+describe("fromBase64", () => {
+  it("should decode UTF-8 string from base64 string", () => {
+    expect(fromBase64("dXNlcjpwYXNzd29yZA==")).to.be.equal("user:password");
+  });
+});