Skip to content

feat(auth): add email verification and resend verification flow#40

Open
aniebietafia wants to merge 2 commits intomainfrom
feat/email-verification
Open

feat(auth): add email verification and resend verification flow#40
aniebietafia wants to merge 2 commits intomainfrom
feat/email-verification

Conversation

@aniebietafia
Copy link
Contributor

@aniebietafia aniebietafia commented Mar 17, 2026

Summary by CodeRabbit

Release Notes

  • New Features

    • Email verification system: Users receive secure verification tokens during signup
    • New endpoint to verify email and activate accounts using the token
    • Resend verification email with rate limiting (3 requests/minute per IP)
    • Verification tokens expire after 24 hours; idempotent verification for already-verified users
  • Documentation

    • Added Email Verification API documentation with endpoint specifications

add VerificationToken model and migration for persistent token storage

implement class-based verification logic via VerificationTokenRepository and AuthVerificationService

add GET /api/v1/auth/verify-email with token validation, expiry handling, and idempotent verified-user behavior

add POST /api/v1/auth/resend-verification with enumeration-safe response and 3/minute rate limit

integrate signup to generate verification tokens and enqueue email dispatch via Kafka email producer

add standardized 429 error handling through custom rate-limit exception handler

document endpoints in docs/auth_verification_api.md

add tests for verification CRUD and API flows (success, missing token, invalid token, expired token, resend cases)

keep code quality gates compliant (ruff, isort, mypy) and auth tests green

Signed-off-by: aniebietafia <[email protected]>
add VerificationToken model and migration for persistent token storage

implement class-based verification logic via VerificationTokenRepository and AuthVerificationService

add GET /api/v1/auth/verify-email with token validation, expiry handling, and idempotent verified-user behavior

add POST /api/v1/auth/resend-verification with enumeration-safe response and 3/minute rate limit

integrate signup to generate verification tokens and enqueue email dispatch via Kafka email producer

add standardized 429 error handling through custom rate-limit exception handler

document endpoints in docs/auth_verification_api.md

add tests for verification CRUD and API flows (success, missing token, invalid token, expired token, resend cases)

keep code quality gates compliant (ruff, isort, mypy) and auth tests green

Signed-off-by: aniebietafia <[email protected]>
@coderabbitai
Copy link

coderabbitai bot commented Mar 17, 2026

📝 Walkthrough

Walkthrough

This pull request implements a complete email verification system for user registration, including a new VerificationToken model, database migration, CRUD operations, verification service, API endpoints for email verification and resend, rate limiting infrastructure, and comprehensive test coverage.

Changes

