From 93ae488196098351badac3aa2e46234e67c91338 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Severin=20Alexander=20B=C3=BChler?= <8782386+SeverinAlexB@users.noreply.github.com> Date: Fri, 16 May 2025 14:05:38 +0300 Subject: [PATCH] feat: Only block disabled users from writing files, not reading #130 --- e2e/src/tests/auth.rs | 14 ++++++++------ .../src/core/err_if_user_is_invalid.rs | 8 ++++++-- pubky-homeserver/src/core/routes/auth.rs | 2 +- pubky-homeserver/src/core/routes/tenants/read.rs | 7 ------- .../src/core/routes/tenants/session.rs | 2 +- pubky-homeserver/src/core/routes/tenants/write.rs | 4 ++-- 6 files changed, 18 insertions(+), 19 deletions(-) diff --git a/e2e/src/tests/auth.rs b/e2e/src/tests/auth.rs index d3bff97..a09270e 100644 --- a/e2e/src/tests/auth.rs +++ b/e2e/src/tests/auth.rs @@ -94,11 +94,11 @@ async fn disabled_user() { .unwrap(); assert_eq!(response.status(), StatusCode::OK); - // Make sure the user cannot read their own file + // Make sure the user can still read their own file let response = client.get(file_url.clone()).send().await.unwrap(); - assert_eq!(response.status(), StatusCode::FORBIDDEN); + assert_eq!(response.status(), StatusCode::OK); - // Make sure the user cannot write to their own file + // Make sure the user cannot write a new file let response = client .put(file_url.clone()) .body(vec![]) @@ -107,9 +107,11 @@ async fn disabled_user() { .unwrap(); assert_eq!(response.status(), StatusCode::FORBIDDEN); - // Make sure the user cannot sign in - let session = client.signin(&keypair).await; - assert!(session.is_err()); + // Make sure the user can still sign in + client + .signin(&keypair) + .await + .expect("Signin should succeed"); } #[tokio::test] diff --git a/pubky-homeserver/src/core/err_if_user_is_invalid.rs b/pubky-homeserver/src/core/err_if_user_is_invalid.rs index af4ed8d..1dbcf52 100644 --- a/pubky-homeserver/src/core/err_if_user_is_invalid.rs +++ b/pubky-homeserver/src/core/err_if_user_is_invalid.rs @@ -6,10 +6,14 @@ use crate::persistence::lmdb::{tables::users::UserQueryError, LmDB}; use super::Error; /// Returns an error if the user doesn't exist or is disabled. -pub fn err_if_user_is_invalid(pubkey: &PublicKey, db: &LmDB) -> super::error::Result<()> { +pub fn err_if_user_is_invalid( + pubkey: &PublicKey, + db: &LmDB, + err_if_disabled: bool, +) -> super::error::Result<()> { match db.get_user(pubkey, &mut db.env.read_txn()?) { Ok(user) => { - if user.disabled { + if err_if_disabled && user.disabled { return Err(Error::with_status(StatusCode::FORBIDDEN)); } } diff --git a/pubky-homeserver/src/core/routes/auth.rs b/pubky-homeserver/src/core/routes/auth.rs index 526bb5b..4b7afa3 100644 --- a/pubky-homeserver/src/core/routes/auth.rs +++ b/pubky-homeserver/src/core/routes/auth.rs @@ -123,7 +123,7 @@ fn create_session_and_cookie( capabilities: &[Capability], user_agent: Option>, ) -> Result { - err_if_user_is_invalid(public_key, &state.db)?; + err_if_user_is_invalid(public_key, &state.db, false)?; // 1) Create session let session_secret = encode(Alphabet::Crockford, &random_bytes::<16>()); diff --git a/pubky-homeserver/src/core/routes/tenants/read.rs b/pubky-homeserver/src/core/routes/tenants/read.rs index 9231afa..8cfea86 100644 --- a/pubky-homeserver/src/core/routes/tenants/read.rs +++ b/pubky-homeserver/src/core/routes/tenants/read.rs @@ -9,7 +9,6 @@ use pkarr::PublicKey; use std::str::FromStr; use crate::core::{ - err_if_user_is_invalid::err_if_user_is_invalid, error::{Error, Result}, extractors::{ListQueryParams, PubkyHost}, AppState, @@ -22,8 +21,6 @@ pub async fn head( headers: HeaderMap, path: OriginalUri, ) -> Result { - err_if_user_is_invalid(pubky.public_key(), &state.db)?; - let rtxn = state.db.env.read_txn()?; get_entry( headers, @@ -41,8 +38,6 @@ pub async fn get( path: OriginalUri, params: ListQueryParams, ) -> Result { - err_if_user_is_invalid(pubky.public_key(), &state.db)?; - let public_key = pubky.public_key().clone(); let path = path.0.path().to_string(); @@ -86,8 +81,6 @@ pub fn list( path: &str, params: ListQueryParams, ) -> Result> { - err_if_user_is_invalid(public_key, &state.db)?; - let txn = state.db.env.read_txn()?; let path = format!("{public_key}{path}"); diff --git a/pubky-homeserver/src/core/routes/tenants/session.rs b/pubky-homeserver/src/core/routes/tenants/session.rs index b1ae257..c4b77de 100644 --- a/pubky-homeserver/src/core/routes/tenants/session.rs +++ b/pubky-homeserver/src/core/routes/tenants/session.rs @@ -14,7 +14,7 @@ pub async fn session( cookies: Cookies, pubky: PubkyHost, ) -> Result { - err_if_user_is_invalid(pubky.public_key(), &state.db)?; + err_if_user_is_invalid(pubky.public_key(), &state.db, false)?; if let Some(secret) = session_secret_from_cookies(&cookies, pubky.public_key()) { if let Some(session) = state.db.get_session(&secret)? { // TODO: add content-type diff --git a/pubky-homeserver/src/core/routes/tenants/write.rs b/pubky-homeserver/src/core/routes/tenants/write.rs index f6768fd..e364547 100644 --- a/pubky-homeserver/src/core/routes/tenants/write.rs +++ b/pubky-homeserver/src/core/routes/tenants/write.rs @@ -45,7 +45,7 @@ pub async fn delete( pubky: PubkyHost, path: OriginalUri, ) -> Result { - err_if_user_is_invalid(pubky.public_key(), &state.db)?; + err_if_user_is_invalid(pubky.public_key(), &state.db, false)?; let public_key = pubky.public_key(); let full_path = path.0.path(); let existing_bytes = state.db.get_entry_content_length(public_key, full_path)?; @@ -69,7 +69,7 @@ pub async fn put( path: OriginalUri, body: Body, ) -> Result { - err_if_user_is_invalid(pubky.public_key(), &state.db)?; + err_if_user_is_invalid(pubky.public_key(), &state.db, true)?; let public_key = pubky.public_key(); let full_path = path.0.path(); let existing_entry_bytes = state.db.get_entry_content_length(public_key, full_path)?;