auth: security audit fixes, rate limits on every route
ci / check (push) Successful in 1m3s
ci / images (push) Successful in 19s
ci / check (pull_request) Successful in 1m5s
ci / images (pull_request) Successful in 5s

Two separate reviews of the SSO branch and all 207 API routes (spec 12 §8.6). Every route was
guarded as meant. Fixed, each with a test:
- login: a directory 503 counts as a failed sign-in; a locked directory account is a refused
  password, not "unavailable"
- open redirect after OpenID sign-in (a tab in next)
- directory email matching only for non-admins without their own password; others connect on
  My settings (POST /api/me/identities/ldap); only admins disconnect a directory account
- OpenID session check reads the refreshed ID token's claims too
- connect is POST /api/auth/oidc/connect (CSRF header)
- stored bind password, client secret and SMTP password tied to their server
- 409 role_managed when a provider sets the role; empty Contrack PATCH is a 400
- @fastify/rate-limit on every route (RATE_LIMIT_PER_MINUTE), 10/min on Test buttons; a
  password asked again shares the login limiter
Live tests pass against lldap 0.6.3, Samba 4 AD and Keycloak 26.8.1.
This commit is contained in:
iamdoubz committed 2026-10-26 09:46:23 -05:00
1 parent 19610777a4
commit 97e67e347d
39 files changed
+1059 -186

No files matched your search

+32 -6
View File
@@ -38,6 +38,7 @@ describe.skipIf(!hasTestDb || !URL_ || !HTTP)('LDAP with a real lldap (12 S4)',
const ann = `ann${tag}`;
const bob = `bob${tag}`;
const cat = `cat${tag}`;
const dee = `dee${tag}`;
const group = `pbx-admins-${tag}`;
let t: Awaited<ReturnType<typeof createTestApp>>;
let jwt: string;
@@ -86,6 +87,7 @@ describe.skipIf(!hasTestDb || !URL_ || !HTTP)('LDAP with a real lldap (12 S4)',
[ann, 'Ann Lee'],
[bob, 'Bob Roe'],
[cat, 'Cat Poe'],
[dee, 'Dee Fox'],
])
await gql(
`mutation { createUser(user: { id: "${id}", email: "${id}@example.com", displayName: "${name}" }) { id } }`,
@@ -96,7 +98,7 @@ describe.skipIf(!hasTestDb || !URL_ || !HTTP)('LDAP with a real lldap (12 S4)',
await gql(`mutation { addUserToGroup(userId: "${ann}", groupId: ${g.createGroup.id}) { ok } }`);
const c = new Client({ url: URL_! });
await c.bind(`uid=admin,ou=people,${BASE}`, ADMIN_PASS);
for (const id of [ann, bob, cat])
for (const id of [ann, bob, cat, dee])
await c.exop(
'1.3.6.1.4.1.4203.1.11.1',
passwordModify(`uid=${id},ou=people,${BASE}`, `${id}-directory-pw`),
@@ -106,14 +108,21 @@ describe.skipIf(!hasTestDb || !URL_ || !HTTP)('LDAP with a real lldap (12 S4)',
t = await createTestApp();
await t.clearMustChange(ADMIN.email);
admin = await t.login(ADMIN.email, ADMIN.password);
await t.addUser(`${ann}@example.com`, 'ann-local-pw-12');
await t.addUser(`${cat}@example.com`, 'cat-local-pw-12');
// New users with the temporary password from Users: the directory email may connect them.
for (const who of [ann, cat]) {
await t.addUser(`${who}@example.com`, `${who}-local-pw-12`);
await t.db
.update(users)
.set({ mustChangePassword: true })
.where(eq(users.email, `${who}@example.com`));
}
await t.addUser(`${dee}@example.com`, 'dee-own-password-12'); // chose her own password
await setSetting(t.db, 'auth.ldap', settings());
}, 60_000);
afterAll(async () => {
await t?.close();
for (const id of [ann, bob, cat])
for (const id of [ann, bob, cat, dee])
await gql(`mutation { deleteUser(userId: "${id}") { ok } }`).catch(() => {});
});
@@ -142,7 +151,7 @@ describe.skipIf(!hasTestDb || !URL_ || !HTTP)('LDAP with a real lldap (12 S4)',
it('signs in by email too, and the local password still works', async () => {
resetLoginLimiter();
expect((await login(`${ann}@example.com`, `${ann}-directory-pw`)).statusCode).toBe(200);
expect((await login(`${ann}@example.com`, 'ann-local-pw-12')).statusCode).toBe(200);
expect((await login(`${ann}@example.com`, `${ann}-local-pw-12`)).statusCode).toBe(200);
});
it('a wrong password, an empty one, and a stranger are refused alike', async () => {
@@ -170,7 +179,7 @@ describe.skipIf(!hasTestDb || !URL_ || !HTTP)('LDAP with a real lldap (12 S4)',
const r = await login(ann, `${ann}-directory-pw`);
expect(r.statusCode).toBe(503);
expect(r.json().error.code).toBe('directory_unavailable');
expect((await login(`${ann}@example.com`, 'ann-local-pw-12')).statusCode).toBe(200);
expect((await login(`${ann}@example.com`, `${ann}-local-pw-12`)).statusCode).toBe(200);
} finally {
await setSetting(t.db, 'auth.ldap', settings());
}
@@ -198,6 +207,23 @@ describe.skipIf(!hasTestDb || !URL_ || !HTTP)('LDAP with a real lldap (12 S4)',
expect(bad.json()).toMatchObject({ ok: false, error: expect.stringContaining('refused') });
});
it('someone with their own password connects on My settings, never by email alone', async () => {
resetLoginLimiter();
expect((await login(dee, `${dee}-directory-pw`)).statusCode).toBe(401);
const mine = await t.login(`${dee}@example.com`, 'dee-own-password-12');
const connect = (password: string) =>
t.app.inject({
method: 'POST',
url: '/api/me/identities/ldap',
headers: { ...H, cookie: mine },
payload: { login: dee, password },
});
expect((await connect('wrong-password')).statusCode).toBe(400);
expect((await connect(`${dee}-directory-pw`)).statusCode).toBe(200);
resetLoginLimiter();
expect((await login(dee, `${dee}-directory-pw`)).statusCode).toBe(200);
});
it('directoryCheck disables someone removed from the directory, and only them (§4.5)', async () => {
resetLoginLimiter();
expect((await login(cat, `${cat}-directory-pw`)).statusCode).toBe(200); // connects cat