From 76c63148152b7591b7c93688175bed8e8da4b8aa Mon Sep 17 00:00:00 2001 From: isxander Date: Sun, 8 Feb 2026 13:56:14 +0000 Subject: [PATCH] fix: session refresh works as intended now --- apps/labrinth/src/auth/validate.rs | 34 ++++++++++-- .../src/database/models/session_item.rs | 52 +++++++++++++++++++ apps/labrinth/src/routes/internal/admin.rs | 1 + apps/labrinth/src/routes/internal/flows.rs | 14 +++-- apps/labrinth/src/routes/internal/session.rs | 45 +++++++++++----- apps/labrinth/src/routes/internal/statuses.rs | 1 + apps/labrinth/src/routes/v3/payouts.rs | 2 + 7 files changed, 129 insertions(+), 20 deletions(-) diff --git a/apps/labrinth/src/auth/validate.rs b/apps/labrinth/src/auth/validate.rs index fe3644ebd2..d6eec59489 100644 --- a/apps/labrinth/src/auth/validate.rs +++ b/apps/labrinth/src/auth/validate.rs @@ -31,6 +31,7 @@ where executor, redis, session_queue, + false, ) .await? else { @@ -60,6 +61,7 @@ where executor, redis, session_queue, + false, ) .await? .ok_or_else(|| AuthenticationError::InvalidCredentials)?; @@ -94,12 +96,38 @@ where Ok((scopes, User::from_full(db_user))) } +pub async fn get_user_from_bearer_token<'a, E>( + req: &HttpRequest, + token: Option<&str>, + executor: E, + redis: &RedisPool, + session_queue: &AuthQueue, + allow_expired: bool, +) -> Result<(Scopes, User), AuthenticationError> +where + E: crate::database::Executor<'a, Database = sqlx::Postgres> + Copy, +{ + let (scopes, db_user) = get_user_record_from_bearer_token( + req, + token, + executor, + redis, + session_queue, + allow_expired, + ) + .await? + .ok_or_else(|| AuthenticationError::InvalidCredentials)?; + + Ok((scopes, User::from_full(db_user))) +} + pub async fn get_user_record_from_bearer_token<'a, 'b, E>( req: &HttpRequest, token: Option<&str>, executor: E, redis: &RedisPool, session_queue: &AuthQueue, + allow_expired: bool, ) -> Result, AuthenticationError> where E: crate::database::Executor<'a, Database = sqlx::Postgres> + Copy, @@ -119,7 +147,7 @@ where .await? .ok_or_else(|| AuthenticationError::InvalidCredentials)?; - if pat.expires < Utc::now() { + if !allow_expired && pat.expires < Utc::now() { return Err(AuthenticationError::InvalidCredentials); } @@ -138,7 +166,7 @@ where .await? .ok_or_else(|| AuthenticationError::InvalidCredentials)?; - if session.expires < Utc::now() { + if !allow_expired && session.expires < Utc::now() { return Err(AuthenticationError::InvalidCredentials); } @@ -168,7 +196,7 @@ where .await? .ok_or(AuthenticationError::InvalidCredentials)?; - if access_token.expires < Utc::now() { + if !allow_expired && access_token.expires < Utc::now() { return Err(AuthenticationError::InvalidCredentials); } diff --git a/apps/labrinth/src/database/models/session_item.rs b/apps/labrinth/src/database/models/session_item.rs index e835bb15ce..573be8fb2d 100644 --- a/apps/labrinth/src/database/models/session_item.rs +++ b/apps/labrinth/src/database/models/session_item.rs @@ -25,6 +25,11 @@ pub struct SessionBuilder { pub ip: String, pub user_agent: String, + + // When None, database default of 14 days will be used + pub expires: Option>, + // When None, database default of 60 days will be used + pub session_expires: Option>, } impl SessionBuilder { @@ -58,6 +63,53 @@ impl SessionBuilder { .execute(&mut *transaction) .await?; + // If we put these Option values into the first query, and they are None, + // it will enter into the DB as NULL, when the desired behavior is database + // default. + match (self.expires, self.session_expires) { + (None, None) => {} + (Some(expires), Some(session_expires)) => { + sqlx::query!( + " + UPDATE sessions + SET expires = $1, refresh_expires = $2 + WHERE id = $3 + ", + expires, + session_expires, + id as DBSessionId, + ) + .execute(&mut *transaction) + .await?; + } + (Some(expires), None) => { + sqlx::query!( + " + UPDATE sessions + SET expires = $1 + WHERE id = $2 + ", + expires, + id as DBSessionId, + ) + .execute(&mut *transaction) + .await?; + } + (None, Some(session_expires)) => { + sqlx::query!( + " + UPDATE sessions + SET refresh_expires = $1 + WHERE id = $2 + ", + session_expires, + id as DBSessionId, + ) + .execute(&mut *transaction) + .await?; + } + } + Ok(id) } } diff --git a/apps/labrinth/src/routes/internal/admin.rs b/apps/labrinth/src/routes/internal/admin.rs index ecc230cfff..c2f2c8cf3a 100644 --- a/apps/labrinth/src/routes/internal/admin.rs +++ b/apps/labrinth/src/routes/internal/admin.rs @@ -57,6 +57,7 @@ pub async fn count_download( &**pool, &redis, &session_queue, + false, ) .await .ok() diff --git a/apps/labrinth/src/routes/internal/flows.rs b/apps/labrinth/src/routes/internal/flows.rs index 84510685bf..2353a296b5 100644 --- a/apps/labrinth/src/routes/internal/flows.rs +++ b/apps/labrinth/src/routes/internal/flows.rs @@ -1082,6 +1082,7 @@ pub async fn init( &**client, &redis, &session_queue, + false, ) .await .ok() @@ -1116,6 +1117,7 @@ pub async fn init( &**client, &redis, &session_queue, + false, ) .await? .ok_or_else(|| AuthenticationError::InvalidCredentials)?; @@ -1310,7 +1312,8 @@ pub async fn auth_callback( }; let session = - issue_session(req, user_id, &mut transaction, &redis).await?; + issue_session(req, user_id, &mut transaction, &redis, None) + .await?; transaction.commit().await?; let redirect_url = format!( @@ -1535,7 +1538,8 @@ pub async fn create_account_with_password( .insert(&mut transaction) .await?; - let session = issue_session(req, user_id, &mut transaction, &redis).await?; + let session = + issue_session(req, user_id, &mut transaction, &redis, None).await?; let res = crate::models::sessions::Session::from(session, true, None); let mailbox: Mailbox = new_account.email.parse().map_err(|_| { @@ -1629,7 +1633,7 @@ pub async fn login_password( } else { let mut transaction = pool.begin().await?; let session = - issue_session(req, user.id, &mut transaction, &redis).await?; + issue_session(req, user.id, &mut transaction, &redis, None).await?; let res = crate::models::sessions::Session::from(session, true, None); transaction.commit().await?; @@ -1759,7 +1763,7 @@ pub async fn login_2fa( DBFlow::remove(&login.flow, &redis).await?; let session = - issue_session(req, user_id, &mut transaction, &redis).await?; + issue_session(req, user_id, &mut transaction, &redis, None).await?; let res = crate::models::sessions::Session::from(session, true, None); transaction.commit().await?; @@ -1947,6 +1951,7 @@ pub async fn remove_2fa( &**pool, &redis, &session_queue, + false, ) .await? .ok_or_else(|| AuthenticationError::InvalidCredentials)?; @@ -2152,6 +2157,7 @@ pub async fn change_password( &**pool, &redis, &session_queue, + false, ) .await? .ok_or_else(|| AuthenticationError::InvalidCredentials)?; diff --git a/apps/labrinth/src/routes/internal/session.rs b/apps/labrinth/src/routes/internal/session.rs index d41cfd9edb..ee11b5957a 100644 --- a/apps/labrinth/src/routes/internal/session.rs +++ b/apps/labrinth/src/routes/internal/session.rs @@ -1,3 +1,4 @@ +use crate::auth::validate::get_user_from_bearer_token; use crate::auth::{AuthenticationError, get_user_from_headers}; use crate::database::models::DBUserId; use crate::database::models::session_item::DBSession; @@ -12,7 +13,7 @@ use crate::util::env::parse_var; use actix_web::http::header::AUTHORIZATION; use actix_web::web::{Data, ServiceConfig, scope}; use actix_web::{HttpRequest, HttpResponse, delete, get, post, web}; -use chrono::Utc; +use chrono::{DateTime, Utc}; use rand::distributions::Alphanumeric; use rand::{Rng, SeedableRng}; use rand_chacha::ChaCha20Rng; @@ -88,6 +89,7 @@ pub async fn issue_session( user_id: DBUserId, transaction: &mut PgTransaction<'_>, redis: &RedisPool, + session_expires: Option>, ) -> Result { let metadata = get_session_metadata(&req).await?; @@ -108,6 +110,8 @@ pub async fn issue_session( country: metadata.country, ip: metadata.ip, user_agent: metadata.user_agent, + expires: None, + session_expires, } .insert(transaction) .await?; @@ -212,15 +216,6 @@ pub async fn refresh( redis: Data, session_queue: Data, ) -> Result { - let current_user = get_user_from_headers( - &req, - &**pool, - &redis, - &session_queue, - Scopes::empty(), - ) - .await? - .1; let session = req .headers() .get(AUTHORIZATION) @@ -229,6 +224,25 @@ pub async fn refresh( ApiError::Authentication(AuthenticationError::InvalidCredentials) })?; + // We should ensure that the authorization given is a session token, and not some other type of token (like a PAT), since this endpoint is only for refreshing sessions. + // This is done by checking the prefix of the token, which should be "mra_" for session tokens. + if !session.starts_with("mra_") { + return Err(ApiError::Authentication( + AuthenticationError::InvalidCredentials, + )); + } + + let current_user = get_user_from_bearer_token( + &req, + Some(session), + &**pool, + &redis, + &session_queue, + true, // Allow expired sessions, since we want to allow refreshing expired sessions + ) + .await? + .1; + let session = DBSession::get(session, &**pool, &redis).await?; if let Some(session) = session { @@ -243,9 +257,14 @@ pub async fn refresh( let mut transaction = pool.begin().await?; DBSession::remove(session.id, &mut transaction).await?; - let new_session = - issue_session(req, session.user_id, &mut transaction, &redis) - .await?; + let new_session = issue_session( + req, + session.user_id, + &mut transaction, + &redis, + Some(session.refresh_expires), + ) + .await?; transaction.commit().await?; DBSession::clear_cache( vec![( diff --git a/apps/labrinth/src/routes/internal/statuses.rs b/apps/labrinth/src/routes/internal/statuses.rs index f8196fec9a..8e676eb048 100644 --- a/apps/labrinth/src/routes/internal/statuses.rs +++ b/apps/labrinth/src/routes/internal/statuses.rs @@ -58,6 +58,7 @@ pub async fn ws_init( &**pool, &redis, &session_queue, + false, ) .await? .ok_or_else(|| { diff --git a/apps/labrinth/src/routes/v3/payouts.rs b/apps/labrinth/src/routes/v3/payouts.rs index 667c32ddaf..68b2b6cd44 100644 --- a/apps/labrinth/src/routes/v3/payouts.rs +++ b/apps/labrinth/src/routes/v3/payouts.rs @@ -439,6 +439,7 @@ pub async fn calculate_fees( &**pool, &redis, &session_queue, + false, ) .await? .ok_or_else(|| { @@ -471,6 +472,7 @@ pub async fn create_payout( &**pool, &redis, &session_queue, + false, ) .await? .ok_or_else(|| {