From e94d0f959bba4ff6eb13c064418209df2bb46d50 Mon Sep 17 00:00:00 2001 From: Timshel Date: Sat, 6 Jan 2024 11:36:35 +0100 Subject: [PATCH] Wrap sso errors as a custom JWT and propagate it as OIDC code --- src/api/identity.rs | 32 +++++++++------------------ src/auth.rs | 27 +++++++++++++++++++++++ src/config.rs | 2 -- src/sso.rs | 54 +++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 88 insertions(+), 27 deletions(-) diff --git a/src/api/identity.rs b/src/api/identity.rs index d23a011cbb..85601f5c31 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -813,29 +813,19 @@ fn prevalidate() -> JsonResult { #[get("/connect/oidc-signin?&", rank = 1)] fn oidcsignin(code: String, state: String, jar: &CookieJar<'_>) -> ApiResult { - let redirect_root = jar - .get(sso::COOKIE_NAME_REDIRECT) - .map(|c| c.value().to_string()) - .unwrap_or(format!("{}/sso-connector.html", CONFIG.domain())); - - let mut url = match url::Url::parse(&redirect_root) { - Err(err) => err!(format!("Failed to parse redirect url ({redirect_root}): {err}")), - Ok(url) => url, - }; - - url.query_pairs_mut().append_pair("code", &code).append_pair("state", &state); - - debug!("Redirection to {url}"); - - Ok(Redirect::temporary(String::from(url))) + sso::format_bitwarden_redirect(&code, &state, jar) } -// No good way to display the error -// Bitwarden client appear to only care for code and state -// cf: https://github.com/bitwarden/clients/blob/8e46ef1ae5be8b62b0d3d0b9d1b1c62088a04638/libs/angular/src/auth/components/sso.component.ts#L68C11-L68C23) -#[get("/connect/oidc-signin?&", rank = 2)] -fn oidcsignin_error(error: String, error_description: Option) -> ApiResult { - err!(format!("SSO login failed with {error} and {:?}", error_description)) +// To display the error we wrap it as JWT token +#[get("/connect/oidc-signin?&&", rank = 2)] +fn oidcsignin_error( + error: String, + error_description: Option, + state: String, + jar: &CookieJar<'_>, +) -> ApiResult { + let as_token = sso::wrap_sso_errors(error, error_description); + sso::format_bitwarden_redirect(&as_token, &state, jar) } #[derive(Debug, Clone, Default, FromForm)] diff --git a/src/auth.rs b/src/auth.rs index 07ea22eb66..0ac4d01dda 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -29,6 +29,7 @@ static JWT_INVITE_ISSUER: Lazy = Lazy::new(|| format!("{}|invite", CONFI static JWT_EMERGENCY_ACCESS_INVITE_ISSUER: Lazy = Lazy::new(|| format!("{}|emergencyaccessinvite", CONFIG.domain_origin())); static JWT_SSOTOKEN_ISSUER: Lazy = Lazy::new(|| format!("{}|ssotoken", CONFIG.domain_origin())); +static JWT_SSO_ERROR_ISSUER: Lazy = Lazy::new(|| format!("{}|ssoerror", CONFIG.domain_origin())); static JWT_DELETE_ISSUER: Lazy = Lazy::new(|| format!("{}|delete", CONFIG.domain_origin())); static JWT_VERIFYEMAIL_ISSUER: Lazy = Lazy::new(|| format!("{}|verifyemail", CONFIG.domain_origin())); static JWT_ADMIN_ISSUER: Lazy = Lazy::new(|| format!("{}|admin", CONFIG.domain_origin())); @@ -117,6 +118,10 @@ pub fn decode_file_download(token: &str) -> Result { decode_jwt(token, JWT_FILE_DOWNLOAD_ISSUER.to_string()) } +pub fn decode_sso_error(token: &str) -> Result { + decode_jwt(token, JWT_SSO_ERROR_ISSUER.to_string()) +} + #[derive(Debug, Serialize, Deserialize)] pub struct LoginJwtClaims { // Not before @@ -422,6 +427,28 @@ pub fn generate_send_claims(send_id: &str, file_id: &str) -> BasicJwtClaims { } } +#[derive(Debug, Serialize, Deserialize)] +pub struct SSOCodeErrorClaims { + // Expiration time + pub exp: i64, + // Issuer + pub iss: String, + + pub error: String, + pub error_description: Option, +} + +pub fn generate_sso_error_claims(error: String, error_description: Option) -> String { + let code_error = SSOCodeErrorClaims { + exp: (Utc::now().naive_utc() + Duration::minutes(2)).timestamp(), + iss: JWT_SSO_ERROR_ISSUER.to_string(), + error, + error_description, + }; + + encode_jwt(&code_error) +} + // // Bearer token authentication // diff --git a/src/config.rs b/src/config.rs index 9033db3362..f83753553d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -621,8 +621,6 @@ make_config! { sso_client_id: String, true, def, String::new(); /// Client Key sso_client_secret: Pass, true, def, String::new(); - /// Silent redirect - sso_auth_failure_silent: bool, true, def, false; /// Authority Server sso_authority: String, true, def, String::new(); /// Scopes required for authorize diff --git a/src/sso.rs b/src/sso.rs index de01121987..7259b318d2 100644 --- a/src/sso.rs +++ b/src/sso.rs @@ -1,4 +1,5 @@ use chrono::Utc; +use rocket::{http::CookieJar, response::Redirect}; use std::collections::HashMap; use std::sync::RwLock; use std::time::Duration; @@ -13,6 +14,7 @@ use openidconnect::{ AccessToken, AuthenticationFlow, AuthorizationCode, ClientId, ClientSecret, CsrfToken, IdToken, Nonce, OAuth2TokenResponse, RefreshToken, Scope, }; +use regex::Regex; use crate::{ api::core::organizations::CollectionData, @@ -36,6 +38,7 @@ static CLIENT_CACHE: RwLock> = RwLock::new(None); static SSO_JWT_VALIDATION: Lazy = Lazy::new(prepare_decoding); static DEFAULT_BW_EXPIRATION: Lazy = Lazy::new(|| chrono::Duration::minutes(5)); +static SSO_ERRORS_REGEX: Lazy = Lazy::new(|| Regex::new(r"^error_(.*)$").unwrap()); // Will Panic if SSO is activated and a key file is present but we can't decode its content pub fn pre_load_sso_jwt_validation() { @@ -239,12 +242,12 @@ impl Decoding { Ok(groups) => groups, Err(err) => { error!("Failed to parse user ({email}) groups: {err}"); - Vec::new() + Vec::with_capacity(0) } } } else { debug!("No groups in {email} access_token"); - Vec::new() + Vec::with_capacity(0) } } @@ -350,12 +353,54 @@ pub struct UserInformation { pub user_name: Option, } +// Wrap the errors in a JWT token to be able to pass it as an OpenID response `code` +pub fn wrap_sso_errors(error: String, error_description: Option) -> String { + format!("error_{}", auth::generate_sso_error_claims(error, error_description)) +} + +// Check if the code is not in fact errors +fn unwrap_sso_erors(code: &str) -> Option> { + SSO_ERRORS_REGEX.captures(code).and_then(|captures| captures.get(1).map(|ma| auth::decode_sso_error(ma.as_str()))) +} + +// Use URL to encode query parameters +pub fn format_bitwarden_redirect(code: &str, state: &str, jar: &CookieJar<'_>) -> ApiResult { + let redirect_root = jar + .get(COOKIE_NAME_REDIRECT) + .map(|c| c.value().to_string()) + .unwrap_or(format!("{}/sso-connector.html", CONFIG.domain())); + + let mut url = match url::Url::parse(&redirect_root) { + Err(err) => err!(format!("Failed to parse redirect url ({redirect_root}): {err}")), + Ok(url) => url, + }; + + url.query_pairs_mut().append_pair("code", code).append_pair("state", state); + + debug!("Redirection to {url}"); + + Ok(Redirect::temporary(String::from(url))) +} + // During the 2FA flow we will // - retrieve the user information and then only discover he needs 2FA. // - second time we will rely on the `AC_CACHE` since the `code` has already been exchanged. // The `nonce` will ensure that the user is authorized only once. // We return only the `UserInformation` to force calling `redeem` to obtain the `refresh_token`. pub async fn exchange_code(code: &String) -> ApiResult { + match unwrap_sso_erors(code) { + Some(Ok(auth::SSOCodeErrorClaims { + error, + error_description, + .. + })) => { + let description = error_description.unwrap_or(String::new()); + err!(format!("Failed to login: {}, {}", error, description)) + } + Some(Err(error)) => err!(format!("Failed to decode SSO error: {error}")), + None => (), + } + if let Some(authenticated_user) = AC_CACHE.get(code) { return Ok(UserInformation { email: authenticated_user.email, @@ -566,8 +611,9 @@ pub async fn sync_groups( let db_user_orgs = UserOrganization::find_any_state_by_user(&user.uuid, conn).await; let user_orgs = db_user_orgs.iter().map(|uo| (uo.org_uuid.clone(), uo)).collect::>(); - let org_groups: Vec = vec![]; - let org_collections: Vec = vec![]; + // Only support `access_all=true` for groups/collections + let org_groups: Vec = Vec::with_capacity(0); + let org_collections: Vec = Vec::with_capacity(0); for group in groups { if let Some(org) = Organization::find_by_name(group, conn).await {