diff --git a/bobbin/crates/bobbin/src/config.rs b/bobbin/crates/bobbin/src/config.rs index a7dd47b0b..9f2db35d7 100644 --- a/bobbin/crates/bobbin/src/config.rs +++ b/bobbin/crates/bobbin/src/config.rs @@ -1,17 +1,19 @@ use std::collections::HashSet; +use std::fmt; use std::net::SocketAddr; use std::path::{Path, PathBuf}; use std::str::FromStr; use anyhow::{Context, anyhow}; +use base64::Engine as _; +use base64::engine::general_purpose::STANDARD as Base64; use bobbin_xrpc::Cors; use confique::Config; use jacquard_common::types::did::Did; +use serde::Deserialize; use trusted_proxies::{ProxyNetError, TrustedProxies}; use url::Url; -use crate::otlp::OtlpAuth; - const SYSTEM_CONFIG_PATH: &str = "/etc/bobbin/config.toml"; const ENV_PREFIX: &str = "BOBBIN_"; @@ -35,6 +37,7 @@ const KNOWN_KEYS: &[&str] = &[ "record_cache.lru_bytes", "search.heap_bytes", "codesearch.zoekt_url", + "codesearch.zoekt_auth", "knot.allow_private", "knot.require_https", "mirror.url", @@ -70,6 +73,7 @@ const KNOWN_ENVS: &[&str] = &[ "BOBBIN_RECORD_LRU_BYTES", "BOBBIN_SEARCH_HEAP_BYTES", "BOBBIN_CODESEARCH_ZOEKT_URL", + "BOBBIN_CODESEARCH_ZOEKT_AUTH", "BOBBIN_KNOT_ALLOW_PRIVATE", "BOBBIN_KNOT_REQUIRE_HTTPS", "BOBBIN_MIRROR_URL", @@ -309,6 +313,11 @@ pub struct CodeSearchConfig { /// Unset disables `org.tangled.temp.search.searchCode`. #[config(env = "BOBBIN_CODESEARCH_ZOEKT_URL")] pub zoekt_url: Option, + + /// basic auth credentials in `user:pass` format for the zoekt origin. + /// unset sends requests without authorization. + #[config(env = "BOBBIN_CODESEARCH_ZOEKT_AUTH")] + pub zoekt_auth: Option, } #[derive(Debug, Config)] @@ -379,7 +388,7 @@ pub struct OtlpConfig { /// basic auth credentials in `user:pass` format for the collector. /// unset sends requests without authorization. #[config(env = "BOBBIN_OTLP_AUTH")] - pub auth: Option, + pub auth: Option, /// service name reported in otlp resource attributes. #[config( @@ -390,6 +399,36 @@ pub struct OtlpConfig { pub service_name: String, } +/// `user:pass` credentials, held as the `Authorization` header value they encode to. +#[derive(Deserialize)] +#[serde(try_from = "String")] +pub struct BasicAuth(String); + +impl TryFrom for BasicAuth { + type Error = &'static str; + + fn try_from(credentials: String) -> Result { + match credentials.split_once(':') { + Some((user, _)) if !user.is_empty() => { + Ok(Self(format!("Basic {}", Base64.encode(credentials)))) + } + _ => Err("credentials must be `user:pass`"), + } + } +} + +impl fmt::Debug for BasicAuth { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("BasicAuth()") + } +} + +impl BasicAuth { + pub fn authorization(&self) -> &str { + &self.0 + } +} + #[allow(clippy::ptr_arg)] fn non_empty(s: &String) -> Result<(), &'static str> { if s.is_empty() { @@ -563,6 +602,15 @@ mod tests { assert!(Cors::parse_env("https://x,*").is_err()); } + #[test] + fn basic_auth_is_validated_encoded_and_redacted() { + let auth = BasicAuth::try_from("user:secretpass".to_owned()).unwrap(); + assert_eq!(auth.authorization(), "Basic dXNlcjpzZWNyZXRwYXNz"); + assert_eq!(format!("{auth:?}"), "BasicAuth()"); + assert!(BasicAuth::try_from("no-colon".to_owned()).is_err()); + assert!(BasicAuth::try_from(":pass".to_owned()).is_err()); + } + #[test] fn known_keys_exactly_match_template_paths() { let template = template(); diff --git a/bobbin/crates/bobbin/src/main.rs b/bobbin/crates/bobbin/src/main.rs index 71e53a16d..6bc70d8fa 100644 --- a/bobbin/crates/bobbin/src/main.rs +++ b/bobbin/crates/bobbin/src/main.rs @@ -280,7 +280,13 @@ async fn run(cfg: BobbinConfig) -> anyhow::Result<()> { .codesearch .zoekt_url .as_ref() - .map(|url| CodeSearch::new(url).map(Arc::new)) + .map(|url| -> anyhow::Result<_> { + let search = CodeSearch::new(url)?; + Ok(Arc::new(match cfg.codesearch.zoekt_auth.as_ref() { + Some(auth) => search.with_authorization(auth.authorization())?, + None => search, + })) + }) .transpose() .context("codesearch.zoekt_url")?; match cfg.codesearch.zoekt_url.as_ref() { diff --git a/bobbin/crates/bobbin/src/otlp.rs b/bobbin/crates/bobbin/src/otlp.rs index 08a5dd776..d6eb1701f 100644 --- a/bobbin/crates/bobbin/src/otlp.rs +++ b/bobbin/crates/bobbin/src/otlp.rs @@ -1,9 +1,6 @@ use std::collections::HashMap; -use std::fmt; use std::time::Duration; -use base64::Engine as _; -use base64::engine::general_purpose::STANDARD as Base64; use bobbin_runtime::{Entropy as _, OsEntropy}; use opentelemetry::{KeyValue, global}; use opentelemetry_appender_tracing::layer::{OpenTelemetryTracingBridge, TracingSpanAttributes}; @@ -13,7 +10,6 @@ use opentelemetry_otlp::{ use opentelemetry_sdk::Resource; use opentelemetry_sdk::logs::SdkLoggerProvider; use opentelemetry_sdk::metrics::{PeriodicReader, SdkMeterProvider}; -use serde::Deserialize; use tracing::Subscriber; use tracing_subscriber::Layer; use tracing_subscriber::filter::{LevelFilter, Targets}; @@ -27,35 +23,6 @@ const EXPORT_QUIET: [&str; 4] = ["h2", "hyper", "reqwest", "opentelemetry"]; const METRICS_INTERVAL: Duration = Duration::from_secs(15); -#[derive(Deserialize)] -#[serde(try_from = "String")] -pub struct OtlpAuth(String); - -impl TryFrom for OtlpAuth { - type Error = &'static str; - - fn try_from(credentials: String) -> Result { - match credentials.split_once(':') { - Some((user, _)) if !user.is_empty() => { - Ok(Self(format!("Basic {}", Base64.encode(credentials)))) - } - _ => Err("otlp.auth must be `user:pass`"), - } - } -} - -impl fmt::Debug for OtlpAuth { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str("OtlpAuth()") - } -} - -impl OtlpAuth { - fn headers(&self) -> HashMap { - HashMap::from([("Authorization".to_owned(), self.0.clone())]) - } -} - pub struct Export { logs: SdkLoggerProvider, metrics: SdkMeterProvider, @@ -66,7 +33,13 @@ impl Export { let Some(base) = &cfg.endpoint else { return Ok(None); }; - let headers = cfg.auth.as_ref().map(OtlpAuth::headers).unwrap_or_default(); + let headers = cfg + .auth + .as_ref() + .map(|auth| { + HashMap::from([("Authorization".to_owned(), auth.authorization().to_owned())]) + }) + .unwrap_or_default(); let logs = LogExporter::builder() .with_http() .with_protocol(Protocol::HttpBinary) @@ -222,18 +195,6 @@ mod tests { assert!(exporter.is_shutdown_called()); } - #[test] - fn auth_is_validated_encoded_and_redacted() { - let auth = OtlpAuth::try_from("user:secretpass".to_owned()).unwrap(); - assert_eq!( - auth.headers().get("Authorization").map(String::as_str), - Some("Basic dXNlcjpzZWNyZXRwYXNz"), - ); - assert_eq!(format!("{auth:?}"), "OtlpAuth()"); - assert!(OtlpAuth::try_from("no-colon".to_owned()).is_err()); - assert!(OtlpAuth::try_from(":pass".to_owned()).is_err()); - } - #[tokio::test] async fn installed_meters_export_xrpc_metrics_per_instance() { use axum::routing::get; diff --git a/bobbin/crates/codesearch/src/lib.rs b/bobbin/crates/codesearch/src/lib.rs index 39e2edb61..338f66d30 100644 --- a/bobbin/crates/codesearch/src/lib.rs +++ b/bobbin/crates/codesearch/src/lib.rs @@ -8,6 +8,7 @@ use std::time::Duration; use base64::Engine as _; use base64::engine::general_purpose::STANDARD as BASE64; use http::StatusCode; +use http::header::{AUTHORIZATION, HeaderValue}; use jacquard_common::DefaultStr; use jacquard_common::types::did::Did; use serde::{Deserialize, Serialize}; @@ -26,6 +27,8 @@ const MAX_ERROR_BODY: usize = 4096; pub enum CodeSearchError { #[error("invalid zoekt url scheme: {0}")] BadScheme(String), + #[error("authorization is not a valid header value")] + BadAuthorization, #[error("http client build: {0}")] Build(String), #[error("network: {0}")] @@ -41,6 +44,7 @@ pub enum CodeSearchError { pub struct CodeSearch { http: reqwest::Client, search_url: Url, + authorization: Option, } impl std::fmt::Debug for CodeSearch { @@ -75,7 +79,20 @@ impl CodeSearch { .connect_timeout(CONNECT_TIMEOUT) .build() .map_err(|e| CodeSearchError::Build(e.to_string()))?; - Ok(Self { http, search_url }) + Ok(Self { + http, + search_url, + authorization: None, + }) + } + + /// Sends `value` as the `Authorization` header on every search. + pub fn with_authorization(mut self, value: &str) -> Result { + let mut value = + HeaderValue::from_str(value).map_err(|_| CodeSearchError::BadAuthorization)?; + value.set_sensitive(true); + self.authorization = Some(value); + Ok(self) } pub async fn search( @@ -94,10 +111,11 @@ impl CodeSearch { }, }; - let resp = self - .http - .post(self.search_url.clone()) - .json(&args) + let mut request = self.http.post(self.search_url.clone()).json(&args); + if let Some(authorization) = &self.authorization { + request = request.header(AUTHORIZATION, authorization.clone()); + } + let resp = request .send() .await .map_err(|e| CodeSearchError::Network(e.to_string()))?; @@ -267,6 +285,15 @@ mod tests { assert!(result.repo_urls.unwrap_or_default().is_empty()); } + #[test] + fn rejects_an_authorization_that_is_not_a_header_value() { + let search = CodeSearch::new(&Url::parse("http://127.0.0.1:1").unwrap()).unwrap(); + assert!(matches!( + search.with_authorization("Basic\nabc"), + Err(CodeSearchError::BadAuthorization), + )); + } + #[test] fn extracts_did_from_url_template() { assert_eq!( diff --git a/bobbin/crates/xrpc/src/codesearch.rs b/bobbin/crates/xrpc/src/codesearch.rs index 7c8d1083d..386ad9da2 100644 --- a/bobbin/crates/xrpc/src/codesearch.rs +++ b/bobbin/crates/xrpc/src/codesearch.rs @@ -200,7 +200,9 @@ fn map_codesearch_err(err: CodeSearchError) -> XrpcError { XrpcError::UpstreamUnavailable(format!("code search: {e}")) } e @ E::Decode(_) => XrpcError::InvalidRecord(format!("code search: {e}")), - e @ (E::BadScheme(_) | E::Build(_)) => XrpcError::Internal(format!("code search: {e}")), + e @ (E::BadScheme(_) | E::Build(_) | E::BadAuthorization) => { + XrpcError::Internal(format!("code search: {e}")) + } } } diff --git a/bobbin/crates/xrpc/tests/code_search.rs b/bobbin/crates/xrpc/tests/code_search.rs index f5fe97a68..793cd1a6a 100644 --- a/bobbin/crates/xrpc/tests/code_search.rs +++ b/bobbin/crates/xrpc/tests/code_search.rs @@ -11,6 +11,8 @@ use bobbin_xrpc::{AppState, CodeSearch, router}; use http::{Request, StatusCode}; use tower::ServiceExt; use url::Url; +use wiremock::matchers::{header, method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; const NSID: &str = "org.tangled.temp.search.searchCode"; @@ -75,3 +77,22 @@ async fn the_nsid_is_a_query_not_a_procedure() { .unwrap(); assert_eq!(resp.status(), StatusCode::METHOD_NOT_ALLOWED); } + +/// The zoekt origin sits behind basic auth, so a configured credential must reach it. +#[tokio::test] +async fn zoekt_gets_the_configured_authorization() { + let zoekt = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/api/search")) + .and(header("authorization", "Basic dXNlcjpwYXNz")) + .respond_with(ResponseTemplate::new(200).set_body_string(r#"{"Result":null}"#)) + .expect(1) + .mount(&zoekt) + .await; + + let search = CodeSearch::new(&Url::parse(&zoekt.uri()).unwrap()) + .unwrap() + .with_authorization("Basic dXNlcjpwYXNz") + .unwrap(); + search.search("fn main", 0, 10).await.unwrap(); +} diff --git a/bobbin/example.toml b/bobbin/example.toml index ffaa91965..18a70db72 100644 --- a/bobbin/example.toml +++ b/bobbin/example.toml @@ -175,6 +175,12 @@ # Can also be specified via environment variable `BOBBIN_CODESEARCH_ZOEKT_URL`. #zoekt_url = +# basic auth credentials in `user:pass` format for the zoekt origin. +# unset sends requests without authorization. +# +# Can also be specified via environment variable `BOBBIN_CODESEARCH_ZOEKT_AUTH`. +#zoekt_auth = + [knot] # Whether to allow the knot proxy to dial private/loopback addresses. Off in # production - on for local testing against a knotserver on localhost.