Add secure password reset email delivery
This commit is contained in:
parent
6a88fe2e0f
commit
9bedf70371
|
|
@ -11,8 +11,9 @@ import secrets
|
|||
from dataclasses import dataclass
|
||||
from datetime import date, datetime, timedelta, timezone
|
||||
from typing import Any, Dict, Iterable, List, Optional, Tuple
|
||||
from urllib.parse import urlsplit
|
||||
|
||||
from flask import Flask, Response, g, jsonify, request
|
||||
from flask import Flask, Response, current_app, g, jsonify, request
|
||||
import yaml
|
||||
from functools import wraps
|
||||
from argon2 import PasswordHasher
|
||||
|
|
@ -67,6 +68,11 @@ rate_limiter = RateLimiter()
|
|||
password_hasher = PasswordHasher(memory_cost=65536, time_cost=3, parallelism=4)
|
||||
|
||||
|
||||
def _naive_utc_now() -> datetime:
|
||||
"""Return UTC without tzinfo for compatibility with existing DB values."""
|
||||
return datetime.now(timezone.utc).replace(tzinfo=None)
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class Semver:
|
||||
major: int
|
||||
|
|
@ -275,6 +281,14 @@ def run_ai_scrutiny_review(scrutiny_report: dict, config: dict, tool_name: str,
|
|||
def create_app() -> Flask:
|
||||
app = Flask(__name__)
|
||||
app.config["MAX_CONTENT_LENGTH"] = MAX_BODY_BYTES
|
||||
app.config.update(
|
||||
CMDFORGE_ENV=os.environ.get("CMDFORGE_ENV", "development"),
|
||||
PUBLIC_BASE_URL=os.environ.get("CMDFORGE_PUBLIC_URL", "").rstrip("/"),
|
||||
MAIL_TRANSPORT=os.environ.get("MAIL_TRANSPORT", "disabled"),
|
||||
MAIL_FROM=os.environ.get("MAIL_FROM", ""),
|
||||
MAIL_TIMEOUT=os.environ.get("MAIL_TIMEOUT", "10"),
|
||||
RESEND_API_KEY=os.environ.get("RESEND_API_KEY", ""),
|
||||
)
|
||||
|
||||
# Initialize database schema once at startup
|
||||
with connect_db() as init_conn:
|
||||
|
|
@ -2300,7 +2314,7 @@ def create_app() -> Flask:
|
|||
|
||||
# Generate a secure token
|
||||
token, token_hash = generate_token()
|
||||
expires_at = datetime.utcnow() + timedelta(hours=1)
|
||||
expires_at = _naive_utc_now() + timedelta(hours=1)
|
||||
|
||||
g.db.execute(
|
||||
"""
|
||||
|
|
@ -2311,10 +2325,44 @@ def create_app() -> Flask:
|
|||
)
|
||||
g.db.commit()
|
||||
|
||||
# Send email (logs to console in dev mode)
|
||||
# Use a configured canonical URL in production. Falling back to the
|
||||
# request URL is development-only because Host headers are untrusted.
|
||||
from cmdforge.web.email import send_password_reset_email
|
||||
base_url = str(current_app.config.get("PUBLIC_BASE_URL", "")).rstrip("/")
|
||||
environment = str(current_app.config.get("CMDFORGE_ENV", "development")).lower()
|
||||
if not base_url and environment != "production":
|
||||
base_url = request.url_root.rstrip("/")
|
||||
send_password_reset_email(publisher["email"], token, base_url)
|
||||
|
||||
parsed_base = urlsplit(base_url)
|
||||
base_url_is_safe = bool(
|
||||
base_url
|
||||
and parsed_base.scheme in {"http", "https"}
|
||||
and parsed_base.netloc
|
||||
and parsed_base.username is None
|
||||
and parsed_base.password is None
|
||||
and not parsed_base.query
|
||||
and not parsed_base.fragment
|
||||
and (environment != "production" or parsed_base.scheme == "https")
|
||||
)
|
||||
|
||||
try:
|
||||
delivered = base_url_is_safe and send_password_reset_email(
|
||||
publisher["email"], token, base_url
|
||||
)
|
||||
except Exception:
|
||||
# Preserve the non-enumerating response while ensuring the token is
|
||||
# invalidated below. Flask logs the exception without email/token
|
||||
# values because they are not part of this message.
|
||||
current_app.logger.exception("Password reset delivery raised unexpectedly")
|
||||
delivered = False
|
||||
if not delivered:
|
||||
# Never leave a usable reset token in the database when its only
|
||||
# intended recipient could not receive it.
|
||||
g.db.execute(
|
||||
"UPDATE password_reset_tokens SET used_at = CURRENT_TIMESTAMP WHERE token_hash = ?",
|
||||
[token_hash],
|
||||
)
|
||||
g.db.commit()
|
||||
|
||||
return success_response
|
||||
|
||||
|
|
@ -2346,7 +2394,7 @@ def create_app() -> Flask:
|
|||
|
||||
try:
|
||||
expires_at = datetime.fromisoformat(row["expires_at"])
|
||||
if datetime.utcnow() > expires_at:
|
||||
if _naive_utc_now() > expires_at:
|
||||
return error_response("TOKEN_EXPIRED", "This reset token has expired", 400)
|
||||
except ValueError:
|
||||
return error_response("INVALID_TOKEN", "Invalid token data", 400)
|
||||
|
|
@ -2388,7 +2436,7 @@ def create_app() -> Flask:
|
|||
|
||||
try:
|
||||
expires_at = datetime.fromisoformat(row["expires_at"])
|
||||
if datetime.utcnow() > expires_at:
|
||||
if _naive_utc_now() > expires_at:
|
||||
return error_response("TOKEN_EXPIRED", "This reset token has expired", 400)
|
||||
except ValueError:
|
||||
return error_response("INVALID_TOKEN", "Invalid token data", 400)
|
||||
|
|
|
|||
|
|
@ -1,72 +1,103 @@
|
|||
"""Email sending utilities for CmdForge web.
|
||||
"""Transactional email utilities for the CmdForge web application.
|
||||
|
||||
In development mode, emails are logged to the console instead of being sent.
|
||||
To enable real email sending, set MAIL_ENABLED=true and configure SMTP settings.
|
||||
Production delivery uses Resend's HTTPS API. Email bodies and reset URLs are
|
||||
never written to logs. Console delivery is an explicit development-only mode.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from html import escape
|
||||
from typing import Optional
|
||||
|
||||
import requests
|
||||
from flask import current_app
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
RESEND_ENDPOINT = "https://api.resend.com/emails"
|
||||
SUPPORTED_TRANSPORTS = {"disabled", "console", "resend"}
|
||||
|
||||
|
||||
def _transport() -> str:
|
||||
transport = str(current_app.config.get("MAIL_TRANSPORT", "disabled")).strip().lower()
|
||||
if transport not in SUPPORTED_TRANSPORTS:
|
||||
logger.error("Unsupported email transport configured; delivery disabled")
|
||||
return "disabled"
|
||||
return transport
|
||||
|
||||
|
||||
def send_email(to: str, subject: str, html_body: str, text_body: Optional[str] = None) -> bool:
|
||||
"""Send an email.
|
||||
"""Deliver an email without logging its recipient or contents.
|
||||
|
||||
In dev mode (MAIL_ENABLED=false or unset), logs the email to console.
|
||||
In production (MAIL_ENABLED=true), sends via SMTP.
|
||||
|
||||
Args:
|
||||
to: Recipient email address
|
||||
subject: Email subject
|
||||
html_body: HTML content of the email
|
||||
text_body: Plain text content (optional)
|
||||
|
||||
Returns:
|
||||
True if email was sent/logged successfully, False otherwise
|
||||
``MAIL_TRANSPORT`` must be one of ``disabled``, ``console``, or ``resend``.
|
||||
Console mode records only that a message was suppressed and reports no
|
||||
delivery, so reset tokens are invalidated. It is also rejected in
|
||||
production. Resend mode requires ``RESEND_API_KEY`` and ``MAIL_FROM``.
|
||||
"""
|
||||
mail_enabled = current_app.config.get("MAIL_ENABLED", False)
|
||||
transport = _transport()
|
||||
environment = str(current_app.config.get("CMDFORGE_ENV", "development")).lower()
|
||||
|
||||
if mail_enabled:
|
||||
# Future: implement real SMTP sending
|
||||
# smtp_host = current_app.config.get("MAIL_SERVER", "localhost")
|
||||
# smtp_port = current_app.config.get("MAIL_PORT", 587)
|
||||
# smtp_user = current_app.config.get("MAIL_USERNAME")
|
||||
# smtp_pass = current_app.config.get("MAIL_PASSWORD")
|
||||
# smtp_tls = current_app.config.get("MAIL_USE_TLS", True)
|
||||
logger.warning("MAIL_ENABLED is true but SMTP is not implemented yet. Falling back to console logging.")
|
||||
if transport == "disabled":
|
||||
logger.warning("Email delivery is disabled")
|
||||
return False
|
||||
|
||||
# Dev mode: log to console
|
||||
logger.info("=" * 60)
|
||||
logger.info("[EMAIL] To: %s", to)
|
||||
logger.info("[EMAIL] Subject: %s", subject)
|
||||
logger.info("[EMAIL] Body:")
|
||||
if transport == "console":
|
||||
if environment == "production":
|
||||
logger.error("Console email transport is not permitted in production")
|
||||
return False
|
||||
logger.info("Development email suppressed (subject length=%d)", len(subject))
|
||||
return False
|
||||
|
||||
api_key = str(current_app.config.get("RESEND_API_KEY") or "").strip()
|
||||
sender = str(current_app.config.get("MAIL_FROM") or "").strip()
|
||||
if not api_key or not sender:
|
||||
logger.error("Resend email transport is missing required configuration")
|
||||
return False
|
||||
|
||||
try:
|
||||
timeout = float(current_app.config.get("MAIL_TIMEOUT", 10.0))
|
||||
except (TypeError, ValueError):
|
||||
logger.error("Invalid email timeout configured")
|
||||
return False
|
||||
if timeout <= 0 or timeout > 60:
|
||||
logger.error("Email timeout must be greater than zero and at most 60 seconds")
|
||||
return False
|
||||
|
||||
payload = {
|
||||
"from": sender,
|
||||
"to": [to],
|
||||
"subject": subject,
|
||||
"html": html_body,
|
||||
}
|
||||
if text_body:
|
||||
logger.info(text_body)
|
||||
else:
|
||||
logger.info(html_body)
|
||||
logger.info("=" * 60)
|
||||
payload["text"] = text_body
|
||||
|
||||
try:
|
||||
response = requests.post(
|
||||
RESEND_ENDPOINT,
|
||||
headers={
|
||||
"Authorization": f"Bearer {api_key}",
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
json=payload,
|
||||
timeout=timeout,
|
||||
)
|
||||
response.raise_for_status()
|
||||
except requests.RequestException as exc:
|
||||
# Do not log the response body, recipient, request headers, or message;
|
||||
# providers can echo sensitive request data in error responses.
|
||||
logger.error("Transactional email delivery failed (%s)", type(exc).__name__)
|
||||
return False
|
||||
|
||||
logger.info("Transactional email accepted by provider")
|
||||
return True
|
||||
|
||||
|
||||
def send_password_reset_email(to: str, token: str, base_url: str) -> bool:
|
||||
"""Send a password reset email.
|
||||
|
||||
Args:
|
||||
to: Recipient email address
|
||||
token: The password reset token
|
||||
base_url: Base URL of the application (e.g., https://cmdforge.brrd.tech)
|
||||
|
||||
Returns:
|
||||
True if email was sent/logged successfully
|
||||
"""
|
||||
"""Send a one-hour password reset link using the configured transport."""
|
||||
reset_url = f"{base_url.rstrip('/')}/reset-password?token={token}"
|
||||
|
||||
html_reset_url = escape(reset_url, quote=True)
|
||||
subject = "Reset your CmdForge password"
|
||||
|
||||
html_body = f"""
|
||||
|
|
@ -86,28 +117,15 @@ def send_password_reset_email(to: str, token: str, base_url: str) -> bool:
|
|||
</head>
|
||||
<body>
|
||||
<div class="container">
|
||||
<div class="header">
|
||||
<div class="logo">CmdForge</div>
|
||||
</div>
|
||||
|
||||
<div class="header"><div class="logo">CmdForge</div></div>
|
||||
<p>Hello,</p>
|
||||
|
||||
<p>You requested to reset your password for your CmdForge account. Click the button below to set a new password:</p>
|
||||
|
||||
<p style="text-align: center; margin: 30px 0;">
|
||||
<a href="{reset_url}" class="button">Reset Password</a>
|
||||
</p>
|
||||
|
||||
<p style="text-align: center; margin: 30px 0;"><a href="{html_reset_url}" class="button">Reset Password</a></p>
|
||||
<p>Or copy and paste this link into your browser:</p>
|
||||
<p class="link">{reset_url}</p>
|
||||
|
||||
<p class="link">{html_reset_url}</p>
|
||||
<p><strong>This link will expire in 1 hour.</strong></p>
|
||||
|
||||
<p>If you didn't request this password reset, you can safely ignore this email. Your password will remain unchanged.</p>
|
||||
|
||||
<div class="footer">
|
||||
<p>This email was sent by CmdForge. If you have questions, visit our website or contact support.</p>
|
||||
</div>
|
||||
<div class="footer"><p>This email was sent by CmdForge.</p></div>
|
||||
</div>
|
||||
</body>
|
||||
</html>
|
||||
|
|
@ -115,20 +133,10 @@ def send_password_reset_email(to: str, token: str, base_url: str) -> bool:
|
|||
|
||||
text_body = f"""Reset your CmdForge password
|
||||
|
||||
Hello,
|
||||
|
||||
You requested to reset your password for your CmdForge account.
|
||||
|
||||
Reset your password by visiting this link:
|
||||
Visit this link to set a new password:
|
||||
{reset_url}
|
||||
|
||||
This link will expire in 1 hour.
|
||||
|
||||
If you didn't request this password reset, you can safely ignore this email.
|
||||
Your password will remain unchanged.
|
||||
|
||||
---
|
||||
CmdForge
|
||||
This link will expire in 1 hour. If you did not request this reset, ignore this email.
|
||||
"""
|
||||
|
||||
return send_email(to, subject, html_body, text_body)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,173 @@
|
|||
"""Tests for transactional email and password-reset delivery safety."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import hashlib
|
||||
import logging
|
||||
from unittest.mock import patch
|
||||
|
||||
import requests
|
||||
from flask import Flask
|
||||
|
||||
from cmdforge.registry.app import create_app, password_hasher
|
||||
from cmdforge.registry.db import connect_db
|
||||
from cmdforge.web.email import send_email
|
||||
|
||||
|
||||
def _email_app(**config) -> Flask:
|
||||
app = Flask(__name__)
|
||||
defaults = {
|
||||
"CMDFORGE_ENV": "development",
|
||||
"MAIL_TRANSPORT": "disabled",
|
||||
"MAIL_FROM": "CmdForge <noreply@example.com>",
|
||||
"MAIL_TIMEOUT": 10,
|
||||
"RESEND_API_KEY": "",
|
||||
}
|
||||
defaults.update(config)
|
||||
app.config.update(defaults)
|
||||
return app
|
||||
|
||||
|
||||
def test_disabled_delivery_does_not_log_message_or_recipient(caplog):
|
||||
app = _email_app()
|
||||
with app.app_context(), caplog.at_level(logging.INFO):
|
||||
assert not send_email(
|
||||
"private@example.com", "Reset subject", "secret reset body", "secret text"
|
||||
)
|
||||
assert "private@example.com" not in caplog.text
|
||||
assert "secret reset body" not in caplog.text
|
||||
assert "secret text" not in caplog.text
|
||||
|
||||
|
||||
def test_console_transport_is_rejected_in_production(caplog):
|
||||
app = _email_app(CMDFORGE_ENV="production", MAIL_TRANSPORT="console")
|
||||
with app.app_context(), caplog.at_level(logging.INFO):
|
||||
assert not send_email("private@example.com", "subject", "body")
|
||||
assert "private@example.com" not in caplog.text
|
||||
assert "body" not in caplog.text
|
||||
|
||||
|
||||
def test_resend_transport_sends_expected_request(monkeypatch):
|
||||
app = _email_app(MAIL_TRANSPORT="resend", RESEND_API_KEY="test-api-key")
|
||||
captured = {}
|
||||
|
||||
class Response:
|
||||
def raise_for_status(self):
|
||||
return None
|
||||
|
||||
def fake_post(url, **kwargs):
|
||||
captured["url"] = url
|
||||
captured.update(kwargs)
|
||||
return Response()
|
||||
|
||||
monkeypatch.setattr(requests, "post", fake_post)
|
||||
with app.app_context():
|
||||
assert send_email("person@example.com", "subject", "<p>body</p>", "body")
|
||||
|
||||
assert captured["url"] == "https://api.resend.com/emails"
|
||||
assert captured["headers"]["Authorization"] == "Bearer test-api-key"
|
||||
assert captured["json"]["to"] == ["person@example.com"]
|
||||
assert captured["timeout"] == 10.0
|
||||
|
||||
|
||||
def test_resend_failure_does_not_log_sensitive_data(monkeypatch, caplog):
|
||||
app = _email_app(MAIL_TRANSPORT="resend", RESEND_API_KEY="test-api-key")
|
||||
|
||||
def fail_post(*args, **kwargs):
|
||||
raise requests.ConnectionError("provider unavailable")
|
||||
|
||||
monkeypatch.setattr(requests, "post", fail_post)
|
||||
with app.app_context(), caplog.at_level(logging.INFO):
|
||||
assert not send_email("private@example.com", "subject", "reset-secret")
|
||||
assert "private@example.com" not in caplog.text
|
||||
assert "reset-secret" not in caplog.text
|
||||
assert "test-api-key" not in caplog.text
|
||||
|
||||
|
||||
def _registry_client(tmp_path, monkeypatch):
|
||||
db_path = tmp_path / "registry.db"
|
||||
monkeypatch.setenv("CMDFORGE_REGISTRY_DB", str(db_path))
|
||||
monkeypatch.setenv("CMDFORGE_ENV", "production")
|
||||
monkeypatch.setenv("CMDFORGE_PUBLIC_URL", "https://cmdforge.example")
|
||||
app = create_app()
|
||||
app.config["TESTING"] = True
|
||||
with connect_db() as db:
|
||||
db.execute(
|
||||
"""
|
||||
INSERT INTO publishers (email, password_hash, slug, display_name)
|
||||
VALUES (?, ?, ?, ?)
|
||||
""",
|
||||
["owner@example.com", password_hasher.hash("initial-password"), "owner", "Owner"],
|
||||
)
|
||||
db.commit()
|
||||
return app.test_client(), db_path
|
||||
|
||||
|
||||
def test_reset_uses_canonical_url_and_leaves_delivered_token_valid(tmp_path, monkeypatch):
|
||||
client, db_path = _registry_client(tmp_path, monkeypatch)
|
||||
captured = {}
|
||||
|
||||
def delivered(to, token, base_url):
|
||||
captured.update(to=to, token=token, base_url=base_url)
|
||||
return True
|
||||
|
||||
with patch("cmdforge.web.email.send_password_reset_email", delivered):
|
||||
response = client.post(
|
||||
"/api/v1/password-reset/request",
|
||||
json={"email": "owner@example.com"},
|
||||
headers={"Host": "attacker.example"},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert captured["base_url"] == "https://cmdforge.example"
|
||||
with connect_db() as db:
|
||||
row = db.execute(
|
||||
"SELECT token_hash, used_at FROM password_reset_tokens"
|
||||
).fetchone()
|
||||
assert row["token_hash"] == hashlib.sha256(captured["token"].encode()).hexdigest()
|
||||
assert row["used_at"] is None
|
||||
|
||||
|
||||
def test_reset_invalidates_token_when_delivery_fails(tmp_path, monkeypatch):
|
||||
client, db_path = _registry_client(tmp_path, monkeypatch)
|
||||
|
||||
with patch("cmdforge.web.email.send_password_reset_email", return_value=False):
|
||||
response = client.post(
|
||||
"/api/v1/password-reset/request", json={"email": "owner@example.com"}
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
with connect_db() as db:
|
||||
row = db.execute("SELECT used_at FROM password_reset_tokens").fetchone()
|
||||
assert row["used_at"] is not None
|
||||
|
||||
|
||||
def test_reset_invalidates_token_without_safe_production_base_url(tmp_path, monkeypatch):
|
||||
client, db_path = _registry_client(tmp_path, monkeypatch)
|
||||
client.application.config["PUBLIC_BASE_URL"] = "http://insecure.example"
|
||||
|
||||
with patch("cmdforge.web.email.send_password_reset_email") as sender:
|
||||
response = client.post(
|
||||
"/api/v1/password-reset/request", json={"email": "owner@example.com"}
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
sender.assert_not_called()
|
||||
with connect_db() as db:
|
||||
row = db.execute("SELECT used_at FROM password_reset_tokens").fetchone()
|
||||
assert row["used_at"] is not None
|
||||
|
||||
|
||||
def test_unknown_email_creates_no_token_and_sends_nothing(tmp_path, monkeypatch):
|
||||
client, db_path = _registry_client(tmp_path, monkeypatch)
|
||||
|
||||
with patch("cmdforge.web.email.send_password_reset_email") as sender:
|
||||
response = client.post(
|
||||
"/api/v1/password-reset/request", json={"email": "missing@example.com"}
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
sender.assert_not_called()
|
||||
with connect_db() as db:
|
||||
count = db.execute("SELECT COUNT(*) FROM password_reset_tokens").fetchone()[0]
|
||||
assert count == 0
|
||||
Loading…
Reference in New Issue