diff --git a/packages/docs/content/docs/experimental/spaces/managing-spaces.md b/packages/docs/content/docs/experimental/spaces/managing-spaces.md index e8626d4..adf0df4 100644 --- a/packages/docs/content/docs/experimental/spaces/managing-spaces.md +++ b/packages/docs/content/docs/experimental/spaces/managing-spaces.md @@ -129,7 +129,7 @@ The `config` object supports: | `membershipPublic` | boolean | `false` | Whether the member list is visible without authentication | | `recordsPublic` | boolean | `false` | Whether records are readable without membership | -Additional fields are preserved as-is. +Additional fields are preserved as-is. One recognized additional field is `allowedCollections` (a JSON array of collection NSID strings), auto-populated at creation time from the space type lexicon's `defs.main.collections`. When present and non-empty, `createRecord`/`putRecord`/`applyWrites` reject writes (`400`) to any collection not on the list; deletes are never restricted. A space without this field, or with an empty list, allows writes to any collection. ## Getting a space diff --git a/src/spaces/routes.rs b/src/spaces/routes.rs index e5fcb8a..0b7b02d 100644 --- a/src/spaces/routes.rs +++ b/src/spaces/routes.rs @@ -601,6 +601,7 @@ async fn apply_writes( rkey, value, } => { + service::check_collection_allowed(&space, &collection)?; let rkey = rkey.unwrap_or_else(generate_tid); let cid = service::content_cid(&value); let record_uri = format!( @@ -629,6 +630,7 @@ async fn apply_writes( value, swap_record, } => { + service::check_collection_allowed(&space, &collection)?; let cid = service::content_cid(&value); let record_uri = format!( "at://{}/space/{}/{}/{}/{}/{}", diff --git a/src/spaces/service.rs b/src/spaces/service.rs index a6bb587..b61d276 100644 --- a/src/spaces/service.rs +++ b/src/spaces/service.rs @@ -54,6 +54,24 @@ pub(crate) async fn require_space_admin( )) } +/// Enforce a space's `allowedCollections` config (`space.config.extra["allowedCollections"]`, +/// a JSON array of collection NSID strings injected from the lexicon +/// registry at space-creation time — see `create_space` below). Enforcement +/// only kicks in when the field is present *and* non-empty, so spaces +/// without the config (or with an empty list) continue to allow any +/// collection, preserving backward compatibility. +pub(crate) fn check_collection_allowed(space: &Space, collection: &str) -> Result<(), AppError> { + if let Some(serde_json::Value::Array(allowed)) = space.config.extra.get("allowedCollections") + && !allowed.is_empty() + && !allowed.iter().any(|v| v.as_str() == Some(collection)) + { + return Err(AppError::BadRequest(format!( + "collection '{collection}' is not allowed in this space" + ))); + } + Ok(()) +} + pub(crate) async fn require_membership( state: &AppState, space: &Space, @@ -109,6 +127,7 @@ pub(crate) async fn create_record( ) -> Result<(String, String), AppError> { let space = resolve_space(state, space_ref).await?; require_membership(state, &space, did, true, space_credential).await?; + check_collection_allowed(&space, collection)?; let rkey = generate_tid(); let cid = content_cid(&record); @@ -145,6 +164,7 @@ pub(crate) async fn put_record( ) -> Result<(String, String), AppError> { let space = resolve_space(state, space_ref).await?; require_membership(state, &space, did, true, space_credential).await?; + check_collection_allowed(&space, collection)?; let cid = content_cid(&record); let record_uri = format!( @@ -597,6 +617,13 @@ mod tests { /// Uses randomised DIDs/skeys so parallel tests sharing the same /// `TEST_DATABASE_URL` database don't collide (no truncation is needed). async fn service_test_db() -> (AppState, String, String) { + service_test_db_with_config(SpaceConfig::default()).await + } + + /// Same as `service_test_db`, but lets the caller supply the seeded + /// space's `config` (e.g. to set `allowedCollections` for enforcement + /// tests). + async fn service_test_db_with_config(config: SpaceConfig) -> (AppState, String, String) { let state = service_empty_db().await; let unique = uuid::Uuid::new_v4().simple().to_string(); @@ -618,7 +645,7 @@ mod tests { mint_policy: MintPolicy::MemberList, app_access: AppAccess::default(), managing_app_did: None, - config: SpaceConfig::default(), + config, revision: None, created_at: now_rfc3339(), updated_at: now_rfc3339(), @@ -644,6 +671,77 @@ mod tests { (state, space_uri, member_did) } + /// Build a `SpaceConfig` whose `allowedCollections` extra is set to the + /// given list of collection NSIDs. + fn config_with_allowed_collections(allowed: &[&str]) -> SpaceConfig { + let mut config = SpaceConfig::default(); + config.extra.insert( + "allowedCollections".to_string(), + serde_json::Value::Array( + allowed + .iter() + .map(|s| serde_json::Value::String(s.to_string())) + .collect(), + ), + ); + config + } + + /// Build a minimal, DB-free `Space` for pure unit tests of + /// `check_collection_allowed`. + fn space_with_config(config: SpaceConfig) -> Space { + Space { + id: "space-id".into(), + did: "did:plc:owner".into(), + authority_did: "did:plc:owner".into(), + creator_did: "did:plc:owner".into(), + type_nsid: "com.example.forum".into(), + skey: "main".into(), + display_name: None, + description: None, + mint_policy: MintPolicy::MemberList, + app_access: AppAccess::default(), + managing_app_did: None, + config, + revision: None, + created_at: now_rfc3339(), + updated_at: now_rfc3339(), + } + } + + #[test] + fn check_collection_allowed_permits_listed_collection() { + let space = space_with_config(config_with_allowed_collections(&["com.example.allowed"])); + assert!(super::check_collection_allowed(&space, "com.example.allowed").is_ok()); + } + + #[test] + fn check_collection_allowed_rejects_unlisted_collection() { + let space = space_with_config(config_with_allowed_collections(&["com.example.allowed"])); + let err = super::check_collection_allowed(&space, "com.example.denied").unwrap_err(); + match err { + crate::error::AppError::BadRequest(msg) => { + assert!( + msg.contains("not allowed"), + "expected 'not allowed' in message, got: {msg}" + ); + } + other => panic!("expected BadRequest, got: {other:?}"), + } + } + + #[test] + fn check_collection_allowed_permits_any_collection_when_config_absent() { + let space = space_with_config(SpaceConfig::default()); + assert!(super::check_collection_allowed(&space, "com.example.anything").is_ok()); + } + + #[test] + fn check_collection_allowed_permits_any_collection_when_list_empty() { + let space = space_with_config(config_with_allowed_collections(&[])); + assert!(super::check_collection_allowed(&space, "com.example.anything").is_ok()); + } + /// `delete_record` builds the record URI from the *caller's own* DID /// (see the `record_uri` construction in `service::delete_record`), so a /// second write-member can never naturally collide with another @@ -1035,6 +1133,151 @@ mod tests { assert!(matches!(err, crate::error::AppError::Forbidden(_))); } + /// A space whose config declares `allowedCollections` must reject a + /// `create_record` write to a collection not on that list. + #[tokio::test] + async fn create_record_rejects_disallowed_collection() { + require_test_db!(); + let (state, space_uri, member_did) = + service_test_db_with_config(config_with_allowed_collections(&["com.example.allowed"])) + .await; + + let err = super::create_record( + &state, + &member_did, + None, + &space_uri, + "com.example.denied", + serde_json::json!({ "text": "no" }), + ) + .await + .unwrap_err(); + match err { + crate::error::AppError::BadRequest(msg) => { + assert!( + msg.contains("not allowed"), + "expected 'not allowed' in message, got: {msg}" + ); + } + other => panic!("expected BadRequest, got: {other:?}"), + } + } + + /// A space whose config declares `allowedCollections` still permits a + /// `create_record` write to a collection that *is* on the list. + #[tokio::test] + async fn create_record_allows_listed_collection() { + require_test_db!(); + let (state, space_uri, member_did) = + service_test_db_with_config(config_with_allowed_collections(&["com.example.allowed"])) + .await; + + let (uri, _cid) = super::create_record( + &state, + &member_did, + None, + &space_uri, + "com.example.allowed", + serde_json::json!({ "text": "yes" }), + ) + .await + .expect("write to an allowed collection should succeed"); + assert!(uri.contains("/com.example.allowed/")); + } + + /// A space without an `allowedCollections` config (the default, + /// backward-compatible case) allows a write to any collection. + #[tokio::test] + async fn create_record_allows_any_collection_when_config_absent() { + require_test_db!(); + let (state, space_uri, member_did) = service_test_db().await; // default config + super::create_record( + &state, + &member_did, + None, + &space_uri, + "com.example.anything", + serde_json::json!({ "text": "ok" }), + ) + .await + .expect("space without allowedCollections config should allow any collection"); + } + + /// A space with an explicitly *empty* `allowedCollections` list also + /// allows a write to any collection (same backward-compat rule). + #[tokio::test] + async fn create_record_allows_any_collection_when_list_empty() { + require_test_db!(); + let (state, space_uri, member_did) = + service_test_db_with_config(config_with_allowed_collections(&[])).await; + super::create_record( + &state, + &member_did, + None, + &space_uri, + "com.example.anything", + serde_json::json!({ "text": "ok" }), + ) + .await + .expect("empty allowedCollections list should allow any collection"); + } + + /// A space whose config declares `allowedCollections` must reject a + /// `put_record` write to a collection not on that list. + #[tokio::test] + async fn put_record_rejects_disallowed_collection() { + require_test_db!(); + let (state, space_uri, member_did) = + service_test_db_with_config(config_with_allowed_collections(&["com.example.allowed"])) + .await; + + let err = super::put_record( + &state, + &member_did, + None, + &space_uri, + "com.example.denied", + "fixedrkey-put-denied", + serde_json::json!({ "text": "no" }), + None, + ) + .await + .unwrap_err(); + match err { + crate::error::AppError::BadRequest(msg) => { + assert!( + msg.contains("not allowed"), + "expected 'not allowed' in message, got: {msg}" + ); + } + other => panic!("expected BadRequest, got: {other:?}"), + } + } + + /// A `put_record` write to a collection ON the space's `allowedCollections` + /// list succeeds (positive-case parity with `create_record_allows_listed_collection`). + #[tokio::test] + async fn put_record_allows_listed_collection() { + require_test_db!(); + let (state, space_uri, member_did) = + service_test_db_with_config(config_with_allowed_collections(&["com.example.allowed"])) + .await; + + let (uri, _cid) = super::put_record( + &state, + &member_did, + None, + &space_uri, + "com.example.allowed", + "fixedrkey-put-allowed", + serde_json::json!({ "text": "yes" }), + None, + ) + .await + .expect("put to an allowed collection should succeed"); + assert!(uri.contains("/com.example.allowed/")); + } + #[tokio::test] async fn create_space_inserts_and_adds_creator_as_writer() { require_test_db!(); diff --git a/tests/spaces_records.rs b/tests/spaces_records.rs index 6203949..5dfc4d2 100644 --- a/tests/spaces_records.rs +++ b/tests/spaces_records.rs @@ -70,6 +70,17 @@ async fn json_of(resp: axum::http::Response) -> Value { /// Create a space authored by `authority` with the given `skey`. Returns the /// space's DB id and its `at://` URI. async fn create_space(app: &TestApp, authority: &str, skey: &str) -> (String, String) { + create_space_with_config(app, authority, skey, SpaceConfig::default()).await +} + +/// Same as `create_space`, but lets the caller supply the seeded space's +/// `config` (e.g. to set `allowedCollections` for enforcement tests). +async fn create_space_with_config( + app: &TestApp, + authority: &str, + skey: &str, + config: SpaceConfig, +) -> (String, String) { let now = now_rfc3339(); let id = Uuid::new_v4().to_string(); let type_nsid = "com.example.records"; @@ -85,7 +96,7 @@ async fn create_space(app: &TestApp, authority: &str, skey: &str) -> (String, St mint_policy: MintPolicy::MemberList, app_access: AppAccess::Open, managing_app_did: None, - config: SpaceConfig::default(), + config, revision: None, created_at: now.clone(), updated_at: now, @@ -97,6 +108,16 @@ async fn create_space(app: &TestApp, authority: &str, skey: &str) -> (String, St (id, uri) } +/// Build a `SpaceConfig` whose `allowedCollections` extra is set to the +/// given list of collection NSIDs. +fn config_with_allowed_collections(allowed: &[&str]) -> SpaceConfig { + let mut config = SpaceConfig::default(); + config + .extra + .insert("allowedCollections".to_string(), json!(allowed)); + config +} + async fn add_member(app: &TestApp, space_id: &str, did: &str, access: SpaceAccess) { spaces_db::add_member( &app.state.db, @@ -1210,3 +1231,166 @@ async fn apply_writes_delete_of_nonexistent_record_returns_not_found() { .unwrap(); assert_eq!(resp.status(), StatusCode::NOT_FOUND); } + +/// A space whose config declares `allowedCollections` must reject an +/// applyWrites `create` op targeting a collection not on that list (400), +/// so a write-member can't use applyWrites to bypass the enforcement that +/// createRecord/putRecord already apply. +#[tokio::test] +#[serial] +async fn apply_writes_rejects_create_op_with_disallowed_collection() { + common::require_db!(); + let app = TestApp::new().await; + enable_spaces(&app).await; + + let authority = rand_did("authority"); + let writer = rand_did("writer"); + let skey = rand_skey("space"); + let (space_id, space_uri) = create_space_with_config( + &app, + &authority, + &skey, + config_with_allowed_collections(&["com.example.allowed"]), + ) + .await; + add_member(&app, &space_id, &writer, SpaceAccess::Write).await; + + let writes = json!([write_op_create( + "com.example.denied", + Some("should-not-exist"), + &json!({ "text": "should be rejected" }), + )]); + let resp = app + .router + .clone() + .oneshot(apply_writes_req( + &space_uri, + writes, + None, + Some(cookie_for(&app, &writer)), + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + + // confirm nothing landed + let get_resp = app + .router + .clone() + .oneshot(get_record_req( + &space_uri, + "com.example.denied", + "should-not-exist", + Some(cookie_for(&app, &writer)), + )) + .await + .unwrap(); + assert_eq!(get_resp.status(), StatusCode::NOT_FOUND); +} + +/// A space whose config declares `allowedCollections` must reject an +/// applyWrites `update` op targeting a collection not on that list (400). +#[tokio::test] +#[serial] +async fn apply_writes_rejects_update_op_with_disallowed_collection() { + common::require_db!(); + let app = TestApp::new().await; + enable_spaces(&app).await; + + let authority = rand_did("authority"); + let writer = rand_did("writer"); + let skey = rand_skey("space"); + let (space_id, space_uri) = create_space_with_config( + &app, + &authority, + &skey, + config_with_allowed_collections(&["com.example.allowed"]), + ) + .await; + add_member(&app, &space_id, &writer, SpaceAccess::Write).await; + + let writes = json!([write_op_update( + "com.example.denied", + "some-rkey", + &json!({ "text": "should be rejected" }), + None, + )]); + let resp = app + .router + .clone() + .oneshot(apply_writes_req( + &space_uri, + writes, + None, + Some(cookie_for(&app, &writer)), + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); +} + +/// applyWrites' `delete` op is NOT gated by `allowedCollections` — deleting +/// a record in a now-disallowed collection must still work (cleanup should +/// never be blocked by a config change). This seeds the record directly via +/// `spaces_db::insert_space_record` (bypassing createRecord's own +/// enforcement) to simulate a record that predates a stricter +/// `allowedCollections` config. +#[tokio::test] +#[serial] +async fn apply_writes_delete_op_ignores_disallowed_collection() { + common::require_db!(); + let app = TestApp::new().await; + enable_spaces(&app).await; + + let authority = rand_did("authority"); + let writer = rand_did("writer"); + let skey = rand_skey("space"); + let (space_id, space_uri) = create_space_with_config( + &app, + &authority, + &skey, + config_with_allowed_collections(&["com.example.allowed"]), + ) + .await; + add_member(&app, &space_id, &writer, SpaceAccess::Write).await; + + let collection = "com.example.denied"; + let rkey = "predates-the-restriction"; + let record_uri = + format!("at://{authority}/space/com.example.records/{skey}/{writer}/{collection}/{rkey}"); + let content = json!({ "text": "grandfathered in" }); + spaces_db::insert_space_record( + &app.state.db, + app.state.db_backend, + &SpaceRecord { + uri: record_uri.clone(), + space_id: space_id.clone(), + author_did: writer.clone(), + collection: collection.to_string(), + rkey: rkey.to_string(), + record: content, + cid: "bafyreigrandfathered0000000000000000000".to_string(), + indexed_at: now_rfc3339(), + }, + ) + .await + .expect("seed record failed"); + + let writes = json!([write_op_delete(collection, rkey, None)]); + let resp = app + .router + .clone() + .oneshot(apply_writes_req( + &space_uri, + writes, + None, + Some(cookie_for(&app, &writer)), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "delete op must not be gated by allowedCollections" + ); +}