element-hq · pixlwave · Oct 29, 2024 · Oct 28, 2024 · Oct 29, 2024 · Oct 29, 2024
@@ -195,12 +195,6 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
 
         if let route = appRouteURLParser.route(from: url) {
             switch route {
-            case .oidcCallback(let url):
-                if stateMachine.state == .softLogout {
-                    softLogoutCoordinator?.handleOIDCRedirectURL(url)
-                } else {
-                    authenticationFlowCoordinator?.handleOIDCRedirectURL(url)
-                }
             case .genericCallLink(let url):
                 if let userSessionFlowCoordinator {
                     userSessionFlowCoordinator.handleAppRoute(route, animated: true)

@@ -153,14 +153,8 @@ final class AppSettings {
 
     /// Any pre-defined static client registrations for OIDC issuers.
     let oidcStaticRegistrations: [URL: String] = ["https://id.thirdroom.io/realms/thirdroom": "elementx"]
-    /// The redirect URL used for OIDC.
-    let oidcRedirectURL = {
-        guard let url = URL(string: "\(InfoPlistReader.main.appScheme):/callback") else {
-            fatalError("Invalid OIDC redirect URL")
-        }
-
-        return url
-    }()
+    /// The redirect URL used for OIDC. This no longer uses universal links so we don't need the bundle ID to avoid conflicts between Element X, Nightly and PR builds.
+    let oidcRedirectURL: URL = "https://element.io/oidc/login"
 
     private(set) lazy var oidcConfiguration = OIDCConfigurationProxy(clientName: InfoPlistReader.main.bundleDisplayName,
                                                                      redirectURI: oidcRedirectURL,

@@ -9,8 +9,6 @@ import Foundation
 import MatrixRustSDK
 
 enum AppRoute: Equatable {
-    /// The callback used to complete login with OIDC.
-    case oidcCallback(url: URL)
     /// The app's home screen.
     case roomList
     /// A room, shown as the root of the stack (popping any child rooms).
@@ -52,7 +50,6 @@ struct AppRouteURLParser {
         urlParsers = [
             MatrixPermalinkParser(),
             ElementWebURLParser(domains: appSettings.elementWebHosts),
-            OIDCCallbackURLParser(appSettings: appSettings),
             ElementCallURLParser()
         ]
     }
@@ -76,16 +73,6 @@ protocol URLParser {
     func route(from url: URL) -> AppRoute?
 }
 
-/// The parser for the OIDC callback URL. This always returns a `.oidcCallback`.
-struct OIDCCallbackURLParser: URLParser {
-    let appSettings: AppSettings
-
-    func route(from url: URL) -> AppRoute? {
-        guard url.absoluteString.starts(with: appSettings.oidcRedirectURL.absoluteString) else { return nil }
-        return .oidcCallback(url: url)
-    }
-}
-
 /// The parser for Element Call links. This always returns a `.genericCallLink`.
 struct ElementCallURLParser: URLParser {
     private let knownHosts = ["call.element.io"]

@@ -65,15 +65,6 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
         fatalError()
     }
 
-    func handleOIDCRedirectURL(_ url: URL) {
-        guard let oidcPresenter else {
-            MXLog.error("Failed to find an OIDC request in progress.")
-            return
-        }
-
-        oidcPresenter.handleUniversalLinkCallback(url)
-    }
-
     // MARK: - Private
 
     private func showStartScreen() {

@@ -163,7 +163,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
             }
         case .roomAlias, .childRoomAlias, .eventOnRoomAlias, .childEventOnRoomAlias:
             break // These are converted to a room ID route one level above.
-        case .roomList, .userProfile, .call, .genericCallLink, .oidcCallback, .settings, .chatBackupSettings:
+        case .roomList, .userProfile, .call, .genericCallLink, .settings, .chatBackupSettings:
             break // These routes can't be handled.
         }
     }

@@ -282,8 +282,6 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
             presentCallScreen(genericCallLink: url)
         case .settings, .chatBackupSettings:
             settingsFlowCoordinator.handleAppRoute(appRoute, animated: animated)
-        case .oidcCallback:
-            break
         }
     }
 

@@ -14,16 +14,6 @@ class OIDCAuthenticationPresenter: NSObject {
     private let oidcRedirectURL: URL
     private let presentationAnchor: UIWindow
 
-    /// The data required to complete a request.
-    struct Request {
-        let session: ASWebAuthenticationSession
-        let oidcData: OIDCAuthorizationDataProxy
-        let continuation: CheckedContinuation<Result<UserSessionProtocol, AuthenticationServiceError>, Never>
-    }
-
-    /// The current request in progress. This is a single use value and will be moved on access.
-    @Consumable private var request: Request?
-
     init(authenticationService: AuthenticationServiceProtocol, oidcRedirectURL: URL, presentationAnchor: UIWindow) {
         self.authenticationService = authenticationService
         self.oidcRedirectURL = oidcRedirectURL
@@ -33,74 +23,35 @@ class OIDCAuthenticationPresenter: NSObject {
 
     /// Presents a web authentication session for the supplied data.
     func authenticate(using oidcData: OIDCAuthorizationDataProxy) async -> Result<UserSessionProtocol, AuthenticationServiceError> {
-        await withCheckedContinuation { continuation in
-            let session = ASWebAuthenticationSession(url: oidcData.url,
-                                                     callbackURLScheme: oidcRedirectURL.scheme) { [weak self] url, error in
-                // This closure won't be called if the scheme is https, see handleUniversalLinkCallback for more info.
-                guard let self else { return }
-
-                guard let url else {
-                    // Check for user cancellation to avoid showing an alert in that instance.
-                    if let nsError = error as? NSError,
-                       nsError.domain == ASWebAuthenticationSessionErrorDomain,
-                       nsError.code == ASWebAuthenticationSessionError.canceledLogin.rawValue {
-                        self.completeAuthentication(throwing: .oidcError(.userCancellation))
-                        return
-                    }
-
-                    self.completeAuthentication(throwing: .oidcError(.unknown))
-                    return
-                }
-
-                completeAuthentication(callbackURL: url)
+        let (url, error) = await withCheckedContinuation { continuation in
+            let session = ASWebAuthenticationSession(url: oidcData.url, callback: .oidcRedirectURL(oidcRedirectURL)) { url, error in
+                continuation.resume(returning: (url, error))
             }
 
             session.prefersEphemeralWebBrowserSession = false
             session.presentationContextProvider = self
 
-            request = Request(session: session, oidcData: oidcData, continuation: continuation)
-
             session.start()
         }
-    }
-
-    /// This method will be used if the `appSettings.oidcRedirectURL`'s scheme is `https`.
-    /// When using a custom scheme, the redirect will be handled by the web auth session's closure.
-    func handleUniversalLinkCallback(_ url: URL) {
-        completeAuthentication(callbackURL: url)
-    }
-
-    /// Completes the authentication by exchanging the callback URL for a user session.
-    private func completeAuthentication(callbackURL: URL) {
-        guard let request else {
-            MXLog.error("Failed to complete authentication. Missing request.")
-            return
-        }
 
-        if callbackURL.scheme?.starts(with: "http") == true {
-            request.session.cancel()
-        }
-
-        Task {
-            switch await authenticationService.loginWithOIDCCallback(callbackURL, data: request.oidcData) {
-            case .success(let userSession):
-                request.continuation.resume(returning: .success(userSession))
-            case .failure(let error):
-                request.continuation.resume(returning: .failure(error))
+        guard let url else {
+            // Check for user cancellation to avoid showing an alert in that instance.
+            if let nsError = error as? NSError,
+               nsError.domain == ASWebAuthenticationSessionErrorDomain,
+               nsError.code == ASWebAuthenticationSessionError.canceledLogin.rawValue {
+                await authenticationService.abortOIDCLogin(data: oidcData)
+                return .failure(.oidcError(.userCancellation))
             }
-        }
-    }
-
-    /// Aborts the authentication with the supplied error.
-    private func completeAuthentication(throwing error: AuthenticationServiceError) {
-        guard let request else {
-            MXLog.error("Failed to throw authentication error. Missing request.")
-            return
+
+            await authenticationService.abortOIDCLogin(data: oidcData)
+            return .failure(.oidcError(.unknown))
         }
 
-        Task {
-            await authenticationService.abortOIDCLogin(data: request.oidcData)
-            request.continuation.resume(returning: .failure(error))
+        switch await authenticationService.loginWithOIDCCallback(url, data: oidcData) {
+        case .success(let userSession):
+            return .success(userSession)
+        case .failure(let error):
+            return .failure(error)
         }
     }
 }
@@ -110,3 +61,15 @@ class OIDCAuthenticationPresenter: NSObject {
 extension OIDCAuthenticationPresenter: ASWebAuthenticationPresentationContextProviding {
     func presentationAnchor(for session: ASWebAuthenticationSession) -> ASPresentationAnchor { presentationAnchor }
 }
+
+extension ASWebAuthenticationSession.Callback {
+    static func oidcRedirectURL(_ url: URL) -> Self {
+        if url.scheme == "https", let host = url.host() {
+            .https(host: host, path: url.path())
+        } else if let scheme = url.scheme {
+            .customScheme(scheme)
+        } else {
+            fatalError("Invalid OIDC redirect URL: \(url)")
+        }
+    }
+}
@@ -85,15 +85,6 @@ final class SoftLogoutScreenCoordinator: CoordinatorProtocol {
         AnyView(SoftLogoutScreen(context: viewModel.context))
     }
 
-    func handleOIDCRedirectURL(_ url: URL) {
-        guard let oidcPresenter else {
-            MXLog.error("Failed to find an OIDC request in progress.")
-            return
-        }
-
-        oidcPresenter.handleUniversalLinkCallback(url)
-    }
-
     // MARK: - Private
 
     private static let loadingIndicatorIdentifier = "\(SoftLogoutScreenCoordinator.self)-Loading"

@@ -28,7 +28,7 @@ class OIDCAccountSettingsPresenter: NSObject {
 
     /// Presents a web authentication session for the supplied data.
     func start() {
-        let session = ASWebAuthenticationSession(url: accountURL, callbackURLScheme: oidcRedirectURL.scheme) { _, _ in }
+        let session = ASWebAuthenticationSession(url: accountURL, callback: .oidcRedirectURL(oidcRedirectURL)) { _, _ in }
         session.prefersEphemeralWebBrowserSession = false
         session.presentationContextProvider = self
         session.start()

@@ -73,33 +73,6 @@ class AppRouteURLParserTests: XCTestCase {
         XCTAssertEqual(appRouteURLParser.route(from: customSchemeURL), nil)
     }
 
-    func testOIDCCallbackRoute() {
-        // Given an OIDC callback for this app.
-        let callbackURL = appSettings.oidcRedirectURL.appending(queryItems: [URLQueryItem(name: "state", value: "12345"),
-                                                                             URLQueryItem(name: "code", value: "67890")])
-
-        // When parsing that route.
-        let route = appRouteURLParser.route(from: callbackURL)
-
-        // Then it should be considered a valid OIDC callback.
-        XCTAssertEqual(route, AppRoute.oidcCallback(url: callbackURL))
-    }
-
-    func testOIDCCallbackAppVariantRoute() {
-        // Given an OIDC callback for a different app variant.
-        let callbackURL = appSettings.oidcRedirectURL
-            .deletingLastPathComponent()
-            .appending(component: "elementz")
-            .appending(queryItems: [URLQueryItem(name: "state", value: "12345"),
-                                    URLQueryItem(name: "code", value: "67890")])
-
-        // When parsing that route in this app.
-        let route = appRouteURLParser.route(from: callbackURL)
-
-        // Then the route shouldn't be considered valid and should be ignored.
-        XCTAssertEqual(route, nil)
-    }
-
     func testMatrixUserURL() {
         let userID = "@test:matrix.org"
         guard let url = URL(string: "https://matrix.to/#/\(userID)") else {