From bab4dba58b2a9c4404484295bcad31a2bcee5eb8 Mon Sep 17 00:00:00 2001 From: balaji-jr Date: Thu, 26 Sep 2024 20:08:28 +0530 Subject: [PATCH 1/2] fixed redirect url validation and included support for custom err messge --- server/src/handlers/http/oidc.rs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/server/src/handlers/http/oidc.rs b/server/src/handlers/http/oidc.rs index 58c550b4c..4c1a0dfc1 100644 --- a/server/src/handlers/http/oidc.rs +++ b/server/src/handlers/http/oidc.rs @@ -29,6 +29,7 @@ use openid::{Options, Token, Userinfo}; use serde::Deserialize; use ulid::Ulid; use url::Url; +use regex::Regex; use crate::{ handlers::{COOKIE_AGE_DAYS, OIDC_SCOPE, SESSION_COOKIE_NAME, USER_COOKIE_NAME}, @@ -64,10 +65,12 @@ pub async fn login( query: web::Query, ) -> Result { let conn = req.connection_info().clone(); - let base_url = format!("{}://{}/", conn.scheme(), conn.host()); - if !base_url.eq(query.redirect.as_str()) { - return Err(OIDCError::BadRequest); + let base_url_without_scheme = format!("{}/", conn.host()); + + if !is_valid_redirect_url(&base_url_without_scheme, query.redirect.as_str() ) { + return Err(OIDCError::BadRequest("Bad Request, Invalid redirect url!".to_string())); } + let oidc_client = req.app_data::>(); let session_key = extract_session_key_from_req(&req).ok(); let (session_key, oidc_client) = match (session_key, oidc_client) { @@ -99,7 +102,7 @@ pub async fn login( [user_cookie, session_cookie], )) } - _ => Err(OIDCError::BadRequest), + _ => Err(OIDCError::BadRequest("Bad Request".to_string())), }, // if it's a valid active session, just redirect back key @ SessionKey::SessionId(_) => { @@ -367,8 +370,8 @@ pub enum OIDCError { ObjectStorageError(#[from] ObjectStorageError), #[error("{0}")] Serde(#[from] serde_json::Error), - #[error("Bad Request")] - BadRequest, + #[error("{0}")] + BadRequest(String), #[error("Unauthorized")] Unauthorized, } @@ -378,7 +381,7 @@ impl actix_web::ResponseError for OIDCError { match self { Self::ObjectStorageError(_) => StatusCode::INTERNAL_SERVER_ERROR, Self::Serde(_) => StatusCode::INTERNAL_SERVER_ERROR, - Self::BadRequest => StatusCode::BAD_REQUEST, + Self::BadRequest(_) => StatusCode::BAD_REQUEST, Self::Unauthorized => StatusCode::UNAUTHORIZED, } } @@ -389,3 +392,10 @@ impl actix_web::ResponseError for OIDCError { .body(self.to_string()) } } + +fn is_valid_redirect_url(base_url_without_scheme: &str, redirect_url: &str) -> bool { + let http_scheme_match_regex = Regex::new(r"^(https?://)").unwrap(); + let redirect_url_without_scheme = http_scheme_match_regex.replace(redirect_url, ""); + + base_url_without_scheme == redirect_url_without_scheme +} From 053ec59b25429a5f50f7057a4d44981eaecf25cc Mon Sep 17 00:00:00 2001 From: balaji-jr Date: Fri, 27 Sep 2024 09:10:43 +0530 Subject: [PATCH 2/2] formatting --- server/src/handlers/http/oidc.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/server/src/handlers/http/oidc.rs b/server/src/handlers/http/oidc.rs index 4c1a0dfc1..e997f1208 100644 --- a/server/src/handlers/http/oidc.rs +++ b/server/src/handlers/http/oidc.rs @@ -26,10 +26,10 @@ use actix_web::{ }; use http::StatusCode; use openid::{Options, Token, Userinfo}; +use regex::Regex; use serde::Deserialize; use ulid::Ulid; use url::Url; -use regex::Regex; use crate::{ handlers::{COOKIE_AGE_DAYS, OIDC_SCOPE, SESSION_COOKIE_NAME, USER_COOKIE_NAME}, @@ -67,8 +67,10 @@ pub async fn login( let conn = req.connection_info().clone(); let base_url_without_scheme = format!("{}/", conn.host()); - if !is_valid_redirect_url(&base_url_without_scheme, query.redirect.as_str() ) { - return Err(OIDCError::BadRequest("Bad Request, Invalid redirect url!".to_string())); + if !is_valid_redirect_url(&base_url_without_scheme, query.redirect.as_str()) { + return Err(OIDCError::BadRequest( + "Bad Request, Invalid Redirect URL!".to_string(), + )); } let oidc_client = req.app_data::>();