Skip to content

refactor(api): remove EmailService #228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sergey3bv wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor(api): remove EmailService #228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 9 additions & 15 deletions api/src/api/http/admin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,17 +647,11 @@ pub async fn batch_create_claim_tokens(
let user_repo = UserRepository::new(pool.clone());
let claim_token_repo = ClaimTokenRepository::new(pool.clone());

// Create email service once outside the loop
let email_service =
req.delivery_email
.as_ref()
.and_then(|_| match crate::email_service::EmailService::new() {
Ok(svc) => Some(svc),
Err(e) => {
tracing::error!("Failed to create email service: {}", e);
None
}
});
// Create email sender reference when batch delivery is requested
let email_sender_for_delivery = req
.delivery_email
.as_ref()
.map(|_| auth_state.state.email_sender.clone());

let mut tokens = Vec::new();
let mut skipped = Vec::new();
Expand Down Expand Up @@ -740,8 +734,10 @@ pub async fn batch_create_claim_tokens(
let claim_url = format!("{}/api/claim?token={}", app_url, token);

// Send email if requested
if let (Some(email), Some(svc)) = (&req.delivery_email, &email_service) {
if let Err(e) = svc.send_claim_email(email, &claim_url).await {
if let (Some(email), Some(sender)) =
(&req.delivery_email, email_sender_for_delivery.as_ref())
{
if let Err(e) = sender.send_claim_email(email, &claim_url).await {
tracing::warn!(
"Failed to send claim email for vine_id={} to {}: {}",
vine_id,
Expand All @@ -750,8 +746,6 @@ pub async fn batch_create_claim_tokens(
);
errors.push(format!("vine_id {}: email delivery failed: {}", vine_id, e));
}
} else if req.delivery_email.is_some() && email_service.is_none() {
errors.push(format!("vine_id {}: email service unavailable", vine_id));
}

tokens.push(BatchClaimTokenEntry {
Expand Down
108 changes: 43 additions & 65 deletions api/src/api/http/auth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -905,24 +905,15 @@ pub async fn register(
METRICS.inc_registration();

// Send verification email (required - user must verify before login)
match crate::email_service::EmailService::new() {
Ok(email_service) => {
if let Err(e) = email_service
.send_verification_email(&req.email, &verification_token)
.await
{
tracing::error!("Failed to send verification email to {}: {}", req.email, e);
// Continue even if email fails - user can resend later
} else {
tracing::info!("Sent verification email to {}", req.email);
}
}
Err(e) => {
tracing::warn!(
"Email service unavailable, skipping verification email: {}",
e
);
}
let email_sender = auth_state.state.email_sender.clone();
if let Err(e) = email_sender
.send_verification_email(&req.email, &verification_token)
.await
{
tracing::error!("Failed to send verification email to {}: {}", req.email, e);
// Continue even if email fails - user can resend later
} else {
tracing::info!("Sent verification email to {}", req.email);
}

tracing::info!(
Expand Down Expand Up @@ -1875,10 +1866,11 @@ pub struct ResendVerificationResponse {
/// Always returns success to prevent email enumeration attacks.
pub async fn resend_verification(
tenant: crate::api::tenant::TenantExtractor,
State(pool): State<PgPool>,
State(auth_state): State<super::routes::AuthState>,
headers: HeaderMap,
Json(mut req): Json<ResendVerificationRequest>,
) -> Json<ResendVerificationResponse> {
let pool = auth_state.state.db.clone();
let tenant_id = tenant.0.id;
if let Some(ref mut email) = req.email {
*email = email.to_lowercase();
Expand Down Expand Up @@ -1966,27 +1958,21 @@ pub async fn resend_verification(
}

// Send verification email (don't await to prevent timing attacks)
let email_sender = auth_state.state.email_sender.clone();
let email_clone = email.clone();
let token_clone = verification_token.clone();
tokio::spawn(async move {
match crate::email_service::EmailService::new() {
Ok(email_service) => {
if let Err(e) = email_service
.send_verification_email(&email_clone, &token_clone)
.await
{
tracing::error!(
"Failed to send verification email to {}: {}",
email_clone,
e
);
} else {
tracing::info!("Sent verification email to {}", email_clone);
}
}
Err(e) => {
tracing::warn!("Email service unavailable: {}", e);
}
if let Err(e) = email_sender
.send_verification_email(&email_clone, &token_clone)
.await
{
tracing::error!(
"Failed to send verification email to {}: {}",
email_clone,
e
);
} else {
tracing::info!("Sent verification email to {}", email_clone);
}
});

Expand All @@ -1996,10 +1982,11 @@ pub async fn resend_verification(
/// Request password reset email
pub async fn forgot_password(
tenant: crate::api::tenant::TenantExtractor,
State(pool): State<PgPool>,
State(auth_state): State<super::routes::AuthState>,
headers: HeaderMap,
Json(mut req): Json<ForgotPasswordRequest>,
) -> Result<Json<ForgotPasswordResponse>, AuthError> {
let pool = auth_state.state.db.clone();
let tenant_id = tenant.0.id;
let endpoint = "/api/auth/forgot-password";
req.email = req.email.to_lowercase();
Expand Down Expand Up @@ -2062,32 +2049,20 @@ pub async fn forgot_password(
.set_password_reset_token(&public_key, tenant_id, &reset_token, reset_expires)
.await?;

// Send password reset email (optional - don't fail if email service unavailable)
match crate::email_service::EmailService::new() {
Ok(email_service) => {
if let Err(e) = email_service
.send_password_reset_email(&req.email, &reset_token)
.await
{
METRICS.inc_auth_email_send_failure("password_reset");
reason_code = Some("email_send_failed");
tracing::error!(
"Failed to send password reset email to {}: {}",
req.email,
e
);
} else {
tracing::info!("Sent password reset email to {}", req.email);
}
}
Err(e) => {
METRICS.inc_auth_email_send_failure("password_reset");
reason_code = Some("email_send_failed");
tracing::warn!(
"Email service unavailable, skipping password reset email: {}",
e
);
}
let email_sender = auth_state.state.email_sender.clone();
if let Err(e) = email_sender
.send_password_reset_email(&req.email, &reset_token)
.await
{
METRICS.inc_auth_email_send_failure("password_reset");
reason_code = Some("email_send_failed");
tracing::error!(
"Failed to send password reset email to {}: {}",
req.email,
e
);
} else {
tracing::info!("Sent password reset email to {}", req.email);
}

super::auth_observability::record_auth_event_and_log(
Expand Down Expand Up @@ -2120,10 +2095,11 @@ pub async fn forgot_password(
/// Reset password with token
pub async fn reset_password(
tenant: crate::api::tenant::TenantExtractor,
State(pool): State<PgPool>,
State(auth_state): State<super::routes::AuthState>,
headers: HeaderMap,
Json(req): Json<ResetPasswordRequest>,
) -> Result<Json<ResetPasswordResponse>, AuthError> {
let pool = auth_state.state.db.clone();
let tenant_id = tenant.0.id;
let endpoint = "/api/auth/reset-password";
tracing::info!(
Expand Down Expand Up @@ -3830,6 +3806,7 @@ mod tests {
bcrypt_sender: bcrypt_queue.sender(),
redis: None,
secret_pool: secret_pool.receiver(),
email_sender: std::sync::Arc::new(crate::email_service::DevEmailSender::new()),
}),
auth_tx: None,
}
Expand Down Expand Up @@ -3935,6 +3912,7 @@ mod tests {
bcrypt_sender: bcrypt_queue.sender(),
redis: None,
secret_pool: secret_pool.receiver(),
email_sender: std::sync::Arc::new(crate::email_service::DevEmailSender::new()),
}),
auth_tx: None,
}
Expand Down
28 changes: 10 additions & 18 deletions api/src/api/http/headless.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,24 +209,15 @@ pub async fn headless_register(
.await?;

// Send verification email
match crate::email_service::EmailService::new() {
Ok(email_service) => {
if let Err(e) = email_service
.send_verification_email(&req.email, &verification_token)
.await
{
tracing::error!("Failed to send verification email to {}: {}", req.email, e);
// Continue - user can request resend later
} else {
tracing::info!("Sent verification email to {}", req.email);
}
}
Err(e) => {
tracing::warn!(
"Email service unavailable, skipping verification email: {}",
e
);
}
let email_sender = auth_state.state.email_sender.clone();
if let Err(e) = email_sender
.send_verification_email(&req.email, &verification_token)
.await
{
tracing::error!("Failed to send verification email to {}: {}", req.email, e);
// Continue - user can request resend later
} else {
tracing::info!("Sent verification email to {}", req.email);
}

// Track successful registration
Expand Down Expand Up @@ -757,6 +748,7 @@ mod tests {
bcrypt_sender: bcrypt_queue.sender(),
redis: None,
secret_pool: secret_pool.receiver(),
email_sender: std::sync::Arc::new(crate::email_service::DevEmailSender::new()),
}),
auth_tx: None,
}
Expand Down
61 changes: 22 additions & 39 deletions api/src/api/http/oauth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2599,27 +2599,18 @@ async fn handle_authorization_code_grant(
);

// Send verification email (optional - don't fail if email service unavailable)
match crate::email_service::EmailService::new() {
Ok(email_service) => {
if let Err(e) = email_service
.send_verification_email(pending_email_val, &verification_token)
.await
{
tracing::error!(
"Failed to send verification email to {}: {}",
pending_email_val,
e
);
} else {
tracing::info!("Sent verification email to {}", pending_email_val);
}
}
Err(e) => {
tracing::warn!(
"Email service unavailable, skipping verification email: {}",
e
);
}
let email_sender = auth_state.state.email_sender.clone();
if let Err(e) = email_sender
.send_verification_email(pending_email_val, &verification_token)
.await
{
tracing::error!(
"Failed to send verification email to {}: {}",
pending_email_val,
e
);
} else {
tracing::info!("Sent verification email to {}", pending_email_val);
}

pending_email_val.clone()
Expand Down Expand Up @@ -3357,24 +3348,15 @@ pub async fn oauth_register(
);

// Send verification email (required - user must verify before OAuth flow completes)
match crate::email_service::EmailService::new() {
Ok(email_service) => {
if let Err(e) = email_service
.send_verification_email(&req.email, &verification_token)
.await
{
tracing::error!("Failed to send verification email to {}: {}", req.email, e);
// Continue even if email fails - user can resend later
} else {
tracing::info!("Sent verification email to {}", req.email);
}
}
Err(e) => {
tracing::warn!(
"Email service unavailable, skipping verification email: {}",
e
);
}
let email_sender = auth_state.state.email_sender.clone();
if let Err(e) = email_sender
.send_verification_email(&req.email, &verification_token)
.await
{
tracing::error!("Failed to send verification email to {}: {}", req.email, e);
// Continue even if email fails - user can resend later
} else {
tracing::info!("Sent verification email to {}", req.email);
}

// DO NOT issue UCAN or set session cookie - user must verify email first
Expand Down Expand Up @@ -4169,6 +4151,7 @@ mod tests {
bcrypt_sender: bcrypt_queue.sender(),
redis: None,
secret_pool: secret_pool.receiver(),
email_sender: std::sync::Arc::new(crate::email_service::DevEmailSender::new()),
}),
auth_tx: None,
}
Expand Down
2 changes: 1 addition & 1 deletion api/src/api/http/routes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub fn api_routes(
.route("/auth/forgot-password", post(auth::forgot_password))
.route("/auth/reset-password", post(auth::reset_password))
.route("/auth/resend-verification", post(auth::resend_verification))
.with_state(pool.clone());
.with_state(auth_state.clone());

// OAuth routes (no authentication required for initial authorize request)
// Public CORS - third parties use authorization_code grant (never see passwords)
Expand Down
38 changes: 0 additions & 38 deletions api/src/email_service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,44 +521,6 @@ pub fn create_email_sender() -> Result<Arc<dyn EmailSender>, String> {
Ok(Arc::new(DevEmailSender::new()))
}

/// Legacy EmailService for backward compatibility during migration
/// TODO: Remove once all usages are migrated to the trait
pub struct EmailService {
inner: Arc<dyn EmailSender>,
}

impl EmailService {
pub fn new() -> Result<Self, String> {
Ok(Self {
inner: create_email_sender()?,
})
}

pub async fn send_verification_email(
&self,
to_email: &str,
verification_token: &str,
) -> Result<(), String> {
self.inner
.send_verification_email(to_email, verification_token)
.await
}

pub async fn send_password_reset_email(
&self,
to_email: &str,
reset_token: &str,
) -> Result<(), String> {
self.inner
.send_password_reset_email(to_email, reset_token)
.await
}

pub async fn send_claim_email(&self, to_email: &str, claim_url: &str) -> Result<(), String> {
self.inner.send_claim_email(to_email, claim_url).await
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
Loading
Loading