Skip to content

Add system email notifications and password recovery#178

Open
Atlas-SZ wants to merge 4 commits intodataelement:mainfrom
Atlas-SZ:main
Open

Add system email notifications and password recovery#178
Atlas-SZ wants to merge 4 commits intodataelement:mainfrom
Atlas-SZ:main

Conversation

@Atlas-SZ
Copy link
Contributor

Summary

This PR adds platform-owned email delivery for password recovery and optional company broadcast emails.

What Changed

  • Added env-driven system SMTP configuration
  • Added forgot-password and reset-password backend endpoints
  • Added single-use password reset tokens with persistence
  • Added frontend forgot-password and reset-password pages
  • Added optional email delivery for company broadcast notifications
  • Localized the new password recovery and broadcast email UI for English and Simplified Chinese
  • Hardened email delivery so SMTP latency or failures do not block the main request path

Validation

  • cd backend && .venv/bin/python -m pytest tests/test_password_reset_and_notifications.py
  • cd backend && .venv/bin/python -m ruff check app/api/auth.py app/api/notification.py app/services/system_email_service.py tests/test_password_reset_and_notifications.py
  • cd frontend && npm run build
  • Manual local validation with Docker PostgreSQL/Redis
  • Manual SMTP validation with a real 163 mailbox
  • Manual browser E2E for reset-password -> login
  • Manual browser validation for English and Simplified Chinese UI copy

Notes

  • SMTP configuration is environment-variable based only
  • emails_sent in broadcast responses now means queued recipients, not guaranteed final SMTP success
  • Background email delivery is in-process, not a durable queue

Risk

  • Background email tasks can still be lost if the server process exits immediately after the response
  • Other languages were intentionally left unchanged; this PR only updates English and Simplified Chinese

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
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
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
The org sync flow now supports a provider-aware configuration model,
adds WeCom directory ingestion, and updates the enterprise settings UI
so admins can configure sync, browse a collapsible org tree, and inspect
member details without manual directory maintenance. The read path for
org sync settings was tightened so only admins can fetch the config and
provider secrets are redacted from API responses while still being
preserved on save when the UI submits blank secret fields.

Constraint: WeCom validation had to remain read-only against the live tenant
Constraint: Existing Feishu org sync settings needed to keep working without a data migration
Rejected: Add a separate WeCom-only settings key | would duplicate provider config paths and UI state
Rejected: Store org sync secrets only in environment variables | conflicts with tenant-managed enterprise settings workflow
Confidence: high
Scope-risk: moderate
Reversibility: clean
Directive: Do not expose org_sync secrets in API responses or relax admin-only access without revisiting the trust boundary
Tested: backend/.venv/bin/python -m ruff check backend/app/api/enterprise.py backend/app/models/org.py backend/app/services/org_sync_service.py backend/tests/test_password_reset_and_notifications.py
Tested: DATABASE_URL=postgresql+asyncpg://postgres:QF20200610@localhost:25432/clawith REDIS_URL=redis://:difyai123456@localhost:16379/0 backend/.venv/bin/python -m pytest backend/tests/test_password_reset_and_notifications.py
Tested: cd frontend && npm run build
Not-tested: Durable background job processing for very large org syncs
@yaojin3616
Copy link
Collaborator

PR 178 评审意见:

  1. 安全性:密码重置 Token 采用了哈希存储、单次使用及过期机制,符合安全规范。
  2. 性能:邮件发送采用异步 + BackgroundTasks,避免了网络延迟对响应速度的影响。
  3. 扩展性:组织架构同步服务重构为多 Provider 模式(飞书 + 企微),架构清晰。
  4. 完整性:包含了数据库迁移、前后端实现、国际化及单元测试,代码质量很高。

建议 (LGTM)

  • 提醒管理员在生产环境配置 PUBLIC_BASE_URL,以确保重置链接有效。
  • 未来可考虑将 SMTP 超时时间配置化。

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants