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, // 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';
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;
}

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) => {
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;
}
35 changes: 19 additions & 16 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,21 +230,11 @@ async function challengeUsernamePassword(session: LoginSession, username: string
);
}

if (!await services.user.validatePassword(user, password)) {
throw new A12nLoginChallengeError(
session,
'Incorrect username or password',
'username-password',
true,
);
}

session.principalId = user.id;
session.passwordValid = true;
session.dirty = true;
evert marked this conversation as resolved.
Show resolved Hide resolved

if (!user.active) {

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

const { success, errorMessage } = await services.user.validateUserCredentials(user, password, ctx);
if (!success && errorMessage) {
throw new A12nLoginChallengeError(
session,
errorMessage,
'username-password',
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