feat(setup): configure the public address and SMTP in the wizard, not .env (#1104)
* feat(setup): configure the public address and SMTP in the wizard, not .env
A fresh install could not configure its own public address. `general_site_url`
and the `email_configs` row already existed as admin settings, but nothing
could reach them:
- docker-compose.yml injected FRONTEND_URL=${FRONTEND_URL:-http://localhost:3000}
and Dockerfile.aio baked in ENV FRONTEND_URL=http://localhost:3000, so
getFrontendBaseUrl() returned on its first branch every time and the setting
was never read. .env.example shipped the same value as an uncommented
placeholder for FRONTEND_URL / ADMIN_URL / API_URL.
- the wizard never asked for the address at all, and skipped its whole config
step unless a CRM-ish feature was selected — so a gallery-only install was
also never offered SMTP, despite gallery links, guest invites and expiry
warnings all going out through email_configs.
- eleven call sites read process.env.FRONTEND_URL directly rather than the
resolver, three of them defaulting to placeholder hosts that reached real
recipients: https://app.example.com in payment-reminder emails, localhost:3005
in admin invitation emails, https://app.example.com in dev template previews.
Stop injecting a default anywhere, and resolve the origin instead:
FRONTEND_URL -> general_site_url -> the origin the request arrived on ->
whichever exists -> ''. A loopback candidate is treated as unconfigured so the
installs that already have http://localhost:3000 baked into their environment
self-heal; the same guard previously lived inline in routes/gallery.js for the
slideshow QR (#848) and is now shared. The empty return is preserved because
shareLinkService and the SSO redirects in routes/auth rely on it to emit
relative urls — callers needing an absolute url use getAbsoluteFrontendUrl(),
which still ends at http://localhost:3000.
The wizard now persists window.location.origin right after the admin account is
created, so an install that skips the rest still has a usable origin for
background jobs that have no request to derive one from, and offers it as an
editable "Public address" field. Settings -> General shows the field read-only
when FRONTEND_URL pins it, instead of silently ignoring edits.
Also drop the `|| 'mailhog'` fallback when seeding email_configs: that host only
exists in the dev compose profile (which does not even start by default), so a
fresh install came up with a live config pointing nowhere while the wizard
showed empty SMTP fields. With no row, blank fields are the truth and
emailProcessor logs "No email configuration found". Developers set
SMTP_HOST=mailhog explicitly.
backend/src/services/emailService.js is deleted: nothing in backend/ references
it, and it was the only consumer of the SMTP_* variables, which misrepresented
how mail is configured.
Refs #705
* fix(setup): keep FRONTEND_URL ahead of ADMIN_URL/APP_URL when resolving links
The previous commit routed two call sites through the resolver but put the
site-specific variable FIRST, silently reversing precedence:
userManagementService was: FRONTEND_URL || ADMIN_URL || localhost:3005
became: ADMIN_URL || resolver
adminEvents/crud was: FRONTEND_URL || APP_URL || ''
became: APP_URL || resolver
An install with both variables set would have flipped which one won. Call the
resolver first instead — it starts with FRONTEND_URL, so the original relative
order is preserved and only the final fallback changes: localhost:3005 (not
even the frontend's port) and '' (a relative link inside an email) both become
the resolved origin.
Refs #705
* fix(setup): unpin loopback FRONTEND_URL, keep ADMIN_URL/APP_URL reachable
Review feedback on #1104.
isEnvPinned() reported ANY FRONTEND_URL as authoritative, including the
loopback values getFrontendBaseUrl() deliberately demotes. An install
upgrading with the old compose default FRONTEND_URL=http://localhost:3000
therefore resolved its origin from general_site_url correctly, but got the
Site URL field rendered read-only in Settings and skipped by the wizard's
seeding - locking the exact operators this change exists to unblock out of
configuring a public address anywhere. The predicate now mirrors the
resolver, and the derived general_site_url_effective the General tab reads
comes from the same helper instead of re-normalising process.env inline.
APP_URL and ADMIN_URL had become dead code: getFrontendBaseUrl() only
returns falsy when NOTHING is configured, so `|| process.env.ADMIN_URL`
after it never ran once a site URL existed - which after this PR is the
normal case. A split-origin install pointing ADMIN_URL at a separate admin
host got invite links on the public gallery origin instead. They are now
passed as an explicit `override` that resolves directly below FRONTEND_URL,
preserving the historic FRONTEND_URL-before-ADMIN_URL order while beating
the database- and request-derived fallbacks.
general_site_url now feeds the CORS allowlist and the
Access-Control-Allow-Origin header, not just email links, so a schemeless
value is an allowlist entry no browser origin can match. Validate it
server-side in PUT /general (isURL with require_protocol, require_tld off
so LAN/NAS installs on http://nas:3000 still work) and client-side in both
surfaces that write it - type="url" never fires in either, since neither
input sits inside a form.
Two more wizard fixes: the General tab no longer reposts general_site_url
while it is env-pinned, because the field then holds the effective env
value rather than the stored one and the round-trip read as a change to a
protected key, 403ing a settings.edit-without-settings.domains admin on an
unrelated save. And SetupConfigStep validates the From address before
posting - /admin/email/config rejects a blank one, which used to surface as
a generic warning while the wizard advanced from its finally block anyway,
discarding every SMTP value the user had typed, password included. A failed
save now keeps them on the step.
* fix(setup): surface a rejected public address instead of swallowing it
Review round 2 follow-up on #1104, pushed onto the branch.
saveSiteUrl() caught and discarded every error. That was defensible before
round 2 added a server-side URL check, but PUT /general can now answer 400 —
and the two validators disagreed:
http://my_nas.local client: accepted server: rejected
http://foo_bar:3000 client: accepted server: rejected
validate() let those through, the 400 was swallowed, `failed` stayed false and
onDone() ran. The operator finished the wizard believing the public address was
stored when nothing had been. That is the silent misconfiguration this whole
change exists to remove, landing on the LAN and NAS installs it targets.
Three parts:
- saveSiteUrl() throws. finish() resolves it before anything else is posted and
puts the message on the address field rather than the generic "some settings
could not be saved" warning. Skip for now still always leaves, by contract,
but warns instead of dropping the value in silence.
- allow_underscores on the server check, for the same reason require_tld is
off: browsers resolve http://my_nas.local and the client accepts it, so
rejecting it server-side only produced the mismatch above. Both validators
now agree across the LAN/NAS, IDN, bare-IP and scheme-less cases.
- LOOPBACK_BASE_RE anchors its host token. Bare prefix matching also demoted
https://localhost-nas.example.com, and now that this predicate gates the
whole resolver rather than just the slideshow QR, being demoted means a
configured address is silently ignored. 127. stays a bare prefix on purpose:
all of 127.0.0.0/8 is loopback.
Resolver suite 31 passing, up from 26. Mutation-checked: restoring the
unanchored regex fails the three new host-boundary cases.
* fix(settings): don't lock the General tab on a site URL nobody typed
Review follow-up on #1104, pushed onto the branch.
general_site_url was free-text until this PR added a server-side check, so an
upgraded install can hold something schemeless that predates it. The tab
flagged that on load, and `disabled={!!siteUrlError}` then killed Save for
EVERY General setting.
An admin holding settings.edit but not settings.domains could not clear it
either: correcting the address is a change to a protected key and 403s. The
tab has no permission gating, so that role was simply locked out of the tab
with no self-service way back.
That is the same role adminSettings.js:85-95 documents the no-op round-trip
allowance for. The allowance only helps if the request is made, and this
blocked it in the browser first.
Validation now waits until the field is actually edited, and an unchanged
value is dropped from the payload rather than reposted — matching what the
env-pinned case already does one line above, and for the same reason.
stored value invalid, untouched Save works, key not sent
edited to something unusable Save blocked
edited to a usable absolute url saved
Four tests, first coverage for this feature. Mutation-checked: removing the
dirty gate fails the untouched-value case.
---------
Co-authored-by: Paul Nothaft <[email protected]>
This commit is contained in:
Luca
@@ -7,9 +7,13 @@ import { Button, Input } from '../common';
|
||||
import type { FeatureKey } from '../../services/featureFlags.service';
|
||||
import { businessProfileService } from '../../services/businessProfile.service';
|
||||
import { emailService, type EmailConfig } from '../../services/email.service';
|
||||
import { settingsService } from '../../services/settings.service';
|
||||
import { isAbsoluteHttpUrl } from '../../utils/url';
|
||||
|
||||
// Features that need working SMTP to deliver anything.
|
||||
const EMAIL_FEATURES: FeatureKey[] = ['reminderEmails', 'incomingMail', 'whatsapp', 'bills'];
|
||||
// Email is NOT feature-gated (#705): a gallery-only install still mails the
|
||||
// gallery link, guest invites and expiry warnings through the same
|
||||
// email_configs row, so hiding SMTP behind the CRM-ish features left the most
|
||||
// basic install unable to deliver anything.
|
||||
|
||||
interface Props {
|
||||
selectedFeatures: Set<FeatureKey>;
|
||||
@@ -18,12 +22,13 @@ interface Props {
|
||||
|
||||
// Lean per-feature config, shown after the "How will you use PicPeak?" step.
|
||||
// Only the sections a selected feature actually needs are rendered; everything
|
||||
// else keeps its seeded defaults and is tunable later in Settings. Saving is
|
||||
// best-effort per section — a failure never traps the user on setup.
|
||||
// else keeps its seeded defaults and is tunable later in Settings. Every field
|
||||
// is optional — "Skip for now" always leaves — but a section the user DID fill
|
||||
// in is validated before it is posted, and a save that fails keeps them on the
|
||||
// step with their input intact rather than advancing into a silent data loss.
|
||||
export const SetupConfigStep: React.FC<Props> = ({ selectedFeatures, onDone }) => {
|
||||
const { t } = useTranslation();
|
||||
const showInvoicing = selectedFeatures.has('bills');
|
||||
const showEmail = EMAIL_FEATURES.some((f) => selectedFeatures.has(f));
|
||||
const [saving, setSaving] = useState(false);
|
||||
|
||||
const [inv, setInv] = useState({
|
||||
@@ -33,14 +38,97 @@ export const SetupConfigStep: React.FC<Props> = ({ selectedFeatures, onDone }) =
|
||||
const [mail, setMail] = useState({
|
||||
smtp_host: '', smtp_port: '587', smtp_user: '', smtp_pass: '', from_email: '', from_name: '',
|
||||
});
|
||||
// Prefilled with the address the admin actually reached the wizard on, which
|
||||
// on a NAS or LAN install is the one thing no default can guess (#705).
|
||||
const [siteUrl, setSiteUrl] = useState(window.location.origin.replace(/\/+$/, ''));
|
||||
const [errors, setErrors] = useState<Record<string, string>>({});
|
||||
|
||||
const invField = (k: keyof typeof inv) => (e: React.ChangeEvent<HTMLInputElement>) =>
|
||||
setInv((p) => ({ ...p, [k]: e.target.value }));
|
||||
const mailField = (k: keyof typeof mail) => (e: React.ChangeEvent<HTMLInputElement>) =>
|
||||
setMail((p) => ({ ...p, [k]: e.target.value }));
|
||||
|
||||
const finish = async () => {
|
||||
// Persist the public origin on BOTH paths — skipping the optional invoicing
|
||||
// and SMTP sections must not also skip the address that every gallery link,
|
||||
// QR code and reminder email is built from.
|
||||
//
|
||||
// Throws rather than swallowing: PUT /general applies its own URL check, and
|
||||
// a rejection here means the wizard would otherwise finish reporting success
|
||||
// with no public address stored at all — the silent misconfiguration this
|
||||
// whole change exists to remove. Callers decide what to do with it.
|
||||
const saveSiteUrl = async () => {
|
||||
const value = siteUrl.trim().replace(/\/+$/, '');
|
||||
if (!value) return;
|
||||
await settingsService.updateSettings({ general_site_url: value });
|
||||
};
|
||||
|
||||
const siteUrlRejected = () => t(
|
||||
'setup.config.siteUrlRejected',
|
||||
'The server rejected this address. Use the full origin, for example https://gallery.example.com or http://192.168.1.50:3000.',
|
||||
);
|
||||
|
||||
// Blocking validation, run before anything is posted. Everything on this
|
||||
// step is optional, but a value that IS filled in has to be usable: the
|
||||
// public address feeds the CORS allowlist (#705), and /admin/email/config
|
||||
// rejects a config whose from_email isn't a valid address — which used to
|
||||
// surface as a generic warning while the wizard advanced anyway, throwing
|
||||
// away every SMTP value including the password (#1104).
|
||||
const validate = () => {
|
||||
const next: Record<string, string> = {};
|
||||
if (siteUrl.trim() && !isAbsoluteHttpUrl(siteUrl)) {
|
||||
next.siteUrl = t('setup.config.siteUrlInvalid', 'Enter the full address including http:// or https://, for example https://gallery.example.com');
|
||||
}
|
||||
if (mail.smtp_host.trim()) {
|
||||
const from = mail.from_email.trim();
|
||||
if (!from) {
|
||||
next.from_email = t('setup.config.fromEmailRequired', 'A From address is required when an SMTP host is set.');
|
||||
} else if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(from)) {
|
||||
next.from_email = t('setup.config.fromEmailInvalid', 'Enter a valid email address.');
|
||||
}
|
||||
const port = parseInt(mail.smtp_port, 10);
|
||||
if (!Number.isInteger(port) || port < 1 || port > 65535) {
|
||||
next.smtp_port = t('setup.config.smtpPortInvalid', 'Enter a port between 1 and 65535.');
|
||||
}
|
||||
}
|
||||
setErrors(next);
|
||||
return Object.keys(next).length === 0;
|
||||
};
|
||||
|
||||
const skip = async () => {
|
||||
if (siteUrl.trim() && !isAbsoluteHttpUrl(siteUrl)) {
|
||||
setErrors({ siteUrl: t('setup.config.siteUrlInvalid', 'Enter the full address including http:// or https://, for example https://gallery.example.com') });
|
||||
return;
|
||||
}
|
||||
setErrors({});
|
||||
setSaving(true);
|
||||
// "Skip for now" always leaves, by contract — but say so rather than
|
||||
// dropping the address without a word.
|
||||
try {
|
||||
await saveSiteUrl();
|
||||
} catch {
|
||||
toast.warn(siteUrlRejected());
|
||||
}
|
||||
setSaving(false);
|
||||
onDone();
|
||||
};
|
||||
|
||||
const finish = async () => {
|
||||
if (!validate()) return;
|
||||
setSaving(true);
|
||||
let failed = false;
|
||||
|
||||
// Separate from the block below so the message lands on the address field
|
||||
// instead of the generic "some settings could not be saved" warning. The
|
||||
// server check is stricter than isAbsoluteHttpUrl in places, so this is
|
||||
// reachable even after validate() has passed.
|
||||
try {
|
||||
await saveSiteUrl();
|
||||
} catch {
|
||||
setSaving(false);
|
||||
setErrors((prev) => ({ ...prev, siteUrl: siteUrlRejected() }));
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
// Invoicing: only persist if they actually started filling it in.
|
||||
if (showInvoicing && inv.companyName.trim()) {
|
||||
@@ -64,7 +152,7 @@ export const SetupConfigStep: React.FC<Props> = ({ selectedFeatures, onDone }) =
|
||||
}
|
||||
}
|
||||
// Email: only persist if a host was entered.
|
||||
if (showEmail && mail.smtp_host.trim()) {
|
||||
if (mail.smtp_host.trim()) {
|
||||
const port = parseInt(mail.smtp_port, 10) || 587;
|
||||
const config: EmailConfig = {
|
||||
smtp_host: mail.smtp_host.trim(),
|
||||
@@ -78,20 +166,39 @@ export const SetupConfigStep: React.FC<Props> = ({ selectedFeatures, onDone }) =
|
||||
};
|
||||
await emailService.updateConfig(config);
|
||||
}
|
||||
} catch (_) {
|
||||
toast.warn(t('setup.config.saveFailed', 'Some settings could not be saved — you can finish them in Settings.'));
|
||||
} catch {
|
||||
// Stay on the step: advancing here discarded everything the user typed,
|
||||
// the SMTP password included, with no way back to re-enter it (#1104).
|
||||
failed = true;
|
||||
toast.warn(t('setup.config.saveFailed', 'Some settings could not be saved — check the values below, or use “Skip for now” and finish in Settings.'));
|
||||
} finally {
|
||||
setSaving(false);
|
||||
onDone();
|
||||
}
|
||||
if (!failed) onDone();
|
||||
};
|
||||
|
||||
return (
|
||||
<div className="space-y-8">
|
||||
<p className="rounded-lg bg-neutral-50 border border-neutral-200 px-3 py-2 text-xs text-neutral-600">
|
||||
{t('setup.config.intro', 'A few details for the features you picked. Anything you skip keeps its default and can be set later in Settings.')}
|
||||
{t('setup.config.intro', 'A few details to finish setting up. Anything you skip keeps its default and can be set later in Settings.')}
|
||||
</p>
|
||||
|
||||
<div className="space-y-3">
|
||||
<h3 className="text-sm font-semibold text-neutral-800">
|
||||
{t('setup.config.siteUrl', 'Public address')}
|
||||
</h3>
|
||||
<p className="text-xs text-neutral-500">
|
||||
{t('setup.config.siteUrlHint', 'Where your clients will reach this gallery. Prefilled with the address you opened right now — change it if you will put PicPeak behind a domain or reverse proxy. You can update this any time in Settings → General.')}
|
||||
</p>
|
||||
<Input
|
||||
type="url"
|
||||
placeholder="https://gallery.example.com"
|
||||
value={siteUrl}
|
||||
onChange={(e) => setSiteUrl(e.target.value)}
|
||||
error={errors.siteUrl}
|
||||
/>
|
||||
</div>
|
||||
|
||||
{showInvoicing && (
|
||||
<div className="space-y-3">
|
||||
<h3 className="text-sm font-semibold text-neutral-800">{t('setup.config.invoicing', 'Invoicing details')}</h3>
|
||||
@@ -119,27 +226,33 @@ export const SetupConfigStep: React.FC<Props> = ({ selectedFeatures, onDone }) =
|
||||
</div>
|
||||
)}
|
||||
|
||||
{showEmail && (
|
||||
<div className="space-y-3">
|
||||
<h3 className="text-sm font-semibold text-neutral-800">{t('setup.config.email', 'Email delivery (SMTP)')}</h3>
|
||||
<p className="text-xs text-neutral-500">{t('setup.config.emailHint', 'Required to send reminders, invoices and notifications.')}</p>
|
||||
<div className="space-y-3">
|
||||
<h3 className="text-sm font-semibold text-neutral-800">{t('setup.config.email', 'Email delivery (SMTP)')}</h3>
|
||||
<p className="text-xs text-neutral-500">{t('setup.config.emailHint', 'Used to send gallery links to your clients, plus guest invites, expiry warnings and any reminders or invoices you enable. Leave blank to set it up later in Settings → Email.')}</p>
|
||||
<div className="grid grid-cols-3 gap-3">
|
||||
<div className="col-span-2"><Input placeholder={t('setup.config.smtpHost', 'SMTP host')} value={mail.smtp_host} onChange={mailField('smtp_host')} /></div>
|
||||
<Input placeholder={t('setup.config.smtpPort', 'Port')} value={mail.smtp_port} onChange={mailField('smtp_port')} />
|
||||
<Input placeholder={t('setup.config.smtpPort', 'Port')} value={mail.smtp_port} onChange={mailField('smtp_port')} error={errors.smtp_port} />
|
||||
</div>
|
||||
<div className="grid grid-cols-2 gap-3">
|
||||
<Input placeholder={t('setup.config.smtpUser', 'Username')} value={mail.smtp_user} onChange={mailField('smtp_user')} autoComplete="off" />
|
||||
<Input type="password" placeholder={t('setup.config.smtpPass', 'Password')} value={mail.smtp_pass} onChange={mailField('smtp_pass')} autoComplete="new-password" />
|
||||
</div>
|
||||
<div className="grid grid-cols-2 gap-3">
|
||||
<Input type="email" placeholder={t('setup.config.fromEmail', 'From address')} value={mail.from_email} onChange={mailField('from_email')} />
|
||||
<Input
|
||||
type="email"
|
||||
placeholder={mail.smtp_host.trim()
|
||||
? t('setup.config.fromEmailRequiredPlaceholder', 'From address (required)')
|
||||
: t('setup.config.fromEmail', 'From address')}
|
||||
value={mail.from_email}
|
||||
onChange={mailField('from_email')}
|
||||
error={errors.from_email}
|
||||
/>
|
||||
<Input placeholder={t('setup.config.fromName', 'From name')} value={mail.from_name} onChange={mailField('from_name')} />
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
<div className="flex gap-3">
|
||||
<Button type="button" variant="outline" size="lg" onClick={onDone} disabled={saving}>
|
||||
<Button type="button" variant="outline" size="lg" onClick={skip} disabled={saving}>
|
||||
{t('setup.config.skip', 'Skip for now')}
|
||||
</Button>
|
||||
<Button type="button" variant="primary" size="lg" isLoading={saving} className="flex-1" onClick={finish}>
|
||||
|
||||
Reference in New Issue
Block a user