From 01e3a31a119d64305fb3ce7be09c5aee46467449 Mon Sep 17 00:00:00 2001
From: Doug <6060466+pixlwave@users.noreply.github.com>
Date: Thu, 23 Feb 2023 12:12:53 +0000
Subject: [PATCH] fix(bindings): Verify entered homeserver before proxy
 availability

The reported errors weren't that helpful to the user.
Additionally attempt discovery when a URL is entered before falling back to the URL directly.
---
 .../src/authentication_service.rs             | 55 +++++++++++++------
 crates/matrix-sdk/src/lib.rs                  |  7 +++
 2 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/bindings/matrix-sdk-ffi/src/authentication_service.rs b/bindings/matrix-sdk-ffi/src/authentication_service.rs
index 8467d4d32..bce8f761e 100644
--- a/bindings/matrix-sdk-ffi/src/authentication_service.rs
+++ b/bindings/matrix-sdk-ffi/src/authentication_service.rs
@@ -2,7 +2,7 @@ use std::sync::{Arc, RwLock};
 
 use futures_util::future::join3;
 use matrix_sdk::{
-    ruma::{OwnedDeviceId, UserId},
+    ruma::{IdParseError, OwnedDeviceId, UserId},
     Session,
 };
 use zeroize::Zeroize;
@@ -28,6 +28,8 @@ impl Drop for AuthenticationService {
 pub enum AuthenticationError {
     #[error("A successful call to configure_homeserver must be made first.")]
     ClientMissing,
+    #[error("{message}")]
+    InvalidServerName { message: String },
     #[error("The homeserver doesn't provide a trusted a sliding sync proxy in its well-known configuration.")]
     SlidingSyncNotAvailable,
     #[error("Login was successful but is missing a valid Session to configure the file store.")]
@@ -42,6 +44,12 @@ impl From<anyhow::Error> for AuthenticationError {
     }
 }
 
+impl From<IdParseError> for AuthenticationError {
+    fn from(e: IdParseError) -> AuthenticationError {
+        AuthenticationError::InvalidServerName { message: e.to_string() }
+    }
+}
+
 #[derive(uniffi::Object)]
 pub struct HomeserverLoginDetails {
     url: String,
@@ -112,32 +120,47 @@ impl AuthenticationService {
 
     /// Updates the service to authenticate with the homeserver for the
     /// specified address.
-    pub fn configure_homeserver(&self, server_name: String) -> Result<(), AuthenticationError> {
+    pub fn configure_homeserver(
+        &self,
+        server_name_or_homeserver_url: String,
+    ) -> Result<(), AuthenticationError> {
         let mut builder = Arc::new(ClientBuilder::new()).base_path(self.base_path.clone());
 
-        if server_name.starts_with("http://") || server_name.starts_with("https://") {
-            builder = builder.homeserver_url(server_name)
-        } else {
-            builder = builder.server_name(server_name);
-        }
+        // Remove any URL scheme from the name to attempt discovery first.
+        let server_name = matrix_sdk::sanitize_server_name(&server_name_or_homeserver_url)
+            .map_err(AuthenticationError::from)?;
 
-        let client = builder.build().map_err(AuthenticationError::from)?;
+        builder = builder.server_name(server_name.to_string());
 
-        // Make sure there's a sliding sync proxy available one way or another.
+        let client = builder
+            .build()
+            .or_else(|e| {
+                if !server_name_or_homeserver_url.starts_with("http://")
+                    && !server_name_or_homeserver_url.starts_with("http://")
+                {
+                    return Err(e);
+                }
+                // When discovery fails, fallback to the homeserver URL if supplied.
+                let mut builder = Arc::new(ClientBuilder::new()).base_path(self.base_path.clone());
+                builder = builder.homeserver_url(server_name_or_homeserver_url);
+                builder.build()
+            })
+            .map_err(AuthenticationError::from)?;
+
+        let details = RUNTIME.block_on(async { self.details_from_client(&client).await })?;
+
+        // Now we've verified that it's a valid homeserver, make sure
+        // there's a sliding sync proxy available one way or another.
         if self.custom_sliding_sync_proxy.read().unwrap().is_none()
             && client.discovered_sliding_sync_proxy().is_none()
         {
             return Err(AuthenticationError::SlidingSyncNotAvailable);
         }
 
-        RUNTIME.block_on(async move {
-            let details = Arc::new(self.details_from_client(&client).await?);
+        *self.client.write().unwrap() = Some(client);
+        *self.homeserver_details.write().unwrap() = Some(Arc::new(details));
 
-            *self.client.write().unwrap() = Some(client);
-            *self.homeserver_details.write().unwrap() = Some(details);
-
-            Ok(())
-        })
+        Ok(())
     }
 
     /// Performs a password login using the current homeserver.
diff --git a/crates/matrix-sdk/src/lib.rs b/crates/matrix-sdk/src/lib.rs
index f69df0506..ae905703c 100644
--- a/crates/matrix-sdk/src/lib.rs
+++ b/crates/matrix-sdk/src/lib.rs
@@ -55,6 +55,7 @@ pub use error::ImageError;
 pub use error::{Error, HttpError, HttpResult, RefreshTokenError, Result, RumaApiError};
 pub use http_client::HttpSend;
 pub use media::Media;
+pub use ruma::{IdParseError, OwnedServerName, ServerName};
 #[cfg(feature = "experimental-sliding-sync")]
 pub use sliding_sync::{
     RoomListEntry, SlidingSync, SlidingSyncBuilder, SlidingSyncMode, SlidingSyncRoom,
@@ -73,3 +74,9 @@ fn init_logging() {
         .with(tracing_subscriber::fmt::layer().with_test_writer())
         .init();
 }
+
+/// Creates a server name from a user supplied string. The string is first
+/// sanitized by removing the http(s) scheme before being parsed.
+pub fn sanitize_server_name(s: &str) -> Result<OwnedServerName, IdParseError> {
+    ServerName::parse(s.trim_start_matches("http://").trim_start_matches("https://"))
+}