From acb69e590beb42b07bb2acf084a62f4cff0213bc Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Sun, 17 May 2026 15:38:15 +0200 Subject: [PATCH 01/59] Create modelardb_auth crate --- Cargo.lock | 9 +++++++++ crates/modelardb_auth/Cargo.toml | 25 +++++++++++++++++++++++++ crates/modelardb_auth/src/lib.rs | 29 +++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+) create mode 100644 crates/modelardb_auth/Cargo.toml create mode 100644 crates/modelardb_auth/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index a765149e..66149ad5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3305,6 +3305,15 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "modelardb_auth" +version = "0.1.0" +dependencies = [ + "modelardb_storage", + "tokio", + "tonic", +] + [[package]] name = "modelardb_bulkloader" version = "0.1.0" diff --git a/crates/modelardb_auth/Cargo.toml b/crates/modelardb_auth/Cargo.toml new file mode 100644 index 00000000..723c0f9f --- /dev/null +++ b/crates/modelardb_auth/Cargo.toml @@ -0,0 +1,25 @@ +# Copyright 2026 The ModelarDB Contributors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +[package] +name = "modelardb_auth" +version = "0.1.0" +license = "Apache-2.0" +edition = "2024" +authors = ["Soeren Kejser Jensen "] + +[dependencies] +modelardb_storage = { path = "../modelardb_storage" } +tonic.workspace = true +tokio.workspace = true \ No newline at end of file diff --git a/crates/modelardb_auth/src/lib.rs b/crates/modelardb_auth/src/lib.rs new file mode 100644 index 00000000..82885e93 --- /dev/null +++ b/crates/modelardb_auth/src/lib.rs @@ -0,0 +1,29 @@ +/* Copyright 2026 The ModelarDB Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +pub fn add(left: u64, right: u64) -> u64 { + left + right +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn it_works() { + let result = add(2, 2); + assert_eq!(result, 4); + } +} From ff4c06b16fbb362d5beeddbdf4b244aedca8d8dd Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Sun, 17 May 2026 15:41:59 +0200 Subject: [PATCH 02/59] Add Permission enum --- crates/modelardb_auth/src/lib.rs | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/crates/modelardb_auth/src/lib.rs b/crates/modelardb_auth/src/lib.rs index 82885e93..cbd5f95b 100644 --- a/crates/modelardb_auth/src/lib.rs +++ b/crates/modelardb_auth/src/lib.rs @@ -13,17 +13,25 @@ * limitations under the License. */ -pub fn add(left: u64, right: u64) -> u64 { - left + right -} +//! Types to support authentication and authorization in ModelarDB. -#[cfg(test)] -mod tests { - use super::*; +/// The permission required to perform an operation in a ModelarDB server. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Permission { + Read, + Write, + Admin, +} - #[test] - fn it_works() { - let result = add(2, 2); - assert_eq!(result, 4); +impl Permission { + /// Return [`true`] if `granted` satisfies this required permission using the + /// hierarchy Admin ⊇ Write ⊇ Read, otherwise [`false`] is returned. + pub fn is_satisfied_by(&self, granted: &Permission) -> bool { + match (self, granted) { + (Permission::Read, _) => true, + (Permission::Write, Permission::Write | Permission::Admin) => true, + (Permission::Admin, Permission::Admin) => true, + _ => false, + } } } From 7481a7121aca8de279b5aec09875d0db246ff67d Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Sun, 17 May 2026 15:45:33 +0200 Subject: [PATCH 03/59] Define the Authenticator trait --- crates/modelardb_auth/src/lib.rs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/crates/modelardb_auth/src/lib.rs b/crates/modelardb_auth/src/lib.rs index cbd5f95b..b4ff7ce4 100644 --- a/crates/modelardb_auth/src/lib.rs +++ b/crates/modelardb_auth/src/lib.rs @@ -15,6 +15,9 @@ //! Types to support authentication and authorization in ModelarDB. +use tonic::metadata::MetadataMap; +use tonic::Status; + /// The permission required to perform an operation in a ModelarDB server. #[derive(Debug, Clone, PartialEq, Eq)] pub enum Permission { @@ -24,8 +27,8 @@ pub enum Permission { } impl Permission { - /// Return [`true`] if `granted` satisfies this required permission using the - /// hierarchy Admin ⊇ Write ⊇ Read, otherwise [`false`] is returned. + /// Return [`true`] if `granted` satisfies this required permission using the hierarchy + /// Admin ⊇ Write ⊇ Read, otherwise [`false`] is returned. pub fn is_satisfied_by(&self, granted: &Permission) -> bool { match (self, granted) { (Permission::Read, _) => true, @@ -35,3 +38,18 @@ impl Permission { } } } + +/// Validates the credentials in `metadata` and checks that the caller has the `required_permission`. +/// Implementations must return `Ok(())` if authorized, `Err(Status::unauthenticated(...))` if +/// credentials are missing or invalid, or `Err(Status::permission_denied(...))` if credentials are +/// valid but the caller lacks the required permission. Error messages must not reveal the specific +/// reason for rejection to prevent information leakage. +pub trait Authenticator: Send + Sync { + fn authorize( + &self, + metadata: &MetadataMap, + required_permission: Permission, + ) -> Result<(), Status>; +} + + From fe13ad888d2d0e22d98ff2adf5d9db17c5e3290a Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Sun, 17 May 2026 15:46:37 +0200 Subject: [PATCH 04/59] Implement NoAuth for Authenticator --- crates/modelardb_auth/src/lib.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/modelardb_auth/src/lib.rs b/crates/modelardb_auth/src/lib.rs index b4ff7ce4..8b38d4a3 100644 --- a/crates/modelardb_auth/src/lib.rs +++ b/crates/modelardb_auth/src/lib.rs @@ -15,8 +15,8 @@ //! Types to support authentication and authorization in ModelarDB. -use tonic::metadata::MetadataMap; use tonic::Status; +use tonic::metadata::MetadataMap; /// The permission required to perform an operation in a ModelarDB server. #[derive(Debug, Clone, PartialEq, Eq)] @@ -52,4 +52,16 @@ pub trait Authenticator: Send + Sync { ) -> Result<(), Status>; } +/// An [`Authenticator`] that grants all permissions to all callers without any validation. Used in +/// the open-source version of ModelarDB where authentication is not required. +pub struct NoAuth; +impl Authenticator for NoAuth { + fn authorize( + &self, + _metadata: &MetadataMap, + _required_permission: Permission, + ) -> Result<(), Status> { + Ok(()) + } +} From 79cb7524bdec6630203186b5e46b27e104bd3019 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Sun, 17 May 2026 15:49:27 +0200 Subject: [PATCH 05/59] Move Authenticator to separate file for easier merging with closed-source --- .../modelardb_auth/src/authenticator/mod.rs | 32 +++++++++++++++++++ crates/modelardb_auth/src/lib.rs | 13 -------- 2 files changed, 32 insertions(+), 13 deletions(-) create mode 100644 crates/modelardb_auth/src/authenticator/mod.rs diff --git a/crates/modelardb_auth/src/authenticator/mod.rs b/crates/modelardb_auth/src/authenticator/mod.rs new file mode 100644 index 00000000..af424e37 --- /dev/null +++ b/crates/modelardb_auth/src/authenticator/mod.rs @@ -0,0 +1,32 @@ +/* Copyright 2026 The ModelarDB Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use tonic::Status; +use tonic::metadata::MetadataMap; + +use crate::Permission; + +/// Validates the credentials in `metadata` and checks that the caller has the `required_permission`. +/// Implementations must return `Ok(())` if authorized, `Err(Status::unauthenticated(...))` if +/// credentials are missing or invalid, or `Err(Status::permission_denied(...))` if credentials are +/// valid but the caller lacks the required permission. Error messages must not reveal the specific +/// reason for rejection to prevent information leakage. +pub trait Authenticator: Send + Sync { + fn authorize( + &self, + metadata: &MetadataMap, + required_permission: Permission, + ) -> Result<(), Status>; +} diff --git a/crates/modelardb_auth/src/lib.rs b/crates/modelardb_auth/src/lib.rs index 8b38d4a3..3fe9bfff 100644 --- a/crates/modelardb_auth/src/lib.rs +++ b/crates/modelardb_auth/src/lib.rs @@ -39,19 +39,6 @@ impl Permission { } } -/// Validates the credentials in `metadata` and checks that the caller has the `required_permission`. -/// Implementations must return `Ok(())` if authorized, `Err(Status::unauthenticated(...))` if -/// credentials are missing or invalid, or `Err(Status::permission_denied(...))` if credentials are -/// valid but the caller lacks the required permission. Error messages must not reveal the specific -/// reason for rejection to prevent information leakage. -pub trait Authenticator: Send + Sync { - fn authorize( - &self, - metadata: &MetadataMap, - required_permission: Permission, - ) -> Result<(), Status>; -} - /// An [`Authenticator`] that grants all permissions to all callers without any validation. Used in /// the open-source version of ModelarDB where authentication is not required. pub struct NoAuth; From 239674abae4d49bb0fe41b5d9c723b01ff1380c2 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Sun, 17 May 2026 15:52:24 +0200 Subject: [PATCH 06/59] Move NoAuth implementation to separate file for consistency --- .../modelardb_auth/src/authenticator/mod.rs | 5 +++ .../src/authenticator/no_auth.rs | 36 +++++++++++++++++++ crates/modelardb_auth/src/lib.rs | 14 +------- 3 files changed, 42 insertions(+), 13 deletions(-) create mode 100644 crates/modelardb_auth/src/authenticator/no_auth.rs diff --git a/crates/modelardb_auth/src/authenticator/mod.rs b/crates/modelardb_auth/src/authenticator/mod.rs index af424e37..8a9878fe 100644 --- a/crates/modelardb_auth/src/authenticator/mod.rs +++ b/crates/modelardb_auth/src/authenticator/mod.rs @@ -13,6 +13,11 @@ * limitations under the License. */ +//! Defines the [`Authenticator`] trait for validating credentials and authorizing access to +//! ModelarDB. + +pub mod no_auth; + use tonic::Status; use tonic::metadata::MetadataMap; diff --git a/crates/modelardb_auth/src/authenticator/no_auth.rs b/crates/modelardb_auth/src/authenticator/no_auth.rs new file mode 100644 index 00000000..ffd840bf --- /dev/null +++ b/crates/modelardb_auth/src/authenticator/no_auth.rs @@ -0,0 +1,36 @@ +/* Copyright 2026 The ModelarDB Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +//! An [`Authenticator`] that grants all permissions to all callers without any validation. Used in +//! the open-source version of ModelarDB where authentication is not required. + +use tonic::Status; +use tonic::metadata::MetadataMap; + +use crate::Permission; +use crate::authenticator::Authenticator; + +/// An [`Authenticator`] that grants all permissions to all callers without any validation. +pub struct NoAuth; + +impl Authenticator for NoAuth { + fn authorize( + &self, + _metadata: &MetadataMap, + _required_permission: Permission, + ) -> Result<(), Status> { + Ok(()) + } +} diff --git a/crates/modelardb_auth/src/lib.rs b/crates/modelardb_auth/src/lib.rs index 3fe9bfff..5c8575f1 100644 --- a/crates/modelardb_auth/src/lib.rs +++ b/crates/modelardb_auth/src/lib.rs @@ -15,8 +15,7 @@ //! Types to support authentication and authorization in ModelarDB. -use tonic::Status; -use tonic::metadata::MetadataMap; +pub mod authenticator; /// The permission required to perform an operation in a ModelarDB server. #[derive(Debug, Clone, PartialEq, Eq)] @@ -39,16 +38,5 @@ impl Permission { } } -/// An [`Authenticator`] that grants all permissions to all callers without any validation. Used in -/// the open-source version of ModelarDB where authentication is not required. -pub struct NoAuth; -impl Authenticator for NoAuth { - fn authorize( - &self, - _metadata: &MetadataMap, - _required_permission: Permission, - ) -> Result<(), Status> { - Ok(()) - } } From acce865a4c724e559b12cb5c4a580d5cabf2352e Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Sun, 17 May 2026 15:57:00 +0200 Subject: [PATCH 07/59] Fix closing bracket bug --- crates/modelardb_auth/src/lib.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/modelardb_auth/src/lib.rs b/crates/modelardb_auth/src/lib.rs index 5c8575f1..49aa45d4 100644 --- a/crates/modelardb_auth/src/lib.rs +++ b/crates/modelardb_auth/src/lib.rs @@ -37,6 +37,3 @@ impl Permission { } } } - - -} From e48b77172b6b4b7bbc2c5011cab7936d3ee96e1b Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Sun, 17 May 2026 16:14:05 +0200 Subject: [PATCH 08/59] Move remote.rs into a separate module folder --- crates/modelardb_server/src/{remote.rs => remote/mod.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename crates/modelardb_server/src/{remote.rs => remote/mod.rs} (100%) diff --git a/crates/modelardb_server/src/remote.rs b/crates/modelardb_server/src/remote/mod.rs similarity index 100% rename from crates/modelardb_server/src/remote.rs rename to crates/modelardb_server/src/remote/mod.rs From b6c072c01dfc38fd77a4b7d7b9a52339147153b7 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Sun, 17 May 2026 16:33:22 +0200 Subject: [PATCH 09/59] Add http-body-util dependency --- Cargo.toml | 1 + crates/modelardb_server/Cargo.toml | 2 ++ 2 files changed, 3 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index d02990b0..f1e9ee28 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,7 @@ delta_kernel = { package = "buoyant_kernel", version = "0.22.2" } deltalake = "0.32.4" dirs = "6.0.0" futures = "0.3.32" +http-body-util = "0.1.3" log = "0.4.29" object_store = "0.13.2" proptest = "1.11.0" diff --git a/crates/modelardb_server/Cargo.toml b/crates/modelardb_server/Cargo.toml index 734227ee..0d42659a 100644 --- a/crates/modelardb_server/Cargo.toml +++ b/crates/modelardb_server/Cargo.toml @@ -33,6 +33,8 @@ dashmap.workspace = true datafusion.workspace = true deltalake = { workspace = true, features = ["datafusion"] } futures.workspace = true +http-body-util.workspace = true +modelardb_auth = { path = "../modelardb_auth" } modelardb_compression = { path = "../modelardb_compression" } modelardb_storage = { path = "../modelardb_storage" } modelardb_types = { path = "../modelardb_types" } From 94ff465caa361900849784eedf8d6af50a9fa22d Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Tue, 26 May 2026 06:14:07 +0200 Subject: [PATCH 10/59] Fixed dependency issue --- Cargo.toml | 3 +++ crates/modelardb_auth/Cargo.toml | 2 -- crates/modelardb_server/Cargo.toml | 4 ++++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f1e9ee28..d9ef63f3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,6 +20,7 @@ resolver = "3" arrow = "58.0.0" arrow-flight = "58.0.0" async-trait = "0.1.89" +axum = "0.8.9" bytes = "1.11.1" chrono = "0.4.44" crossbeam-channel = "0.5.15" @@ -31,6 +32,7 @@ delta_kernel = { package = "buoyant_kernel", version = "0.22.2" } deltalake = "0.32.4" dirs = "6.0.0" futures = "0.3.32" +http = "1.4.0" http-body-util = "0.1.3" log = "0.4.29" object_store = "0.13.2" @@ -49,6 +51,7 @@ tokio = "1.52.2" tokio-stream = "0.1.18" toml = "1.1.2" tonic = "0.14.4" +tower = "0.5.3" tracing = "0.1.44" tracing-subscriber = "0.3.23" url = "2.5.8" diff --git a/crates/modelardb_auth/Cargo.toml b/crates/modelardb_auth/Cargo.toml index 723c0f9f..54fa79cf 100644 --- a/crates/modelardb_auth/Cargo.toml +++ b/crates/modelardb_auth/Cargo.toml @@ -20,6 +20,4 @@ edition = "2024" authors = ["Soeren Kejser Jensen "] [dependencies] -modelardb_storage = { path = "../modelardb_storage" } tonic.workspace = true -tokio.workspace = true \ No newline at end of file diff --git a/crates/modelardb_server/Cargo.toml b/crates/modelardb_server/Cargo.toml index 0d42659a..99eb20d5 100644 --- a/crates/modelardb_server/Cargo.toml +++ b/crates/modelardb_server/Cargo.toml @@ -26,6 +26,7 @@ path = "src/main.rs" [dependencies] arrow-flight.workspace = true async-trait.workspace = true +axum.workspace = true bytes.workspace = true crossbeam-channel.workspace = true crossbeam-queue.workspace = true @@ -33,6 +34,7 @@ dashmap.workspace = true datafusion.workspace = true deltalake = { workspace = true, features = ["datafusion"] } futures.workspace = true +http.workspace = true http-body-util.workspace = true modelardb_auth = { path = "../modelardb_auth" } modelardb_compression = { path = "../modelardb_compression" } @@ -43,10 +45,12 @@ prost.workspace = true rand.workspace = true serde.workspace = true snmalloc-rs = { workspace = true, features = ["build_cc"] } +sqlparser.workspace = true tokio = { workspace = true, features = ["rt-multi-thread", "signal"] } tokio-stream.workspace = true toml.workspace = true tonic.workspace = true +tower.workspace = true # Log is a dependency so the compile time filters for log and tracing can be set to the same values. log = { workspace = true, features = ["max_level_debug", "release_max_level_info"] } From 38d264a134f5cc8c5373d1f648183429d570ce28 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Tue, 26 May 2026 08:19:24 +0200 Subject: [PATCH 11/59] Add tower auth middleware layer --- .../modelardb_server/src/remote/auth_layer.rs | 246 ++++++++++++++++++ crates/modelardb_server/src/remote/mod.rs | 2 + 2 files changed, 248 insertions(+) create mode 100644 crates/modelardb_server/src/remote/auth_layer.rs diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs new file mode 100644 index 00000000..4b467715 --- /dev/null +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -0,0 +1,246 @@ +/* Copyright 2026 The ModelarDB Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +//! Tower middleware layer that enforces authentication and authorization on all incoming Apache +//! Arrow Flight requests. The layer runs before [`FlightServiceHandler`](super::FlightServiceHandler) +//! and checks the cluster key or bearer token before the request reaches the handler. For DoGet +//! requests the SQL ticket is decoded, and the required permission is determined from the parsed +//! statement. + +use std::future::Future; +use std::pin::Pin; +use std::sync::Arc; +use std::task::{Context, Poll}; + +use arrow_flight::Ticket; +use axum::body::Body; +use http::{Request, Response}; +use http_body_util::BodyExt; +use modelardb_auth::Permission; +use modelardb_auth::authenticator::Authenticator; +use modelardb_storage::parser::{self, ModelarDbStatement}; +use prost::Message; +use sqlparser::ast::Statement; +use tokio::sync::RwLock; +use tonic::Status; +use tonic::metadata::MetadataMap; +use tower::{Layer, Service}; +use tracing::warn; + +use crate::ClusterMode; +use crate::configuration::ConfigurationManager; + +const LIST_FLIGHTS_PATH: &str = "/arrow.flight.protocol.FlightService/ListFlights"; +const GET_FLIGHT_INFO_PATH: &str = "/arrow.flight.protocol.FlightService/GetFlightInfo"; +const GET_SCHEMA_PATH: &str = "/arrow.flight.protocol.FlightService/GetSchema"; +const DO_GET_PATH: &str = "/arrow.flight.protocol.FlightService/DoGet"; +const DO_PUT_PATH: &str = "/arrow.flight.protocol.FlightService/DoPut"; +const DO_ACTION_PATH: &str = "/arrow.flight.protocol.FlightService/DoAction"; +const LIST_ACTIONS_PATH: &str = "/arrow.flight.protocol.FlightService/ListActions"; + +/// [`Layer`] that enforces authentication and authorization on all incoming Apache Arrow Flight +/// requests. +#[derive(Clone)] +pub(super) struct AuthLayer { + /// The [`Authenticator`] to use for authentication once the auth layer has determined the + /// required permission for a client request. + authenticator: Arc, + /// The [`ConfigurationManager`] to use for cluster key validation for internal cluster requests. + configuration_manager: Arc>, +} + +impl AuthLayer { + pub(super) fn new( + authenticator: Arc, + configuration_manager: Arc>, + ) -> Self { + Self { + authenticator, + configuration_manager, + } + } +} + +impl Layer for AuthLayer { + type Service = AuthService; + + fn layer(&self, inner: S) -> AuthService { + AuthService { + inner, + authenticator: self.authenticator.clone(), + configuration_manager: self.configuration_manager.clone(), + } + } +} + +/// Middleware layer that enforces authentication and authorization on all incoming Apache Arrow +/// Flight requests. +#[derive(Clone)] +pub(super) struct AuthService { + /// The [`FlightServiceHandler`](super::FlightServiceHandler) that processes the request after + /// authorization succeeds. + inner: S, + /// The [`Authenticator`] to use for authentication once the auth layer has determined the + /// required permission for a client request. + authenticator: Arc, + /// The [`ConfigurationManager`] to use for cluster key validation for internal cluster requests. + configuration_manager: Arc>, +} + +impl Service> for AuthService +where + S: Service, Response = Response> + Clone + Send + 'static, + S::Future: Send + 'static, +{ + type Response = S::Response; + type Error = S::Error; + type Future = Pin> + Send>>; + + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.inner.poll_ready(cx) + } + + fn call(&mut self, request: Request) -> Self::Future { + // Calling a cloned service that has not been polled for readiness may cause a panic. + // Instead, take the service that poll_ready prepared and replace it with a clone for + // the next request cycle. + // + // See https://docs.rs/tower/latest/tower/trait.Service.html#be-careful-when-cloning-inner-services. + let clone = self.inner.clone(); + let mut inner = std::mem::replace(&mut self.inner, clone); + + let authenticator = self.authenticator.clone(); + let configuration_manager = self.configuration_manager.clone(); + + Box::pin(async move { + match authorize(request, &*authenticator, &configuration_manager).await { + Ok(request) => inner.call(request).await, + Err(status) => Ok(status.into_http()), + } + }) + } +} + +/// Full authorization flow. Return the (possibly reconstructed) request on success, or a +/// [`Status`] describing that the request is unauthorized on failure. +async fn authorize( + request: Request, + authenticator: &dyn Authenticator, + configuration_manager: &RwLock, +) -> Result, Status> { + let path = request.uri().path().to_owned(); + let metadata = MetadataMap::from_headers(request.headers().clone()); + + // Cluster key check must happen before the Authenticator since internal cluster requests bypass + // auth entirely. + if let Some(request_key) = metadata.get("x-cluster-key") { + let configuration_manager = configuration_manager.read().await; + return match configuration_manager.cluster_mode() { + ClusterMode::MultiNode(cluster) if cluster.key() == request_key => Ok(request), + _ => { + warn!("Request to {path} rejected: invalid cluster key."); + Err(Status::unauthenticated("Unauthorized.")) + } + }; + } + + // ListActions is a public discovery endpoint. + if path == LIST_ACTIONS_PATH { + return Ok(request); + } + + // Decode the ticket and parse the SQL to determine the required permission. + if path == DO_GET_PATH { + return authorize_do_get(request, authenticator, &metadata).await; + } + + // For all other endpoints the path determines the permission. + let required_permission = match path.as_str() { + LIST_FLIGHTS_PATH | GET_FLIGHT_INFO_PATH | GET_SCHEMA_PATH => Permission::Read, + DO_PUT_PATH => Permission::Write, + DO_ACTION_PATH => Permission::Admin, + _ => { + warn!("Request to {path} rejected: invalid path."); + return Err(Status::unauthenticated("Unauthorized.")); + } + }; + + authenticator.authorize(&metadata, required_permission)?; + + Ok(request) +} + +/// Buffer the DoGet body, decode the gRPC [`Ticket`] protobuf, parse the SQL, determine the +/// required permission, authorize, then reconstruct the request with the original bytes. +async fn authorize_do_get( + request: Request, + authenticator: &dyn Authenticator, + metadata: &MetadataMap, +) -> Result, Status> { + let (parts, body) = request.into_parts(); + + // Collect the full body. + let bytes = body + .collect() + .await + .map_err(|_| Status::unauthenticated("Unauthorized."))? + .to_bytes(); + + // gRPC has a 1-byte compression flag, a 4-byte length, and an N bytes protobuf message. + if bytes.len() < 5 { + warn!( + "DoGet body too short to be a valid gRPC message ({} bytes).", + bytes.len() + ); + return Err(Status::unauthenticated("Unauthorized.")); + } + + let ticket = Ticket::decode(&bytes[5..]).map_err(|error| { + warn!("Failed to decode DoGet Ticket: {}.", error); + Status::unauthenticated("Unauthorized.") + })?; + + let sql = str::from_utf8(&ticket.ticket).map_err(|error| { + warn!("DoGet Ticket is not valid UTF-8: {}.", error); + Status::unauthenticated("Unauthorized.") + })?; + + let statement = parser::tokenize_and_parse_sql_statement(sql).map_err(|error| { + warn!("Failed to parse DoGet SQL for permission check: {}.", error); + Status::unauthenticated("Unauthorized.") + })?; + + authenticator.authorize(metadata, permission_for_statement(&statement))?; + + // Reconstruct the request with the original bytes so the server receives it intact. + Ok(Request::from_parts(parts, Body::from(bytes))) +} + +/// Map a parsed [`ModelarDbStatement`] to the required [`Permission`]. +fn permission_for_statement(statement: &ModelarDbStatement) -> Permission { + match statement { + ModelarDbStatement::CreateNormalTable { .. } => Permission::Admin, + ModelarDbStatement::CreateTimeSeriesTable(_) => Permission::Admin, + ModelarDbStatement::DropTable(_) => Permission::Admin, + ModelarDbStatement::TruncateTable(_, _) => Permission::Admin, + ModelarDbStatement::Vacuum(_, _, _) => Permission::Admin, + ModelarDbStatement::IncludeSelect(_, _) => Permission::Read, + ModelarDbStatement::Statement(statement) => match statement { + Statement::Insert(_) => Permission::Write, + Statement::Query(_) | Statement::Explain { .. } => Permission::Read, + _ => Permission::Admin, + }, + } +} diff --git a/crates/modelardb_server/src/remote/mod.rs b/crates/modelardb_server/src/remote/mod.rs index 68733de3..d3c37407 100644 --- a/crates/modelardb_server/src/remote/mod.rs +++ b/crates/modelardb_server/src/remote/mod.rs @@ -17,6 +17,8 @@ //! [`FlightServiceHandler`]. An Apache Arrow Flight server that process requests //! using [`FlightServiceHandler`] can be started with [`start_apache_arrow_flight_server()`]. +mod auth_layer; + use std::collections::HashMap; use std::error::Error; use std::net::SocketAddr; From b8d34b179dbde2ef0e0dd5e1bcfe6ec23cd47fa6 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Tue, 26 May 2026 08:27:44 +0200 Subject: [PATCH 12/59] Make the handling of cloning inner services slightly more clear --- crates/modelardb_server/src/remote/auth_layer.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index 4b467715..52b58d34 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -113,9 +113,9 @@ where } fn call(&mut self, request: Request) -> Self::Future { - // Calling a cloned service that has not been polled for readiness may cause a panic. - // Instead, take the service that poll_ready prepared and replace it with a clone for - // the next request cycle. + // A cloned service may not be ready and panics if called before poll_ready. Use mem::replace + // to take the ready service out of self.inner and put a fresh clone in its place, which + // will be polled for readiness on the next request. // // See https://docs.rs/tower/latest/tower/trait.Service.html#be-careful-when-cloning-inner-services. let clone = self.inner.clone(); From 42459c686ea5e140feb75ca808aeb462f44d9a0c Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Tue, 26 May 2026 09:55:12 +0200 Subject: [PATCH 13/59] Remove axum dependency --- crates/modelardb_server/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/modelardb_server/Cargo.toml b/crates/modelardb_server/Cargo.toml index 99eb20d5..391305d5 100644 --- a/crates/modelardb_server/Cargo.toml +++ b/crates/modelardb_server/Cargo.toml @@ -26,7 +26,6 @@ path = "src/main.rs" [dependencies] arrow-flight.workspace = true async-trait.workspace = true -axum.workspace = true bytes.workspace = true crossbeam-channel.workspace = true crossbeam-queue.workspace = true From a9a7668b154f6a569693c8b31fbeb6135e2c97a7 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Tue, 26 May 2026 09:55:56 +0200 Subject: [PATCH 14/59] Use tonic body instead of axum explicitly --- crates/modelardb_server/src/remote/auth_layer.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index 52b58d34..7ad0984a 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -25,9 +25,8 @@ use std::sync::Arc; use std::task::{Context, Poll}; use arrow_flight::Ticket; -use axum::body::Body; use http::{Request, Response}; -use http_body_util::BodyExt; +use http_body_util::{BodyExt, Full}; use modelardb_auth::Permission; use modelardb_auth::authenticator::Authenticator; use modelardb_storage::parser::{self, ModelarDbStatement}; @@ -35,6 +34,7 @@ use prost::Message; use sqlparser::ast::Statement; use tokio::sync::RwLock; use tonic::Status; +use tonic::body::Body; use tonic::metadata::MetadataMap; use tower::{Layer, Service}; use tracing::warn; @@ -225,7 +225,7 @@ async fn authorize_do_get( authenticator.authorize(metadata, permission_for_statement(&statement))?; // Reconstruct the request with the original bytes so the server receives it intact. - Ok(Request::from_parts(parts, Body::from(bytes))) + Ok(Request::from_parts(parts, Body::new(Full::new(bytes)))) } /// Map a parsed [`ModelarDbStatement`] to the required [`Permission`]. From 9e38f44783da13d4370249a9535ecdb398d9752c Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Tue, 26 May 2026 10:03:25 +0200 Subject: [PATCH 15/59] Add auth layer to Apache Arrow Flight server --- crates/modelardb_server/src/main.rs | 4 +++- crates/modelardb_server/src/remote/mod.rs | 15 +++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/crates/modelardb_server/src/main.rs b/crates/modelardb_server/src/main.rs index ce5cfbd0..fc8d18cf 100644 --- a/crates/modelardb_server/src/main.rs +++ b/crates/modelardb_server/src/main.rs @@ -26,6 +26,7 @@ mod storage; use std::sync::{Arc, LazyLock}; use std::{env, process}; +use modelardb_auth::authenticator::no_auth::NoAuth; use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt}; use crate::cluster::Cluster; @@ -90,7 +91,8 @@ async fn main() -> Result<()> { context.replay_write_ahead_log().await?; // Start the Apache Arrow Flight interface. - remote::start_apache_arrow_flight_server(context, *PORT).await?; + let authenticator = Arc::new(NoAuth); + remote::start_apache_arrow_flight_server(context, authenticator, *PORT).await?; Ok(()) } diff --git a/crates/modelardb_server/src/remote/mod.rs b/crates/modelardb_server/src/remote/mod.rs index d3c37407..ae0f64de 100644 --- a/crates/modelardb_server/src/remote/mod.rs +++ b/crates/modelardb_server/src/remote/mod.rs @@ -45,6 +45,7 @@ use datafusion::physical_plan::{EmptyRecordBatchStream, SendableRecordBatchStrea use deltalake::arrow::datatypes::Schema; use futures::StreamExt; use futures::stream::{self, BoxStream, SelectAll}; +use modelardb_auth::authenticator::Authenticator; use modelardb_storage::parser::{self, ModelarDbStatement}; use modelardb_types::flight::protocol; use modelardb_types::functions; @@ -62,16 +63,25 @@ use crate::ClusterMode; use crate::cluster::Cluster; use crate::context::Context; use crate::error::{ModelarDbServerError, Result}; +use crate::remote::auth_layer::AuthLayer; /// Start an Apache Arrow Flight server on 0.0.0.0:`port` that passes `context` to the methods that -/// process the requests through [`FlightServiceHandler`]. -pub async fn start_apache_arrow_flight_server(context: Arc, port: u16) -> Result<()> { +/// process the requests through [`FlightServiceHandler`]. All requests are passed through the +/// [`AuthLayer`] that authenticates using `authenticator` before they are passed to the +/// [`FlightServiceHandler`]. +pub async fn start_apache_arrow_flight_server( + context: Arc, + authenticator: Arc, + port: u16, +) -> Result<()> { let localhost_with_port = "0.0.0.0:".to_owned() + &port.to_string(); let localhost_with_port: SocketAddr = localhost_with_port.parse().map_err(|error| { ModelarDbServerError::InvalidArgument(format!( "Unable to parse {localhost_with_port}: {error}" )) })?; + + let auth_layer = AuthLayer::new(authenticator, context.configuration_manager.clone()); let handler = FlightServiceHandler::new(context); // Increase the maximum message size from 4 MiB to 16 MiB to allow bulk-loading larger batches. @@ -81,6 +91,7 @@ pub async fn start_apache_arrow_flight_server(context: Arc, port: u16) info!("Starting Apache Arrow Flight on {}.", localhost_with_port); Server::builder() + .layer(auth_layer) .add_service(flight_service_server) .serve(localhost_with_port) .await From ef7f24f84342851897b49b1b8df6d475ca056da8 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Tue, 26 May 2026 10:07:16 +0200 Subject: [PATCH 16/59] Use matches! macro to fix clippy issue --- crates/modelardb_auth/src/lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/modelardb_auth/src/lib.rs b/crates/modelardb_auth/src/lib.rs index 49aa45d4..c0e743e8 100644 --- a/crates/modelardb_auth/src/lib.rs +++ b/crates/modelardb_auth/src/lib.rs @@ -29,11 +29,11 @@ impl Permission { /// Return [`true`] if `granted` satisfies this required permission using the hierarchy /// Admin ⊇ Write ⊇ Read, otherwise [`false`] is returned. pub fn is_satisfied_by(&self, granted: &Permission) -> bool { - match (self, granted) { - (Permission::Read, _) => true, - (Permission::Write, Permission::Write | Permission::Admin) => true, - (Permission::Admin, Permission::Admin) => true, - _ => false, - } + matches!( + (self, granted), + (Permission::Read, _) + | (Permission::Write, Permission::Write | Permission::Admin) + | (Permission::Admin, Permission::Admin) + ) } } From aa5097496dec3e78c17fc79aec1347cf2325d6dd Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Tue, 26 May 2026 10:45:23 +0200 Subject: [PATCH 17/59] Add simple unit tests for Permission and NoAuth --- .../src/authenticator/no_auth.rs | 32 ++++++++++++ crates/modelardb_auth/src/lib.rs | 50 +++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/crates/modelardb_auth/src/authenticator/no_auth.rs b/crates/modelardb_auth/src/authenticator/no_auth.rs index ffd840bf..b78bf25f 100644 --- a/crates/modelardb_auth/src/authenticator/no_auth.rs +++ b/crates/modelardb_auth/src/authenticator/no_auth.rs @@ -34,3 +34,35 @@ impl Authenticator for NoAuth { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_no_auth_allows_read() { + assert!( + NoAuth + .authorize(&MetadataMap::new(), Permission::Read) + .is_ok() + ); + } + + #[test] + fn test_no_auth_allows_write() { + assert!( + NoAuth + .authorize(&MetadataMap::new(), Permission::Write) + .is_ok() + ); + } + + #[test] + fn test_no_auth_allows_admin() { + assert!( + NoAuth + .authorize(&MetadataMap::new(), Permission::Admin) + .is_ok() + ); + } +} diff --git a/crates/modelardb_auth/src/lib.rs b/crates/modelardb_auth/src/lib.rs index c0e743e8..14a306fb 100644 --- a/crates/modelardb_auth/src/lib.rs +++ b/crates/modelardb_auth/src/lib.rs @@ -37,3 +37,53 @@ impl Permission { ) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_read_satisfied_by_read() { + assert!(Permission::Read.is_satisfied_by(&Permission::Read)); + } + + #[test] + fn test_read_satisfied_by_write() { + assert!(Permission::Read.is_satisfied_by(&Permission::Write)); + } + + #[test] + fn test_read_satisfied_by_admin() { + assert!(Permission::Read.is_satisfied_by(&Permission::Admin)); + } + + #[test] + fn test_write_not_satisfied_by_read() { + assert!(!Permission::Write.is_satisfied_by(&Permission::Read)); + } + + #[test] + fn test_write_satisfied_by_write() { + assert!(Permission::Write.is_satisfied_by(&Permission::Write)); + } + + #[test] + fn test_write_satisfied_by_admin() { + assert!(Permission::Write.is_satisfied_by(&Permission::Admin)); + } + + #[test] + fn test_admin_not_satisfied_by_read() { + assert!(!Permission::Admin.is_satisfied_by(&Permission::Read)); + } + + #[test] + fn test_admin_not_satisfied_by_write() { + assert!(!Permission::Admin.is_satisfied_by(&Permission::Write)); + } + + #[test] + fn test_admin_satisfied_by_admin() { + assert!(Permission::Admin.is_satisfied_by(&Permission::Admin)); + } +} From e027f0e511b8ee23a0fdfa916f41540ee7a2051f Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Wed, 27 May 2026 08:20:00 +0200 Subject: [PATCH 18/59] Add mock authenticator --- crates/modelardb_auth/Cargo.toml | 3 + .../modelardb_auth/src/authenticator/mock.rs | 58 +++++++++++++++++++ .../modelardb_auth/src/authenticator/mod.rs | 2 + crates/modelardb_server/Cargo.toml | 1 + 4 files changed, 64 insertions(+) create mode 100644 crates/modelardb_auth/src/authenticator/mock.rs diff --git a/crates/modelardb_auth/Cargo.toml b/crates/modelardb_auth/Cargo.toml index 54fa79cf..70cd1ed1 100644 --- a/crates/modelardb_auth/Cargo.toml +++ b/crates/modelardb_auth/Cargo.toml @@ -21,3 +21,6 @@ authors = ["Soeren Kejser Jensen "] [dependencies] tonic.workspace = true + +[features] +testing = [] diff --git a/crates/modelardb_auth/src/authenticator/mock.rs b/crates/modelardb_auth/src/authenticator/mock.rs new file mode 100644 index 00000000..3a2521d0 --- /dev/null +++ b/crates/modelardb_auth/src/authenticator/mock.rs @@ -0,0 +1,58 @@ +/* Copyright 2026 The ModelarDB Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +//! A mock [`Authenticator`] for use in tests. Always grants access and records every permission +//! that was requested so tests can assert which permission the auth layer required for a given +//! request. + +use std::sync::Mutex; + +use tonic::Status; +use tonic::metadata::MetadataMap; + +use crate::Permission; +use crate::authenticator::Authenticator; + +/// An [`Authenticator`] for use in tests that always grants access and records every +/// [`Permission`] passed to [`authorize`](Authenticator::authorize). +pub struct MockAuthenticator { + /// Every [`Permission`] passed to [`authorize`](Authenticator::authorize). + calls: Mutex>, +} + +impl MockAuthenticator { + pub fn new() -> Self { + Self { + calls: Mutex::new(vec![]), + } + } + + /// Return a copy of all permissions passed to [`authorize`](Authenticator::authorize) + /// in the order they were received. + pub fn calls(&self) -> Vec { + self.calls.lock().unwrap().clone() + } +} + +impl Authenticator for MockAuthenticator { + fn authorize( + &self, + _metadata: &MetadataMap, + required_permission: Permission, + ) -> Result<(), Status> { + self.calls.lock().unwrap().push(required_permission); + Ok(()) + } +} diff --git a/crates/modelardb_auth/src/authenticator/mod.rs b/crates/modelardb_auth/src/authenticator/mod.rs index 8a9878fe..252a7a77 100644 --- a/crates/modelardb_auth/src/authenticator/mod.rs +++ b/crates/modelardb_auth/src/authenticator/mod.rs @@ -16,6 +16,8 @@ //! Defines the [`Authenticator`] trait for validating credentials and authorizing access to //! ModelarDB. +#[cfg(feature = "testing")] +pub mod mock; pub mod no_auth; use tonic::Status; diff --git a/crates/modelardb_server/Cargo.toml b/crates/modelardb_server/Cargo.toml index 391305d5..7fed69ff 100644 --- a/crates/modelardb_server/Cargo.toml +++ b/crates/modelardb_server/Cargo.toml @@ -58,6 +58,7 @@ tracing-subscriber.workspace = true [dev-dependencies] modelardb_test = { path = "../modelardb_test" } +modelardb_auth = { path = "../modelardb_auth", features = ["testing"] } proptest.workspace = true tempfile.workspace = true From 180b69e88e74db29e19d2c825df3d837b4815212 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Wed, 27 May 2026 09:12:01 +0200 Subject: [PATCH 19/59] Add helper methods for auth layer tests --- .../modelardb_server/src/remote/auth_layer.rs | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index 7ad0984a..3a3b8c24 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -244,3 +244,66 @@ fn permission_for_statement(statement: &ModelarDbStatement) -> Permission { }, } } + +#[cfg(test)] +mod tests { + use super::*; + + use modelardb_storage::data_folder::DataFolder; + use modelardb_types::types::{Node, ServerMode}; + use tempfile::TempDir; + + use crate::cluster::Cluster; + + fn empty_request(path: &str) -> Request { + Request::builder().uri(path).body(Body::empty()).unwrap() + } + + fn empty_request_with_cluster_key(path: &str, key: &str) -> Request { + Request::builder() + .uri(path) + .header("x-cluster-key", key) + .body(Body::empty()) + .unwrap() + } + + async fn single_node_configuration_manager( + temp_dir: &TempDir, + ) -> Arc> { + let local_url = temp_dir.path().to_str().unwrap(); + let local_data_folder = DataFolder::open_local_url(local_url).await.unwrap(); + + Arc::new(RwLock::new( + ConfigurationManager::try_new(local_data_folder, ClusterMode::SingleNode) + .await + .unwrap(), + )) + } + + async fn multi_node_configuration_manager( + temp_dir: &TempDir, + ) -> (Arc>, String) { + let local_url = temp_dir.path().to_str().unwrap(); + + let local_data_folder = DataFolder::open_local_url(local_url).await.unwrap(); + let remote_data_folder = DataFolder::open_local_url(local_url).await.unwrap(); + + let edge_node = Node::new("edge".to_owned(), ServerMode::Edge); + let cluster = Cluster::try_new(edge_node, remote_data_folder) + .await + .unwrap(); + + let key = cluster.key().to_str().unwrap().to_owned(); + + let configuration_manager = Arc::new(RwLock::new( + ConfigurationManager::try_new( + local_data_folder, + ClusterMode::MultiNode(Box::new(cluster)), + ) + .await + .unwrap(), + )); + + (configuration_manager, key) + } +} From 8e5ddb83e7ce70a04aab783dab6e73c7492b4d33 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Wed, 27 May 2026 09:16:50 +0200 Subject: [PATCH 20/59] Add more specific warn messages for cluster key handling --- crates/modelardb_server/src/remote/auth_layer.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index 3a3b8c24..4b510a0c 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -149,10 +149,14 @@ async fn authorize( let configuration_manager = configuration_manager.read().await; return match configuration_manager.cluster_mode() { ClusterMode::MultiNode(cluster) if cluster.key() == request_key => Ok(request), - _ => { + ClusterMode::MultiNode(_) => { warn!("Request to {path} rejected: invalid cluster key."); Err(Status::unauthenticated("Unauthorized.")) } + _ => { + warn!("Request to {path} rejected: cluster key sent to single-node server."); + Err(Status::unauthenticated("Unauthorized.")) + } }; } From 67208bc6e482b82fd7676274badc03867a3b8846 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Wed, 27 May 2026 10:09:45 +0200 Subject: [PATCH 21/59] Use actual error messages instead of only unauthenticated("Unauthorized.") --- .../modelardb_server/src/remote/auth_layer.rs | 44 ++++++------------- 1 file changed, 13 insertions(+), 31 deletions(-) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index 4b510a0c..d87b1714 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -37,10 +37,10 @@ use tonic::Status; use tonic::body::Body; use tonic::metadata::MetadataMap; use tower::{Layer, Service}; -use tracing::warn; use crate::ClusterMode; use crate::configuration::ConfigurationManager; +use crate::remote::error_to_status_invalid_argument; const LIST_FLIGHTS_PATH: &str = "/arrow.flight.protocol.FlightService/ListFlights"; const GET_FLIGHT_INFO_PATH: &str = "/arrow.flight.protocol.FlightService/GetFlightInfo"; @@ -134,7 +134,7 @@ where } /// Full authorization flow. Return the (possibly reconstructed) request on success, or a -/// [`Status`] describing that the request is unauthorized on failure. +/// [`Status`] on failure. async fn authorize( request: Request, authenticator: &dyn Authenticator, @@ -149,14 +149,8 @@ async fn authorize( let configuration_manager = configuration_manager.read().await; return match configuration_manager.cluster_mode() { ClusterMode::MultiNode(cluster) if cluster.key() == request_key => Ok(request), - ClusterMode::MultiNode(_) => { - warn!("Request to {path} rejected: invalid cluster key."); - Err(Status::unauthenticated("Unauthorized.")) - } - _ => { - warn!("Request to {path} rejected: cluster key sent to single-node server."); - Err(Status::unauthenticated("Unauthorized.")) - } + ClusterMode::MultiNode(_) => Err(Status::internal("Invalid cluster key.")), + _ => Err(Status::internal("Cluster key sent to single-node server.")), }; } @@ -176,8 +170,7 @@ async fn authorize( DO_PUT_PATH => Permission::Write, DO_ACTION_PATH => Permission::Admin, _ => { - warn!("Request to {path} rejected: invalid path."); - return Err(Status::unauthenticated("Unauthorized.")); + return Err(Status::invalid_argument("Unknown path.")); } }; @@ -199,32 +192,21 @@ async fn authorize_do_get( let bytes = body .collect() .await - .map_err(|_| Status::unauthenticated("Unauthorized."))? + .map_err(|_| Status::invalid_argument("Failed to unpack request body."))? .to_bytes(); // gRPC has a 1-byte compression flag, a 4-byte length, and an N bytes protobuf message. if bytes.len() < 5 { - warn!( - "DoGet body too short to be a valid gRPC message ({} bytes).", - bytes.len() - ); - return Err(Status::unauthenticated("Unauthorized.")); + return Err(Status::invalid_argument( + "Request body too short to be a valid gRPC message.", + )); } - let ticket = Ticket::decode(&bytes[5..]).map_err(|error| { - warn!("Failed to decode DoGet Ticket: {}.", error); - Status::unauthenticated("Unauthorized.") - })?; - - let sql = str::from_utf8(&ticket.ticket).map_err(|error| { - warn!("DoGet Ticket is not valid UTF-8: {}.", error); - Status::unauthenticated("Unauthorized.") - })?; + let ticket = Ticket::decode(&bytes[5..]).map_err(error_to_status_invalid_argument)?; + let sql = str::from_utf8(&ticket.ticket).map_err(error_to_status_invalid_argument)?; - let statement = parser::tokenize_and_parse_sql_statement(sql).map_err(|error| { - warn!("Failed to parse DoGet SQL for permission check: {}.", error); - Status::unauthenticated("Unauthorized.") - })?; + let statement = + parser::tokenize_and_parse_sql_statement(sql).map_err(error_to_status_invalid_argument)?; authenticator.authorize(metadata, permission_for_statement(&statement))?; From 3f321bc2399fb81c400f32f77b7257a402d9500c Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Wed, 27 May 2026 11:12:53 +0200 Subject: [PATCH 22/59] Removed cluster_key_in_request() --- crates/modelardb_server/src/remote/mod.rs | 27 +++-------------------- 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/crates/modelardb_server/src/remote/mod.rs b/crates/modelardb_server/src/remote/mod.rs index ae0f64de..780e1100 100644 --- a/crates/modelardb_server/src/remote/mod.rs +++ b/crates/modelardb_server/src/remote/mod.rs @@ -60,7 +60,6 @@ use tonic::{Request, Response, Status, Streaming}; use tracing::{debug, error, info}; use crate::ClusterMode; -use crate::cluster::Cluster; use crate::context::Context; use crate::error::{ModelarDbServerError, Result}; use crate::remote::auth_layer::AuthLayer; @@ -265,26 +264,6 @@ pub fn table_name_from_flight_descriptor( .ok_or_else(|| Status::invalid_argument("No table name in FlightDescriptor.path.")) } -/// Return `true` if the request contains the cluster key and `false` if not. If the request -/// contains a key that does not match the cluster key, return [`Status`]. -#[allow(clippy::result_large_err)] -fn cluster_key_in_request( - cluster: &Cluster, - request_metadata: &MetadataMap, -) -> StdResult { - if let Some(request_key) = request_metadata.get("x-cluster-key") { - if cluster.key() == request_key { - Ok(true) - } else { - Err(Status::invalid_argument( - "The cluster key in the request does not match the cluster key in the configuration.", - )) - } - } else { - Ok(false) - } -} - /// Return an empty stream of [`RecordBatches`](datafusion::arrow::record_batch::RecordBatch) that /// can be returned when a SQL command has been successfully executed but did not produce any rows /// to return. @@ -343,7 +322,7 @@ impl FlightServiceHandler { // If the cluster key is in the request, the request is from a peer node, which means the // table has already been created in the remote data folder and propagated to all nodes. if let ClusterMode::MultiNode(cluster) = configuration_manager.cluster_mode() - && !cluster_key_in_request(cluster, request_metadata)? + && request_metadata.get("x-cluster-key").is_none() { cluster .create_cluster_normal_table(table_name, schema) @@ -372,7 +351,7 @@ impl FlightServiceHandler { // If the cluster key is in the request, the request is from a peer node, which means the // table has already been created in the remote data folder and propagated to all nodes. if let ClusterMode::MultiNode(cluster) = configuration_manager.cluster_mode() - && !cluster_key_in_request(cluster, request_metadata)? + && request_metadata.get("x-cluster-key").is_none() { cluster .create_cluster_time_series_table(time_series_table_metadata) @@ -401,7 +380,7 @@ impl FlightServiceHandler { // If the cluster key is in the request, the request is from a peer node, which means the // tables have already been dropped in the remote data folder and propagated to all nodes. if let ClusterMode::MultiNode(cluster) = configuration_manager.cluster_mode() - && !cluster_key_in_request(cluster, request_metadata)? + && request_metadata.get("x-cluster-key").is_none() { cluster .drop_cluster_tables(table_names) From 320aa767ad74633d00a3a21533d9b681a654babb Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Wed, 27 May 2026 11:28:07 +0200 Subject: [PATCH 23/59] Only pass cluster key instead of entire configuration manager to auth layer --- .../modelardb_server/src/remote/auth_layer.rs | 82 ++++--------------- crates/modelardb_server/src/remote/mod.rs | 7 +- 2 files changed, 23 insertions(+), 66 deletions(-) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index d87b1714..131e36a0 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -32,14 +32,11 @@ use modelardb_auth::authenticator::Authenticator; use modelardb_storage::parser::{self, ModelarDbStatement}; use prost::Message; use sqlparser::ast::Statement; -use tokio::sync::RwLock; use tonic::Status; use tonic::body::Body; -use tonic::metadata::MetadataMap; +use tonic::metadata::{Ascii, MetadataMap, MetadataValue}; use tower::{Layer, Service}; -use crate::ClusterMode; -use crate::configuration::ConfigurationManager; use crate::remote::error_to_status_invalid_argument; const LIST_FLIGHTS_PATH: &str = "/arrow.flight.protocol.FlightService/ListFlights"; @@ -57,18 +54,19 @@ pub(super) struct AuthLayer { /// The [`Authenticator`] to use for authentication once the auth layer has determined the /// required permission for a client request. authenticator: Arc, - /// The [`ConfigurationManager`] to use for cluster key validation for internal cluster requests. - configuration_manager: Arc>, + /// The cluster key to use for validating internal cluster requests or [`None`] if running a + /// single-node server. + maybe_cluster_key: Option>, } impl AuthLayer { pub(super) fn new( authenticator: Arc, - configuration_manager: Arc>, + maybe_cluster_key: Option>, ) -> Self { Self { authenticator, - configuration_manager, + maybe_cluster_key, } } } @@ -80,7 +78,7 @@ impl Layer for AuthLayer { AuthService { inner, authenticator: self.authenticator.clone(), - configuration_manager: self.configuration_manager.clone(), + maybe_cluster_key: self.maybe_cluster_key.clone(), } } } @@ -95,8 +93,9 @@ pub(super) struct AuthService { /// The [`Authenticator`] to use for authentication once the auth layer has determined the /// required permission for a client request. authenticator: Arc, - /// The [`ConfigurationManager`] to use for cluster key validation for internal cluster requests. - configuration_manager: Arc>, + /// The cluster key to use for validating internal cluster requests or [`None`] if running a + /// single-node server. + maybe_cluster_key: Option>, } impl Service> for AuthService @@ -122,10 +121,10 @@ where let mut inner = std::mem::replace(&mut self.inner, clone); let authenticator = self.authenticator.clone(); - let configuration_manager = self.configuration_manager.clone(); + let maybe_cluster_key = self.maybe_cluster_key.clone(); Box::pin(async move { - match authorize(request, &*authenticator, &configuration_manager).await { + match authorize(request, &*authenticator, &maybe_cluster_key).await { Ok(request) => inner.call(request).await, Err(status) => Ok(status.into_http()), } @@ -138,7 +137,7 @@ where async fn authorize( request: Request, authenticator: &dyn Authenticator, - configuration_manager: &RwLock, + maybe_cluster_key: &Option>, ) -> Result, Status> { let path = request.uri().path().to_owned(); let metadata = MetadataMap::from_headers(request.headers().clone()); @@ -146,11 +145,10 @@ async fn authorize( // Cluster key check must happen before the Authenticator since internal cluster requests bypass // auth entirely. if let Some(request_key) = metadata.get("x-cluster-key") { - let configuration_manager = configuration_manager.read().await; - return match configuration_manager.cluster_mode() { - ClusterMode::MultiNode(cluster) if cluster.key() == request_key => Ok(request), - ClusterMode::MultiNode(_) => Err(Status::internal("Invalid cluster key.")), - _ => Err(Status::internal("Cluster key sent to single-node server.")), + return match maybe_cluster_key { + Some(key) if key == request_key => Ok(request), + Some(_) => Err(Status::internal("Invalid cluster key.")), + None => Err(Status::internal("Cluster key sent to single-node server.")), }; } @@ -235,12 +233,6 @@ fn permission_for_statement(statement: &ModelarDbStatement) -> Permission { mod tests { use super::*; - use modelardb_storage::data_folder::DataFolder; - use modelardb_types::types::{Node, ServerMode}; - use tempfile::TempDir; - - use crate::cluster::Cluster; - fn empty_request(path: &str) -> Request { Request::builder().uri(path).body(Body::empty()).unwrap() } @@ -252,44 +244,4 @@ mod tests { .body(Body::empty()) .unwrap() } - - async fn single_node_configuration_manager( - temp_dir: &TempDir, - ) -> Arc> { - let local_url = temp_dir.path().to_str().unwrap(); - let local_data_folder = DataFolder::open_local_url(local_url).await.unwrap(); - - Arc::new(RwLock::new( - ConfigurationManager::try_new(local_data_folder, ClusterMode::SingleNode) - .await - .unwrap(), - )) - } - - async fn multi_node_configuration_manager( - temp_dir: &TempDir, - ) -> (Arc>, String) { - let local_url = temp_dir.path().to_str().unwrap(); - - let local_data_folder = DataFolder::open_local_url(local_url).await.unwrap(); - let remote_data_folder = DataFolder::open_local_url(local_url).await.unwrap(); - - let edge_node = Node::new("edge".to_owned(), ServerMode::Edge); - let cluster = Cluster::try_new(edge_node, remote_data_folder) - .await - .unwrap(); - - let key = cluster.key().to_str().unwrap().to_owned(); - - let configuration_manager = Arc::new(RwLock::new( - ConfigurationManager::try_new( - local_data_folder, - ClusterMode::MultiNode(Box::new(cluster)), - ) - .await - .unwrap(), - )); - - (configuration_manager, key) - } } diff --git a/crates/modelardb_server/src/remote/mod.rs b/crates/modelardb_server/src/remote/mod.rs index 780e1100..edd18bcf 100644 --- a/crates/modelardb_server/src/remote/mod.rs +++ b/crates/modelardb_server/src/remote/mod.rs @@ -80,7 +80,12 @@ pub async fn start_apache_arrow_flight_server( )) })?; - let auth_layer = AuthLayer::new(authenticator, context.configuration_manager.clone()); + let maybe_cluster_key = match context.configuration_manager.read().await.cluster_mode() { + ClusterMode::MultiNode(cluster) => Some(cluster.key().clone()), + ClusterMode::SingleNode => None, + }; + + let auth_layer = AuthLayer::new(authenticator, maybe_cluster_key); let handler = FlightServiceHandler::new(context); // Increase the maximum message size from 4 MiB to 16 MiB to allow bulk-loading larger batches. From f77b9fe5f6d27f413972195057ec481ebb9433ee Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 28 May 2026 06:33:48 +0200 Subject: [PATCH 24/59] Add tests for cluster key validation in auth layer --- .../modelardb_server/src/remote/auth_layer.rs | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index 131e36a0..30d116e7 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -233,6 +233,52 @@ fn permission_for_statement(statement: &ModelarDbStatement) -> Permission { mod tests { use super::*; + use modelardb_auth::authenticator::mock::MockAuthenticator; + + const CLUSTER_KEY: &str = "cluster_key"; + + #[tokio::test] + async fn test_multi_node_with_valid_cluster_key_bypasses_authenticator() { + let authenticator = Arc::new(MockAuthenticator::new()); + + let request = empty_request_with_cluster_key(DO_PUT_PATH, CLUSTER_KEY); + let cluster_key = Some(MetadataValue::from_static(CLUSTER_KEY)); + + let result = authorize(request, &*authenticator, &cluster_key).await; + + assert!(result.is_ok()); + assert!(authenticator.calls().is_empty()); + } + + #[tokio::test] + async fn test_multi_node_with_invalid_cluster_key() { + let authenticator = Arc::new(MockAuthenticator::new()); + + let request = empty_request_with_cluster_key(LIST_FLIGHTS_PATH, "invalid_key"); + let cluster_key = Some(MetadataValue::from_static(CLUSTER_KEY)); + + let result = authorize(request, &*authenticator, &cluster_key).await; + + assert_eq!( + result.unwrap_err().to_string(), + "code: 'Internal error', message: \"Invalid cluster key.\"" + ); + } + + #[tokio::test] + async fn test_single_node_with_cluster_key() { + let authenticator = Arc::new(MockAuthenticator::new()); + + let request = empty_request_with_cluster_key(LIST_FLIGHTS_PATH, CLUSTER_KEY); + + let result = authorize(request, &*authenticator, &None).await; + + assert_eq!( + result.unwrap_err().to_string(), + "code: 'Internal error', message: \"Cluster key sent to single-node server.\"" + ); + } + fn empty_request(path: &str) -> Request { Request::builder().uri(path).body(Body::empty()).unwrap() } From 37432c1e76d6c036ad27367c6285cd86f58548e6 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 28 May 2026 06:36:33 +0200 Subject: [PATCH 25/59] Add test to ensure list actions bypasses authenticator --- .../modelardb_server/src/remote/auth_layer.rs | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index 30d116e7..1e4d38b1 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -268,7 +268,6 @@ mod tests { #[tokio::test] async fn test_single_node_with_cluster_key() { let authenticator = Arc::new(MockAuthenticator::new()); - let request = empty_request_with_cluster_key(LIST_FLIGHTS_PATH, CLUSTER_KEY); let result = authorize(request, &*authenticator, &None).await; @@ -279,10 +278,6 @@ mod tests { ); } - fn empty_request(path: &str) -> Request { - Request::builder().uri(path).body(Body::empty()).unwrap() - } - fn empty_request_with_cluster_key(path: &str, key: &str) -> Request { Request::builder() .uri(path) @@ -290,4 +285,19 @@ mod tests { .body(Body::empty()) .unwrap() } + + #[tokio::test] + async fn test_list_actions_bypasses_authenticator() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = empty_request(LIST_ACTIONS_PATH); + + let result = authorize(request, &*authenticator, &None).await; + + assert!(result.is_ok()); + assert!(authenticator.calls().is_empty()); + } + + fn empty_request(path: &str) -> Request { + Request::builder().uri(path).body(Body::empty()).unwrap() + } } From 6ac6c3e14c34a9b20b8732557f7e9cd484af45da Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 28 May 2026 06:39:36 +0200 Subject: [PATCH 26/59] Add test for authorizing an unknown path --- .../modelardb_server/src/remote/auth_layer.rs | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index 1e4d38b1..7d8f1d4e 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -238,7 +238,7 @@ mod tests { const CLUSTER_KEY: &str = "cluster_key"; #[tokio::test] - async fn test_multi_node_with_valid_cluster_key_bypasses_authenticator() { + async fn test_authorize_multi_node_with_valid_cluster_key_bypasses_authenticator() { let authenticator = Arc::new(MockAuthenticator::new()); let request = empty_request_with_cluster_key(DO_PUT_PATH, CLUSTER_KEY); @@ -251,7 +251,7 @@ mod tests { } #[tokio::test] - async fn test_multi_node_with_invalid_cluster_key() { + async fn test_authorize_multi_node_with_invalid_cluster_key() { let authenticator = Arc::new(MockAuthenticator::new()); let request = empty_request_with_cluster_key(LIST_FLIGHTS_PATH, "invalid_key"); @@ -266,7 +266,7 @@ mod tests { } #[tokio::test] - async fn test_single_node_with_cluster_key() { + async fn test_authorize_single_node_with_cluster_key() { let authenticator = Arc::new(MockAuthenticator::new()); let request = empty_request_with_cluster_key(LIST_FLIGHTS_PATH, CLUSTER_KEY); @@ -287,7 +287,7 @@ mod tests { } #[tokio::test] - async fn test_list_actions_bypasses_authenticator() { + async fn test_authorize_list_actions_bypasses_authenticator() { let authenticator = Arc::new(MockAuthenticator::new()); let request = empty_request(LIST_ACTIONS_PATH); @@ -297,6 +297,19 @@ mod tests { assert!(authenticator.calls().is_empty()); } + #[tokio::test] + async fn test_authorize_unknown_path() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = empty_request("/unknown/path"); + + let result = authorize(request, &*authenticator, &None).await; + + assert_eq!( + result.unwrap_err().to_string(), + "code: 'Client specified an invalid argument', message: \"Unknown path.\"" + ); + } + fn empty_request(path: &str) -> Request { Request::builder().uri(path).body(Body::empty()).unwrap() } From 7a6aaf8be17360fc151dbac2e9dda752e0acb3d6 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 28 May 2026 06:50:18 +0200 Subject: [PATCH 27/59] Add tests for permission mapping --- .../modelardb_server/src/remote/auth_layer.rs | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index 7d8f1d4e..3c2a64e9 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -297,6 +297,61 @@ mod tests { assert!(authenticator.calls().is_empty()); } + #[tokio::test] + async fn test_authorize_list_flights_calls_authenticator_with_read() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = empty_request(LIST_FLIGHTS_PATH); + + let result = authorize(request, &*authenticator, &None).await; + + assert!(result.is_ok()); + assert_eq!(authenticator.calls(), vec![Permission::Read]); + } + + #[tokio::test] + async fn test_authorize_get_flight_info_calls_authenticator_with_read() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = empty_request(GET_FLIGHT_INFO_PATH); + + let result = authorize(request, &*authenticator, &None).await; + + assert!(result.is_ok()); + assert_eq!(authenticator.calls(), vec![Permission::Read]); + } + + #[tokio::test] + async fn test_authorize_get_schema_calls_authenticator_with_read() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = empty_request(GET_SCHEMA_PATH); + + let result = authorize(request, &*authenticator, &None).await; + + assert!(result.is_ok()); + assert_eq!(authenticator.calls(), vec![Permission::Read]); + } + + #[tokio::test] + async fn test_authorize_do_put_calls_authenticator_with_write() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = empty_request(DO_PUT_PATH); + + let result = authorize(request, &*authenticator, &None).await; + + assert!(result.is_ok()); + assert_eq!(authenticator.calls(), vec![Permission::Write]); + } + + #[tokio::test] + async fn test_authorize_do_action_calls_authenticator_with_admin() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = empty_request(DO_ACTION_PATH); + + let result = authorize(request, &*authenticator, &None).await; + + assert!(result.is_ok()); + assert_eq!(authenticator.calls(), vec![Permission::Admin]); + } + #[tokio::test] async fn test_authorize_unknown_path() { let authenticator = Arc::new(MockAuthenticator::new()); From e4db166d936423442344468035d1b6ab364111c2 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 28 May 2026 07:05:36 +0200 Subject: [PATCH 28/59] Add test util method to create do get request --- .../modelardb_server/src/remote/auth_layer.rs | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index 3c2a64e9..85a4b1ed 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -193,7 +193,8 @@ async fn authorize_do_get( .map_err(|_| Status::invalid_argument("Failed to unpack request body."))? .to_bytes(); - // gRPC has a 1-byte compression flag, a 4-byte length, and an N bytes protobuf message. + // gRPC data frames have a 1-byte compression flag, a 4-byte length, and an N bytes message as + // defined in https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md. if bytes.len() < 5 { return Err(Status::invalid_argument( "Request body too short to be a valid gRPC message.", @@ -352,6 +353,25 @@ mod tests { assert_eq!(authenticator.calls(), vec![Permission::Admin]); } + fn do_get_request(sql: &str) -> Request { + let ticket = Ticket { + ticket: sql.as_bytes().to_vec().into(), + }; + let ticket_bytes = ticket.encode_to_vec(); + + // Construct a gRPC frame with the 1-byte compression flag, 4-byte message length, and message. + let mut frame = Vec::with_capacity(5 + ticket_bytes.len()); + + frame.push(0u8); + frame.extend_from_slice(&(ticket_bytes.len() as u32).to_be_bytes()); + frame.extend_from_slice(&ticket_bytes); + + Request::builder() + .uri(DO_GET_PATH) + .body(Body::new(Full::new(bytes::Bytes::from(frame)))) + .unwrap() + } + #[tokio::test] async fn test_authorize_unknown_path() { let authenticator = Arc::new(MockAuthenticator::new()); From 6ac3e8de60f1696ab3ec7d386ba5aefa8424045d Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 28 May 2026 07:26:26 +0200 Subject: [PATCH 29/59] Add permission mapping tests for do_get --- .../modelardb_server/src/remote/auth_layer.rs | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index 85a4b1ed..56e3d878 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -235,6 +235,7 @@ mod tests { use super::*; use modelardb_auth::authenticator::mock::MockAuthenticator; + use modelardb_test::table::{NORMAL_TABLE_SQL, TIME_SERIES_TABLE_SQL}; const CLUSTER_KEY: &str = "cluster_key"; @@ -353,6 +354,105 @@ mod tests { assert_eq!(authenticator.calls(), vec![Permission::Admin]); } + #[tokio::test] + async fn test_authorize_do_get_with_create_normal_table_calls_authenticator_with_admin() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = do_get_request(NORMAL_TABLE_SQL); + + let result = authorize(request, &*authenticator, &None).await; + + assert!(result.is_ok()); + assert_eq!(authenticator.calls(), vec![Permission::Admin]); + } + + #[tokio::test] + async fn test_authorize_do_get_with_create_time_series_table_calls_authenticator_with_admin() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = do_get_request(TIME_SERIES_TABLE_SQL); + + let result = authorize(request, &*authenticator, &None).await; + + assert!(result.is_ok()); + assert_eq!(authenticator.calls(), vec![Permission::Admin]); + } + + #[tokio::test] + async fn test_authorize_do_get_with_drop_table_calls_authenticator_with_admin() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = do_get_request("DROP TABLE test_table"); + + let result = authorize(request, &*authenticator, &None).await; + + assert!(result.is_ok()); + assert_eq!(authenticator.calls(), vec![Permission::Admin]); + } + + #[tokio::test] + async fn test_authorize_do_get_with_truncate_table_calls_authenticator_with_admin() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = do_get_request("TRUNCATE test_table"); + + let result = authorize(request, &*authenticator, &None).await; + + assert!(result.is_ok()); + assert_eq!(authenticator.calls(), vec![Permission::Admin]); + } + + #[tokio::test] + async fn test_authorize_do_get_with_vacuum_calls_authenticator_with_admin() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = do_get_request("VACUUM test_table"); + + let result = authorize(request, &*authenticator, &None).await; + + assert!(result.is_ok()); + assert_eq!(authenticator.calls(), vec![Permission::Admin]); + } + + #[tokio::test] + async fn test_authorize_do_get_with_include_select_calls_authenticator_with_read() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = do_get_request("INCLUDE 'grpc://192.168.1.2:9999' SELECT * FROM test_table"); + + let result = authorize(request, &*authenticator, &None).await; + + assert!(result.is_ok()); + assert_eq!(authenticator.calls(), vec![Permission::Read]); + } + + #[tokio::test] + async fn test_authorize_do_get_with_insert_calls_authenticator_with_write() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = do_get_request("INSERT INTO test_table VALUES (1, 2, 3)"); + + let result = authorize(request, &*authenticator, &None).await; + + assert!(result.is_ok()); + assert_eq!(authenticator.calls(), vec![Permission::Write]); + } + + #[tokio::test] + async fn test_authorize_do_get_with_select_calls_authenticator_with_read() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = do_get_request("SELECT * FROM test_table"); + + let result = authorize(request, &*authenticator, &None).await; + + assert!(result.is_ok()); + assert_eq!(authenticator.calls(), vec![Permission::Read]); + } + + #[tokio::test] + async fn test_authorize_do_get_with_explain_calls_authenticator_with_read() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = do_get_request("EXPLAIN SELECT * FROM test_table"); + + let result = authorize(request, &*authenticator, &None).await; + + assert!(result.is_ok()); + assert_eq!(authenticator.calls(), vec![Permission::Read]); + } + fn do_get_request(sql: &str) -> Request { let ticket = Ticket { ticket: sql.as_bytes().to_vec().into(), From 0d9f565d138e4b15787afe4c8207b06ade5ddb01 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 28 May 2026 07:31:20 +0200 Subject: [PATCH 30/59] Add test to ensure that do get reconstructs the body correctly --- .../modelardb_server/src/remote/auth_layer.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index 56e3d878..88d0dbae 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -453,6 +453,25 @@ mod tests { assert_eq!(authenticator.calls(), vec![Permission::Read]); } + #[tokio::test] + async fn test_authorize_do_get_body_is_reconstructed_intact() { + let authenticator = Arc::new(MockAuthenticator::new()); + let sql = "SELECT * FROM test_table"; + + // Capture the original body bytes before calling authorize(). + let original_request = do_get_request(sql); + let (_, original_body) = original_request.into_parts(); + let original_bytes = original_body.collect().await.unwrap().to_bytes(); + + let request = do_get_request(sql); + let result = authorize(request, &*authenticator, &None).await; + + let (_, reconstructed_body) = result.unwrap().into_parts(); + let reconstructed_bytes = reconstructed_body.collect().await.unwrap().to_bytes(); + + assert_eq!(original_bytes, reconstructed_bytes); + } + fn do_get_request(sql: &str) -> Request { let ticket = Ticket { ticket: sql.as_bytes().to_vec().into(), From b61afb9334a8ba53354387efba92ff11b31c0ab2 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 28 May 2026 07:38:14 +0200 Subject: [PATCH 31/59] Add test to ensure we handle do get request with too short body --- .../modelardb_server/src/remote/auth_layer.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index 88d0dbae..e4f9d63a 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -491,6 +491,23 @@ mod tests { .unwrap() } + #[tokio::test] + async fn test_authorize_do_get_with_body_too_short() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = Request::builder() + .uri(DO_GET_PATH) + .body(Body::new(Full::new(bytes::Bytes::from(vec![0u8; 4])))) + .unwrap(); + + let result = authorize(request, &*authenticator, &None).await; + + assert_eq!( + result.unwrap_err().to_string(), + "code: 'Client specified an invalid argument', \ + message: \"Request body too short to be a valid gRPC message.\"" + ); + } + #[tokio::test] async fn test_authorize_unknown_path() { let authenticator = Arc::new(MockAuthenticator::new()); From 0d5469f4fa456114a373ed1dab7fbefe97461422 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 28 May 2026 07:39:05 +0200 Subject: [PATCH 32/59] Add test to ensure we handle do get request with invalid protobuf message --- .../modelardb_server/src/remote/auth_layer.rs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index e4f9d63a..54ae4522 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -508,6 +508,29 @@ mod tests { ); } + #[tokio::test] + async fn test_authorize_do_get_with_invalid_protobuf_is_rejected() { + let authenticator = Arc::new(MockAuthenticator::new()); + + // Valid 5-byte gRPC frame header but invalid protobuf bytes in the message. + let mut frame = vec![0u8; 9]; + frame[1..5].copy_from_slice(&4u32.to_be_bytes()); + frame[5..].copy_from_slice(&[0xFF, 0xFF, 0xFF, 0xFF]); + + let request = Request::builder() + .uri(DO_GET_PATH) + .body(Body::new(Full::new(bytes::Bytes::from(frame)))) + .unwrap(); + + let result = authorize(request, &*authenticator, &None).await; + + assert_eq!( + result.unwrap_err().to_string(), + "code: 'Client specified an invalid argument', \ + message: \"failed to decode Protobuf message: invalid varint\"" + ); + } + #[tokio::test] async fn test_authorize_unknown_path() { let authenticator = Arc::new(MockAuthenticator::new()); From bcbfd7b3a25894a67f1b7671e2ca8c5dfc63c7e5 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 28 May 2026 07:42:50 +0200 Subject: [PATCH 33/59] Add test to ensure authorize fails when parsing invalid sql --- crates/modelardb_server/src/remote/auth_layer.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index 54ae4522..f8c5959e 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -509,7 +509,7 @@ mod tests { } #[tokio::test] - async fn test_authorize_do_get_with_invalid_protobuf_is_rejected() { + async fn test_authorize_do_get_with_invalid_protobuf() { let authenticator = Arc::new(MockAuthenticator::new()); // Valid 5-byte gRPC frame header but invalid protobuf bytes in the message. @@ -530,6 +530,20 @@ mod tests { message: \"failed to decode Protobuf message: invalid varint\"" ); } + + #[tokio::test] + async fn test_authorize_do_get_with_unparseable_sql() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = do_get_request("invalid sql"); + + let result = authorize(request, &*authenticator, &None).await; + + assert_eq!( + result.unwrap_err().to_string(), + "code: 'Client specified an invalid argument', \ + message: \"Parser Error: sql parser error: Expected: an SQL statement, found: invalid at Line: 1, Column: 1\"" + ); + } #[tokio::test] async fn test_authorize_unknown_path() { From 91538937c349d38997c9e192a113ded7f1709a1d Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 28 May 2026 07:45:16 +0200 Subject: [PATCH 34/59] Add test to ensure authorize fails when parsing non utf8 tickets --- .../modelardb_server/src/remote/auth_layer.rs | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index f8c5959e..08d3a87c 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -530,7 +530,36 @@ mod tests { message: \"failed to decode Protobuf message: invalid varint\"" ); } - + + #[tokio::test] + async fn test_authorize_do_get_with_non_utf8_ticket() { + let authenticator = Arc::new(MockAuthenticator::new()); + + // Encode a Ticket with invalid UTF-8 bytes. + let ticket = Ticket { + ticket: vec![0xFF, 0xFE].into(), + }; + let ticket_bytes = ticket.encode_to_vec(); + + let mut frame = Vec::with_capacity(5 + ticket_bytes.len()); + frame.push(0u8); + frame.extend_from_slice(&(ticket_bytes.len() as u32).to_be_bytes()); + frame.extend_from_slice(&ticket_bytes); + + let request = Request::builder() + .uri(DO_GET_PATH) + .body(Body::new(Full::new(bytes::Bytes::from(frame)))) + .unwrap(); + + let result = authorize(request, &*authenticator, &None).await; + + assert_eq!( + result.unwrap_err().to_string(), + "code: 'Client specified an invalid argument', \ + message: \"invalid utf-8 sequence of 1 bytes from index 0\"" + ); + } + #[tokio::test] async fn test_authorize_do_get_with_unparseable_sql() { let authenticator = Arc::new(MockAuthenticator::new()); From 15ce3401852f54fc7c2163014ee2a785d5520425 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 28 May 2026 07:58:38 +0200 Subject: [PATCH 35/59] Remove duplicated test code with test util functions --- .../modelardb_server/src/remote/auth_layer.rs | 68 ++++++++----------- 1 file changed, 27 insertions(+), 41 deletions(-) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index 08d3a87c..ea632562 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -472,25 +472,6 @@ mod tests { assert_eq!(original_bytes, reconstructed_bytes); } - fn do_get_request(sql: &str) -> Request { - let ticket = Ticket { - ticket: sql.as_bytes().to_vec().into(), - }; - let ticket_bytes = ticket.encode_to_vec(); - - // Construct a gRPC frame with the 1-byte compression flag, 4-byte message length, and message. - let mut frame = Vec::with_capacity(5 + ticket_bytes.len()); - - frame.push(0u8); - frame.extend_from_slice(&(ticket_bytes.len() as u32).to_be_bytes()); - frame.extend_from_slice(&ticket_bytes); - - Request::builder() - .uri(DO_GET_PATH) - .body(Body::new(Full::new(bytes::Bytes::from(frame)))) - .unwrap() - } - #[tokio::test] async fn test_authorize_do_get_with_body_too_short() { let authenticator = Arc::new(MockAuthenticator::new()); @@ -513,14 +494,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); // Valid 5-byte gRPC frame header but invalid protobuf bytes in the message. - let mut frame = vec![0u8; 9]; - frame[1..5].copy_from_slice(&4u32.to_be_bytes()); - frame[5..].copy_from_slice(&[0xFF, 0xFF, 0xFF, 0xFF]); - - let request = Request::builder() - .uri(DO_GET_PATH) - .body(Body::new(Full::new(bytes::Bytes::from(frame)))) - .unwrap(); + let request = raw_frame_request(&[0xFF, 0xFF, 0xFF, 0xFF]); let result = authorize(request, &*authenticator, &None).await; @@ -536,20 +510,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); // Encode a Ticket with invalid UTF-8 bytes. - let ticket = Ticket { - ticket: vec![0xFF, 0xFE].into(), - }; - let ticket_bytes = ticket.encode_to_vec(); - - let mut frame = Vec::with_capacity(5 + ticket_bytes.len()); - frame.push(0u8); - frame.extend_from_slice(&(ticket_bytes.len() as u32).to_be_bytes()); - frame.extend_from_slice(&ticket_bytes); - - let request = Request::builder() - .uri(DO_GET_PATH) - .body(Body::new(Full::new(bytes::Bytes::from(frame)))) - .unwrap(); + let request = ticket_frame_request(vec![0xFF, 0xFE]); let result = authorize(request, &*authenticator, &None).await; @@ -574,6 +535,31 @@ mod tests { ); } + fn do_get_request(sql: &str) -> Request { + ticket_frame_request(sql.as_bytes().to_vec()) + } + + fn ticket_frame_request(ticket_bytes: Vec) -> Request { + let ticket = Ticket { + ticket: ticket_bytes.into(), + }; + + raw_frame_request(&ticket.encode_to_vec()) + } + + fn raw_frame_request(message_bytes: &[u8]) -> Request { + // Construct a gRPC frame with the 1-byte compression flag, 4-byte message length, and message. + let mut frame = Vec::with_capacity(5 + message_bytes.len()); + frame.push(0u8); + frame.extend_from_slice(&(message_bytes.len() as u32).to_be_bytes()); + frame.extend_from_slice(message_bytes); + + Request::builder() + .uri(DO_GET_PATH) + .body(Body::new(Full::new(bytes::Bytes::from(frame)))) + .unwrap() + } + #[tokio::test] async fn test_authorize_unknown_path() { let authenticator = Arc::new(MockAuthenticator::new()); From e090e742fc989b0da65b260a8ed94c1f0f7692e6 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 28 May 2026 10:21:23 +0200 Subject: [PATCH 36/59] Fix clippy warning for missing default --- crates/modelardb_auth/src/authenticator/mock.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/modelardb_auth/src/authenticator/mock.rs b/crates/modelardb_auth/src/authenticator/mock.rs index 3a2521d0..91bca601 100644 --- a/crates/modelardb_auth/src/authenticator/mock.rs +++ b/crates/modelardb_auth/src/authenticator/mock.rs @@ -46,6 +46,12 @@ impl MockAuthenticator { } } +impl Default for MockAuthenticator { + fn default() -> Self { + Self::new() + } +} + impl Authenticator for MockAuthenticator { fn authorize( &self, From 8c8e5e2a357ecd45ef77fbaa0b3b6b1d4247d088 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Fri, 29 May 2026 11:04:42 +0200 Subject: [PATCH 37/59] Remove unused dependency --- Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index d9ef63f3..95159f5c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,6 @@ resolver = "3" arrow = "58.0.0" arrow-flight = "58.0.0" async-trait = "0.1.89" -axum = "0.8.9" bytes = "1.11.1" chrono = "0.4.44" crossbeam-channel = "0.5.15" From f70d87c710e59f83ee9ea88a7110cceba9414163 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Mon, 1 Jun 2026 07:20:52 +0200 Subject: [PATCH 38/59] Add BearerInterceptor for attaching bearer tokens to outgoing requests --- .../src/operations/client.rs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/crates/modelardb_embedded/src/operations/client.rs b/crates/modelardb_embedded/src/operations/client.rs index 6fc70d26..10439fdd 100644 --- a/crates/modelardb_embedded/src/operations/client.rs +++ b/crates/modelardb_embedded/src/operations/client.rs @@ -18,6 +18,7 @@ use std::any::Any; use std::collections::HashMap; use std::pin::Pin; +use std::result::Result as StdResult; use std::str; use std::str::FromStr; use std::sync::Arc; @@ -34,6 +35,7 @@ use datafusion::error::DataFusionError; use datafusion::execution::RecordBatchStream; use datafusion::physical_plan::stream::RecordBatchStreamAdapter; use futures::{StreamExt, TryStreamExt, stream}; +use tonic::metadata::AsciiMetadataValue; use tonic::transport::{Channel, Endpoint}; use tonic::{Request, Status}; @@ -44,6 +46,26 @@ use crate::operations::{ }; use crate::{Aggregate, TableType}; +/// Tonic interceptor that attaches a bearer token to every outgoing request when one is present. +#[derive(Clone)] +struct BearerInterceptor { + /// The value of the `authorization` header to attach. This is either `Bearer ` or + /// [`None`] if no token has been provided. + maybe_authorization: Option, +} + +impl tonic::service::Interceptor for BearerInterceptor { + fn call(&mut self, mut request: Request<()>) -> StdResult, Status> { + if let Some(authorization) = &self.maybe_authorization { + request + .metadata_mut() + .insert("authorization", authorization.clone()); + } + + Ok(request) + } +} + /// Client for connecting to ModelarDB Apache Arrow Flight servers. #[derive(Clone)] pub struct Client { From 99f7c31eb7efc7c9907cfa134264df9e1594fb4a Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Mon, 1 Jun 2026 07:26:43 +0200 Subject: [PATCH 39/59] Use BearerInterceptor when connecting to embedded Client --- .../src/operations/client.rs | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/crates/modelardb_embedded/src/operations/client.rs b/crates/modelardb_embedded/src/operations/client.rs index 10439fdd..455c92f3 100644 --- a/crates/modelardb_embedded/src/operations/client.rs +++ b/crates/modelardb_embedded/src/operations/client.rs @@ -35,6 +35,7 @@ use datafusion::error::DataFusionError; use datafusion::execution::RecordBatchStream; use datafusion::physical_plan::stream::RecordBatchStreamAdapter; use futures::{StreamExt, TryStreamExt, stream}; +use tonic::codegen::InterceptedService; use tonic::metadata::AsciiMetadataValue; use tonic::transport::{Channel, Endpoint}; use tonic::{Request, Status}; @@ -70,15 +71,28 @@ impl tonic::service::Interceptor for BearerInterceptor { #[derive(Clone)] pub struct Client { /// Apache Arrow Flight client connected to the Apache Arrow Flight server of the ModelarDB node. - pub(crate) flight_client: FlightServiceClient, + pub(crate) flight_client: FlightServiceClient>, } impl Client { - /// Create a new [`Client`] that is connected to the node with `url`. If a connection - /// to the node could not be established, [`ModelarDbEmbeddedError`] is returned. - pub async fn connect(url: &str) -> Result { + /// Create a new [`Client`] that is connected to the node with `url`. If `maybe_token` is + /// provided, it is attached as a bearer token to every request. If a connection to the node + /// could not be established, [`ModelarDbEmbeddedError`] is returned. + pub async fn connect(url: &str, maybe_token: Option<&str>) -> Result { + let maybe_authorization = maybe_token + .map(|token| { + format!("Bearer {token}") + .parse::() + .map_err(|error| ModelarDbEmbeddedError::InvalidArgument(error.to_string())) + }) + .transpose()?; + + let interceptor = BearerInterceptor { + maybe_authorization, + }; + let connection = Endpoint::new(url.to_owned())?.connect().await?; - let flight_client = FlightServiceClient::new(connection); + let flight_client = FlightServiceClient::with_interceptor(connection, interceptor); Ok(Client { flight_client }) } From 20f471de4b587492380f88972a603b6d2b516b4d Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Mon, 1 Jun 2026 07:30:55 +0200 Subject: [PATCH 40/59] Make it possible to still initialize lazy client --- crates/modelardb_embedded/src/operations/client.rs | 4 ++-- crates/modelardb_embedded/src/operations/data_folder.rs | 9 +++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/modelardb_embedded/src/operations/client.rs b/crates/modelardb_embedded/src/operations/client.rs index 455c92f3..c40b87c6 100644 --- a/crates/modelardb_embedded/src/operations/client.rs +++ b/crates/modelardb_embedded/src/operations/client.rs @@ -49,10 +49,10 @@ use crate::{Aggregate, TableType}; /// Tonic interceptor that attaches a bearer token to every outgoing request when one is present. #[derive(Clone)] -struct BearerInterceptor { +pub(super) struct BearerInterceptor { /// The value of the `authorization` header to attach. This is either `Bearer ` or /// [`None`] if no token has been provided. - maybe_authorization: Option, + pub(super) maybe_authorization: Option, } impl tonic::service::Interceptor for BearerInterceptor { diff --git a/crates/modelardb_embedded/src/operations/data_folder.rs b/crates/modelardb_embedded/src/operations/data_folder.rs index e77cf571..a16ba7f9 100644 --- a/crates/modelardb_embedded/src/operations/data_folder.rs +++ b/crates/modelardb_embedded/src/operations/data_folder.rs @@ -590,7 +590,7 @@ mod tests { use tempfile::TempDir; use tonic::transport::Channel; - use crate::operations::client::Client; + use crate::operations::client::{BearerInterceptor, Client}; use crate::record_batch_stream_to_record_batch; const NORMAL_TABLE_NAME: &str = "normal_table"; @@ -2744,9 +2744,14 @@ mod tests { } fn lazy_modelardb_client() -> Client { + let interceptor = BearerInterceptor { + maybe_authorization: None, + }; + Client { - flight_client: FlightServiceClient::new( + flight_client: FlightServiceClient::with_interceptor( Channel::from_static("localhost").connect_lazy(), + interceptor, ), } } From 1df0d02c394581b2e53e9dae3d74395abec88806 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Mon, 1 Jun 2026 07:45:26 +0200 Subject: [PATCH 41/59] Add maybe_token_ptr parameter to C-API --- crates/modelardb_embedded/src/capi.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/modelardb_embedded/src/capi.rs b/crates/modelardb_embedded/src/capi.rs index 012d9142..41c0ccec 100644 --- a/crates/modelardb_embedded/src/capi.rs +++ b/crates/modelardb_embedded/src/capi.rs @@ -199,18 +199,23 @@ unsafe fn open_azure( /// Creates a [`Client`] that is connected to the Apache Arrow Flight server URL in `url_ptr` /// and returns a pointer to the [`Client`] or a zero-initialized pointer if an error occurs. -/// Assumes `url_ptr` points to a valid C string. +/// Assumes `url_ptr` points to a valid C string and `maybe_token_ptr` points to a valid C string +/// or is null if no bearer token should be attached to requests. #[unsafe(no_mangle)] -pub unsafe extern "C" fn modelardb_embedded_connect(url_ptr: *const c_char) -> *const c_void { - let maybe_client = unsafe { connect(url_ptr) }; +pub unsafe extern "C" fn modelardb_embedded_connect( + url_ptr: *const c_char, + maybe_token_ptr: *const c_char, +) -> *const c_void { + let maybe_client = unsafe { connect(url_ptr, maybe_token_ptr) }; set_error_and_return_value_ptr(maybe_client) } /// See documentation for [`modelardb_embedded_connect()`]. -unsafe fn connect(url_ptr: *const c_char) -> Result { +unsafe fn connect(url_ptr: *const c_char, maybe_token_ptr: *const c_char) -> Result { let url_str = unsafe { c_char_ptr_to_str(url_ptr)? }; + let maybe_token = unsafe { c_char_ptr_to_maybe_str(maybe_token_ptr)? }; - TOKIO_RUNTIME.block_on(Client::connect(url_str)) + TOKIO_RUNTIME.block_on(Client::connect(url_str, maybe_token)) } /// Moves the value in `maybe_value` to a [`Box`] and returns a pointer to it if `maybe_value` is From aa5b6f27054c3c928f011ce584a0fcce56d40924 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Mon, 1 Jun 2026 07:48:17 +0200 Subject: [PATCH 42/59] Update C header to match C-API change --- crates/modelardb_embedded/bindings/c/modelardb_embedded.h | 2 +- crates/modelardb_embedded/src/operations/client.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/modelardb_embedded/bindings/c/modelardb_embedded.h b/crates/modelardb_embedded/bindings/c/modelardb_embedded.h index e4c40b4d..8d422bc8 100644 --- a/crates/modelardb_embedded/bindings/c/modelardb_embedded.h +++ b/crates/modelardb_embedded/bindings/c/modelardb_embedded.h @@ -95,7 +95,7 @@ void* modelardb_embedded_open_azure(const char* account_name_ptr, const char* container_name_ptr); // Connect to a ModelarDB server at the given Arrow Flight URL. -void* modelardb_embedded_connect(const char* url_ptr); +void* modelardb_embedded_connect(const char* url_ptr, const char* maybe_token_ptr); // Close and deallocate the data folder or client. int modelardb_embedded_close(void* maybe_operations_ptr, diff --git a/crates/modelardb_embedded/src/operations/client.rs b/crates/modelardb_embedded/src/operations/client.rs index c40b87c6..ab18672f 100644 --- a/crates/modelardb_embedded/src/operations/client.rs +++ b/crates/modelardb_embedded/src/operations/client.rs @@ -49,7 +49,7 @@ use crate::{Aggregate, TableType}; /// Tonic interceptor that attaches a bearer token to every outgoing request when one is present. #[derive(Clone)] -pub(super) struct BearerInterceptor { +pub(crate) struct BearerInterceptor { /// The value of the `authorization` header to attach. This is either `Bearer ` or /// [`None`] if no token has been provided. pub(super) maybe_authorization: Option, From 513f26d46c79a54b7e6893d94b8c018ff6b83f5b Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Mon, 1 Jun 2026 07:58:52 +0200 Subject: [PATCH 43/59] Add optional token parameter to Python API --- .../bindings/python/modelardb/operations.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/modelardb_embedded/bindings/python/modelardb/operations.py b/crates/modelardb_embedded/bindings/python/modelardb/operations.py index f62c3a7a..323e1fc5 100644 --- a/crates/modelardb_embedded/bindings/python/modelardb/operations.py +++ b/crates/modelardb_embedded/bindings/python/modelardb/operations.py @@ -221,17 +221,20 @@ def open_azure(cls, account_name: str, access_key: str, container_name: str): return self @classmethod - def connect(cls, url: str): + def connect(cls, url: str, token: None | str = None): """Create a connection to an :obj:`Operations` node. :param url: The URL of the ModelarDB node to connect to. :type url: str + :param token: A bearer token to attach to every request, or ``None`` if no authentication is required. + :type token: str, optional """ self: Operations = cls() url_ptr = ffi.new("char[]", bytes(url, "UTF-8")) + maybe_token_ptr = ffi.new("char[]", bytes(token, "UTF-8")) if token else ffi.NULL - self.__operations_ptr = self.__library.modelardb_embedded_connect(url_ptr) + self.__operations_ptr = self.__library.modelardb_embedded_connect(url_ptr, maybe_token_ptr) self.__is_data_folder = False if self.__operations_ptr == ffi.NULL: @@ -756,10 +759,12 @@ def open_azure(account_name: str, access_key: str, container_name: str) -> Opera return Operations.open_azure(account_name, access_key, container_name) -def connect(url: str) -> Operations: +def connect(url: str, token: None | str = None) -> Operations: """Create a connection to an :obj:`Operations` node. :param url: The URL of the ModelarDB node to connect to. :type url: str + :param token: A bearer token to attach to every request, or ``None`` if no authentication is required. + :type token: str, optional """ - return Operations.connect(url) + return Operations.connect(url, token) From c8276c7441b54ba7f41a691c56246fe5b6cbc228 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Mon, 1 Jun 2026 08:30:10 +0200 Subject: [PATCH 44/59] Move BearerInterceptor to modelardb_auth --- Cargo.lock | 1 + crates/modelardb_auth/src/lib.rs | 24 +++++++++++++++++++ crates/modelardb_embedded/Cargo.toml | 1 + .../src/operations/client.rs | 22 +---------------- .../src/operations/data_folder.rs | 3 ++- 5 files changed, 29 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 66149ad5..7b3be1eb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3364,6 +3364,7 @@ dependencies = [ "datafusion", "deltalake", "futures", + "modelardb_auth", "modelardb_compression", "modelardb_storage", "modelardb_types", diff --git a/crates/modelardb_auth/src/lib.rs b/crates/modelardb_auth/src/lib.rs index 14a306fb..0756da3d 100644 --- a/crates/modelardb_auth/src/lib.rs +++ b/crates/modelardb_auth/src/lib.rs @@ -15,6 +15,10 @@ //! Types to support authentication and authorization in ModelarDB. +use tonic::metadata::AsciiMetadataValue; +use tonic::service::Interceptor; +use tonic::{Request, Status}; + pub mod authenticator; /// The permission required to perform an operation in a ModelarDB server. @@ -38,6 +42,26 @@ impl Permission { } } +/// Tonic interceptor that attaches a bearer token to every outgoing request when one is present. +#[derive(Clone)] +pub struct BearerInterceptor { + /// The value of the `authorization` header to attach. This is either `Bearer ` or + /// [`None`] if no token has been provided. + pub maybe_authorization: Option, +} + +impl Interceptor for BearerInterceptor { + fn call(&mut self, mut request: Request<()>) -> Result, Status> { + if let Some(authorization) = &self.maybe_authorization { + request + .metadata_mut() + .insert("authorization", authorization.clone()); + } + + Ok(request) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/modelardb_embedded/Cargo.toml b/crates/modelardb_embedded/Cargo.toml index d3b89eb0..530030c4 100644 --- a/crates/modelardb_embedded/Cargo.toml +++ b/crates/modelardb_embedded/Cargo.toml @@ -31,6 +31,7 @@ async-trait.workspace = true datafusion.workspace = true deltalake = { workspace = true, features = ["datafusion", "s3"] } futures.workspace = true +modelardb_auth = { path = "../modelardb_auth" } modelardb_compression = { path = "../modelardb_compression" } modelardb_storage = { path = "../modelardb_storage" } modelardb_types = { path = "../modelardb_types" } diff --git a/crates/modelardb_embedded/src/operations/client.rs b/crates/modelardb_embedded/src/operations/client.rs index ab18672f..8e9c41af 100644 --- a/crates/modelardb_embedded/src/operations/client.rs +++ b/crates/modelardb_embedded/src/operations/client.rs @@ -18,7 +18,6 @@ use std::any::Any; use std::collections::HashMap; use std::pin::Pin; -use std::result::Result as StdResult; use std::str; use std::str::FromStr; use std::sync::Arc; @@ -35,6 +34,7 @@ use datafusion::error::DataFusionError; use datafusion::execution::RecordBatchStream; use datafusion::physical_plan::stream::RecordBatchStreamAdapter; use futures::{StreamExt, TryStreamExt, stream}; +use modelardb_auth::BearerInterceptor; use tonic::codegen::InterceptedService; use tonic::metadata::AsciiMetadataValue; use tonic::transport::{Channel, Endpoint}; @@ -47,26 +47,6 @@ use crate::operations::{ }; use crate::{Aggregate, TableType}; -/// Tonic interceptor that attaches a bearer token to every outgoing request when one is present. -#[derive(Clone)] -pub(crate) struct BearerInterceptor { - /// The value of the `authorization` header to attach. This is either `Bearer ` or - /// [`None`] if no token has been provided. - pub(super) maybe_authorization: Option, -} - -impl tonic::service::Interceptor for BearerInterceptor { - fn call(&mut self, mut request: Request<()>) -> StdResult, Status> { - if let Some(authorization) = &self.maybe_authorization { - request - .metadata_mut() - .insert("authorization", authorization.clone()); - } - - Ok(request) - } -} - /// Client for connecting to ModelarDB Apache Arrow Flight servers. #[derive(Clone)] pub struct Client { diff --git a/crates/modelardb_embedded/src/operations/data_folder.rs b/crates/modelardb_embedded/src/operations/data_folder.rs index a16ba7f9..8555aeef 100644 --- a/crates/modelardb_embedded/src/operations/data_folder.rs +++ b/crates/modelardb_embedded/src/operations/data_folder.rs @@ -583,6 +583,7 @@ mod tests { use datafusion::physical_expr::{LexOrdering, PhysicalSortExpr}; use datafusion::physical_plan::expressions::Column; use datafusion::physical_plan::sorts::sort; + use modelardb_auth::BearerInterceptor; use modelardb_types::types::{ ArrowTimestamp, ArrowValue, ErrorBound, GeneratedColumn, TimeSeriesTableMetadata, TimestampArray, ValueArray, @@ -590,7 +591,7 @@ mod tests { use tempfile::TempDir; use tonic::transport::Channel; - use crate::operations::client::{BearerInterceptor, Client}; + use crate::operations::client::Client; use crate::record_batch_stream_to_record_batch; const NORMAL_TABLE_NAME: &str = "normal_table"; From b0adbbb38e6803859279914a164a858b835a8d86 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Mon, 1 Jun 2026 10:07:10 +0200 Subject: [PATCH 45/59] Add token to client doc --- Cargo.lock | 7 +++++-- crates/modelardb_client/src/main.rs | 12 ++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7b3be1eb..dc7d7612 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3309,8 +3309,6 @@ dependencies = [ name = "modelardb_auth" version = "0.1.0" dependencies = [ - "modelardb_storage", - "tokio", "tonic", ] @@ -3386,7 +3384,10 @@ dependencies = [ "datafusion", "deltalake", "futures", + "http 1.4.2", + "http-body-util", "log", + "modelardb_auth", "modelardb_compression", "modelardb_storage", "modelardb_test", @@ -3397,11 +3398,13 @@ dependencies = [ "rand 0.10.1", "serde", "snmalloc-rs", + "sqlparser", "tempfile", "tokio", "tokio-stream", "toml", "tonic", + "tower", "tracing", "tracing-subscriber", ] diff --git a/crates/modelardb_client/src/main.rs b/crates/modelardb_client/src/main.rs index 440bd543..ff1e607d 100644 --- a/crates/modelardb_client/src/main.rs +++ b/crates/modelardb_client/src/main.rs @@ -53,12 +53,12 @@ const DEFAULT_PORT: u16 = 9999; const TRANSPORT_ERROR: &str = "transport error: no messages received."; /// Parse the command line arguments to extract the host running the server to connect to, the -/// server port to connect to, and the file containing the queries to execute on the server. If the -/// server host is not provided it defaults to [`DEFAULT_HOST`], if the server port is not provided -/// it defaults to [`DEFAULT_PORT`], and if the file containing queries is not provided a -/// read-eval-print loop is opened. Returns [`ModelarDbClientError`] if the command line arguments -/// cannot be parsed, the client cannot connect to the server, or the file containing the queries -/// cannot be read. +/// server port to connect to, the file containing the queries to execute on the server, and an +/// optional bearer token for authentication. If the server host is not provided, it defaults to +/// [`DEFAULT_HOST`], if the server port is not provided, it defaults to [`DEFAULT_PORT`], and if +/// the file containing queries is not provided, a read-eval-print loop is opened. Returns +/// [`ModelarDbClientError`] if the command line arguments cannot be parsed, the client cannot +/// connect to the server, or the file containing the queries cannot be read. #[tokio::main(flavor = "current_thread")] async fn main() -> Result<()> { // Parse the command line arguments. From 1e68daf1f3ebea90c5ddb5037c9f47b0da530349 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Mon, 1 Jun 2026 10:21:45 +0200 Subject: [PATCH 46/59] Add very simple arg parsing for --token parameter --- crates/modelardb_client/src/main.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/crates/modelardb_client/src/main.rs b/crates/modelardb_client/src/main.rs index ff1e607d..e3695604 100644 --- a/crates/modelardb_client/src/main.rs +++ b/crates/modelardb_client/src/main.rs @@ -61,9 +61,18 @@ const TRANSPORT_ERROR: &str = "transport error: no messages received."; /// connect to the server, or the file containing the queries cannot be read. #[tokio::main(flavor = "current_thread")] async fn main() -> Result<()> { - // Parse the command line arguments. - let args = env::args().collect::>(); - let (host, port, maybe_query_file) = match &args[1..] { + // Extract optional --token from the arguments before positional matching. + let mut args = env::args().skip(1).collect::>(); + let maybe_token = if let Some(pos) = args.iter().position(|arg| arg == "--token") { + let token = args.remove(pos + 1); + args.remove(pos); + Some(token) + } else { + None + }; + + // Parse the remaining command line arguments. + let (host, port, maybe_query_file) = match args.as_slice() { [] => (DEFAULT_HOST, DEFAULT_PORT, None), [query_file] if StdPath::new(&query_file).exists() => { let query_file = StdPath::new(&query_file).to_path_buf(); @@ -84,7 +93,7 @@ async fn main() -> Result<()> { let binary_name = binary_path.file_name().unwrap().to_str().unwrap(); // Punctuation at the end does not seem to be common in the usage message of Unix tools. - eprintln!("Usage: {binary_name} [host or host:port] [query_file]",); + eprintln!("Usage: {binary_name} [--token token] [host or host:port] [query_file]"); process::exit(1); } }; From 45492615a3f93328b2e521575257d75d759831ac Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Mon, 1 Jun 2026 10:31:57 +0200 Subject: [PATCH 47/59] Connect with optional bearer token in client --- Cargo.lock | 1 + crates/modelardb_client/Cargo.toml | 1 + crates/modelardb_client/src/main.rs | 52 ++++++++++++++++++++++------- 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dc7d7612..fe5a333a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3337,6 +3337,7 @@ dependencies = [ "arrow-flight", "bytes", "dirs", + "modelardb_auth", "rustyline", "tokio", "tonic", diff --git a/crates/modelardb_client/Cargo.toml b/crates/modelardb_client/Cargo.toml index 1e76b3d2..57c3b0b1 100644 --- a/crates/modelardb_client/Cargo.toml +++ b/crates/modelardb_client/Cargo.toml @@ -28,6 +28,7 @@ arrow-flight.workspace = true arrow = { workspace = true, features = ["prettyprint"] } bytes.workspace = true dirs.workspace = true +modelardb_auth = { path = "../modelardb_auth" } rustyline.workspace = true tokio.workspace = true tonic.workspace = true diff --git a/crates/modelardb_client/src/main.rs b/crates/modelardb_client/src/main.rs index e3695604..e2725275 100644 --- a/crates/modelardb_client/src/main.rs +++ b/crates/modelardb_client/src/main.rs @@ -35,8 +35,11 @@ use arrow::util::pretty; use arrow_flight::flight_service_client::FlightServiceClient; use arrow_flight::{Action, Criteria, FlightData, FlightDescriptor, Ticket, utils}; use bytes::Bytes; +use modelardb_auth::BearerInterceptor; use rustyline::Editor; use rustyline::history::FileHistory; +use tonic::codegen::InterceptedService; +use tonic::metadata::AsciiMetadataValue; use tonic::transport::{Channel, Endpoint}; use tonic::{Request, Streaming}; @@ -52,6 +55,10 @@ const DEFAULT_PORT: u16 = 9999; /// Error to emit when the server does not provide a response when one is expected. const TRANSPORT_ERROR: &str = "transport error: no messages received."; +/// [`FlightServiceClient`] with a [`BearerInterceptor`] that attaches an authorization header. +type AuthenticatedFlightClient = + FlightServiceClient>; + /// Parse the command line arguments to extract the host running the server to connect to, the /// server port to connect to, the file containing the queries to execute on the server, and an /// optional bearer token for authentication. If the server host is not provided, it defaults to @@ -99,7 +106,7 @@ async fn main() -> Result<()> { }; // Execute the queries. - let flight_service_client = connect(host, port).await?; + let flight_service_client = connect(host, port, maybe_token).await?; if let Some(query_file) = maybe_query_file { execute_queries_from_a_file(flight_service_client, &query_file).await } else { @@ -124,17 +131,38 @@ fn parse_host_port(maybe_host_port: &str) -> Result<(&str, u16)> { Ok((host, port)) } -/// Connect to the server at `host`:`port`. Returns [`ModelarDbClientError`] if a connection to the -/// server cannot be established. -async fn connect(host: &str, port: u16) -> Result> { +/// Connect to the server at `host`:`port` with an optional bearer `maybe_token`. Returns +/// [`ModelarDbClientError`] if a connection to the server cannot be established or the token is +/// not a valid ASCII metadata value. +async fn connect( + host: &str, + port: u16, + maybe_token: Option, +) -> Result { + let maybe_authorization = maybe_token + .map(|token| { + format!("Bearer {token}") + .parse::() + .map_err(|error| ModelarDbClientError::InvalidArgument(error.to_string())) + }) + .transpose()?; + + let interceptor = BearerInterceptor { + maybe_authorization, + }; + let address = format!("grpc://{host}:{port}"); let connection = Endpoint::new(address)?.connect().await?; - Ok(FlightServiceClient::new(connection)) + + Ok(FlightServiceClient::with_interceptor( + connection, + interceptor, + )) } /// Execute the commands and queries in `query_file`. async fn execute_queries_from_a_file( - mut flight_service_client: FlightServiceClient, + mut flight_service_client: AuthenticatedFlightClient, query_file: &StdPath, ) -> Result<()> { let file = File::open(query_file)?; @@ -161,7 +189,7 @@ async fn execute_queries_from_a_file( /// Execute commands and queries in a read-eval-print loop. async fn execute_queries_from_a_repl( - mut flight_service_client: FlightServiceClient, + mut flight_service_client: AuthenticatedFlightClient, ) -> Result<()> { // Create the read-eval-print loop. let mut editor = Editor::::new()?; @@ -196,7 +224,7 @@ async fn execute_queries_from_a_repl( /// Execute a command or a query. Returns [`ModelarDbClientError`] if the command or query could not /// be executed or their result could not be retrieved. async fn execute_and_print_command_or_query( - flight_service_client: &mut FlightServiceClient, + flight_service_client: &mut AuthenticatedFlightClient, command_or_query: &str, ) { let start_time = Instant::now(); @@ -220,7 +248,7 @@ async fn execute_and_print_command_or_query( /// * The command could not be executed. /// * The result could not be retrieved. async fn execute_command( - flight_service_client: &mut FlightServiceClient, + flight_service_client: &mut AuthenticatedFlightClient, command_and_argument: &str, ) -> Result<()> { let mut command_and_argument = command_and_argument.split(' '); @@ -293,7 +321,7 @@ async fn execute_command( /// Retrieve the names of the tables available on the server. Returns [`ModelarDbClientError`] if /// the request could not be performed or the tables names could not be retrieved. async fn retrieve_table_names( - flight_service_client: &mut FlightServiceClient, + flight_service_client: &mut AuthenticatedFlightClient, ) -> Result> { let criteria = Criteria { expression: Bytes::new(), @@ -324,7 +352,7 @@ async fn retrieve_table_names( /// Execute an action. Returns [`ModelarDbClientError`] if the action could not be executed. async fn execute_action( - flight_service_client: &mut FlightServiceClient, + flight_service_client: &mut AuthenticatedFlightClient, action_type: &str, action_body: &str, ) -> Result<()> { @@ -348,7 +376,7 @@ async fn execute_action( /// Execute a query and print each batch in the result set. Returns [`ModelarDbClientError`] if the /// query could not be executed or the batches in the result set could not be printed. async fn execute_query_and_print_result( - flight_service_client: &mut FlightServiceClient, + flight_service_client: &mut AuthenticatedFlightClient, query: &str, ) -> Result<()> { // Execute the query. From 46544beeebda62e69fc3fcf9005d65e6f0ff998b Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Mon, 1 Jun 2026 10:40:41 +0200 Subject: [PATCH 48/59] Move parsing of token into AsciiMetadataValue to BearerInterceptor::try_new --- crates/modelardb_auth/src/lib.rs | 19 +++++++++++++++++++ crates/modelardb_client/src/main.rs | 13 +------------ .../src/operations/client.rs | 13 +------------ 3 files changed, 21 insertions(+), 24 deletions(-) diff --git a/crates/modelardb_auth/src/lib.rs b/crates/modelardb_auth/src/lib.rs index 0756da3d..649ccdaf 100644 --- a/crates/modelardb_auth/src/lib.rs +++ b/crates/modelardb_auth/src/lib.rs @@ -50,6 +50,25 @@ pub struct BearerInterceptor { pub maybe_authorization: Option, } +impl BearerInterceptor { + /// Create a new [`BearerInterceptor`] that attaches the bearer token in `maybe_token` to every + /// outgoing request. If `maybe_token` is [`None`], no token is attached. If `maybe_token` is + /// [`Some`] but the token is an invalid ASCII string, [`Status`] is returned. + pub fn try_new(maybe_token: Option<&str>) -> Result { + let maybe_authorization = maybe_token + .map(|token| { + format!("Bearer {token}") + .parse::() + .map_err(|error| Status::invalid_argument(format!("Invalid token: {error}."))) + }) + .transpose()?; + + Ok(Self { + maybe_authorization, + }) + } +} + impl Interceptor for BearerInterceptor { fn call(&mut self, mut request: Request<()>) -> Result, Status> { if let Some(authorization) = &self.maybe_authorization { diff --git a/crates/modelardb_client/src/main.rs b/crates/modelardb_client/src/main.rs index e2725275..11c8bcc4 100644 --- a/crates/modelardb_client/src/main.rs +++ b/crates/modelardb_client/src/main.rs @@ -39,7 +39,6 @@ use modelardb_auth::BearerInterceptor; use rustyline::Editor; use rustyline::history::FileHistory; use tonic::codegen::InterceptedService; -use tonic::metadata::AsciiMetadataValue; use tonic::transport::{Channel, Endpoint}; use tonic::{Request, Streaming}; @@ -139,17 +138,7 @@ async fn connect( port: u16, maybe_token: Option, ) -> Result { - let maybe_authorization = maybe_token - .map(|token| { - format!("Bearer {token}") - .parse::() - .map_err(|error| ModelarDbClientError::InvalidArgument(error.to_string())) - }) - .transpose()?; - - let interceptor = BearerInterceptor { - maybe_authorization, - }; + let interceptor = BearerInterceptor::try_new(maybe_token.as_deref())?; let address = format!("grpc://{host}:{port}"); let connection = Endpoint::new(address)?.connect().await?; diff --git a/crates/modelardb_embedded/src/operations/client.rs b/crates/modelardb_embedded/src/operations/client.rs index 8e9c41af..14365b19 100644 --- a/crates/modelardb_embedded/src/operations/client.rs +++ b/crates/modelardb_embedded/src/operations/client.rs @@ -36,7 +36,6 @@ use datafusion::physical_plan::stream::RecordBatchStreamAdapter; use futures::{StreamExt, TryStreamExt, stream}; use modelardb_auth::BearerInterceptor; use tonic::codegen::InterceptedService; -use tonic::metadata::AsciiMetadataValue; use tonic::transport::{Channel, Endpoint}; use tonic::{Request, Status}; @@ -59,17 +58,7 @@ impl Client { /// provided, it is attached as a bearer token to every request. If a connection to the node /// could not be established, [`ModelarDbEmbeddedError`] is returned. pub async fn connect(url: &str, maybe_token: Option<&str>) -> Result { - let maybe_authorization = maybe_token - .map(|token| { - format!("Bearer {token}") - .parse::() - .map_err(|error| ModelarDbEmbeddedError::InvalidArgument(error.to_string())) - }) - .transpose()?; - - let interceptor = BearerInterceptor { - maybe_authorization, - }; + let interceptor = BearerInterceptor::try_new(maybe_token)?; let connection = Endpoint::new(url.to_owned())?.connect().await?; let flight_client = FlightServiceClient::with_interceptor(connection, interceptor); From e82e3fd183e92de08856e64f66b74e106318638d Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Mon, 22 Jun 2026 08:53:28 +0200 Subject: [PATCH 49/59] Pass authorization header down for INCLUDE requests --- crates/modelardb_server/src/remote/mod.rs | 27 ++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/crates/modelardb_server/src/remote/mod.rs b/crates/modelardb_server/src/remote/mod.rs index edd18bcf..fbe2205d 100644 --- a/crates/modelardb_server/src/remote/mod.rs +++ b/crates/modelardb_server/src/remote/mod.rs @@ -54,7 +54,7 @@ use prost::Message; use tokio::sync::mpsc::{self, Sender}; use tokio::task; use tokio_stream::wrappers::ReceiverStream; -use tonic::metadata::MetadataMap; +use tonic::metadata::{AsciiMetadataValue, MetadataMap}; use tonic::transport::{Endpoint, Server}; use tonic::{Request, Response, Status, Streaming}; use tracing::{debug, error, info}; @@ -108,6 +108,7 @@ pub async fn start_apache_arrow_flight_server( async fn execute_query_at_addresses_and_union( sql: &str, addresses: Vec, + maybe_authorization: Option, local_sendable_record_batch_stream: Pin>, ) -> Result>> { // Remove INCLUDE address+ as sqlparser seems incapable of doing so. @@ -124,8 +125,12 @@ async fn execute_query_at_addresses_and_union( unioned_sendable_record_batch_streams.push(local_sendable_record_batch_stream); for address in addresses { - let remote_sendable_record_batch_stream = - execute_query_at_address(sql_select.to_owned(), address.to_owned()).await?; + let remote_sendable_record_batch_stream = execute_query_at_address( + sql_select.to_owned(), + address.to_owned(), + maybe_authorization.clone(), + ) + .await?; unioned_sendable_record_batch_streams.push(remote_sendable_record_batch_stream); } @@ -140,13 +145,22 @@ async fn execute_query_at_addresses_and_union( async fn execute_query_at_address( sql: String, address: String, + maybe_authorization: Option, ) -> Result>> { // Connect and execute query. let connection = Endpoint::new(address.clone())?.connect().await?; let mut flight_client = FlightServiceClient::new(connection); - let ticket = Ticket { ticket: sql.into() }; - let mut stream = flight_client.do_get(ticket).await?.into_inner(); + // Insert the authorization header if it is present. The header is added directly to avoid + // extracting the token from the authorization header. + let mut request = Request::new(Ticket { ticket: sql.into() }); + if let Some(authorization) = maybe_authorization { + request + .metadata_mut() + .insert("authorization", authorization); + } + + let mut stream = flight_client.do_get(request).await?.into_inner(); // Read schema of record batches. let flight_data = stream.message().await?.ok_or_else(|| { @@ -684,9 +698,12 @@ impl FlightService for FlightServiceHandler { .await .map_err(error_to_status_internal)?; + let maybe_authorization = request.metadata().get("authorization").cloned(); + execute_query_at_addresses_and_union( &sql, addresses, + maybe_authorization, local_sendable_record_batch_stream, ) .await From 5e842b40233170c3ae382985d82131ee9ca17dbf Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Mon, 22 Jun 2026 10:02:57 +0200 Subject: [PATCH 50/59] Make Authenticator::authorize() async --- Cargo.lock | 2 ++ crates/modelardb_auth/Cargo.toml | 4 ++++ .../modelardb_auth/src/authenticator/mock.rs | 4 +++- .../modelardb_auth/src/authenticator/mod.rs | 4 +++- .../src/authenticator/no_auth.rs | 19 ++++++++++++------- .../modelardb_server/src/remote/auth_layer.rs | 4 ++-- 6 files changed, 26 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fe5a333a..b6b2ca93 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3309,6 +3309,8 @@ dependencies = [ name = "modelardb_auth" version = "0.1.0" dependencies = [ + "async-trait", + "tokio", "tonic", ] diff --git a/crates/modelardb_auth/Cargo.toml b/crates/modelardb_auth/Cargo.toml index 70cd1ed1..0942367d 100644 --- a/crates/modelardb_auth/Cargo.toml +++ b/crates/modelardb_auth/Cargo.toml @@ -20,7 +20,11 @@ edition = "2024" authors = ["Soeren Kejser Jensen "] [dependencies] +async-trait.workspace = true tonic.workspace = true +[dev-dependencies] +tokio.workspace = true + [features] testing = [] diff --git a/crates/modelardb_auth/src/authenticator/mock.rs b/crates/modelardb_auth/src/authenticator/mock.rs index 91bca601..10396f7f 100644 --- a/crates/modelardb_auth/src/authenticator/mock.rs +++ b/crates/modelardb_auth/src/authenticator/mock.rs @@ -19,6 +19,7 @@ use std::sync::Mutex; +use async_trait::async_trait; use tonic::Status; use tonic::metadata::MetadataMap; @@ -52,8 +53,9 @@ impl Default for MockAuthenticator { } } +#[async_trait] impl Authenticator for MockAuthenticator { - fn authorize( + async fn authorize( &self, _metadata: &MetadataMap, required_permission: Permission, diff --git a/crates/modelardb_auth/src/authenticator/mod.rs b/crates/modelardb_auth/src/authenticator/mod.rs index 252a7a77..6a1e99c8 100644 --- a/crates/modelardb_auth/src/authenticator/mod.rs +++ b/crates/modelardb_auth/src/authenticator/mod.rs @@ -20,6 +20,7 @@ pub mod mock; pub mod no_auth; +use async_trait::async_trait; use tonic::Status; use tonic::metadata::MetadataMap; @@ -30,8 +31,9 @@ use crate::Permission; /// credentials are missing or invalid, or `Err(Status::permission_denied(...))` if credentials are /// valid but the caller lacks the required permission. Error messages must not reveal the specific /// reason for rejection to prevent information leakage. +#[async_trait] pub trait Authenticator: Send + Sync { - fn authorize( + async fn authorize( &self, metadata: &MetadataMap, required_permission: Permission, diff --git a/crates/modelardb_auth/src/authenticator/no_auth.rs b/crates/modelardb_auth/src/authenticator/no_auth.rs index b78bf25f..ec2240bc 100644 --- a/crates/modelardb_auth/src/authenticator/no_auth.rs +++ b/crates/modelardb_auth/src/authenticator/no_auth.rs @@ -16,6 +16,7 @@ //! An [`Authenticator`] that grants all permissions to all callers without any validation. Used in //! the open-source version of ModelarDB where authentication is not required. +use async_trait::async_trait; use tonic::Status; use tonic::metadata::MetadataMap; @@ -25,8 +26,9 @@ use crate::authenticator::Authenticator; /// An [`Authenticator`] that grants all permissions to all callers without any validation. pub struct NoAuth; +#[async_trait] impl Authenticator for NoAuth { - fn authorize( + async fn authorize( &self, _metadata: &MetadataMap, _required_permission: Permission, @@ -39,29 +41,32 @@ impl Authenticator for NoAuth { mod tests { use super::*; - #[test] - fn test_no_auth_allows_read() { + #[tokio::test] + async fn test_no_auth_allows_read() { assert!( NoAuth .authorize(&MetadataMap::new(), Permission::Read) + .await .is_ok() ); } - #[test] - fn test_no_auth_allows_write() { + #[tokio::test] + async fn test_no_auth_allows_write() { assert!( NoAuth .authorize(&MetadataMap::new(), Permission::Write) + .await .is_ok() ); } - #[test] - fn test_no_auth_allows_admin() { + #[tokio::test] + async fn test_no_auth_allows_admin() { assert!( NoAuth .authorize(&MetadataMap::new(), Permission::Admin) + .await .is_ok() ); } diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index ea632562..dc2eb272 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -172,7 +172,7 @@ async fn authorize( } }; - authenticator.authorize(&metadata, required_permission)?; + authenticator.authorize(&metadata, required_permission).await?; Ok(request) } @@ -207,7 +207,7 @@ async fn authorize_do_get( let statement = parser::tokenize_and_parse_sql_statement(sql).map_err(error_to_status_invalid_argument)?; - authenticator.authorize(metadata, permission_for_statement(&statement))?; + authenticator.authorize(metadata, permission_for_statement(&statement)).await?; // Reconstruct the request with the original bytes so the server receives it intact. Ok(Request::from_parts(parts, Body::new(Full::new(bytes)))) From 9f61378be0d808f3b72f55f6a93662989653a424 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Mon, 22 Jun 2026 11:22:44 +0200 Subject: [PATCH 51/59] Add tests for BearerInterceptor --- crates/modelardb_auth/src/lib.rs | 37 ++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/crates/modelardb_auth/src/lib.rs b/crates/modelardb_auth/src/lib.rs index 649ccdaf..a7904b67 100644 --- a/crates/modelardb_auth/src/lib.rs +++ b/crates/modelardb_auth/src/lib.rs @@ -85,6 +85,7 @@ impl Interceptor for BearerInterceptor { mod tests { use super::*; + // Tests for Permission. #[test] fn test_read_satisfied_by_read() { assert!(Permission::Read.is_satisfied_by(&Permission::Read)); @@ -129,4 +130,40 @@ mod tests { fn test_admin_satisfied_by_admin() { assert!(Permission::Admin.is_satisfied_by(&Permission::Admin)); } + + // Tests for BearerInterceptor. + #[test] + fn test_try_new_bearer_interceptor_with_invalid_token() { + let result = BearerInterceptor::try_new(Some("sec\nret")); + + assert_eq!( + result.err().unwrap().to_string(), + "code: 'Client specified an invalid argument', \ + message: \"Invalid token: failed to parse metadata value.\"" + ); + } + + #[test] + fn test_bearer_interceptor_call_attaches_authorization_header() { + let mut interceptor = BearerInterceptor::try_new(Some("secret")).unwrap(); + let request = interceptor.call(Request::new(())).unwrap(); + + assert_eq!( + request + .metadata() + .get("authorization") + .unwrap() + .to_str() + .unwrap(), + "Bearer secret" + ); + } + + #[test] + fn test_bearer_interceptor_call_without_token_attaches_nothing() { + let mut interceptor = BearerInterceptor::try_new(None).unwrap(); + let request = interceptor.call(Request::new(())).unwrap(); + + assert!(request.metadata().get("authorization").is_none()); + } } From e0911da9ed4d709e3a5d010a882624be951551f9 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Mon, 22 Jun 2026 11:39:02 +0200 Subject: [PATCH 52/59] Formatted with Rustfmt --- crates/modelardb_server/src/remote/auth_layer.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index dc2eb272..2d808236 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -172,7 +172,9 @@ async fn authorize( } }; - authenticator.authorize(&metadata, required_permission).await?; + authenticator + .authorize(&metadata, required_permission) + .await?; Ok(request) } @@ -207,7 +209,9 @@ async fn authorize_do_get( let statement = parser::tokenize_and_parse_sql_statement(sql).map_err(error_to_status_invalid_argument)?; - authenticator.authorize(metadata, permission_for_statement(&statement)).await?; + authenticator + .authorize(metadata, permission_for_statement(&statement)) + .await?; // Reconstruct the request with the original bytes so the server receives it intact. Ok(Request::from_parts(parts, Body::new(Full::new(bytes)))) From 2708cac5d6155ba399575e7c2094fa075c8f5b32 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Tue, 23 Jun 2026 09:45:32 +0200 Subject: [PATCH 53/59] Add token to clap parsing in modelardb_client --- crates/modelardb_client/src/main.rs | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/crates/modelardb_client/src/main.rs b/crates/modelardb_client/src/main.rs index e595625b..0cab02b0 100644 --- a/crates/modelardb_client/src/main.rs +++ b/crates/modelardb_client/src/main.rs @@ -34,8 +34,8 @@ use arrow::util::pretty; use arrow_flight::flight_service_client::FlightServiceClient; use arrow_flight::{Action, Criteria, FlightData, FlightDescriptor, Ticket, utils}; use bytes::Bytes; -use modelardb_auth::BearerInterceptor; use clap::Parser; +use modelardb_auth::BearerInterceptor; use rustyline::Editor; use rustyline::history::FileHistory; use tonic::codegen::InterceptedService; @@ -48,6 +48,10 @@ use crate::helper::ClientHelper; /// Error to emit when the server does not provide a response when one is expected. const TRANSPORT_ERROR: &str = "transport error: no messages received."; +/// [`FlightServiceClient`] with a [`BearerInterceptor`] that attaches an authorization header. +type AuthenticatedFlightClient = + FlightServiceClient>; + /// Command line arguments for the ModelarDB client. #[derive(Parser)] #[command( @@ -66,6 +70,11 @@ struct ClientArgs { #[arg(long, default_value_t = 9999, env = "MODELARDB_PORT")] port: u16, + /// Bearer token for authenticating requests sent to the modelardbd instance. If not provided, + /// requests are sent without an authorization header. + #[arg(long, env = "MODELARDB_TOKEN")] + token: Option, + /// Path to a file containing SQL queries to execute. If not provided, an interactive /// read-eval-print loop is opened. query_file: Option, @@ -80,7 +89,7 @@ async fn main() -> Result<()> { let args = ClientArgs::parse(); // Execute the queries. - let flight_service_client = connect(&args.host, args.port).await?; + let flight_service_client = connect(&args.host, args.port, args.token).await?; if let Some(query_file) = args.query_file { execute_queries_from_a_file(flight_service_client, &query_file).await } else { @@ -88,9 +97,16 @@ async fn main() -> Result<()> { } } -/// Connect to the server at `host`:`port`. Returns [`ModelarDbClientError`] if a connection to the -/// server cannot be established. -async fn connect(host: &str, port: u16) -> Result> { +/// Connect to the server at `host`:`port` with an optional bearer `maybe_token`. Returns +/// [`ModelarDbClientError`] if a connection to the server cannot be established or the token is +/// not a valid ASCII metadata value. +async fn connect( + host: &str, + port: u16, + maybe_token: Option, +) -> Result { + let interceptor = BearerInterceptor::try_new(maybe_token.as_deref())?; + let address = format!("grpc://{host}:{port}"); let connection = Endpoint::new(address)?.connect().await?; From a7ae64450a407c0be0fc86f5969c805c4aecd357 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 2 Jul 2026 06:37:32 +0200 Subject: [PATCH 54/59] Update Authenticator based on comments from @skejserjensen --- .../modelardb_auth/src/authenticator/mock.rs | 15 +++++---- .../modelardb_auth/src/authenticator/mod.rs | 12 ++++--- .../src/authenticator/no_auth.rs | 4 +-- .../modelardb_server/src/remote/auth_layer.rs | 32 +++++++++---------- 4 files changed, 35 insertions(+), 28 deletions(-) diff --git a/crates/modelardb_auth/src/authenticator/mock.rs b/crates/modelardb_auth/src/authenticator/mock.rs index 10396f7f..464107c4 100644 --- a/crates/modelardb_auth/src/authenticator/mock.rs +++ b/crates/modelardb_auth/src/authenticator/mock.rs @@ -29,25 +29,28 @@ use crate::authenticator::Authenticator; /// An [`Authenticator`] for use in tests that always grants access and records every /// [`Permission`] passed to [`authorize`](Authenticator::authorize). pub struct MockAuthenticator { - /// Every [`Permission`] passed to [`authorize`](Authenticator::authorize). - calls: Mutex>, + /// Every [`Permission`] passed to [`authorize`](Authenticator::authorize). A [`Mutex`] is used + /// because [`authorize`](Authenticator::authorize) records through `&self`, which requires + /// interior mutability. + permissions: Mutex>, } impl MockAuthenticator { pub fn new() -> Self { Self { - calls: Mutex::new(vec![]), + permissions: Mutex::new(vec![]), } } /// Return a copy of all permissions passed to [`authorize`](Authenticator::authorize) /// in the order they were received. - pub fn calls(&self) -> Vec { - self.calls.lock().unwrap().clone() + pub fn permissions(&self) -> Vec { + self.permissions.lock().unwrap().clone() } } impl Default for MockAuthenticator { + // Trait implemented to silence clippy warning. fn default() -> Self { Self::new() } @@ -60,7 +63,7 @@ impl Authenticator for MockAuthenticator { _metadata: &MetadataMap, required_permission: Permission, ) -> Result<(), Status> { - self.calls.lock().unwrap().push(required_permission); + self.permissions.lock().unwrap().push(required_permission); Ok(()) } } diff --git a/crates/modelardb_auth/src/authenticator/mod.rs b/crates/modelardb_auth/src/authenticator/mod.rs index 6a1e99c8..890fd132 100644 --- a/crates/modelardb_auth/src/authenticator/mod.rs +++ b/crates/modelardb_auth/src/authenticator/mod.rs @@ -27,10 +27,14 @@ use tonic::metadata::MetadataMap; use crate::Permission; /// Validates the credentials in `metadata` and checks that the caller has the `required_permission`. -/// Implementations must return `Ok(())` if authorized, `Err(Status::unauthenticated(...))` if -/// credentials are missing or invalid, or `Err(Status::permission_denied(...))` if credentials are -/// valid but the caller lacks the required permission. Error messages must not reveal the specific -/// reason for rejection to prevent information leakage. +/// Implementations must return: +/// +/// - `Ok(())` if authorized. +/// - `Err(Status::unauthenticated(...))` if credentials are missing or invalid. +/// - `Err(Status::permission_denied(...))` if credentials are valid but the caller lacks the +/// required permission. +/// +/// Error messages must not reveal the specific reason for rejection to prevent information leakage. #[async_trait] pub trait Authenticator: Send + Sync { async fn authorize( diff --git a/crates/modelardb_auth/src/authenticator/no_auth.rs b/crates/modelardb_auth/src/authenticator/no_auth.rs index ec2240bc..a229bacd 100644 --- a/crates/modelardb_auth/src/authenticator/no_auth.rs +++ b/crates/modelardb_auth/src/authenticator/no_auth.rs @@ -13,8 +13,8 @@ * limitations under the License. */ -//! An [`Authenticator`] that grants all permissions to all callers without any validation. Used in -//! the open-source version of ModelarDB where authentication is not required. +//! An [`Authenticator`] that grants all permissions to all callers without any validation. Used +//! when authentication is not required. use async_trait::async_trait; use tonic::Status; diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index 2d808236..464b4a22 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -253,7 +253,7 @@ mod tests { let result = authorize(request, &*authenticator, &cluster_key).await; assert!(result.is_ok()); - assert!(authenticator.calls().is_empty()); + assert!(authenticator.permissions().is_empty()); } #[tokio::test] @@ -300,7 +300,7 @@ mod tests { let result = authorize(request, &*authenticator, &None).await; assert!(result.is_ok()); - assert!(authenticator.calls().is_empty()); + assert!(authenticator.permissions().is_empty()); } #[tokio::test] @@ -311,7 +311,7 @@ mod tests { let result = authorize(request, &*authenticator, &None).await; assert!(result.is_ok()); - assert_eq!(authenticator.calls(), vec![Permission::Read]); + assert_eq!(authenticator.permissions(), vec![Permission::Read]); } #[tokio::test] @@ -322,7 +322,7 @@ mod tests { let result = authorize(request, &*authenticator, &None).await; assert!(result.is_ok()); - assert_eq!(authenticator.calls(), vec![Permission::Read]); + assert_eq!(authenticator.permissions(), vec![Permission::Read]); } #[tokio::test] @@ -333,7 +333,7 @@ mod tests { let result = authorize(request, &*authenticator, &None).await; assert!(result.is_ok()); - assert_eq!(authenticator.calls(), vec![Permission::Read]); + assert_eq!(authenticator.permissions(), vec![Permission::Read]); } #[tokio::test] @@ -344,7 +344,7 @@ mod tests { let result = authorize(request, &*authenticator, &None).await; assert!(result.is_ok()); - assert_eq!(authenticator.calls(), vec![Permission::Write]); + assert_eq!(authenticator.permissions(), vec![Permission::Write]); } #[tokio::test] @@ -355,7 +355,7 @@ mod tests { let result = authorize(request, &*authenticator, &None).await; assert!(result.is_ok()); - assert_eq!(authenticator.calls(), vec![Permission::Admin]); + assert_eq!(authenticator.permissions(), vec![Permission::Admin]); } #[tokio::test] @@ -366,7 +366,7 @@ mod tests { let result = authorize(request, &*authenticator, &None).await; assert!(result.is_ok()); - assert_eq!(authenticator.calls(), vec![Permission::Admin]); + assert_eq!(authenticator.permissions(), vec![Permission::Admin]); } #[tokio::test] @@ -377,7 +377,7 @@ mod tests { let result = authorize(request, &*authenticator, &None).await; assert!(result.is_ok()); - assert_eq!(authenticator.calls(), vec![Permission::Admin]); + assert_eq!(authenticator.permissions(), vec![Permission::Admin]); } #[tokio::test] @@ -388,7 +388,7 @@ mod tests { let result = authorize(request, &*authenticator, &None).await; assert!(result.is_ok()); - assert_eq!(authenticator.calls(), vec![Permission::Admin]); + assert_eq!(authenticator.permissions(), vec![Permission::Admin]); } #[tokio::test] @@ -399,7 +399,7 @@ mod tests { let result = authorize(request, &*authenticator, &None).await; assert!(result.is_ok()); - assert_eq!(authenticator.calls(), vec![Permission::Admin]); + assert_eq!(authenticator.permissions(), vec![Permission::Admin]); } #[tokio::test] @@ -410,7 +410,7 @@ mod tests { let result = authorize(request, &*authenticator, &None).await; assert!(result.is_ok()); - assert_eq!(authenticator.calls(), vec![Permission::Admin]); + assert_eq!(authenticator.permissions(), vec![Permission::Admin]); } #[tokio::test] @@ -421,7 +421,7 @@ mod tests { let result = authorize(request, &*authenticator, &None).await; assert!(result.is_ok()); - assert_eq!(authenticator.calls(), vec![Permission::Read]); + assert_eq!(authenticator.permissions(), vec![Permission::Read]); } #[tokio::test] @@ -432,7 +432,7 @@ mod tests { let result = authorize(request, &*authenticator, &None).await; assert!(result.is_ok()); - assert_eq!(authenticator.calls(), vec![Permission::Write]); + assert_eq!(authenticator.permissions(), vec![Permission::Write]); } #[tokio::test] @@ -443,7 +443,7 @@ mod tests { let result = authorize(request, &*authenticator, &None).await; assert!(result.is_ok()); - assert_eq!(authenticator.calls(), vec![Permission::Read]); + assert_eq!(authenticator.permissions(), vec![Permission::Read]); } #[tokio::test] @@ -454,7 +454,7 @@ mod tests { let result = authorize(request, &*authenticator, &None).await; assert!(result.is_ok()); - assert_eq!(authenticator.calls(), vec![Permission::Read]); + assert_eq!(authenticator.permissions(), vec![Permission::Read]); } #[tokio::test] From 7cd5abfd0b395a265c04dce860204b88df001b22 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 2 Jul 2026 07:19:26 +0200 Subject: [PATCH 55/59] Use AsciiMetadataValue instead of MetadataValue --- crates/modelardb_auth/src/lib.rs | 26 ++++++++++--------- crates/modelardb_server/src/cluster.rs | 9 ++++--- .../modelardb_server/src/remote/auth_layer.rs | 16 ++++++------ 3 files changed, 27 insertions(+), 24 deletions(-) diff --git a/crates/modelardb_auth/src/lib.rs b/crates/modelardb_auth/src/lib.rs index a7904b67..763c383f 100644 --- a/crates/modelardb_auth/src/lib.rs +++ b/crates/modelardb_auth/src/lib.rs @@ -32,34 +32,36 @@ pub enum Permission { impl Permission { /// Return [`true`] if `granted` satisfies this required permission using the hierarchy /// Admin ⊇ Write ⊇ Read, otherwise [`false`] is returned. + #[allow(clippy::match_like_matches_macro)] pub fn is_satisfied_by(&self, granted: &Permission) -> bool { - matches!( - (self, granted), - (Permission::Read, _) - | (Permission::Write, Permission::Write | Permission::Admin) - | (Permission::Admin, Permission::Admin) - ) + match (self, granted) { + (Permission::Read, _) => true, + (Permission::Write, Permission::Write | Permission::Admin) => true, + (Permission::Admin, Permission::Admin) => true, + _ => false, + } } } /// Tonic interceptor that attaches a bearer token to every outgoing request when one is present. #[derive(Clone)] pub struct BearerInterceptor { - /// The value of the `authorization` header to attach. This is either `Bearer ` or - /// [`None`] if no token has been provided. + /// The value of the `authorization` header. This is either `Bearer ` or [`None`] if no + /// token has been provided. The header is only attached to an outgoing request if this is not + /// [`None`]. pub maybe_authorization: Option, } impl BearerInterceptor { /// Create a new [`BearerInterceptor`] that attaches the bearer token in `maybe_token` to every - /// outgoing request. If `maybe_token` is [`None`], no token is attached. If `maybe_token` is - /// [`Some`] but the token is an invalid ASCII string, [`Status`] is returned. + /// outgoing request if `maybe_token` is not [`None`]. If `maybe_token` is [`Some`] but the + /// token is not ASCII, [`Status`] is returned. pub fn try_new(maybe_token: Option<&str>) -> Result { let maybe_authorization = maybe_token .map(|token| { format!("Bearer {token}") .parse::() - .map_err(|error| Status::invalid_argument(format!("Invalid token: {error}."))) + .map_err(|error| Status::invalid_argument(format!("Token is not ASCII: {error}."))) }) .transpose()?; @@ -139,7 +141,7 @@ mod tests { assert_eq!( result.err().unwrap().to_string(), "code: 'Client specified an invalid argument', \ - message: \"Invalid token: failed to parse metadata value.\"" + message: \"Token is not ASCII: failed to parse metadata value.\"" ); } diff --git a/crates/modelardb_server/src/cluster.rs b/crates/modelardb_server/src/cluster.rs index 6f8eb8e6..311b9b97 100644 --- a/crates/modelardb_server/src/cluster.rs +++ b/crates/modelardb_server/src/cluster.rs @@ -30,7 +30,7 @@ use modelardb_types::types::{Node, TimeSeriesTableMetadata}; use rand::rng; use rand::seq::IteratorRandom; use tonic::Request; -use tonic::metadata::{Ascii, MetadataValue}; +use tonic::metadata::AsciiMetadataValue; use tonic::transport::Endpoint; use crate::context::Context; @@ -44,7 +44,7 @@ pub(crate) struct Cluster { node: Node, /// Key identifying the cluster. The key is used to validate communication within the cluster /// between nodes. - key: MetadataValue, + key: AsciiMetadataValue, /// The remote data folder that each node in the cluster should be synchronized with. /// When a table is created, dropped, vacuumed, or truncated, it is done in the /// remote data folder first. @@ -66,7 +66,8 @@ impl Cluster { let key = remote_data_folder.cluster_key().await?.to_string(); // Convert the key to a MetadataValue since it is used in tonic requests. - let key = MetadataValue::from_str(&key).expect("UUID Version 4 should be valid ASCII."); + let key = + AsciiMetadataValue::from_str(&key).expect("UUID Version 4 should be valid ASCII."); Ok(Self { node, @@ -81,7 +82,7 @@ impl Cluster { } /// Return the key identifying the cluster. - pub(crate) fn key(&self) -> &MetadataValue { + pub(crate) fn key(&self) -> &AsciiMetadataValue { &self.key } diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index 464b4a22..acb253b2 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -13,7 +13,7 @@ * limitations under the License. */ -//! Tower middleware layer that enforces authentication and authorization on all incoming Apache +//! Tower middleware layer that enforces authentication and authorization for all incoming Apache //! Arrow Flight requests. The layer runs before [`FlightServiceHandler`](super::FlightServiceHandler) //! and checks the cluster key or bearer token before the request reaches the handler. For DoGet //! requests the SQL ticket is decoded, and the required permission is determined from the parsed @@ -34,7 +34,7 @@ use prost::Message; use sqlparser::ast::Statement; use tonic::Status; use tonic::body::Body; -use tonic::metadata::{Ascii, MetadataMap, MetadataValue}; +use tonic::metadata::{AsciiMetadataValue, MetadataMap}; use tower::{Layer, Service}; use crate::remote::error_to_status_invalid_argument; @@ -56,13 +56,13 @@ pub(super) struct AuthLayer { authenticator: Arc, /// The cluster key to use for validating internal cluster requests or [`None`] if running a /// single-node server. - maybe_cluster_key: Option>, + maybe_cluster_key: Option, } impl AuthLayer { pub(super) fn new( authenticator: Arc, - maybe_cluster_key: Option>, + maybe_cluster_key: Option, ) -> Self { Self { authenticator, @@ -95,7 +95,7 @@ pub(super) struct AuthService { authenticator: Arc, /// The cluster key to use for validating internal cluster requests or [`None`] if running a /// single-node server. - maybe_cluster_key: Option>, + maybe_cluster_key: Option, } impl Service> for AuthService @@ -137,7 +137,7 @@ where async fn authorize( request: Request, authenticator: &dyn Authenticator, - maybe_cluster_key: &Option>, + maybe_cluster_key: &Option, ) -> Result, Status> { let path = request.uri().path().to_owned(); let metadata = MetadataMap::from_headers(request.headers().clone()); @@ -248,7 +248,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = empty_request_with_cluster_key(DO_PUT_PATH, CLUSTER_KEY); - let cluster_key = Some(MetadataValue::from_static(CLUSTER_KEY)); + let cluster_key = Some(AsciiMetadataValue::from_static(CLUSTER_KEY)); let result = authorize(request, &*authenticator, &cluster_key).await; @@ -261,7 +261,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = empty_request_with_cluster_key(LIST_FLIGHTS_PATH, "invalid_key"); - let cluster_key = Some(MetadataValue::from_static(CLUSTER_KEY)); + let cluster_key = Some(AsciiMetadataValue::from_static(CLUSTER_KEY)); let result = authorize(request, &*authenticator, &cluster_key).await; From a112d8ce32e054ebf5b47184c3ae8e2ebb736420 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 2 Jul 2026 07:24:50 +0200 Subject: [PATCH 56/59] Make calling LIST_ACTIONS require admin permission --- .../modelardb_server/src/remote/auth_layer.rs | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index acb253b2..ac8bbcb3 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -152,11 +152,6 @@ async fn authorize( }; } - // ListActions is a public discovery endpoint. - if path == LIST_ACTIONS_PATH { - return Ok(request); - } - // Decode the ticket and parse the SQL to determine the required permission. if path == DO_GET_PATH { return authorize_do_get(request, authenticator, &metadata).await; @@ -166,7 +161,7 @@ async fn authorize( let required_permission = match path.as_str() { LIST_FLIGHTS_PATH | GET_FLIGHT_INFO_PATH | GET_SCHEMA_PATH => Permission::Read, DO_PUT_PATH => Permission::Write, - DO_ACTION_PATH => Permission::Admin, + DO_ACTION_PATH | LIST_ACTIONS_PATH => Permission::Admin, _ => { return Err(Status::invalid_argument("Unknown path.")); } @@ -180,7 +175,7 @@ async fn authorize( } /// Buffer the DoGet body, decode the gRPC [`Ticket`] protobuf, parse the SQL, determine the -/// required permission, authorize, then reconstruct the request with the original bytes. +/// required permission, authorize, then reconstruct the request byte-for-byte. async fn authorize_do_get( request: Request, authenticator: &dyn Authenticator, @@ -292,17 +287,6 @@ mod tests { .unwrap() } - #[tokio::test] - async fn test_authorize_list_actions_bypasses_authenticator() { - let authenticator = Arc::new(MockAuthenticator::new()); - let request = empty_request(LIST_ACTIONS_PATH); - - let result = authorize(request, &*authenticator, &None).await; - - assert!(result.is_ok()); - assert!(authenticator.permissions().is_empty()); - } - #[tokio::test] async fn test_authorize_list_flights_calls_authenticator_with_read() { let authenticator = Arc::new(MockAuthenticator::new()); @@ -358,6 +342,17 @@ mod tests { assert_eq!(authenticator.permissions(), vec![Permission::Admin]); } + #[tokio::test] + async fn test_authorize_list_actions_calls_authenticator_with_admin() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = empty_request(LIST_ACTIONS_PATH); + + let result = authorize(request, &*authenticator, &None).await; + + assert!(result.is_ok()); + assert_eq!(authenticator.permissions(), vec![Permission::Admin]); + } + #[tokio::test] async fn test_authorize_do_get_with_create_normal_table_calls_authenticator_with_admin() { let authenticator = Arc::new(MockAuthenticator::new()); From b011abc951b9833ee79925d29831662960ab2f6a Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 2 Jul 2026 07:59:52 +0200 Subject: [PATCH 57/59] Use None instead of NoAuth --- crates/modelardb_server/src/main.rs | 4 +- .../modelardb_server/src/remote/auth_layer.rs | 90 +++++++++++-------- crates/modelardb_server/src/remote/mod.rs | 9 +- 3 files changed, 59 insertions(+), 44 deletions(-) diff --git a/crates/modelardb_server/src/main.rs b/crates/modelardb_server/src/main.rs index 115557d1..9a015b3e 100644 --- a/crates/modelardb_server/src/main.rs +++ b/crates/modelardb_server/src/main.rs @@ -26,7 +26,6 @@ mod storage; use std::sync::Arc; use clap::{Parser, Subcommand}; -use modelardb_auth::authenticator::no_auth::NoAuth; use modelardb_types::types::CloudCredentials; use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt}; @@ -160,8 +159,7 @@ async fn main() -> Result<()> { context.replay_write_ahead_log().await?; // Start the Apache Arrow Flight interface. - let authenticator = Arc::new(NoAuth); - remote::start_apache_arrow_flight_server(context, authenticator, args.port).await?; + remote::start_apache_arrow_flight_server(context, None, args.port).await?; Ok(()) } diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index ac8bbcb3..f65446b5 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -51,9 +51,9 @@ const LIST_ACTIONS_PATH: &str = "/arrow.flight.protocol.FlightService/ListAction /// requests. #[derive(Clone)] pub(super) struct AuthLayer { - /// The [`Authenticator`] to use for authentication once the auth layer has determined the - /// required permission for a client request. - authenticator: Arc, + /// The [`Authenticator`] to use once the required permission has been determined, or [`None`] + /// if authentication is disabled and every non-cluster request should be allowed. + maybe_authenticator: Option>, /// The cluster key to use for validating internal cluster requests or [`None`] if running a /// single-node server. maybe_cluster_key: Option, @@ -61,11 +61,11 @@ pub(super) struct AuthLayer { impl AuthLayer { pub(super) fn new( - authenticator: Arc, + maybe_authenticator: Option>, maybe_cluster_key: Option, ) -> Self { Self { - authenticator, + maybe_authenticator, maybe_cluster_key, } } @@ -77,7 +77,7 @@ impl Layer for AuthLayer { fn layer(&self, inner: S) -> AuthService { AuthService { inner, - authenticator: self.authenticator.clone(), + maybe_authenticator: self.maybe_authenticator.clone(), maybe_cluster_key: self.maybe_cluster_key.clone(), } } @@ -90,9 +90,9 @@ pub(super) struct AuthService { /// The [`FlightServiceHandler`](super::FlightServiceHandler) that processes the request after /// authorization succeeds. inner: S, - /// The [`Authenticator`] to use for authentication once the auth layer has determined the - /// required permission for a client request. - authenticator: Arc, + /// The [`Authenticator`] to use once the required permission has been determined, or [`None`] + /// if authentication is disabled and every non-cluster request should be allowed. + maybe_authenticator: Option>, /// The cluster key to use for validating internal cluster requests or [`None`] if running a /// single-node server. maybe_cluster_key: Option, @@ -120,11 +120,11 @@ where let clone = self.inner.clone(); let mut inner = std::mem::replace(&mut self.inner, clone); - let authenticator = self.authenticator.clone(); + let maybe_authenticator = self.maybe_authenticator.clone(); let maybe_cluster_key = self.maybe_cluster_key.clone(); Box::pin(async move { - match authorize(request, &*authenticator, &maybe_cluster_key).await { + match authorize(request, maybe_authenticator.as_deref(), &maybe_cluster_key).await { Ok(request) => inner.call(request).await, Err(status) => Ok(status.into_http()), } @@ -136,10 +136,9 @@ where /// [`Status`] on failure. async fn authorize( request: Request, - authenticator: &dyn Authenticator, + maybe_authenticator: Option<&dyn Authenticator>, maybe_cluster_key: &Option, ) -> Result, Status> { - let path = request.uri().path().to_owned(); let metadata = MetadataMap::from_headers(request.headers().clone()); // Cluster key check must happen before the Authenticator since internal cluster requests bypass @@ -152,7 +151,14 @@ async fn authorize( }; } + // When no Authenticator is configured, authentication is disabled and every non-cluster request + // is allowed. + let Some(authenticator) = maybe_authenticator else { + return Ok(request); + }; + // Decode the ticket and parse the SQL to determine the required permission. + let path = request.uri().path().to_owned(); if path == DO_GET_PATH { return authorize_do_get(request, authenticator, &metadata).await; } @@ -245,7 +251,7 @@ mod tests { let request = empty_request_with_cluster_key(DO_PUT_PATH, CLUSTER_KEY); let cluster_key = Some(AsciiMetadataValue::from_static(CLUSTER_KEY)); - let result = authorize(request, &*authenticator, &cluster_key).await; + let result = authorize(request, Some(&*authenticator), &cluster_key).await; assert!(result.is_ok()); assert!(authenticator.permissions().is_empty()); @@ -258,7 +264,7 @@ mod tests { let request = empty_request_with_cluster_key(LIST_FLIGHTS_PATH, "invalid_key"); let cluster_key = Some(AsciiMetadataValue::from_static(CLUSTER_KEY)); - let result = authorize(request, &*authenticator, &cluster_key).await; + let result = authorize(request, Some(&*authenticator), &cluster_key).await; assert_eq!( result.unwrap_err().to_string(), @@ -271,7 +277,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = empty_request_with_cluster_key(LIST_FLIGHTS_PATH, CLUSTER_KEY); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert_eq!( result.unwrap_err().to_string(), @@ -287,12 +293,22 @@ mod tests { .unwrap() } + #[tokio::test] + async fn test_authorize_without_authenticator_allows_request_without_parsing() { + // A body that would fail ticket decoding if authorize_do_get() ran. + let request = raw_frame_request(&[0xFF, 0xFF, 0xFF, 0xFF]); + + let result = authorize(request, None, &None).await; + + assert!(result.is_ok()); + } + #[tokio::test] async fn test_authorize_list_flights_calls_authenticator_with_read() { let authenticator = Arc::new(MockAuthenticator::new()); let request = empty_request(LIST_FLIGHTS_PATH); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert!(result.is_ok()); assert_eq!(authenticator.permissions(), vec![Permission::Read]); @@ -303,7 +319,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = empty_request(GET_FLIGHT_INFO_PATH); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert!(result.is_ok()); assert_eq!(authenticator.permissions(), vec![Permission::Read]); @@ -314,7 +330,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = empty_request(GET_SCHEMA_PATH); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert!(result.is_ok()); assert_eq!(authenticator.permissions(), vec![Permission::Read]); @@ -325,7 +341,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = empty_request(DO_PUT_PATH); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert!(result.is_ok()); assert_eq!(authenticator.permissions(), vec![Permission::Write]); @@ -336,7 +352,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = empty_request(DO_ACTION_PATH); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert!(result.is_ok()); assert_eq!(authenticator.permissions(), vec![Permission::Admin]); @@ -347,7 +363,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = empty_request(LIST_ACTIONS_PATH); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert!(result.is_ok()); assert_eq!(authenticator.permissions(), vec![Permission::Admin]); @@ -358,7 +374,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = do_get_request(NORMAL_TABLE_SQL); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert!(result.is_ok()); assert_eq!(authenticator.permissions(), vec![Permission::Admin]); @@ -369,7 +385,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = do_get_request(TIME_SERIES_TABLE_SQL); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert!(result.is_ok()); assert_eq!(authenticator.permissions(), vec![Permission::Admin]); @@ -380,7 +396,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = do_get_request("DROP TABLE test_table"); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert!(result.is_ok()); assert_eq!(authenticator.permissions(), vec![Permission::Admin]); @@ -391,7 +407,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = do_get_request("TRUNCATE test_table"); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert!(result.is_ok()); assert_eq!(authenticator.permissions(), vec![Permission::Admin]); @@ -402,7 +418,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = do_get_request("VACUUM test_table"); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert!(result.is_ok()); assert_eq!(authenticator.permissions(), vec![Permission::Admin]); @@ -413,7 +429,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = do_get_request("INCLUDE 'grpc://192.168.1.2:9999' SELECT * FROM test_table"); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert!(result.is_ok()); assert_eq!(authenticator.permissions(), vec![Permission::Read]); @@ -424,7 +440,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = do_get_request("INSERT INTO test_table VALUES (1, 2, 3)"); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert!(result.is_ok()); assert_eq!(authenticator.permissions(), vec![Permission::Write]); @@ -435,7 +451,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = do_get_request("SELECT * FROM test_table"); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert!(result.is_ok()); assert_eq!(authenticator.permissions(), vec![Permission::Read]); @@ -446,7 +462,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = do_get_request("EXPLAIN SELECT * FROM test_table"); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert!(result.is_ok()); assert_eq!(authenticator.permissions(), vec![Permission::Read]); @@ -463,7 +479,7 @@ mod tests { let original_bytes = original_body.collect().await.unwrap().to_bytes(); let request = do_get_request(sql); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; let (_, reconstructed_body) = result.unwrap().into_parts(); let reconstructed_bytes = reconstructed_body.collect().await.unwrap().to_bytes(); @@ -479,7 +495,7 @@ mod tests { .body(Body::new(Full::new(bytes::Bytes::from(vec![0u8; 4])))) .unwrap(); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert_eq!( result.unwrap_err().to_string(), @@ -495,7 +511,7 @@ mod tests { // Valid 5-byte gRPC frame header but invalid protobuf bytes in the message. let request = raw_frame_request(&[0xFF, 0xFF, 0xFF, 0xFF]); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert_eq!( result.unwrap_err().to_string(), @@ -511,7 +527,7 @@ mod tests { // Encode a Ticket with invalid UTF-8 bytes. let request = ticket_frame_request(vec![0xFF, 0xFE]); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert_eq!( result.unwrap_err().to_string(), @@ -525,7 +541,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = do_get_request("invalid sql"); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert_eq!( result.unwrap_err().to_string(), @@ -564,7 +580,7 @@ mod tests { let authenticator = Arc::new(MockAuthenticator::new()); let request = empty_request("/unknown/path"); - let result = authorize(request, &*authenticator, &None).await; + let result = authorize(request, Some(&*authenticator), &None).await; assert_eq!( result.unwrap_err().to_string(), diff --git a/crates/modelardb_server/src/remote/mod.rs b/crates/modelardb_server/src/remote/mod.rs index fbe2205d..5fcbe826 100644 --- a/crates/modelardb_server/src/remote/mod.rs +++ b/crates/modelardb_server/src/remote/mod.rs @@ -66,11 +66,12 @@ use crate::remote::auth_layer::AuthLayer; /// Start an Apache Arrow Flight server on 0.0.0.0:`port` that passes `context` to the methods that /// process the requests through [`FlightServiceHandler`]. All requests are passed through the -/// [`AuthLayer`] that authenticates using `authenticator` before they are passed to the -/// [`FlightServiceHandler`]. +/// [`AuthLayer`], which authenticates them using `maybe_authenticator` before they are passed to +/// the [`FlightServiceHandler`]. If `maybe_authenticator` is [`None`], authentication is disabled, +/// and every request that is not an internal cluster request is allowed. pub async fn start_apache_arrow_flight_server( context: Arc, - authenticator: Arc, + maybe_authenticator: Option>, port: u16, ) -> Result<()> { let localhost_with_port = "0.0.0.0:".to_owned() + &port.to_string(); @@ -85,7 +86,7 @@ pub async fn start_apache_arrow_flight_server( ClusterMode::SingleNode => None, }; - let auth_layer = AuthLayer::new(authenticator, maybe_cluster_key); + let auth_layer = AuthLayer::new(maybe_authenticator, maybe_cluster_key); let handler = FlightServiceHandler::new(context); // Increase the maximum message size from 4 MiB to 16 MiB to allow bulk-loading larger batches. From 76cdc54dc4da98e15b2319898f75b257f20f1697 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Thu, 2 Jul 2026 08:00:08 +0200 Subject: [PATCH 58/59] Remove NoAuth --- Cargo.lock | 1 - crates/modelardb_auth/Cargo.toml | 3 - .../modelardb_auth/src/authenticator/mod.rs | 1 - .../src/authenticator/no_auth.rs | 73 ------------------- 4 files changed, 78 deletions(-) delete mode 100644 crates/modelardb_auth/src/authenticator/no_auth.rs diff --git a/Cargo.lock b/Cargo.lock index 57ee8266..6f5a4c85 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3506,7 +3506,6 @@ name = "modelardb_auth" version = "0.1.0" dependencies = [ "async-trait", - "tokio", "tonic", ] diff --git a/crates/modelardb_auth/Cargo.toml b/crates/modelardb_auth/Cargo.toml index 0942367d..c36c0927 100644 --- a/crates/modelardb_auth/Cargo.toml +++ b/crates/modelardb_auth/Cargo.toml @@ -23,8 +23,5 @@ authors = ["Soeren Kejser Jensen "] async-trait.workspace = true tonic.workspace = true -[dev-dependencies] -tokio.workspace = true - [features] testing = [] diff --git a/crates/modelardb_auth/src/authenticator/mod.rs b/crates/modelardb_auth/src/authenticator/mod.rs index 890fd132..0af878fa 100644 --- a/crates/modelardb_auth/src/authenticator/mod.rs +++ b/crates/modelardb_auth/src/authenticator/mod.rs @@ -18,7 +18,6 @@ #[cfg(feature = "testing")] pub mod mock; -pub mod no_auth; use async_trait::async_trait; use tonic::Status; diff --git a/crates/modelardb_auth/src/authenticator/no_auth.rs b/crates/modelardb_auth/src/authenticator/no_auth.rs deleted file mode 100644 index a229bacd..00000000 --- a/crates/modelardb_auth/src/authenticator/no_auth.rs +++ /dev/null @@ -1,73 +0,0 @@ -/* Copyright 2026 The ModelarDB Contributors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -//! An [`Authenticator`] that grants all permissions to all callers without any validation. Used -//! when authentication is not required. - -use async_trait::async_trait; -use tonic::Status; -use tonic::metadata::MetadataMap; - -use crate::Permission; -use crate::authenticator::Authenticator; - -/// An [`Authenticator`] that grants all permissions to all callers without any validation. -pub struct NoAuth; - -#[async_trait] -impl Authenticator for NoAuth { - async fn authorize( - &self, - _metadata: &MetadataMap, - _required_permission: Permission, - ) -> Result<(), Status> { - Ok(()) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[tokio::test] - async fn test_no_auth_allows_read() { - assert!( - NoAuth - .authorize(&MetadataMap::new(), Permission::Read) - .await - .is_ok() - ); - } - - #[tokio::test] - async fn test_no_auth_allows_write() { - assert!( - NoAuth - .authorize(&MetadataMap::new(), Permission::Write) - .await - .is_ok() - ); - } - - #[tokio::test] - async fn test_no_auth_allows_admin() { - assert!( - NoAuth - .authorize(&MetadataMap::new(), Permission::Admin) - .await - .is_ok() - ); - } -} From cb3705cd387aeff49fb122266f8189df679149cb Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Fri, 3 Jul 2026 12:02:22 +0200 Subject: [PATCH 59/59] Add support for optimize in auth layer --- crates/modelardb_server/src/remote/auth_layer.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/modelardb_server/src/remote/auth_layer.rs b/crates/modelardb_server/src/remote/auth_layer.rs index f65446b5..0443f927 100644 --- a/crates/modelardb_server/src/remote/auth_layer.rs +++ b/crates/modelardb_server/src/remote/auth_layer.rs @@ -226,6 +226,7 @@ fn permission_for_statement(statement: &ModelarDbStatement) -> Permission { ModelarDbStatement::DropTable(_) => Permission::Admin, ModelarDbStatement::TruncateTable(_, _) => Permission::Admin, ModelarDbStatement::Vacuum(_, _, _) => Permission::Admin, + ModelarDbStatement::Optimize(_, _, _) => Permission::Admin, ModelarDbStatement::IncludeSelect(_, _) => Permission::Read, ModelarDbStatement::Statement(statement) => match statement { Statement::Insert(_) => Permission::Write, @@ -424,6 +425,17 @@ mod tests { assert_eq!(authenticator.permissions(), vec![Permission::Admin]); } + #[tokio::test] + async fn test_authorize_do_get_with_optimize_calls_authenticator_with_admin() { + let authenticator = Arc::new(MockAuthenticator::new()); + let request = do_get_request("OPTIMIZE test_table"); + + let result = authorize(request, Some(&*authenticator), &None).await; + + assert!(result.is_ok()); + assert_eq!(authenticator.permissions(), vec![Permission::Admin]); + } + #[tokio::test] async fn test_authorize_do_get_with_include_select_calls_authenticator_with_read() { let authenticator = Arc::new(MockAuthenticator::new());