Skip to content

Commit d11a087

Browse files
authored
fixed redirect url validation and included support for custom err messsge (#944)
also add support for custom err message fixes #942
1 parent 715a546 commit d11a087

File tree

1 file changed

+19
-7
lines changed

1 file changed

+19
-7
lines changed

server/src/handlers/http/oidc.rs

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ use actix_web::{
2626
};
2727
use http::StatusCode;
2828
use openid::{Options, Token, Userinfo};
29+
use regex::Regex;
2930
use serde::Deserialize;
3031
use ulid::Ulid;
3132
use url::Url;
@@ -64,10 +65,14 @@ pub async fn login(
6465
query: web::Query<RedirectAfterLogin>,
6566
) -> Result<HttpResponse, OIDCError> {
6667
let conn = req.connection_info().clone();
67-
let base_url = format!("{}://{}/", conn.scheme(), conn.host());
68-
if !base_url.eq(query.redirect.as_str()) {
69-
return Err(OIDCError::BadRequest);
68+
let base_url_without_scheme = format!("{}/", conn.host());
69+
70+
if !is_valid_redirect_url(&base_url_without_scheme, query.redirect.as_str()) {
71+
return Err(OIDCError::BadRequest(
72+
"Bad Request, Invalid Redirect URL!".to_string(),
73+
));
7074
}
75+
7176
let oidc_client = req.app_data::<Data<DiscoveredClient>>();
7277
let session_key = extract_session_key_from_req(&req).ok();
7378
let (session_key, oidc_client) = match (session_key, oidc_client) {
@@ -99,7 +104,7 @@ pub async fn login(
99104
[user_cookie, session_cookie],
100105
))
101106
}
102-
_ => Err(OIDCError::BadRequest),
107+
_ => Err(OIDCError::BadRequest("Bad Request".to_string())),
103108
},
104109
// if it's a valid active session, just redirect back
105110
key @ SessionKey::SessionId(_) => {
@@ -367,8 +372,8 @@ pub enum OIDCError {
367372
ObjectStorageError(#[from] ObjectStorageError),
368373
#[error("{0}")]
369374
Serde(#[from] serde_json::Error),
370-
#[error("Bad Request")]
371-
BadRequest,
375+
#[error("{0}")]
376+
BadRequest(String),
372377
#[error("Unauthorized")]
373378
Unauthorized,
374379
}
@@ -378,7 +383,7 @@ impl actix_web::ResponseError for OIDCError {
378383
match self {
379384
Self::ObjectStorageError(_) => StatusCode::INTERNAL_SERVER_ERROR,
380385
Self::Serde(_) => StatusCode::INTERNAL_SERVER_ERROR,
381-
Self::BadRequest => StatusCode::BAD_REQUEST,
386+
Self::BadRequest(_) => StatusCode::BAD_REQUEST,
382387
Self::Unauthorized => StatusCode::UNAUTHORIZED,
383388
}
384389
}
@@ -389,3 +394,10 @@ impl actix_web::ResponseError for OIDCError {
389394
.body(self.to_string())
390395
}
391396
}
397+
398+
fn is_valid_redirect_url(base_url_without_scheme: &str, redirect_url: &str) -> bool {
399+
let http_scheme_match_regex = Regex::new(r"^(https?://)").unwrap();
400+
let redirect_url_without_scheme = http_scheme_match_regex.replace(redirect_url, "");
401+
402+
base_url_without_scheme == redirect_url_without_scheme
403+
}

0 commit comments

Comments
 (0)