diff --git a/.config/nextest.toml b/.config/nextest.toml index b494bd53f6..e78bd401a1 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -9,3 +9,10 @@ serial-integration = { max-threads = 1 } filter = 'test(test::bottomless)' test-group = 'serial-integration' retries = 3 + +[[profile.default.overrides]] +# Under cfg(test) this test runs with TXN_TIMEOUT = 100 ms and a 100 ms SQLite +# busy timeout. When other test binaries compete for the runner's CPUs it fails +# with "database is locked" on every retry. Give it the whole runner. +filter = 'test(connection::connection_core::test::test_many_concurrent)' +threads-required = 'num-cpus' diff --git a/libsql-server/src/http/user/db_factory.rs b/libsql-server/src/http/user/db_factory.rs index 2a7c4c5752..3183f7fb7a 100644 --- a/libsql-server/src/http/user/db_factory.rs +++ b/libsql-server/src/http/user/db_factory.rs @@ -7,12 +7,12 @@ use hyper::HeaderMap; use libsql_replication::rpc::replication::NAMESPACE_METADATA_KEY; use crate::auth::Authenticated; -use crate::connection::MakeConnection; +use crate::connection::{MakeConnection, RequestContext}; use crate::database::Connection; use crate::error::Error; use crate::namespace::NamespaceName; -use super::AppState; +use super::{authenticate_for_namespace, AppState}; pub struct MakeConnectionExtractor(pub Arc>); @@ -90,25 +90,39 @@ fn try_namespace_from_metadata(metadata: &axum::http::HeaderValue) -> Result>); +/// Connection maker and request context for endpoints that select the namespace through a +/// `:namespace` URL path segment (`/dev/:namespace/v:version/pipeline`). +/// +/// Both fields are derived from the path namespace, so authentication (JWT key lookup), +/// the connection, and the [`RequestContext`] that statement authorization later consults +/// all refer to the same namespace. Any `x-namespace` / `Host` header on the request is +/// ignored for namespace selection on these routes. +pub struct PathNamespaceRequest { + pub connection_maker: Arc>, + pub ctx: RequestContext, +} + #[async_trait::async_trait] -impl FromRequestParts for MakeConnectionExtractorPath { +impl FromRequestParts for PathNamespaceRequest { type Rejection = Error; async fn from_request_parts( parts: &mut Parts, state: &AppState, ) -> Result { - let auth = Authenticated::from_request_parts(parts, state).await?; let Path((ns, _)) = Path::<(NamespaceName, String)>::from_request_parts(parts, state) .await .map_err(|e| Error::InvalidPath(e.to_string()))?; - Ok(Self( - state - .namespaces - .with_authenticated(ns, auth, |ns| ns.db.connection_maker()) - .await?, - )) + let auth = authenticate_for_namespace(parts, state, &ns).await?; + let connection_maker = state + .namespaces + .with_authenticated(ns.clone(), auth.clone(), |ns| ns.db.connection_maker()) + .await?; + let ctx = RequestContext::new(auth, ns, state.namespaces.meta_store().clone()); + Ok(Self { + connection_maker, + ctx, + }) } } diff --git a/libsql-server/src/http/user/extract.rs b/libsql-server/src/http/user/extract.rs index 89dd529150..5ea9d95d63 100644 --- a/libsql-server/src/http/user/extract.rs +++ b/libsql-server/src/http/user/extract.rs @@ -1,12 +1,13 @@ use axum::extract::FromRequestParts; -use crate::{ - auth::{Auth, Jwt}, - connection::RequestContext, -}; +use crate::connection::RequestContext; -use super::{db_factory, AppState}; +use super::{authenticate_for_namespace, db_factory, AppState}; +/// Request context for header-routed endpoints: the namespace comes from the +/// `x-namespace` / `Host` headers. Path-routed endpoints must use +/// [`db_factory::PathNamespaceRequest`] instead, so the authorized namespace is the one in +/// the URL. #[async_trait::async_trait] impl FromRequestParts for RequestContext { type Rejection = crate::error::Error; @@ -20,21 +21,10 @@ impl FromRequestParts for RequestContext { state.disable_default_namespace, state.disable_namespaces, )?; - // todo dupe #auth - let namespace_jwt_keys = state - .namespaces - .with(namespace.clone(), |ns| ns.jwt_keys()) - .await??; - - let auth = namespace_jwt_keys - .map(Jwt::new) - .map(Auth::new) - .unwrap_or_else(|| state.user_auth_strategy.clone()); - - let context = super::build_context(&parts.headers, &auth.user_strategy.required_fields()); + let auth = authenticate_for_namespace(parts, state, &namespace).await?; Ok(Self::new( - auth.authenticate(context)?, + auth, namespace, state.namespaces.meta_store().clone(), )) diff --git a/libsql-server/src/http/user/mod.rs b/libsql-server/src/http/user/mod.rs index 1575a3574b..8c184a0a80 100644 --- a/libsql-server/src/http/user/mod.rs +++ b/libsql-server/src/http/user/mod.rs @@ -38,11 +38,11 @@ use tower_http::{compression::CompressionLayer, cors}; use crate::auth::{Auth, AuthError, Authenticated, Jwt, Permission, UserAuthContext}; use crate::connection::{Connection, RequestContext}; use crate::error::Error; -use crate::http::user::db_factory::MakeConnectionExtractorPath; +use crate::http::user::db_factory::PathNamespaceRequest; use crate::http::user::timing::timings_middleware; use crate::http::user::types::HttpQuery; use crate::metrics::LEGACY_HTTP_CALL; -use crate::namespace::NamespaceStore; +use crate::namespace::{NamespaceName, NamespaceStore}; use crate::net::Accept; use crate::query::{self, Query}; use crate::query_analysis::{predict_final_state, Statement, TxnStatus}; @@ -203,10 +203,18 @@ async fn handle_fallback() -> impl IntoResponse { (StatusCode::NOT_FOUND).into_response() } +/// Hrana pipeline endpoint routed by URL path (`/dev/:namespace/v:version/pipeline`). +/// +/// Both the connection and the request context come from [`PathNamespaceRequest`], which +/// derives them from the `:namespace` path segment. They must not be extracted +/// independently: the header-based [`RequestContext`] extractor resolves the namespace +/// from `x-namespace`/`Host`, which need not match the path. async fn handle_hrana_pipeline( AxumState(state): AxumState, - MakeConnectionExtractorPath(connection_maker): MakeConnectionExtractorPath, - ctx: RequestContext, + PathNamespaceRequest { + connection_maker, + ctx, + }: PathNamespaceRequest, axum::extract::Path((_, version)): axum::extract::Path<(String, String)>, req: Request, ) -> Result, Error> { @@ -472,23 +480,36 @@ impl FromRequestParts for Authenticated { state.disable_default_namespace, state.disable_namespaces, )?; - // todo dupe #auth - let namespace_jwt_keys = state - .namespaces - .with(ns.clone(), |ns| ns.jwt_keys()) - .await??; - - let auth = namespace_jwt_keys - .map(Jwt::new) - .map(Auth::new) - .unwrap_or_else(|| state.user_auth_strategy.clone()); - - let context = build_context(&parts.headers, &auth.user_strategy.required_fields()); - - Ok(auth.authenticate(context)?) + authenticate_for_namespace(parts, state, &ns).await } } +/// Authenticate a request against `namespace`: use the namespace's own JWT keys when it has +/// them, otherwise the server-wide user auth strategy. +/// +/// Every extractor that produces an [`Authenticated`] or a [`RequestContext`] goes through +/// here, so the namespace whose keys verify the token is always the one the caller is about +/// to authorize against. +pub(super) async fn authenticate_for_namespace( + parts: &Parts, + state: &AppState, + namespace: &NamespaceName, +) -> Result { + let namespace_jwt_keys = state + .namespaces + .with(namespace.clone(), |ns| ns.jwt_keys()) + .await??; + + let auth = namespace_jwt_keys + .map(Jwt::new) + .map(Auth::new) + .unwrap_or_else(|| state.user_auth_strategy.clone()); + + let context = build_context(&parts.headers, &auth.user_strategy.required_fields()); + + Ok(auth.authenticate(context)?) +} + fn build_context( headers: &hyper::HeaderMap, required_fields: &Vec<&'static str>, diff --git a/libsql-server/tests/standalone/auth.rs b/libsql-server/tests/standalone/auth.rs index 6ad1dbdc9c..96a658c913 100644 --- a/libsql-server/tests/standalone/auth.rs +++ b/libsql-server/tests/standalone/auth.rs @@ -1,5 +1,6 @@ use std::time::Duration; +use hyper::header::HeaderName; use libsql::Database; use crate::common::auth::{encode, key_pair}; @@ -50,6 +51,107 @@ fn jwt_auth_namespace_access() { sim.run().unwrap(); } +/// The path-routed Hrana endpoint authorizes statements against the namespace in the URL. +/// A namespace header on the same request must not change which namespace is authorized. +#[test] +fn jwt_auth_path_route_authorizes_path_namespace() { + let mut sim = turmoil::Builder::new() + .simulation_duration(Duration::from_secs(1000)) + .build(); + + sim.host("primary", make_standalone_server); + + sim.client("test", async { + let client = Client::new(); + + let (enc, jwt_key) = key_pair(); + + for ns in ["mine", "other"] { + assert!(client + .post( + &format!("http://primary:9090/v1/namespaces/{ns}/create"), + serde_json::json!({ "jwt_key": jwt_key }) + ) + .await + .unwrap() + .status() + .is_success()); + } + + let claims = serde_json::json!({ + "p": { + "rw": { "ns": ["mine"] }, + "ro": { "ns": ["other"] }, + }, + }); + let token = encode(&claims, &enc); + let bearer = format!("Bearer {token}"); + let create_table = serde_json::json!({ + "requests": [ + { "type": "execute", "stmt": { "sql": "create table t (x)" } }, + { "type": "close" } + ] + }); + + // The token may write `mine` through the path route. + let resp = client + .post_with_headers( + "http://primary:8080/dev/mine/v3/pipeline", + &[(hyper::header::AUTHORIZATION, bearer.as_str())], + &create_table, + ) + .await + .unwrap(); + assert!(resp.status().is_success()); + let body = resp.json_value().await.unwrap(); + assert_eq!( + body["results"][0]["type"], "ok", + "write to own namespace rejected: {body}" + ); + + // `other` in the path, with a header naming a different namespace. + let resp = client + .post_with_headers( + "http://primary:8080/dev/other/v3/pipeline", + &[ + (hyper::header::AUTHORIZATION, bearer.as_str()), + (HeaderName::from_static("x-namespace"), "mine"), + ], + &create_table, + ) + .await + .unwrap(); + assert_eq!( + resp.status(), + hyper::StatusCode::FORBIDDEN, + "write to other through the path route was not rejected" + ); + let body = resp.body_string().await.unwrap(); + assert!( + body.contains("Write permission to namespace other"), + "unexpected error: {body}" + ); + + // The other namespace is untouched. + let other_db = Database::open_remote_with_connector( + "http://other.primary:8080", + &token, + TurmoilConnector, + )?; + let other_conn = other_db.connect().unwrap(); + let mut rows = other_conn + .query("select count(*) from sqlite_master where name = 't'", ()) + .await + .unwrap(); + let count = rows.next().await.unwrap().unwrap().get::(0).unwrap(); + assert_eq!(count, 0, "table was created in the other namespace"); + + Ok(()) + }); + + sim.run().unwrap(); +} + #[test] fn jwt_auth_gid_scope() { std::env::set_var("LIBSQL_GID", "my_gid");