review: consolidate validatePassword and loginActivity logic into a s…

…ingle userService method, replacing existing implementations

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,

src/login/controller/login.ts

@@ -7,7 +7,7 @@ import { EventType } from '../../log/types.js';
 import { MFALoginSession } from '../../mfa/types.js';
 import * as webAuthnService from '../../mfa/webauthn/service.js';
 import { hasUsers, PrincipalService } from '../../principal/service.js';
-import { getSetting, requireSetting } from '../../server-settings.js';
+import { getSetting } from '../../server-settings.js';
 import * as services from '../../services.js';
 import { PrincipalIdentity, User } from '../../types.js';
 import { loginForm } from '../formats/html.js';
@@ -59,24 +59,6 @@ class LoginController extends Controller {
 
     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)) {
-      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');
-    }
-
     if (!user.active) {
       log(EventType.loginFailedInactive, ctx.ip(), user.id, ctx.request.headers.get('User-Agent'));
       return this.redirectToLogin(ctx, '', 'This account is inactive. Please contact Admin');
@@ -90,7 +72,10 @@ class LoginController extends Controller {
       return;
     }
 
-    await services.loginActivity.resetFailedLoginAttempts(user);
+    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,

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<OAuth2Code> {
+export async function challenge(client: AppClient, session: LoginSession, parameters: ChallengeRequest, ctx: Context): Promise<OAuth2Code> {
 
   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<User> {
+async function challengeUsernamePassword(session: LoginSession, username: string, password: string, ctx: Context): Promise<User> {
 
   const principalService = new services.principal.PrincipalService('insecure');
   let user: Principal;
@@ -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;
 
   if (!user.active) {
-
     throw new A12nLoginChallengeError(
       session,
       'This account is not active. Please contact support',
@@ -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;
 }
 

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(),

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<void> {
 
@@ -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<AuthenticationResult> {
+  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,
+  };
+}
+