Restore the .oidcCallback route for external authentication. (#5391)

* Restore the .oidcCallback route (partially reverts #3461) for external authentication.

* Make sure OIDC also works for non-http URLs.

* Remove oidcAuthentication from the state machine.

There isn't a reliable way to detect failure/cancellation when e.g. the user returns from an external app without interacting with the MAS page.
This commit is contained in:
Doug
2026-05-05 12:47:07 +01:00
committed by GitHub
parent 0ca41efece
commit e989463d91
17 changed files with 181 additions and 52 deletions

View File

@@ -276,6 +276,12 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
case .accountProvisioningLink: case .accountProvisioningLink:
handleAppRoute(route, handleAppRoute(route,
windowType: windowType) windowType: windowType)
case .oidcCallback(let url):
if stateMachine.state == .softLogout {
softLogoutCoordinator?.handleOIDCRedirectURL(url)
} else {
authenticationFlowCoordinator?.handleOIDCRedirectURL(url)
}
case .userProfile(let userID): case .userProfile(let userID):
if isExternalURL { if isExternalURL {
handleAppRoute(route, handleAppRoute(route,
@@ -712,6 +718,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
let parameters = SoftLogoutScreenCoordinatorParameters(authenticationService: authenticationService, let parameters = SoftLogoutScreenCoordinatorParameters(authenticationService: authenticationService,
credentials: credentials, credentials: credentials,
keyBackupNeeded: false, keyBackupNeeded: false,
appMediator: appMediator,
appSettings: appSettings, appSettings: appSettings,
userIndicatorController: ServiceLocator.shared.userIndicatorController) userIndicatorController: ServiceLocator.shared.userIndicatorController)
let coordinator = SoftLogoutScreenCoordinator(parameters: parameters) let coordinator = SoftLogoutScreenCoordinator(parameters: parameters)

View File

@@ -14,6 +14,9 @@ import MatrixRustSDK
enum AppRoute: Hashable { enum AppRoute: Hashable {
/// An account provisioning link generated externally. /// An account provisioning link generated externally.
case accountProvisioningLink(AccountProvisioningParameters) case accountProvisioningLink(AccountProvisioningParameters)
/// An external callback used to complete login with OIDC. This is only used when authentication
/// requires an external app so cannot be done within the built in web authentication session.
case oidcCallback(url: URL)
/// The app's home screen. /// The app's home screen.
case roomList case roomList
@@ -58,6 +61,7 @@ enum AppRoute: Hashable {
var isAuthenticationRoute: Bool { var isAuthenticationRoute: Bool {
switch self { switch self {
case .accountProvisioningLink: true case .accountProvisioningLink: true
case .oidcCallback: true
default: false default: false
} }
} }
@@ -82,7 +86,8 @@ struct AppRouteURLParser {
AppGroupURLParser(), AppGroupURLParser(),
MatrixPermalinkParser(), MatrixPermalinkParser(),
ElementWebURLParser(domains: appSettings.elementWebHosts), ElementWebURLParser(domains: appSettings.elementWebHosts),
AccountProvisioningURLParser(domain: appSettings.accountProvisioningHost) AccountProvisioningURLParser(domain: appSettings.accountProvisioningHost),
OIDCCallbackURLParser(redirectURL: appSettings.oidcRedirectURL)
] ]
} }
@@ -198,3 +203,13 @@ private struct AccountProvisioningURLParser: URLParser {
return .accountProvisioningLink(.init(accountProvider: serverName, loginHint: loginHint)) return .accountProvisioningLink(.init(accountProvider: serverName, loginHint: loginHint))
} }
} }
/// The parser for the OIDC callback URL. This always returns a `.oidcCallback`.
struct OIDCCallbackURLParser: URLParser {
let redirectURL: URL
func route(from url: URL) -> AppRoute? {
guard url.absoluteString.starts(with: redirectURL.absoluteString) else { return nil }
return .oidcCallback(url: url)
}
}

View File

@@ -251,8 +251,9 @@ final class AppSettings {
/// Any pre-defined static client registrations for OIDC issuers. /// Any pre-defined static client registrations for OIDC issuers.
let oidcStaticRegistrations: [URL: String] = ["https://id.thirdroom.io/realms/thirdroom": "elementx"] let oidcStaticRegistrations: [URL: String] = ["https://id.thirdroom.io/realms/thirdroom": "elementx"]
/// 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. /// The redirect URL used for OIDC. For the normal case we don't actually need the bundle ID as the web authentication session handles the redirect internally.
private(set) var oidcRedirectURL: URL = "https://element.io/oidc/login" /// However in the case where MAS sends the user to an external app, we need to make sure that the system will open the correct variant of the app (e.g. Nightly).
private(set) var oidcRedirectURL: URL! = URL(string: "https://element.io/oauth/ios/\(InfoPlistReader.main.bundleIdentifier)")
private(set) lazy var oidcConfiguration = OIDCConfiguration(clientName: InfoPlistReader.main.bundleDisplayName, private(set) lazy var oidcConfiguration = OIDCConfiguration(clientName: InfoPlistReader.main.bundleDisplayName,
redirectURI: oidcRedirectURL, redirectURI: oidcRedirectURL,

View File

@@ -40,8 +40,6 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
case serverConfirmationScreen case serverConfirmationScreen
/// The screen to choose a different server. /// The screen to choose a different server.
case serverSelectionScreen case serverSelectionScreen
/// The web authentication session is being presented.
case oidcAuthentication
/// The screen to login with a password. /// The screen to login with a password.
case loginScreen case loginScreen
@@ -76,10 +74,6 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
/// The user is no longer selecting a server. /// The user is no longer selecting a server.
case dismissedServerSelection case dismissedServerSelection
/// Show the web authentication session for OIDC (using the parameters in the `userInfo`).
case continueWithOIDC
/// The web authentication session was aborted.
case cancelledOIDCAuthentication(previousState: State)
/// Show the screen to login with password (with the optional login hint in the `userInfo`). /// Show the screen to login with password (with the optional login hint in the `userInfo`).
case continueWithPassword case continueWithPassword
/// The password login was aborted. /// The password login was aborted.
@@ -157,6 +151,8 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
} }
func clearRoute(animated: Bool) { func clearRoute(animated: Bool) {
oidcPresenter?.cancel() // Handle ongoing OIDC authentication first.
switch stateMachine.state { switch stateMachine.state {
case .initial, .startScreen: case .initial, .startScreen:
break break
@@ -168,9 +164,6 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
case .serverSelectionScreen: case .serverSelectionScreen:
navigationStackCoordinator.setSheetCoordinator(nil) navigationStackCoordinator.setSheetCoordinator(nil)
navigationStackCoordinator.popToRoot(animated: animated) navigationStackCoordinator.popToRoot(animated: animated)
case .oidcAuthentication:
oidcPresenter?.cancel()
navigationStackCoordinator.popToRoot(animated: animated)
case .loginScreen: case .loginScreen:
navigationStackCoordinator.popToRoot(animated: animated) navigationStackCoordinator.popToRoot(animated: animated)
case .bugReportFlow: case .bugReportFlow:
@@ -182,6 +175,15 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
} }
} }
func handleOIDCRedirectURL(_ url: URL) {
guard let oidcPresenter else {
MXLog.error("Failed to find an OIDC request in progress.")
return
}
oidcPresenter.handleUniversalLinkCallback(url)
}
// MARK: - Setup // MARK: - Setup
private func configureStateMachine() { private func configureStateMachine() {
@@ -220,16 +222,6 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
} }
stateMachine.addRoutes(event: .dismissedServerSelection, transitions: [.serverSelectionScreen => .serverConfirmationScreen]) stateMachine.addRoutes(event: .dismissedServerSelection, transitions: [.serverSelectionScreen => .serverConfirmationScreen])
stateMachine.addRoutes(event: .continueWithOIDC, transitions: [.serverConfirmationScreen => .oidcAuthentication,
.startScreen => .oidcAuthentication]) { [weak self] context in
guard let (oidcData, window) = context.userInfo as? (OIDCAuthorizationDataProxy, UIWindow) else {
fatalError("Missing the OIDC data and presentation anchor.")
}
self?.showOIDCAuthentication(oidcData: oidcData, presentationAnchor: window, fromState: context.fromState)
}
stateMachine.addRoutes(event: .cancelledOIDCAuthentication(previousState: .serverConfirmationScreen), transitions: [.oidcAuthentication => .serverConfirmationScreen])
stateMachine.addRoutes(event: .cancelledOIDCAuthentication(previousState: .startScreen), transitions: [.oidcAuthentication => .startScreen])
stateMachine.addRoutes(event: .continueWithPassword, transitions: [.serverConfirmationScreen => .loginScreen, stateMachine.addRoutes(event: .continueWithPassword, transitions: [.serverConfirmationScreen => .loginScreen,
.startScreen => .loginScreen]) { [weak self] context in .startScreen => .loginScreen]) { [weak self] context in
let loginHint = context.userInfo as? String let loginHint = context.userInfo as? String
@@ -255,7 +247,8 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
// Completion // Completion
stateMachine.addRoutes(event: .signedIn, transitions: [.qrCodeLoginScreen => .complete, stateMachine.addRoutes(event: .signedIn, transitions: [.qrCodeLoginScreen => .complete,
.oidcAuthentication => .complete, .serverConfirmationScreen => .complete, // OIDC authentication
.startScreen => .complete, // Direct OIDC authentication
.loginScreen => .complete]) { [weak self] context in .loginScreen => .complete]) { [weak self] context in
guard let userSession = context.userInfo as? UserSessionProtocol else { fatalError("The user session wasn't included in the context") } guard let userSession = context.userInfo as? UserSessionProtocol else { fatalError("The user session wasn't included in the context") }
self?.userHasSignedIn(userSession: userSession) self?.userHasSignedIn(userSession: userSession)
@@ -308,7 +301,7 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
stateMachine.tryEvent(.confirmServer(.register)) stateMachine.tryEvent(.confirmServer(.register))
case .loginDirectlyWithOIDC(let oidcData, let window): case .loginDirectlyWithOIDC(let oidcData, let window):
stateMachine.tryEvent(.continueWithOIDC, userInfo: (oidcData, window)) showOIDCAuthentication(oidcData: oidcData, presentationAnchor: window)
case .loginDirectlyWithPassword(let loginHint): case .loginDirectlyWithPassword(let loginHint):
stateMachine.tryEvent(.continueWithPassword, userInfo: loginHint) stateMachine.tryEvent(.continueWithPassword, userInfo: loginHint)
@@ -382,7 +375,7 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
switch action { switch action {
case .continueWithOIDC(let oidcData, let window): case .continueWithOIDC(let oidcData, let window):
stateMachine.tryEvent(.continueWithOIDC, userInfo: (oidcData, window)) showOIDCAuthentication(oidcData: oidcData, presentationAnchor: window)
case .continueWithPassword: case .continueWithPassword:
stateMachine.tryEvent(.continueWithPassword) stateMachine.tryEvent(.continueWithPassword)
case .changeServer: case .changeServer:
@@ -424,10 +417,14 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
} }
} }
private func showOIDCAuthentication(oidcData: OIDCAuthorizationDataProxy, presentationAnchor: UIWindow, fromState: State) { /// **Note:** We have intentionally excluded this presentation from the state machine as it doesn't mutate our navigation stack and there
/// isn't a robust way to detect why the user returned to the app when the MAS URL directly opens an external app for authentication without
/// presenting a web authentication session.
private func showOIDCAuthentication(oidcData: OIDCAuthorizationDataProxy, presentationAnchor: UIWindow) {
let presenter = OIDCAuthenticationPresenter(authenticationService: authenticationService, let presenter = OIDCAuthenticationPresenter(authenticationService: authenticationService,
oidcRedirectURL: appSettings.oidcRedirectURL, oidcRedirectURL: appSettings.oidcRedirectURL,
presentationAnchor: presentationAnchor, presentationAnchor: presentationAnchor,
appMediator: appMediator,
userIndicatorController: userIndicatorController) userIndicatorController: userIndicatorController)
oidcPresenter = presenter oidcPresenter = presenter
@@ -436,8 +433,7 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
case .success(let userSession): case .success(let userSession):
stateMachine.tryEvent(.signedIn, userInfo: userSession) stateMachine.tryEvent(.signedIn, userInfo: userSession)
case .failure: case .failure:
stateMachine.tryEvent(.cancelledOIDCAuthentication(previousState: fromState)) break // Nothing to do, any alerts will be handled by the presenter.
// Nothing more to do, the alerts are handled by the presenter.
} }
oidcPresenter = nil oidcPresenter = nil
} }

View File

@@ -174,7 +174,7 @@ class ChatsTabFlowCoordinator: FlowCoordinatorProtocol {
} }
case .globalSearch: case .globalSearch:
presentGlobalSearch() presentGlobalSearch()
case .accountProvisioningLink, .settings, .chatBackupSettings, .call: case .accountProvisioningLink, .oidcCallback, .settings, .chatBackupSettings, .call:
break // These routes cannot be handled. break // These routes cannot be handled.
} }
} }
@@ -690,6 +690,7 @@ class ChatsTabFlowCoordinator: FlowCoordinatorProtocol {
private func startEncryptionResetFlow(animated: Bool) { private func startEncryptionResetFlow(animated: Bool) {
let sheetNavigationStackCoordinator = NavigationStackCoordinator() let sheetNavigationStackCoordinator = NavigationStackCoordinator()
let parameters = EncryptionResetFlowCoordinatorParameters(userSession: userSession, let parameters = EncryptionResetFlowCoordinatorParameters(userSession: userSession,
appMediator: flowParameters.appMediator,
appSettings: flowParameters.appSettings, appSettings: flowParameters.appSettings,
userIndicatorController: flowParameters.userIndicatorController, userIndicatorController: flowParameters.userIndicatorController,
navigationStackCoordinator: sheetNavigationStackCoordinator, navigationStackCoordinator: sheetNavigationStackCoordinator,

View File

@@ -19,6 +19,7 @@ enum EncryptionResetFlowCoordinatorAction: Equatable {
struct EncryptionResetFlowCoordinatorParameters { struct EncryptionResetFlowCoordinatorParameters {
let userSession: UserSessionProtocol let userSession: UserSessionProtocol
let appMediator: AppMediatorProtocol
let appSettings: AppSettings let appSettings: AppSettings
let userIndicatorController: UserIndicatorControllerProtocol let userIndicatorController: UserIndicatorControllerProtocol
let navigationStackCoordinator: NavigationStackCoordinator let navigationStackCoordinator: NavigationStackCoordinator
@@ -27,6 +28,7 @@ struct EncryptionResetFlowCoordinatorParameters {
class EncryptionResetFlowCoordinator: FlowCoordinatorProtocol { class EncryptionResetFlowCoordinator: FlowCoordinatorProtocol {
private let userSession: UserSessionProtocol private let userSession: UserSessionProtocol
private let appMediator: AppMediatorProtocol
private let appSettings: AppSettings private let appSettings: AppSettings
private let userIndicatorController: UserIndicatorControllerProtocol private let userIndicatorController: UserIndicatorControllerProtocol
@@ -62,6 +64,7 @@ class EncryptionResetFlowCoordinator: FlowCoordinatorProtocol {
init(parameters: EncryptionResetFlowCoordinatorParameters) { init(parameters: EncryptionResetFlowCoordinatorParameters) {
userSession = parameters.userSession userSession = parameters.userSession
appMediator = parameters.appMediator
appSettings = parameters.appSettings appSettings = parameters.appSettings
userIndicatorController = parameters.userIndicatorController userIndicatorController = parameters.userIndicatorController
navigationStackCoordinator = parameters.navigationStackCoordinator navigationStackCoordinator = parameters.navigationStackCoordinator
@@ -158,6 +161,7 @@ class EncryptionResetFlowCoordinator: FlowCoordinatorProtocol {
// As of iOS 16, there is an issue on the simulator with accessing the cookie but it works on a device. 🤷 // As of iOS 16, there is an issue on the simulator with accessing the cookie but it works on a device. 🤷
accountSettingsPresenter = OIDCAccountSettingsPresenter(accountURL: url, accountSettingsPresenter = OIDCAccountSettingsPresenter(accountURL: url,
presentationAnchor: windowManager.mainWindow, presentationAnchor: windowManager.mainWindow,
appMediator: appMediator,
appSettings: appSettings) appSettings: appSettings)
accountSettingsPresenter?.start() accountSettingsPresenter?.start()
} }

View File

@@ -80,8 +80,8 @@ class EncryptionSettingsFlowCoordinator: FlowCoordinatorProtocol {
MXLog.info("Handling app route: \(appRoute)") MXLog.info("Handling app route: \(appRoute)")
switch appRoute { switch appRoute {
case .accountProvisioningLink: case .accountProvisioningLink, .oidcCallback:
break // We always ignore this flow when logged in. break // We always ignore these flows when logged in.
case .roomList, .room, .roomAlias, .childRoom, .childRoomAlias, case .roomList, .room, .roomAlias, .childRoom, .childRoomAlias,
.roomDetails, .roomMemberDetails, .userProfile, .thread, .roomDetails, .roomMemberDetails, .userProfile, .thread,
.event, .eventOnRoomAlias, .childEvent, .childEventOnRoomAlias, .event, .eventOnRoomAlias, .childEvent, .childEventOnRoomAlias,

View File

@@ -20,6 +20,7 @@ class OnboardingFlowCoordinator: FlowCoordinatorProtocol {
private let userSession: UserSessionProtocol private let userSession: UserSessionProtocol
private let appLockService: AppLockServiceProtocol private let appLockService: AppLockServiceProtocol
private let analyticsService: AnalyticsService private let analyticsService: AnalyticsService
private let appMediator: AppMediatorProtocol
private let appSettings: AppSettings private let appSettings: AppSettings
private let notificationManager: NotificationManagerProtocol private let notificationManager: NotificationManagerProtocol
private let userIndicatorController: UserIndicatorControllerProtocol private let userIndicatorController: UserIndicatorControllerProtocol
@@ -66,6 +67,7 @@ class OnboardingFlowCoordinator: FlowCoordinatorProtocol {
userSession = flowParameters.userSession userSession = flowParameters.userSession
self.appLockService = appLockService self.appLockService = appLockService
analyticsService = flowParameters.analytics analyticsService = flowParameters.analytics
appMediator = flowParameters.appMediator
appSettings = flowParameters.appSettings appSettings = flowParameters.appSettings
notificationManager = flowParameters.notificationManager notificationManager = flowParameters.notificationManager
userIndicatorController = flowParameters.userIndicatorController userIndicatorController = flowParameters.userIndicatorController
@@ -315,6 +317,7 @@ class OnboardingFlowCoordinator: FlowCoordinatorProtocol {
private func startEncryptionResetFlow() { private func startEncryptionResetFlow() {
let resetNavigationStackCoordinator = NavigationStackCoordinator() let resetNavigationStackCoordinator = NavigationStackCoordinator()
let coordinator = EncryptionResetFlowCoordinator(parameters: .init(userSession: userSession, let coordinator = EncryptionResetFlowCoordinator(parameters: .init(userSession: userSession,
appMediator: appMediator,
appSettings: appSettings, appSettings: appSettings,
userIndicatorController: userIndicatorController, userIndicatorController: userIndicatorController,
navigationStackCoordinator: resetNavigationStackCoordinator, navigationStackCoordinator: resetNavigationStackCoordinator,

View File

@@ -201,7 +201,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
} }
case .roomAlias, .childRoomAlias, .eventOnRoomAlias, .childEventOnRoomAlias: case .roomAlias, .childRoomAlias, .eventOnRoomAlias, .childEventOnRoomAlias:
break // These are converted to a room ID route one level above. break // These are converted to a room ID route one level above.
case .accountProvisioningLink, .roomList, .userProfile, .call, .settings, .chatBackupSettings, .globalSearch: case .accountProvisioningLink, .oidcCallback, .roomList, .userProfile, .call, .settings, .chatBackupSettings, .globalSearch:
break // These routes can't be handled. break // These routes can't be handled.
case .transferOwnership(let roomID): case .transferOwnership(let roomID):
guard self.roomID == roomID else { fatalError("Navigation route doesn't belong to this room flow.") } guard self.roomID == roomID else { fatalError("Navigation route doesn't belong to this room flow.") }

View File

@@ -120,7 +120,8 @@ final class RoomMembersFlowCoordinator: FlowCoordinatorProtocol {
} }
case .roomAlias, .childRoomAlias, .eventOnRoomAlias, .childEventOnRoomAlias: case .roomAlias, .childRoomAlias, .eventOnRoomAlias, .childEventOnRoomAlias:
break // These are converted to a room ID route one level above. break // These are converted to a room ID route one level above.
case .accountProvisioningLink, .roomList, .room, .roomDetails, .event, case .accountProvisioningLink, .oidcCallback,
.roomList, .room, .roomDetails, .event,
.userProfile, .call, .settings, .chatBackupSettings, .userProfile, .call, .settings, .chatBackupSettings,
.share, .transferOwnership, .thread, .globalSearch: .share, .transferOwnership, .thread, .globalSearch:
break break

View File

@@ -303,6 +303,7 @@ class SettingsFlowCoordinator: FlowCoordinatorProtocol {
// As of iOS 16, there is an issue on the simulator with accessing the cookie but it works on a device. 🤷 // As of iOS 16, there is an issue on the simulator with accessing the cookie but it works on a device. 🤷
accountSettingsPresenter = OIDCAccountSettingsPresenter(accountURL: url, accountSettingsPresenter = OIDCAccountSettingsPresenter(accountURL: url,
presentationAnchor: flowParameters.windowManager.mainWindow, presentationAnchor: flowParameters.windowManager.mainWindow,
appMediator: flowParameters.appMediator,
appSettings: flowParameters.appSettings, appSettings: flowParameters.appSettings,
continuation: continuation) continuation: continuation)
accountSettingsPresenter?.start() accountSettingsPresenter?.start()

View File

@@ -121,8 +121,8 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
MXLog.info("Handling app route: \(appRoute)") MXLog.info("Handling app route: \(appRoute)")
switch appRoute { switch appRoute {
case .accountProvisioningLink: case .accountProvisioningLink, .oidcCallback:
break // We always ignore this flow when logged in. break // We always ignore these flows when logged in.
case .settings, .chatBackupSettings: case .settings, .chatBackupSettings:
if ProcessInfo.processInfo.isiOSAppOnMac, flowParameters.windowManager.secondaryWindowsEnabled { if ProcessInfo.processInfo.isiOSAppOnMac, flowParameters.windowManager.secondaryWindowsEnabled {
startSettingsFlow(detached: true) startSettingsFlow(detached: true)

View File

@@ -9,31 +9,57 @@
import AuthenticationServices import AuthenticationServices
/// Presents a web authentication session for an OIDC request. /// Presents a web authentication session for an OIDC request.
///
/// In certain instances the URL may require opening an external app instead of using a WAS. Because of this
/// it is recommended to not encode the OIDC authentication within any state machines, as there is no guarantee
/// that any cancellations/failures will be communicated upwards.
@MainActor @MainActor
class OIDCAuthenticationPresenter: NSObject { class OIDCAuthenticationPresenter: NSObject {
private let authenticationService: AuthenticationServiceProtocol private let authenticationService: AuthenticationServiceProtocol
private let oidcRedirectURL: URL private let oidcRedirectURL: URL
private let presentationAnchor: UIWindow private let presentationAnchor: UIWindow
private let appMediator: AppMediatorProtocol
private let userIndicatorController: UserIndicatorControllerProtocol private let userIndicatorController: UserIndicatorControllerProtocol
private var activeSession: ASWebAuthenticationSession? /// The data required to complete a request.
struct Request {
let session: ASWebAuthenticationSession
let continuation: CheckedContinuation<Response, Never>
}
struct Response {
let url: URL?
let isExternal: Bool
let error: Error?
}
private var activeRequest: Request?
init(authenticationService: AuthenticationServiceProtocol, init(authenticationService: AuthenticationServiceProtocol,
oidcRedirectURL: URL, oidcRedirectURL: URL,
presentationAnchor: UIWindow, presentationAnchor: UIWindow,
appMediator: AppMediatorProtocol,
userIndicatorController: UserIndicatorControllerProtocol) { userIndicatorController: UserIndicatorControllerProtocol) {
self.authenticationService = authenticationService self.authenticationService = authenticationService
self.oidcRedirectURL = oidcRedirectURL self.oidcRedirectURL = oidcRedirectURL
self.presentationAnchor = presentationAnchor self.presentationAnchor = presentationAnchor
self.appMediator = appMediator
self.userIndicatorController = userIndicatorController self.userIndicatorController = userIndicatorController
super.init() super.init()
} }
/// Presents a web authentication session for the supplied data. /// Presents a web authentication session for the supplied data.
///
/// **Note:** The failure case cannot be relied upon as a signal that the authentication has ended.
/// In particular if the authentication URL requires opening an external app, then the user may return
/// to the app without completing (or cancelling) the authentication.
func authenticate(using oidcData: OIDCAuthorizationDataProxy) async -> Result<UserSessionProtocol, AuthenticationServiceError> { func authenticate(using oidcData: OIDCAuthorizationDataProxy) async -> Result<UserSessionProtocol, AuthenticationServiceError> {
let (url, error) = await withCheckedContinuation { continuation in let response = await withCheckedContinuation { continuation in
let session = ASWebAuthenticationSession(url: oidcData.url, callback: .oidcRedirectURL(oidcRedirectURL)) { url, error in let authenticationURL = oidcData.url
continuation.resume(returning: (url, error))
let session = ASWebAuthenticationSession(url: authenticationURL, callback: .oidcRedirectURL(oidcRedirectURL)) { url, error in
MXLog.info("Handling callback from the session.")
continuation.resume(returning: Response(url: url, isExternal: false, error: error))
} }
session.prefersEphemeralWebBrowserSession = false session.prefersEphemeralWebBrowserSession = false
@@ -42,21 +68,31 @@ class OIDCAuthenticationPresenter: NSObject {
"X-Element-User-Agent": UserAgentBuilder.makeASCIIUserAgent() "X-Element-User-Agent": UserAgentBuilder.makeASCIIUserAgent()
] ]
activeSession = session activeRequest = Request(session: session, continuation: continuation)
session.start()
if authenticationURL.scheme == "https" || authenticationURL.scheme == "http" {
session.start()
} else {
appMediator.open(authenticationURL)
}
} }
activeSession = nil if response.isExternal {
// Manually dismiss the web authentication session if the login was completed outside of the app.
// Note: This doesn't trigger a callback so no need to worry about the continuation being called twice.
activeRequest?.session.cancel()
}
activeRequest = nil
guard let url else { guard let url = response.url else {
// Check for user cancellation to avoid showing an alert in that instance. // Check for user cancellation (on the WAS sheet) to avoid showing an alert in that instance.
if error?.isOIDCUserCancellation == true { if response.error?.isOIDCUserCancellation == true {
// No need to show an error here, just abort and return a failure. // No need to show an error here, just abort and return a failure.
await authenticationService.abortOIDCLogin(data: oidcData) await authenticationService.abortOIDCLogin(data: oidcData)
return .failure(.oidcError(.userCancellation)) return .failure(.oidcError(.userCancellation))
} }
let errorDescription = error.map(String.init(describing:)) ?? "Unknown error" let errorDescription = response.error.map(String.init(describing:)) ?? "Unknown error"
MXLog.error("Missing callback URL from the web authentication session: \(errorDescription)") MXLog.error("Missing callback URL from the web authentication session: \(errorDescription)")
showFailureIndicator() showFailureIndicator()
@@ -71,7 +107,7 @@ class OIDCAuthenticationPresenter: NSObject {
switch await authenticationService.loginWithOIDCCallback(url) { switch await authenticationService.loginWithOIDCCallback(url) {
case .success(let userSession): case .success(let userSession):
return .success(userSession) return .success(userSession)
case .failure(.oidcError(.userCancellation)): case .failure(.oidcError(.userCancellation)): // Check for user cancellation (on the MAS web page)
// No need to show an error here, just return the failure. // No need to show an error here, just return the failure.
return .failure(.oidcError(.userCancellation)) return .failure(.oidcError(.userCancellation))
case .failure(let error): case .failure(let error):
@@ -81,8 +117,20 @@ class OIDCAuthenticationPresenter: NSObject {
} }
} }
/// This method will be used when the web authentication session redirects the user to an external
/// authentication app. During normal use the redirect is handled automatically by the session's closure.
func handleUniversalLinkCallback(_ url: URL) {
guard let activeRequest else {
MXLog.error("Failed to handle universal link callback. Missing request.")
return
}
MXLog.info("Handling callback from a universal link.")
activeRequest.continuation.resume(returning: Response(url: url, isExternal: true, error: nil))
}
func cancel() { func cancel() {
activeSession?.cancel() activeRequest?.session.cancel()
activeRequest = nil // Programatically cancelling the session doesn't trigger a callback.
} }
private var loadingIndicatorID: String { private var loadingIndicatorID: String {

View File

@@ -13,6 +13,7 @@ struct SoftLogoutScreenCoordinatorParameters {
let authenticationService: AuthenticationServiceProtocol let authenticationService: AuthenticationServiceProtocol
let credentials: SoftLogoutScreenCredentials let credentials: SoftLogoutScreenCredentials
let keyBackupNeeded: Bool let keyBackupNeeded: Bool
let appMediator: AppMediatorProtocol
let appSettings: AppSettings let appSettings: AppSettings
let userIndicatorController: UserIndicatorControllerProtocol let userIndicatorController: UserIndicatorControllerProtocol
} }
@@ -90,6 +91,15 @@ final class SoftLogoutScreenCoordinator: CoordinatorProtocol {
AnyView(SoftLogoutScreen(context: viewModel.context)) 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 // MARK: - Private
private static let loadingIndicatorIdentifier = "\(SoftLogoutScreenCoordinator.self)-Loading" private static let loadingIndicatorIdentifier = "\(SoftLogoutScreenCoordinator.self)-Loading"
@@ -149,6 +159,7 @@ final class SoftLogoutScreenCoordinator: CoordinatorProtocol {
let presenter = OIDCAuthenticationPresenter(authenticationService: parameters.authenticationService, let presenter = OIDCAuthenticationPresenter(authenticationService: parameters.authenticationService,
oidcRedirectURL: parameters.appSettings.oidcRedirectURL, oidcRedirectURL: parameters.appSettings.oidcRedirectURL,
presentationAnchor: presentationAnchor, presentationAnchor: presentationAnchor,
appMediator: parameters.appMediator,
userIndicatorController: parameters.userIndicatorController) userIndicatorController: parameters.userIndicatorController)
self.oidcPresenter = presenter self.oidcPresenter = presenter
switch await presenter.authenticate(using: oidcData) { switch await presenter.authenticate(using: oidcData) {

View File

@@ -17,17 +17,24 @@ import AuthenticationServices
@MainActor @MainActor
class OIDCAccountSettingsPresenter: NSObject { class OIDCAccountSettingsPresenter: NSObject {
private let accountURL: URL private let accountURL: URL
private let presentationAnchor: UIWindow
private let oidcRedirectURL: URL private let oidcRedirectURL: URL
private let presentationAnchor: UIWindow
private let appMediator: AppMediatorProtocol
typealias Continuation = AsyncStream<Result<Void, OIDCError>>.Continuation typealias Continuation = AsyncStream<Result<Void, OIDCError>>.Continuation
private let continuation: Continuation? private let continuation: Continuation?
init(accountURL: URL, presentationAnchor: UIWindow, appSettings: AppSettings, continuation: Continuation? = nil) { init(accountURL: URL,
presentationAnchor: UIWindow,
appMediator: AppMediatorProtocol,
appSettings: AppSettings,
continuation: Continuation? = nil) {
self.accountURL = accountURL self.accountURL = accountURL
self.presentationAnchor = presentationAnchor
oidcRedirectURL = appSettings.oidcRedirectURL oidcRedirectURL = appSettings.oidcRedirectURL
self.presentationAnchor = presentationAnchor
self.appMediator = appMediator
self.continuation = continuation self.continuation = continuation
super.init() super.init()
} }
@@ -53,7 +60,11 @@ class OIDCAccountSettingsPresenter: NSObject {
"X-Element-User-Agent": UserAgentBuilder.makeASCIIUserAgent() "X-Element-User-Agent": UserAgentBuilder.makeASCIIUserAgent()
] ]
session.start() if accountURL.scheme == "https" || accountURL.scheme == "http" {
session.start()
} else {
appMediator.open(accountURL)
}
} }
} }

View File

@@ -735,6 +735,7 @@ class MockScreen: Identifiable {
let navigationStackCoordinator = NavigationStackCoordinator() let navigationStackCoordinator = NavigationStackCoordinator()
let coordinator = EncryptionResetFlowCoordinator(parameters: .init(userSession: userSession, let coordinator = EncryptionResetFlowCoordinator(parameters: .init(userSession: userSession,
appMediator: AppMediatorMock.default,
appSettings: ServiceLocator.shared.settings, appSettings: ServiceLocator.shared.settings,
userIndicatorController: userIndicatorController, userIndicatorController: userIndicatorController,
navigationStackCoordinator: navigationStackCoordinator, navigationStackCoordinator: navigationStackCoordinator,

View File

@@ -20,6 +20,35 @@ struct AppRouteURLParserTests {
appRouteURLParser = AppRouteURLParser(appSettings: appSettings) appRouteURLParser = AppRouteURLParser(appSettings: appSettings)
} }
@Test
func oidcCallbackRoute() {
// 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.
#expect(route == .oidcCallback(url: callbackURL))
}
@Test
func oidcCallbackAppVariantRoute() {
// Given an OIDC callback for a different app variant.
let callbackURL = appSettings.oidcRedirectURL
.deletingLastPathComponent()
.appending(component: "io.element.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.
#expect(route == nil)
}
@Test @Test
func matrixUserURL() throws { func matrixUserURL() throws {
let userID = "@test:matrix.org" let userID = "@test:matrix.org"