From ceff2ec686094364cd452f10314d7239c8a9763c Mon Sep 17 00:00:00 2001 From: Nutomic Date: Thu, 5 Jan 2023 01:42:30 +0000 Subject: [PATCH] Use enum for registration mode setting (#2604) * Use enum for registration mode setting * fix tests --- api_tests/package.json | 2 +- api_tests/src/shared.ts | 3 +- api_tests/yarn.lock | 8 ++-- .../local_user/change_password_after_reset.rs | 28 +++++++------ crates/api_common/src/site.rs | 8 ++-- crates/api_common/src/utils.rs | 4 +- crates/api_crud/src/lib.rs | 16 -------- crates/api_crud/src/site/create.rs | 10 ++--- crates/api_crud/src/site/mod.rs | 16 ++++++++ crates/api_crud/src/site/update.rs | 18 ++++---- crates/api_crud/src/user/create.rs | 13 +++--- crates/db_schema/src/impls/local_site.rs | 41 ++++++++++++++++++- crates/db_schema/src/schema.rs | 6 ++- crates/db_schema/src/source/local_site.rs | 24 ++++++++--- crates/routes/src/nodeinfo.rs | 5 ++- .../down.sql | 31 ++++++++++++++ .../up.sql | 25 +++++++++++ 17 files changed, 186 insertions(+), 72 deletions(-) create mode 100644 migrations/2022-12-05-110642_registration_mode/down.sql create mode 100644 migrations/2022-12-05-110642_registration_mode/up.sql diff --git a/api_tests/package.json b/api_tests/package.json index f32d87fe..4d74cbb2 100644 --- a/api_tests/package.json +++ b/api_tests/package.json @@ -18,7 +18,7 @@ "eslint": "^8.25.0", "eslint-plugin-prettier": "^4.0.0", "jest": "^27.0.6", - "lemmy-js-client": "0.17.0-rc.60", + "lemmy-js-client": "0.17.0-rc.61", "node-fetch": "^2.6.1", "prettier": "^2.7.1", "ts-jest": "^27.0.3", diff --git a/api_tests/src/shared.ts b/api_tests/src/shared.ts index 26675c6d..08054818 100644 --- a/api_tests/src/shared.ts +++ b/api_tests/src/shared.ts @@ -64,6 +64,7 @@ import { GetCommentsResponse, FeaturePost, PostFeatureType, + RegistrationMode, } from "lemmy-js-client"; export interface API { @@ -145,7 +146,7 @@ export async function setupLogins() { // Registration applications are now enabled by default, need to disable them let editSiteForm: EditSite = { - require_application: false, + registration_mode: RegistrationMode.Open, federation_debug: true, rate_limit_message: 999, rate_limit_post: 999, diff --git a/api_tests/yarn.lock b/api_tests/yarn.lock index c4c0c37d..3701e2f1 100644 --- a/api_tests/yarn.lock +++ b/api_tests/yarn.lock @@ -2363,10 +2363,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.17.0-rc.60: - version "0.17.0-rc.60" - resolved "https://registry.yarnpkg.com/lemmy-js-client/-/lemmy-js-client-0.17.0-rc.60.tgz#6cabac42d842eb1f152d230be018090050476614" - integrity sha512-Nl+DUBJde0KpKywNkX5wof9PDCmr6RuJlgukVfQRmW5rznYRKYnI1V+awf+9mUx1eNQ8jhnjVO+6XxOzMjloZA== +lemmy-js-client@0.17.0-rc.61: + version "0.17.0-rc.61" + resolved "https://registry.yarnpkg.com/lemmy-js-client/-/lemmy-js-client-0.17.0-rc.61.tgz#c01e129a3d4c3483ecf337f1e4acf0ad91f9684f" + integrity sha512-xauBCD5i4vlUEWqsTMIXLCXeIjAK7ivVIN3C/g+RMAM7mD3CTcRkDZUerwnvLipIfr7V/4iYLWZW0orBaiV1CQ== dependencies: node-fetch "2.6.6" diff --git a/crates/api/src/local_user/change_password_after_reset.rs b/crates/api/src/local_user/change_password_after_reset.rs index c6de10d7..392fcaa3 100644 --- a/crates/api/src/local_user/change_password_after_reset.rs +++ b/crates/api/src/local_user/change_password_after_reset.rs @@ -6,6 +6,7 @@ use lemmy_api_common::{ utils::password_length_check, }; use lemmy_db_schema::source::{ + local_site::RegistrationMode, local_user::LocalUser, password_reset_request::PasswordResetRequest, }; @@ -45,19 +46,20 @@ impl Perform for PasswordChangeAfterReset { // Return the jwt if login is allowed let site_view = SiteView::read_local(context.pool()).await?; - let jwt = - if site_view.local_site.require_application && !updated_local_user.accepted_application { - None - } else { - Some( - Claims::jwt( - updated_local_user.id.0, - &context.secret().jwt_secret, - &context.settings().hostname, - )? - .into(), - ) - }; + let jwt = if site_view.local_site.registration_mode == RegistrationMode::RequireApplication + && !updated_local_user.accepted_application + { + None + } else { + Some( + Claims::jwt( + updated_local_user.id.0, + &context.secret().jwt_secret, + &context.settings().hostname, + )? + .into(), + ) + }; Ok(LoginResponse { jwt, diff --git a/crates/api_common/src/site.rs b/crates/api_common/src/site.rs index 9e660136..30d819f3 100644 --- a/crates/api_common/src/site.rs +++ b/crates/api_common/src/site.rs @@ -1,7 +1,7 @@ use crate::sensitive::Sensitive; use lemmy_db_schema::{ newtypes::{CommentId, CommunityId, LanguageId, PersonId, PostId}, - source::{language::Language, tagline::Tagline}, + source::{language::Language, local_site::RegistrationMode, tagline::Tagline}, ListingType, ModlogActionType, SearchType, @@ -116,11 +116,9 @@ pub struct CreateSite { pub icon: Option, pub banner: Option, pub enable_downvotes: Option, - pub open_registration: Option, pub enable_nsfw: Option, pub community_creation_admin_only: Option, pub require_email_verification: Option, - pub require_application: Option, pub application_question: Option, pub private_instance: Option, pub default_theme: Option, @@ -151,6 +149,7 @@ pub struct CreateSite { pub allowed_instances: Option>, pub blocked_instances: Option>, pub taglines: Option>, + pub registration_mode: Option, pub auth: Sensitive, } @@ -162,11 +161,9 @@ pub struct EditSite { pub icon: Option, pub banner: Option, pub enable_downvotes: Option, - pub open_registration: Option, pub enable_nsfw: Option, pub community_creation_admin_only: Option, pub require_email_verification: Option, - pub require_application: Option, pub application_question: Option, pub private_instance: Option, pub default_theme: Option, @@ -197,6 +194,7 @@ pub struct EditSite { pub allowed_instances: Option>, pub blocked_instances: Option>, pub taglines: Option>, + pub registration_mode: Option, pub auth: Sensitive, } diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index c8534499..22d8e611 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -9,7 +9,7 @@ use lemmy_db_schema::{ community::{Community, CommunityUpdateForm}, email_verification::{EmailVerification, EmailVerificationForm}, instance::Instance, - local_site::LocalSite, + local_site::{LocalSite, RegistrationMode}, local_site_rate_limit::LocalSiteRateLimit, password_reset_request::PasswordResetRequest, person::{Person, PersonUpdateForm}, @@ -488,7 +488,7 @@ pub async fn check_registration_application( local_site: &LocalSite, pool: &DbPool, ) -> Result<(), LemmyError> { - if local_site.require_application + if local_site.registration_mode == RegistrationMode::RequireApplication && !local_user_view.local_user.accepted_application && !local_user_view.person.admin { diff --git a/crates/api_crud/src/lib.rs b/crates/api_crud/src/lib.rs index 30c9ebd4..d37dfbee 100644 --- a/crates/api_crud/src/lib.rs +++ b/crates/api_crud/src/lib.rs @@ -1,6 +1,5 @@ use actix_web::web::Data; use lemmy_api_common::context::LemmyContext; -use lemmy_db_schema::source::local_site::LocalSite; use lemmy_utils::{error::LemmyError, ConnectionId}; mod comment; @@ -20,18 +19,3 @@ pub trait PerformCrud { websocket_id: Option, ) -> Result; } - -/// Make sure if applications are required, that there is an application questionnaire -pub fn check_application_question( - application_question: &Option>, - local_site: &LocalSite, - require_application: &Option, -) -> Result<(), LemmyError> { - if require_application.unwrap_or(false) - && (application_question == &Some(None) - || (application_question.is_none() && local_site.application_question.is_none())) - { - return Err(LemmyError::from_message("application_question_required")); - } - Ok(()) -} diff --git a/crates/api_crud/src/site/create.rs b/crates/api_crud/src/site/create.rs index 370159e4..8b0b3696 100644 --- a/crates/api_crud/src/site/create.rs +++ b/crates/api_crud/src/site/create.rs @@ -1,4 +1,4 @@ -use crate::{check_application_question, PerformCrud}; +use crate::{site::check_application_question, PerformCrud}; use activitypub_federation::core::signatures::generate_actor_keypair; use actix_web::web::Data; use lemmy_api_common::{ @@ -71,8 +71,9 @@ impl PerformCrud for CreateSite { let application_question = diesel_option_overwrite(&data.application_question); check_application_question( &application_question, - &local_site, - &data.require_application, + data + .registration_mode + .unwrap_or(local_site.registration_mode), )?; let actor_id: DbUrl = Url::parse(&context.settings().get_protocol_and_hostname())?.into(); @@ -99,11 +100,10 @@ impl PerformCrud for CreateSite { // Set the site setup to true .site_setup(Some(true)) .enable_downvotes(data.enable_downvotes) - .open_registration(data.open_registration) + .registration_mode(data.registration_mode) .enable_nsfw(data.enable_nsfw) .community_creation_admin_only(data.community_creation_admin_only) .require_email_verification(data.require_email_verification) - .require_application(data.require_application) .application_question(application_question) .private_instance(data.private_instance) .default_theme(data.default_theme.clone()) diff --git a/crates/api_crud/src/site/mod.rs b/crates/api_crud/src/site/mod.rs index 845da049..5155e3d0 100644 --- a/crates/api_crud/src/site/mod.rs +++ b/crates/api_crud/src/site/mod.rs @@ -1,3 +1,19 @@ +use lemmy_db_schema::source::local_site::RegistrationMode; +use lemmy_utils::error::LemmyError; + mod create; mod read; mod update; + +pub fn check_application_question( + application_question: &Option>, + registration_mode: RegistrationMode, +) -> Result<(), LemmyError> { + if registration_mode == RegistrationMode::RequireApplication + && application_question.as_ref().unwrap_or(&None).is_none() + { + Err(LemmyError::from_message("application_question_required")) + } else { + Ok(()) + } +} diff --git a/crates/api_crud/src/site/update.rs b/crates/api_crud/src/site/update.rs index 818d3875..8cc852bb 100644 --- a/crates/api_crud/src/site/update.rs +++ b/crates/api_crud/src/site/update.rs @@ -1,4 +1,4 @@ -use crate::{check_application_question, PerformCrud}; +use crate::{site::check_application_question, PerformCrud}; use actix_web::web::Data; use lemmy_api_common::{ context::LemmyContext, @@ -17,7 +17,7 @@ use lemmy_db_schema::{ actor_language::SiteLanguage, federation_allowlist::FederationAllowList, federation_blocklist::FederationBlockList, - local_site::{LocalSite, LocalSiteUpdateForm}, + local_site::{LocalSite, LocalSiteUpdateForm, RegistrationMode}, local_site_rate_limit::{LocalSiteRateLimit, LocalSiteRateLimitUpdateForm}, local_user::LocalUser, site::{Site, SiteUpdateForm}, @@ -61,8 +61,9 @@ impl PerformCrud for EditSite { let application_question = diesel_option_overwrite(&data.application_question); check_application_question( &application_question, - &local_site, - &data.require_application, + data + .registration_mode + .unwrap_or(local_site.registration_mode), )?; if let Some(default_post_listing_type) = &data.default_post_listing_type { @@ -99,11 +100,10 @@ impl PerformCrud for EditSite { let local_site_form = LocalSiteUpdateForm::builder() .enable_downvotes(data.enable_downvotes) - .open_registration(data.open_registration) + .registration_mode(data.registration_mode) .enable_nsfw(data.enable_nsfw) .community_creation_admin_only(data.community_creation_admin_only) .require_email_verification(data.require_email_verification) - .require_application(data.require_application) .application_question(application_question) .private_instance(data.private_instance) .default_theme(data.default_theme.clone()) @@ -155,11 +155,13 @@ impl PerformCrud for EditSite { // will be able to log in. It really only wants this to be a requirement for NEW signups. // So if it was set from false, to true, you need to update all current users columns to be verified. + let old_require_application = + local_site.registration_mode == RegistrationMode::RequireApplication; let new_require_application = update_local_site .as_ref() - .map(|ols| ols.require_application) + .map(|ols| ols.registration_mode == RegistrationMode::RequireApplication) .unwrap_or(false); - if !local_site.require_application && new_require_application { + if !old_require_application && new_require_application { LocalUser::set_all_users_registration_applications_accepted(context.pool()) .await .map_err(|e| LemmyError::from_error_message(e, "couldnt_set_all_registrations_accepted"))?; diff --git a/crates/api_crud/src/user/create.rs b/crates/api_crud/src/user/create.rs index f11c45b3..a6979382 100644 --- a/crates/api_crud/src/user/create.rs +++ b/crates/api_crud/src/user/create.rs @@ -19,6 +19,7 @@ use lemmy_api_common::{ use lemmy_db_schema::{ aggregates::structs::PersonAggregates, source::{ + local_site::RegistrationMode, local_user::{LocalUser, LocalUserInsertForm}, person::{Person, PersonInsertForm}, registration_application::{RegistrationApplication, RegistrationApplicationInsertForm}, @@ -47,8 +48,10 @@ impl PerformCrud for Register { let site_view = SiteView::read_local(context.pool()).await?; let local_site = site_view.local_site; + let require_registration_application = + local_site.registration_mode == RegistrationMode::RequireApplication; - if !local_site.open_registration { + if local_site.registration_mode == RegistrationMode::Closed { return Err(LemmyError::from_message("registration_closed")); } @@ -59,7 +62,7 @@ impl PerformCrud for Register { return Err(LemmyError::from_message("email_required")); } - if local_site.site_setup && local_site.require_application && data.answer.is_none() { + if local_site.site_setup && require_registration_application && data.answer.is_none() { return Err(LemmyError::from_message( "registration_application_answer_required", )); @@ -141,7 +144,7 @@ impl PerformCrud for Register { } }; - if local_site.site_setup && local_site.require_application { + if local_site.site_setup && require_registration_application { // Create the registration application let form = RegistrationApplicationInsertForm { local_user_id: inserted_local_user.id, @@ -166,7 +169,7 @@ impl PerformCrud for Register { // Log the user in directly if the site is not setup, or email verification and application aren't required if !local_site.site_setup - || (!local_site.require_application && !local_site.require_email_verification) + || (!require_registration_application && !local_site.require_email_verification) { login_response.jwt = Some( Claims::jwt( @@ -195,7 +198,7 @@ impl PerformCrud for Register { login_response.verify_email_sent = true; } - if local_site.require_application { + if require_registration_application { login_response.registration_created = true; } } diff --git a/crates/db_schema/src/impls/local_site.rs b/crates/db_schema/src/impls/local_site.rs index b9e920d1..dd3d06d6 100644 --- a/crates/db_schema/src/impls/local_site.rs +++ b/crates/db_schema/src/impls/local_site.rs @@ -1,10 +1,25 @@ use crate::{ schema::local_site::dsl::local_site, - source::local_site::{LocalSite, LocalSiteInsertForm, LocalSiteUpdateForm}, + source::local_site::{ + LocalSite, + LocalSiteInsertForm, + LocalSiteUpdateForm, + RegistrationMode, + RegistrationModeType, + }, utils::{get_conn, DbPool}, }; -use diesel::{dsl::insert_into, result::Error}; +use diesel::{ + deserialize, + deserialize::FromSql, + dsl::insert_into, + pg::{Pg, PgValue}, + result::Error, + serialize, + serialize::{IsNull, Output, ToSql}, +}; use diesel_async::RunQueryDsl; +use std::io::Write; impl LocalSite { pub async fn create(pool: &DbPool, form: &LocalSiteInsertForm) -> Result { @@ -30,3 +45,25 @@ impl LocalSite { diesel::delete(local_site).execute(conn).await } } + +impl ToSql for RegistrationMode { + fn to_sql<'b>(&'b self, out: &mut Output<'b, '_, Pg>) -> serialize::Result { + match *self { + RegistrationMode::Closed => out.write_all(b"closed")?, + RegistrationMode::RequireApplication => out.write_all(b"require_application")?, + RegistrationMode::Open => out.write_all(b"open")?, + } + Ok(IsNull::No) + } +} + +impl FromSql for RegistrationMode { + fn from_sql(bytes: PgValue<'_>) -> deserialize::Result { + match bytes.as_bytes() { + b"closed" => Ok(RegistrationMode::Closed), + b"require_application" => Ok(RegistrationMode::RequireApplication), + b"open" => Ok(RegistrationMode::Open), + _ => Err("Unrecognized enum variant".into()), + } + } +} diff --git a/crates/db_schema/src/schema.rs b/crates/db_schema/src/schema.rs index 97def1ff..4dea567c 100644 --- a/crates/db_schema/src/schema.rs +++ b/crates/db_schema/src/schema.rs @@ -672,16 +672,17 @@ table! { } table! { + use crate::source::local_site::RegistrationModeType; + use diesel::sql_types::*; + local_site(id) { id -> Int4, site_id -> Int4, site_setup -> Bool, enable_downvotes -> Bool, - open_registration -> Bool, enable_nsfw -> Bool, community_creation_admin_only -> Bool, require_email_verification -> Bool, - require_application -> Bool, application_question -> Nullable, private_instance -> Bool, default_theme -> Text, @@ -696,6 +697,7 @@ table! { federation_worker_count -> Int4, captcha_enabled -> Bool, captcha_difficulty -> Text, + registration_mode -> RegistrationModeType, published -> Timestamp, updated -> Nullable, } diff --git a/crates/db_schema/src/source/local_site.rs b/crates/db_schema/src/source/local_site.rs index 4ae68ec1..40744a53 100644 --- a/crates/db_schema/src/source/local_site.rs +++ b/crates/db_schema/src/source/local_site.rs @@ -13,11 +13,9 @@ pub struct LocalSite { pub site_id: SiteId, pub site_setup: bool, pub enable_downvotes: bool, - pub open_registration: bool, pub enable_nsfw: bool, pub community_creation_admin_only: bool, pub require_email_verification: bool, - pub require_application: bool, pub application_question: Option, pub private_instance: bool, pub default_theme: String, @@ -32,6 +30,7 @@ pub struct LocalSite { pub federation_worker_count: i32, pub captcha_enabled: bool, pub captcha_difficulty: String, + pub registration_mode: RegistrationMode, pub published: chrono::NaiveDateTime, pub updated: Option, } @@ -45,11 +44,9 @@ pub struct LocalSiteInsertForm { pub site_id: SiteId, pub site_setup: Option, pub enable_downvotes: Option, - pub open_registration: Option, pub enable_nsfw: Option, pub community_creation_admin_only: Option, pub require_email_verification: Option, - pub require_application: Option, pub application_question: Option, pub private_instance: Option, pub default_theme: Option, @@ -64,6 +61,7 @@ pub struct LocalSiteInsertForm { pub federation_worker_count: Option, pub captcha_enabled: Option, pub captcha_difficulty: Option, + pub registration_mode: Option, } #[derive(Clone, TypedBuilder)] @@ -73,11 +71,9 @@ pub struct LocalSiteInsertForm { pub struct LocalSiteUpdateForm { pub site_setup: Option, pub enable_downvotes: Option, - pub open_registration: Option, pub enable_nsfw: Option, pub community_creation_admin_only: Option, pub require_email_verification: Option, - pub require_application: Option, pub application_question: Option>, pub private_instance: Option, pub default_theme: Option, @@ -92,5 +88,21 @@ pub struct LocalSiteUpdateForm { pub federation_worker_count: Option, pub captcha_enabled: Option, pub captcha_difficulty: Option, + pub registration_mode: Option, pub updated: Option>, } + +#[cfg(feature = "full")] +#[derive(SqlType)] +#[diesel(postgres_type(name = "registration_mode_enum"))] +pub struct RegistrationModeType; + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[cfg_attr(feature = "full", derive(FromSqlRow, AsExpression))] +#[cfg_attr(feature = "full", diesel(sql_type = RegistrationModeType))] +#[serde(rename_all = "lowercase")] +pub enum RegistrationMode { + Closed, + RequireApplication, + Open, +} diff --git a/crates/routes/src/nodeinfo.rs b/crates/routes/src/nodeinfo.rs index 13786e3b..e1c70a87 100644 --- a/crates/routes/src/nodeinfo.rs +++ b/crates/routes/src/nodeinfo.rs @@ -1,6 +1,7 @@ use actix_web::{error::ErrorBadRequest, web, Error, HttpResponse, Result}; use anyhow::anyhow; use lemmy_api_common::context::LemmyContext; +use lemmy_db_schema::source::local_site::RegistrationMode; use lemmy_db_views::structs::SiteView; use lemmy_utils::{error::LemmyError, version}; use serde::{Deserialize, Serialize}; @@ -37,7 +38,7 @@ async fn node_info(context: web::Data) -> Result) -> Result