Fix session variable inconsistency in verify_user.py

[DogukanUrker]

Bug: Session Variable Inconsistency in verify_user.py

Severity: Medium
CVSS: 5.3

Description

In verify_user.py, the verification_code is stored as a global variable instead of in the session. This creates inconsistencies:

• Global variable persists across requests (security risk)
• Not tied to specific user session (multi-user issues)
• Not cleared after successful verification

Current Code (Line 51)

Fix Required

1. Store verification_code in session:
2. Clear session variable after successful verification
3. Add session timeout for verification codes (e.g., 15 minutes)

References

• Session management best practices
• CWE-613: Insufficient session expiration

[DogukanUrker]

🤖 AI-generated implementation plan — local LLM (ornith-35b) via Hermes Agent.
Not human-reviewed. Do NOT merge or act on this without human approval.

Problem

Issue #267 reports that verification_code is stored as a global variable in app/routes/verify_user.py, creating session-inconsistency risks: not tied to a specific user, persists across requests, and is not cleared after successful verification.

Current state in main

The issue is already partially fixed. The current code at HEAD uses session["verification_code"] for both storage and retrieval:

• app/routes/verify_user.py:94 — session["verification_code"] = verification_code
• app/routes/verify_user.py:58 — if code == session.get("verification_code"):

The route_guards.py decorator (login_required) ensures all routes are bound to an authenticated session. The app/utils/route_guards.py module was recently introduced and is used by verify_user.py:21.

However, two real issues remain:

1. No cleanup after verification — After a successful code match (verify_user.py:58-60), user.is_verified is set to "True" and the session is committed, but session["verification_code"] is never cleared. The next request with the same session still has the old code available.
2. No expiration/ttl — There is no time-based expiry on the stored verification code. A leaked or brute-forced code remains valid for the lifetime of the session.

The issue's claim that the code is "stored as a global variable" is outdated — it was likely written against a previous version of the file. The current code correctly uses Flask's session object.

Root cause

The original implementation stored the code in a module-level or global variable. The fix moved it into session["verification_code"], but the cleanup step (delete the key after successful verification) and an expiration mechanism were never added.

Proposed fix

1. Clear the session key after successful verification — After db.session.commit() at verify_user.py:60, add session.pop("verification_code", None) to prevent reuse.
2. Add a TTL to the stored code — Store the timestamp alongside the code (e.g., session["verification_code"] = {"code": ..., "expires_at": ...}) and reject codes older than 15 minutes. This addresses CWE-613 (insufficient session expiration).

Concrete changes:

• app/routes/verify_user.py:58-60 — After successful verification, pop the verification_code key from the session.
• app/routes/verify_user.py:93-94 — Store an expiry timestamp alongside the code.
• app/routes/verify_user.py:58 — Check expiry before comparing the code.

Files to change

• app/routes/verify_user.py — Add cleanup after success and TTL check on input.

Test plan

• No existing tests cover verify_user (confirmed: tests/ contains zero references to verify_user or verification_code).
• Recommend adding E2E tests in tests/e2e/:
  • Verify that a correct code verifies the user and redirects.
  • Verify that an incorrect code shows an error.
  • Verify that a stale/expired code is rejected.
  • Verify that re-submitting a verified code after success is rejected (cleanup check).

Risks & edge cases

• Race condition: If two verification requests arrive simultaneously, the TTL check prevents double-verification, but the first to commit wins. The current is_verified == "False" guard at line 51 already handles this.
• Session fixation: The login_required guard in route_guards.py ensures only authenticated users can reach this endpoint, mitigating session fixation.
• Email delivery failure: If SMTP fails (not covered by this issue), the code is never stored in the session, so no cleanup is needed in that path.
• Session garbage collection: Flask's default session lifetime applies. The TTL is a defense-in-depth layer on top of that.

Out of scope

• Adding tests (this is a plan, not an implementation).
• SMTP or email delivery changes.
• Route guard refactoring (already handled by the new route_guards.py).
• Any other routes — only verify_user.py is affected.