diff --git a/src/error.rs b/src/error.rs --- a/src/error.rs +++ b/src/error.rs @@ -52,6 +52,7 @@ BadGateway(String), BadRequest(String), Conflict(String), + FeatureDisabled(String), Forbidden(String), InsufficientPermissions(String), Internal(String), @@ -78,6 +79,7 @@ AppError::BadGateway(msg) => write!(f, "bad gateway: {msg}"), AppError::BadRequest(msg) => write!(f, "bad request: {msg}"), AppError::Conflict(msg) => write!(f, "conflict: {msg}"), + AppError::FeatureDisabled(msg) => write!(f, "feature disabled: {msg}"), AppError::Forbidden(msg) => write!(f, "forbidden: {msg}"), AppError::InsufficientPermissions(perm) => write!(f, "Missing permission: {perm}"), AppError::Internal(msg) => write!(f, "internal error: {msg}"), @@ -142,6 +144,13 @@ }); (status, axum::Json(body)).into_response() } + AppError::FeatureDisabled(msg) => { + let body = serde_json::json!({ + "error": "FeatureDisabled", + "message": msg, + }); + (StatusCode::NOT_FOUND, axum::Json(body)).into_response() + } AppError::InsufficientPermissions(perm) => { let body = serde_json::json!({ "error": "InsufficientPermissions", @@ -181,6 +190,7 @@ AppError::NotFound(msg) => (StatusCode::NOT_FOUND, msg.clone()), AppError::PdsError(..) | AppError::AuthDpopNonce(..) + | AppError::FeatureDisabled(..) | AppError::InsufficientPermissions(..) | AppError::RateLimited { .. } | AppError::ScriptError { .. } => unreachable!(), @@ -276,6 +286,15 @@ } #[tokio::test] + async fn feature_disabled_returns_404() { + let (status, body) = + response_parts(AppError::FeatureDisabled("spaces not enabled".into())).await; + assert_eq!(status, StatusCode::NOT_FOUND); + assert_eq!(body["error"], "FeatureDisabled"); + assert_eq!(body["message"], "spaces not enabled"); + } + + #[tokio::test] async fn not_found_returns_404() { let (status, body) = response_parts(AppError::NotFound("no such thing".into())).await; assert_eq!(status, StatusCode::NOT_FOUND); @@ -342,6 +361,10 @@ assert_eq!( AppError::Internal("z".into()).to_string(), "internal error: z" + ); + assert_eq!( + AppError::FeatureDisabled("x".into()).to_string(), + "feature disabled: x" ); assert_eq!(AppError::NotFound("w".into()).to_string(), "not found: w"); assert_eq!( diff --git a/src/feature_flags.rs b/src/feature_flags.rs new file mode 100644 --- /dev/null +++ b/src/feature_flags.rs @@ -0,0 +1,45 @@ +use sqlx::AnyPool; + +use crate::admin::settings::get_setting; +use crate::db::DatabaseBackend; + +pub struct FeatureFlag; + +impl FeatureFlag { + pub const SPACES_ENABLED: &str = "feature.spaces_enabled"; +} + +pub async fn is_enabled(pool: &AnyPool, key: &str, backend: DatabaseBackend) -> bool { + get_setting(pool, key, backend) + .await + .map(|v| v.eq_ignore_ascii_case("true")) + .unwrap_or(false) +} + +#[derive(serde::Serialize)] +pub struct FeatureFlagStatus { + pub key: String, + pub name: String, + pub description: String, + pub enabled: bool, +} + +pub async fn list_flags(pool: &AnyPool, backend: DatabaseBackend) -> Vec { + let all_flags = [( + FeatureFlag::SPACES_ENABLED, + "Permissioned Spaces", + "Collaborative data spaces with granular permissions, membership, and invites.", + )]; + + let mut result = Vec::new(); + for (key, name, description) in all_flags { + let enabled = is_enabled(pool, key, backend).await; + result.push(FeatureFlagStatus { + key: key.to_string(), + name: name.to_string(), + description: description.to_string(), + enabled, + }); + } + result +} diff --git a/src/feature_middleware.rs b/src/feature_middleware.rs new file mode 100644 --- /dev/null +++ b/src/feature_middleware.rs @@ -0,0 +1,35 @@ +use axum::extract::{Request, State}; +use axum::middleware::Next; +use axum::response::Response; + +use crate::AppState; +use crate::error::AppError; + +async fn require_feature( + flag_key: &'static str, + State(state): State, + req: Request, + next: Next, +) -> Result { + if !crate::feature_flags::is_enabled(&state.db, flag_key, state.db_backend).await { + return Err(AppError::FeatureDisabled(format!( + "The feature '{}' is not currently enabled on this instance", + flag_key + ))); + } + Ok(next.run(req).await) +} + +pub async fn require_spaces( + state: State, + req: Request, + next: Next, +) -> Result { + require_feature( + crate::feature_flags::FeatureFlag::SPACES_ENABLED, + state, + req, + next, + ) + .await +} diff --git a/src/lib.rs b/src/lib.rs --- a/src/lib.rs +++ b/src/lib.rs @@ -10,6 +10,8 @@ pub mod error; pub mod event_log; pub mod external_auth; +pub mod feature_flags; +pub mod feature_middleware; pub mod jetstream; pub mod labeler; pub mod lexicon; diff --git a/src/server.rs b/src/server.rs --- a/src/server.rs +++ b/src/server.rs @@ -62,7 +62,12 @@ let serve_dir = ServeDir::new(&static_dir).not_found_service(spa_fallback); let domain_routes = Router::new() - .merge(crate::spaces::routes::space_routes()) + .merge( + crate::spaces::routes::space_routes().layer(axum::middleware::from_fn_with_state( + state.clone(), + crate::feature_middleware::require_spaces, + )), + ) .nest("/auth", crate::auth::routes::routes()) .nest("/external-auth", crate::external_auth::routes()) .nest("/oauth", crate::oauth::routes::routes()) @@ -188,6 +193,13 @@ _ => env!("CARGO_PKG_VERSION"), }; + let spaces_enabled = crate::feature_flags::is_enabled( + pool, + crate::feature_flags::FeatureFlag::SPACES_ENABLED, + backend, + ) + .await; + Json(serde_json::json!({ "public_url": domain_url, "version": version, @@ -199,6 +211,9 @@ "default_rate_limit_refill_rate": state.config.default_rate_limit_refill_rate, "app_name": app_name, "logo_url": logo_url, + "features": { + "spaces": spaces_enabled, + }, })) } diff --git a/tests/e2e_feature_flags.rs b/tests/e2e_feature_flags.rs new file mode 100644 --- /dev/null +++ b/tests/e2e_feature_flags.rs @@ -0,0 +1,302 @@ +mod common; + +use axum::body::Body; +use axum::http::{Method, Request, StatusCode}; +use http_body_util::BodyExt; +use serde_json::{Value, json}; +use serial_test::serial; +use tower::ServiceExt; + +use common::app::TestApp; + +async fn json_body(resp: axum::response::Response) -> Value { + let body = resp.into_body().collect().await.unwrap().to_bytes(); + serde_json::from_slice(&body).unwrap() +} + +fn admin_get( + uri: &str, + cookie: (axum::http::HeaderName, axum::http::HeaderValue), +) -> Request { + Request::builder() + .uri(uri) + .header(cookie.0, cookie.1) + .body(Body::empty()) + .unwrap() +} + +fn admin_put( + uri: &str, + cookie: (axum::http::HeaderName, axum::http::HeaderValue), + body: &Value, +) -> Request { + Request::builder() + .method(Method::PUT) + .uri(uri) + .header(cookie.0, cookie.1) + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(body).unwrap())) + .unwrap() +} + +fn admin_delete( + uri: &str, + cookie: (axum::http::HeaderName, axum::http::HeaderValue), +) -> Request { + Request::builder() + .method(Method::DELETE) + .uri(uri) + .header(cookie.0, cookie.1) + .body(Body::empty()) + .unwrap() +} + +#[tokio::test] +#[serial] +#[ignore] +async fn space_routes_blocked_when_flag_disabled() { + let app = TestApp::new().await; + + let resp = app + .router + .clone() + .oneshot( + Request::builder() + .uri("/xrpc/dev.happyview.space.list") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + let body = json_body(resp).await; + assert_eq!(body["error"], "FeatureDisabled"); +} + +#[tokio::test] +#[serial] +#[ignore] +async fn space_routes_allowed_after_enabling_flag() { + let app = TestApp::new().await; + + // Enable the feature flag + let resp = app + .router + .clone() + .oneshot(admin_put( + "/admin/settings/feature.spaces_enabled", + app.admin_cookie(), + &json!({ "value": "true" }), + )) + .await + .unwrap(); + assert!(resp.status().is_success()); + + // Space routes should now pass through (will get auth error, not FeatureDisabled) + let resp = app + .router + .clone() + .oneshot( + Request::builder() + .uri("/xrpc/dev.happyview.space.list") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + let body = json_body(resp).await; + assert_ne!( + body["error"].as_str().unwrap_or(""), + "FeatureDisabled", + "expected request to pass through feature gate" + ); +} + +#[tokio::test] +#[serial] +#[ignore] +async fn space_routes_blocked_again_after_disabling_flag() { + let app = TestApp::new().await; + + // Enable + let resp = app + .router + .clone() + .oneshot(admin_put( + "/admin/settings/feature.spaces_enabled", + app.admin_cookie(), + &json!({ "value": "true" }), + )) + .await + .unwrap(); + assert!(resp.status().is_success()); + + // Disable + let resp = app + .router + .clone() + .oneshot(admin_delete( + "/admin/settings/feature.spaces_enabled", + app.admin_cookie(), + )) + .await + .unwrap(); + assert!(resp.status().is_success()); + + // Space routes should be blocked again + let resp = app + .router + .clone() + .oneshot( + Request::builder() + .uri("/xrpc/dev.happyview.space.list") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + let body = json_body(resp).await; + assert_eq!(body["error"], "FeatureDisabled"); +} + +#[tokio::test] +#[serial] +#[ignore] +async fn admin_feature_flags_lists_flags() { + let app = TestApp::new().await; + + let resp = app + .router + .clone() + .oneshot(admin_get("/admin/feature-flags", app.admin_cookie())) + .await + .unwrap(); + + assert_eq!(resp.status(), StatusCode::OK); + let body = json_body(resp).await; + let flags = body.as_array().expect("expected array"); + assert!(!flags.is_empty()); + + let spaces_flag = flags + .iter() + .find(|f| f["key"] == "feature.spaces_enabled") + .expect("spaces flag not found"); + assert_eq!(spaces_flag["enabled"], false); + assert!(spaces_flag["name"].as_str().is_some()); + assert!(spaces_flag["description"].as_str().is_some()); +} + +#[tokio::test] +#[serial] +#[ignore] +async fn admin_feature_flags_reflects_enabled_state() { + let app = TestApp::new().await; + + // Enable the flag + let resp = app + .router + .clone() + .oneshot(admin_put( + "/admin/settings/feature.spaces_enabled", + app.admin_cookie(), + &json!({ "value": "true" }), + )) + .await + .unwrap(); + assert!(resp.status().is_success()); + + let resp = app + .router + .clone() + .oneshot(admin_get("/admin/feature-flags", app.admin_cookie())) + .await + .unwrap(); + + assert_eq!(resp.status(), StatusCode::OK); + let body = json_body(resp).await; + let flags = body.as_array().unwrap(); + let spaces_flag = flags + .iter() + .find(|f| f["key"] == "feature.spaces_enabled") + .unwrap(); + assert_eq!(spaces_flag["enabled"], true); +} + +#[tokio::test] +#[serial] +#[ignore] +async fn config_endpoint_includes_features() { + let app = TestApp::new().await; + + // Default: spaces disabled + let resp = app + .router + .clone() + .oneshot( + Request::builder() + .uri("/config") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(resp.status(), StatusCode::OK); + let body = json_body(resp).await; + assert_eq!(body["features"]["spaces"], false); + + // Enable the flag + let resp = app + .router + .clone() + .oneshot(admin_put( + "/admin/settings/feature.spaces_enabled", + app.admin_cookie(), + &json!({ "value": "true" }), + )) + .await + .unwrap(); + assert!(resp.status().is_success()); + + // Now config should reflect enabled + let resp = app + .router + .clone() + .oneshot( + Request::builder() + .uri("/config") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(resp.status(), StatusCode::OK); + let body = json_body(resp).await; + assert_eq!(body["features"]["spaces"], true); +} + +#[tokio::test] +#[serial] +#[ignore] +async fn admin_feature_flags_requires_auth() { + let app = TestApp::new().await; + + let resp = app + .router + .clone() + .oneshot( + Request::builder() + .uri("/admin/feature-flags") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); +} diff --git a/src/admin/feature_flags.rs b/src/admin/feature_flags.rs new file mode 100644 --- /dev/null +++ b/src/admin/feature_flags.rs @@ -0,0 +1,18 @@ +use axum::Json; +use axum::extract::State; + +use crate::AppState; +use crate::error::AppError; +use crate::feature_flags; + +use super::auth::UserAuth; +use super::permissions::Permission; + +pub(super) async fn list( + State(state): State, + auth: UserAuth, +) -> Result>, AppError> { + auth.require(Permission::SettingsManage).await?; + let flags = feature_flags::list_flags(&state.db, state.db_backend).await; + Ok(Json(flags)) +} diff --git a/src/admin/mod.rs b/src/admin/mod.rs --- a/src/admin/mod.rs +++ b/src/admin/mod.rs @@ -5,6 +5,7 @@ mod dead_letters; mod domains; mod events; +mod feature_flags; mod labelers; mod lexicons; mod network_lexicons; @@ -72,6 +73,7 @@ "/labelers/{did}", patch(labelers::update).delete(labelers::delete), ) + .route("/feature-flags", get(feature_flags::list)) .route("/settings", get(settings::list)) .route( "/settings/logo", diff --git a/src/admin/settings.rs b/src/admin/settings.rs --- a/src/admin/settings.rs +++ b/src/admin/settings.rs @@ -17,6 +17,7 @@ const ENV_FALLBACKS: &[(&str, &str)] = &[ ("app_name", "APP_NAME"), ("client_uri", "CLIENT_URI"), + ("feature.spaces_enabled", "FEATURE_SPACES_ENABLED"), ("logo_uri", "LOGO_URI"), ("tos_uri", "TOS_URI"), ("policy_uri", "POLICY_URI"), diff --git a/src/delegation/mod.rs b/src/delegation/mod.rs --- a/src/delegation/mod.rs +++ b/src/delegation/mod.rs @@ -42,6 +42,7 @@ } } + #[allow(clippy::should_implement_trait)] pub fn from_str(s: &str) -> Option { match s { "owner" => Some(DelegateRole::Owner), diff --git a/src/delegation/unlink_account.rs b/src/delegation/unlink_account.rs --- a/src/delegation/unlink_account.rs +++ b/src/delegation/unlink_account.rs @@ -55,17 +55,16 @@ db::delete_delegated_account(&state.db, state.db_backend, account_did).await?; // Delete the DPoP session for the target account using the stored api_client_id - if let Some(api_client_id) = stored_api_client_id { - if let Err(e) = crate::oauth::sessions::delete_dpop_session( + if let Some(api_client_id) = stored_api_client_id + && let Err(e) = crate::oauth::sessions::delete_dpop_session( &state.db, state.db_backend, &api_client_id, account_did, ) .await - { - tracing::warn!(account_did, %e, "failed to clean up DPoP session on unlink"); - } + { + tracing::warn!(account_did, %e, "failed to clean up DPoP session on unlink"); } log_event( diff --git a/src/lua/atproto_api.rs b/src/lua/atproto_api.rs --- a/src/lua/atproto_api.rs +++ b/src/lua/atproto_api.rs @@ -266,6 +266,15 @@ lua.create_async_function(move |_lua, (space_uri, did): (String, String)| { let state = state_clone.clone(); async move { + if !crate::feature_flags::is_enabled( + &state.db, + crate::feature_flags::FeatureFlag::SPACES_ENABLED, + state.db_backend, + ) + .await + { + return Err(mlua::Error::runtime("spaces feature is not enabled")); + } let uri = crate::spaces::SpaceUri::parse(&space_uri) .map_err(|e| mlua::Error::runtime(format!("invalid space URI: {e}")))?; let space = crate::spaces::db::get_space_by_address( @@ -298,6 +307,15 @@ lua.create_async_function(move |_lua, (space_uri, did): (String, String)| { let state = state_clone.clone(); async move { + if !crate::feature_flags::is_enabled( + &state.db, + crate::feature_flags::FeatureFlag::SPACES_ENABLED, + state.db_backend, + ) + .await + { + return Err(mlua::Error::runtime("spaces feature is not enabled")); + } let uri = crate::spaces::SpaceUri::parse(&space_uri) .map_err(|e| mlua::Error::runtime(format!("invalid space URI: {e}")))?; let space = crate::spaces::db::get_space_by_address( @@ -329,6 +347,15 @@ let list_members_fn = lua.create_async_function(move |lua, space_uri: String| { let state = state_clone.clone(); async move { + if !crate::feature_flags::is_enabled( + &state.db, + crate::feature_flags::FeatureFlag::SPACES_ENABLED, + state.db_backend, + ) + .await + { + return Err(mlua::Error::runtime("spaces feature is not enabled")); + } let uri = crate::spaces::SpaceUri::parse(&space_uri) .map_err(|e| mlua::Error::runtime(format!("invalid space URI: {e}")))?; let space = crate::spaces::db::get_space_by_address( @@ -368,6 +395,15 @@ let query_fn = lua.create_async_function(move |lua, opts: mlua::Table| { let state = state_clone.clone(); async move { + if !crate::feature_flags::is_enabled( + &state.db, + crate::feature_flags::FeatureFlag::SPACES_ENABLED, + state.db_backend, + ) + .await + { + return Err(mlua::Error::runtime("spaces feature is not enabled")); + } let space_uri: String = opts .get("space_uri") .map_err(|_| mlua::Error::runtime("space_uri is required"))?; diff --git a/src/lua/context.rs b/src/lua/context.rs --- a/src/lua/context.rs +++ b/src/lua/context.rs @@ -32,6 +32,7 @@ } /// Set global context variables for a procedure script. +#[allow(clippy::too_many_arguments)] pub fn set_procedure_context( lua: &Lua, method: &str, diff --git a/src/xrpc/procedure.rs b/src/xrpc/procedure.rs --- a/src/xrpc/procedure.rs +++ b/src/xrpc/procedure.rs @@ -346,6 +346,7 @@ } } +#[allow(clippy::too_many_arguments)] async fn handle_dpop_procedure( state: &AppState, claims: &Claims,