Revert "Dont ignore errors during login (fixes #4319) (#4321)" (#4380)

This reverts commit 4163e0465e.

Co-authored-by: Dessalines <dessalines@users.noreply.github.com>
This commit is contained in:
Nutomic 2024-01-19 17:21:43 +01:00 committed by GitHub
parent 516db012bf
commit 3d6f7ff911
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 45 additions and 37 deletions

View file

@ -112,19 +112,18 @@ test("Delete user", async () => {
).toBe(true); ).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, { let invalid_auth = new LemmyHttp(alphaUrl, {
headers: { Authorization: "Bearer foobar" }, headers: { Authorization: "Bearer foobar" },
fetchFunction, fetchFunction,
}); });
await expect(getSite(invalid_auth)).rejects.toStrictEqual( let site = await getSite(invalid_auth);
Error("incorrect_login"), expect(site.my_user).toBeUndefined();
); expect(site.site_view).toBeDefined();
let form: GetPosts = {}; let form: GetPosts = {};
await expect(invalid_auth.getPosts(form)).rejects.toStrictEqual( let posts = invalid_auth.getPosts(form);
Error("incorrect_login"), expect((await posts).posts).toBeDefined();
);
}); });
test("Create user with Arabic name", async () => { test("Create user with Arabic name", async () => {

View file

@ -2,11 +2,15 @@ use actix_web::{http::header::Header, HttpRequest};
use actix_web_httpauth::headers::authorization::{Authorization, Bearer}; use actix_web_httpauth::headers::authorization::{Authorization, Bearer};
use base64::{engine::general_purpose::STANDARD_NO_PAD as base64, Engine}; use base64::{engine::general_purpose::STANDARD_NO_PAD as base64, Engine};
use captcha::Captcha; 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_schema::source::local_site::LocalSite;
use lemmy_db_views::structs::LocalUserView; use lemmy_db_views::structs::LocalUserView;
use lemmy_utils::{ use lemmy_utils::{
error::{LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult}, error::{LemmyError, LemmyErrorExt, LemmyErrorExt2, LemmyErrorType, LemmyResult},
utils::slurs::check_slurs, utils::slurs::check_slurs,
}; };
use std::io::Cursor; use std::io::Cursor;
@ -137,6 +141,20 @@ pub(crate) fn build_totp_2fa(
.with_lemmy_type(LemmyErrorType::CouldntGenerateTotp) .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)] #[cfg(test)]
mod tests { mod tests {
#![allow(clippy::unwrap_used)] #![allow(clippy::unwrap_used)]

View file

@ -1,10 +1,10 @@
use crate::read_auth_token; use crate::{local_user_view_from_jwt, read_auth_token};
use actix_web::{ use actix_web::{
web::{Data, Json}, web::{Data, Json},
HttpRequest, HttpRequest,
}; };
use lemmy_api_common::{claims::Claims, context::LemmyContext, SuccessResponse}; use lemmy_api_common::{context::LemmyContext, SuccessResponse};
use lemmy_utils::error::{LemmyError, LemmyErrorExt2, LemmyErrorType}; use lemmy_utils::error::{LemmyError, LemmyErrorType};
/// Returns an error message if the auth token is invalid for any reason. Necessary because other /// 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. /// endpoints silently treat any call with invalid auth as unauthenticated.
@ -15,9 +15,7 @@ pub async fn validate_auth(
) -> Result<Json<SuccessResponse>, LemmyError> { ) -> Result<Json<SuccessResponse>, LemmyError> {
let jwt = read_auth_token(&req)?; let jwt = read_auth_token(&req)?;
if let Some(jwt) = jwt { if let Some(jwt) = jwt {
Claims::validate(&jwt, &context) local_user_view_from_jwt(&jwt, &context).await?;
.await
.with_lemmy_type(LemmyErrorType::NotLoggedIn)?;
} else { } else {
Err(LemmyErrorType::NotLoggedIn)?; Err(LemmyErrorType::NotLoggedIn)?;
} }

View file

@ -6,7 +6,7 @@ use lemmy_db_schema::{
newtypes::LocalUserId, newtypes::LocalUserId,
source::login_token::{LoginToken, LoginTokenCreateForm}, source::login_token::{LoginToken, LoginTokenCreateForm},
}; };
use lemmy_utils::error::{LemmyErrorType, LemmyResult}; use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult};
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
#[derive(Debug, Serialize, Deserialize)] #[derive(Debug, Serialize, Deserialize)]
@ -25,7 +25,8 @@ impl Claims {
validation.required_spec_claims.remove("exp"); validation.required_spec_claims.remove("exp");
let jwt_secret = &context.secret().jwt_secret; let jwt_secret = &context.secret().jwt_secret;
let key = DecodingKey::from_secret(jwt_secret.as_ref()); let key = DecodingKey::from_secret(jwt_secret.as_ref());
let claims = decode::<Claims>(jwt, &key, &validation)?; let claims =
decode::<Claims>(jwt, &key, &validation).with_lemmy_type(LemmyErrorType::NotLoggedIn)?;
let user_id = LocalUserId(claims.claims.sub.parse()?); let user_id = LocalUserId(claims.claims.sub.parse()?);
let is_valid = LoginToken::validate(&mut context.pool(), user_id, jwt).await?; let is_valid = LoginToken::validate(&mut context.pool(), user_id, jwt).await?;
if !is_valid { if !is_valid {

View file

@ -1,6 +1,6 @@
use lemmy_api_common::{claims::Claims, context::LemmyContext, utils::check_user_valid}; use lemmy_api_common::{claims::Claims, context::LemmyContext, utils::check_user_valid};
use lemmy_db_views::structs::LocalUserView; use lemmy_db_views::structs::LocalUserView;
use lemmy_utils::error::{LemmyError, LemmyErrorExt2, LemmyErrorType}; use lemmy_utils::error::LemmyError;
pub mod feeds; pub mod feeds;
pub mod images; pub mod images;
@ -12,9 +12,7 @@ async fn local_user_view_from_jwt(
jwt: &str, jwt: &str,
context: &LemmyContext, context: &LemmyContext,
) -> Result<LocalUserView, LemmyError> { ) -> Result<LocalUserView, LemmyError> {
let local_user_id = Claims::validate(jwt, context) let local_user_id = Claims::validate(jwt, context).await?;
.await
.with_lemmy_type(LemmyErrorType::NotLoggedIn)?;
let local_user_view = LocalUserView::read(&mut context.pool(), local_user_id).await?; let local_user_view = LocalUserView::read(&mut context.pool(), local_user_id).await?;
check_user_valid(&local_user_view.person)?; check_user_valid(&local_user_view.person)?;

View file

@ -7,14 +7,8 @@ use actix_web::{
}; };
use core::future::Ready; use core::future::Ready;
use futures_util::future::LocalBoxFuture; use futures_util::future::LocalBoxFuture;
use lemmy_api::read_auth_token; use lemmy_api::{local_user_view_from_jwt, read_auth_token};
use lemmy_api_common::{ use lemmy_api_common::context::LemmyContext;
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 reqwest::header::HeaderValue;
use std::{future::ready, rc::Rc}; use std::{future::ready, rc::Rc};
@ -73,14 +67,14 @@ where
let jwt = read_auth_token(req.request())?; let jwt = read_auth_token(req.request())?;
if let Some(jwt) = &jwt { if let Some(jwt) = &jwt {
let local_user_id = Claims::validate(jwt, &context) // Ignore any invalid auth so the site can still be used
.await // TODO: this means it will be impossible to get any error message for invalid jwt. Need
.with_lemmy_type(LemmyErrorType::IncorrectLogin)?; // to add a separate endpoint for that.
let local_user_view = LocalUserView::read(&mut context.pool(), local_user_id) // https://github.com/LemmyNet/lemmy/issues/3702
.await let local_user_view = local_user_view_from_jwt(jwt, &context).await.ok();
.map_err(LemmyError::from)?; if let Some(local_user_view) = local_user_view {
check_user_valid(&local_user_view.person)?; req.extensions_mut().insert(local_user_view);
req.extensions_mut().insert(local_user_view); }
} }
let mut res = svc.call(req).await?; let mut res = svc.call(req).await?;