From e5681258f0ff9de2435c161f1543418b01cf5dc7 Mon Sep 17 00:00:00 2001 From: Timshel Date: Tue, 28 Apr 2026 16:33:45 +0000 Subject: [PATCH 01/17] SSO fallback to UserInfo preferred_username (#7128) Co-authored-by: Timshel --- src/sso.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sso.rs b/src/sso.rs index 2f56f3a6..4844cf1a 100644 --- a/src/sso.rs +++ b/src/sso.rs @@ -283,7 +283,7 @@ pub async fn exchange_code( let email_verified = id_claims.email_verified().or(user_info.email_verified()); - let user_name = id_claims.preferred_username().map(|un| un.to_string()); + let user_name = id_claims.preferred_username().or(user_info.preferred_username()).map(|un| un.to_string()); let refresh_token = token_response.refresh_token().map(|t| t.secret()); if refresh_token.is_none() && CONFIG.sso_scopes_vec().contains(&"offline_access".to_string()) { From cc57e60886e147e38f4099742b110a37d1255f7d Mon Sep 17 00:00:00 2001 From: Timshel Date: Tue, 28 Apr 2026 16:33:49 +0000 Subject: [PATCH 02/17] Dummy identifier need to pass for a guid (#7154) Co-authored-by: Timshel --- src/sso.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sso.rs b/src/sso.rs index 4844cf1a..26ea7375 100644 --- a/src/sso.rs +++ b/src/sso.rs @@ -17,7 +17,7 @@ use crate::{ CONFIG, }; -pub static FAKE_SSO_IDENTIFIER: &str = "vaultwarden-dummy-oidc-identifier"; +pub static FAKE_SSO_IDENTIFIER: &str = "00000000-01DC-01DC-01DC-000000000000"; static SSO_JWT_ISSUER: LazyLock = LazyLock::new(|| format!("{}|sso", CONFIG.domain_origin())); From fd2b6528a9f47f730bad223076a1ee90da5806f9 Mon Sep 17 00:00:00 2001 From: Stefan Melmuk <509385+stefan0xC@users.noreply.github.com> Date: Tue, 28 Apr 2026 18:33:52 +0200 Subject: [PATCH 03/17] add new /identity/accounts/prelogin/password (#7156) --- src/api/identity.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/api/identity.rs b/src/api/identity.rs index b6d659c6..c38fcd34 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -41,6 +41,7 @@ pub fn routes() -> Vec { routes![ login, prelogin, + prelogin_password, identity_register, register_verification_email, register_finish, @@ -982,6 +983,11 @@ async fn prelogin(data: Json, conn: DbConn) -> Json { _prelogin(data, conn).await } +#[post("/accounts/prelogin/password", data = "")] +async fn prelogin_password(data: Json, conn: DbConn) -> Json { + _prelogin(data, conn).await +} + #[post("/accounts/register", data = "")] async fn identity_register(data: Json, conn: DbConn) -> JsonResult { _register(data, false, conn).await From 7883da554e875fbd3b710ac4ec85601a666b7bef Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 28 Apr 2026 19:34:03 +0300 Subject: [PATCH 04/17] Add DuckDuckGo browser device type (#7147) - sync with upstream --- src/db/models/device.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/db/models/device.rs b/src/db/models/device.rs index 1026574c..7364a2ec 100644 --- a/src/db/models/device.rs +++ b/src/db/models/device.rs @@ -25,7 +25,7 @@ pub struct Device { pub user_uuid: UserId, pub name: String, - pub atype: i32, // https://github.com/bitwarden/server/blob/9ebe16587175b1c0e9208f84397bb75d0d595510/src/Core/Enums/DeviceType.cs + pub atype: i32, // https://github.com/bitwarden/server/blob/8d547dcc280babab70dd4a3c94ced6a34b12dfbf/src/Core/Enums/DeviceType.cs pub push_uuid: Option, pub push_token: Option, @@ -332,6 +332,8 @@ pub enum DeviceType { MacOsCLI = 24, #[display("Linux CLI")] LinuxCLI = 25, + #[display("DuckDuckGo")] + DuckDuckGoBrowser = 26, } impl DeviceType { @@ -363,6 +365,7 @@ impl DeviceType { 23 => DeviceType::WindowsCLI, 24 => DeviceType::MacOsCLI, 25 => DeviceType::LinuxCLI, + 26 => DeviceType::DuckDuckGoBrowser, _ => DeviceType::UnknownBrowser, } } From 454b8e2a35f9b23050cc22736574fdf298da1bee Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 28 Apr 2026 19:34:15 +0300 Subject: [PATCH 05/17] Apply `duration_suboptimal_units` lint findings (#7144) Quote from lint description: "Using a smaller unit for a duration that is evenly divisible by a larger unit reduces readability. Readers have to mentally convert values, which can be error-prone and makes the code less clear." --- Cargo.toml | 1 + src/api/core/sends.rs | 2 +- src/api/core/two_factor/webauthn.rs | 2 +- src/db/models/attachment.rs | 2 +- src/util.rs | 2 +- 5 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 79eebec0..3d719a66 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -301,6 +301,7 @@ branches_sharing_code = "deny" case_sensitive_file_extension_comparisons = "deny" cast_lossless = "deny" clone_on_ref_ptr = "deny" +duration_suboptimal_units = "deny" equatable_if_let = "deny" excessive_precision = "deny" filter_map_next = "deny" diff --git a/src/api/core/sends.rs b/src/api/core/sends.rs index 10bf85be..22abb396 100644 --- a/src/api/core/sends.rs +++ b/src/api/core/sends.rs @@ -574,7 +574,7 @@ async fn download_url(host: &Host, send_id: &SendId, file_id: &SendFileId) -> Re Ok(format!("{}/api/sends/{send_id}/{file_id}?t={token}", &host.host)) } else { - Ok(operator.presign_read(&format!("{send_id}/{file_id}"), Duration::from_secs(5 * 60)).await?.uri().to_string()) + Ok(operator.presign_read(&format!("{send_id}/{file_id}"), Duration::from_mins(5)).await?.uri().to_string()) } } diff --git a/src/api/core/two_factor/webauthn.rs b/src/api/core/two_factor/webauthn.rs index 0ec0e30e..ad17ce36 100644 --- a/src/api/core/two_factor/webauthn.rs +++ b/src/api/core/two_factor/webauthn.rs @@ -38,7 +38,7 @@ static WEBAUTHN: LazyLock = LazyLock::new(|| { let webauthn = WebauthnBuilder::new(&rp_id, &rp_origin) .expect("Creating WebauthnBuilder failed") .rp_name(&domain) - .timeout(Duration::from_millis(60000)); + .timeout(Duration::from_mins(1)); webauthn.build().expect("Building Webauthn failed") }); diff --git a/src/db/models/attachment.rs b/src/db/models/attachment.rs index 4273c22a..7611b927 100644 --- a/src/db/models/attachment.rs +++ b/src/db/models/attachment.rs @@ -50,7 +50,7 @@ impl Attachment { let token = encode_jwt(&generate_file_download_claims(self.cipher_uuid.clone(), self.id.clone())); Ok(format!("{host}/attachments/{}/{}?token={token}", self.cipher_uuid, self.id)) } else { - Ok(operator.presign_read(&self.get_file_path(), Duration::from_secs(5 * 60)).await?.uri().to_string()) + Ok(operator.presign_read(&self.get_file_path(), Duration::from_mins(5)).await?.uri().to_string()) } } diff --git a/src/util.rs b/src/util.rs index 182b7b3b..06f00b98 100644 --- a/src/util.rs +++ b/src/util.rs @@ -734,7 +734,7 @@ where warn!("Can't connect to database, retrying: {e:?}"); - sleep(Duration::from_millis(1_000)).await; + sleep(Duration::from_secs(1)).await; } } } From fcbdebd6d7a6c202af5e711c632d7a2d38449bb8 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 28 Apr 2026 19:34:40 +0300 Subject: [PATCH 06/17] Apply `ref_option` lint findings (#7143) Quote from the lint description: "More flexibility, better memory optimization, and more idiomatic Rust code. &Option in a function signature breaks encapsulation because the caller must own T and move it into an Option to call with it. When returned, the owner must internally store it as Option in order to return it. At a lower level, &Option points to memory with the presence bit flag plus the T value, whereas Option<&T> is usually optimized to a single pointer, so it may be more optimal." --- Cargo.toml | 1 + src/api/admin.rs | 2 +- src/api/core/accounts.rs | 18 ++++++------ src/api/core/ciphers.rs | 14 ++++----- src/api/core/mod.rs | 2 +- src/api/core/organizations.rs | 4 +-- src/api/identity.rs | 54 +++++++++++++++++------------------ src/api/notifications.rs | 2 +- src/api/push.rs | 4 +-- src/config.rs | 6 ++-- 10 files changed, 54 insertions(+), 53 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3d719a66..1d8a6ca0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -323,6 +323,7 @@ needless_continue = "deny" needless_lifetimes = "deny" option_option = "deny" redundant_clone = "deny" +ref_option = "deny" string_add_assign = "deny" unnecessary_join = "deny" unnecessary_self_imports = "deny" diff --git a/src/api/admin.rs b/src/api/admin.rs index 9a782046..02c976cc 100644 --- a/src/api/admin.rs +++ b/src/api/admin.rs @@ -469,7 +469,7 @@ async fn deauth_user(user_id: UserId, _token: AdminToken, conn: DbConn, nt: Noti if CONFIG.push_enabled() { for device in Device::find_push_devices_by_user(&user.uuid, &conn).await { - match unregister_push_device(&device.push_uuid).await { + match unregister_push_device(device.push_uuid.as_ref()).await { Ok(r) => r, Err(e) => error!("Unable to unregister devices from Bitwarden server: {e}"), }; diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index fa6a3fd2..a8f9768e 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -137,7 +137,7 @@ struct KeysData { } /// Trims whitespace from password hints, and converts blank password hints to `None`. -fn clean_password_hint(password_hint: &Option) -> Option { +fn clean_password_hint(password_hint: Option<&String>) -> Option { match password_hint { None => None, Some(h) => match h.trim() { @@ -147,7 +147,7 @@ fn clean_password_hint(password_hint: &Option) -> Option { } } -fn enforce_password_hint_setting(password_hint: &Option) -> EmptyResult { +fn enforce_password_hint_setting(password_hint: Option<&String>) -> EmptyResult { if password_hint.is_some() && !CONFIG.password_hints_allowed() { err!("Password hints have been disabled by the administrator. Remove the hint and try again."); } @@ -245,8 +245,8 @@ pub async fn _register(data: Json, email_verification: bool, conn: // Check against the password hint setting here so if it fails, the user // can retry without losing their invitation below. - let password_hint = clean_password_hint(&data.master_password_hint); - enforce_password_hint_setting(&password_hint)?; + let password_hint = clean_password_hint(data.master_password_hint.as_ref()); + enforce_password_hint_setting(password_hint.as_ref())?; let mut user = match User::find_by_mail(&email, &conn).await { Some(user) => { @@ -353,8 +353,8 @@ async fn post_set_password(data: Json, headers: Headers, conn: // Check against the password hint setting here so if it fails, // the user can retry without losing their invitation below. - let password_hint = clean_password_hint(&data.master_password_hint); - enforce_password_hint_setting(&password_hint)?; + let password_hint = clean_password_hint(data.master_password_hint.as_ref()); + enforce_password_hint_setting(password_hint.as_ref())?; set_kdf_data(&mut user, &data.kdf)?; @@ -515,8 +515,8 @@ async fn post_password(data: Json, headers: Headers, conn: DbCon err!("Invalid password") } - user.password_hint = clean_password_hint(&data.master_password_hint); - enforce_password_hint_setting(&user.password_hint)?; + user.password_hint = clean_password_hint(data.master_password_hint.as_ref()); + enforce_password_hint_setting(user.password_hint.as_ref())?; log_user_event(EventType::UserChangedPassword as i32, &user.uuid, headers.device.atype, &headers.ip.ip, &conn) .await; @@ -1438,7 +1438,7 @@ async fn put_clear_device_token(device_id: DeviceId, conn: DbConn) -> EmptyResul if let Some(device) = Device::find_by_uuid(&device_id, &conn).await { Device::clear_push_token_by_uuid(&device_id, &conn).await?; - unregister_push_device(&device.push_uuid).await?; + unregister_push_device(device.push_uuid.as_ref()).await?; } Ok(()) diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index 6d4e1f41..29aa859e 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -630,7 +630,7 @@ async fn post_ciphers_import(data: Json, headers: Headers, conn: DbC let mut user = headers.user; user.update_revision(&conn).await?; - nt.send_user_update(UpdateType::SyncVault, &user, &headers.device.push_uuid, &conn).await; + nt.send_user_update(UpdateType::SyncVault, &user, headers.device.push_uuid.as_ref(), &conn).await; Ok(()) } @@ -1005,7 +1005,7 @@ async fn put_cipher_share_selected( } // Multi share actions do not send out a push for each cipher, we need to send a general sync here - nt.send_user_update(UpdateType::SyncCiphers, &headers.user, &headers.device.push_uuid, &conn).await; + nt.send_user_update(UpdateType::SyncCiphers, &headers.user, headers.device.push_uuid.as_ref(), &conn).await; Ok(()) } @@ -1618,7 +1618,7 @@ async fn move_cipher_selected( .await; } else { // Multi move actions do not send out a push for each cipher, we need to send a general sync here - nt.send_user_update(UpdateType::SyncCiphers, &headers.user, &headers.device.push_uuid, &conn).await; + nt.send_user_update(UpdateType::SyncCiphers, &headers.user, headers.device.push_uuid.as_ref(), &conn).await; } if cipher_count != accessible_ciphers_count { @@ -1670,7 +1670,7 @@ async fn purge_org_vault( match Membership::find_confirmed_by_user_and_org(&user.uuid, &organization.org_id, &conn).await { Some(member) if member.atype == MembershipType::Owner => { Cipher::delete_all_by_organization(&organization.org_id, &conn).await?; - nt.send_user_update(UpdateType::SyncVault, &user, &headers.device.push_uuid, &conn).await; + nt.send_user_update(UpdateType::SyncVault, &user, headers.device.push_uuid.as_ref(), &conn).await; log_event( EventType::OrganizationPurgedVault as i32, @@ -1710,7 +1710,7 @@ async fn purge_personal_vault( } user.update_revision(&conn).await?; - nt.send_user_update(UpdateType::SyncVault, &user, &headers.device.push_uuid, &conn).await; + nt.send_user_update(UpdateType::SyncVault, &user, headers.device.push_uuid.as_ref(), &conn).await; Ok(()) } @@ -1805,7 +1805,7 @@ async fn _delete_multiple_ciphers( } // Multi delete actions do not send out a push for each cipher, we need to send a general sync here - nt.send_user_update(UpdateType::SyncCiphers, &headers.user, &headers.device.push_uuid, &conn).await; + nt.send_user_update(UpdateType::SyncCiphers, &headers.user, headers.device.push_uuid.as_ref(), &conn).await; Ok(()) } @@ -1873,7 +1873,7 @@ async fn _restore_multiple_ciphers( } // Multi move actions do not send out a push for each cipher, we need to send a general sync here - nt.send_user_update(UpdateType::SyncCiphers, &headers.user, &headers.device.push_uuid, conn).await; + nt.send_user_update(UpdateType::SyncCiphers, &headers.user, headers.device.push_uuid.as_ref(), conn).await; Ok(Json(json!({ "data": ciphers, diff --git a/src/api/core/mod.rs b/src/api/core/mod.rs index 038b9a6d..e0a56e4d 100644 --- a/src/api/core/mod.rs +++ b/src/api/core/mod.rs @@ -124,7 +124,7 @@ async fn post_eq_domains(data: Json, headers: Headers, conn: Db user.save(&conn).await?; - nt.send_user_update(UpdateType::SyncSettings, &user, &headers.device.push_uuid, &conn).await; + nt.send_user_update(UpdateType::SyncSettings, &user, headers.device.push_uuid.as_ref(), &conn).await; Ok(Json(json!({}))) } diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 318001dc..434f0f9d 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -1463,7 +1463,7 @@ async fn _confirm_invite( let save_result = member_to_confirm.save(conn).await; if let Some(user) = User::find_by_uuid(&member_to_confirm.user_uuid, conn).await { - nt.send_user_update(UpdateType::SyncOrgKeys, &user, &headers.device.push_uuid, conn).await; + nt.send_user_update(UpdateType::SyncOrgKeys, &user, headers.device.push_uuid.as_ref(), conn).await; } save_result @@ -1721,7 +1721,7 @@ async fn _delete_member( .await; if let Some(user) = User::find_by_uuid(&member_to_delete.user_uuid, conn).await { - nt.send_user_update(UpdateType::SyncOrgKeys, &user, &headers.device.push_uuid, conn).await; + nt.send_user_update(UpdateType::SyncOrgKeys, &user, headers.device.push_uuid.as_ref(), conn).await; } member_to_delete.delete(conn).await diff --git a/src/api/identity.rs b/src/api/identity.rs index c38fcd34..72323f73 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -65,43 +65,43 @@ async fn login( let login_result = match data.grant_type.as_ref() { "refresh_token" => { - _check_is_some(&data.refresh_token, "refresh_token cannot be blank")?; + _check_is_some(data.refresh_token.as_ref(), "refresh_token cannot be blank")?; _refresh_login(data, &conn, &client_header.ip).await } "password" if CONFIG.sso_enabled() && CONFIG.sso_only() => err!("SSO sign-in is required"), "password" => { - _check_is_some(&data.client_id, "client_id cannot be blank")?; - _check_is_some(&data.password, "password cannot be blank")?; - _check_is_some(&data.scope, "scope cannot be blank")?; - _check_is_some(&data.username, "username cannot be blank")?; + _check_is_some(data.client_id.as_ref(), "client_id cannot be blank")?; + _check_is_some(data.password.as_ref(), "password cannot be blank")?; + _check_is_some(data.scope.as_ref(), "scope cannot be blank")?; + _check_is_some(data.username.as_ref(), "username cannot be blank")?; - _check_is_some(&data.device_identifier, "device_identifier cannot be blank")?; - _check_is_some(&data.device_name, "device_name cannot be blank")?; - _check_is_some(&data.device_type, "device_type cannot be blank")?; + _check_is_some(data.device_identifier.as_ref(), "device_identifier cannot be blank")?; + _check_is_some(data.device_name.as_ref(), "device_name cannot be blank")?; + _check_is_some(data.device_type.as_ref(), "device_type cannot be blank")?; - _password_login(data, &mut user_id, &conn, &client_header.ip, &client_version).await + _password_login(data, &mut user_id, &conn, &client_header.ip, client_version.as_ref()).await } "client_credentials" => { - _check_is_some(&data.client_id, "client_id cannot be blank")?; - _check_is_some(&data.client_secret, "client_secret cannot be blank")?; - _check_is_some(&data.scope, "scope cannot be blank")?; + _check_is_some(data.client_id.as_ref(), "client_id cannot be blank")?; + _check_is_some(data.client_secret.as_ref(), "client_secret cannot be blank")?; + _check_is_some(data.scope.as_ref(), "scope cannot be blank")?; - _check_is_some(&data.device_identifier, "device_identifier cannot be blank")?; - _check_is_some(&data.device_name, "device_name cannot be blank")?; - _check_is_some(&data.device_type, "device_type cannot be blank")?; + _check_is_some(data.device_identifier.as_ref(), "device_identifier cannot be blank")?; + _check_is_some(data.device_name.as_ref(), "device_name cannot be blank")?; + _check_is_some(data.device_type.as_ref(), "device_type cannot be blank")?; _api_key_login(data, &mut user_id, &conn, &client_header.ip).await } "authorization_code" if CONFIG.sso_enabled() => { - _check_is_some(&data.client_id, "client_id cannot be blank")?; - _check_is_some(&data.code, "code cannot be blank")?; - _check_is_some(&data.code_verifier, "code verifier cannot be blank")?; + _check_is_some(data.client_id.as_ref(), "client_id cannot be blank")?; + _check_is_some(data.code.as_ref(), "code cannot be blank")?; + _check_is_some(data.code_verifier.as_ref(), "code verifier cannot be blank")?; - _check_is_some(&data.device_identifier, "device_identifier cannot be blank")?; - _check_is_some(&data.device_name, "device_name cannot be blank")?; - _check_is_some(&data.device_type, "device_type cannot be blank")?; + _check_is_some(data.device_identifier.as_ref(), "device_identifier cannot be blank")?; + _check_is_some(data.device_name.as_ref(), "device_name cannot be blank")?; + _check_is_some(data.device_type.as_ref(), "device_type cannot be blank")?; - _sso_login(data, &mut user_id, &conn, &client_header.ip, &client_version).await + _sso_login(data, &mut user_id, &conn, &client_header.ip, client_version.as_ref()).await } "authorization_code" => err!("SSO sign-in is not available"), t => err!("Invalid type", t), @@ -177,7 +177,7 @@ async fn _sso_login( user_id: &mut Option, conn: &DbConn, ip: &ClientIp, - client_version: &Option, + client_version: Option<&ClientVersion>, ) -> JsonResult { AuthMethod::Sso.check_scope(data.scope.as_ref())?; @@ -320,7 +320,7 @@ async fn _password_login( user_id: &mut Option, conn: &DbConn, ip: &ClientIp, - client_version: &Option, + client_version: Option<&ClientVersion>, ) -> JsonResult { // Validate scope AuthMethod::Password.check_scope(data.scope.as_ref())?; @@ -734,7 +734,7 @@ async fn twofactor_auth( data: &ConnectData, device: &mut Device, ip: &ClientIp, - client_version: &Option, + client_version: Option<&ClientVersion>, conn: &DbConn, ) -> ApiResult> { let twofactors = TwoFactor::find_by_user(&user.uuid, conn).await; @@ -879,7 +879,7 @@ async fn _json_err_twofactor( providers: &[i32], user_id: &UserId, data: &ConnectData, - client_version: &Option, + client_version: Option<&ClientVersion>, conn: &DbConn, ) -> ApiResult { let mut result = json!({ @@ -1114,7 +1114,7 @@ struct ConnectData { #[field(name = uncased("code_verifier"))] code_verifier: Option, } -fn _check_is_some(value: &Option, msg: &str) -> EmptyResult { +fn _check_is_some(value: Option<&T>, msg: &str) -> EmptyResult { if value.is_none() { err!(msg) } diff --git a/src/api/notifications.rs b/src/api/notifications.rs index 492fdb19..b1d64472 100644 --- a/src/api/notifications.rs +++ b/src/api/notifications.rs @@ -338,7 +338,7 @@ impl WebSocketUsers { } // NOTE: The last modified date needs to be updated before calling these methods - pub async fn send_user_update(&self, ut: UpdateType, user: &User, push_uuid: &Option, conn: &DbConn) { + pub async fn send_user_update(&self, ut: UpdateType, user: &User, push_uuid: Option<&PushId>, conn: &DbConn) { // Skip any processing if both WebSockets and Push are not active if *NOTIFICATIONS_DISABLED { return; diff --git a/src/api/push.rs b/src/api/push.rs index 5000869d..e3ff1383 100644 --- a/src/api/push.rs +++ b/src/api/push.rs @@ -135,7 +135,7 @@ pub async fn register_push_device(device: &mut Device, conn: &DbConn) -> EmptyRe Ok(()) } -pub async fn unregister_push_device(push_id: &Option) -> EmptyResult { +pub async fn unregister_push_device(push_id: Option<&PushId>) -> EmptyResult { if !CONFIG.push_enabled() || push_id.is_none() { return Ok(()); } @@ -206,7 +206,7 @@ pub async fn push_logout(user: &User, acting_device: Option<&Device>, conn: &DbC } } -pub async fn push_user_update(ut: UpdateType, user: &User, push_uuid: &Option, conn: &DbConn) { +pub async fn push_user_update(ut: UpdateType, user: &User, push_uuid: Option<&PushId>, conn: &DbConn) { if Device::check_user_has_push_device(&user.uuid, conn).await { tokio::task::spawn(send_to_push_relay(json!({ "userId": user.uuid, diff --git a/src/config.rs b/src/config.rs index 6ff09467..ae995f69 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1076,7 +1076,7 @@ fn validate_config(cfg: &ConfigItems, on_update: bool) -> Result<(), Error> { validate_internal_sso_issuer_url(&cfg.sso_authority)?; validate_internal_sso_redirect_url(&cfg.sso_callback_path)?; - validate_sso_master_password_policy(&cfg.sso_master_password_policy)?; + validate_sso_master_password_policy(cfg.sso_master_password_policy.as_ref())?; } if cfg._enable_yubico { @@ -1271,7 +1271,7 @@ fn validate_internal_sso_redirect_url(sso_callback_path: &String) -> Result, + sso_master_password_policy: Option<&String>, ) -> Result, Error> { let policy = sso_master_password_policy.as_ref().map(|mpp| serde_json::from_str::(mpp)); @@ -1725,7 +1725,7 @@ impl Config { } pub fn sso_master_password_policy_value(&self) -> Option { - validate_sso_master_password_policy(&self.sso_master_password_policy()).ok().flatten() + validate_sso_master_password_policy(self.sso_master_password_policy().as_ref()).ok().flatten() } pub fn sso_scopes_vec(&self) -> Vec { From 62748100f04178b9b2df7f9fd8f9884fab59fbd8 Mon Sep 17 00:00:00 2001 From: Timshel Date: Tue, 28 Apr 2026 17:09:47 +0000 Subject: [PATCH 07/17] Fix hardcoded sso identifier (#7157) Co-authored-by: Timshel --- src/api/core/organizations.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 434f0f9d..cbff2099 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -1979,7 +1979,7 @@ async fn list_policies_token(org_id: OrganizationId, token: &str, conn: DbConn) } // Called during the SSO enrollment return the default policy -#[get("/organizations/vaultwarden-dummy-oidc-identifier/policies/master-password", rank = 1)] +#[get("/organizations/00000000-01DC-01DC-01DC-000000000000/policies/master-password", rank = 1)] fn get_dummy_master_password_policy() -> JsonResult { let (enabled, data) = match CONFIG.sso_master_password_policy_value() { Some(policy) if CONFIG.sso_enabled() => (true, policy.to_string()), From 5cc7360816eba3b8a9898218e066d46abd2d5625 Mon Sep 17 00:00:00 2001 From: Mathijs van Veluw Date: Wed, 29 Apr 2026 22:10:26 +0200 Subject: [PATCH 08/17] Update crates and fix a nightly lint (#7161) Updated all the crates including two which reported a possible CVE Updated Typos Signed-off-by: BlackDex --- .github/workflows/typos.yml | 2 +- Cargo.lock | 116 +++++++++++++++--------------- Cargo.toml | 12 ++-- src/db/models/emergency_access.rs | 5 +- 4 files changed, 67 insertions(+), 68 deletions(-) diff --git a/.github/workflows/typos.yml b/.github/workflows/typos.yml index b3ee311b..375600ed 100644 --- a/.github/workflows/typos.yml +++ b/.github/workflows/typos.yml @@ -23,4 +23,4 @@ jobs: # When this version is updated, do not forget to update this in `.pre-commit-config.yaml` too - name: Spell Check Repo - uses: crate-ci/typos@cf5f1c29a8ac336af8568821ec41919923b05a83 # v1.45.1 + uses: crate-ci/typos@7c572958218557a3272c2d6719629443b5cc26fd # v1.45.2 diff --git a/Cargo.lock b/Cargo.lock index 298a8d80..b9fcceef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -152,9 +152,9 @@ dependencies = [ [[package]] name = "async-compression" -version = "0.4.41" +version = "0.4.42" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d0f9ee0f6e02ffd7ad5816e9464499fba7b3effd01123b515c41d1697c43dad1" +checksum = "e79b3f8a79cccc2898f31920fc69f304859b3bd567490f75ebf51ae1c792a9ac" dependencies = [ "compression-codecs", "compression-core", @@ -903,9 +903,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.2.60" +version = "1.2.61" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "43c5703da9466b66a946814e1adf53ea2c90f10063b86290cc9eb67ce3478a20" +checksum = "d16d90359e986641506914ba71350897565610e87ce0ad9e6f28569db3dd5c6d" dependencies = [ "find-msvc-tools", "jobserver", @@ -994,9 +994,9 @@ dependencies = [ [[package]] name = "compression-codecs" -version = "0.4.37" +version = "0.4.38" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eb7b51a7d9c967fc26773061ba86150f19c50c0d65c887cb1fbe295fd16619b7" +checksum = "ce2548391e9c1929c21bf6aa2680af86fe4c1b33e6cea9ac1cfeec0bd11218cf" dependencies = [ "brotli", "compression-core", @@ -1008,9 +1008,9 @@ dependencies = [ [[package]] name = "compression-core" -version = "0.4.31" +version = "0.4.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "75984efb6ed102a0d42db99afb6c1948f0380d1d91808d5529916e6c08b49d8d" +checksum = "cc14f565cf027a105f7a44ccf9e5b424348421a1d8952a8fc9d499d313107789" [[package]] name = "concurrent-queue" @@ -1387,9 +1387,9 @@ dependencies = [ [[package]] name = "data-encoding" -version = "2.10.0" +version = "2.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d7a1e2f27636f116493b8b860f5546edb47c8d8f8ea73e1d2a20be88e28d1fea" +checksum = "a4ae5f15dda3c708c0ade84bfee31ccab44a3da4f88015ed22f63732abe300c8" [[package]] name = "data-url" @@ -1521,9 +1521,9 @@ dependencies = [ [[package]] name = "diesel" -version = "2.3.7" +version = "2.3.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f4ae09a41a4b89f94ec1e053623da8340d996bc32c6517d325a9daad9b239358" +checksum = "78df0e4e8c596662edb07fbfbb7f23769cca35049827df5f909084d956b6aeaf" dependencies = [ "bigdecimal", "bitflags", @@ -1558,9 +1558,9 @@ dependencies = [ [[package]] name = "diesel_derives" -version = "2.3.7" +version = "2.3.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "47618bf0fac06bb670c036e48404c26a865e6a71af4114dfd97dfe89936e404e" +checksum = "0b79402bd1cfb25b65650f0f4901d0e79c095729e2139c8ab779d025968c7099" dependencies = [ "diesel_table_macro_syntax", "dsl_auto_type", @@ -1571,9 +1571,9 @@ dependencies = [ [[package]] name = "diesel_migrations" -version = "2.3.1" +version = "2.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "745fd255645f0f1135f9ec55c7b00e0882192af9683ab4731e4bba3da82b8f9c" +checksum = "28d0f4a98124ba6d4ca75da535f65984badec16a003b6e2f94a01e31a79490b8" dependencies = [ "diesel", "migrations_internals", @@ -2455,9 +2455,9 @@ checksum = "df3b46402a9d5adb4c86a0cf463f42e19994e3ee891101b1841f30a545cb49a9" [[package]] name = "hybrid-array" -version = "0.4.10" +version = "0.4.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3944cf8cf766b40e2a1a333ee5e9b563f854d5fa49d6a8ca2764e97c6eddb214" +checksum = "08d46837a0ed51fe95bd3b05de33cd64a1ee88fc797477ca48446872504507c5" dependencies = [ "typenum", ] @@ -2515,7 +2515,7 @@ dependencies = [ "http 1.4.0", "hyper 1.9.0", "hyper-util", - "rustls 0.23.38", + "rustls 0.23.40", "rustls-native-certs", "tokio", "tokio-rustls 0.26.4", @@ -2679,9 +2679,9 @@ dependencies = [ [[package]] name = "idna_adapter" -version = "1.2.1" +version = "1.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3acae9609540aa318d1bc588455225fb2085b9ed0c4f6bd0d9d5bcd86f1a0344" +checksum = "cb68373c0d6620ef8105e855e7745e18b0d00d3bdb07fb532e434244cdb9a714" dependencies = [ "icu_normalizer", "icu_properties", @@ -2792,9 +2792,9 @@ checksum = "47f142fe24a9c9944451e8349de0a56af5f3e7226dc46f3ed4d4ecc0b85af75e" [[package]] name = "jiff" -version = "0.2.23" +version = "0.2.24" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a3546dc96b6d42c5f24902af9e2538e82e39ad350b0c766eb3fbf2d8f3d8359" +checksum = "f00b5dbd620d61dfdcb6007c9c1f6054ebd75319f163d886a9055cec1155073d" dependencies = [ "jiff-static", "jiff-tzdb-platform", @@ -2807,9 +2807,9 @@ dependencies = [ [[package]] name = "jiff-static" -version = "0.2.23" +version = "0.2.24" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2a8c8b344124222efd714b73bb41f8b5120b27a7cc1c75593a6ff768d9d05aa4" +checksum = "e000de030ff8022ea1da3f466fbb0f3a809f5e51ed31f6dd931c35181ad8e6d7" dependencies = [ "proc-macro2", "quote", @@ -2903,9 +2903,9 @@ dependencies = [ [[package]] name = "js-sys" -version = "0.3.95" +version = "0.3.97" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2964e92d1d9dc3364cae4d718d93f227e3abb088e747d92e0395bfdedf1c12ca" +checksum = "a1840c94c045fbcf8ba2812c95db44499f7c64910a912551aaaa541decebcacf" dependencies = [ "cfg-if", "futures-util", @@ -3005,7 +3005,7 @@ dependencies = [ "nom 8.0.0", "percent-encoding", "quoted_printable", - "rustls 0.23.38", + "rustls 0.23.40", "rustls-native-certs", "serde", "socket2 0.6.3", @@ -3017,9 +3017,9 @@ dependencies = [ [[package]] name = "libc" -version = "0.2.185" +version = "0.2.186" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "52ff2c0fe9bc6cb6b14a0592c2ff4fa9ceb83eea9db979b0487cd054946a2b8f" +checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" [[package]] name = "libm" @@ -3038,9 +3038,9 @@ dependencies = [ [[package]] name = "libsqlite3-sys" -version = "0.36.0" +version = "0.37.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "95b4103cffefa72eb8428cb6b47d6627161e51c2739fc5e3b734584157bc642a" +checksum = "b1f111c8c41e7c61a49cd34e44c7619462967221a6443b0ec299e0ac30cfb9b1" dependencies = [ "cc", "pkg-config", @@ -3647,9 +3647,9 @@ checksum = "35fb2e5f958ec131621fdd531e9fc186ed768cbe395337403ae56c17a74c68ec" [[package]] name = "pastey" -version = "0.2.1" +version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b867cad97c0791bbd3aaa6472142568c6c9e8f71937e98379f584cfb0cf35bec" +checksum = "c5a797f0e07bdf071d15742978fc3128ec6c22891c31a3a931513263904c982a" [[package]] name = "pbkdf2" @@ -4070,7 +4070,7 @@ dependencies = [ "quinn-proto", "quinn-udp", "rustc-hash", - "rustls 0.23.38", + "rustls 0.23.40", "socket2 0.6.3", "thiserror 2.0.18", "tokio", @@ -4090,7 +4090,7 @@ dependencies = [ "rand 0.9.4", "ring", "rustc-hash", - "rustls 0.23.38", + "rustls 0.23.40", "rustls-pki-types", "slab", "thiserror 2.0.18", @@ -4371,7 +4371,7 @@ dependencies = [ "percent-encoding", "pin-project-lite", "quinn", - "rustls 0.23.38", + "rustls 0.23.40", "rustls-native-certs", "rustls-pki-types", "serde", @@ -4537,13 +4537,13 @@ dependencies = [ [[package]] name = "rpassword" -version = "7.4.0" +version = "7.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "66d4c8b64f049c6721ec8ccec37ddfc3d641c4a7fca57e8f2a89de509c73df39" +checksum = "2501c67132bd19c3005b0111fba298907ef002c8c1cf68e25634707e38bf66fe" dependencies = [ "libc", "rtoolbox", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -4648,9 +4648,9 @@ dependencies = [ [[package]] name = "rustls" -version = "0.23.38" +version = "0.23.40" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "69f9466fb2c14ea04357e91413efb882e2a6d4a406e625449bc0a5d360d53a21" +checksum = "ef86cd5876211988985292b91c96a8f2d298df24e75989a43a3c73f2d4d8168b" dependencies = [ "log", "once_cell", @@ -4684,9 +4684,9 @@ dependencies = [ [[package]] name = "rustls-pki-types" -version = "1.14.0" +version = "1.14.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "be040f8b0a225e40375822a563fa9524378b9d63112f53e19ffff34df5d33fdd" +checksum = "30a7197ae7eb376e574fe940d068c30fe0462554a3ddbe4eca7838e049c937a9" dependencies = [ "web-time", "zeroize", @@ -5493,7 +5493,7 @@ version = "0.26.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1729aa945f29d91ba541258c8df89027d5792d85a8841fb65e8bf0f4ede4ef61" dependencies = [ - "rustls 0.23.38", + "rustls 0.23.40", "tokio", ] @@ -5911,7 +5911,7 @@ dependencies = [ "opendal", "openidconnect", "openssl", - "pastey 0.2.1", + "pastey 0.2.2", "percent-encoding", "pico-args", "rand 0.10.1", @@ -6006,9 +6006,9 @@ dependencies = [ [[package]] name = "wasm-bindgen" -version = "0.2.118" +version = "0.2.120" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0bf938a0bacb0469e83c1e148908bd7d5a6010354cf4fb73279b7447422e3a89" +checksum = "df52b6d9b87e0c74c9edfa1eb2d9bf85e5d63515474513aa50fa181b3c4f5db1" dependencies = [ "cfg-if", "once_cell", @@ -6019,9 +6019,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-futures" -version = "0.4.68" +version = "0.4.70" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f371d383f2fb139252e0bfac3b81b265689bf45b6874af544ffa4c975ac1ebf8" +checksum = "af934872acec734c2d80e6617bbb5ff4f12b052dd8e6332b0817bce889516084" dependencies = [ "js-sys", "wasm-bindgen", @@ -6029,9 +6029,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro" -version = "0.2.118" +version = "0.2.120" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eeff24f84126c0ec2db7a449f0c2ec963c6a49efe0698c4242929da037ca28ed" +checksum = "78b1041f495fb322e64aca85f5756b2172e35cd459376e67f2a6c9dffcedb103" dependencies = [ "quote", "wasm-bindgen-macro-support", @@ -6039,9 +6039,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro-support" -version = "0.2.118" +version = "0.2.120" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9d08065faf983b2b80a79fd87d8254c409281cf7de75fc4b773019824196c904" +checksum = "9dcd0ff20416988a18ac686d4d4d0f6aae9ebf08a389ff5d29012b05af2a1b41" dependencies = [ "bumpalo", "proc-macro2", @@ -6052,9 +6052,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-shared" -version = "0.2.118" +version = "0.2.120" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5fd04d9e306f1907bd13c6361b5c6bfc7b3b3c095ed3f8a9246390f8dbdee129" +checksum = "49757b3c82ebf16c57d69365a142940b384176c24df52a087fb748e2085359ea" dependencies = [ "unicode-ident", ] @@ -6108,9 +6108,9 @@ dependencies = [ [[package]] name = "web-sys" -version = "0.3.95" +version = "0.3.97" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4f2dfbb17949fa2088e5d39408c48368947b86f7834484e87b73de55bc14d97d" +checksum = "2eadbac71025cd7b0834f20d1fe8472e8495821b4e9801eb0a60bd1f19827602" dependencies = [ "js-sys", "wasm-bindgen", diff --git a/Cargo.toml b/Cargo.toml index 1d8a6ca0..aba94374 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -88,14 +88,14 @@ serde_json = "1.0.149" # A safe, extensible ORM and Query builder # Currently pinned diesel to v2.3.3 as newer version break MySQL/MariaDB compatibility -diesel = { version = "2.3.7", features = ["chrono", "r2d2", "numeric"] } -diesel_migrations = "2.3.1" +diesel = { version = "2.3.8", features = ["chrono", "r2d2", "numeric"] } +diesel_migrations = "2.3.2" derive_more = { version = "2.1.1", features = ["from", "into", "as_ref", "deref", "display"] } diesel-derive-newtype = "2.1.2" # Bundled/Static SQLite -libsqlite3-sys = { version = "0.36.0", features = ["bundled"], optional = true } +libsqlite3-sys = { version = "0.37.0", features = ["bundled"], optional = true } # Crypto-related libraries rand = "0.10.1" @@ -114,7 +114,7 @@ time = "0.3.47" job_scheduler_ng = "2.4.0" # Data encoding library Hex/Base32/Base64 -data-encoding = "2.10.0" +data-encoding = "2.11.0" # JWT library jsonwebtoken = { version = "10.3.0", features = ["use_pem", "rust_crypto"], default-features = false } @@ -168,7 +168,7 @@ openssl = "0.10.78" pico-args = "0.5.0" # Macro ident concatenation -pastey = "0.2.1" +pastey = "0.2.2" governor = "0.10.4" # OIDC for SSO @@ -188,7 +188,7 @@ which = "8.0.2" argon2 = "0.5.3" # Reading a password from the cli for generating the Argon2id ADMIN_TOKEN -rpassword = "7.4.0" +rpassword = "7.5.1" # Loading a dynamic CSS Stylesheet grass_compiler = { version = "0.13.4", default-features = false } diff --git a/src/db/models/emergency_access.rs b/src/db/models/emergency_access.rs index cf7f5385..5ea334a4 100644 --- a/src/db/models/emergency_access.rs +++ b/src/db/models/emergency_access.rs @@ -85,7 +85,8 @@ impl EmergencyAccess { pub async fn to_json_grantee_details(&self, conn: &DbConn) -> Option { let grantee_user = if let Some(grantee_uuid) = &self.grantee_uuid { User::find_by_uuid(grantee_uuid, conn).await.expect("Grantee user not found.") - } else if let Some(email) = self.email.as_deref() { + } else { + let email = self.email.as_deref()?; match User::find_by_mail(email, conn).await { Some(user) => user, None => { @@ -94,8 +95,6 @@ impl EmergencyAccess { return None; } } - } else { - return None; }; Some(json!({ From a354e57659d26149fde0d91b76f83fce94e8f277 Mon Sep 17 00:00:00 2001 From: Mathijs van Veluw Date: Wed, 29 Apr 2026 22:20:59 +0200 Subject: [PATCH 09/17] Fix Host/IP resolving (#7162) IPv4 addresses can also be in decimal or hex formats. These were not checked during the Global IP check, and could bypass it. We now convert everything to the right format before running this check and it will catch these formats. Also updated the `is_global()` function to match Rust's still unstable version. And updated the Image Magic checks to be more precise and filter out any possible broken or invalid formats. While at it, also added several checks to ensure these special formatted IPv4 addresses are still blocked and punycode domains are also correctly resolved. Signed-off-by: BlackDex --- .pre-commit-config.yaml | 2 +- .typos.toml | 2 + src/api/icons.rs | 107 ++++++--------- src/http_client.rs | 282 +++++++++++++++++++++++++++++++++++++--- src/util.rs | 24 +++- 5 files changed, 323 insertions(+), 94 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b16ae4c6..f10cef65 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -18,7 +18,7 @@ repos: # When this version is updated, do not forget to update this in `.github/workflows/typos.yaml` too - repo: https://github.com/crate-ci/typos - rev: cf5f1c29a8ac336af8568821ec41919923b05a83 # v1.45.1 + rev: 7c572958218557a3272c2d6719629443b5cc26fd # v1.45.2 hooks: - id: typos diff --git a/.typos.toml b/.typos.toml index 59f6d7d6..87c0c4a6 100644 --- a/.typos.toml +++ b/.typos.toml @@ -23,4 +23,6 @@ extend-ignore-re = [ # https://github.com/bitwarden/server/blob/dff9f1cf538198819911cf2c20f8cda3307701c5/src/Notifications/HubHelpers.cs#L86 # https://github.com/bitwarden/clients/blob/9612a4ac45063e372a6fbe87eb253c7cb3c588fb/libs/common/src/auth/services/anonymous-hub.service.ts#L45 "AuthRequestResponseRecieved", + # Ignore Punycode/IDN tests + "xn--.+" ] diff --git a/src/api/icons.rs b/src/api/icons.rs index da83d0c4..b3a66f4d 100644 --- a/src/api/icons.rs +++ b/src/api/icons.rs @@ -19,7 +19,7 @@ use svg_hush::{data_url_filter, Filter}; use crate::{ config::PathType, error::Error, - http_client::{get_reqwest_client_builder, should_block_address, CustomHttpClientError}, + http_client::{get_reqwest_client_builder, get_valid_host, should_block_host, CustomHttpClientError}, util::Cached, CONFIG, }; @@ -81,19 +81,19 @@ static ICON_SIZE_REGEX: LazyLock = LazyLock::new(|| Regex::new(r"(?x)(\d+ // The function name `icon_external` is checked in the `on_response` function in `AppHeaders` // It is used to prevent sending a specific header which breaks icon downloads. // If this function needs to be renamed, also adjust the code in `util.rs` -#[get("//icon.png")] -fn icon_external(domain: &str) -> Cached> { - if !is_valid_domain(domain) { - warn!("Invalid domain: {domain}"); +#[get("//icon.png")] +fn icon_external(host: &str) -> Cached> { + let Ok(host) = get_valid_host(host) else { + warn!("Invalid host: {host}"); + return Cached::ttl(None, CONFIG.icon_cache_negttl(), true); + }; + + if should_block_host(&host).is_err() { + warn!("Blocked address: {host}"); return Cached::ttl(None, CONFIG.icon_cache_negttl(), true); } - if should_block_address(domain) { - warn!("Blocked address: {domain}"); - return Cached::ttl(None, CONFIG.icon_cache_negttl(), true); - } - - let url = CONFIG._icon_service_url().replace("{}", domain); + let url = CONFIG._icon_service_url().replace("{}", &host.to_string()); let redir = match CONFIG.icon_redirect_code() { 301 => Some(Redirect::moved(url)), // legacy permanent redirect 302 => Some(Redirect::found(url)), // legacy temporary redirect @@ -107,12 +107,21 @@ fn icon_external(domain: &str) -> Cached> { Cached::ttl(redir, CONFIG.icon_cache_ttl(), true) } -#[get("//icon.png")] -async fn icon_internal(domain: &str) -> Cached<(ContentType, Vec)> { +#[get("//icon.png")] +async fn icon_internal(host: &str) -> Cached<(ContentType, Vec)> { const FALLBACK_ICON: &[u8] = include_bytes!("../static/images/fallback-icon.png"); - if !is_valid_domain(domain) { - warn!("Invalid domain: {domain}"); + let Ok(host) = get_valid_host(host) else { + warn!("Invalid host: {host}"); + return Cached::ttl( + (ContentType::new("image", "png"), FALLBACK_ICON.to_vec()), + CONFIG.icon_cache_negttl(), + true, + ); + }; + + if should_block_host(&host).is_err() { + warn!("Blocked address: {host}"); return Cached::ttl( (ContentType::new("image", "png"), FALLBACK_ICON.to_vec()), CONFIG.icon_cache_negttl(), @@ -120,16 +129,7 @@ async fn icon_internal(domain: &str) -> Cached<(ContentType, Vec)> { ); } - if should_block_address(domain) { - warn!("Blocked address: {domain}"); - return Cached::ttl( - (ContentType::new("image", "png"), FALLBACK_ICON.to_vec()), - CONFIG.icon_cache_negttl(), - true, - ); - } - - match get_icon(domain).await { + match get_icon(&host.to_string()).await { Some((icon, icon_type)) => { Cached::ttl((ContentType::new("image", icon_type), icon), CONFIG.icon_cache_ttl(), true) } @@ -137,42 +137,6 @@ async fn icon_internal(domain: &str) -> Cached<(ContentType, Vec)> { } } -/// Returns if the domain provided is valid or not. -/// -/// This does some manual checks and makes use of Url to do some basic checking. -/// domains can't be larger then 63 characters (not counting multiple subdomains) according to the RFC's, but we limit the total size to 255. -fn is_valid_domain(domain: &str) -> bool { - const ALLOWED_CHARS: &str = "-."; - - // If parsing the domain fails using Url, it will not work with reqwest. - if let Err(parse_error) = url::Url::parse(format!("https://{domain}").as_str()) { - debug!("Domain parse error: '{domain}' - {parse_error:?}"); - return false; - } else if domain.is_empty() - || domain.contains("..") - || domain.starts_with('.') - || domain.starts_with('-') - || domain.ends_with('-') - { - debug!( - "Domain validation error: '{domain}' is either empty, contains '..', starts with an '.', starts or ends with a '-'" - ); - return false; - } else if domain.len() > 255 { - debug!("Domain validation error: '{domain}' exceeds 255 characters"); - return false; - } - - for c in domain.chars() { - if !c.is_alphanumeric() && !ALLOWED_CHARS.contains(c) { - debug!("Domain validation error: '{domain}' contains an invalid character '{c}'"); - return false; - } - } - - true -} - async fn get_icon(domain: &str) -> Option<(Vec, String)> { let path = format!("{domain}.png"); @@ -367,7 +331,7 @@ async fn get_icon_url(domain: &str) -> Result { tld = domain_parts.next_back().unwrap(), base = domain_parts.next_back().unwrap() ); - if is_valid_domain(&base_domain) { + if get_valid_host(&base_domain).is_ok() { let sslbase = format!("https://{base_domain}"); let httpbase = format!("http://{base_domain}"); debug!("[get_icon_url]: Trying without subdomains '{base_domain}'"); @@ -378,7 +342,7 @@ async fn get_icon_url(domain: &str) -> Result { // When the domain is not an IP, and has less then 2 dots, try to add www. infront of it. } else if is_ip.is_err() && domain.matches('.').count() < 2 { let www_domain = format!("www.{domain}"); - if is_valid_domain(&www_domain) { + if get_valid_host(&www_domain).is_ok() { let sslwww = format!("https://{www_domain}"); let httpwww = format!("http://{www_domain}"); debug!("[get_icon_url]: Trying with www. prefix '{www_domain}'"); @@ -618,14 +582,17 @@ fn get_icon_type(bytes: &[u8]) -> Option<&'static str> { None } + // Some details can be found here: + // - https://www.garykessler.net/library/file_sigs_GCK_latest.html + // - https://en.wikipedia.org/wiki/List_of_file_signatures match bytes { - [137, 80, 78, 71, ..] => Some("png"), - [0, 0, 1, 0, ..] => Some("x-icon"), - [82, 73, 70, 70, ..] => Some("webp"), - [255, 216, 255, ..] => Some("jpeg"), - [71, 73, 70, 56, ..] => Some("gif"), - [66, 77, ..] => Some("bmp"), - [60, 115, 118, 103, ..] => Some("svg+xml"), // Normal svg + [137, 80, 78, 71, 13, 10, 26, 10, ..] => Some("png"), + [0, 0, 1, 0, n1, n2, ..] if u16::from_le_bytes([*n1, *n2]) > 0 => Some("x-icon"), // https://en.wikipedia.org/wiki/ICO_(file_format) + [82, 73, 70, 70, _, _, _, _, 87, 69, 66, 80, ..] => Some("webp"), // Only match WebP Images + [255, 216, 255, b, ..] if *b >= 0xC0 => Some("jpeg"), + [71, 73, 70, 56, 55 | 57, 97, ..] => Some("gif"), + [66, 77, _, _, _, _, 0, 0, 0, 0, ..] => Some("bmp"), // https://en.wikipedia.org/wiki/BMP_file_format + [60, 115, 118, 103, ..] => Some("svg+xml"), // Normal svg [60, 63, 120, 109, 108, ..] => check_svg_after_xml_declaration(bytes), // An svg starting with None, } diff --git a/src/http_client.rs b/src/http_client.rs index df52e2bc..d39b884d 100644 --- a/src/http_client.rs +++ b/src/http_client.rs @@ -1,7 +1,6 @@ use std::{ fmt, net::{IpAddr, SocketAddr}, - str::FromStr, sync::{Arc, LazyLock, Mutex}, time::Duration, }; @@ -59,16 +58,6 @@ pub fn get_reqwest_client_builder() -> ClientBuilder { .timeout(Duration::from_secs(10)) } -pub fn should_block_address(domain_or_ip: &str) -> bool { - if let Ok(ip) = IpAddr::from_str(domain_or_ip) { - if should_block_ip(ip) { - return true; - } - } - - should_block_address_regex(domain_or_ip) -} - fn should_block_ip(ip: IpAddr) -> bool { if !CONFIG.http_request_block_non_global_ips() { return false; @@ -100,11 +89,54 @@ fn should_block_address_regex(domain_or_ip: &str) -> bool { is_match } -fn should_block_host(host: &Host<&str>) -> Result<(), CustomHttpClientError> { +pub fn get_valid_host(host: &str) -> Result { + let Ok(host) = Host::parse(host) else { + return Err(CustomHttpClientError::Invalid { + domain: host.to_string(), + }); + }; + + // Some extra checks to validate hosts + match host { + Host::Domain(ref domain) => { + // Host::parse() does not verify length or all possible invalid characters + // We do some extra checks here to prevent issues + if domain.len() > 253 { + debug!("Domain validation error: '{domain}' exceeds 253 characters"); + return Err(CustomHttpClientError::Invalid { + domain: host.to_string(), + }); + } + if !domain.split('.').all(|label| { + !label.is_empty() + // Labels can't be longer than 63 chars + && label.len() <= 63 + // Labels are not allowed to start or end with a hyphen `-` + && !label.starts_with('-') + && !label.ends_with('-') + // Only ASCII Alphanumeric characters are allowed + // We already received a punycoded domain back, so no unicode should exists here + && label.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') + }) { + debug!( + "Domain validation error: '{domain}' labels contain invalid characters or exceed the maximum length" + ); + return Err(CustomHttpClientError::Invalid { + domain: host.to_string(), + }); + } + } + Host::Ipv4(_) | Host::Ipv6(_) => {} + } + + Ok(host) +} + +pub fn should_block_host>(host: &Host) -> Result<(), CustomHttpClientError> { let (ip, host_str): (Option, String) = match host { Host::Ipv4(ip) => (Some(IpAddr::V4(*ip)), ip.to_string()), Host::Ipv6(ip) => (Some(IpAddr::V6(*ip)), ip.to_string()), - Host::Domain(d) => (None, (*d).to_string()), + Host::Domain(d) => (None, d.as_ref().to_string()), }; if let Some(ip) = ip { @@ -134,6 +166,9 @@ pub enum CustomHttpClientError { domain: Option, ip: IpAddr, }, + Invalid { + domain: String, + }, } impl CustomHttpClientError { @@ -155,7 +190,7 @@ impl fmt::Display for CustomHttpClientError { match self { Self::Blocked { domain, - } => write!(f, "Blocked domain: {domain} matched HTTP_REQUEST_BLOCK_REGEX"), + } => write!(f, "Blocked domain: '{domain}' matched HTTP_REQUEST_BLOCK_REGEX"), Self::NonGlobalIp { domain: Some(domain), ip, @@ -163,7 +198,10 @@ impl fmt::Display for CustomHttpClientError { Self::NonGlobalIp { domain: None, ip, - } => write!(f, "IP {ip} is not a global IP!"), + } => write!(f, "IP '{ip}' is not a global IP!"), + Self::Invalid { + domain, + } => write!(f, "Invalid host: '{domain}' contains invalid characters or exceeds the maximum length"), } } } @@ -217,7 +255,13 @@ impl CustomDnsResolver { } fn pre_resolve(name: &str) -> Result<(), CustomHttpClientError> { - if should_block_address(name) { + let Ok(host) = get_valid_host(name) else { + return Err(CustomHttpClientError::Invalid { + domain: name.to_string(), + }); + }; + + if should_block_host(&host).is_err() { return Err(CustomHttpClientError::Blocked { domain: name.to_string(), }); @@ -308,3 +352,209 @@ pub(crate) mod aws { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::util::is_global_hardcoded; + use std::net::Ipv4Addr; + use url::Host; + + // === + // IPv4 numeric-format normalization + fn parse_to_ip(s: &str) -> Option { + match Host::parse(s).ok()? { + Host::Ipv4(v4) => Some(IpAddr::V4(v4)), + Host::Ipv6(v6) => Some(IpAddr::V6(v6)), + Host::Domain(_) => None, + } + } + + #[test] + fn dotted_decimal_loopback_normalizes() { + let ip = parse_to_ip("127.0.0.1").unwrap(); + assert_eq!(ip, IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1))); + assert!(!is_global_hardcoded(ip)); + } + + #[test] + fn single_decimal_loopback_normalizes() { + // 127.0.0.1 == 2130706433 + let ip = parse_to_ip("2130706433").unwrap(); + assert_eq!(ip, IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1))); + assert!(!is_global_hardcoded(ip)); + } + + #[test] + fn hex_loopback_normalizes() { + let ip = parse_to_ip("0x7f000001").unwrap(); + assert_eq!(ip, IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1))); + assert!(!is_global_hardcoded(ip)); + } + + #[test] + fn dotted_hex_loopback_normalizes() { + let ip = parse_to_ip("0x7f.0.0.1").unwrap(); + assert_eq!(ip, IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1))); + assert!(!is_global_hardcoded(ip)); + } + + #[test] + fn octal_loopback_normalizes() { + // 017700000001 == 127.0.0.1 + let ip = parse_to_ip("017700000001").unwrap(); + assert_eq!(ip, IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1))); + assert!(!is_global_hardcoded(ip)); + } + + #[test] + fn dotted_octal_loopback_normalizes() { + let ip = parse_to_ip("0177.0.0.01").unwrap(); + assert_eq!(ip, IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1))); + assert!(!is_global_hardcoded(ip)); + } + + #[test] + fn aws_metadata_decimal_blocked() { + // 169.254.169.254 == 2852039166 (link-local, AWS IMDS) + let ip = parse_to_ip("2852039166").unwrap(); + assert_eq!(ip, IpAddr::V4(Ipv4Addr::new(169, 254, 169, 254))); + assert!(!is_global_hardcoded(ip)); + } + + #[test] + fn rfc1918_hex_blocked() { + // 10.0.0.1 + let ip = parse_to_ip("0x0a000001").unwrap(); + assert!(!is_global_hardcoded(ip)); + } + + #[test] + fn public_ip_decimal_allowed() { + // 8.8.8.8 == 134744072 + let ip = parse_to_ip("134744072").unwrap(); + assert_eq!(ip, IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8))); + assert!(is_global_hardcoded(ip)); + } + + // === + // get_valid_host integration: numeric forms become Host::Ipv4 + #[test] + fn get_valid_host_normalizes_decimal_int() { + let h = get_valid_host("2130706433").expect("valid"); + assert!(matches!(h, Host::Ipv4(ip) if ip == Ipv4Addr::new(127, 0, 0, 1))); + } + + #[test] + fn get_valid_host_normalizes_hex() { + let h = get_valid_host("0x7f000001").expect("valid"); + assert!(matches!(h, Host::Ipv4(ip) if ip == Ipv4Addr::new(127, 0, 0, 1))); + } + + #[test] + fn get_valid_host_normalizes_octal() { + let h = get_valid_host("017700000001").expect("valid"); + assert!(matches!(h, Host::Ipv4(ip) if ip == Ipv4Addr::new(127, 0, 0, 1))); + } + + // === + // IPv6 formats + #[test] + fn ipv6_loopback_blocked() { + let h = get_valid_host("[::1]").expect("valid"); + let Host::Ipv6(ip) = h else { + panic!("expected v6") + }; + assert!(!is_global_hardcoded(IpAddr::V6(ip))); + } + + #[test] + fn ipv4_mapped_in_ipv6_loopback_blocked() { + // ::ffff:127.0.0.1 — v4-mapped form; is_global_hardcoded blocks via ::ffff:0:0/96 + let h = get_valid_host("[::ffff:127.0.0.1]").expect("valid"); + let Host::Ipv6(ip) = h else { + panic!("expected v6") + }; + assert!(!is_global_hardcoded(IpAddr::V6(ip))); + } + + #[test] + fn ipv6_unique_local_blocked() { + let h = get_valid_host("[fc00::1]").expect("valid"); + let Host::Ipv6(ip) = h else { + panic!("expected v6") + }; + assert!(!is_global_hardcoded(IpAddr::V6(ip))); + } + + // === + // Punycode / IDN + #[test] + fn punycode_passthrough() { + let h = get_valid_host("xn--deadbeafcaf-lbb.test").expect("valid"); + match h { + Host::Domain(d) => assert_eq!(d, "xn--deadbeafcaf-lbb.test"), + _ => panic!("expected domain"), + } + } + + #[test] + fn idn_unicode_gets_punycoded() { + let h = get_valid_host("deadbeafcafé.test").expect("valid"); + match h { + Host::Domain(d) => assert_eq!(d, "xn--deadbeafcaf-lbb.test"), + _ => panic!("expected domain"), + } + } + + #[test] + fn idn_unicode_gets_punycoded_tld() { + let h = get_valid_host("deadbeaf.café").expect("valid"); + match h { + Host::Domain(d) => assert_eq!(d, "deadbeaf.xn--caf-dma"), + _ => panic!("expected domain"), + } + } + + #[test] + fn idn_emoji_gets_punycoded() { + let h = get_valid_host("xn--t88h.test").expect("valid"); // 🛡️.test + match h { + Host::Domain(d) => assert_eq!(d, "xn--t88h.test"), + _ => panic!("expected domain"), + } + } + + #[test] + fn idn_unicode_to_punycode_roundtrip() { + let from_unicode = get_valid_host("🛡️.test").expect("valid"); + let from_puny = get_valid_host("xn--t88h.test").expect("valid"); + match (from_unicode, from_puny) { + (Host::Domain(a), Host::Domain(b)) => assert_eq!(a, b), + _ => panic!("expected domains"), + } + } + + #[test] + fn invalid_punycode_rejected() { + // bare invalid punycode + assert!(get_valid_host("xn--").is_err()); + } + + #[test] + fn underscore_in_label_rejected() { + assert!(get_valid_host("dead_beaf.cafe").is_err()); + } + + #[test] + fn label_too_long_rejected() { + let label = "a".repeat(64); + assert!(get_valid_host(&format!("{label}.test")).is_err()); + } + + #[test] + fn domain_too_long_rejected() { + let big = "a.".repeat(130) + "test"; // > 253 + assert!(get_valid_host(&big).is_err()); + } +} diff --git a/src/util.rs b/src/util.rs index 06f00b98..5cd78eed 100644 --- a/src/util.rs +++ b/src/util.rs @@ -818,14 +818,18 @@ pub fn is_global_hardcoded(ip: std::net::IpAddr) -> bool { std::net::IpAddr::V4(ip) => { !(ip.octets()[0] == 0 // "This network" || ip.is_private() - || (ip.octets()[0] == 100 && (ip.octets()[1] & 0b1100_0000 == 0b0100_0000)) //ip.is_shared() + || (ip.octets()[0] == 100 && (ip.octets()[1] & 0b1100_0000 == 0b0100_0000)) // ip.is_shared() || ip.is_loopback() || ip.is_link_local() // addresses reserved for future protocols (`192.0.0.0/24`) - ||(ip.octets()[0] == 192 && ip.octets()[1] == 0 && ip.octets()[2] == 0) + // .9 and .10 are documented as globally reachable so they're excluded + || ( + ip.octets()[0] == 192 && ip.octets()[1] == 0 && ip.octets()[2] == 0 + && ip.octets()[3] != 9 && ip.octets()[3] != 10 + ) || ip.is_documentation() || (ip.octets()[0] == 198 && (ip.octets()[1] & 0xfe) == 18) // ip.is_benchmarking() - || (ip.octets()[0] & 240 == 240 && !ip.is_broadcast()) //ip.is_reserved() + || (ip.octets()[0] & 240 == 240 && !ip.is_broadcast()) // ip.is_reserved() || ip.is_broadcast()) } std::net::IpAddr::V6(ip) => { @@ -849,11 +853,17 @@ pub fn is_global_hardcoded(ip: std::net::IpAddr) -> bool { // AS112-v6 (`2001:4:112::/48`) || matches!(ip.segments(), [0x2001, 4, 0x112, _, _, _, _, _]) // ORCHIDv2 (`2001:20::/28`) - || matches!(ip.segments(), [0x2001, b, _, _, _, _, _, _] if (0x20..=0x2F).contains(&b)) + // Drone Remote ID Protocol Entity Tags (DETs) Prefix (`2001:30::/28`)` + || matches!(ip.segments(), [0x2001, b, _, _, _, _, _, _] if (0x20..=0x3F).contains(&b)) )) - || ((ip.segments()[0] == 0x2001) && (ip.segments()[1] == 0xdb8)) // ip.is_documentation() - || ((ip.segments()[0] & 0xfe00) == 0xfc00) //ip.is_unique_local() - || ((ip.segments()[0] & 0xffc0) == 0xfe80)) //ip.is_unicast_link_local() + // 6to4 (`2002::/16`) – it's not explicitly documented as globally reachable, + // IANA says N/A. + || matches!(ip.segments(), [0x2002, _, _, _, _, _, _, _]) + || matches!(ip.segments(), [0x2001, 0xdb8, ..] | [0x3fff, 0..=0x0fff, ..]) // ip.is_documentation() + // Segment Routing (SRv6) SIDs (`5f00::/16`) + || matches!(ip.segments(), [0x5f00, ..]) + || ip.is_unique_local() + || ip.is_unicast_link_local()) } } } From d297e274a35dccd0f5d935e9d5934e0f7e9c0a87 Mon Sep 17 00:00:00 2001 From: Mathijs van Veluw Date: Wed, 29 Apr 2026 22:25:36 +0200 Subject: [PATCH 10/17] Several SSO Fixes (#7163) * Ensure SSO token is only usable on the same client This commit adds an extra check via cookies to ensure the same browser/client is used to request and provide the SSO token. Previously it would be able to provide a custom link which attackers could use to steal data. While an attacker would still need the Master Password to be able to decrypt or execute specific actions, they were able to fetch encrypted data. Solved with some help of Claude Code. Signed-off-by: BlackDex * Check email-verified on SSO login/create This commit prevents possible account takeover via SSO which doesn't check/validate or provide validated status of the email. It was checked at other locations, but was skipped here. Signed-off-by: BlackDex * Prevent data disclosure via SSO endpoints This commit prevents some data disclosure and user enumeration by only returning the fake SSO identifier. Since we do not check the identifier anywhere useful, returning the fake one is just fine. During an invite to an org, that link contains the correct UUID and will be used for the master password requirements. For anything else, server admins should set the `SSO_MASTER_PASSWORD_POLICY` env variable. Signed-off-by: BlackDex * Adjust admin layout to fix issues when SSO is enabled Signed-off-by: BlackDex --------- Signed-off-by: BlackDex --- .../down.sql | 1 + .../2026-04-25-120000_sso_auth_binding/up.sql | 1 + .../down.sql | 1 + .../2026-04-25-120000_sso_auth_binding/up.sql | 1 + .../down.sql | 1 + .../2026-04-25-120000_sso_auth_binding/up.sql | 1 + src/api/core/organizations.rs | 42 ++++------- src/api/identity.rs | 71 +++++++++++++++++-- src/crypto.rs | 7 ++ src/db/models/sso_auth.rs | 10 ++- src/db/schema.rs | 1 + src/sso.rs | 3 +- src/sso_client.rs | 3 +- src/static/scripts/admin.css | 15 +++- src/static/templates/admin/base.hbs | 2 +- src/static/templates/admin/diagnostics.hbs | 2 +- src/static/templates/admin/login.hbs | 2 +- src/static/templates/admin/organizations.hbs | 2 +- src/static/templates/admin/settings.hbs | 2 +- src/static/templates/admin/users.hbs | 4 +- 20 files changed, 125 insertions(+), 47 deletions(-) create mode 100644 migrations/mysql/2026-04-25-120000_sso_auth_binding/down.sql create mode 100644 migrations/mysql/2026-04-25-120000_sso_auth_binding/up.sql create mode 100644 migrations/postgresql/2026-04-25-120000_sso_auth_binding/down.sql create mode 100644 migrations/postgresql/2026-04-25-120000_sso_auth_binding/up.sql create mode 100644 migrations/sqlite/2026-04-25-120000_sso_auth_binding/down.sql create mode 100644 migrations/sqlite/2026-04-25-120000_sso_auth_binding/up.sql diff --git a/migrations/mysql/2026-04-25-120000_sso_auth_binding/down.sql b/migrations/mysql/2026-04-25-120000_sso_auth_binding/down.sql new file mode 100644 index 00000000..17e3d8c7 --- /dev/null +++ b/migrations/mysql/2026-04-25-120000_sso_auth_binding/down.sql @@ -0,0 +1 @@ +ALTER TABLE sso_auth DROP COLUMN binding_hash; diff --git a/migrations/mysql/2026-04-25-120000_sso_auth_binding/up.sql b/migrations/mysql/2026-04-25-120000_sso_auth_binding/up.sql new file mode 100644 index 00000000..53ee8063 --- /dev/null +++ b/migrations/mysql/2026-04-25-120000_sso_auth_binding/up.sql @@ -0,0 +1 @@ +ALTER TABLE sso_auth ADD COLUMN binding_hash TEXT; diff --git a/migrations/postgresql/2026-04-25-120000_sso_auth_binding/down.sql b/migrations/postgresql/2026-04-25-120000_sso_auth_binding/down.sql new file mode 100644 index 00000000..17e3d8c7 --- /dev/null +++ b/migrations/postgresql/2026-04-25-120000_sso_auth_binding/down.sql @@ -0,0 +1 @@ +ALTER TABLE sso_auth DROP COLUMN binding_hash; diff --git a/migrations/postgresql/2026-04-25-120000_sso_auth_binding/up.sql b/migrations/postgresql/2026-04-25-120000_sso_auth_binding/up.sql new file mode 100644 index 00000000..53ee8063 --- /dev/null +++ b/migrations/postgresql/2026-04-25-120000_sso_auth_binding/up.sql @@ -0,0 +1 @@ +ALTER TABLE sso_auth ADD COLUMN binding_hash TEXT; diff --git a/migrations/sqlite/2026-04-25-120000_sso_auth_binding/down.sql b/migrations/sqlite/2026-04-25-120000_sso_auth_binding/down.sql new file mode 100644 index 00000000..17e3d8c7 --- /dev/null +++ b/migrations/sqlite/2026-04-25-120000_sso_auth_binding/down.sql @@ -0,0 +1 @@ +ALTER TABLE sso_auth DROP COLUMN binding_hash; diff --git a/migrations/sqlite/2026-04-25-120000_sso_auth_binding/up.sql b/migrations/sqlite/2026-04-25-120000_sso_auth_binding/up.sql new file mode 100644 index 00000000..53ee8063 --- /dev/null +++ b/migrations/sqlite/2026-04-25-120000_sso_auth_binding/up.sql @@ -0,0 +1 @@ +ALTER TABLE sso_auth ADD COLUMN binding_hash TEXT; diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index cbff2099..31311a65 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -907,36 +907,21 @@ async fn _get_org_details( Ok(json!(ciphers_json)) } -#[derive(Deserialize)] -#[serde(rename_all = "camelCase")] -struct OrgDomainDetails { - email: String, -} - // Returning a Domain/Organization here allow to prefill it and prevent prompting the user -// So we either return an Org name associated to the user or a dummy value. +// So we return a dummy value, since we only support a single SSO integration, and do not use the response anywhere // In use since `v2025.6.0`, appears to use only the first `organizationIdentifier` -#[post("/organizations/domain/sso/verified", data = "")] -async fn get_org_domain_sso_verified(data: Json, conn: DbConn) -> JsonResult { - let data: OrgDomainDetails = data.into_inner(); - - let identifiers = match Organization::find_org_user_email(&data.email, &conn) - .await - .into_iter() - .map(|o| (o.name, o.uuid.to_string())) - .collect::>() - { - v if !v.is_empty() => v, - _ => vec![(FAKE_SSO_IDENTIFIER.to_string(), FAKE_SSO_IDENTIFIER.to_string())], - }; - +#[post("/organizations/domain/sso/verified")] +fn get_org_domain_sso_verified() -> JsonResult { + // Always return a dummy value, no matter if SSO is enabled or not Ok(Json(json!({ "object": "list", - "data": identifiers.into_iter().map(|(name, identifier)| json!({ - "organizationName": name, // appear unused - "organizationIdentifier": identifier, - "domainName": CONFIG.domain(), // appear unused - })).collect::>() + "data": [{ + "organizationIdentifier": FAKE_SSO_IDENTIFIER, + // These appear to be unused + "organizationName": FAKE_SSO_IDENTIFIER, + "domainName": CONFIG.domain() + }], + "continuationToken": null }))) } @@ -3049,10 +3034,7 @@ async fn put_reset_password_enrollment( err!("User to enroll isn't member of required organization", "The user_id and acting user do not match"); } - let Some(mut membership) = Membership::find_confirmed_by_user_and_org(&headers.user.uuid, &org_id, &conn).await - else { - err!("User to enroll isn't member of required organization") - }; + let mut membership = headers.membership; check_reset_password_applicable(&org_id, &conn).await?; diff --git a/src/api/identity.rs b/src/api/identity.rs index 72323f73..569deaf9 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -2,6 +2,7 @@ use chrono::Utc; use num_traits::FromPrimitive; use rocket::{ form::{Form, FromForm}, + http::{Cookie, CookieJar, SameSite}, response::Redirect, serde::json::Json, Route, @@ -23,7 +24,8 @@ use crate::{ ApiResult, EmptyResult, JsonResult, }, auth, - auth::{generate_organization_api_key_login_claims, AuthMethod, ClientHeaders, ClientIp, ClientVersion}, + auth::{generate_organization_api_key_login_claims, AuthMethod, ClientHeaders, ClientIp, ClientVersion, Secure}, + crypto, db::{ models::{ AuthRequest, AuthRequestId, Device, DeviceId, EventType, Invitation, OIDCCodeWrapper, OrganizationApiKey, @@ -228,7 +230,33 @@ async fn _sso_login( } ) } - Some((user, None)) => Some((user, None)), + Some((user, None)) => match user_infos.email_verified { + None if !CONFIG.sso_allow_unknown_email_verification() => { + error!( + "Login failure ({}), existing non SSO user ({}) with same email ({}) and email verification status is unknown", + user_infos.identifier, user.uuid, user.email + ); + err_silent!( + "Email verification status is unknown", + ErrorEvent { + event: EventType::UserFailedLogIn + } + ) + } + Some(false) => { + error!( + "Login failure ({}), existing non SSO user ({}) with same email ({}) and email is not verified", + user_infos.identifier, user.uuid, user.email + ); + err_silent!( + "Email is not verified by the SSO provider", + ErrorEvent { + event: EventType::UserFailedLogIn + } + ) + } + _ => Some((user, None)), + }, }, Some((user, sso_user)) => Some((user, Some(sso_user))), }; @@ -1133,13 +1161,16 @@ fn prevalidate() -> JsonResult { } } +const SSO_BINDING_COOKIE: &str = "VW_SSO_BINDING"; + #[get("/connect/oidc-signin?&", rank = 1)] -async fn oidcsignin(code: OIDCCode, state: String, mut conn: DbConn) -> ApiResult { +async fn oidcsignin(code: OIDCCode, state: String, cookies: &CookieJar<'_>, mut conn: DbConn) -> ApiResult { _oidcsignin_redirect( state, OIDCCodeWrapper::Ok { code, }, + cookies, &mut conn, ) .await @@ -1152,6 +1183,7 @@ async fn oidcsignin_error( state: String, error: String, error_description: Option, + cookies: &CookieJar<'_>, mut conn: DbConn, ) -> ApiResult { _oidcsignin_redirect( @@ -1160,6 +1192,7 @@ async fn oidcsignin_error( error, error_description, }, + cookies, &mut conn, ) .await @@ -1171,6 +1204,7 @@ async fn oidcsignin_error( async fn _oidcsignin_redirect( base64_state: String, code_response: OIDCCodeWrapper, + cookies: &CookieJar<'_>, conn: &mut DbConn, ) -> ApiResult { let state = sso::decode_state(&base64_state)?; @@ -1179,6 +1213,17 @@ async fn _oidcsignin_redirect( None => err!(format!("Cannot retrieve sso_auth for {state}")), Some(sso_auth) => sso_auth, }; + + // Browser-binding check + // The cookie was set on /connect/authorize and must come from the same browser that initiated the flow. + let cookie_value = cookies.get(SSO_BINDING_COOKIE).map(|c| c.value().to_string()); + let provided_hash = cookie_value.as_deref().map(|v| crypto::sha256_hex(v.as_bytes())); + match (sso_auth.binding_hash.as_deref(), provided_hash.as_deref()) { + (Some(expected), Some(actual)) if crypto::ct_eq(expected, actual) => {} + _ => err!(format!("SSO session binding mismatch for {state}")), + } + cookies.remove(Cookie::build(SSO_BINDING_COOKIE).path("/identity/connect/").build()); + sso_auth.code_response = Some(code_response); sso_auth.updated_at = Utc::now().naive_utc(); sso_auth.save(conn).await?; @@ -1225,7 +1270,7 @@ struct AuthorizeData { // The `redirect_uri` will change depending of the client (web, android, ios ..) #[get("/connect/authorize?")] -async fn authorize(data: AuthorizeData, conn: DbConn) -> ApiResult { +async fn authorize(data: AuthorizeData, cookies: &CookieJar<'_>, secure: Secure, conn: DbConn) -> ApiResult { let AuthorizeData { client_id, redirect_uri, @@ -1239,7 +1284,23 @@ async fn authorize(data: AuthorizeData, conn: DbConn) -> ApiResult { err!("Unsupported code challenge method"); } - let auth_url = sso::authorize_url(state, code_challenge, &client_id, &redirect_uri, conn).await?; + // Generate browser-binding token. Stored hashed in DB; raw value handed to the browser as a cookie. + // Validated on /connect/oidc-signin + let binding_token = data_encoding::BASE64URL_NOPAD.encode(&crypto::get_random_bytes::<32>()); + let binding_hash = crypto::sha256_hex(binding_token.as_bytes()); + + let auth_url = + sso::authorize_url(state, code_challenge, &client_id, &redirect_uri, Some(binding_hash), conn).await?; + + cookies.add( + Cookie::build((SSO_BINDING_COOKIE, binding_token)) + .path("/identity/connect/") + .max_age(time::Duration::seconds(sso::SSO_AUTH_EXPIRATION.num_seconds())) + .same_site(SameSite::Lax) // Lax is needed because the IdP runs on a different FQDN + .http_only(true) + .secure(secure.https) + .build(), + ); Ok(Redirect::temporary(String::from(auth_url))) } diff --git a/src/crypto.rs b/src/crypto.rs index 1930f380..46d305a5 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -113,3 +113,10 @@ pub fn ct_eq, U: AsRef<[u8]>>(a: T, b: U) -> bool { use subtle::ConstantTimeEq; a.as_ref().ct_eq(b.as_ref()).into() } + +// +// SHA256 +// +pub fn sha256_hex(data: &[u8]) -> String { + HEXLOWER.encode(digest::digest(&digest::SHA256, data).as_ref()) +} diff --git a/src/db/models/sso_auth.rs b/src/db/models/sso_auth.rs index fec0433a..2c6eec6d 100644 --- a/src/db/models/sso_auth.rs +++ b/src/db/models/sso_auth.rs @@ -54,11 +54,18 @@ pub struct SsoAuth { pub auth_response: Option, pub created_at: NaiveDateTime, pub updated_at: NaiveDateTime, + pub binding_hash: Option, } /// Local methods impl SsoAuth { - pub fn new(state: OIDCState, client_challenge: OIDCCodeChallenge, nonce: String, redirect_uri: String) -> Self { + pub fn new( + state: OIDCState, + client_challenge: OIDCCodeChallenge, + nonce: String, + redirect_uri: String, + binding_hash: Option, + ) -> Self { let now = Utc::now().naive_utc(); SsoAuth { @@ -70,6 +77,7 @@ impl SsoAuth { updated_at: now, code_response: None, auth_response: None, + binding_hash, } } } diff --git a/src/db/schema.rs b/src/db/schema.rs index 914b4fe9..147440e5 100644 --- a/src/db/schema.rs +++ b/src/db/schema.rs @@ -265,6 +265,7 @@ table! { auth_response -> Nullable, created_at -> Timestamp, updated_at -> Timestamp, + binding_hash -> Nullable, } } diff --git a/src/sso.rs b/src/sso.rs index 26ea7375..7505f84f 100644 --- a/src/sso.rs +++ b/src/sso.rs @@ -188,6 +188,7 @@ pub async fn authorize_url( client_challenge: OIDCCodeChallenge, client_id: &str, raw_redirect_uri: &str, + binding_hash: Option, conn: DbConn, ) -> ApiResult { let redirect_uri = match client_id { @@ -203,7 +204,7 @@ pub async fn authorize_url( _ => err!(format!("Unsupported client {client_id}")), }; - let (auth_url, sso_auth) = Client::authorize_url(state, client_challenge, redirect_uri).await?; + let (auth_url, sso_auth) = Client::authorize_url(state, client_challenge, redirect_uri, binding_hash).await?; sso_auth.save(&conn).await?; Ok(auth_url) } diff --git a/src/sso_client.rs b/src/sso_client.rs index 6204ab48..abff6bcb 100644 --- a/src/sso_client.rs +++ b/src/sso_client.rs @@ -117,6 +117,7 @@ impl Client { state: OIDCState, client_challenge: OIDCCodeChallenge, redirect_uri: String, + binding_hash: Option, ) -> ApiResult<(Url, SsoAuth)> { let scopes = CONFIG.sso_scopes_vec().into_iter().map(Scope::new); let base64_state = data_encoding::BASE64.encode(state.to_string().as_bytes()); @@ -139,7 +140,7 @@ impl Client { } let (auth_url, _, nonce) = auth_req.url(); - Ok((auth_url, SsoAuth::new(state, client_challenge, nonce.secret().clone(), redirect_uri))) + Ok((auth_url, SsoAuth::new(state, client_challenge, nonce.secret().clone(), redirect_uri, binding_hash))) } pub async fn exchange_code( diff --git a/src/static/scripts/admin.css b/src/static/scripts/admin.css index 0df56771..c7c6f443 100644 --- a/src/static/scripts/admin.css +++ b/src/static/scripts/admin.css @@ -1,6 +1,17 @@ body { padding-top: 75px; } +/* Some extra width's for the main layout */ +@media (min-width: 1600px) { + .container-xxl { + max-width: 1520px; + } +} +@media (min-width: 1800px) { + .container-xxl { + max-width: 1720px; + } +} img { width: 48px; height: 48px; @@ -38,8 +49,8 @@ img { max-width: 130px; } #users-table .vw-actions, #orgs-table .vw-actions { - min-width: 155px; - max-width: 160px; + min-width: 170px; + max-width: 180px; } #users-table .vw-org-cell { max-height: 120px; diff --git a/src/static/templates/admin/base.hbs b/src/static/templates/admin/base.hbs index f56d8262..e1dcacb5 100644 --- a/src/static/templates/admin/base.hbs +++ b/src/static/templates/admin/base.hbs @@ -27,7 +27,7 @@