Explorar el Código

Fix Google and Apple OAuth sign-in reliability and align macOS target to 14.4.

Coalesce duplicate OAuth triggers, add session timeout recovery, improve callback host parsing and completion detection, and restore login modal dismiss controls.

Co-authored-by: Cursor <cursoragent@cursor.com>
AhtashamShahzad1 hace 1 mes
padre
commit
c901898e23

+ 4 - 4
Reddit App.xcodeproj/project.pbxproj

@@ -178,7 +178,7 @@
 				GCC_WARN_UNUSED_FUNCTION = YES;
 				GCC_WARN_UNUSED_VARIABLE = YES;
 				LOCALIZATION_PREFERS_STRING_CATALOGS = YES;
-				MACOSX_DEPLOYMENT_TARGET = 14.0;
+				MACOSX_DEPLOYMENT_TARGET = 14.4;
 				MTL_ENABLE_DEBUG_INFO = INCLUDE_SOURCE;
 				MTL_FAST_MATH = YES;
 				ONLY_ACTIVE_ARCH = YES;
@@ -235,7 +235,7 @@
 				GCC_WARN_UNUSED_FUNCTION = YES;
 				GCC_WARN_UNUSED_VARIABLE = YES;
 				LOCALIZATION_PREFERS_STRING_CATALOGS = YES;
-				MACOSX_DEPLOYMENT_TARGET = 14.0;
+				MACOSX_DEPLOYMENT_TARGET = 14.4;
 				MTL_ENABLE_DEBUG_INFO = NO;
 				MTL_FAST_MATH = YES;
 				SDKROOT = macosx;
@@ -262,7 +262,7 @@
 					"$(inherited)",
 					"@executable_path/../Frameworks",
 				);
-				MACOSX_DEPLOYMENT_TARGET = 14.0;
+				MACOSX_DEPLOYMENT_TARGET = 14.4;
 				MARKETING_VERSION = 1.0;
 				PRODUCT_BUNDLE_IDENTIFIER = com.Reddora;
 				PRODUCT_NAME = "$(TARGET_NAME)";
@@ -295,7 +295,7 @@
 					"$(inherited)",
 					"@executable_path/../Frameworks",
 				);
-				MACOSX_DEPLOYMENT_TARGET = 14.0;
+				MACOSX_DEPLOYMENT_TARGET = 14.4;
 				MARKETING_VERSION = 1.0;
 				PRODUCT_BUNDLE_IDENTIFIER = com.Reddora;
 				PRODUCT_NAME = "$(TARGET_NAME)";

+ 55 - 30
Reddit App/Managers/OAuthAuthenticationManager.swift

@@ -2,11 +2,12 @@ import AppKit
 import AuthenticationServices
 import WebKit
 
