From a1cf05404ca2664582137e65a6616077bec3ac7e Mon Sep 17 00:00:00 2001 From: Schalli Date: Wed, 22 Jul 2026 14:51:23 +0200 Subject: [PATCH] =?UTF-8?q?feat(auth):=20LDAP=20login=20=E2=80=94=20authen?= =?UTF-8?q?ticate=20imported=20users=20against=20the=20directory?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LDAP-imported users have no local passwordHash, and validateUser only checked the local password, so they could never log in. Now a passwordless user with an ldapDn is authenticated by binding as their OWN DN with the entered password against the tenant's active LDAP config (reusing the ldaps TLS-skip option). Empty passwords are rejected before binding to avoid AD's unauthenticated-bind bypass. Local-password users are unchanged. LdapService.verifyUserCredentials added; LdapModule now exports LdapConfigService; AuthModule imports LdapModule (no circular dep). 8 new specs (bind success/fail, empty-password guard, login via bind, wrong pw, no config, no ldapDn, inactive). API 226 green, tsc clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/api/src/auth/auth.module.ts | 2 + apps/api/src/auth/auth.service.spec.ts | 109 +++++++++++++++++++++++++ apps/api/src/auth/auth.service.ts | 34 +++++++- apps/api/src/ldap/ldap.module.ts | 2 +- apps/api/src/ldap/ldap.service.spec.ts | 41 ++++++++++ apps/api/src/ldap/ldap.service.ts | 35 ++++++++ 6 files changed, 220 insertions(+), 3 deletions(-) create mode 100644 apps/api/src/auth/auth.service.spec.ts diff --git a/apps/api/src/auth/auth.module.ts b/apps/api/src/auth/auth.module.ts index 4917d5d..30bb994 100644 --- a/apps/api/src/auth/auth.module.ts +++ b/apps/api/src/auth/auth.module.ts @@ -2,6 +2,7 @@ import { Module } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { JwtModule } from '@nestjs/jwt'; import { PassportModule } from '@nestjs/passport'; +import { LdapModule } from '../ldap/ldap.module'; import { MailModule } from '../mail/mail.module'; import { AuthController } from './auth.controller'; import { AuthService } from './auth.service'; @@ -19,6 +20,7 @@ import { LocalStrategy } from './strategies/local.strategy'; inject: [ConfigService], }), MailModule, + LdapModule, ], controllers: [AuthController], providers: [AuthService, LocalStrategy, JwtStrategy], diff --git a/apps/api/src/auth/auth.service.spec.ts b/apps/api/src/auth/auth.service.spec.ts new file mode 100644 index 0000000..29641e9 --- /dev/null +++ b/apps/api/src/auth/auth.service.spec.ts @@ -0,0 +1,109 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { AuthService } from './auth.service'; + +/** + * validateUser — LDAP login path (AUTH-06 follow-up): users imported from LDAP + * have no local passwordHash and must be authenticated by binding as their own + * DN against the tenant's directory. These tests cover that branch; the local + * password path (argon2) is unchanged and exercised elsewhere. + */ +describe('AuthService.validateUser — LDAP login', () => { + let service: AuthService; + let prisma: any; + let ldapService: any; + let ldapConfigService: any; + + const ldapUser = { + id: 'u1', + tenantId: 't1', + username: 'alice', + passwordHash: null, + ldapDn: 'CN=alice,OU=Users,DC=ctl,DC=local', + isActive: true, + }; + + beforeEach(() => { + vi.clearAllMocks(); + prisma = { + user: { + findUnique: vi.fn().mockResolvedValue(ldapUser), + update: vi.fn().mockResolvedValue({}), + }, + }; + ldapService = { verifyUserCredentials: vi.fn() }; + ldapConfigService = { + getConfig: vi.fn().mockResolvedValue({ + serverUrl: 'ldaps://balios.ctl.local:636', + isActive: true, + tlsRejectUnauthorized: false, + }), + }; + service = new AuthService( + prisma, + {} as any, + {} as any, + {} as any, + ldapService, + ldapConfigService, + ); + }); + + it('authenticates an LDAP user via a successful directory bind', async () => { + ldapService.verifyUserCredentials.mockResolvedValue(true); + + const result = await service.validateUser('Alice', 'ad-password'); + + expect(result).toEqual(ldapUser); + expect(ldapService.verifyUserCredentials).toHaveBeenCalledWith( + { + serverUrl: 'ldaps://balios.ctl.local:636', + tlsRejectUnauthorized: false, + }, + ldapUser.ldapDn, + 'ad-password', + ); + expect(prisma.user.update).toHaveBeenCalledWith({ + where: { id: 'u1' }, + data: { lastLoginAt: expect.any(Date) }, + }); + }); + + it('rejects an LDAP user when the directory bind fails', async () => { + ldapService.verifyUserCredentials.mockResolvedValue(false); + + const result = await service.validateUser('alice', 'wrong'); + + expect(result).toBeNull(); + expect(prisma.user.update).not.toHaveBeenCalled(); + }); + + it('rejects an LDAP user when no active LDAP config exists', async () => { + ldapConfigService.getConfig.mockResolvedValue(null); + + const result = await service.validateUser('alice', 'pw'); + + expect(result).toBeNull(); + expect(ldapService.verifyUserCredentials).not.toHaveBeenCalled(); + }); + + it('rejects a passwordless user that has no ldapDn (never binds)', async () => { + prisma.user.findUnique.mockResolvedValue({ + ...ldapUser, + ldapDn: null, + }); + + const result = await service.validateUser('alice', 'pw'); + + expect(result).toBeNull(); + expect(ldapService.verifyUserCredentials).not.toHaveBeenCalled(); + }); + + it('rejects an inactive LDAP user before any bind', async () => { + prisma.user.findUnique.mockResolvedValue({ ...ldapUser, isActive: false }); + + const result = await service.validateUser('alice', 'pw'); + + expect(result).toBeNull(); + expect(ldapService.verifyUserCredentials).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/api/src/auth/auth.service.ts b/apps/api/src/auth/auth.service.ts index c819b28..7f83bb7 100644 --- a/apps/api/src/auth/auth.service.ts +++ b/apps/api/src/auth/auth.service.ts @@ -9,6 +9,8 @@ import { JwtService } from '@nestjs/jwt'; import * as argon2 from 'argon2'; import { randomUUID } from 'crypto'; import { Response } from 'express'; +import { LdapConfigService } from '../ldap/ldap-config.service'; +import { LdapService } from '../ldap/ldap.service'; import { MailService } from '../mail/mail.service'; import { PrismaService } from '../prisma/prisma.service'; @@ -21,6 +23,8 @@ export class AuthService { private jwtService: JwtService, private configService: ConfigService, private mailService: MailService, + private ldapService: LdapService, + private ldapConfigService: LdapConfigService, ) {} /** @@ -40,9 +44,35 @@ export class AuthService { return null; } - // LDAP users without local password cannot log in via local auth + // LDAP users have no local password — authenticate them against the + // directory by binding as their OWN DN with the password they entered. if (!user.passwordHash) { - return null; + if (!user.ldapDn) { + return null; + } + + const config = await this.ldapConfigService.getConfig(user.tenantId); + if (!config || !config.isActive) { + return null; + } + + const ok = await this.ldapService.verifyUserCredentials( + { + serverUrl: config.serverUrl, + tlsRejectUnauthorized: config.tlsRejectUnauthorized, + }, + user.ldapDn, + password, + ); + if (!ok) { + return null; + } + + await this.prisma.user.update({ + where: { id: user.id }, + data: { lastLoginAt: new Date() }, + }); + return user; } const isPasswordValid = await argon2.verify(user.passwordHash, password); diff --git a/apps/api/src/ldap/ldap.module.ts b/apps/api/src/ldap/ldap.module.ts index 68bac86..e91e668 100644 --- a/apps/api/src/ldap/ldap.module.ts +++ b/apps/api/src/ldap/ldap.module.ts @@ -16,6 +16,6 @@ import { LdapService } from './ldap.service'; imports: [ScheduleModule.forRoot(), UserModule], controllers: [LdapController], providers: [LdapService, LdapConfigService, LdapSyncScheduler], - exports: [LdapService], + exports: [LdapService, LdapConfigService], }) export class LdapModule {} diff --git a/apps/api/src/ldap/ldap.service.spec.ts b/apps/api/src/ldap/ldap.service.spec.ts index 9d696c4..e8d9d10 100644 --- a/apps/api/src/ldap/ldap.service.spec.ts +++ b/apps/api/src/ldap/ldap.service.spec.ts @@ -308,3 +308,44 @@ describe('LdapService.testConnection — TLS verification opt-out (ldaps)', () = expect(opts.tlsOptions).toBeUndefined(); }); }); + +describe('LdapService.verifyUserCredentials — LDAP login bind', () => { + let service: LdapService; + + beforeEach(() => { + vi.clearAllMocks(); + mockUnbind.mockResolvedValue(undefined); + service = new LdapService({} as any, {} as any); + }); + + it('returns true when the user bind succeeds', async () => { + mockBind.mockResolvedValue(undefined); + const ok = await service.verifyUserCredentials( + { serverUrl: 'ldaps://ad:636', tlsRejectUnauthorized: false }, + 'CN=alice,DC=x', + 'correct-pw', + ); + expect(ok).toBe(true); + expect(mockBind).toHaveBeenCalledWith('CN=alice,DC=x', 'correct-pw'); + }); + + it('returns false when the user bind fails (wrong password)', async () => { + mockBind.mockRejectedValue(new Error('invalid credentials')); + const ok = await service.verifyUserCredentials( + { serverUrl: 'ldaps://ad:636' }, + 'CN=alice,DC=x', + 'wrong-pw', + ); + expect(ok).toBe(false); + }); + + it('rejects an empty password WITHOUT binding (no anonymous-bind bypass)', async () => { + const ok = await service.verifyUserCredentials( + { serverUrl: 'ldaps://ad:636' }, + 'CN=alice,DC=x', + '', + ); + expect(ok).toBe(false); + expect(mockBind).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/api/src/ldap/ldap.service.ts b/apps/api/src/ldap/ldap.service.ts index cc7b882..45787a9 100644 --- a/apps/api/src/ldap/ldap.service.ts +++ b/apps/api/src/ldap/ldap.service.ts @@ -152,6 +152,41 @@ export class LdapService { } } + /** + * Authenticate a user for LOGIN by binding as their OWN DN with the password + * they entered (distinct from the service-account bind used for sync/search). + * Returns true only on a successful authenticated bind. + * + * SECURITY: an empty password is rejected up front — many AD servers treat a + * bind with a DN and empty password as an unauthenticated/anonymous bind that + * "succeeds", which would let anyone log in as any LDAP user. Never allow it. + */ + async verifyUserCredentials( + config: { serverUrl: string; tlsRejectUnauthorized?: boolean | null }, + userDn: string, + password: string, + ): Promise { + if (!userDn || !password) { + return false; + } + + const client = new Client( + this.buildClientOptions(config.serverUrl, config.tlsRejectUnauthorized), + ); + try { + await client.bind(userDn, password); + return true; + } catch { + return false; + } finally { + try { + await client.unbind(); + } catch { + // Ignore unbind errors + } + } + } + /** * Discover groups and organizational units under the configured base DN. * Used by the admin UI to build a selective import filter (groupFilterDns).