Mitarbeiter-Zugang: Review-Punkte (fail closed, Kunden-Konto bleibt, Probelauf robuster)

- removeEmployeeEntry: nur 404 heisst "kein Konto", jeder andere Fehler bricht ab
- Mitarbeiter loeschen (Admin-Panel): Konto ohne Mitarbeiter-Label (Kunde mit
  altem employees-Eintrag) bleibt bestehen, nur der Eintrag geht
- Mitarbeiter bearbeiten gibt nie automatisch das Label (der Eintrag kann von
  einem Kunden stammen); nur Neuanlage bekommt "mitarbeiter"
- --rechte: --offen-lassen <ids>, unlesbare Listen als Fehler statt Abbruch
  (--anwenden verweigert dann), --sicherung <datei> + docker-cp-Hinweis,
  Bucket-Einstellung transformations wird mitgeschickt
- getEmployeeByEmail entfernt (unbenutzt)
- Tests: write/users-unverified, zweiter Lauf idempotent, >100 Dokumente,
  Loeschen mit Kunden- und Mitarbeiter-Konto

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

scripts/mitarbeiter-zugang.mjs

@@ -6,8 +6,8 @@
  *   docker exec -it -w /app webklar-kundenbereich-server node scripts/mitarbeiter-zugang.mjs --status
  *   ... --label <userId>[,<userId>...] [--yes]     Label "mitarbeiter" vergeben (nur ausdrueckliche Konten)
  *   ... --eintrag-loeschen <employeeId> [--yes]    employees-Eintrag eines Kunden loeschen (Konto bleibt)
- *   ... --rechte                                   Probelauf: welche Rechte sich aendern
- *   ... --rechte --anwenden [--yes]                Rechte umstellen, Sicherung als JSON (Pfad wird ausgegeben)
+ *   ... --rechte [--offen-lassen <id>[,<id>...]]  Probelauf: welche Rechte sich aendern (IDs bleiben offen)
+ *   ... --rechte --anwenden [--sicherung <datei>] [--yes]  Rechte umstellen, vorher Sicherung als JSON
  *   ... --rechte --zurueck <sicherung.json> [--yes]  Rechte aus einer Sicherung wiederherstellen
  */
 import 'dotenv/config'
@@ -99,14 +99,19 @@ async function permissions() {
     return report(await applyPermissionChanges(changes, { direction: 'before' }))
   }
 
-  const changes = await planPermissionChanges()
+  const keep = (flag('--offen-lassen') || '').split(',').map((id) => id.trim()).filter(Boolean)
+  const { changes, errors } = await planPermissionChanges(undefined, { keep })
+  for (const e of errors) console.log(`NICHT LESBAR ${e.path}: ${e.error}`)
+  if (keep.length) console.log(`Bleiben offen: ${keep.join(', ')}`)
   if (!changes.length) return console.log('Keine offenen Rechte (any/guests/users) gefunden.')
   printChanges(changes)
   if (!args.includes('--anwenden')) return console.log(`Probelauf: ${changes.length} Stellen. Mit --anwenden umstellen.`)
+  if (errors.length) throw new Error('Nicht alles lesbar (siehe oben) - erst den Key-Scope klären, dann erneut.')
 
-  const backup = join(tmpdir(), `mitarbeiter-rechte-${new Date().toISOString().replace(/[:.]/g, '-')}.json`)
+  const backup = flag('--sicherung') || join(tmpdir(), `mitarbeiter-rechte-${new Date().toISOString().replace(/[:.]/g, '-')}.json`)
   writeFileSync(backup, JSON.stringify(changes, null, 2))
   console.log(`Sicherung: ${backup}`)
+  console.log(`  aus dem Container holen: docker cp webklar-kundenbereich-server:${backup} /root/reports/`)
   if (!(await confirm(`${changes.length} Stellen nur noch für Mitarbeiter freigeben?`))) return console.log('Abgebrochen.')
   report(await applyPermissionChanges(changes))
 }
