Use a universal link for the OIDC callback. (#1681)

ElementX/Sources/Application/AppCoordinator.swift

@@ -45,6 +45,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationCoordinatorDelegate,
     
     private var userSessionFlowCoordinator: UserSessionFlowCoordinator?
     private var authenticationCoordinator: AuthenticationCoordinator?
+    private var softLogoutCoordinator: SoftLogoutScreenCoordinator?
     
     private let backgroundTaskService: BackgroundTaskServiceProtocol
 
@@ -145,6 +146,12 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationCoordinatorDelegate,
         
         if let route = appRouteURLParser.route(from: url) {
             switch route {
+            case .oidcCallback(let url):
+                if stateMachine.state == .softLogout {
+                    softLogoutCoordinator?.handleOIDCRedirectURL(url)
+                } else {
+                    authenticationCoordinator?.handleOIDCRedirectURL(url)
+                }
             case .genericCallLink(let url):
                 if let userSessionFlowCoordinator {
                     userSessionFlowCoordinator.handleAppRoute(route, animated: true)
@@ -158,11 +165,6 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationCoordinatorDelegate,
             return true
         }
         
-        // Until we have an OIDC callback AppRoute, handle it manually.
-        if url.absoluteString.starts(with: appSettings.oidcRedirectURL.absoluteString) {
-            MXLog.error("OIDC callback through Universal Links not implemented.")
-        }
-        
         return false
     }
     
@@ -170,6 +172,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationCoordinatorDelegate,
     
     func authenticationCoordinator(_ authenticationCoordinator: AuthenticationCoordinator, didLoginWithSession userSession: UserSessionProtocol) {
         self.userSession = userSession
+        self.authenticationCoordinator = nil
         stateMachine.processEvent(.createdUserSession)
     }
     
@@ -305,8 +308,10 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationCoordinatorDelegate,
                 break
             case (_, .signOut(let isSoft), .signingOut):
                 self.logout(isSoft: isSoft)
-            case (.signingOut, .completedSigningOut(let isSoft), .signedOut):
+            case (.signingOut, .completedSigningOut, .signedOut):
-                self.presentSplashScreen(isSoftLogout: isSoft)
+                self.presentSplashScreen(isSoftLogout: false)
+            case (.signingOut, .showSoftLogout, .softLogout):
+                self.presentSplashScreen(isSoftLogout: true)
             case (.signedIn, .clearCache, .initial):
                 self.clearCache()
             default:
@@ -370,7 +375,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationCoordinatorDelegate,
                                                                    keyBackupNeeded: false,
                                                                    userIndicatorController: ServiceLocator.shared.userIndicatorController)
             let coordinator = SoftLogoutScreenCoordinator(parameters: parameters)
-            
+            self.softLogoutCoordinator = coordinator
             coordinator.actions
                 .sink { [weak self] action in
                     guard let self else { return }
@@ -378,8 +383,10 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationCoordinatorDelegate,
                     switch action {
                     case .signedIn(let session):
                         self.userSession = session
+                        self.softLogoutCoordinator = nil
                         stateMachine.processEvent(.createdUserSession)
                     case .clearAllData:
+                        self.softLogoutCoordinator = nil
                         stateMachine.processEvent(.signOut(isSoft: false))
                     }
                 }
@@ -437,7 +444,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationCoordinatorDelegate,
         userSessionFlowCoordinator?.stop()
         
         guard !isSoft else {
-            stateMachine.processEvent(.completedSigningOut(isSoft: isSoft))
+            stateMachine.processEvent(.showSoftLogout)
             hideLoadingIndicator()
             return
         }
@@ -459,7 +466,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationCoordinatorDelegate,
             ServiceLocator.shared.analytics.optOut()
             ServiceLocator.shared.analytics.resetConsentState()
             
-            stateMachine.processEvent(.completedSigningOut(isSoft: isSoft))
+            stateMachine.processEvent(.completedSigningOut)
             
             // Handle OIDC's RP-Initiated Logout if needed. Don't fallback to an ASWebAuthenticationSession
             // as it looks weird to show an alert to the user asking them to sign in to their provider.

ElementX/Sources/Application/AppCoordinatorStateMachine.swift

@@ -22,8 +22,10 @@ class AppCoordinatorStateMachine {
     enum State: StateType {
         /// The initial state, used before the AppCoordinator starts
         case initial
-        /// Showing the login screen
+        /// Showing the authentication flow
         case signedOut
+        /// Showing the soft logout flow
+        case softLogout
         /// Opening an existing session.
         case restoringSession
         
@@ -45,15 +47,17 @@ class AppCoordinatorStateMachine {
         /// Restoring session failed.
         case failedRestoringSession
         
-        /// A session has been created
+        /// A session has been created.
         case createdUserSession
         
-        /// Request sign out
+        /// Request sign out.
         case signOut(isSoft: Bool)
-        /// Signing out completed
+        /// Request the soft logout screen.
-        case completedSigningOut(isSoft: Bool)
+        case showSoftLogout
+        /// Signing out completed.
+        case completedSigningOut
         
-        /// Request cache clearing
+        /// Request cache clearing.
         case clearCache
     }
     
@@ -70,11 +74,15 @@ class AppCoordinatorStateMachine {
 
     private func configure() {
         stateMachine.addRoutes(event: .startWithAuthentication, transitions: [.initial => .signedOut])
-        stateMachine.addRoutes(event: .createdUserSession, transitions: [.signedOut => .signedIn])
+        stateMachine.addRoutes(event: .createdUserSession, transitions: [.signedOut => .signedIn,
+                                                                         .softLogout => .signedIn])
         stateMachine.addRoutes(event: .startWithExistingSession, transitions: [.initial => .restoringSession])
         stateMachine.addRoutes(event: .createdUserSession, transitions: [.restoringSession => .signedIn])
         stateMachine.addRoutes(event: .failedRestoringSession, transitions: [.restoringSession => .signedOut])
         
+        stateMachine.addRoutes(event: .completedSigningOut, transitions: [.signingOut(isSoft: false) => .signedOut])
+        stateMachine.addRoutes(event: .showSoftLogout, transitions: [.signingOut(isSoft: true) => .softLogout])
+        
         stateMachine.addRoutes(event: .clearCache, transitions: [.signedIn => .initial])
 
         // Transitions with associated values need to be handled through `addRouteMapping`
@@ -82,8 +90,6 @@ class AppCoordinatorStateMachine {
             switch (event, fromState) {
             case (.signOut(let isSoft), _):
                 return .signingOut(isSoft: isSoft)
-            case (.completedSigningOut, .signingOut):
-                return .signedOut
             default:
                 return nil
             }

ElementX/Sources/Application/AppSettings.swift

@@ -120,10 +120,15 @@ final class AppSettings {
     /// The URL that is opened when tapping the Learn more button on the sliding sync alert during authentication.
     let slidingSyncLearnMoreURL: URL = "https://github.com/matrix-org/sliding-sync/blob/main/docs/Landing.md"
     
-    /// The redirect URL used for OIDC.
-    let oidcRedirectURL: URL = "io.element:/callback"
     /// 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. The bundle ID suffix avoids app association conflicts between Element X, Nightly and PR builds.
+    let oidcRedirectURL = {
+        guard let bundleIDSuffix = InfoPlistReader.main.bundleIdentifier.split(separator: ".").last,
+              let redirectURL = URL(string: "https://mobile.element.io/oidc/\(bundleIDSuffix)")
+        else { fatalError("Failed creating a valid OIDC redirect URL.") }
+        return redirectURL
+    }()
 
     /// The date that the call to `/login` completed successfully. This is used to put
     /// a hard wall on the history of encrypted messages until we have key backup.

ElementX/Sources/Application/Navigation/AppRoutes.swift

@@ -30,7 +30,7 @@ struct AppRouteURLParser {
     
     init(appSettings: AppSettings) {
         urlParsers = [
-            ElementAppURLParser(appSettings: appSettings),
+            OIDCCallbackURLParser(appSettings: appSettings),
             ElementCallURLParser()
         ]
     }
@@ -50,34 +50,24 @@ struct AppRouteURLParser {
 ///
 /// The following Universal Links are missing parsers.
 /// - app.element.io
+/// - staging.element.io
+/// - develop.element.io
 /// - mobile.element.io
 protocol URLParser {
     func route(from url: URL) -> AppRoute?
 }
 
-/// The parser for the main Element website.
+/// The parser for the OIDC callback URL. This always returns a `.oidcCallback`.
-struct ElementAppURLParser: URLParser {
+struct OIDCCallbackURLParser: URLParser {
-    private let knownHosts = ["app.element.io", "staging.element.io", "develop.element.io"]
-    
     let appSettings: AppSettings
     
     func route(from url: URL) -> AppRoute? {
-        guard let host = url.host, knownHosts.contains(host) else {
+        guard url.absoluteString.starts(with: appSettings.oidcRedirectURL.absoluteString) else { return nil }
-            return nil
+        return .oidcCallback(url: url)
-        }
-        
-        let pathComponents = url.pathComponents
-        
-        // OIDC callback URL.
-        if pathComponents.count == 3, pathComponents[0] == "mobile", pathComponents[1] == "oidc" {
-            return .oidcCallback(url: url)
-        }
-        
-        return nil
     }
 }
 
-/// The parser for Element Call links. This always returns a `.genericCallLink`
+/// The parser for Element Call links. This always returns a `.genericCallLink`.
 struct ElementCallURLParser: URLParser {
     private let knownHosts = ["call.element.io"]
     private let customSchemeURLQueryParameterName = "url"

ElementX/Sources/Screens/Authentication/AuthenticationCoordinator.swift

@@ -32,6 +32,8 @@ class AuthenticationCoordinator: CoordinatorProtocol {
     
     private var cancellables = Set<AnyCancellable>()
     
+    private var oidcPresenter: OIDCAuthenticationPresenter?
+    
     weak var delegate: AuthenticationCoordinatorDelegate?
     
     init(authenticationService: AuthenticationServiceProxyProtocol,
@@ -53,6 +55,15 @@ class AuthenticationCoordinator: CoordinatorProtocol {
     func stop() {
         stopLoading()
     }
+    
+    func handleOIDCRedirectURL(_ url: URL) {
+        guard let oidcPresenter else {
+            MXLog.error("Failed to find an OIDC request in progress.")
+            return
+        }
+        
+        oidcPresenter.handleUniversalLinkCallback(url)
+    }
         
     // MARK: - Private
     
@@ -167,12 +178,14 @@ class AuthenticationCoordinator: CoordinatorProtocol {
                 let presenter = OIDCAuthenticationPresenter(authenticationService: authenticationService,
                                                             oidcRedirectURL: appSettings.oidcRedirectURL,
                                                             presentationAnchor: presentationAnchor)
+                self.oidcPresenter = presenter
                 switch await presenter.authenticate(using: oidcData) {
                 case .success(let userSession):
                     userHasSignedIn(userSession: userSession)
                 case .failure(let error):
                     handleError(error)
                 }
+                oidcPresenter = nil
             }
         }
     }

ElementX/Sources/Screens/Authentication/OIDCAuthenticationPresenter.swift

@@ -23,6 +23,16 @@ 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: OIDCAuthenticationDataProxy
+        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: AuthenticationServiceProxyProtocol, oidcRedirectURL: URL, presentationAnchor: UIWindow) {
         self.authenticationService = authenticationService
         self.oidcRedirectURL = oidcRedirectURL
@@ -35,41 +45,70 @@ class OIDCAuthenticationPresenter: NSObject {
         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 {
-                        continuation.resume(returning: .failure(AuthenticationServiceError.oidcError(.userCancellation)))
+                        self.completeAuthentication(throwing: .oidcError(.userCancellation))
                         return
                     }
                     
-                    continuation.resume(returning: .failure(AuthenticationServiceError.oidcError(.unknown)))
+                    self.completeAuthentication(throwing: .oidcError(.unknown))
                     return
                 }
                 
-                completeAuthentication(callbackURL: url, data: oidcData, continuation: continuation)
+                completeAuthentication(callbackURL: url)
             }
             
             session.prefersEphemeralWebBrowserSession = false
             session.presentationContextProvider = self
+            
+            request = Request(session: session, oidcData: oidcData, continuation: continuation)
+            
             session.start()
         }
     }
     
-    private func completeAuthentication(callbackURL: URL,
+    /// This method will be used if the `appSettings.oidcRedirectURL`'s scheme is `https`.
-                                        data: OIDCAuthenticationDataProxy,
+    /// When using a custom scheme, the redirect will be handled by the web auth session's closure.
-                                        continuation: CheckedContinuation<Result<UserSessionProtocol, AuthenticationServiceError>, Never>) {
+    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: data) {
+            switch await authenticationService.loginWithOIDCCallback(callbackURL, data: request.oidcData) {
             case .success(let userSession):
-                continuation.resume(returning: .success(userSession))
+                request.continuation.resume(returning: .success(userSession))
             case .failure(let error):
-                continuation.resume(returning: .failure(error))
+                request.continuation.resume(returning: .failure(error))
             }
         }
     }
+    
+    /// 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
+        }
+        
+        request.continuation.resume(returning: .failure(error))
+    }
 }
 
 // MARK: ASWebAuthenticationPresentationContextProviding

ElementX/Sources/Screens/Authentication/SoftLogoutScreen/SoftLogoutScreenCoordinator.swift

@@ -48,6 +48,7 @@ final class SoftLogoutScreenCoordinator: CoordinatorProtocol {
     private var cancellables = Set<AnyCancellable>()
     
     private var authenticationService: AuthenticationServiceProxyProtocol { parameters.authenticationService }
+    private var oidcPresenter: OIDCAuthenticationPresenter?
     
     var actions: AnyPublisher<SoftLogoutScreenCoordinatorResult, Never> {
         actionsSubject.eraseToAnyPublisher()
@@ -92,6 +93,15 @@ 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 = "SoftLogoutLoading"
@@ -151,12 +161,14 @@ final class SoftLogoutScreenCoordinator: CoordinatorProtocol {
                 let presenter = OIDCAuthenticationPresenter(authenticationService: parameters.authenticationService,
                                                             oidcRedirectURL: ServiceLocator.shared.settings.oidcRedirectURL,
                                                             presentationAnchor: presentationAnchor)
+                self.oidcPresenter = presenter
                 switch await presenter.authenticate(using: oidcData) {
                 case .success(let userSession):
                     actionsSubject.send(.signedIn(userSession))
                 case .failure(let error):
                     handleError(error)
                 }
+                self.oidcPresenter = nil
             }
         }
     }

UnitTests/Sources/AppRouteURLParserTests.swift

@@ -19,20 +19,29 @@ import XCTest
 @testable import ElementX
 
 class AppRouteURLParserTests: XCTestCase {
+    var appSettings: AppSettings!
+    var appRouteURLParser: AppRouteURLParser!
+    
+    override func setUp() {
+        AppSettings.reset()
+        appSettings = AppSettings()
+        appRouteURLParser = AppRouteURLParser(appSettings: appSettings)
+    }
+    
     func testElementCallRoutes() {
         guard let url = URL(string: "https://call.element.io/test") else {
             XCTFail("URL invalid")
             return
         }
         
-        XCTAssertEqual(AppRouteURLParser(appSettings: ServiceLocator.shared.settings).route(from: url), AppRoute.genericCallLink(url: url))
+        XCTAssertEqual(appRouteURLParser.route(from: url), AppRoute.genericCallLink(url: url))
         
         guard let customSchemeURL = URL(string: "io.element.call:/?url=https%3A%2F%2Fcall.element.io%2Ftest") else {
             XCTFail("URL invalid")
             return
         }
         
-        XCTAssertEqual(AppRouteURLParser(appSettings: ServiceLocator.shared.settings).route(from: customSchemeURL), AppRoute.genericCallLink(url: url))
+        XCTAssertEqual(appRouteURLParser.route(from: customSchemeURL), AppRoute.genericCallLink(url: url))
     }
     
     func testCustomDomainUniversalLinkCallRoutes() {
@@ -41,7 +50,7 @@ class AppRouteURLParserTests: XCTestCase {
             return
         }
         
-        XCTAssertEqual(AppRouteURLParser(appSettings: ServiceLocator.shared.settings).route(from: url), nil)
+        XCTAssertEqual(appRouteURLParser.route(from: url), nil)
     }
     
     func testCustomSchemeLinkCallRoutes() {
@@ -61,7 +70,7 @@ class AppRouteURLParserTests: XCTestCase {
             return
         }
         
-        XCTAssertEqual(AppRouteURLParser(appSettings: ServiceLocator.shared.settings).route(from: customSchemeURL), AppRoute.genericCallLink(url: url))
+        XCTAssertEqual(appRouteURLParser.route(from: customSchemeURL), AppRoute.genericCallLink(url: url))
     }
     
     func testHttpCustomSchemeLinkCallRoutes() {
@@ -70,6 +79,33 @@ class AppRouteURLParserTests: XCTestCase {
             return
         }
         
-        XCTAssertEqual(AppRouteURLParser(appSettings: ServiceLocator.shared.settings).route(from: customSchemeURL), nil)
+        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)
     }
 }

changelog.d/1734.change

@@ -0,0 +1 @@
+Use a universal link for OIDC callbacks.