Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
27 changes: 26 additions & 1 deletion client/src/app/(auth)/layout.css
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

.auth-register,
.auth-login,
.auth-forgot {
.auth-forgot,
.auth-verify {
display: grid;
grid-template-columns: repeat(2, 1fr);
gap: 0;
Expand All @@ -16,6 +17,30 @@
}
}

.auth-verify {
grid-template-columns: 1fr;
place-items: center;
background-image: radial-gradient(
at 50% 50%,
rgb(from var(--primary-500) r g b / 25%) 0,
transparent 80%
);
}

.container {
border: 1px solid rgb(from white r g b / 25%);
border-radius: 8px;
padding: 2rem;
display: flex;
flex-direction: column;
gap: 2rem;
text-align: center;
text-wrap: balance;
width: min(95%, 600px);
background: var(--primary-900);
box-shadow: var(--box-shadow-xl);
}

.form-container {
width: 100%;
height: 100%;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export default function RegisterCredentials({
label="username"
name="username"
id="username"
autoComplete="on"
autoComplete="username"
type="text"
placeholder="Username"
className="auth-form-field"
Expand All @@ -49,7 +49,7 @@ export default function RegisterCredentials({
name="password"
id="password"
type="password"
autoComplete="on"
autoComplete="new-password"
placeholder="Password"
className="auth-form-field"
required
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export default function RegisterEmail({
id="email"
type="email"
placeholder="Enter your email"
autoComplete="on"
autoComplete="email"
className="auth-form-field"
required
errors={formState.errors?.email ?? []}
Expand Down
21 changes: 21 additions & 0 deletions client/src/app/(auth)/verify/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
"use client";
import { useSearchParams } from "next/navigation";

export default function VerifyPage() {
const searchParams = useSearchParams();
const token = searchParams.get("token");

// TODO: Implement actual verification logic by sending the token to the server for validation

return (
<div className="auth-verify">
<div className="container">
<h1>Verify your email</h1>
<p>
A verification link has been sent to your email. Please check your
inbox and click the link to verify your account.
</p>
Comment thread
reijjo marked this conversation as resolved.
</div>
</div>
);
}
44 changes: 23 additions & 21 deletions client/src/lib/api/auth.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import {
checkDuplicateEmail,
checkDuplicateUsername,
createUser,
} from "./auth";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";

import { checkDuplicateEmail, checkDuplicateUsername, createUser } from "./auth";

import type { RegisterUserData } from "../types/auth";

const jsonResponse = (body: unknown, status = 200) =>
Expand All @@ -17,7 +20,9 @@ describe("auth api", () => {
beforeEach(() => {
fetchMock.mockReset();
vi.stubGlobal("fetch", fetchMock);
consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined);
consoleErrorSpy = vi
.spyOn(console, "error")
.mockImplementation(() => undefined);
});

afterEach(() => {
Expand All @@ -37,24 +42,21 @@ describe("auth api", () => {
"user@example.com&admin=true",
"user%40example.com%26admin%3Dtrue",
],
])(
"encodes input safely for %s",
async (_label, email, encodedEmail) => {
fetchMock.mockResolvedValue(
jsonResponse({
success: true,
message: "No duplicate email/username found.",
}),
);
])("encodes input safely for %s", async (_label, email, encodedEmail) => {
fetchMock.mockResolvedValue(
jsonResponse({
success: true,
message: "No duplicate email/username found.",
}),
);

const result = await checkDuplicateEmail(email);
const result = await checkDuplicateEmail(email);

expect(fetchMock).toHaveBeenCalledWith(
`http://localhost:3001/auth/available?email=${encodedEmail}`,
);
expect(result.success).toBe(true);
},
);
expect(fetchMock).toHaveBeenCalledWith(
`http://localhost:3001/api/auth/available?email=${encodedEmail}`,
);
expect(result.success).toBe(true);
});

it("returns API error payload when backend responds with non-OK", async () => {
fetchMock.mockResolvedValue(
Expand Down Expand Up @@ -119,7 +121,7 @@ describe("auth api", () => {
const result = await checkDuplicateUsername(username);

expect(fetchMock).toHaveBeenCalledWith(
`http://localhost:3001/auth/available?username=${encodedUsername}`,
`http://localhost:3001/api/auth/available?username=${encodedUsername}`,
);
expect(result.success).toBe(true);
},
Expand All @@ -144,7 +146,7 @@ describe("auth api", () => {
const result = await createUser(credentials);

expect(fetchMock).toHaveBeenCalledWith(
"http://localhost:3001/auth/register",
"http://localhost:3001/api/auth/register",
expect.objectContaining({
method: "POST",
headers: { "Content-Type": "application/json" },
Expand Down
9 changes: 8 additions & 1 deletion client/src/lib/api/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type {
RegisterUserData,
} from "../types/auth";

const AUTH_URL = `${config.BACKEND_URL}/auth`;
const AUTH_URL = `${config.BACKEND_URL}/api/auth`;

// auth/available
// GET
Expand Down Expand Up @@ -86,3 +86,10 @@ export const loggingIn = async (
return { success: false, error: "Network error" };
}
};

// auth/verify
// GET
// Verify email with token
// export const verifyAccount = async

// TOdO: Implement verifyAccount function when needed
Comment thread
reijjo marked this conversation as resolved.
3 changes: 3 additions & 0 deletions rust-server/.cargo/config.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Force single-threaded test execution to avoid database race conditions.
# Each test truncates the database, so concurrent execution causes conflicts.
RUST_TEST_THREADS = "1"
20 changes: 20 additions & 0 deletions rust-server/src/db/queries.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use sqlx::{PgPool, postgres::PgRow};
use uuid::Uuid;

use crate::errors::AppError;

Expand All @@ -17,3 +18,22 @@ pub async fn find_user_by_username(db: &PgPool, username: &str) -> Result<Option
.await
.map_err(AppError::Sql)
}

#[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")
.bind(id)
.fetch_optional(db)
.await
.map_err(AppError::Sql)
}
Comment thread
reijjo marked this conversation as resolved.

pub async fn find_token_by_value(db: &PgPool, token: &str) -> Result<Option<PgRow>, AppError> {
let row = sqlx::query("SELECT * FROM tokens WHERE token = $1")
.bind(token)
.fetch_optional(db)
.await
.map_err(AppError::Sql)?;

Ok(row)
}
Comment thread
reijjo marked this conversation as resolved.
90 changes: 82 additions & 8 deletions rust-server/src/features/auth/handlers.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
use crate::db::queries::{find_user_by_email, find_user_by_username};
use crate::features::auth::queries::delete_user;
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::features::auth::service::new_user;
use crate::features::auth::types::{AvailabilityQuery, VerifyQuery};
use crate::state::AppState;
use crate::utils::api_response::ApiResponse;
use crate::utils::password::hash_password;
use crate::{errors::AppError, features::auth::types::RegisterData};
use axum::extract::rejection::JsonRejection;
use axum::extract::{Json, State};
use axum::extract::{Json, Query, State};
use chrono::{DateTime, Utc};
use sqlx::Row;
use tokio::task::spawn_blocking;
use uuid::Uuid;
use validator::Validate;

// ----------------------
Expand All @@ -25,11 +29,6 @@ pub async fn register_user(
};

let cleaned_data = validate_registerdata(payload)?;

let hashed_password = spawn_blocking(move || hash_password(&cleaned_data.password))
.await
.map_err(|_| AppError::internal("Threading error"))??;

let db = state.db()?;

if find_user_by_email(db, &cleaned_data.email).await?.is_some() {
Expand All @@ -43,6 +42,10 @@ pub async fn register_user(
return Err(AppError::conflict("Username already in use"));
}

let hashed_password = spawn_blocking(move || hash_password(&cleaned_data.password))
.await
.map_err(|_| AppError::internal("Threading error"))??;

let (user_id, token) = new_user(
db,
&cleaned_data.email,
Expand Down Expand Up @@ -76,6 +79,77 @@ pub async fn register_user(
))
}

// ----------------------
// /api/auth/available - params: email, username
// GET
// Check if email or username is available
// ----------------------
pub async fn check_availability(
State(state): State<AppState>,
Query(params): Query<AvailabilityQuery>,
) -> Result<ApiResponse<()>, AppError> {
match (&params.email, &params.username) {
(None, None) => return Err(AppError::bad_request("Invalid query")),
(Some(_), Some(_)) => return Err(AppError::bad_request("Too many params")),
_ => {}
}

let db = state.db()?;

if let Some(email) = &params.email {
let email = email.trim().to_lowercase();
if email.is_empty() {
return Err(AppError::bad_request("Invalid query"));
}
if find_user_by_email(db, &email).await?.is_some() {
return Err(AppError::conflict("Email already in use"));
}
}

if let Some(username) = &params.username {
let username = username.trim().to_lowercase();
if username.is_empty() {
return Err(AppError::bad_request("Invalid query"));
}
if find_user_by_username(db, &username).await?.is_some() {
return Err(AppError::conflict("Username already in use"));
}
}
Comment thread
reijjo marked this conversation as resolved.

Ok(ApiResponse::ok("No duplicates found", None))
}

// ----------------------
// /api/auth/verify - params: token
// GET
// Account verification
// ----------------------
pub async fn verify_account(
State(state): State<AppState>,
Query(params): Query<VerifyQuery>,
) -> Result<ApiResponse<()>, AppError> {
let token = match &params.token {
Some(t) if !t.trim().is_empty() => t.trim().to_string(),
_ => return Err(AppError::bad_request("Invalid or missing token")),
};

let db = state.db()?;
let row = find_token_by_value(db, &token)
.await?
.ok_or_else(|| AppError::not_found("Invalid token"))?;

let user_id: Uuid = row.get("user_id");
let expires_at: DateTime<Utc> = row.get("expires_at");

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

verify_user(db, user_id).await?;

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 | 🟡 Minor

Silent success on non-existent users.

Looking at verify_user in auth/queries.rs, it runs an UPDATE without checking rows_affected. If user_id doesn't exist (e.g., user deleted between token creation and verification), the query silently succeeds and you return "Email verified successfully" for a ghost user.

Consider checking the result:

pub async fn verify_user<'e, E>(db: E, user_id: Uuid) -> Result<(), AppError>
where
    E: Executor<'e, Database = Postgres>,
{
    let result = sqlx::query("UPDATE users SET verified = true WHERE id = $1")
        .bind(user_id)
        .execute(db)
        .await
        .map_err(AppError::Sql)?;

    if result.rows_affected() == 0 {
        return Err(AppError::not_found("User not found"));
    }
    Ok(())
}

This is edge-case defensive coding, but it prevents confusing scenarios in production.

🤖 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` at line 148, verify_user currently
executes an UPDATE but ignores the execution result so a non-existent user
(rows_affected == 0) will appear as a successful verification; change
verify_user in auth/queries.rs to capture the sqlx::query(...).execute(db).await
result, check result.rows_affected(), and return an AppError::not_found (or
appropriate error) when rows_affected() == 0, ensuring the existing caller in
handlers.rs (which calls verify_user(db, user_id).await?) will propagate the
error instead of reporting "Email verified successfully".


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

// -----------------
// Validate data
// -----------------
Expand Down
14 changes: 14 additions & 0 deletions rust-server/src/features/auth/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,17 @@ where

Ok(())
}

// Verify account - UPDATE
pub async fn verify_user<'e, E>(db: E, user_id: Uuid) -> Result<(), AppError>
where
E: Executor<'e, Database = Postgres>,
{
sqlx::query("UPDATE users SET verified = true WHERE id = $1")
.bind(user_id)
.execute(db)
.await
.map_err(AppError::Sql)?;

Ok(())
}
15 changes: 12 additions & 3 deletions rust-server/src/features/auth/routes.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
use super::handlers::register_user;
use crate::state::AppState;
use axum::{Router, routing::post};
use crate::{
features::auth::handlers::{check_availability, verify_account},
state::AppState,
};
use axum::{
Router,
routing::{get, post},
};

pub fn auth_router() -> Router<AppState> {
Router::new().route("/auth/register", post(register_user))
Router::new()
.route("/register", post(register_user))
.route("/available", get(check_availability))
.route("/verify", get(verify_account))
}
Loading
Loading