Skip to content
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

Block user accounts if an incorrect password was entered 5 times #527

Merged
merged 9 commits into from
Oct 23, 2024
20 changes: 19 additions & 1 deletion src/db-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ interface Tables {
server_settings: ServerSettingsRecord;
user_app_permissions: UserAppPermissionsRecord;
user_log: UserLogRecord;
user_login_activity: UserLoginActivityRecord;
user_passwords: UserPasswordsRecord;
user_privileges: UserPrivilegesRecord;
user_totp: UserTotpRecord;
Expand Down Expand Up @@ -158,7 +159,7 @@ export type PrincipalsRecord = {
modified_at: number;

/**
* System are built-in and cannot be deleted
* System principals are built-in and cannot be deleted
*/
system: number;
};
Expand Down Expand Up @@ -212,6 +213,23 @@ export type UserLogRecord = {
user_agent: string | null;
country: string | null;
};
export type UserLoginActivityRecord = {

/**
* Foreign key to the ‘principals’ table, representing the user
*/
principal_id: number;

/**
* Tracks the number of consecutive failed login attempts
*/
failed_login_attempts: number;

/**
* Indicates if the user's account is currently locked due to suspicious activity, such as too many failed login attempts
*/
account_locked: number;
};
export type UserPasswordsRecord = {
user_id: number;
password: string;
Expand Down
5 changes: 5 additions & 0 deletions src/log/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ export enum EventType {

oauth2BadRedirect = 11,
generateAccessToken = 12,

accountLocked = 13,
loginFailedAccountLocked = 14,
evert marked this conversation as resolved.
Show resolved Hide resolved
}

export type LogEntry = {
Expand All @@ -34,4 +37,6 @@ export const eventTypeString = new Map<EventType, string>([
[EventType.loginFailedNotVerified,'login-failed-notverified'],
[EventType.tokenRevoked, 'token-revoked'],
[EventType.oauth2BadRedirect, 'oauth2-badredirect'],
[EventType.accountLocked, 'account-locked'],
[EventType.loginFailedAccountLocked, 'login-failed-account-locked'],
]);
25 changes: 20 additions & 5 deletions src/login/controller/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ import log from '../../log/service.js';
import { EventType } from '../../log/types.js';
import { MFALoginSession } from '../../mfa/types.js';
import * as webAuthnService from '../../mfa/webauthn/service.js';
import { getSetting } from '../../server-settings.js';
import { hasUsers, PrincipalService } from '../../principal/service.js';
import { getSetting, requireSetting } from '../../server-settings.js';
import * as services from '../../services.js';
import { PrincipalIdentity, User } from '../../types.js';
import { isValidRedirect } from '../utilities.js';
import { loginForm } from '../formats/html.js';
import * as services from '../../services.js';
import { isValidRedirect } from '../utilities.js';

class LoginController extends Controller {

Expand Down Expand Up @@ -56,9 +56,23 @@ class LoginController extends Controller {
throw err;
}
}
const user = await principalService.findByIdentity(identity) as User;

const user = (await principalService.findByIdentity(identity)) as User;

if (await services.loginActivity.isAccountLocked(user)) {
await services.loginActivity.incrementFailedLoginAttempts(user);
log(EventType.loginFailedAccountLocked, ctx.ip(), user.id, ctx.request.headers.get('User-Agent'));
return this.redirectToLogin(ctx, '', `Too many failed login attempts, please contact ${requireSetting('smtp.emailFrom')} to unlock your account.`);
}

if (!await services.user.validatePassword(user, ctx.request.body.password)) {
evert marked this conversation as resolved.
Show resolved Hide resolved
const incrementedAttempts = await services.loginActivity.incrementFailedLoginAttempts(user);

if (services.loginActivity.reachedMaxAttempts(incrementedAttempts)) {
log(EventType.accountLocked, ctx.ip(), user.id, ctx.request.headers.get('User-Agent'));
return this.redirectToLogin(ctx, '', `Too many failed login attempts, please contact ${requireSetting('smtp.emailFrom')} to unlock your account.`);
}

log(EventType.loginFailed, ctx.ip(), user.id);
return this.redirectToLogin(ctx, '', 'Incorrect username or password');
}
Expand All @@ -76,6 +90,8 @@ class LoginController extends Controller {
return;
}

await services.loginActivity.resetFailedLoginAttempts(user);

ctx.session = {
user: user,
};
Expand All @@ -88,7 +104,6 @@ class LoginController extends Controller {
}

ctx.response.redirect(303, getSetting('login.defaultRedirect'));

}

redirectToLogin(ctx: Context<any>, msg: string, error: string) {
Expand Down
65 changes: 65 additions & 0 deletions src/login/login-activity/service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { UserLoginActivityRecord } from 'knex/types/tables.js';
evert marked this conversation as resolved.
Show resolved Hide resolved
import db from '../../database.js';
import { User } from '../../types.js';

const MAX_FAILED_ATTEMPTS = 5;

export function reachedMaxAttempts(attempts: number) {
return attempts === MAX_FAILED_ATTEMPTS;
evert marked this conversation as resolved.
Show resolved Hide resolved
}

async function getLoginActivity(user: User): Promise<UserLoginActivityRecord | undefined> {
evert marked this conversation as resolved.
Show resolved Hide resolved
return await db<UserLoginActivityRecord>('user_login_activity')
.where({ principal_id: user.id })
.first();
}

async function ensureUserLoginActivityRecord(user: User): Promise<void> {
await db('user_login_activity')
.insert({
principal_id: user.id,
failed_login_attempts: 0,
account_locked: 0,
})
.onConflict('principal_id')
.ignore();
}

export async function incrementFailedLoginAttempts(user: User): Promise<number> {
await ensureUserLoginActivityRecord(user);

return await db.transaction(async (trx) => {
YunhwanJeong marked this conversation as resolved.
Show resolved Hide resolved

const loginActivity = await trx('user_login_activity')
.where({ principal_id: user.id })
.forUpdate()
.first();

const newAttempts = loginActivity!.failed_login_attempts + 1;

await trx('user_login_activity')
.where({ principal_id: user.id })
.update({
failed_login_attempts: newAttempts,
account_locked: newAttempts >= MAX_FAILED_ATTEMPTS ? 1 : 0,
});

return newAttempts;
});
}

export async function resetFailedLoginAttempts(user: User): Promise<void> {
await db('user_login_activity')
.where({ principal_id: user.id })
.update({
failed_login_attempts: 0,
account_locked: 0,
});
}

export async function isAccountLocked(user: User): Promise<boolean> {
await ensureUserLoginActivityRecord(user);

const loginActivity = await getLoginActivity(user);
return !!loginActivity?.account_locked;
}
34 changes: 34 additions & 0 deletions src/migrations/20240908210700_block_failed_login_attempts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { Knex } from 'knex';

export async function up(knex: Knex): Promise<void> {
await knex.schema.createTable('user_login_activity', (table) => {
table
.integer('principal_id')
.unsigned()
.notNullable()
.primary()
.comment('Foreign key to the ‘principals’ table, representing the user');
table
.foreign('principal_id')
.references('id')
.inTable('principals')
.onDelete('CASCADE');
table
.integer('failed_login_attempts')
.unsigned()
.defaultTo(0)
.notNullable()
.comment('Tracks the number of consecutive failed login attempts');
table
.boolean('account_locked')
.defaultTo(false)
.notNullable()
.comment(
"Indicates if the user's account is currently locked due to suspicious activity, such as too many failed login attempts"
);
});
}

export async function down(knex: Knex): Promise<void> {
await knex.schema.dropTable('user_login_activity');
}
5 changes: 3 additions & 2 deletions src/services.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
export * as appClient from './app-client/service.js';
export * as log from './log/service.js';
export * as loginActivity from './login/login-activity/service.js';
export * as mfaTotp from './mfa/totp/service.js';
export * as oauth2 from './oauth2/service.js';
export * as principal from './principal/service.js';
export * as principalIdentity from './principal-identity/service.js';
export * as principal from './principal/service.js';
export * as privilege from './privilege/service.js';
export * as resetPassword from './reset-password/service.js';
export * as user from './user/service.js';
export * as userAuthFactor from './user-auth-factor/service.js';
export * as user from './user/service.js';
export * as verificationToken from './verification-token/service.js';