Use an https callback for OIDC once again. (#3461)

* Use the new WAS callback type and return back to the https callback for OIDC.

* Simplify OIDCAuthenticationPresenter now it doesn't need to handle universal links.

* Remove old unit tests.
This commit is contained in:
Doug
2024-10-29 11:28:05 +00:00
committed by GitHub
parent a583892e01
commit 589df7d76e
10 changed files with 34 additions and 143 deletions
@@ -195,12 +195,6 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
if let route = appRouteURLParser.route(from: url) { if let route = appRouteURLParser.route(from: url) {
switch route { switch route {
case .oidcCallback(let url):
if stateMachine.state == .softLogout {
softLogoutCoordinator?.handleOIDCRedirectURL(url)
} else {
authenticationFlowCoordinator?.handleOIDCRedirectURL(url)
}
case .genericCallLink(let url): case .genericCallLink(let url):
if let userSessionFlowCoordinator { if let userSessionFlowCoordinator {
userSessionFlowCoordinator.handleAppRoute(route, animated: true) userSessionFlowCoordinator.handleAppRoute(route, animated: true)
@@ -153,14 +153,8 @@ 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. /// 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 = { let oidcRedirectURL: URL = "https://element.io/oidc/login"
guard let url = URL(string: "\(InfoPlistReader.main.appScheme):/callback") else {
fatalError("Invalid OIDC redirect URL")
}
return url
}()
private(set) lazy var oidcConfiguration = OIDCConfigurationProxy(clientName: InfoPlistReader.main.bundleDisplayName, private(set) lazy var oidcConfiguration = OIDCConfigurationProxy(clientName: InfoPlistReader.main.bundleDisplayName,
redirectURI: oidcRedirectURL, redirectURI: oidcRedirectURL,
@@ -9,8 +9,6 @@ import Foundation
import MatrixRustSDK import MatrixRustSDK
enum AppRoute: Equatable { enum AppRoute: Equatable {
/// The callback used to complete login with OIDC.
case oidcCallback(url: URL)
/// The app's home screen. /// The app's home screen.
case roomList case roomList
/// A room, shown as the root of the stack (popping any child rooms). /// A room, shown as the root of the stack (popping any child rooms).
@@ -52,7 +50,6 @@ struct AppRouteURLParser {
urlParsers = [ urlParsers = [
MatrixPermalinkParser(), MatrixPermalinkParser(),
ElementWebURLParser(domains: appSettings.elementWebHosts), ElementWebURLParser(domains: appSettings.elementWebHosts),
OIDCCallbackURLParser(appSettings: appSettings),
ElementCallURLParser() ElementCallURLParser()
] ]
} }
@@ -76,16 +73,6 @@ protocol URLParser {
func route(from url: URL) -> AppRoute? 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`. /// The parser for Element Call links. This always returns a `.genericCallLink`.
struct ElementCallURLParser: URLParser { struct ElementCallURLParser: URLParser {
private let knownHosts = ["call.element.io"] private let knownHosts = ["call.element.io"]
@@ -65,15 +65,6 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
fatalError() 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 // MARK: - Private
private func showStartScreen() { private func showStartScreen() {
@@ -163,7 +163,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 .roomList, .userProfile, .call, .genericCallLink, .oidcCallback, .settings, .chatBackupSettings: case .roomList, .userProfile, .call, .genericCallLink, .settings, .chatBackupSettings:
break // These routes can't be handled. break // These routes can't be handled.
} }
} }
@@ -282,8 +282,6 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
presentCallScreen(genericCallLink: url) presentCallScreen(genericCallLink: url)
case .settings, .chatBackupSettings: case .settings, .chatBackupSettings:
settingsFlowCoordinator.handleAppRoute(appRoute, animated: animated) settingsFlowCoordinator.handleAppRoute(appRoute, animated: animated)
case .oidcCallback:
break
} }
} }
@@ -14,16 +14,6 @@ class OIDCAuthenticationPresenter: NSObject {
private let oidcRedirectURL: URL private let oidcRedirectURL: URL
private let presentationAnchor: UIWindow 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) { init(authenticationService: AuthenticationServiceProtocol, oidcRedirectURL: URL, presentationAnchor: UIWindow) {
self.authenticationService = authenticationService self.authenticationService = authenticationService
self.oidcRedirectURL = oidcRedirectURL self.oidcRedirectURL = oidcRedirectURL
@@ -33,74 +23,35 @@ class OIDCAuthenticationPresenter: NSObject {
/// Presents a web authentication session for the supplied data. /// Presents a web authentication session for the supplied data.
func authenticate(using oidcData: OIDCAuthorizationDataProxy) async -> Result<UserSessionProtocol, AuthenticationServiceError> { func authenticate(using oidcData: OIDCAuthorizationDataProxy) async -> Result<UserSessionProtocol, AuthenticationServiceError> {
await withCheckedContinuation { continuation in let (url, error) = await withCheckedContinuation { continuation in
let session = ASWebAuthenticationSession(url: oidcData.url, let session = ASWebAuthenticationSession(url: oidcData.url, callback: .oidcRedirectURL(oidcRedirectURL)) { url, error in
callbackURLScheme: oidcRedirectURL.scheme) { [weak self] url, error in continuation.resume(returning: (url, error))
// 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)
} }
session.prefersEphemeralWebBrowserSession = false session.prefersEphemeralWebBrowserSession = false
session.presentationContextProvider = self session.presentationContextProvider = self
request = Request(session: session, oidcData: oidcData, continuation: continuation)
session.start() session.start()
} }
}
/// This method will be used if the `appSettings.oidcRedirectURL`'s scheme is `https`. guard let url else {
/// When using a custom scheme, the redirect will be handled by the web auth session's closure. // Check for user cancellation to avoid showing an alert in that instance.
func handleUniversalLinkCallback(_ url: URL) { if let nsError = error as? NSError,
completeAuthentication(callbackURL: url) nsError.domain == ASWebAuthenticationSessionErrorDomain,
} nsError.code == ASWebAuthenticationSessionError.canceledLogin.rawValue {
await authenticationService.abortOIDCLogin(data: oidcData)
/// Completes the authentication by exchanging the callback URL for a user session. return .failure(.oidcError(.userCancellation))
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))
} }
}
}
/// Aborts the authentication with the supplied error. await authenticationService.abortOIDCLogin(data: oidcData)
private func completeAuthentication(throwing error: AuthenticationServiceError) { return .failure(.oidcError(.unknown))
guard let request else {
MXLog.error("Failed to throw authentication error. Missing request.")
return
} }
Task { switch await authenticationService.loginWithOIDCCallback(url, data: oidcData) {
await authenticationService.abortOIDCLogin(data: request.oidcData) case .success(let userSession):
request.continuation.resume(returning: .failure(error)) return .success(userSession)
case .failure(let error):
return .failure(error)
} }
} }
} }
@@ -110,3 +61,15 @@ class OIDCAuthenticationPresenter: NSObject {
extension OIDCAuthenticationPresenter: ASWebAuthenticationPresentationContextProviding { extension OIDCAuthenticationPresenter: ASWebAuthenticationPresentationContextProviding {
func presentationAnchor(for session: ASWebAuthenticationSession) -> ASPresentationAnchor { presentationAnchor } 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)) 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"
@@ -28,7 +28,7 @@ class OIDCAccountSettingsPresenter: NSObject {
/// Presents a web authentication session for the supplied data. /// Presents a web authentication session for the supplied data.
func start() { func start() {
let session = ASWebAuthenticationSession(url: accountURL, callbackURLScheme: oidcRedirectURL.scheme) { _, _ in } let session = ASWebAuthenticationSession(url: accountURL, callback: .oidcRedirectURL(oidcRedirectURL)) { _, _ in }
session.prefersEphemeralWebBrowserSession = false session.prefersEphemeralWebBrowserSession = false
session.presentationContextProvider = self session.presentationContextProvider = self
session.start() session.start()
@@ -73,33 +73,6 @@ class AppRouteURLParserTests: XCTestCase {
XCTAssertEqual(appRouteURLParser.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)
}
func testMatrixUserURL() { func testMatrixUserURL() {
let userID = "@test:matrix.org" let userID = "@test:matrix.org"
guard let url = URL(string: "https://matrix.to/#/\(userID)") else { guard let url = URL(string: "https://matrix.to/#/\(userID)") else {