From 02a9ab5d48e6f3e8a17796fd3ab27d3209ccd90f Mon Sep 17 00:00:00 2001 From: Timshel Date: Thu, 28 Nov 2024 13:59:24 +0100 Subject: [PATCH] Base64 encode state before sending it to providers --- src/api/identity.rs | 28 ++++++++++++++++++++-------- src/sso.rs | 12 +++++++----- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/api/identity.rs b/src/api/identity.rs index f63f0146..306d4aa7 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -931,10 +931,10 @@ fn prevalidate() -> JsonResult { #[get("/connect/oidc-signin?&", rank = 1)] async fn oidcsignin(code: String, state: String, conn: DbConn) -> ApiResult { oidcsignin_redirect( - state.clone(), - sso::OIDCCodeWrapper::Ok { + state, + |decoded_state| sso::OIDCCodeWrapper::Ok { + state: decoded_state, code, - state, }, &conn, ) @@ -951,9 +951,9 @@ async fn oidcsignin_error( conn: DbConn, ) -> ApiResult { oidcsignin_redirect( - state.clone(), - sso::OIDCCodeWrapper::Error { - state, + state, + |decoded_state| sso::OIDCCodeWrapper::Error { + state: decoded_state, error, error_description, }, @@ -962,9 +962,21 @@ async fn oidcsignin_error( .await } +// The state was encoded using Base64 to ensure no issue with providers. // iss and scope parameters are needed for redirection to work on IOS. -async fn oidcsignin_redirect(state: String, wrapper: sso::OIDCCodeWrapper, conn: &DbConn) -> ApiResult { - let code = sso::encode_code_claims(wrapper); +async fn oidcsignin_redirect( + base64_state: String, + wrapper: impl FnOnce(String) -> sso::OIDCCodeWrapper, + conn: &DbConn, +) -> ApiResult { + let state = match data_encoding::BASE64.decode(base64_state.as_bytes()) { + Ok(vec) => match String::from_utf8(vec) { + Ok(valid) => valid, + Err(_) => err!(format!("Invalid utf8 chars in {base64_state} after base64 decoding")), + }, + Err(_) => err!(format!("Failed to decode {base64_state} using base64")), + }; + let code = sso::encode_code_claims(wrapper(state.clone())); let nonce = match SsoNonce::find(&state, conn).await { Some(n) => n, diff --git a/src/sso.rs b/src/sso.rs index ad57fbbc..0df5700c 100644 --- a/src/sso.rs +++ b/src/sso.rs @@ -81,8 +81,8 @@ pub fn encode_ssotoken_claims() -> String { #[derive(Debug, Serialize, Deserialize)] pub enum OIDCCodeWrapper { Ok { - code: String, state: String, + code: String, }, Error { state: String, @@ -209,9 +209,11 @@ impl CoreClientExt for CoreClient { } // The `nonce` allow to protect against replay attacks +// The `state` is encoded using base64 to ensure no issue with providers (It contains the Organization identifier). // redirect_uri from: https://github.com/bitwarden/server/blob/main/src/Identity/IdentityServer/ApiClient.cs pub async fn authorize_url(state: String, client_id: &str, raw_redirect_uri: &str, mut conn: DbConn) -> ApiResult { let scopes = CONFIG.sso_scopes_vec().into_iter().map(Scope::new); + let base64_state = data_encoding::BASE64.encode(state.as_bytes()); let redirect_uri = match client_id { "web" | "browser" => format!("{}/sso-connector.html", CONFIG.domain()), @@ -230,7 +232,7 @@ pub async fn authorize_url(state: String, client_id: &str, raw_redirect_uri: &st let mut auth_req = client .authorize_url( AuthenticationFlow::::AuthorizationCode, - || CsrfToken::new(state), + || CsrfToken::new(base64_state), Nonce::new_random, ) .add_scopes(scopes) @@ -244,9 +246,9 @@ pub async fn authorize_url(state: String, client_id: &str, raw_redirect_uri: &st None }; - let (auth_url, csrf_state, nonce) = auth_req.url(); + let (auth_url, _, nonce) = auth_req.url(); - let sso_nonce = SsoNonce::new(csrf_state.secret().to_string(), nonce.secret().to_string(), verifier, redirect_uri); + let sso_nonce = SsoNonce::new(state, nonce.secret().to_string(), verifier, redirect_uri); sso_nonce.save(&mut conn).await?; Ok(auth_url) @@ -276,8 +278,8 @@ async fn decode_code_claims(code: &str, conn: &mut DbConn) -> ApiResult<(String, match auth::decode_jwt::(code, SSO_JWT_ISSUER.to_string()) { Ok(code_claims) => match code_claims.code { OIDCCodeWrapper::Ok { - code, state, + code, } => Ok((code, state)), OIDCCodeWrapper::Error { state,