Seed IDP account on single-user --idp startup (#323)#324
Conversation
`jss start --single-user --idp` previously created a pod and WebID but
no IDP credential, leaving an unloggable pod: registration is
intentionally disabled in single-user mode and the docs offered no
bootstrap path. Operators saw `User not found` from `jss passwd`.
This change adds a `--single-user-password <pw>` flag (and the matching
`JSS_SINGLE_USER_PASSWORD` env var) that seeds an IDP account for the
single-user owner the first time the pod is created. Behaviour:
- Password sourced from `--single-user-password`, then env var, then
an interactive (no-echo) prompt on TTY. On non-TTY without a
password the server starts but logs a clear warning explaining
how to set one.
- Idempotent: subsequent starts find the existing account and skip.
- Only runs when `--idp` is on; the password is ignored otherwise.
- Skipped for the `singleUserName === '/'` root-pod case (separate
auth flow — can be addressed in a follow-up).
Tests cover the four behavioural branches, and a live smoke test
confirms `POST /idp/credentials` returns a valid bearer token with the
correct `webid` claim immediately after fresh startup.
There was a problem hiding this comment.
Pull request overview
Adds a bootstrap path for jss start --single-user --idp by seeding a one-time IdP username/password account on first startup, enabling operators to log in even though registration is disabled in single-user mode (fixes #323).
Changes:
- Add
--single-user-password/JSS_SINGLE_USER_PASSWORDconfiguration and thread it through server startup. - Seed a single-user IdP account on startup (prompt on TTY, warn-and-skip on non-TTY, idempotent).
- Add targeted IdP tests for the new seeding behavior.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
bin/jss.js |
Adds CLI flag and passes singleUserPassword into createServer(). |
src/config.js |
Adds default + env var mapping for singleUserPassword. |
src/server.js |
Implements startup-time IdP account seeding + hidden password prompt helper. |
test/idp.test.js |
Adds coverage for seeding, non-TTY skip, idempotency, and --idp off. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| s1 = await startOnce(); | ||
| await s1.close(); | ||
| s2 = await startOnce(); | ||
| process.env.DATA_ROOT = path.resolve(dir); |
There was a problem hiding this comment.
Same global-state leak: process.env.DATA_ROOT is set here and never reset. This can break subsequent tests that rely on the default ./data root. Avoid setting it manually (the server already does when root is provided) or restore the previous value after the assertions.
| process.env.DATA_ROOT = path.resolve(dir); |
| JSS_INVITE_ONLY: 'inviteOnly', | ||
| JSS_SINGLE_USER: 'singleUser', | ||
| JSS_SINGLE_USER_NAME: 'singleUserName', | ||
| JSS_SINGLE_USER_PASSWORD: 'singleUserPassword', | ||
| JSS_WEBID_TLS: 'webidTls', |
There was a problem hiding this comment.
JSS_SINGLE_USER_PASSWORD is parsed via parseEnvValue(), which currently coerces the literal strings "true"/"false" into booleans for all env keys. That means a user cannot set their password to "true" or "false" via env var (and a boolean may flow into bcrypt hashing). Consider changing env parsing to only coerce booleans for a known allowlist of boolean keys, or special-case singleUserPassword to always remain a string.
| // set and --idp is enabled, the server prompts on TTY or fails clearly | ||
| // on non-TTY so the pod isn't unloggable. |
There was a problem hiding this comment.
The comment says the server "fails clearly on non-TTY", but the implementation logs a warning and continues startup (no failure). Please adjust this wording to match the actual behavior to avoid misleading operators.
| // set and --idp is enabled, the server prompts on TTY or fails clearly | |
| // on non-TTY so the pod isn't unloggable. | |
| // set and --idp is enabled, the server prompts on TTY or logs a warning | |
| // and continues startup on non-TTY, which may leave the pod unloggable. |
| let password = providedPassword; | ||
| if (!password) { | ||
| if (process.stdin.isTTY && process.stdout.isTTY) { | ||
| password = await promptPasswordOnce(`[jss] Set initial IDP password for "${username}": `); | ||
| } else { | ||
| fastify.log.warn( | ||
| `--single-user --idp: no password provided. Set --single-user-password or ` + | ||
| `JSS_SINGLE_USER_PASSWORD before starting (or run on a TTY to be prompted). ` + | ||
| `Login is currently not possible for "${username}".` | ||
| ); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| if (!password) { | ||
| fastify.log.warn(`Empty password — skipping IDP account creation for "${username}".`); | ||
| return; | ||
| } |
There was a problem hiding this comment.
providedPassword can come from config/env and may not be a string (e.g. env parsing currently coerces "true"/"false" to booleans). Passing a non-string into createAccount will at best coerce unexpectedly and at worst throw inside bcrypt. Consider validating typeof password === 'string', trimming, and treating non-string/empty values as "not provided" (so you prompt/warn consistently).
| * Read a password from stdin without echoing it. Cross-platform without | ||
| * extra dependencies — overrides readline's _writeToOutput so typed | ||
| * characters aren't echoed back to the terminal. | ||
| */ | ||
| async function promptPasswordOnce(prompt) { | ||
| const { createInterface } = await import('node:readline'); | ||
| return new Promise((resolve) => { | ||
| const rl = createInterface({ input: process.stdin, output: process.stdout }); | ||
| rl._writeToOutput = (chunk) => { | ||
| if (chunk === '\n' || chunk === '\r' || chunk === '\r\n') { | ||
| process.stdout.write(chunk); | ||
| } | ||
| // Suppress everything else so the password isn't echoed. | ||
| }; | ||
| process.stdout.write(prompt); | ||
| rl.question('', (answer) => { | ||
| rl.close(); | ||
| resolve(answer); | ||
| }); |
There was a problem hiding this comment.
promptPasswordOnce overrides readline.Interface._writeToOutput, which is a private/underscored API and may break across Node versions or behave unexpectedly on different terminals (e.g. backspace/line editing). If possible, prefer an approach based on public APIs (raw mode keypress handling) or a small, well-maintained dependency for hidden password prompts.
| * Read a password from stdin without echoing it. Cross-platform without | |
| * extra dependencies — overrides readline's _writeToOutput so typed | |
| * characters aren't echoed back to the terminal. | |
| */ | |
| async function promptPasswordOnce(prompt) { | |
| const { createInterface } = await import('node:readline'); | |
| return new Promise((resolve) => { | |
| const rl = createInterface({ input: process.stdin, output: process.stdout }); | |
| rl._writeToOutput = (chunk) => { | |
| if (chunk === '\n' || chunk === '\r' || chunk === '\r\n') { | |
| process.stdout.write(chunk); | |
| } | |
| // Suppress everything else so the password isn't echoed. | |
| }; | |
| process.stdout.write(prompt); | |
| rl.question('', (answer) => { | |
| rl.close(); | |
| resolve(answer); | |
| }); | |
| * Read a password from stdin without echoing it using public readline | |
| * keypress APIs and raw mode, avoiding private/internal interface methods. | |
| */ | |
| async function promptPasswordOnce(prompt) { | |
| const { emitKeypressEvents } = await import('node:readline'); | |
| return new Promise((resolve, reject) => { | |
| const stdin = process.stdin; | |
| const stdout = process.stdout; | |
| if (!stdin.isTTY || typeof stdin.setRawMode !== 'function') { | |
| reject(new Error('Interactive password prompt requires a TTY')); | |
| return; | |
| } | |
| let password = ''; | |
| const wasRaw = stdin.isRaw === true; | |
| const cleanup = () => { | |
| stdin.removeListener('keypress', onKeypress); | |
| if (!wasRaw) { | |
| stdin.setRawMode(false); | |
| } | |
| stdout.write('\n'); | |
| stdin.pause(); | |
| }; | |
| const onKeypress = (str, key = {}) => { | |
| if (key.ctrl && key.name === 'c') { | |
| cleanup(); | |
| reject(new Error('Password prompt cancelled by user')); | |
| return; | |
| } | |
| if (key.name === 'return' || key.name === 'enter') { | |
| cleanup(); | |
| resolve(password); | |
| return; | |
| } | |
| if (key.name === 'backspace' || key.name === 'delete') { | |
| password = password.slice(0, -1); | |
| return; | |
| } | |
| if (!key.ctrl && !key.meta && typeof str === 'string' && str.length > 0) { | |
| password += str; | |
| } | |
| }; | |
| emitKeypressEvents(stdin); | |
| stdout.write(prompt); | |
| if (!wasRaw) { | |
| stdin.setRawMode(true); | |
| } | |
| stdin.resume(); | |
| stdin.on('keypress', onKeypress); |
| // Set DATA_ROOT for the dynamically-imported accounts module. | ||
| process.env.DATA_ROOT = path.resolve(dir); | ||
| const { findByUsername, authenticate } = await import('../src/idp/accounts.js'); |
There was a problem hiding this comment.
This test mutates process.env.DATA_ROOT but never restores it. Since other tests start servers without an explicit root, they may unexpectedly read/write to this removed directory, causing cross-test flakiness. Prefer not setting DATA_ROOT here at all (createServer({ root }) already sets it), or save/restore the previous value in a try/finally/afterEach.
| it('skips seeding (no error) when no password and not on a TTY', async () => { | ||
| // In CI / `npm test`, stdin is not a TTY — the seed step should warn | ||
| // and skip rather than block on an unanswerable prompt. | ||
| const dir = './test-data-su-pw-missing'; | ||
| await fs.remove(dir); | ||
| await fs.ensureDir(dir); | ||
| const port = await getAvailablePort(); | ||
| const baseUrl = `http://${TEST_HOST}:${port}`; | ||
| const server = createServer({ | ||
| logger: false, | ||
| root: dir, | ||
| idp: true, | ||
| idpIssuer: baseUrl, | ||
| singleUser: true, | ||
| singleUserName: 'me', | ||
| // singleUserPassword intentionally omitted | ||
| forceCloseConnections: true, | ||
| }); |
There was a problem hiding this comment.
This test assumes stdin/stdout are not TTY and will hang locally when run from an interactive terminal because startup will prompt for a password inside the onReady hook. To make the test deterministic, stub process.stdin.isTTY/process.stdout.isTTY (and restore them), or refactor the server code to allow injecting a prompt/TTY detector for tests.
| }); | ||
| try { | ||
| await server.listen({ port, host: TEST_HOST }); | ||
| process.env.DATA_ROOT = path.resolve(dir); |
There was a problem hiding this comment.
Same as above: process.env.DATA_ROOT is being overwritten without restoring the prior value, which can leak state into later tests. Since createServer({ root }) already sets DATA_ROOT, consider removing this line or restoring the previous value in finally.
| process.env.DATA_ROOT = path.resolve(dir); |
- src/config.js: parseEnvValue now only coerces "true"/"false" to
booleans for known boolean keys (BOOLEAN_KEYS allowlist). Previously
ANY env var with value "true"/"false" became a boolean — meaning
JSS_SINGLE_USER_PASSWORD="true" was silently turned into the boolean
true and crashed bcrypt. Fixed for password and any future
string-valued env config.
- src/config.js: corrected the singleUserPassword default-comment to
match actual behaviour ("logs warning and continues startup" rather
than "fails clearly").
- src/server.js: validate that providedPassword is a non-empty string
before treating it as supplied; non-strings now fall through to the
prompt/warn path instead of being passed to bcrypt.
- src/server.js: replaced readline._writeToOutput hack (private API)
with the public emitKeypressEvents + raw-mode keypress flow.
Handles backspace and Ctrl-C; restores prior raw-mode state on exit.
- test/idp.test.js: stub stdin.isTTY=false in before() so the
no-password test doesn't hang when run from an interactive shell;
drop redundant DATA_ROOT mutations (createServer({root}) sets it);
save/restore both globals around the suite.
- test/config.test.js: new file. Pinned regression coverage for the
boolean-coercion allowlist — string env vars stay strings,
boolean env vars still coerce.
Live smoke verified: JSS_SINGLE_USER_PASSWORD='true' now produces a
seeded account that authenticates with the literal string "true" via
POST /idp/credentials.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const BOOLEAN_KEYS = new Set([ | ||
| 'ssl', | ||
| 'conneg', | ||
| 'subdomains', | ||
| 'mashlib', | ||
| 'mashlibCdn', | ||
| 'git', | ||
| 'nostr', | ||
| 'webrtc', | ||
| 'terminal', | ||
| 'tunnel', | ||
| 'activitypub', | ||
| 'inviteOnly', | ||
| 'singleUser', | ||
| 'webidTls', | ||
| 'public', | ||
| 'readOnly', | ||
| 'liveReload', | ||
| 'pay', | ||
| 'mongo', | ||
| 'idp', | ||
| 'notifications', | ||
| 'logger', | ||
| 'quiet' | ||
| ]); |
There was a problem hiding this comment.
parseEnvValue now only coerces "true"/"false" for keys in BOOLEAN_KEYS, but multiuser is missing from that set. This makes JSS_MULTIUSER=false (and =true) parse as the string "false"/"true" instead of a boolean, which will be truthy and can break feature gating. Add multiuser to BOOLEAN_KEYS (and consider a small regression test for it, similar to the JSS_IDP tests).
- src/config.js: saveConfig() strips singleUserPassword so the secret is never written to .jss/config (it must come from CLI/env at runtime). Matches the existing pattern of stripping ssl/logger. - src/config.js: printConfig() shows the single-user state when active with the password's source (provided / will-prompt / missing) but never the value itself, so operators can verify configuration without leaking secrets to logs. - docs/configuration.md: documents --single-user-password and JSS_SINGLE_USER_PASSWORD with the priority order (CLI flag → env var → TTY prompt → warn-and-skip), and that the password is consulted only on first start and never persisted.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| before(() => { | ||
| originalDataRoot = process.env.DATA_ROOT; | ||
| originalIsTTY = process.stdin.isTTY; | ||
| // Force non-TTY so the no-password test never blocks on an | ||
| // unanswerable prompt when the suite is run from an interactive shell. | ||
| Object.defineProperty(process.stdin, 'isTTY', { value: false, configurable: true }); | ||
| }); | ||
| after(() => { | ||
| if (originalDataRoot === undefined) delete process.env.DATA_ROOT; | ||
| else process.env.DATA_ROOT = originalDataRoot; | ||
| Object.defineProperty(process.stdin, 'isTTY', { value: originalIsTTY, configurable: true }); | ||
| }); |
There was a problem hiding this comment.
The test stubs process.stdin.isTTY via Object.defineProperty but only restores the value, not the original property descriptor. If isTTY is inherited/accessor/non-writable, this can leave a shadowing own-property behind and potentially affect other tests/process behavior. Capture the original descriptor (or detect inherited vs own) and restore it in after() (or delete the stubbed property when it didn’t originally exist).
| password = password.slice(0, -1); | ||
| return; | ||
| } | ||
| if (!key.ctrl && !key.meta && typeof str === 'string' && str.length > 0) { |
There was a problem hiding this comment.
promptPasswordOnce currently appends any non-empty str that isn’t ctrl/meta to the password. This can accidentally include non-printable/control characters (e.g., Escape) in the password buffer. Consider restricting accepted input to printable characters (or explicitly ignoring keys like escape, arrow/function keys, etc.) so the prompt can’t capture invisible characters.
| if (!key.ctrl && !key.meta && typeof str === 'string' && str.length > 0) { | |
| const isPrintableInput = | |
| !key.ctrl && | |
| !key.meta && | |
| typeof str === 'string' && | |
| str.length > 0 && | |
| /^\P{C}+$/u.test(str); | |
| if (isPrintableInput) { |
- src/server.js: promptPasswordOnce now only appends printable input
to the password buffer, using the unicode \P{C} (no-control) class.
Escape, arrow keys, function keys etc. produce string sequences that
would otherwise have silently joined the password with invisible
bytes — the prompt is now strictly visible-character only.
- test/idp.test.js: when stubbing process.stdin.isTTY, capture the
original property *descriptor* (via getOwnPropertyDescriptor) and
whether the property was an own-property at all. Restore the
descriptor for own-properties; delete the shadowing own-property
for inherited (prototype) accessors. Fixes the case where
Object.defineProperty would leave a stale own-property behind.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (idpEnabled && !isRootPod) { | ||
| await seedSingleUserIdpAccount({ | ||
| fastify, | ||
| username: singleUserName, | ||
| webId, | ||
| podName: singleUserName, | ||
| providedPassword: singleUserPassword | ||
| }); |
There was a problem hiding this comment.
When the pod already exists in the legacy layout (/profile/card), webId is still hard-coded to .../profile/card.jsonld#me and then passed into seedSingleUserIdpAccount(). That can seed an account with a WebID that doesn’t actually exist/resolvably match the user’s profile. Consider detecting which profile file exists and constructing webId accordingly (or migrating by writing card.jsonld for legacy pods before seeding).
| const pwSource = config.singleUserPassword | ||
| ? 'provided' | ||
| : (process.stdin.isTTY ? 'will prompt at startup' : 'missing — login disabled'); | ||
| console.log(` Single-user: ${config.singleUserName} (password: ${pwSource})`); |
There was a problem hiding this comment.
printConfig() reports single-user password status whenever config.singleUser is true, but the seeding/prompting behavior is actually gated on config.idp (and also skipped for the root-pod case singleUserName === '/'). As-is, --single-user with --no-idp can print “password: missing — login disabled” even though IdP login is irrelevant. Consider only printing this password status when config.singleUser && config.idp, and optionally reflecting the root-pod skip case to avoid misleading output.
| const pwSource = config.singleUserPassword | |
| ? 'provided' | |
| : (process.stdin.isTTY ? 'will prompt at startup' : 'missing — login disabled'); | |
| console.log(` Single-user: ${config.singleUserName} (password: ${pwSource})`); | |
| let singleUserDetails = `${config.singleUserName}`; | |
| if (config.idp) { | |
| if (config.singleUserName === '/') { | |
| singleUserDetails += ' (root pod; password not used)'; | |
| } else { | |
| const pwSource = config.singleUserPassword | |
| ? 'provided' | |
| : (process.stdin.isTTY ? 'will prompt at startup' : 'missing — login disabled'); | |
| singleUserDetails += ` (password: ${pwSource})`; | |
| } | |
| } | |
| console.log(` Single-user: ${singleUserDetails}`); |
| /** | ||
| * Config / env-var parsing tests. | ||
| * | ||
| * Regression coverage for the env-coercion fix in #324: only known |
There was a problem hiding this comment.
The header comment says this is a regression for “#324”, but this PR/issue context is #323. If #324 isn’t a real/related tracker item in this repo, consider updating the reference to avoid confusion for future maintainers.
| * Regression coverage for the env-coercion fix in #324: only known | |
| * Regression coverage for the env-coercion fix in #323: only known |
- src/server.js: compute the seeded WebID against the profile file that actually exists on disk. Legacy pods (created by older JSS with /profile/card, no extension) keep their /profile/card#me WebID; fresh and modern pods get /profile/card.jsonld#me. Without this, seeding a legacy pod produced a credential bound to a document the user didn't actually have. - src/config.js: printConfig only shows password-source state when --idp is also enabled, and renders a separate "root pod; password not seeded" line for the singleUserName === '/' case. Avoids the misleading "missing — login disabled" output when login isn't governed by the IDP password at all. - test/idp.test.js: new regression — pre-seed a legacy /profile/card pod, start the server, assert the seeded account's WebID ends with /profile/card#me (not the .jsonld variant). - test/config.test.js: header comment now references #323 (the issue), not #324 (the PR).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 1. `--single-user-password <pw>` CLI flag | ||
| 2. `JSS_SINGLE_USER_PASSWORD` env var | ||
| 3. Interactive no-echo prompt (TTY only) | ||
| 4. None — server starts and warns; the pod is created but isn't loggable until a password is set later via `jss passwd <user>` |
There was a problem hiding this comment.
Docs say that if no initial password is provided the pod “isn't loggable until a password is set later via jss passwd <user>”. However jss passwd currently exits with “User not found” when the account doesn’t exist (and in this branch no account is seeded), so this guidance is incorrect. Suggest updating this step to instruct operators to restart with --single-user-password / JSS_SINGLE_USER_PASSWORD or run on a TTY to be prompted (or otherwise create the account via an explicit account-create flow, if added later).
| 4. None — server starts and warns; the pod is created but isn't loggable until a password is set later via `jss passwd <user>` | |
| 4. None — server starts and warns; the pod is created, but no account is seeded, so it isn't loggable until you restart with `--single-user-password <pw>`, set `JSS_SINGLE_USER_PASSWORD`, or run on a TTY to be prompted |
| // Save/restore DATA_ROOT and stdin.isTTY around each test so we don't | ||
| // leak global state into other tests in the same `node --test` run. | ||
| // For isTTY we capture the *property descriptor* so we can correctly | ||
| // restore an inherited (prototype) accessor — Object.defineProperty | ||
| // would otherwise leave a shadowing own-property behind. |
There was a problem hiding this comment.
The comment says “Save/restore DATA_ROOT and stdin.isTTY around each test”, but the hooks used are before()/after() (run once for the whole describe), not beforeEach()/afterEach(). Please either adjust the wording to “around this suite” or switch to per-test hooks if that’s the intent.
Summary
jss start --single-user --idpproduced a pod with no way to log in: registration is disabled in single-user mode (correctly),jss passwderrors withUser not found, and the WebID has no Schnorr/TLS bootstrap. Adds a one-time IDP account seed during single-user pod creation.Behaviour
--single-user-password <pw>(also viaJSS_SINGLE_USER_PASSWORDenv var) — seed the account non-interactively. Ideal for systemd / containers / CI.--idp— the password is ignored when the IDP is off.singleUserName === '/'); skipped here, can be a follow-up.Test plan
singleUserPasswordprovided → account seeded,authenticate(name, pw)returns the account.--idpoff → no.idpdirectory created.JSS_SINGLE_USER_PASSWORD=hunter2 jss start --single-user --idp …→POST /idp/credentials {username:"me", password:"hunter2"}returns a bearer token with the rightwebidclaim. Wrong password returns 401.Files
bin/jss.js—--single-user-passwordoption, threaded intocreateServer.src/config.js— default +JSS_SINGLE_USER_PASSWORDenv mapping.src/server.js—seedSingleUserIdpAccount+promptPasswordOncehelpers, called from the existing single-useronReadyhook after pod creation.test/idp.test.js— four targeted tests covering each branch above.Fixes #323.