diff --git a/ElementX/Sources/Application/AppCoordinator.swift b/ElementX/Sources/Application/AppCoordinator.swift index 09ed73d65..36b866447 100644 --- a/ElementX/Sources/Application/AppCoordinator.swift +++ b/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): - self.presentSplashScreen(isSoftLogout: isSoft) + case (.signingOut, .completedSigningOut, .signedOut): + 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. diff --git a/ElementX/Sources/Application/AppCoordinatorStateMachine.swift b/ElementX/Sources/Application/AppCoordinatorStateMachine.swift index c974f488f..714cb6e04 100644 --- a/ElementX/Sources/Application/AppCoordinatorStateMachine.swift +++ b/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 - case completedSigningOut(isSoft: Bool) + /// Request the soft logout screen. + 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 } diff --git a/ElementX/Sources/Application/AppSettings.swift b/ElementX/Sources/Application/AppSettings.swift index 8e5299ab8..5169a653a 100644 --- a/ElementX/Sources/Application/AppSettings.swift +++ b/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. diff --git a/ElementX/Sources/Application/Navigation/AppRoutes.swift b/ElementX/Sources/Application/Navigation/AppRoutes.swift index 494b083e1..41ca9e17c 100644 --- a/ElementX/Sources/Application/Navigation/AppRoutes.swift +++ b/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. -struct ElementAppURLParser: URLParser { - private let knownHosts = ["app.element.io", "staging.element.io", "develop.element.io"] - +/// 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 let host = url.host, knownHosts.contains(host) else { - return nil - } - - let pathComponents = url.pathComponents - - // OIDC callback URL. - if pathComponents.count == 3, pathComponents[0] == "mobile", pathComponents[1] == "oidc" { - return .oidcCallback(url: url) - } - - return nil + 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` +/// The parser for Element Call links. This always returns a `.genericCallLink`. struct ElementCallURLParser: URLParser { private let knownHosts = ["call.element.io"] private let customSchemeURLQueryParameterName = "url" diff --git a/ElementX/Sources/Screens/Authentication/AuthenticationCoordinator.swift b/ElementX/Sources/Screens/Authentication/AuthenticationCoordinator.swift index 87817f244..16c800f0c 100644 --- a/ElementX/Sources/Screens/Authentication/AuthenticationCoordinator.swift +++ b/ElementX/Sources/Screens/Authentication/AuthenticationCoordinator.swift @@ -32,6 +32,8 @@ class AuthenticationCoordinator: CoordinatorProtocol { private var cancellables = Set() + 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 } } } diff --git a/ElementX/Sources/Screens/Authentication/OIDCAuthenticationPresenter.swift b/ElementX/Sources/Screens/Authentication/OIDCAuthenticationPresenter.swift index 7c8da41ab..7789fe29a 100644 --- a/ElementX/Sources/Screens/Authentication/OIDCAuthenticationPresenter.swift +++ b/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, 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, - data: OIDCAuthenticationDataProxy, - continuation: CheckedContinuation, Never>) { + /// 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: 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 diff --git a/ElementX/Sources/Screens/Authentication/SoftLogoutScreen/SoftLogoutScreenCoordinator.swift b/ElementX/Sources/Screens/Authentication/SoftLogoutScreen/SoftLogoutScreenCoordinator.swift index c2fae31ab..16fc75d76 100644 --- a/ElementX/Sources/Screens/Authentication/SoftLogoutScreen/SoftLogoutScreenCoordinator.swift +++ b/ElementX/Sources/Screens/Authentication/SoftLogoutScreen/SoftLogoutScreenCoordinator.swift @@ -48,6 +48,7 @@ final class SoftLogoutScreenCoordinator: CoordinatorProtocol { private var cancellables = Set() private var authenticationService: AuthenticationServiceProxyProtocol { parameters.authenticationService } + private var oidcPresenter: OIDCAuthenticationPresenter? var actions: AnyPublisher { 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 } } } diff --git a/UnitTests/Sources/AppRouteURLParserTests.swift b/UnitTests/Sources/AppRouteURLParserTests.swift index 89159b12d..7a01191a8 100644 --- a/UnitTests/Sources/AppRouteURLParserTests.swift +++ b/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) } } diff --git a/changelog.d/1734.change b/changelog.d/1734.change new file mode 100644 index 000000000..699c4a9f8 --- /dev/null +++ b/changelog.d/1734.change @@ -0,0 +1 @@ +Use a universal link for OIDC callbacks. \ No newline at end of file