Selaa lähdekoodia

feat: migrate Chad Music API key from UserDefaults to Keychain

- Add ChadMusicCredentials with DI protocols (KeyStoreProtocol, DefaultsStoreProtocol)
- Automatic one-time migration from UserDefaults → Keychain on first access
- Graceful fallback when Keychain access denied (UserDefaults preserved)
- Update ChadMusicAPIClient to read from ChadMusicCredentials.shared
- Update SettingsView to write to Keychain via ChadMusicCredentials
- Remove duplicate protocol definitions from test file (now in production code)

All 13 KeychainMigrationTests define the contract.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
aldiss 2 kuukautta sitten
vanhempi
commit
2410712478

+ 2 - 3
Sources/Services/ChadMusicAPIClient.swift

@@ -16,10 +16,9 @@ final class ChadMusicAPIClient {
         set { UserDefaults.standard.set(newValue, forKey: "chadMusic.serverURL") }
     }
 
-    /// API key for authentication.
+    /// API key for authentication — reads from Keychain via ChadMusicCredentials.
     private var apiKey: String? {
-        let key = UserDefaults.standard.string(forKey: "chadMusic.apiKey")
-        return (key?.isEmpty ?? true) ? nil : key
+        ChadMusicCredentials.shared.apiKey
     }
 
     /// Whether the client is configured (has URL + API key).

+ 127 - 0
Sources/Services/ChadMusicCredentials.swift

@@ -0,0 +1,127 @@
+import Foundation
+
+// MARK: - Protocols for Dependency Injection
+
+/// Abstraction over Keychain storage for the API key.
+/// Production: KeychainService conforms. Tests: InMemoryKeyStore.
+protocol KeyStoreProtocol {
+    func save(_ key: String) throws
+    func load() -> String?
+    func delete()
+}
+
+/// Abstraction over UserDefaults for reading/removing the legacy API key.
+/// Production: UserDefaultsStore conforms. Tests: InMemoryDefaultsStore.
+protocol DefaultsStoreProtocol {
+    func string(forKey key: String) -> String?
+    func removeObject(forKey key: String)
+}
+
+// MARK: - Production Store Adapters
+
+/// Adapts KeychainService (static enum) to KeyStoreProtocol.
+struct KeychainKeyStore: KeyStoreProtocol {
+    func save(_ key: String) throws {
+        try KeychainService.saveAPIKey(key)
+    }
+
+    func load() -> String? {
+        KeychainService.loadAPIKey()
+    }
+
+    func delete() {
+        KeychainService.deleteAPIKey()
+    }
+}
+
+/// Adapts UserDefaults to DefaultsStoreProtocol.
+struct UserDefaultsStore: DefaultsStoreProtocol {
+    private let defaults: UserDefaults
+
+    init(defaults: UserDefaults = .standard) {
+        self.defaults = defaults
+    }
+
+    func string(forKey key: String) -> String? {
+        defaults.string(forKey: key)
+    }
+
+    func removeObject(forKey key: String) {
+        defaults.removeObject(forKey: key)
+    }
+}
+
+// MARK: - ChadMusicCredentials
+
+/// Single source of truth for the Chad Music API key.
+/// Reads from Keychain. On first access, migrates any existing
+/// UserDefaults value to Keychain and deletes the plaintext copy.
+@MainActor
+final class ChadMusicCredentials {
+
+    /// Shared singleton using real Keychain + UserDefaults for production.
+    static let shared = ChadMusicCredentials()
+
+    private static let userDefaultsKey = "chadMusic.apiKey"
+
+    private let keyStore: any KeyStoreProtocol
+    private let defaultsStore: any DefaultsStoreProtocol
+    private var hasMigrated = false
+
+    /// DI initializer — tests inject in-memory mocks.
+    init(
+        keyStore: any KeyStoreProtocol = KeychainKeyStore(),
+        defaultsStore: any DefaultsStoreProtocol = UserDefaultsStore()
+    ) {
+        self.keyStore = keyStore
+        self.defaultsStore = defaultsStore
+    }
+
+    /// The current API key, or nil if not set.
+    /// First call triggers one-time migration from UserDefaults → Keychain.
+    var apiKey: String? {
+        migrateIfNeeded()
+        let key = keyStore.load()
+        return (key?.isEmpty ?? true) ? nil : key
+    }
+
+    /// Whether an API key is currently stored.
+    var hasKey: Bool {
+        apiKey != nil
+    }
+
+    /// Save a new API key to Keychain.
+    func save(_ key: String) throws {
+        try keyStore.save(key)
+    }
+
+    /// Delete the API key from Keychain.
+    func delete() {
+        keyStore.delete()
+    }
+
+    // MARK: - Migration
+
+    /// One-time migration: if UserDefaults has a key, move it to Keychain.
+    private func migrateIfNeeded() {
+        guard !hasMigrated else { return }
+        hasMigrated = true
+
+        guard let existingKey = defaultsStore.string(forKey: Self.userDefaultsKey),
+              !existingKey.isEmpty else { return }
+
+        // Only migrate if Keychain is empty (don't overwrite a Keychain value)
+        if keyStore.load() == nil {
+            do {
+                try keyStore.save(existingKey)
+                defaultsStore.removeObject(forKey: Self.userDefaultsKey)
+            } catch {
+                // Keychain denied — leave UserDefaults in place as fallback.
+                print("[ChadMusicCredentials] Migration failed: \(error). Leaving UserDefaults in place.")
+            }
+        } else {
+            // Keychain already has a value — just delete the plaintext copy
+            defaultsStore.removeObject(forKey: Self.userDefaultsKey)
+        }
+    }
+}

