Skip to content

Commit

Permalink
Merge pull request #527 from YunhwanJeong/519-block-failed-login-atte…
Browse files Browse the repository at this point in the history
…mpts

Block user accounts if an incorrect password was entered 5 times
  • Loading branch information
evert authored Oct 23, 2024
2 parents 31d06ad + 3a52f2f commit d65e2ad
Show file tree
Hide file tree
Showing 10 changed files with 219 additions and 41 deletions.
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, // Logged at the time the account is locked
loginFailedAccountLocked = 14, // Logged when a login attempt is made on a locked account
}

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'],
]);
6 changes: 3 additions & 3 deletions src/login/controller/authorization-challenge.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { Controller } from '@curveball/controller';
import { UnsupportedMediaType } from '@curveball/http-errors';
import { Context } from '@curveball/kernel';
import { AuthorizationChallengeRequest } from '../../api-types.js';
import { getAppClientFromBasicAuth } from '../../app-client/service.js';
import { UnauthorizedClient } from '../../oauth2/errors.js';
import { UnsupportedMediaType } from '@curveball/http-errors';
import { AuthorizationChallengeRequest } from '../../api-types.js';
import * as loginService from '../service.js';

/**
Expand Down Expand Up @@ -44,7 +44,7 @@ class AuthorizationChallengeController extends Controller {
const request = ctx.request.body;

const session = await loginService.getSession(client, request);
const code = await loginService.challenge(client, session, request);
const code = await loginService.challenge(client, session, request, ctx);

ctx.response.body = {
authorization_code: code.code,
Expand Down
18 changes: 9 additions & 9 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 } 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,12 +56,8 @@ class LoginController extends Controller {
throw err;
}
}
const user = await principalService.findByIdentity(identity) as User;

if (!await services.user.validatePassword(user, ctx.request.body.password)) {
log(EventType.loginFailed, ctx.ip(), user.id);
return this.redirectToLogin(ctx, '', 'Incorrect username or password');
}
const user = (await principalService.findByIdentity(identity)) as User;

if (!user.active) {
log(EventType.loginFailedInactive, ctx.ip(), user.id, ctx.request.headers.get('User-Agent'));
Expand All @@ -76,6 +72,11 @@ class LoginController extends Controller {
return;
}

const { success, errorMessage } = await services.user.validateUserCredentials(user, ctx.request.body.password, ctx);
if (!success && errorMessage) {
return this.redirectToLogin(ctx, '', errorMessage);
}

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

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

}

redirectToLogin(ctx: Context<any>, msg: string, error: string) {
Expand Down
70 changes: 70 additions & 0 deletions src/login/login-activity/service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { NotFound } from '@curveball/http-errors';
import { UserLoginActivityRecord } from 'knex/types/tables.js';
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;
}

async function getLoginActivity(user: User): Promise<UserLoginActivityRecord> {
const loginActivity = await db<UserLoginActivityRecord>('user_login_activity')
.where({ principal_id: user.id })
.first();

if (!loginActivity) throw new NotFound(`Login activity record for user with ID ${user.id} was not found.`);

return loginActivity;
}

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) => {

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;
}
21 changes: 12 additions & 9 deletions src/login/service.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { AppClient, Principal, PrincipalIdentity, User } from '../types.js';
import { getSessionStore } from '../session-store.js';
import { InvalidGrant, OAuth2Error } from '../oauth2/errors.js';
import * as services from '../services.js';
import { BadRequest, NotFound } from '@curveball/http-errors';
import { Context } from '@curveball/kernel';
import { AuthorizationChallengeRequest } from '../api-types.js';
import { InvalidGrant, OAuth2Error } from '../oauth2/errors.js';
import { OAuth2Code } from '../oauth2/types.js';
import * as services from '../services.js';
import { getSessionStore } from '../session-store.js';
import { AppClient, Principal, PrincipalIdentity, User } from '../types.js';

type ChallengeRequest = AuthorizationChallengeRequest;

Expand Down Expand Up @@ -131,7 +132,7 @@ async function deleteSession(session: LoginSession) {
* If more credentials are needed or if any information is incorrect, an error
* will be thrown.
*/
export async function challenge(client: AppClient, session: LoginSession, parameters: ChallengeRequest): Promise<OAuth2Code> {
export async function challenge(client: AppClient, session: LoginSession, parameters: ChallengeRequest, ctx: Context): Promise<OAuth2Code> {

try {
if (!session.principalId) {
Expand All @@ -148,6 +149,7 @@ export async function challenge(client: AppClient, session: LoginSession, parame
session,
parameters.username!,
parameters.password!,
ctx,
);

}
Expand Down Expand Up @@ -183,7 +185,7 @@ export async function challenge(client: AppClient, session: LoginSession, parame

}

async function challengeUsernamePassword(session: LoginSession, username: string, password: string): Promise<User> {
async function challengeUsernamePassword(session: LoginSession, username: string, password: string, ctx: Context): Promise<User> {

const principalService = new services.principal.PrincipalService('insecure');
let user: Principal;
Expand Down Expand Up @@ -228,10 +230,11 @@ async function challengeUsernamePassword(session: LoginSession, username: string
);
}

if (!await services.user.validatePassword(user, password)) {
const { success, errorMessage } = await services.user.validateUserCredentials(user, password, ctx);
if (!success && errorMessage) {
throw new A12nLoginChallengeError(
session,
'Incorrect username or password',
errorMessage,
'username-password',
true,
);
Expand All @@ -242,7 +245,6 @@ async function challengeUsernamePassword(session: LoginSession, username: string
session.dirty = true;

if (!user.active) {

throw new A12nLoginChallengeError(
session,
'This account is not active. Please contact support',
Expand All @@ -258,6 +260,7 @@ async function challengeUsernamePassword(session: LoginSession, username: string
true
);
}

return user;
}

Expand Down
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');
}
28 changes: 12 additions & 16 deletions src/oauth2/controller/token.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
import Controller from '@curveball/controller';
import { Context } from '@curveball/core';
import { NotFound } from '@curveball/http-errors';
import {
getAppClientFromBasicAuth,
getAppClientFromBody,
} from '../../app-client/service.js';
import log from '../../log/service.js';
import { EventType } from '../../log/types.js';
import * as principalIdentityService from '../../principal-identity/service.js';
import { PrincipalService } from '../../principal/service.js';
import { AppClient, User } from '../../types.js';
import * as userAppPermissions from '../../user-app-permissions/service.js';
import * as userService from '../../user/service.js';
import { User, AppClient } from '../../types.js';
import { InvalidGrant, InvalidRequest, UnsupportedGrantType } from '../errors.js';
import * as oauth2Service from '../service.js';
import {
getAppClientFromBasicAuth,
getAppClientFromBody,
} from '../../app-client/service.js';
import * as userAppPermissions from '../../user-app-permissions/service.js';
import * as principalIdentityService from '../../principal-identity/service.js';

class TokenController extends Controller {

Expand Down Expand Up @@ -126,15 +126,6 @@ class TokenController extends Controller {
throw new InvalidGrant('Unknown username or password');
}

if (!await userService.validatePassword(user, ctx.request.body.password)) {
await log(
EventType.loginFailed,
ctx.ip(),
user.id
);
throw new InvalidGrant('Unknown username or password');
}

if (!user.active) {
await log(
EventType.loginFailedInactive,
Expand All @@ -145,6 +136,11 @@ class TokenController extends Controller {
throw new InvalidGrant('User Inactive');
}

const { success, errorMessage } = await userService.validateUserCredentials(user, ctx.request.body.password, ctx);
if (!success && errorMessage) {
throw new InvalidGrant(errorMessage);
}

await log(
EventType.loginSuccess,
ctx.ip(),
Expand Down
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';
Loading

0 comments on commit d65e2ad

Please sign in to comment.