From 685db337b54b771e90ea752eaff342fa2b9ae447 Mon Sep 17 00:00:00 2001 From: William Chong Date: Mon, 4 May 2026 15:34:37 +0400 Subject: [PATCH 1/8] feat: add Bearer token authentication support The bridge work in DEV-1644 needs Bearer auth on the wire, but the Rust client only knows how to send Basic. This adds an Authentication enum alongside the existing Credentials struct, with Basic and Bearer variants, so per-call options can carry either flavor. The migration is intentionally non-breaking. Credentials and its constructor are unchanged, From for Authentication is provided, and .authenticated() now takes impl Into so existing callsites keep compiling without changes. Bumps the crate to 1.1.0. Closes DEV-1643. --- eventstore-macros/src/lib.rs | 12 +- kurrentdb/Cargo.toml | 2 +- kurrentdb/src/http/mod.rs | 28 +++- .../src/http/persistent_subscriptions.rs | 50 ++---- kurrentdb/src/operations/gossip.rs | 7 +- kurrentdb/src/options/mod.rs | 4 +- kurrentdb/src/request.rs | 158 ++++++++++++++++-- kurrentdb/src/types.rs | 40 +++++ 8 files changed, 233 insertions(+), 68 deletions(-) diff --git a/eventstore-macros/src/lib.rs b/eventstore-macros/src/lib.rs index b85a3906..308022ba 100644 --- a/eventstore-macros/src/lib.rs +++ b/eventstore-macros/src/lib.rs @@ -92,9 +92,15 @@ pub fn options(input: TokenStream) -> TokenStream { } impl #name { - /// Performs the command with the given credentials. - pub fn authenticated(mut self, credentials: crate::types::Credentials) -> Self { - self.common_operation_options.credentials = Some(credentials); + /// Performs the command with the given authentication. + /// + /// Accepts either a `Credentials` (Basic auth) or an `Authentication` + /// value, the latter allowing Bearer token authentication. + pub fn authenticated(mut self, authentication: A) -> Self + where + A: Into, + { + self.common_operation_options.authentication = Some(authentication.into()); self } diff --git a/kurrentdb/Cargo.toml b/kurrentdb/Cargo.toml index 24476b40..b8add70b 100755 --- a/kurrentdb/Cargo.toml +++ b/kurrentdb/Cargo.toml @@ -2,7 +2,7 @@ authors = ["Yorick Laupa "] edition = "2024" name = "kurrentdb" -version = "1.0.0" +version = "1.1.0" # Uncomment if you want to update messages.rs code-gen. # We disabled codegen.rs because it requires having `protoc` installed on your machine diff --git a/kurrentdb/src/http/mod.rs b/kurrentdb/src/http/mod.rs index c50bd307..2150cfba 100644 --- a/kurrentdb/src/http/mod.rs +++ b/kurrentdb/src/http/mod.rs @@ -1,17 +1,33 @@ use tracing::error; pub mod persistent_subscriptions; +/// Resolves the authentication to use for an HTTP call: per-operation override +/// first, falling back to the connection-string default user. +pub(crate) fn resolve_authentication( + options: &crate::options::CommonOperationOptions, + settings: &crate::ClientSettings, +) -> Option { + options.authentication.clone().or_else(|| { + settings + .default_authenticated_user() + .as_ref() + .map(|c| crate::Authentication::Basic(c.clone())) + }) +} + pub fn http_configure_auth( builder: reqwest::RequestBuilder, - creds_opt: Option<&crate::Credentials>, + auth_opt: Option<&crate::Authentication>, ) -> reqwest::RequestBuilder { - if let Some(creds) = creds_opt { - builder.basic_auth( + match auth_opt { + Some(crate::Authentication::Basic(creds)) => builder.basic_auth( unsafe { std::str::from_utf8_unchecked(creds.login.as_ref()) }, unsafe { Some(std::str::from_utf8_unchecked(creds.password.as_ref())) }, - ) - } else { - builder + ), + Some(crate::Authentication::Bearer(token)) => { + builder.bearer_auth(unsafe { std::str::from_utf8_unchecked(token.as_ref()) }) + } + None => builder, } } diff --git a/kurrentdb/src/http/persistent_subscriptions.rs b/kurrentdb/src/http/persistent_subscriptions.rs index 6254e234..e0951317 100644 --- a/kurrentdb/src/http/persistent_subscriptions.rs +++ b/kurrentdb/src/http/persistent_subscriptions.rs @@ -33,14 +33,8 @@ pub(crate) async fn replay_parked_messages( builder = builder.query(&[("stopAt", stop_at.to_string().as_str())]) } - builder = super::http_configure_auth( - builder, - options - .common_operation_options - .credentials - .as_ref() - .or_else(|| settings.default_authenticated_user().as_ref()), - ); + let auth = super::resolve_authentication(&options.common_operation_options, settings); + builder = super::http_configure_auth(builder, auth.as_ref()); super::http_execute_request(builder).await?; @@ -132,14 +126,8 @@ pub(crate) async fn list_all_persistent_subscriptions( .get(format!("{}/subscriptions", handle.url())) .header("content-type", "application/json"); - builder = super::http_configure_auth( - builder, - options - .common_operation_options - .credentials - .as_ref() - .or_else(|| settings.default_authenticated_user().as_ref()), - ); + let auth = super::resolve_authentication(&options.common_operation_options, settings); + builder = super::http_configure_auth(builder, auth.as_ref()); let resp = super::http_execute_request(builder).await?; @@ -179,14 +167,8 @@ where )) .header("content-type", "application/json"); - builder = super::http_configure_auth( - builder, - options - .common_operation_options - .credentials - .as_ref() - .or_else(|| settings.default_authenticated_user().as_ref()), - ); + let auth = super::resolve_authentication(&options.common_operation_options, settings); + builder = super::http_configure_auth(builder, auth.as_ref()); let resp = super::http_execute_request(builder).await?; @@ -228,14 +210,8 @@ where )) .header("content-type", "application/json"); - builder = super::http_configure_auth( - builder, - options - .common_operation_options - .credentials - .as_ref() - .or_else(|| settings.default_authenticated_user().as_ref()), - ); + let auth = super::resolve_authentication(&options.common_operation_options, settings); + builder = super::http_configure_auth(builder, auth.as_ref()); let resp = super::http_execute_request(builder).await?; @@ -258,14 +234,8 @@ pub(crate) async fn restart_persistent_subscription_subsystem( .header("content-type", "application/json") .header("content-length", "0"); - builder = super::http_configure_auth( - builder, - options - .common_operation_options - .credentials - .as_ref() - .or_else(|| settings.default_authenticated_user().as_ref()), - ); + let auth = super::resolve_authentication(&options.common_operation_options, settings); + builder = super::http_configure_auth(builder, auth.as_ref()); super::http_execute_request(builder).await?; diff --git a/kurrentdb/src/operations/gossip.rs b/kurrentdb/src/operations/gossip.rs index 799b4b90..aeae0f37 100644 --- a/kurrentdb/src/operations/gossip.rs +++ b/kurrentdb/src/operations/gossip.rs @@ -78,9 +78,14 @@ pub(crate) async fn http_read( .danger_accept_invalid_certs(!setts.tls_verify_cert) .build()?; + let default_auth = setts + .default_user_name + .as_ref() + .map(|c| crate::Authentication::Basic(c.clone())); + let resp = http_configure_auth( client.get(format!("{}/gossip", handle.url())), - setts.default_user_name.as_ref(), + default_auth.as_ref(), ) .send() .await?; diff --git a/kurrentdb/src/options/mod.rs b/kurrentdb/src/options/mod.rs index 57693d88..6670cdcd 100644 --- a/kurrentdb/src/options/mod.rs +++ b/kurrentdb/src/options/mod.rs @@ -1,6 +1,6 @@ use std::time::Duration; -use crate::Credentials; +use crate::Authentication; pub mod append_to_stream; pub mod batch_append; @@ -21,7 +21,7 @@ pub(crate) trait Options { #[derive(Clone, Default)] pub(crate) struct CommonOperationOptions { - pub(crate) credentials: Option, + pub(crate) authentication: Option, pub(crate) requires_leader: bool, pub(crate) deadline: Option, } diff --git a/kurrentdb/src/request.rs b/kurrentdb/src/request.rs index a02a2f67..b6950b97 100644 --- a/kurrentdb/src/request.rs +++ b/kurrentdb/src/request.rs @@ -1,6 +1,7 @@ use crate::options::CommonOperationOptions; -use crate::{ClientSettings, NodePreference}; +use crate::{Authentication, ClientSettings, Credentials, NodePreference}; use base64::Engine; +use std::borrow::Cow; pub(crate) fn build_request_metadata( settings: &ClientSettings, @@ -11,22 +12,19 @@ where use tonic::metadata::MetadataValue; let mut metadata = tonic::metadata::MetadataMap::new(); - let credentials = options - .credentials + let authentication: Option> = options + .authentication .as_ref() - .or_else(|| settings.default_authenticated_user().as_ref()); + .map(Cow::Borrowed) + .or_else(|| { + settings + .default_authenticated_user() + .as_ref() + .map(|c| Cow::Owned(Authentication::Basic(c.clone()))) + }); - if let Some(creds) = credentials { - let login = String::from_utf8_lossy(&creds.login).into_owned(); - let password = String::from_utf8_lossy(&creds.password).into_owned(); - - let basic_auth_string = - base64::engine::general_purpose::STANDARD.encode(format!("{}:{}", login, password)); - let basic_auth = format!("Basic {}", basic_auth_string); - let header_value = MetadataValue::try_from(basic_auth.as_str()) - .expect("Auth header value should be valid metadata header value"); - - metadata.insert("authorization", header_value); + if let Some(auth) = authentication.as_deref() { + metadata.insert("authorization", build_authorization_header(auth)); } if options.requires_leader || settings.node_preference() == NodePreference::Leader { @@ -42,3 +40,133 @@ where metadata } + +fn build_authorization_header( + auth: &Authentication, +) -> tonic::metadata::MetadataValue { + use tonic::metadata::MetadataValue; + + let header = match auth { + Authentication::Basic(Credentials { login, password }) => { + let login = String::from_utf8_lossy(login); + let password = String::from_utf8_lossy(password); + let encoded = base64::engine::general_purpose::STANDARD + .encode(format!("{}:{}", login, password)); + format!("Basic {}", encoded) + } + Authentication::Bearer(token) => { + let token = String::from_utf8_lossy(token); + format!("Bearer {}", token) + } + }; + + MetadataValue::try_from(header.as_str()) + .expect("Auth header value should be valid metadata header value") +} + +#[cfg(test)] +mod auth_tests { + use super::*; + use crate::AppendToStreamOptions; + use crate::options::Options; + + #[test] + fn basic_authentication_produces_base64_basic_header() { + let auth = Authentication::basic("admin", "changeit"); + let header = build_authorization_header(&auth); + // base64("admin:changeit") = YWRtaW46Y2hhbmdlaXQ= + assert_eq!(header.to_str().unwrap(), "Basic YWRtaW46Y2hhbmdlaXQ="); + } + + #[test] + fn bearer_authentication_produces_bearer_header_verbatim() { + let auth = Authentication::bearer("abc.def.ghi"); + let header = build_authorization_header(&auth); + assert_eq!(header.to_str().unwrap(), "Bearer abc.def.ghi"); + } + + #[test] + fn basic_authentication_with_special_chars_encodes_correctly() { + let auth = Authentication::basic("user@example.com", "p@ss:word"); + let header = build_authorization_header(&auth); + // base64("user@example.com:p@ss:word") = dXNlckBleGFtcGxlLmNvbTpwQHNzOndvcmQ= + assert_eq!( + header.to_str().unwrap(), + "Basic dXNlckBleGFtcGxlLmNvbTpwQHNzOndvcmQ=" + ); + } + + #[test] + fn credentials_convert_into_basic_authentication() { + let auth: Authentication = Credentials::new("admin", "changeit").into(); + let header = build_authorization_header(&auth); + assert_eq!(header.to_str().unwrap(), "Basic YWRtaW46Y2hhbmdlaXQ="); + } + + fn settings_from(connection_string: &str) -> ClientSettings { + connection_string + .parse::() + .expect("valid connection string") + } + + #[test] + fn no_auth_anywhere_produces_no_authorization_header() { + let settings = settings_from("esdb://localhost:2113?tls=false"); + let options = AppendToStreamOptions::default(); + let metadata = build_request_metadata(&settings, options.common_operation_options()); + + assert!(metadata.get("authorization").is_none()); + } + + #[test] + fn default_user_from_connection_string_falls_through_as_basic() { + let settings = settings_from("esdb://admin:changeit@localhost:2113?tls=false"); + let options = AppendToStreamOptions::default(); + let metadata = build_request_metadata(&settings, options.common_operation_options()); + + assert_eq!( + metadata.get("authorization").unwrap().to_str().unwrap(), + "Basic YWRtaW46Y2hhbmdlaXQ=" + ); + } + + #[test] + fn per_call_bearer_overrides_default_user() { + let settings = settings_from("esdb://admin:changeit@localhost:2113?tls=false"); + let options = AppendToStreamOptions::default() + .authenticated(Authentication::bearer("call-token")); + let metadata = build_request_metadata(&settings, options.common_operation_options()); + + assert_eq!( + metadata.get("authorization").unwrap().to_str().unwrap(), + "Bearer call-token" + ); + } + + #[test] + fn authenticated_builder_accepts_credentials_directly() { + let settings = settings_from("esdb://localhost:2113?tls=false"); + let options = + AppendToStreamOptions::default().authenticated(Credentials::new("alice", "secret")); + let metadata = build_request_metadata(&settings, options.common_operation_options()); + + // base64("alice:secret") = YWxpY2U6c2VjcmV0 + assert_eq!( + metadata.get("authorization").unwrap().to_str().unwrap(), + "Basic YWxpY2U6c2VjcmV0" + ); + } + + #[test] + fn authenticated_builder_accepts_authentication_bearer() { + let settings = settings_from("esdb://localhost:2113?tls=false"); + let options = + AppendToStreamOptions::default().authenticated(Authentication::bearer("eyJ.payload")); + let metadata = build_request_metadata(&settings, options.common_operation_options()); + + assert_eq!( + metadata.get("authorization").unwrap().to_str().unwrap(), + "Bearer eyJ.payload" + ); + } +} diff --git a/kurrentdb/src/types.rs b/kurrentdb/src/types.rs index 097359cb..61249c3d 100755 --- a/kurrentdb/src/types.rs +++ b/kurrentdb/src/types.rs @@ -53,6 +53,46 @@ impl Credentials { } } +/// Authentication mode used when sending a request to KurrentDB. +/// +/// Supports HTTP Basic auth (login + password) and Bearer token auth (e.g. an +/// OAuth/OIDC access token). `Authentication` implements `From`, +/// so any API that accepts `impl Into` continues to accept a +/// plain `Credentials` value unchanged. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum Authentication { + /// HTTP Basic authentication using login and password. + Basic(Credentials), + + /// Bearer token authentication. The token is sent verbatim in the + /// `Authorization: Bearer ` header. + Bearer(Bytes), +} + +impl Authentication { + /// Creates an `Authentication` value using HTTP Basic authentication. + pub fn basic(login: S, password: S) -> Self + where + S: Into, + { + Authentication::Basic(Credentials::new(login, password)) + } + + /// Creates an `Authentication` value using Bearer token authentication. + pub fn bearer(token: S) -> Self + where + S: Into, + { + Authentication::Bearer(token.into()) + } +} + +impl From for Authentication { + fn from(credentials: Credentials) -> Self { + Authentication::Basic(credentials) + } +} + struct CredsVisitor; impl<'de> Visitor<'de> for CredsVisitor { From b95a9e9e5f8b087ceacb6358b9295adf37a569d3 Mon Sep 17 00:00:00 2001 From: William Chong Date: Mon, 4 May 2026 15:34:38 +0400 Subject: [PATCH 2/8] fix: drop unsafe utf8 conversion in http_configure_auth http_configure_auth was reading login/password/token bytes through from_utf8_unchecked, but Credentials::new and Authentication::bearer both accept arbitrary Into, so a perfectly safe caller could hand in non-UTF-8 and trigger UB. Swapped to from_utf8_lossy, which is what request.rs already does for the gRPC path. reqwest's basic_auth and bearer_auth take impl Display, and Cow implements Display, so it slots in directly. --- kurrentdb/src/http/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kurrentdb/src/http/mod.rs b/kurrentdb/src/http/mod.rs index 2150cfba..e5130a00 100644 --- a/kurrentdb/src/http/mod.rs +++ b/kurrentdb/src/http/mod.rs @@ -21,11 +21,11 @@ pub fn http_configure_auth( ) -> reqwest::RequestBuilder { match auth_opt { Some(crate::Authentication::Basic(creds)) => builder.basic_auth( - unsafe { std::str::from_utf8_unchecked(creds.login.as_ref()) }, - unsafe { Some(std::str::from_utf8_unchecked(creds.password.as_ref())) }, + String::from_utf8_lossy(creds.login.as_ref()), + Some(String::from_utf8_lossy(creds.password.as_ref())), ), Some(crate::Authentication::Bearer(token)) => { - builder.bearer_auth(unsafe { std::str::from_utf8_unchecked(token.as_ref()) }) + builder.bearer_auth(String::from_utf8_lossy(token.as_ref())) } None => builder, } From c041ab62995ee40e97bcc32a234501a8c3c5e1c6 Mon Sep 17 00:00:00 2001 From: William Chong Date: Mon, 4 May 2026 15:34:38 +0400 Subject: [PATCH 3/8] fix: don't panic when a bearer token has invalid header chars build_authorization_header used .expect() on MetadataValue::try_from. For Bearer that's risky: the token goes onto the wire verbatim, and HTTP/2 rejects control chars (NUL, LF, CR, others below 0x20, DEL), so a token someone read from a file or env var without trimming the trailing newline would crash the client. Now the function returns Option>. On Err it emits a tracing::warn! tagged with the auth kind (basic/bearer, never the token itself) and returns None. build_request_metadata skips the authorization header when None, the server replies AccessDenied, and the warn log gives operators a clear hint about what went wrong. Worth flagging: tonic's MetadataValue is more permissive than the name implies. It accepts bytes 0x80 through 0xFF, full UTF-8, and U+FFFD. What actually breaks is the control-char set above; the new tests cover that. --- kurrentdb/src/request.rs | 63 +++++++++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 11 deletions(-) diff --git a/kurrentdb/src/request.rs b/kurrentdb/src/request.rs index b6950b97..4cb97f76 100644 --- a/kurrentdb/src/request.rs +++ b/kurrentdb/src/request.rs @@ -24,7 +24,9 @@ where }); if let Some(auth) = authentication.as_deref() { - metadata.insert("authorization", build_authorization_header(auth)); + if let Some(header_value) = build_authorization_header(auth) { + metadata.insert("authorization", header_value); + } } if options.requires_leader || settings.node_preference() == NodePreference::Leader { @@ -43,25 +45,38 @@ where fn build_authorization_header( auth: &Authentication, -) -> tonic::metadata::MetadataValue { +) -> Option> { use tonic::metadata::MetadataValue; - let header = match auth { + let (header, kind) = match auth { Authentication::Basic(Credentials { login, password }) => { let login = String::from_utf8_lossy(login); let password = String::from_utf8_lossy(password); let encoded = base64::engine::general_purpose::STANDARD .encode(format!("{}:{}", login, password)); - format!("Basic {}", encoded) + (format!("Basic {}", encoded), "basic") } Authentication::Bearer(token) => { let token = String::from_utf8_lossy(token); - format!("Bearer {}", token) + (format!("Bearer {}", token), "bearer") } }; - MetadataValue::try_from(header.as_str()) - .expect("Auth header value should be valid metadata header value") + match MetadataValue::try_from(header.as_str()) { + Ok(value) => Some(value), + Err(_) => { + // HTTP/2 header values reject control characters (NUL, LF, CR, others < 0x20 + // except tab, plus DEL). Bearer tokens are sent verbatim, so an untrimmed + // trailing newline or similar in the token would panic if we used `.expect()`. + // We skip the header instead; the server will respond with AccessDenied, and + // this log explains why. The token itself is never logged. + tracing::warn!( + auth_kind = kind, + "authentication value contains characters that are not valid in a gRPC metadata header; the Authorization header will be omitted" + ); + None + } + } } #[cfg(test)] @@ -73,7 +88,7 @@ mod auth_tests { #[test] fn basic_authentication_produces_base64_basic_header() { let auth = Authentication::basic("admin", "changeit"); - let header = build_authorization_header(&auth); + let header = build_authorization_header(&auth).expect("ASCII header"); // base64("admin:changeit") = YWRtaW46Y2hhbmdlaXQ= assert_eq!(header.to_str().unwrap(), "Basic YWRtaW46Y2hhbmdlaXQ="); } @@ -81,14 +96,14 @@ mod auth_tests { #[test] fn bearer_authentication_produces_bearer_header_verbatim() { let auth = Authentication::bearer("abc.def.ghi"); - let header = build_authorization_header(&auth); + let header = build_authorization_header(&auth).expect("ASCII header"); assert_eq!(header.to_str().unwrap(), "Bearer abc.def.ghi"); } #[test] fn basic_authentication_with_special_chars_encodes_correctly() { let auth = Authentication::basic("user@example.com", "p@ss:word"); - let header = build_authorization_header(&auth); + let header = build_authorization_header(&auth).expect("ASCII header"); // base64("user@example.com:p@ss:word") = dXNlckBleGFtcGxlLmNvbTpwQHNzOndvcmQ= assert_eq!( header.to_str().unwrap(), @@ -99,10 +114,36 @@ mod auth_tests { #[test] fn credentials_convert_into_basic_authentication() { let auth: Authentication = Credentials::new("admin", "changeit").into(); - let header = build_authorization_header(&auth); + let header = build_authorization_header(&auth).expect("ASCII header"); assert_eq!(header.to_str().unwrap(), "Basic YWRtaW46Y2hhbmdlaXQ="); } + #[test] + fn bearer_with_embedded_newline_returns_none_instead_of_panicking() { + // HTTP/2 header values reject embedded LF. A token with a trailing newline is + // a realistic failure mode (e.g. read from a file without trimming). The function + // must not panic; it must return None so the caller skips the header. + let auth = Authentication::bearer("token\nleak"); + assert!(build_authorization_header(&auth).is_none()); + } + + #[test] + fn bearer_with_embedded_null_returns_none_instead_of_panicking() { + let auth = Authentication::bearer("token\0bad"); + assert!(build_authorization_header(&auth).is_none()); + } + + #[test] + fn build_request_metadata_skips_bearer_token_with_invalid_chars() { + let settings = settings_from("esdb://localhost:2113?tls=false"); + let options = AppendToStreamOptions::default() + .authenticated(Authentication::bearer("token\nleak")); + // Must not panic. The authorization header should simply be absent so the server + // responds with AccessDenied rather than the client crashing. + let metadata = build_request_metadata(&settings, options.common_operation_options()); + assert!(metadata.get("authorization").is_none()); + } + fn settings_from(connection_string: &str) -> ClientSettings { connection_string .parse::() From 8c4f0b20a9108be0f50ce225854d57cce8c25a66 Mon Sep 17 00:00:00 2001 From: William Chong Date: Mon, 4 May 2026 15:34:39 +0400 Subject: [PATCH 4/8] refactor: review feedback on the auth code A pass of small cleanups after running the diff through code review. Authentication grew a kind() helper so the tracing labels aren't typed out as raw "basic"/"bearer" strings each time. The nested if-let in build_request_metadata flattens to a single .as_deref().and_then(...) chain. The three near-duplicate "bearer with invalid char" tests collapse into one parameterized loop. settings_from moves to the top of the test module so it isn't buried mid-file. Comment-wise: trimmed narrative explanations down to the WHY, and added a brief note on why http_configure_auth uses from_utf8_lossy. --- kurrentdb/src/http/mod.rs | 2 ++ kurrentdb/src/request.rs | 58 +++++++++++++++++---------------------- kurrentdb/src/types.rs | 7 +++++ 3 files changed, 34 insertions(+), 33 deletions(-) diff --git a/kurrentdb/src/http/mod.rs b/kurrentdb/src/http/mod.rs index e5130a00..5fe6b6ce 100644 --- a/kurrentdb/src/http/mod.rs +++ b/kurrentdb/src/http/mod.rs @@ -19,6 +19,8 @@ pub fn http_configure_auth( builder: reqwest::RequestBuilder, auth_opt: Option<&crate::Authentication>, ) -> reqwest::RequestBuilder { + // Lossy decode: non-UTF8 bytes become U+FFFD, which the server will reject as + // a bad header rather than this client invoking UB via from_utf8_unchecked. match auth_opt { Some(crate::Authentication::Basic(creds)) => builder.basic_auth( String::from_utf8_lossy(creds.login.as_ref()), diff --git a/kurrentdb/src/request.rs b/kurrentdb/src/request.rs index 4cb97f76..10e1d640 100644 --- a/kurrentdb/src/request.rs +++ b/kurrentdb/src/request.rs @@ -23,10 +23,8 @@ where .map(|c| Cow::Owned(Authentication::Basic(c.clone()))) }); - if let Some(auth) = authentication.as_deref() { - if let Some(header_value) = build_authorization_header(auth) { - metadata.insert("authorization", header_value); - } + if let Some(header_value) = authentication.as_deref().and_then(build_authorization_header) { + metadata.insert("authorization", header_value); } if options.requires_leader || settings.node_preference() == NodePreference::Leader { @@ -48,30 +46,27 @@ fn build_authorization_header( ) -> Option> { use tonic::metadata::MetadataValue; - let (header, kind) = match auth { + let header = match auth { Authentication::Basic(Credentials { login, password }) => { let login = String::from_utf8_lossy(login); let password = String::from_utf8_lossy(password); let encoded = base64::engine::general_purpose::STANDARD .encode(format!("{}:{}", login, password)); - (format!("Basic {}", encoded), "basic") + format!("Basic {}", encoded) } Authentication::Bearer(token) => { let token = String::from_utf8_lossy(token); - (format!("Bearer {}", token), "bearer") + format!("Bearer {}", token) } }; match MetadataValue::try_from(header.as_str()) { Ok(value) => Some(value), Err(_) => { - // HTTP/2 header values reject control characters (NUL, LF, CR, others < 0x20 - // except tab, plus DEL). Bearer tokens are sent verbatim, so an untrimmed - // trailing newline or similar in the token would panic if we used `.expect()`. - // We skip the header instead; the server will respond with AccessDenied, and - // this log explains why. The token itself is never logged. + // HTTP/2 header values reject control characters; an untrimmed newline + // in a bearer token would otherwise panic. Token is never logged. tracing::warn!( - auth_kind = kind, + auth_kind = auth.kind(), "authentication value contains characters that are not valid in a gRPC metadata header; the Authorization header will be omitted" ); None @@ -85,6 +80,12 @@ mod auth_tests { use crate::AppendToStreamOptions; use crate::options::Options; + fn settings_from(connection_string: &str) -> ClientSettings { + connection_string + .parse::() + .expect("valid connection string") + } + #[test] fn basic_authentication_produces_base64_basic_header() { let auth = Authentication::basic("admin", "changeit"); @@ -119,18 +120,17 @@ mod auth_tests { } #[test] - fn bearer_with_embedded_newline_returns_none_instead_of_panicking() { - // HTTP/2 header values reject embedded LF. A token with a trailing newline is - // a realistic failure mode (e.g. read from a file without trimming). The function - // must not panic; it must return None so the caller skips the header. - let auth = Authentication::bearer("token\nleak"); - assert!(build_authorization_header(&auth).is_none()); - } - - #[test] - fn bearer_with_embedded_null_returns_none_instead_of_panicking() { - let auth = Authentication::bearer("token\0bad"); - assert!(build_authorization_header(&auth).is_none()); + fn bearer_with_invalid_header_chars_returns_none_instead_of_panicking() { + // HTTP/2 header values reject NUL, LF, CR, and other control bytes. A token + // read from a file or env var with a trailing newline is the realistic case. + for token in ["token\nleak", "token\0bad", "token\rbreak"] { + let auth = Authentication::bearer(token); + assert!( + build_authorization_header(&auth).is_none(), + "expected None for {:?}", + token + ); + } } #[test] @@ -138,18 +138,10 @@ mod auth_tests { let settings = settings_from("esdb://localhost:2113?tls=false"); let options = AppendToStreamOptions::default() .authenticated(Authentication::bearer("token\nleak")); - // Must not panic. The authorization header should simply be absent so the server - // responds with AccessDenied rather than the client crashing. let metadata = build_request_metadata(&settings, options.common_operation_options()); assert!(metadata.get("authorization").is_none()); } - fn settings_from(connection_string: &str) -> ClientSettings { - connection_string - .parse::() - .expect("valid connection string") - } - #[test] fn no_auth_anywhere_produces_no_authorization_header() { let settings = settings_from("esdb://localhost:2113?tls=false"); diff --git a/kurrentdb/src/types.rs b/kurrentdb/src/types.rs index 61249c3d..f9ce72c5 100755 --- a/kurrentdb/src/types.rs +++ b/kurrentdb/src/types.rs @@ -85,6 +85,13 @@ impl Authentication { { Authentication::Bearer(token.into()) } + + pub(crate) fn kind(&self) -> &'static str { + match self { + Authentication::Basic(_) => "basic", + Authentication::Bearer(_) => "bearer", + } + } } impl From for Authentication { From 104671d6ed0c2fd5de0832801e411412ca0de3cc Mon Sep 17 00:00:00 2001 From: William Chong Date: Mon, 4 May 2026 15:34:39 +0400 Subject: [PATCH 5/8] fix: appease Rust 1.95 clippy CI bumped to 1.95 and three new lints fired on pre-existing code, none of it from this PR. Silenced dead_code on the generated common.rs module (unused proto types are noise from prost-build), collapsed the nested if-let in the connection-id selector with a &&-let, and rewrote two `% 2 == 0` checks in the random node tiebreak as .is_multiple_of(2). --- kurrentdb/src/event_store/generated.rs | 1 + kurrentdb/src/grpc.rs | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/kurrentdb/src/event_store/generated.rs b/kurrentdb/src/event_store/generated.rs index 6e4dfdb8..b3142d6d 100644 --- a/kurrentdb/src/event_store/generated.rs +++ b/kurrentdb/src/event_store/generated.rs @@ -8,6 +8,7 @@ use chrono::{DateTime, Utc}; use std::ops::Add; use std::time::{Duration, SystemTime}; +#[allow(dead_code)] pub mod common; pub mod google_rpc; pub mod gossip; diff --git a/kurrentdb/src/grpc.rs b/kurrentdb/src/grpc.rs index c6ddb02b..1016b503 100644 --- a/kurrentdb/src/grpc.rs +++ b/kurrentdb/src/grpc.rs @@ -949,10 +949,10 @@ impl NodeConnection { loop { if let Some(request) = request.take() { - if self.id != request.correlation { - if let Some(handle) = self.handle.clone() { - return Ok(handle); - } + if self.id != request.correlation + && let Some(handle) = self.handle.clone() + { + return Ok(handle); } failed_endpoint = self.handle.take().map(|h| h.endpoint); @@ -1439,7 +1439,7 @@ fn determine_best_node( let member_opt = members.min_by(|a, b| { if let NodePreference::Random = preference { - if rng.next_u32() % 2 == 0 { + if rng.next_u32().is_multiple_of(2) { return Ordering::Greater; } @@ -1447,7 +1447,7 @@ fn determine_best_node( } if preference.match_preference(&a.state) && preference.match_preference(&b.state) { - if rng.next_u32() % 2 == 0 { + if rng.next_u32().is_multiple_of(2) { return Ordering::Less; } else { return Ordering::Greater; From 09b93622bd2107eba4cc80d9a41acc168cbaa9e2 Mon Sep 17 00:00:00 2001 From: William Chong Date: Mon, 4 May 2026 15:34:40 +0400 Subject: [PATCH 6/8] style: cargo fmt CI's `cargo fmt --check` caught a few line-wraps in request.rs that I missed locally. --- kurrentdb/src/request.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/kurrentdb/src/request.rs b/kurrentdb/src/request.rs index 10e1d640..3cb0be60 100644 --- a/kurrentdb/src/request.rs +++ b/kurrentdb/src/request.rs @@ -23,7 +23,10 @@ where .map(|c| Cow::Owned(Authentication::Basic(c.clone()))) }); - if let Some(header_value) = authentication.as_deref().and_then(build_authorization_header) { + if let Some(header_value) = authentication + .as_deref() + .and_then(build_authorization_header) + { metadata.insert("authorization", header_value); } @@ -50,8 +53,8 @@ fn build_authorization_header( Authentication::Basic(Credentials { login, password }) => { let login = String::from_utf8_lossy(login); let password = String::from_utf8_lossy(password); - let encoded = base64::engine::general_purpose::STANDARD - .encode(format!("{}:{}", login, password)); + let encoded = + base64::engine::general_purpose::STANDARD.encode(format!("{}:{}", login, password)); format!("Basic {}", encoded) } Authentication::Bearer(token) => { @@ -136,8 +139,8 @@ mod auth_tests { #[test] fn build_request_metadata_skips_bearer_token_with_invalid_chars() { let settings = settings_from("esdb://localhost:2113?tls=false"); - let options = AppendToStreamOptions::default() - .authenticated(Authentication::bearer("token\nleak")); + let options = + AppendToStreamOptions::default().authenticated(Authentication::bearer("token\nleak")); let metadata = build_request_metadata(&settings, options.common_operation_options()); assert!(metadata.get("authorization").is_none()); } @@ -166,8 +169,8 @@ mod auth_tests { #[test] fn per_call_bearer_overrides_default_user() { let settings = settings_from("esdb://admin:changeit@localhost:2113?tls=false"); - let options = AppendToStreamOptions::default() - .authenticated(Authentication::bearer("call-token")); + let options = + AppendToStreamOptions::default().authenticated(Authentication::bearer("call-token")); let metadata = build_request_metadata(&settings, options.common_operation_options()); assert_eq!( From a4fa77c137b1160a9ac6e3153126d0b9310ab3d4 Mon Sep 17 00:00:00 2001 From: William Chong Date: Mon, 4 May 2026 15:34:40 +0400 Subject: [PATCH 7/8] chore: drop comments that don't earn their keep The auth code had accumulated a lot of comments that just restated the type or function name. Stripped the Basic variant doc, the basic/bearer constructor docs, and the resolve_authentication docstring. Removed the narrative explanation of the from_utf8_lossy choice; the change reads clearly without it. Trimmed the panic-prevention comment to one line and tightened the .authenticated() rustdoc and the parameterized test's rationale comment. Net is -11 lines of comments. Kept only the WHYs that aren't obvious from the code: the From migration path on the enum, "sent verbatim" on the Bearer variant, base64 hints in the tests, the security intent of not logging tokens, and the panic-vector context. --- eventstore-macros/src/lib.rs | 3 +-- kurrentdb/src/http/mod.rs | 4 ---- kurrentdb/src/request.rs | 6 ++---- kurrentdb/src/types.rs | 6 +----- 4 files changed, 4 insertions(+), 15 deletions(-) diff --git a/eventstore-macros/src/lib.rs b/eventstore-macros/src/lib.rs index 308022ba..f3afd49c 100644 --- a/eventstore-macros/src/lib.rs +++ b/eventstore-macros/src/lib.rs @@ -94,8 +94,7 @@ pub fn options(input: TokenStream) -> TokenStream { impl #name { /// Performs the command with the given authentication. /// - /// Accepts either a `Credentials` (Basic auth) or an `Authentication` - /// value, the latter allowing Bearer token authentication. + /// Accepts `Credentials` or `Authentication` (the latter for Bearer tokens). pub fn authenticated(mut self, authentication: A) -> Self where A: Into, diff --git a/kurrentdb/src/http/mod.rs b/kurrentdb/src/http/mod.rs index 5fe6b6ce..16e53222 100644 --- a/kurrentdb/src/http/mod.rs +++ b/kurrentdb/src/http/mod.rs @@ -1,8 +1,6 @@ use tracing::error; pub mod persistent_subscriptions; -/// Resolves the authentication to use for an HTTP call: per-operation override -/// first, falling back to the connection-string default user. pub(crate) fn resolve_authentication( options: &crate::options::CommonOperationOptions, settings: &crate::ClientSettings, @@ -19,8 +17,6 @@ pub fn http_configure_auth( builder: reqwest::RequestBuilder, auth_opt: Option<&crate::Authentication>, ) -> reqwest::RequestBuilder { - // Lossy decode: non-UTF8 bytes become U+FFFD, which the server will reject as - // a bad header rather than this client invoking UB via from_utf8_unchecked. match auth_opt { Some(crate::Authentication::Basic(creds)) => builder.basic_auth( String::from_utf8_lossy(creds.login.as_ref()), diff --git a/kurrentdb/src/request.rs b/kurrentdb/src/request.rs index 3cb0be60..79324bd4 100644 --- a/kurrentdb/src/request.rs +++ b/kurrentdb/src/request.rs @@ -66,8 +66,7 @@ fn build_authorization_header( match MetadataValue::try_from(header.as_str()) { Ok(value) => Some(value), Err(_) => { - // HTTP/2 header values reject control characters; an untrimmed newline - // in a bearer token would otherwise panic. Token is never logged. + // An untrimmed newline in a bearer token would panic. Token is never logged. tracing::warn!( auth_kind = auth.kind(), "authentication value contains characters that are not valid in a gRPC metadata header; the Authorization header will be omitted" @@ -124,8 +123,7 @@ mod auth_tests { #[test] fn bearer_with_invalid_header_chars_returns_none_instead_of_panicking() { - // HTTP/2 header values reject NUL, LF, CR, and other control bytes. A token - // read from a file or env var with a trailing newline is the realistic case. + // Trailing newlines from untrimmed file/env reads are the realistic failure mode. for token in ["token\nleak", "token\0bad", "token\rbreak"] { let auth = Authentication::bearer(token); assert!( diff --git a/kurrentdb/src/types.rs b/kurrentdb/src/types.rs index f9ce72c5..9bd54b73 100755 --- a/kurrentdb/src/types.rs +++ b/kurrentdb/src/types.rs @@ -61,16 +61,13 @@ impl Credentials { /// plain `Credentials` value unchanged. #[derive(Clone, Debug, PartialEq, Eq)] pub enum Authentication { - /// HTTP Basic authentication using login and password. Basic(Credentials), - /// Bearer token authentication. The token is sent verbatim in the - /// `Authorization: Bearer ` header. + /// Sent verbatim in the `Authorization: Bearer ` header. Bearer(Bytes), } impl Authentication { - /// Creates an `Authentication` value using HTTP Basic authentication. pub fn basic(login: S, password: S) -> Self where S: Into, @@ -78,7 +75,6 @@ impl Authentication { Authentication::Basic(Credentials::new(login, password)) } - /// Creates an `Authentication` value using Bearer token authentication. pub fn bearer(token: S) -> Self where S: Into, From 8e74654df3a9733af44dba289d89675fb0eefeff Mon Sep 17 00:00:00 2001 From: William Chong Date: Mon, 4 May 2026 16:17:04 +0400 Subject: [PATCH 8/8] test: cover HTTP auth path and trim redundant gRPC auth tests Add unit tests for http_configure_auth and resolve_authentication, the HTTP auth surface used by gossip and persistent subscriptions, which had no coverage. Drop three gRPC tests whose behavior is already exercised elsewhere (From via the .authenticated() builder, the bearer builder path via the override test, and the private builder's invalid-char handling via the public metadata test). --- kurrentdb/src/http/mod.rs | 78 +++++++++++++++++++++++++++++++++++++++ kurrentdb/src/request.rs | 37 ------------------- 2 files changed, 78 insertions(+), 37 deletions(-) diff --git a/kurrentdb/src/http/mod.rs b/kurrentdb/src/http/mod.rs index 16e53222..f06bfc54 100644 --- a/kurrentdb/src/http/mod.rs +++ b/kurrentdb/src/http/mod.rs @@ -80,3 +80,81 @@ pub async fn http_execute_request( } } } + +#[cfg(test)] +mod auth_tests { + use super::*; + use crate::options::CommonOperationOptions; + use crate::{Authentication, ClientSettings, Credentials}; + + fn settings_from(connection_string: &str) -> ClientSettings { + connection_string + .parse::() + .expect("valid connection string") + } + + fn authorization_header(builder: reqwest::RequestBuilder) -> Option { + let request = builder.build().expect("buildable request"); + request + .headers() + .get(reqwest::header::AUTHORIZATION) + .map(|v| v.to_str().expect("ASCII header").to_owned()) + } + + fn fresh_builder() -> reqwest::RequestBuilder { + reqwest::Client::new().get("http://localhost/") + } + + #[test] + fn http_configure_auth_with_basic_sets_basic_authorization_header() { + let auth = Authentication::basic("admin", "changeit"); + let header = authorization_header(http_configure_auth(fresh_builder(), Some(&auth))) + .expect("authorization header present"); + assert_eq!(header, "Basic YWRtaW46Y2hhbmdlaXQ="); + } + + #[test] + fn http_configure_auth_with_bearer_sets_bearer_authorization_header() { + let auth = Authentication::bearer("abc.def.ghi"); + let header = authorization_header(http_configure_auth(fresh_builder(), Some(&auth))) + .expect("authorization header present"); + assert_eq!(header, "Bearer abc.def.ghi"); + } + + #[test] + fn http_configure_auth_with_none_leaves_authorization_unset() { + assert!(authorization_header(http_configure_auth(fresh_builder(), None)).is_none()); + } + + #[test] + fn resolve_authentication_prefers_per_call_over_default_user() { + let settings = settings_from("esdb://admin:changeit@localhost:2113?tls=false"); + let common = CommonOperationOptions { + authentication: Some(Authentication::bearer("call-token")), + ..Default::default() + }; + + let resolved = resolve_authentication(&common, &settings).expect("present"); + assert_eq!(resolved, Authentication::bearer("call-token")); + } + + #[test] + fn resolve_authentication_falls_back_to_default_user_as_basic() { + let settings = settings_from("esdb://admin:changeit@localhost:2113?tls=false"); + let common = CommonOperationOptions::default(); + + let resolved = resolve_authentication(&common, &settings).expect("present"); + assert_eq!( + resolved, + Authentication::Basic(Credentials::new("admin", "changeit")) + ); + } + + #[test] + fn resolve_authentication_returns_none_when_neither_configured() { + let settings = settings_from("esdb://localhost:2113?tls=false"); + let common = CommonOperationOptions::default(); + + assert!(resolve_authentication(&common, &settings).is_none()); + } +} diff --git a/kurrentdb/src/request.rs b/kurrentdb/src/request.rs index 79324bd4..4bc51af0 100644 --- a/kurrentdb/src/request.rs +++ b/kurrentdb/src/request.rs @@ -66,7 +66,6 @@ fn build_authorization_header( match MetadataValue::try_from(header.as_str()) { Ok(value) => Some(value), Err(_) => { - // An untrimmed newline in a bearer token would panic. Token is never logged. tracing::warn!( auth_kind = auth.kind(), "authentication value contains characters that are not valid in a gRPC metadata header; the Authorization header will be omitted" @@ -92,7 +91,6 @@ mod auth_tests { fn basic_authentication_produces_base64_basic_header() { let auth = Authentication::basic("admin", "changeit"); let header = build_authorization_header(&auth).expect("ASCII header"); - // base64("admin:changeit") = YWRtaW46Y2hhbmdlaXQ= assert_eq!(header.to_str().unwrap(), "Basic YWRtaW46Y2hhbmdlaXQ="); } @@ -107,33 +105,12 @@ mod auth_tests { fn basic_authentication_with_special_chars_encodes_correctly() { let auth = Authentication::basic("user@example.com", "p@ss:word"); let header = build_authorization_header(&auth).expect("ASCII header"); - // base64("user@example.com:p@ss:word") = dXNlckBleGFtcGxlLmNvbTpwQHNzOndvcmQ= assert_eq!( header.to_str().unwrap(), "Basic dXNlckBleGFtcGxlLmNvbTpwQHNzOndvcmQ=" ); } - #[test] - fn credentials_convert_into_basic_authentication() { - let auth: Authentication = Credentials::new("admin", "changeit").into(); - let header = build_authorization_header(&auth).expect("ASCII header"); - assert_eq!(header.to_str().unwrap(), "Basic YWRtaW46Y2hhbmdlaXQ="); - } - - #[test] - fn bearer_with_invalid_header_chars_returns_none_instead_of_panicking() { - // Trailing newlines from untrimmed file/env reads are the realistic failure mode. - for token in ["token\nleak", "token\0bad", "token\rbreak"] { - let auth = Authentication::bearer(token); - assert!( - build_authorization_header(&auth).is_none(), - "expected None for {:?}", - token - ); - } - } - #[test] fn build_request_metadata_skips_bearer_token_with_invalid_chars() { let settings = settings_from("esdb://localhost:2113?tls=false"); @@ -184,23 +161,9 @@ mod auth_tests { AppendToStreamOptions::default().authenticated(Credentials::new("alice", "secret")); let metadata = build_request_metadata(&settings, options.common_operation_options()); - // base64("alice:secret") = YWxpY2U6c2VjcmV0 assert_eq!( metadata.get("authorization").unwrap().to_str().unwrap(), "Basic YWxpY2U6c2VjcmV0" ); } - - #[test] - fn authenticated_builder_accepts_authentication_bearer() { - let settings = settings_from("esdb://localhost:2113?tls=false"); - let options = - AppendToStreamOptions::default().authenticated(Authentication::bearer("eyJ.payload")); - let metadata = build_request_metadata(&settings, options.common_operation_options()); - - assert_eq!( - metadata.get("authorization").unwrap().to_str().unwrap(), - "Bearer eyJ.payload" - ); - } }