Wrap sso errors as a custom JWT and propagate it as OIDC code

src/api/identity.rs

@@ -813,29 +813,19 @@ fn prevalidate() -> JsonResult {
 
 #[get("/connect/oidc-signin?<code>&<state>", rank = 1)]
 fn oidcsignin(code: String, state: String, jar: &CookieJar<'_>) -> ApiResult<Redirect> {
-    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?<error>&<error_description>", rank = 2)]
-fn oidcsignin_error(error: String, error_description: Option<String>) -> ApiResult<Redirect> {
-    err!(format!("SSO login failed with {error} and {:?}", error_description))
+// To display the error we wrap it as JWT token
+#[get("/connect/oidc-signin?<error>&<error_description>&<state>", rank = 2)]
+fn oidcsignin_error(
+    error: String,
+    error_description: Option<String>,
+    state: String,
+    jar: &CookieJar<'_>,
+) -> ApiResult<Redirect> {
+    let as_token = sso::wrap_sso_errors(error, error_description);
+    sso::format_bitwarden_redirect(&as_token, &state, jar)
 }
 
 #[derive(Debug, Clone, Default, FromForm)]

src/auth.rs

@@ -29,6 +29,7 @@ static JWT_INVITE_ISSUER: Lazy<String> = Lazy::new(|| format!("{}|invite", CONFI
 static JWT_EMERGENCY_ACCESS_INVITE_ISSUER: Lazy<String> =
     Lazy::new(|| format!("{}|emergencyaccessinvite", CONFIG.domain_origin()));
 static JWT_SSOTOKEN_ISSUER: Lazy<String> = Lazy::new(|| format!("{}|ssotoken", CONFIG.domain_origin()));
+static JWT_SSO_ERROR_ISSUER: Lazy<String> = Lazy::new(|| format!("{}|ssoerror", CONFIG.domain_origin()));
 static JWT_DELETE_ISSUER: Lazy<String> = Lazy::new(|| format!("{}|delete", CONFIG.domain_origin()));
 static JWT_VERIFYEMAIL_ISSUER: Lazy<String> = Lazy::new(|| format!("{}|verifyemail", CONFIG.domain_origin()));
 static JWT_ADMIN_ISSUER: Lazy<String> = Lazy::new(|| format!("{}|admin", CONFIG.domain_origin()));
@@ -117,6 +118,10 @@ pub fn decode_file_download(token: &str) -> Result<FileDownloadClaims, Error> {
     decode_jwt(token, JWT_FILE_DOWNLOAD_ISSUER.to_string())
 }
 
+pub fn decode_sso_error(token: &str) -> Result<SSOCodeErrorClaims, Error> {
+    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<String>,
+}
+
+pub fn generate_sso_error_claims(error: String, error_description: Option<String>) -> 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
 //

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

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<Option<CoreClient>> = RwLock::new(None);
 static SSO_JWT_VALIDATION: Lazy<Decoding> = Lazy::new(prepare_decoding);
 
 static DEFAULT_BW_EXPIRATION: Lazy<chrono::Duration> = Lazy::new(|| chrono::Duration::minutes(5));
+static SSO_ERRORS_REGEX: Lazy<Regex> = 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<String>,
 }
 
+// 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>) -> 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<Result<auth::SSOCodeErrorClaims, crate::error::Error>> {
+    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<Redirect> {
+    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<UserInformation> {
+    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::<HashMap<_, _>>();
 
-        let org_groups: Vec<String> = vec![];
-        let org_collections: Vec<CollectionData> = vec![];
+        // Only support `access_all=true` for groups/collections
+        let org_groups: Vec<String> = Vec::with_capacity(0);
+        let org_collections: Vec<CollectionData> = Vec::with_capacity(0);
 
         for group in groups {
             if let Some(org) = Organization::find_by_name(group, conn).await {