From eb93df149927a5221055428a082d3c81cb604059 Mon Sep 17 00:00:00 2001 From: Oliver Seifert Date: Sat, 7 Jun 2025 17:38:57 +0200 Subject: [PATCH 1/2] feat(auth): Implement issuer allow-list and required-auth flag - Adds `--allowed-oidc-issuer` CLI flag to restrict which OIDC providers are trusted. If not set, a warning is logged. - Adds `--auth-required` CLI flag to disable anonymous user creation, requiring all connections to provide a valid JWT. - Fixes a bug where requests with invalid tokens would fall back to creating an anonymous user instead of being rejected. --- crates/client-api/src/auth.rs | 60 ++++++++++++++++----- crates/client-api/src/lib.rs | 7 ++- crates/core/src/auth/token_validation.rs | 63 ++++++++++++++++++++-- crates/standalone/src/lib.rs | 21 ++++++-- crates/standalone/src/subcommands/start.rs | 23 +++++++- crates/testing/src/modules.rs | 13 +++-- 6 files changed, 160 insertions(+), 27 deletions(-) diff --git a/crates/client-api/src/auth.rs b/crates/client-api/src/auth.rs index 7437e6ef8cf..3dca2677669 100644 --- a/crates/client-api/src/auth.rs +++ b/crates/client-api/src/auth.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::time::{Duration, SystemTime}; use axum::extract::{Query, Request, State}; @@ -197,8 +198,16 @@ pub struct JwtKeyAuthProvider { pub type DefaultJwtAuthProvider = JwtKeyAuthProvider; // Create a new AuthEnvironment using the default caching validator. -pub fn default_auth_environment(keys: JwtKeys, local_issuer: String) -> JwtKeyAuthProvider { - let validator = new_validator(keys.public.clone(), local_issuer.clone()); +pub fn default_auth_environment( + keys: JwtKeys, + local_issuer: String, + allowed_oidc_issuers: Option> +) -> JwtKeyAuthProvider { + let validator = new_validator( + keys.public.clone(), + local_issuer.clone(), + allowed_oidc_issuers, + ); JwtKeyAuthProvider::new(keys, local_issuer, validator) } @@ -269,23 +278,36 @@ impl axum::extract::FromRequestParts for Space type Rejection = AuthorizationRejection; async fn from_request_parts(parts: &mut request::Parts, state: &S) -> Result { let Some(creds) = SpacetimeCreds::from_request_parts(parts)? else { + // No token was provided at all. This is a legitimate anonymous user. return Ok(Self { auth: None }); }; - let claims = state + // Credentials ARE present. They MUST now be validated successfully. + let validation_result = state .jwt_auth_provider() .validator() .validate_token(&creds.token) - .await - .map_err(AuthorizationRejection::Custom)?; - - let auth = SpacetimeAuth { - creds, - identity: claims.identity, - subject: claims.subject, - issuer: claims.issuer, - }; - Ok(Self { auth: Some(auth) }) + .await; + + match validation_result { + Ok(claims) => { + // The token is valid. Create the auth struct. + let auth = SpacetimeAuth { + creds, + identity: claims.identity, + subject: claims.subject, + issuer: claims.issuer, + }; + Ok(Self { auth: Some(auth) }) + } + Err(validation_error) => { + // The token was present but INVALID. + // This is a hard failure. We must reject the request. + // We are explicitly returning an Err that will halt the request + // with a 401 Unauthorized status. + Err(AuthorizationRejection::Custom(validation_error)) + } + } } } @@ -423,7 +445,17 @@ pub async fn anon_auth_middleware( mut req: Request, next: Next, ) -> axum::response::Result { - let auth = auth.get_or_create(&worker_ctx).await?; + let auth = match auth.get() { + None => { + if worker_ctx.auth_required() { + log::warn!("Rejecting anonymous connection because --auth-required is set."); + Err(AuthorizationRejection::Required.into()) + } else { + SpacetimeAuth::alloc(&worker_ctx).await + } + } + Some(authorization) => {Ok(authorization)} + }?; req.extensions_mut().insert(auth.clone()); let resp = next.run(req).await; Ok((auth.into_headers(), resp)) diff --git a/crates/client-api/src/lib.rs b/crates/client-api/src/lib.rs index 37b120ed17c..9c817ddf393 100644 --- a/crates/client-api/src/lib.rs +++ b/crates/client-api/src/lib.rs @@ -30,9 +30,10 @@ pub mod util; pub trait NodeDelegate: Send + Sync { fn gather_metrics(&self) -> Vec; fn client_actor_index(&self) -> &ClientActorIndex; - + type JwtAuthProviderT: auth::JwtAuthProvider; fn jwt_auth_provider(&self) -> &Self::JwtAuthProviderT; + fn auth_required(&self) -> bool; /// Return the leader [`Host`] of `database_id`. /// /// Returns `None` if the current leader is not hosted by this node. @@ -363,7 +364,9 @@ impl NodeDelegate for Arc { fn jwt_auth_provider(&self) -> &Self::JwtAuthProviderT { (**self).jwt_auth_provider() } - + fn auth_required(&self) -> bool { + (**self).auth_required() + } async fn leader(&self, database_id: u64) -> anyhow::Result> { (**self).leader(database_id).await } diff --git a/crates/core/src/auth/token_validation.rs b/crates/core/src/auth/token_validation.rs index 680b139d20c..ff9d5581fa7 100644 --- a/crates/core/src/auth/token_validation.rs +++ b/crates/core/src/auth/token_validation.rs @@ -10,6 +10,7 @@ pub use jsonwebtoken::{DecodingKey, EncodingKey}; use jwks::Jwks; use lazy_static::lazy_static; use serde::Serialize; +use std::collections::HashSet; use std::sync::Arc; use std::time::Duration; use thiserror; @@ -118,13 +119,36 @@ where } } -pub type DefaultValidator = FullTokenValidator; +pub type DefaultValidator = FullTokenValidator< + AllowedIssuersValidator +>; + +pub fn new_validator( + local_key: DecodingKey, + local_issuer: String, + // Accept an optional list of allowed OIDC issuers. + allowed_oidc_issuers: Option>, +) -> DefaultValidator { + // If the allow-list is empty or not provided, log a prominent security warning. + if allowed_oidc_issuers.is_none() || allowed_oidc_issuers.as_ref().unwrap().is_empty() { + log::warn!( + "SECURITY WARNING: No OIDC issuer allow-list is configured. \ + Any valid OIDC token from ANY issuer will be accepted. \ + This is NOT recommended for production environments. \ + Please configure 'allowed_oidc_issuers'." + ); + } + + let caching_validator = CachingOidcTokenValidator::get_default(); + let oidc_validator = AllowedIssuersValidator { + allowed_issuers: allowed_oidc_issuers, + inner_validator: caching_validator, + }; -pub fn new_validator(local_key: DecodingKey, local_issuer: String) -> FullTokenValidator { FullTokenValidator { local_key, local_issuer, - oidc_validator: CachingOidcTokenValidator::get_default(), + oidc_validator, } } @@ -197,6 +221,39 @@ impl CachingOidcTokenValidator { } } +/// A validator that wraps another validator and adds an issuer allow-list. +/// If `allowed_issuers` is `None`, it will permit any issuer (with a warning). +pub struct AllowedIssuersValidator { + // We use an Option to clearly distinguish between "enforce this list" + // and "allow any issuer". + allowed_issuers: Option>, + inner_validator: T, +} + +#[async_trait] +impl TokenValidator for AllowedIssuersValidator { + async fn validate_token( + &self, + token: &str, + ) -> Result { + if let Some(allowed) = &self.allowed_issuers { + // Get the issuer without full validation first. + let issuer = get_raw_issuer(token)?; + + // Check if the issuer is in our allow-list. + if !allowed.contains(&issuer) { + log::warn!("Token validation failed: issuer '{}' is not in the allowed list.", issuer); + return Err(TokenValidationError::Other(anyhow::anyhow!( + "Issuer is not in the allowed list" + ))); + } + } + + // If the check passes (or if no list is configured), proceed with the inner validator. + self.inner_validator.validate_token(token).await + } +} + // Jwks fetcher for the async cache. struct KeyFetcher; diff --git a/crates/standalone/src/lib.rs b/crates/standalone/src/lib.rs index 6021694c9a7..6b600e88b88 100644 --- a/crates/standalone/src/lib.rs +++ b/crates/standalone/src/lib.rs @@ -3,6 +3,7 @@ pub mod subcommands; pub mod util; pub mod version; +use std::collections::HashSet; use crate::control_db::ControlDb; use crate::subcommands::{extract_schema, start}; use anyhow::{ensure, Context, Ok}; @@ -40,6 +41,7 @@ pub struct StandaloneEnv { metrics_registry: prometheus::Registry, _pid_file: PidFile, auth_provider: auth::DefaultJwtAuthProvider, + pub auth_required: bool, } impl StandaloneEnv { @@ -48,6 +50,8 @@ impl StandaloneEnv { certs: &CertificateAuthority, data_dir: Arc, db_cores: JobCores, + allowed_oidc_issuers: Option>, + auth_required: bool, ) -> anyhow::Result> { let _pid_file = data_dir.pid_file()?; let meta_path = data_dir.metadata_toml(); @@ -75,7 +79,11 @@ impl StandaloneEnv { let client_actor_index = ClientActorIndex::new(); let jwt_keys = certs.get_or_create_keys()?; - let auth_env = auth::default_auth_environment(jwt_keys, LOCALHOST.to_owned()); + let auth_env = auth::default_auth_environment( + jwt_keys, + LOCALHOST.to_owned(), + allowed_oidc_issuers, + ); let metrics_registry = prometheus::Registry::new(); metrics_registry.register(Box::new(&*WORKER_METRICS)).unwrap(); @@ -90,6 +98,7 @@ impl StandaloneEnv { metrics_registry, _pid_file, auth_provider: auth_env, + auth_required, })) } @@ -140,6 +149,10 @@ impl NodeDelegate for StandaloneEnv { &self.auth_provider } + fn auth_required(&self) -> bool { + self.auth_required + } + async fn leader(&self, database_id: u64) -> anyhow::Result> { let leader = match self.control_db.get_leader_replica_by_database(database_id) { Some(leader) => leader, @@ -519,9 +532,11 @@ mod tests { page_pool_max_size: None, }; - let _env = StandaloneEnv::init(config, &ca, data_dir.clone(), Default::default()).await?; + // We pass `None` here to test the default behavior (allow all). + // TODO: Test with list of allowed OIDC issuers + let _env = StandaloneEnv::init(config, &ca, data_dir.clone(), Default::default(), None, false).await?; // Ensure that we have a lock. - assert!(StandaloneEnv::init(config, &ca, data_dir.clone(), Default::default()) + assert!(StandaloneEnv::init(config, &ca, data_dir.clone(), Default::default(), None, false) .await .is_err()); diff --git a/crates/standalone/src/subcommands/start.rs b/crates/standalone/src/subcommands/start.rs index 54ae5a1cefa..831f509ae86 100644 --- a/crates/standalone/src/subcommands/start.rs +++ b/crates/standalone/src/subcommands/start.rs @@ -1,10 +1,11 @@ +use std::collections::HashSet; use std::sync::Arc; use crate::StandaloneEnv; use anyhow::Context; use axum::extract::DefaultBodyLimit; use clap::ArgAction::SetTrue; -use clap::{Arg, ArgMatches}; +use clap::{Arg, ArgAction, ArgMatches}; use spacetimedb::config::{CertificateAuthority, ConfigFile}; use spacetimedb::db::{Config, Storage}; use spacetimedb::startup::{self, TracingOptions}; @@ -73,6 +74,20 @@ pub fn cli() -> clap::Command { "The maximum size of the page pool in bytes. Should be a multiple of 64KiB. The default is 8GiB.", ), ) + .arg( + Arg::new("allowed-oidc-issuer") + .long("allowed-oidc-issuer") + .help("Allow tokens from this OIDC issuer. Can be specified multiple times. If not set, all issuers are allowed (NOT RECOMMENDED FOR PRODUCTION).") + // This allows the user to provide the flag multiple times, + // e.g., --allowed-oidc-issuer https://a.com --allowed-oidc-issuer https://b.com + .action(ArgAction::Append) + ) + .arg( + Arg::new("auth-required") + .long("auth-required") + .action(SetTrue) + .help("If specified, anonymous user creation is disabled and all connections must provide a valid JWT.") + ) // .after_help("Run `spacetime help start` for more detailed information.") } @@ -104,6 +119,10 @@ pub async fn exec(args: &ArgMatches, db_cores: JobCores) -> anyhow::Result<()> { storage, page_pool_max_size, }; + let allowed_oidc_issuers: Option> = + args.get_many::("allowed-oidc-issuer") + .map(|vals| vals.cloned().collect()); + let auth_required = args.get_flag("auth-required"); banner(); let exe_name = std::env::current_exe()?; @@ -144,7 +163,7 @@ pub async fn exec(args: &ArgMatches, db_cores: JobCores) -> anyhow::Result<()> { .context("cannot omit --jwt-{pub,priv}-key-path when those options are not specified in config.toml")?; let data_dir = Arc::new(data_dir.clone()); - let ctx = StandaloneEnv::init(db_config, &certs, data_dir, db_cores).await?; + let ctx = StandaloneEnv::init(db_config, &certs, data_dir, db_cores, allowed_oidc_issuers, auth_required).await?; worker_metrics::spawn_jemalloc_stats(listen_addr.clone()); worker_metrics::spawn_tokio_stats(listen_addr.clone()); worker_metrics::spawn_page_pool_stats(listen_addr.clone(), ctx.page_pool().clone()); diff --git a/crates/testing/src/modules.rs b/crates/testing/src/modules.rs index 22c55b9af8a..93397fc9b19 100644 --- a/crates/testing/src/modules.rs +++ b/crates/testing/src/modules.rs @@ -179,9 +179,16 @@ impl CompiledModule { let certs = CertificateAuthority::in_cli_config_dir(&paths.cli_config_dir); let env = - spacetimedb_standalone::StandaloneEnv::init(config, &certs, paths.data_dir.into(), Default::default()) - .await - .unwrap(); + spacetimedb_standalone::StandaloneEnv::init( + config, + &certs, + paths.data_dir.into(), + Default::default(), + None, + false + ) + .await + .unwrap(); // TODO: Fix this when we update identity generation. let identity = Identity::ZERO; let db_identity = SpacetimeAuth::alloc(&env).await.unwrap().identity; From b9bc404d0d00a76160320e36f791b96a36da3287 Mon Sep 17 00:00:00 2001 From: Oliver Seifert Date: Mon, 9 Jun 2025 13:51:00 +0200 Subject: [PATCH 2/2] fix: formatted all files --- crates/client-api/src/auth.rs | 18 +++++------------- crates/client-api/src/lib.rs | 2 +- crates/core/src/auth/token_validation.rs | 16 +++++++--------- crates/standalone/src/lib.rs | 18 ++++++++---------- crates/standalone/src/subcommands/start.rs | 16 ++++++++++++---- crates/testing/src/modules.rs | 21 ++++++++++----------- 6 files changed, 43 insertions(+), 48 deletions(-) diff --git a/crates/client-api/src/auth.rs b/crates/client-api/src/auth.rs index 3dca2677669..3b3ac2fc2b8 100644 --- a/crates/client-api/src/auth.rs +++ b/crates/client-api/src/auth.rs @@ -199,15 +199,11 @@ pub type DefaultJwtAuthProvider = JwtKeyAuthProvider; // Create a new AuthEnvironment using the default caching validator. pub fn default_auth_environment( - keys: JwtKeys, + keys: JwtKeys, local_issuer: String, - allowed_oidc_issuers: Option> + allowed_oidc_issuers: Option>, ) -> JwtKeyAuthProvider { - let validator = new_validator( - keys.public.clone(), - local_issuer.clone(), - allowed_oidc_issuers, - ); + let validator = new_validator(keys.public.clone(), local_issuer.clone(), allowed_oidc_issuers); JwtKeyAuthProvider::new(keys, local_issuer, validator) } @@ -283,11 +279,7 @@ impl axum::extract::FromRequestParts for Space }; // Credentials ARE present. They MUST now be validated successfully. - let validation_result = state - .jwt_auth_provider() - .validator() - .validate_token(&creds.token) - .await; + let validation_result = state.jwt_auth_provider().validator().validate_token(&creds.token).await; match validation_result { Ok(claims) => { @@ -454,7 +446,7 @@ pub async fn anon_auth_middleware( SpacetimeAuth::alloc(&worker_ctx).await } } - Some(authorization) => {Ok(authorization)} + Some(authorization) => Ok(authorization), }?; req.extensions_mut().insert(auth.clone()); let resp = next.run(req).await; diff --git a/crates/client-api/src/lib.rs b/crates/client-api/src/lib.rs index 9c817ddf393..c8dbe3f27c0 100644 --- a/crates/client-api/src/lib.rs +++ b/crates/client-api/src/lib.rs @@ -30,7 +30,7 @@ pub mod util; pub trait NodeDelegate: Send + Sync { fn gather_metrics(&self) -> Vec; fn client_actor_index(&self) -> &ClientActorIndex; - + type JwtAuthProviderT: auth::JwtAuthProvider; fn jwt_auth_provider(&self) -> &Self::JwtAuthProviderT; fn auth_required(&self) -> bool; diff --git a/crates/core/src/auth/token_validation.rs b/crates/core/src/auth/token_validation.rs index ff9d5581fa7..12299e24713 100644 --- a/crates/core/src/auth/token_validation.rs +++ b/crates/core/src/auth/token_validation.rs @@ -119,12 +119,10 @@ where } } -pub type DefaultValidator = FullTokenValidator< - AllowedIssuersValidator ->; +pub type DefaultValidator = FullTokenValidator>; pub fn new_validator( - local_key: DecodingKey, + local_key: DecodingKey, local_issuer: String, // Accept an optional list of allowed OIDC issuers. allowed_oidc_issuers: Option>, @@ -232,17 +230,17 @@ pub struct AllowedIssuersValidator { #[async_trait] impl TokenValidator for AllowedIssuersValidator { - async fn validate_token( - &self, - token: &str, - ) -> Result { + async fn validate_token(&self, token: &str) -> Result { if let Some(allowed) = &self.allowed_issuers { // Get the issuer without full validation first. let issuer = get_raw_issuer(token)?; // Check if the issuer is in our allow-list. if !allowed.contains(&issuer) { - log::warn!("Token validation failed: issuer '{}' is not in the allowed list.", issuer); + log::warn!( + "Token validation failed: issuer '{}' is not in the allowed list.", + issuer + ); return Err(TokenValidationError::Other(anyhow::anyhow!( "Issuer is not in the allowed list" ))); diff --git a/crates/standalone/src/lib.rs b/crates/standalone/src/lib.rs index 6b600e88b88..3ede0fcc471 100644 --- a/crates/standalone/src/lib.rs +++ b/crates/standalone/src/lib.rs @@ -3,7 +3,6 @@ pub mod subcommands; pub mod util; pub mod version; -use std::collections::HashSet; use crate::control_db::ControlDb; use crate::subcommands::{extract_schema, start}; use anyhow::{ensure, Context, Ok}; @@ -29,6 +28,7 @@ use spacetimedb_client_api_messages::name::{DomainName, InsertDomainResult, Regi use spacetimedb_paths::server::{ModuleLogsDir, PidFile, ServerDataDir}; use spacetimedb_paths::standalone::StandaloneDataDirExt; use spacetimedb_table::page_pool::PagePool; +use std::collections::HashSet; use std::sync::Arc; pub use spacetimedb_client_api::routes::subscribe::{BIN_PROTOCOL, TEXT_PROTOCOL}; @@ -79,11 +79,7 @@ impl StandaloneEnv { let client_actor_index = ClientActorIndex::new(); let jwt_keys = certs.get_or_create_keys()?; - let auth_env = auth::default_auth_environment( - jwt_keys, - LOCALHOST.to_owned(), - allowed_oidc_issuers, - ); + let auth_env = auth::default_auth_environment(jwt_keys, LOCALHOST.to_owned(), allowed_oidc_issuers); let metrics_registry = prometheus::Registry::new(); metrics_registry.register(Box::new(&*WORKER_METRICS)).unwrap(); @@ -152,7 +148,7 @@ impl NodeDelegate for StandaloneEnv { fn auth_required(&self) -> bool { self.auth_required } - + async fn leader(&self, database_id: u64) -> anyhow::Result> { let leader = match self.control_db.get_leader_replica_by_database(database_id) { Some(leader) => leader, @@ -536,9 +532,11 @@ mod tests { // TODO: Test with list of allowed OIDC issuers let _env = StandaloneEnv::init(config, &ca, data_dir.clone(), Default::default(), None, false).await?; // Ensure that we have a lock. - assert!(StandaloneEnv::init(config, &ca, data_dir.clone(), Default::default(), None, false) - .await - .is_err()); + assert!( + StandaloneEnv::init(config, &ca, data_dir.clone(), Default::default(), None, false) + .await + .is_err() + ); Ok(()) } diff --git a/crates/standalone/src/subcommands/start.rs b/crates/standalone/src/subcommands/start.rs index 831f509ae86..115d0d1ae0b 100644 --- a/crates/standalone/src/subcommands/start.rs +++ b/crates/standalone/src/subcommands/start.rs @@ -119,9 +119,9 @@ pub async fn exec(args: &ArgMatches, db_cores: JobCores) -> anyhow::Result<()> { storage, page_pool_max_size, }; - let allowed_oidc_issuers: Option> = - args.get_many::("allowed-oidc-issuer") - .map(|vals| vals.cloned().collect()); + let allowed_oidc_issuers: Option> = args + .get_many::("allowed-oidc-issuer") + .map(|vals| vals.cloned().collect()); let auth_required = args.get_flag("auth-required"); banner(); @@ -163,7 +163,15 @@ pub async fn exec(args: &ArgMatches, db_cores: JobCores) -> anyhow::Result<()> { .context("cannot omit --jwt-{pub,priv}-key-path when those options are not specified in config.toml")?; let data_dir = Arc::new(data_dir.clone()); - let ctx = StandaloneEnv::init(db_config, &certs, data_dir, db_cores, allowed_oidc_issuers, auth_required).await?; + let ctx = StandaloneEnv::init( + db_config, + &certs, + data_dir, + db_cores, + allowed_oidc_issuers, + auth_required, + ) + .await?; worker_metrics::spawn_jemalloc_stats(listen_addr.clone()); worker_metrics::spawn_tokio_stats(listen_addr.clone()); worker_metrics::spawn_page_pool_stats(listen_addr.clone(), ctx.page_pool().clone()); diff --git a/crates/testing/src/modules.rs b/crates/testing/src/modules.rs index 93397fc9b19..4706580b9d4 100644 --- a/crates/testing/src/modules.rs +++ b/crates/testing/src/modules.rs @@ -178,17 +178,16 @@ impl CompiledModule { }; let certs = CertificateAuthority::in_cli_config_dir(&paths.cli_config_dir); - let env = - spacetimedb_standalone::StandaloneEnv::init( - config, - &certs, - paths.data_dir.into(), - Default::default(), - None, - false - ) - .await - .unwrap(); + let env = spacetimedb_standalone::StandaloneEnv::init( + config, + &certs, + paths.data_dir.into(), + Default::default(), + None, + false, + ) + .await + .unwrap(); // TODO: Fix this when we update identity generation. let identity = Identity::ZERO; let db_identity = SpacetimeAuth::alloc(&env).await.unwrap().identity;