From 089cca60ce17faced5e1a5ff9ea39092bc3b2f2b Mon Sep 17 00:00:00 2001 From: aecsocket Date: Sun, 16 Nov 2025 21:49:48 +0000 Subject: [PATCH] Fix PayPal SSO OAuth callback (#4758) * Maybe fix PayPal SSO * cargo sqlx prepare * maybe works * Attempt 2 of fixing * Fix vue * Try adding more logging to flow --- .../LegacyPaypalDetailsStage.vue | 3 +- ...2c47b7a0d23ca747c753a5710b3eb3cf7c621.json | 17 -- ...13edaed963e65d9e8fb9c58c116bba24fe2ac.json | 17 ++ apps/labrinth/src/auth/mod.rs | 6 + .../labrinth/src/database/models/flow_item.rs | 1 + apps/labrinth/src/routes/internal/flows.rs | 281 ++++++++++++------ 6 files changed, 219 insertions(+), 106 deletions(-) delete mode 100644 apps/labrinth/.sqlx/query-0d23c47e3f6803078016c4ae5d52c47b7a0d23ca747c753a5710b3eb3cf7c621.json create mode 100644 apps/labrinth/.sqlx/query-77d39c54d5ca1622b53f029cb4c13edaed963e65d9e8fb9c58c116bba24fe2ac.json diff --git a/apps/frontend/src/components/ui/dashboard/withdraw-stages/LegacyPaypalDetailsStage.vue b/apps/frontend/src/components/ui/dashboard/withdraw-stages/LegacyPaypalDetailsStage.vue index 5f4d3838b..ccc451b9c 100644 --- a/apps/frontend/src/components/ui/dashboard/withdraw-stages/LegacyPaypalDetailsStage.vue +++ b/apps/frontend/src/components/ui/dashboard/withdraw-stages/LegacyPaypalDetailsStage.vue @@ -140,9 +140,10 @@ const paypalEmail = computed(() => { const paypalAuthUrl = computed(() => { const route = useRoute() + const authToken = useCookie('auth-token') const separator = route.fullPath.includes('?') ? '&' : '?' const returnUrl = `${route.fullPath}${separator}paypal_auth_return=true` - return getAuthUrl('paypal', returnUrl) + return `${getAuthUrl('paypal', returnUrl)}&auth_token=${authToken.value}` }) function handlePayPalAuth() { diff --git a/apps/labrinth/.sqlx/query-0d23c47e3f6803078016c4ae5d52c47b7a0d23ca747c753a5710b3eb3cf7c621.json b/apps/labrinth/.sqlx/query-0d23c47e3f6803078016c4ae5d52c47b7a0d23ca747c753a5710b3eb3cf7c621.json deleted file mode 100644 index 1b911274b..000000000 --- a/apps/labrinth/.sqlx/query-0d23c47e3f6803078016c4ae5d52c47b7a0d23ca747c753a5710b3eb3cf7c621.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n UPDATE users\n SET paypal_country = $1, paypal_email = $2, paypal_id = $3\n WHERE (id = $4)\n ", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Text", - "Text", - "Text", - "Int8" - ] - }, - "nullable": [] - }, - "hash": "0d23c47e3f6803078016c4ae5d52c47b7a0d23ca747c753a5710b3eb3cf7c621" -} diff --git a/apps/labrinth/.sqlx/query-77d39c54d5ca1622b53f029cb4c13edaed963e65d9e8fb9c58c116bba24fe2ac.json b/apps/labrinth/.sqlx/query-77d39c54d5ca1622b53f029cb4c13edaed963e65d9e8fb9c58c116bba24fe2ac.json new file mode 100644 index 000000000..acc7285b8 --- /dev/null +++ b/apps/labrinth/.sqlx/query-77d39c54d5ca1622b53f029cb4c13edaed963e65d9e8fb9c58c116bba24fe2ac.json @@ -0,0 +1,17 @@ +{ + "db_name": "PostgreSQL", + "query": "\n UPDATE users\n SET paypal_country = $1, paypal_email = $2, paypal_id = $3\n WHERE id = $4\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Text", + "Int8" + ] + }, + "nullable": [] + }, + "hash": "77d39c54d5ca1622b53f029cb4c13edaed963e65d9e8fb9c58c116bba24fe2ac" +} diff --git a/apps/labrinth/src/auth/mod.rs b/apps/labrinth/src/auth/mod.rs index 2dc9311dc..953d978c5 100644 --- a/apps/labrinth/src/auth/mod.rs +++ b/apps/labrinth/src/auth/mod.rs @@ -18,6 +18,8 @@ use thiserror::Error; #[derive(Error, Debug)] pub enum AuthenticationError { + #[error(transparent)] + Internal(#[from] eyre::Report), #[error("Environment Error")] Env(#[from] dotenvy::Error), #[error("An unknown database error occurred: {0}")] @@ -53,6 +55,9 @@ pub enum AuthenticationError { impl actix_web::ResponseError for AuthenticationError { fn status_code(&self) -> StatusCode { match self { + AuthenticationError::Internal(..) => { + StatusCode::INTERNAL_SERVER_ERROR + } AuthenticationError::Env(..) => StatusCode::INTERNAL_SERVER_ERROR, AuthenticationError::Sqlx(..) => StatusCode::INTERNAL_SERVER_ERROR, AuthenticationError::Database(..) => { @@ -87,6 +92,7 @@ impl actix_web::ResponseError for AuthenticationError { impl AuthenticationError { pub fn error_name(&self) -> &'static str { match self { + AuthenticationError::Internal(..) => "internal_error", AuthenticationError::Env(..) => "environment_error", AuthenticationError::Sqlx(..) => "database_error", AuthenticationError::Database(..) => "database_error", diff --git a/apps/labrinth/src/database/models/flow_item.rs b/apps/labrinth/src/database/models/flow_item.rs index 192823d86..b7b853d9d 100644 --- a/apps/labrinth/src/database/models/flow_item.rs +++ b/apps/labrinth/src/database/models/flow_item.rs @@ -20,6 +20,7 @@ pub enum DBFlow { user_id: Option, url: String, provider: AuthProvider, + existing_user_id: Option, }, Login2FA { user_id: DBUserId, diff --git a/apps/labrinth/src/routes/internal/flows.rs b/apps/labrinth/src/routes/internal/flows.rs index 246f05349..bc02b05fe 100644 --- a/apps/labrinth/src/routes/internal/flows.rs +++ b/apps/labrinth/src/routes/internal/flows.rs @@ -2,9 +2,9 @@ use crate::auth::validate::{ get_full_user_from_headers, get_user_record_from_bearer_token, }; use crate::auth::{AuthProvider, AuthenticationError, get_user_from_headers}; -use crate::database::models::DBUser; use crate::database::models::flow_item::DBFlow; use crate::database::models::notification_item::NotificationBuilder; +use crate::database::models::{DBUser, DBUserId}; use crate::database::redis::RedisPool; use crate::file_hosting::{FileHost, FileHostPublicity}; use crate::models::notifications::NotificationBody; @@ -16,6 +16,7 @@ use crate::routes::ApiError; use crate::routes::internal::session::issue_session; use crate::util::captcha::check_hcaptcha; use crate::util::env::parse_strings_from_var; +use crate::util::error::Context; use crate::util::ext::get_image_ext; use crate::util::img::upload_image_optimized; use crate::util::validate::validation_errors_to_string; @@ -27,6 +28,7 @@ use ariadne::ids::base62_impl::{parse_base62, to_base62}; use ariadne::ids::random_base62_rng; use base64::Engine; use chrono::{Duration, Utc}; +use eyre::eyre; use lettre::message::Mailbox; use rand_chacha::ChaCha20Rng; use rand_chacha::rand_core::SeedableRng; @@ -36,6 +38,7 @@ use sqlx::postgres::PgPool; use std::collections::HashMap; use std::str::FromStr; use std::sync::Arc; +use tracing::info; use validator::Validate; use zxcvbn::Score; @@ -1046,6 +1049,9 @@ pub struct AuthorizationInit { #[serde(default)] pub provider: AuthProvider, pub token: Option, + /// If the user is already logged in, and is linking a PayPal account, + /// this will be set to the user's auth token from the frontend. + pub auth_token: Option, } #[derive(Serialize, Deserialize)] pub struct Authorization { @@ -1063,6 +1069,33 @@ pub async fn init( redis: Data, session_queue: Data, ) -> Result { + // If a user is logging into an OAuth method while already logged in, + // this may be present. + // + // This can happen when linking to a PayPal account (logging in) when already + // logged in. + let existing_user_id = if let Some(auth_token) = &info.auth_token { + get_user_record_from_bearer_token( + &req, + Some(auth_token), + &**client, + &redis, + &session_queue, + ) + .await + .ok() + .flatten() + .map(|(_scopes, user)| user.id) + } else { + None + }; + + info!( + ?existing_user_id, + auth_token_present = %info.auth_token.is_some(), + "Starting authentication flow" + ); + let url = url::Url::parse(&info.url).map_err(|_| AuthenticationError::Url)?; @@ -1095,6 +1128,7 @@ pub async fn init( user_id, url: info.url, provider: info.provider, + existing_user_id, } .insert(Duration::minutes(30), &redis) .await?; @@ -1120,111 +1154,182 @@ pub async fn auth_callback( let state = state_string.clone(); let res: Result = async move { - let flow = DBFlow::get(&state, &redis).await?; + let flow = DBFlow::get(&state, &redis) + .await + .wrap_err("failed to fetch flow state")? + .wrap_err("no flow for state")?; // Extract cookie header from request - if let Some(DBFlow::OAuth { - user_id, - provider, - url, - }) = flow - { - DBFlow::remove(&state, &redis).await?; + let DBFlow::OAuth { + user_id, + provider, + url, + existing_user_id, + } = flow + else { + return Err(AuthenticationError::Internal(eyre!( + "invalid flow kind" + ))); + }; - let token = provider.get_token(query).await?; - let oauth_user = provider.get_user(&token).await?; + DBFlow::remove(&state, &redis) + .await + .wrap_err("failed to remove flow")?; - let user_id_opt = provider.get_user_id(&oauth_user.id, &**client).await?; + let token = provider + .get_token(query) + .await + .wrap_err("failed to get token from provider")?; + let oauth_user = provider + .get_user(&token) + .await + .wrap_err("failed to get user from provider")?; - let mut transaction = client.begin().await?; - if let Some(id) = user_id { - if user_id_opt.is_some() { - return Err(AuthenticationError::DuplicateUser); - } + let user_id_opt = provider + .get_user_id(&oauth_user.id, &**client) + .await + .wrap_err("failed to get user ID from provider")?; - provider - .update_user_id(id, Some(&oauth_user.id), &mut transaction) + let mut transaction = client + .begin() + .await + .wrap_err("failed to begin transaction")?; + + // PayPal isn't actually an SSO method; we allow users to link their PayPal + // account to their Modrinth account via this OAuth flow. However, we MUST + // NOT actually create an account with their username. + // + // Instead, we check who they're already logged in as, and just update + // the PayPal info for that account. + if provider == AuthProvider::PayPal { + let existing_user_id = existing_user_id.wrap_err( + "attempting to link a PayPal account without being logged in", + )?; + + sqlx::query!( + " + UPDATE users + SET paypal_country = $1, paypal_email = $2, paypal_id = $3 + WHERE id = $4 + ", + oauth_user.country, + oauth_user.email, + oauth_user.id, + existing_user_id as DBUserId, + ) + .execute(&mut *transaction) + .await + .wrap_err("failed to update user PayPal info")?; + + transaction + .commit() + .await + .wrap_err("failed to commit transaction")?; + crate::database::models::DBUser::clear_caches( + &[(existing_user_id, None)], + &redis, + ) + .await + .wrap_err("failed to clear user caches")?; + + return Ok(HttpResponse::TemporaryRedirect() + .append_header(("Location", &*url)) + .json(serde_json::json!({ "url": url }))); + } + + if let Some(id) = user_id { + if user_id_opt.is_some() { + return Err(AuthenticationError::DuplicateUser); + } + + provider + .update_user_id(id, Some(&oauth_user.id), &mut transaction) + .await?; + + let user = + crate::database::models::DBUser::get_id(id, &**client, &redis) .await?; - let user = crate::database::models::DBUser::get_id(id, &**client, &redis).await?; + if let Some(user) = user { + NotificationBuilder { + body: NotificationBody::AuthProviderAdded { + provider: provider.as_str().to_string(), + }, + } + .insert(user.id, &mut transaction, &redis) + .await?; + } - if provider == AuthProvider::PayPal { - sqlx::query!( - " - UPDATE users - SET paypal_country = $1, paypal_email = $2, paypal_id = $3 - WHERE (id = $4) - ", - oauth_user.country, - oauth_user.email, - oauth_user.id, - id as crate::database::models::ids::DBUserId, - ) - .execute(&mut *transaction) - .await?; - } else if let Some(user) = user { - NotificationBuilder { body: NotificationBody::AuthProviderAdded { provider: provider.as_str().to_string() } } - .insert(user.id, &mut transaction, &redis) + transaction.commit().await?; + crate::database::models::DBUser::clear_caches( + &[(id, None)], + &redis, + ) + .await?; + + Ok(HttpResponse::TemporaryRedirect() + .append_header(("Location", &*url)) + .json(serde_json::json!({ "url": url }))) + } else { + let user_id = if let Some(user_id) = user_id_opt { + let user = crate::database::models::DBUser::get_id( + user_id, &**client, &redis, + ) + .await? + .ok_or_else(|| AuthenticationError::InvalidCredentials)?; + + if user.totp_secret.is_some() { + let flow = DBFlow::Login2FA { user_id: user.id } + .insert(Duration::minutes(30), &redis) .await?; + + let redirect_url = format!( + "{}{}error=2fa_required&flow={}", + url, + if url.contains('?') { "&" } else { "?" }, + flow + ); + + return Ok(HttpResponse::TemporaryRedirect() + .append_header(("Location", &*redirect_url)) + .json(serde_json::json!({ "url": redirect_url }))); } - transaction.commit().await?; - crate::database::models::DBUser::clear_caches(&[(id, None)], &redis).await?; - - Ok(HttpResponse::TemporaryRedirect() - .append_header(("Location", &*url)) - .json(serde_json::json!({ "url": url }))) + user_id } else { - let user_id = if let Some(user_id) = user_id_opt { - let user = crate::database::models::DBUser::get_id(user_id, &**client, &redis) - .await? - .ok_or_else(|| AuthenticationError::InvalidCredentials)?; + oauth_user + .create_account( + provider, + &mut transaction, + &client, + &file_host, + &redis, + ) + .await? + }; - if user.totp_secret.is_some() { - let flow = DBFlow::Login2FA { user_id: user.id } - .insert(Duration::minutes(30), &redis) - .await?; + let session = + issue_session(req, user_id, &mut transaction, &redis).await?; + transaction.commit().await?; - let redirect_url = format!( - "{}{}error=2fa_required&flow={}", - url, - if url.contains('?') { "&" } else { "?" }, - flow - ); - - return Ok(HttpResponse::TemporaryRedirect() - .append_header(("Location", &*redirect_url)) - .json(serde_json::json!({ "url": redirect_url }))); - } - - user_id + let redirect_url = format!( + "{}{}code={}{}", + url, + if url.contains('?') { '&' } else { '?' }, + session.session, + if user_id_opt.is_none() { + "&new_account=true" } else { - oauth_user.create_account(provider, &mut transaction, &client, &file_host, &redis).await? - }; + "" + } + ); - let session = issue_session(req, user_id, &mut transaction, &redis).await?; - transaction.commit().await?; - - let redirect_url = format!( - "{}{}code={}{}", - url, - if url.contains('?') { '&' } else { '?' }, - session.session, - if user_id_opt.is_none() { - "&new_account=true" - } else { - "" - } - ); - - Ok(HttpResponse::TemporaryRedirect() - .append_header(("Location", &*redirect_url)) - .json(serde_json::json!({ "url": redirect_url }))) - } - } else { - Err::(AuthenticationError::InvalidCredentials) + Ok(HttpResponse::TemporaryRedirect() + .append_header(("Location", &*redirect_url)) + .json(serde_json::json!({ "url": redirect_url }))) } - }.await; + } + .await; Ok(res?) }