Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 1 addition & 1 deletion rust-server/src/db/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub async fn find_user_by_username(db: &PgPool, username: &str) -> Result<Option

#[allow(dead_code)]
pub async fn find_user_by_id(db: &PgPool, id: Uuid) -> Result<Option<PgRow>, AppError> {
sqlx::query("SELECT id FROM users WHERE id = $1")
sqlx::query("SELECT id, email FROM users WHERE id = $1")
.bind(id)
.fetch_optional(db)
.await
Expand Down
35 changes: 27 additions & 8 deletions rust-server/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,16 @@ use crate::db::connect::DbError;
#[derive(Debug)]
#[allow(dead_code)]
pub enum AppError {
NotFound(String),
Internal(String),
Database(String),
BadRequest(String),
Json(JsonRejection),
Validation(ValidationErrors),
Sql(sqlx::Error),
Conflict(String),
NotFound(String), // 404
Internal(String), // 500
Database(String), // 503
BadRequest(String), // 400
Json(JsonRejection), // 400 with detailed JSON error
Validation(ValidationErrors), // 400 with validation error details
Sql(sqlx::Error), // 404 for RowNotFound, 409 for unique constraint violation, 500 for others
Conflict(String), // 409
Gone(String), // 410
TooManyRequests(String), // 429
}

#[derive(Serialize)]
Expand All @@ -30,25 +32,40 @@ struct ErrorBody {

#[allow(dead_code)]
impl AppError {
// 404 Not Found
pub fn not_found(message: impl Into<String>) -> Self {
Self::NotFound(message.into())
}

// 500 Internal Server Error
pub fn internal(message: impl Into<String>) -> Self {
Self::Internal(message.into())
}

// 503 Service Unavailable (for database errors)
pub fn database(message: impl Into<String>) -> Self {
Self::Database(message.into())
}

// 400 Bad Request
pub fn bad_request(message: impl Into<String>) -> Self {
Self::BadRequest(message.into())
}

// 409 Conflict
pub fn conflict(message: impl Into<String>) -> Self {
Self::Conflict(message.into())
}

// 410 Gone
pub fn gone(message: impl Into<String>) -> Self {
Self::Gone(message.into())
}

// 429 Too Many Requests
pub fn too_many_requests(message: impl Into<String>) -> Self {
Self::TooManyRequests(message.into())
}
}

impl IntoResponse for AppError {
Expand All @@ -58,6 +75,8 @@ impl IntoResponse for AppError {
AppError::Internal(message) => (StatusCode::INTERNAL_SERVER_ERROR, message),
AppError::Database(message) => (StatusCode::SERVICE_UNAVAILABLE, message),
AppError::BadRequest(message) => (StatusCode::BAD_REQUEST, message),
AppError::Gone(message) => (StatusCode::GONE, message),
AppError::TooManyRequests(message) => (StatusCode::TOO_MANY_REQUESTS, message),
AppError::Json(rejection) => {
let message = match rejection {
JsonRejection::JsonDataError(e) => format!("Invalid JSON: {}", e),
Expand Down
49 changes: 45 additions & 4 deletions rust-server/src/features/auth/handlers.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use crate::db::queries::{find_token_by_value, find_user_by_email, find_user_by_username};
use crate::features::auth::queries::{delete_user, verify_user};
use crate::db::queries::{
find_token_by_value, find_user_by_email, find_user_by_id, find_user_by_username,
};
use crate::features::auth::queries::{delete_user, update_verification_token, verify_user};
use crate::features::auth::service::new_user;
use crate::features::auth::types::{AvailabilityQuery, VerifyQuery};
use crate::features::auth::types::{AvailabilityQuery, ResendTokenData, VerifyQuery};
use crate::state::AppState;
use crate::utils::api_response::ApiResponse;
use crate::utils::password::hash_password;
Expand Down Expand Up @@ -142,14 +144,53 @@ pub async fn verify_account(
let expires_at: DateTime<Utc> = row.get("expires_at");

if Utc::now() > expires_at {
return Err(AppError::bad_request("Token has expired"));
return Err(AppError::gone("Token has expired"));
}

verify_user(db, user_id).await?;

Ok(ApiResponse::ok("Email verified successfully", None))
}

// ----------------------
// /api/auth/verify
// POST
// Update verification token (for resending verification email)
// ----------------------
pub async fn resend_token(
State(state): State<AppState>,
Query(params): Query<ResendTokenData>,
) -> Result<ApiResponse<()>, AppError> {
let token = &params.token;
Comment thread
reijjo marked this conversation as resolved.
Outdated

let db = state.db()?;
let result = find_token_by_value(db, token)
.await?
.ok_or_else(|| AppError::not_found("Token not found"));

let user = find_user_by_id(db, result?.get("user_id"))
.await?
.ok_or_else(|| AppError::not_found("User not found"))?;
eprint!("RESULT: {:#?}", user);
Comment thread
reijjo marked this conversation as resolved.
Outdated

let new_token = update_verification_token(db, user.get("id")).await?;
Comment thread
reijjo marked this conversation as resolved.

if !state.config.app_env.is_test()
&& let Err(email_err) = state
.email
.send_verification_email(user.get("email"), &new_token)
.await
{
tracing::error!("Failed to send verification email: {:#?}", email_err);
return Err(email_err);
}
Comment on lines +170 to +188

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Token rotation before email delivery can lock users out after transient mail failures.

Line 176 updates the token first; if Line 181–182 email send fails, the previously delivered token is invalidated and the user may no longer have a usable token to retry resend.

Suggested direction
-    let new_token = update_verification_token(db, user.get("id")).await?;
+    // Keep previous token data for compensation if email send fails.
+    let old_token: String = result?.get("token");
+    let old_expires_at: DateTime<Utc> = result?.get("expires_at");
+    let new_token = update_verification_token(db, user.get("id")).await?;
@@
     if !state.config.app_env.is_test()
         && let Err(email_err) = state
             .email
             .send_verification_email(user.get("email"), &new_token)
             .await
     {
+        // restore previous token state so the last delivered link remains usable
+        restore_verification_token(db, user.get("id"), &old_token, old_expires_at).await?;
         tracing::error!("Failed to send verification email: {:#?}", email_err);
         return Err(email_err);
     }

If you want, I can draft a minimal restore_verification_token(...) query helper too.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@rust-server/src/features/auth/handlers.rs` around lines 167 - 186, The code
rotates the verification token via update_verification_token(db, user.get("id"))
before attempting to send the email, which can lock the user out if
send_verification_email fails; change the flow so you only persist the new token
after the email send succeeds (call update_verification_token after
send_verification_email), or if you must pre-create the token, capture the old
token and on email send failure call a restore_verification_token(db,
user.get("id"), old_token) helper to revert to the previous token and return the
email error; update calls around find_token_by_value, update_verification_token,
and send_verification_email accordingly.


Ok(ApiResponse::ok(
"Verification email resent. Check your inbox.",
None,
))
}

// -----------------
// Validate data
// -----------------
Expand Down
23 changes: 21 additions & 2 deletions rust-server/src/features/auth/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use sqlx::{Executor, Postgres, types::Uuid};

use crate::errors::AppError;

// Create user - POST
// Create user - CREATE
pub async fn create_user<'e, E>(
db: E,
email: &str,
Expand All @@ -27,7 +27,7 @@ where
Ok(row)
}

// Create verification token - POST
// Create verification token - CREATE
pub async fn create_verification_token<'e, E>(
db: E,
user_id: Uuid,
Expand Down Expand Up @@ -75,3 +75,22 @@ where

Ok(())
}

// Update token - UPDATE (for resending verification email)
pub async fn update_verification_token<'e, E>(db: E, user_id: Uuid) -> Result<String, AppError>
where
E: Executor<'e, Database = Postgres>,
{
let new_token = Uuid::new_v4().to_string();
let new_expires_at = Utc::now() + chrono::Duration::hours(24);

sqlx::query("UPDATE tokens SET token = $1, expires_at = $2 WHERE user_id = $3")
.bind(&new_token)
.bind(new_expires_at)
.bind(user_id)
.execute(db)
.await
.map_err(AppError::Sql)?;

Ok(new_token)
Comment on lines +87 to +95

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Check rows_affected to avoid returning non-persisted tokens.

Right now, Line 87–95 returns Ok(new_token) even when no token row exists for that user_id. That can send a token that will never verify.

Suggested fix
-    sqlx::query("UPDATE tokens SET token = $1, expires_at = $2 WHERE user_id = $3")
+    let result = sqlx::query("UPDATE tokens SET token = $1, expires_at = $2 WHERE user_id = $3")
         .bind(&new_token)
         .bind(new_expires_at)
         .bind(user_id)
         .execute(db)
         .await
         .map_err(AppError::Sql)?;
 
+    if result.rows_affected() != 1 {
+        return Err(AppError::not_found("Verification token not found for user"));
+    }
+
     Ok(new_token)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
sqlx::query("UPDATE tokens SET token = $1, expires_at = $2 WHERE user_id = $3")
.bind(&new_token)
.bind(new_expires_at)
.bind(user_id)
.execute(db)
.await
.map_err(AppError::Sql)?;
Ok(new_token)
let result = sqlx::query("UPDATE tokens SET token = $1, expires_at = $2 WHERE user_id = $3")
.bind(&new_token)
.bind(new_expires_at)
.bind(user_id)
.execute(db)
.await
.map_err(AppError::Sql)?;
if result.rows_affected() != 1 {
return Err(AppError::not_found("Verification token not found for user"));
}
Ok(new_token)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@rust-server/src/features/auth/queries.rs` around lines 87 - 95, The UPDATE
currently always returns Ok(new_token) after running
sqlx::query(...).bind(...).execute(db).await, which can return zero rows if no
token row exists for that user_id; change this to capture the execute result
into a variable (e.g. let res =
sqlx::query(...).execute(db).await.map_err(AppError::Sql)?), check
res.rows_affected(), and if it is 0 return an appropriate error (e.g.
AppError::NotFound or a specific token-not-persisted error) instead of
Ok(new_token); only return Ok(new_token) when rows_affected() > 0.

}
4 changes: 2 additions & 2 deletions rust-server/src/features/auth/routes.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::handlers::register_user;
use crate::{
features::auth::handlers::{check_availability, verify_account},
features::auth::handlers::{check_availability, resend_token, verify_account},
state::AppState,
};
use axum::{
Expand All @@ -12,5 +12,5 @@ pub fn auth_router() -> Router<AppState> {
Router::new()
.route("/register", post(register_user))
.route("/available", get(check_availability))
.route("/verify", get(verify_account))
.route("/verify", get(verify_account).post(resend_token))
}
9 changes: 8 additions & 1 deletion rust-server/src/features/auth/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ pub struct RegisterData {
pub password: String,
}

// -----------------------
// Resend token data
// -----------------------
#[derive(Deserialize)]
pub struct ResendTokenData {
pub token: String,
}

// -----------------------
// Queries (params)
// -----------------------
Expand All @@ -38,7 +46,6 @@ pub struct AvailabilityQuery {
}

#[derive(Deserialize)]

pub struct VerifyQuery {
pub token: Option<String>,
}
Expand Down
Loading