fix(setup): address PR #714 review — password UX, script token, race, nits
Blockers: - SetupPage now mirrors the server password rule (>=8 with upper/lower/digit) so a green client isn't bounced by the server; server errors carry a `field` (routes/setup.js) that the client maps to a translated key instead of rendering raw English. New i18n: setup.invalidToken, setup.passwordRequirements. - picpeak-setup.sh: the ADMIN_CREDENTIALS.txt block no longer dead-ends on the wizard path — when no legacy admin was seeded it prints the one-time setup token (from data/SETUP_TOKEN / docker compose logs) and points at /setup. Concern: - createInitialAdmin creates the admin + burns the token in ONE transaction, atomically claiming the token (null-if-present, expect 1 row) so a double-submit can't create two super_admins. Cross-DB (whereNotNull, trx-only writes). Added a concurrency test. Nits: - SetupPage redirects to /login when /setup/status errors (no form flash on a configured instance). - Dropped the unused DATABASE_URL from docker-compose.yml. - Documented why secrets are chmod 644 (three different reader users).
This commit is contained in:
@@ -113,6 +113,18 @@ describe('setupService (first-run bootstrap)', () => {
|
|||||||
).rejects.toMatchObject({ statusCode: 409 });
|
).rejects.toMatchObject({ statusCode: 409 });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('serialises a double-submit — two concurrent valid-token calls create only one admin', async () => {
|
||||||
|
const token = await setupService.ensureSetupToken();
|
||||||
|
const results = await Promise.allSettled([
|
||||||
|
setupService.createInitialAdmin({ token, email: '[email protected]', password: VALID_PW }),
|
||||||
|
setupService.createInitialAdmin({ token, email: '[email protected]', password: VALID_PW }),
|
||||||
|
]);
|
||||||
|
const fulfilled = results.filter((r) => r.status === 'fulfilled');
|
||||||
|
expect(fulfilled).toHaveLength(1); // the atomic token claim lets exactly one win
|
||||||
|
const count = await db('admin_users').count({ c: '*' }).first();
|
||||||
|
expect(Number(count.c)).toBe(1);
|
||||||
|
});
|
||||||
|
|
||||||
it('ensureSetupToken clears any stale token once an admin exists', async () => {
|
it('ensureSetupToken clears any stale token once an admin exists', async () => {
|
||||||
const token = await setupService.ensureSetupToken();
|
const token = await setupService.ensureSetupToken();
|
||||||
await setupService.createInitialAdmin({ token, email: '[email protected]', password: VALID_PW });
|
await setupService.createInitialAdmin({ token, email: '[email protected]', password: VALID_PW });
|
||||||
|
|||||||
@@ -45,7 +45,9 @@ router.post('/admin', [
|
|||||||
res.status(201).json({ user: result.user });
|
res.status(201).json({ user: result.user });
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
if (err.statusCode) {
|
if (err.statusCode) {
|
||||||
return res.status(err.statusCode).json({ error: err.message });
|
// `field` (token/email/password) lets the client show a translated
|
||||||
|
// message instead of rendering the raw English error verbatim.
|
||||||
|
return res.status(err.statusCode).json({ error: err.message, field: err.details || undefined });
|
||||||
}
|
}
|
||||||
logger.error('[setup] createInitialAdmin failed', { error: err.message });
|
logger.error('[setup] createInitialAdmin failed', { error: err.message });
|
||||||
return res.status(500).json({ error: 'Setup failed' });
|
return res.status(500).json({ error: 'Setup failed' });
|
||||||
|
|||||||
@@ -104,22 +104,39 @@ async function createInitialAdmin({ token, email, password, ip }) {
|
|||||||
if (!role) {
|
if (!role) {
|
||||||
throw new ConflictError('super_admin role missing — database not initialised');
|
throw new ConflictError('super_admin role missing — database not initialised');
|
||||||
}
|
}
|
||||||
|
|
||||||
const passwordHash = await bcrypt.hash(password, getBcryptRounds());
|
const passwordHash = await bcrypt.hash(password, getBcryptRounds());
|
||||||
const inserted = await db('admin_users').insert({
|
|
||||||
username: cleanEmail,
|
|
||||||
email: cleanEmail,
|
|
||||||
password_hash: passwordHash,
|
|
||||||
role_id: role.id,
|
|
||||||
is_active: formatBoolean(true),
|
|
||||||
must_change_password: formatBoolean(false),
|
|
||||||
created_at: new Date(),
|
|
||||||
updated_at: new Date(),
|
|
||||||
}).returning('id');
|
|
||||||
const id = inserted[0]?.id || inserted[0];
|
|
||||||
|
|
||||||
// Burn the one-time token (DB + file) — the endpoint is now permanently closed.
|
// Create the admin and burn the token ATOMICALLY. The claim (null the token
|
||||||
await clearSetupToken();
|
// row expecting exactly one match) serialises concurrent valid-token submits,
|
||||||
|
// so a double-submit can't create two super_admins. All writes use `trx`
|
||||||
|
// (never the global db) to avoid the SQLite in-transaction deadlock.
|
||||||
|
const id = await db.transaction(async (trx) => {
|
||||||
|
const claimed = await trx('app_settings')
|
||||||
|
.where({ setting_key: SETUP_TOKEN_KEY })
|
||||||
|
.whereNotNull('setting_value')
|
||||||
|
.update({ setting_value: null, updated_at: new Date() });
|
||||||
|
if (claimed !== 1) {
|
||||||
|
throw new ConflictError('Setup already completed — an admin account exists');
|
||||||
|
}
|
||||||
|
const cnt = await trx('admin_users').count({ c: '*' }).first();
|
||||||
|
if (Number(cnt?.c || 0) !== 0) {
|
||||||
|
throw new ConflictError('Setup already completed — an admin account exists');
|
||||||
|
}
|
||||||
|
const inserted = await trx('admin_users').insert({
|
||||||
|
username: cleanEmail,
|
||||||
|
email: cleanEmail,
|
||||||
|
password_hash: passwordHash,
|
||||||
|
role_id: role.id,
|
||||||
|
is_active: formatBoolean(true),
|
||||||
|
must_change_password: formatBoolean(false),
|
||||||
|
created_at: new Date(),
|
||||||
|
updated_at: new Date(),
|
||||||
|
}).returning('id');
|
||||||
|
return inserted[0]?.id || inserted[0];
|
||||||
|
});
|
||||||
|
|
||||||
|
// DB token cleared inside the tx; remove the on-disk file too (best-effort).
|
||||||
|
try { fs.unlinkSync(setupTokenFilePath()); } catch (_) { /* best-effort */ }
|
||||||
logger.info(`[setup] Initial super_admin created (id=${id}, email=${cleanEmail})`);
|
logger.info(`[setup] Initial super_admin created (id=${id}, email=${cleanEmail})`);
|
||||||
|
|
||||||
const authToken = jwt.sign(
|
const authToken = jwt.sign(
|
||||||
|
|||||||
@@ -28,6 +28,9 @@ services:
|
|||||||
if [ -n "$$REDIS_PASSWORD" ]; then printf '%s' "$$REDIS_PASSWORD" > /run/secrets/redis_password;
|
if [ -n "$$REDIS_PASSWORD" ]; then printf '%s' "$$REDIS_PASSWORD" > /run/secrets/redis_password;
|
||||||
else tr -dc A-Za-z0-9 < /dev/urandom | head -c 48 > /run/secrets/redis_password; fi
|
else tr -dc A-Za-z0-9 < /dev/urandom | head -c 48 > /run/secrets/redis_password; fi
|
||||||
fi
|
fi
|
||||||
|
# 644: the readers run as three different users (postgres, redis, nodejs),
|
||||||
|
# so a non-root reader must be able to read them. The volume is private to
|
||||||
|
# these containers and never host-exposed.
|
||||||
chmod 644 /run/secrets/jwt_secret /run/secrets/db_password /run/secrets/redis_password
|
chmod 644 /run/secrets/jwt_secret /run/secrets/db_password /run/secrets/redis_password
|
||||||
volumes:
|
volumes:
|
||||||
- picpeak-secrets:/run/secrets
|
- picpeak-secrets:/run/secrets
|
||||||
|
|||||||
+3
-1
@@ -24,6 +24,9 @@ services:
|
|||||||
if [ -n "$$REDIS_PASSWORD" ]; then printf '%s' "$$REDIS_PASSWORD" > /run/secrets/redis_password;
|
if [ -n "$$REDIS_PASSWORD" ]; then printf '%s' "$$REDIS_PASSWORD" > /run/secrets/redis_password;
|
||||||
else tr -dc A-Za-z0-9 < /dev/urandom | head -c 48 > /run/secrets/redis_password; fi
|
else tr -dc A-Za-z0-9 < /dev/urandom | head -c 48 > /run/secrets/redis_password; fi
|
||||||
fi
|
fi
|
||||||
|
# 644: the readers run as three different users (postgres, redis, nodejs),
|
||||||
|
# so a non-root reader must be able to read them. The volume is private to
|
||||||
|
# these containers and never host-exposed.
|
||||||
chmod 644 /run/secrets/jwt_secret /run/secrets/db_password /run/secrets/redis_password
|
chmod 644 /run/secrets/jwt_secret /run/secrets/db_password /run/secrets/redis_password
|
||||||
volumes:
|
volumes:
|
||||||
- picpeak-secrets:/run/secrets
|
- picpeak-secrets:/run/secrets
|
||||||
@@ -43,7 +46,6 @@ services:
|
|||||||
- ADMIN_EMAIL=${ADMIN_EMAIL:[email protected]}
|
- ADMIN_EMAIL=${ADMIN_EMAIL:[email protected]}
|
||||||
- ADMIN_PASSWORD=${ADMIN_PASSWORD:-}
|
- ADMIN_PASSWORD=${ADMIN_PASSWORD:-}
|
||||||
- DATABASE_CLIENT=pg
|
- DATABASE_CLIENT=pg
|
||||||
- DATABASE_URL=postgresql://${DB_USER}:${DB_PASSWORD:-}@postgres:5432/${DB_NAME}
|
|
||||||
- DB_TYPE=postgresql
|
- DB_TYPE=postgresql
|
||||||
- DB_HOST=postgres
|
- DB_HOST=postgres
|
||||||
- DB_PORT=5432
|
- DB_PORT=5432
|
||||||
|
|||||||
@@ -3470,6 +3470,8 @@
|
|||||||
"tokenHint": "Wird beim ersten Start in den Server-Logs ausgegeben (auch in data/SETUP_TOKEN gespeichert).",
|
"tokenHint": "Wird beim ersten Start in den Server-Logs ausgegeben (auch in data/SETUP_TOKEN gespeichert).",
|
||||||
"tokenRequired": "Der Setup-Token ist erforderlich",
|
"tokenRequired": "Der Setup-Token ist erforderlich",
|
||||||
"tokenLocationHint": "Nicht gefunden? Führen Sie aus: docker compose logs backend | grep -i \"setup token\"",
|
"tokenLocationHint": "Nicht gefunden? Führen Sie aus: docker compose logs backend | grep -i \"setup token\"",
|
||||||
|
"invalidToken": "Dieser Setup-Token ist ungültig.",
|
||||||
|
"passwordRequirements": "Verwenden Sie mindestens 8 Zeichen mit einem Groß- und einem Kleinbuchstaben sowie einer Ziffer.",
|
||||||
"emailLabel": "E-Mail-Adresse",
|
"emailLabel": "E-Mail-Adresse",
|
||||||
"emailPlaceholder": "[email protected]",
|
"emailPlaceholder": "[email protected]",
|
||||||
"emailRequired": "E-Mail ist erforderlich",
|
"emailRequired": "E-Mail ist erforderlich",
|
||||||
|
|||||||
@@ -3366,6 +3366,8 @@
|
|||||||
"tokenHint": "Printed to the server logs on first start (also saved to data/SETUP_TOKEN).",
|
"tokenHint": "Printed to the server logs on first start (also saved to data/SETUP_TOKEN).",
|
||||||
"tokenRequired": "The setup token is required",
|
"tokenRequired": "The setup token is required",
|
||||||
"tokenLocationHint": "Can't find it? Run: docker compose logs backend | grep -i \"setup token\"",
|
"tokenLocationHint": "Can't find it? Run: docker compose logs backend | grep -i \"setup token\"",
|
||||||
|
"invalidToken": "That setup token is not valid.",
|
||||||
|
"passwordRequirements": "Use at least 8 characters with an upper-case letter, a lower-case letter and a number.",
|
||||||
"emailLabel": "Email address",
|
"emailLabel": "Email address",
|
||||||
"emailPlaceholder": "[email protected]",
|
"emailPlaceholder": "[email protected]",
|
||||||
"emailRequired": "Email is required",
|
"emailRequired": "Email is required",
|
||||||
|
|||||||
@@ -19,7 +19,7 @@ export const SetupPage: React.FC = () => {
|
|||||||
const navigate = useNavigate();
|
const navigate = useNavigate();
|
||||||
const { login } = useAdminAuth();
|
const { login } = useAdminAuth();
|
||||||
|
|
||||||
const { data: status, isLoading: statusLoading } = useQuery({
|
const { data: status, isLoading: statusLoading, isError: statusError } = useQuery({
|
||||||
queryKey: ['setup-status'],
|
queryKey: ['setup-status'],
|
||||||
queryFn: setupService.getSetupStatus,
|
queryFn: setupService.getSetupStatus,
|
||||||
retry: false,
|
retry: false,
|
||||||
@@ -34,8 +34,10 @@ export const SetupPage: React.FC = () => {
|
|||||||
if (statusLoading) {
|
if (statusLoading) {
|
||||||
return <Loading fullScreen />;
|
return <Loading fullScreen />;
|
||||||
}
|
}
|
||||||
// Setup already done → nothing to bootstrap here.
|
// Setup already done, OR the status couldn't be read (e.g. a transient 500) →
|
||||||
if (status && !status.needsAdmin) {
|
// go to login rather than flashing the create-admin form on a configured
|
||||||
|
// instance. Only render the wizard when we know an admin is genuinely missing.
|
||||||
|
if (statusError || !status?.needsAdmin) {
|
||||||
return <Navigate to="/admin/login" replace />;
|
return <Navigate to="/admin/login" replace />;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -50,7 +52,9 @@ export const SetupPage: React.FC = () => {
|
|||||||
if (!form.email) next.email = t('setup.emailRequired');
|
if (!form.email) next.email = t('setup.emailRequired');
|
||||||
else if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(form.email)) next.email = t('setup.invalidEmail');
|
else if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(form.email)) next.email = t('setup.invalidEmail');
|
||||||
if (!form.password) next.password = t('setup.passwordRequired');
|
if (!form.password) next.password = t('setup.passwordRequired');
|
||||||
else if (form.password.length < 8) next.password = t('setup.passwordMinLength');
|
// Mirror the server's rule (validatePassword): >=8 chars with upper, lower
|
||||||
|
// and a digit — so the user isn't bounced by the server after a green client.
|
||||||
|
else if (!/^(?=.*[a-z])(?=.*[A-Z])(?=.*\d).{8,}$/.test(form.password)) next.password = t('setup.passwordRequirements');
|
||||||
if (form.confirm !== form.password) next.confirm = t('setup.passwordMismatch');
|
if (form.confirm !== form.password) next.confirm = t('setup.passwordMismatch');
|
||||||
setErrors(next);
|
setErrors(next);
|
||||||
return Object.keys(next).length === 0;
|
return Object.keys(next).length === 0;
|
||||||
@@ -81,18 +85,27 @@ export const SetupPage: React.FC = () => {
|
|||||||
toast.success(t('setup.success'));
|
toast.success(t('setup.success'));
|
||||||
navigate('/admin/dashboard', { replace: true });
|
navigate('/admin/dashboard', { replace: true });
|
||||||
} catch (error: any) {
|
} catch (error: any) {
|
||||||
const apiErrors = error.response?.data?.errors;
|
const httpStatus = error.response?.status;
|
||||||
if (error.response?.status === 429) {
|
const data = error.response?.data;
|
||||||
|
// Map the server's field back to a translated message instead of
|
||||||
|
// rendering its raw English error verbatim.
|
||||||
|
const fieldKey: Record<string, string> = {
|
||||||
|
token: 'setup.invalidToken',
|
||||||
|
email: 'setup.invalidEmail',
|
||||||
|
password: 'setup.passwordRequirements',
|
||||||
|
};
|
||||||
|
if (httpStatus === 429) {
|
||||||
toast.error(t('setup.tooManyAttempts'));
|
toast.error(t('setup.tooManyAttempts'));
|
||||||
} else if (Array.isArray(apiErrors) && apiErrors.length) {
|
} else if (httpStatus === 409) {
|
||||||
setErrors({ form: apiErrors[0]?.msg || t('setup.genericError') });
|
|
||||||
} else if (error.response?.status === 409) {
|
|
||||||
// Someone else finished setup first — send to login.
|
// Someone else finished setup first — send to login.
|
||||||
navigate('/admin/login', { replace: true });
|
navigate('/admin/login', { replace: true });
|
||||||
} else if (error.response?.data?.error) {
|
} else if (data?.field && fieldKey[data.field]) {
|
||||||
setErrors({ form: error.response.data.error });
|
setErrors({ [data.field]: t(fieldKey[data.field]) });
|
||||||
|
} else if (Array.isArray(data?.errors) && data.errors.length) {
|
||||||
|
const p = data.errors[0]?.path || data.errors[0]?.param;
|
||||||
|
setErrors(p && fieldKey[p] ? { [p]: t(fieldKey[p]) } : { form: t('setup.genericError') });
|
||||||
} else {
|
} else {
|
||||||
toast.error(t('setup.genericError'));
|
setErrors({ form: t('setup.genericError') });
|
||||||
}
|
}
|
||||||
} finally {
|
} finally {
|
||||||
setIsSubmitting(false);
|
setIsSubmitting(false);
|
||||||
|
|||||||
+29
-10
@@ -563,24 +563,43 @@ EOF
|
|||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
|
|
||||||
# Always surface the admin credentials file (#427: iSchumi reported
|
# Surface how to finish setup. Two paths:
|
||||||
# admins couldn't find the generated password — the migration writes it
|
# - Legacy: if ADMIN_PASSWORD was set, migration seeded an admin and wrote
|
||||||
# to the in-container path and we never copied it to the host unless
|
# data/ADMIN_CREDENTIALS.txt (#427) — print those credentials.
|
||||||
# --reset-admin-password was used). Best-effort: a missing file just
|
# - Default (no ADMIN_PASSWORD): no admin is seeded; the app shows a
|
||||||
# means the migration ran on a pre-existing DB and didn't generate one.
|
# first-run wizard at /setup guarded by a one-time token
|
||||||
|
# (data/SETUP_TOKEN). Print the token and point the operator there.
|
||||||
|
local login_base="${DOMAIN_NAME:+https://$DOMAIN_NAME}${DOMAIN_NAME:-http://YOUR_HOST_IP:3000}"
|
||||||
if docker compose cp backend:/app/data/ADMIN_CREDENTIALS.txt "$app_dir/data/ADMIN_CREDENTIALS.txt" 2>/dev/null; then
|
if docker compose cp backend:/app/data/ADMIN_CREDENTIALS.txt "$app_dir/data/ADMIN_CREDENTIALS.txt" 2>/dev/null; then
|
||||||
chown "$host_uid":"$host_gid" "$app_dir/data/ADMIN_CREDENTIALS.txt" 2>/dev/null || true
|
chown "$host_uid":"$host_gid" "$app_dir/data/ADMIN_CREDENTIALS.txt" 2>/dev/null || true
|
||||||
chmod 600 "$app_dir/data/ADMIN_CREDENTIALS.txt" 2>/dev/null || true
|
chmod 600 "$app_dir/data/ADMIN_CREDENTIALS.txt" 2>/dev/null || true
|
||||||
log_step "Admin credentials saved to: $app_dir/data/ADMIN_CREDENTIALS.txt"
|
log_step "Admin credentials saved to: $app_dir/data/ADMIN_CREDENTIALS.txt"
|
||||||
# Show the password in the install output so the operator can log
|
|
||||||
# in immediately. The file remains as a backup record.
|
|
||||||
echo
|
echo
|
||||||
echo "--------------------------------------------------"
|
echo "--------------------------------------------------"
|
||||||
grep -E '^Email:|^Password:' "$app_dir/data/ADMIN_CREDENTIALS.txt" 2>/dev/null || true
|
grep -E '^Email:|^Password:' "$app_dir/data/ADMIN_CREDENTIALS.txt" 2>/dev/null || true
|
||||||
echo "--------------------------------------------------"
|
echo "--------------------------------------------------"
|
||||||
echo " Login URL: ${DOMAIN_NAME:+https://$DOMAIN_NAME}${DOMAIN_NAME:-http://YOUR_HOST_IP:3000}/admin"
|
echo " Login URL: $login_base/admin"
|
||||||
echo " Full credentials file: $app_dir/data/ADMIN_CREDENTIALS.txt"
|
echo " Delete the credentials file after recording the password."
|
||||||
echo " Delete the file after recording the password."
|
echo
|
||||||
|
else
|
||||||
|
# First-run wizard path — surface the one-time setup token.
|
||||||
|
docker compose cp backend:/app/data/SETUP_TOKEN "$app_dir/data/SETUP_TOKEN" 2>/dev/null || true
|
||||||
|
chown "$host_uid":"$host_gid" "$app_dir/data/SETUP_TOKEN" 2>/dev/null || true
|
||||||
|
local setup_token
|
||||||
|
setup_token="$(cat "$app_dir/data/SETUP_TOKEN" 2>/dev/null)"
|
||||||
|
[ -z "$setup_token" ] && setup_token="$(docker compose logs backend 2>/dev/null | grep -i 'setup token:' | tail -1 | sed -E 's/.*setup token: *([A-Za-z0-9_-]+).*/\1/')"
|
||||||
|
echo
|
||||||
|
echo "--------------------------------------------------"
|
||||||
|
echo " Finish setup in your browser — create the admin account:"
|
||||||
|
echo " 1. Open $login_base/admin"
|
||||||
|
echo " 2. Enter this one-time setup token:"
|
||||||
|
if [ -n "$setup_token" ]; then
|
||||||
|
echo " $setup_token"
|
||||||
|
else
|
||||||
|
echo " (run: cd $app_dir && docker compose logs backend | grep -i \"setup token\")"
|
||||||
|
fi
|
||||||
|
echo " 3. Set your admin email and password."
|
||||||
|
echo "--------------------------------------------------"
|
||||||
echo
|
echo
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user