@@ -118,7 +123,7 @@ async function main() {
   const employeeId = flag('--eintrag-loeschen')
   if (employeeId) return removeEntry(employeeId)
   if (args.includes('--rechte')) return permissions()
-  console.log('Usage: --status | --label <userId>[,...] | --eintrag-loeschen <employeeId> | --rechte [--anwenden | --zurueck <datei>] [--yes]')
+  console.log('Usage: --status | --label <userId>[,...] | --eintrag-loeschen <employeeId> | --rechte [--offen-lassen <id>,...] [--anwenden [--sicherung <datei>] | --zurueck <datei>] [--yes]')
 }
 
 main().catch((err) => {

server/services/appwriteAdmin.js

@@ -223,15 +223,6 @@ export async function getEmployeeByUserId(userId) {
   return docs[0] || null
 }
 
-export async function getEmployeeByEmail(email) {
-  if (!email) return null
-  const docs = await listDocuments(config.collections.employees, [
-    Query.equal('email', email.trim().toLowerCase()),
-    Query.limit(1),
-  ])
-  return docs[0] || null
-}
-
 export async function getPortalAccessByCustomerId(customerId) {
   const docs = await listDocuments(config.collections.customerPortalAccess, [
     Query.equal('customerId', customerId),

server/services/employeeAdmin.js

@@ -14,7 +14,7 @@ import {
   updateDocument,
   Query,
 } from './appwriteAdmin.js'
-import { ADMIN_LABEL, STAFF_LABEL, isAdminAccount } from './staffLabels.js'
+import { ADMIN_LABEL, STAFF_LABEL, isAdminAccount, isStaffAccount } from './staffLabels.js'
 
 function httpError(status, message) {
   const error = new Error(message)
@@ -46,12 +46,13 @@ async function employeeWithUser(employee) {
   return sanitizeEmployee(employee, user)
 }
 
-// Jeder Mitarbeiter traegt das Label "mitarbeiter" (Zugang zum Ticketsystem, siehe staffLabels.js);
-// isAdmin undefined laesst "admin" unveraendert
-async function setStaffLabels(userId, isAdmin) {
+// Neue Mitarbeiter bekommen das Label "mitarbeiter" (Zugang zum Ticketsystem, siehe staffLabels.js). Beim
+// Bearbeiten nie automatisch: ein employees-Eintrag kann von einem Kunden stammen (frueher selbst anlegbar);
+// Zugang fuer Bestandskonten nur per scripts/mitarbeiter-zugang.mjs --label
+async function setStaffLabels(userId, { isAdmin, grantStaff = false }) {
   const user = await getUserById(userId)
   const labels = new Set(user.labels || [])
-  labels.add(STAFF_LABEL)
+  if (grantStaff) labels.add(STAFF_LABEL)
   if (isAdmin === true) labels.add(ADMIN_LABEL)
   if (isAdmin === false) labels.delete(ADMIN_LABEL)
   await updateAppwriteUserLabels(userId, [...labels])
@@ -81,7 +82,7 @@ export async function createEmployeeWithLogin(payload) {
   const user = await createAppwriteUser({ email, password, name: displayName })
   let employee
   try {
-    await setStaffLabels(user.$id, isAdmin)
+    await setStaffLabels(user.$id, { isAdmin, grantStaff: true })
     employee = await createDocument(config.collections.employees, {
       userId: user.$id,
       displayName,
@@ -135,7 +136,9 @@ export async function updateEmployeeWithLogin(employeeId, payload, actorUserId)
   if (password) {
     await updateAppwriteUserPassword(employee.userId, password)
   }
-  await setStaffLabels(employee.userId, payload?.isAdmin === undefined ? undefined : Boolean(payload.isAdmin))
+  if (payload?.isAdmin !== undefined) {
+    await setStaffLabels(employee.userId, { isAdmin: Boolean(payload.isAdmin) })
+  }
 
   const updated = await updateDocument(config.collections.employees, employeeId, {
     displayName,
@@ -153,7 +156,16 @@ export async function deleteEmployeeWithLogin(employeeId, actorUserId) {
     throw httpError(400, 'Der eigene angemeldete Account kann nicht gelöscht werden')
   }
 
+  // Konto ohne Mitarbeiter-Label (z. B. Kunde mit altem employees-Eintrag): nur den Eintrag loeschen, das Konto ist
+  // sein Portal-Login. Konto nicht lesbar -> abbrechen statt zu raten
+  const user = employee.userId
+    ? await getUserById(employee.userId).catch((err) => {
+        if (err.status === 404) return null
+        throw err
+      })
+    : null
   await deleteDocument(config.collections.employees, employeeId)
+  if (!isStaffAccount(user)) return { success: true, accountKept: Boolean(user) }
   await deleteAppwriteUser(employee.userId).catch((error) => {
     error.message = `Mitarbeiter entfernt, Login konnte nicht gelöscht werden: ${error.message}`
     throw error

server/services/staffAccessAdmin.js

@@ -92,7 +92,13 @@ export async function grantStaffLabel(userId, request = appwriteAdminRequest) {
 export async function removeEmployeeEntry(employeeId, request = appwriteAdminRequest) {
   const path = `/databases/${config.appwrite.databaseId}/collections/${config.collections.employees}/documents/${employeeId}`
   const employee = await request(path)
-  const user = employee.userId ? await request(`/users/${employee.userId}`).catch(() => null) : null
+  // Nur "Konto gibt es nicht" (404) zaehlt als kein Mitarbeiter; jeder andere Fehler bricht ab (fail closed)
+  const user = employee.userId
+    ? await request(`/users/${employee.userId}`).catch((err) => {
+        if (err.status === 404) return null
+        throw err
+      })
+    : null
   if (isStaffAccount(user)) throw new Error(`${employee.displayName || employeeId} hat das Mitarbeiter-Label - Eintrag bleibt.`)
   await request(path, { method: 'DELETE' })
   return { employeeId, name: employee.displayName || '', userId: employee.userId || '' }
@@ -100,18 +106,31 @@ export async function removeEmployeeEntry(employeeId, request = appwriteAdminReq
 
 /**
  * Alle Stellen mit offenen Rechten: Collections und Buckets, bei Dokument- bzw. Datei-Sicherheit auch einzelne
- * Dokumente und Dateien. Jede Aenderung traegt before/after, damit sie sich zuruecknehmen laesst.
+ * Dokumente und Dateien. Jede Aenderung traegt before/after, damit sie sich zuruecknehmen laesst. keep = IDs von
+ * Collections/Buckets, die bewusst offen bleiben (samt ihrer Dokumente/Dateien). Eine Liste, die sich nicht lesen
+ * laesst (z. B. fehlender Key-Scope), landet in errors statt den ganzen Lauf abzubrechen.
  */
-export async function planPermissionChanges(request = appwriteAdminRequest) {
+export async function planPermissionChanges(request = appwriteAdminRequest, { keep = [] } = {}) {
   const changes = []
+  const errors = []
+  const kept = new Set(keep)
   const add = (change, before) => {
     const after = staffOnlyPermissions(before)
     if (!samePermissions(before, after)) changes.push({ ...change, before, after })
   }
+  const tryList = async (path, key) => {
+    try {
+      return await listAll(request, path, key)
+    } catch (err) {
+      errors.push({ path, error: err.message })
+      return []
+    }
+  }
 
-  for (const database of await listAll(request, '/databases', 'databases')) {
+  for (const database of await tryList('/databases', 'databases')) {
     const collectionsPath = `/databases/${database.$id}/collections`
-    for (const collection of await listAll(request, collectionsPath, 'collections')) {
+    for (const collection of await tryList(collectionsPath, 'collections')) {
+      if (kept.has(collection.$id)) continue
       add(
         {
           kind: 'collection',
@@ -123,13 +142,14 @@ export async function planPermissionChanges(request = appwriteAdminRequest) {
         collection.$permissions || []
       )
       if (!collection.documentSecurity) continue
-      for (const document of await listAll(request, `${collectionsPath}/${collection.$id}/documents`, 'documents')) {
+      for (const document of await tryList(`${collectionsPath}/${collection.$id}/documents`, 'documents')) {
         add({ kind: 'document', databaseId: database.$id, collectionId: collection.$id, id: document.$id }, document.$permissions || [])
       }
     }
   }
 
-  for (const bucket of await listAll(request, '/storage/buckets', 'buckets')) {
+  for (const bucket of await tryList('/storage/buckets', 'buckets')) {
+    if (kept.has(bucket.$id)) continue
     add(
       {
         kind: 'bucket',
@@ -144,16 +164,18 @@ export async function planPermissionChanges(request = appwriteAdminRequest) {
           compression: bucket.compression,
           encryption: bucket.encryption,
           antivirus: bucket.antivirus,
+          // neuere Appwrite-Versionen: Bild-Transformationen; mitschicken, sonst setzt das PUT den Standard
+          ...(bucket.transformations === undefined ? {} : { transformations: bucket.transformations }),
         },
       },
       bucket.$permissions || []
     )
     if (!bucket.fileSecurity) continue
-    for (const file of await listAll(request, `/storage/buckets/${bucket.$id}/files`, 'files')) {
+    for (const file of await tryList(`/storage/buckets/${bucket.$id}/files`, 'files')) {
       add({ kind: 'file', bucketId: bucket.$id, id: file.$id }, file.$permissions || [])
     }
   }
-  return changes
+  return { changes, errors }
 }
 
 function writeChange(change, permissions, request) {

test/staffAccess.test.js

@@ -17,7 +17,7 @@ const {
   staffOnlyPermissions,
 } = await import('../server/services/staffAccessAdmin.js')
 const { isStaffAccount, isAdminAccount } = await import('../server/services/staffLabels.js')
-const { createEmployeeWithLogin, updateEmployeeWithLogin } = await import('../server/services/employeeAdmin.js')
+const { createEmployeeWithLogin, deleteEmployeeWithLogin, updateEmployeeWithLogin } = await import('../server/services/employeeAdmin.js')
 after(() => Promise.all([aw.close(), st.close()]))
 
 const STAFF = ['label:mitarbeiter', 'label:admin']
@@ -44,6 +44,15 @@ test('staffOnlyPermissions: any/guests/users -> Mitarbeiter-Labels, andere Rolle
   )
   assert.deepEqual(staffOnlyPermissions(['read("label:mitarbeiter")', 'read("team:x")']), ['read("label:mitarbeiter")', 'read("team:x")'])
   assert.deepEqual(staffOnlyPermissions([]), [])
+  assert.deepEqual(staffOnlyPermissions(['write("users")', 'read("users/unverified")']), [
+    ...STAFF.map((r) => `write("${r}")`),
+    ...STAFF.map((r) => `read("${r}")`),
+  ])
+})
+
+test('staffOnlyPermissions: zweiter Lauf aendert nichts mehr', () => {
+  const once = staffOnlyPermissions(['read("users")', 'create("any")', 'delete("user:u1")'])
+  assert.deepEqual(staffOnlyPermissions(once), once)
 })
 
 // Nachbau der Appwrite-Verwaltungs-API fuer Rechte (Datenbanken, Collections, Dokumente, Buckets, Dateien)
@@ -62,7 +71,7 @@ function fakeAdminApi() {
     allowedFileExtensions: ['pdf'], compression: 'none', encryption: true, antivirus: true, $permissions: ['read("users")', 'create("users")'],
   }
   const writes = []
-  const request = async (path, { method = 'GET', body } = {}) => {
+  const request = async (path, { method = 'GET', body, queries = [] } = {}) => {
     if (method !== 'GET') {
       if (path.endsWith('/collections/workorders') && body.name === undefined) throw new Error('name fehlt')
       if (path.startsWith('/storage/') && method === 'PUT' && path.endsWith('/broken')) throw new Error('missing scope (buckets.write)')
@@ -71,16 +80,22 @@ function fakeAdminApi() {
     }
     if (path === '/databases') return { databases: [{ $id: 'woms-database' }] }
     if (path === '/databases/woms-database/collections') return { collections: Object.values(collections) }
-    if (path === '/databases/woms-database/collections/notes/documents') return { documents }
+    if (path === '/databases/woms-database/collections/notes/documents') {
+      // Seiten zu 100 per cursorAfter wie Appwrite
+      const cursor = queries.map((q) => (typeof q === 'string' ? JSON.parse(q) : q)).find((q) => q.method === 'cursorAfter')?.values[0]
+      const start = cursor ? documents.findIndex((d) => d.$id === cursor) + 1 : 0
+      return { documents: documents.slice(start, start + 100) }
+    }
     if (path === '/storage/buckets') return { buckets: [bucket, { ...bucket, $id: 'broken', name: 'Kaputt' }] }
     throw new Error(`unerwartet ${path}`)
   }
-  return { request, writes }
+  return { request, writes, documents }
 }
 
 test('planPermissionChanges: offene Collections, Dokumente und Buckets, geschlossene nicht', async () => {
   const api = fakeAdminApi()
-  const changes = await planPermissionChanges(api.request)
+  const { changes, errors } = await planPermissionChanges(api.request)
+  assert.deepEqual(errors, [])
   assert.deepEqual(changes.map((c) => `${c.kind}:${c.id}`), ['collection:workorders', 'document:n2', 'bucket:legal-files', 'bucket:broken'])
   const tickets = changes[0]
   assert.deepEqual(tickets.before, ['read("users")', 'update("users")'])
@@ -88,9 +103,28 @@ test('planPermissionChanges: offene Collections, Dokumente und Buckets, geschlos
   assert.deepEqual(tickets.settings, { name: 'Work Orders', documentSecurity: false, enabled: true })
 })
 
+test('planPermissionChanges: liest ueber 100 Dokumente seitenweise, offen-lassen und unlesbare Listen', async () => {
+  const api = fakeAdminApi()
+  const many = Array.from({ length: 230 }, (_, i) => ({ $id: `d${String(i).padStart(3, '0')}`, $permissions: ['read("users")'] }))
+  api.documents.splice(0, api.documents.length, ...many)
+  const all = await planPermissionChanges(api.request)
+  assert.equal(all.changes.filter((c) => c.kind === 'document').length, 230)
+
+  const kept = await planPermissionChanges(api.request, { keep: ['notes', 'legal-files'] })
+  assert.deepEqual(kept.changes.map((c) => `${c.kind}:${c.id}`), ['collection:workorders', 'bucket:broken'])
+
+  const failing = async (path, options) => {
+    if (path === '/storage/buckets') throw new Error('missing scope (buckets.read)')
+    return api.request(path, options)
+  }
+  const partial = await planPermissionChanges(failing)
+  assert.deepEqual(partial.errors, [{ path: '/storage/buckets', error: 'missing scope (buckets.read)' }])
+  assert.ok(partial.changes.some((c) => c.id === 'workorders'))
+})
+
 test('applyPermissionChanges: schreibt mit allen Einstellungen, meldet Fehler je Stelle, Ruecknahme schreibt before', async () => {
   const api = fakeAdminApi()
-  const changes = await planPermissionChanges(api.request)
+  const { changes } = await planPermissionChanges(api.request)
   const results = await applyPermissionChanges(changes, { request: api.request })
   assert.deepEqual(results.map((r) => r.ok), [true, true, true, false])
   assert.match(results[3].error, /buckets\.write/)
@@ -155,6 +189,33 @@ test('removeEmployeeEntry: loescht nur den Eintrag eines Kunden, nie das Konto;
   assert.equal(aw.docs('employees').has('e_m2'), true)
 })
 
+test('removeEmployeeEntry: Konto-Abruf gestoert -> Abbruch, Eintrag bleibt (fail closed)', async () => {
+  aw.seed('employees', { $id: 'e_m3', userId: 'u_m2', displayName: 'M2' })
+  aw.failUserLookups(true)
+  try {
+    await assert.rejects(removeEmployeeEntry('e_m3'))
+  } finally {
+    aw.failUserLookups(false)
+  }
+  assert.equal(aw.docs('employees').has('e_m3'), true)
+})
+
+test('Mitarbeiter loeschen (Admin-Panel): Eintrag eines Kunden weg, sein Konto bleibt; Mitarbeiter-Konto wird geloescht', async () => {
+  aw.addUser({ $id: 'u_k3', email: 'k3@example.invalid', name: 'K3' })
+  aw.seed('employees', { $id: 'e_k3', userId: 'u_k3', displayName: 'K3' })
+  const res = await deleteEmployeeWithLogin('e_k3', 'u_kenso')
+  assert.equal(res.accountKept, true)
+  assert.equal(aw.docs('employees').has('e_k3'), false)
+  assert.ok(aw.user('u_k3'))
+  assert.ok(!aw.userCalls().includes('delete:u_k3'))
+
+  aw.addUser({ $id: 'u_m4', email: 'm4@webklar.com', name: 'M4', labels: ['mitarbeiter'] })
+  aw.seed('employees', { $id: 'e_m4', userId: 'u_m4', displayName: 'M4' })
+  await deleteEmployeeWithLogin('e_m4', 'u_kenso')
+  assert.equal(aw.user('u_m4'), null)
+  assert.ok(aw.userCalls().includes('delete:u_m4'))
+})
+
 test('Mitarbeiter anlegen: Label mitarbeiter (+ admin), Bearbeiten ergaenzt es und laesst admin ohne Angabe stehen', async () => {
   const { employee } = await createEmployeeWithLogin({ displayName: 'Neu', email: 'neu@webklar.com', password: 'neu-neu-neu', isAdmin: false })
   assert.deepEqual(aw.user(employee.userId).labels, ['mitarbeiter'])
@@ -163,11 +224,13 @@ test('Mitarbeiter anlegen: Label mitarbeiter (+ admin), Bearbeiten ergaenzt es u
   assert.deepEqual(aw.user(chef.userId).labels.sort(), ['admin', 'mitarbeiter'])
   assert.equal(chef.isAdmin, true)
 
-  // Altbestand ohne Label: jede Bearbeitung holt es nach, admin bleibt ohne Angabe unveraendert
-  aw.addUser({ $id: 'u_alt', email: 'alt@webklar.com', name: 'Alt', labels: ['admin'] })
+  // Bearbeiten gibt nie automatisch Zugang (der Eintrag kann von einem Kunden stammen); admin nur auf Angabe
+  aw.addUser({ $id: 'u_alt', email: 'alt@webklar.com', name: 'Alt', labels: [] })
   aw.seed('employees', { $id: 'e_alt', userId: 'u_alt', displayName: 'Alt', email: 'alt@webklar.com' })
   await updateEmployeeWithLogin('e_alt', { shortcode: 'ALT' }, 'u_kenso')
-  assert.deepEqual(aw.user('u_alt').labels.sort(), ['admin', 'mitarbeiter'])
+  assert.deepEqual(aw.user('u_alt').labels, [])
+  await updateEmployeeWithLogin('e_alt', { isAdmin: true }, 'u_kenso')
+  assert.deepEqual(aw.user('u_alt').labels, ['admin'])
   await updateEmployeeWithLogin('e_alt', { isAdmin: false }, 'u_kenso')
-  assert.deepEqual(aw.user('u_alt').labels, ['mitarbeiter'])
+  assert.deepEqual(aw.user('u_alt').labels, [])
 })

test/support/appwriteMock.js

@@ -236,6 +236,12 @@ export async function startAppwriteMock({ schema = {}, datetimes = {}, defaultLi
       userCalls.push(`create:${userId}`)
       return [201, publicUser(users.get(userId))]
     }
+    const deleteUserPath = url.pathname.match(/^\/v1\/users\/([^/]+)$/)
+    if (deleteUserPath && req.method === 'DELETE') {
+      if (!users.delete(deleteUserPath[1])) return [404, { message: 'User not found', code: 404, type: 'user_not_found' }]
+      userCalls.push(`delete:${deleteUserPath[1]}`)
+      return [204, null]
+    }
     const labelsPath = url.pathname.match(/^\/v1\/users\/([^/]+)\/labels$/)
     if (labelsPath && req.method === 'PUT') {
       const user = users.get(labelsPath[1])