From 7531476066552e6926f979d6db6781475fc10abc Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 12 Nov 2024 15:04:04 +0100 Subject: [PATCH] Admin approval for new articles --- config/defaults.toml | 3 + .../down.sql | 1 + .../2024-11-12-131724_article-approval/up.sql | 1 + src/backend/api/article.rs | 31 +++++++- src/backend/api/mod.rs | 14 ++++ src/backend/config.rs | 4 ++ src/backend/database/article.rs | 22 ++++++ src/backend/database/schema.rs | 1 + src/backend/federation/objects/article.rs | 1 + src/backend/mod.rs | 1 + src/common/mod.rs | 8 ++- src/frontend/api.rs | 18 +++++ src/frontend/pages/user_profile.rs | 2 +- tests/common.rs | 13 ++-- tests/test.rs | 70 +++++++++++++++---- 15 files changed, 165 insertions(+), 25 deletions(-) create mode 100644 migrations/2024-11-12-131724_article-approval/down.sql create mode 100644 migrations/2024-11-12-131724_article-approval/up.sql diff --git a/config/defaults.toml b/config/defaults.toml index 83f7333..341c32f 100644 --- a/config/defaults.toml +++ b/config/defaults.toml @@ -1,6 +1,9 @@ # Whether users can create new accounts registration_open = true +# Whether admins need to approve new articles +article_approval = false + # Details about the PostgreSQL database connection [database] # Database connection url diff --git a/migrations/2024-11-12-131724_article-approval/down.sql b/migrations/2024-11-12-131724_article-approval/down.sql new file mode 100644 index 0000000..fbe1f23 --- /dev/null +++ b/migrations/2024-11-12-131724_article-approval/down.sql @@ -0,0 +1 @@ +alter table article drop column approved; \ No newline at end of file diff --git a/migrations/2024-11-12-131724_article-approval/up.sql b/migrations/2024-11-12-131724_article-approval/up.sql new file mode 100644 index 0000000..f2d63ea --- /dev/null +++ b/migrations/2024-11-12-131724_article-approval/up.sql @@ -0,0 +1 @@ +alter table article add column approved bool not null default true; \ No newline at end of file diff --git a/src/backend/api/article.rs b/src/backend/api/article.rs index cc7fa03..dc99d64 100644 --- a/src/backend/api/article.rs +++ b/src/backend/api/article.rs @@ -1,3 +1,4 @@ +use super::check_is_admin; use crate::{ backend::{ database::{ @@ -14,6 +15,7 @@ use crate::{ utils::{extract_domain, http_protocol_str}, validation::can_edit_article, ApiConflict, + ApproveArticleForm, ArticleView, CreateArticleForm, DbArticle, @@ -66,6 +68,7 @@ pub(in crate::backend::api) async fn create_article( instance_id: local_instance.id, local: true, protected: false, + approved: !data.config.article_approval, }; let article = DbArticle::create(form, &data)?; @@ -208,6 +211,7 @@ pub(in crate::backend::api) async fn fork_article( instance_id: local_instance.id, local: true, protected: false, + approved: data.config.article_approval, }; let article = DbArticle::create(form, &data)?; @@ -278,10 +282,31 @@ pub(in crate::backend::api) async fn protect_article( data: Data, Form(lock_params): Form, ) -> MyResult> { - if !user.local_user.admin { - return Err(anyhow!("Only admin can lock articles").into()); - } + check_is_admin(&user)?; let article = DbArticle::update_protected(lock_params.article_id, lock_params.protected, &data)?; Ok(Json(article)) } + +/// Get a list of all unresolved edit conflicts. +#[debug_handler] +pub async fn list_approval_required( + Extension(user): Extension, + data: Data, +) -> MyResult>> { + check_is_admin(&user)?; + let articles = DbArticle::list_approval_required(&data)?; + Ok(Json(articles)) +} + +/// Get a list of all unresolved edit conflicts. +#[debug_handler] +pub async fn approve_article( + Extension(user): Extension, + data: Data, + Form(approve_params): Form, +) -> MyResult> { + check_is_admin(&user)?; + let article = DbArticle::update_approved(approve_params.article_id, true, &data)?; + Ok(Json(article)) +} diff --git a/src/backend/api/mod.rs b/src/backend/api/mod.rs index ac6f364..24f4cba 100644 --- a/src/backend/api/mod.rs +++ b/src/backend/api/mod.rs @@ -28,6 +28,8 @@ use crate::{ common::{ApiConflict, LocalUserView}, }; use activitypub_federation::config::Data; +use anyhow::anyhow; +use article::{approve_article, list_approval_required}; use axum::{ body::Body, http::{Request, StatusCode}, @@ -57,6 +59,11 @@ pub fn api_routes() -> Router<()> { .route("/article/fork", post(fork_article)) .route("/article/resolve", get(resolve_article)) .route("/article/protect", post(protect_article)) + .route( + "/article/list/approval_required", + get(list_approval_required), + ) + .route("/article/approve", post(approve_article)) .route("/edit_conflicts", get(edit_conflicts)) .route("/instance", get(get_instance)) .route("/instance/follow", post(follow_instance)) @@ -86,6 +93,13 @@ async fn auth( Ok(response) } +fn check_is_admin(user: &LocalUserView) -> MyResult<()> { + if !user.local_user.admin { + return Err(anyhow!("Only admin can perform this action").into()); + } + Ok(()) +} + /// Get a list of all unresolved edit conflicts. #[debug_handler] async fn edit_conflicts( diff --git a/src/backend/config.rs b/src/backend/config.rs index 3f51ccf..5015e3a 100644 --- a/src/backend/config.rs +++ b/src/backend/config.rs @@ -14,6 +14,10 @@ pub struct IbisConfig { #[default = true] #[doku(example = "true")] pub registration_open: bool, + /// Whether admins need to approve new articles + #[default = false] + #[doku(example = "false")] + pub article_approval: bool, /// Details of the initial admin account pub setup: IbisConfigSetup, pub federation: IbisConfigFederation, diff --git a/src/backend/database/article.rs b/src/backend/database/article.rs index 994fb75..1ad4567 100644 --- a/src/backend/database/article.rs +++ b/src/backend/database/article.rs @@ -38,6 +38,7 @@ pub struct DbArticleForm { pub instance_id: InstanceId, pub local: bool, pub protected: bool, + pub approved: bool, } // TODO: get rid of unnecessary methods @@ -79,6 +80,13 @@ impl DbArticle { .get_result::(conn.deref_mut())?) } + pub fn update_approved(id: ArticleId, approved: bool, data: &IbisData) -> MyResult { + let mut conn = data.db_pool.get()?; + Ok(diesel::update(article::dsl::article.find(id)) + .set(article::dsl::approved.eq(approved)) + .get_result::(conn.deref_mut())?) + } + pub fn read(id: ArticleId, data: &IbisData) -> MyResult { let mut conn = data.db_pool.get()?; Ok(article::table.find(id).get_result(conn.deref_mut())?) @@ -152,6 +160,7 @@ impl DbArticle { let mut query = article::table .inner_join(edit::table) .inner_join(instance::table) + .filter(article::dsl::approved.eq(true)) .group_by(article::dsl::id) .order_by(max(edit::dsl::created).desc()) .select(article::all_columns) @@ -196,4 +205,17 @@ impl DbArticle { None => Ok(EditVersion::default()), } } + + pub fn list_approval_required(data: &IbisData) -> MyResult> { + let mut conn = data.db_pool.get()?; + let query = article::table + .inner_join(edit::table) + .group_by(article::dsl::id) + .filter(article::dsl::approved.eq(false)) + .order_by(max(edit::dsl::created).desc()) + .select(article::all_columns) + .into_boxed(); + + Ok(query.get_results(&mut conn)?) + } } diff --git a/src/backend/database/schema.rs b/src/backend/database/schema.rs index 727bfd0..1affb5a 100644 --- a/src/backend/database/schema.rs +++ b/src/backend/database/schema.rs @@ -10,6 +10,7 @@ diesel::table! { instance_id -> Int4, local -> Bool, protected -> Bool, + approved -> Bool, } } diff --git a/src/backend/federation/objects/article.rs b/src/backend/federation/objects/article.rs index 84d4503..51d52a2 100644 --- a/src/backend/federation/objects/article.rs +++ b/src/backend/federation/objects/article.rs @@ -79,6 +79,7 @@ impl Object for DbArticle { local: false, instance_id: instance.id, protected: json.protected, + approved: true, }; let article = DbArticle::create_or_update(form, data)?; diff --git a/src/backend/mod.rs b/src/backend/mod.rs index 377af61..6a6e19a 100644 --- a/src/backend/mod.rs +++ b/src/backend/mod.rs @@ -160,6 +160,7 @@ async fn setup(data: &Data) -> Result<(), Error> { instance_id: instance.id, local: true, protected: true, + approved: true, }; let article = DbArticle::create(form, data)?; // also create an article so its included in most recently edited list diff --git a/src/common/mod.rs b/src/common/mod.rs index 2f01e83..6c47fef 100644 --- a/src/common/mod.rs +++ b/src/common/mod.rs @@ -29,7 +29,7 @@ pub struct GetArticleForm { pub id: Option, } -#[derive(Deserialize, Serialize, Clone)] +#[derive(Deserialize, Serialize, Clone, Default)] pub struct ListArticlesForm { pub only_local: Option, pub instance_id: Option, @@ -58,6 +58,7 @@ pub struct DbArticle { pub instance_id: InstanceId, pub local: bool, pub protected: bool, + pub approved: bool, } /// Represents a single change to the article. @@ -214,6 +215,11 @@ pub struct ForkArticleForm { pub new_title: String, } +#[derive(Deserialize, Serialize)] +pub struct ApproveArticleForm { + pub article_id: ArticleId, +} + #[derive(Deserialize, Serialize, Debug)] pub struct GetInstance { pub id: Option, diff --git a/src/frontend/api.rs b/src/frontend/api.rs index fc665f3..808c493 100644 --- a/src/frontend/api.rs +++ b/src/frontend/api.rs @@ -1,7 +1,9 @@ use crate::{ common::{ + newtypes::ArticleId, utils::http_protocol_str, ApiConflict, + ApproveArticleForm, ArticleView, CreateArticleForm, DbArticle, @@ -130,6 +132,22 @@ impl ApiClient { .await } + pub async fn list_articles_approval_required(&self) -> MyResult> { + let req = self + .client + .get(self.request_endpoint("/api/v1/article/list/approval_required")); + handle_json_res(req).await + } + + pub async fn approve_article(&self, article_id: ArticleId) -> MyResult { + let form = ApproveArticleForm { article_id }; + let req = self + .client + .post(self.request_endpoint("/api/v1/article/approve")) + .form(&form); + handle_json_res(req).await + } + pub async fn search(&self, search_form: &SearchArticleForm) -> MyResult> { self.get_query("/api/v1/search", Some(search_form)).await } diff --git a/src/frontend/pages/user_profile.rs b/src/frontend/pages/user_profile.rs index 660e6be..b9c24eb 100644 --- a/src/frontend/pages/user_profile.rs +++ b/src/frontend/pages/user_profile.rs @@ -38,7 +38,7 @@ pub fn UserProfile() -> impl IntoView { .get() .map(|person: DbPerson| { view! { -

