Emit a more specific error for 'too many failed login attempts'. (#621)

* Emit a more specific error for 'too many failed login attempts'.

This small refactoring allows us to emit
`too_many_failed_login_attempts` from the `authorization_challenge`
system. Before, it emitted `invalid_username_or_password` which is
confusing.

* Update changelog

* lint

changelog.md

@@ -10,6 +10,9 @@ Changelog
 * Docker image could not be built on arm64 due to a transative dependency using
   an old version of libsodium.
 * Reduced docker image from 366MB to 216MB with no loss of functionality.
+* This small refactoring allows us to emit `too_many_failed_login_attempts`
+  from the `authorization_challenge` system. Before, it emitted
+  `invalid_username_or_password` which is confusing.
 
 
 0.29.0 (2025-02-07)

src/login/challenge/password.ts

@@ -2,6 +2,7 @@ import { AbstractLoginChallenge } from './abstract.js';
 import { AuthorizationChallengeRequest } from '../types.js';
 import { A12nLoginChallengeError } from '../error.js';
 import * as services from '../../services.js';
+import { IncorrectPassword, TooManyLoginAttemptsError } from '../../user/error.js';
 
 type PasswordParameters = {
   password: string;
@@ -37,12 +38,22 @@ export class LoginChallengePassword extends AbstractLoginChallenge<PasswordParam
    */
   async checkResponse(parameters: PasswordParameters): Promise<boolean> {
 
-    const { success, errorMessage } = await services.user.validateUserCredentials(this.principal, parameters.password, this.log);
-    if (!success && errorMessage) {
-      throw new A12nLoginChallengeError(
-        errorMessage,
-        'username_or_password_invalid',
-      );
+    try {
+      await services.user.validateUserCredentials(this.principal, parameters.password, this.log);
+    } catch (err) {
+      if (err instanceof IncorrectPassword) {
+        throw new A12nLoginChallengeError(
+          err.message,
+          'username_or_password_invalid',
+        );
+      } else if (err instanceof TooManyLoginAttemptsError) {
+        throw new A12nLoginChallengeError(
+          err.message,
+          'too_many_failed_login_attempts',
+        );
+      } else {
+        throw err;
+      }
     }
 
     return true;

src/login/controller/login.ts

@@ -11,6 +11,7 @@ import * as services from '../../services.js';
 import { PrincipalIdentity, User } from '../../types.js';
 import { loginForm } from '../formats/html.js';
 import { isValidRedirect, setLoginSession } from '../utilities.js';
+import { IncorrectPassword, TooManyLoginAttemptsError } from '../../user/error.js';
 
 /**
  * The Login controller renders the basic login form
@@ -80,9 +81,14 @@ class LoginController extends Controller {
       return;
     }
 
-    const { success, errorMessage } = await services.user.validateUserCredentials(user, ctx.request.body.password, log);
-    if (!success && errorMessage) {
-      return this.redirectToLogin(ctx, '', errorMessage);
+    try {
+      await services.user.validateUserCredentials(user, ctx.request.body.password, log);
+    } catch (err) {
+      if (err instanceof IncorrectPassword || err instanceof TooManyLoginAttemptsError) {
+        return this.redirectToLogin(ctx, '', err.message);
+      } else {
+        throw err;
+      }
     }
 
     setLoginSession(ctx, user);

src/login/error.ts

@@ -25,7 +25,9 @@ type ChallengeErrorCode =
   // The email address used to log in was not verified.
   | 'email_not_verified'
   // Email verification expired and the user must enter a OTP to continue
-  | 'email_verification_code_invalid';
+  | 'email_verification_code_invalid'
+  // User tried to log in with an incorrect password one too many times.
+  | 'too_many_failed_login_attempts';
 
 type ExtraParams = {
   censored_email?: string;
@@ -68,3 +70,10 @@ export class A12nLoginChallengeError extends OAuth2Error {
   }
 
 }
+
+/**
+ * Errors thrown from validateUserCredentials.
+ */
+export class TooManyLoginAttemptsError extends Error {}
+export class IncorrectPassword extends Error {}
+

src/oauth2/controller/token.ts

@@ -13,6 +13,7 @@ import * as userAppPermissions from '../../user-app-permissions/service.js';
 import * as userService from '../../user/service.js';
 import { InvalidGrant, InvalidRequest, UnsupportedGrantType } from '../errors.js';
 import * as oauth2Service from '../service.js';
+import { IncorrectPassword, TooManyLoginAttemptsError } from '../../user/error.js';
 
 class TokenController extends Controller {
 
@@ -135,9 +136,14 @@ class TokenController extends Controller {
       throw new InvalidGrant('User Inactive');
     }
 
-    const { success, errorMessage } = await userService.validateUserCredentials(user, ctx.request.body.password, log);
-    if (!success && errorMessage) {
-      throw new InvalidGrant(errorMessage);
+    try {
+      await userService.validateUserCredentials(user, ctx.request.body.password, log);
+    } catch (err) {
+      if (err instanceof IncorrectPassword || err instanceof TooManyLoginAttemptsError) {
+        throw new InvalidGrant(err.message);
+      } else {
+        throw err;
+      }
     }
 
     await log('login-success');

src/user/error.ts

@@ -0,0 +1,5 @@
+/**
+ * Errors thrown from validateUserCredentials.
+ */
+export class TooManyLoginAttemptsError extends Error {}
+export class IncorrectPassword extends Error {}

src/user/service.ts

@@ -5,6 +5,7 @@ import { UserEventLogger } from '../log/types.js';
 import * as loginActivityService from '../login/login-activity/service.js';
 import { getSetting } from '../server-settings.js';
 import { User } from '../types.js';
+import { IncorrectPassword, TooManyLoginAttemptsError } from './error.js';
 
 export async function createPassword(user: User, password: string): Promise<void> {
 
@@ -71,15 +72,15 @@ function assertValidPassword(password: string) {
 
 }
 
-type AuthenticationResult = {
-  success: boolean;
-  errorMessage?: string;
-}
 
 /**
  * Validate the user password and handle login attempts.
+ *
+ * Throws one of the following errors:
+ *   * TooManyLoginAttemptsError
+ *   * IncorrectPassword
  */
-export async function validateUserCredentials(user: User, password: string, log: UserEventLogger): Promise<AuthenticationResult> {
+export async function validateUserCredentials(user: User, password: string, log: UserEventLogger): Promise<boolean> {
 
   const admin = getSetting('smtp.emailFrom') || 'an administrator';
 
@@ -88,35 +89,25 @@ export async function validateUserCredentials(user: User, password: string, log:
   if (await loginActivityService.isAccountLocked(user)) {
     await loginActivityService.incrementFailedLoginAttempts(user);
     await log('login-failed-account-locked');
-    return {
-      success: false,
-      errorMessage: TOO_MANY_FAILED_ATTEMPTS,
-    };
+    throw new TooManyLoginAttemptsError(TOO_MANY_FAILED_ATTEMPTS);
   }
 
   if (!await validatePassword(user, password)) {
     const incrementedAttempts = await loginActivityService.incrementFailedLoginAttempts(user);
 
     if (loginActivityService.reachedMaxAttempts(incrementedAttempts)) {
       await log('account-locked');
-      return {
-        success: false,
-        errorMessage: TOO_MANY_FAILED_ATTEMPTS,
-      };
+      throw new TooManyLoginAttemptsError(TOO_MANY_FAILED_ATTEMPTS);
     }
 
     await log('password-check-failed');
-    return {
-      success: false,
-      errorMessage: 'Incorrect username or password',
-    };
+    throw new IncorrectPassword();
+
   }
 
   await log('password-check-success');
   await loginActivityService.resetFailedLoginAttempts(user);
 
-  return {
-    success: true,
-  };
+  return true;
 }