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
This commit is contained in:
aecsocket
2025-11-16 21:49:48 +00:00
committed by GitHub
parent 20484ed7aa
commit 089cca60ce
6 changed files with 219 additions and 106 deletions

View File

@@ -140,9 +140,10 @@ const paypalEmail = computed(() => {
const paypalAuthUrl = computed(() => { const paypalAuthUrl = computed(() => {
const route = useRoute() const route = useRoute()
const authToken = useCookie('auth-token')
const separator = route.fullPath.includes('?') ? '&' : '?' const separator = route.fullPath.includes('?') ? '&' : '?'
const returnUrl = `${route.fullPath}${separator}paypal_auth_return=true` const returnUrl = `${route.fullPath}${separator}paypal_auth_return=true`
return getAuthUrl('paypal', returnUrl) return `${getAuthUrl('paypal', returnUrl)}&auth_token=${authToken.value}`
}) })
function handlePayPalAuth() { function handlePayPalAuth() {

View File

@@ -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"
}

View File

@@ -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"
}

View File

@@ -18,6 +18,8 @@ use thiserror::Error;
#[derive(Error, Debug)] #[derive(Error, Debug)]
pub enum AuthenticationError { pub enum AuthenticationError {
#[error(transparent)]
Internal(#[from] eyre::Report),
#[error("Environment Error")] #[error("Environment Error")]
Env(#[from] dotenvy::Error), Env(#[from] dotenvy::Error),
#[error("An unknown database error occurred: {0}")] #[error("An unknown database error occurred: {0}")]
@@ -53,6 +55,9 @@ pub enum AuthenticationError {
impl actix_web::ResponseError for AuthenticationError { impl actix_web::ResponseError for AuthenticationError {
fn status_code(&self) -> StatusCode { fn status_code(&self) -> StatusCode {
match self { match self {
AuthenticationError::Internal(..) => {
StatusCode::INTERNAL_SERVER_ERROR
}
AuthenticationError::Env(..) => StatusCode::INTERNAL_SERVER_ERROR, AuthenticationError::Env(..) => StatusCode::INTERNAL_SERVER_ERROR,
AuthenticationError::Sqlx(..) => StatusCode::INTERNAL_SERVER_ERROR, AuthenticationError::Sqlx(..) => StatusCode::INTERNAL_SERVER_ERROR,
AuthenticationError::Database(..) => { AuthenticationError::Database(..) => {
@@ -87,6 +92,7 @@ impl actix_web::ResponseError for AuthenticationError {
impl AuthenticationError { impl AuthenticationError {
pub fn error_name(&self) -> &'static str { pub fn error_name(&self) -> &'static str {
match self { match self {
AuthenticationError::Internal(..) => "internal_error",
AuthenticationError::Env(..) => "environment_error", AuthenticationError::Env(..) => "environment_error",
AuthenticationError::Sqlx(..) => "database_error", AuthenticationError::Sqlx(..) => "database_error",
AuthenticationError::Database(..) => "database_error", AuthenticationError::Database(..) => "database_error",

View File

@@ -20,6 +20,7 @@ pub enum DBFlow {
user_id: Option<DBUserId>, user_id: Option<DBUserId>,
url: String, url: String,
provider: AuthProvider, provider: AuthProvider,
existing_user_id: Option<DBUserId>,
}, },
Login2FA { Login2FA {
user_id: DBUserId, user_id: DBUserId,

View File

@@ -2,9 +2,9 @@ use crate::auth::validate::{
get_full_user_from_headers, get_user_record_from_bearer_token, get_full_user_from_headers, get_user_record_from_bearer_token,
}; };
use crate::auth::{AuthProvider, AuthenticationError, get_user_from_headers}; 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::flow_item::DBFlow;
use crate::database::models::notification_item::NotificationBuilder; use crate::database::models::notification_item::NotificationBuilder;
use crate::database::models::{DBUser, DBUserId};
use crate::database::redis::RedisPool; use crate::database::redis::RedisPool;
use crate::file_hosting::{FileHost, FileHostPublicity}; use crate::file_hosting::{FileHost, FileHostPublicity};
use crate::models::notifications::NotificationBody; use crate::models::notifications::NotificationBody;
@@ -16,6 +16,7 @@ use crate::routes::ApiError;
use crate::routes::internal::session::issue_session; use crate::routes::internal::session::issue_session;
use crate::util::captcha::check_hcaptcha; use crate::util::captcha::check_hcaptcha;
use crate::util::env::parse_strings_from_var; use crate::util::env::parse_strings_from_var;
use crate::util::error::Context;
use crate::util::ext::get_image_ext; use crate::util::ext::get_image_ext;
use crate::util::img::upload_image_optimized; use crate::util::img::upload_image_optimized;
use crate::util::validate::validation_errors_to_string; 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 ariadne::ids::random_base62_rng;
use base64::Engine; use base64::Engine;
use chrono::{Duration, Utc}; use chrono::{Duration, Utc};
use eyre::eyre;
use lettre::message::Mailbox; use lettre::message::Mailbox;
use rand_chacha::ChaCha20Rng; use rand_chacha::ChaCha20Rng;
use rand_chacha::rand_core::SeedableRng; use rand_chacha::rand_core::SeedableRng;
@@ -36,6 +38,7 @@ use sqlx::postgres::PgPool;
use std::collections::HashMap; use std::collections::HashMap;
use std::str::FromStr; use std::str::FromStr;
use std::sync::Arc; use std::sync::Arc;
use tracing::info;
use validator::Validate; use validator::Validate;
use zxcvbn::Score; use zxcvbn::Score;
@@ -1046,6 +1049,9 @@ pub struct AuthorizationInit {
#[serde(default)] #[serde(default)]
pub provider: AuthProvider, pub provider: AuthProvider,
pub token: Option<String>, pub token: Option<String>,
/// 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<String>,
} }
#[derive(Serialize, Deserialize)] #[derive(Serialize, Deserialize)]
pub struct Authorization { pub struct Authorization {
@@ -1063,6 +1069,33 @@ pub async fn init(
redis: Data<RedisPool>, redis: Data<RedisPool>,
session_queue: Data<AuthQueue>, session_queue: Data<AuthQueue>,
) -> Result<HttpResponse, AuthenticationError> { ) -> Result<HttpResponse, AuthenticationError> {
// 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 = let url =
url::Url::parse(&info.url).map_err(|_| AuthenticationError::Url)?; url::Url::parse(&info.url).map_err(|_| AuthenticationError::Url)?;
@@ -1095,6 +1128,7 @@ pub async fn init(
user_id, user_id,
url: info.url, url: info.url,
provider: info.provider, provider: info.provider,
existing_user_id,
} }
.insert(Duration::minutes(30), &redis) .insert(Duration::minutes(30), &redis)
.await?; .await?;
@@ -1120,111 +1154,182 @@ pub async fn auth_callback(
let state = state_string.clone(); let state = state_string.clone();
let res: Result<HttpResponse, AuthenticationError> = async move { let res: Result<HttpResponse, AuthenticationError> = 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 // Extract cookie header from request
if let Some(DBFlow::OAuth { let DBFlow::OAuth {
user_id, user_id,
provider, provider,
url, url,
}) = flow existing_user_id,
{ } = flow
DBFlow::remove(&state, &redis).await?; else {
return Err(AuthenticationError::Internal(eyre!(
"invalid flow kind"
)));
};
let token = provider.get_token(query).await?; DBFlow::remove(&state, &redis)
let oauth_user = provider.get_user(&token).await?; .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?; let user_id_opt = provider
if let Some(id) = user_id { .get_user_id(&oauth_user.id, &**client)
if user_id_opt.is_some() { .await
return Err(AuthenticationError::DuplicateUser); .wrap_err("failed to get user ID from provider")?;
}
provider let mut transaction = client
.update_user_id(id, Some(&oauth_user.id), &mut transaction) .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?; .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 { transaction.commit().await?;
sqlx::query!( crate::database::models::DBUser::clear_caches(
" &[(id, None)],
UPDATE users &redis,
SET paypal_country = $1, paypal_email = $2, paypal_id = $3 )
WHERE (id = $4) .await?;
",
oauth_user.country, Ok(HttpResponse::TemporaryRedirect()
oauth_user.email, .append_header(("Location", &*url))
oauth_user.id, .json(serde_json::json!({ "url": url })))
id as crate::database::models::ids::DBUserId, } else {
) let user_id = if let Some(user_id) = user_id_opt {
.execute(&mut *transaction) let user = crate::database::models::DBUser::get_id(
.await?; user_id, &**client, &redis,
} else if let Some(user) = user { )
NotificationBuilder { body: NotificationBody::AuthProviderAdded { provider: provider.as_str().to_string() } } .await?
.insert(user.id, &mut transaction, &redis) .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?; .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?; user_id
crate::database::models::DBUser::clear_caches(&[(id, None)], &redis).await?;
Ok(HttpResponse::TemporaryRedirect()
.append_header(("Location", &*url))
.json(serde_json::json!({ "url": url })))
} else { } else {
let user_id = if let Some(user_id) = user_id_opt { oauth_user
let user = crate::database::models::DBUser::get_id(user_id, &**client, &redis) .create_account(
.await? provider,
.ok_or_else(|| AuthenticationError::InvalidCredentials)?; &mut transaction,
&client,
&file_host,
&redis,
)
.await?
};
if user.totp_secret.is_some() { let session =
let flow = DBFlow::Login2FA { user_id: user.id } issue_session(req, user_id, &mut transaction, &redis).await?;
.insert(Duration::minutes(30), &redis) transaction.commit().await?;
.await?;
let redirect_url = format!( let redirect_url = format!(
"{}{}error=2fa_required&flow={}", "{}{}code={}{}",
url, url,
if url.contains('?') { "&" } else { "?" }, if url.contains('?') { '&' } else { '?' },
flow session.session,
); if user_id_opt.is_none() {
"&new_account=true"
return Ok(HttpResponse::TemporaryRedirect()
.append_header(("Location", &*redirect_url))
.json(serde_json::json!({ "url": redirect_url })));
}
user_id
} else { } else {
oauth_user.create_account(provider, &mut transaction, &client, &file_host, &redis).await? ""
}; }
);
let session = issue_session(req, user_id, &mut transaction, &redis).await?; Ok(HttpResponse::TemporaryRedirect()
transaction.commit().await?; .append_header(("Location", &*redirect_url))
.json(serde_json::json!({ "url": redirect_url })))
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::<HttpResponse, AuthenticationError>(AuthenticationError::InvalidCredentials)
} }
}.await; }
.await;
Ok(res?) Ok(res?)
} }