fix: switch keychain to Data Protection to eliminate auth prompts

[datlechin]

Summary

• Add centralized KeychainHelper using the Data Protection keychain (kSecUseDataProtectionKeychain: true) per Apple TN3137, eliminating per-item ACL prompts that appeared on every table open
• Revert kSecAttrAccessible to AfterFirstUnlock (was WhenUnlockedThisDeviceOnly), restoring background reconnection while screen is locked
• Refactor ConnectionStorage, AIKeyStorage, and LicenseStorage to delegate all keychain operations to KeychainHelper
• Add one-time migration from legacy file-based keychain to Data Protection keychain on app launch

Closes #326

Test plan

• Launch app — no keychain authorization dialog should appear
• Verify existing passwords still load (migration works)
• Lock screen, wait for ConnectionHealthMonitor 30s ping — reconnection should succeed (AfterFirstUnlock)
• Fresh install: save a connection with password, relaunch, verify password persists
• Run KeychainHelperTests and KeychainAccessControlTests

Summary by CodeRabbit

• New Features

  • Centralized, data-protected secure storage for keys with a one-time legacy migration at startup.

• Bug Fixes

  • Keychain authorization prompt no longer appears on every table open.

• Tests

  • Added tests covering secure storage save/load/delete, overwrite behavior, missing keys, and migration flag.

• Chores

  • Changelog updated with migration/fix notes.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 880788b4-3c65-4fa6-ab9e-78fac7ae4356

📥 Commits

Reviewing files that changed from the base of the PR and between f8bc0fa and 67176a6.

📒 Files selected for processing (2)

• TablePro/Core/Storage/KeychainHelper.swift
• TablePro/Core/Storage/LicenseStorage.swift

---

📝 Walkthrough

Walkthrough

Centralizes Keychain access into a new KeychainHelper, updates storage classes to delegate keychain operations to it, adds a startup migration call to move legacy items into Data Protection-backed keychain storage, and adds tests and changelog entries documenting the change.

Changes

Cohort / File(s)	Summary
Keychain Helper TablePro/Core/Storage/KeychainHelper.swift	Added new singleton KeychainHelper with save/load/delete (Data Protection), string helpers, and migrateFromLegacyKeychainIfNeeded() to migrate legacy items.
Storage Layer Delegation TablePro/Core/Storage/AIKeyStorage.swift, TablePro/Core/Storage/ConnectionStorage.swift, TablePro/Core/Storage/LicenseStorage.swift	Replaced direct SecItem* usage with KeychainHelper.shared calls; removed manual query construction and Security imports; public APIs unchanged.
App Startup TablePro/AppDelegate.swift	Invoke KeychainHelper.shared.migrateFromLegacyKeychainIfNeeded() in applicationDidFinishLaunching for one-time migration.
Tests TableProTests/Core/Storage/KeychainHelperTests.swift, TableProTests/Core/Storage/KeychainAccessControlTests.swift	Added tests for KeychainHelper round-trip, delete, upsert, nonexistent keys, migration flag; updated access-control test to expect data-protection flag and AfterFirstUnlock constant.
Changelog CHANGELOG.md	Added Unreleased note that Keychain authorization prompt no longer appears on every table open.

Sequence Diagram(s)

sequenceDiagram
    participant App as App Startup (AppDelegate)
    participant Helper as KeychainHelper
    participant Legacy as Legacy Keychain
    participant DPKeychain as Data Protection Keychain

    App->>Helper: migrateFromLegacyKeychainIfNeeded()
    Helper->>Legacy: Query legacy items
    Legacy-->>Helper: Return items

    loop per legacy item
        Helper->>DPKeychain: Save item (kSecUseDataProtectionKeychain)
        DPKeychain-->>Helper: Success / Error
        alt Success
            Helper->>Legacy: Delete legacy item
            Legacy-->>Helper: Deleted
        end
        Helper->>Helper: Log progress
    end

    Helper->>Helper: Set migration flag in UserDefaults
    Helper-->>App: Migration complete

Loading

sequenceDiagram
    participant Storage as Storage class (AIKeyStorage / ConnectionStorage / LicenseStorage)
    participant Helper as KeychainHelper (singleton)
    participant DPKeychain as Data Protection Keychain

    rect rgba(100,150,200,0.5)
    Storage->>Helper: saveString(value, forKey:)
    Helper->>DPKeychain: Save item with Data Protection
    DPKeychain-->>Helper: Success / Error
    Helper-->>Storage: Bool / result
    end

    rect rgba(150,200,100,0.5)
    Storage->>Helper: loadString(forKey:)
    Helper->>DPKeychain: Query item with Data Protection flag
    DPKeychain-->>Helper: Data / not found
    Helper-->>Storage: String? result
    end

    rect rgba(200,150,100,0.5)
    Storage->>Helper: delete(key:)
    Helper->>DPKeychain: Delete item
    DPKeychain-->>Helper: Success / not found
    Helper-->>Storage: Void
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• feat: add SSH TOTP/two-factor authentication support #315: Modifies ConnectionStorage keychain usage (adds TOTP secret methods) and is likely related to the centralization of keychain access in this PR.

Poem

🐰 I hopped through keys at break of dawn,

Moved treasures safe so prompts are gone,
Data-protected, tucked in tight,
No more popups in the night,
A little hop—your app runs right!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely describes the main change: switching keychain to Data Protection to eliminate authentication prompts.
Linked Issues check	✅ Passed	The PR successfully addresses issue #326 by implementing Data Protection keychain and migration logic to eliminate repeated authentication prompts.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the keychain refactoring objective: new KeychainHelper, migration logic, storage refactoring, and related tests.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/keychain-data-protection

📝 Coding Plan

• Generate coding plan for human review comments

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}