From 06e82fe7619ced71d59bc712e3e6bc6f2c04da30 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 10 Nov 2020 16:45:10 +0100 Subject: [PATCH 1/2] Add pending status for federated follows --- lemmy_api/src/community.rs | 2 ++ lemmy_api/src/user.rs | 1 + lemmy_apub/src/activities/send/user.rs | 17 ++++++++++++++++- lemmy_apub/src/inbox/community_inbox.rs | 2 ++ lemmy_apub/src/inbox/user_inbox.rs | 18 ++++++------------ lemmy_db/src/community.rs | 15 +++++++++++++++ lemmy_db/src/lib.rs | 3 +++ lemmy_db/src/schema.rs | 1 + .../down.sql | 1 + .../up.sql | 1 + 10 files changed, 48 insertions(+), 13 deletions(-) create mode 100644 migrations/2020-11-10-150835_community_follower_pending/down.sql create mode 100644 migrations/2020-11-10-150835_community_follower_pending/up.sql diff --git a/lemmy_api/src/community.rs b/lemmy_api/src/community.rs index 4c9d769ba..a9e130b9b 100644 --- a/lemmy_api/src/community.rs +++ b/lemmy_api/src/community.rs @@ -188,6 +188,7 @@ impl Perform for CreateCommunity { let community_follower_form = CommunityFollowerForm { community_id: inserted_community.id, user_id: user.id, + pending: false, }; let follow = move |conn: &'_ _| CommunityFollower::follow(conn, &community_follower_form); @@ -479,6 +480,7 @@ impl Perform for FollowCommunity { let community_follower_form = CommunityFollowerForm { community_id: data.community_id, user_id: user.id, + pending: false, }; if community.local { diff --git a/lemmy_api/src/user.rs b/lemmy_api/src/user.rs index 41f77271a..4828888ff 100644 --- a/lemmy_api/src/user.rs +++ b/lemmy_api/src/user.rs @@ -251,6 +251,7 @@ impl Perform for Register { let community_follower_form = CommunityFollowerForm { community_id: main_community.id, user_id: inserted_user.id, + pending: false, }; let follow = move |conn: &'_ _| CommunityFollower::follow(conn, &community_follower_form); diff --git a/lemmy_apub/src/activities/send/user.rs b/lemmy_apub/src/activities/send/user.rs index 39b10ef5f..2839d83ba 100644 --- a/lemmy_apub/src/activities/send/user.rs +++ b/lemmy_apub/src/activities/send/user.rs @@ -12,7 +12,12 @@ use activitystreams::{ base::{AnyBase, BaseExt, ExtendsExt}, object::ObjectExt, }; -use lemmy_db::{community::Community, user::User_, DbPool}; +use lemmy_db::{ + community::{Community, CommunityFollower, CommunityFollowerForm}, + user::User_, + DbPool, + Followable, +}; use lemmy_structs::blocking; use lemmy_utils::LemmyError; use lemmy_websocket::LemmyContext; @@ -44,6 +49,16 @@ impl ActorType for User_ { }) .await??; + let community_follower_form = CommunityFollowerForm { + community_id: community.id, + user_id: self.id, + pending: true, + }; + blocking(&context.pool(), move |conn| { + CommunityFollower::follow(conn, &community_follower_form).ok() + }) + .await?; + let mut follow = Follow::new(self.actor_id.to_owned(), community.actor_id()?); follow .set_context(activitystreams::context()) diff --git a/lemmy_apub/src/inbox/community_inbox.rs b/lemmy_apub/src/inbox/community_inbox.rs index 6cd4a017f..137f3fea4 100644 --- a/lemmy_apub/src/inbox/community_inbox.rs +++ b/lemmy_apub/src/inbox/community_inbox.rs @@ -191,6 +191,7 @@ async fn handle_follow( let community_follower_form = CommunityFollowerForm { community_id: community.id, user_id: user.id, + pending: false, }; // This will fail if they're already a follower, but ignore the error. @@ -245,6 +246,7 @@ async fn handle_undo_follow( let community_follower_form = CommunityFollowerForm { community_id: community.id, user_id: user.id, + pending: false, }; // This will fail if they aren't a follower, but ignore the error. diff --git a/lemmy_apub/src/inbox/user_inbox.rs b/lemmy_apub/src/inbox/user_inbox.rs index 624a2b234..f28df83af 100644 --- a/lemmy_apub/src/inbox/user_inbox.rs +++ b/lemmy_apub/src/inbox/user_inbox.rs @@ -46,7 +46,7 @@ use actix_web::{web, HttpRequest, HttpResponse}; use anyhow::{anyhow, Context}; use diesel::NotFound; use lemmy_db::{ - community::{Community, CommunityFollower, CommunityFollowerForm}, + community::{Community, CommunityFollower}, private_message::PrivateMessage, user::User_, Followable, @@ -173,8 +173,6 @@ async fn receive_accept( let accept = Accept::from_any_base(activity)?.context(location_info!())?; verify_activity_domains_valid(&accept, &actor.actor_id()?, false)?; - // TODO: we should check that we actually sent this activity, because the remote instance - // could just put a fake Follow let object = accept.object().to_owned().one().context(location_info!())?; let follow = Follow::from_any_base(object)?.context(location_info!())?; verify_activity_domains_valid(&follow, &user.actor_id()?, false)?; @@ -188,17 +186,13 @@ async fn receive_accept( let community = get_or_fetch_and_upsert_community(&community_uri, context, request_counter).await?; - // Now you need to add this to the community follower - let community_follower_form = CommunityFollowerForm { - community_id: community.id, - user_id: user.id, - }; - - // This will fail if they're already a follower + let community_id = community.id; + let user_id = user.id; + // This will throw an error if no follow was requested blocking(&context.pool(), move |conn| { - CommunityFollower::follow(conn, &community_follower_form).ok() + CommunityFollower::follow_accepted(conn, community_id, user_id) }) - .await?; + .await??; Ok(()) } diff --git a/lemmy_db/src/community.rs b/lemmy_db/src/community.rs index 733f63a15..4a11557c0 100644 --- a/lemmy_db/src/community.rs +++ b/lemmy_db/src/community.rs @@ -275,6 +275,7 @@ pub struct CommunityFollower { pub id: i32, pub community_id: i32, pub user_id: i32, + pub pending: bool, pub published: chrono::NaiveDateTime, } @@ -283,6 +284,7 @@ pub struct CommunityFollower { pub struct CommunityFollowerForm { pub community_id: i32, pub user_id: i32, + pub pending: bool, } impl Followable for CommunityFollower { @@ -295,6 +297,19 @@ impl Followable for CommunityFollower { .values(community_follower_form) .get_result::(conn) } + fn follow_accepted(conn: &PgConnection, community_id_: i32, user_id_: i32) -> Result + where + Self: Sized, + { + use crate::schema::community_follower::dsl::*; + diesel::update( + community_follower + .filter(community_id.eq(community_id_)) + .filter(user_id.eq(user_id_)), + ) + .set(pending.eq(true)) + .get_result::(conn) + } fn unfollow( conn: &PgConnection, community_follower_form: &CommunityFollowerForm, diff --git a/lemmy_db/src/lib.rs b/lemmy_db/src/lib.rs index 40f6c3d26..c17bc0254 100644 --- a/lemmy_db/src/lib.rs +++ b/lemmy_db/src/lib.rs @@ -54,6 +54,9 @@ pub trait Crud { pub trait Followable { fn follow(conn: &PgConnection, form: &T) -> Result + where + Self: Sized; + fn follow_accepted(conn: &PgConnection, community_id: i32, user_id: i32) -> Result where Self: Sized; fn unfollow(conn: &PgConnection, form: &T) -> Result diff --git a/lemmy_db/src/schema.rs b/lemmy_db/src/schema.rs index ec1e25995..7eeea9686 100644 --- a/lemmy_db/src/schema.rs +++ b/lemmy_db/src/schema.rs @@ -149,6 +149,7 @@ table! { id -> Int4, community_id -> Int4, user_id -> Int4, + pending -> Bool, published -> Timestamp, } } diff --git a/migrations/2020-11-10-150835_community_follower_pending/down.sql b/migrations/2020-11-10-150835_community_follower_pending/down.sql new file mode 100644 index 000000000..72cbeaa37 --- /dev/null +++ b/migrations/2020-11-10-150835_community_follower_pending/down.sql @@ -0,0 +1 @@ +ALTER TABLE community_follower DROP COLUMN pending; \ No newline at end of file diff --git a/migrations/2020-11-10-150835_community_follower_pending/up.sql b/migrations/2020-11-10-150835_community_follower_pending/up.sql new file mode 100644 index 000000000..599bed624 --- /dev/null +++ b/migrations/2020-11-10-150835_community_follower_pending/up.sql @@ -0,0 +1 @@ +ALTER TABLE community_follower ADD COLUMN pending BOOLEAN DEFAULT FALSE; \ No newline at end of file From 964d95de5cacf4f290519d0437e5d8c57cd707d9 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 10 Nov 2020 17:11:08 +0100 Subject: [PATCH 2/2] Fix unit tests --- Cargo.lock | 3 ++ Cargo.toml | 3 ++ lemmy_apub/src/http/mod.rs | 3 +- lemmy_db/src/activity.rs | 19 +++++++----- lemmy_db/src/comment.rs | 2 +- lemmy_db/src/comment_view.rs | 2 +- lemmy_db/src/community.rs | 6 ++-- lemmy_db/src/moderator.rs | 4 +-- lemmy_db/src/password_reset_request.rs | 2 +- lemmy_db/src/post.rs | 2 +- lemmy_db/src/post_view.rs | 2 +- lemmy_db/src/private_message.rs | 4 +-- lemmy_db/src/schema.rs | 6 ++-- lemmy_db/src/user.rs | 2 +- lemmy_db/src/user_mention.rs | 4 +-- test.sh | 5 +++- tests/integration_test.rs | 40 +++++++++++++------------- 17 files changed, 63 insertions(+), 46 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 27d585cf2..c5f984783 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1852,8 +1852,10 @@ dependencies = [ name = "lemmy_server" version = "0.0.1" dependencies = [ + "activitystreams", "actix", "actix-files", + "actix-rt", "actix-web", "actix-web-actors", "anyhow", @@ -1877,6 +1879,7 @@ dependencies = [ "reqwest", "rss", "serde 1.0.117", + "serde_json", "sha2", "strum", "tokio 0.3.1", diff --git a/Cargo.toml b/Cargo.toml index 055dac505..99d755cc0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,6 +46,9 @@ tokio = "0.3" sha2 = "0.9" anyhow = "1.0" reqwest = { version = "0.10", features = ["json"] } +activitystreams = "0.7.0-alpha.4" +actix-rt = { version = "1.1", default-features = false } +serde_json = { version = "1.0", features = ["preserve_order"]} [dev-dependencies.cargo-husky] version = "1" diff --git a/lemmy_apub/src/http/mod.rs b/lemmy_apub/src/http/mod.rs index 91af36b2d..4f31f6a5e 100644 --- a/lemmy_apub/src/http/mod.rs +++ b/lemmy_apub/src/http/mod.rs @@ -54,7 +54,8 @@ pub async fn get_activity( }) .await??; - if !activity.local || activity.sensitive { + let sensitive = activity.sensitive.unwrap_or(true); + if !activity.local || sensitive { Ok(HttpResponse::NotFound().finish()) } else { Ok(create_apub_response(&activity.data)) diff --git a/lemmy_db/src/activity.rs b/lemmy_db/src/activity.rs index b0ec1df69..190dd411c 100644 --- a/lemmy_db/src/activity.rs +++ b/lemmy_db/src/activity.rs @@ -12,22 +12,22 @@ use std::{ #[table_name = "activity"] pub struct Activity { pub id: i32, - pub ap_id: String, pub data: Value, pub local: bool, - pub sensitive: bool, pub published: chrono::NaiveDateTime, pub updated: Option, + pub ap_id: Option, + pub sensitive: Option, } #[derive(Insertable, AsChangeset)] #[table_name = "activity"] pub struct ActivityForm { - pub ap_id: String, pub data: Value, pub local: bool, - pub sensitive: bool, pub updated: Option, + pub ap_id: String, + pub sensitive: bool, } impl Crud for Activity { @@ -53,6 +53,10 @@ impl Crud for Activity { .set(new_activity) .get_result::(conn) } + fn delete(conn: &PgConnection, activity_id: i32) -> Result { + use crate::schema::activity::dsl::*; + diesel::delete(activity.find(activity_id)).execute(conn) + } } impl Activity { @@ -115,7 +119,7 @@ mod tests { avatar: None, banner: None, admin: false, - banned: false, + banned: Some(false), published: None, updated: None, show_nsfw: false, @@ -162,11 +166,11 @@ mod tests { let inserted_activity = Activity::create(&conn, &activity_form).unwrap(); let expected_activity = Activity { - ap_id: ap_id.to_string(), + ap_id: Some(ap_id.to_string()), id: inserted_activity.id, data: test_json, local: true, - sensitive: false, + sensitive: Some(false), published: inserted_activity.published, updated: None, }; @@ -174,6 +178,7 @@ mod tests { let read_activity = Activity::read(&conn, inserted_activity.id).unwrap(); let read_activity_by_apub_id = Activity::read_from_apub_id(&conn, ap_id).unwrap(); User_::delete(&conn, inserted_creator.id).unwrap(); + Activity::delete(&conn, inserted_activity.id).unwrap(); assert_eq!(expected_activity, read_activity); assert_eq!(expected_activity, read_activity_by_apub_id); diff --git a/lemmy_db/src/comment.rs b/lemmy_db/src/comment.rs index 02c84187e..9b0928257 100644 --- a/lemmy_db/src/comment.rs +++ b/lemmy_db/src/comment.rs @@ -280,7 +280,7 @@ mod tests { avatar: None, banner: None, admin: false, - banned: false, + banned: Some(false), published: None, updated: None, show_nsfw: false, diff --git a/lemmy_db/src/comment_view.rs b/lemmy_db/src/comment_view.rs index 568083ce9..e1299cb14 100644 --- a/lemmy_db/src/comment_view.rs +++ b/lemmy_db/src/comment_view.rs @@ -519,7 +519,7 @@ mod tests { avatar: None, banner: None, admin: false, - banned: false, + banned: Some(false), published: None, updated: None, show_nsfw: false, diff --git a/lemmy_db/src/community.rs b/lemmy_db/src/community.rs index 4a11557c0..3473f25c7 100644 --- a/lemmy_db/src/community.rs +++ b/lemmy_db/src/community.rs @@ -275,8 +275,8 @@ pub struct CommunityFollower { pub id: i32, pub community_id: i32, pub user_id: i32, - pub pending: bool, pub published: chrono::NaiveDateTime, + pub pending: Option, } #[derive(Insertable, AsChangeset, Clone)] @@ -341,7 +341,7 @@ mod tests { avatar: None, banner: None, admin: false, - banned: false, + banned: Some(false), published: None, updated: None, show_nsfw: false, @@ -407,6 +407,7 @@ mod tests { let community_follower_form = CommunityFollowerForm { community_id: inserted_community.id, user_id: inserted_user.id, + pending: false, }; let inserted_community_follower = @@ -416,6 +417,7 @@ mod tests { id: inserted_community_follower.id, community_id: inserted_community.id, user_id: inserted_user.id, + pending: Some(false), published: inserted_community_follower.published, }; diff --git a/lemmy_db/src/moderator.rs b/lemmy_db/src/moderator.rs index fb74def08..c0c0ff106 100644 --- a/lemmy_db/src/moderator.rs +++ b/lemmy_db/src/moderator.rs @@ -416,7 +416,7 @@ mod tests { avatar: None, banner: None, admin: false, - banned: false, + banned: Some(false), published: None, updated: None, show_nsfw: false, @@ -445,7 +445,7 @@ mod tests { avatar: None, banner: None, admin: false, - banned: false, + banned: Some(false), published: None, updated: None, show_nsfw: false, diff --git a/lemmy_db/src/password_reset_request.rs b/lemmy_db/src/password_reset_request.rs index 20db73ec5..8ae18cbd4 100644 --- a/lemmy_db/src/password_reset_request.rs +++ b/lemmy_db/src/password_reset_request.rs @@ -100,7 +100,7 @@ mod tests { avatar: None, banner: None, admin: false, - banned: false, + banned: Some(false), published: None, updated: None, show_nsfw: false, diff --git a/lemmy_db/src/post.rs b/lemmy_db/src/post.rs index 41401431b..787d5e6c4 100644 --- a/lemmy_db/src/post.rs +++ b/lemmy_db/src/post.rs @@ -349,7 +349,7 @@ mod tests { avatar: None, banner: None, admin: false, - banned: false, + banned: Some(false), published: None, updated: None, show_nsfw: false, diff --git a/lemmy_db/src/post_view.rs b/lemmy_db/src/post_view.rs index 15652477e..38d9a0211 100644 --- a/lemmy_db/src/post_view.rs +++ b/lemmy_db/src/post_view.rs @@ -416,7 +416,7 @@ mod tests { published: None, updated: None, admin: false, - banned: false, + banned: Some(false), show_nsfw: false, theme: "browser".into(), default_sort_type: SortType::Hot as i16, diff --git a/lemmy_db/src/private_message.rs b/lemmy_db/src/private_message.rs index 4775ceba2..503a26abf 100644 --- a/lemmy_db/src/private_message.rs +++ b/lemmy_db/src/private_message.rs @@ -157,7 +157,7 @@ mod tests { avatar: None, banner: None, admin: false, - banned: false, + banned: Some(false), published: None, updated: None, show_nsfw: false, @@ -186,7 +186,7 @@ mod tests { avatar: None, banner: None, admin: false, - banned: false, + banned: Some(false), published: None, updated: None, show_nsfw: false, diff --git a/lemmy_db/src/schema.rs b/lemmy_db/src/schema.rs index 7eeea9686..65838b1a8 100644 --- a/lemmy_db/src/schema.rs +++ b/lemmy_db/src/schema.rs @@ -1,12 +1,12 @@ table! { activity (id) { id -> Int4, - ap_id -> Text, data -> Jsonb, local -> Bool, - sensitive -> Bool, published -> Timestamp, updated -> Nullable, + ap_id -> Nullable, + sensitive -> Nullable, } } @@ -149,8 +149,8 @@ table! { id -> Int4, community_id -> Int4, user_id -> Int4, - pending -> Bool, published -> Timestamp, + pending -> Nullable, } } diff --git a/lemmy_db/src/user.rs b/lemmy_db/src/user.rs index 8dc613bb3..378bbad9c 100644 --- a/lemmy_db/src/user.rs +++ b/lemmy_db/src/user.rs @@ -196,7 +196,7 @@ mod tests { avatar: None, banner: None, admin: false, - banned: false, + banned: Some(false), published: None, updated: None, show_nsfw: false, diff --git a/lemmy_db/src/user_mention.rs b/lemmy_db/src/user_mention.rs index 79f93801a..68f566332 100644 --- a/lemmy_db/src/user_mention.rs +++ b/lemmy_db/src/user_mention.rs @@ -96,7 +96,7 @@ mod tests { avatar: None, banner: None, admin: false, - banned: false, + banned: Some(false), published: None, updated: None, show_nsfw: false, @@ -125,7 +125,7 @@ mod tests { avatar: None, banner: None, admin: false, - banned: false, + banned: Some(false), published: None, updated: None, show_nsfw: false, diff --git a/test.sh b/test.sh index beb499bdf..02e4faeed 100755 --- a/test.sh +++ b/test.sh @@ -2,4 +2,7 @@ export DATABASE_URL=postgres://lemmy:password@localhost:5432/lemmy diesel migration run export LEMMY_DATABASE_URL=postgres://lemmy:password@localhost:5432/lemmy -RUST_TEST_THREADS=1 cargo test --workspace --no-fail-fast +# Integration tests only work on stable due to a bug in config-rs +# https://github.com/mehcode/config-rs/issues/158 +RUST_BACKTRACE=1 RUST_TEST_THREADS=1 \ + cargo +stable test --workspace --no-fail-fast diff --git a/tests/integration_test.rs b/tests/integration_test.rs index f7b32151e..2a79dd4b5 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -16,6 +16,18 @@ use diesel::{ PgConnection, }; use http_signature_normalization_actix::PrepareVerifyError; +use lemmy_api::match_websocket_operation; +use lemmy_apub::{ + activity_queue::create_activity_queue, + inbox::{ + community_inbox, + community_inbox::community_inbox, + shared_inbox, + shared_inbox::shared_inbox, + user_inbox, + user_inbox::user_inbox, + }, +}; use lemmy_db::{ community::{Community, CommunityForm}, user::{User_, *}, @@ -24,22 +36,8 @@ use lemmy_db::{ SortType, }; use lemmy_rate_limit::{rate_limiter::RateLimiter, RateLimit}; -use lemmy_server::{ - apub::{ - activity_queue::create_activity_queue, - inbox::{ - community_inbox, - community_inbox::community_inbox, - shared_inbox, - shared_inbox::shared_inbox, - user_inbox, - user_inbox::user_inbox, - }, - }, - websocket::chat_server::ChatServer, - LemmyContext, -}; use lemmy_utils::{apub::generate_actor_keypair, settings::Settings}; +use lemmy_websocket::{chat_server::ChatServer, LemmyContext}; use reqwest::Client; use serde::{Deserialize, Serialize}; use std::sync::Arc; @@ -61,11 +59,12 @@ fn create_context() -> LemmyContext { let chat_server = ChatServer::startup( pool.clone(), rate_limiter.clone(), + |c, i, o, d| Box::pin(match_websocket_operation(c, i, o, d)), Client::default(), activity_queue.clone(), ) .start(); - LemmyContext::new( + LemmyContext::create( pool, chat_server, Client::default(), @@ -84,7 +83,7 @@ fn create_user(conn: &PgConnection, name: &str) -> User_ { avatar: None, banner: None, admin: false, - banned: false, + banned: Some(false), updated: None, published: None, show_nsfw: false, @@ -177,7 +176,7 @@ async fn test_user_inbox_expired_signature() { let connection = &context.pool().get().unwrap(); let user = create_user(connection, "user_inbox_cgsax"); let activity = - create_activity::>(user.actor_id); + create_activity::>(user.actor_id); let path = Path:: { 0: "username".to_string(), }; @@ -196,8 +195,9 @@ async fn test_community_inbox_expired_signature() { let user = create_user(connection, "community_inbox_hrxa"); let community = create_community(connection, user.id); let request = create_http_request(); - let activity = - create_activity::>(user.actor_id); + let activity = create_activity::>( + user.actor_id, + ); let path = Path:: { 0: community.name }; let response = community_inbox(request, activity, path, web::Data::new(context)).await; assert_eq!(