From 8da32fbdf8476fe695ef61bb09716defb7f1e7cd Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Mon, 18 Sep 2023 17:14:09 +0200 Subject: [PATCH 01/15] wip --- Cargo.lock | 14 ++--- Cargo.toml | 1 + crates/api/src/local_user/ban_person.rs | 8 +++ crates/api/src/local_user/change_password.rs | 19 ++---- .../local_user/change_password_after_reset.rs | 3 + crates/api/src/local_user/login.rs | 31 +++++----- crates/api/src/local_user/logout.rs | 21 +++++++ crates/api/src/local_user/mod.rs | 1 + crates/api/src/local_user/save_settings.rs | 12 +--- crates/api_common/Cargo.toml | 2 + crates/api_common/src/claims.rs | 58 +++++++++++++++++++ crates/api_common/src/lib.rs | 2 + crates/api_common/src/utils.rs | 33 +++++------ crates/api_crud/src/site/read.rs | 11 +--- crates/api_crud/src/user/create.rs | 24 ++++---- crates/api_crud/src/user/delete.rs | 4 +- crates/db_schema/src/impls/local_user.rs | 6 +- crates/db_schema/src/impls/login_token.rs | 54 +++++++++++++++++ crates/db_schema/src/impls/mod.rs | 1 + crates/db_schema/src/impls/person.rs | 5 +- crates/db_schema/src/schema.rs | 11 +++- crates/db_schema/src/source/local_user.rs | 2 - crates/db_schema/src/source/login_token.rs | 19 ++++++ crates/db_schema/src/source/mod.rs | 1 + crates/routes/src/feeds.rs | 1 - crates/routes/src/images.rs | 20 ++----- crates/utils/Cargo.toml | 1 - crates/utils/src/claims.rs | 35 ----------- crates/utils/src/lib.rs | 6 +- .../2023-09-18-141700_login-token/down.sql | 4 ++ .../2023-09-18-141700_login-token/up.sql | 9 +++ src/api_routes_http.rs | 2 + src/session_middleware.rs | 7 ++- 33 files changed, 273 insertions(+), 155 deletions(-) create mode 100644 crates/api/src/local_user/logout.rs create mode 100644 crates/api_common/src/claims.rs create mode 100644 crates/db_schema/src/impls/login_token.rs create mode 100644 crates/db_schema/src/source/login_token.rs delete mode 100644 crates/utils/src/claims.rs create mode 100644 migrations/2023-09-18-141700_login-token/down.sql create mode 100644 migrations/2023-09-18-141700_login-token/up.sql diff --git a/Cargo.lock b/Cargo.lock index bb72acb690..c4fe2f81f9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2644,6 +2644,7 @@ dependencies = [ "encoding", "futures", "getrandom", + "jsonwebtoken", "lemmy_db_schema", "lemmy_db_views", "lemmy_db_views_actor", @@ -2920,7 +2921,6 @@ dependencies = [ "html2text", "http", "itertools 0.11.0", - "jsonwebtoken", "lettre", "markdown-it", "once_cell", @@ -3349,9 +3349,9 @@ dependencies = [ [[package]] name = "num-bigint" -version = "0.4.3" +version = "0.4.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f93ab6289c7b344a8a9f60f88d80aa20032336fe78da341afc91c8a2341fc75f" +checksum = "608e7659b5c3d7cba262d894801b9ec9d00de989e8a82bd4bef91d08da45cdc0" dependencies = [ "autocfg", "num-integer", @@ -3381,9 +3381,9 @@ dependencies = [ [[package]] name = "num-traits" -version = "0.2.15" +version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "578ede34cf02f8924ab9447f50c28075b4d3e5b269972345e7e0372b38c6cdcd" +checksum = "f30b0abd723be7e2ffca1272140fac1a2f084c77ec3e123c192b66af1ee9e6c2" dependencies = [ "autocfg", ] @@ -3652,9 +3652,9 @@ checksum = "8835116a5c179084a830efb3adc117ab007512b535bc1a21c991d3b32a6b44dd" [[package]] name = "pem" -version = "1.1.0" +version = "1.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "03c64931a1a212348ec4f3b4362585eca7159d0d09cbdf4a7f74f02173596fd4" +checksum = "a8835c273a76a90455d7344889b0964598e3316e2a79ede8e36f16bdcf2228b8" dependencies = [ "base64 0.13.1", ] diff --git a/Cargo.toml b/Cargo.toml index 72ab87d4f3..bbe05f952c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -82,6 +82,7 @@ actix-web = { version = "4.3.1", default-features = false, features = [ "compress-brotli", "compress-gzip", "compress-zstd", + "cookies" ] } tracing = "0.1.37" tracing-actix-web = { version = "0.7.5", default-features = false } diff --git a/crates/api/src/local_user/ban_person.rs b/crates/api/src/local_user/ban_person.rs index 3a83d6c6e3..54e64a8695 100644 --- a/crates/api/src/local_user/ban_person.rs +++ b/crates/api/src/local_user/ban_person.rs @@ -8,11 +8,13 @@ use lemmy_api_common::{ }; use lemmy_db_schema::{ source::{ + login_token::LoginToken, moderator::{ModBan, ModBanForm}, person::{Person, PersonUpdateForm}, }, traits::Crud, }; +use lemmy_db_views::structs::LocalUserView; use lemmy_db_views_actor::structs::PersonView; use lemmy_utils::{ error::{LemmyError, LemmyErrorExt, LemmyErrorType}, @@ -44,6 +46,12 @@ pub async fn ban_from_site( .await .with_lemmy_type(LemmyErrorType::CouldntUpdateUser)?; + let local_user_id = LocalUserView::read_person(&mut context.pool(), data.person_id) + .await? + .local_user + .id; + LoginToken::invalidate_all(&mut context.pool(), local_user_id).await?; + // Remove their data if that's desired let remove_data = data.remove_data.unwrap_or(false); if remove_data { diff --git a/crates/api/src/local_user/change_password.rs b/crates/api/src/local_user/change_password.rs index 532c8ed7cb..478466169d 100644 --- a/crates/api/src/local_user/change_password.rs +++ b/crates/api/src/local_user/change_password.rs @@ -1,15 +1,13 @@ use actix_web::web::{Data, Json}; use bcrypt::verify; use lemmy_api_common::{ + claims::Claims, context::LemmyContext, person::{ChangePassword, LoginResponse}, utils::{local_user_view_from_jwt, password_length_check}, }; -use lemmy_db_schema::source::local_user::LocalUser; -use lemmy_utils::{ - claims::Claims, - error::{LemmyError, LemmyErrorType}, -}; +use lemmy_db_schema::source::{local_user::LocalUser, login_token::LoginToken}; +use lemmy_utils::error::{LemmyError, LemmyErrorType}; #[tracing::instrument(skip(context))] pub async fn change_password( @@ -40,16 +38,11 @@ pub async fn change_password( let updated_local_user = LocalUser::update_password(&mut context.pool(), local_user_id, &new_password).await?; + LoginToken::invalidate_all(&mut context.pool(), local_user_view.local_user.id).await?; + // Return the jwt Ok(Json(LoginResponse { - jwt: Some( - Claims::jwt( - updated_local_user.id.0, - &context.secret().jwt_secret, - &context.settings().hostname, - )? - .into(), - ), + jwt: Some(Claims::generate(updated_local_user.id, &context).await?), verify_email_sent: false, registration_created: false, })) 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 1bb501dd62..b3e193dc71 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::{ }; use lemmy_db_schema::source::{ local_user::LocalUser, + login_token::LoginToken, password_reset_request::PasswordResetRequest, }; use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType}; @@ -34,6 +35,8 @@ pub async fn change_password_after_reset( .await .with_lemmy_type(LemmyErrorType::CouldntUpdateUser)?; + LoginToken::invalidate_all(&mut context.pool(), local_user_id).await?; + Ok(Json(LoginResponse { jwt: None, verify_email_sent: false, diff --git a/crates/api/src/local_user/login.rs b/crates/api/src/local_user/login.rs index 915aff939d..1a059fae20 100644 --- a/crates/api/src/local_user/login.rs +++ b/crates/api/src/local_user/login.rs @@ -1,13 +1,16 @@ -use actix_web::web::{Data, Json}; +use actix_web::{ + web::{Data, Json}, + HttpResponse, +}; use bcrypt::verify; use lemmy_api_common::{ + claims::Claims, context::LemmyContext, person::{Login, LoginResponse}, - utils::{check_registration_application, check_user_valid}, + utils::{check_registration_application, check_user_valid, create_login_cookie}, }; use lemmy_db_views::structs::{LocalUserView, SiteView}; use lemmy_utils::{ - claims::Claims, error::{LemmyError, LemmyErrorExt, LemmyErrorType}, utils::validation::check_totp_2fa_valid, }; @@ -16,7 +19,7 @@ use lemmy_utils::{ pub async fn login( data: Json, context: Data, -) -> Result, LemmyError> { +) -> Result { let site_view = SiteView::read_local(&mut context.pool()).await?; // Fetch that username / email @@ -61,17 +64,15 @@ pub async fn login( &local_user_view.person.name, )?; - // Return the jwt - Ok(Json(LoginResponse { - jwt: Some( - Claims::jwt( - local_user_view.local_user.id.0, - &context.secret().jwt_secret, - &context.settings().hostname, - )? - .into(), - ), + let jwt = Claims::generate(local_user_view.local_user.id, &context).await?; + + let json = LoginResponse { + jwt: Some(jwt.clone()), verify_email_sent: false, registration_created: false, - })) + }; + + let mut res = HttpResponse::Ok().json(json); + res.add_cookie(&create_login_cookie(jwt))?; + Ok(res) } diff --git a/crates/api/src/local_user/logout.rs b/crates/api/src/local_user/logout.rs new file mode 100644 index 0000000000..259413682e --- /dev/null +++ b/crates/api/src/local_user/logout.rs @@ -0,0 +1,21 @@ +use activitypub_federation::config::Data; +use actix_web::{cookie::Cookie, HttpRequest, HttpResponse}; +use lemmy_api_common::{context::LemmyContext, utils::AUTH_COOKIE_NAME}; +use lemmy_db_schema::source::login_token::LoginToken; +use lemmy_db_views::structs::LocalUserView; +use lemmy_utils::error::LemmyResult; + +#[tracing::instrument(skip(context))] +pub async fn logout( + local_user_view: LocalUserView, + context: Data, +) -> LemmyResult { + // TODO: need to retrieve auth token. middleware could write it to request extensions + let jwt = todo!(); + LoginToken::invalidate(&mut context.pool(), jwt).await?; + + let mut res = HttpResponse::Ok().finish(); + let mut cookie = Cookie::new(AUTH_COOKIE_NAME, ""); + res.add_removal_cookie(&cookie)?; + Ok(res) +} diff --git a/crates/api/src/local_user/mod.rs b/crates/api/src/local_user/mod.rs index 806fa66a21..042ce69792 100644 --- a/crates/api/src/local_user/mod.rs +++ b/crates/api/src/local_user/mod.rs @@ -6,6 +6,7 @@ pub mod change_password_after_reset; pub mod get_captcha; pub mod list_banned; pub mod login; +pub mod logout; pub mod notifications; pub mod report_count; pub mod reset_password; diff --git a/crates/api/src/local_user/save_settings.rs b/crates/api/src/local_user/save_settings.rs index 8368eada0f..e79fccecad 100644 --- a/crates/api/src/local_user/save_settings.rs +++ b/crates/api/src/local_user/save_settings.rs @@ -1,5 +1,6 @@ use actix_web::web::{Data, Json}; use lemmy_api_common::{ + claims::Claims, context::LemmyContext, person::{LoginResponse, SaveUserSettings}, utils::{local_user_view_from_jwt, sanitize_html_api_opt, send_verification_email}, @@ -15,7 +16,6 @@ use lemmy_db_schema::{ }; use lemmy_db_views::structs::SiteView; use lemmy_utils::{ - claims::Claims, error::{LemmyError, LemmyErrorExt, LemmyErrorType}, utils::validation::{ build_totp_2fa, @@ -159,15 +159,9 @@ pub async fn save_user_settings( }; // Return the jwt + let jwt = Some(Claims::generate(local_user_view.local_user.id, &context).await?); Ok(Json(LoginResponse { - jwt: Some( - Claims::jwt( - updated_local_user.id.0, - &context.secret().jwt_secret, - &context.settings().hostname, - )? - .into(), - ), + jwt, verify_email_sent: false, registration_created: false, })) diff --git a/crates/api_common/Cargo.toml b/crates/api_common/Cargo.toml index 8a23a4cb24..a8522030e5 100644 --- a/crates/api_common/Cargo.toml +++ b/crates/api_common/Cargo.toml @@ -34,6 +34,7 @@ full = [ "actix-web", "futures", "once_cell", + "jsonwebtoken" ] [dependencies] @@ -64,5 +65,6 @@ reqwest = { workspace = true, optional = true } ts-rs = { workspace = true, optional = true } once_cell = { workspace = true, optional = true } actix-web = { workspace = true, optional = true } +jsonwebtoken = { version = "8.3.0", optional = true } # necessary for wasmt compilation getrandom = { version = "0.2.10", features = ["js"] } diff --git a/crates/api_common/src/claims.rs b/crates/api_common/src/claims.rs new file mode 100644 index 0000000000..969f71a224 --- /dev/null +++ b/crates/api_common/src/claims.rs @@ -0,0 +1,58 @@ +use crate::{context::LemmyContext, sensitive::Sensitive}; +use chrono::Utc; +use jsonwebtoken::{decode, encode, DecodingKey, EncodingKey, Header, Validation}; +use lemmy_db_schema::{ + newtypes::LocalUserId, + source::login_token::{LoginToken, LoginTokenCreateForm}, +}; +use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult}; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Serialize, Deserialize)] +pub struct Claims { + /// local_user_id, standard claim by RFC 7519. + pub sub: String, + pub iss: String, + /// Time when this token was issued as UNIX-timestamp in seconds + pub iat: i64, +} + +impl Claims { + pub async fn validate(jwt: &str, context: &LemmyContext) -> LemmyResult { + let mut validation = Validation::default(); + validation.validate_exp = false; + validation.required_spec_claims.remove("exp"); + let jwt_secret = &context.secret().jwt_secret; + let key = DecodingKey::from_secret(jwt_secret.as_ref()); + let claims = + decode::(jwt, &key, &validation).with_lemmy_type(LemmyErrorType::NotLoggedIn)?; + let user_id = LocalUserId(claims.claims.sub.parse()?); + let is_valid = LoginToken::validate(&mut context.pool(), user_id, jwt).await?; + if !is_valid { + return Err(LemmyErrorType::NotLoggedIn)?; + } + Ok(user_id) + } + + pub async fn generate( + user_id: LocalUserId, + context: &LemmyContext, + ) -> LemmyResult> { + let hostname = context.settings().hostname.clone(); + let my_claims = Claims { + sub: user_id.0.to_string(), + iss: hostname, + iat: Utc::now().timestamp(), + }; + + let secret = &context.secret().jwt_secret; + let key = EncodingKey::from_secret(secret.as_ref()); + let token = encode(&Header::default(), &my_claims, &key)?; + let form = LoginTokenCreateForm { + token: token.clone(), + user_id, + }; + LoginToken::create(&mut context.pool(), form).await?; + Ok(Sensitive::new(token)) + } +} diff --git a/crates/api_common/src/lib.rs b/crates/api_common/src/lib.rs index 652cbaf43a..ff8f65364e 100644 --- a/crates/api_common/src/lib.rs +++ b/crates/api_common/src/lib.rs @@ -1,5 +1,7 @@ #[cfg(feature = "full")] pub mod build_response; +#[cfg(feature = "full")] +pub mod claims; pub mod comment; pub mod community; #[cfg(feature = "full")] diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index 9d8181a2ed..c9b6c7c1eb 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -1,9 +1,11 @@ use crate::{ + claims::Claims, context::LemmyContext, request::purge_image_from_pictrs, sensitive::Sensitive, site::FederatedInstances, }; +use actix_web::cookie::{Cookie, SameSite}; use anyhow::Context; use chrono::{DateTime, Utc}; use lemmy_db_schema::{ @@ -33,7 +35,6 @@ use lemmy_db_views_actor::structs::{ CommunityView, }; use lemmy_utils::{ - claims::Claims, email::{send_email, translations::Lang}, error::{LemmyError, LemmyErrorExt, LemmyErrorExt2, LemmyErrorType}, location_info, @@ -139,10 +140,7 @@ pub async fn local_user_view_from_jwt( jwt: &str, context: &LemmyContext, ) -> Result { - let claims = Claims::decode(jwt, &context.secret().jwt_secret) - .with_lemmy_type(LemmyErrorType::NotLoggedIn)? - .claims; - let local_user_id = LocalUserId(claims.sub); + let local_user_id = Claims::validate(jwt, context).await?; let local_user_view = LocalUserView::read(&mut context.pool(), local_user_id).await?; check_user_valid( local_user_view.person.banned, @@ -150,8 +148,6 @@ pub async fn local_user_view_from_jwt( local_user_view.person.deleted, )?; - check_validator_time(&local_user_view.local_user.validator_time, &claims)?; - Ok(local_user_view) } @@ -173,19 +169,6 @@ pub async fn local_user_view_from_jwt_opt_new( } } -/// Checks if user's token was issued before user's password reset. -pub fn check_validator_time( - validator_time: &DateTime, - claims: &Claims, -) -> Result<(), LemmyError> { - let user_validation_time = validator_time.timestamp(); - if user_validation_time > claims.iat { - Err(LemmyErrorType::NotLoggedIn)? - } else { - Ok(()) - } -} - pub fn check_user_valid( banned: bool, ban_expires: Option>, @@ -829,6 +812,14 @@ pub fn sanitize_html_federation_opt(data: &Option) -> Option { data.as_ref().map(|d| sanitize_html_federation(d)) } +pub fn create_login_cookie(jwt: Sensitive) -> Cookie<'static> { + let mut cookie = Cookie::new(AUTH_COOKIE_NAME, jwt.into_inner()); + cookie.set_secure(true); + cookie.set_same_site(SameSite::Strict); + cookie.set_http_only(true); + cookie +} + #[cfg(test)] mod tests { #![allow(clippy::unwrap_used)] @@ -853,3 +844,5 @@ mod tests { assert!(honeypot_check(&Some("message".to_string())).is_err()); } } + +pub static AUTH_COOKIE_NAME: &str = "auth"; diff --git a/crates/api_crud/src/site/read.rs b/crates/api_crud/src/site/read.rs index 62d96492a5..c398dc81d7 100644 --- a/crates/api_crud/src/site/read.rs +++ b/crates/api_crud/src/site/read.rs @@ -1,9 +1,10 @@ use actix_web::web::{Data, Json, Query}; use lemmy_api_common::{ + claims::Claims, context::LemmyContext, sensitive::Sensitive, site::{GetSite, GetSiteResponse, MyUserInfo}, - utils::{check_user_valid, check_validator_time}, + utils::check_user_valid, }; use lemmy_db_schema::{ newtypes::LocalUserId, @@ -22,7 +23,6 @@ use lemmy_db_views_actor::structs::{ PersonView, }; use lemmy_utils::{ - claims::Claims, error::{LemmyError, LemmyErrorExt, LemmyErrorType}, version, }; @@ -102,10 +102,7 @@ async fn local_user_settings_view_from_jwt_opt( ) -> Option { match jwt { Some(jwt) => { - let claims = Claims::decode(jwt.as_ref(), &context.secret().jwt_secret) - .ok()? - .claims; - let local_user_id = LocalUserId(claims.sub); + let local_user_id = Claims::validate(jwt.as_ref(), context).await.ok()?; let local_user_view = LocalUserView::read(&mut context.pool(), local_user_id) .await .ok()?; @@ -116,8 +113,6 @@ async fn local_user_settings_view_from_jwt_opt( ) .ok()?; - check_validator_time(&local_user_view.local_user.validator_time, &claims).ok()?; - Some(local_user_view) } None => None, diff --git a/crates/api_crud/src/user/create.rs b/crates/api_crud/src/user/create.rs index 02c95cb043..cabb0e7f20 100644 --- a/crates/api_crud/src/user/create.rs +++ b/crates/api_crud/src/user/create.rs @@ -1,9 +1,11 @@ use activitypub_federation::{config::Data, http_signatures::generate_actor_keypair}; -use actix_web::web::Json; +use actix_web::{web::Json, HttpResponse}; use lemmy_api_common::{ + claims::Claims, context::LemmyContext, person::{LoginResponse, Register}, utils::{ + create_login_cookie, generate_inbox_url, generate_local_apub_endpoint, generate_shared_inbox_url, @@ -29,7 +31,6 @@ use lemmy_db_schema::{ }; use lemmy_db_views::structs::{LocalUserView, SiteView}; use lemmy_utils::{ - claims::Claims, error::{LemmyError, LemmyErrorExt, LemmyErrorType}, utils::{ slurs::{check_slurs, check_slurs_opt}, @@ -41,7 +42,7 @@ use lemmy_utils::{ pub async fn register( data: Json, context: Data, -) -> Result, LemmyError> { +) -> Result { let site_view = SiteView::read_local(&mut context.pool()).await?; let local_site = site_view.local_site; let require_registration_application = @@ -158,6 +159,7 @@ pub async fn register( .await?; } + let mut res = HttpResponse::Ok(); let mut login_response = LoginResponse { jwt: None, registration_created: false, @@ -168,14 +170,12 @@ pub async fn register( if !local_site.site_setup || (!require_registration_application && !local_site.require_email_verification) { - login_response.jwt = Some( - Claims::jwt( - inserted_local_user.id.0, - &context.secret().jwt_secret, - &context.settings().hostname, - )? - .into(), - ); + let jwt = Claims::generate(inserted_local_user.id, &context).await?; + res + .cookie(create_login_cookie(jwt.clone())) + .await + .expect("set auth cookie"); + login_response.jwt = Some(jwt); } else { if local_site.require_email_verification { let local_user_view = LocalUserView { @@ -205,5 +205,5 @@ pub async fn register( } } - Ok(Json(login_response)) + Ok(res.json(login_response)) } diff --git a/crates/api_crud/src/user/delete.rs b/crates/api_crud/src/user/delete.rs index bf1dcdab1d..51c10d38bc 100644 --- a/crates/api_crud/src/user/delete.rs +++ b/crates/api_crud/src/user/delete.rs @@ -7,7 +7,7 @@ use lemmy_api_common::{ send_activity::{ActivityChannel, SendActivityData}, utils::{local_user_view_from_jwt, purge_user_account}, }; -use lemmy_db_schema::source::person::Person; +use lemmy_db_schema::source::{login_token::LoginToken, person::Person}; use lemmy_utils::error::{LemmyError, LemmyErrorType}; #[tracing::instrument(skip(context))] @@ -33,6 +33,8 @@ pub async fn delete_account( Person::delete_account(&mut context.pool(), local_user_view.person.id).await?; } + LoginToken::invalidate_all(&mut context.pool(), local_user_view.local_user.id).await?; + ActivityChannel::submit_activity( SendActivityData::DeleteUser(local_user_view.person, data.delete_content), &context, diff --git a/crates/db_schema/src/impls/local_user.rs b/crates/db_schema/src/impls/local_user.rs index 6ef3421d32..3318ac810f 100644 --- a/crates/db_schema/src/impls/local_user.rs +++ b/crates/db_schema/src/impls/local_user.rs @@ -6,7 +6,6 @@ use crate::{ email_verified, local_user, password_encrypted, - validator_time, }, source::{ actor_language::{LocalUserLanguage, SiteLanguage}, @@ -29,10 +28,7 @@ impl LocalUser { let password_hash = hash(new_password, DEFAULT_COST).expect("Couldn't hash password"); diesel::update(local_user.find(local_user_id)) - .set(( - password_encrypted.eq(password_hash), - validator_time.eq(naive_now()), - )) + .set((password_encrypted.eq(password_hash),)) .get_result::(conn) .await } diff --git a/crates/db_schema/src/impls/login_token.rs b/crates/db_schema/src/impls/login_token.rs new file mode 100644 index 0000000000..b13c6f3d81 --- /dev/null +++ b/crates/db_schema/src/impls/login_token.rs @@ -0,0 +1,54 @@ +use crate::{ + diesel::{ExpressionMethods, QueryDsl}, + newtypes::LocalUserId, + schema::login_token::{dsl::login_token, token, user_id}, + source::login_token::{LoginToken, LoginTokenCreateForm}, + utils::{get_conn, DbPool}, +}; +use diesel::{delete, dsl::exists, insert_into, result::Error, select}; +use diesel_async::RunQueryDsl; + +impl LoginToken { + pub async fn create(pool: &mut DbPool<'_>, form: LoginTokenCreateForm) -> Result { + let conn = &mut get_conn(pool).await?; + insert_into(login_token) + .values(form) + .get_result::(conn) + .await + } + + /// Check if the given token is valid for user. + pub async fn validate( + pool: &mut DbPool<'_>, + user_id_: LocalUserId, + token_: &str, + ) -> Result { + let conn = &mut get_conn(pool).await?; + select(exists( + login_token + .filter(user_id.eq(user_id_)) + .filter(token.eq(token_)), + )) + .get_result(conn) + .await + } + + /// Invalidate specific token on user logout. + pub async fn invalidate(pool: &mut DbPool<'_>, token_: &str) -> Result { + let conn = &mut get_conn(pool).await?; + delete(login_token.filter(token.eq(token_))) + .execute(conn) + .await + } + + /// Invalidate all logins of given user on password reset/change, account deletion or site ban. + pub async fn invalidate_all( + pool: &mut DbPool<'_>, + user_id_: LocalUserId, + ) -> Result { + let conn = &mut get_conn(pool).await?; + delete(login_token.filter(user_id.eq(user_id_))) + .execute(conn) + .await + } +} diff --git a/crates/db_schema/src/impls/mod.rs b/crates/db_schema/src/impls/mod.rs index a8aabf18d0..5a6318c4a6 100644 --- a/crates/db_schema/src/impls/mod.rs +++ b/crates/db_schema/src/impls/mod.rs @@ -16,6 +16,7 @@ pub mod language; pub mod local_site; pub mod local_site_rate_limit; pub mod local_user; +pub mod login_token; pub mod moderator; pub mod password_reset_request; pub mod person; diff --git a/crates/db_schema/src/impls/person.rs b/crates/db_schema/src/impls/person.rs index 723062aebf..9116b51388 100644 --- a/crates/db_schema/src/impls/person.rs +++ b/crates/db_schema/src/impls/person.rs @@ -68,10 +68,7 @@ impl Person { // Set the local user info to none diesel::update(local_user::table.filter(local_user::person_id.eq(person_id))) - .set(( - local_user::email.eq::>(None), - local_user::validator_time.eq(naive_now()), - )) + .set((local_user::email.eq::>(None),)) .execute(conn) .await?; diff --git a/crates/db_schema/src/schema.rs b/crates/db_schema/src/schema.rs index 62b6ea80a4..4d23c23ee7 100644 --- a/crates/db_schema/src/schema.rs +++ b/crates/db_schema/src/schema.rs @@ -419,7 +419,6 @@ diesel::table! { interface_language -> Varchar, show_avatars -> Bool, send_notifications_to_email -> Bool, - validator_time -> Timestamptz, show_scores -> Bool, show_bot_accounts -> Bool, show_read_posts -> Bool, @@ -445,6 +444,14 @@ diesel::table! { } } +diesel::table! { + login_token (id) { + id -> Int4, + token -> Text, + user_id -> Int4, + } +} + diesel::table! { mod_add (id) { id -> Int4, @@ -933,6 +940,7 @@ diesel::joinable!(local_site_rate_limit -> local_site (local_site_id)); diesel::joinable!(local_user -> person (person_id)); diesel::joinable!(local_user_language -> language (language_id)); diesel::joinable!(local_user_language -> local_user (local_user_id)); +diesel::joinable!(login_token -> local_user (user_id)); diesel::joinable!(mod_add_community -> community (community_id)); diesel::joinable!(mod_ban_from_community -> community (community_id)); diesel::joinable!(mod_feature_post -> person (mod_person_id)); @@ -1010,6 +1018,7 @@ diesel::allow_tables_to_appear_in_same_query!( local_site_rate_limit, local_user, local_user_language, + login_token, mod_add, mod_add_community, mod_ban, diff --git a/crates/db_schema/src/source/local_user.rs b/crates/db_schema/src/source/local_user.rs index 84ada46afb..510949fc8c 100644 --- a/crates/db_schema/src/source/local_user.rs +++ b/crates/db_schema/src/source/local_user.rs @@ -35,8 +35,6 @@ pub struct LocalUser { /// Whether to show avatars. pub show_avatars: bool, pub send_notifications_to_email: bool, - /// A validation ID used in logging out sessions. - pub validator_time: DateTime, /// Whether to show comment / post scores. pub show_scores: bool, /// Whether to show bot accounts. diff --git a/crates/db_schema/src/source/login_token.rs b/crates/db_schema/src/source/login_token.rs new file mode 100644 index 0000000000..040e206906 --- /dev/null +++ b/crates/db_schema/src/source/login_token.rs @@ -0,0 +1,19 @@ +use crate::newtypes::LocalUserId; +#[cfg(feature = "full")] +use crate::schema::login_token; + +#[derive(Clone, PartialEq, Eq, Debug)] +#[cfg_attr(feature = "full", derive(Queryable, Identifiable))] +#[cfg_attr(feature = "full", diesel(table_name = login_token))] +pub struct LoginToken { + pub id: i32, + pub token: String, + pub user_id: LocalUserId, +} + +#[cfg_attr(feature = "full", derive(Insertable, AsChangeset))] +#[cfg_attr(feature = "full", diesel(table_name = login_token))] +pub struct LoginTokenCreateForm { + pub token: String, + pub user_id: LocalUserId, +} diff --git a/crates/db_schema/src/source/mod.rs b/crates/db_schema/src/source/mod.rs index a200806cdf..e5a4c25692 100644 --- a/crates/db_schema/src/source/mod.rs +++ b/crates/db_schema/src/source/mod.rs @@ -21,6 +21,7 @@ pub mod language; pub mod local_site; pub mod local_site_rate_limit; pub mod local_user; +pub mod login_token; pub mod moderator; pub mod password_reset_request; pub mod person; diff --git a/crates/routes/src/feeds.rs b/crates/routes/src/feeds.rs index 49cef6d415..0d1eb2bdd6 100644 --- a/crates/routes/src/feeds.rs +++ b/crates/routes/src/feeds.rs @@ -22,7 +22,6 @@ use lemmy_db_views_actor::{ }; use lemmy_utils::{ cache_header::cache_1hour, - claims::Claims, error::LemmyError, utils::markdown::markdown_to_html, }; diff --git a/crates/routes/src/images.rs b/crates/routes/src/images.rs index 5133cfd823..472e4371d5 100644 --- a/crates/routes/src/images.rs +++ b/crates/routes/src/images.rs @@ -11,7 +11,7 @@ use actix_web::{ HttpResponse, }; use futures::stream::{Stream, StreamExt}; -use lemmy_api_common::{context::LemmyContext, utils::local_user_view_from_jwt}; +use lemmy_api_common::{claims::Claims, context::LemmyContext, utils::local_user_view_from_jwt}; use lemmy_db_schema::{ newtypes::LocalUserId, source::{ @@ -19,7 +19,8 @@ use lemmy_db_schema::{ local_site::LocalSite, }, }; -use lemmy_utils::{claims::Claims, rate_limit::RateLimitCell, REQWEST_TIMEOUT}; +use lemmy_db_views::structs::LocalUserView; +use lemmy_utils::{rate_limit::RateLimitCell, REQWEST_TIMEOUT}; use reqwest::Body; use reqwest_middleware::{ClientWithMiddleware, RequestBuilder}; use serde::{Deserialize, Serialize}; @@ -94,22 +95,14 @@ fn adapt_request( async fn upload( req: HttpRequest, body: web::Payload, - client: web::Data, + local_user_view: LocalUserView, context: web::Data, ) -> Result { // TODO: check rate limit here - let jwt = req.cookie("jwt").ok_or(error::ErrorUnauthorized( - "No auth header for picture upload", - ))?; - let claims = Claims::decode(jwt.value(), &context.secret().jwt_secret); - if claims.is_err() { - return Ok(HttpResponse::Unauthorized().finish()); - }; - let pictrs_config = context.settings().pictrs_config()?; let image_url = format!("{}image", pictrs_config.url); - let mut client_req = adapt_request(&req, &client, image_url); + let mut client_req = adapt_request(&req, &context.client(), image_url); if let Some(addr) = req.head().peer_addr { client_req = client_req.header("X-Forwarded-For", addr.to_string()) @@ -123,10 +116,9 @@ async fn upload( let status = res.status(); let images = res.json::().await.map_err(error::ErrorBadRequest)?; if let Some(images) = &images.files { - let local_user_id = LocalUserId(claims?.claims.sub); for uploaded_image in images { let form = ImageUploadForm { - local_user_id, + local_user_id: local_user_view.local_user.id, pictrs_alias: uploaded_image.file.to_string(), pictrs_delete_token: uploaded_image.delete_token.to_string(), }; diff --git a/crates/utils/Cargo.toml b/crates/utils/Cargo.toml index 9cafd0c11a..822f3b9504 100644 --- a/crates/utils/Cargo.toml +++ b/crates/utils/Cargo.toml @@ -44,7 +44,6 @@ openssl = "0.10.55" html2text = "0.6.0" deser-hjson = "1.2.0" 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"] } diff --git a/crates/utils/src/claims.rs b/crates/utils/src/claims.rs deleted file mode 100644 index cebd422ac4..0000000000 --- a/crates/utils/src/claims.rs +++ /dev/null @@ -1,35 +0,0 @@ -use crate::error::LemmyError; -use chrono::Utc; -use jsonwebtoken::{decode, encode, DecodingKey, EncodingKey, Header, TokenData, Validation}; -use serde::{Deserialize, Serialize}; -type Jwt = String; - -#[derive(Debug, Serialize, Deserialize)] -pub struct Claims { - /// local_user_id, standard claim by RFC 7519. - pub sub: i32, - pub iss: String, - /// Time when this token was issued as UNIX-timestamp in seconds - pub iat: i64, -} - -impl Claims { - pub fn decode(jwt: &str, jwt_secret: &str) -> Result, LemmyError> { - let mut validation = Validation::default(); - validation.validate_exp = false; - validation.required_spec_claims.remove("exp"); - let key = DecodingKey::from_secret(jwt_secret.as_ref()); - Ok(decode::(jwt, &key, &validation)?) - } - - pub fn jwt(local_user_id: i32, jwt_secret: &str, hostname: &str) -> Result { - let my_claims = Claims { - sub: local_user_id, - iss: hostname.to_string(), - iat: Utc::now().timestamp(), - }; - - let key = EncodingKey::from_secret(jwt_secret.as_ref()); - Ok(encode(&Header::default(), &my_claims, &key)?) - } -} diff --git a/crates/utils/src/lib.rs b/crates/utils/src/lib.rs index c0553de316..6f261febd7 100644 --- a/crates/utils/src/lib.rs +++ b/crates/utils/src/lib.rs @@ -6,13 +6,11 @@ extern crate smart_default; pub mod apub; pub mod cache_header; pub mod email; -pub mod rate_limit; -pub mod settings; - -pub mod claims; pub mod error; +pub mod rate_limit; pub mod request; pub mod response; +pub mod settings; pub mod utils; pub mod version; diff --git a/migrations/2023-09-18-141700_login-token/down.sql b/migrations/2023-09-18-141700_login-token/down.sql new file mode 100644 index 0000000000..193485e2b9 --- /dev/null +++ b/migrations/2023-09-18-141700_login-token/down.sql @@ -0,0 +1,4 @@ +DROP TABLE login_token; + +ALTER TABLE local_user + ADD COLUMN validator_time timestamp NOT NULL DEFAULT now(); diff --git a/migrations/2023-09-18-141700_login-token/up.sql b/migrations/2023-09-18-141700_login-token/up.sql new file mode 100644 index 0000000000..a2ce39a0e1 --- /dev/null +++ b/migrations/2023-09-18-141700_login-token/up.sql @@ -0,0 +1,9 @@ +CREATE TABLE login_token ( + id serial PRIMARY KEY, + token text NOT NULL UNIQUE, + user_id int REFERENCES local_user ON UPDATE CASCADE ON DELETE CASCADE NOT NULL +); + +-- not needed anymore as we invalidate login tokens on password change +ALTER TABLE local_user + DROP COLUMN validator_time; diff --git a/src/api_routes_http.rs b/src/api_routes_http.rs index 74003b8d8f..d3feb3f39b 100644 --- a/src/api_routes_http.rs +++ b/src/api_routes_http.rs @@ -23,6 +23,7 @@ use lemmy_api::{ get_captcha::get_captcha, list_banned::list_banned_users, login::login, + logout::logout, notifications::{ list_mentions::list_mentions, list_replies::list_replies, @@ -269,6 +270,7 @@ pub fn config(cfg: &mut web::ServiceConfig, rate_limit: &RateLimitCell) { .route("/block", web::post().to(block_person)) // Account actions. I don't like that they're in /user maybe /accounts .route("/login", web::post().to(login)) + .route("/logout", web::post().to(logout)) .route("/delete_account", web::post().to(delete_account)) .route("/password_reset", web::post().to(reset_password)) .route( diff --git a/src/session_middleware.rs b/src/session_middleware.rs index 5824f33df7..845c834bea 100644 --- a/src/session_middleware.rs +++ b/src/session_middleware.rs @@ -8,13 +8,14 @@ use actix_web::{ }; use core::future::Ready; use futures_util::future::LocalBoxFuture; -use lemmy_api_common::{context::LemmyContext, utils::local_user_view_from_jwt}; +use lemmy_api_common::{ + context::LemmyContext, + utils::{local_user_view_from_jwt, AUTH_COOKIE_NAME}, +}; use lemmy_utils::error::{LemmyError, LemmyErrorType}; use reqwest::header::HeaderValue; use std::{future::ready, rc::Rc}; -static AUTH_COOKIE_NAME: &str = "auth"; - #[derive(Clone)] pub struct SessionMiddleware { context: LemmyContext, From 20e38c7b07e127f51c47e058c2bc5b52cec237fe Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 19 Sep 2023 13:35:16 +0200 Subject: [PATCH 02/15] stuff --- Cargo.lock | 1 + api_tests/src/user.spec.ts | 13 +++ crates/api/src/lib.rs | 94 ++++++------------- crates/api/src/local_user/logout.rs | 12 ++- crates/api/src/local_user/save_settings.rs | 43 ++------- crates/api_common/Cargo.toml | 4 + crates/api_common/src/claims.rs | 63 +++++++++++++ crates/api_common/src/lib.rs | 17 ++++ crates/api_common/src/utils.rs | 17 +--- crates/api_crud/src/site/read.rs | 32 +------ crates/apub/src/api/list_comments.rs | 5 +- crates/apub/src/api/list_posts.rs | 5 +- crates/apub/src/api/read_community.rs | 5 +- crates/apub/src/api/read_person.rs | 5 +- crates/apub/src/api/resolve_object.rs | 5 +- crates/apub/src/api/search.rs | 5 +- crates/db_schema/src/impls/local_user.rs | 2 +- crates/db_schema/src/source/local_user.rs | 2 +- .../src/registration_application_view.rs | 1 - crates/routes/src/feeds.rs | 83 +++++++--------- crates/routes/src/images.rs | 3 +- src/session_middleware.rs | 38 +------- 22 files changed, 202 insertions(+), 253 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c4fe2f81f9..9788a6a12b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2658,6 +2658,7 @@ dependencies = [ "rosetta-i18n", "serde", "serde_with", + "serial_test", "tokio", "tracing", "ts-rs", diff --git a/api_tests/src/user.spec.ts b/api_tests/src/user.spec.ts index f488ebe1e6..9aeb2263cb 100644 --- a/api_tests/src/user.spec.ts +++ b/api_tests/src/user.spec.ts @@ -121,3 +121,16 @@ test("Requests with invalid auth should be treated as unauthenticated", async () let posts = invalid_auth.client.getPosts(form); expect((await posts).posts).toBeDefined(); }); + +test("Logout", async () => { + let userRes = await registerUser(alpha); + expect(userRes.jwt).toBeDefined(); + let user: API = { + client: alpha.client, + auth: userRes.jwt ?? "", + }; + + // TODO: requires lemmy-js-client update + user.client.login(); + expect(false); +}); \ No newline at end of file diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index 65c8d05511..a7d183a529 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -1,12 +1,14 @@ 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_api_common::utils::{AUTH_COOKIE_NAME, local_site_to_slur_regex}; use lemmy_db_schema::source::local_site::LocalSite; use lemmy_utils::{ error::{LemmyError, LemmyErrorExt, LemmyErrorType}, utils::slurs::check_slurs, }; use std::io::Cursor; +use actix_web::cookie::SameSite; +use actix_web::HttpRequest; pub mod comment; pub mod comment_report; @@ -67,71 +69,29 @@ pub(crate) fn check_report_reason(reason: &str, local_site: &LocalSite) -> Resul } } -#[cfg(test)] -mod tests { - #![allow(clippy::unwrap_used)] - #![allow(clippy::indexing_slicing)] - - use lemmy_api_common::utils::check_validator_time; - use lemmy_db_schema::{ - source::{ - instance::Instance, - local_user::{LocalUser, LocalUserInsertForm}, - person::{Person, PersonInsertForm}, - secret::Secret, - }, - traits::Crud, - utils::build_db_pool_for_tests, - }; - use lemmy_utils::{claims::Claims, settings::SETTINGS}; - use serial_test::serial; - - #[tokio::test] - #[serial] - async fn test_should_not_validate_user_token_after_password_change() { - let pool = &build_db_pool_for_tests().await; - let pool = &mut pool.into(); - let secret = Secret::init(pool).await.unwrap(); - let settings = &SETTINGS.to_owned(); - - let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string()) - .await - .unwrap(); - - let new_person = PersonInsertForm::builder() - .name("Gerry9812".into()) - .public_key("pubkey".to_string()) - .instance_id(inserted_instance.id) - .build(); - - let inserted_person = Person::create(pool, &new_person).await.unwrap(); - - let local_user_form = LocalUserInsertForm::builder() - .person_id(inserted_person.id) - .password_encrypted("123456".to_string()) - .build(); - - let inserted_local_user = LocalUser::create(pool, &local_user_form).await.unwrap(); - - let jwt = Claims::jwt( - inserted_local_user.id.0, - &secret.jwt_secret, - &settings.hostname, - ) - .unwrap(); - let claims = Claims::decode(&jwt, &secret.jwt_secret).unwrap().claims; - let check = check_validator_time(&inserted_local_user.validator_time, &claims); - assert!(check.is_ok()); - - // The check should fail, since the validator time is now newer than the jwt issue time - let updated_local_user = - LocalUser::update_password(pool, inserted_local_user.id, "password111") - .await - .unwrap(); - let check_after = check_validator_time(&updated_local_user.validator_time, &claims); - assert!(check_after.is_err()); - - let num_deleted = Person::delete(pool, inserted_person.id).await.unwrap(); - assert_eq!(1, num_deleted); +pub fn read_auth_token(req: &HttpRequest) -> Result, LemmyError> { +// Try reading jwt from auth header + let auth_header = req + .headers() + .get(AUTH_COOKIE_NAME) + .and_then(|h| h.to_str().ok()); + let jwt = if let Some(a) = auth_header { + Some(a.to_string()) } + // If that fails, try auth cookie. Dont use the `jwt` cookie from lemmy-ui because + // its not http-only. + else { + let auth_cookie = req.cookie(AUTH_COOKIE_NAME); + if let Some(a) = &auth_cookie { + // ensure that its marked as httponly and secure + let secure = a.secure().unwrap_or_default(); + let http_only = a.http_only().unwrap_or_default(); + let same_site = a.same_site(); + if !secure || !http_only || same_site != Some(SameSite::Strict) { + return Err(LemmyError::from(LemmyErrorType::AuthCookieInsecure).into()); + } + } + auth_cookie.map(|c| c.value().to_string()) + }; + Ok(jwt) } diff --git a/crates/api/src/local_user/logout.rs b/crates/api/src/local_user/logout.rs index 259413682e..9bb42e2798 100644 --- a/crates/api/src/local_user/logout.rs +++ b/crates/api/src/local_user/logout.rs @@ -4,18 +4,20 @@ use lemmy_api_common::{context::LemmyContext, utils::AUTH_COOKIE_NAME}; use lemmy_db_schema::source::login_token::LoginToken; use lemmy_db_views::structs::LocalUserView; use lemmy_utils::error::LemmyResult; +use crate::read_auth_token; #[tracing::instrument(skip(context))] pub async fn logout( - local_user_view: LocalUserView, + req: HttpRequest, + // require login + _local_user_view: LocalUserView, context: Data, ) -> LemmyResult { - // TODO: need to retrieve auth token. middleware could write it to request extensions - let jwt = todo!(); - LoginToken::invalidate(&mut context.pool(), jwt).await?; + let jwt = read_auth_token(&req)?.expect("user is logged in"); + LoginToken::invalidate(&mut context.pool(), &jwt).await?; let mut res = HttpResponse::Ok().finish(); - let mut cookie = Cookie::new(AUTH_COOKIE_NAME, ""); + let cookie = Cookie::new(AUTH_COOKIE_NAME, ""); res.add_removal_cookie(&cookie)?; Ok(res) } diff --git a/crates/api/src/local_user/save_settings.rs b/crates/api/src/local_user/save_settings.rs index e79fccecad..672437da45 100644 --- a/crates/api/src/local_user/save_settings.rs +++ b/crates/api/src/local_user/save_settings.rs @@ -1,10 +1,5 @@ use actix_web::web::{Data, Json}; -use lemmy_api_common::{ - claims::Claims, - context::LemmyContext, - person::{LoginResponse, SaveUserSettings}, - utils::{local_user_view_from_jwt, sanitize_html_api_opt, send_verification_email}, -}; +use lemmy_api_common::{context::LemmyContext, person::{SaveUserSettings}, SuccessResponse, utils::{local_user_view_from_jwt, sanitize_html_api_opt, send_verification_email}}; use lemmy_db_schema::{ source::{ actor_language::LocalUserLanguage, @@ -30,7 +25,7 @@ use lemmy_utils::{ pub async fn save_user_settings( data: Json, context: Data, -) -> Result, LemmyError> { +) -> Result, LemmyError> { let local_user_view = local_user_view_from_jwt(&data.auth, &context).await?; let site_view = SiteView::read_local(&mut context.pool()).await?; @@ -47,15 +42,18 @@ pub async fn save_user_settings( if let Some(Some(email)) = &email { let previous_email = local_user_view.local_user.email.clone().unwrap_or_default(); - // Only send the verification email if there was an email change - if previous_email.ne(email) { + // if email was changed, check that it is not taken and send verification mail + if &previous_email != email { + if LocalUser::is_email_taken(&mut context.pool(), email).await? { + return Err(LemmyErrorType::EmailAlreadyExists)?; + } send_verification_email( &local_user_view, email, &mut context.pool(), context.settings(), ) - .await?; + .await?; } } @@ -141,28 +139,7 @@ pub async fn save_user_settings( ..Default::default() }; - let local_user_res = - LocalUser::update(&mut context.pool(), local_user_id, &local_user_form).await; - let updated_local_user = match local_user_res { - Ok(u) => u, - Err(e) => { - let err_type = if e.to_string() - == "duplicate key value violates unique constraint \"local_user_email_key\"" - { - LemmyErrorType::EmailAlreadyExists - } else { - LemmyErrorType::UserAlreadyExists - }; - - return Err(e).with_lemmy_type(err_type); - } - }; + LocalUser::update(&mut context.pool(), local_user_id, &local_user_form).await?; - // Return the jwt - let jwt = Some(Claims::generate(local_user_view.local_user.id, &context).await?); - Ok(Json(LoginResponse { - jwt, - verify_email_sent: false, - registration_created: false, - })) + Ok(Json(SuccessResponse::new())) } diff --git a/crates/api_common/Cargo.toml b/crates/api_common/Cargo.toml index a8522030e5..24c24dffe3 100644 --- a/crates/api_common/Cargo.toml +++ b/crates/api_common/Cargo.toml @@ -68,3 +68,7 @@ actix-web = { workspace = true, optional = true } jsonwebtoken = { version = "8.3.0", optional = true } # necessary for wasmt compilation getrandom = { version = "0.2.10", features = ["js"] } + +[dev-dependencies] +serial_test = { workspace = true } +reqwest-middleware = { workspace = true } \ No newline at end of file diff --git a/crates/api_common/src/claims.rs b/crates/api_common/src/claims.rs index 969f71a224..e1b3845fb5 100644 --- a/crates/api_common/src/claims.rs +++ b/crates/api_common/src/claims.rs @@ -56,3 +56,66 @@ impl Claims { Ok(Sensitive::new(token)) } } + +#[cfg(test)] +mod tests { + #![allow(clippy::unwrap_used)] + #![allow(clippy::indexing_slicing)] + + use reqwest::Client; + use lemmy_db_schema::{ + source::{ + instance::Instance, + local_user::{LocalUser, LocalUserInsertForm}, + person::{Person, PersonInsertForm}, + secret::Secret, + }, + traits::Crud, + utils::build_db_pool_for_tests, + }; + + use serial_test::serial; + use crate::claims::Claims; + use crate::context::LemmyContext; + use lemmy_utils::rate_limit::{RateLimitCell, RateLimitConfig}; + use reqwest_middleware::ClientBuilder; + + #[tokio::test] + #[serial] + async fn test_should_not_validate_user_token_after_password_change() { + let pool_ = build_db_pool_for_tests().await; + let pool = &mut (&pool_).into(); + let secret = Secret::init(pool).await.unwrap(); + let context = LemmyContext::create(pool_.clone(), ClientBuilder::new(Client::default()).build(), secret, RateLimitCell::new(RateLimitConfig::builder().build()).await.clone()); + + let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string()) + .await + .unwrap(); + + let new_person = PersonInsertForm::builder() + .name("Gerry9812".into()) + .public_key("pubkey".to_string()) + .instance_id(inserted_instance.id) + .build(); + + let inserted_person = Person::create(pool, &new_person).await.unwrap(); + + let local_user_form = LocalUserInsertForm::builder() + .person_id(inserted_person.id) + .password_encrypted("123456".to_string()) + .build(); + + let inserted_local_user = LocalUser::create(pool, &local_user_form).await.unwrap(); + + let jwt = Claims::generate( + inserted_local_user.id,&context + ).await + .unwrap(); + + let valid = Claims::validate(&jwt, &context).await; + assert!(valid.is_ok()); + + let num_deleted = Person::delete(pool, inserted_person.id).await.unwrap(); + assert_eq!(1, num_deleted); + } +} diff --git a/crates/api_common/src/lib.rs b/crates/api_common/src/lib.rs index ff8f65364e..25abb1df62 100644 --- a/crates/api_common/src/lib.rs +++ b/crates/api_common/src/lib.rs @@ -23,3 +23,20 @@ pub extern crate lemmy_db_schema; pub extern crate lemmy_db_views; pub extern crate lemmy_db_views_actor; pub extern crate lemmy_db_views_moderator; + +use serde::{Deserialize, Serialize}; +use ts_rs::TS; + +#[derive(Debug, Serialize, Deserialize, Clone, Default)] +#[cfg_attr(feature = "full", derive(TS))] +#[cfg_attr(feature = "full", ts(export))] +/// Saves settings for your user. +pub struct SuccessResponse { + pub success: bool, +} + +impl SuccessResponse { + pub fn new() -> Self { + SuccessResponse { success: true } + } +} \ No newline at end of file diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index c9b6c7c1eb..75e16fc6ec 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -10,7 +10,7 @@ use anyhow::Context; use chrono::{DateTime, Utc}; use lemmy_db_schema::{ impls::person::is_banned, - newtypes::{CommunityId, DbUrl, LocalUserId, PersonId, PostId}, + newtypes::{CommunityId, DbUrl, PersonId, PostId}, source::{ comment::{Comment, CommentUpdateForm}, community::{Community, CommunityModerator, CommunityUpdateForm}, @@ -36,7 +36,7 @@ use lemmy_db_views_actor::structs::{ }; use lemmy_utils::{ email::{send_email, translations::Lang}, - error::{LemmyError, LemmyErrorExt, LemmyErrorExt2, LemmyErrorType}, + error::{LemmyError, LemmyErrorExt, LemmyErrorType}, location_info, rate_limit::RateLimitConfig, settings::structs::Settings, @@ -47,6 +47,8 @@ use rosetta_i18n::{Language, LanguageId}; use tracing::warn; use url::{ParseError, Url}; +pub static AUTH_COOKIE_NAME: &str = "auth"; + #[tracing::instrument(skip_all)] pub async fn is_mod_or_admin( pool: &mut DbPool<'_>, @@ -158,16 +160,6 @@ pub async fn local_user_view_from_jwt_opt( ) -> Option { local_user_view_from_jwt(jwt?, context).await.ok() } -#[tracing::instrument(skip_all)] -pub async fn local_user_view_from_jwt_opt_new( - local_user_view: &mut Option, - jwt: Option<&Sensitive>, - context: &LemmyContext, -) { - if local_user_view.is_none() { - *local_user_view = local_user_view_from_jwt_opt(jwt, context).await; - } -} pub fn check_user_valid( banned: bool, @@ -845,4 +837,3 @@ mod tests { } } -pub static AUTH_COOKIE_NAME: &str = "auth"; diff --git a/crates/api_crud/src/site/read.rs b/crates/api_crud/src/site/read.rs index c398dc81d7..54d98747c7 100644 --- a/crates/api_crud/src/site/read.rs +++ b/crates/api_crud/src/site/read.rs @@ -1,13 +1,9 @@ use actix_web::web::{Data, Json, Query}; use lemmy_api_common::{ - claims::Claims, context::LemmyContext, - sensitive::Sensitive, site::{GetSite, GetSiteResponse, MyUserInfo}, - utils::check_user_valid, }; use lemmy_db_schema::{ - newtypes::LocalUserId, source::{ actor_language::{LocalUserLanguage, SiteLanguage}, language::Language, @@ -30,6 +26,7 @@ use lemmy_utils::{ #[tracing::instrument(skip(context))] pub async fn get_site( data: Query, + local_user_view: Option, context: Data, ) -> Result, LemmyError> { let site_view = SiteView::read_local(&mut context.pool()).await?; @@ -37,9 +34,7 @@ pub async fn get_site( let admins = PersonView::admins(&mut context.pool()).await?; // Build the local user - let my_user = if let Some(local_user_view) = - local_user_settings_view_from_jwt_opt(data.auth.as_ref(), &context).await - { + let my_user = if let Some(local_user_view) = local_user_view { let person_id = local_user_view.person.id; let local_user_id = local_user_view.local_user.id; @@ -95,26 +90,3 @@ pub async fn get_site( })) } -#[tracing::instrument(skip_all)] -async fn local_user_settings_view_from_jwt_opt( - jwt: Option<&Sensitive>, - context: &LemmyContext, -) -> Option { - match jwt { - Some(jwt) => { - let local_user_id = Claims::validate(jwt.as_ref(), context).await.ok()?; - let local_user_view = LocalUserView::read(&mut context.pool(), local_user_id) - .await - .ok()?; - check_user_valid( - local_user_view.person.banned, - local_user_view.person.ban_expires, - local_user_view.person.deleted, - ) - .ok()?; - - Some(local_user_view) - } - None => None, - } -} diff --git a/crates/apub/src/api/list_comments.rs b/crates/apub/src/api/list_comments.rs index cec2ed9c59..57f30c8b3e 100644 --- a/crates/apub/src/api/list_comments.rs +++ b/crates/apub/src/api/list_comments.rs @@ -8,7 +8,7 @@ use actix_web::web::{Json, Query}; use lemmy_api_common::{ comment::{GetComments, GetCommentsResponse}, context::LemmyContext, - utils::{check_private_instance, local_user_view_from_jwt_opt_new}, + utils::{check_private_instance}, }; use lemmy_db_schema::{ source::{comment::Comment, community::Community, local_site::LocalSite}, @@ -21,9 +21,8 @@ use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType}; pub async fn list_comments( data: Query, context: Data, - mut local_user_view: Option, + local_user_view: Option, ) -> Result, LemmyError> { - local_user_view_from_jwt_opt_new(&mut local_user_view, data.auth.as_ref(), &context).await; let local_site = LocalSite::read(&mut context.pool()).await?; check_private_instance(&local_user_view, &local_site)?; diff --git a/crates/apub/src/api/list_posts.rs b/crates/apub/src/api/list_posts.rs index 425e2adf2a..ee5b6ad903 100644 --- a/crates/apub/src/api/list_posts.rs +++ b/crates/apub/src/api/list_posts.rs @@ -8,7 +8,7 @@ use actix_web::web::{Json, Query}; use lemmy_api_common::{ context::LemmyContext, post::{GetPosts, GetPostsResponse}, - utils::{check_private_instance, local_user_view_from_jwt_opt_new}, + utils::{check_private_instance}, }; use lemmy_db_schema::source::{community::Community, local_site::LocalSite}; use lemmy_db_views::{ @@ -21,9 +21,8 @@ use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType}; pub async fn list_posts( data: Query, context: Data, - mut local_user_view: Option, + local_user_view: Option, ) -> Result, LemmyError> { - local_user_view_from_jwt_opt_new(&mut local_user_view, data.auth.as_ref(), &context).await; let local_site = LocalSite::read(&mut context.pool()).await?; check_private_instance(&local_user_view, &local_site)?; diff --git a/crates/apub/src/api/read_community.rs b/crates/apub/src/api/read_community.rs index 76f7f580dd..afa6fb8293 100644 --- a/crates/apub/src/api/read_community.rs +++ b/crates/apub/src/api/read_community.rs @@ -4,7 +4,7 @@ use actix_web::web::{Json, Query}; use lemmy_api_common::{ community::{GetCommunity, GetCommunityResponse}, context::LemmyContext, - utils::{check_private_instance, is_mod_or_admin_opt, local_user_view_from_jwt_opt_new}, + utils::{check_private_instance, is_mod_or_admin_opt}, }; use lemmy_db_schema::source::{ actor_language::CommunityLanguage, @@ -20,9 +20,8 @@ use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorExt2, LemmyErrorTy pub async fn get_community( data: Query, context: Data, - mut local_user_view: Option, + local_user_view: Option, ) -> Result, LemmyError> { - local_user_view_from_jwt_opt_new(&mut local_user_view, data.auth.as_ref(), &context).await; let local_site = LocalSite::read(&mut context.pool()).await?; if data.name.is_none() && data.id.is_none() { diff --git a/crates/apub/src/api/read_person.rs b/crates/apub/src/api/read_person.rs index 754a942f19..e270b08750 100644 --- a/crates/apub/src/api/read_person.rs +++ b/crates/apub/src/api/read_person.rs @@ -4,7 +4,7 @@ use actix_web::web::{Json, Query}; use lemmy_api_common::{ context::LemmyContext, person::{GetPersonDetails, GetPersonDetailsResponse}, - utils::{check_private_instance, local_user_view_from_jwt_opt_new}, + utils::{check_private_instance}, }; use lemmy_db_schema::{ source::{local_site::LocalSite, person::Person}, @@ -18,14 +18,13 @@ use lemmy_utils::error::{LemmyError, LemmyErrorExt2, LemmyErrorType}; pub async fn read_person( data: Query, context: Data, - mut local_user_view: Option, + local_user_view: Option, ) -> Result, LemmyError> { // Check to make sure a person name or an id is given if data.username.is_none() && data.person_id.is_none() { Err(LemmyErrorType::NoIdGiven)? } - local_user_view_from_jwt_opt_new(&mut local_user_view, data.auth.as_ref(), &context).await; let local_site = LocalSite::read(&mut context.pool()).await?; check_private_instance(&local_user_view, &local_site)?; diff --git a/crates/apub/src/api/resolve_object.rs b/crates/apub/src/api/resolve_object.rs index b8c0cef147..d48fbc69d1 100644 --- a/crates/apub/src/api/resolve_object.rs +++ b/crates/apub/src/api/resolve_object.rs @@ -9,7 +9,7 @@ use diesel::NotFound; use lemmy_api_common::{ context::LemmyContext, site::{ResolveObject, ResolveObjectResponse}, - utils::{check_private_instance, local_user_view_from_jwt_opt_new}, + utils::{check_private_instance}, }; use lemmy_db_schema::{newtypes::PersonId, source::local_site::LocalSite, utils::DbPool}; use lemmy_db_views::structs::{CommentView, LocalUserView, PostView}; @@ -20,9 +20,8 @@ use lemmy_utils::error::{LemmyError, LemmyErrorExt2, LemmyErrorType}; pub async fn resolve_object( data: Query, context: Data, - mut local_user_view: Option, + local_user_view: Option, ) -> Result, LemmyError> { - local_user_view_from_jwt_opt_new(&mut local_user_view, data.auth.as_ref(), &context).await; let local_site = LocalSite::read(&mut context.pool()).await?; check_private_instance(&local_user_view, &local_site)?; let person_id = local_user_view.map(|v| v.person.id); diff --git a/crates/apub/src/api/search.rs b/crates/apub/src/api/search.rs index 262f91681b..0c7231e8f7 100644 --- a/crates/apub/src/api/search.rs +++ b/crates/apub/src/api/search.rs @@ -4,7 +4,7 @@ use actix_web::web::{Json, Query}; use lemmy_api_common::{ context::LemmyContext, site::{Search, SearchResponse}, - utils::{check_private_instance, is_admin, local_user_view_from_jwt_opt_new}, + utils::{check_private_instance, is_admin}, }; use lemmy_db_schema::{ source::{community::Community, local_site::LocalSite}, @@ -19,9 +19,8 @@ use lemmy_utils::error::LemmyError; pub async fn search( data: Query, context: Data, - mut local_user_view: Option, + local_user_view: Option, ) -> Result, LemmyError> { - local_user_view_from_jwt_opt_new(&mut local_user_view, data.auth.as_ref(), &context).await; let local_site = LocalSite::read(&mut context.pool()).await?; check_private_instance(&local_user_view, &local_site)?; diff --git a/crates/db_schema/src/impls/local_user.rs b/crates/db_schema/src/impls/local_user.rs index 3318ac810f..eccbd578c2 100644 --- a/crates/db_schema/src/impls/local_user.rs +++ b/crates/db_schema/src/impls/local_user.rs @@ -12,7 +12,7 @@ use crate::{ local_user::{LocalUser, LocalUserInsertForm, LocalUserUpdateForm}, }, traits::Crud, - utils::{get_conn, naive_now, DbPool}, + utils::{get_conn, DbPool}, }; use bcrypt::{hash, DEFAULT_COST}; use diesel::{dsl::insert_into, result::Error, ExpressionMethods, QueryDsl}; diff --git a/crates/db_schema/src/source/local_user.rs b/crates/db_schema/src/source/local_user.rs index 510949fc8c..5c5a452207 100644 --- a/crates/db_schema/src/source/local_user.rs +++ b/crates/db_schema/src/source/local_user.rs @@ -6,7 +6,7 @@ use crate::{ PostListingMode, SortType, }; -use chrono::{DateTime, Utc}; + use serde::{Deserialize, Serialize}; use serde_with::skip_serializing_none; #[cfg(feature = "full")] diff --git a/crates/db_views/src/registration_application_view.rs b/crates/db_views/src/registration_application_view.rs index 19e300ac72..55e0fc90a9 100644 --- a/crates/db_views/src/registration_application_view.rs +++ b/crates/db_views/src/registration_application_view.rs @@ -254,7 +254,6 @@ mod tests { interface_language: inserted_sara_local_user.interface_language, show_avatars: inserted_sara_local_user.show_avatars, send_notifications_to_email: inserted_sara_local_user.send_notifications_to_email, - validator_time: inserted_sara_local_user.validator_time, show_bot_accounts: inserted_sara_local_user.show_bot_accounts, show_scores: inserted_sara_local_user.show_scores, show_read_posts: inserted_sara_local_user.show_read_posts, diff --git a/crates/routes/src/feeds.rs b/crates/routes/src/feeds.rs index 0d1eb2bdd6..ba075581c8 100644 --- a/crates/routes/src/feeds.rs +++ b/crates/routes/src/feeds.rs @@ -3,17 +3,15 @@ use anyhow::anyhow; use chrono::{DateTime, Utc}; use lemmy_api_common::context::LemmyContext; use lemmy_db_schema::{ - newtypes::LocalUserId, - source::{community::Community, local_user::LocalUser, person::Person}, - traits::{ApubActor, Crud}, - utils::DbPool, + source::{community::Community, person::Person}, + traits::{ApubActor}, CommentSortType, ListingType, SortType, }; use lemmy_db_views::{ post_view::PostQuery, - structs::{LocalUserView, PostView, SiteView}, + structs::{PostView, SiteView}, }; use lemmy_db_views_actor::{ comment_reply_view::CommentReplyQuery, @@ -36,6 +34,8 @@ use rss::{ use serde::Deserialize; use std::{collections::BTreeMap, str::FromStr}; +use lemmy_api_common::utils::local_user_view_from_jwt; + const RSS_FETCH_LIMIT: i64 = 20; #[derive(Deserialize)] @@ -181,50 +181,41 @@ async fn get_feed( _ => return Err(ErrorBadRequest(LemmyError::from(anyhow!("wrong_type")))), }; - let jwt_secret = context.secret().jwt_secret.clone(); - let protocol_and_hostname = context.settings().get_protocol_and_hostname(); - let builder = match request_type { RequestType::User => { get_feed_user( - &mut context.pool(), + &context, &info.sort_type()?, &info.get_limit(), &info.get_page(), ¶m, - &protocol_and_hostname, ) .await } RequestType::Community => { get_feed_community( - &mut context.pool(), + &context, &info.sort_type()?, &info.get_limit(), &info.get_page(), ¶m, - &protocol_and_hostname, ) .await } RequestType::Front => { get_feed_front( - &mut context.pool(), - &jwt_secret, + &context, &info.sort_type()?, &info.get_limit(), &info.get_page(), ¶m, - &protocol_and_hostname, ) .await } RequestType::Inbox => { get_feed_inbox( - &mut context.pool(), - &jwt_secret, + &context, ¶m, - &protocol_and_hostname, ) .await } @@ -242,15 +233,14 @@ async fn get_feed( #[tracing::instrument(skip_all)] async fn get_feed_user( - pool: &mut DbPool<'_>, + context: &LemmyContext, sort_type: &SortType, limit: &i64, page: &i64, user_name: &str, - protocol_and_hostname: &str, ) -> Result { - let site_view = SiteView::read_local(pool).await?; - let person = Person::read_from_name(pool, user_name, false).await?; + let site_view = SiteView::read_local(&mut context.pool()).await?; + let person = Person::read_from_name(&mut context.pool(), user_name, false).await?; let posts = PostQuery { listing_type: (Some(ListingType::All)), @@ -260,10 +250,10 @@ async fn get_feed_user( page: (Some(*page)), ..Default::default() } - .list(pool) + .list(&mut context.pool()) .await?; - let items = create_post_items(posts, protocol_and_hostname)?; + let items = create_post_items(posts, &context.settings().get_protocol_and_hostname())?; let mut channel_builder = ChannelBuilder::default(); channel_builder @@ -277,15 +267,14 @@ async fn get_feed_user( #[tracing::instrument(skip_all)] async fn get_feed_community( - pool: &mut DbPool<'_>, + context: &LemmyContext, sort_type: &SortType, limit: &i64, page: &i64, community_name: &str, - protocol_and_hostname: &str, ) -> Result { - let site_view = SiteView::read_local(pool).await?; - let community = Community::read_from_name(pool, community_name, false).await?; + let site_view = SiteView::read_local(&mut context.pool()).await?; + let community = Community::read_from_name(&mut context.pool(), community_name, false).await?; let posts = PostQuery { sort: (Some(*sort_type)), @@ -294,10 +283,10 @@ async fn get_feed_community( page: (Some(*page)), ..Default::default() } - .list(pool) + .list(&mut context.pool()) .await?; - let items = create_post_items(posts, protocol_and_hostname)?; + let items = create_post_items(posts, &context.settings().get_protocol_and_hostname())?; let mut channel_builder = ChannelBuilder::default(); channel_builder @@ -315,17 +304,14 @@ async fn get_feed_community( #[tracing::instrument(skip_all)] async fn get_feed_front( - pool: &mut DbPool<'_>, - jwt_secret: &str, + context: &LemmyContext, sort_type: &SortType, limit: &i64, page: &i64, jwt: &str, - protocol_and_hostname: &str, ) -> Result { - let site_view = SiteView::read_local(pool).await?; - let local_user_id = LocalUserId(Claims::decode(jwt, jwt_secret)?.claims.sub); - let local_user = LocalUserView::read(pool, local_user_id).await?; + let site_view = SiteView::read_local(&mut context.pool()).await?; + let local_user = local_user_view_from_jwt(jwt, context).await?; let posts = PostQuery { listing_type: (Some(ListingType::Subscribed)), @@ -335,10 +321,11 @@ async fn get_feed_front( page: (Some(*page)), ..Default::default() } - .list(pool) + .list(&mut context.pool()) .await?; - let items = create_post_items(posts, protocol_and_hostname)?; + let protocol_and_hostname =context.settings().get_protocol_and_hostname(); + let items = create_post_items(posts, &protocol_and_hostname)?; let mut channel_builder = ChannelBuilder::default(); channel_builder @@ -356,16 +343,13 @@ async fn get_feed_front( #[tracing::instrument(skip_all)] async fn get_feed_inbox( - pool: &mut DbPool<'_>, - jwt_secret: &str, + context: &LemmyContext, jwt: &str, - protocol_and_hostname: &str, ) -> Result { - let site_view = SiteView::read_local(pool).await?; - let local_user_id = LocalUserId(Claims::decode(jwt, jwt_secret)?.claims.sub); - let local_user = LocalUser::read(pool, local_user_id).await?; - let person_id = local_user.person_id; - let show_bot_accounts = local_user.show_bot_accounts; + let site_view = SiteView::read_local(&mut context.pool()).await?; + let local_user = local_user_view_from_jwt(jwt, context).await?; + let person_id = local_user.local_user.person_id; + let show_bot_accounts = local_user.local_user.show_bot_accounts; let sort = CommentSortType::New; @@ -377,7 +361,7 @@ async fn get_feed_inbox( limit: (Some(RSS_FETCH_LIMIT)), ..Default::default() } - .list(pool) + .list(&mut context.pool()) .await?; let mentions = PersonMentionQuery { @@ -388,10 +372,11 @@ async fn get_feed_inbox( limit: (Some(RSS_FETCH_LIMIT)), ..Default::default() } - .list(pool) + .list(&mut context.pool()) .await?; - let items = create_reply_and_mention_items(replies, mentions, protocol_and_hostname)?; + let protocol_and_hostname = context.settings().get_protocol_and_hostname(); + let items = create_reply_and_mention_items(replies, mentions, &protocol_and_hostname)?; let mut channel_builder = ChannelBuilder::default(); channel_builder diff --git a/crates/routes/src/images.rs b/crates/routes/src/images.rs index 472e4371d5..237ea8e144 100644 --- a/crates/routes/src/images.rs +++ b/crates/routes/src/images.rs @@ -11,9 +11,8 @@ use actix_web::{ HttpResponse, }; use futures::stream::{Stream, StreamExt}; -use lemmy_api_common::{claims::Claims, context::LemmyContext, utils::local_user_view_from_jwt}; +use lemmy_api_common::{context::LemmyContext, utils::local_user_view_from_jwt}; use lemmy_db_schema::{ - newtypes::LocalUserId, source::{ image_upload::{ImageUpload, ImageUploadForm}, local_site::LocalSite, diff --git a/src/session_middleware.rs b/src/session_middleware.rs index 845c834bea..d3bbf44a14 100644 --- a/src/session_middleware.rs +++ b/src/session_middleware.rs @@ -1,20 +1,14 @@ -use actix_web::{ - body::MessageBody, - cookie::SameSite, - dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform}, - http::header::CACHE_CONTROL, - Error, - HttpMessage, -}; +use actix_web::{body::MessageBody, dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform}, http::header::CACHE_CONTROL, Error, HttpMessage}; use core::future::Ready; use futures_util::future::LocalBoxFuture; use lemmy_api_common::{ context::LemmyContext, - utils::{local_user_view_from_jwt, AUTH_COOKIE_NAME}, + utils::{local_user_view_from_jwt}, }; -use lemmy_utils::error::{LemmyError, LemmyErrorType}; + use reqwest::header::HeaderValue; use std::{future::ready, rc::Rc}; +use lemmy_api::read_auth_token; #[derive(Clone)] pub struct SessionMiddleware { @@ -68,29 +62,7 @@ where let context = self.context.clone(); Box::pin(async move { - // Try reading jwt from auth header - let auth_header = req - .headers() - .get(AUTH_COOKIE_NAME) - .and_then(|h| h.to_str().ok()); - let jwt = if let Some(a) = auth_header { - Some(a.to_string()) - } - // If that fails, try auth cookie. Dont use the `jwt` cookie from lemmy-ui because - // its not http-only. - else { - let auth_cookie = req.cookie(AUTH_COOKIE_NAME); - if let Some(a) = &auth_cookie { - // ensure that its marked as httponly and secure - let secure = a.secure().unwrap_or_default(); - let http_only = a.http_only().unwrap_or_default(); - let same_site = a.same_site(); - if !secure || !http_only || same_site != Some(SameSite::Strict) { - return Err(LemmyError::from(LemmyErrorType::AuthCookieInsecure).into()); - } - } - auth_cookie.map(|c| c.value().to_string()) - }; + let jwt = read_auth_token(req.request())?; if let Some(jwt) = &jwt { // Ignore any invalid auth so the site can still be used From 71f0a599bd81815345a5615b010bb628997502ca Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 19 Sep 2023 13:36:58 +0200 Subject: [PATCH 03/15] fmt --- api_tests/src/user.spec.ts | 2 +- crates/api/src/lib.rs | 13 ++++--- crates/api/src/local_user/logout.rs | 2 +- crates/api/src/local_user/save_settings.rs | 9 +++-- crates/api_common/src/claims.rs | 42 ++++++++++++---------- crates/api_common/src/lib.rs | 10 +++--- crates/api_common/src/utils.rs | 1 - crates/api_crud/src/site/read.rs | 11 +++--- crates/apub/src/api/list_comments.rs | 2 +- crates/apub/src/api/list_posts.rs | 2 +- crates/apub/src/api/read_person.rs | 4 +-- crates/apub/src/api/resolve_object.rs | 2 +- crates/db_schema/src/source/local_user.rs | 1 - crates/routes/src/feeds.rs | 23 ++++-------- crates/routes/src/images.rs | 8 ++--- src/session_middleware.rs | 16 +++++---- 16 files changed, 70 insertions(+), 78 deletions(-) diff --git a/api_tests/src/user.spec.ts b/api_tests/src/user.spec.ts index 9aeb2263cb..9416c11133 100644 --- a/api_tests/src/user.spec.ts +++ b/api_tests/src/user.spec.ts @@ -133,4 +133,4 @@ test("Logout", async () => { // TODO: requires lemmy-js-client update user.client.login(); expect(false); -}); \ No newline at end of file +}); diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index a7d183a529..adca0de5e3 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -1,14 +1,13 @@ +use actix_web::{cookie::SameSite, HttpRequest}; use base64::{engine::general_purpose::STANDARD_NO_PAD as base64, Engine}; use captcha::Captcha; -use lemmy_api_common::utils::{AUTH_COOKIE_NAME, local_site_to_slur_regex}; +use lemmy_api_common::utils::{local_site_to_slur_regex, AUTH_COOKIE_NAME}; use lemmy_db_schema::source::local_site::LocalSite; use lemmy_utils::{ error::{LemmyError, LemmyErrorExt, LemmyErrorType}, utils::slurs::check_slurs, }; use std::io::Cursor; -use actix_web::cookie::SameSite; -use actix_web::HttpRequest; pub mod comment; pub mod comment_report; @@ -70,11 +69,11 @@ pub(crate) fn check_report_reason(reason: &str, local_site: &LocalSite) -> Resul } pub fn read_auth_token(req: &HttpRequest) -> Result, LemmyError> { -// Try reading jwt from auth header + // Try reading jwt from auth header let auth_header = req - .headers() - .get(AUTH_COOKIE_NAME) - .and_then(|h| h.to_str().ok()); + .headers() + .get(AUTH_COOKIE_NAME) + .and_then(|h| h.to_str().ok()); let jwt = if let Some(a) = auth_header { Some(a.to_string()) } diff --git a/crates/api/src/local_user/logout.rs b/crates/api/src/local_user/logout.rs index 9bb42e2798..2ba7c00cc7 100644 --- a/crates/api/src/local_user/logout.rs +++ b/crates/api/src/local_user/logout.rs @@ -1,10 +1,10 @@ +use crate::read_auth_token; use activitypub_federation::config::Data; use actix_web::{cookie::Cookie, HttpRequest, HttpResponse}; use lemmy_api_common::{context::LemmyContext, utils::AUTH_COOKIE_NAME}; use lemmy_db_schema::source::login_token::LoginToken; use lemmy_db_views::structs::LocalUserView; use lemmy_utils::error::LemmyResult; -use crate::read_auth_token; #[tracing::instrument(skip(context))] pub async fn logout( diff --git a/crates/api/src/local_user/save_settings.rs b/crates/api/src/local_user/save_settings.rs index 672437da45..3408c84775 100644 --- a/crates/api/src/local_user/save_settings.rs +++ b/crates/api/src/local_user/save_settings.rs @@ -1,5 +1,10 @@ use actix_web::web::{Data, Json}; -use lemmy_api_common::{context::LemmyContext, person::{SaveUserSettings}, SuccessResponse, utils::{local_user_view_from_jwt, sanitize_html_api_opt, send_verification_email}}; +use lemmy_api_common::{ + context::LemmyContext, + person::SaveUserSettings, + utils::{local_user_view_from_jwt, sanitize_html_api_opt, send_verification_email}, + SuccessResponse, +}; use lemmy_db_schema::{ source::{ actor_language::LocalUserLanguage, @@ -53,7 +58,7 @@ pub async fn save_user_settings( &mut context.pool(), context.settings(), ) - .await?; + .await?; } } diff --git a/crates/api_common/src/claims.rs b/crates/api_common/src/claims.rs index e1b3845fb5..e68c2b585f 100644 --- a/crates/api_common/src/claims.rs +++ b/crates/api_common/src/claims.rs @@ -62,7 +62,7 @@ mod tests { #![allow(clippy::unwrap_used)] #![allow(clippy::indexing_slicing)] - use reqwest::Client; + use crate::{claims::Claims, context::LemmyContext}; use lemmy_db_schema::{ source::{ instance::Instance, @@ -73,12 +73,10 @@ mod tests { traits::Crud, utils::build_db_pool_for_tests, }; - - use serial_test::serial; - use crate::claims::Claims; - use crate::context::LemmyContext; use lemmy_utils::rate_limit::{RateLimitCell, RateLimitConfig}; + use reqwest::Client; use reqwest_middleware::ClientBuilder; + use serial_test::serial; #[tokio::test] #[serial] @@ -86,31 +84,37 @@ mod tests { let pool_ = build_db_pool_for_tests().await; let pool = &mut (&pool_).into(); let secret = Secret::init(pool).await.unwrap(); - let context = LemmyContext::create(pool_.clone(), ClientBuilder::new(Client::default()).build(), secret, RateLimitCell::new(RateLimitConfig::builder().build()).await.clone()); + let context = LemmyContext::create( + pool_.clone(), + ClientBuilder::new(Client::default()).build(), + secret, + RateLimitCell::new(RateLimitConfig::builder().build()) + .await + .clone(), + ); let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string()) - .await - .unwrap(); + .await + .unwrap(); let new_person = PersonInsertForm::builder() - .name("Gerry9812".into()) - .public_key("pubkey".to_string()) - .instance_id(inserted_instance.id) - .build(); + .name("Gerry9812".into()) + .public_key("pubkey".to_string()) + .instance_id(inserted_instance.id) + .build(); let inserted_person = Person::create(pool, &new_person).await.unwrap(); let local_user_form = LocalUserInsertForm::builder() - .person_id(inserted_person.id) - .password_encrypted("123456".to_string()) - .build(); + .person_id(inserted_person.id) + .password_encrypted("123456".to_string()) + .build(); let inserted_local_user = LocalUser::create(pool, &local_user_form).await.unwrap(); - let jwt = Claims::generate( - inserted_local_user.id,&context - ).await - .unwrap(); + let jwt = Claims::generate(inserted_local_user.id, &context) + .await + .unwrap(); let valid = Claims::validate(&jwt, &context).await; assert!(valid.is_ok()); diff --git a/crates/api_common/src/lib.rs b/crates/api_common/src/lib.rs index 25abb1df62..9bea92dd5b 100644 --- a/crates/api_common/src/lib.rs +++ b/crates/api_common/src/lib.rs @@ -32,11 +32,11 @@ use ts_rs::TS; #[cfg_attr(feature = "full", ts(export))] /// Saves settings for your user. pub struct SuccessResponse { - pub success: bool, + pub success: bool, } impl SuccessResponse { - pub fn new() -> Self { - SuccessResponse { success: true } - } -} \ No newline at end of file + pub fn new() -> Self { + SuccessResponse { success: true } + } +} diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index 75e16fc6ec..9ec75c3ac6 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -836,4 +836,3 @@ mod tests { assert!(honeypot_check(&Some("message".to_string())).is_err()); } } - diff --git a/crates/api_crud/src/site/read.rs b/crates/api_crud/src/site/read.rs index 54d98747c7..09ed46cb86 100644 --- a/crates/api_crud/src/site/read.rs +++ b/crates/api_crud/src/site/read.rs @@ -3,12 +3,10 @@ use lemmy_api_common::{ context::LemmyContext, site::{GetSite, GetSiteResponse, MyUserInfo}, }; -use lemmy_db_schema::{ - source::{ - actor_language::{LocalUserLanguage, SiteLanguage}, - language::Language, - tagline::Tagline, - }, +use lemmy_db_schema::source::{ + actor_language::{LocalUserLanguage, SiteLanguage}, + language::Language, + tagline::Tagline, }; use lemmy_db_views::structs::{CustomEmojiView, LocalUserView, SiteView}; use lemmy_db_views_actor::structs::{ @@ -89,4 +87,3 @@ pub async fn get_site( custom_emojis, })) } - diff --git a/crates/apub/src/api/list_comments.rs b/crates/apub/src/api/list_comments.rs index 57f30c8b3e..7d1de019e6 100644 --- a/crates/apub/src/api/list_comments.rs +++ b/crates/apub/src/api/list_comments.rs @@ -8,7 +8,7 @@ use actix_web::web::{Json, Query}; use lemmy_api_common::{ comment::{GetComments, GetCommentsResponse}, context::LemmyContext, - utils::{check_private_instance}, + utils::check_private_instance, }; use lemmy_db_schema::{ source::{comment::Comment, community::Community, local_site::LocalSite}, diff --git a/crates/apub/src/api/list_posts.rs b/crates/apub/src/api/list_posts.rs index ee5b6ad903..dc3618c50f 100644 --- a/crates/apub/src/api/list_posts.rs +++ b/crates/apub/src/api/list_posts.rs @@ -8,7 +8,7 @@ use actix_web::web::{Json, Query}; use lemmy_api_common::{ context::LemmyContext, post::{GetPosts, GetPostsResponse}, - utils::{check_private_instance}, + utils::check_private_instance, }; use lemmy_db_schema::source::{community::Community, local_site::LocalSite}; use lemmy_db_views::{ diff --git a/crates/apub/src/api/read_person.rs b/crates/apub/src/api/read_person.rs index e270b08750..26ad287f14 100644 --- a/crates/apub/src/api/read_person.rs +++ b/crates/apub/src/api/read_person.rs @@ -4,7 +4,7 @@ use actix_web::web::{Json, Query}; use lemmy_api_common::{ context::LemmyContext, person::{GetPersonDetails, GetPersonDetailsResponse}, - utils::{check_private_instance}, + utils::check_private_instance, }; use lemmy_db_schema::{ source::{local_site::LocalSite, person::Person}, @@ -18,7 +18,7 @@ use lemmy_utils::error::{LemmyError, LemmyErrorExt2, LemmyErrorType}; pub async fn read_person( data: Query, context: Data, - local_user_view: Option, + local_user_view: Option, ) -> Result, LemmyError> { // Check to make sure a person name or an id is given if data.username.is_none() && data.person_id.is_none() { diff --git a/crates/apub/src/api/resolve_object.rs b/crates/apub/src/api/resolve_object.rs index d48fbc69d1..e081377f6f 100644 --- a/crates/apub/src/api/resolve_object.rs +++ b/crates/apub/src/api/resolve_object.rs @@ -9,7 +9,7 @@ use diesel::NotFound; use lemmy_api_common::{ context::LemmyContext, site::{ResolveObject, ResolveObjectResponse}, - utils::{check_private_instance}, + utils::check_private_instance, }; use lemmy_db_schema::{newtypes::PersonId, source::local_site::LocalSite, utils::DbPool}; use lemmy_db_views::structs::{CommentView, LocalUserView, PostView}; diff --git a/crates/db_schema/src/source/local_user.rs b/crates/db_schema/src/source/local_user.rs index 5c5a452207..40e892906e 100644 --- a/crates/db_schema/src/source/local_user.rs +++ b/crates/db_schema/src/source/local_user.rs @@ -6,7 +6,6 @@ use crate::{ PostListingMode, SortType, }; - use serde::{Deserialize, Serialize}; use serde_with::skip_serializing_none; #[cfg(feature = "full")] diff --git a/crates/routes/src/feeds.rs b/crates/routes/src/feeds.rs index ba075581c8..e79c7be069 100644 --- a/crates/routes/src/feeds.rs +++ b/crates/routes/src/feeds.rs @@ -1,10 +1,10 @@ use actix_web::{error::ErrorBadRequest, web, Error, HttpRequest, HttpResponse, Result}; use anyhow::anyhow; use chrono::{DateTime, Utc}; -use lemmy_api_common::context::LemmyContext; +use lemmy_api_common::{context::LemmyContext, utils::local_user_view_from_jwt}; use lemmy_db_schema::{ source::{community::Community, person::Person}, - traits::{ApubActor}, + traits::ApubActor, CommentSortType, ListingType, SortType, @@ -34,8 +34,6 @@ use rss::{ use serde::Deserialize; use std::{collections::BTreeMap, str::FromStr}; -use lemmy_api_common::utils::local_user_view_from_jwt; - const RSS_FETCH_LIMIT: i64 = 20; #[derive(Deserialize)] @@ -212,13 +210,7 @@ async fn get_feed( ) .await } - RequestType::Inbox => { - get_feed_inbox( - &context, - ¶m, - ) - .await - } + RequestType::Inbox => get_feed_inbox(&context, ¶m).await, } .map_err(ErrorBadRequest)?; @@ -324,7 +316,7 @@ async fn get_feed_front( .list(&mut context.pool()) .await?; - let protocol_and_hostname =context.settings().get_protocol_and_hostname(); + let protocol_and_hostname = context.settings().get_protocol_and_hostname(); let items = create_post_items(posts, &protocol_and_hostname)?; let mut channel_builder = ChannelBuilder::default(); @@ -342,10 +334,7 @@ async fn get_feed_front( } #[tracing::instrument(skip_all)] -async fn get_feed_inbox( - context: &LemmyContext, - jwt: &str, -) -> Result { +async fn get_feed_inbox(context: &LemmyContext, jwt: &str) -> Result { let site_view = SiteView::read_local(&mut context.pool()).await?; let local_user = local_user_view_from_jwt(jwt, context).await?; let person_id = local_user.local_user.person_id; @@ -372,7 +361,7 @@ async fn get_feed_inbox( limit: (Some(RSS_FETCH_LIMIT)), ..Default::default() } - .list(&mut context.pool()) + .list(&mut context.pool()) .await?; let protocol_and_hostname = context.settings().get_protocol_and_hostname(); diff --git a/crates/routes/src/images.rs b/crates/routes/src/images.rs index 237ea8e144..fa7311dcac 100644 --- a/crates/routes/src/images.rs +++ b/crates/routes/src/images.rs @@ -12,11 +12,9 @@ use actix_web::{ }; use futures::stream::{Stream, StreamExt}; use lemmy_api_common::{context::LemmyContext, utils::local_user_view_from_jwt}; -use lemmy_db_schema::{ - source::{ - image_upload::{ImageUpload, ImageUploadForm}, - local_site::LocalSite, - }, +use lemmy_db_schema::source::{ + image_upload::{ImageUpload, ImageUploadForm}, + local_site::LocalSite, }; use lemmy_db_views::structs::LocalUserView; use lemmy_utils::{rate_limit::RateLimitCell, REQWEST_TIMEOUT}; diff --git a/src/session_middleware.rs b/src/session_middleware.rs index d3bbf44a14..313a48c954 100644 --- a/src/session_middleware.rs +++ b/src/session_middleware.rs @@ -1,14 +1,16 @@ -use actix_web::{body::MessageBody, dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform}, http::header::CACHE_CONTROL, Error, HttpMessage}; +use actix_web::{ + body::MessageBody, + dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform}, + http::header::CACHE_CONTROL, + Error, + HttpMessage, +}; use core::future::Ready; use futures_util::future::LocalBoxFuture; -use lemmy_api_common::{ - context::LemmyContext, - utils::{local_user_view_from_jwt}, -}; - +use lemmy_api::read_auth_token; +use lemmy_api_common::{context::LemmyContext, utils::local_user_view_from_jwt}; use reqwest::header::HeaderValue; use std::{future::ready, rc::Rc}; -use lemmy_api::read_auth_token; #[derive(Clone)] pub struct SessionMiddleware { From 8bfc2020851ddad311e9d83fab4a7e69b24cfebe Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 19 Sep 2023 13:38:11 +0200 Subject: [PATCH 04/15] fmt 2 --- Cargo.toml | 2 +- crates/api_common/Cargo.toml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index bbe05f952c..64bc35d273 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -82,7 +82,7 @@ actix-web = { version = "4.3.1", default-features = false, features = [ "compress-brotli", "compress-gzip", "compress-zstd", - "cookies" + "cookies", ] } tracing = "0.1.37" tracing-actix-web = { version = "0.7.5", default-features = false } diff --git a/crates/api_common/Cargo.toml b/crates/api_common/Cargo.toml index 24c24dffe3..5325350c8c 100644 --- a/crates/api_common/Cargo.toml +++ b/crates/api_common/Cargo.toml @@ -34,7 +34,7 @@ full = [ "actix-web", "futures", "once_cell", - "jsonwebtoken" + "jsonwebtoken", ] [dependencies] @@ -71,4 +71,4 @@ getrandom = { version = "0.2.10", features = ["js"] } [dev-dependencies] serial_test = { workspace = true } -reqwest-middleware = { workspace = true } \ No newline at end of file +reqwest-middleware = { workspace = true } From ea42469e1ebea5a0dbcfb57ff90c99a49a5605e4 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 19 Sep 2023 15:56:57 +0200 Subject: [PATCH 05/15] fmt 3 --- migrations/2023-09-18-141700_login-token/down.sql | 1 + migrations/2023-09-18-141700_login-token/up.sql | 1 + 2 files changed, 2 insertions(+) diff --git a/migrations/2023-09-18-141700_login-token/down.sql b/migrations/2023-09-18-141700_login-token/down.sql index 193485e2b9..826e05e708 100644 --- a/migrations/2023-09-18-141700_login-token/down.sql +++ b/migrations/2023-09-18-141700_login-token/down.sql @@ -2,3 +2,4 @@ DROP TABLE login_token; ALTER TABLE local_user ADD COLUMN validator_time timestamp NOT NULL DEFAULT now(); + diff --git a/migrations/2023-09-18-141700_login-token/up.sql b/migrations/2023-09-18-141700_login-token/up.sql index a2ce39a0e1..4289a82eed 100644 --- a/migrations/2023-09-18-141700_login-token/up.sql +++ b/migrations/2023-09-18-141700_login-token/up.sql @@ -7,3 +7,4 @@ CREATE TABLE login_token ( -- not needed anymore as we invalidate login tokens on password change ALTER TABLE local_user DROP COLUMN validator_time; + From 37c2803d75c693ab001fa7906b375fe63f025fbd Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 19 Sep 2023 16:12:49 +0200 Subject: [PATCH 06/15] fix default feature --- crates/api_common/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/api_common/src/lib.rs b/crates/api_common/src/lib.rs index 9bea92dd5b..ad52ac1ea7 100644 --- a/crates/api_common/src/lib.rs +++ b/crates/api_common/src/lib.rs @@ -25,10 +25,9 @@ pub extern crate lemmy_db_views_actor; pub extern crate lemmy_db_views_moderator; use serde::{Deserialize, Serialize}; -use ts_rs::TS; #[derive(Debug, Serialize, Deserialize, Clone, Default)] -#[cfg_attr(feature = "full", derive(TS))] +#[cfg_attr(feature = "full", derive(ts_rs::TS))] #[cfg_attr(feature = "full", ts(export))] /// Saves settings for your user. pub struct SuccessResponse { From 4381bb8b88f2a428dd2ab7fb644948f307d112cf Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Thu, 21 Sep 2023 15:31:47 +0200 Subject: [PATCH 07/15] use Authorization header --- crates/api/src/lib.rs | 14 +++++++++++--- src/session_middleware.rs | 1 - 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index 6ea3dae0fb..19e984e712 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -1,4 +1,4 @@ -use actix_web::{cookie::SameSite, HttpRequest}; +use actix_web::{cookie::SameSite, http::header::AUTHORIZATION, HttpRequest}; use base64::{engine::general_purpose::STANDARD_NO_PAD as base64, Engine}; use captcha::Captcha; use lemmy_api_common::utils::{local_site_to_slur_regex, AUTH_COOKIE_NAME}; @@ -74,10 +74,18 @@ pub fn read_auth_token(req: &HttpRequest) -> Result, LemmyError> // Try reading jwt from auth header let auth_header = req .headers() - .get(AUTH_COOKIE_NAME) + .get(AUTHORIZATION) .and_then(|h| h.to_str().ok()); let jwt = if let Some(a) = auth_header { - Some(a.to_string()) + // Looks like `Bearer `, we only need the second part + // https://swagger.io/docs/specification/authentication/bearer-authentication/ + Some( + a.split(" ") + .collect::>() + .get(1) + .expect("authorization header includes token") + .to_string(), + ) } // If that fails, try auth cookie. Dont use the `jwt` cookie from lemmy-ui because // its not http-only. diff --git a/src/session_middleware.rs b/src/session_middleware.rs index 12958376f8..adfd980d81 100644 --- a/src/session_middleware.rs +++ b/src/session_middleware.rs @@ -5,7 +5,6 @@ use actix_web::{ Error, HttpMessage, }; -use chrono::{DateTime, Utc}; use core::future::Ready; use futures_util::future::LocalBoxFuture; use lemmy_api::read_auth_token; From 53a11779727cef47b688fdbbbc76ced58d6ffdd5 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Fri, 22 Sep 2023 11:27:49 +0200 Subject: [PATCH 08/15] store ip and user agent for each login --- crates/api/src/lib.rs | 7 ++++--- crates/api/src/local_user/change_password.rs | 8 ++++++-- crates/api/src/local_user/login.rs | 4 +++- crates/api_common/src/claims.rs | 14 ++++++++++++++ crates/api_crud/src/user/create.rs | 5 +++-- crates/db_schema/src/schema.rs | 3 +++ crates/db_schema/src/source/login_token.rs | 6 ++++++ migrations/2023-09-18-141700_login-token/up.sql | 5 ++++- 8 files changed, 43 insertions(+), 9 deletions(-) diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index 19e984e712..5bae0d3708 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -80,11 +80,12 @@ pub fn read_auth_token(req: &HttpRequest) -> Result, LemmyError> // Looks like `Bearer `, we only need the second part // https://swagger.io/docs/specification/authentication/bearer-authentication/ Some( - a.split(" ") + (*a + .split(' ') .collect::>() .get(1) - .expect("authorization header includes token") - .to_string(), + .expect("authorization header includes token")) + .to_string(), ) } // If that fails, try auth cookie. Dont use the `jwt` cookie from lemmy-ui because diff --git a/crates/api/src/local_user/change_password.rs b/crates/api/src/local_user/change_password.rs index 5612a72e4f..ab5b32dd95 100644 --- a/crates/api/src/local_user/change_password.rs +++ b/crates/api/src/local_user/change_password.rs @@ -1,4 +1,7 @@ -use actix_web::web::{Data, Json}; +use actix_web::{ + web::{Data, Json}, + HttpRequest, +}; use bcrypt::verify; use lemmy_api_common::{ claims::Claims, @@ -13,6 +16,7 @@ use lemmy_utils::error::{LemmyError, LemmyErrorType}; #[tracing::instrument(skip(context))] pub async fn change_password( data: Json, + req: HttpRequest, context: Data, local_user_view: LocalUserView, ) -> Result, LemmyError> { @@ -42,7 +46,7 @@ pub async fn change_password( // Return the jwt Ok(Json(LoginResponse { - jwt: Some(Claims::generate(updated_local_user.id, &context).await?), + jwt: Some(Claims::generate(updated_local_user.id, req, &context).await?), verify_email_sent: false, registration_created: false, })) diff --git a/crates/api/src/local_user/login.rs b/crates/api/src/local_user/login.rs index 5c5e3ee463..f5764be13b 100644 --- a/crates/api/src/local_user/login.rs +++ b/crates/api/src/local_user/login.rs @@ -1,6 +1,7 @@ use crate::check_totp_2fa_valid; use actix_web::{ web::{Data, Json}, + HttpRequest, HttpResponse, }; use bcrypt::verify; @@ -22,6 +23,7 @@ use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType}; #[tracing::instrument(skip(context))] pub async fn login( data: Json, + req: HttpRequest, context: Data, ) -> Result { let site_view = SiteView::read_local(&mut context.pool()).await?; @@ -65,7 +67,7 @@ pub async fn login( check_totp_2fa_valid(&local_user_view, &data.totp_2fa_token, &site_view.site.name)?; } - let jwt = Claims::generate(local_user_view.local_user.id, &context).await?; + let jwt = Claims::generate(local_user_view.local_user.id, req, &context).await?; let json = LoginResponse { jwt: Some(jwt.clone()), diff --git a/crates/api_common/src/claims.rs b/crates/api_common/src/claims.rs index e68c2b585f..814e1bdd23 100644 --- a/crates/api_common/src/claims.rs +++ b/crates/api_common/src/claims.rs @@ -1,4 +1,5 @@ use crate::{context::LemmyContext, sensitive::Sensitive}; +use actix_web::{http::header::USER_AGENT, HttpRequest}; use chrono::Utc; use jsonwebtoken::{decode, encode, DecodingKey, EncodingKey, Header, Validation}; use lemmy_db_schema::{ @@ -36,6 +37,7 @@ impl Claims { pub async fn generate( user_id: LocalUserId, + req: HttpRequest, context: &LemmyContext, ) -> LemmyResult> { let hostname = context.settings().hostname.clone(); @@ -48,9 +50,21 @@ impl Claims { let secret = &context.secret().jwt_secret; let key = EncodingKey::from_secret(secret.as_ref()); let token = encode(&Header::default(), &my_claims, &key)?; + let ip = req + .connection_info() + .realip_remote_addr() + .map(ToString::to_string); + let user_agent = req + .headers() + .get(USER_AGENT) + .map(|ua| ua.to_str().ok()) + .flatten() + .map(ToString::to_string); let form = LoginTokenCreateForm { token: token.clone(), user_id, + ip, + user_agent, }; LoginToken::create(&mut context.pool(), form).await?; Ok(Sensitive::new(token)) diff --git a/crates/api_crud/src/user/create.rs b/crates/api_crud/src/user/create.rs index 13da2b01c4..18b9b9d2c6 100644 --- a/crates/api_crud/src/user/create.rs +++ b/crates/api_crud/src/user/create.rs @@ -1,5 +1,5 @@ use activitypub_federation::{config::Data, http_signatures::generate_actor_keypair}; -use actix_web::{web::Json, HttpResponse}; +use actix_web::{web::Json, HttpRequest, HttpResponse}; use lemmy_api_common::{ claims::Claims, context::LemmyContext, @@ -42,6 +42,7 @@ use lemmy_utils::{ #[tracing::instrument(skip(context))] pub async fn register( data: Json, + req: HttpRequest, context: Data, ) -> Result { let site_view = SiteView::read_local(&mut context.pool()).await?; @@ -176,7 +177,7 @@ pub async fn register( if !local_site.site_setup || (!require_registration_application && !local_site.require_email_verification) { - let jwt = Claims::generate(inserted_local_user.id, &context).await?; + let jwt = Claims::generate(inserted_local_user.id, req, &context).await?; res .cookie(create_login_cookie(jwt.clone())) .await diff --git a/crates/db_schema/src/schema.rs b/crates/db_schema/src/schema.rs index 7122e668b6..0d9f08b82b 100644 --- a/crates/db_schema/src/schema.rs +++ b/crates/db_schema/src/schema.rs @@ -458,6 +458,9 @@ diesel::table! { id -> Int4, token -> Text, user_id -> Int4, + published -> Timestamptz, + ip -> Nullable, + user_agent -> Nullable, } } diff --git a/crates/db_schema/src/source/login_token.rs b/crates/db_schema/src/source/login_token.rs index 040e206906..0051118f4d 100644 --- a/crates/db_schema/src/source/login_token.rs +++ b/crates/db_schema/src/source/login_token.rs @@ -1,6 +1,7 @@ use crate::newtypes::LocalUserId; #[cfg(feature = "full")] use crate::schema::login_token; +use chrono::{DateTime, Utc}; #[derive(Clone, PartialEq, Eq, Debug)] #[cfg_attr(feature = "full", derive(Queryable, Identifiable))] @@ -9,6 +10,9 @@ pub struct LoginToken { pub id: i32, pub token: String, pub user_id: LocalUserId, + pub published: DateTime, + pub ip: Option, + pub user_agent: Option, } #[cfg_attr(feature = "full", derive(Insertable, AsChangeset))] @@ -16,4 +20,6 @@ pub struct LoginToken { pub struct LoginTokenCreateForm { pub token: String, pub user_id: LocalUserId, + pub ip: Option, + pub user_agent: Option, } diff --git a/migrations/2023-09-18-141700_login-token/up.sql b/migrations/2023-09-18-141700_login-token/up.sql index 4289a82eed..5159ae4f2b 100644 --- a/migrations/2023-09-18-141700_login-token/up.sql +++ b/migrations/2023-09-18-141700_login-token/up.sql @@ -1,7 +1,10 @@ CREATE TABLE login_token ( id serial PRIMARY KEY, token text NOT NULL UNIQUE, - user_id int REFERENCES local_user ON UPDATE CASCADE ON DELETE CASCADE NOT NULL + user_id int REFERENCES local_user ON UPDATE CASCADE ON DELETE CASCADE NOT NULL, + published timestamptz NOT NULL DEFAULT now(), + ip text, + user_agent text ); -- not needed anymore as we invalidate login tokens on password change From 053dff9387280487ce9b54bcbe7f3331bcd7c1f9 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Fri, 22 Sep 2023 11:49:11 +0200 Subject: [PATCH 09/15] add list_logins endpoint --- crates/api/src/local_user/list_logins.rs | 14 ++++++++++++++ crates/api/src/local_user/mod.rs | 1 + crates/api_common/src/claims.rs | 7 ++++--- crates/db_schema/src/impls/login_token.rs | 12 ++++++++++++ crates/db_schema/src/source/login_token.rs | 3 ++- src/api_routes_http.rs | 4 +++- src/session_middleware.rs | 4 +++- 7 files changed, 39 insertions(+), 6 deletions(-) create mode 100644 crates/api/src/local_user/list_logins.rs diff --git a/crates/api/src/local_user/list_logins.rs b/crates/api/src/local_user/list_logins.rs new file mode 100644 index 0000000000..f1ae76be58 --- /dev/null +++ b/crates/api/src/local_user/list_logins.rs @@ -0,0 +1,14 @@ +use actix_web::web::{Data, Json}; +use lemmy_api_common::context::LemmyContext; +use lemmy_db_schema::source::login_token::LoginToken; +use lemmy_db_views::structs::LocalUserView; +use lemmy_utils::error::LemmyError; + +pub async fn list_logins( + context: Data, + local_user_view: LocalUserView, +) -> Result>, LemmyError> { + let logins = LoginToken::list(&mut context.pool(), local_user_view.local_user.id).await?; + + Ok(Json(logins)) +} diff --git a/crates/api/src/local_user/mod.rs b/crates/api/src/local_user/mod.rs index e97adc1954..1b58713f15 100644 --- a/crates/api/src/local_user/mod.rs +++ b/crates/api/src/local_user/mod.rs @@ -6,6 +6,7 @@ pub mod change_password_after_reset; pub mod generate_totp_secret; pub mod get_captcha; pub mod list_banned; +pub mod list_logins; pub mod login; pub mod logout; pub mod notifications; diff --git a/crates/api_common/src/claims.rs b/crates/api_common/src/claims.rs index 814e1bdd23..3e4d8bf230 100644 --- a/crates/api_common/src/claims.rs +++ b/crates/api_common/src/claims.rs @@ -57,8 +57,7 @@ impl Claims { let user_agent = req .headers() .get(USER_AGENT) - .map(|ua| ua.to_str().ok()) - .flatten() + .and_then(|ua| ua.to_str().ok()) .map(ToString::to_string); let form = LoginTokenCreateForm { token: token.clone(), @@ -77,6 +76,7 @@ mod tests { #![allow(clippy::indexing_slicing)] use crate::{claims::Claims, context::LemmyContext}; + use actix_web::test::TestRequest; use lemmy_db_schema::{ source::{ instance::Instance, @@ -126,7 +126,8 @@ mod tests { let inserted_local_user = LocalUser::create(pool, &local_user_form).await.unwrap(); - let jwt = Claims::generate(inserted_local_user.id, &context) + let req = TestRequest::default().to_http_request(); + let jwt = Claims::generate(inserted_local_user.id, req, &context) .await .unwrap(); diff --git a/crates/db_schema/src/impls/login_token.rs b/crates/db_schema/src/impls/login_token.rs index b13c6f3d81..b1d1124d68 100644 --- a/crates/db_schema/src/impls/login_token.rs +++ b/crates/db_schema/src/impls/login_token.rs @@ -33,6 +33,18 @@ impl LoginToken { .await } + pub async fn list( + pool: &mut DbPool<'_>, + user_id_: LocalUserId, + ) -> Result, Error> { + let conn = &mut get_conn(pool).await?; + + login_token + .filter(user_id.eq(user_id_)) + .get_results(conn) + .await + } + /// Invalidate specific token on user logout. pub async fn invalidate(pool: &mut DbPool<'_>, token_: &str) -> Result { let conn = &mut get_conn(pool).await?; diff --git a/crates/db_schema/src/source/login_token.rs b/crates/db_schema/src/source/login_token.rs index 0051118f4d..435311d819 100644 --- a/crates/db_schema/src/source/login_token.rs +++ b/crates/db_schema/src/source/login_token.rs @@ -2,8 +2,9 @@ use crate::newtypes::LocalUserId; #[cfg(feature = "full")] use crate::schema::login_token; use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; -#[derive(Clone, PartialEq, Eq, Debug)] +#[derive(Clone, PartialEq, Eq, Debug, Serialize, Deserialize)] #[cfg_attr(feature = "full", derive(Queryable, Identifiable))] #[cfg_attr(feature = "full", diesel(table_name = login_token))] pub struct LoginToken { diff --git a/src/api_routes_http.rs b/src/api_routes_http.rs index 9124204052..173bce1992 100644 --- a/src/api_routes_http.rs +++ b/src/api_routes_http.rs @@ -23,6 +23,7 @@ use lemmy_api::{ generate_totp_secret::generate_totp_secret, get_captcha::get_captcha, list_banned::list_banned_users, + list_logins::list_logins, login::login, logout::logout, notifications::{ @@ -293,7 +294,8 @@ pub fn config(cfg: &mut web::ServiceConfig, rate_limit: &RateLimitCell) { .route("/verify_email", web::post().to(verify_email)) .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)), + .route("/totp/update", web::post().to(update_totp)) + .route("/list_logins", web::get().to(list_logins)), ) // Admin Actions .service( diff --git a/src/session_middleware.rs b/src/session_middleware.rs index adfd980d81..1d19cec9c0 100644 --- a/src/session_middleware.rs +++ b/src/session_middleware.rs @@ -124,6 +124,7 @@ mod tests { #![allow(clippy::indexing_slicing)] use super::*; + use actix_web::test::TestRequest; use lemmy_db_schema::{ source::{ instance::Instance, @@ -177,7 +178,8 @@ mod tests { let inserted_local_user = LocalUser::create(pool, &local_user_form).await.unwrap(); - let jwt = Claims::generate(inserted_local_user.id, &context) + let req = TestRequest::default().to_http_request(); + let jwt = Claims::generate(inserted_local_user.id, req, &context) .await .unwrap(); From 3763978ea67df7f4d266e55f37ee34c79e8e4380 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Fri, 22 Sep 2023 14:48:22 +0200 Subject: [PATCH 10/15] serde(skip) for token --- crates/db_schema/src/source/login_token.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/db_schema/src/source/login_token.rs b/crates/db_schema/src/source/login_token.rs index 435311d819..17bd5a43ac 100644 --- a/crates/db_schema/src/source/login_token.rs +++ b/crates/db_schema/src/source/login_token.rs @@ -9,6 +9,7 @@ use serde::{Deserialize, Serialize}; #[cfg_attr(feature = "full", diesel(table_name = login_token))] pub struct LoginToken { pub id: i32, + #[serde(skip)] pub token: String, pub user_id: LocalUserId, pub published: DateTime, From b2bacd1a30f92ea3456a2af08d872bb991e9dfa7 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Fri, 22 Sep 2023 16:17:07 +0200 Subject: [PATCH 11/15] fix api tests --- api_tests/src/comment.spec.ts | 2 +- api_tests/src/community.spec.ts | 2 +- api_tests/src/post.spec.ts | 19 +++++++++++++------ api_tests/src/shared.ts | 21 ++++++++++++++++----- api_tests/src/user.spec.ts | 19 +++---------------- crates/api/src/local_user/login.rs | 3 ++- crates/api_crud/src/user/create.rs | 9 +++------ 7 files changed, 39 insertions(+), 36 deletions(-) diff --git a/api_tests/src/comment.spec.ts b/api_tests/src/comment.spec.ts index 6ced2bf330..006a094b86 100644 --- a/api_tests/src/comment.spec.ts +++ b/api_tests/src/comment.spec.ts @@ -229,7 +229,7 @@ test.skip("Remove a comment from admin and community on the same instance", asyn test("Remove a comment from admin and community on different instance", async () => { let alpha_user = await registerUser(alpha); let newAlphaApi = new LemmyHttp(alphaUrl, { - headers: { auth: alpha_user.jwt ?? "" }, + headers: { Authorization: "Bearer " + alpha_user.jwt ?? "" }, }); // New alpha user creates a community, post, and comment. diff --git a/api_tests/src/community.spec.ts b/api_tests/src/community.spec.ts index b81dd900c5..312f61ff93 100644 --- a/api_tests/src/community.spec.ts +++ b/api_tests/src/community.spec.ts @@ -252,7 +252,7 @@ test("moderator view", async () => { // register a new user with their own community on alpha and post to it let registerUserRes = await registerUser(alpha); let otherUser = new LemmyHttp(alphaUrl, { - headers: { auth: registerUserRes.jwt ?? "" }, + headers: { Authorization: "Bearer " + registerUserRes.jwt ?? "" }, }); let otherCommunity = (await createCommunity(otherUser)).community_view; diff --git a/api_tests/src/post.spec.ts b/api_tests/src/post.spec.ts index 51a10293be..17fc435ed8 100644 --- a/api_tests/src/post.spec.ts +++ b/api_tests/src/post.spec.ts @@ -36,10 +36,11 @@ import { waitUntil, waitForPost, alphaUrl, + loginUser, } from "./shared"; import { PostView } from "lemmy-js-client/dist/types/PostView"; import { CreatePost } from "lemmy-js-client/dist/types/CreatePost"; -import { LemmyHttp } from "lemmy-js-client"; +import { LemmyHttp, Login } from "lemmy-js-client"; let betaCommunity: CommunityView | undefined; @@ -381,11 +382,12 @@ test("Enforce site ban for federated user", async () => { // create a test user let alphaUserJwt = await registerUser(alpha); expect(alphaUserJwt).toBeDefined(); - let alpha_user = new LemmyHttp(alphaUrl, { - headers: { auth: alphaUserJwt.jwt ?? "" }, + var alpha_user = new LemmyHttp(alphaUrl, { + headers: { Authorization: "Bearer " + alphaUserJwt.jwt ?? "" }, }); - let alphaUserActorId = (await getSite(alpha_user)).my_user?.local_user_view - .person.actor_id; + let alphaUserPerson = (await getSite(alpha_user)).my_user?.local_user_view + .person; + let alphaUserActorId = alphaUserPerson?.actor_id; if (!alphaUserActorId) { throw "Missing alpha user actor id"; } @@ -431,8 +433,13 @@ test("Enforce site ban for federated user", async () => { ); expect(unBanAlpha.banned).toBe(false); + // Login gets invalidated by ban, need to login again + let newAlphaUserJwt = await loginUser(alpha, alphaUserPerson?.name!); + alpha_user.setHeaders({ + Authorization: "Bearer " + newAlphaUserJwt.jwt ?? "", + }); // alpha makes new post in beta community, it federates - let postRes2 = await createPost(alpha_user, betaCommunity.community.id); + let postRes2 = await createPost(alpha_user, betaCommunity!.community.id); let searchBeta3 = await waitForPost(beta, postRes2.post_view.post); let alphaUserOnBeta2 = await resolvePerson(beta, alphaUserActorId!); diff --git a/api_tests/src/shared.ts b/api_tests/src/shared.ts index a1868f8f2c..8b6379bcbb 100644 --- a/api_tests/src/shared.ts +++ b/api_tests/src/shared.ts @@ -124,11 +124,11 @@ export async function setupLogins() { resDelta, resEpsilon, ]); - alpha.setHeaders({ auth: res[0].jwt ?? "" }); - beta.setHeaders({ auth: res[1].jwt ?? "" }); - gamma.setHeaders({ auth: res[2].jwt ?? "" }); - delta.setHeaders({ auth: res[3].jwt ?? "" }); - epsilon.setHeaders({ auth: res[4].jwt ?? "" }); + alpha.setHeaders({ Authorization: "Bearer " + res[0].jwt ?? "" }); + beta.setHeaders({ Authorization: "Bearer " + res[1].jwt ?? "" }); + gamma.setHeaders({ Authorization: "Bearer " + res[2].jwt ?? "" }); + delta.setHeaders({ Authorization: "Bearer " + res[3].jwt ?? "" }); + epsilon.setHeaders({ Authorization: "Bearer " + res[4].jwt ?? "" }); // Registration applications are now enabled by default, need to disable them let editSiteForm: EditSite = { @@ -619,6 +619,17 @@ export async function registerUser( return api.register(form); } +export async function loginUser( + api: LemmyHttp, + username: string, +): Promise { + let form: Login = { + username_or_email: username, + password: password, + }; + return api.login(form); +} + export async function saveUserSettingsBio( api: LemmyHttp, ): Promise { diff --git a/api_tests/src/user.spec.ts b/api_tests/src/user.spec.ts index e1fc421b55..a59afcd5ee 100644 --- a/api_tests/src/user.spec.ts +++ b/api_tests/src/user.spec.ts @@ -41,7 +41,7 @@ test("Create user", async () => { let userRes = await registerUser(alpha); expect(userRes.jwt).toBeDefined(); let user = new LemmyHttp(alphaUrl, { - headers: { auth: userRes.jwt ?? "" }, + headers: { Authorization: "Bearer " + userRes.jwt ?? "" }, }); let site = await getSite(user); @@ -63,7 +63,7 @@ test("Delete user", async () => { let userRes = await registerUser(alpha); expect(userRes.jwt).toBeDefined(); let user = new LemmyHttp(alphaUrl, { - headers: { auth: userRes.jwt ?? "" }, + headers: { Authorization: "Bearer " + userRes.jwt ?? "" }, }); // make a local post and comment @@ -109,7 +109,7 @@ test("Delete user", async () => { test("Requests with invalid auth should be treated as unauthenticated", async () => { let invalid_auth = new LemmyHttp(alphaUrl, { - headers: { auth: "" }, + headers: { Authorization: "Bearer asd" }, }); let site = await getSite(invalid_auth); expect(site.my_user).toBeUndefined(); @@ -119,16 +119,3 @@ test("Requests with invalid auth should be treated as unauthenticated", async () let posts = invalid_auth.getPosts(form); expect((await posts).posts).toBeDefined(); }); - -test("Logout", async () => { - let userRes = await registerUser(alpha); - expect(userRes.jwt).toBeDefined(); - let user: API = { - client: alpha.client, - auth: userRes.jwt ?? "", - }; - - // TODO: requires lemmy-js-client update - user.client.login(); - expect(false); -}); diff --git a/crates/api/src/local_user/login.rs b/crates/api/src/local_user/login.rs index f5764be13b..903c39370c 100644 --- a/crates/api/src/local_user/login.rs +++ b/crates/api/src/local_user/login.rs @@ -1,5 +1,6 @@ use crate::check_totp_2fa_valid; use actix_web::{ + http::StatusCode, web::{Data, Json}, HttpRequest, HttpResponse, @@ -75,7 +76,7 @@ pub async fn login( registration_created: false, }; - let mut res = HttpResponse::Ok().json(json); + let mut res = HttpResponse::build(StatusCode::OK).json(json); res.add_cookie(&create_login_cookie(jwt))?; Ok(res) } diff --git a/crates/api_crud/src/user/create.rs b/crates/api_crud/src/user/create.rs index 18b9b9d2c6..ee0b714c09 100644 --- a/crates/api_crud/src/user/create.rs +++ b/crates/api_crud/src/user/create.rs @@ -1,5 +1,5 @@ use activitypub_federation::{config::Data, http_signatures::generate_actor_keypair}; -use actix_web::{web::Json, HttpRequest, HttpResponse}; +use actix_web::{http::StatusCode, web::Json, HttpRequest, HttpResponse, HttpResponseBuilder}; use lemmy_api_common::{ claims::Claims, context::LemmyContext, @@ -166,7 +166,7 @@ pub async fn register( .await?; } - let mut res = HttpResponse::Ok(); + let mut res = HttpResponseBuilder::new(StatusCode::OK); let mut login_response = LoginResponse { jwt: None, registration_created: false, @@ -178,10 +178,7 @@ pub async fn register( || (!require_registration_application && !local_site.require_email_verification) { let jwt = Claims::generate(inserted_local_user.id, req, &context).await?; - res - .cookie(create_login_cookie(jwt.clone())) - .await - .expect("set auth cookie"); + res.cookie(create_login_cookie(jwt.clone())); login_response.jwt = Some(jwt); } else { if local_site.require_email_verification { From 45570a600ea50a4a132ac0bf6695af63654ad6b3 Mon Sep 17 00:00:00 2001 From: Dessalines Date: Thu, 28 Sep 2023 08:37:25 -0400 Subject: [PATCH 12/15] A few suggestions for login_token (#3991) * A few suggestions. * Fixing SQL format. --- crates/api/src/lib.rs | 32 +++++++++---------- crates/api/src/local_user/logout.rs | 4 +-- crates/api_common/src/claims.rs | 5 +-- crates/db_schema/src/impls/local_user.rs | 14 +++++--- crates/db_schema/src/impls/person.rs | 2 +- .../2023-09-18-141700_login-token/up.sql | 4 +++ 6 files changed, 36 insertions(+), 25 deletions(-) diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index 3488559f0a..328ee1c975 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -73,26 +73,26 @@ pub(crate) fn check_report_reason(reason: &str, local_site: &LocalSite) -> Resul pub fn read_auth_token(req: &HttpRequest) -> Result, LemmyError> { // Try reading jwt from auth header - let auth_header = Authorization::::parse(req).ok(); - let jwt = if let Some(a) = auth_header { - Some(a.as_ref().token().to_string()) + if let Ok(header) = Authorization::::parse(req) { + Ok(Some(header.as_ref().token().to_string())) } // If that fails, try auth cookie. Dont use the `jwt` cookie from lemmy-ui because // its not http-only. - else { - let auth_cookie = req.cookie(AUTH_COOKIE_NAME); - if let Some(a) = &auth_cookie { - // ensure that its marked as httponly and secure - let secure = a.secure().unwrap_or_default(); - let http_only = a.http_only().unwrap_or_default(); - let same_site = a.same_site(); - if !secure || !http_only || same_site != Some(SameSite::Strict) { - return Err(LemmyError::from(LemmyErrorType::AuthCookieInsecure)); - } + else if let Some(cookie) = &req.cookie(AUTH_COOKIE_NAME) { + // ensure that its marked as httponly and secure + let secure = cookie.secure().unwrap_or_default(); + let http_only = cookie.http_only().unwrap_or_default(); + let same_site = cookie.same_site(); + if !secure || !http_only || same_site != Some(SameSite::Strict) { + Err(LemmyError::from(LemmyErrorType::AuthCookieInsecure)) + } else { + Ok(Some(cookie.value().to_string())) } - auth_cookie.map(|c| c.value().to_string()) - }; - Ok(jwt) + } + // Otherwise, there's no auth + else { + Ok(None) + } } pub(crate) fn check_totp_2fa_valid( diff --git a/crates/api/src/local_user/logout.rs b/crates/api/src/local_user/logout.rs index 2ba7c00cc7..a2cc83b3f0 100644 --- a/crates/api/src/local_user/logout.rs +++ b/crates/api/src/local_user/logout.rs @@ -4,7 +4,7 @@ use actix_web::{cookie::Cookie, HttpRequest, HttpResponse}; use lemmy_api_common::{context::LemmyContext, utils::AUTH_COOKIE_NAME}; use lemmy_db_schema::source::login_token::LoginToken; use lemmy_db_views::structs::LocalUserView; -use lemmy_utils::error::LemmyResult; +use lemmy_utils::error::{LemmyErrorType, LemmyResult}; #[tracing::instrument(skip(context))] pub async fn logout( @@ -13,7 +13,7 @@ pub async fn logout( _local_user_view: LocalUserView, context: Data, ) -> LemmyResult { - let jwt = read_auth_token(&req)?.expect("user is logged in"); + let jwt = read_auth_token(&req)?.ok_or(LemmyErrorType::NotLoggedIn)?; LoginToken::invalidate(&mut context.pool(), &jwt).await?; let mut res = HttpResponse::Ok().finish(); diff --git a/crates/api_common/src/claims.rs b/crates/api_common/src/claims.rs index 3e4d8bf230..6676840dc8 100644 --- a/crates/api_common/src/claims.rs +++ b/crates/api_common/src/claims.rs @@ -30,9 +30,10 @@ impl Claims { let user_id = LocalUserId(claims.claims.sub.parse()?); let is_valid = LoginToken::validate(&mut context.pool(), user_id, jwt).await?; if !is_valid { - return Err(LemmyErrorType::NotLoggedIn)?; + Err(LemmyErrorType::NotLoggedIn)? + } else { + Ok(user_id) } - Ok(user_id) } pub async fn generate( diff --git a/crates/db_schema/src/impls/local_user.rs b/crates/db_schema/src/impls/local_user.rs index eccbd578c2..3206322e4b 100644 --- a/crates/db_schema/src/impls/local_user.rs +++ b/crates/db_schema/src/impls/local_user.rs @@ -12,7 +12,11 @@ use crate::{ local_user::{LocalUser, LocalUserInsertForm, LocalUserUpdateForm}, }, traits::Crud, - utils::{get_conn, DbPool}, + utils::{ + functions::{coalesce, lower}, + get_conn, + DbPool, + }, }; use bcrypt::{hash, DEFAULT_COST}; use diesel::{dsl::insert_into, result::Error, ExpressionMethods, QueryDsl}; @@ -54,9 +58,11 @@ impl LocalUser { pub async fn is_email_taken(pool: &mut DbPool<'_>, email_: &str) -> Result { use diesel::dsl::{exists, select}; let conn = &mut get_conn(pool).await?; - select(exists(local_user.filter(email.eq(email_)))) - .get_result(conn) - .await + select(exists( + local_user.filter(lower(coalesce(email, "")).eq(email_.to_lowercase())), + )) + .get_result(conn) + .await } } diff --git a/crates/db_schema/src/impls/person.rs b/crates/db_schema/src/impls/person.rs index 9116b51388..1a29744395 100644 --- a/crates/db_schema/src/impls/person.rs +++ b/crates/db_schema/src/impls/person.rs @@ -68,7 +68,7 @@ impl Person { // Set the local user info to none diesel::update(local_user::table.filter(local_user::person_id.eq(person_id))) - .set((local_user::email.eq::>(None),)) + .set(local_user::email.eq::>(None)) .execute(conn) .await?; diff --git a/migrations/2023-09-18-141700_login-token/up.sql b/migrations/2023-09-18-141700_login-token/up.sql index 5159ae4f2b..97a8b0cbda 100644 --- a/migrations/2023-09-18-141700_login-token/up.sql +++ b/migrations/2023-09-18-141700_login-token/up.sql @@ -7,6 +7,10 @@ CREATE TABLE login_token ( user_agent text ); +CREATE INDEX idx_login_token_user ON login_token (user_id); + +CREATE INDEX idx_login_token_user_token ON login_token (user_id, token); + -- not needed anymore as we invalidate login tokens on password change ALTER TABLE local_user DROP COLUMN validator_time; From 2ea45bed95f78c108069ed7f2145d58c8eaaddc3 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Mon, 2 Oct 2023 15:26:34 +0200 Subject: [PATCH 13/15] review --- crates/api/src/lib.rs | 8 +++----- crates/api_common/src/utils.rs | 4 ++-- crates/db_schema/src/source/login_token.rs | 5 +++++ crates/utils/src/error.rs | 1 + 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index 328ee1c975..300b89ffee 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -1,4 +1,4 @@ -use actix_web::{cookie::SameSite, http::header::Header, HttpRequest}; +use actix_web::{http::header::Header, HttpRequest}; use actix_web_httpauth::headers::authorization::{Authorization, Bearer}; use base64::{engine::general_purpose::STANDARD_NO_PAD as base64, Engine}; use captcha::Captcha; @@ -76,14 +76,12 @@ pub fn read_auth_token(req: &HttpRequest) -> Result, LemmyError> if let Ok(header) = Authorization::::parse(req) { Ok(Some(header.as_ref().token().to_string())) } - // If that fails, try auth cookie. Dont use the `jwt` cookie from lemmy-ui because - // its not http-only. + // If that fails, try to read from cookie else if let Some(cookie) = &req.cookie(AUTH_COOKIE_NAME) { // ensure that its marked as httponly and secure let secure = cookie.secure().unwrap_or_default(); let http_only = cookie.http_only().unwrap_or_default(); - let same_site = cookie.same_site(); - if !secure || !http_only || same_site != Some(SameSite::Strict) { + if !secure || !http_only { Err(LemmyError::from(LemmyErrorType::AuthCookieInsecure)) } else { Ok(Some(cookie.value().to_string())) diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index 17e9c9407b..d726b95940 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -44,7 +44,7 @@ use rosetta_i18n::{Language, LanguageId}; use tracing::warn; use url::{ParseError, Url}; -pub static AUTH_COOKIE_NAME: &str = "auth"; +pub static AUTH_COOKIE_NAME: &str = "jwt"; #[tracing::instrument(skip_all)] pub async fn is_mod_or_admin( @@ -754,7 +754,7 @@ pub fn sanitize_html_federation_opt(data: &Option) -> Option { pub fn create_login_cookie(jwt: Sensitive) -> Cookie<'static> { let mut cookie = Cookie::new(AUTH_COOKIE_NAME, jwt.into_inner()); cookie.set_secure(true); - cookie.set_same_site(SameSite::Strict); + cookie.set_same_site(SameSite::Lax); cookie.set_http_only(true); cookie } diff --git a/crates/db_schema/src/source/login_token.rs b/crates/db_schema/src/source/login_token.rs index 17bd5a43ac..008b96e048 100644 --- a/crates/db_schema/src/source/login_token.rs +++ b/crates/db_schema/src/source/login_token.rs @@ -4,15 +4,20 @@ use crate::schema::login_token; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; +/// Stores data related to a specific user login session. #[derive(Clone, PartialEq, Eq, Debug, Serialize, Deserialize)] #[cfg_attr(feature = "full", derive(Queryable, Identifiable))] #[cfg_attr(feature = "full", diesel(table_name = login_token))] pub struct LoginToken { pub id: i32, + /// Jwt token for this login #[serde(skip)] pub token: String, pub user_id: LocalUserId, + /// Time of login pub published: DateTime, + /// IP address where login was made from, allows invalidating logins by IP address. + /// Could be stored in truncated format, or store derived information for better privacy. pub ip: Option, pub user_agent: Option, } diff --git a/crates/utils/src/error.rs b/crates/utils/src/error.rs index 079d0fc945..0e2294a863 100644 --- a/crates/utils/src/error.rs +++ b/crates/utils/src/error.rs @@ -212,6 +212,7 @@ pub enum LemmyErrorType { CouldntSendWebmention, ContradictingFilters, InstanceBlockAlreadyExists, + #[serde(rename = "`jwt` cookie must be marked secure and httponly")] AuthCookieInsecure, Unknown(String), } From 414376f00702f11aa2672c4f4b5e18d80b91636a Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Thu, 5 Oct 2023 10:45:13 +0200 Subject: [PATCH 14/15] review --- crates/utils/src/error.rs | 2 +- migrations/2023-09-18-141700_login-token/up.sql | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/utils/src/error.rs b/crates/utils/src/error.rs index 0e2294a863..3df8f399b6 100644 --- a/crates/utils/src/error.rs +++ b/crates/utils/src/error.rs @@ -212,7 +212,7 @@ pub enum LemmyErrorType { CouldntSendWebmention, ContradictingFilters, InstanceBlockAlreadyExists, - #[serde(rename = "`jwt` cookie must be marked secure and httponly")] + /// `jwt` cookie must be marked secure and httponly AuthCookieInsecure, Unknown(String), } diff --git a/migrations/2023-09-18-141700_login-token/up.sql b/migrations/2023-09-18-141700_login-token/up.sql index 97a8b0cbda..c7ec3e7dc8 100644 --- a/migrations/2023-09-18-141700_login-token/up.sql +++ b/migrations/2023-09-18-141700_login-token/up.sql @@ -7,8 +7,6 @@ CREATE TABLE login_token ( user_agent text ); -CREATE INDEX idx_login_token_user ON login_token (user_id); - CREATE INDEX idx_login_token_user_token ON login_token (user_id, token); -- not needed anymore as we invalidate login tokens on password change From c60b0011a5a4be11966057224edbdce44d725c23 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Fri, 6 Oct 2023 10:35:45 +0200 Subject: [PATCH 15/15] rename cookie --- crates/api_common/src/utils.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index d726b95940..234a5ad390 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -44,7 +44,7 @@ use rosetta_i18n::{Language, LanguageId}; use tracing::warn; use url::{ParseError, Url}; -pub static AUTH_COOKIE_NAME: &str = "jwt"; +pub static AUTH_COOKIE_NAME: &str = "auth"; #[tracing::instrument(skip_all)] pub async fn is_mod_or_admin(