From 89b7c981f5f19bd009afc19c3d244efb5598b5b9 Mon Sep 17 00:00:00 2001 From: Dessalines Date: Wed, 20 Sep 2023 05:49:26 -0400 Subject: [PATCH 1/5] Fixing GetPostsResponse serialization. (#3967) --- crates/api_common/src/post.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/api_common/src/post.rs b/crates/api_common/src/post.rs index 7c3aabb60..bb5c1cccc 100644 --- a/crates/api_common/src/post.rs +++ b/crates/api_common/src/post.rs @@ -82,6 +82,7 @@ pub struct GetPosts { pub page_cursor: Option, } +#[skip_serializing_none] #[derive(Serialize, Deserialize, Debug, Clone)] #[cfg_attr(feature = "full", derive(TS))] #[cfg_attr(feature = "full", ts(export))] From 50f81cf1573c8a37e9b35c28af92f95e453a7927 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Wed, 20 Sep 2023 11:56:13 +0200 Subject: [PATCH 2/5] User can block instances (fixes #2397) (#3869) * User can block instances (fixes #2397) * update comments * review comments * use route * update * add api test * update tests * fix * fix test * ci --------- Co-authored-by: Dessalines --- api_tests/package.json | 2 +- api_tests/src/community.spec.ts | 37 ++++++++ api_tests/src/shared.ts | 16 ++++ api_tests/yarn.lock | 8 +- crates/api/src/site/block.rs | 41 +++++++++ crates/api/src/site/mod.rs | 1 + crates/api_common/src/site.rs | 22 ++++- crates/api_crud/src/site/read.rs | 6 ++ crates/db_schema/src/aggregates/structs.rs | 3 +- crates/db_schema/src/impls/instance_block.rs | 36 ++++++++ crates/db_schema/src/impls/mod.rs | 1 + crates/db_schema/src/schema.rs | 14 +++ crates/db_schema/src/source/instance_block.rs | 26 ++++++ crates/db_schema/src/source/mod.rs | 1 + crates/db_views/src/comment_view.rs | 9 ++ crates/db_views/src/post_view.rs | 90 ++++++++++++++++++- crates/db_views_actor/src/community_view.rs | 20 ++++- .../db_views_actor/src/instance_block_view.rs | 27 ++++++ crates/db_views_actor/src/lib.rs | 2 + crates/db_views_actor/src/structs.rs | 13 +++ crates/utils/src/error.rs | 1 + .../down.sql | 21 +++++ .../up.sql | 51 +++++++++++ src/api_routes_http.rs | 4 +- 24 files changed, 441 insertions(+), 11 deletions(-) create mode 100644 crates/api/src/site/block.rs create mode 100644 crates/db_schema/src/impls/instance_block.rs create mode 100644 crates/db_schema/src/source/instance_block.rs create mode 100644 crates/db_views_actor/src/instance_block_view.rs create mode 100644 migrations/2023-08-09-101305_user_instance_block/down.sql create mode 100644 migrations/2023-08-09-101305_user_instance_block/up.sql diff --git a/api_tests/package.json b/api_tests/package.json index 06c2c46ae..e57d37d08 100644 --- a/api_tests/package.json +++ b/api_tests/package.json @@ -19,7 +19,7 @@ "eslint": "^8.40.0", "eslint-plugin-prettier": "^4.0.0", "jest": "^29.5.0", - "lemmy-js-client": "0.19.0-rc.3", + "lemmy-js-client": "0.19.0-rc.5", "prettier": "^3.0.0", "ts-jest": "^29.1.0", "typescript": "^5.0.4" diff --git a/api_tests/src/community.spec.ts b/api_tests/src/community.spec.ts index a2af84440..effd2169e 100644 --- a/api_tests/src/community.spec.ts +++ b/api_tests/src/community.spec.ts @@ -24,6 +24,7 @@ import { getComments, createComment, getCommunityByName, + blockInstance, waitUntil, delay, } from "./shared"; @@ -348,3 +349,39 @@ test("Get community for different casing on domain", async () => { .community_view; assertCommunityFederation(betaCommunity, communityRes.community_view); }); + +test("User blocks instance, communities are hidden", async () => { + // create community and post on beta + let communityRes = await createCommunity(beta); + expect(communityRes.community_view.community.name).toBeDefined(); + let postRes = await createPost( + beta, + communityRes.community_view.community.id, + ); + expect(postRes.post_view.post.id).toBeDefined(); + + // fetch post to alpha + let alphaPost = await resolvePost(alpha, postRes.post_view.post); + expect(alphaPost.post?.post).toBeDefined(); + + // post should be included in listing + let listing = await getPosts(alpha, "All"); + let listing_ids = listing.posts.map(p => p.post.ap_id); + expect(listing_ids).toContain(postRes.post_view.post.ap_id); + + // block the beta instance + await blockInstance(alpha, alphaPost.post!.community.instance_id, true); + + // after blocking, post should not be in listing + let listing2 = await getPosts(alpha, "All"); + let listing_ids2 = listing2.posts.map(p => p.post.ap_id); + expect(listing_ids2.indexOf(postRes.post_view.post.ap_id)).toBe(-1); + + // unblock instance again + await blockInstance(alpha, alphaPost.post!.community.instance_id, false); + + // post should be included in listing + let listing3 = await getPosts(alpha, "All"); + let listing_ids3 = listing3.posts.map(p => p.post.ap_id); + expect(listing_ids3).toContain(postRes.post_view.post.ap_id); +}); diff --git a/api_tests/src/shared.ts b/api_tests/src/shared.ts index d116d329f..f7039942d 100644 --- a/api_tests/src/shared.ts +++ b/api_tests/src/shared.ts @@ -1,8 +1,11 @@ import { + BlockInstance, + BlockInstanceResponse, GetReplies, GetRepliesResponse, GetUnreadCount, GetUnreadCountResponse, + InstanceId, LemmyHttp, } from "lemmy-js-client"; import { CreatePost } from "lemmy-js-client/dist/types/CreatePost"; @@ -817,6 +820,19 @@ export function getPosts( return api.client.getPosts(form); } +export function blockInstance( + api: API, + instance_id: InstanceId, + block: boolean, +): Promise { + let form: BlockInstance = { + instance_id, + block, + auth: api.auth, + }; + return api.client.blockInstance(form); +} + export function delay(millis = 500) { return new Promise(resolve => setTimeout(resolve, millis)); } diff --git a/api_tests/yarn.lock b/api_tests/yarn.lock index 17c5a2849..3b6a85146 100644 --- a/api_tests/yarn.lock +++ b/api_tests/yarn.lock @@ -2174,10 +2174,10 @@ kleur@^3.0.3: resolved "https://registry.yarnpkg.com/kleur/-/kleur-3.0.3.tgz#a79c9ecc86ee1ce3fa6206d1216c501f147fc07e" integrity sha512-eTIzlVOSUR+JxdDFepEYcBMtZ9Qqdef+rnzWdRZuMbOywu5tO2w2N7rqjoANZ5k9vywhL6Br1VRjUIgTQx4E8w== -lemmy-js-client@0.19.0-rc.3: - version "0.19.0-rc.3" - resolved "https://registry.yarnpkg.com/lemmy-js-client/-/lemmy-js-client-0.19.0-rc.3.tgz#1efbfd5ce492319227a41cb020fc1cf9b2e7c075" - integrity sha512-RmibQ3+YTvqsQ89II2I29pfPmVAWiSObGAU9Nc/AGYfyvaCya7f5+TirKwHdKA2eWDWLOTnD4rm6WgcgAwvhWw== +lemmy-js-client@0.19.0-rc.5: + version "0.19.0-rc.5" + resolved "https://registry.yarnpkg.com/lemmy-js-client/-/lemmy-js-client-0.19.0-rc.5.tgz#a0d3e0ac829b1e46124edcf3a0343ae04317d51a" + integrity sha512-Z1T95Ht1VZNvWlLH9XpVnO2oC7LhMT81jTiU5BhYPIkEtGhOwN91jk5uI1oyI6/d4v9lwbrsyzFYPsiuMmXTFQ== dependencies: cross-fetch "^3.1.5" form-data "^4.0.0" diff --git a/crates/api/src/site/block.rs b/crates/api/src/site/block.rs new file mode 100644 index 000000000..3278767cf --- /dev/null +++ b/crates/api/src/site/block.rs @@ -0,0 +1,41 @@ +use activitypub_federation::config::Data; +use actix_web::web::Json; +use lemmy_api_common::{ + context::LemmyContext, + site::{BlockInstance, BlockInstanceResponse}, + utils::local_user_view_from_jwt, +}; +use lemmy_db_schema::{ + source::instance_block::{InstanceBlock, InstanceBlockForm}, + traits::Blockable, +}; +use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType}; + +#[tracing::instrument(skip(context))] +pub async fn block_instance( + data: Json, + context: Data, +) -> Result, LemmyError> { + let local_user_view = local_user_view_from_jwt(&data.auth, &context).await?; + + let instance_id = data.instance_id; + let person_id = local_user_view.person.id; + let instance_block_form = InstanceBlockForm { + person_id, + instance_id, + }; + + if data.block { + InstanceBlock::block(&mut context.pool(), &instance_block_form) + .await + .with_lemmy_type(LemmyErrorType::InstanceBlockAlreadyExists)?; + } else { + InstanceBlock::unblock(&mut context.pool(), &instance_block_form) + .await + .with_lemmy_type(LemmyErrorType::InstanceBlockAlreadyExists)?; + } + + Ok(Json(BlockInstanceResponse { + blocked: data.block, + })) +} diff --git a/crates/api/src/site/mod.rs b/crates/api/src/site/mod.rs index 85c20e272..d63c77ad9 100644 --- a/crates/api/src/site/mod.rs +++ b/crates/api/src/site/mod.rs @@ -1,3 +1,4 @@ +pub mod block; pub mod federated_instances; pub mod leave_admin; pub mod mod_log; diff --git a/crates/api_common/src/site.rs b/crates/api_common/src/site.rs index 0dc796ed8..7ed94f2de 100644 --- a/crates/api_common/src/site.rs +++ b/crates/api_common/src/site.rs @@ -1,6 +1,6 @@ use crate::sensitive::Sensitive; use lemmy_db_schema::{ - newtypes::{CommentId, CommunityId, LanguageId, PersonId, PostId}, + newtypes::{CommentId, CommunityId, InstanceId, LanguageId, PersonId, PostId}, source::{instance::Instance, language::Language, tagline::Tagline}, ListingType, ModlogActionType, @@ -21,6 +21,7 @@ use lemmy_db_views_actor::structs::{ CommunityFollowerView, CommunityModeratorView, CommunityView, + InstanceBlockView, PersonBlockView, PersonView, }; @@ -320,6 +321,7 @@ pub struct MyUserInfo { pub follows: Vec, pub moderates: Vec, pub community_blocks: Vec, + pub instance_blocks: Vec, pub person_blocks: Vec, pub discussion_languages: Vec, } @@ -450,3 +452,21 @@ pub struct GetUnreadRegistrationApplicationCount { pub struct GetUnreadRegistrationApplicationCountResponse { pub registration_applications: i64, } + +#[derive(Debug, Serialize, Deserialize, Clone, Default)] +#[cfg_attr(feature = "full", derive(TS))] +#[cfg_attr(feature = "full", ts(export))] +/// Block an instance as user +pub struct BlockInstance { + pub instance_id: InstanceId, + pub block: bool, + pub auth: Sensitive, +} + +#[skip_serializing_none] +#[derive(Debug, Serialize, Deserialize, Clone)] +#[cfg_attr(feature = "full", derive(TS))] +#[cfg_attr(feature = "full", ts(export))] +pub struct BlockInstanceResponse { + pub blocked: bool, +} diff --git a/crates/api_crud/src/site/read.rs b/crates/api_crud/src/site/read.rs index 62d96492a..d435b8684 100644 --- a/crates/api_crud/src/site/read.rs +++ b/crates/api_crud/src/site/read.rs @@ -18,6 +18,7 @@ use lemmy_db_views_actor::structs::{ CommunityBlockView, CommunityFollowerView, CommunityModeratorView, + InstanceBlockView, PersonBlockView, PersonView, }; @@ -52,6 +53,10 @@ pub async fn get_site( .await .with_lemmy_type(LemmyErrorType::SystemErrLogin)?; + let instance_blocks = InstanceBlockView::for_person(&mut context.pool(), person_id) + .await + .with_lemmy_type(LemmyErrorType::SystemErrLogin)?; + let person_id = local_user_view.person.id; let person_blocks = PersonBlockView::for_person(&mut context.pool(), person_id) .await @@ -70,6 +75,7 @@ pub async fn get_site( follows, moderates, community_blocks, + instance_blocks, person_blocks, discussion_languages, }) diff --git a/crates/db_schema/src/aggregates/structs.rs b/crates/db_schema/src/aggregates/structs.rs index f24f02f49..03ff9a640 100644 --- a/crates/db_schema/src/aggregates/structs.rs +++ b/crates/db_schema/src/aggregates/structs.rs @@ -1,4 +1,4 @@ -use crate::newtypes::{CommentId, CommunityId, PersonId, PostId, SiteId}; +use crate::newtypes::{CommentId, CommunityId, InstanceId, PersonId, PostId, SiteId}; #[cfg(feature = "full")] use crate::schema::{ comment_aggregates, @@ -100,6 +100,7 @@ pub struct PostAggregates { pub community_id: CommunityId, pub creator_id: PersonId, pub controversy_rank: f64, + pub instance_id: InstanceId, /// A rank that amplifies smaller communities pub scaled_rank: f64, } diff --git a/crates/db_schema/src/impls/instance_block.rs b/crates/db_schema/src/impls/instance_block.rs new file mode 100644 index 000000000..f7f81a234 --- /dev/null +++ b/crates/db_schema/src/impls/instance_block.rs @@ -0,0 +1,36 @@ +use crate::{ + schema::instance_block::dsl::{instance_block, instance_id, person_id}, + source::instance_block::{InstanceBlock, InstanceBlockForm}, + traits::Blockable, + utils::{get_conn, DbPool}, +}; +use diesel::{dsl::insert_into, result::Error, ExpressionMethods, QueryDsl}; +use diesel_async::RunQueryDsl; + +#[async_trait] +impl Blockable for InstanceBlock { + type Form = InstanceBlockForm; + async fn block(pool: &mut DbPool<'_>, instance_block_form: &Self::Form) -> Result { + let conn = &mut get_conn(pool).await?; + insert_into(instance_block) + .values(instance_block_form) + .on_conflict((person_id, instance_id)) + .do_update() + .set(instance_block_form) + .get_result::(conn) + .await + } + async fn unblock( + pool: &mut DbPool<'_>, + instance_block_form: &Self::Form, + ) -> Result { + let conn = &mut get_conn(pool).await?; + diesel::delete( + instance_block + .filter(person_id.eq(instance_block_form.person_id)) + .filter(instance_id.eq(instance_block_form.instance_id)), + ) + .execute(conn) + .await + } +} diff --git a/crates/db_schema/src/impls/mod.rs b/crates/db_schema/src/impls/mod.rs index a8aabf18d..a46131f83 100644 --- a/crates/db_schema/src/impls/mod.rs +++ b/crates/db_schema/src/impls/mod.rs @@ -12,6 +12,7 @@ pub mod federation_allowlist; pub mod federation_blocklist; pub mod image_upload; pub mod instance; +pub mod instance_block; pub mod language; pub mod local_site; pub mod local_site_rate_limit; diff --git a/crates/db_schema/src/schema.rs b/crates/db_schema/src/schema.rs index 62b6ea80a..bec1ab40e 100644 --- a/crates/db_schema/src/schema.rs +++ b/crates/db_schema/src/schema.rs @@ -337,6 +337,15 @@ diesel::table! { } } +diesel::table! { + instance_block (id) { + id -> Int4, + person_id -> Int4, + instance_id -> Int4, + published -> Timestamptz, + } +} + diesel::table! { language (id) { id -> Int4, @@ -712,6 +721,7 @@ diesel::table! { community_id -> Int4, creator_id -> Int4, controversy_rank -> Float8, + instance_id -> Int4, scaled_rank -> Float8, } } @@ -928,6 +938,8 @@ diesel::joinable!(federation_allowlist -> instance (instance_id)); diesel::joinable!(federation_blocklist -> instance (instance_id)); diesel::joinable!(federation_queue_state -> instance (instance_id)); diesel::joinable!(image_upload -> local_user (local_user_id)); +diesel::joinable!(instance_block -> instance (instance_id)); +diesel::joinable!(instance_block -> person (person_id)); diesel::joinable!(local_site -> site (site_id)); diesel::joinable!(local_site_rate_limit -> local_site (local_site_id)); diesel::joinable!(local_user -> person (person_id)); @@ -960,6 +972,7 @@ diesel::joinable!(post -> community (community_id)); diesel::joinable!(post -> language (language_id)); diesel::joinable!(post -> person (creator_id)); diesel::joinable!(post_aggregates -> community (community_id)); +diesel::joinable!(post_aggregates -> instance (instance_id)); diesel::joinable!(post_aggregates -> person (creator_id)); diesel::joinable!(post_aggregates -> post (post_id)); diesel::joinable!(post_like -> person (person_id)); @@ -1005,6 +1018,7 @@ diesel::allow_tables_to_appear_in_same_query!( federation_queue_state, image_upload, instance, + instance_block, language, local_site, local_site_rate_limit, diff --git a/crates/db_schema/src/source/instance_block.rs b/crates/db_schema/src/source/instance_block.rs new file mode 100644 index 000000000..1aa215e45 --- /dev/null +++ b/crates/db_schema/src/source/instance_block.rs @@ -0,0 +1,26 @@ +use crate::newtypes::{InstanceId, PersonId}; +#[cfg(feature = "full")] +use crate::schema::instance_block; +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; + +#[derive(Clone, PartialEq, Eq, Debug, Serialize, Deserialize)] +#[cfg_attr(feature = "full", derive(Queryable, Associations, Identifiable))] +#[cfg_attr( + feature = "full", + diesel(belongs_to(crate::source::instance::Instance)) +)] +#[cfg_attr(feature = "full", diesel(table_name = instance_block))] +pub struct InstanceBlock { + pub id: i32, + pub person_id: PersonId, + pub instance_id: InstanceId, + pub published: DateTime, +} + +#[cfg_attr(feature = "full", derive(Insertable, AsChangeset))] +#[cfg_attr(feature = "full", diesel(table_name = instance_block))] +pub struct InstanceBlockForm { + pub person_id: PersonId, + pub instance_id: InstanceId, +} diff --git a/crates/db_schema/src/source/mod.rs b/crates/db_schema/src/source/mod.rs index a200806cd..8288126ec 100644 --- a/crates/db_schema/src/source/mod.rs +++ b/crates/db_schema/src/source/mod.rs @@ -17,6 +17,7 @@ pub mod federation_allowlist; pub mod federation_blocklist; pub mod image_upload; pub mod instance; +pub mod instance_block; pub mod language; pub mod local_site; pub mod local_site_rate_limit; diff --git a/crates/db_views/src/comment_view.rs b/crates/db_views/src/comment_view.rs index b93f12842..a3f35b0a9 100644 --- a/crates/db_views/src/comment_view.rs +++ b/crates/db_views/src/comment_view.rs @@ -23,6 +23,7 @@ use lemmy_db_schema::{ community_follower, community_moderator, community_person_ban, + instance_block, local_user_language, person, person_block, @@ -121,6 +122,13 @@ fn queries<'a>() -> Queries< let local_user_id_join = local_user_id.unwrap_or(LocalUserId(-1)); let mut query = all_joins(comment::table.into_boxed(), person_id) + .left_join( + instance_block::table.on( + community::instance_id + .eq(instance_block::instance_id) + .and(instance_block::person_id.eq(person_id_join)), + ), + ) .left_join( community_block::table.on( community::id @@ -221,6 +229,7 @@ fn queries<'a>() -> Queries< // Don't show blocked communities or persons if options.post_id.is_none() { + query = query.filter(instance_block::person_id.is_null()); query = query.filter(community_block::person_id.is_null()); } query = query.filter(person_block::person_id.is_null()); diff --git a/crates/db_views/src/post_view.rs b/crates/db_views/src/post_view.rs index 607af986d..87c31d7e5 100644 --- a/crates/db_views/src/post_view.rs +++ b/crates/db_views/src/post_view.rs @@ -28,6 +28,7 @@ use lemmy_db_schema::{ community_follower, community_moderator, community_person_ban, + instance_block, local_user_language, person, person_block, @@ -420,7 +421,7 @@ fn queries<'a>() -> Queries< ), )); - // Don't show blocked communities or persons + // Don't show blocked instances, communities or persons query = query.filter(not(exists( community_block::table.filter( post_aggregates::community_id @@ -428,6 +429,13 @@ fn queries<'a>() -> Queries< .and(community_block::person_id.eq(person_id_join)), ), ))); + query = query.filter(not(exists( + instance_block::table.filter( + post_aggregates::instance_id + .eq(instance_block::instance_id) + .and(instance_block::person_id.eq(person_id_join)), + ), + ))); query = query.filter(not(is_creator_blocked(person_id))); } let now = diesel::dsl::now.into_sql::(); @@ -706,6 +714,7 @@ mod tests { community::{Community, CommunityInsertForm}, community_block::{CommunityBlock, CommunityBlockForm}, instance::Instance, + instance_block::{InstanceBlock, InstanceBlockForm}, language::Language, local_user::{LocalUser, LocalUserInsertForm, LocalUserUpdateForm}, person::{Person, PersonInsertForm}, @@ -1239,6 +1248,84 @@ mod tests { cleanup(data, pool).await; } + #[tokio::test] + #[serial] + async fn post_listing_instance_block() { + let pool = &build_db_pool_for_tests().await; + let pool = &mut pool.into(); + let data = init_data(pool).await; + + let blocked_instance = Instance::read_or_create(pool, "another_domain.tld".to_string()) + .await + .unwrap(); + + let community_form = CommunityInsertForm::builder() + .name("test_community_4".to_string()) + .title("none".to_owned()) + .public_key("pubkey".to_string()) + .instance_id(blocked_instance.id) + .build(); + let inserted_community = Community::create(pool, &community_form).await.unwrap(); + + let post_form = PostInsertForm::builder() + .name("blocked instance post".to_string()) + .creator_id(data.inserted_bot.id) + .community_id(inserted_community.id) + .language_id(Some(LanguageId(1))) + .build(); + + let post_from_blocked_instance = Post::create(pool, &post_form).await.unwrap(); + + // no instance block, should return all posts + let post_listings_all = PostQuery { + local_user: Some(&data.local_user_view), + ..Default::default() + } + .list(pool) + .await + .unwrap(); + assert_eq!(post_listings_all.len(), 3); + + // block the instance + let block_form = InstanceBlockForm { + person_id: data.local_user_view.person.id, + instance_id: blocked_instance.id, + }; + InstanceBlock::block(pool, &block_form).await.unwrap(); + + // now posts from communities on that instance should be hidden + let post_listings_blocked = PostQuery { + local_user: Some(&data.local_user_view), + ..Default::default() + } + .list(pool) + .await + .unwrap(); + assert_eq!(post_listings_blocked.len(), 2); + assert_ne!( + post_listings_blocked[0].post.id, + post_from_blocked_instance.id + ); + assert_ne!( + post_listings_blocked[1].post.id, + post_from_blocked_instance.id + ); + + // after unblocking it should return all posts again + InstanceBlock::unblock(pool, &block_form).await.unwrap(); + let post_listings_blocked = PostQuery { + local_user: Some(&data.local_user_view), + ..Default::default() + } + .list(pool) + .await + .unwrap(); + assert_eq!(post_listings_blocked.len(), 3); + + Instance::delete(pool, blocked_instance.id).await.unwrap(); + cleanup(data, pool).await; + } + async fn cleanup(data: Data, pool: &mut DbPool<'_>) { let num_deleted = Post::delete(pool, data.inserted_post.id).await.unwrap(); Community::delete(pool, data.inserted_community.id) @@ -1359,6 +1446,7 @@ mod tests { scaled_rank: 0.3621, community_id: inserted_post.community_id, creator_id: inserted_post.creator_id, + instance_id: data.inserted_instance.id, }, subscribed: SubscribedType::NotSubscribed, read: false, diff --git a/crates/db_views_actor/src/community_view.rs b/crates/db_views_actor/src/community_view.rs index 974267061..f646e21f8 100644 --- a/crates/db_views_actor/src/community_view.rs +++ b/crates/db_views_actor/src/community_view.rs @@ -12,7 +12,14 @@ use diesel::{ use diesel_async::RunQueryDsl; use lemmy_db_schema::{ newtypes::{CommunityId, PersonId}, - schema::{community, community_aggregates, community_block, community_follower, local_user}, + schema::{ + community, + community_aggregates, + community_block, + community_follower, + instance_block, + local_user, + }, source::{community::CommunityFollower, local_user::LocalUser}, utils::{fuzzy_search, limit_and_offset, DbConn, DbPool, ListFn, Queries, ReadFn}, ListingType, @@ -36,6 +43,13 @@ fn queries<'a>() -> Queries< .and(community_follower::person_id.eq(person_id_join)), ), ) + .left_join( + instance_block::table.on( + community::instance_id + .eq(instance_block::instance_id) + .and(instance_block::person_id.eq(person_id_join)), + ), + ) .left_join( community_block::table.on( community::id @@ -131,8 +145,10 @@ fn queries<'a>() -> Queries< }; } - // Don't show blocked communities or nsfw communities if not enabled in profile + // Don't show blocked communities and communities on blocked instances. nsfw communities are + // also hidden (based on profile setting) if options.local_user.is_some() { + query = query.filter(instance_block::person_id.is_null()); query = query.filter(community_block::person_id.is_null()); query = query.filter(community::nsfw.eq(false).or(local_user::show_nsfw.eq(true))); } else { diff --git a/crates/db_views_actor/src/instance_block_view.rs b/crates/db_views_actor/src/instance_block_view.rs new file mode 100644 index 000000000..05820862a --- /dev/null +++ b/crates/db_views_actor/src/instance_block_view.rs @@ -0,0 +1,27 @@ +use crate::structs::InstanceBlockView; +use diesel::{result::Error, ExpressionMethods, JoinOnDsl, NullableExpressionMethods, QueryDsl}; +use diesel_async::RunQueryDsl; +use lemmy_db_schema::{ + newtypes::PersonId, + schema::{instance, instance_block, person, site}, + utils::{get_conn, DbPool}, +}; + +impl InstanceBlockView { + pub async fn for_person(pool: &mut DbPool<'_>, person_id: PersonId) -> Result, Error> { + let conn = &mut get_conn(pool).await?; + instance_block::table + .inner_join(person::table) + .inner_join(instance::table) + .left_join(site::table.on(site::instance_id.eq(instance::id))) + .select(( + person::all_columns, + instance::all_columns, + site::all_columns.nullable(), + )) + .filter(instance_block::person_id.eq(person_id)) + .order_by(instance_block::published) + .load::(conn) + .await + } +} diff --git a/crates/db_views_actor/src/lib.rs b/crates/db_views_actor/src/lib.rs index 686945505..e9f8e4189 100644 --- a/crates/db_views_actor/src/lib.rs +++ b/crates/db_views_actor/src/lib.rs @@ -11,6 +11,8 @@ pub mod community_person_ban_view; #[cfg(feature = "full")] pub mod community_view; #[cfg(feature = "full")] +pub mod instance_block_view; +#[cfg(feature = "full")] pub mod person_block_view; #[cfg(feature = "full")] pub mod person_mention_view; diff --git a/crates/db_views_actor/src/structs.rs b/crates/db_views_actor/src/structs.rs index 3728bb02c..bdc9e6bbd 100644 --- a/crates/db_views_actor/src/structs.rs +++ b/crates/db_views_actor/src/structs.rs @@ -6,9 +6,11 @@ use lemmy_db_schema::{ comment::Comment, comment_reply::CommentReply, community::Community, + instance::Instance, person::Person, person_mention::PersonMention, post::Post, + site::Site, }, SubscribedType, }; @@ -26,6 +28,17 @@ pub struct CommunityBlockView { pub community: Community, } +#[skip_serializing_none] +#[derive(Debug, Serialize, Deserialize, Clone)] +#[cfg_attr(feature = "full", derive(TS, Queryable))] +#[cfg_attr(feature = "full", ts(export))] +/// An instance block by a user. +pub struct InstanceBlockView { + pub person: Person, + pub instance: Instance, + pub site: Option, +} + #[derive(Debug, Serialize, Deserialize, Clone)] #[cfg_attr(feature = "full", derive(TS, Queryable))] #[cfg_attr(feature = "full", ts(export))] diff --git a/crates/utils/src/error.rs b/crates/utils/src/error.rs index 9326805fb..8c3523363 100644 --- a/crates/utils/src/error.rs +++ b/crates/utils/src/error.rs @@ -209,6 +209,7 @@ pub enum LemmyErrorType { InvalidUrlScheme, CouldntSendWebmention, ContradictingFilters, + InstanceBlockAlreadyExists, AuthCookieInsecure, Unknown(String), } diff --git a/migrations/2023-08-09-101305_user_instance_block/down.sql b/migrations/2023-08-09-101305_user_instance_block/down.sql new file mode 100644 index 000000000..e24a6ea0f --- /dev/null +++ b/migrations/2023-08-09-101305_user_instance_block/down.sql @@ -0,0 +1,21 @@ +DROP TABLE instance_block; + +ALTER TABLE post_aggregates + DROP COLUMN instance_id; + +CREATE OR REPLACE FUNCTION post_aggregates_post () + RETURNS TRIGGER + LANGUAGE plpgsql + AS $$ +BEGIN + IF (TG_OP = 'INSERT') THEN + INSERT INTO post_aggregates (post_id, published, newest_comment_time, newest_comment_time_necro, community_id, creator_id) + VALUES (NEW.id, NEW.published, NEW.published, NEW.published, NEW.community_id, NEW.creator_id); + ELSIF (TG_OP = 'DELETE') THEN + DELETE FROM post_aggregates + WHERE post_id = OLD.id; + END IF; + RETURN NULL; +END +$$; + diff --git a/migrations/2023-08-09-101305_user_instance_block/up.sql b/migrations/2023-08-09-101305_user_instance_block/up.sql new file mode 100644 index 000000000..7b3668a82 --- /dev/null +++ b/migrations/2023-08-09-101305_user_instance_block/up.sql @@ -0,0 +1,51 @@ +CREATE TABLE instance_block ( + id serial PRIMARY KEY, + person_id int REFERENCES person ON UPDATE CASCADE ON DELETE CASCADE NOT NULL, + instance_id int REFERENCES instance ON UPDATE CASCADE ON DELETE CASCADE NOT NULL, + published timestamptz NOT NULL DEFAULT now(), + UNIQUE (person_id, instance_id) +); + +ALTER TABLE post_aggregates + ADD COLUMN instance_id integer REFERENCES instance ON UPDATE CASCADE ON DELETE CASCADE; + +CREATE OR REPLACE FUNCTION post_aggregates_post () + RETURNS TRIGGER + LANGUAGE plpgsql + AS $$ +BEGIN + IF (TG_OP = 'INSERT') THEN + INSERT INTO post_aggregates (post_id, published, newest_comment_time, newest_comment_time_necro, community_id, creator_id, instance_id) + SELECT + NEW.id, + NEW.published, + NEW.published, + NEW.published, + NEW.community_id, + NEW.creator_id, + community.instance_id + FROM + community + WHERE + NEW.community_id = community.id; + ELSIF (TG_OP = 'DELETE') THEN + DELETE FROM post_aggregates + WHERE post_id = OLD.id; + END IF; + RETURN NULL; +END +$$; + +UPDATE + post_aggregates +SET + instance_id = community.instance_id +FROM + post + JOIN community ON post.community_id = community.id +WHERE + post.id = post_aggregates.post_id; + +ALTER TABLE post_aggregates + ALTER COLUMN instance_id SET NOT NULL; + diff --git a/src/api_routes_http.rs b/src/api_routes_http.rs index 74003b8d8..183034d05 100644 --- a/src/api_routes_http.rs +++ b/src/api_routes_http.rs @@ -56,6 +56,7 @@ use lemmy_api::{ resolve::resolve_pm_report, }, site::{ + block::block_instance, federated_instances::get_federated_instances, leave_admin::leave_admin, mod_log::get_mod_log, @@ -129,7 +130,8 @@ pub fn config(cfg: &mut web::ServiceConfig, rate_limit: &RateLimitCell) { .route("", web::get().to(get_site)) // Admin Actions .route("", web::post().to(create_site)) - .route("", web::put().to(update_site)), + .route("", web::put().to(update_site)) + .route("/block", web::post().to(block_instance)), ) .service( web::resource("/modlog") From a866b3424dd8fe22c8120ecc2a7e46ae821ce864 Mon Sep 17 00:00:00 2001 From: SleeplessOne1917 Date: Wed, 20 Sep 2023 13:14:51 +0000 Subject: [PATCH 3/5] Make local federated instances use comnpose file for build (#3968) * Make local federated instances use comnpose file for build * Remove commented out part of script --- docker/federation/docker-compose.yml | 10 ++++++---- docker/federation/start-local-instances.bash | 7 ++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/docker/federation/docker-compose.yml b/docker/federation/docker-compose.yml index 39ff95248..634a97449 100644 --- a/docker/federation/docker-compose.yml +++ b/docker/federation/docker-compose.yml @@ -2,16 +2,18 @@ version: "3.7" x-ui-default: &ui-default init: true - image: dessalines/lemmy-ui:0.18.1 + image: dessalines/lemmy-ui:0.18.4 # assuming lemmy-ui is cloned besides lemmy directory # build: - # context: ../../../lemmy-ui - # dockerfile: dev.dockerfile + # context: ../../../lemmy-ui + # dockerfile: dev.dockerfile environment: - LEMMY_UI_HTTPS=false x-lemmy-default: &lemmy-default - image: lemmy-federation:latest + build: + context: ../.. + dockerfile: docker/Dockerfile environment: - RUST_BACKTRACE=1 - RUST_LOG="warn,lemmy_server=debug,lemmy_api=debug,lemmy_api_common=debug,lemmy_api_crud=debug,lemmy_apub=debug,lemmy_db_schema=debug,lemmy_db_views=debug,lemmy_db_views_actor=debug,lemmy_db_views_moderator=debug,lemmy_routes=debug,lemmy_utils=debug,lemmy_websocket=debug" diff --git a/docker/federation/start-local-instances.bash b/docker/federation/start-local-instances.bash index b2350d3a8..716f4d45c 100755 --- a/docker/federation/start-local-instances.bash +++ b/docker/federation/start-local-instances.bash @@ -1,14 +1,11 @@ #!/bin/bash set -e -sudo docker-compose down - -sudo docker build ../../ --file ../Dockerfile -t lemmy-federation:latest +sudo docker compose down for Item in alpha beta gamma delta epsilon ; do sudo mkdir -p volumes/pictrs_$Item sudo chown -R 991:991 volumes/pictrs_$Item done -#sudo docker-compose pull --ignore-pull-failures || true -sudo docker-compose up +sudo docker compose up From 77b2d236b952bc50667be8c9f40b56e180951e07 Mon Sep 17 00:00:00 2001 From: Dessalines Date: Wed, 20 Sep 2023 09:28:56 -0400 Subject: [PATCH 4/5] Forgot to add ts export to pagination cursor. (#3971) --- crates/db_views/src/structs.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/db_views/src/structs.rs b/crates/db_views/src/structs.rs index dd0bba184..10847b053 100644 --- a/crates/db_views/src/structs.rs +++ b/crates/db_views/src/structs.rs @@ -94,6 +94,7 @@ pub struct PostReportView { /// (api users love to make assumptions (e.g. parse stuff that looks like numbers as numbers) about apis that aren't part of the spec #[derive(Serialize, Deserialize, Debug, Clone)] #[cfg_attr(feature = "full", derive(ts_rs::TS))] +#[cfg_attr(feature = "full", ts(export))] pub struct PaginationCursor(pub(crate) String); #[skip_serializing_none] From 22608ae983cf6ce73ea6f3acd43b05c2a9bd8a60 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Wed, 20 Sep 2023 16:49:54 +0200 Subject: [PATCH 5/5] Rework the way 2FA is enabled/disabled (fixes #3309) (#3959) * Rework the way 2FA is enabled/disabled (fixes #3309) * postgres format * change algo to sha1 for better compat * review comments * review * clippy --------- Co-authored-by: Dessalines --- Cargo.lock | 2 +- crates/api/Cargo.toml | 1 + crates/api/src/lib.rs | 63 ++++++++++++++++++- .../src/local_user/generate_totp_secret.rs | 47 ++++++++++++++ crates/api/src/local_user/login.rs | 13 ++-- crates/api/src/local_user/mod.rs | 2 + crates/api/src/local_user/save_settings.rs | 24 +------ crates/api/src/local_user/update_totp.rs | 54 ++++++++++++++++ crates/api_common/src/person.rs | 26 ++++++-- crates/db_schema/src/schema.rs | 2 +- crates/db_schema/src/source/local_user.rs | 7 +-- .../src/registration_application_view.rs | 2 +- crates/utils/Cargo.toml | 1 - crates/utils/src/error.rs | 4 +- crates/utils/src/utils/validation.rs | 60 +----------------- .../down.sql | 6 ++ .../2023-09-11-110040_rework-2fa-setup/up.sql | 6 ++ src/api_routes_http.rs | 6 +- 18 files changed, 221 insertions(+), 105 deletions(-) create mode 100644 crates/api/src/local_user/generate_totp_secret.rs create mode 100644 crates/api/src/local_user/update_totp.rs create mode 100644 migrations/2023-09-11-110040_rework-2fa-setup/down.sql create mode 100644 migrations/2023-09-11-110040_rework-2fa-setup/up.sql diff --git a/Cargo.lock b/Cargo.lock index bb72acb69..dee04c62b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2627,6 +2627,7 @@ dependencies = [ "serial_test", "sitemap-rs", "tokio", + "totp-rs", "tracing", "url", "uuid", @@ -2937,7 +2938,6 @@ dependencies = [ "strum", "strum_macros", "tokio", - "totp-rs", "tracing", "tracing-error", "ts-rs", diff --git a/crates/api/Cargo.toml b/crates/api/Cargo.toml index f5b0e3924..d99fb7691 100644 --- a/crates/api/Cargo.toml +++ b/crates/api/Cargo.toml @@ -34,6 +34,7 @@ chrono = { workspace = true } url = { workspace = true } wav = "1.0.0" sitemap-rs = "0.2.0" +totp-rs = { version = "5.0.2", features = ["gen_secret", "otpauth"] } [dev-dependencies] serial_test = { workspace = true } diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index 65c8d0551..22a021970 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -2,11 +2,13 @@ use base64::{engine::general_purpose::STANDARD_NO_PAD as base64, Engine}; use captcha::Captcha; use lemmy_api_common::utils::local_site_to_slur_regex; use lemmy_db_schema::source::local_site::LocalSite; +use lemmy_db_views::structs::LocalUserView; use lemmy_utils::{ - error::{LemmyError, LemmyErrorExt, LemmyErrorType}, + error::{LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult}, utils::slurs::check_slurs, }; use std::io::Cursor; +use totp_rs::{Secret, TOTP}; pub mod comment; pub mod comment_report; @@ -67,11 +69,63 @@ pub(crate) fn check_report_reason(reason: &str, local_site: &LocalSite) -> Resul } } +pub(crate) fn check_totp_2fa_valid( + local_user_view: &LocalUserView, + totp_token: &Option, + site_name: &str, +) -> LemmyResult<()> { + // Throw an error if their token is missing + let token = totp_token + .as_deref() + .ok_or(LemmyErrorType::MissingTotpToken)?; + let secret = local_user_view + .local_user + .totp_2fa_secret + .as_deref() + .ok_or(LemmyErrorType::MissingTotpSecret)?; + + let totp = build_totp_2fa(site_name, &local_user_view.person.name, secret)?; + + let check_passed = totp.check_current(token)?; + if !check_passed { + return Err(LemmyErrorType::IncorrectTotpToken.into()); + } + + Ok(()) +} + +pub(crate) fn generate_totp_2fa_secret() -> String { + Secret::generate_secret().to_string() +} + +pub(crate) fn build_totp_2fa( + site_name: &str, + username: &str, + secret: &str, +) -> Result { + let sec = Secret::Raw(secret.as_bytes().to_vec()); + let sec_bytes = sec + .to_bytes() + .map_err(|_| LemmyErrorType::CouldntParseTotpSecret)?; + + TOTP::new( + totp_rs::Algorithm::SHA1, + 6, + 1, + 30, + sec_bytes, + Some(site_name.to_string()), + username.to_string(), + ) + .with_lemmy_type(LemmyErrorType::CouldntGenerateTotp) +} + #[cfg(test)] mod tests { #![allow(clippy::unwrap_used)] #![allow(clippy::indexing_slicing)] + use super::*; use lemmy_api_common::utils::check_validator_time; use lemmy_db_schema::{ source::{ @@ -134,4 +188,11 @@ mod tests { let num_deleted = Person::delete(pool, inserted_person.id).await.unwrap(); assert_eq!(1, num_deleted); } + + #[test] + fn test_build_totp() { + let generated_secret = generate_totp_2fa_secret(); + let totp = build_totp_2fa("lemmy", "my_name", &generated_secret); + assert!(totp.is_ok()); + } } diff --git a/crates/api/src/local_user/generate_totp_secret.rs b/crates/api/src/local_user/generate_totp_secret.rs new file mode 100644 index 000000000..a983beaaa --- /dev/null +++ b/crates/api/src/local_user/generate_totp_secret.rs @@ -0,0 +1,47 @@ +use crate::{build_totp_2fa, generate_totp_2fa_secret}; +use activitypub_federation::config::Data; +use actix_web::web::Json; +use lemmy_api_common::{ + context::LemmyContext, + person::GenerateTotpSecretResponse, + sensitive::Sensitive, +}; +use lemmy_db_schema::{ + source::local_user::{LocalUser, LocalUserUpdateForm}, + traits::Crud, +}; +use lemmy_db_views::structs::{LocalUserView, SiteView}; +use lemmy_utils::error::{LemmyError, LemmyErrorType}; + +/// Generate a new secret for two-factor-authentication. Afterwards you need to call [toggle_totp] +/// to enable it. This can only be called if 2FA is currently disabled. +#[tracing::instrument(skip(context))] +pub async fn generate_totp_secret( + local_user_view: LocalUserView, + context: Data, +) -> Result, LemmyError> { + let site_view = SiteView::read_local(&mut context.pool()).await?; + + if local_user_view.local_user.totp_2fa_enabled { + return Err(LemmyErrorType::TotpAlreadyEnabled)?; + } + + let secret = generate_totp_2fa_secret(); + let secret_url = + build_totp_2fa(&site_view.site.name, &local_user_view.person.name, &secret)?.get_url(); + + let local_user_form = LocalUserUpdateForm { + totp_2fa_secret: Some(Some(secret)), + ..Default::default() + }; + LocalUser::update( + &mut context.pool(), + local_user_view.local_user.id, + &local_user_form, + ) + .await?; + + Ok(Json(GenerateTotpSecretResponse { + totp_secret_url: Sensitive::new(secret_url), + })) +} diff --git a/crates/api/src/local_user/login.rs b/crates/api/src/local_user/login.rs index 915aff939..5dd3e29d8 100644 --- a/crates/api/src/local_user/login.rs +++ b/crates/api/src/local_user/login.rs @@ -1,3 +1,4 @@ +use crate::check_totp_2fa_valid; use actix_web::web::{Data, Json}; use bcrypt::verify; use lemmy_api_common::{ @@ -9,7 +10,6 @@ use lemmy_db_views::structs::{LocalUserView, SiteView}; use lemmy_utils::{ claims::Claims, error::{LemmyError, LemmyErrorExt, LemmyErrorType}, - utils::validation::check_totp_2fa_valid, }; #[tracing::instrument(skip(context))] @@ -53,13 +53,10 @@ pub async fn login( check_registration_application(&local_user_view, &site_view.local_site, &mut context.pool()) .await?; - // Check the totp - check_totp_2fa_valid( - &local_user_view.local_user.totp_2fa_secret, - &data.totp_2fa_token, - &site_view.site.name, - &local_user_view.person.name, - )?; + // Check the totp if enabled + if local_user_view.local_user.totp_2fa_enabled { + check_totp_2fa_valid(&local_user_view, &data.totp_2fa_token, &site_view.site.name)?; + } // Return the jwt Ok(Json(LoginResponse { diff --git a/crates/api/src/local_user/mod.rs b/crates/api/src/local_user/mod.rs index 806fa66a2..d5f351116 100644 --- a/crates/api/src/local_user/mod.rs +++ b/crates/api/src/local_user/mod.rs @@ -3,6 +3,7 @@ pub mod ban_person; pub mod block; pub mod change_password; pub mod change_password_after_reset; +pub mod generate_totp_secret; pub mod get_captcha; pub mod list_banned; pub mod login; @@ -10,4 +11,5 @@ pub mod notifications; pub mod report_count; pub mod reset_password; pub mod save_settings; +pub mod update_totp; pub mod verify_email; diff --git a/crates/api/src/local_user/save_settings.rs b/crates/api/src/local_user/save_settings.rs index 8368eada0..045a3c2f7 100644 --- a/crates/api/src/local_user/save_settings.rs +++ b/crates/api/src/local_user/save_settings.rs @@ -17,13 +17,7 @@ use lemmy_db_views::structs::SiteView; use lemmy_utils::{ claims::Claims, error::{LemmyError, LemmyErrorExt, LemmyErrorType}, - utils::validation::{ - build_totp_2fa, - generate_totp_2fa_secret, - is_valid_bio_field, - is_valid_display_name, - is_valid_matrix_id, - }, + utils::validation::{is_valid_bio_field, is_valid_display_name, is_valid_matrix_id}, }; #[tracing::instrument(skip(context))] @@ -105,20 +99,6 @@ pub async fn save_user_settings( LocalUserLanguage::update(&mut context.pool(), discussion_languages, local_user_id).await?; } - // If generate_totp is Some(false), this will clear it out from the database. - let (totp_2fa_secret, totp_2fa_url) = if let Some(generate) = data.generate_totp_2fa { - if generate { - let secret = generate_totp_2fa_secret(); - let url = - build_totp_2fa(&site_view.site.name, &local_user_view.person.name, &secret)?.get_url(); - (Some(Some(secret)), Some(Some(url))) - } else { - (Some(None), Some(None)) - } - } else { - (None, None) - }; - let local_user_form = LocalUserUpdateForm { email, show_avatars: data.show_avatars, @@ -134,8 +114,6 @@ pub async fn save_user_settings( default_listing_type, theme, interface_language: data.interface_language.clone(), - totp_2fa_secret, - totp_2fa_url, open_links_in_new_tab: data.open_links_in_new_tab, infinite_scroll_enabled: data.infinite_scroll_enabled, ..Default::default() diff --git a/crates/api/src/local_user/update_totp.rs b/crates/api/src/local_user/update_totp.rs new file mode 100644 index 000000000..15833ae8a --- /dev/null +++ b/crates/api/src/local_user/update_totp.rs @@ -0,0 +1,54 @@ +use crate::check_totp_2fa_valid; +use actix_web::web::{Data, Json}; +use lemmy_api_common::{ + context::LemmyContext, + person::{UpdateTotp, UpdateTotpResponse}, +}; +use lemmy_db_schema::{ + source::local_user::{LocalUser, LocalUserUpdateForm}, + traits::Crud, +}; +use lemmy_db_views::structs::{LocalUserView, SiteView}; +use lemmy_utils::error::LemmyError; + +/// Enable or disable two-factor-authentication. The current setting is determined from +/// [LocalUser.totp_2fa_enabled]. +/// +/// To enable, you need to first call [generate_totp_secret] and then pass a valid token to this +/// function. +/// +/// Disabling is only possible if 2FA was previously enabled. Again it is necessary to pass a valid +/// token. +#[tracing::instrument(skip(context))] +pub async fn update_totp( + data: Json, + local_user_view: LocalUserView, + context: Data, +) -> Result, LemmyError> { + let site_view = SiteView::read_local(&mut context.pool()).await?; + + check_totp_2fa_valid( + &local_user_view, + &Some(data.totp_token.clone()), + &site_view.site.name, + )?; + + // toggle the 2fa setting + let local_user_form = LocalUserUpdateForm { + totp_2fa_enabled: Some(data.enabled), + // if totp is enabled, leave unchanged. otherwise clear secret + totp_2fa_secret: if data.enabled { None } else { Some(None) }, + ..Default::default() + }; + + LocalUser::update( + &mut context.pool(), + local_user_view.local_user.id, + &local_user_form, + ) + .await?; + + Ok(Json(UpdateTotpResponse { + enabled: data.enabled, + })) +} diff --git a/crates/api_common/src/person.rs b/crates/api_common/src/person.rs index 8d58ebf68..3f43f30df 100644 --- a/crates/api_common/src/person.rs +++ b/crates/api_common/src/person.rs @@ -119,10 +119,6 @@ pub struct SaveUserSettings { pub show_new_post_notifs: Option, /// A list of languages you are able to see discussion in. pub discussion_languages: Option>, - /// Generates a TOTP / 2-factor authentication token. - /// - /// None leaves it as is, true will generate or regenerate it, false clears it out. - pub generate_totp_2fa: Option, pub auth: Sensitive, /// Open links in a new tab pub open_links_in_new_tab: Option, @@ -443,3 +439,25 @@ pub struct VerifyEmail { #[cfg_attr(feature = "full", ts(export))] /// A response to verifying your email. pub struct VerifyEmailResponse {} + +#[derive(Debug, Serialize, Deserialize, Clone)] +#[cfg_attr(feature = "full", derive(TS))] +#[cfg_attr(feature = "full", ts(export))] +pub struct GenerateTotpSecretResponse { + pub totp_secret_url: Sensitive, +} + +#[derive(Debug, Serialize, Deserialize, Clone)] +#[cfg_attr(feature = "full", derive(TS))] +#[cfg_attr(feature = "full", ts(export))] +pub struct UpdateTotp { + pub totp_token: String, + pub enabled: bool, +} + +#[derive(Debug, Serialize, Deserialize, Clone)] +#[cfg_attr(feature = "full", derive(TS))] +#[cfg_attr(feature = "full", ts(export))] +pub struct UpdateTotpResponse { + pub enabled: bool, +} diff --git a/crates/db_schema/src/schema.rs b/crates/db_schema/src/schema.rs index bec1ab40e..f347bfe3b 100644 --- a/crates/db_schema/src/schema.rs +++ b/crates/db_schema/src/schema.rs @@ -436,13 +436,13 @@ diesel::table! { email_verified -> Bool, accepted_application -> Bool, totp_2fa_secret -> Nullable, - totp_2fa_url -> Nullable, open_links_in_new_tab -> Bool, blur_nsfw -> Bool, auto_expand -> Bool, infinite_scroll_enabled -> Bool, admin -> Bool, post_listing_mode -> PostListingModeEnum, + totp_2fa_enabled -> Bool, } } diff --git a/crates/db_schema/src/source/local_user.rs b/crates/db_schema/src/source/local_user.rs index 84ada46af..98e01bd1a 100644 --- a/crates/db_schema/src/source/local_user.rs +++ b/crates/db_schema/src/source/local_user.rs @@ -51,8 +51,6 @@ pub struct LocalUser { pub accepted_application: bool, #[serde(skip)] pub totp_2fa_secret: Option, - /// A URL to add their 2-factor auth. - pub totp_2fa_url: Option, /// Open links in a new tab. pub open_links_in_new_tab: bool, pub blur_nsfw: bool, @@ -62,6 +60,7 @@ pub struct LocalUser { /// Whether the person is an admin. pub admin: bool, pub post_listing_mode: PostListingMode, + pub totp_2fa_enabled: bool, } #[derive(Clone, TypedBuilder)] @@ -88,13 +87,13 @@ pub struct LocalUserInsertForm { pub email_verified: Option, pub accepted_application: Option, pub totp_2fa_secret: Option>, - pub totp_2fa_url: Option>, pub open_links_in_new_tab: Option, pub blur_nsfw: Option, pub auto_expand: Option, pub infinite_scroll_enabled: Option, pub admin: Option, pub post_listing_mode: Option, + pub totp_2fa_enabled: Option, } #[derive(Clone, Default)] @@ -117,11 +116,11 @@ pub struct LocalUserUpdateForm { pub email_verified: Option, pub accepted_application: Option, pub totp_2fa_secret: Option>, - pub totp_2fa_url: Option>, pub open_links_in_new_tab: Option, pub blur_nsfw: Option, pub auto_expand: Option, pub infinite_scroll_enabled: Option, pub admin: Option, pub post_listing_mode: Option, + pub totp_2fa_enabled: Option, } diff --git a/crates/db_views/src/registration_application_view.rs b/crates/db_views/src/registration_application_view.rs index 19e300ac7..6a2ed6133 100644 --- a/crates/db_views/src/registration_application_view.rs +++ b/crates/db_views/src/registration_application_view.rs @@ -262,12 +262,12 @@ mod tests { email_verified: inserted_sara_local_user.email_verified, accepted_application: inserted_sara_local_user.accepted_application, totp_2fa_secret: inserted_sara_local_user.totp_2fa_secret, - totp_2fa_url: inserted_sara_local_user.totp_2fa_url, password_encrypted: inserted_sara_local_user.password_encrypted, open_links_in_new_tab: inserted_sara_local_user.open_links_in_new_tab, infinite_scroll_enabled: inserted_sara_local_user.infinite_scroll_enabled, admin: false, post_listing_mode: inserted_sara_local_user.post_listing_mode, + totp_2fa_enabled: inserted_sara_local_user.totp_2fa_enabled, }, creator: Person { id: inserted_sara_person.id, diff --git a/crates/utils/Cargo.toml b/crates/utils/Cargo.toml index 9cafd0c11..c726e441f 100644 --- a/crates/utils/Cargo.toml +++ b/crates/utils/Cargo.toml @@ -47,7 +47,6 @@ smart-default = "0.7.1" jsonwebtoken = "8.3.0" lettre = { version = "0.10.4", features = ["tokio1", "tokio1-native-tls"] } markdown-it = "0.5.1" -totp-rs = { version = "5.0.2", features = ["gen_secret", "otpauth"] } ts-rs = { workspace = true, optional = true } enum-map = "2.6" diff --git a/crates/utils/src/error.rs b/crates/utils/src/error.rs index 8c3523363..079d0fc94 100644 --- a/crates/utils/src/error.rs +++ b/crates/utils/src/error.rs @@ -159,8 +159,11 @@ pub enum LemmyErrorType { InvalidBodyField, BioLengthOverflow, MissingTotpToken, + MissingTotpSecret, IncorrectTotpToken, CouldntParseTotpSecret, + CouldntGenerateTotp, + TotpAlreadyEnabled, CouldntLikeComment, CouldntSaveComment, CouldntCreateReport, @@ -192,7 +195,6 @@ pub enum LemmyErrorType { InvalidUrl, EmailSendFailed, Slurs, - CouldntGenerateTotp, CouldntFindObject, RegistrationDenied(String), FederationDisabled, diff --git a/crates/utils/src/utils/validation.rs b/crates/utils/src/utils/validation.rs index 44464bc9f..f2b8fa2a7 100644 --- a/crates/utils/src/utils/validation.rs +++ b/crates/utils/src/utils/validation.rs @@ -1,8 +1,7 @@ -use crate::error::{LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult}; +use crate::error::{LemmyErrorExt, LemmyErrorType, LemmyResult}; use itertools::Itertools; use once_cell::sync::Lazy; use regex::{Regex, RegexBuilder}; -use totp_rs::{Secret, TOTP}; use url::Url; static VALID_ACTOR_NAME_REGEX: Lazy = @@ -238,54 +237,6 @@ pub fn clean_url_params(url: &Url) -> Url { url_out } -pub fn check_totp_2fa_valid( - totp_secret: &Option, - totp_token: &Option, - site_name: &str, - username: &str, -) -> LemmyResult<()> { - // Check only if they have a totp_secret in the DB - if let Some(totp_secret) = totp_secret { - // Throw an error if their token is missing - let token = totp_token - .as_deref() - .ok_or(LemmyErrorType::MissingTotpToken)?; - - let totp = build_totp_2fa(site_name, username, totp_secret)?; - - let check_passed = totp.check_current(token)?; - if !check_passed { - Err(LemmyErrorType::IncorrectTotpToken.into()) - } else { - Ok(()) - } - } else { - Ok(()) - } -} - -pub fn generate_totp_2fa_secret() -> String { - Secret::generate_secret().to_string() -} - -pub fn build_totp_2fa(site_name: &str, username: &str, secret: &str) -> Result { - let sec = Secret::Raw(secret.as_bytes().to_vec()); - let sec_bytes = sec - .to_bytes() - .map_err(|_| LemmyErrorType::CouldntParseTotpSecret)?; - - TOTP::new( - totp_rs::Algorithm::SHA256, - 6, - 1, - 30, - sec_bytes, - Some(site_name.to_string()), - username.to_string(), - ) - .with_lemmy_type(LemmyErrorType::CouldntGenerateTotp) -} - pub fn check_site_visibility_valid( current_private_instance: bool, current_federation_enabled: bool, @@ -319,7 +270,6 @@ mod tests { #![allow(clippy::unwrap_used)] #![allow(clippy::indexing_slicing)] - use super::build_totp_2fa; use crate::{ error::LemmyErrorType, utils::validation::{ @@ -327,7 +277,6 @@ mod tests { check_site_visibility_valid, check_url_scheme, clean_url_params, - generate_totp_2fa_secret, is_valid_actor_name, is_valid_bio_field, is_valid_display_name, @@ -400,13 +349,6 @@ mod tests { assert!(is_valid_matrix_id("@dess:matrix.org t").is_err()); } - #[test] - fn test_build_totp() { - let generated_secret = generate_totp_2fa_secret(); - let totp = build_totp_2fa("lemmy", "my_name", &generated_secret); - assert!(totp.is_ok()); - } - #[test] fn test_valid_site_name() { let valid_names = [ diff --git a/migrations/2023-09-11-110040_rework-2fa-setup/down.sql b/migrations/2023-09-11-110040_rework-2fa-setup/down.sql new file mode 100644 index 000000000..ec4c6e813 --- /dev/null +++ b/migrations/2023-09-11-110040_rework-2fa-setup/down.sql @@ -0,0 +1,6 @@ +ALTER TABLE local_user + ADD COLUMN totp_2fa_url text; + +ALTER TABLE local_user + DROP COLUMN totp_2fa_enabled; + diff --git a/migrations/2023-09-11-110040_rework-2fa-setup/up.sql b/migrations/2023-09-11-110040_rework-2fa-setup/up.sql new file mode 100644 index 000000000..8e72a120c --- /dev/null +++ b/migrations/2023-09-11-110040_rework-2fa-setup/up.sql @@ -0,0 +1,6 @@ +ALTER TABLE local_user + DROP COLUMN totp_2fa_url; + +ALTER TABLE local_user + ADD COLUMN totp_2fa_enabled boolean NOT NULL DEFAULT FALSE; + diff --git a/src/api_routes_http.rs b/src/api_routes_http.rs index 183034d05..fb62e82ec 100644 --- a/src/api_routes_http.rs +++ b/src/api_routes_http.rs @@ -20,6 +20,7 @@ use lemmy_api::{ block::block_person, change_password::change_password, change_password_after_reset::change_password_after_reset, + generate_totp_secret::generate_totp_secret, get_captcha::get_captcha, list_banned::list_banned_users, login::login, @@ -34,6 +35,7 @@ use lemmy_api::{ report_count::report_count, reset_password::reset_password, save_settings::save_user_settings, + update_totp::update_totp, verify_email::verify_email, }, post::{ @@ -287,7 +289,9 @@ pub fn config(cfg: &mut web::ServiceConfig, rate_limit: &RateLimitCell) { .route("/report_count", web::get().to(report_count)) .route("/unread_count", web::get().to(unread_count)) .route("/verify_email", web::post().to(verify_email)) - .route("/leave_admin", web::post().to(leave_admin)), + .route("/leave_admin", web::post().to(leave_admin)) + .route("/totp/generate", web::post().to(generate_totp_secret)) + .route("/totp/update", web::post().to(update_totp)), ) // Admin Actions .service(