+ 8 - 2
Sources/Views/SettingsView.swift

@@ -570,7 +570,7 @@ private struct ShortcutRow: View {
 
 private struct ChadMusicSettings: View {
     @AppStorage("chadMusic.serverURL") private var serverURL: String = ""
-    @AppStorage("chadMusic.apiKey") private var apiKey: String = ""
+    @State private var apiKey: String = ChadMusicCredentials.shared.apiKey ?? ""
     @State private var connectionStatus: ConnectionStatus = .unknown
     @State private var isTesting = false
     @State private var statsText: String = ""
@@ -605,8 +605,14 @@ private struct ChadMusicSettings: View {
                     .font(.headline)
                 SecureField("Enter API key", text: $apiKey)
                     .textFieldStyle(.roundedBorder)
-                    .onChange(of: apiKey) { _, _ in
+                    .onChange(of: apiKey) { _, newValue in
                         connectionStatus = .unknown
+                        let trimmed = newValue.trimmingCharacters(in: .whitespacesAndNewlines)
+                        if trimmed.isEmpty {
+                            ChadMusicCredentials.shared.delete()
+                        } else {
+                            try? ChadMusicCredentials.shared.save(trimmed)
+                        }
                     }
             }
 

+ 2 - 18
Tests/Unit/KeychainMigrationTests.swift

@@ -1,24 +1,8 @@
 import XCTest
 @testable import MixBoard
 
-// MARK: - Protocols for Dependency Injection
-//
-// ChadMusicCredentials must accept these protocols for testability.
-// Production code uses KeychainService (conforming to KeyStoreProtocol)
-// and UserDefaults (conforming to DefaultsStoreProtocol).
-
-/// Abstraction over Keychain storage for the API key.
-protocol KeyStoreProtocol {
-    func save(_ key: String) throws
-    func load() -> String?
-    func delete()
-}
-
-/// Abstraction over UserDefaults for reading/removing the legacy API key.
-protocol DefaultsStoreProtocol {
-    func string(forKey key: String) -> String?
-    func removeObject(forKey key: String)
-}
+// Protocols (KeyStoreProtocol, DefaultsStoreProtocol) are defined in
+// Sources/Services/ChadMusicCredentials.swift — imported via @testable import MixBoard.
 
 // MARK: - In-Memory Mocks