Cohort / File(s) Summary
Database Schema
alembic/versions/4b4b6b5d1c2a_add_verification_tokens_table.py
Creates verification_tokens table with id (PK), user_id (FK), token (unique, indexed), expires_at, and created_at columns; includes corresponding downgrade logic.
Data Models
app/models/verification_token.py, app/models/__init__.py
Introduces VerificationToken SQLAlchemy model with UUID token, 24-hour expiration default, and UTC timezone-aware timestamps; exported from models package.
CRUD Operations
app/crud/verification_token.py
Implements VerificationTokenRepository with methods for token creation with expiration, retrieval, deletion, and cleanup of unexpired tokens; provides module-level wrapper functions.
Core Infrastructure
app/core/config.py, app/core/rate_limiter.py, app/main.py
Adds VERIFICATION_TOKEN_EXPIRE_HOURS config field; introduces SlowAPI-based rate limiter with RateLimitExceeded exception handler; registers limiter in FastAPI app state.
Authentication Service
app/services/auth_verification.py
Implements AuthVerificationService with methods for token creation, email verification with expiration/format validation, and resend verification with token cleanup and email enqueueing; includes error handling and UTC timezone awareness.
API Endpoints
app/api/v1/endpoints/auth.py
Adds GET /verify-email endpoint for token validation and user activation; adds POST /resend-verification endpoint with rate limiting; extends signup to create verification tokens and send verification emails.
Request/Response Schemas
app/schemas/auth.py
Introduces VerifyEmailResponse and ResendVerificationRequest Pydantic models for type validation.
Tests
tests/test_auth/test_email_verification.py, tests/test_auth/test_verification_token_crud.py
Provides comprehensive test coverage for email verification flows (success, missing/invalid/expired tokens, idempotence, enumeration safety) and token CRUD operations with in-memory SQLite fixtures.
Configuration & Documentation
.env.example, app/crud/auth_verification_api.md
Updates .env with VERIFICATION_TOKEN_EXPIRE_HOURS setting; documents API endpoints with request/response examples, error codes, and rate limit details.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Client
    participant Server
    participant DB
    participant EmailProducer

    User->>Client: POST /signup with email
    Client->>Server: signup request
    Server->>DB: create user
    Server->>DB: create verification token
    DB-->>Server: token created
    Server->>EmailProducer: enqueue verification email
    EmailProducer-->>Server: acknowledged
    Server-->>Client: signup success
    Client-->>User: show verification message

    User->>User: click verification link
    User->>Client: GET /verify-email?token=xxx
    Client->>Server: verify-email request with token
    Server->>DB: fetch token
    DB-->>Server: token found
    Server->>Server: validate token format & expiration
    Server->>DB: update user.verified = true
    Server->>DB: delete token
    DB-->>Server: operations complete
    Server-->>Client: verification success
    Client-->>User: email verified
Loading
sequenceDiagram
    participant User
    participant Client
    participant Server
    participant DB
    participant EmailProducer
    participant RateLimiter

    User->>Client: POST /resend-verification with email
    Client->>Server: resend request
    Server->>RateLimiter: check rate limit (3/min)
    alt Rate limit exceeded
        RateLimiter-->>Server: RateLimitExceeded
        Server-->>Client: 429 error
    else Rate limit OK
        RateLimiter-->>Server: allowed
        Server->>DB: find user by email
        DB-->>Server: user found
        Server->>DB: delete unexpired tokens for user
        Server->>DB: create new verification token
        DB-->>Server: token created
        Server->>EmailProducer: enqueue verification email
        EmailProducer-->>Server: acknowledged
        Server-->>Client: success (generic message)
        Client-->>User: resend email sent
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • Email Verification Endpoint #27: Directly implements the email verification endpoints, token model, service logic, rate-limited resend flow, and comprehensive test coverage as specified in this issue.

Possibly related PRs

Suggested labels

backend, tests, migrations, config, size/M

Poem

🐰 A token hops from signup's door,
Through emails fast, we verify more,
Rate-limited bounds keep spammers away,
Database tracks each verification day,
thump thump goes the heart—verification's play!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately and clearly describes the main changes: introducing email verification and resend verification functionality to the authentication flow.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/email-verification
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@aniebietafia aniebietafia linked an issue Mar 17, 2026 that may be closed by this pull request
9 tasks
)
except Exception as exc:
logger.warning(
"Failed to enqueue verification resend for %s: %s", payload.email, exc

Check failure

Code scanning / CodeQL

Log Injection High

This log entry depends on a
user-provided value
.

Copilot Autofix

AI 1 day ago

In general, to fix log injection, you should sanitize any user-controlled values before including them in log messages, especially by removing or neutralizing newline and carriage-return characters so a user cannot split or visually manipulate log entries. This is typically done by replacing \r and \n with empty strings or safe placeholders on a local variable that you only use for logging, so you do not alter the original data used for application logic.

For this specific code, the best minimal fix is to sanitize payload.email right before logging it in the except block of resend_verification. We will introduce a local variable, e.g. safe_email, that converts payload.email to str and strips \r and \n characters using .replace('\r', '').replace('\n', ''). We then pass safe_email to logger.warning instead of the raw payload.email. This keeps the functional behavior (resend logic, response) unchanged and only affects what is recorded in logs. No new imports or helpers are strictly necessary; the sanitization can be an inline transformation on that one line.

Concretely:

  • Edit app/api/v1/endpoints/auth.py within the resend_verification function.
  • In the except Exception as exc: block, before logger.warning(...), define safe_email = str(payload.email).replace('\r', '').replace('\n', '').
  • Change the logger.warning call to use safe_email as the first %s argument.
  • Leave all other behavior intact.
Suggested changeset 1
app/api/v1/endpoints/auth.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/api/v1/endpoints/auth.py b/app/api/v1/endpoints/auth.py
--- a/app/api/v1/endpoints/auth.py
+++ b/app/api/v1/endpoints/auth.py
@@ -197,8 +197,9 @@
             email_producer=email_producer,
         )
     except Exception as exc:
