From 38bb41cb4dc4349b962854fbdd7c909aaafb186f Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 16 Jan 2024 09:53:56 +0100 Subject: [PATCH] Revert "Dont ignore errors during login (fixes #4319) (#4321)" This reverts commit 4163e0465e11ae78c0c0d9931205179605637590. --- api_tests/src/user.spec.ts | 13 +++++------ crates/api/src/lib.rs | 22 ++++++++++++++++-- crates/api/src/local_user/validate_auth.rs | 10 ++++----- crates/api_common/src/claims.rs | 5 +++-- crates/routes/src/lib.rs | 6 ++--- src/session_middleware.rs | 26 +++++++++------------- 6 files changed, 45 insertions(+), 37 deletions(-) diff --git a/api_tests/src/user.spec.ts b/api_tests/src/user.spec.ts index 527ab74d4..ccfc5e1fe 100644 --- a/api_tests/src/user.spec.ts +++ b/api_tests/src/user.spec.ts @@ -112,19 +112,18 @@ test("Delete user", async () => { ).toBe(true); }); -test("Requests with invalid auth should throw error", async () => { +test("Requests with invalid auth should be treated as unauthenticated", async () => { let invalid_auth = new LemmyHttp(alphaUrl, { headers: { Authorization: "Bearer foobar" }, fetchFunction, }); - await expect(getSite(invalid_auth)).rejects.toStrictEqual( - Error("incorrect_login"), - ); + let site = await getSite(invalid_auth); + expect(site.my_user).toBeUndefined(); + expect(site.site_view).toBeDefined(); let form: GetPosts = {}; - await expect(invalid_auth.getPosts(form)).rejects.toStrictEqual( - Error("incorrect_login"), - ); + let posts = invalid_auth.getPosts(form); + expect((await posts).posts).toBeDefined(); }); test("Create user with Arabic name", async () => { diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index 7d85cc78f..faa74824e 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -2,11 +2,15 @@ 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, AUTH_COOKIE_NAME}; +use lemmy_api_common::{ + claims::Claims, + context::LemmyContext, + utils::{check_user_valid, 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::{ - error::{LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult}, + error::{LemmyError, LemmyErrorExt, LemmyErrorExt2, LemmyErrorType, LemmyResult}, utils::slurs::check_slurs, }; use std::io::Cursor; @@ -137,6 +141,20 @@ pub(crate) fn build_totp_2fa( .with_lemmy_type(LemmyErrorType::CouldntGenerateTotp) } +#[tracing::instrument(skip_all)] +pub async fn local_user_view_from_jwt( + jwt: &str, + context: &LemmyContext, +) -> Result { + 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)?; + + Ok(local_user_view) +} + #[cfg(test)] mod tests { #![allow(clippy::unwrap_used)] diff --git a/crates/api/src/local_user/validate_auth.rs b/crates/api/src/local_user/validate_auth.rs index 65c63e585..d95195dc9 100644 --- a/crates/api/src/local_user/validate_auth.rs +++ b/crates/api/src/local_user/validate_auth.rs @@ -1,10 +1,10 @@ -use crate::read_auth_token; +use crate::{local_user_view_from_jwt, read_auth_token}; use actix_web::{ web::{Data, Json}, HttpRequest, }; -use lemmy_api_common::{claims::Claims, context::LemmyContext, SuccessResponse}; -use lemmy_utils::error::{LemmyError, LemmyErrorExt2, LemmyErrorType}; +use lemmy_api_common::{context::LemmyContext, SuccessResponse}; +use lemmy_utils::error::{LemmyError, LemmyErrorType}; /// Returns an error message if the auth token is invalid for any reason. Necessary because other /// endpoints silently treat any call with invalid auth as unauthenticated. @@ -15,9 +15,7 @@ pub async fn validate_auth( ) -> Result, LemmyError> { let jwt = read_auth_token(&req)?; if let Some(jwt) = jwt { - Claims::validate(&jwt, &context) - .await - .with_lemmy_type(LemmyErrorType::NotLoggedIn)?; + local_user_view_from_jwt(&jwt, &context).await?; } else { Err(LemmyErrorType::NotLoggedIn)?; } diff --git a/crates/api_common/src/claims.rs b/crates/api_common/src/claims.rs index 58541af43..981b7fe14 100644 --- a/crates/api_common/src/claims.rs +++ b/crates/api_common/src/claims.rs @@ -6,7 +6,7 @@ use lemmy_db_schema::{ newtypes::LocalUserId, source::login_token::{LoginToken, LoginTokenCreateForm}, }; -use lemmy_utils::error::{LemmyErrorType, LemmyResult}; +use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult}; use serde::{Deserialize, Serialize}; #[derive(Debug, Serialize, Deserialize)] @@ -25,7 +25,8 @@ impl Claims { 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)?; + 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 { diff --git a/crates/routes/src/lib.rs b/crates/routes/src/lib.rs index b1e97cd3c..ec28fda45 100644 --- a/crates/routes/src/lib.rs +++ b/crates/routes/src/lib.rs @@ -1,6 +1,6 @@ use lemmy_api_common::{claims::Claims, context::LemmyContext, utils::check_user_valid}; use lemmy_db_views::structs::LocalUserView; -use lemmy_utils::error::{LemmyError, LemmyErrorExt2, LemmyErrorType}; +use lemmy_utils::error::LemmyError; pub mod feeds; pub mod images; @@ -12,9 +12,7 @@ async fn local_user_view_from_jwt( jwt: &str, context: &LemmyContext, ) -> Result { - let local_user_id = Claims::validate(jwt, context) - .await - .with_lemmy_type(LemmyErrorType::NotLoggedIn)?; + 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)?; diff --git a/src/session_middleware.rs b/src/session_middleware.rs index ab7eb3d62..2c4e36913 100644 --- a/src/session_middleware.rs +++ b/src/session_middleware.rs @@ -7,14 +7,8 @@ use actix_web::{ }; 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_utils::error::{LemmyError, LemmyErrorExt2, LemmyErrorType}; +use lemmy_api::{local_user_view_from_jwt, read_auth_token}; +use lemmy_api_common::context::LemmyContext; use reqwest::header::HeaderValue; use std::{future::ready, rc::Rc}; @@ -73,14 +67,14 @@ where let jwt = read_auth_token(req.request())?; if let Some(jwt) = &jwt { - let local_user_id = Claims::validate(jwt, &context) - .await - .with_lemmy_type(LemmyErrorType::IncorrectLogin)?; - let local_user_view = LocalUserView::read(&mut context.pool(), local_user_id) - .await - .map_err(LemmyError::from)?; - check_user_valid(&local_user_view.person)?; - req.extensions_mut().insert(local_user_view); + // 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?;