From 4cb583d76c8a802b543a63e7506aa35a18d4fb7f Mon Sep 17 00:00:00 2001 From: x032205 Date: Tue, 9 Sep 2025 17:22:27 -0400 Subject: [PATCH] Swap to a single service function with a callback --- backend/src/keystore/keystore.ts | 3 +- .../routes/v1/identity-ldap-auth-router.ts | 43 +++---- .../identity-ldap-auth-service.ts | 108 +++++++++--------- .../identity-ldap-auth-types.ts | 10 -- 4 files changed, 68 insertions(+), 96 deletions(-) diff --git a/backend/src/keystore/keystore.ts b/backend/src/keystore/keystore.ts index 45589e529..a6135e0ca 100644 --- a/backend/src/keystore/keystore.ts +++ b/backend/src/keystore/keystore.ts @@ -1,9 +1,10 @@ +import { Cluster, Redis } from "ioredis"; + import { buildRedisFromConfig, TRedisConfigKeys } from "@app/lib/config/redis"; import { pgAdvisoryLockHashText } from "@app/lib/crypto/hashtext"; import { applyJitter } from "@app/lib/dates"; import { delay as delayMs } from "@app/lib/delay"; import { ExecutionResult, Redlock, Settings } from "@app/lib/red-lock"; -import { Redis, Cluster } from "ioredis"; export const PgSqlLock = { BootUpMigration: 2023, diff --git a/backend/src/server/routes/v1/identity-ldap-auth-router.ts b/backend/src/server/routes/v1/identity-ldap-auth-router.ts index af743fb1d..512f253e0 100644 --- a/backend/src/server/routes/v1/identity-ldap-auth-router.ts +++ b/backend/src/server/routes/v1/identity-ldap-auth-router.ts @@ -8,7 +8,7 @@ import { Authenticator } from "@fastify/passport"; import fastifySession from "@fastify/session"; -import { FastifyRequest } from "fastify"; +import { FastifyReply, FastifyRequest } from "fastify"; import { IncomingMessage } from "http"; import LdapStrategy from "passport-ldapauth"; import { z } from "zod"; @@ -136,40 +136,23 @@ export const registerIdentityLdapAuthRouter = async (server: FastifyZodProvider) } }, preValidation: [ - async (req, res) => { - const { lock } = await server.services.identityLdapAuth.checkLdapLockout({ - identityId: req.body.identityId, - username: req.body.username - }); - - try { - const passportRes = await ( + (req, res) => { + const passportAuth = (request: FastifyRequest, reply: FastifyReply) => + ( passport.authenticate("ldapauth", { failWithError: true, session: false }) as any - )(req, res); + )(request, reply); - await server.services.identityLdapAuth.resetLdapLockoutCounter({ - identityId: req.body.identityId, - username: req.body.username - }); - - return passportRes; - } catch (error) { - if ((error as any).status === 401) { - await server.services.identityLdapAuth.incrementLdapLockout({ - identityId: req.body.identityId, - username: req.body.username - }); - - throw new UnauthorizedError({ message: "Invalid credentials" }); - } - - throw error; - } finally { - await lock.release(); - } + const { identityId, username } = req.body; + return server.services.identityLdapAuth.withLdapLockout( + { + identityId, + username + }, + () => passportAuth(req, res) + ); } ], handler: async (req) => { diff --git a/backend/src/services/identity-ldap-auth/identity-ldap-auth-service.ts b/backend/src/services/identity-ldap-auth/identity-ldap-auth-service.ts index c279ca33d..447df4c9d 100644 --- a/backend/src/services/identity-ldap-auth/identity-ldap-auth-service.ts +++ b/backend/src/services/identity-ldap-auth/identity-ldap-auth-service.ts @@ -43,9 +43,7 @@ import { TCheckLdapAuthLockoutDTO, TClearLdapAuthLockoutsDTO, TGetLdapAuthDTO, - TIncrementLdapAuthLockoutDTO, TLoginLdapAuthDTO, - TResetLdapAuthLockoutCounterDTO, TRevokeLdapAuthDTO, TUpdateLdapAuthDTO } from "./identity-ldap-auth-types"; @@ -653,7 +651,10 @@ export const identityLdapAuthServiceFactory = ({ return revokedIdentityLdapAuth; }; - const checkLdapLockout = async ({ identityId, username }: TCheckLdapAuthLockoutDTO) => { + const withLdapLockout = async ( + { identityId, username }: TCheckLdapAuthLockoutDTO, + authFn: () => Promise + ): Promise => { const LOCKOUT_KEY = `lockout:identity:${identityId}:${IdentityAuthMethod.LDAP_AUTH}:${username.trim().toLowerCase()}`; let lock: Awaited>; @@ -667,65 +668,64 @@ export const identityLdapAuthServiceFactory = ({ logger.info( `identity login failed to acquire lock [identityId=${identityId}] [authMethod=${IdentityAuthMethod.LDAP_AUTH}]` ); - throw new RateLimitError({ message: "Rate limit exceeded" }); + throw new RateLimitError({ message: "Failed to acquire lock: rate limit exceeded" }); } - const lockoutRaw = await keyStore.getItem(LOCKOUT_KEY); - - if (lockoutRaw) { - const lockout = JSON.parse(lockoutRaw) as LockoutObject; - - if (lockout.lockedOut) { - await lock.release(); - throw new UnauthorizedError({ - message: "This identity auth method is temporarily locked, please try again later" - }); - } - } - - return { lock }; - }; - - const incrementLdapLockout = async ({ identityId, username }: TIncrementLdapAuthLockoutDTO) => { - const identityLdapAuth = await identityLdapAuthDAL.findOne({ identityId }); - if (!identityLdapAuth) { - throw new UnauthorizedError({ - message: "Invalid credentials" - }); - } - - if (identityLdapAuth.lockoutEnabled) { - const LOCKOUT_KEY = `lockout:identity:${identityId}:${IdentityAuthMethod.LDAP_AUTH}:${username.trim().toLowerCase()}`; - - let lockout: LockoutObject = { - lockedOut: false, - failedAttempts: 0 - }; - + try { const lockoutRaw = await keyStore.getItem(LOCKOUT_KEY); if (lockoutRaw) { - lockout = JSON.parse(lockoutRaw) as LockoutObject; + const lockout = JSON.parse(lockoutRaw) as LockoutObject; + if (lockout.lockedOut) { + throw new UnauthorizedError({ + message: "This identity auth method is temporarily locked, please try again later" + }); + } } - lockout.failedAttempts += 1; - if (lockout.failedAttempts >= identityLdapAuth.lockoutThreshold) { - lockout.lockedOut = true; - } + const result = await authFn(); - await keyStore.setItemWithExpiry( - LOCKOUT_KEY, - lockout.lockedOut ? identityLdapAuth.lockoutDurationSeconds : identityLdapAuth.lockoutCounterResetSeconds, - JSON.stringify(lockout) - ); + await keyStore.deleteItem(LOCKOUT_KEY); + + return result; + } catch (error) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access + if ((error as any).status === 401) { + const identityLdapAuth = await identityLdapAuthDAL.findOne({ identityId }); + if (!identityLdapAuth) { + throw new UnauthorizedError({ message: "Invalid credentials" }); + } + + if (identityLdapAuth.lockoutEnabled) { + let lockout: LockoutObject = { + lockedOut: false, + failedAttempts: 0 + }; + + const lockoutRaw = await keyStore.getItem(LOCKOUT_KEY); + if (lockoutRaw) { + lockout = JSON.parse(lockoutRaw) as LockoutObject; + } + + lockout.failedAttempts += 1; + if (lockout.failedAttempts >= identityLdapAuth.lockoutThreshold) { + lockout.lockedOut = true; + } + + await keyStore.setItemWithExpiry( + LOCKOUT_KEY, + lockout.lockedOut ? identityLdapAuth.lockoutDurationSeconds : identityLdapAuth.lockoutCounterResetSeconds, + JSON.stringify(lockout) + ); + } + + throw new UnauthorizedError({ message: "Invalid credentials" }); + } + throw error; + } finally { + await lock.release(); } }; - const resetLdapLockoutCounter = async ({ identityId, username }: TResetLdapAuthLockoutCounterDTO) => { - await keyStore.deleteItem( - `lockout:identity:${identityId}:${IdentityAuthMethod.LDAP_AUTH}:${username.trim().toLowerCase()}` - ); - }; - const clearLdapAuthLockouts = async ({ identityId, actorId, @@ -765,9 +765,7 @@ export const identityLdapAuthServiceFactory = ({ login, revokeIdentityLdapAuth, getLdapAuth, - checkLdapLockout, - incrementLdapLockout, - resetLdapLockoutCounter, + withLdapLockout, clearLdapAuthLockouts }; }; diff --git a/backend/src/services/identity-ldap-auth/identity-ldap-auth-types.ts b/backend/src/services/identity-ldap-auth/identity-ldap-auth-types.ts index d6a4aba49..a4aea7573 100644 --- a/backend/src/services/identity-ldap-auth/identity-ldap-auth-types.ts +++ b/backend/src/services/identity-ldap-auth/identity-ldap-auth-types.ts @@ -73,13 +73,3 @@ export type TCheckLdapAuthLockoutDTO = { identityId: string; username: string; }; - -export type TIncrementLdapAuthLockoutDTO = { - identityId: string; - username: string; -}; - -export type TResetLdapAuthLockoutCounterDTO = { - identityId: string; - username: string; -};