Review fix

.env.template

@@ -471,31 +471,42 @@
 # SSO_ENABLED=false
 ## Prevent users from logging in directly without going through SSO
 # SSO_ONLY=false
+
 ## On SSO Signup if a user with a matching email already exists make the association
 # SSO_SIGNUPS_MATCH_EMAIL=true
+
 ## Allow unknown email verification status. Allowing this with `SSO_SIGNUPS_MATCH_EMAIL=true` open potential account takeover.
 # SSO_ALLOW_UNKNOWN_EMAIL_VERIFICATION=false
 ## Base URL of the OIDC server (auto-discovery is used)
 ##  - Should not include the `/.well-known/openid-configuration` part and no trailing `/`
 ##  - ${SSO_AUTHORITY}/.well-known/openid-configuration should return a json document: https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationResponse
 # SSO_AUTHORITY=https://auth.example.com
+
 ## Authorization request scopes. Optional SSO scopes, override if email and profile are not enough (`openid` is implicit).
 #SSO_SCOPES="email profile"
+
 ## Additionnal authorization url parameters (ex: to obtain a `refresh_token` with Google Auth).
 # SSO_AUTHORIZE_EXTRA_PARAMS="access_type=offline&prompt=consent"
+
 ## Activate PKCE for the Auth Code flow.
 # SSO_PKCE=true
+
 ## Regex to add additionnal trusted audience to Id Token (by default only the client_id is trusted).
 # SSO_AUDIENCE_TRUSTED='^$'
+
 ## Set your Client ID and Client Key
 # SSO_CLIENT_ID=11111
 # SSO_CLIENT_SECRET=AAAAAAAAAAAAAAAAAAAAAAAA
+
 ## Optional Master password policy (minComplexity=[0-4]), `enforceOnLogin` is not supported at the moment.
 # SSO_MASTER_PASSWORD_POLICY='{"enforceOnLogin":false,"minComplexity":3,"minLength":12,"requireLower":false,"requireNumbers":false,"requireSpecial":false,"requireUpper":false}'
+
 ## Use sso only for authentication not the session lifecycle
 # SSO_AUTH_ONLY_NOT_SESSION=false
+
 ## Client cache for discovery endpoint. Duration in seconds (0 to disable).
 # SSO_CLIENT_CACHE_EXPIRATION=0
+
 ## Log all the tokens, LOG_LEVEL=debug is required
 # SSO_DEBUG_TOKENS=false
 

SSO.md

@@ -47,7 +47,7 @@ Additionally:
 - Signup will be blocked if the Provider reports the email as `unverified`.
 - Changing the email needs to be done by the user since it requires updating the `key`.
   On login if the email returned by the provider is not the one saved an email will be sent to the user to ask him to update it.
-- If set `SIGNUPS_DOMAINS_WHITELIST` is applied on SSO signup and when attempting to change the email.
+- If set, `SIGNUPS_DOMAINS_WHITELIST` is applied on SSO signup and when attempting to change the email.
 
 This means that if you ever need to change the provider url or the provider itself; you'll have to first delete the association
 then ensure that `SSO_SIGNUPS_MATCH_EMAIL` is activated to allow a new association.
