Skip to content

Commit

Permalink
fix: return updated failed login attempts (#4474)
Browse files Browse the repository at this point in the history
* fix: return updated failed login attempts

* fix: testing coverage

* fix: update naming

* fix: testing tweak
  • Loading branch information
ColinBuyck authored Nov 14, 2024
1 parent eeafe62 commit fc05ac1
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 7 deletions.
5 changes: 3 additions & 2 deletions api/src/passports/mfa.strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { defaultValidationPipeOptions } from '../utilities/default-validation-pi
import { Login } from '../dtos/auth/login.dto';
import { MfaType } from '../enums/mfa/mfa-type-enum';
import {
isUserLockedOut,
checkUserLockout,
singleUseCodePresent,
singleUseCodeInvalid,
} from '../utilities/passport-validator-utilities';
Expand Down Expand Up @@ -57,7 +57,8 @@ export class MfaStrategy extends PassportStrategy(Strategy, 'mfa') {
`user ${dto.email} attempted to log in, but does not exist`,
);
}
isUserLockedOut(
//check if user is locked out and update failed login attempts count
rawUser.failedLoginAttemptsCount = checkUserLockout(
rawUser.lastLoginAt,
rawUser.failedLoginAttemptsCount,
Number(process.env.AUTH_LOCK_LOGIN_AFTER_FAILED_ATTEMPTS),
Expand Down
4 changes: 2 additions & 2 deletions api/src/passports/single-use-code.strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { defaultValidationPipeOptions } from '../utilities/default-validation-pi
import { LoginViaSingleUseCode } from '../dtos/auth/login-single-use-code.dto';
import { OrderByEnum } from '../enums/shared/order-by-enum';
import {
isUserLockedOut,
checkUserLockout,
singleUseCodePresent,
singleUseCodeInvalid,
} from '../utilities/passport-validator-utilities';
Expand Down Expand Up @@ -86,7 +86,7 @@ export class SingleUseCodeStrategy extends PassportStrategy(
);
}

isUserLockedOut(
rawUser.failedLoginAttemptsCount = checkUserLockout(
rawUser.lastLoginAt,
rawUser.failedLoginAttemptsCount,
Number(process.env.AUTH_LOCK_LOGIN_AFTER_FAILED_ATTEMPTS),
Expand Down
7 changes: 4 additions & 3 deletions api/src/utilities/passport-validator-utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ import { HttpException, HttpStatus } from '@nestjs/common';
* @param failedLoginAttemptsCount the number of times the user failed to log in (stored in db)
* @param maxAttempts the maximum number of attempts before user is considered locked out (env variable)
*
* @returns throws error if user is already locked out
* @returns updated value of failed login attempts
*/
export function isUserLockedOut(
export function checkUserLockout(
lastLoginAt: Date,
failedLoginAttemptsCount: number,
maxAttempts: number,
cooldown: number,
): void {
): number {
if (lastLoginAt && failedLoginAttemptsCount >= maxAttempts) {
// if a user has logged in, but has since gone over their max failed login attempts
const retryAfter = new Date(lastLoginAt.getTime() + cooldown);
Expand All @@ -33,6 +33,7 @@ export function isUserLockedOut(
);
}
}
return failedLoginAttemptsCount;
}

/**
Expand Down
47 changes: 47 additions & 0 deletions api/test/unit/passports/passport-validator-utilities.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { checkUserLockout } from '../../../src/utilities/passport-validator-utilities';

describe('Testing checkUserLockout', () => {
it('should return without erroring and set failedLoginAttemptsCount to be 0 if lockout expired', () => {
const val = {
lastLoginAt: new Date('01/01/2024'),
failedLoginAttemptsCount: 5,
};
const updatedFailedLoginCount = checkUserLockout(
val.lastLoginAt,
val.failedLoginAttemptsCount,
5,
10,
);
expect(updatedFailedLoginCount).toEqual(0);
});

it('should return without erroring and leave failed login count unchanged if user is and was not locked out', () => {
const val = {
lastLoginAt: new Date('01/01/2024'),
failedLoginAttemptsCount: 2,
};
const updatedFailedLoginCount = checkUserLockout(
val.lastLoginAt,
val.failedLoginAttemptsCount,
5,
10,
);
expect(updatedFailedLoginCount).toEqual(2);
});

it('should error if user is still in lockout period', () => {
const val = {
lastLoginAt: new Date(),
failedLoginAttemptsCount: 5,
};
expect(
async () =>
await checkUserLockout(
val.lastLoginAt,
val.failedLoginAttemptsCount,
5,
10,
),
).rejects.toThrowError(`Failed login attempts exceeded.`);
});
});

0 comments on commit fc05ac1

Please sign in to comment.