+        safe_email = str(payload.email).replace("\r", "").replace("\n", "")
         logger.warning(
-            "Failed to enqueue verification resend for %s: %s", payload.email, exc
+            "Failed to enqueue verification resend for %s: %s", safe_email, exc
         )
 
     return ActionAcknowledgement(
EOF
@@ -197,8 +197,9 @@
email_producer=email_producer,
)
except Exception as exc:
safe_email = str(payload.email).replace("\r", "").replace("\n", "")
logger.warning(
"Failed to enqueue verification resend for %s: %s", payload.email, exc
"Failed to enqueue verification resend for %s: %s", safe_email, exc
)

return ActionAcknowledgement(
Copilot is powered by AI and may make mistakes. Always verify output.
from alembic import op

# revision identifiers, used by Alembic.
revision: str = "4b4b6b5d1c2a"

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable 'revision' is not used.

Copilot Autofix

AI 1 day ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.


# revision identifiers, used by Alembic.
revision: str = "4b4b6b5d1c2a"
down_revision: str | Sequence[str] | None = "11781e907181"

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable 'down_revision' is not used.

Copilot Autofix

AI 1 day ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

# revision identifiers, used by Alembic.
revision: str = "4b4b6b5d1c2a"
down_revision: str | Sequence[str] | None = "11781e907181"
branch_labels: str | Sequence[str] | None = None

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable 'branch_labels' is not used.

Copilot Autofix

AI 1 day ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

revision: str = "4b4b6b5d1c2a"
down_revision: str | Sequence[str] | None = "11781e907181"
branch_labels: str | Sequence[str] | None = None
depends_on: str | Sequence[str] | None = None

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable 'depends_on' is not used.

Copilot Autofix

AI 1 day ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (3)
app/models/verification_token.py (1)

10-11: Avoid a second source of truth for token expiry.

app/crud/verification_token.py already computes expires_at from settings.VERIFICATION_TOKEN_EXPIRE_HOURS, so this hard-coded 24-hour fallback is dead today. If any future insert path relies on the model default, it will silently ignore the configured TTL.

Also applies to: 26-28

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

In `@app/models/verification_token.py` around lines 10 - 11, The model-level
default_expiry() currently hardcodes a 24-hour TTL which duplicates and can
contradict the TTL computed in app/crud/verification_token.py; update
default_expiry() to derive its timedelta from
settings.VERIFICATION_TOKEN_EXPIRE_HOURS (import settings) so the model default
and CRUD insertion use the same single source of truth for expires_at, or remove
the model default entirely and rely solely on the CRUD layer—adjust the
expires_at field and any references to default_expiry accordingly (see
default_expiry, expires_at and app/crud/verification_token.py).
app/crud/auth_verification_api.md (1)

82-83: Clarify the idempotency note.

Successful verification deletes the token, so the same link is not 200-safe after first use; it falls through to INVALID_TOKEN. Please narrow this note to the case where the user is already verified and the token row still exists.

📝 Suggested wording
-- Already verified users are handled idempotently by `GET /verify-email` and receive `200`.
+- If the user is already verified and the token row still exists, `GET /verify-email` returns `200`.
+- After a successful verification consumes the token, the same link returns `INVALID_TOKEN`.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/crud/auth_verification_api.md` around lines 82 - 83, Update the
idempotency note for GET /verify-email to clarify that tokens are single-use and
are deleted upon successful verification so reusing the same link will typically
yield INVALID_TOKEN; only treat the request as idempotent (returning 200) in the
specific case where the user is already verified and the token row still exists.
Replace the existing second bullet with wording that explicitly states: "Already
verified users are handled idempotently by GET /verify-email only when the
verification token row still exists; otherwise a reused link will result in
INVALID_TOKEN." Ensure references to "successful verification deletes the
token", "GET /verify-email", and "INVALID_TOKEN" remain in the doc so the
behavior is clear.
tests/test_auth/test_email_verification.py (1)

167-193: Add the resend failure-path regression test.

This suite covers the happy path, but not the case where send_email() raises after a resend. That is the path currently able to invalidate the old token while the API still returns 200, so it is worth pinning once the service logic is adjusted.

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

In `@tests/test_auth/test_email_verification.py` around lines 167 - 193, Add a
regression test that simulates send_email raising and asserts the old
verification token is NOT invalidated: create a test (e.g.,
test_resend_verification_failure_does_not_invalidate_token) that uses the same
helpers (_create_unverified_user and _get_verification_token), capture
old_value, set email_producer_mock.send_email.side_effect = Exception("send
failure"), call POST /api/v1/auth/resend-verification, assert
email_producer_mock.send_email was awaited once, and then fetch the token via
_get_verification_token and assert its token equals the previously captured
old_value to ensure a failed send does not rotate the token.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@alembic/versions/4b4b6b5d1c2a_add_verification_tokens_table.py`:
- Around line 32-43: Replace the redundant single-column index
ix_verification_tokens_id with a composite index on (user_id, expires_at) so
queries like delete_unexpired_tokens_for_user (in
app/crud/verification_token.py) can use an index; specifically, remove the
op.create_index call that creates ix_verification_tokens_id and instead add
op.create_index(op.f("ix_verification_tokens_user_id_expires_at"),
"verification_tokens", ["user_id", "expires_at"], unique=False). Make the same
replacement for the corresponding index in the later block (the second
occurrence around lines 47-50) so both upgrade and downgrade/other index
definitions reflect the composite index. Ensure ix_verification_tokens_token
(unique on token) remains unchanged.

In `@app/api/v1/endpoints/auth.py`:
- Around line 193-202: The current broad except around
auth_verification_service.resend_verification_email masks DB/token failures;
narrow the error handling so persistence errors propagate and only
email-producer failures are caught. Move the try/except so it surrounds just the
call to the email_producer (or have resend_verification_email raise a specific
EmailProducerError), catch that specific exception (e.g., EmailProducerError or
the producer's raised exception) and log via logger.warning("Failed to enqueue
verification resend for %s: %s", payload.email, exc); do not catch Exception
from within the overall resend_verification_email call so lookup/token mutation
errors surface as failures.

In `@app/crud/verification_token.py`:
- Around line 15-23: The create_token method (and the similar
revoke/current-token helper at lines 32-42) commits independently which can
cause a gap when called back-to-back from app/services/auth_verification.py;
change these helpers to support a no-commit mode (e.g. add an optional commit:
bool = True parameter or provide internal variants like _create_token_no_commit
and _revoke_current_tokens_no_commit) so they perform db.add/db.flush/db.refresh
without db.commit when commit is False, and then update the service layer to
call both helpers and perform a single db.commit() after both have succeeded;
reference the create_token function and the revoke_current_tokens helper when
implementing this change.

In `@app/services/auth_verification.py`:
- Around line 87-99: Don't delete unexpired tokens before emailing — instead
reuse an existing token if present or create a new one, then enqueue the email,
and only remove other tokens after send_email succeeds. Concretely: use the
repository method that returns existing tokens (e.g.,
get_unexpired_tokens_for_user) to pick/reuse a token; if none exists call
create_token to make one; build verification_link from that token; await
email_producer.send_email(...); only after send_email resolves successfully call
delete_unexpired_tokens_for_user (or a repository method that deletes other
tokens while preserving the just-sent token) to remove stale tokens.

In `@tests/test_auth/test_email_verification.py`:
- Around line 113-116: Replace the hard-coded UUID string in
test_verify_email_invalid_token_returns_custom_error with a programmatically
generated UUID (using uuid.uuid4()) so the request to
"/api/v1/auth/verify-email?token=..." still exercises the INVALID_TOKEN branch
without committing a literal that trips Gitleaks; update the test's client.get
call to interpolate uuid.uuid4().hex or str(uuid.uuid4()) for the token and
import uuid at the top of the test file if not already present.

---

Nitpick comments:
In `@app/crud/auth_verification_api.md`:
- Around line 82-83: Update the idempotency note for GET /verify-email to
clarify that tokens are single-use and are deleted upon successful verification
so reusing the same link will typically yield INVALID_TOKEN; only treat the
request as idempotent (returning 200) in the specific case where the user is
already verified and the token row still exists. Replace the existing second
bullet with wording that explicitly states: "Already verified users are handled
idempotently by GET /verify-email only when the verification token row still
exists; otherwise a reused link will result in INVALID_TOKEN." Ensure references
to "successful verification deletes the token", "GET /verify-email", and
"INVALID_TOKEN" remain in the doc so the behavior is clear.

In `@app/models/verification_token.py`:
- Around line 10-11: The model-level default_expiry() currently hardcodes a
24-hour TTL which duplicates and can contradict the TTL computed in
app/crud/verification_token.py; update default_expiry() to derive its timedelta
from settings.VERIFICATION_TOKEN_EXPIRE_HOURS (import settings) so the model
default and CRUD insertion use the same single source of truth for expires_at,
or remove the model default entirely and rely solely on the CRUD layer—adjust
the expires_at field and any references to default_expiry accordingly (see
default_expiry, expires_at and app/crud/verification_token.py).

In `@tests/test_auth/test_email_verification.py`:
- Around line 167-193: Add a regression test that simulates send_email raising
and asserts the old verification token is NOT invalidated: create a test (e.g.,
test_resend_verification_failure_does_not_invalidate_token) that uses the same
helpers (_create_unverified_user and _get_verification_token), capture
old_value, set email_producer_mock.send_email.side_effect = Exception("send
failure"), call POST /api/v1/auth/resend-verification, assert
email_producer_mock.send_email was awaited once, and then fetch the token via
_get_verification_token and assert its token equals the previously captured
old_value to ensure a failed send does not rotate the token.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ca2c2dd1-6f7e-4303-a531-88580cbf3fed

📥 Commits

Reviewing files that changed from the base of the PR and between d582054 and 6428617.

📒 Files selected for processing (14)
  • .env.example
  • alembic/versions/4b4b6b5d1c2a_add_verification_tokens_table.py
  • app/api/v1/endpoints/auth.py
  • app/core/config.py
  • app/core/rate_limiter.py
  • app/crud/auth_verification_api.md
  • app/crud/verification_token.py
  • app/main.py
  • app/models/__init__.py
  • app/models/verification_token.py
  • app/schemas/auth.py
  • app/services/auth_verification.py
  • tests/test_auth/test_email_verification.py
  • tests/test_auth/test_verification_token_crud.py

Comment on lines +32 to +43
op.create_index(
op.f("ix_verification_tokens_id"),
"verification_tokens",
["id"],
unique=False,
)
op.create_index(
op.f("ix_verification_tokens_token"),
"verification_tokens",
["token"],
unique=True,
)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify repository query patterns that filter by user_id/expires_at for verification tokens.
fd 'verification_token\.py$' app tests | xargs -r rg -n -C2 'user_id|expires_at|delete_unexpired_tokens_for_user|get_token\('

Repository: Brints/FluentMeet

Length of output: 3144


🏁 Script executed:

cat -n alembic/versions/4b4b6b5d1c2a_add_verification_tokens_table.py

Repository: Brints/FluentMeet

Length of output: 1916


Replace redundant id index with composite index supporting user-based token cleanup.

The index on id at line 32 is redundant—primary keys automatically have index support. More critically, the delete_unexpired_tokens_for_user() method in app/crud/verification_token.py:32-37 filters by both user_id and expires_at, but no index covers this pattern, causing full table scans as data grows.

Replace the id index with a composite index on (user_id, expires_at):

Suggested migration adjustment
 def upgrade() -> None:
     op.create_index(
-        op.f("ix_verification_tokens_id"),
+        op.f("ix_verification_tokens_user_id_expires_at"),
         "verification_tokens",
-        ["id"],
+        ["user_id", "expires_at"],
         unique=False,
     )

 def downgrade() -> None:
     op.drop_index(
         op.f("ix_verification_tokens_token"), table_name="verification_tokens"
     )
     op.drop_index(
-        op.f("ix_verification_tokens_id"), table_name="verification_tokens"
+        op.f("ix_verification_tokens_user_id_expires_at"), table_name="verification_tokens"
     )
     op.drop_table("verification_tokens")

Also applies to: 47-50

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

In `@alembic/versions/4b4b6b5d1c2a_add_verification_tokens_table.py` around lines
32 - 43, Replace the redundant single-column index ix_verification_tokens_id
with a composite index on (user_id, expires_at) so queries like
delete_unexpired_tokens_for_user (in app/crud/verification_token.py) can use an
index; specifically, remove the op.create_index call that creates
ix_verification_tokens_id and instead add
op.create_index(op.f("ix_verification_tokens_user_id_expires_at"),
"verification_tokens", ["user_id", "expires_at"], unique=False). Make the same
replacement for the corresponding index in the later block (the second
occurrence around lines 47-50) so both upgrade and downgrade/other index
definitions reflect the composite index. Ensure ix_verification_tokens_token
(unique on token) remains unchanged.

Comment on lines +193 to +202
try:
await auth_verification_service.resend_verification_email(
db=db,
email=str(payload.email),
email_producer=email_producer,
)
except Exception as exc:
logger.warning(
"Failed to enqueue verification resend for %s: %s", payload.email, exc
)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't mask token/database failures as a successful resend.

resend_verification_email() does user lookup and token mutation before it touches the email producer. This except Exception converts failures in those DB steps into a 200 "If an account..." response, which hides outages and can leave state partially changed. Catch only the producer failure path, or let the persistence errors propagate.

🧰 Tools
🪛 GitHub Check: CodeQL

[failure] 201-201: Log Injection
This log entry depends on a user-provided value.

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

In `@app/api/v1/endpoints/auth.py` around lines 193 - 202, The current broad
except around auth_verification_service.resend_verification_email masks DB/token
failures; narrow the error handling so persistence errors propagate and only
email-producer failures are caught. Move the try/except so it surrounds just the
call to the email_producer (or have resend_verification_email raise a specific
EmailProducerError), catch that specific exception (e.g., EmailProducerError or
the producer's raised exception) and log via logger.warning("Failed to enqueue
verification resend for %s: %s", payload.email, exc); do not catch Exception
from within the overall resend_verification_email call so lookup/token mutation
errors surface as failures.

Comment on lines +15 to +23
def create_token(self, db: Session, user_id: int) -> VerificationToken:
expires_at = datetime.now(UTC) + timedelta(
hours=settings.VERIFICATION_TOKEN_EXPIRE_HOURS
)
verification_token = VerificationToken(user_id=user_id, expires_at=expires_at)
db.add(verification_token)
db.commit()
db.refresh(verification_token)
return verification_token
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Let resend own the transaction boundary.

In app/services/auth_verification.py, Lines 87-88 call these helpers back-to-back during resend. Because both methods commit independently, a failure between them can revoke the user's current valid token before the replacement exists. Expose no-commit variants or move the single commit() up to the service layer.

Also applies to: 32-42

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

In `@app/crud/verification_token.py` around lines 15 - 23, The create_token method
(and the similar revoke/current-token helper at lines 32-42) commits
independently which can cause a gap when called back-to-back from
app/services/auth_verification.py; change these helpers to support a no-commit
mode (e.g. add an optional commit: bool = True parameter or provide internal
variants like _create_token_no_commit and _revoke_current_tokens_no_commit) so
they perform db.add/db.flush/db.refresh without db.commit when commit is False,
and then update the service layer to call both helpers and perform a single
db.commit() after both have succeeded; reference the create_token function and
the revoke_current_tokens helper when implementing this change.

Comment on lines +87 to +99
self._token_repository.delete_unexpired_tokens_for_user(db=db, user_id=user.id)
token = self._token_repository.create_token(db=db, user_id=user.id)

verification_link = (
f"{settings.FRONTEND_BASE_URL}/verify-email?token={token.token}"
)
await email_producer.send_email(
to=user.email,
subject="Verify your FluentMeet account",
html_body=None,
template_data={"verification_link": verification_link},
template="verification",
)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Keep the current link valid until the replacement is queued.

Line 87 deletes the current unexpired token before Line 93 tries to enqueue the replacement email. If send_email() fails, the endpoint still returns 200 and the user loses the only verification link they already had. Reuse the existing token or delete the old one only after queueing succeeds.

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

In `@app/services/auth_verification.py` around lines 87 - 99, Don't delete
unexpired tokens before emailing — instead reuse an existing token if present or
create a new one, then enqueue the email, and only remove other tokens after
send_email succeeds. Concretely: use the repository method that returns existing
tokens (e.g., get_unexpired_tokens_for_user) to pick/reuse a token; if none
exists call create_token to make one; build verification_link from that token;
await email_producer.send_email(...); only after send_email resolves
successfully call delete_unexpired_tokens_for_user (or a repository method that
deletes other tokens while preserving the just-sent token) to remove stale
tokens.

Comment on lines +113 to +116
def test_verify_email_invalid_token_returns_custom_error(client: TestClient) -> None:
response = client.get(
"/api/v1/auth/verify-email?token=8f14e45f-ceea-4f6a-9fef-3d4d3e0d1be1"
)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Replace the UUID literal that trips Gitleaks.

The scanner is flagging this hard-coded value as a generic secret. uuid4() exercises the same INVALID_TOKEN branch without adding noisy security findings.

🧪 Suggested change
+from uuid import uuid4
 ...
 def test_verify_email_invalid_token_returns_custom_error(client: TestClient) -> None:
-    response = client.get(
-        "/api/v1/auth/verify-email?token=8f14e45f-ceea-4f6a-9fef-3d4d3e0d1be1"
-    )
+    response = client.get(f"/api/v1/auth/verify-email?token={uuid4()}")
📝 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
def test_verify_email_invalid_token_returns_custom_error(client: TestClient) -> None:
response = client.get(
"/api/v1/auth/verify-email?token=8f14e45f-ceea-4f6a-9fef-3d4d3e0d1be1"
)
from uuid import uuid4
def test_verify_email_invalid_token_returns_custom_error(client: TestClient) -> None:
response = client.get(f"/api/v1/auth/verify-email?token={uuid4()}")
🧰 Tools
🪛 Gitleaks (8.30.0)

[high] 115-115: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

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

In `@tests/test_auth/test_email_verification.py` around lines 113 - 116, Replace
the hard-coded UUID string in
test_verify_email_invalid_token_returns_custom_error with a programmatically
generated UUID (using uuid.uuid4()) so the request to
"/api/v1/auth/verify-email?token=..." still exercises the INVALID_TOKEN branch
without committing a literal that trips Gitleaks; update the test's client.get
call to interpolate uuid.uuid4().hex or str(uuid.uuid4()) for the token and
import uuid at the top of the test file if not already present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Email Verification Endpoint

1 participant