From dc327652a5b3738ba72fc9d1a3077f813d6e99a4 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Mon, 9 Oct 2023 12:46:12 +0200 Subject: [PATCH] Add db table for login tokens which allows for invalidation (#3818) * wip * stuff * fmt * fmt 2 * fmt 3 * fix default feature * use Authorization header * store ip and user agent for each login * add list_logins endpoint * serde(skip) for token * fix api tests * A few suggestions for login_token (#3991) * A few suggestions. * Fixing SQL format. * review * review * rename cookie --------- Co-authored-by: Dessalines --- Cargo.lock | 16 +- Cargo.toml | 3 +- api_tests/src/post.spec.ts | 15 +- api_tests/src/shared.ts | 11 ++ crates/api/Cargo.toml | 1 + crates/api/src/lib.rs | 26 +++- crates/api/src/local_user/ban_person.rs | 7 + crates/api/src/local_user/change_password.rs | 25 ++-- .../local_user/change_password_after_reset.rs | 3 + crates/api/src/local_user/list_logins.rs | 14 ++ crates/api/src/local_user/login.rs | 38 ++--- crates/api/src/local_user/logout.rs | 23 +++ crates/api/src/local_user/mod.rs | 2 + crates/api/src/local_user/save_settings.rs | 44 ++---- crates/api_common/Cargo.toml | 6 + crates/api_common/src/claims.rs | 141 ++++++++++++++++++ crates/api_common/src/lib.rs | 18 +++ crates/api_common/src/utils.rs | 18 ++- crates/api_crud/src/site/read.rs | 2 +- crates/api_crud/src/user/create.rs | 22 ++- crates/api_crud/src/user/delete.rs | 4 +- crates/db_schema/src/impls/local_user.rs | 20 +-- crates/db_schema/src/impls/login_token.rs | 66 ++++++++ crates/db_schema/src/impls/mod.rs | 1 + crates/db_schema/src/impls/person.rs | 5 +- crates/db_schema/src/schema.rs | 14 +- crates/db_schema/src/source/local_user.rs | 3 - crates/db_schema/src/source/login_token.rs | 32 ++++ crates/db_schema/src/source/mod.rs | 1 + .../src/registration_application_view.rs | 1 - crates/routes/src/feeds.rs | 92 ++++-------- crates/routes/src/images.rs | 6 +- crates/routes/src/lib.rs | 20 +++ crates/utils/Cargo.toml | 1 - crates/utils/src/claims.rs | 35 ----- crates/utils/src/error.rs | 1 + crates/utils/src/lib.rs | 6 +- crates/utils/translations | 2 +- .../2023-09-18-141700_login-token/down.sql | 5 + .../2023-09-18-141700_login-token/up.sql | 15 ++ src/api_routes_http.rs | 6 +- src/session_middleware.rs | 103 ++++--------- 42 files changed, 586 insertions(+), 288 deletions(-) create mode 100644 crates/api/src/local_user/list_logins.rs 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 04fbed792..41f132612 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2625,6 +2625,7 @@ version = "0.19.0-beta.7" dependencies = [ "activitypub_federation", "actix-web", + "actix-web-httpauth", "anyhow", "async-trait", "base64 0.21.2", @@ -2660,6 +2661,7 @@ dependencies = [ "encoding", "futures", "getrandom", + "jsonwebtoken", "lemmy_db_schema", "lemmy_db_views", "lemmy_db_views_actor", @@ -2673,6 +2675,7 @@ dependencies = [ "rosetta-i18n", "serde", "serde_with", + "serial_test", "tokio", "tracing", "ts-rs", @@ -2938,7 +2941,6 @@ dependencies = [ "html2text", "http", "itertools 0.11.0", - "jsonwebtoken", "lettre", "markdown-it", "once_cell", @@ -3366,9 +3368,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", @@ -3398,9 +3400,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", ] @@ -3669,9 +3671,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 9eb9e90db..5e5cc654f 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 } @@ -135,6 +136,7 @@ lemmy_utils = { workspace = true } lemmy_db_schema = { workspace = true } lemmy_api_common = { workspace = true } lemmy_routes = { workspace = true } +lemmy_federate = { version = "0.19.0-beta.7", path = "crates/federate" } activitypub_federation = { workspace = true } diesel = { workspace = true } diesel-async = { workspace = true } @@ -169,4 +171,3 @@ actix-web-prom = { version = "0.6.0", optional = true } serial_test = { workspace = true } clap = { version = "4.3.19", features = ["derive"] } actix-web-httpauth = "0.8.1" -lemmy_federate = { version = "0.19.0-beta.7", path = "crates/federate" } diff --git a/api_tests/src/post.spec.ts b/api_tests/src/post.spec.ts index d87361875..49840d49f 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; @@ -391,8 +392,9 @@ test("Enforce site ban for federated user", async () => { let 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"; } @@ -438,8 +440,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 f145e4c6f..51431f084 100644 --- a/api_tests/src/shared.ts +++ b/api_tests/src/shared.ts @@ -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/crates/api/Cargo.toml b/crates/api/Cargo.toml index d99fb7691..d9c4c1051 100644 --- a/crates/api/Cargo.toml +++ b/crates/api/Cargo.toml @@ -35,6 +35,7 @@ url = { workspace = true } wav = "1.0.0" sitemap-rs = "0.2.0" totp-rs = { version = "5.0.2", features = ["gen_secret", "otpauth"] } +actix-web-httpauth = "0.8.1" [dev-dependencies] serial_test = { workspace = true } diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index 04df09308..300b89ffe 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -1,6 +1,8 @@ +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; -use lemmy_api_common::utils::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_db_views::structs::LocalUserView; use lemmy_utils::{ @@ -69,6 +71,28 @@ 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 + if let Ok(header) = Authorization::::parse(req) { + Ok(Some(header.as_ref().token().to_string())) + } + // 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(); + if !secure || !http_only { + Err(LemmyError::from(LemmyErrorType::AuthCookieInsecure)) + } else { + Ok(Some(cookie.value().to_string())) + } + } + // Otherwise, there's no auth + else { + Ok(None) + } +} + pub(crate) fn check_totp_2fa_valid( local_user_view: &LocalUserView, totp_token: &Option, diff --git a/crates/api/src/local_user/ban_person.rs b/crates/api/src/local_user/ban_person.rs index ecc09220d..e8eaecc0a 100644 --- a/crates/api/src/local_user/ban_person.rs +++ b/crates/api/src/local_user/ban_person.rs @@ -8,6 +8,7 @@ use lemmy_api_common::{ }; use lemmy_db_schema::{ source::{ + login_token::LoginToken, moderator::{ModBan, ModBanForm}, person::{Person, PersonUpdateForm}, }, @@ -44,6 +45,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 9b491d821..ab5b32dd9 100644 --- a/crates/api/src/local_user/change_password.rs +++ b/crates/api/src/local_user/change_password.rs @@ -1,20 +1,22 @@ -use actix_web::web::{Data, Json}; +use actix_web::{ + web::{Data, Json}, + HttpRequest, +}; use bcrypt::verify; use lemmy_api_common::{ + claims::Claims, context::LemmyContext, person::{ChangePassword, LoginResponse}, utils::password_length_check, }; -use lemmy_db_schema::source::local_user::LocalUser; +use lemmy_db_schema::source::{local_user::LocalUser, login_token::LoginToken}; use lemmy_db_views::structs::LocalUserView; -use lemmy_utils::{ - claims::Claims, - error::{LemmyError, LemmyErrorType}, -}; +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> { @@ -40,16 +42,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, req, &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 1bb501dd6..b3e193dc7 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/list_logins.rs b/crates/api/src/local_user/list_logins.rs new file mode 100644 index 000000000..f1ae76be5 --- /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/login.rs b/crates/api/src/local_user/login.rs index 46f547671..981b76a09 100644 --- a/crates/api/src/local_user/login.rs +++ b/crates/api/src/local_user/login.rs @@ -1,10 +1,16 @@ use crate::check_totp_2fa_valid; -use actix_web::web::{Data, Json}; +use actix_web::{ + http::StatusCode, + web::{Data, Json}, + HttpRequest, + HttpResponse, +}; use bcrypt::verify; use lemmy_api_common::{ + claims::Claims, context::LemmyContext, person::{Login, LoginResponse}, - utils::check_user_valid, + utils::{check_user_valid, create_login_cookie}, }; use lemmy_db_schema::{ source::{local_site::LocalSite, registration_application::RegistrationApplication}, @@ -12,16 +18,14 @@ use lemmy_db_schema::{ RegistrationMode, }; use lemmy_db_views::structs::{LocalUserView, SiteView}; -use lemmy_utils::{ - claims::Claims, - error::{LemmyError, LemmyErrorExt, LemmyErrorType}, -}; +use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType}; #[tracing::instrument(skip(context))] pub async fn login( data: Json, + req: HttpRequest, context: Data, -) -> Result, LemmyError> { +) -> Result { let site_view = SiteView::read_local(&mut context.pool()).await?; // Fetch that username / email @@ -63,19 +67,17 @@ pub async fn login( check_totp_2fa_valid(&local_user_view, &data.totp_2fa_token, &site_view.site.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, req, &context).await?; + + let json = LoginResponse { + jwt: Some(jwt.clone()), verify_email_sent: false, registration_created: false, - })) + }; + + let mut res = HttpResponse::build(StatusCode::OK).json(json); + res.add_cookie(&create_login_cookie(jwt))?; + Ok(res) } async fn check_registration_application( diff --git a/crates/api/src/local_user/logout.rs b/crates/api/src/local_user/logout.rs new file mode 100644 index 000000000..a2cc83b3f --- /dev/null +++ b/crates/api/src/local_user/logout.rs @@ -0,0 +1,23 @@ +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::{LemmyErrorType, LemmyResult}; + +#[tracing::instrument(skip(context))] +pub async fn logout( + req: HttpRequest, + // require login + _local_user_view: LocalUserView, + context: Data, +) -> LemmyResult { + let jwt = read_auth_token(&req)?.ok_or(LemmyErrorType::NotLoggedIn)?; + LoginToken::invalidate(&mut context.pool(), &jwt).await?; + + let mut res = HttpResponse::Ok().finish(); + let 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 d5f351116..1b58713f1 100644 --- a/crates/api/src/local_user/mod.rs +++ b/crates/api/src/local_user/mod.rs @@ -6,7 +6,9 @@ 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; 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 7d3b675f6..a88dc431c 100644 --- a/crates/api/src/local_user/save_settings.rs +++ b/crates/api/src/local_user/save_settings.rs @@ -1,8 +1,9 @@ use actix_web::web::{Data, Json}; use lemmy_api_common::{ context::LemmyContext, - person::{LoginResponse, SaveUserSettings}, + person::SaveUserSettings, utils::{sanitize_html_api_opt, send_verification_email}, + SuccessResponse, }; use lemmy_db_schema::{ source::{ @@ -15,7 +16,6 @@ use lemmy_db_schema::{ }; use lemmy_db_views::structs::{LocalUserView, SiteView}; use lemmy_utils::{ - claims::Claims, error::{LemmyError, LemmyErrorExt, LemmyErrorType}, utils::validation::{is_valid_bio_field, is_valid_display_name, is_valid_matrix_id}, }; @@ -25,7 +25,7 @@ pub async fn save_user_settings( data: Json, context: Data, local_user_view: LocalUserView, -) -> Result, LemmyError> { +) -> Result, LemmyError> { let site_view = SiteView::read_local(&mut context.pool()).await?; let bio = sanitize_html_api_opt(&data.bio); @@ -41,8 +41,11 @@ 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, @@ -119,34 +122,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 - }; + LocalUser::update(&mut context.pool(), local_user_id, &local_user_form).await?; - return Err(e).with_lemmy_type(err_type); - } - }; - - // Return the jwt - Ok(Json(LoginResponse { - jwt: Some( - Claims::jwt( - updated_local_user.id.0, - &context.secret().jwt_secret, - &context.settings().hostname, - )? - .into(), - ), - verify_email_sent: false, - registration_created: false, - })) + Ok(Json(SuccessResponse::default())) } diff --git a/crates/api_common/Cargo.toml b/crates/api_common/Cargo.toml index 8a23a4cb2..5325350c8 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,10 @@ 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"] } + +[dev-dependencies] +serial_test = { workspace = true } +reqwest-middleware = { workspace = true } diff --git a/crates/api_common/src/claims.rs b/crates/api_common/src/claims.rs new file mode 100644 index 000000000..6676840dc --- /dev/null +++ b/crates/api_common/src/claims.rs @@ -0,0 +1,141 @@ +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::{ + 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 { + Err(LemmyErrorType::NotLoggedIn)? + } else { + Ok(user_id) + } + } + + pub async fn generate( + user_id: LocalUserId, + req: HttpRequest, + 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 ip = req + .connection_info() + .realip_remote_addr() + .map(ToString::to_string); + let user_agent = req + .headers() + .get(USER_AGENT) + .and_then(|ua| ua.to_str().ok()) + .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)) + } +} + +#[cfg(test)] +mod tests { + #![allow(clippy::unwrap_used)] + #![allow(clippy::indexing_slicing)] + + use crate::{claims::Claims, context::LemmyContext}; + use actix_web::test::TestRequest; + 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::rate_limit::{RateLimitCell, RateLimitConfig}; + use reqwest::Client; + use reqwest_middleware::ClientBuilder; + 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 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 req = TestRequest::default().to_http_request(); + let jwt = Claims::generate(inserted_local_user.id, req, &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 652cbaf43..6f7da52ee 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")] @@ -21,3 +23,19 @@ 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}; + +#[derive(Debug, Serialize, Deserialize, Clone)] +#[cfg_attr(feature = "full", derive(ts_rs::TS))] +#[cfg_attr(feature = "full", ts(export))] +/// Saves settings for your user. +pub struct SuccessResponse { + pub success: bool, +} + +impl Default for SuccessResponse { + fn default() -> Self { + SuccessResponse { success: true } + } +} diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index 26ef18c9e..234a5ad39 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -1,4 +1,10 @@ -use crate::{context::LemmyContext, request::purge_image_from_pictrs, site::FederatedInstances}; +use crate::{ + 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::{ @@ -38,6 +44,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<'_>, @@ -743,6 +751,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::Lax); + cookie.set_http_only(true); + cookie +} + #[cfg(test)] mod tests { #![allow(clippy::unwrap_used)] diff --git a/crates/api_crud/src/site/read.rs b/crates/api_crud/src/site/read.rs index 2f16b8696..aceee29d4 100644 --- a/crates/api_crud/src/site/read.rs +++ b/crates/api_crud/src/site/read.rs @@ -24,8 +24,8 @@ use lemmy_utils::{ #[tracing::instrument(skip(context))] pub async fn get_site( - context: Data, local_user_view: Option, + context: Data, ) -> Result, LemmyError> { let site_view = SiteView::read_local(&mut context.pool()).await?; diff --git a/crates/api_crud/src/user/create.rs b/crates/api_crud/src/user/create.rs index f153f147c..e9ac6d5cb 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::{http::StatusCode, web::Json, HttpRequest, HttpResponse, HttpResponseBuilder}; 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, @@ -30,7 +32,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,8 +42,9 @@ use lemmy_utils::{ #[tracing::instrument(skip(context))] pub async fn register( data: Json, + req: HttpRequest, 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 = @@ -164,6 +166,7 @@ pub async fn register( .await?; } + let mut res = HttpResponseBuilder::new(StatusCode::OK); let mut login_response = LoginResponse { jwt: None, registration_created: false, @@ -174,14 +177,9 @@ 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, req, &context).await?; + res.cookie(create_login_cookie(jwt.clone())); + login_response.jwt = Some(jwt); } else { if local_site.require_email_verification { let local_user_view = LocalUserView { @@ -211,5 +209,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 b3abb532f..8df9c5204 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::purge_user_account, }; -use lemmy_db_schema::source::person::Person; +use lemmy_db_schema::source::{login_token::LoginToken, person::Person}; use lemmy_db_views::structs::LocalUserView; use lemmy_utils::error::{LemmyError, LemmyErrorType}; @@ -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 6ef3421d3..3206322e4 100644 --- a/crates/db_schema/src/impls/local_user.rs +++ b/crates/db_schema/src/impls/local_user.rs @@ -6,14 +6,17 @@ use crate::{ email_verified, local_user, password_encrypted, - validator_time, }, source::{ actor_language::{LocalUserLanguage, SiteLanguage}, local_user::{LocalUser, LocalUserInsertForm, LocalUserUpdateForm}, }, traits::Crud, - utils::{get_conn, naive_now, DbPool}, + utils::{ + functions::{coalesce, lower}, + get_conn, + DbPool, + }, }; use bcrypt::{hash, DEFAULT_COST}; use diesel::{dsl::insert_into, result::Error, ExpressionMethods, QueryDsl}; @@ -29,10 +32,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 } @@ -58,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/login_token.rs b/crates/db_schema/src/impls/login_token.rs new file mode 100644 index 000000000..b1d1124d6 --- /dev/null +++ b/crates/db_schema/src/impls/login_token.rs @@ -0,0 +1,66 @@ +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 + } + + 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?; + 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 a46131f83..3cf0f1066 100644 --- a/crates/db_schema/src/impls/mod.rs +++ b/crates/db_schema/src/impls/mod.rs @@ -17,6 +17,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 723062aeb..1a2974439 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 f347bfe3b..0d9f08b82 100644 --- a/crates/db_schema/src/schema.rs +++ b/crates/db_schema/src/schema.rs @@ -428,7 +428,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, @@ -454,6 +453,17 @@ diesel::table! { } } +diesel::table! { + login_token (id) { + id -> Int4, + token -> Text, + user_id -> Int4, + published -> Timestamptz, + ip -> Nullable, + user_agent -> Nullable, + } +} + diesel::table! { mod_add (id) { id -> Int4, @@ -945,6 +955,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)); @@ -1024,6 +1035,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 98e01bd1a..709a34410 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 chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use serde_with::skip_serializing_none; #[cfg(feature = "full")] @@ -35,8 +34,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 000000000..008b96e04 --- /dev/null +++ b/crates/db_schema/src/source/login_token.rs @@ -0,0 +1,32 @@ +use crate::newtypes::LocalUserId; +#[cfg(feature = "full")] +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, +} + +#[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, + pub ip: Option, + pub user_agent: Option, +} diff --git a/crates/db_schema/src/source/mod.rs b/crates/db_schema/src/source/mod.rs index 8288126ec..9879ef35f 100644 --- a/crates/db_schema/src/source/mod.rs +++ b/crates/db_schema/src/source/mod.rs @@ -22,6 +22,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_views/src/registration_application_view.rs b/crates/db_views/src/registration_application_view.rs index 6a2ed6133..a83b4c5df 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 49cef6d41..446db86ea 100644 --- a/crates/routes/src/feeds.rs +++ b/crates/routes/src/feeds.rs @@ -1,19 +1,18 @@ +use crate::local_user_view_from_jwt; 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_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, @@ -22,7 +21,6 @@ use lemmy_db_views_actor::{ }; use lemmy_utils::{ cache_header::cache_1hour, - claims::Claims, error::LemmyError, utils::markdown::markdown_to_html, }; @@ -182,53 +180,38 @@ 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, - ¶m, - &protocol_and_hostname, ) .await } + RequestType::Inbox => get_feed_inbox(&context, ¶m).await, } .map_err(ErrorBadRequest)?; @@ -243,15 +226,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)), @@ -261,10 +243,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 @@ -278,15 +260,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)), @@ -295,10 +276,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 @@ -316,17 +297,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)), @@ -336,10 +314,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,17 +335,11 @@ async fn get_feed_front( } #[tracing::instrument(skip_all)] -async fn get_feed_inbox( - pool: &mut DbPool<'_>, - jwt_secret: &str, - 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; +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; + let show_bot_accounts = local_user.local_user.show_bot_accounts; let sort = CommentSortType::New; @@ -378,7 +351,7 @@ async fn get_feed_inbox( limit: (Some(RSS_FETCH_LIMIT)), ..Default::default() } - .list(pool) + .list(&mut context.pool()) .await?; let mentions = PersonMentionQuery { @@ -389,10 +362,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 a6ddf666f..a537300d2 100644 --- a/crates/routes/src/images.rs +++ b/crates/routes/src/images.rs @@ -93,17 +93,15 @@ fn adapt_request( async fn upload( req: HttpRequest, body: web::Payload, - client: web::Data, - context: web::Data, // require login local_user_view: LocalUserView, + context: web::Data, ) -> Result { // TODO: check rate limit here - 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()) diff --git a/crates/routes/src/lib.rs b/crates/routes/src/lib.rs index e158fa550..28da113ef 100644 --- a/crates/routes/src/lib.rs +++ b/crates/routes/src/lib.rs @@ -1,4 +1,24 @@ +use lemmy_api_common::{claims::Claims, context::LemmyContext, utils::check_user_valid}; +use lemmy_db_views::structs::LocalUserView; +use lemmy_utils::error::LemmyError; + pub mod feeds; pub mod images; pub mod nodeinfo; pub mod webfinger; + +#[tracing::instrument(skip_all)] +async fn local_user_view_from_jwt( + jwt: &str, + context: &LemmyContext, +) -> Result { + 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, + local_user_view.person.ban_expires, + local_user_view.person.deleted, + )?; + + Ok(local_user_view) +} diff --git a/crates/utils/Cargo.toml b/crates/utils/Cargo.toml index c726e441f..20611702e 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" ts-rs = { workspace = true, optional = true } diff --git a/crates/utils/src/claims.rs b/crates/utils/src/claims.rs deleted file mode 100644 index cebd422ac..000000000 --- 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/error.rs b/crates/utils/src/error.rs index 6032a9791..f5b7a0be8 100644 --- a/crates/utils/src/error.rs +++ b/crates/utils/src/error.rs @@ -213,6 +213,7 @@ pub enum LemmyErrorType { CouldntSendWebmention, ContradictingFilters, InstanceBlockAlreadyExists, + /// `jwt` cookie must be marked secure and httponly AuthCookieInsecure, Unknown(String), } diff --git a/crates/utils/src/lib.rs b/crates/utils/src/lib.rs index c0553de31..6f261febd 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/crates/utils/translations b/crates/utils/translations index e943f97fe..18da10858 160000 --- a/crates/utils/translations +++ b/crates/utils/translations @@ -1 +1 @@ -Subproject commit e943f97fe481dc425acdebc8872bf1fdcabaf875 +Subproject commit 18da10858d8c63750beb06247947f25d91944741 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 000000000..826e05e70 --- /dev/null +++ b/migrations/2023-09-18-141700_login-token/down.sql @@ -0,0 +1,5 @@ +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 000000000..c7ec3e7dc --- /dev/null +++ b/migrations/2023-09-18-141700_login-token/up.sql @@ -0,0 +1,15 @@ +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, + published timestamptz NOT NULL DEFAULT now(), + ip text, + user_agent text +); + +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; + diff --git a/src/api_routes_http.rs b/src/api_routes_http.rs index fb62e82ec..173bce199 100644 --- a/src/api_routes_http.rs +++ b/src/api_routes_http.rs @@ -23,7 +23,9 @@ 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::{ list_mentions::list_mentions, list_replies::list_replies, @@ -273,6 +275,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( @@ -291,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 218ed863e..1d19cec9c 100644 --- a/src/session_middleware.rs +++ b/src/session_middleware.rs @@ -1,30 +1,23 @@ use actix_web::{ body::MessageBody, - cookie::SameSite, dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform}, - http::header::{Header, CACHE_CONTROL}, + http::header::CACHE_CONTROL, Error, HttpMessage, }; -use actix_web_httpauth::headers::authorization::{Authorization, Bearer}; -use chrono::{DateTime, Utc}; use core::future::Ready; use futures_util::future::LocalBoxFuture; +use lemmy_api::read_auth_token; use lemmy_api_common::{ + claims::Claims, context::LemmyContext, lemmy_db_views::structs::LocalUserView, utils::check_user_valid, }; -use lemmy_db_schema::newtypes::LocalUserId; -use lemmy_utils::{ - claims::Claims, - error::{LemmyError, LemmyErrorExt2, LemmyErrorType}, -}; +use lemmy_utils::error::{LemmyError, LemmyErrorExt2, LemmyErrorType}; use reqwest::header::HeaderValue; use std::{future::ready, rc::Rc}; -static AUTH_COOKIE_NAME: &str = "auth"; - #[derive(Clone)] pub struct SessionMiddleware { context: LemmyContext, @@ -77,25 +70,7 @@ where let context = self.context.clone(); Box::pin(async move { - let auth_header = Authorization::::parse(&req).ok(); - let jwt = if let Some(a) = auth_header { - Some(a.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).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 @@ -130,10 +105,9 @@ 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 + .with_lemmy_type(LemmyErrorType::NotLoggedIn)?; let local_user_view = LocalUserView::read(&mut context.pool(), local_user_id).await?; check_user_valid( local_user_view.person.banned, @@ -141,27 +115,16 @@ 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) } -/// Checks if user's token was issued before user's password reset. -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(()) - } -} - #[cfg(test)] mod tests { #![allow(clippy::unwrap_used)] #![allow(clippy::indexing_slicing)] use super::*; + use actix_web::test::TestRequest; use lemmy_db_schema::{ source::{ instance::Instance, @@ -172,21 +135,29 @@ mod tests { traits::Crud, utils::build_db_pool_for_tests, }; - use lemmy_utils::{claims::Claims, settings::SETTINGS}; + use lemmy_utils::rate_limit::{RateLimitCell, RateLimitConfig}; + use reqwest::Client; + use reqwest_middleware::ClientBuilder; use serial_test::serial; - use std::env; + use std::env::set_current_dir; #[tokio::test] #[serial] async fn test_session_auth() { - let pool = &build_db_pool_for_tests().await; - let pool = &mut pool.into(); - let secret = Secret::init(pool).await.unwrap(); + // hack, necessary so that config file can be loaded from hardcoded, relative path + set_current_dir("crates/utils").unwrap(); - // test.sh sets `LEMMY_CONFIG_LOCATION=../../config/config.hjson` for code under crates folder. - // this results in a config not found error, so we need to unset this var and use default. - env::remove_var("LEMMY_CONFIG_LOCATION"); - let settings = &SETTINGS.to_owned(); + 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 @@ -207,23 +178,13 @@ mod tests { 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()); + let req = TestRequest::default().to_http_request(); + let jwt = Claims::generate(inserted_local_user.id, req, &context) + .await + .unwrap(); - // 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 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);