Skip to content

Commit 7d1ab03

Browse files
authored
EQMS-1402: limit password login max attempts (#10184)
Signed-off-by: Alexey Zinoviev <alexey.zinoviev@xored.com>
1 parent 90e83a2 commit 7d1ab03

File tree

9 files changed

+2133
-2029
lines changed

9 files changed

+2133
-2029
lines changed

.vscode/launch.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,7 @@
201201
"FRONT_URL": "http://huly.local:8087",
202202
"STATS_URL": "http://huly.local:4900",
203203
"MAIL_URL": "",
204+
"MAX_FAILED_LOGIN_ATTEMPTS": "5",
204205
// "DB_NS": "account-2",
205206
// "WS_LIVENESS_DAYS": "1",
206207
// "WORKSPACE_LIMIT_PER_USER": "1",

common/config/rush/pnpm-lock.yaml

Lines changed: 2024 additions & 2024 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

server/account/src/__tests__/operations.test.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1338,7 +1338,8 @@ describe('account operations', () => {
13381338

13391339
const mockDb = {
13401340
account: {
1341-
findOne: jest.fn()
1341+
findOne: jest.fn(),
1342+
update: jest.fn()
13421343
},
13431344
person: {
13441345
findOne: jest.fn()
@@ -1568,7 +1569,8 @@ describe('account operations', () => {
15681569

15691570
const mockDb = {
15701571
account: {
1571-
findOne: jest.fn()
1572+
findOne: jest.fn(),
1573+
update: jest.fn()
15721574
},
15731575
person: {
15741576
findOne: jest.fn(),
@@ -2178,7 +2180,8 @@ describe('account operations', () => {
21782180

21792181
const mockDb = {
21802182
account: {
2181-
findOne: jest.fn()
2183+
findOne: jest.fn(),
2184+
update: jest.fn()
21822185
},
21832186
person: {
21842187
findOne: jest.fn()

server/account/src/__tests__/postgres.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,7 @@ describe('AccountPostgresDbCollection', () => {
336336
a.locale,
337337
a.automatic,
338338
a.max_workspaces,
339+
a.failed_login_attempts,
339340
p.hash,
340341
p.salt
341342
FROM global_account.account as a

server/account/src/collections/postgres/migrations.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ export function getMigrations (ns: string): [string, string][] {
3838
getV17Migration(ns),
3939
getV18Migration(ns),
4040
getV19Migration(ns),
41-
getV20Migration(ns)
41+
getV20Migration(ns),
42+
getV21Migration(ns)
4243
]
4344
}
4445

@@ -568,3 +569,13 @@ function getV20Migration (ns: string): [string, string] {
568569
`
569570
]
570571
}
572+
573+
function getV21Migration (ns: string): [string, string] {
574+
return [
575+
'account_db_v21_add_failed_login_attempts',
576+
`
577+
ALTER TABLE ${ns}.account
578+
ADD COLUMN IF NOT EXISTS failed_login_attempts SMALLINT DEFAULT 0;
579+
`
580+
]
581+
}

server/account/src/collections/postgres/postgres.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,7 @@ export class AccountPostgresDbCollection
437437
a.locale,
438438
a.automatic,
439439
a.max_workspaces,
440+
a.failed_login_attempts,
440441
p.hash,
441442
p.salt
442443
FROM ${this.getTableName()} as a

server/account/src/operations.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,10 @@ import {
117117
getWorkspacesInfoWithStatusByIds,
118118
doMergePersons,
119119
getWorkspaceJoinInfo,
120-
signUpByGrant
120+
signUpByGrant,
121+
isAccountPasswordLocked,
122+
recordFailedLoginAttempt,
123+
resetFailedLoginAttempts
121124
} from './utils'
122125

123126
// Note: it is IMPORTANT to always destructure params passed here to avoid sending extra params
@@ -152,6 +155,8 @@ export async function loginAsGuest (
152155

153156
/**
154157
* Given an email and password, logs the user in and returns the account information and token.
158+
* If the account has too many failed login attempts, password login is blocked.
159+
* The user must use an alternative method (e.g., OTP) to unlock the account.
155160
*/
156161
export async function login (
157162
ctx: MeasureContext,
@@ -184,15 +189,34 @@ export async function login (
184189
throw new PlatformError(new Status(Severity.ERROR, platform.status.AccountNotFound, {}))
185190
}
186191

192+
// Check if account is locked due to too many failed login attempts
193+
if (isAccountPasswordLocked(existingAccount)) {
194+
ctx.warn('Login attempt on locked account - password login locked', {
195+
email: normalizedEmail,
196+
failedAttempts: existingAccount.failedLoginAttempts
197+
})
198+
throw new PlatformError(
199+
new Status(Severity.ERROR, platform.status.PasswordLoginLocked, { account: normalizedEmail })
200+
)
201+
}
202+
187203
const person = await db.person.findOne({ uuid: emailSocialId.personUuid })
188204
if (person == null) {
189205
throw new PlatformError(new Status(Severity.ERROR, platform.status.InternalServerError, {}))
190206
}
191207

192208
if (!verifyPassword(password, existingAccount.hash, existingAccount.salt)) {
209+
try {
210+
await recordFailedLoginAttempt(db, existingAccount.uuid)
211+
} catch (err) {
212+
ctx.warn('Failed to record failed login attempt', { error: err, account: existingAccount.uuid })
213+
}
193214
throw new PlatformError(new Status(Severity.ERROR, platform.status.AccountNotFound, {}))
194215
}
195216

217+
// Successful login - reset failed attempts counter
218+
await resetFailedLoginAttempts(db, existingAccount.uuid)
219+
196220
const isConfirmed = emailSocialId.verifiedOn != null
197221

198222
const extraToken: Record<string, string> = isAdminEmail(normalizedEmail) ? { admin: 'true' } : {}
@@ -477,6 +501,8 @@ export async function validateOtp (
477501
throw new PlatformError(new Status(Severity.ERROR, platform.status.InternalServerError, {}))
478502
}
479503

504+
await resetFailedLoginAttempts(db, emailSocialId.personUuid as AccountUuid)
505+
480506
const extraToken: Record<string, string> = isAdminEmail(normalizedEmail) ? { admin: 'true' } : {}
481507

482508
return {

server/account/src/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ export interface Account {
6464
hash?: Buffer | null
6565
salt?: Buffer | null
6666
maxWorkspaces?: number
67+
failedLoginAttempts?: number // Number of consecutive failed login attempts
6768
}
6869

6970
// TODO: type data with generic type

server/account/src/utils.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,66 @@ export function verifyPassword (password: string, hash?: Buffer | null, salt?: B
342342
return Buffer.compare(hash as any, hashWithSalt(password, salt) as any) === 0
343343
}
344344

345+
// 0 or negative value means no limit
346+
const maxFailedLoginAttempts =
347+
process.env.MAX_FAILED_LOGIN_ATTEMPTS != null ? parseInt(process.env.MAX_FAILED_LOGIN_ATTEMPTS) : 5
348+
349+
/**
350+
* Check if an account is locked due to too many failed login attempts.
351+
* An account is locked if failedLoginAttempts >= maxFailedLoginAttempts.
352+
* @param account The account to check
353+
* @returns true if account is locked, false otherwise
354+
*/
355+
export function isAccountPasswordLocked (account: Account): boolean {
356+
if (maxFailedLoginAttempts <= 0) {
357+
return false
358+
}
359+
360+
const failedAttempts = account.failedLoginAttempts ?? 0
361+
362+
return failedAttempts >= maxFailedLoginAttempts
363+
}
364+
365+
/**
366+
* Record a failed login attempt for an account.
367+
* Increments the failed attempts counter.
368+
* @param db Database instance
369+
* @param accountUuid Account UUID
370+
*/
371+
export async function recordFailedLoginAttempt (db: AccountDB, accountUuid: AccountUuid): Promise<void> {
372+
const account = await db.account.findOne({ uuid: accountUuid })
373+
if (account == null) {
374+
return
375+
}
376+
377+
const currentAttempts = account.failedLoginAttempts ?? 0
378+
const newAttempts = currentAttempts + 1
379+
380+
await db.account.update(
381+
{ uuid: accountUuid },
382+
{
383+
failedLoginAttempts: newAttempts
384+
}
385+
)
386+
}
387+
388+
/**
389+
* Reset failed login attempts for an account.
390+
* Called when:
391+
* - User successfully logs in with password
392+
* - User successfully authenticates via OTP or other alternative method
393+
* @param db Database instance
394+
* @param accountUuid Account UUID
395+
*/
396+
export async function resetFailedLoginAttempts (db: AccountDB, accountUuid: AccountUuid): Promise<void> {
397+
await db.account.update(
398+
{ uuid: accountUuid },
399+
{
400+
failedLoginAttempts: 0
401+
}
402+
)
403+
}
404+
345405
export function cleanEmail (email: string): string {
346406
return email.toLowerCase().trim()
347407
}

0 commit comments

Comments
 (0)