From dbf96a8feeffbb1c8f062dfaf8bdd7e6215237f2 Mon Sep 17 00:00:00 2001 From: Eugene Date: Thu, 24 Oct 2024 00:04:37 +0200 Subject: [PATCH] fixed #1093 - allow multiple return domains for SSO, prefer host header over external_host --- warpgate-common/src/config/mod.rs | 35 ++++++++++++------- warpgate-common/src/error.rs | 6 ++-- warpgate-protocol-http/src/api/info.rs | 15 +++----- .../src/api/sso_provider_detail.rs | 11 +++--- .../src/api/sso_provider_list.rs | 2 +- warpgate-protocol-http/src/lib.rs | 5 ++- warpgate-protocol-ssh/src/server/session.rs | 2 +- warpgate-sso/src/config.rs | 1 + 8 files changed, 42 insertions(+), 35 deletions(-) diff --git a/warpgate-common/src/config/mod.rs b/warpgate-common/src/config/mod.rs index ae46938ac..b44d96904 100644 --- a/warpgate-common/src/config/mod.rs +++ b/warpgate-common/src/config/mod.rs @@ -5,7 +5,7 @@ use std::path::PathBuf; use std::time::Duration; use defaults::*; -use poem::http::{self, uri}; +use poem::http::uri; use poem_openapi::{Object, Union}; use serde::{Deserialize, Serialize}; pub use target::*; @@ -381,7 +381,7 @@ pub struct WarpgateConfig { } impl WarpgateConfig { - pub fn _external_host_from_config(&self) -> Option<(Scheme, String, Option)> { + pub fn external_host_from_config(&self) -> Option<(Scheme, String, Option)> { if let Some(external_host) = self.store.external_host.as_ref() { #[allow(clippy::unwrap_used)] let external_host = external_host.split(":").next().unwrap(); @@ -399,8 +399,8 @@ impl WarpgateConfig { } } - // Extract external host:port from request headers - pub fn _external_host_from_request( + /// Extract external host:port from request headers + pub fn external_host_from_request( &self, request: &poem::Request, ) -> Option<(Scheme, String, Option)> { @@ -410,11 +410,10 @@ impl WarpgateConfig { // Try the Host header first scheme = request.uri().scheme().cloned().unwrap_or(scheme); - if let Some(host_header) = request.header(http::header::HOST).map(|x| x.to_string()) { - if let Ok(host_port) = Url::parse(&format!("https://{host_header}/")) { - host = host_port.host_str().map(Into::into).or(host); - port = host_port.port(); - } + let original_url = request.original_uri(); + if let Some(original_host) = original_url.host() { + host = Some(original_host.to_string()); + port = original_url.port().map(|x| x.as_u16()); } // But prefer X-Forwarded-* headers if enabled @@ -443,14 +442,24 @@ impl WarpgateConfig { pub fn construct_external_url( &self, for_request: Option<&poem::Request>, + domain_whitelist: Option<&[String]>, ) -> Result { - let Some((scheme, host, port)) = self - ._external_host_from_config() - .or(for_request.and_then(|r| self._external_host_from_request(r))) + let Some((scheme, host, port)) = for_request + .and_then(|r| self.external_host_from_request(r)) + .or(self.external_host_from_config()) else { - return Err(WarpgateError::ExternalHostNotSet); + return Err(WarpgateError::ExternalHostUnknown); }; + if let Some(list) = domain_whitelist { + if !list.contains(&host) { + return Err(WarpgateError::ExternalHostNotWhitelisted( + host.clone(), + list.iter().map(|x| x.to_string()).collect(), + )); + } + } + let mut url = format!("{scheme}://{host}"); if let Some(port) = port { // can't `match` `Scheme` diff --git a/warpgate-common/src/error.rs b/warpgate-common/src/error.rs index be3029ac1..f75274521 100644 --- a/warpgate-common/src/error.rs +++ b/warpgate-common/src/error.rs @@ -21,8 +21,10 @@ pub enum WarpgateError { UrlParse(#[from] url::ParseError), #[error("deserialization failed: {0}")] DeserializeJson(#[from] serde_json::Error), - #[error("external_url config option is not set")] - ExternalHostNotSet, + #[error("no valid Host header found and `external_host` config option is not set")] + ExternalHostUnknown, + #[error("current hostname ({0}) is not on the whitelist ({1:?})")] + ExternalHostNotWhitelisted(String, Vec), #[error("URL contains no host")] NoHostInUrl, #[error("Inconsistent state error")] diff --git a/warpgate-protocol-http/src/api/info.rs b/warpgate-protocol-http/src/api/info.rs index 09678c979..8ba945fe7 100644 --- a/warpgate-protocol-http/src/api/info.rs +++ b/warpgate-protocol-http/src/api/info.rs @@ -35,10 +35,6 @@ enum InstanceInfoResponse { Ok(Json), } -fn strip_port(host: &str) -> Option<&str> { - host.split(':').next() -} - #[OpenApi] impl Api { #[oai(path = "/info", method = "get", operation_id = "get_info")] @@ -50,18 +46,15 @@ impl Api { ) -> poem::Result { let config = services.config.lock().await; let external_host = config - .store - .external_host - .as_deref() - .and_then(strip_port) - .or_else(|| req.header(http::header::HOST).and_then(strip_port)) - .or_else(|| req.original_uri().host()); + .construct_external_url(Some(req), None)? + .host() + .map(|x| x.to_string()); Ok(InstanceInfoResponse::Ok(Json(Info { version: env!("CARGO_PKG_VERSION").to_string(), username: session.get_username(), selected_target: session.get_target_name(), - external_host: external_host.map(str::to_string), + external_host, authorized_via_ticket: matches!( session.get_auth(), Some(SessionAuthorization::Ticket { .. }) diff --git a/warpgate-protocol-http/src/api/sso_provider_detail.rs b/warpgate-protocol-http/src/api/sso_provider_detail.rs index 3248f1c47..dd465e290 100644 --- a/warpgate-protocol-http/src/api/sso_provider_detail.rs +++ b/warpgate-protocol-http/src/api/sso_provider_detail.rs @@ -54,15 +54,18 @@ impl Api { let name = name.0; - let mut return_url = config.construct_external_url(Some(req))?; - return_url.set_path("@warpgate/api/sso/return"); - debug!("Return URL: {}", &return_url); - let Some(provider_config) = config.store.sso_providers.iter().find(|p| p.name == *name) else { return Ok(StartSsoResponse::NotFound); }; + let mut return_url = config.construct_external_url( + Some(req), + provider_config.return_domain_whitelist.as_deref(), + )?; + return_url.set_path("@warpgate/api/sso/return"); + debug!("Return URL: {}", &return_url); + let client = SsoClient::new(provider_config.provider.clone()); let sso_req = client diff --git a/warpgate-protocol-http/src/api/sso_provider_list.rs b/warpgate-protocol-http/src/api/sso_provider_list.rs index 7037f5d7e..26c683922 100644 --- a/warpgate-protocol-http/src/api/sso_provider_list.rs +++ b/warpgate-protocol-http/src/api/sso_provider_list.rs @@ -300,7 +300,7 @@ impl Api { let config = services.config.lock().await; - let return_url = config.construct_external_url(Some(req))?; + let return_url = config.construct_external_url(Some(req), None)?; debug!("Return URL: {}", &return_url); let Some(provider_config) = config diff --git a/warpgate-protocol-http/src/lib.rs b/warpgate-protocol-http/src/lib.rs index 4493523aa..7c8fe3ba4 100644 --- a/warpgate-protocol-http/src/lib.rs +++ b/warpgate-protocol-http/src/lib.rs @@ -124,9 +124,8 @@ impl ProtocolServer for HTTPProtocolServer { let url = req.original_uri().clone(); let client_ip = get_client_ip(&req).await?; - let response = ep.call(req).await.map_err(|e| { - log_request_error(&method, &url, &client_ip, &e); - e + let response = ep.call(req).await.inspect_err(|e| { + log_request_error(&method, &url, &client_ip, e); })?; log_request_result(&method, &url, &client_ip, &response.status()); diff --git a/warpgate-protocol-ssh/src/server/session.rs b/warpgate-protocol-ssh/src/server/session.rs index 5a54f1750..68a1d9051 100644 --- a/warpgate-protocol-ssh/src/server/session.rs +++ b/warpgate-protocol-ssh/src/server/session.rs @@ -1370,7 +1370,7 @@ impl ServerSession { .config .lock() .await - .construct_external_url(None) + .construct_external_url(None, None) { Ok(url) => url, Err(error) => { diff --git a/warpgate-sso/src/config.rs b/warpgate-sso/src/config.rs index 623c4223c..dfd36419b 100644 --- a/warpgate-sso/src/config.rs +++ b/warpgate-sso/src/config.rs @@ -21,6 +21,7 @@ pub struct SsoProviderConfig { pub name: String, pub label: Option, pub provider: SsoInternalProviderConfig, + pub return_domain_whitelist: Option>, } impl SsoProviderConfig {