From dc21cb23191f25766b766ae421cfa6cbea87c1a4 Mon Sep 17 00:00:00 2001 From: Sebastian Hedtrich Date: Tue, 18 Aug 2026 21:34:43 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20Ger=C3=A4te-Pairing=20-=20Schl=C3=BCssel?= =?UTF-8?q?-Code=20stimmte=20nie=20mit=20dem=20angezeigten=20Code=20=C3=BC?= =?UTF-8?q?berein?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SnapshotService.CreateAndUploadAsync lädt in zwei Schritten hoch: Schritt 1 holt einen Code vom Server, Schritt 2 verschlüsselt den Sync-Schlüssel mit diesem Code und lädt erneut hoch. SnapshotStore.Store() vergab bei jedem Aufruf bedingungslos einen neuen Zufallscode - der dem Nutzer am Ende angezeigte Code war dadurch nie derselbe, mit dem der Schlüssel tatsächlich verschlüsselt wurde. Jede Kopplung musste deterministisch an der Schlüssel-Entschlüsselung scheitern. SnapshotUploadRequest bekommt ein optionales Code-Feld; Store() aktualisiert bei vorhandenem, passendem Code denselben Eintrag statt einen neuen mit neuem Code anzulegen. Betrifft LehrerApp.Api - der Server muss neu deployt werden. Co-Authored-By: Claude Sonnet 5 --- LehrerApp.Api.Tests/SnapshotStoreTests.cs | 89 +++++++++++++++++++++++ LehrerApp.Api/SnapshotStore.cs | 16 ++++ LehrerApp.Sync/Models/SyncModels.cs | 4 + LehrerApp.Sync/SnapshotService.cs | 3 +- TODO.md | 18 +++++ 5 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 LehrerApp.Api.Tests/SnapshotStoreTests.cs diff --git a/LehrerApp.Api.Tests/SnapshotStoreTests.cs b/LehrerApp.Api.Tests/SnapshotStoreTests.cs new file mode 100644 index 0000000..2619ec2 --- /dev/null +++ b/LehrerApp.Api.Tests/SnapshotStoreTests.cs @@ -0,0 +1,89 @@ +using LehrerApp.Sync.Models; +using Xunit; + +namespace LehrerApp.Api.Tests; + +public sealed class SnapshotStoreTests +{ + [Fact] + public void Store_ZweiterAufrufOhneCode_ErzeugtNeuenCode() + { + using var temp = new TempSnapshotStore(); + + var first = temp.Store.Store("user1", new SnapshotUploadRequest { EncryptedPayload = "p1" }); + var second = temp.Store.Store("user1", new SnapshotUploadRequest { EncryptedPayload = "p2" }); + + Assert.NotEqual(first.Code, second.Code); + } + + /// Regression: SnapshotService.CreateAndUploadAsync lädt in zwei Schritten hoch — Schritt 1 + /// ohne EncryptedSyncKey (holt einen Code vom Server), Schritt 2 mit dem gerade erst mit + /// diesem Code verschlüsselten Schlüssel. Ohne den zurückgereichten Code erzeugte Store() bei + /// jedem Aufruf einen NEUEN Zufallscode — der an den Nutzer angezeigte Code (aus Schritt 2) + /// passte dann nie zu dem Code, mit dem der Schlüssel tatsächlich verschlüsselt wurde, und das + /// Einlösen auf dem zweiten Gerät schlug beim Schlüssel-Entschlüsseln immer fehl. + [Fact] + public void Store_ZweiterAufrufMitCodeDesErstenSchritts_AktualisiertDenselbenEintrag() + { + using var temp = new TempSnapshotStore(); + + var step1 = temp.Store.Store("user1", new SnapshotUploadRequest { EncryptedPayload = "payload" }); + var step2 = temp.Store.Store("user1", new SnapshotUploadRequest + { + EncryptedPayload = "payload", EncryptedSyncKey = "encrypted-key", Code = step1.Code, + }); + + Assert.Equal(step1.Code, step2.Code); + + var retrieved = temp.Store.Retrieve("user1", step2.Code); + Assert.NotNull(retrieved); + Assert.Equal("encrypted-key", retrieved!.EncryptedSyncKey); + } + + [Fact] + public void Store_CodeGehoertZuAnderemNutzer_ErzeugtStattdessenNeuenEintrag() + { + using var temp = new TempSnapshotStore(); + var step1 = temp.Store.Store("user1", new SnapshotUploadRequest { EncryptedPayload = "payload" }); + + var result = temp.Store.Store("user2", new SnapshotUploadRequest + { + EncryptedPayload = "payload", EncryptedSyncKey = "encrypted-key", Code = step1.Code, + }); + + Assert.NotEqual(step1.Code, result.Code); + Assert.NotNull(temp.Store.Retrieve("user2", result.Code)); + } + + [Fact] + public void Retrieve_LoeschtDenEintragNachDemErstenAbruf() + { + using var temp = new TempSnapshotStore(); + var uploaded = temp.Store.Store("user1", new SnapshotUploadRequest { EncryptedPayload = "payload" }); + + var first = temp.Store.Retrieve("user1", uploaded.Code); + var second = temp.Store.Retrieve("user1", uploaded.Code); + + Assert.NotNull(first); + Assert.Null(second); + } + + private sealed class TempSnapshotStore : IDisposable + { + private readonly string _directory = Path.Combine( + Path.GetTempPath(), $"lehrerapp-api-tests-{Guid.NewGuid():N}"); + public LehrerApp.Api.SnapshotStore Store { get; } + + public TempSnapshotStore() + { + Directory.CreateDirectory(_directory); + Store = new LehrerApp.Api.SnapshotStore(_directory); + } + + public void Dispose() + { + Store.Dispose(); + if (Directory.Exists(_directory)) Directory.Delete(_directory, recursive: true); + } + } +} diff --git a/LehrerApp.Api/SnapshotStore.cs b/LehrerApp.Api/SnapshotStore.cs index 7d2a4e9..55a89f4 100644 --- a/LehrerApp.Api/SnapshotStore.cs +++ b/LehrerApp.Api/SnapshotStore.cs @@ -19,6 +19,22 @@ public class SnapshotStore(string dataPath) : IDisposable public SnapshotUploadResponse Store(string userId, SnapshotUploadRequest req) { + // Zweiter Upload-Schritt (Schlüssel nachreichen, mit dem im ersten Schritt vergebenen Code + // verschlüsselt): denselben Eintrag aktualisieren statt einen neuen mit neuem Code + // anzulegen — sonst würde der Schlüssel dauerhaft mit dem falschen Code verknüpft bleiben + // und die Entschlüsselung auf dem Empfängergerät schlägt fehl (Bug, siehe TODO.md 10.3.1). + if (!string.IsNullOrEmpty(req.Code)) + { + var existing = Col.FindOne(e => e.UserId == userId && e.Code == req.Code.ToUpperInvariant()); + if (existing is not null) + { + existing.EncryptedPayload = req.EncryptedPayload; + existing.EncryptedSyncKey = req.EncryptedSyncKey; + Col.Update(existing); + return new() { Code = existing.Code, ExpiresAt = existing.ExpiresAt }; + } + } + Col.DeleteMany(e => e.UserId == userId); var code = NewCode(); var entry = new SnapshotEntry { Code = code, UserId = userId, diff --git a/LehrerApp.Sync/Models/SyncModels.cs b/LehrerApp.Sync/Models/SyncModels.cs index 1145634..042fdfb 100644 --- a/LehrerApp.Sync/Models/SyncModels.cs +++ b/LehrerApp.Sync/Models/SyncModels.cs @@ -69,6 +69,10 @@ public class SnapshotUploadRequest public string EncryptedPayload { get; init; } = ""; public string EncryptedSyncKey { get; init; } = ""; public DeviceType DeviceType { get; init; } + /// Gesetzt beim zweiten Upload-Schritt (Schlüssel nachreichen, siehe SnapshotService. + /// CreateAndUploadAsync) — muss dem im ersten Schritt vom Server vergebenen Code entsprechen, + /// damit derselbe Eintrag aktualisiert statt ein neuer (mit neuem Code) angelegt wird. + public string? Code { get; init; } } public class SnapshotUploadResponse { diff --git a/LehrerApp.Sync/SnapshotService.cs b/LehrerApp.Sync/SnapshotService.cs index 1105f51..8856ba6 100644 --- a/LehrerApp.Sync/SnapshotService.cs +++ b/LehrerApp.Sync/SnapshotService.cs @@ -39,7 +39,8 @@ public class SnapshotService( var encKey = SyncCrypto.EncryptKeyWithCode(syncKey, init.Code); var r2 = await http.PostAsJsonAsync("/api/snapshot/upload", new SnapshotUploadRequest { EncryptedPayload = encPayload, - EncryptedSyncKey = encKey, DeviceType = deviceType }, ct); + EncryptedSyncKey = encKey, DeviceType = deviceType, + Code = init.Code }, ct); r2.EnsureSuccessStatusCode(); var result = await r2.Content.ReadFromJsonAsync(ct) ?? throw new InvalidOperationException("Leere Server-Antwort."); diff --git a/TODO.md b/TODO.md index 361bbcf..33bda7e 100644 --- a/TODO.md +++ b/TODO.md @@ -1605,6 +1605,24 @@ die Docker-Verifikation unter 10.2.4 (kein Docker im Entwicklungsstand verfügba SDK-Host (`dotnet`/`dotnet.exe`) statt auf die App selbst, ein Neustart ohne Argumente hätte dort nur die dotnet-CLI-Hilfe gezeigt statt die App neu zu starten — die ursprünglichen Kommandozeilenargumente werden in diesem Fall jetzt erneut mitgegeben. + + **Nachtrag (eigentlicher Bugfix — Codes stimmten nie überein):** Der obige Dialog machte + den Fehlschlag zwar sichtbar, aber er trat *immer* auf — auch bei korrekt eingegebenem, noch + gültigem Code. Ursache, vom Nutzer selbst bis in den `catch`-Block von `SyncCrypto. + DecryptKeyWithCode` zurückverfolgt: `SnapshotService.CreateAndUploadAsync` lädt in zwei + Schritten hoch — Schritt 1 (ohne Schlüssel) holt vom Server einen frisch vergebenen Code, + Schritt 2 verschlüsselt den Sync-Schlüssel mit *diesem* Code und lädt erneut hoch. Beide + Schritte trafen serverseitig auf `SnapshotStore.Store()`, das aber bei **jedem** Aufruf + bedingungslos einen neuen `NewCode()` vergab und den vorherigen Eintrag löschte. Der dem + Nutzer am Ende angezeigte Code (aus Schritt 2) war damit nie derselbe, mit dem der Schlüssel + tatsächlich verschlüsselt wurde (aus Schritt 1) — jede Kopplung musste zwingend an der + Schlüssel-Entschlüsselung scheitern, unabhängig von Tippfehlern, Ablaufzeit oder + Netzwerkproblemen. Behoben: `SnapshotUploadRequest` bekommt ein optionales `Code`-Feld; + `SnapshotStore.Store()` aktualisiert bei vorhandenem, zum Nutzer passendem Code denselben + Eintrag (`Col.Update`) statt einen neuen mit neuem Code anzulegen; `CreateAndUploadAsync` + reicht den in Schritt 1 erhaltenen Code in Schritt 2 zurück. **Wichtig für den Rollout:** + diese Änderung betrifft `LehrerApp.Api` (Server) — ein reines Neubauen des Desktop-Clients + reicht nicht, der Server muss neu deployt werden. - [ ] **10.3.2** Warnung und Wiederherstellungspfad bei verlorenem Schlüssel. - [x] **10.3.3** Prüfen, welche Daten unverschlüsselt über `PlainEventStore` laufen — personenbezogene Daten dürfen das nicht. `Grade`/`ExamResult` (beide mit `StudentId` plus