From f8cc70a1b4bbe310d3090d1cc0e05c8af0672bee Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 19 Jun 2026 03:07:12 +0000 Subject: [PATCH] auto: prevent concurrent unlink account lockout Co-authored-by: Ummara Ali Syeda --- server/routes/connections.js | 66 ++++++++++------ server/tests/connections-unit.test.js | 108 ++++++++++++++++++++++++++ 2 files changed, 151 insertions(+), 23 deletions(-) create mode 100644 server/tests/connections-unit.test.js diff --git a/server/routes/connections.js b/server/routes/connections.js index 471f702..589a62c 100644 --- a/server/routes/connections.js +++ b/server/routes/connections.js @@ -67,36 +67,56 @@ router.post('/password', authenticateToken, async (req, res) => { }) router.delete('/:provider', authenticateToken, async (req, res) => { + const columnMap = { + password: 'password_hash', + google: 'google_id', + github: 'github_id', + orcid: 'orcid_id', + } + try { const { provider } = req.params if (!PROVIDERS.includes(provider)) { return res.status(400).json({ error: 'Invalid provider' }) } - const user = await getAuthUser(req.user.userId) - if (!user) return res.status(404).json({ error: 'User not found' }) - - if (!canUnlinkProvider(user, provider)) { - return res.status(400).json({ - error: 'Add another sign-in method before removing this one', - code: 'LAST_SIGN_IN_METHOD', - }) + const client = await pool.connect() + try { + await client.query('BEGIN') + + const result = await client.query( + `SELECT ${AUTH_FIELDS} FROM users WHERE id = $1 FOR UPDATE`, + [req.user.userId] + ) + const user = result.rows[0] || null + + if (!user) { + await client.query('ROLLBACK') + return res.status(404).json({ error: 'User not found' }) + } + + if (!canUnlinkProvider(user, provider)) { + await client.query('ROLLBACK') + return res.status(400).json({ + error: 'Add another sign-in method before removing this one', + code: 'LAST_SIGN_IN_METHOD', + }) + } + + const updatedResult = await client.query( + `UPDATE users SET ${columnMap[provider]} = NULL WHERE id = $1 RETURNING ${AUTH_FIELDS}`, + [req.user.userId] + ) + + await client.query('COMMIT') + + res.json(buildConnectionsResponse(updatedResult.rows[0])) + } catch (err) { + await client.query('ROLLBACK') + throw err + } finally { + client.release() } - - const columnMap = { - password: 'password_hash', - google: 'google_id', - github: 'github_id', - orcid: 'orcid_id', - } - - await pool.query( - `UPDATE users SET ${columnMap[provider]} = NULL WHERE id = $1`, - [req.user.userId] - ) - - const updated = await getAuthUser(req.user.userId) - res.json(buildConnectionsResponse(updated)) } catch (err) { console.error('DELETE /users/me/connections/:provider error:', err) res.status(500).json({ error: 'Internal server error' }) diff --git a/server/tests/connections-unit.test.js b/server/tests/connections-unit.test.js new file mode 100644 index 0000000..49d3ce9 --- /dev/null +++ b/server/tests/connections-unit.test.js @@ -0,0 +1,108 @@ +jest.mock('../db', () => ({ + query: jest.fn(), + connect: jest.fn(), + end: jest.fn(), +})) + +process.env.JWT_SECRET = process.env.JWT_SECRET || 'test-secret' + +const request = require('supertest') +const app = require('../index') +const pool = require('../db') +const { signToken } = require('../lib/session') + +function mockClient() { + return { + query: jest.fn(), + release: jest.fn(), + } +} + +describe('DELETE /users/me/connections/:provider transaction guard', () => { + beforeEach(() => { + pool.query.mockReset() + pool.connect.mockReset() + }) + + it('rechecks unlink eligibility while holding a row lock', async () => { + const client = mockClient() + pool.connect.mockResolvedValue(client) + client.query + .mockResolvedValueOnce({ rows: [] }) // BEGIN + .mockResolvedValueOnce({ + rows: [{ + id: 'user-1', + email: 'user@example.com', + password_hash: 'hash', + google_id: null, + github_id: null, + orcid_id: null, + email_verified: true, + }], + }) + .mockResolvedValueOnce({ rows: [] }) // ROLLBACK + + const token = signToken({ userId: 'user-1', username: 'user' }) + const res = await request(app) + .delete('/users/me/connections/password') + .set('Cookie', `token=${token}`) + + expect(res.status).toBe(400) + expect(res.body.code).toBe('LAST_SIGN_IN_METHOD') + expect(client.query).toHaveBeenCalledWith( + expect.stringContaining('FOR UPDATE'), + ['user-1'] + ) + expect(client.query).not.toHaveBeenCalledWith( + expect.stringContaining('UPDATE users SET password_hash = NULL'), + expect.any(Array) + ) + expect(client.query).toHaveBeenCalledWith('ROLLBACK') + expect(client.release).toHaveBeenCalled() + }) + + it('commits after unlinking when another sign-in method remains', async () => { + const client = mockClient() + pool.connect.mockResolvedValue(client) + client.query + .mockResolvedValueOnce({ rows: [] }) // BEGIN + .mockResolvedValueOnce({ + rows: [{ + id: 'user-1', + email: 'user@example.com', + password_hash: 'hash', + google_id: 'google-1', + github_id: null, + orcid_id: null, + email_verified: true, + }], + }) + .mockResolvedValueOnce({ + rows: [{ + id: 'user-1', + email: 'user@example.com', + password_hash: null, + google_id: 'google-1', + github_id: null, + orcid_id: null, + email_verified: true, + }], + }) + .mockResolvedValueOnce({ rows: [] }) // COMMIT + + const token = signToken({ userId: 'user-1', username: 'user' }) + const res = await request(app) + .delete('/users/me/connections/password') + .set('Cookie', `token=${token}`) + + expect(res.status).toBe(200) + expect(res.body.sign_in_method_count).toBe(1) + expect(res.body.connections.find(c => c.provider === 'password').linked).toBe(false) + expect(client.query).toHaveBeenCalledWith( + expect.stringContaining('UPDATE users SET password_hash = NULL'), + ['user-1'] + ) + expect(client.query).toHaveBeenCalledWith('COMMIT') + expect(client.release).toHaveBeenCalled() + }) +})