Dont ignore errors during login (fixes #4319) (#4321)

* Dont ignore errors during login (fixes #4319)

* fix test

* fmt

api_tests/src/user.spec.ts

@@ -112,18 +112,19 @@ test("Delete user", async () => {
   ).toBe(true);
 });
 
-test("Requests with invalid auth should be treated as unauthenticated", async () => {
+test("Requests with invalid auth should throw error", async () => {
   let invalid_auth = new LemmyHttp(alphaUrl, {
     headers: { Authorization: "Bearer foobar" },
     fetchFunction,
   });
-  let site = await getSite(invalid_auth);
-  expect(site.my_user).toBeUndefined();
-  expect(site.site_view).toBeDefined();
+  await expect(getSite(invalid_auth)).rejects.toStrictEqual(
+    Error("incorrect_login"),
+  );
 
   let form: GetPosts = {};
-  let posts = invalid_auth.getPosts(form);
-  expect((await posts).posts).toBeDefined();
+  await expect(invalid_auth.getPosts(form)).rejects.toStrictEqual(
+    Error("incorrect_login"),
+  );
 });
 
 test("Create user with Arabic name", async () => {

crates/api/src/lib.rs

@@ -2,15 +2,11 @@ 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::{
-  claims::Claims,
-  context::LemmyContext,
-  utils::{check_user_valid, local_site_to_slur_regex, AUTH_COOKIE_NAME},
-};
+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::{
-  error::{LemmyError, LemmyErrorExt, LemmyErrorExt2, LemmyErrorType, LemmyResult},
+  error::{LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult},
   utils::slurs::check_slurs,
 };
 use std::io::Cursor;
@@ -141,20 +137,6 @@ 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<LocalUserView, LemmyError> {
-  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)]

crates/api/src/local_user/validate_auth.rs

@@ -1,10 +1,10 @@
-use crate::{local_user_view_from_jwt, read_auth_token};
+use crate::read_auth_token;
 use actix_web::{
   web::{Data, Json},
   HttpRequest,
 };
-use lemmy_api_common::{context::LemmyContext, SuccessResponse};
-use lemmy_utils::error::{LemmyError, LemmyErrorType};
+use lemmy_api_common::{claims::Claims, context::LemmyContext, SuccessResponse};
+use lemmy_utils::error::{LemmyError, LemmyErrorExt2, 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,7 +15,9 @@ pub async fn validate_auth(
 ) -> Result<Json<SuccessResponse>, LemmyError> {
   let jwt = read_auth_token(&req)?;
   if let Some(jwt) = jwt {
-    local_user_view_from_jwt(&jwt, &context).await?;
+    Claims::validate(&jwt, &context)
+      .await
+      .with_lemmy_type(LemmyErrorType::NotLoggedIn)?;
   } else {
     Err(LemmyErrorType::NotLoggedIn)?;
   }

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::{LemmyErrorExt, LemmyErrorType, LemmyResult};
+use lemmy_utils::error::{LemmyErrorType, LemmyResult};
 use serde::{Deserialize, Serialize};
 
 #[derive(Debug, Serialize, Deserialize)]
@@ -25,8 +25,7 @@ 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::<Claims>(jwt, &key, &validation).with_lemmy_type(LemmyErrorType::NotLoggedIn)?;
+    let claims = decode::<Claims>(jwt, &key, &validation)?;
     let user_id = LocalUserId(claims.claims.sub.parse()?);
     let is_valid = LoginToken::validate(&mut context.pool(), user_id, jwt).await?;
     if !is_valid {

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;
+use lemmy_utils::error::{LemmyError, LemmyErrorExt2, LemmyErrorType};
 
 pub mod feeds;
 pub mod images;
@@ -12,7 +12,9 @@ async fn local_user_view_from_jwt(
   jwt: &str,
   context: &LemmyContext,
 ) -> Result<LocalUserView, LemmyError> {
-  let local_user_id = Claims::validate(jwt, context).await?;
+  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)?;
 

src/session_middleware.rs

@@ -7,8 +7,14 @@ use actix_web::{
 };
 use core::future::Ready;
 use futures_util::future::LocalBoxFuture;
-use lemmy_api::{local_user_view_from_jwt, read_auth_token};
-use lemmy_api_common::context::LemmyContext;
+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 reqwest::header::HeaderValue;
 use std::{future::ready, rc::Rc};
 
@@ -67,15 +73,15 @@ where
       let jwt = read_auth_token(req.request())?;
 
       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 {
+        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);
       }
-      }
 
       let mut res = svc.call(req).await?;