From 3d1109bf8aafa8631de51b57aa5d0ce90872a49a Mon Sep 17 00:00:00 2001
From: Quentin Gliech <quenting@element.io>
Date: Mon, 30 Oct 2023 14:05:18 +0100
Subject: [PATCH] Make the upstream provider URL better display & fix test

---
 crates/handlers/src/views/login.rs |  8 ++------
 crates/templates/src/functions.rs  | 21 ++++++++++++++++-----
 templates/pages/login.html         |  2 +-
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/crates/handlers/src/views/login.rs b/crates/handlers/src/views/login.rs
index 94b9cf20b..c3715bd1e 100644
--- a/crates/handlers/src/views/login.rs
+++ b/crates/handlers/src/views/login.rs
@@ -388,15 +388,11 @@ mod test {
         let response = state.request(Request::get("/login").empty()).await;
         response.assert_status(StatusCode::OK);
         response.assert_header_value(CONTENT_TYPE, "text/html; charset=utf-8");
-        assert!(response
-            .body()
-            .contains(&escape_html(&first_provider.issuer)));
+        assert!(response.body().contains(&escape_html("first.com/")));
         assert!(response
             .body()
             .contains(&escape_html(&first_provider_login.path_and_query())));
-        assert!(response
-            .body()
-            .contains(&escape_html(&second_provider.issuer)));
+        assert!(response.body().contains(&escape_html("second.com/")));
         assert!(response
             .body()
             .contains(&escape_html(&second_provider_login.path_and_query())));
diff --git a/crates/templates/src/functions.rs b/crates/templates/src/functions.rs
index eac977975..b80a84279 100644
--- a/crates/templates/src/functions.rs
+++ b/crates/templates/src/functions.rs
@@ -118,10 +118,10 @@ fn filter_to_params(params: &Value, kwargs: Kwargs) -> Result<String, Error> {
 }
 
 /// Filter which simplifies a URL to its domain name for HTTP(S) URLs
-fn filter_simplify_url(url: &str) -> String {
+fn filter_simplify_url(url: &str, kwargs: Kwargs) -> Result<String, minijinja::Error> {
     // Do nothing if the URL is not valid
     let Ok(mut url) = Url::from_str(url) else {
-        return url.to_owned();
+        return Ok(url.to_owned());
     };
 
     // Always at least remove the query parameters and fragment
@@ -130,15 +130,26 @@ fn filter_simplify_url(url: &str) -> String {
 
     // Do nothing else for non-HTTPS URLs
     if url.scheme() != "https" {
-        return url.to_string();
+        return Ok(url.to_string());
     }
 
+    let keep_path = kwargs.get::<Option<bool>>("keep_path")?.unwrap_or_default();
+    kwargs.assert_all_used()?;
+
     // Only return the domain name
     let Some(domain) = url.domain() else {
-        return url.to_string();
+        return Ok(url.to_string());
     };
 
-    domain.to_owned()
+    if keep_path {
+        Ok(format!(
+            "{domain}{path}",
+            domain = domain,
+            path = url.path(),
+        ))
+    } else {
+        Ok(domain.to_owned())
+    }
 }
 
 enum ParamsWhere {
diff --git a/templates/pages/login.html b/templates/pages/login.html
index 2e4901410..e2f0fec22 100644
--- a/templates/pages/login.html
+++ b/templates/pages/login.html
@@ -79,7 +79,7 @@ <h1 class="title">{{ _("mas.login.headline") }}</h1>
       {% set params = next["params"] | default({}) | to_params(prefix="?") %}
       {% for provider in providers %}
         <a class="cpd-button" data-kind="secondary" data-size="lg" href="{{ ('/upstream/authorize/' ~ provider.id ~ params) | prefix_url }}">
-          {{ _("mas.login.continue_with_provider", provider=provider.issuer | simplify_url) }}
+          {{ _("mas.login.continue_with_provider", provider=provider.issuer | simplify_url(keep_path=True)) }}
         </a>
       {% endfor %}
     {% endif %}