From b2aee565f3884df5faf229964db493e448d64d97 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Tue, 29 Aug 2023 16:47:57 +0200 Subject: [PATCH] Allow passing auth via header or cookie (#3725) * Allow passing auth via header or cookie * revert submodule * taplo * fix build * working * convert apub api methods * also set cache-control header * opt * clippy * deduplicate code, ignore invalid auth * clippy --------- Co-authored-by: Dessalines --- Cargo.lock | 2 + crates/api_common/src/utils.rs | 10 +++ crates/apub/src/api/list_comments.rs | 7 +- crates/apub/src/api/list_posts.rs | 7 +- crates/apub/src/api/read_community.rs | 6 +- crates/apub/src/api/read_person.rs | 7 +- crates/apub/src/api/resolve_object.rs | 7 +- crates/apub/src/api/search.rs | 7 +- crates/db_views/Cargo.toml | 5 +- crates/db_views/src/local_user_view.rs | 15 ++++ crates/utils/src/error.rs | 1 + src/lib.rs | 10 ++- src/session_middleware.rs | 120 +++++++++++++++++++++++++ 13 files changed, 184 insertions(+), 20 deletions(-) create mode 100644 src/session_middleware.rs diff --git a/Cargo.lock b/Cargo.lock index a44dfb916..2ed7ea6ee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2738,10 +2738,12 @@ dependencies = [ name = "lemmy_db_views" version = "0.18.1" dependencies = [ + "actix-web", "diesel", "diesel-async", "diesel_ltree", "lemmy_db_schema", + "lemmy_utils", "serde", "serde_with", "serial_test", diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index 63095f41a..61cd2a26a 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -159,6 +159,16 @@ 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; + } +} /// Checks if user's token was issued before user's password reset. pub fn check_validator_time( diff --git a/crates/apub/src/api/list_comments.rs b/crates/apub/src/api/list_comments.rs index 5d7aa387b..cec2ed9c5 100644 --- a/crates/apub/src/api/list_comments.rs +++ b/crates/apub/src/api/list_comments.rs @@ -8,21 +8,22 @@ 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}, + utils::{check_private_instance, local_user_view_from_jwt_opt_new}, }; use lemmy_db_schema::{ source::{comment::Comment, community::Community, local_site::LocalSite}, traits::Crud, }; -use lemmy_db_views::comment_view::CommentQuery; +use lemmy_db_views::{comment_view::CommentQuery, structs::LocalUserView}; use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType}; #[tracing::instrument(skip(context))] pub async fn list_comments( data: Query, context: Data, + mut local_user_view: Option, ) -> Result, LemmyError> { - let local_user_view = local_user_view_from_jwt_opt(data.auth.as_ref(), &context).await; + 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 c43e1381e..76c7c7cec 100644 --- a/crates/apub/src/api/list_posts.rs +++ b/crates/apub/src/api/list_posts.rs @@ -8,18 +8,19 @@ 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}, + utils::{check_private_instance, local_user_view_from_jwt_opt_new}, }; use lemmy_db_schema::source::{community::Community, local_site::LocalSite}; -use lemmy_db_views::post_view::PostQuery; +use lemmy_db_views::{post_view::PostQuery, structs::LocalUserView}; use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType}; #[tracing::instrument(skip(context))] pub async fn list_posts( data: Query, context: Data, + mut local_user_view: Option, ) -> Result, LemmyError> { - let local_user_view = local_user_view_from_jwt_opt(data.auth.as_ref(), &context).await; + 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 9f6e43571..c4db1a298 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}, + utils::{check_private_instance, is_mod_or_admin_opt, local_user_view_from_jwt_opt_new}, }; use lemmy_db_schema::source::{ actor_language::CommunityLanguage, @@ -12,6 +12,7 @@ use lemmy_db_schema::source::{ local_site::LocalSite, site::Site, }; +use lemmy_db_views::structs::LocalUserView; use lemmy_db_views_actor::structs::{CommunityModeratorView, CommunityView}; use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorExt2, LemmyErrorType}; @@ -19,8 +20,9 @@ use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorExt2, LemmyErrorTy pub async fn get_community( data: Query, context: Data, + mut local_user_view: Option, ) -> Result, LemmyError> { - let local_user_view = local_user_view_from_jwt_opt(data.auth.as_ref(), &context).await; + 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 23d6457af..735ae77b9 100644 --- a/crates/apub/src/api/read_person.rs +++ b/crates/apub/src/api/read_person.rs @@ -4,13 +4,13 @@ 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}, + utils::{check_private_instance, local_user_view_from_jwt_opt_new}, }; use lemmy_db_schema::{ source::{local_site::LocalSite, person::Person}, utils::post_to_comment_sort_type, }; -use lemmy_db_views::{comment_view::CommentQuery, post_view::PostQuery}; +use lemmy_db_views::{comment_view::CommentQuery, post_view::PostQuery, structs::LocalUserView}; use lemmy_db_views_actor::structs::{CommunityModeratorView, PersonView}; use lemmy_utils::error::{LemmyError, LemmyErrorExt2, LemmyErrorType}; @@ -18,13 +18,14 @@ use lemmy_utils::error::{LemmyError, LemmyErrorExt2, LemmyErrorType}; pub async fn read_person( data: Query, context: Data, + mut 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() { return Err(LemmyErrorType::NoIdGiven)?; } - let local_user_view = local_user_view_from_jwt_opt(data.auth.as_ref(), &context).await; + 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 f5a703c99..d1cf2546c 100644 --- a/crates/apub/src/api/resolve_object.rs +++ b/crates/apub/src/api/resolve_object.rs @@ -9,10 +9,10 @@ use diesel::NotFound; use lemmy_api_common::{ context::LemmyContext, site::{ResolveObject, ResolveObjectResponse}, - utils::{check_private_instance, local_user_view_from_jwt_opt}, + utils::{check_private_instance, local_user_view_from_jwt_opt_new}, }; use lemmy_db_schema::{newtypes::PersonId, source::local_site::LocalSite, utils::DbPool}; -use lemmy_db_views::structs::{CommentView, PostView}; +use lemmy_db_views::structs::{CommentView, LocalUserView, PostView}; use lemmy_db_views_actor::structs::{CommunityView, PersonView}; use lemmy_utils::error::{LemmyError, LemmyErrorExt2, LemmyErrorType}; @@ -20,8 +20,9 @@ use lemmy_utils::error::{LemmyError, LemmyErrorExt2, LemmyErrorType}; pub async fn resolve_object( data: Query, context: Data, + mut local_user_view: Option, ) -> Result, LemmyError> { - let local_user_view = local_user_view_from_jwt_opt(data.auth.as_ref(), &context).await; + 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 55d86b081..262f91681 100644 --- a/crates/apub/src/api/search.rs +++ b/crates/apub/src/api/search.rs @@ -4,14 +4,14 @@ 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}, + utils::{check_private_instance, is_admin, local_user_view_from_jwt_opt_new}, }; use lemmy_db_schema::{ source::{community::Community, local_site::LocalSite}, utils::{post_to_comment_sort_type, post_to_person_sort_type}, SearchType, }; -use lemmy_db_views::{comment_view::CommentQuery, post_view::PostQuery}; +use lemmy_db_views::{comment_view::CommentQuery, post_view::PostQuery, structs::LocalUserView}; use lemmy_db_views_actor::{community_view::CommunityQuery, person_view::PersonQuery}; use lemmy_utils::error::LemmyError; @@ -19,8 +19,9 @@ use lemmy_utils::error::LemmyError; pub async fn search( data: Query, context: Data, + mut local_user_view: Option, ) -> Result, LemmyError> { - let local_user_view = local_user_view_from_jwt_opt(data.auth.as_ref(), &context).await; + 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_views/Cargo.toml b/crates/db_views/Cargo.toml index 54a3aab6c..69fa24403 100644 --- a/crates/db_views/Cargo.toml +++ b/crates/db_views/Cargo.toml @@ -13,16 +13,18 @@ doctest = false [features] full = [ - "lemmy_db_schema/full", + "lemmy_utils", "diesel", "diesel-async", "diesel_ltree", "tracing", "ts-rs", + "actix-web", ] [dependencies] lemmy_db_schema = { workspace = true } +lemmy_utils = { workspace = true, optional = true } diesel = { workspace = true, optional = true } diesel-async = { workspace = true, optional = true } diesel_ltree = { workspace = true, optional = true } @@ -30,6 +32,7 @@ serde = { workspace = true } serde_with = { workspace = true } tracing = { workspace = true, optional = true } ts-rs = { workspace = true, optional = true } +actix-web = { workspace = true, optional = true } [dev-dependencies] serial_test = { workspace = true } diff --git a/crates/db_views/src/local_user_view.rs b/crates/db_views/src/local_user_view.rs index d0d461545..21acad7cb 100644 --- a/crates/db_views/src/local_user_view.rs +++ b/crates/db_views/src/local_user_view.rs @@ -1,4 +1,5 @@ use crate::structs::LocalUserView; +use actix_web::{dev::Payload, FromRequest, HttpMessage, HttpRequest}; use diesel::{result::Error, BoolExpressionMethods, ExpressionMethods, JoinOnDsl, QueryDsl}; use diesel_async::RunQueryDsl; use lemmy_db_schema::{ @@ -9,6 +10,8 @@ use lemmy_db_schema::{ traits::JoinView, utils::{functions::lower, DbConn, DbPool, ListFn, Queries, ReadFn}, }; +use lemmy_utils::error::{LemmyError, LemmyErrorType}; +use std::future::{ready, Ready}; type LocalUserViewTuple = (LocalUser, Person, PersonAggregates); @@ -116,3 +119,15 @@ impl JoinView for LocalUserView { } } } + +impl FromRequest for LocalUserView { + type Error = LemmyError; + type Future = Ready>; + + fn from_request(req: &HttpRequest, _payload: &mut Payload) -> Self::Future { + ready(match req.extensions().get::() { + Some(c) => Ok(c.clone()), + None => Err(LemmyErrorType::IncorrectLogin.into()), + }) + } +} diff --git a/crates/utils/src/error.rs b/crates/utils/src/error.rs index 0dd9ba6cb..e1b370387 100644 --- a/crates/utils/src/error.rs +++ b/crates/utils/src/error.rs @@ -208,6 +208,7 @@ pub enum LemmyErrorType { InvalidUrlScheme, CouldntSendWebmention, ContradictingFilters, + AuthCookieInsecure, Unknown(String), } diff --git a/src/lib.rs b/src/lib.rs index 4950aff82..55135723b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,10 +4,15 @@ pub mod code_migrations; pub mod prometheus_metrics; pub mod root_span_builder; pub mod scheduled_tasks; +pub mod session_middleware; #[cfg(feature = "console")] pub mod telemetry; -use crate::{code_migrations::run_advanced_migrations, root_span_builder::QuieterRootSpanBuilder}; +use crate::{ + code_migrations::run_advanced_migrations, + root_span_builder::QuieterRootSpanBuilder, + session_middleware::SessionMiddleware, +}; use activitypub_federation::config::{FederationConfig, FederationMiddleware}; use actix_cors::Cors; use actix_web::{ @@ -204,7 +209,8 @@ pub async fn start_lemmy_server() -> Result<(), LemmyError> { .wrap(ErrorHandlers::new().default_handler(jsonify_plain_text_errors)) .app_data(Data::new(context.clone())) .app_data(Data::new(rate_limit_cell.clone())) - .wrap(FederationMiddleware::new(federation_config.clone())); + .wrap(FederationMiddleware::new(federation_config.clone())) + .wrap(SessionMiddleware::new(context.clone())); #[cfg(feature = "prometheus-metrics")] let app = app.wrap(prom_api_metrics.clone()); diff --git a/src/session_middleware.rs b/src/session_middleware.rs new file mode 100644 index 000000000..5824f33df --- /dev/null +++ b/src/session_middleware.rs @@ -0,0 +1,120 @@ +use actix_web::{ + body::MessageBody, + cookie::SameSite, + 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_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, +} + +impl SessionMiddleware { + pub fn new(context: LemmyContext) -> Self { + SessionMiddleware { context } + } +} +impl Transform for SessionMiddleware +where + S: Service, Error = Error> + 'static, + S::Future: 'static, + B: MessageBody + 'static, +{ + type Response = ServiceResponse; + type Error = Error; + type Transform = SessionService; + type InitError = (); + type Future = Ready>; + + fn new_transform(&self, service: S) -> Self::Future { + ready(Ok(SessionService { + service: Rc::new(service), + context: self.context.clone(), + })) + } +} + +pub struct SessionService { + service: Rc, + context: LemmyContext, +} + +impl Service for SessionService +where + S: Service, Error = Error> + 'static, + S::Future: 'static, + B: 'static, +{ + type Response = ServiceResponse; + type Error = Error; + type Future = LocalBoxFuture<'static, Result>; + + forward_ready!(service); + + fn call(&self, req: ServiceRequest) -> Self::Future { + let svc = self.service.clone(); + 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()) + }; + + if let Some(jwt) = &jwt { + // Ignore any invalid auth so the site can still be used + // TODO: this means it will be impossible to get any error message for invalid jwt. Need + // to add a separate endpoint for that. + // https://github.com/LemmyNet/lemmy/issues/3702 + let local_user_view = local_user_view_from_jwt(jwt, &context).await.ok(); + if let Some(local_user_view) = local_user_view { + req.extensions_mut().insert(local_user_view); + } + } + + let mut res = svc.call(req).await?; + + // Add cache-control header. If user is authenticated, mark as private. Otherwise cache + // up to one minute. + let cache_value = if jwt.is_some() { + "private" + } else { + "public, max-age=60" + }; + res + .headers_mut() + .insert(CACHE_CONTROL, HeaderValue::from_static(cache_value)); + Ok(res) + }) + } +}