Skip to content

Commit 5e73711

Browse files
yash-pouranikyash-pouranik
authored
Merge pull request #162 from geturbackend/revert-160-feature/public-api-login-lockout
Revert "Add Redis-backed per-email login lockout after 5 failures"
2 parents 37557c8 + 3ebca2b commit 5e73711

6 files changed

Lines changed: 4 additions & 259 deletions

File tree

‎apps/public-api/src/__tests__/userAuth.email.test.js‎

Lines changed: 1 addition & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,6 @@ jest.mock('@urbackend/common', () => {
8585
getRefreshSession: jest.fn(),
8686
persistRefreshSession: jest.fn().mockResolvedValue(undefined),
8787
revokeSessionChain: jest.fn().mockResolvedValue(undefined),
88-
checkLockout: jest.fn().mockResolvedValue({ locked: false, retryAfterSeconds: 0 }),
89-
recordFailedAttempt: jest.fn().mockResolvedValue({ locked: false, retryAfterSeconds: 0, attempts: 1 }),
90-
clearLockout: jest.fn().mockResolvedValue(undefined),
9188
};
9289
});
9390

@@ -102,7 +99,7 @@ jest.mock('../utils/refreshToken', () => ({
10299

103100
const bcrypt = require('bcryptjs');
104101
const jwt = require('jsonwebtoken');
105-
const { redis, authEmailQueue, __mockModel: mockModel, checkLockout, recordFailedAttempt, clearLockout } = require('@urbackend/common');
102+
const { redis, authEmailQueue, __mockModel: mockModel } = require('@urbackend/common');
106103
const { issueAuthTokens } = require('../utils/refreshToken');
107104
const controller = require('../controllers/userAuth.controller');
108105

@@ -241,8 +238,6 @@ describe('Email Authentication Flow', () => {
241238
type: 'verification'
242239
}));
243240

244-
expect(clearLockout).toHaveBeenCalledWith('project_1', 'new@user.com');
245-
246241
expect(issueAuthTokens).toHaveBeenCalled();
247242
expect(res.status).toHaveBeenCalledWith(201);
248243
expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
@@ -338,9 +333,7 @@ describe('Email Authentication Flow', () => {
338333

339334
await controller.login(req, res);
340335

341-
expect(checkLockout).toHaveBeenCalledWith('project_1', 'test@user.com');
342336
expect(bcrypt.compare).toHaveBeenCalledWith('password123', 'hashed_pw');
343-
expect(clearLockout).toHaveBeenCalledWith('project_1', 'test@user.com');
344337
expect(issueAuthTokens).toHaveBeenCalled();
345338
expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
346339
token: 'signed_access_token'
@@ -363,62 +356,8 @@ describe('Email Authentication Flow', () => {
363356

364357
await controller.login(req, res);
365358

366-
expect(recordFailedAttempt).toHaveBeenCalledWith('project_1', 'test@user.com');
367359
expect(res.status).toHaveBeenCalledWith(400);
368360
});
369-
370-
test('returns 423 when account is already locked', async () => {
371-
const req = makeReq({
372-
body: { email: 'test@user.com', password: 'password123' }
373-
});
374-
const res = makeRes();
375-
376-
checkLockout.mockResolvedValueOnce({ locked: true, retryAfterSeconds: 900 });
377-
378-
await controller.login(req, res);
379-
380-
expect(bcrypt.compare).not.toHaveBeenCalled();
381-
expect(recordFailedAttempt).not.toHaveBeenCalled();
382-
expect(res.status).toHaveBeenCalledWith(423);
383-
});
384-
385-
test('returns 423 when failures reach lockout threshold', async () => {
386-
const req = makeReq({
387-
body: { email: 'test@user.com', password: 'wrongpassword' }
388-
});
389-
const res = makeRes();
390-
391-
mockModel.findOne.mockReturnValueOnce({
392-
select: jest.fn().mockResolvedValueOnce({
393-
_id: 'user_123',
394-
password: 'hashed_pw'
395-
})
396-
});
397-
bcrypt.compare.mockResolvedValueOnce(false);
398-
recordFailedAttempt.mockResolvedValueOnce({ locked: true, retryAfterSeconds: 900, attempts: 5 });
399-
400-
await controller.login(req, res);
401-
402-
expect(res.status).toHaveBeenCalledWith(423);
403-
});
404-
405-
test('user-not-found branch records attempt and returns 423 when lockout is reached', async () => {
406-
const req = makeReq({
407-
body: { email: 'missing@user.com', password: 'wrongpassword' }
408-
});
409-
const res = makeRes();
410-
411-
checkLockout.mockResolvedValueOnce({ locked: false, retryAfterSeconds: 0 });
412-
mockModel.findOne.mockReturnValueOnce({
413-
select: jest.fn().mockResolvedValueOnce(null)
414-
});
415-
recordFailedAttempt.mockResolvedValueOnce({ locked: true, retryAfterSeconds: 900, attempts: 5 });
416-
417-
await controller.login(req, res);
418-
419-
expect(recordFailedAttempt).toHaveBeenCalledWith('project_1', 'missing@user.com');
420-
expect(res.status).toHaveBeenCalledWith(423);
421-
});
422361
});
423362

424363
describe('requestPasswordReset', () => {
@@ -457,7 +396,6 @@ describe('Email Authentication Flow', () => {
457396
{ email: 'reset@user.com' },
458397
{ $set: { password: 'hashed_pw' } }
459398
);
460-
expect(clearLockout).toHaveBeenCalledWith('project_1', 'reset@user.com');
461399
expect(redis.del).toHaveBeenCalled();
462400
expect(res.json).toHaveBeenCalled();
463401
});

‎apps/public-api/src/__tests__/userAuth.refresh.test.js‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,6 @@ jest.mock('@urbackend/common', () => {
7373
getConnection: jest.fn().mockResolvedValue({}),
7474
getCompiledModel: jest.fn(() => mockModel),
7575
__mockModel: mockModel,
76-
checkLockout: jest.fn().mockResolvedValue({ locked: false, retryAfterSeconds: 0 }),
77-
recordFailedAttempt: jest.fn().mockResolvedValue({ locked: false, retryAfterSeconds: 0, attempts: 1 }),
78-
clearLockout: jest.fn().mockResolvedValue(undefined),
7976
// session manager exports
8077
getRefreshSession: jest.fn(),
8178
persistRefreshSession: jest.fn().mockResolvedValue(undefined),

‎apps/public-api/src/__tests__/userAuth.social.test.js‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,6 @@ jest.mock('@urbackend/common', () => {
6969
sanitize: jest.fn((value) => value),
7070
getConnection: jest.fn().mockResolvedValue({}),
7171
getCompiledModel: jest.fn(() => mockUsersModel),
72-
checkLockout: jest.fn().mockResolvedValue({ locked: false, retryAfterSeconds: 0 }),
73-
recordFailedAttempt: jest.fn().mockResolvedValue({ locked: false, retryAfterSeconds: 0, attempts: 1 }),
74-
clearLockout: jest.fn().mockResolvedValue(undefined),
7572
decrypt: jest.fn((encrypted) => {
7673
if (!encrypted?.encrypted) return null;
7774
if (encrypted.encrypted === 'github') return 'github_secret';

‎apps/public-api/src/controllers/userAuth.controller.js‎

Lines changed: 3 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ const crypto = require('crypto');
66
const {redis} = require('@urbackend/common');
77
const {Project} = require('@urbackend/common');
88
const { authEmailQueue } = require('@urbackend/common');
9-
const { checkLockout, recordFailedAttempt, clearLockout } = require('@urbackend/common');
10-
const { AppError } = require('@urbackend/common');
119
const { getRefreshSession, persistRefreshSession, revokeSessionChain } = require('@urbackend/common');
1210
const { loginSchema, userSignupSchema, resetPasswordSchema, onlyEmailSchema, verifyOtpSchema, changePasswordSchema, sanitize } = require('@urbackend/common');
1311
const { getConnection } = require('@urbackend/common');
@@ -1006,13 +1004,6 @@ module.exports.signup = async (req, res) => {
10061004
// Model.create handles validation and default values
10071005
const result = await Model.create(newUserPayload);
10081006

1009-
try {
1010-
// Fail-open: if Redis is unavailable, do not block successful signup.
1011-
await clearLockout(String(project._id), normalizedEmail);
1012-
} catch (lockErr) {
1013-
console.error('[login-lockout] clearLockout failed after signup:', lockErr?.message || lockErr);
1014-
}
1015-
10161007
await redis.set(`project:${project._id}:otp:verification:${normalizedEmail}`, otp, 'EX', 300);
10171008
await setPublicOtpCooldown(project._id, normalizedEmail, 'verification');
10181009

@@ -1055,31 +1046,11 @@ module.exports.signup = async (req, res) => {
10551046
* Issues access and refresh tokens upon successful authentication.
10561047
* @route POST /api/userAuth/login
10571048
*/
1058-
module.exports.login = async (req, res, next) => {
1059-
const sendAuthError = (statusCode, message) => {
1060-
if (typeof next === 'function') {
1061-
return next(new AppError(statusCode, message));
1062-
}
1063-
return res.status(statusCode).json({ error: message });
1064-
};
1065-
1049+
module.exports.login = async (req, res) => {
10661050
try {
10671051
const project = req.project;
10681052
const { email, password } = loginSchema.parse(req.body);
10691053
const normalizedEmail = email.toLowerCase().trim();
1070-
const projectId = String(project._id);
1071-
1072-
let lockStatus = { locked: false, retryAfterSeconds: 0 };
1073-
try {
1074-
// Fail-open: if Redis is unavailable, do not block login attempts.
1075-
lockStatus = await checkLockout(projectId, normalizedEmail);
1076-
} catch (lockErr) {
1077-
console.error('[login-lockout] checkLockout failed:', lockErr?.message || lockErr);
1078-
}
1079-
1080-
if (lockStatus.locked) {
1081-
return sendAuthError(423, `Account temporarily locked. Try again in ${lockStatus.retryAfterSeconds} seconds.`);
1082-
}
10831054

10841055
const usersColConfig = project.collections.find(c => c.name === 'users');
10851056
if (!usersColConfig) return res.status(404).json({ error: "Auth collection not found" });
@@ -1089,43 +1060,10 @@ module.exports.login = async (req, res, next) => {
10891060

10901061
const user = await Model.findOne({ email: normalizedEmail }).select('+password');
10911062

1092-
if (!user) {
1093-
let failedStatus = { locked: false, retryAfterSeconds: 0, attempts: 0 };
1094-
try {
1095-
// Fail-open: if Redis is unavailable, do not block login on attempt tracking.
1096-
failedStatus = await recordFailedAttempt(projectId, normalizedEmail);
1097-
} catch (attemptErr) {
1098-
console.error('[login-lockout] recordFailedAttempt failed (user missing):', attemptErr?.message || attemptErr);
1099-
}
1100-
1101-
if (failedStatus.locked) {
1102-
return sendAuthError(423, `Account temporarily locked. Try again in ${failedStatus.retryAfterSeconds} seconds.`);
1103-
}
1104-
return sendAuthError(400, 'Invalid email or password');
1105-
}
1063+
if (!user) return res.status(400).json({ error: "Invalid email or password" });
11061064

11071065
const validPass = await bcrypt.compare(password, user.password);
1108-
if (!validPass) {
1109-
let failedStatus = { locked: false, retryAfterSeconds: 0, attempts: 0 };
1110-
try {
1111-
// Fail-open: if Redis is unavailable, do not block login on attempt tracking.
1112-
failedStatus = await recordFailedAttempt(projectId, normalizedEmail);
1113-
} catch (attemptErr) {
1114-
console.error('[login-lockout] recordFailedAttempt failed (invalid password):', attemptErr?.message || attemptErr);
1115-
}
1116-
1117-
if (failedStatus.locked) {
1118-
return sendAuthError(423, `Account temporarily locked. Try again in ${failedStatus.retryAfterSeconds} seconds.`);
1119-
}
1120-
return sendAuthError(400, 'Invalid email or password');
1121-
}
1122-
1123-
try {
1124-
// Fail-open: if Redis is unavailable, do not block successful login.
1125-
await clearLockout(projectId, normalizedEmail);
1126-
} catch (clearErr) {
1127-
console.error('[login-lockout] clearLockout failed:', clearErr?.message || clearErr);
1128-
}
1066+
if (!validPass) return res.status(400).json({ error: "Invalid email or password" });
11291067

11301068
const issuedTokens = await issueAuthTokens({
11311069
project,
@@ -1420,7 +1358,6 @@ module.exports.resetPasswordUser = async (req, res) => {
14201358
try {
14211359
const project = req.project;
14221360
const { email, otp, newPassword } = resetPasswordSchema.parse(req.body);
1423-
const normalizedEmail = email.toLowerCase().trim();
14241361

14251362
const redisKey = `project:${project._id}:otp:reset:${email}`;
14261363
const storedOtp = await redis.get(redisKey);
@@ -1442,13 +1379,6 @@ module.exports.resetPasswordUser = async (req, res) => {
14421379

14431380
if (result.matchedCount === 0) return res.status(404).json({ error: "User not found" });
14441381

1445-
try {
1446-
// Fail-open: if Redis is unavailable, do not block password recovery success.
1447-
await clearLockout(String(project._id), normalizedEmail);
1448-
} catch (lockErr) {
1449-
console.error('[login-lockout] clearLockout failed after password reset:', lockErr?.message || lockErr);
1450-
}
1451-
14521382
await redis.del(redisKey);
14531383
res.json({ message: "Password updated successfully" });
14541384

‎packages/common/src/index.js‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,6 @@ const { validateData, validateUpdateData } = require("./utils/validateData");
9494
const sessionManager = require("./utils/session.manager");
9595
const planLimits = require("./utils/planLimits");
9696
const AppError = require("./utils/AppError");
97-
const { checkLockout, recordFailedAttempt, clearLockout } = require("./utils/loginLockout");
9897

9998
module.exports = {
10099
connectDB,
@@ -180,7 +179,4 @@ module.exports = {
180179
getPresignedUploadUrl,
181180
verifyUploadedFile,
182181
ApiAnalytics,
183-
checkLockout,
184-
recordFailedAttempt,
185-
clearLockout,
186182
};

‎packages/common/src/utils/loginLockout.js‎

Lines changed: 0 additions & 113 deletions
This file was deleted.

0 commit comments

Comments
 (0)