{user_title(&person)}

+

{user_title(&person)}

TODO: create actual user profile

} }) diff --git a/tests/common.rs b/tests/common.rs index dcd2908..23b0266 100644 --- a/tests/common.rs +++ b/tests/common.rs @@ -2,7 +2,7 @@ use ibis::{ backend::{ - config::{IbisConfig, IbisConfigDatabase, IbisConfigFederation}, + config::{IbisConfig, IbisConfigDatabase, IbisConfigFederation, IbisConfigSetup}, start, }, common::RegisterUserForm, @@ -31,7 +31,7 @@ pub struct TestData { } impl TestData { - pub async fn start() -> Self { + pub async fn start(article_approval: bool) -> Self { static INIT: Once = Once::new(); INIT.call_once(|| { env_logger::builder() @@ -67,9 +67,9 @@ impl TestData { } let (alpha, beta, gamma) = join!( - IbisInstance::start(alpha_db_path, port_alpha, "alpha"), - IbisInstance::start(beta_db_path, port_beta, "beta"), - IbisInstance::start(gamma_db_path, port_gamma, "gamma") + IbisInstance::start(alpha_db_path, port_alpha, "alpha", article_approval), + IbisInstance::start(beta_db_path, port_beta, "beta", article_approval), + IbisInstance::start(gamma_db_path, port_gamma, "gamma", article_approval) ); Self { alpha, beta, gamma } @@ -115,7 +115,7 @@ impl IbisInstance { }) } - async fn start(db_path: String, port: i32, username: &str) -> Self { + async fn start(db_path: String, port: i32, username: &str, article_approval: bool) -> Self { let connection_url = format!("postgresql://ibis:password@/ibis?host={db_path}"); let hostname = format!("127.0.0.1:{port}"); let domain = format!("localhost:{port}"); @@ -129,6 +129,7 @@ impl IbisInstance { domain: domain.clone(), ..Default::default() }, + article_approval, ..Default::default() }; let client = ClientBuilder::new().cookie_store(true).build().unwrap(); diff --git a/tests/test.rs b/tests/test.rs index adacdf3..b1215d3 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -26,7 +26,7 @@ use url::Url; #[tokio::test] async fn test_create_read_and_edit_local_article() -> MyResult<()> { - let data = TestData::start().await; + let data = TestData::start(false).await; // create article let create_form = CreateArticleForm { @@ -88,7 +88,7 @@ async fn test_create_read_and_edit_local_article() -> MyResult<()> { #[tokio::test] async fn test_create_duplicate_article() -> MyResult<()> { - let data = TestData::start().await; + let data = TestData::start(false).await; // create article let create_form = CreateArticleForm { @@ -108,7 +108,7 @@ async fn test_create_duplicate_article() -> MyResult<()> { #[tokio::test] async fn test_follow_instance() -> MyResult<()> { - let data = TestData::start().await; + let data = TestData::start(false).await; // check initial state let alpha_user = data.alpha.my_profile().await?; @@ -134,7 +134,7 @@ async fn test_follow_instance() -> MyResult<()> { #[tokio::test] async fn test_synchronize_articles() -> MyResult<()> { - let data = TestData::start().await; + let data = TestData::start(false).await; // create article on alpha let create_form = CreateArticleForm { @@ -201,7 +201,7 @@ async fn test_synchronize_articles() -> MyResult<()> { #[tokio::test] async fn test_edit_local_article() -> MyResult<()> { - let data = TestData::start().await; + let data = TestData::start(false).await; let beta_instance = data .alpha @@ -258,7 +258,7 @@ async fn test_edit_local_article() -> MyResult<()> { #[tokio::test] async fn test_edit_remote_article() -> MyResult<()> { - let data = TestData::start().await; + let data = TestData::start(false).await; let beta_id_on_alpha = data .alpha @@ -338,7 +338,7 @@ async fn test_edit_remote_article() -> MyResult<()> { #[tokio::test] async fn test_local_edit_conflict() -> MyResult<()> { - let data = TestData::start().await; + let data = TestData::start(false).await; // create new article let create_form = CreateArticleForm { @@ -399,7 +399,7 @@ async fn test_local_edit_conflict() -> MyResult<()> { #[tokio::test] async fn test_federated_edit_conflict() -> MyResult<()> { - let data = TestData::start().await; + let data = TestData::start(false).await; let beta_id_on_alpha = data .alpha @@ -486,7 +486,7 @@ async fn test_federated_edit_conflict() -> MyResult<()> { #[tokio::test] async fn test_overlapping_edits_no_conflict() -> MyResult<()> { - let data = TestData::start().await; + let data = TestData::start(false).await; // create new article let create_form = CreateArticleForm { @@ -529,7 +529,7 @@ async fn test_overlapping_edits_no_conflict() -> MyResult<()> { #[tokio::test] async fn test_fork_article() -> MyResult<()> { - let data = TestData::start().await; + let data = TestData::start(false).await; // create article let create_form = CreateArticleForm { @@ -581,7 +581,7 @@ async fn test_fork_article() -> MyResult<()> { #[tokio::test] async fn test_user_registration_login() -> MyResult<()> { - let data = TestData::start().await; + let data = TestData::start(false).await; let username = "my_user"; let password = "hunter2"; let register_data = RegisterUserForm { @@ -616,7 +616,7 @@ async fn test_user_registration_login() -> MyResult<()> { #[tokio::test] async fn test_user_profile() -> MyResult<()> { - let data = TestData::start().await; + let data = TestData::start(false).await; // Create an article and federate it, in order to federate the user who created it let create_form = CreateArticleForm { @@ -644,7 +644,7 @@ async fn test_user_profile() -> MyResult<()> { #[tokio::test] async fn test_lock_article() -> MyResult<()> { - let data = TestData::start().await; + let data = TestData::start(false).await; // create article let create_form = CreateArticleForm { @@ -691,7 +691,7 @@ async fn test_lock_article() -> MyResult<()> { #[tokio::test] async fn test_synchronize_instances() -> MyResult<()> { - let data = TestData::start().await; + let data = TestData::start(false).await; // fetch alpha instance on beta data.beta @@ -727,3 +727,45 @@ async fn test_synchronize_instances() -> MyResult<()> { data.stop() } + +#[tokio::test] +async fn test_article_approval_required() -> MyResult<()> { + let data = TestData::start(true).await; + + // create article + let create_form = CreateArticleForm { + title: "Manu_Chao".to_string(), + text: TEST_ARTICLE_DEFAULT_TEXT.to_string(), + summary: "create article".to_string(), + }; + let create_res = data.alpha.create_article(&create_form).await?; + assert!(!create_res.article.approved); + + let list_all = data.alpha.list_articles(Default::default()).await?; + assert_eq!(1, list_all.len()); + assert!(list_all.iter().all(|a| a.id != create_res.article.id)); + + // login as admin to handle approvals + let form = LoginUserForm { + username: "ibis".to_string(), + password: "ibis".to_string(), + }; + data.alpha.login(form).await?; + + let list_approval_required = data.alpha.list_articles_approval_required().await?; + assert_eq!(1, list_approval_required.len()); + assert_eq!(create_res.article.id, list_approval_required[0].id); + + let approve = data + .alpha + .approve_article(list_approval_required[0].id) + .await?; + assert_eq!(create_res.article.id, approve.id); + assert!(approve.approved); + + let list_all = data.alpha.list_articles(Default::default()).await?; + assert_eq!(2, list_all.len()); + assert!(list_all.iter().any(|a| a.id == create_res.article.id)); + + data.stop() +}