Просмотр исходного кода

fix(auth): stop repeated macOS Keychain prompts for OAuth tokens.

Store tokens in the Data Protection keychain, migrate legacy items once, and cache reads so widgets and the menu bar do not hit Keychain on every UI refresh.

Co-authored-by: Cursor <cursoragent@cursor.com>
AhtashamShahzad1 1 месяц назад
Родитель
Сommit
c2a34f39ec

+ 78 - 10
meetings_app/Auth/GoogleOAuthService.swift

@@ -57,12 +57,24 @@ final class GoogleOAuthService: NSObject {
     private static let keychainService = "ai.companion.for.meet.google.oauth.tokens"
     private static let keychainService = "ai.companion.for.meet.google.oauth.tokens"
     private let tokenStore = KeychainTokenStore(service: GoogleOAuthService.keychainService)
     private let tokenStore = KeychainTokenStore(service: GoogleOAuthService.keychainService)
     /// Legacy store used by older builds that keyed off the bundle identifier.
     /// Legacy store used by older builds that keyed off the bundle identifier.
-    private let legacyTokenStore = KeychainTokenStore()
+    private let legacyTokenStore = KeychainTokenStore(
+        service: Bundle.main.bundleIdentifier ?? "meetings_app",
+        useDataProtectionKeychain: false
+    )
     @MainActor private var inAppOAuthWindowController: InAppOAuthWindowController?
     @MainActor private var inAppOAuthWindowController: InAppOAuthWindowController?
+    private var cachedTokens: GoogleOAuthTokens?
+    private var didLoadTokensThisSession = false
+    private let tokenLoadLock = NSLock()
     private override init() {}
     private override init() {}
     
     
     private let lastSignedInEmailDefaultsKey = "google.oauth.lastSignedInEmail"
     private let lastSignedInEmailDefaultsKey = "google.oauth.lastSignedInEmail"
     private let ignoreLegacyTokensDefaultsKey = "google.oauth.ignoreLegacyTokens"
     private let ignoreLegacyTokensDefaultsKey = "google.oauth.ignoreLegacyTokens"
+    private let legacyKeychainMigrationDoneKey = "google.oauth.legacyKeychainMigrationDone"
+
+    /// Whether Google OAuth tokens are stored locally (uses a single Keychain read per app launch).
+    var isSignedIn: Bool {
+        loadTokens() != nil
+    }
     
     
     func lastSignedInEmailHint() -> String? {
     func lastSignedInEmailHint() -> String? {
         let value = UserDefaults.standard
         let value = UserDefaults.standard
@@ -118,36 +130,84 @@ final class GoogleOAuthService: NSObject {
     }
     }
 
 
     func signOut() throws {
     func signOut() throws {
+        invalidateTokenCache()
         try tokenStore.deleteTokens()
         try tokenStore.deleteTokens()
         UserDefaults.standard.set(true, forKey: ignoreLegacyTokensDefaultsKey)
         UserDefaults.standard.set(true, forKey: ignoreLegacyTokensDefaultsKey)
-        // Best-effort cleanup for legacy items. Some old items cannot be deleted after signing changes.
         try? legacyTokenStore.deleteTokens()
         try? legacyTokenStore.deleteTokens()
+        notifyTokensDidChange()
     }
     }
 
 
     func signOutAndRevoke() async throws {
     func signOutAndRevoke() async throws {
-        // Ensure tokens are in the current store so sign-out is effective.
-        let existing = try loadTokensOrMigrateIfNeeded()
+        let existing = try loadTokensOrMigrateIfNeeded(forceReload: true)
         if let existing { try await revokeTokenIfPossible(existing) }
         if let existing { try await revokeTokenIfPossible(existing) }
+        invalidateTokenCache()
         try tokenStore.deleteTokens()
         try tokenStore.deleteTokens()
         UserDefaults.standard.set(true, forKey: ignoreLegacyTokensDefaultsKey)
         UserDefaults.standard.set(true, forKey: ignoreLegacyTokensDefaultsKey)
         try? legacyTokenStore.deleteTokens()
         try? legacyTokenStore.deleteTokens()
+        notifyTokensDidChange()
     }
     }
 
 
     func loadTokens() -> GoogleOAuthTokens? {
     func loadTokens() -> GoogleOAuthTokens? {
         try? loadTokensOrMigrateIfNeeded()
         try? loadTokensOrMigrateIfNeeded()
     }
     }
+
+    private func invalidateTokenCache() {
+        tokenLoadLock.lock()
+        cachedTokens = nil
+        didLoadTokensThisSession = false
+        tokenLoadLock.unlock()
+    }
+
+    private func cacheTokens(_ tokens: GoogleOAuthTokens?) {
+        cachedTokens = tokens
+        didLoadTokensThisSession = true
+    }
+
+    private func notifyTokensDidChange() {
+        NotificationCenter.default.post(name: .googleOAuthTokensDidChange, object: nil)
+    }
     
     
     /// Loads from the current store; if empty, attempts to read legacy tokens and migrate them forward.
     /// Loads from the current store; if empty, attempts to read legacy tokens and migrate them forward.
-    /// This avoids being stuck with an undeletable legacy Keychain item after signing/bundle-id changes.
-    private func loadTokensOrMigrateIfNeeded() throws -> GoogleOAuthTokens? {
-        if let tokens = try tokenStore.readTokens() { return tokens }
-        
-        // If the user explicitly signed out, do not resurrect a stuck legacy Keychain item.
+    private func loadTokensOrMigrateIfNeeded(forceReload: Bool = false) throws -> GoogleOAuthTokens? {
+        tokenLoadLock.lock()
+        defer { tokenLoadLock.unlock() }
+
+        if didLoadTokensThisSession, forceReload == false {
+            return cachedTokens
+        }
+
+        if let tokens = try tokenStore.readTokens() {
+            cacheTokens(tokens)
+            return tokens
+        }
+
         if UserDefaults.standard.bool(forKey: ignoreLegacyTokensDefaultsKey) {
         if UserDefaults.standard.bool(forKey: ignoreLegacyTokensDefaultsKey) {
+            cacheTokens(nil)
             return nil
             return nil
         }
         }
+
+        if UserDefaults.standard.bool(forKey: legacyKeychainMigrationDoneKey) == false {
+            if let migrated = try migrateLegacyKeychainItemsOnce() {
+                cacheTokens(migrated)
+                UserDefaults.standard.set(true, forKey: legacyKeychainMigrationDoneKey)
+                notifyTokensDidChange()
+                return migrated
+            }
+            UserDefaults.standard.set(true, forKey: legacyKeychainMigrationDoneKey)
+        }
+
+        cacheTokens(nil)
+        return nil
+    }
+
+    /// One-time migration from pre–Data Protection keychain items (avoids repeated login prompts).
+    private func migrateLegacyKeychainItemsOnce() throws -> GoogleOAuthTokens? {
+        if let legacy = try tokenStore.readLegacyFileBasedTokens() {
+            try tokenStore.writeTokens(legacy)
+            tokenStore.deleteLegacyFileBasedTokens()
+            return legacy
+        }
         if let legacy = try legacyTokenStore.readTokens() {
         if let legacy = try legacyTokenStore.readTokens() {
-            // Migrate forward; legacy delete may fail with -25244 (invalid owner edit) after signing changes.
             try tokenStore.writeTokens(legacy)
             try tokenStore.writeTokens(legacy)
             try? legacyTokenStore.deleteTokens()
             try? legacyTokenStore.deleteTokens()
             return legacy
             return legacy
@@ -181,6 +241,9 @@ final class GoogleOAuthService: NSObject {
             if let refreshed = try await refreshTokens(tokens) {
             if let refreshed = try await refreshTokens(tokens) {
                 tokens = refreshed
                 tokens = refreshed
                 try tokenStore.writeTokens(tokens)
                 try tokenStore.writeTokens(tokens)
+                tokenLoadLock.lock()
+                cacheTokens(tokens)
+                tokenLoadLock.unlock()
                 return tokens.accessToken
                 return tokens.accessToken
             }
             }
         }
         }
@@ -188,6 +251,11 @@ final class GoogleOAuthService: NSObject {
         let tokens = try await interactiveSignIn(presentingWindow: presentingWindow)
         let tokens = try await interactiveSignIn(presentingWindow: presentingWindow)
         try tokenStore.writeTokens(tokens)
         try tokenStore.writeTokens(tokens)
         UserDefaults.standard.set(false, forKey: ignoreLegacyTokensDefaultsKey)
         UserDefaults.standard.set(false, forKey: ignoreLegacyTokensDefaultsKey)
+        UserDefaults.standard.set(true, forKey: legacyKeychainMigrationDoneKey)
+        tokenLoadLock.lock()
+        cacheTokens(tokens)
+        tokenLoadLock.unlock()
+        notifyTokensDidChange()
         return tokens.accessToken
         return tokens.accessToken
     }
     }
 
 

+ 53 - 35
meetings_app/Auth/KeychainTokenStore.swift

@@ -1,20 +1,22 @@
 import Foundation
 import Foundation
 import Security
 import Security
 
 
-/// Stores OAuth tokens in macOS Keychain.
+/// Stores OAuth tokens in the macOS Data Protection keychain (stable across Xcode rebuilds).
 final class KeychainTokenStore {
 final class KeychainTokenStore {
     private let service: String
     private let service: String
     private let account: String
     private let account: String
-    private let accessGroup: String?
+    private let useDataProtectionKeychain: Bool
 
 
-    init(service: String = Bundle.main.bundleIdentifier ?? "meetings_app",
-         account: String = "googleOAuthTokens",
-         accessGroup: String? = nil) {
+    init(
+        service: String = Bundle.main.bundleIdentifier ?? "meetings_app",
+        account: String = "googleOAuthTokens",
+        useDataProtectionKeychain: Bool = true
+    ) {
         self.service = service
         self.service = service
         self.account = account
         self.account = account
-        self.accessGroup = accessGroup
+        self.useDataProtectionKeychain = useDataProtectionKeychain
     }
     }
-    
+
     private func makeKeychainError(status: OSStatus, message: String) -> NSError {
     private func makeKeychainError(status: OSStatus, message: String) -> NSError {
         let statusMessage = SecCopyErrorMessageString(status, nil) as String? ?? "OSStatus \(status)"
         let statusMessage = SecCopyErrorMessageString(status, nil) as String? ?? "OSStatus \(status)"
         return NSError(
         return NSError(
@@ -26,50 +28,66 @@ final class KeychainTokenStore {
                 "keychainStatus": Int(status),
                 "keychainStatus": Int(status),
                 "keychainStatusMessage": statusMessage,
                 "keychainStatusMessage": statusMessage,
                 "keychainService": service,
                 "keychainService": service,
-                "keychainAccount": account,
-                "keychainAccessGroup": accessGroup as Any
+                "keychainAccount": account
             ]
             ]
         )
         )
     }
     }
 
 
-    func readTokens() throws -> GoogleOAuthTokens? {
+    private func baseQuery() -> [String: Any] {
         var query: [String: Any] = [
         var query: [String: Any] = [
             kSecClass as String: kSecClassGenericPassword,
             kSecClass as String: kSecClassGenericPassword,
             kSecAttrService as String: service,
             kSecAttrService as String: service,
-            kSecAttrAccount as String: account,
-            kSecReturnData as String: true,
-            kSecMatchLimit as String: kSecMatchLimitOne
+            kSecAttrAccount as String: account
         ]
         ]
-        if let accessGroup {
-            query[kSecAttrAccessGroup as String] = accessGroup
+        if useDataProtectionKeychain {
+            query[kSecUseDataProtectionKeychain as String] = true
         }
         }
+        return query
+    }
+
+    func readTokens() throws -> GoogleOAuthTokens? {
+        var query = baseQuery()
+        query[kSecReturnData as String] = true
+        query[kSecMatchLimit as String] = kSecMatchLimitOne
 
 
         var result: CFTypeRef?
         var result: CFTypeRef?
         let status = SecItemCopyMatching(query as CFDictionary, &result)
         let status = SecItemCopyMatching(query as CFDictionary, &result)
         if status == errSecItemNotFound { return nil }
         if status == errSecItemNotFound { return nil }
         guard status == errSecSuccess else {
         guard status == errSecSuccess else {
-            throw makeKeychainError(status: status, message: "Failed to read OAuth tokens from Keychain.".localized)
+            throw makeKeychainError(status: status, message: "Failed to read OAuth tokens from Keychain.")
         }
         }
         guard let data = result as? Data else {
         guard let data = result as? Data else {
             throw NSError(
             throw NSError(
                 domain: NSOSStatusErrorDomain,
                 domain: NSOSStatusErrorDomain,
                 code: Int(errSecDecode),
                 code: Int(errSecDecode),
-                userInfo: [NSLocalizedDescriptionKey: "Keychain returned an invalid token payload.".localized]
+                userInfo: [NSLocalizedDescriptionKey: "Keychain returned an invalid token payload."]
             )
             )
         }
         }
         return try JSONDecoder().decode(GoogleOAuthTokens.self, from: data)
         return try JSONDecoder().decode(GoogleOAuthTokens.self, from: data)
     }
     }
 
 
-    func writeTokens(_ tokens: GoogleOAuthTokens) throws {
-        let data = try JSONEncoder().encode(tokens)
-        var baseQuery: [String: Any] = [
+    /// Reads tokens written by older builds in the legacy file-based login keychain.
+    func readLegacyFileBasedTokens() throws -> GoogleOAuthTokens? {
+        guard useDataProtectionKeychain else { return nil }
+        var query: [String: Any] = [
             kSecClass as String: kSecClassGenericPassword,
             kSecClass as String: kSecClassGenericPassword,
             kSecAttrService as String: service,
             kSecAttrService as String: service,
-            kSecAttrAccount as String: account
+            kSecAttrAccount as String: account,
+            kSecReturnData as String: true,
+            kSecMatchLimit as String: kSecMatchLimitOne
         ]
         ]
-        if let accessGroup {
-            baseQuery[kSecAttrAccessGroup as String] = accessGroup
-        }
+
+        var result: CFTypeRef?
+        let status = SecItemCopyMatching(query as CFDictionary, &result)
+        if status == errSecItemNotFound { return nil }
+        guard status == errSecSuccess else { return nil }
+        guard let data = result as? Data else { return nil }
+        return try JSONDecoder().decode(GoogleOAuthTokens.self, from: data)
+    }
+
+    func writeTokens(_ tokens: GoogleOAuthTokens) throws {
+        let data = try JSONEncoder().encode(tokens)
+        let baseQuery = baseQuery()
 
 
         let attributesToUpdate: [String: Any] = [
         let attributesToUpdate: [String: Any] = [
             kSecValueData as String: data
             kSecValueData as String: data
@@ -78,7 +96,7 @@ final class KeychainTokenStore {
         let updateStatus = SecItemUpdate(baseQuery as CFDictionary, attributesToUpdate as CFDictionary)
         let updateStatus = SecItemUpdate(baseQuery as CFDictionary, attributesToUpdate as CFDictionary)
         if updateStatus == errSecSuccess { return }
         if updateStatus == errSecSuccess { return }
         if updateStatus != errSecItemNotFound {
         if updateStatus != errSecItemNotFound {
-            throw makeKeychainError(status: updateStatus, message: "Failed to update OAuth tokens in Keychain.".localized)
+            throw makeKeychainError(status: updateStatus, message: "Failed to update OAuth tokens in Keychain.")
         }
         }
 
 
         var addQuery = baseQuery
         var addQuery = baseQuery
@@ -86,24 +104,24 @@ final class KeychainTokenStore {
         addQuery[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlock
         addQuery[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlock
         let addStatus = SecItemAdd(addQuery as CFDictionary, nil)
         let addStatus = SecItemAdd(addQuery as CFDictionary, nil)
         guard addStatus == errSecSuccess else {
         guard addStatus == errSecSuccess else {
-            throw makeKeychainError(status: addStatus, message: "Failed to save OAuth tokens to Keychain.".localized)
+            throw makeKeychainError(status: addStatus, message: "Failed to save OAuth tokens to Keychain.")
         }
         }
     }
     }
 
 
     func deleteTokens() throws {
     func deleteTokens() throws {
+        let status = SecItemDelete(baseQuery() as CFDictionary)
+        guard status == errSecSuccess || status == errSecItemNotFound else {
+            throw makeKeychainError(status: status, message: "Failed to delete OAuth tokens from Keychain.")
+        }
+    }
+
+    func deleteLegacyFileBasedTokens() {
+        guard useDataProtectionKeychain else { return }
         var query: [String: Any] = [
         var query: [String: Any] = [
             kSecClass as String: kSecClassGenericPassword,
             kSecClass as String: kSecClassGenericPassword,
             kSecAttrService as String: service,
             kSecAttrService as String: service,
             kSecAttrAccount as String: account
             kSecAttrAccount as String: account
         ]
         ]
-        if let accessGroup {
-            query[kSecAttrAccessGroup as String] = accessGroup
-        }
-
-        let status = SecItemDelete(query as CFDictionary)
-        guard status == errSecSuccess || status == errSecItemNotFound else {
-            throw makeKeychainError(status: status, message: "Failed to delete OAuth tokens from Keychain.".localized)
-        }
+        _ = SecItemDelete(query as CFDictionary)
     }
     }
 }
 }
-

+ 4 - 1
meetings_app/StatusBar/StatusBarController.swift

@@ -31,6 +31,9 @@ final class StatusBarController: NSObject {
         observers.append(center.addObserver(forName: .statusBarSignOutRequested, object: nil, queue: .main) { [weak self] _ in
         observers.append(center.addObserver(forName: .statusBarSignOutRequested, object: nil, queue: .main) { [weak self] _ in
             self?.rebuildMenu()
             self?.rebuildMenu()
         })
         })
+        observers.append(center.addObserver(forName: .googleOAuthTokensDidChange, object: nil, queue: .main) { [weak self] _ in
+            self?.rebuildMenu()
+        })
         observers.append(center.addObserver(forName: .widgetSignInRequested, object: nil, queue: .main) { [weak self] _ in
         observers.append(center.addObserver(forName: .widgetSignInRequested, object: nil, queue: .main) { [weak self] _ in
             self?.rebuildMenu()
             self?.rebuildMenu()
         })
         })
@@ -62,7 +65,7 @@ final class StatusBarController: NSObject {
         addOpenSidebarItem(title: "AI Companion".localized, pageRaw: 5, to: menu)
         addOpenSidebarItem(title: "AI Companion".localized, pageRaw: 5, to: menu)
         addOpenSidebarItem(title: "Settings".localized, pageRaw: 4, to: menu)
         addOpenSidebarItem(title: "Settings".localized, pageRaw: 4, to: menu)
 
 
-        let signedIn = authService.loadTokens() != nil
+        let signedIn = authService.isSignedIn
         if signedIn && hasPremiumAccess {
         if signedIn && hasPremiumAccess {
             let topMeetings = WidgetMeetingStore.load().prefix(3).map { $0 }
             let topMeetings = WidgetMeetingStore.load().prefix(3).map { $0 }
             menu.addItem(.separator())
             menu.addItem(.separator())

+ 10 - 5
meetings_app/Widgets/DesktopWidgetView.swift

@@ -5,11 +5,7 @@ struct DesktopWidgetView: View {
     let variant: WidgetVariant
     let variant: WidgetVariant
     var isPreview: Bool
     var isPreview: Bool
     private let authService = GoogleOAuthService.shared
     private let authService = GoogleOAuthService.shared
-
-    private var isSignedIn: Bool {
-        if isPreview { return true }
-        return authService.loadTokens() != nil
-    }
+    @State private var isSignedIn = false
 
 
     private var topMeetings: [WidgetMeetingSnapshot] {
     private var topMeetings: [WidgetMeetingSnapshot] {
         WidgetMeetingStore.load().prefix(3).map { $0 }
         WidgetMeetingStore.load().prefix(3).map { $0 }
@@ -44,6 +40,14 @@ struct DesktopWidgetView: View {
             .padding(.bottom, variant.size == .medium ? 16 : 14)
             .padding(.bottom, variant.size == .medium ? 16 : 14)
             .padding(.top, variant.size == .medium ? 18 : 10)
             .padding(.top, variant.size == .medium ? 18 : 10)
         }
         }
+        .onAppear { refreshSignedInState() }
+        .onReceive(NotificationCenter.default.publisher(for: .googleOAuthTokensDidChange)) { _ in
+            refreshSignedInState()
+        }
+    }
+
+    private func refreshSignedInState() {
+        isSignedIn = isPreview || authService.isSignedIn
     }
     }
 
 
     @ViewBuilder
     @ViewBuilder
@@ -362,4 +366,5 @@ extension Notification.Name {
     static let statusBarPremiumAccessChanged = Notification.Name("statusBarPremiumAccessChanged")
     static let statusBarPremiumAccessChanged = Notification.Name("statusBarPremiumAccessChanged")
     static let statusBarSignOutRequested = Notification.Name("statusBarSignOutRequested")
     static let statusBarSignOutRequested = Notification.Name("statusBarSignOutRequested")
     static let meetingsSnapshotUpdated = Notification.Name("meetingsSnapshotUpdated")
     static let meetingsSnapshotUpdated = Notification.Name("meetingsSnapshotUpdated")
+    static let googleOAuthTokensDidChange = Notification.Name("googleOAuthTokensDidChange")
 }
 }