From 4febec5b8f94602a9f46606786bbd5293786e149 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Mon, 23 Dec 2024 15:13:33 +0100 Subject: [PATCH 1/3] middleware/session: Inline `RequestSession` trait The `session()` fn is only used in a single place, so we might as well inline the code and remove the trait. --- src/auth.rs | 11 ++++++----- src/middleware/session.rs | 13 ------------- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 5d5d314da94..57b979c0877 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1,7 +1,7 @@ use crate::controllers; use crate::controllers::util::RequestPartsExt; use crate::middleware::log_request::RequestLogExt; -use crate::middleware::session::RequestSession; +use crate::middleware::session::SessionExtension; use crate::models::token::{CrateScope, EndpointScope}; use crate::models::{ApiToken, User}; use crate::util::errors::{ @@ -176,11 +176,12 @@ async fn authenticate_via_cookie( parts: &Parts, conn: &mut AsyncPgConnection, ) -> AppResult> { - let user_id_from_session = parts - .session() - .get("user_id") - .and_then(|s| s.parse::().ok()); + let session = parts + .extensions() + .get::() + .expect("missing cookie session"); + let user_id_from_session = session.get("user_id").and_then(|s| s.parse::().ok()); let Some(id) = user_id_from_session else { return Ok(None); }; diff --git a/src/middleware/session.rs b/src/middleware/session.rs index 5a8c04baa37..585f93e7804 100644 --- a/src/middleware/session.rs +++ b/src/middleware/session.rs @@ -1,4 +1,3 @@ -use crate::controllers::util::RequestPartsExt; use axum::extract::{Extension, FromRequestParts, Request}; use axum::middleware::Next; use axum::response::{IntoResponse, Response}; @@ -83,18 +82,6 @@ impl Session { } } -pub trait RequestSession { - fn session(&self) -> &SessionExtension; -} - -impl RequestSession for T { - fn session(&self) -> &SessionExtension { - self.extensions() - .get::() - .expect("missing cookie session") - } -} - pub fn decode(cookie: Cookie<'_>) -> HashMap { let mut ret = HashMap::new(); let bytes = general_purpose::STANDARD From 2fbc4859a43c8f6f0b239d8543167e0fa1b3b4ab Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Mon, 23 Dec 2024 15:18:14 +0100 Subject: [PATCH 2/3] middleware/session: Remove `Deref` derive The `Deref` makes the private parts of this struct publicly available on the outside, which is not what we want, so instead we access the `Arc` directly. --- src/middleware/session.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/middleware/session.rs b/src/middleware/session.rs index 585f93e7804..1c7ba269507 100644 --- a/src/middleware/session.rs +++ b/src/middleware/session.rs @@ -5,7 +5,6 @@ use axum_extra::extract::SignedCookieJar; use base64::{engine::general_purpose, Engine}; use cookie::time::Duration; use cookie::{Cookie, SameSite}; -use derive_more::Deref; use parking_lot::RwLock; use std::collections::HashMap; use std::sync::Arc; @@ -13,7 +12,7 @@ use std::sync::Arc; static COOKIE_NAME: &str = "cargo_session"; static MAX_AGE_DAYS: i64 = 90; -#[derive(Clone, FromRequestParts, Deref)] +#[derive(Clone, FromRequestParts)] #[from_request(via(Extension))] pub struct SessionExtension(Arc>); @@ -23,18 +22,18 @@ impl SessionExtension { } pub fn get(&self, key: &str) -> Option { - let session = self.read(); + let session = self.0.read(); session.data.get(key).cloned() } pub fn insert(&self, key: String, value: String) -> Option { - let mut session = self.write(); + let mut session = self.0.write(); session.dirty = true; session.data.insert(key, value) } pub fn remove(&self, key: &str) -> Option { - let mut session = self.write(); + let mut session = self.0.write(); session.dirty = true; session.data.remove(key) } @@ -53,7 +52,7 @@ pub async fn attach_session(jar: SignedCookieJar, mut req: Request, next: Next) let response = next.run(req).await; // Check if the session data was mutated - let session = session.read(); + let session = session.0.read(); if session.dirty { // Return response with additional `Set-Cookie` header let encoded = encode(&session.data); From f302ddd7b81d61fe26c06a7488f00614b6e88a68 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Mon, 23 Dec 2024 15:30:20 +0100 Subject: [PATCH 3/3] Extract `crates_io_session` crate --- Cargo.lock | 12 ++++++++++++ Cargo.toml | 3 ++- crates/crates_io_session/Cargo.toml | 17 +++++++++++++++++ .../crates_io_session/src/lib.rs | 0 src/auth.rs | 2 +- src/controllers/session.rs | 2 +- src/middleware.rs | 6 ++++-- src/tests/util.rs | 3 +-- 8 files changed, 38 insertions(+), 7 deletions(-) create mode 100644 crates/crates_io_session/Cargo.toml rename src/middleware/session.rs => crates/crates_io_session/src/lib.rs (100%) diff --git a/Cargo.lock b/Cargo.lock index 4565caba05d..a1c6ba669e6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1063,6 +1063,7 @@ dependencies = [ "crates_io_index", "crates_io_markdown", "crates_io_pagerduty", + "crates_io_session", "crates_io_tarball", "crates_io_team_repo", "crates_io_test_db", @@ -1249,6 +1250,17 @@ dependencies = [ "tokio", ] +[[package]] +name = "crates_io_session" +version = "0.0.0" +dependencies = [ + "axum", + "axum-extra", + "base64 0.22.1", + "cookie", + "parking_lot", +] + [[package]] name = "crates_io_smoke_test" version = "0.0.0" diff --git a/Cargo.toml b/Cargo.toml index ebb7cc02a96..10cf104b450 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,7 +49,7 @@ aws-ip-ranges = "=0.962.0" aws-sdk-cloudfront = "=1.57.0" aws-sdk-sqs = "=1.51.0" axum = { version = "=0.7.9", features = ["macros", "matched-path"] } -axum-extra = { version = "=0.9.6", features = ["cookie-signed", "erased-json", "query", "typed-header"] } +axum-extra = { version = "=0.9.6", features = ["erased-json", "query", "typed-header"] } base64 = "=0.22.1" bigdecimal = { version = "=0.4.7", features = ["serde"] } bon = "=3.3.1" @@ -63,6 +63,7 @@ crates_io_github = { path = "crates/crates_io_github" } crates_io_index = { path = "crates/crates_io_index" } crates_io_markdown = { path = "crates/crates_io_markdown" } crates_io_pagerduty = { path = "crates/crates_io_pagerduty" } +crates_io_session = { path = "crates/crates_io_session" } crates_io_tarball = { path = "crates/crates_io_tarball" } crates_io_team_repo = { path = "crates/crates_io_team_repo" } crates_io_worker = { path = "crates/crates_io_worker" } diff --git a/crates/crates_io_session/Cargo.toml b/crates/crates_io_session/Cargo.toml new file mode 100644 index 00000000000..84e44de40b9 --- /dev/null +++ b/crates/crates_io_session/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "crates_io_session" +version = "0.0.0" +license = "MIT OR Apache-2.0" +edition = "2021" + +[lints] +workspace = true + +[dependencies] +axum = { version = "=0.7.9", features = ["macros"] } +axum-extra = { version = "=0.9.6", features = ["cookie-signed"] } +base64 = "=0.22.1" +cookie = { version = "=0.18.1", features = ["secure"] } +parking_lot = "=0.12.3" + +[dev-dependencies] diff --git a/src/middleware/session.rs b/crates/crates_io_session/src/lib.rs similarity index 100% rename from src/middleware/session.rs rename to crates/crates_io_session/src/lib.rs diff --git a/src/auth.rs b/src/auth.rs index 57b979c0877..baa6a1c4f27 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1,7 +1,6 @@ use crate::controllers; use crate::controllers::util::RequestPartsExt; use crate::middleware::log_request::RequestLogExt; -use crate::middleware::session::SessionExtension; use crate::models::token::{CrateScope, EndpointScope}; use crate::models::{ApiToken, User}; use crate::util::errors::{ @@ -9,6 +8,7 @@ use crate::util::errors::{ }; use crate::util::token::HashedToken; use chrono::Utc; +use crates_io_session::SessionExtension; use diesel_async::AsyncPgConnection; use http::header; use http::request::Parts; diff --git a/src/controllers/session.rs b/src/controllers/session.rs index 8889e86984f..c67ffa678d9 100644 --- a/src/controllers/session.rs +++ b/src/controllers/session.rs @@ -11,13 +11,13 @@ use oauth2::{AuthorizationCode, CsrfToken, Scope, TokenResponse}; use crate::app::AppState; use crate::email::Emails; use crate::middleware::log_request::RequestLogExt; -use crate::middleware::session::SessionExtension; use crate::models::{NewUser, User}; use crate::schema::users; use crate::util::diesel::is_read_only_error; use crate::util::errors::{bad_request, server_error, AppResult}; use crate::views::EncodableMe; use crates_io_github::GithubUser; +use crates_io_session::SessionExtension; /// Begin authentication flow. /// diff --git a/src/middleware.rs b/src/middleware.rs index 362eac9f0fb..d223a5b6de6 100644 --- a/src/middleware.rs +++ b/src/middleware.rs @@ -8,7 +8,6 @@ pub mod log_request; pub mod normalize_path; pub mod real_ip; mod require_user_agent; -pub mod session; mod static_or_continue; mod update_metrics; @@ -59,7 +58,10 @@ pub fn apply_axum_middleware(state: AppState, router: Router<()>) -> Router { state.config.cargo_compat_status_code_config, cargo_compat::middleware, )) - .layer(from_fn_with_state(state.clone(), session::attach_session)) + .layer(from_fn_with_state( + state.clone(), + crates_io_session::attach_session, + )) .layer(from_fn_with_state( state.clone(), require_user_agent::require_user_agent, diff --git a/src/tests/util.rs b/src/tests/util.rs index f699cd5fe2e..8d89762ac9b 100644 --- a/src/tests/util.rs +++ b/src/tests/util.rs @@ -19,7 +19,6 @@ //! `MockCookieUser` and `MockTokenUser` provide an `as_model` function which returns a reference //! to the underlying database model value (`User` and `ApiToken` respectively). -use crate::middleware::session; use crate::models::{ApiToken, CreatedApiToken, User}; use crate::tests::{ CategoryListResponse, CategoryResponse, CrateList, CrateResponse, GoodCrate, OwnerResp, @@ -72,7 +71,7 @@ pub fn encode_session_header(session_key: &cookie::Key, user_id: i32) -> String map.insert("user_id".into(), user_id.to_string()); // encode the map into a cookie value string - let encoded = session::encode(&map); + let encoded = crates_io_session::encode(&map); // put the cookie into a signed cookie jar let cookie = Cookie::build((cookie_name, encoded));