From be33d488b13083d8fdd88392f231acd837890721 Mon Sep 17 00:00:00 2001 From: Atlas Date: Tue, 24 Mar 2026 10:39:45 +0800 Subject: [PATCH 1/5] Add platform email delivery for password recovery and broadcast notices This change adds a platform-owned SMTP delivery path and wires it into forgot-password plus optional enterprise broadcast email delivery. The reset flow now issues single-use database-backed tokens, exposes public reset endpoints, and adds frontend pages for requesting and consuming reset links. The README and env example now document the required SMTP and public URL configuration for local and production setups. Constraint: SMTP configuration must come from environment variables rather than admin-managed secrets Constraint: Forgot-password responses must not reveal whether an email address exists Rejected: Reusing per-agent email tooling | wrong trust boundary for system-owned auth mail Rejected: Stateless reset JWTs | harder to revoke and audit than DB-backed single-use tokens Confidence: high Scope-risk: moderate Reversibility: clean Directive: Do not move reset-link generation away from PUBLIC_BASE_URL without verifying frontend route compatibility Tested: backend pytest tests/test_password_reset_and_notifications.py Tested: frontend npm run build Tested: manual local validation of forgot-password, reset-password, and SMTP delivery with Docker PostgreSQL/Redis Not-tested: full end-to-end browser automation for clicking the emailed reset link --- .env.example | 15 ++ README.md | 31 +++ .../versions/add_password_reset_tokens.py | 32 +++ backend/app/api/auth.py | 78 ++++++- backend/app/api/notification.py | 39 +++- backend/app/config.py | 11 + backend/app/main.py | 1 + backend/app/models/password_reset_token.py | 23 ++ backend/app/schemas/schemas.py | 10 +- .../app/services/password_reset_service.py | 78 +++++++ backend/app/services/system_email_service.py | 82 +++++++ backend/entrypoint.sh | 1 + .../test_password_reset_and_notifications.py | 211 ++++++++++++++++++ frontend/src/App.tsx | 4 + frontend/src/pages/EnterpriseSettings.tsx | 26 ++- frontend/src/pages/ForgotPassword.tsx | 83 +++++++ frontend/src/pages/Login.tsx | 13 +- frontend/src/pages/ResetPassword.tsx | 111 +++++++++ frontend/src/services/api.ts | 11 +- 19 files changed, 848 insertions(+), 12 deletions(-) create mode 100644 backend/alembic/versions/add_password_reset_tokens.py create mode 100644 backend/app/models/password_reset_token.py create mode 100644 backend/app/services/password_reset_service.py create mode 100644 backend/app/services/system_email_service.py create mode 100644 backend/tests/test_password_reset_and_notifications.py create mode 100644 frontend/src/pages/ForgotPassword.tsx create mode 100644 frontend/src/pages/ResetPassword.tsx diff --git a/.env.example b/.env.example index c27b9b06..029cca14 100644 --- a/.env.example +++ b/.env.example @@ -24,3 +24,18 @@ FEISHU_REDIRECT_URI=http://localhost:3000/auth/feishu/callback # Jina AI API key (for jina_search and jina_read tools — get one at https://jina.ai) # Without a key, the tools still work but with lower rate limits JINA_API_KEY= + +# Public app URL used in user-facing links, such as password reset emails +PUBLIC_BASE_URL=http://localhost:3008 + +# System email delivery (used for forgot-password and optional broadcast emails) +SYSTEM_EMAIL_FROM_ADDRESS= +SYSTEM_EMAIL_FROM_NAME=Clawith +SYSTEM_SMTP_HOST= +SYSTEM_SMTP_PORT=465 +SYSTEM_SMTP_USERNAME= +SYSTEM_SMTP_PASSWORD= +SYSTEM_SMTP_SSL=true + +# Password reset token lifetime in minutes +PASSWORD_RESET_TOKEN_EXPIRE_MINUTES=30 diff --git a/README.md b/README.md index d55a3b86..1806152e 100644 --- a/README.md +++ b/README.md @@ -156,6 +156,37 @@ Agent workspace files (soul.md, memory, skills, workspace files) are stored in ` The first user to register automatically becomes the **platform admin**. Open the app, click "Register", and create your account. +### System Email and Password Reset + +Clawith can send platform-owned emails for password reset and optional broadcast delivery. Configure SMTP in `.env`: + +```bash +PUBLIC_BASE_URL=http://localhost:3008 +SYSTEM_EMAIL_FROM_ADDRESS=bot@example.com +SYSTEM_EMAIL_FROM_NAME=Clawith +SYSTEM_SMTP_HOST=smtp.example.com +SYSTEM_SMTP_PORT=465 +SYSTEM_SMTP_USERNAME=bot@example.com +SYSTEM_SMTP_PASSWORD=your-app-password +SYSTEM_SMTP_SSL=true +PASSWORD_RESET_TOKEN_EXPIRE_MINUTES=30 +``` + +`PUBLIC_BASE_URL` must point to the user-facing frontend because reset links are generated as `/reset-password?token=...`. + +Quick local validation: + +```bash +cd backend && .venv/bin/python -m pytest tests/test_password_reset_and_notifications.py +cd frontend && npm run build +``` + +Manual flow: +1. Open `http://localhost:3008/login` +2. Click `Forgot password?` +3. Submit a registered email +4. Open the emailed reset link and set a new password + ### Network Troubleshooting If `git clone` is slow or times out: diff --git a/backend/alembic/versions/add_password_reset_tokens.py b/backend/alembic/versions/add_password_reset_tokens.py new file mode 100644 index 00000000..cd0fe56e --- /dev/null +++ b/backend/alembic/versions/add_password_reset_tokens.py @@ -0,0 +1,32 @@ +"""Add password_reset_tokens table. + +Revision ID: add_password_reset_tokens +Revises: multi_tenant_registration +""" + +from alembic import op + +revision = "add_password_reset_tokens" +down_revision = "multi_tenant_registration" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.execute(""" + CREATE TABLE IF NOT EXISTS password_reset_tokens ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + user_id UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE, + token_hash VARCHAR(128) NOT NULL UNIQUE, + expires_at TIMESTAMPTZ NOT NULL, + used_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() + ) + """) + op.execute("CREATE INDEX IF NOT EXISTS ix_password_reset_tokens_user_id ON password_reset_tokens(user_id)") + op.execute("CREATE INDEX IF NOT EXISTS ix_password_reset_tokens_token_hash ON password_reset_tokens(token_hash)") + op.execute("CREATE INDEX IF NOT EXISTS ix_password_reset_tokens_expires_at ON password_reset_tokens(expires_at)") + + +def downgrade() -> None: + op.execute("DROP TABLE IF EXISTS password_reset_tokens") diff --git a/backend/app/api/auth.py b/backend/app/api/auth.py index bf221945..ea9c5a07 100644 --- a/backend/app/api/auth.py +++ b/backend/app/api/auth.py @@ -1,6 +1,6 @@ """Authentication API routes.""" -import uuid +from datetime import datetime, timezone from fastapi import APIRouter, Depends, HTTPException, status from loguru import logger @@ -9,8 +9,17 @@ from app.core.security import create_access_token, get_current_user, hash_password, verify_password from app.database import get_db +from app.models.password_reset_token import PasswordResetToken from app.models.user import User -from app.schemas.schemas import TokenResponse, UserLogin, UserOut, UserRegister, UserUpdate +from app.schemas.schemas import ( + ForgotPasswordRequest, + ResetPasswordRequest, + TokenResponse, + UserLogin, + UserOut, + UserRegister, + UserUpdate, +) router = APIRouter(prefix="/auth", tags=["auth"]) @@ -141,6 +150,71 @@ async def login(data: UserLogin, db: AsyncSession = Depends(get_db)): ) +@router.post("/forgot-password") +async def forgot_password(data: ForgotPasswordRequest, db: AsyncSession = Depends(get_db)): + """Request a password reset link without revealing account existence.""" + generic_response = { + "ok": True, + "message": "If an account with that email exists, a password reset email has been sent.", + } + + result = await db.execute(select(User).where(User.email == data.email)) + user = result.scalar_one_or_none() + if not user or not user.is_active: + return generic_response + + try: + from app.services.password_reset_service import build_password_reset_url, create_password_reset_token + from app.services.system_email_service import send_system_email + + raw_token, expires_at = await create_password_reset_token(db, user.id) + await db.commit() + + reset_url = await build_password_reset_url(db, raw_token) + expiry_minutes = int((expires_at - datetime.now(timezone.utc)).total_seconds() // 60) + await send_system_email( + user.email, + "Reset your Clawith password", + ( + f"Hello {user.display_name or user.username},\n\n" + f"We received a request to reset your Clawith password.\n\n" + f"Reset link: {reset_url}\n\n" + f"This link expires in {expiry_minutes} minutes. If you did not request this, you can ignore this email." + ), + ) + except Exception as exc: + logger.warning(f"Failed to process password reset email for {data.email}: {exc}") + + return generic_response + + +@router.post("/reset-password") +async def reset_password(data: ResetPasswordRequest, db: AsyncSession = Depends(get_db)): + """Reset a password using a valid single-use token.""" + from app.services.password_reset_service import consume_password_reset_token + + token = await consume_password_reset_token(db, data.token) + if not token: + raise HTTPException(status_code=400, detail="Invalid or expired reset token") + + result = await db.execute(select(User).where(User.id == token.user_id)) + user = result.scalar_one_or_none() + if not user or not user.is_active: + raise HTTPException(status_code=400, detail="Invalid or expired reset token") + + user.password_hash = hash_password(data.new_password) + + # Invalidate any other older token rows for the same user. + other_tokens = await db.execute(select(PasswordResetToken).where(PasswordResetToken.user_id == user.id)) + now = datetime.now(timezone.utc) + for row in other_tokens.scalars().all(): + if row.id != token.id and row.used_at is None: + row.used_at = now + + await db.flush() + return {"ok": True} + + @router.get("/me", response_model=UserOut) async def get_me(current_user: User = Depends(get_current_user)): """Get current user profile.""" diff --git a/backend/app/api/notification.py b/backend/app/api/notification.py index 6e1828b8..3e8d31bf 100644 --- a/backend/app/api/notification.py +++ b/backend/app/api/notification.py @@ -116,6 +116,7 @@ async def mark_all_read( class BroadcastRequest(BaseModel): title: str = Field(..., max_length=200) body: str = Field("", max_length=1000) + send_email: bool = False @router.post("/notifications/broadcast") @@ -138,12 +139,22 @@ async def broadcast_notification( sender_name = current_user.display_name or current_user.username or "Admin" count_users = 0 count_agents = 0 + count_emails = 0 + + if req.send_email: + from app.services.system_email_service import get_system_email_config + + try: + get_system_email_config() + except Exception as exc: + raise HTTPException(400, f"System email is not configured: {exc}") # Notify all users in tenant users_result = await db.execute( select(User).where(User.tenant_id == tenant_id, User.id != current_user.id) ) - for user in users_result.scalars().all(): + users = users_result.scalars().all() + for user in users: await send_notification( db, user_id=user.id, type="broadcast", @@ -167,6 +178,28 @@ async def broadcast_notification( ) count_agents += 1 - await db.commit() - return {"ok": True, "users_notified": count_users, "agents_notified": count_agents} + if req.send_email: + from app.services.system_email_service import send_system_email + + for user in users: + if not user.email: + continue + await send_system_email( + user.email, + req.title, + ( + f"{req.body}\n\n" + f"Sent by: {sender_name}" + if req.body.strip() + else f"Sent by: {sender_name}" + ), + ) + count_emails += 1 + await db.commit() + return { + "ok": True, + "users_notified": count_users, + "agents_notified": count_agents, + "emails_sent": count_emails, + } diff --git a/backend/app/config.py b/backend/app/config.py index 1f23fb1d..247cc8f9 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -64,11 +64,21 @@ class Settings(BaseSettings): JWT_SECRET_KEY: str = "change-me-jwt-secret" JWT_ALGORITHM: str = "HS256" JWT_ACCESS_TOKEN_EXPIRE_MINUTES: int = 60 * 24 # 24 hours + PASSWORD_RESET_TOKEN_EXPIRE_MINUTES: int = 60 # File Storage AGENT_DATA_DIR: str = _default_agent_data_dir() AGENT_TEMPLATE_DIR: str = "/app/agent_template" + # System email (platform-owned outbound mail) + SYSTEM_EMAIL_FROM_ADDRESS: str = "" + SYSTEM_EMAIL_FROM_NAME: str = "Clawith" + SYSTEM_SMTP_HOST: str = "" + SYSTEM_SMTP_PORT: int = 465 + SYSTEM_SMTP_USERNAME: str = "" + SYSTEM_SMTP_PASSWORD: str = "" + SYSTEM_SMTP_SSL: bool = True + # Docker (for Agent containers) DOCKER_NETWORK: str = "clawith_network" OPENCLAW_IMAGE: str = "openclaw:local" @@ -78,6 +88,7 @@ class Settings(BaseSettings): FEISHU_APP_ID: str = "" FEISHU_APP_SECRET: str = "" FEISHU_REDIRECT_URI: str = "" + PUBLIC_BASE_URL: str = "" # CORS CORS_ORIGINS: list[str] = ["http://localhost:3000", "http://localhost:5173"] diff --git a/backend/app/main.py b/backend/app/main.py index bb6fbb49..ebbe7962 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -98,6 +98,7 @@ async def lifespan(app: FastAPI): import app.models.trigger # noqa import app.models.notification # noqa import app.models.gateway_message # noqa + import app.models.password_reset_token # noqa async with engine.begin() as conn: await conn.run_sync(Base.metadata.create_all) logger.info("[startup] Database tables ready") diff --git a/backend/app/models/password_reset_token.py b/backend/app/models/password_reset_token.py new file mode 100644 index 00000000..737cc6bf --- /dev/null +++ b/backend/app/models/password_reset_token.py @@ -0,0 +1,23 @@ +"""Password reset token model.""" + +import uuid +from datetime import datetime + +from sqlalchemy import DateTime, ForeignKey, String, func +from sqlalchemy.dialects.postgresql import UUID +from sqlalchemy.orm import Mapped, mapped_column + +from app.database import Base + + +class PasswordResetToken(Base): + """Single-use password reset token.""" + + __tablename__ = "password_reset_tokens" + + id: Mapped[uuid.UUID] = mapped_column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) + user_id: Mapped[uuid.UUID] = mapped_column(UUID(as_uuid=True), ForeignKey("users.id", ondelete="CASCADE"), nullable=False, index=True) + token_hash: Mapped[str] = mapped_column(String(128), nullable=False, unique=True, index=True) + expires_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False, index=True) + used_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now(), nullable=False) diff --git a/backend/app/schemas/schemas.py b/backend/app/schemas/schemas.py index 5f96c2a0..6ea70a57 100644 --- a/backend/app/schemas/schemas.py +++ b/backend/app/schemas/schemas.py @@ -21,6 +21,15 @@ class UserLogin(BaseModel): password: str +class ForgotPasswordRequest(BaseModel): + email: EmailStr + + +class ResetPasswordRequest(BaseModel): + token: str = Field(min_length=20, max_length=512) + new_password: str = Field(min_length=6, max_length=128) + + class TokenResponse(BaseModel): access_token: str token_type: str = "bearer" @@ -441,4 +450,3 @@ class GatewaySendMessageRequest(BaseModel): target: str # Name of target person or agent content: str = Field(min_length=1) channel: str | None = None # Optional: "feishu", "agent", etc. Auto-detected if omitted. - diff --git a/backend/app/services/password_reset_service.py b/backend/app/services/password_reset_service.py new file mode 100644 index 00000000..b2c9085b --- /dev/null +++ b/backend/app/services/password_reset_service.py @@ -0,0 +1,78 @@ +"""Password reset token lifecycle helpers.""" + +from __future__ import annotations + +import hashlib +import secrets +import uuid +from datetime import datetime, timedelta, timezone + +from sqlalchemy import select, update +from sqlalchemy.ext.asyncio import AsyncSession + +from app.config import get_settings +from app.models.password_reset_token import PasswordResetToken +from app.models.system_settings import SystemSetting + + +def _hash_token(token: str) -> str: + """Hash a raw reset token before persistence or lookup.""" + return hashlib.sha256(token.encode("utf-8")).hexdigest() + + +async def create_password_reset_token(db: AsyncSession, user_id: uuid.UUID) -> tuple[str, datetime]: + """Create a new single-use token and invalidate older unused tokens.""" + now = datetime.now(timezone.utc) + await db.execute( + update(PasswordResetToken) + .where(PasswordResetToken.user_id == user_id, PasswordResetToken.used_at.is_(None)) + .values(used_at=now) + ) + + raw_token = secrets.token_urlsafe(32) + expires_at = now + timedelta(minutes=get_settings().PASSWORD_RESET_TOKEN_EXPIRE_MINUTES) + db.add( + PasswordResetToken( + user_id=user_id, + token_hash=_hash_token(raw_token), + expires_at=expires_at, + ) + ) + await db.flush() + return raw_token, expires_at + + +async def get_public_base_url(db: AsyncSession) -> str: + """Resolve the public base URL used for user-facing links.""" + result = await db.execute(select(SystemSetting).where(SystemSetting.key == "platform")) + setting = result.scalar_one_or_none() + if setting and setting.value and setting.value.get("public_base_url"): + return str(setting.value["public_base_url"]).strip().rstrip("/") + + env_value = getattr(get_settings(), "PUBLIC_BASE_URL", "") if hasattr(get_settings(), "PUBLIC_BASE_URL") else "" + env_value = str(env_value).strip().rstrip("/") + if env_value: + return env_value + + raise RuntimeError("Public base URL is not configured.") + + +async def build_password_reset_url(db: AsyncSession, raw_token: str) -> str: + """Build the user-facing reset URL.""" + base_url = await get_public_base_url(db) + return f"{base_url}/reset-password?token={raw_token}" + + +async def consume_password_reset_token(db: AsyncSession, raw_token: str) -> PasswordResetToken | None: + """Load a valid reset token and mark it used.""" + now = datetime.now(timezone.utc) + result = await db.execute( + select(PasswordResetToken).where(PasswordResetToken.token_hash == _hash_token(raw_token)) + ) + token = result.scalar_one_or_none() + if not token or token.used_at or token.expires_at <= now: + return None + + token.used_at = now + await db.flush() + return token diff --git a/backend/app/services/system_email_service.py b/backend/app/services/system_email_service.py new file mode 100644 index 00000000..9a78d146 --- /dev/null +++ b/backend/app/services/system_email_service.py @@ -0,0 +1,82 @@ +"""System-owned outbound email service.""" + +from __future__ import annotations + +import smtplib +import ssl +from dataclasses import dataclass +from datetime import datetime +from email.mime.multipart import MIMEMultipart +from email.mime.text import MIMEText +from email.utils import formataddr, make_msgid + +from app.config import get_settings +from app.services.email_service import _force_ipv4 + + +class SystemEmailConfigError(RuntimeError): + """Raised when system email configuration is missing or invalid.""" + + +@dataclass(slots=True) +class SystemEmailConfig: + """Resolved system email configuration.""" + + from_address: str + from_name: str + smtp_host: str + smtp_port: int + smtp_username: str + smtp_password: str + smtp_ssl: bool + + +def get_system_email_config() -> SystemEmailConfig: + """Resolve and validate the env-driven system email configuration.""" + settings = get_settings() + from_address = settings.SYSTEM_EMAIL_FROM_ADDRESS.strip() + smtp_host = settings.SYSTEM_SMTP_HOST.strip() + smtp_username = settings.SYSTEM_SMTP_USERNAME.strip() or from_address + smtp_password = settings.SYSTEM_SMTP_PASSWORD + + if not from_address or not smtp_host or not smtp_password: + raise SystemEmailConfigError( + "System email is not configured. Set SYSTEM_EMAIL_FROM_ADDRESS, SYSTEM_SMTP_HOST, and SYSTEM_SMTP_PASSWORD." + ) + + return SystemEmailConfig( + from_address=from_address, + from_name=settings.SYSTEM_EMAIL_FROM_NAME.strip() or "Clawith", + smtp_host=smtp_host, + smtp_port=settings.SYSTEM_SMTP_PORT, + smtp_username=smtp_username, + smtp_password=smtp_password, + smtp_ssl=settings.SYSTEM_SMTP_SSL, + ) + + +async def send_system_email(to: str, subject: str, body: str) -> None: + """Send a plain-text system email.""" + config = get_system_email_config() + + msg = MIMEMultipart() + msg["From"] = formataddr((config.from_name, config.from_address)) + msg["To"] = to + msg["Subject"] = subject + msg["Message-ID"] = make_msgid() + msg["Date"] = datetime.now().strftime("%a, %d %b %Y %H:%M:%S %z") + msg.attach(MIMEText(body, "plain", "utf-8")) + + with _force_ipv4(): + if config.smtp_ssl: + context = ssl.create_default_context() + with smtplib.SMTP_SSL(config.smtp_host, config.smtp_port, context=context, timeout=15) as server: + server.login(config.smtp_username, config.smtp_password) + server.sendmail(config.from_address, [to], msg.as_string()) + else: + with smtplib.SMTP(config.smtp_host, config.smtp_port, timeout=15) as server: + server.ehlo() + server.starttls(context=ssl.create_default_context()) + server.ehlo() + server.login(config.smtp_username, config.smtp_password) + server.sendmail(config.from_address, [to], msg.as_string()) diff --git a/backend/entrypoint.sh b/backend/entrypoint.sh index 8668d048..f944ddfb 100755 --- a/backend/entrypoint.sh +++ b/backend/entrypoint.sh @@ -47,6 +47,7 @@ async def main(): import app.models.trigger # noqa import app.models.notification # noqa import app.models.gateway_message # noqa + import app.models.password_reset_token # noqa # Create all tables that don't exist yet (safe to run on every startup) async with engine.begin() as conn: diff --git a/backend/tests/test_password_reset_and_notifications.py b/backend/tests/test_password_reset_and_notifications.py new file mode 100644 index 00000000..891afcf8 --- /dev/null +++ b/backend/tests/test_password_reset_and_notifications.py @@ -0,0 +1,211 @@ +import uuid +from datetime import datetime, timedelta, timezone +from types import SimpleNamespace + +import pytest +from fastapi import HTTPException + +from app.api import auth as auth_api +from app.api.notification import BroadcastRequest, broadcast_notification +from app.core.security import verify_password +from app.models.password_reset_token import PasswordResetToken +from app.models.user import User +from app.schemas.schemas import ForgotPasswordRequest, ResetPasswordRequest +from app.services import password_reset_service +from app.services.system_email_service import SystemEmailConfigError + + +class DummyScalars: + def __init__(self, values): + self._values = list(values) + + def all(self): + return list(self._values) + + +class DummyResult: + def __init__(self, value=None, values=None): + self._value = value + self._values = list(values or []) + + def scalar_one_or_none(self): + return self._value + + def scalars(self): + return DummyScalars(self._values) + + +class RecordingDB: + def __init__(self, responses=None): + self.responses = list(responses or []) + self.executed = [] + self.added = [] + self.flushed = False + self.committed = False + + async def execute(self, statement): + self.executed.append(statement) + if self.responses: + return self.responses.pop(0) + return DummyResult() + + def add(self, obj): + self.added.append(obj) + + async def flush(self): + self.flushed = True + + async def commit(self): + self.committed = True + + +def make_user(**overrides): + values = { + "id": uuid.uuid4(), + "username": "alice", + "email": "alice@example.com", + "password_hash": "old-hash", + "display_name": "Alice", + "role": "member", + "tenant_id": uuid.uuid4(), + "is_active": True, + } + values.update(overrides) + return User(**values) + + +@pytest.mark.asyncio +async def test_create_password_reset_token_invalidates_older_tokens(monkeypatch): + monkeypatch.setattr( + password_reset_service, + "get_settings", + lambda: SimpleNamespace(PASSWORD_RESET_TOKEN_EXPIRE_MINUTES=15, PUBLIC_BASE_URL=""), + ) + db = RecordingDB() + user_id = uuid.uuid4() + + raw_token, expires_at = await password_reset_service.create_password_reset_token(db, user_id) + + assert db.flushed is True + assert len(db.executed) == 1 + assert "UPDATE password_reset_tokens" in str(db.executed[0]) + assert len(db.added) == 1 + saved_token = db.added[0] + assert isinstance(saved_token, PasswordResetToken) + assert saved_token.user_id == user_id + assert saved_token.token_hash != raw_token + assert len(raw_token) >= 20 + assert expires_at > datetime.now(timezone.utc) + + +@pytest.mark.asyncio +async def test_build_password_reset_url_uses_env_public_base_url(monkeypatch): + monkeypatch.setattr( + password_reset_service, + "get_settings", + lambda: SimpleNamespace(PASSWORD_RESET_TOKEN_EXPIRE_MINUTES=30, PUBLIC_BASE_URL="https://app.example.com/"), + ) + db = RecordingDB([DummyResult(None)]) + + url = await password_reset_service.build_password_reset_url(db, "abc123") + + assert url == "https://app.example.com/reset-password?token=abc123" + + +@pytest.mark.asyncio +async def test_consume_password_reset_token_rejects_expired_tokens(): + expired = PasswordResetToken( + user_id=uuid.uuid4(), + token_hash="hashed", + expires_at=datetime.now(timezone.utc) - timedelta(minutes=1), + ) + db = RecordingDB([DummyResult(expired)]) + + token = await password_reset_service.consume_password_reset_token(db, "raw-token") + + assert token is None + assert expired.used_at is None + + +@pytest.mark.asyncio +async def test_forgot_password_returns_generic_response_for_unknown_email(): + db = RecordingDB([DummyResult(None)]) + + response = await auth_api.forgot_password(ForgotPasswordRequest(email="missing@example.com"), db) + + assert response == { + "ok": True, + "message": "If an account with that email exists, a password reset email has been sent.", + } + + +@pytest.mark.asyncio +async def test_forgot_password_hides_email_delivery_failures(monkeypatch): + user = make_user() + db = RecordingDB([DummyResult(user)]) + + async def fake_create_password_reset_token(*_args, **_kwargs): + raise RuntimeError("smtp failed") + + monkeypatch.setattr(password_reset_service, "create_password_reset_token", fake_create_password_reset_token) + + response = await auth_api.forgot_password(ForgotPasswordRequest(email=user.email), db) + + assert response["ok"] is True + assert "password reset email" in response["message"] + + +@pytest.mark.asyncio +async def test_reset_password_updates_user_and_invalidates_other_tokens(monkeypatch): + user = make_user(password_hash=auth_api.hash_password("old-password")) + consumed = PasswordResetToken( + id=uuid.uuid4(), + user_id=user.id, + token_hash="current", + expires_at=datetime.now(timezone.utc) + timedelta(minutes=30), + ) + older = PasswordResetToken( + id=uuid.uuid4(), + user_id=user.id, + token_hash="older", + expires_at=datetime.now(timezone.utc) + timedelta(minutes=30), + ) + db = RecordingDB([DummyResult(user), DummyResult(values=[consumed, older])]) + + async def fake_consume_password_reset_token(*_args, **_kwargs): + return consumed + + monkeypatch.setattr(password_reset_service, "consume_password_reset_token", fake_consume_password_reset_token) + + response = await auth_api.reset_password( + ResetPasswordRequest(token="t" * 20, new_password="new-password"), + db, + ) + + assert response == {"ok": True} + assert verify_password("new-password", user.password_hash) + assert older.used_at is not None + assert db.flushed is True + + +@pytest.mark.asyncio +async def test_broadcast_notification_rejects_missing_system_email_config(monkeypatch): + current_user = make_user(role="org_admin") + + def fake_get_system_email_config(): + raise SystemEmailConfigError("missing smtp host") + + monkeypatch.setattr( + "app.services.system_email_service.get_system_email_config", + fake_get_system_email_config, + ) + + with pytest.raises(HTTPException) as excinfo: + await broadcast_notification( + BroadcastRequest(title="Maintenance", body="Tonight", send_email=True), + current_user=current_user, + db=RecordingDB(), + ) + + assert excinfo.value.status_code == 400 + assert "System email is not configured" in excinfo.value.detail diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 9531b273..3776dd07 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -3,6 +3,8 @@ import { useAuthStore } from './stores'; import { useEffect, useState } from 'react'; import { authApi } from './services/api'; import Login from './pages/Login'; +import ForgotPassword from './pages/ForgotPassword'; +import ResetPassword from './pages/ResetPassword'; import CompanySetup from './pages/CompanySetup'; import Layout from './pages/Layout'; import Dashboard from './pages/Dashboard'; @@ -104,6 +106,8 @@ export default function App() { } /> + } /> + } /> } /> }> } /> diff --git a/frontend/src/pages/EnterpriseSettings.tsx b/frontend/src/pages/EnterpriseSettings.tsx index a6b1dad0..860440a6 100644 --- a/frontend/src/pages/EnterpriseSettings.tsx +++ b/frontend/src/pages/EnterpriseSettings.tsx @@ -935,8 +935,9 @@ function BroadcastSection() { const { t } = useTranslation(); const [title, setTitle] = useState(''); const [body, setBody] = useState(''); + const [sendEmail, setSendEmail] = useState(false); const [sending, setSending] = useState(false); - const [result, setResult] = useState<{ users: number; agents: number } | null>(null); + const [result, setResult] = useState<{ users: number; agents: number; emails: number } | null>(null); const handleSend = async () => { if (!title.trim()) return; @@ -947,7 +948,7 @@ function BroadcastSection() { const res = await fetch('/api/notifications/broadcast', { method: 'POST', headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${token}` }, - body: JSON.stringify({ title: title.trim(), body: body.trim() }), + body: JSON.stringify({ title: title.trim(), body: body.trim(), send_email: sendEmail }), }); if (!res.ok) { const err = await res.json().catch(() => ({})); @@ -956,9 +957,14 @@ function BroadcastSection() { return; } const data = await res.json(); - setResult({ users: data.users_notified, agents: data.agents_notified }); + setResult({ + users: data.users_notified, + agents: data.agents_notified, + emails: data.emails_sent || 0, + }); setTitle(''); setBody(''); + setSendEmail(false); } catch (e: any) { alert(e.message || 'Failed'); } @@ -989,13 +995,25 @@ function BroadcastSection() { rows={3} style={{ resize: 'vertical', fontSize: '13px', marginBottom: '12px' }} /> +
{result && ( - {t('enterprise.broadcast.sent', `Sent to ${result.users} users and ${result.agents} agents`, { users: result.users, agents: result.agents })} + {t( + 'enterprise.broadcast.sentWithEmail', + `Sent to ${result.users} users, ${result.agents} agents, and ${result.emails} email recipients`, + { users: result.users, agents: result.agents, emails: result.emails }, + )} )}
diff --git a/frontend/src/pages/ForgotPassword.tsx b/frontend/src/pages/ForgotPassword.tsx new file mode 100644 index 00000000..c74696c7 --- /dev/null +++ b/frontend/src/pages/ForgotPassword.tsx @@ -0,0 +1,83 @@ +import { useEffect, useState } from 'react'; +import { Link } from 'react-router-dom'; +import { authApi } from '../services/api'; + +export default function ForgotPassword() { + const [email, setEmail] = useState(''); + const [loading, setLoading] = useState(false); + const [error, setError] = useState(''); + const [message, setMessage] = useState(''); + + useEffect(() => { + document.documentElement.setAttribute('data-theme', 'dark'); + }, []); + + const handleSubmit = async (e: React.FormEvent) => { + e.preventDefault(); + setError(''); + setMessage(''); + setLoading(true); + + try { + const res = await authApi.forgotPassword({ email: email.trim() }); + setMessage(res.message); + } catch (err: any) { + setError(err.message || 'Failed to request password reset'); + } finally { + setLoading(false); + } + }; + + return ( +
+
+
+
+
+ + Clawith +
+

Forgot password

+

+ Enter your account email and we will send a reset link if the account exists. +

+
+ + {error && ( +
+ ⚠ {error} +
+ )} + + {message && ( +
+ ✓ {message} +
+ )} + +
+
+ + setEmail(e.target.value)} + required + autoFocus + placeholder="name@company.com" + /> +
+ + +
+ +
+ Remembered your password? Back to login +
+
+
+
+ ); +} diff --git a/frontend/src/pages/Login.tsx b/frontend/src/pages/Login.tsx index caffef70..85e70347 100644 --- a/frontend/src/pages/Login.tsx +++ b/frontend/src/pages/Login.tsx @@ -1,5 +1,5 @@ import { useState, useEffect } from 'react'; -import { useNavigate } from 'react-router-dom'; +import { Link, useNavigate } from 'react-router-dom'; import { useTranslation } from 'react-i18next'; import { useAuthStore } from '../stores'; import { authApi } from '../services/api'; @@ -181,6 +181,17 @@ export default function Login() { /> + {!isRegister && ( +
+ + {t('auth.forgotPassword', 'Forgot password?')} + +
+ )} + + + +
+ Back to login +
+ + + + ); +} diff --git a/frontend/src/services/api.ts b/frontend/src/services/api.ts index 283f82e4..a62c168b 100644 --- a/frontend/src/services/api.ts +++ b/frontend/src/services/api.ts @@ -15,7 +15,10 @@ async function request(url: string, options: RequestInit = {}): Promise { if (!res.ok) { // Auto-logout on expired/invalid token (but not on auth endpoints — let them show errors) - const isAuthEndpoint = url.startsWith('/auth/login') || url.startsWith('/auth/register'); + const isAuthEndpoint = url.startsWith('/auth/login') + || url.startsWith('/auth/register') + || url.startsWith('/auth/forgot-password') + || url.startsWith('/auth/reset-password'); if (res.status === 401 && !isAuthEndpoint) { localStorage.removeItem('token'); localStorage.removeItem('user'); @@ -133,6 +136,12 @@ export const authApi = { login: (data: { username: string; password: string }) => request('/auth/login', { method: 'POST', body: JSON.stringify(data) }), + forgotPassword: (data: { email: string }) => + request<{ ok: boolean; message: string }>('/auth/forgot-password', { method: 'POST', body: JSON.stringify(data) }), + + resetPassword: (data: { token: string; new_password: string }) => + request<{ ok: boolean }>('/auth/reset-password', { method: 'POST', body: JSON.stringify(data) }), + me: () => request('/auth/me'), updateMe: (data: Partial) => From ca3c2f09191773ec41eaa10ef3bcc670312b3dc2 Mon Sep 17 00:00:00 2001 From: Atlas Date: Tue, 24 Mar 2026 10:47:07 +0800 Subject: [PATCH 2/5] Localize password recovery and broadcast email UI copy This change removes English-only fallback text from the new password recovery flow and broadcast email controls so the feature matches the existing bilingual frontend behavior. The new auth pages now read from i18n, and both English and Simplified Chinese dictionaries include the new strings. Constraint: New user-facing copy must follow the existing frontend i18n structure instead of introducing page-local string tables Rejected: Leaving default English fallbacks in component code | inconsistent with the rest of the localized UI Confidence: high Scope-risk: narrow Reversibility: clean Directive: Add future auth and enterprise UI copy to locale files at the same time as the component change Tested: frontend npm run build Tested: manual browser check of /forgot-password Chinese rendering Not-tested: full language-switch regression across all newly added strings --- frontend/src/i18n/en.json | 33 +++++++++++++++++++++++++-- frontend/src/i18n/zh.json | 29 +++++++++++++++++++++++ frontend/src/pages/ForgotPassword.tsx | 16 +++++++------ frontend/src/pages/ResetPassword.tsx | 28 ++++++++++++----------- 4 files changed, 84 insertions(+), 22 deletions(-) diff --git a/frontend/src/i18n/en.json b/frontend/src/i18n/en.json index 51e9a1b5..ece9aa2e 100644 --- a/frontend/src/i18n/en.json +++ b/frontend/src/i18n/en.json @@ -79,7 +79,27 @@ "invitationHint": "Token consumption is significant, so invitation codes are required. We recommend deploying your own instance and configuring leading models for the best experience.", "usernamePlaceholder": "Enter username", "passwordPlaceholder": "Enter password", - "emailPlaceholder": "you@example.com" + "emailPlaceholder": "you@example.com", + "forgotPassword": "Forgot password?", + "forgotPasswordTitle": "Forgot password", + "forgotPasswordSubtitle": "Enter your account email and we will send a reset link if the account exists.", + "forgotPasswordRequestFailed": "Failed to request password reset", + "sendResetLink": "Send reset link", + "rememberedPassword": "Remembered your password?", + "backToLogin": "Back to login", + "resetPasswordTitle": "Reset password", + "resetPasswordSubtitle": "Choose a new password for your account.", + "resetPasswordMissingToken": "Reset token is missing from the link.", + "resetPasswordTooShort": "New password must be at least 6 characters.", + "resetPasswordMismatch": "Passwords do not match.", + "resetPasswordFailed": "Failed to reset password", + "resetPasswordSuccess": "Password updated. Redirecting to login...", + "newPassword": "New password", + "newPasswordPlaceholder": "At least 6 characters", + "confirmNewPassword": "Confirm new password", + "confirmNewPasswordPlaceholder": "Repeat your new password", + "updatePassword": "Update password", + "emailPlaceholderReset": "name@company.com" }, "roles": { "platformAdmin": "Platform Admin", @@ -755,6 +775,15 @@ "saved": "Saved", "themeColor": "Theme Color" }, + "broadcast": { + "title": "Broadcast Notification", + "description": "Send a notification to all users and agents in this company.", + "titlePlaceholder": "Notification title", + "bodyPlaceholder": "Optional details...", + "sendEmail": "Also send email to users with a configured address", + "send": "Send Broadcast", + "sentWithEmail": "Sent to {{users}} users, {{agents}} agents, and {{emails}} email recipients" + }, "kb": { "title": "Company Knowledge Base", "rootDir": "Root", @@ -921,4 +950,4 @@ "ws_note": "WebSocket mode requires no public IP, callback URL, or domain verification (ICP). The connection is managed automatically." } } -} \ No newline at end of file +} diff --git a/frontend/src/i18n/zh.json b/frontend/src/i18n/zh.json index 08bef963..9e8e2c77 100644 --- a/frontend/src/i18n/zh.json +++ b/frontend/src/i18n/zh.json @@ -86,6 +86,26 @@ "usernamePlaceholder": "请输入用户名", "passwordPlaceholder": "请输入密码", "emailPlaceholder": "your@email.com", + "forgotPassword": "忘记密码？", + "forgotPasswordTitle": "忘记密码", + "forgotPasswordSubtitle": "输入你的账号邮箱；如果该账号存在，系统会发送一封重置链接邮件。", + "forgotPasswordRequestFailed": "请求密码重置失败", + "sendResetLink": "发送重置链接", + "rememberedPassword": "想起密码了？", + "backToLogin": "返回登录", + "resetPasswordTitle": "重置密码", + "resetPasswordSubtitle": "为你的账号设置一个新密码。", + "resetPasswordMissingToken": "链接中缺少重置令牌。", + "resetPasswordTooShort": "新密码至少需要 6 个字符。", + "resetPasswordMismatch": "两次输入的密码不一致。", + "resetPasswordFailed": "重置密码失败", + "resetPasswordSuccess": "密码已更新，正在跳转到登录页...", + "newPassword": "新密码", + "newPasswordPlaceholder": "至少 6 个字符", + "confirmNewPassword": "确认新密码", + "confirmNewPasswordPlaceholder": "再次输入新密码", + "updatePassword": "更新密码", + "emailPlaceholderReset": "name@company.com", "companyDisabled": "你的公司已被停用，请联系平台管理员。", "invalidCredentials": "用户名或密码错误。", "accountDisabled": "你的账号已被停用。", @@ -826,6 +846,15 @@ "saved": "已保存", "themeColor": "主题色" }, + "broadcast": { + "title": "广播通知", + "description": "向本公司所有用户和数字员工发送通知。", + "titlePlaceholder": "通知标题", + "bodyPlaceholder": "可选补充说明...", + "sendEmail": "同时给已配置邮箱地址的用户发送邮件", + "send": "发送广播", + "sentWithEmail": "已发送给 {{users}} 位用户、{{agents}} 个数字员工，并投递到 {{emails}} 个邮箱" + }, "companyName": { "title": "公司名称", "placeholder": "输入公司名称" diff --git a/frontend/src/pages/ForgotPassword.tsx b/frontend/src/pages/ForgotPassword.tsx index c74696c7..26c05f92 100644 --- a/frontend/src/pages/ForgotPassword.tsx +++ b/frontend/src/pages/ForgotPassword.tsx @@ -1,8 +1,10 @@ import { useEffect, useState } from 'react'; import { Link } from 'react-router-dom'; +import { useTranslation } from 'react-i18next'; import { authApi } from '../services/api'; export default function ForgotPassword() { + const { t } = useTranslation(); const [email, setEmail] = useState(''); const [loading, setLoading] = useState(false); const [error, setError] = useState(''); @@ -22,7 +24,7 @@ export default function ForgotPassword() { const res = await authApi.forgotPassword({ email: email.trim() }); setMessage(res.message); } catch (err: any) { - setError(err.message || 'Failed to request password reset'); + setError(err.message || t('auth.forgotPasswordRequestFailed', 'Failed to request password reset')); } finally { setLoading(false); } @@ -37,9 +39,9 @@ export default function ForgotPassword() { Clawith -

Forgot password

+

{t('auth.forgotPasswordTitle', 'Forgot password')}

- Enter your account email and we will send a reset link if the account exists. + {t('auth.forgotPasswordSubtitle', 'Enter your account email and we will send a reset link if the account exists.')}

@@ -57,24 +59,24 @@ export default function ForgotPassword() {
- + setEmail(e.target.value)} required autoFocus - placeholder="name@company.com" + placeholder={t('auth.emailPlaceholderReset', 'name@company.com')} />
- Remembered your password? Back to login + {t('auth.rememberedPassword', 'Remembered your password?')} {t('auth.backToLogin', 'Back to login')}
diff --git a/frontend/src/pages/ResetPassword.tsx b/frontend/src/pages/ResetPassword.tsx index 674d86d4..a2432c5c 100644 --- a/frontend/src/pages/ResetPassword.tsx +++ b/frontend/src/pages/ResetPassword.tsx @@ -1,8 +1,10 @@ import { useEffect, useMemo, useState } from 'react'; import { Link, useNavigate, useSearchParams } from 'react-router-dom'; +import { useTranslation } from 'react-i18next'; import { authApi } from '../services/api'; export default function ResetPassword() { + const { t } = useTranslation(); const navigate = useNavigate(); const [params] = useSearchParams(); const token = useMemo(() => params.get('token') || '', [params]); @@ -21,15 +23,15 @@ export default function ResetPassword() { setError(''); if (!token) { - setError('Reset token is missing from the link.'); + setError(t('auth.resetPasswordMissingToken', 'Reset token is missing from the link.')); return; } if (password.length < 6) { - setError('New password must be at least 6 characters.'); + setError(t('auth.resetPasswordTooShort', 'New password must be at least 6 characters.')); return; } if (password !== confirmPassword) { - setError('Passwords do not match.'); + setError(t('auth.resetPasswordMismatch', 'Passwords do not match.')); return; } @@ -39,7 +41,7 @@ export default function ResetPassword() { setSuccess(true); window.setTimeout(() => navigate('/login'), 1200); } catch (err: any) { - setError(err.message || 'Failed to reset password'); + setError(err.message || t('auth.resetPasswordFailed', 'Failed to reset password')); } finally { setLoading(false); } @@ -54,9 +56,9 @@ export default function ResetPassword() { Clawith -

Reset password

+

{t('auth.resetPasswordTitle', 'Reset password')}

- Choose a new password for your account. + {t('auth.resetPasswordSubtitle', 'Choose a new password for your account.')}

@@ -68,41 +70,41 @@ export default function ResetPassword() { {success && (
- ✓ Password updated. Redirecting to login... + ✓ {t('auth.resetPasswordSuccess', 'Password updated. Redirecting to login...')}
)}
- + setPassword(e.target.value)} required autoFocus - placeholder="At least 6 characters" + placeholder={t('auth.newPasswordPlaceholder', 'At least 6 characters')} />
- + setConfirmPassword(e.target.value)} required - placeholder="Repeat your new password" + placeholder={t('auth.confirmNewPasswordPlaceholder', 'Repeat your new password')} />
- Back to login + {t('auth.backToLogin', 'Back to login')}
From e8203b8502035c319c61071e8ef0ee6fa6d8e93b Mon Sep 17 00:00:00 2001 From: Atlas Date: Tue, 24 Mar 2026 10:56:23 +0800 Subject: [PATCH 3/5] Prevent SMTP latency from blocking password recovery and broadcasts This change moves system email delivery off the main request path for forgot-password and broadcast email sends. Password recovery now queues email delivery after persisting the reset token, and broadcast email sends are isolated per recipient so one SMTP failure does not abort the entire operation. Constraint: Keep the current env-driven SMTP configuration and avoid adding queue infrastructure Constraint: Requests must remain responsive even when SMTP is slow or misconfigured Rejected: Leave SMTP on the request path | risks slow or stalled user-facing requests Rejected: Add a durable mail queue now | too much scope for a targeted hardening pass Confidence: high Scope-risk: narrow Reversibility: clean Directive: Treat emails_sent in broadcast responses as queued recipients, not guaranteed SMTP success Tested: backend pytest tests/test_password_reset_and_notifications.py Tested: backend ruff check app/api/auth.py app/api/notification.py app/services/system_email_service.py tests/test_password_reset_and_notifications.py Tested: manual browser E2E reset-password -> login flow after the hardening change Not-tested: process-crash behavior between response return and background email completion --- backend/app/api/auth.py | 29 +++-- backend/app/api/notification.py | 30 ++++-- backend/app/services/system_email_service.py | 72 ++++++++++++- .../test_password_reset_and_notifications.py | 100 +++++++++++++++++- 4 files changed, 204 insertions(+), 27 deletions(-) diff --git a/backend/app/api/auth.py b/backend/app/api/auth.py index ea9c5a07..d1915af1 100644 --- a/backend/app/api/auth.py +++ b/backend/app/api/auth.py @@ -2,7 +2,7 @@ from datetime import datetime, timezone -from fastapi import APIRouter, Depends, HTTPException, status +from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, status from loguru import logger from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession @@ -151,7 +151,11 @@ async def login(data: UserLogin, db: AsyncSession = Depends(get_db)): @router.post("/forgot-password") -async def forgot_password(data: ForgotPasswordRequest, db: AsyncSession = Depends(get_db)): +async def forgot_password( + data: ForgotPasswordRequest, + background_tasks: BackgroundTasks, + db: AsyncSession = Depends(get_db), +): """Request a password reset link without revealing account existence.""" generic_response = { "ok": True, @@ -165,22 +169,25 @@ async def forgot_password(data: ForgotPasswordRequest, db: AsyncSession = Depend try: from app.services.password_reset_service import build_password_reset_url, create_password_reset_token - from app.services.system_email_service import send_system_email + from app.services.system_email_service import ( + get_system_email_config, + run_background_email_job, + send_password_reset_email, + ) + get_system_email_config() raw_token, expires_at = await create_password_reset_token(db, user.id) await db.commit() reset_url = await build_password_reset_url(db, raw_token) expiry_minutes = int((expires_at - datetime.now(timezone.utc)).total_seconds() // 60) - await send_system_email( + background_tasks.add_task( + run_background_email_job, + send_password_reset_email, user.email, - "Reset your Clawith password", - ( - f"Hello {user.display_name or user.username},\n\n" - f"We received a request to reset your Clawith password.\n\n" - f"Reset link: {reset_url}\n\n" - f"This link expires in {expiry_minutes} minutes. If you did not request this, you can ignore this email." - ), + user.display_name or user.username, + reset_url, + expiry_minutes, ) except Exception as exc: logger.warning(f"Failed to process password reset email for {data.email}: {exc}") diff --git a/backend/app/api/notification.py b/backend/app/api/notification.py index 3e8d31bf..81cd47a5 100644 --- a/backend/app/api/notification.py +++ b/backend/app/api/notification.py @@ -3,7 +3,7 @@ import uuid from typing import Optional -from fastapi import APIRouter, Depends, HTTPException, Query +from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Query from pydantic import BaseModel, Field from sqlalchemy import select, func, update from sqlalchemy.ext.asyncio import AsyncSession @@ -122,6 +122,7 @@ class BroadcastRequest(BaseModel): @router.post("/notifications/broadcast") async def broadcast_notification( req: BroadcastRequest, + background_tasks: BackgroundTasks, current_user: User = Depends(get_current_user), db: AsyncSession = Depends(get_db), ): @@ -140,6 +141,7 @@ async def broadcast_notification( count_users = 0 count_agents = 0 count_emails = 0 + email_recipients = [] if req.send_email: from app.services.system_email_service import get_system_email_config @@ -179,24 +181,32 @@ async def broadcast_notification( count_agents += 1 if req.send_email: - from app.services.system_email_service import send_system_email + from app.services.system_email_service import ( + BroadcastEmailRecipient, + deliver_broadcast_emails, + run_background_email_job, + ) for user in users: if not user.email: continue - await send_system_email( - user.email, - req.title, - ( - f"{req.body}\n\n" - f"Sent by: {sender_name}" - if req.body.strip() - else f"Sent by: {sender_name}" + email_recipients.append( + BroadcastEmailRecipient( + email=user.email, + subject=req.title, + body=( + f"{req.body}\n\n" + f"Sent by: {sender_name}" + if req.body.strip() + else f"Sent by: {sender_name}" + ), ), ) count_emails += 1 await db.commit() + if email_recipients: + background_tasks.add_task(run_background_email_job, deliver_broadcast_emails, email_recipients) return { "ok": True, "users_notified": count_users, diff --git a/backend/app/services/system_email_service.py b/backend/app/services/system_email_service.py index 9a78d146..6ec47e99 100644 --- a/backend/app/services/system_email_service.py +++ b/backend/app/services/system_email_service.py @@ -2,8 +2,12 @@ from __future__ import annotations +import asyncio +import inspect +import logging import smtplib import ssl +from collections.abc import Iterable from dataclasses import dataclass from datetime import datetime from email.mime.multipart import MIMEMultipart @@ -13,6 +17,8 @@ from app.config import get_settings from app.services.email_service import _force_ipv4 +logger = logging.getLogger(__name__) + class SystemEmailConfigError(RuntimeError): """Raised when system email configuration is missing or invalid.""" @@ -31,6 +37,15 @@ class SystemEmailConfig: smtp_ssl: bool +@dataclass(slots=True) +class BroadcastEmailRecipient: + """Prepared broadcast recipient payload.""" + + email: str + subject: str + body: str + + def get_system_email_config() -> SystemEmailConfig: """Resolve and validate the env-driven system email configuration.""" settings = get_settings() @@ -55,8 +70,8 @@ def get_system_email_config() -> SystemEmailConfig: ) -async def send_system_email(to: str, subject: str, body: str) -> None: - """Send a plain-text system email.""" +def _send_system_email_sync(to: str, subject: str, body: str) -> None: + """Send a plain-text system email synchronously.""" config = get_system_email_config() msg = MIMEMultipart() @@ -80,3 +95,56 @@ async def send_system_email(to: str, subject: str, body: str) -> None: server.ehlo() server.login(config.smtp_username, config.smtp_password) server.sendmail(config.from_address, [to], msg.as_string()) + + +async def send_system_email(to: str, subject: str, body: str) -> None: + """Send a plain-text system email without blocking the event loop.""" + await asyncio.to_thread(_send_system_email_sync, to, subject, body) + + +async def send_password_reset_email( + to: str, + display_name: str, + reset_url: str, + expiry_minutes: int, +) -> None: + """Send a password reset email.""" + await send_system_email( + to, + "Reset your Clawith password", + ( + f"Hello {display_name},\n\n" + f"We received a request to reset your Clawith password.\n\n" + f"Reset link: {reset_url}\n\n" + f"This link expires in {expiry_minutes} minutes. If you did not request this, you can ignore this email." + ), + ) + + +async def deliver_broadcast_emails(recipients: Iterable[BroadcastEmailRecipient]) -> None: + """Deliver broadcast emails while isolating per-recipient failures.""" + for recipient in recipients: + try: + await send_system_email(recipient.email, recipient.subject, recipient.body) + except Exception as exc: + logger.warning("Failed to deliver broadcast email to %s: %s", recipient.email, exc) + + +def fire_and_forget(coro) -> None: + """Run an awaitable in the background without failing the request.""" + task = asyncio.create_task(coro) + + def _consume_task_result(done_task: asyncio.Task) -> None: + try: + done_task.result() + except Exception as exc: + logger.warning("Background email task failed: %s", exc) + + task.add_done_callback(_consume_task_result) + + +def run_background_email_job(job, *args, **kwargs) -> None: + """Bridge Starlette background tasks to async email jobs.""" + result = job(*args, **kwargs) + if inspect.isawaitable(result): + fire_and_forget(result) diff --git a/backend/tests/test_password_reset_and_notifications.py b/backend/tests/test_password_reset_and_notifications.py index 891afcf8..ddc457ad 100644 --- a/backend/tests/test_password_reset_and_notifications.py +++ b/backend/tests/test_password_reset_and_notifications.py @@ -4,6 +4,7 @@ import pytest from fastapi import HTTPException +from starlette.background import BackgroundTasks from app.api import auth as auth_api from app.api.notification import BroadcastRequest, broadcast_notification @@ -130,29 +131,63 @@ async def test_consume_password_reset_token_rejects_expired_tokens(): @pytest.mark.asyncio async def test_forgot_password_returns_generic_response_for_unknown_email(): db = RecordingDB([DummyResult(None)]) + background_tasks = BackgroundTasks() - response = await auth_api.forgot_password(ForgotPasswordRequest(email="missing@example.com"), db) + response = await auth_api.forgot_password( + ForgotPasswordRequest(email="missing@example.com"), + background_tasks, + db, + ) assert response == { "ok": True, "message": "If an account with that email exists, a password reset email has been sent.", } + assert background_tasks.tasks == [] @pytest.mark.asyncio async def test_forgot_password_hides_email_delivery_failures(monkeypatch): user = make_user() db = RecordingDB([DummyResult(user)]) + background_tasks = BackgroundTasks() - async def fake_create_password_reset_token(*_args, **_kwargs): + def fake_get_system_email_config(): raise RuntimeError("smtp failed") - monkeypatch.setattr(password_reset_service, "create_password_reset_token", fake_create_password_reset_token) + monkeypatch.setattr("app.services.system_email_service.get_system_email_config", fake_get_system_email_config) - response = await auth_api.forgot_password(ForgotPasswordRequest(email=user.email), db) + response = await auth_api.forgot_password(ForgotPasswordRequest(email=user.email), background_tasks, db) assert response["ok"] is True assert "password reset email" in response["message"] + assert background_tasks.tasks == [] + + +@pytest.mark.asyncio +async def test_forgot_password_queues_background_email(monkeypatch): + user = make_user() + db = RecordingDB([DummyResult(user)]) + background_tasks = BackgroundTasks() + + async def fake_create_password_reset_token(*_args, **_kwargs): + return "raw-token", datetime.now(timezone.utc) + timedelta(minutes=30) + + async def fake_build_password_reset_url(*_args, **_kwargs): + return "https://app.example.com/reset-password?token=raw-token" + + monkeypatch.setattr(password_reset_service, "create_password_reset_token", fake_create_password_reset_token) + monkeypatch.setattr(password_reset_service, "build_password_reset_url", fake_build_password_reset_url) + monkeypatch.setattr( + "app.services.system_email_service.get_system_email_config", + lambda: SimpleNamespace(from_address="bot@example.com"), + ) + + response = await auth_api.forgot_password(ForgotPasswordRequest(email=user.email), background_tasks, db) + + assert response["ok"] is True + assert db.committed is True + assert len(background_tasks.tasks) == 1 @pytest.mark.asyncio @@ -203,9 +238,66 @@ def fake_get_system_email_config(): with pytest.raises(HTTPException) as excinfo: await broadcast_notification( BroadcastRequest(title="Maintenance", body="Tonight", send_email=True), + background_tasks=BackgroundTasks(), current_user=current_user, db=RecordingDB(), ) assert excinfo.value.status_code == 400 assert "System email is not configured" in excinfo.value.detail + + +@pytest.mark.asyncio +async def test_broadcast_notification_queues_email_delivery(monkeypatch): + current_user = make_user(role="org_admin") + target_user = make_user(email="bob@example.com", tenant_id=current_user.tenant_id) + db = RecordingDB([ + DummyResult(values=[target_user]), + DummyResult(values=[]), + ]) + background_tasks = BackgroundTasks() + + monkeypatch.setattr( + "app.services.system_email_service.get_system_email_config", + lambda: SimpleNamespace(from_address="bot@example.com"), + ) + notifications = [] + + async def fake_send_notification(*_args, **kwargs): + notifications.append(kwargs) + + monkeypatch.setattr("app.services.notification_service.send_notification", fake_send_notification) + + response = await broadcast_notification( + BroadcastRequest(title="Maintenance", body="Tonight", send_email=True), + background_tasks=background_tasks, + current_user=current_user, + db=db, + ) + + assert response["ok"] is True + assert response["emails_sent"] == 1 + assert db.committed is True + assert len(notifications) == 1 + assert len(background_tasks.tasks) == 1 + + +@pytest.mark.asyncio +async def test_deliver_broadcast_emails_continues_after_single_failure(monkeypatch): + from app.services.system_email_service import BroadcastEmailRecipient, deliver_broadcast_emails + + delivered = [] + + async def fake_send_system_email(email: str, subject: str, body: str) -> None: + if email == "bad@example.com": + raise RuntimeError("smtp down") + delivered.append((email, subject, body)) + + monkeypatch.setattr("app.services.system_email_service.send_system_email", fake_send_system_email) + + await deliver_broadcast_emails([ + BroadcastEmailRecipient(email="bad@example.com", subject="s1", body="b1"), + BroadcastEmailRecipient(email="good@example.com", subject="s2", body="b2"), + ]) + + assert delivered == [("good@example.com", "s2", "b2")] From d1f2a0bda70a7d4cbce3cc43293ceb2f6f83b393 Mon Sep 17 00:00:00 2001 From: Atlas Date: Thu, 26 Mar 2026 13:18:39 +0800 Subject: [PATCH 4/5] Improve production readiness of password reset email delivery Address PR #178 review follow-ups by making system SMTP timeout configurable and clarifying PUBLIC_BASE_URL expectations for production reset links.\n\nConstraint: Keep SMTP env-driven and avoid introducing queue infrastructure or behavior changes to auth responses\nRejected: Leaving SMTP timeout hardcoded at 15s | blocks operator tuning for slow providers\nConfidence: high\nScope-risk: narrow\nReversibility: clean\nDirective: Keep reset-link generation on PUBLIC_BASE_URL/public_base_url and avoid localhost values in production\nTested: backend/.venv/bin/python -m pytest tests/test_password_reset_and_notifications.py\nTested: backend/.venv/bin/python -m ruff check app/config.py app/services/system_email_service.py app/services/password_reset_service.py tests/test_password_reset_and_notifications.py\nNot-tested: live SMTP provider behavior under high latency --- .env.example | 4 +- README.md | 2 + backend/app/config.py | 1 + .../app/services/password_reset_service.py | 5 +- backend/app/services/system_email_service.py | 13 ++- .../test_password_reset_and_notifications.py | 92 ++++++++++++++++++- 6 files changed, 112 insertions(+), 5 deletions(-) diff --git a/.env.example b/.env.example index 029cca14..4bd17f0a 100644 --- a/.env.example +++ b/.env.example @@ -25,7 +25,8 @@ FEISHU_REDIRECT_URI=http://localhost:3000/auth/feishu/callback # Without a key, the tools still work but with lower rate limits JINA_API_KEY= -# Public app URL used in user-facing links, such as password reset emails +# Public app URL used in user-facing links, such as password reset emails. +# Production must use your real public HTTPS domain (not localhost). PUBLIC_BASE_URL=http://localhost:3008 # System email delivery (used for forgot-password and optional broadcast emails) @@ -36,6 +37,7 @@ SYSTEM_SMTP_PORT=465 SYSTEM_SMTP_USERNAME= SYSTEM_SMTP_PASSWORD= SYSTEM_SMTP_SSL=true +SYSTEM_SMTP_TIMEOUT_SECONDS=15 # Password reset token lifetime in minutes PASSWORD_RESET_TOKEN_EXPIRE_MINUTES=30 diff --git a/README.md b/README.md index 1806152e..b2436faf 100644 --- a/README.md +++ b/README.md @@ -169,10 +169,12 @@ SYSTEM_SMTP_PORT=465 SYSTEM_SMTP_USERNAME=bot@example.com SYSTEM_SMTP_PASSWORD=your-app-password SYSTEM_SMTP_SSL=true +SYSTEM_SMTP_TIMEOUT_SECONDS=15 PASSWORD_RESET_TOKEN_EXPIRE_MINUTES=30 ``` `PUBLIC_BASE_URL` must point to the user-facing frontend because reset links are generated as `/reset-password?token=...`. +In production, set it to your public HTTPS domain (for example `https://app.example.com`), not a localhost address. Quick local validation: diff --git a/backend/app/config.py b/backend/app/config.py index 247cc8f9..9fd42d70 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -78,6 +78,7 @@ class Settings(BaseSettings): SYSTEM_SMTP_USERNAME: str = "" SYSTEM_SMTP_PASSWORD: str = "" SYSTEM_SMTP_SSL: bool = True + SYSTEM_SMTP_TIMEOUT_SECONDS: int = 15 # Docker (for Agent containers) DOCKER_NETWORK: str = "clawith_network" diff --git a/backend/app/services/password_reset_service.py b/backend/app/services/password_reset_service.py index b2c9085b..c7fa063d 100644 --- a/backend/app/services/password_reset_service.py +++ b/backend/app/services/password_reset_service.py @@ -54,7 +54,10 @@ async def get_public_base_url(db: AsyncSession) -> str: if env_value: return env_value - raise RuntimeError("Public base URL is not configured.") + raise RuntimeError( + "Public base URL is not configured. Set platform public_base_url or PUBLIC_BASE_URL " + "(required in production for reset links)." + ) async def build_password_reset_url(db: AsyncSession, raw_token: str) -> str: diff --git a/backend/app/services/system_email_service.py b/backend/app/services/system_email_service.py index 6ec47e99..a0f3573d 100644 --- a/backend/app/services/system_email_service.py +++ b/backend/app/services/system_email_service.py @@ -35,6 +35,7 @@ class SystemEmailConfig: smtp_username: str smtp_password: str smtp_ssl: bool + smtp_timeout_seconds: int @dataclass(slots=True) @@ -59,6 +60,8 @@ def get_system_email_config() -> SystemEmailConfig: "System email is not configured. Set SYSTEM_EMAIL_FROM_ADDRESS, SYSTEM_SMTP_HOST, and SYSTEM_SMTP_PASSWORD." ) + smtp_timeout_seconds = max(1, int(settings.SYSTEM_SMTP_TIMEOUT_SECONDS)) + return SystemEmailConfig( from_address=from_address, from_name=settings.SYSTEM_EMAIL_FROM_NAME.strip() or "Clawith", @@ -67,6 +70,7 @@ def get_system_email_config() -> SystemEmailConfig: smtp_username=smtp_username, smtp_password=smtp_password, smtp_ssl=settings.SYSTEM_SMTP_SSL, + smtp_timeout_seconds=smtp_timeout_seconds, ) @@ -85,11 +89,16 @@ def _send_system_email_sync(to: str, subject: str, body: str) -> None: with _force_ipv4(): if config.smtp_ssl: context = ssl.create_default_context() - with smtplib.SMTP_SSL(config.smtp_host, config.smtp_port, context=context, timeout=15) as server: + with smtplib.SMTP_SSL( + config.smtp_host, + config.smtp_port, + context=context, + timeout=config.smtp_timeout_seconds, + ) as server: server.login(config.smtp_username, config.smtp_password) server.sendmail(config.from_address, [to], msg.as_string()) else: - with smtplib.SMTP(config.smtp_host, config.smtp_port, timeout=15) as server: + with smtplib.SMTP(config.smtp_host, config.smtp_port, timeout=config.smtp_timeout_seconds) as server: server.ehlo() server.starttls(context=ssl.create_default_context()) server.ehlo() diff --git a/backend/tests/test_password_reset_and_notifications.py b/backend/tests/test_password_reset_and_notifications.py index ddc457ad..dc1ceb7c 100644 --- a/backend/tests/test_password_reset_and_notifications.py +++ b/backend/tests/test_password_reset_and_notifications.py @@ -1,3 +1,4 @@ +import contextlib import uuid from datetime import datetime, timedelta, timezone from types import SimpleNamespace @@ -12,7 +13,7 @@ from app.models.password_reset_token import PasswordResetToken from app.models.user import User from app.schemas.schemas import ForgotPasswordRequest, ResetPasswordRequest -from app.services import password_reset_service +from app.services import password_reset_service, system_email_service from app.services.system_email_service import SystemEmailConfigError @@ -190,6 +191,95 @@ async def fake_build_password_reset_url(*_args, **_kwargs): assert len(background_tasks.tasks) == 1 +def test_system_email_config_uses_configured_timeout(monkeypatch): + monkeypatch.setattr( + system_email_service, + "get_settings", + lambda: SimpleNamespace( + SYSTEM_EMAIL_FROM_ADDRESS="bot@example.com", + SYSTEM_EMAIL_FROM_NAME="Clawith", + SYSTEM_SMTP_HOST="smtp.example.com", + SYSTEM_SMTP_PORT=465, + SYSTEM_SMTP_USERNAME="", + SYSTEM_SMTP_PASSWORD="secret", + SYSTEM_SMTP_SSL=True, + SYSTEM_SMTP_TIMEOUT_SECONDS=42, + ), + ) + + config = system_email_service.get_system_email_config() + + assert config.smtp_timeout_seconds == 42 + + +def test_system_email_config_clamps_non_positive_timeout(monkeypatch): + monkeypatch.setattr( + system_email_service, + "get_settings", + lambda: SimpleNamespace( + SYSTEM_EMAIL_FROM_ADDRESS="bot@example.com", + SYSTEM_EMAIL_FROM_NAME="Clawith", + SYSTEM_SMTP_HOST="smtp.example.com", + SYSTEM_SMTP_PORT=465, + SYSTEM_SMTP_USERNAME="", + SYSTEM_SMTP_PASSWORD="secret", + SYSTEM_SMTP_SSL=True, + SYSTEM_SMTP_TIMEOUT_SECONDS=0, + ), + ) + + config = system_email_service.get_system_email_config() + + assert config.smtp_timeout_seconds == 1 + + +def test_send_system_email_uses_configured_timeout(monkeypatch): + captured = {} + + class DummySMTPSSL: + def __init__(self, host: str, port: int, context=None, timeout: int | None = None): + captured["host"] = host + captured["port"] = port + captured["timeout"] = timeout + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc, tb): + return False + + def login(self, username: str, password: str): + captured["username"] = username + captured["password"] = password + + def sendmail(self, from_address: str, to_addresses: list[str], message: str): + captured["from"] = from_address + captured["to"] = to_addresses + captured["has_message"] = bool(message) + + monkeypatch.setattr( + system_email_service, + "get_system_email_config", + lambda: system_email_service.SystemEmailConfig( + from_address="bot@example.com", + from_name="Clawith", + smtp_host="smtp.example.com", + smtp_port=465, + smtp_username="bot@example.com", + smtp_password="secret", + smtp_ssl=True, + smtp_timeout_seconds=27, + ), + ) + monkeypatch.setattr(system_email_service.smtplib, "SMTP_SSL", DummySMTPSSL) + monkeypatch.setattr(system_email_service, "_force_ipv4", lambda: contextlib.nullcontext()) + + system_email_service._send_system_email_sync("alice@example.com", "subject", "body") + + assert captured["timeout"] == 27 + assert captured["to"] == ["alice@example.com"] + + @pytest.mark.asyncio async def test_reset_password_updates_user_and_invalidates_other_tokens(monkeypatch): user = make_user(password_hash=auth_api.hash_password("old-password")) From c15b8f06d087945f6cd2e69bf4891b144c5da971 Mon Sep 17 00:00:00 2001 From: Atlas Date: Fri, 27 Mar 2026 09:08:22 +0800 Subject: [PATCH 5/5] Keep password reset migration on the mainline Alembic chain The password reset migration was anchored to an older branch point, which created a second Alembic head alongside add_daily_token_usage. Reattach it to the current mainline so upgrades remain linear for PR #178.\n\nConstraint: Preserve the password reset schema change without introducing a merge migration in this PR\nRejected: Add a separate Alembic merge revision | extra migration noise for a single isolated table addition\nConfidence: high\nScope-risk: narrow\nReversibility: clean\nDirective: New migrations in feature PRs must be rebased onto the current Alembic head before review\nTested: cd backend && .venv/bin/alembic heads\nTested: cd backend && .venv/bin/python -m pytest tests/test_password_reset_and_notifications.py\nNot-tested: full backend migration upgrade/downgrade cycle on a populated database --- backend/alembic/versions/add_password_reset_tokens.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/alembic/versions/add_password_reset_tokens.py b/backend/alembic/versions/add_password_reset_tokens.py index cd0fe56e..d1d44ec3 100644 --- a/backend/alembic/versions/add_password_reset_tokens.py +++ b/backend/alembic/versions/add_password_reset_tokens.py @@ -1,13 +1,13 @@ """Add password_reset_tokens table. Revision ID: add_password_reset_tokens -Revises: multi_tenant_registration +Revises: add_daily_token_usage """ from alembic import op revision = "add_password_reset_tokens" -down_revision = "multi_tenant_registration" +down_revision = "add_daily_token_usage" branch_labels = None depends_on = None