-
Notifications
You must be signed in to change notification settings - Fork 0
fix(auth): equalize login password-KDF work for unknown users #661
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
57016ce
08f7299
6a2d2ac
e8db839
7fdb03e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,8 @@ import { normalizeAttachmentStatusBudgetMs, normalizeAttachmentStatusConcurrency | |
| import { chat as orchestratorChat } from './orchestrator.mjs'; | ||
| import { computeEvm } from '../analytics.js'; // pure math, shared with the client | ||
|
|
||
| const DUMMY_HASH = hashPassword(''); | ||
|
|
||
| const getOrg = (id) => db.prepare('SELECT * FROM orgs WHERE id = ?').get(id); | ||
|
|
||
| // Append-only audit trail. Never throws into the request path. | ||
|
|
@@ -192,9 +194,9 @@ app.post('/api/auth/signup', async (c) => { | |
| app.post('/api/auth/login', async (c) => { | ||
| const { email, password } = await c.req.json().catch(() => ({})); | ||
| const u = db.prepare('SELECT * FROM users WHERE email = ?').get(email || ''); | ||
| // Pass password through only when it is a string — verifyPassword rejects | ||
| // non-strings (objects/arrays) so they never match an empty-password hash. | ||
| if (!u || typeof password !== 'string' || !verifyPassword(password, u.password_hash)) { | ||
| // Always evaluate the password against a hash to mitigate timing attacks. | ||
| const isMatch = verifyPassword(typeof password === 'string' ? password : '', u ? u.password_hash : DUMMY_HASH); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# password_hash의 스키마 제약과 모든 기록 경로를 확인합니다.
rg -n -C 5 '\bpassword_hash\b|CREATE TABLE|ALTER TABLE|hashPassword\s*\(' .Repository: ContextualWisdomLab/scopeweave Length of output: 19665 Information Disclosure (CWE-208) Reachability: External · Exploitability: Difficult
🤖 Prompt for AI Agents |
||
| if (!u || typeof password !== 'string' || !isMatch) { | ||
| return c.json({ error: 'invalid credentials' }, 401); | ||
| } | ||
| return c.json({ token: signToken({ sub: u.id, email: u.email, tv: u.token_version }) }); | ||
|
|
@@ -1352,7 +1354,8 @@ app.post('/api/auth/change-password', requireAuth, async (c) => { | |
| const { oldPassword, newPassword } = await c.req.json().catch(() => ({})); | ||
| if (typeof newPassword !== 'string' || newPassword.length < 8) return c.json({ error: 'new password (min 8) required' }, 400); | ||
| const u = db.prepare('SELECT password_hash FROM users WHERE id = ?').get(uid); | ||
| if (!u || typeof oldPassword !== 'string' || !verifyPassword(oldPassword, u.password_hash)) { | ||
| const isMatch = verifyPassword(typeof oldPassword === 'string' ? oldPassword : '', u ? u.password_hash : DUMMY_HASH); | ||
| if (!u || typeof oldPassword !== 'string' || !isMatch) { | ||
| return c.json({ error: 'current password incorrect' }, 403); | ||
| } | ||
| db.prepare('UPDATE users SET password_hash = ? WHERE id = ?').run(hashPassword(newPassword), uid); | ||
|
|
@@ -1365,7 +1368,8 @@ app.delete('/api/account', requireAuth, async (c) => { | |
| const uid = c.get('user').sub; | ||
| const { password } = await c.req.json().catch(() => ({})); | ||
| const u = db.prepare('SELECT password_hash FROM users WHERE id = ?').get(uid); | ||
| if (!u || typeof password !== 'string' || !verifyPassword(password, u.password_hash)) { | ||
| const isMatch = verifyPassword(typeof password === 'string' ? password : '', u ? u.password_hash : DUMMY_HASH); | ||
| if (!u || typeof password !== 'string' || !isMatch) { | ||
| return c.json({ error: 'password required to delete account' }, 403); | ||
| } | ||
| db.exec('BEGIN'); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| import assert from 'node:assert'; | ||
| import { spawnSync } from 'node:child_process'; | ||
|
|
||
| const SECRET = '0123456789abcdef0123456789abcdef'; | ||
|
|
||
| const script = ` | ||
| import assert from 'node:assert'; | ||
| import { hashPassword, verifyPassword } from './server/auth.mjs'; | ||
|
|
||
| const DUMMY_HASH = hashPassword(''); | ||
|
|
||
| function simulateLogin(email, password, userFound) { | ||
| const passwordHash = userFound ? hashPassword('correct_password') : null; | ||
| const u = userFound ? { password_hash: passwordHash } : null; | ||
|
|
||
| const isMatch = verifyPassword(typeof password === 'string' ? password : '', u ? u.password_hash : DUMMY_HASH); | ||
|
|
||
| if (!u || typeof password !== 'string' || !isMatch) { | ||
|
Comment on lines
+12
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift 실제 인증 엔드포인트를 호출하도록 테스트를 변경하세요.
🤖 Prompt for AI Agents |
||
| return false; | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| try { | ||
| // Test invalid user, returns false but evaluates DUMMY_HASH | ||
| assert.strictEqual(simulateLogin('invalid@test.com', 'pass', false), false); | ||
|
|
||
| // Test valid user, wrong password, returns false | ||
| assert.strictEqual(simulateLogin('valid@test.com', 'wrong', true), false); | ||
|
|
||
| // Test valid user, correct password, returns true | ||
| assert.strictEqual(simulateLogin('valid@test.com', 'correct_password', true), true); | ||
|
|
||
| // Test type mismatch on password, returns false | ||
| assert.strictEqual(simulateLogin('valid@test.com', { invalid: 'type' }, true), false); | ||
|
|
||
| console.log('✓ auth timing integration tests passed'); | ||
| } catch (err) { | ||
| console.error('Test failed:', err); | ||
| process.exit(1); | ||
| } | ||
| `; | ||
|
|
||
| const result = spawnSync(process.execPath, ['--input-type=module', '--eval', script], { | ||
| cwd: process.cwd(), | ||
| env: { ...process.env, SCOPEWEAVE_JWT_SECRET: SECRET }, | ||
| encoding: 'utf8', | ||
| }); | ||
|
|
||
| assert.equal(result.status, 0, result.stderr || result.stdout); | ||
| process.stdout.write(result.stdout); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
보안 학습 문구를 현재 verifier 동작에 맞게 수정하세요.
server/auth.mjs:71-78의verifyPassword는 비문자열 입력에서false를 반환하며TypeError를 발생시키지 않습니다. 또한 현재 변경은 전체 요청의 constant-time 실행을 보장하는 것이 아니라 비밀번호 검증 경로의 비용을 균형화합니다. 두 설명을 실제 동작에 맞게 수정하세요.🤖 Prompt for AI Agents