diff --git a/PolyPilot.Tests/ConnectionSettingsTests.cs b/PolyPilot.Tests/ConnectionSettingsTests.cs index ddc1e517f6..aab9f19576 100644 --- a/PolyPilot.Tests/ConnectionSettingsTests.cs +++ b/PolyPilot.Tests/ConnectionSettingsTests.cs @@ -483,6 +483,79 @@ public void VsCodeVariant_DisplayName_ReturnsCorrectLabel() Assert.Equal("VS Code Insiders", VsCodeVariant.Insiders.DisplayName()); } + [Fact] + public void ServerPassword_SerializesToJson_OnDesktop() + { + // Regression test for PR 341: ServerPassword must be included in JSON serialization + // on desktop platforms (including Mac Catalyst). Previously, [JsonIgnore] on MACCATALYST + // caused the password to be silently dropped, breaking Direct Sharing persistence. + var settings = new ConnectionSettings + { + ServerPassword = "test-password-123", + DirectSharingEnabled = true + }; + + var json = JsonSerializer.Serialize(settings, new JsonSerializerOptions { WriteIndented = true }); + Assert.Contains("\"ServerPassword\"", json); + Assert.Contains("test-password-123", json); + } + + [Fact] + public void ServerPassword_DeserializesFromJson_OnDesktop() + { + // Regression test: password round-trips through JSON on desktop platforms + var json = """{"Mode":0,"Host":"localhost","Port":4321,"ServerPassword":"my-secret","DirectSharingEnabled":true}"""; + var loaded = JsonSerializer.Deserialize(json); + + Assert.NotNull(loaded); + Assert.Equal("my-secret", loaded!.ServerPassword); + Assert.True(loaded.DirectSharingEnabled); + } + + [Fact] + public void DirectSharing_RequiresServerPassword_ForAutoStart() + { + // Validates the auto-start guard logic from Dashboard.StartDirectSharingIfEnabled: + // DirectSharingEnabled=true with empty password should NOT auto-start. + var settings = new ConnectionSettings + { + DirectSharingEnabled = true, + ServerPassword = null + }; + bool shouldAutoStart = settings.DirectSharingEnabled && !string.IsNullOrEmpty(settings.ServerPassword); + Assert.False(shouldAutoStart, "Should not auto-start without password"); + + settings.ServerPassword = ""; + shouldAutoStart = settings.DirectSharingEnabled && !string.IsNullOrEmpty(settings.ServerPassword); + Assert.False(shouldAutoStart, "Should not auto-start with empty password"); + + settings.ServerPassword = "real-password"; + shouldAutoStart = settings.DirectSharingEnabled && !string.IsNullOrEmpty(settings.ServerPassword); + Assert.True(shouldAutoStart, "Should auto-start with password set"); + } + + [Fact] + public void AllSecretFields_PresentInJson_OnDesktop() + { + // Ensures RemoteToken, LanToken, and ServerPassword all serialize on desktop. + // On iOS/Android they get [JsonIgnore] and go to SecureStorage, + // but on desktop (including Mac Catalyst) they must be in plain JSON. + var settings = new ConnectionSettings + { + RemoteToken = "remote-tok", + LanToken = "lan-tok", + ServerPassword = "srv-pass" + }; + + var json = JsonSerializer.Serialize(settings); + Assert.Contains("\"RemoteToken\"", json); + Assert.Contains("\"LanToken\"", json); + Assert.Contains("\"ServerPassword\"", json); + Assert.Contains("remote-tok", json); + Assert.Contains("lan-tok", json); + Assert.Contains("srv-pass", json); + } + private void Dispose() { try { Directory.Delete(_testDir, true); } catch { } diff --git a/PolyPilot/Models/ConnectionSettings.cs b/PolyPilot/Models/ConnectionSettings.cs index 0425e04b37..1fd653b60b 100644 --- a/PolyPilot/Models/ConnectionSettings.cs +++ b/PolyPilot/Models/ConnectionSettings.cs @@ -70,8 +70,9 @@ public class ConnectionSettings public bool AutoStartServer { get; set; } = false; public string? RemoteUrl { get; set; } - // Secrets: stored in SecureStorage on iOS/Android/MacCatalyst; plain JSON on other desktop. -#if IOS || ANDROID || MACCATALYST + // Secrets: stored in SecureStorage on iOS/Android; plain JSON on desktop (incl. Mac Catalyst). + // Mac Catalyst runs without app sandbox, making Keychain unreliable for SecureStorage. +#if IOS || ANDROID private string? _remoteToken; [System.Text.Json.Serialization.JsonIgnore] public string? RemoteToken @@ -215,7 +216,11 @@ public static ConnectionSettings Load() if (settings.Theme == UiTheme.InternationalWomensDay) settings.Theme = UiTheme.System; -#if IOS || ANDROID || MACCATALYST +#if MACCATALYST + // Reverse migration: PR 341 moved secrets to SecureStorage on Mac Catalyst, + // but Keychain is unreliable without app sandboxing. Recover secrets to plain JSON. + RecoverSecretsFromSecureStorage(settings); +#elif IOS || ANDROID settings.MigrateAndLoadMobileSecrets(rawJson); #endif @@ -260,7 +265,7 @@ public void Save() { var dir = Path.GetDirectoryName(SettingsPath)!; Directory.CreateDirectory(dir); -#if IOS || ANDROID || MACCATALYST +#if IOS || ANDROID SaveMobileSecretsIfDirty(); var json = JsonSerializer.Serialize(this, new JsonSerializerOptions { WriteIndented = true }); #else @@ -271,7 +276,65 @@ public void Save() catch { } } -#if IOS || ANDROID || MACCATALYST +#if MACCATALYST + /// + /// One-time reverse migration: PR 341 moved ServerPassword/RemoteToken/LanToken to + /// SecureStorage on Mac Catalyst. Since Mac Catalyst runs without sandbox, Keychain + /// is unreliable. This recovers any values and writes them back to plain JSON. + /// Only removes each Keychain entry after confirming that specific value was recovered. + /// + private static void RecoverSecretsFromSecureStorage(ConnectionSettings settings) + { + try + { + bool needsSave = false; + bool recoveredRemote = false, recoveredLan = false, recoveredPass = false; + + if (string.IsNullOrEmpty(settings.RemoteToken)) + { + var val = ReadSecureStorage("polypilot.connection.remoteToken"); + if (!string.IsNullOrEmpty(val)) { settings.RemoteToken = val; needsSave = true; recoveredRemote = true; } + } + if (string.IsNullOrEmpty(settings.LanToken)) + { + var val = ReadSecureStorage("polypilot.connection.lanToken"); + if (!string.IsNullOrEmpty(val)) { settings.LanToken = val; needsSave = true; recoveredLan = true; } + } + if (string.IsNullOrEmpty(settings.ServerPassword)) + { + var val = ReadSecureStorage("polypilot.connection.serverPassword"); + if (!string.IsNullOrEmpty(val)) { settings.ServerPassword = val; needsSave = true; recoveredPass = true; } + } + + if (needsSave) + { + settings.Save(); + + // Per-key cleanup: only remove a Keychain entry if that specific value was recovered + // and Save() wrote the file. Prevents data loss if Keychain read fails transiently + // for one secret but succeeds for another. + if (File.Exists(SettingsPath)) + { + if (recoveredRemote) + try { SecureStorage.Default.Remove("polypilot.connection.remoteToken"); } catch { } + if (recoveredLan) + try { SecureStorage.Default.Remove("polypilot.connection.lanToken"); } catch { } + if (recoveredPass) + try { SecureStorage.Default.Remove("polypilot.connection.serverPassword"); } catch { } + } + } + } + catch { } + } + + private static string? ReadSecureStorage(string key) + { + try { return Task.Run(() => SecureStorage.Default.GetAsync(key)).GetAwaiter().GetResult(); } + catch { return null; } + } +#endif + +#if IOS || ANDROID private const string RemoteTokenKey = "polypilot.connection.remoteToken"; private const string LanTokenKey = "polypilot.connection.lanToken"; private const string ServerPasswordKey = "polypilot.connection.serverPassword";