Переглянути джерело

Fix Google OAuth logout and in-app sign-in

- Add Keychain OSStatus details to error alerts for easier debugging.
- Avoid logout loops by migrating legacy Keychain tokens to a stable service and ignoring legacy tokens after sign-out.
- Restore in-app OAuth (WKWebView) flow instead of opening the default browser.

Co-authored-by: Cursor <cursoragent@cursor.com>
huzaifahayat12 2 місяців тому
батько
коміт
cc4360bba4

+ 16 - 12
meetings_app/Assets.xcassets/StatusBarIcon.imageset/Contents.json

@@ -1,21 +1,25 @@
 {
-  "images": [
+  "images" : [
     {
-      "idiom": "universal",
-      "filename": "StatusBarIcon@1x.png",
-      "scale": "1x"
+      "filename" : "StatusBarIcon@1x.png",
+      "idiom" : "universal",
+      "scale" : "1x"
     },
     {
-      "idiom": "universal",
-      "filename": "StatusBarIcon@2x.png",
-      "scale": "2x"
+      "filename" : "StatusBarIcon@2x.png",
+      "idiom" : "universal",
+      "scale" : "2x"
+    },
+    {
+      "idiom" : "universal",
+      "scale" : "3x"
     }
   ],
-  "info": {
-    "version": 1,
-    "author": "xcode"
+  "info" : {
+    "author" : "xcode",
+    "version" : 1
   },
-  "properties": {
-    "template-rendering-intent": "template"
+  "properties" : {
+    "template-rendering-intent" : "template"
   }
 }

+ 66 - 9
meetings_app/Auth/GoogleOAuthService.swift

@@ -52,11 +52,17 @@ final class GoogleOAuthService: NSObject {
         "https://www.googleapis.com/auth/meetings.space.readonly"
     ]
 
-    private let tokenStore = KeychainTokenStore()
+    /// Use a stable, app-controlled Keychain "service" identifier so tokens remain manageable even if
+    /// bundle identifier or signing identity changes between builds.
+    private static let keychainService = "ai.companion.for.meet.google.oauth.tokens"
+    private let tokenStore = KeychainTokenStore(service: GoogleOAuthService.keychainService)
+    /// Legacy store used by older builds that keyed off the bundle identifier.
+    private let legacyTokenStore = KeychainTokenStore()
     @MainActor private var inAppOAuthWindowController: InAppOAuthWindowController?
     private override init() {}
     
     private let lastSignedInEmailDefaultsKey = "google.oauth.lastSignedInEmail"
+    private let ignoreLegacyTokensDefaultsKey = "google.oauth.ignoreLegacyTokens"
     
     func lastSignedInEmailHint() -> String? {
         let value = UserDefaults.standard
@@ -113,18 +119,40 @@ final class GoogleOAuthService: NSObject {
 
     func signOut() throws {
         try tokenStore.deleteTokens()
+        UserDefaults.standard.set(true, forKey: ignoreLegacyTokensDefaultsKey)
+        // Best-effort cleanup for legacy items. Some old items cannot be deleted after signing changes.
+        try? legacyTokenStore.deleteTokens()
     }
 
     func signOutAndRevoke() async throws {
-        let existing = try tokenStore.readTokens()
-        if let existing {
-            try await revokeTokenIfPossible(existing)
-        }
+        // Ensure tokens are in the current store so sign-out is effective.
+        let existing = try loadTokensOrMigrateIfNeeded()
+        if let existing { try await revokeTokenIfPossible(existing) }
         try tokenStore.deleteTokens()
+        UserDefaults.standard.set(true, forKey: ignoreLegacyTokensDefaultsKey)
+        try? legacyTokenStore.deleteTokens()
     }
 
     func loadTokens() -> GoogleOAuthTokens? {
-        try? tokenStore.readTokens()
+        try? loadTokensOrMigrateIfNeeded()
+    }
+    
+    /// 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.
+        if UserDefaults.standard.bool(forKey: ignoreLegacyTokensDefaultsKey) {
+            return nil
+        }
+        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? legacyTokenStore.deleteTokens()
+            return legacy
+        }
+        return nil
     }
 
     func fetchUserProfile(accessToken: String) async throws -> GoogleUserProfile {
@@ -146,7 +174,7 @@ final class GoogleOAuthService: NSObject {
     }
 
     func validAccessToken(presentingWindow: NSWindow?) async throws -> String {
-        if var tokens = try tokenStore.readTokens() {
+        if var tokens = try loadTokensOrMigrateIfNeeded() {
             if tokens.expiresAt.timeIntervalSinceNow > 60 {
                 return tokens.accessToken
             }
@@ -159,6 +187,7 @@ final class GoogleOAuthService: NSObject {
 
         let tokens = try await interactiveSignIn(presentingWindow: presentingWindow)
         try tokenStore.writeTokens(tokens)
+        UserDefaults.standard.set(false, forKey: ignoreLegacyTokensDefaultsKey)
         return tokens.accessToken
     }
 
@@ -201,11 +230,25 @@ final class GoogleOAuthService: NSObject {
         components.queryItems = queryItems
 
         guard let authURL = components.url else { throw GoogleOAuthError.invalidCallbackURL }
+        
         let opened = await MainActor.run { [self] in
-            openAuthURLInDefaultBrowser(authURL)
+            openAuthURLInAppBrowser(authURL, presentingWindow: presentingWindow)
         }
         guard opened else { throw GoogleOAuthError.unableToOpenBrowser }
-        let callbackURL = try await loopback.waitForCallback()
+        
+        let callbackURL: URL
+        do {
+            callbackURL = try await loopback.waitForCallback()
+        } catch {
+            await MainActor.run { [weak self] in
+                self?.closeInAppOAuthWindow()
+            }
+            throw error
+        }
+        
+        await MainActor.run { [weak self] in
+            self?.closeInAppOAuthWindow()
+        }
 
         guard let returnedState = URLComponents(url: callbackURL, resolvingAgainstBaseURL: false)?
             .queryItems?.first(where: { $0.name == "state" })?.value,
@@ -232,6 +275,20 @@ final class GoogleOAuthService: NSObject {
     private func openAuthURLInDefaultBrowser(_ url: URL) -> Bool {
         NSWorkspace.shared.open(url)
     }
+    
+    @MainActor
+    private func openAuthURLInAppBrowser(_ url: URL, presentingWindow: NSWindow?) -> Bool {
+        if inAppOAuthWindowController == nil {
+            inAppOAuthWindowController = InAppOAuthWindowController()
+        }
+        guard let controller = inAppOAuthWindowController else { return false }
+        controller.alignWithPresentingWindow(presentingWindow)
+        controller.showWindow(nil)
+        controller.window?.makeKeyAndOrderFront(nil)
+        NSApp.activate(ignoringOtherApps: true)
+        controller.load(url: url)
+        return true
+    }
 
     @MainActor
     private func closeInAppOAuthWindow() {

+ 21 - 20
meetings_app/Auth/KeychainTokenStore.swift

@@ -14,6 +14,23 @@ final class KeychainTokenStore {
         self.account = account
         self.accessGroup = accessGroup
     }
+    
+    private func makeKeychainError(status: OSStatus, message: String) -> NSError {
+        let statusMessage = SecCopyErrorMessageString(status, nil) as String? ?? "OSStatus \(status)"
+        return NSError(
+            domain: NSOSStatusErrorDomain,
+            code: Int(status),
+            userInfo: [
+                NSLocalizedDescriptionKey: message,
+                NSLocalizedFailureReasonErrorKey: statusMessage,
+                "keychainStatus": Int(status),
+                "keychainStatusMessage": statusMessage,
+                "keychainService": service,
+                "keychainAccount": account,
+                "keychainAccessGroup": accessGroup as Any
+            ]
+        )
+    }
 
     func readTokens() throws -> GoogleOAuthTokens? {
         var query: [String: Any] = [
@@ -31,11 +48,7 @@ final class KeychainTokenStore {
         let status = SecItemCopyMatching(query as CFDictionary, &result)
         if status == errSecItemNotFound { return nil }
         guard status == errSecSuccess else {
-            throw NSError(
-                domain: NSOSStatusErrorDomain,
-                code: Int(status),
-                userInfo: [NSLocalizedDescriptionKey: "Failed to read OAuth tokens from Keychain."]
-            )
+            throw makeKeychainError(status: status, message: "Failed to read OAuth tokens from Keychain.")
         }
         guard let data = result as? Data else {
             throw NSError(
@@ -65,11 +78,7 @@ final class KeychainTokenStore {
         let updateStatus = SecItemUpdate(baseQuery as CFDictionary, attributesToUpdate as CFDictionary)
         if updateStatus == errSecSuccess { return }
         if updateStatus != errSecItemNotFound {
-            throw NSError(
-                domain: NSOSStatusErrorDomain,
-                code: Int(updateStatus),
-                userInfo: [NSLocalizedDescriptionKey: "Failed to update OAuth tokens in Keychain."]
-            )
+            throw makeKeychainError(status: updateStatus, message: "Failed to update OAuth tokens in Keychain.")
         }
 
         var addQuery = baseQuery
@@ -77,11 +86,7 @@ final class KeychainTokenStore {
         addQuery[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlock
         let addStatus = SecItemAdd(addQuery as CFDictionary, nil)
         guard addStatus == errSecSuccess else {
-            throw NSError(
-                domain: NSOSStatusErrorDomain,
-                code: Int(addStatus),
-                userInfo: [NSLocalizedDescriptionKey: "Failed to save OAuth tokens to Keychain."]
-            )
+            throw makeKeychainError(status: addStatus, message: "Failed to save OAuth tokens to Keychain.")
         }
     }
 
@@ -97,11 +102,7 @@ final class KeychainTokenStore {
 
         let status = SecItemDelete(query as CFDictionary)
         guard status == errSecSuccess || status == errSecItemNotFound else {
-            throw NSError(
-                domain: NSOSStatusErrorDomain,
-                code: Int(status),
-                userInfo: [NSLocalizedDescriptionKey: "Failed to delete OAuth tokens from Keychain."]
-            )
+            throw makeKeychainError(status: status, message: "Failed to delete OAuth tokens from Keychain.")
         }
     }
 }

+ 21 - 1
meetings_app/ViewController.swift

@@ -10277,7 +10277,27 @@ private extension ViewController {
             let alert = NSAlert()
             alert.alertStyle = .warning
             alert.messageText = title
-            alert.informativeText = error.localizedDescription
+            var lines: [String] = []
+            lines.append(error.localizedDescription)
+            
+            if let localizedError = error as? LocalizedError, let reason = localizedError.failureReason, reason.isEmpty == false {
+                lines.append(reason)
+            }
+            
+            let nsError = error as NSError
+            if let status = nsError.userInfo["keychainStatus"] as? Int,
+               let statusMessage = nsError.userInfo["keychainStatusMessage"] as? String {
+                lines.append("Keychain: \(status) (\(statusMessage))")
+            } else if nsError.domain == NSOSStatusErrorDomain {
+                lines.append("OSStatus: \(nsError.code)")
+            }
+            
+            if let service = nsError.userInfo["keychainService"] as? String,
+               let account = nsError.userInfo["keychainAccount"] as? String {
+                lines.append("Item: service=\(service), account=\(account)")
+            }
+            
+            alert.informativeText = lines.joined(separator: "\n")
             alert.addButton(withTitle: "OK")
             alert.runModal()
         }