diff --git a/src/db-types.ts b/src/db-types.ts index cf715115..35ad7715 100644 --- a/src/db-types.ts +++ b/src/db-types.ts @@ -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; @@ -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; }; @@ -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; diff --git a/src/log/types.ts b/src/log/types.ts index a1cf1b20..d60c192c 100644 --- a/src/log/types.ts +++ b/src/log/types.ts @@ -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 = { @@ -34,4 +37,6 @@ export const eventTypeString = new Map([ [EventType.loginFailedNotVerified,'login-failed-notverified'], [EventType.tokenRevoked, 'token-revoked'], [EventType.oauth2BadRedirect, 'oauth2-badredirect'], + [EventType.accountLocked, 'account-locked'], + [EventType.loginFailedAccountLocked, 'login-failed-account-locked'], ]); diff --git a/src/login/controller/authorization-challenge.ts b/src/login/controller/authorization-challenge.ts index 56156ddf..aa74fccf 100644 --- a/src/login/controller/authorization-challenge.ts +++ b/src/login/controller/authorization-challenge.ts @@ -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'; /** @@ -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, diff --git a/src/login/controller/login.ts b/src/login/controller/login.ts index cfa479b8..919290c1 100644 --- a/src/login/controller/login.ts +++ b/src/login/controller/login.ts @@ -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 { @@ -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')); @@ -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, }; @@ -88,7 +89,6 @@ class LoginController extends Controller { } ctx.response.redirect(303, getSetting('login.defaultRedirect')); - } redirectToLogin(ctx: Context, msg: string, error: string) { diff --git a/src/login/login-activity/service.ts b/src/login/login-activity/service.ts new file mode 100644 index 00000000..f10c7176 --- /dev/null +++ b/src/login/login-activity/service.ts @@ -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 { + const loginActivity = await db('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 { + 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 { + 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 { + 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 { + await ensureUserLoginActivityRecord(user); + + const loginActivity = await getLoginActivity(user); + return !!loginActivity.account_locked; +} diff --git a/src/login/service.ts b/src/login/service.ts index 29c9421c..c9caa457 100644 --- a/src/login/service.ts +++ b/src/login/service.ts @@ -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; @@ -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 { +export async function challenge(client: AppClient, session: LoginSession, parameters: ChallengeRequest, ctx: Context): Promise { try { if (!session.principalId) { @@ -148,6 +149,7 @@ export async function challenge(client: AppClient, session: LoginSession, parame session, parameters.username!, parameters.password!, + ctx, ); } @@ -183,7 +185,7 @@ export async function challenge(client: AppClient, session: LoginSession, parame } -async function challengeUsernamePassword(session: LoginSession, username: string, password: string): Promise { +async function challengeUsernamePassword(session: LoginSession, username: string, password: string, ctx: Context): Promise { const principalService = new services.principal.PrincipalService('insecure'); let user: Principal; @@ -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, ); @@ -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', @@ -258,6 +260,7 @@ async function challengeUsernamePassword(session: LoginSession, username: string true ); } + return user; } diff --git a/src/migrations/20240908210700_block_failed_login_attempts.ts b/src/migrations/20240908210700_block_failed_login_attempts.ts new file mode 100644 index 00000000..c3e4eb5a --- /dev/null +++ b/src/migrations/20240908210700_block_failed_login_attempts.ts @@ -0,0 +1,34 @@ +import { Knex } from 'knex'; + +export async function up(knex: Knex): Promise { + 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 { + await knex.schema.dropTable('user_login_activity'); +} diff --git a/src/oauth2/controller/token.ts b/src/oauth2/controller/token.ts index 81454e82..857f903a 100644 --- a/src/oauth2/controller/token.ts +++ b/src/oauth2/controller/token.ts @@ -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 { @@ -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, @@ -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(), diff --git a/src/services.ts b/src/services.ts index c403a4ae..e13a8229 100644 --- a/src/services.ts +++ b/src/services.ts @@ -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'; diff --git a/src/user/service.ts b/src/user/service.ts index 9933aba4..dfba8b8a 100644 --- a/src/user/service.ts +++ b/src/user/service.ts @@ -1,7 +1,12 @@ +import { Context } from '@curveball/core'; +import { UnprocessableContent } from '@curveball/http-errors'; import * as bcrypt from 'bcrypt'; import db from '../database.js'; +import log from '../log/service.js'; +import { EventType } from '../log/types.js'; +import * as loginActivityService from '../login/login-activity/service.js'; +import { requireSetting } from '../server-settings.js'; import { User } from '../types.js'; -import { UnprocessableContent } from '@curveball/http-errors'; export async function createPassword(user: User, password: string): Promise { @@ -67,3 +72,49 @@ function assertValidPassword(password: string) { } } + +type AuthenticationResult = { + success: boolean; + errorMessage?: string; +} + +/** + * Validate the user password and handle login attempts. + */ +export async function validateUserCredentials(user: User, password: string, ctx: Context): Promise { + const TOO_MANY_FAILED_ATTEMPTS = `Too many failed login attempts, please contact ${requireSetting('smtp.emailFrom')} to unlock your account.`; + + if (await loginActivityService.isAccountLocked(user)) { + await loginActivityService.incrementFailedLoginAttempts(user); + log(EventType.loginFailedAccountLocked, ctx.ip(), user.id, ctx.request.headers.get('User-Agent')); + return { + success: false, + errorMessage: TOO_MANY_FAILED_ATTEMPTS, + }; + } + + if (!await validatePassword(user, password)) { + const incrementedAttempts = await loginActivityService.incrementFailedLoginAttempts(user); + + if (loginActivityService.reachedMaxAttempts(incrementedAttempts)) { + log(EventType.accountLocked, ctx.ip(), user.id, ctx.request.headers.get('User-Agent')); + return { + success: false, + errorMessage: TOO_MANY_FAILED_ATTEMPTS, + }; + } + + log(EventType.loginFailed, ctx.ip(), user.id); + return { + success: false, + errorMessage: 'Incorrect username or password', + }; + } + + await loginActivityService.resetFailedLoginAttempts(user); + + return { + success: true, + }; +} +