fixed #1074 - strip trailing slash in SSO issuer URLs and log errors …

…properly
warp-tech · Oct 14, 2024 · 80ee6cc · 80ee6cc
1 parent 17a49f3
commit 80ee6cc
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 12 deletions.
diff --git a/warpgate-protocol-http/src/lib.rs b/warpgate-protocol-http/src/lib.rs
@@ -19,7 +19,7 @@ use async_trait::async_trait;
 use common::page_admin_auth;
 pub use common::{SsoLoginState, PROTOCOL_NAME};
 use http::HeaderValue;
-use logging::{get_client_ip, log_request_result, span_for_request};
+use logging::{get_client_ip, log_request_error, log_request_result, span_for_request};
 use poem::endpoint::{EmbeddedFileEndpoint, EmbeddedFilesEndpoint};
 use poem::listener::{Listener, RustlsConfig, TcpListener};
 use poem::middleware::SetHeader;
@@ -124,9 +124,12 @@ impl ProtocolServer for HTTPProtocolServer {
                         let url = req.original_uri().clone();
                         let client_ip = get_client_ip(&req).await?;
 
-                        let response = ep.call(req).await?;
+                        let response = ep.call(req).await.map_err(|e| {
+                            log_request_error(&method, &url, &client_ip, &e);
+                            e
+                        })?;
 
-                        log_request_result(&method, &url, client_ip, &response.status());
+                        log_request_result(&method, &url, &client_ip, &response.status());
                         Ok(response)
                     }),
             )

diff --git a/warpgate-protocol-http/src/logging.rs b/warpgate-protocol-http/src/logging.rs
@@ -1,3 +1,5 @@
+use std::error::Error;
+
 use http::{Method, StatusCode, Uri};
 use poem::web::Data;
 use poem::{FromRequest, Request};
@@ -26,14 +28,18 @@ pub async fn span_for_request(req: &Request) -> poem::Result<Span> {
     })
 }
 
-pub fn log_request_result(method: &Method, url: &Uri, client_ip: String, status: &StatusCode) {
+pub fn log_request_result(method: &Method, url: &Uri, client_ip: &str, status: &StatusCode) {
     if status.is_server_error() || status.is_client_error() {
         warn!(%method, %url, %status, %client_ip, "Request failed");
     } else {
         info!(%method, %url, %status, %client_ip, "Request");
     }
 }
 
+pub fn log_request_error<E: Error>(method: &Method, url: &Uri, client_ip: &str, error: E) {
+    error!(%method, %url, %error, %client_ip, "Request failed");
+}
+
 pub async fn get_client_ip(req: &Request) -> poem::Result<String> {
     let services = Data::<&Services>::from_request_without_body(req).await.ok();
     let trust_x_forwarded_headers = if let Some(services) = services {

diff --git a/warpgate-protocol-http/src/proxy.rs b/warpgate-protocol-http/src/proxy.rs
@@ -294,7 +294,7 @@ pub async fn proxy_normal_request(
     log_request_result(
         req.method(),
         req.original_uri(),
-        get_client_ip(req).await?,
+        &get_client_ip(req).await?,
         &status,
     );
 

diff --git a/warpgate-sso/src/config.rs b/warpgate-sso/src/config.rs
@@ -155,13 +155,14 @@ impl SsoInternalProviderConfig {
             }
             SsoInternalProviderConfig::Custom { issuer_url, .. } => {
                 let mut url = issuer_url.url().clone();
-                let path = url.path();
-                let path = path
-                    .strip_suffix(".well-known/openid-configuration")
-                    .unwrap_or(path)
-                    .to_owned();
-                url.set_path(&path);
-                IssuerUrl::from_url(url)
+                let path = url.path().to_owned();
+                if let Some(path) = path.strip_suffix("/.well-known/openid-configuration") {
+                    url.set_path(path);
+                    let url_string = url.to_string();
+                    IssuerUrl::new(url_string.trim_end_matches('/').into())?
+                } else {
+                    issuer_url.clone()
+                }
             }
         })
     }