feat: add reset-password CLI command#526
Open
TheThomasY wants to merge 4 commits intositeboon:mainfrom
Open
Conversation
- Add tests/reset-password.test.js using node:test runner - Covers RESET-03 (validation), RESET-04 (DB update), RESET-05 (JWT), RESET-06 (output) - In-memory SQLite with token_version column defines post-Plan-02 schema contract - All 9 tests pass against inline implementations at Wave 0
- Add token_version INTEGER DEFAULT 0 to init.sql users table - Add runMigrations() block to ALTER TABLE for existing installs - Update getUserById SELECT to include token_version - Update getFirstUser SELECT to include token_version - Add userDb.updatePassword(userId, hash) that atomically increments token_version
- generateToken now embeds tokenVersion: user.token_version in JWT payload - authenticateToken rejects tokens where decoded.tokenVersion != DB token_version - authenticateWebSocket also verifies token version (closes anti-pattern gap) - Platform mode (IS_PLATFORM) paths are unchanged
- Add resetPassword() async function with hidden password prompts - Add case 'reset-password': to main switch in main() - Import better-sqlite3 and bcrypt dynamically (avoids module side effects) - Validate password length (>=6) and confirmation match before DB write - Hash password with bcrypt.hash (await) before synchronous DB update - Increment token_version atomically with password_hash update - Handle missing user (No user account found) and missing DB gracefully - Run token_version migration inline if column missing - Update showHelp() to list reset-password in Commands section
TheThomasY
force-pushed
the
feature/cli-reset-password
branch
from
c4a71a9 to
cabed8d
Compare
Contributor
📝 WalkthroughWalkthroughThe pull request adds a complete password reset workflow consisting of: a new CLI command (reset-password) that interactively prompts for and validates a new password, database schema updates to track token versions per user, authentication middleware enhancements to invalidate sessions via token version comparison, and comprehensive test coverage validating the entire flow. Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI
participant Database as DB
participant Auth as Auth System
User->>CLI: run reset-password
CLI->>CLI: open SQLite connection
CLI->>Database: create token_version column if missing
Database-->>CLI: migration complete
CLI->>Database: fetch active user
Database-->>CLI: user data returned
CLI->>User: prompt for new password
User->>CLI: enter password
CLI->>CLI: validate password length & confirmation
CLI->>CLI: hash password with bcrypt
CLI->>Database: updatePassword(userId, passwordHash)
Database->>Database: update password_hash & increment token_version
Database-->>CLI: update confirmed
CLI->>User: [OK] Password reset successful
rect rgba(100, 150, 200, 0.5)
Note over User,Auth: Subsequent API Request with Old Token
User->>Auth: request with old JWT (tokenVersion: 0)
Auth->>Database: fetch current user token_version (e.g., 1)
Database-->>Auth: current version: 1
Auth->>Auth: compare token version (0) vs DB (1): mismatch
Auth->>User: 401 Session expired. Please log in again.
end
rect rgba(100, 150, 200, 0.5)
Note over User,Auth: New Login Request
User->>Auth: login with credentials
Auth->>Database: validate credentials & fetch user
Database-->>Auth: user + current token_version: 1
Auth->>Auth: generateToken with embedded tokenVersion: 1
Auth->>User: new JWT returned
end
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/cli.js`:
- Around line 268-295: The code assumes the users table exists and throws when
querying it; before running the PRAGMA or SELECT, check for the existence of the
users table (e.g. query sqlite_master for name='users') and if it does not
exist, close the Database (created via new Database(dbPath)) and print the
friendly message then exit; alternatively, catch the specific "no such table:
users" error around the PRAGMA / user SELECT and treat it the same as a missing
user by closing db, logging the `${c.error('[ERROR]')} No user account found.
Start the server and complete setup first.` message and calling process.exit(1),
ensuring references to getDatabasePath, Database, db.prepare(...), token_version
and the users SELECT/`user` variable are preserved.
In `@server/middleware/auth.js`:
- Around line 77-80: The token payload in middleware/auth.js assumes
user.token_version exists, but the register flow (server/routes/auth.js calling
userDb.createUser) returns only {id, username}, causing tokens without a
tokenVersion; fix by ensuring newly created users include a token_version (e.g.,
set token_version: 0) in userDb.createUser’s return value or by making the token
payload resilient (in middleware/auth.js set tokenVersion to user.token_version
?? 0) so existing registration tokens include a usable version; update either
userDb.createUser or the token creation code (references: userDb.createUser,
register flow in server/routes/auth.js, middleware/auth.js token creation)
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f5d12607-7d49-4b10-9dc0-ba3dc434e436
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
server/cli.jsserver/database/db.jsserver/database/init.sqlserver/middleware/auth.jstests/reset-password.test.js
Comment on lines
+268
to
+295
| // Open DB directly — avoids side effects of importing server/database/db.js | ||
| const dbPath = getDatabasePath(); | ||
| let db; | ||
| try { | ||
| db = new Database(dbPath); | ||
| } catch (err) { | ||
| console.error(`${c.error('[ERROR]')} Could not open database at: ${dbPath}`); | ||
| console.error(` ${err.message}`); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| // Ensure token_version column exists (CLI may run before server has run migrations) | ||
| try { | ||
| const columns = db.prepare('PRAGMA table_info(users)').all().map(col => col.name); | ||
| if (!columns.includes('token_version')) { | ||
| db.exec('ALTER TABLE users ADD COLUMN token_version INTEGER DEFAULT 0'); | ||
| } | ||
| } catch (err) { | ||
| // Non-fatal: migration may fail if column already added concurrently | ||
| } | ||
|
|
||
| // Get the single user | ||
| const user = db.prepare('SELECT id, username FROM users WHERE is_active = 1 LIMIT 1').get(); | ||
| if (!user) { | ||
| db.close(); | ||
| console.error(`${c.error('[ERROR]')} No user account found. Start the server and complete setup first.`); | ||
| process.exit(1); | ||
| } |
Contributor
There was a problem hiding this comment.
Handle an uninitialized database before querying users.
This path still assumes the users table exists after the column check. On a fresh install or empty DB, Line 290 throws no such table: users, so the command falls through to the generic top-level error instead of the intended friendly “complete setup first” path.
Suggested guard
// Open DB directly — avoids side effects of importing server/database/db.js
const dbPath = getDatabasePath();
+ if (!fs.existsSync(dbPath)) {
+ console.error(`${c.error('[ERROR]')} No initialized database found. Start the server and complete setup first.`);
+ process.exit(1);
+ }
+
let db;
try {
db = new Database(dbPath);
@@
try {
const columns = db.prepare('PRAGMA table_info(users)').all().map(col => col.name);
+ if (columns.length === 0) {
+ db.close();
+ console.error(`${c.error('[ERROR]')} No initialized database found. Start the server and complete setup first.`);
+ process.exit(1);
+ }
if (!columns.includes('token_version')) {
db.exec('ALTER TABLE users ADD COLUMN token_version INTEGER DEFAULT 0');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Open DB directly — avoids side effects of importing server/database/db.js | |
| const dbPath = getDatabasePath(); | |
| let db; | |
| try { | |
| db = new Database(dbPath); | |
| } catch (err) { | |
| console.error(`${c.error('[ERROR]')} Could not open database at: ${dbPath}`); | |
| console.error(` ${err.message}`); | |
| process.exit(1); | |
| } | |
| // Ensure token_version column exists (CLI may run before server has run migrations) | |
| try { | |
| const columns = db.prepare('PRAGMA table_info(users)').all().map(col => col.name); | |
| if (!columns.includes('token_version')) { | |
| db.exec('ALTER TABLE users ADD COLUMN token_version INTEGER DEFAULT 0'); | |
| } | |
| } catch (err) { | |
| // Non-fatal: migration may fail if column already added concurrently | |
| } | |
| // Get the single user | |
| const user = db.prepare('SELECT id, username FROM users WHERE is_active = 1 LIMIT 1').get(); | |
| if (!user) { | |
| db.close(); | |
| console.error(`${c.error('[ERROR]')} No user account found. Start the server and complete setup first.`); | |
| process.exit(1); | |
| } | |
| // Open DB directly — avoids side effects of importing server/database/db.js | |
| const dbPath = getDatabasePath(); | |
| if (!fs.existsSync(dbPath)) { | |
| console.error(`${c.error('[ERROR]')} No initialized database found. Start the server and complete setup first.`); | |
| process.exit(1); | |
| } | |
| let db; | |
| try { | |
| db = new Database(dbPath); | |
| } catch (err) { | |
| console.error(`${c.error('[ERROR]')} Could not open database at: ${dbPath}`); | |
| console.error(` ${err.message}`); | |
| process.exit(1); | |
| } | |
| // Ensure token_version column exists (CLI may run before server has run migrations) | |
| try { | |
| const columns = db.prepare('PRAGMA table_info(users)').all().map(col => col.name); | |
| if (columns.length === 0) { | |
| db.close(); | |
| console.error(`${c.error('[ERROR]')} No initialized database found. Start the server and complete setup first.`); | |
| process.exit(1); | |
| } | |
| if (!columns.includes('token_version')) { | |
| db.exec('ALTER TABLE users ADD COLUMN token_version INTEGER DEFAULT 0'); | |
| } | |
| } catch (err) { | |
| // Non-fatal: migration may fail if column already added concurrently | |
| } | |
| // Get the single user | |
| const user = db.prepare('SELECT id, username FROM users WHERE is_active = 1 LIMIT 1').get(); | |
| if (!user) { | |
| db.close(); | |
| console.error(`${c.error('[ERROR]')} No user account found. Start the server and complete setup first.`); | |
| process.exit(1); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/cli.js` around lines 268 - 295, The code assumes the users table
exists and throws when querying it; before running the PRAGMA or SELECT, check
for the existence of the users table (e.g. query sqlite_master for name='users')
and if it does not exist, close the Database (created via new Database(dbPath))
and print the friendly message then exit; alternatively, catch the specific "no
such table: users" error around the PRAGMA / user SELECT and treat it the same
as a missing user by closing db, logging the `${c.error('[ERROR]')} No user
account found. Start the server and complete setup first.` message and calling
process.exit(1), ensuring references to getDatabasePath, Database,
db.prepare(...), token_version and the users SELECT/`user` variable are
preserved.
Comment on lines
+77
to
+80
| { | ||
| userId: user.id, | ||
| username: user.username, | ||
| tokenVersion: user.token_version // embed for invalidation on password reset |
Contributor
There was a problem hiding this comment.
Don't assume every token issuer already has token_version.
In server/routes/auth.js, Lines 50-54, the register flow still signs the object returned by userDb.createUser(), and in server/database/db.js, Lines 144-148, that object is only { id, username }. This payload therefore emits a token without a usable version for newly registered users, and the first authenticated request is rejected by the new version checks.
Minimal compatibility fix
return jwt.sign(
{
userId: user.id,
username: user.username,
- tokenVersion: user.token_version // embed for invalidation on password reset
+ tokenVersion: user.token_version ?? 0
},
JWT_SECRET
// No expiration - token lasts until version incremented
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/middleware/auth.js` around lines 77 - 80, The token payload in
middleware/auth.js assumes user.token_version exists, but the register flow
(server/routes/auth.js calling userDb.createUser) returns only {id, username},
causing tokens without a tokenVersion; fix by ensuring newly created users
include a token_version (e.g., set token_version: 0) in userDb.createUser’s
return value or by making the token payload resilient (in middleware/auth.js set
tokenVersion to user.token_version ?? 0) so existing registration tokens include
a usable version; update either userDb.createUser or the token creation code
(references: userDb.createUser, register flow in server/routes/auth.js,
middleware/auth.js token creation) accordingly.
Comment on lines
+30
to
+121
| // RESET-03: Password validation (pure logic, no DB needed) | ||
| describe('password validation', () => { | ||
| test('rejects passwords shorter than 6 characters', () => { | ||
| const validate = (pw, confirm) => { | ||
| if (pw.length < 6) return { ok: false, error: 'Password must be at least 6 characters.' }; | ||
| if (pw !== confirm) return { ok: false, error: 'Passwords do not match.' }; | ||
| return { ok: true }; | ||
| }; | ||
| assert.equal(validate('abc', 'abc').ok, false); | ||
| assert.match(validate('abc', 'abc').error, /6 characters/); | ||
| }); | ||
|
|
||
| test('rejects mismatched confirmation', () => { | ||
| const validate = (pw, confirm) => { | ||
| if (pw.length < 6) return { ok: false, error: 'Password must be at least 6 characters.' }; | ||
| if (pw !== confirm) return { ok: false, error: 'Passwords do not match.' }; | ||
| return { ok: true }; | ||
| }; | ||
| assert.equal(validate('validpass', 'differentpass').ok, false); | ||
| assert.match(validate('validpass', 'differentpass').error, /do not match/); | ||
| }); | ||
|
|
||
| test('accepts valid matching password', () => { | ||
| const validate = (pw, confirm) => { | ||
| if (pw.length < 6) return { ok: false, error: 'Password must be at least 6 characters.' }; | ||
| if (pw !== confirm) return { ok: false, error: 'Passwords do not match.' }; | ||
| return { ok: true }; | ||
| }; | ||
| assert.equal(validate('validpass', 'validpass').ok, true); | ||
| }); | ||
| }); | ||
|
|
||
| // RESET-04: DB password update | ||
| describe('database password update', () => { | ||
| test('updatePassword changes password_hash in DB', async () => { | ||
| const newHash = await bcrypt.hash('newpassword123', 12); | ||
| // updatePassword method does NOT exist yet — this will fail until Plan 02 adds it to db.js | ||
| // For now, implement inline against the test DB to define the expected contract: | ||
| const stmt = db.prepare('UPDATE users SET password_hash = ?, token_version = token_version + 1 WHERE id = ?'); | ||
| const result = stmt.run(newHash, testUser.id); | ||
| assert.equal(result.changes, 1); | ||
|
|
||
| const updated = db.prepare('SELECT * FROM users WHERE id = ?').get(testUser.id); | ||
| const matches = await bcrypt.compare('newpassword123', updated.password_hash); | ||
| assert.equal(matches, true); | ||
| }); | ||
| }); | ||
|
|
||
| // RESET-05: JWT token invalidation via token_version | ||
| describe('JWT token invalidation', () => { | ||
| test('token_version increments after password update', () => { | ||
| const before_version = db.prepare('SELECT token_version FROM users WHERE id = ?').get(testUser.id).token_version; | ||
| db.prepare('UPDATE users SET token_version = token_version + 1 WHERE id = ?').run(testUser.id); | ||
| const after_version = db.prepare('SELECT token_version FROM users WHERE id = ?').get(testUser.id).token_version; | ||
| assert.equal(after_version, before_version + 1); | ||
| }); | ||
|
|
||
| test('token with old tokenVersion is rejected when version incremented', () => { | ||
| // Simulate: user was on version 0, old token embedded tokenVersion: 0 | ||
| // After reset, DB version is 1 — old token should be detected as stale | ||
| const oldToken = jwt.sign({ userId: testUser.id, username: 'admin', tokenVersion: 0 }, JWT_SECRET); | ||
| const decoded = jwt.verify(oldToken, JWT_SECRET); | ||
| const currentUser = db.prepare('SELECT token_version FROM users WHERE id = ?').get(testUser.id); | ||
| // Middleware check: decoded.tokenVersion !== currentUser.token_version → reject | ||
| assert.notEqual(decoded.tokenVersion, currentUser.token_version); | ||
| }); | ||
|
|
||
| test('token with current tokenVersion is accepted', () => { | ||
| const currentVersion = db.prepare('SELECT token_version FROM users WHERE id = ?').get(testUser.id).token_version; | ||
| const freshToken = jwt.sign({ userId: testUser.id, username: 'admin', tokenVersion: currentVersion }, JWT_SECRET); | ||
| const decoded = jwt.verify(freshToken, JWT_SECRET); | ||
| assert.equal(decoded.tokenVersion, currentVersion); | ||
| }); | ||
| }); | ||
|
|
||
| // RESET-06: Output message format | ||
| describe('output messages', () => { | ||
| test('success message format uses [OK] prefix', () => { | ||
| // Verifies the color helper pattern from cli.js | ||
| const colors = { green: '\x1b[32m', reset: '\x1b[0m' }; | ||
| const ok = (text) => `${colors.green}${text}${colors.reset}`; | ||
| const msg = `${ok('[OK]')} Password updated successfully.`; | ||
| assert.match(msg, /\[OK\]/); | ||
| assert.match(msg, /Password updated successfully/); | ||
| }); | ||
|
|
||
| test('error message format uses [ERROR] prefix', () => { | ||
| const colors = { yellow: '\x1b[33m', reset: '\x1b[0m' }; | ||
| const error = (text) => `${colors.yellow}${text}${colors.reset}`; | ||
| const msg = `${error('[ERROR]')} Passwords do not match.`; | ||
| assert.match(msg, /\[ERROR\]/); | ||
| }); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Please test the real reset flow, not reimplemented stand-ins.
The suite redefines password validation, runs raw SQL instead of userDb.updatePassword(), signs and verifies JWTs without generateToken() or authenticateToken(), and hardcodes CLI color strings instead of asserting actual server/cli.js output. It also shares one mutable DB, so the stale-token assertions depend on prior tests having already incremented token_version. As written, these tests can stay green while the shipped flow is broken—for example, the new generateToken() path currently rejects freshly registered sessions.
Contributor
|
@TheThomasY can you check the conflicts and resolve them ? The latest release introduces a new jwt |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
reset-passwordinteractive CLI command toserver/cli.jstoken_versionto the DB schema to invalidate all existing sessions on resetCloses #369
How to test
Automated tests:
Summary by CodeRabbit
Release Notes
New Features
reset-passwordCLI command for secure password management.Tests