fix(email): stop duplicate legacy invitation email and fix its onboarding link - #36455
Conversation
Problem: When PROXY_BASE_URL has a trailing slash (e.g., http://example.com/), the invitation link becomes http://example.com//ui/onboarding?invitation_id=... with a double slash, which can cause routing issues and direct users to the swagger page instead of the onboarding page. Additionally, if invitation creation fails for any reason (no user_id, prisma unavailable, etc.), the email falls back to base_url which points to swagger at the root path instead of the onboarding page. Changes: - Strip trailing slashes from base_url before constructing invitation links - Add comprehensive debug/warning logging to trace invitation link construction - Log when fallbacks occur so admins can diagnose why users land on swagger - Add info-level logging for successful invitation link creation This ensures: 1. No double-slash URLs regardless of PROXY_BASE_URL format 2. Clear visibility when invitation creation fails and why 3. Easier debugging of email delivery issues
Greptile SummaryThe PR consolidates invitation-email dispatch so the legacy path is used only as a fallback, constructs onboarding invitation URLs consistently, normalizes dashboard URLs, and adds credential visibility controls to email settings.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| enterprise/litellm_enterprise/enterprise_callbacks/send_emails/base_email.py | Constructs the modern invitation URL and passes it to the invitation email template without logging the invitation credential. |
| litellm/proxy/hooks/user_management_event_hooks.py | Tracks enterprise email delivery outcomes and invokes the legacy sender only when no enterprise logger succeeds. |
| litellm/integrations/SlackAlerting/slack_alerting.py | Adds invitation lookup or creation so legacy emails link to the onboarding flow. |
| litellm/proxy/proxy_server.py | Normalizes login and onboarding dashboard URL construction to avoid trailing-slash inconsistencies. |
| ui/litellm-dashboard/src/components/email_settings.tsx | Adds per-field visibility controls for credential-like email settings. |
Reviews (4): Last reviewed commit: "chore: satisfy LIT002 in invitation-emai..." | Re-trigger Greptile
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
Add eye/eye-off toggle buttons to all email alert configuration fields so admins can reveal masked credentials to verify values without editing Changes: - Add useState hook to track visibility state per field - Add Eye/EyeOff icons from lucide-react - Wrap input in relative container with toggle button - Toggle input type between password and text - Add test coverage for visibility toggle behavior
a85a762 to
062797b
Compare
Replace absolute positioning with shadcn InputGroup components for proper integration of the eye icon inside the input box Changes: - Use InputGroup, InputGroupInput, InputGroupAddon, InputGroupButton - Eye icon now properly integrated inside input border - Remove custom absolute positioning hack - Update test to handle input-group-control data-slot selector
bd1a8f3 to
0c93f6e
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Logging the full invitation URL with invitation_id exposes a bearer credential. If logs are accessible to unauthorized principals, they could use the logged invitation_id to claim the invitation, set a password, and hijack the account Removed: - Debug log showing full invitation_link with invitation_id - Debug log showing base_url and invitation_id parameters - Info log showing full invitation_link Kept only: - Info log confirming invitation creation succeeded (no credential) - Warning logs for error paths (no credential exposure)
URLs should not have a trailing slash before the query string. Changed from patterns like /ui/?login=success to /ui?login=success Fixed in: - Email invitation links (/ui/onboarding?invitation_id=...) - SSO login redirects (/ui?login=success) - Onboarding token links (/ui/onboarding?token=...)
0c93f6e to
9d9435b
Compare
…tion button
The email template button now explicitly uses {invitation_link} instead
of relying on {base_url} being overwritten. This ensures the Accept
Invitation button takes users to the onboarding page where they can
create their password and login
206bc5f to
b66edf2
Compare
|
@greptile-apps review this |
When a new user is created with send_invite_email=true, both the modern enterprise (v2) email logger and the legacy v1 SlackAlerting path fired unconditionally, so a v2 deployment sent two invitation emails: the modern "Accept Invitation" one and the legacy "Get Started here" one built off the bare base URL. Route the v2 send through a single helper that returns whether any registered enterprise email logger actually delivered, and only fall back to the legacy email when v2 did not send (enterprise package absent, no email logger configured, or every send raised). This keeps the legacy path as a real fallback for OSS deployments without regressing them. Also fix the legacy fallback itself so it is not broken when it does run: build a proper /ui/onboarding?invitation_id=... link (looked up or created for the user, trailing slash stripped before the query) instead of the bare base URL, and relabel the button from "Get Started here" to "Accept Invitation". Adds regression tests covering both that v2 suppresses the legacy duplicate on success and that a failing v2 send still falls back to legacy.
The eye-icon refactor masked every email setting field (SMTP_HOST, SMTP_PORT, EMAIL_LOGO_URL) as a password and gave each one a show/hide toggle, so the first "Show credential" button belonged to SMTP_HOST rather than the password. That broke the credential-visibility unit test and hid non-secret values from admins for no reason. Render the masked input and the eye toggle only for sensitive keys (PASSWORD/SECRET/KEY/TOKEN); everything else stays a plain text input. This restores the intended behavior and makes the unit test pass.
The /login, /v2/login and onboarding routes now build the dashboard redirect as /ui?login=success (no trailing slash before the query), matching the URL construction fix on this branch. Update the pinned test expectations that still asserted the old /ui/?login=success so they match the code.
…tellm_fix_invitation_email_link
The broad except in _construct_user_invitation_link is intentional: any DB or invitation-creation failure falls back to the base URL. Annotate it so the strict-rule budget gate does not count it as a new blind-except.
Build the enterprise email logger set as a tuple and unpack a generator into asyncio.gather instead of list literals, and mark the two prisma find_many dict kwargs (which the client requires as dicts) with reasoned mutable-ok.
b0fac57
into
litellm_internal_staging
TLDR
Problem this solves:
How it solves it:
email flow:
(small ui fix): show icon for email fields

Relevant issues
Linear ticket
Pre-Submission checklist
Screenshots / Proof of Fix
Will test locally with a running proxy instance to verify correct invitation link construction and logging
Type
🐛 Bug Fix
Changes
enterprise/litellm_enterprise/enterprise_callbacks/send_emails/base_email.py (v2 path):
litellm/proxy/hooks/user_management_event_hooks.py (dispatch):
litellm/integrations/SlackAlerting/slack_alerting.py (legacy fallback):
litellm/integrations/email_templates/templates.py:
tests/test_litellm/proxy/hooks/test_send_invite_email.py:
Final Attestation