@@ -118,22 +118,7 @@ More details on how to use it in [README.md](playwright/README.md#openid-connect
 ## Auth0
 
 Not working due to the following issue https://github.com/ramosbugs/openidconnect-rs/issues/23 (they appear not to follow the spec).
-A feature flag is available to bypass the issue but since it's a compile time feature you will have to patch with something like:
-
-```patch
-diff --git a/Cargo.toml b/Cargo.toml
-index 0524a7be..9999e852 100644
---- a/Cargo.toml
-+++ b/Cargo.toml
-@@ -150,7 +150,7 @@ paste = "1.0.15"
- governor = "0.6.3"
-
- # OIDC for SSO
--openidconnect = "3.5.0"
-+openidconnect = { version = "3.5.0", features = ["accept-rfc3339-timestamps"] }
- mini-moka = "0.10.2"
-```
-
+A feature flag is available (`oidc-accept-rfc3339-timestamps`) to bypass the issue but you will need to compile the server with it.
 There is no plan at the moment to either always activate the feature nor make a specific distribution for Auth0.
 
 ## Authelia
@@ -291,7 +276,7 @@ There is some issue to handle redirection from your browser (used for sso login)
 
 ### Chrome
 
-Probably not much hope, an [issue](https://github.com/bitwarden/clients/issues/2606) is open on the subject and it appears that both Linux and Windows are not working.
+Some user report having ([issues](https://github.com/bitwarden/clients/issues/12929)).
 
 ## Firefox
 

src/api/core/accounts.rs

@@ -1211,7 +1211,7 @@ struct SecretVerificationRequest {
 
 // Change the KDF Iterations if necessary
 pub async fn kdf_upgrade(user: &mut User, pwd_hash: &str, conn: &mut DbConn) -> ApiResult<()> {
-    if user.password_iterations != CONFIG.password_iterations() {
+    if user.password_iterations < CONFIG.password_iterations() {
         user.password_iterations = CONFIG.password_iterations();
         user.set_password(pwd_hash, None, false, None);
 

src/api/identity.rs

@@ -36,7 +36,6 @@ pub fn routes() -> Vec<Route> {
         identity_register,
         register_verification_email,
         register_finish,
-        _prevalidate,
         prevalidate,
         authorize,
         oidcsignin,
@@ -990,7 +989,7 @@ struct ConnectData {
     #[field(name = uncased("authrequest"))]
     auth_request: Option<AuthRequestId>,
     // Needed for authorization code
-    #[form(field = uncased("code"))]
+    #[field(name = uncased("code"))]
     code: Option<String>,
 }
 fn _check_is_some<T>(value: &Option<T>, msg: &str) -> EmptyResult {
@@ -1000,12 +999,6 @@ fn _check_is_some<T>(value: &Option<T>, msg: &str) -> EmptyResult {
     Ok(())
 }
 
-// Deprecated but still needed for Mobile apps
-#[get("/account/prevalidate")]
-fn _prevalidate() -> JsonResult {
-    prevalidate()
-}
-
 #[get("/sso/prevalidate")]
 fn prevalidate() -> JsonResult {
     if CONFIG.sso_enabled() {
@@ -1032,7 +1025,7 @@ async fn oidcsignin(code: OIDCCode, state: String, conn: DbConn) -> ApiResult<Re
 }
 
 // Bitwarden client appear to only care for code and state so we pipe it through
-// cf: https://github.com/bitwarden/clients/blob/8e46ef1ae5be8b62b0d3d0b9d1b1c62088a04638/libs/angular/src/auth/components/sso.component.ts#L68C11-L68C23)
+// cf: https://github.com/bitwarden/clients/blob/80b74b3300e15b4ae414dc06044cc9b02b6c10a6/libs/auth/src/angular/sso/sso.component.ts#L141
 #[get("/connect/oidc-signin?<state>&<error>&<error_description>", rank = 2)]
 async fn oidcsignin_error(
     state: String,

src/config.rs

@@ -458,7 +458,7 @@ make_config! {
         /// Duo Auth context cleanup schedule |> Cron schedule of the job that cleans expired Duo contexts from the database. Does nothing if Duo MFA is disabled or set to use the legacy iframe prompt.
         /// Defaults to once every minute. Set blank to disable this job.
         duo_context_purge_schedule:   String, false,  def,    "30 * * * * *".to_string();
-        /// Purge incomplete sso nonce. |> Cron schedule of the job that cleans leftover nonce in db due to incomplete sso login.
+        /// Purge incomplete SSO nonce. |> Cron schedule of the job that cleans leftover nonce in db due to incomplete SSO login.
         /// Defaults to daily. Set blank to disable this job.
         purge_incomplete_sso_nonce: String, false,  def,   "0 20 0 * * *".to_string();
     },
@@ -682,10 +682,10 @@ make_config! {
     /// OpenID Connect SSO settings
     sso {
         /// Enabled
-        sso_enabled:                    bool,   false,   def,    false;
-        /// Only sso login |> Disable Email+Master Password login
+        sso_enabled:                    bool,   true,   def,    false;
+        /// Only SSO login |> Disable Email+Master Password login
         sso_only:                       bool,   true,   def,    false;
-        /// Allow email association |> Associate existing non-sso user based on email
+        /// Allow email association |> Associate existing non-SSO user based on email
         sso_signups_match_email:        bool,   true,   def,    true;
         /// Allow unknown email verification status |> Allowing this with `SSO_SIGNUPS_MATCH_EMAIL=true` open potential account takeover.
         sso_allow_unknown_email_verification: bool, false, def, false;
@@ -701,13 +701,13 @@ make_config! {
         sso_authorize_extra_params:     String, false,  def,    String::new();
         /// Use PKCE during Authorization flow
         sso_pkce:                       bool,   false,   def,    true;
-        /// Regex for additionnal trusted Id token audience |> By default only the client_id is trsuted.
+        /// Regex for additionnal trusted Id token audience |> By default only the client_id is trusted.
         sso_audience_trusted:           String, false,  option;
         /// CallBack Path |> Generated from Domain.
         sso_callback_path:              String, false,  generated, |c| generate_sso_callback_path(&c.domain);
-        /// Optional sso master password policy |> Ex format: '{"enforceOnLogin":false,"minComplexity":3,"minLength":12,"requireLower":false,"requireNumbers":false,"requireSpecial":false,"requireUpper":false}'
+        /// Optional SSO master password policy |> Ex format: '{"enforceOnLogin":false,"minComplexity":3,"minLength":12,"requireLower":false,"requireNumbers":false,"requireSpecial":false,"requireUpper":false}'
         sso_master_password_policy:     String, true,  option;
-        /// Use sso only for auth not the session lifecycle |> Use default Vaultwarden session lifecycle (Idle refresh token valid for 30days)
+        /// Use SSO only for auth not the session lifecycle |> Use default Vaultwarden session lifecycle (Idle refresh token valid for 30days)
         sso_auth_only_not_session:      bool,   true,   def,    false;
         /// Client cache for discovery endpoint. |> Duration in seconds (0 or less to disable). More details: https://github.com/dani-garcia/vaultwarden/blob/sso-support/SSO.md#client-cache
         sso_client_cache_expiration:    u64,    true,   def,    0;
@@ -955,10 +955,9 @@ fn validate_config(cfg: &ConfigItems) -> Result<(), Error> {
             err!("`SSO_CLIENT_ID`, `SSO_CLIENT_SECRET` and `SSO_AUTHORITY` must be set for SSO support")
         }
 
-        internal_sso_issuer_url(&cfg.sso_authority)?;
-        internal_sso_redirect_url(&cfg.sso_callback_path)?;
+        validate_internal_sso_issuer_url(&cfg.sso_authority)?;
+        validate_internal_sso_redirect_url(&cfg.sso_callback_path)?;
         check_master_password_policy(&cfg.sso_master_password_policy)?;
-        internal_sso_authorize_extra_params_vec(&cfg.sso_authorize_extra_params)?;
     }
 
     if cfg._enable_yubico {
@@ -1138,27 +1137,20 @@ fn validate_config(cfg: &ConfigItems) -> Result<(), Error> {
     Ok(())
 }
 
-fn internal_sso_issuer_url(sso_authority: &String) -> Result<openidconnect::IssuerUrl, Error> {
+fn validate_internal_sso_issuer_url(sso_authority: &String) -> Result<openidconnect::IssuerUrl, Error> {
     match openidconnect::IssuerUrl::new(sso_authority.clone()) {
         Err(err) => err!(format!("Invalid sso_authority UR ({sso_authority}): {err}")),
         Ok(issuer_url) => Ok(issuer_url),
     }
 }
 
-fn internal_sso_redirect_url(sso_callback_path: &String) -> Result<openidconnect::RedirectUrl, Error> {
+fn validate_internal_sso_redirect_url(sso_callback_path: &String) -> Result<openidconnect::RedirectUrl, Error> {
     match openidconnect::RedirectUrl::new(sso_callback_path.clone()) {
         Err(err) => err!(format!("Invalid sso_callback_path ({sso_callback_path} built using `domain`) URL: {err}")),
         Ok(redirect_url) => Ok(redirect_url),
     }
 }
 
-fn internal_sso_authorize_extra_params_vec(config: &str) -> Result<Vec<(String, String)>, Error> {
-    match parse_param_list(config.to_owned(), '&', '=') {
-        Err(e) => err!(format!("Invalid SSO_AUTHORIZE_EXTRA_PARAMS: {e}")),
-        Ok(params) => Ok(params),
-    }
-}
-
 fn check_master_password_policy(sso_master_password_policy: &Option<String>) -> Result<(), Error> {
     let policy = sso_master_password_policy.as_ref().map(|mpp| serde_json::from_str::<serde_json::Value>(mpp));
     if let Some(Err(error)) = policy {
@@ -1244,26 +1236,6 @@ fn smtp_convert_deprecated_ssl_options(smtp_ssl: Option<bool>, smtp_explicit_tls
     "starttls".to_string()
 }
 
-/// Allow to parse a list of Key/Values (Ex: `key1=value&key2=value2`)
-/// - line break are handled as `separator`
-fn parse_param_list(config: String, separator: char, kv_separator: char) -> Result<Vec<(String, String)>, Error> {
-    config
-        .lines()
-        .flat_map(|l| l.split(separator))
-        .map(|l| l.trim())
-        .filter(|l| !l.is_empty())
-        .map(|l| {
-            let split = l.split(kv_separator).collect::<Vec<&str>>();
-            match &split[..] {
-                [key, value] => Ok(((*key).to_string(), (*value).to_string())),
-                _ => {
-                    err!(format!("Failed to parse ({l}). Expected key{kv_separator}value"))
-                }
-            }
-        })
-        .collect()
-}
-
 fn opendal_operator_for_path(path: &str) -> Result<opendal::Operator, Error> {
     // Cache of previously built operators by path
     static OPERATORS_BY_PATH: LazyLock<dashmap::DashMap<String, opendal::Operator>> =
@@ -1459,7 +1431,7 @@ impl Config {
 
     // The registration link should be hidden if
     //  - Signup is not allowed and email whitelist is empty unless mail is disabled and invitations are allowed
-    //  - The sso is activated and password login is disabled.
+    //  - The SSO is activated and password login is disabled.
     pub fn is_signup_disabled(&self) -> bool {
         (!self.signups_allowed()
             && self.signups_domains_whitelist().is_empty()
@@ -1582,19 +1554,19 @@ impl Config {
     }
 
     pub fn sso_issuer_url(&self) -> Result<openidconnect::IssuerUrl, Error> {
-        internal_sso_issuer_url(&self.sso_authority())
+        validate_internal_sso_issuer_url(&self.sso_authority())
     }
 
     pub fn sso_redirect_url(&self) -> Result<openidconnect::RedirectUrl, Error> {
-        internal_sso_redirect_url(&self.sso_callback_path())
+        validate_internal_sso_redirect_url(&self.sso_callback_path())
     }
 
     pub fn sso_scopes_vec(&self) -> Vec<String> {
         self.sso_scopes().split_whitespace().map(str::to_string).collect()
     }
 
-    pub fn sso_authorize_extra_params_vec(&self) -> Result<Vec<(String, String)>, Error> {
-        internal_sso_authorize_extra_params_vec(&self.sso_authorize_extra_params())
+    pub fn sso_authorize_extra_params_vec(&self) -> Vec<(String, String)> {
+        url::form_urlencoded::parse(self.sso_authorize_extra_params().as_bytes()).into_owned().collect()
     }
 }
 
@@ -1760,54 +1732,3 @@ handlebars::handlebars_helper!(webver: | web_vault_version: String |
 handlebars::handlebars_helper!(vwver: | vw_version: String |
     semver::VersionReq::parse(&vw_version).expect("Invalid Vaultwarden version compare string").matches(&VW_VERSION)
 );
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn test_parse_param_list() {
-        let config = "key1=value&key2=value2&".to_string();
-        let parsed = parse_param_list(config, '&', '=');
-
-        assert_eq!(
-            parsed.unwrap(),
-            vec![("key1".to_string(), "value".to_string()), ("key2".to_string(), "value2".to_string())]
-        );
-    }
-
-    #[test]
-    fn test_parse_param_list_lines() {
-        let config = r#"
-        key1=value
-        key2=value2
-        "#
-        .to_string();
-        let parsed = parse_param_list(config, '&', '=');
-
-        assert_eq!(
-            parsed.unwrap(),
-            vec![("key1".to_string(), "value".to_string()), ("key2".to_string(), "value2".to_string())]
-        );
-    }
-
-    #[test]
-    fn test_parse_param_list_mixed() {
-        let config = r#"key1=value&key2=value2&
-        &key3=value3&&
-        &key4=value4
-        "#
-        .to_string();
-        let parsed = parse_param_list(config, '&', '=');
-
-        assert_eq!(
-            parsed.unwrap(),
-            vec![
-                ("key1".to_string(), "value".to_string()),
-                ("key2".to_string(), "value2".to_string()),
-                ("key3".to_string(), "value3".to_string()),
-                ("key4".to_string(), "value4".to_string()),
-            ]
-        );
-    }
-}

src/db/models/event.rs

@@ -89,7 +89,7 @@ pub enum EventType {
     OrganizationUserUpdated = 1502,
     OrganizationUserRemoved = 1503, // Organization user data was deleted
     OrganizationUserUpdatedGroups = 1504,
-    OrganizationUserUnlinkedSso = 1505, // Not supported
+    OrganizationUserUnlinkedSso = 1505,
     OrganizationUserResetPasswordEnroll = 1506,
     OrganizationUserResetPasswordWithdraw = 1507,
     OrganizationUserAdminResetPassword = 1508,

src/sso_client.rs

@@ -124,7 +124,7 @@ impl Client {
                 Nonce::new_random,
             )
             .add_scopes(scopes)
-            .add_extra_params(CONFIG.sso_authorize_extra_params_vec()?);
+            .add_extra_params(CONFIG.sso_authorize_extra_params_vec());
 
         let verifier = if CONFIG.sso_pkce() {
             let (pkce_challenge, pkce_verifier) = PkceCodeChallenge::new_random_sha256();