diff --git a/server/routes/auth.js b/server/routes/auth.js index 5b6980f..f30924f 100644 --- a/server/routes/auth.js +++ b/server/routes/auth.js @@ -10,7 +10,7 @@ import { updateDocument, Query, } from '../services/appwriteAdmin.js' -import { loginWithAppwrite, getLoginCooldownRemainingSec } from '../services/appwriteClient.js' +import { loginWithAppwrite, getLoginCooldownRemainingSec, sessionNeedsSecondFactor } from '../services/appwriteClient.js' import { SESSION_REVOKED_MESSAGE, clearPortalSession, @@ -169,7 +169,8 @@ router.post('/login', async (req, res) => { const staff = await resolveStaffAccount(user.$id, email.trim(), account) if (staff) { // Mitarbeiter mit Appwrite-2FA: erst der Code aus der Mail macht daraus eine Portal-Sitzung - if (account?.mfa === true) { + // mfa-Flag ODER Appwrite verlangt einen zweiten Faktor: scheitert der Konto-Abruf, entscheidet Appwrite + if (account?.mfa === true || (await sessionNeedsSecondFactor(user.sessionCookie))) { const { key, maskedEmail } = await startStaffMfa({ userId: user.$id, email: staff.email, sessionCookie: user.sessionCookie }).catch((err) => { if (err.type === 'user_email_not_verified') { err.status = 403 diff --git a/server/services/appwriteClient.js b/server/services/appwriteClient.js index ac1da88..aeb4f80 100644 --- a/server/services/appwriteClient.js +++ b/server/services/appwriteClient.js @@ -64,6 +64,21 @@ export function appwriteSessionFetch(path, { method = 'GET', body, sessionCookie }) } +/** + * Braucht die Sitzung noch einen zweiten Faktor (Appwrite-2FA)? Fragt Appwrite selbst statt des + * mfa-Flags aus dem Admin-Abruf, damit ein gestoerter Abruf nicht ohne Code durchlaesst. Andere + * Fehler werden geworfen (fail closed). + */ +export async function sessionNeedsSecondFactor(sessionCookie) { + try { + await appwriteSessionFetch('/account', { sessionCookie }) + return false + } catch (err) { + if (err.type === 'user_more_factors_required') return true + throw err + } +} + /** * Login via Appwrite Auth REST. userId kommt aus der Session – kein users.read nötig. */ diff --git a/server/services/staffMfa.js b/server/services/staffMfa.js index e48bb3f..30290e7 100644 --- a/server/services/staffMfa.js +++ b/server/services/staffMfa.js @@ -51,7 +51,9 @@ export async function startStaffMfa({ userId, email, sessionCookie }) { /** @returns {Promise<{ok: true, userId: string, email: string} | {ok: false, reason: 'invalid' | 'gone'}>} */ export async function verifyStaffMfa(key, otp) { const entry = liveEntry(key) - if (!entry) return { ok: false, reason: 'gone' } + if (!entry || entry.attempts >= MAX_ATTEMPTS) return { ok: false, reason: 'gone' } + // Versuch vor dem Appwrite-Aufruf reservieren, sonst zaehlen parallele Anfragen alle als erster Versuch + entry.attempts += 1 try { await appwriteSessionFetch('/account/mfa/challenges', { method: 'PUT', @@ -59,8 +61,10 @@ export async function verifyStaffMfa(key, otp) { sessionCookie: entry.sessionCookie, }) } catch (err) { - if (err.type !== 'user_invalid_token') throw err - entry.attempts += 1 + if (err.type !== 'user_invalid_token') { + entry.attempts -= 1 + throw err + } if (entry.attempts >= MAX_ATTEMPTS) pending.delete(key) return { ok: false, reason: 'invalid' } } diff --git a/test/staffLogin.test.js b/test/staffLogin.test.js index e7af920..8d07790 100644 --- a/test/staffLogin.test.js +++ b/test/staffLogin.test.js @@ -101,3 +101,14 @@ test('2FA an, E-Mail nicht bestaetigt: Hinweis statt Sitzung', async () => { assert.equal(res.body.error, '2FA ist für dein Konto noch nicht eingerichtet – bitte Kenso Bescheid geben.') assert.equal(cookieNamed(res, SESSION), null) }) + +test('2FA-Pruefung schlaegt nicht offen fehl, wenn der Konto-Abruf scheitert', async () => { + aw.failUserLookups(true) + try { + const res = await login('kenso@webklar.com', 'mit-mit-mit') + assert.equal(cookieNamed(res, SESSION), null) + assert.equal(res.body.mfaRequired, true) + } finally { + aw.failUserLookups(false) + } +}) diff --git a/test/staffMfa.test.js b/test/staffMfa.test.js index ab6dd40..0a98267 100644 --- a/test/staffMfa.test.js +++ b/test/staffMfa.test.js @@ -79,3 +79,11 @@ test('Code erneut senden: neue Mail, nur der neue Code gilt', async () => { assert.equal((await verifyStaffMfa(key, newOtp)).ok, true) assert.equal(await resendStaffMfa(key), false) }) + +test('Parallele Fehlversuche: hoechstens 5 Versuche zaehlen', async () => { + const { key } = await start() + const results = await Promise.all(Array.from({ length: 10 }, () => verifyStaffMfa(key, '000000'))) + assert.equal(results.filter((r) => r.reason === 'invalid').length, 5) + assert.equal(results.filter((r) => r.reason === 'gone').length, 5) + assert.deepEqual(await verifyStaffMfa(key, aw.lastOtp('u_staff')), { ok: false, reason: 'gone' }) +}) diff --git a/test/support/appwriteMock.js b/test/support/appwriteMock.js index 94bb91a..fe51213 100644 --- a/test/support/appwriteMock.js +++ b/test/support/appwriteMock.js @@ -20,6 +20,7 @@ export async function startAppwriteMock({ schema = {} } = {}) { const mails = new Map() const userCalls = [] let lastLocale = null + let failUserLookups = false // Wie Appwrite (Validator UID): 'unique()' ist erlaubt, sonst [A-Za-z0-9][A-Za-z0-9._-]{0,35} const validDocumentId = (id) => id === 'unique()' || (typeof id === 'string' && /^[A-Za-z0-9][A-Za-z0-9._-]{0,35}$/.test(id)) @@ -160,6 +161,7 @@ export async function startAppwriteMock({ schema = {} } = {}) { const docPath = url.pathname.match(/^\/v1\/databases\/[^/]+\/collections\/([^/]+)\/documents(?:\/([^/]+))?$/) const userPath = url.pathname.match(/^\/v1\/users\/([^/]+)$/) if (userPath && req.method === 'GET') { + if (failUserLookups) return [503, { message: 'Nachbau: Konto-Abruf gestoert', code: 503, type: 'general_server_error' }] return [200, users.has(userPath[1]) ? publicUser(users.get(userPath[1])) : { $id: userPath[1], passwordUpdate: '' }] } // GET /v1/account (requireAppwriteStaff, Ticketsystem-JWT): jwt-admin = Mitarbeiter mit Label admin, @@ -252,6 +254,8 @@ export async function startAppwriteMock({ schema = {} } = {}) { expireChallenges: () => challenges.forEach((c) => (c.expired = true)), mailsSent: (userId) => mails.get(userId) || 0, lastLocale: () => lastLocale, + /** GET /v1/users/:id scheitert (503), bis wieder false gesetzt wird */ + failUserLookups: (on) => (failUserLookups = Boolean(on)), userCalls: () => [...userCalls], close: () => new Promise((resolve) => server.close(resolve)), }