From c706489480948eb5cd313cb279f19c718e132e0f Mon Sep 17 00:00:00 2001 From: NguyenND Date: Fri, 17 Jul 2026 12:54:07 -0400 Subject: [PATCH] Jul 17 - Fill the gaps between Single-tenant mode and Multi-tenant mode - MT7 --- app/api/__init__.py | 2 +- app/api/scheduled.py | 4 +- app/billing/emails.py | 11 ++- app/models/issue.py | 2 +- app/routes/customers.py | 11 ++- app/routes/facility_qr.py | 2 +- app/utils/mail_utils.py | 154 ++++++++++++++++++++++++++++++++++++++ app/utils/storage.py | 8 +- 8 files changed, 178 insertions(+), 16 deletions(-) create mode 100644 app/utils/mail_utils.py diff --git a/app/api/__init__.py b/app/api/__init__.py index 9b248ff..79cf58a 100644 --- a/app/api/__init__.py +++ b/app/api/__init__.py @@ -50,7 +50,7 @@ def register_api(app): from app.api.comments import bp as comments_bp api_bp.register_blueprint(comments_bp) - # MT-5: Planned inspection assignments (plan-mode schedules) + # phase43: Planned inspection assignments (plan-mode schedules) from app.api.scheduled import bp as scheduled_bp api_bp.register_blueprint(scheduled_bp) diff --git a/app/api/scheduled.py b/app/api/scheduled.py index 07b8e0b..2ac4be6 100644 --- a/app/api/scheduled.py +++ b/app/api/scheduled.py @@ -1,7 +1,7 @@ """ app/api/scheduled.py -------------------- -Mobile API endpoint for planned inspection assignments (MT-5, phase43). +Mobile API endpoint for planned inspection assignments (phase43). GET /api/v1/scheduled-inspections Returns ACTIVE, PLAN-MODE schedules the caller is responsible for. @@ -17,7 +17,7 @@ Why plan-mode only `mode='auto'` schedules materialise themselves into a real Inspection at next_run_at, which the iPad already fetches via /api/v1/inspections. Returning them here too would show the same work twice, and "Start" is meaningless for a -schedule that starts itself. This mirrors the web dashboard panel (MT-4). +schedule that starts itself. This mirrors the web dashboard panel (phase43). A plan-mode schedule is a PLAN, not an inspection — see app/models/inspection_schedule.py for the full lifecycle. diff --git a/app/billing/emails.py b/app/billing/emails.py index b0bfeb8..b164e26 100644 --- a/app/billing/emails.py +++ b/app/billing/emails.py @@ -8,7 +8,6 @@ app/templates/billing/email/. import logging import threading -from urllib.parse import urlparse from flask import current_app, render_template from flask_mail import Message from app import mail @@ -92,10 +91,14 @@ def send_billing_email(to_addr: str, event_type: str, context_dict: dict): app = current_app._get_current_object() - # Derive noreply sender from APP_BASE_URL so billing emails match the tenant domain. + # Branded From: the display NAME tracks the tenant, the ADDRESS stays the + # authenticated identity unless the domain is DNS-authorized. Resolved HERE, + # in the request context, because _send() runs on a background thread where + # g.tenant no longer exists. The previous `noreply@{netloc}` sent from the + # tenant's host regardless of SPF authorization. See app/utils/mail_utils.py. try: - _netloc = urlparse(app.config.get('APP_BASE_URL', '')).netloc - _sender = f'noreply@{_netloc}' if _netloc else app.config.get('MAIL_DEFAULT_SENDER', '') + from app.utils.mail_utils import branded_sender + _sender = branded_sender(app.config.get('APP_BASE_URL', '')) except Exception: _sender = app.config.get('MAIL_DEFAULT_SENDER', '') diff --git a/app/models/issue.py b/app/models/issue.py index 10d9fce..186e26b 100644 --- a/app/models/issue.py +++ b/app/models/issue.py @@ -118,7 +118,7 @@ class Issue(db.Model): # Display labels for handler_type. The web templates hardcode these inline; # this mapping exists so the mobile API can return a human-readable label - # without the client duplicating the strings. (phase43 / MT-5) + # without the client duplicating the strings. (phase43) HANDLER_LABELS = { 'internal': 'Janitorial Staff', 'facility': 'Facility Staff', diff --git a/app/routes/customers.py b/app/routes/customers.py index a27a803..26bd127 100644 --- a/app/routes/customers.py +++ b/app/routes/customers.py @@ -179,7 +179,6 @@ def _send_invite_email(user, token, base_url=None): from flask import current_app, render_template_string from flask_mail import Message from app import mail - from urllib.parse import urlparse import threading if not current_app.config.get('MAIL_SERVER'): @@ -189,8 +188,14 @@ def _send_invite_email(user, token, base_url=None): effective_base = (base_url or current_app.config.get('APP_BASE_URL', '')).rstrip('/') setup_link = f'{effective_base}{url_for("customers.set_password", token=token)}' - host = urlparse(effective_base).netloc or 'janitorialqc.local' - sender = f'noreply@{host}' + # Branded From as a (display_name, address) tuple. The display NAME tracks + # the tenant (e.g. "Gov Services QC"); the ADDRESS is branded only for + # DNS-authorized domains and otherwise stays the authenticated identity so + # the mail always delivers. The previous `noreply@{host}` sent from whatever + # host the browser was on, which fails SPF/DMARC for any domain this mail + # server isn't authorized for. See app/utils/mail_utils.py and rule 64. + from app.utils.mail_utils import branded_sender + sender = branded_sender(effective_base) html_body = render_template_string(""" diff --git a/app/routes/facility_qr.py b/app/routes/facility_qr.py index 5df03cd..c3d33e0 100644 --- a/app/routes/facility_qr.py +++ b/app/routes/facility_qr.py @@ -66,7 +66,7 @@ def _save_report_photos(file_list): under "Photo Evidence" on the web (rule 44 — never `result_photos`). Writes go through `_save_photo`, which validates magic bytes and routes to - the active storage backend (MT-2). + the active storage backend (see app/utils/storage.py). """ from app.routes.inspections import _save_photo saved = [] diff --git a/app/utils/mail_utils.py b/app/utils/mail_utils.py new file mode 100644 index 0000000..b1c7d61 --- /dev/null +++ b/app/utils/mail_utils.py @@ -0,0 +1,154 @@ +""" +app/utils/mail_utils.py +----------------------- +Helpers for constructing outbound email sender identities. + +`branded_sender()` returns a Flask-Mail sender as a ``(display_name, address)`` +tuple so outbound branded email (customer invitations, billing notices) shows a +per-tenant identity — WITHOUT risking deliverability. + +Two independent things vary: + + 1. DISPLAY NAME (what the recipient sees, e.g. "Gov Services QC") + Always applied. Costs nothing, needs no DNS. This is the safe default. + + 2. FROM ADDRESS (the actual @domain, e.g. jqc.noreply@govservicesinc.com) + Only used for domains that are KNOWN to authorize this mail server + (the authenticated sender's own domain, plus anything you add to + SENDER_AUTHORIZED_DOMAINS). Every other domain keeps the authenticated + address so the message still passes SPF/DMARC and delivers. + +Result, with no DNS work: + Invited from gov.jqc.app -> + From: "Gov Services QC" + (branded NAME, deliverable authenticated ADDRESS) + +Once a tenant's own domain authorizes this mail server (SPF include + DKIM), +add it to SENDER_AUTHORIZED_DOMAINS and it upgrades to: + From: "Gov Services QC" + +Ported from the single-tenant tree with one multi-tenant change: the display +name comes from the TENANT, not a hardcoded host→name dict. Resolution order: + + 1. TenantSettings.company_name — what the tenant typed into Branding + 2. Tenant.name (control plane) — always known for a resolved tenant + 3. BRAND_NAMES[domain] — ST's host map, kept for non-tenant hosts + 4. DEFAULT_BRAND_NAME + +See CLAUDE.md rule 64. +""" + +from urllib.parse import urlparse +from flask import current_app, g + +# ── Per-domain display names (single-tenant / non-tenant hosts only) ───────── +# In multi-tenant mode the tenant record supplies the name and this map is never +# consulted. It remains for the single-tenant deployment and for hosts that +# resolve to no tenant. +BRAND_NAMES = { + 'ltservicesinc.com': 'LT Services QC', + 'govservicesinc.com': 'Gov Services QC', + 'efs.com': 'EFS QC', +} +DEFAULT_BRAND_NAME = 'Janitorial QC' + +# ── Domains cleared to use a BRANDED FROM ADDRESS ──────────────────────────── +# Add a registrable domain here ONLY after its DNS authorizes this mail server +# (SPF `include:` + DKIM). Until then the domain still gets its branded display +# NAME but sends from the authenticated address, so it always delivers. +# The authenticated sender's own domain is always treated as authorized and does +# NOT need to be listed here. +# NOTE: written as set([...]) deliberately. A brace literal containing only +# comments is an empty *dict*, which silently becomes a *set* the moment a line +# is uncommented. Membership works either way, so this is cosmetic — but the +# declared type shouldn't change based on whether a comment is uncommented. +SENDER_AUTHORIZED_DOMAINS = set([ + # 'govservicesinc.com', # ← uncomment once SPF+DKIM are live for it + # 'efs.com', +]) + + +def _authenticated_sender() -> str: + """The SMTP identity we authenticate as — always deliverable.""" + return (current_app.config.get('MAIL_DEFAULT_SENDER') + or current_app.config.get('MAIL_USERNAME') + or 'jqc.noreply@janitorialqc.local') + + +def _host_domain(base_url): + """Registrable domain (last two labels) of base_url, or None if unparseable.""" + base = (base_url or current_app.config.get('APP_BASE_URL', '') or '').strip().rstrip('/') + if not base: + return None + netloc = urlparse(base).netloc or urlparse('//' + base).netloc + host = netloc.split('@')[-1].split(':')[0].strip().lower() # drop userinfo/port + if not host or '.' not in host: + return None + labels = [l for l in host.split('.') if l] + return '.'.join(labels[-2:]) if len(labels) >= 2 else host + + +def _tenant_brand_name(): + """Display name from the current tenant, or None. + + Prefers what the tenant typed into Branding (TenantSettings.company_name), + then the control-plane tenant name. Returns None in single-tenant mode or + when no tenant is bound. + + Deliberately swallows errors: TenantSettings.query raises ProgrammingError + when a freshly provisioned tenant DB has not been migrated yet, and an + un-migrated tenant must not break sending mail. + """ + if not current_app.config.get('MULTI_TENANT_ENABLED'): + return None + + tenant = getattr(g, 'tenant', None) if g else None + if tenant is None: + return None + + try: + from app.models.tenant_settings import TenantSettings + settings = TenantSettings.get_or_default() + if settings is not None: + name = (settings.company_name or '').strip() + if name: + return name + except Exception: + pass + + return (getattr(tenant, 'name', '') or '').strip() or None + + +def branded_sender(base_url=None): + """Return a ``(display_name, address)`` sender tuple for branded email. + + The display name tracks the tenant (falling back to the host's domain); the + address is branded only for the authenticated domain and any + SENDER_AUTHORIZED_DOMAINS, and otherwise stays the authenticated address so + the mail still delivers. + + Falls back to the bare authenticated sender string whenever the host cannot + be parsed (localhost, empty, no dot), so this can never yield an invalid From. + """ + auth_sender = _authenticated_sender() + if '@' not in auth_sender: + return auth_sender + local, auth_domain = auth_sender.rsplit('@', 1) + auth_domain = auth_domain.lower() + + tenant_name = _tenant_brand_name() + domain = _host_domain(base_url) + + if not domain: + # Unknown host → safe authenticated address; still brand the name if a + # tenant is bound. + return (tenant_name or DEFAULT_BRAND_NAME, auth_sender) + + display_name = tenant_name or BRAND_NAMES.get(domain, DEFAULT_BRAND_NAME) + + if domain == auth_domain or domain in SENDER_AUTHORIZED_DOMAINS: + address = f'{local}@{domain}' # branded address — DNS-authorized + else: + address = auth_sender # keep deliverable authenticated address + + return (display_name, address) diff --git a/app/utils/storage.py b/app/utils/storage.py index 4bec499..50f2640 100644 --- a/app/utils/storage.py +++ b/app/utils/storage.py @@ -9,11 +9,11 @@ addition: per-tenant object-key prefixing. See "Tenant isolation" below. One interface, backend selected by config ``STORAGE_BACKEND``: - ``local`` (default): files under ``app/static/uploads``, served via ``url_for('static', ...)``. **Byte-for-byte identical** to the behavior - before this abstraction existed — MT-2 is a no-op. + before this abstraction existed — introducing this seam is a no-op. - ``s3``: Cloudflare R2 / any S3-compatible store. Private bucket; browser/API URLs are short-lived presigned GETs. Requires ``boto3`` and the ``R2_*`` config keys. boto3 is imported lazily, so a ``local`` deploy needs neither. - Inert until MT-8 flips STORAGE_BACKEND per tenant. + Inert until STORAGE_BACKEND is flipped to s3 per tenant at R2 cutover. The stored **key** is always the relative path ``uploads//`` — the exact string persisted in the DB (`Issue.photo_path`, `result_photos[]`, @@ -34,9 +34,9 @@ payload. Callers stay tenant-agnostic; the DB stays portable. The **local** backend deliberately does NOT prefix: its layout is the existing on-disk tree, and prefixing would relocate every existing file (that is not a -no-op, and MT-2 must be one). Local mode therefore keeps today's shared +no-op, and this seam must be one). Local mode therefore keeps today's shared ``app/static/uploads`` directory across tenants — an isolation weakness that -predates this module and is retired when a tenant moves to ``s3`` in MT-8. +predates this module and is retired when a tenant moves to ``s3`` at cutover. Public module-level API (delegates to the active backend):