-enum OAuthError: LocalizedError {
+enum OAuthError: LocalizedError, Equatable {
     case alreadyInProgress
     case failedToStart
     case cancelled
     case invalidCallback
+    case timedOut
     case unsupportedPlatform
 
     var errorDescription: String? {
@@ -19,6 +20,8 @@ enum OAuthError: LocalizedError {
             "Sign-in was cancelled."
         case .invalidCallback:
             "Sign-in did not return to Reddit."
+        case .timedOut:
+            "Sign-in timed out. Please try again."
         case .unsupportedPlatform:
             "Sign-in requires macOS 14.4 or later. Please update your system."
         }
@@ -29,8 +32,13 @@ enum OAuthError: LocalizedError {
 final class OAuthAuthenticationManager: NSObject {
     static let shared = OAuthAuthenticationManager()
 
+    private static let authTimeoutSeconds: UInt64 = 300
+
     private var authSession: ASWebAuthenticationSession?
     private var isAuthenticating = false
+    private var currentAuthURL: URL?
+    private var pendingCompletions: [(Result<URL, Error>) -> Void] = []
+    private var authTimeoutTask: Task<Void, Never>?
     private weak var presentationWindow: NSWindow?
 
     private override init() {
@@ -42,9 +50,12 @@ final class OAuthAuthenticationManager: NSObject {
         presentationWindow: NSWindow? = nil,
         completion: @escaping (Result<URL, Error>) -> Void
     ) {
-        guard !isAuthenticating else {
-            // Reddit's Google/Apple buttons can fire both decidePolicyFor and
-            // createWebViewWith for the same OAuth URL; ignore the duplicate.
+        if isAuthenticating {
+            if let currentAuthURL, currentAuthURL.absoluteString == url.absoluteString {
+                pendingCompletions.append(completion)
+                return
+            }
+            completion(.failure(OAuthError.alreadyInProgress))
             return
         }
 
@@ -52,7 +63,6 @@ final class OAuthAuthenticationManager: NSObject {
             self.presentationWindow = presentationWindow
             startAuthenticationSession(url: url, completion: completion)
         } else {
-            NSWorkspace.shared.open(url)
             completion(.failure(OAuthError.unsupportedPlatform))
         }
     }
@@ -63,6 +73,8 @@ final class OAuthAuthenticationManager: NSObject {
         completion: @escaping (Result<URL, Error>) -> Void
     ) {
         isAuthenticating = true
+        currentAuthURL = url
+        pendingCompletions = [completion]
         NSApp.activate(ignoringOtherApps: true)
 
         let callbackHost = RedditWebAuthHelper.oauthCallbackHost(for: url)
@@ -72,11 +84,7 @@ final class OAuthAuthenticationManager: NSObject {
             callback: .https(host: callbackHost, path: "")
         ) { [weak self] callbackURL, error in
             Task { @MainActor in
-                self?.finishAuthentication(
-                    callbackURL: callbackURL,
-                    error: error,
-                    completion: completion
-                )
+                self?.finishAuthentication(callbackURL: callbackURL, error: error)
             }
         }
 
@@ -84,41 +92,58 @@ final class OAuthAuthenticationManager: NSObject {
         session.presentationContextProvider = self
         authSession = session
 
+        startAuthTimeout()
+
         guard session.start() else {
-            isAuthenticating = false
-            authSession = nil
-            presentationWindow = nil
-            completion(.failure(OAuthError.failedToStart))
+            deliverResult(.failure(OAuthError.failedToStart))
             return
         }
     }
 
-    private func finishAuthentication(
-        callbackURL: URL?,
-        error: Error?,
-        completion: @escaping (Result<URL, Error>) -> Void
-    ) {
-        isAuthenticating = false
-        authSession = nil
-        presentationWindow = nil
+    private func startAuthTimeout() {
+        authTimeoutTask?.cancel()
+        authTimeoutTask = Task { [weak self] in
+            try? await Task.sleep(nanoseconds: Self.authTimeoutSeconds * 1_000_000_000)
+            guard !Task.isCancelled else { return }
+            await MainActor.run {
+                guard let self, self.isAuthenticating else { return }
+                self.authSession?.cancel()
+                self.deliverResult(.failure(OAuthError.timedOut))
+            }
+        }
+    }
+
+    private func finishAuthentication(callbackURL: URL?, error: Error?) {
+        let result: Result<URL, Error>
 
         if let error {
             let nsError = error as NSError
             if nsError.domain == ASWebAuthenticationSessionErrorDomain,
                nsError.code == ASWebAuthenticationSessionError.canceledLogin.rawValue {
-                completion(.failure(OAuthError.cancelled))
+                result = .failure(OAuthError.cancelled)
             } else {
-                completion(.failure(error))
+                result = .failure(error)
             }
-            return
+        } else if let callbackURL, RedditWebAuthHelper.isRedditURL(callbackURL) {
+            result = .success(callbackURL)
+        } else {
+            result = .failure(OAuthError.invalidCallback)
         }
 
-        guard let callbackURL, RedditWebAuthHelper.isRedditURL(callbackURL) else {
-            completion(.failure(OAuthError.invalidCallback))
-            return
-        }
+        deliverResult(result)
+    }
+
+    private func deliverResult(_ result: Result<URL, Error>) {
+        authTimeoutTask?.cancel()
+        authTimeoutTask = nil
+        isAuthenticating = false
+        authSession = nil
+        presentationWindow = nil
+        currentAuthURL = nil
 
-        completion(.success(callbackURL))
+        let completions = pendingCompletions
+        pendingCompletions = []
+        completions.forEach { $0(result) }
     }
 }
 

+ 18 - 24
Reddit App/Managers/RedditOAuthPanelController.swift

@@ -9,6 +9,7 @@ final class RedditOAuthPanelController: NSObject {
     private var webView: WKWebView?
     private var onComplete: (() -> Void)?
     private var onAuthError: ((String) -> Void)?
+    private var finishWorkItem: DispatchWorkItem?
 
     private override init() {
         super.init()
@@ -51,6 +52,8 @@ final class RedditOAuthPanelController: NSObject {
     }
 
     func dismiss() {
+        finishWorkItem?.cancel()
+        finishWorkItem = nil
         panel?.close()
         panel = nil
         webView = nil
@@ -64,6 +67,17 @@ final class RedditOAuthPanelController: NSObject {
         completion?()
     }
 
+    private func scheduleFinishIfNeeded(for url: URL) {
+        guard RedditWebAuthHelper.shouldCompleteOAuthSignIn(for: url) else { return }
+
+        finishWorkItem?.cancel()
+        let workItem = DispatchWorkItem { [weak self] in
+            self?.finish()
+        }
+        finishWorkItem = workItem
+        DispatchQueue.main.asyncAfter(deadline: .now() + 0.75, execute: workItem)
+    }
+
     private func beginOAuthSignIn(url: URL, in webView: WKWebView) {
         OAuthAuthenticationManager.shared.authenticate(
             url: url,
@@ -73,6 +87,8 @@ final class RedditOAuthPanelController: NSObject {
             switch result {
             case .success(let callbackURL):
                 webView.load(URLRequest(url: callbackURL))
+            case .failure(let error as OAuthError) where error == .cancelled:
+                break
             case .failure(let error):
                 self.onAuthError?(error.localizedDescription)
                 self.finish()
@@ -83,11 +99,8 @@ final class RedditOAuthPanelController: NSObject {
 
 extension RedditOAuthPanelController: WKNavigationDelegate {
     func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) {
-        guard let url = webView.url, RedditWebAuthHelper.isRedditURL(url) else { return }
-
-        if shouldFinish(for: url) {
-            finish()
-        }
+        guard let url = webView.url else { return }
+        scheduleFinishIfNeeded(for: url)
     }
 
     func webView(
@@ -106,27 +119,8 @@ extension RedditOAuthPanelController: WKNavigationDelegate {
             return
         }
 
-        if shouldFinish(for: url) {
-            decisionHandler(.allow)
-            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) { [weak self] in
-                self?.finish()
-            }
-            return
-        }
-
         decisionHandler(.allow)
     }
-
-    private func shouldFinish(for url: URL) -> Bool {
-        guard let host = url.host?.lowercased() else { return false }
-        let path = url.path.lowercased()
-
-        if host == "www.reddit.com" || host == "reddit.com" {
-            return !path.contains("login") && !path.contains("register")
-        }
-
-        return RedditWebAuthHelper.looksLikeLoggedInURL(url)
-    }
 }
 
 extension RedditOAuthPanelController: WKUIDelegate {

+ 2 - 17
Reddit App/Utilities/RedditAuthChromeStyle.swift

@@ -1,27 +1,12 @@
 import Foundation
 
 enum RedditAuthChromeStyle {
-    /// Hides Reddit auth modal close buttons in the embedded WebView.
+    /// Placeholder for optional Reddit auth UI tweaks. Close buttons are left visible
+    /// so users can dismiss the login modal if sign-in fails or stalls.
     static let injectionScript = """
     (function() {
       const STYLE_ID = 'reddora-auth-chrome-style';
       if (document.getElementById(STYLE_ID)) return;
-
-      const style = document.createElement('style');
-      style.id = STYLE_ID;
-      style.textContent = `
-        auth-flow-modal button[aria-label="Close"],
-        auth-flow-modal button[aria-label="close"],
-        faceplate-modal button[aria-label="Close"],
-        shreddit-signup-drawer button[aria-label="Close"],
-        [role="dialog"] button[aria-label="Close"] {
-          display: none !important;
-          visibility: hidden !important;
-          pointer-events: none !important;
-        }
-      `;
-
-      (document.head || document.documentElement).appendChild(style);
     })();
     """
 }

+ 101 - 32
Reddit App/Utilities/RedditWebAuthHelper.swift

@@ -3,23 +3,33 @@ import Foundation
 enum RedditWebAuthHelper {
     static let redditHomeURL = URL(string: "https://www.reddit.com")!
 
+    private static let oauthHosts = [
+        "accounts.google.com",
+        "appleid.apple.com",
+        "idmsa.apple.com",
+        "gsa.apple.com",
+        "login.microsoftonline.com",
+        "login.live.com",
+        "account.live.com",
+        "login.microsoft.com",
+        "www.facebook.com",
+        "m.facebook.com",
+        "facebook.com",
+        "auth0.com",
+    ]
+
+    private static let authFlowPathPrefixes = [
+        "/login",
+        "/register",
+        "/password",
+        "/account/login",
+        "/account/register",
+        "/account/signup",
+    ]
+
     static func isOAuthProviderURL(_ url: URL) -> Bool {
         guard let host = url.host?.lowercased() else { return false }
 
-        let oauthHosts = [
-            "accounts.google.com",
-            "appleid.apple.com",
-            "idmsa.apple.com",
-            "login.microsoftonline.com",
-            "login.live.com",
-            "account.live.com",
-            "login.microsoft.com",
-            "www.facebook.com",
-            "m.facebook.com",
-            "facebook.com",
-            "auth0.com",
-        ]
-
         if oauthHosts.contains(where: { host == $0 || host.hasSuffix(".\($0)") }) {
             return true
         }
@@ -31,6 +41,13 @@ enum RedditWebAuthHelper {
                 || path.contains("gsi")
         }
 
+        if host == "apple.com" || host.hasSuffix(".apple.com") {
+            let path = url.path.lowercased()
+            return path.contains("auth")
+                || path.contains("signin")
+                || path.contains("sso")
+        }
+
         if host == "facebook.com" || host.hasSuffix(".facebook.com") {
             let path = url.path.lowercased()
             return path.contains("oauth") || path.contains("login") || path.contains("dialog")
@@ -74,43 +91,95 @@ enum RedditWebAuthHelper {
         return redditHosts.contains(where: { host == $0 || host.hasSuffix(".\($0)") })
     }
 
-    static func looksLikeLoggedInURL(_ url: URL) -> Bool {
+    static func isAuthFlowURL(_ url: URL) -> Bool {
         guard isRedditURL(url) else { return false }
 
         let path = url.path.lowercased()
-        let blockedPaths = [
-            "/login",
-            "/register",
-            "/password",
-            "/account/login",
-        ]
-
-        if blockedPaths.contains(where: { path.hasPrefix($0) }) {
-            return false
+        return authFlowPathPrefixes.contains { prefix in
+            path == prefix || path.hasPrefix("\(prefix)/")
         }
+    }
+
+    static func looksLikeLoggedInURL(_ url: URL) -> Bool {
+        guard isRedditURL(url) else { return false }
+        if isAuthFlowURL(url) { return false }
+
+        let path = url.path.lowercased()
 
         if url.host?.lowercased() == "accounts.reddit.com" {
+            if path.contains("/oauth") || path.contains("/authorize") || path.contains("/login") {
+                return false
+            }
             return path.isEmpty || path == "/" || path.hasPrefix("/api/")
         }
 
         return true
     }
 
+    /// Returns true when the OAuth completion panel can close and hand off to the main WebView.
+    static func shouldCompleteOAuthSignIn(for url: URL) -> Bool {
+        guard isRedditURL(url) else { return false }
+        if isAuthFlowURL(url) { return false }
+
+        let host = url.host?.lowercased() ?? ""
+        let path = url.path.lowercased()
+
+        if host == "accounts.reddit.com" {
+            if path.contains("/oauth")
+                || path.contains("/authorize")
+                || path.contains("/login")
+                || path.hasPrefix("/api/") {
+                return false
+            }
+            return path.isEmpty || path == "/"
+        }
+
+        if host == "www.reddit.com"
+            || host == "reddit.com"
+            || host == "new.reddit.com"
+            || host == "old.reddit.com" {
+            return true
+        }
+
+        return looksLikeLoggedInURL(url)
+    }
+
     static func oauthCallbackHost(for url: URL) -> String {
+        if let host = parsedRedirectHost(from: url) {
+            return host
+        }
+        return "accounts.reddit.com"
+    }
+
+    private static func parsedRedirectHost(from url: URL) -> String? {
         guard let components = URLComponents(url: url, resolvingAgainstBaseURL: false) else {
-            return "accounts.reddit.com"
+            return nil
         }
 
-        let redirectURI = components.queryItems?.first(where: { $0.name == "redirect_uri" })?.value
-            ?? components.queryItems?.first(where: { $0.name == "redirect_url" })?.value
+        let redirectValue = components.queryItems?.first(where: {
+            $0.name == "redirect_uri" || $0.name == "redirect_url" || $0.name == "redirect"
+        })?.value
+
+        guard let redirectValue else { return nil }
 
-        if let redirectURI,
-           let redirectURL = URL(string: redirectURI),
-           let host = redirectURL.host?.lowercased(),
-           isRedditURL(redirectURL) {
+        let decoded = redirectValue.removingPercentEncoding ?? redirectValue
+        if let host = hostFromRedirectString(decoded) {
             return host
         }
 
-        return "accounts.reddit.com"
+        if !decoded.contains("://"), let host = hostFromRedirectString("https://\(decoded)") {
+            return host
+        }
+
+        return nil
+    }
+
+    private static func hostFromRedirectString(_ value: String) -> String? {
+        guard let redirectURL = URL(string: value),
+              let host = redirectURL.host?.lowercased(),
+              isRedditURL(redirectURL) else {
+            return nil
+        }
+        return host
     }
 }