fix: Code-Review-Korrekturen für PR #1

- Cross-Tenant-Datenleck in get_group_members: Account-Filter im
  LEFT JOIN contacts hinzugefügt (c.account = g.account)
- CROSS JOIN in get_group durch sauberen JOIN ON g.id = gm.group_id
  ersetzt
- Leere Gruppen werden jetzt gespeichert (parse_group gibt auch
  leere member_uids-Listen zurück)
- ON DELETE CASCADE Doku korrigiert: CASCADE greift nur beim Löschen
  einer Gruppe, nicht beim Löschen eines Kontakts
- Migration für Bestands-DBs in README dokumentiert (sync_state
  leeren für vollen Re-Sync)
- Index idx_group_members_member_uid für get_groups_for_contact
- Unbenutzte is_admin-Variablen in neuen Endpoints bereinigt
This commit is contained in:
2026-08-07 15:30:17 +02:00
parent 94a279f207
commit cccb4217b0
4 changed files with 29 additions and 17 deletions
+17 -3
View File
@@ -162,9 +162,23 @@ Gruppen sind über die API abrufbar:
- `GET /api/groups/{id}/members` — Nur Members einer Gruppe
- `GET /api/contacts/{id}` — Enthält `groups`-Feld mit Gruppennamen
Wird ein Kontakt gelöscht, wird die Mitgliedschaft in Gruppen
automatisch entfernt (`ON DELETE CASCADE`). Die Gruppe selbst bleibt
erhalten.
Wird eine Gruppe gelöscht, werden zugehörige Memberschaften automatisch
entfernt (`ON DELETE CASCADE`). Gelöschte Mitglied-Kontakte verbleiben
als Member-Eintrag in der Gruppe (ohne aufgelöste Kontaktdaten). Beim
nächsten vollen Re-Sync werden tote Memberschaften bereinigt.
### Migration bei erstem Deploy
Bei Bestands-DBs lagen Gruppen bisher als normale Kontakte in der
`contacts`-Tabelle. Nach dem Deploy müssen diese einmalig bereinigt
werden:
1. Sync-Container stoppen: `docker compose stop icloud-contacts-sync`
2. Sync-State zurücksetzen: `DELETE FROM sync_state;` (erzwingt vollen Re-Sync)
3. Container neu starten: `docker compose start icloud-contacts-sync`
Beim nächsten Sync-Lauf werden alle vCards neu klassifiziziert —
Gruppen landen in `groups`, Kontakte bleiben in `contacts`.
## 11. Web-Ansicht und API (interner Zugriff über Authelia)
+2 -1
View File
@@ -77,7 +77,8 @@ CREATE TABLE IF NOT EXISTS group_members (
group_id INT NOT NULL,
member_uid VARCHAR(255) NOT NULL,
FOREIGN KEY (group_id) REFERENCES `groups`(id) ON DELETE CASCADE,
UNIQUE KEY uq_group_member (group_id, member_uid)
UNIQUE KEY uq_group_member (group_id, member_uid),
KEY idx_group_members_member_uid (member_uid)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
-- Protokoll der Geburtstags-Mails, verhindert Doppelversand am selben Tag.
+10 -9
View File
@@ -215,12 +215,11 @@ def list_sync_runs(current_user: str = Depends(get_current_user)):
@app.get("/api/groups", response_model=GroupListResponse)
def list_groups(
request: Request,
limit: int = Query(default=50, le=200),
offset: int = Query(default=0, ge=0),
current_user: str = Depends(get_current_user),
):
account_name, is_admin = resolve_account_for_user(current_user)
account_name, _ = resolve_account_for_user(current_user)
with db.get_connection() as conn:
where_clause, params = _account_filter_clause(account_name)
@@ -245,7 +244,7 @@ def list_groups(
@app.get("/api/groups/{group_id}", response_model=GroupDetailOut)
def get_group(group_id: int, current_user: str = Depends(get_current_user)):
account_name, is_admin = resolve_account_for_user(current_user)
account_name, _ = resolve_account_for_user(current_user)
with db.get_connection() as conn:
where_clause, params = _account_filter_clause(account_name)
@@ -265,9 +264,9 @@ def get_group(group_id: int, current_user: str = Depends(get_current_user)):
cur.execute(
"""SELECT gm.member_uid, c.id, c.full_name, c.given_name, c.family_name
FROM group_members gm
JOIN `groups` g ON g.id = gm.group_id
LEFT JOIN contacts c ON c.account = g.account AND c.uid = gm.member_uid
CROSS JOIN `groups` g
WHERE g.id = %s AND gm.group_id = g.id""",
WHERE g.id = %s""",
(group_id,),
)
members = cur.fetchall()
@@ -286,24 +285,26 @@ def get_group(group_id: int, current_user: str = Depends(get_current_user)):
@app.get("/api/groups/{group_id}/members")
def get_group_members(group_id: int, current_user: str = Depends(get_current_user)):
account_name, is_admin = resolve_account_for_user(current_user)
account_name, _ = resolve_account_for_user(current_user)
with db.get_connection() as conn:
where_clause, params = _account_filter_clause(account_name)
with conn.cursor() as cur:
cur.execute(
f"""SELECT g.id FROM `groups` g {where_clause}
f"""SELECT g.id, g.account FROM `groups` g {where_clause}
{"AND" if where_clause else "WHERE"} g.id = %s""",
params + [group_id],
)
if not cur.fetchone():
group = cur.fetchone()
if not group:
return {}
cur.execute(
"""SELECT gm.member_uid, c.id, c.full_name, c.given_name, c.family_name,
c.organization, c.birthday, c.photo_url
FROM group_members gm
LEFT JOIN contacts c ON c.uid = gm.member_uid
JOIN `groups` g ON g.id = gm.group_id
LEFT JOIN contacts c ON c.account = g.account AND c.uid = gm.member_uid
WHERE gm.group_id = %s""",
(group_id,),
)
-4
View File
@@ -61,10 +61,6 @@ def parse_group(raw_text: str, account: str, etag: str | None = None) -> dict |
else:
member_uids.append(value)
if not member_uids:
logger.debug("Gruppe %s hat keine Members, überspringe", uid)
return None
return {
"account": account,
"uid": uid,