From dbeec752b86a2ffd18a235c7c7d971cb094e0eee Mon Sep 17 00:00:00 2001 From: tom27052006 <83423411+tom27052006@users.noreply.github.com> Date: Fri, 31 Jul 2026 22:51:51 +0200 Subject: [PATCH] Add device approval by an organization administrator Completes the trusted device flow for the case it was missing: a member who unlocks with a trusted device, has no other device of their own left to ask, and therefore has no way back into their vault. They can now turn to the administrators of their organization, who hand them their own user key encrypted for the key pair of the asking device. New `atype` on auth_requests, mirroring bitwarden/server's AuthRequestType. It decides who may answer a request and how long it stays open: 15 minutes between the user's own devices, a week for an administrator, and half a day for their answer once given. The purge job applies that per type instead of dropping everything after 15 minutes, and both the answer and the anonymous lookup now refuse an expired request, which they did not before. POST /auth-requests/admin-request ask, one request per org GET /organizations//auth-requests what is waiting for an answer POST /organizations//auth-requests/ approve or deny one POST /organizations//auth-requests/deny deny several POST /organizations//auth-requests answer several Asking requires authentication, so the anonymous `POST /auth-requests` now refuses the type. Answering goes through the organization the request was addressed to and needs admin rights there; the asking user cannot answer their own request through `PUT /auth-requests/`, which would make the whole detour pointless. A denial is saved but not announced, so a request that did not come from the member does not learn that it was seen. The administrator's view leaves out the access code, which is the asking device's proof and none of their business. The administrators are mailed when a request arrives, the member when one of their devices was let in, so an approval nobody asked for does not pass unnoticed. Two fixes without which none of this is reachable from a client: - `UserDecryptionOptions.TrustedDeviceOption` reported `HasAdminApproval` and `HasManageResetPasswordPermission` as a flat false. The clients decide on `hasAdminApproval || hasMasterPassword` whether a login is a returning user or a brand new one, so a member without a master password was shown the screen for creating an account, on every device but the one they first trusted. Both are now derived from the account recovery enrollment and the role. - `PUT /organizations//users//reset-password-enrollment` demanded a master password whenever a key was supplied. An account that unlocks with a trusted device has none, and the clients send nothing but the key when they enroll during registration, so enrolling was impossible for exactly the accounts that need it most. Upstream carves out the same exception, keyed on the organization's SSO configuration rather than on a server-wide setting as here. Enrolling now also accepts a pending invitation, as upstream does, so a just-provisioned member does not stay invited forever with nobody able to confirm them. --- .../down.sql | 1 + .../up.sql | 1 + .../down.sql | 1 + .../up.sql | 1 + .../down.sql | 1 + .../up.sql | 1 + src/api/core/accounts.rs | 201 +++++++++----- src/api/core/organizations.rs | 245 +++++++++++++++++- src/api/identity.rs | 25 +- src/config.rs | 2 + src/db/models/auth_request.rs | 199 +++++++++++++- src/db/models/mod.rs | 2 +- src/db/schema.rs | 1 + src/mail.rs | 48 ++++ .../email/device_approval_requested.hbs | 6 + .../email/device_approval_requested.html.hbs | 16 ++ .../email/trusted_device_admin_approval.hbs | 9 + .../trusted_device_admin_approval.html.hbs | 22 ++ 18 files changed, 695 insertions(+), 87 deletions(-) create mode 100644 migrations/mysql/2026-07-31-130000_add_auth_request_type/down.sql create mode 100644 migrations/mysql/2026-07-31-130000_add_auth_request_type/up.sql create mode 100644 migrations/postgresql/2026-07-31-130000_add_auth_request_type/down.sql create mode 100644 migrations/postgresql/2026-07-31-130000_add_auth_request_type/up.sql create mode 100644 migrations/sqlite/2026-07-31-130000_add_auth_request_type/down.sql create mode 100644 migrations/sqlite/2026-07-31-130000_add_auth_request_type/up.sql create mode 100644 src/static/templates/email/device_approval_requested.hbs create mode 100644 src/static/templates/email/device_approval_requested.html.hbs create mode 100644 src/static/templates/email/trusted_device_admin_approval.hbs create mode 100644 src/static/templates/email/trusted_device_admin_approval.html.hbs diff --git a/migrations/mysql/2026-07-31-130000_add_auth_request_type/down.sql b/migrations/mysql/2026-07-31-130000_add_auth_request_type/down.sql new file mode 100644 index 00000000..3e6ab6ee --- /dev/null +++ b/migrations/mysql/2026-07-31-130000_add_auth_request_type/down.sql @@ -0,0 +1 @@ +ALTER TABLE auth_requests DROP COLUMN atype; diff --git a/migrations/mysql/2026-07-31-130000_add_auth_request_type/up.sql b/migrations/mysql/2026-07-31-130000_add_auth_request_type/up.sql new file mode 100644 index 00000000..90fba887 --- /dev/null +++ b/migrations/mysql/2026-07-31-130000_add_auth_request_type/up.sql @@ -0,0 +1 @@ +ALTER TABLE auth_requests ADD COLUMN atype INTEGER NOT NULL DEFAULT 0; diff --git a/migrations/postgresql/2026-07-31-130000_add_auth_request_type/down.sql b/migrations/postgresql/2026-07-31-130000_add_auth_request_type/down.sql new file mode 100644 index 00000000..3e6ab6ee --- /dev/null +++ b/migrations/postgresql/2026-07-31-130000_add_auth_request_type/down.sql @@ -0,0 +1 @@ +ALTER TABLE auth_requests DROP COLUMN atype; diff --git a/migrations/postgresql/2026-07-31-130000_add_auth_request_type/up.sql b/migrations/postgresql/2026-07-31-130000_add_auth_request_type/up.sql new file mode 100644 index 00000000..90fba887 --- /dev/null +++ b/migrations/postgresql/2026-07-31-130000_add_auth_request_type/up.sql @@ -0,0 +1 @@ +ALTER TABLE auth_requests ADD COLUMN atype INTEGER NOT NULL DEFAULT 0; diff --git a/migrations/sqlite/2026-07-31-130000_add_auth_request_type/down.sql b/migrations/sqlite/2026-07-31-130000_add_auth_request_type/down.sql new file mode 100644 index 00000000..3e6ab6ee --- /dev/null +++ b/migrations/sqlite/2026-07-31-130000_add_auth_request_type/down.sql @@ -0,0 +1 @@ +ALTER TABLE auth_requests DROP COLUMN atype; diff --git a/migrations/sqlite/2026-07-31-130000_add_auth_request_type/up.sql b/migrations/sqlite/2026-07-31-130000_add_auth_request_type/up.sql new file mode 100644 index 00000000..90fba887 --- /dev/null +++ b/migrations/sqlite/2026-07-31-130000_add_auth_request_type/up.sql @@ -0,0 +1 @@ +ALTER TABLE auth_requests ADD COLUMN atype INTEGER NOT NULL DEFAULT 0; diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index 81a0ceae..a831f0df 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -20,9 +20,10 @@ use crate::{ db::{ DbConn, DbPool, models::{ - AuthRequest, AuthRequestId, Cipher, CipherId, Device, DeviceId, DeviceType, DeviceWithAuthRequest, - EmergencyAccess, EmergencyAccessId, EventType, Folder, FolderId, Invitation, Membership, MembershipId, - OrgPolicy, OrgPolicyType, Organization, OrganizationId, Send, SendId, User, UserId, UserKdfType, + AuthRequest, AuthRequestId, AuthRequestType, Cipher, CipherId, Device, DeviceId, DeviceType, + DeviceWithAuthRequest, EmergencyAccess, EmergencyAccessId, EventType, Folder, FolderId, Invitation, + Membership, MembershipId, MembershipType, OrgPolicy, OrgPolicyType, Organization, OrganizationId, Send, + SendId, User, UserId, UserKdfType, }, }, mail, @@ -76,6 +77,7 @@ pub fn routes() -> Vec { post_devices_lost_trust, get_tasks, post_auth_request, + post_admin_auth_request, get_auth_request, put_auth_request, get_auth_request_response, @@ -1806,9 +1808,26 @@ struct AuthRequestRequest { device_identifier: DeviceId, email: String, public_key: String, - // Not used for now - // #[serde(alias = "type")] - // _type: i32, + #[serde(default, rename = "type")] + atype: i32, +} + +fn auth_request_json(auth_request: &AuthRequest) -> Value { + json!({ + "id": auth_request.uuid, + "publicKey": auth_request.public_key, + "type": auth_request.atype, + "requestDeviceType": DeviceType::from_i32(auth_request.device_type).to_string(), + "requestDeviceIdentifier": auth_request.request_device_identifier, + "requestIpAddress": auth_request.request_ip, + "key": auth_request.enc_key, + "masterPasswordHash": auth_request.master_password_hash, + "creationDate": format_date(&auth_request.creation_date), + "responseDate": auth_request.response_date.as_ref().map(format_date), + "requestApproved": auth_request.approved.unwrap_or(false), + "origin": CONFIG.domain_origin(), + "object": "auth-request" + }) } #[post("/auth-requests", data = "")] @@ -1820,6 +1839,12 @@ async fn post_auth_request( ) -> JsonResult { let data = data.into_inner(); + // Asking an administrator for approval means telling them who is asking, so that one is only + // available to a caller who has already proven who they are. See `post_admin_auth_request`. + if AuthRequestType::from_i32(data.atype) == Some(AuthRequestType::AdminApproval) { + err!("You must be authenticated to create a request of that type") + } + let Some(user) = User::find_by_mail(&data.email, &conn).await else { err!("AuthRequest doesn't exist", "User not found") }; @@ -1830,8 +1855,14 @@ async fn post_auth_request( _ => err!("AuthRequest doesn't exist", "Device verification failed"), }; + let Some(atype) = AuthRequestType::from_i32(data.atype) else { + err!("Unknown auth request type") + }; + let mut auth_request = AuthRequest::new( user.uuid.clone(), + None, + atype, data.device_identifier.clone(), client_headers.device_type, client_headers.ip.ip.to_string(), @@ -1851,19 +1882,91 @@ async fn post_auth_request( ) .await; - Ok(Json(json!({ - "id": auth_request.uuid, - "publicKey": auth_request.public_key, - "requestDeviceType": DeviceType::from_i32(auth_request.device_type).to_string(), - "requestIpAddress": auth_request.request_ip, - "key": null, - "masterPasswordHash": null, - "creationDate": format_date(&auth_request.creation_date), - "responseDate": null, - "requestApproved": false, - "origin": CONFIG.domain_origin(), - "object": "auth-request" - }))) + Ok(Json(auth_request_json(&auth_request))) +} + +/// Asks the administrators of every organization the user belongs to to let this device in. +/// +/// The way out for someone who unlocks with trusted devices and has no other device left to ask. +/// One request per organization, so whichever administrator gets there first can answer. +/// https://github.com/bitwarden/server/blob/main/src/Api/Auth/Controllers/AuthRequestsController.cs +#[post("/auth-requests/admin-request", data = "")] +async fn post_admin_auth_request(data: Json, headers: Headers, conn: DbConn) -> JsonResult { + let data = data.into_inner(); + + if AuthRequestType::from_i32(data.atype) != Some(AuthRequestType::AdminApproval) { + err!("Invalid auth request type, expected admin approval") + } + + if data.device_identifier != headers.device.uuid { + err!("AuthRequest doesn't exist", "Device verification failed") + } + + let memberships = Membership::find_by_user(&headers.user.uuid, &conn).await; + if memberships.is_empty() { + err!("User does not belong to any organization") + } + + log_user_event( + EventType::UserRequestedDeviceApproval as i32, + &headers.user.uuid, + headers.device.atype, + &headers.ip.ip, + &conn, + ) + .await; + + let mut first_request = None; + for membership in memberships { + let mut auth_request = AuthRequest::new( + headers.user.uuid.clone(), + Some(membership.org_uuid.clone()), + AuthRequestType::AdminApproval, + data.device_identifier.clone(), + headers.device.atype, + headers.ip.ip.to_string(), + data.access_code.clone(), + data.public_key.clone(), + ); + auth_request.save(&conn).await?; + + notify_device_approval_requested(&headers.user, &membership.org_uuid, &conn).await; + + if first_request.is_none() { + first_request = Some(auth_request); + } + } + + // Guaranteed by the emptiness check above + let auth_request = first_request.expect("at least one organization"); + Ok(Json(auth_request_json(&auth_request))) +} + +/// Mails everyone in the organization who could answer the request. Failing to reach them must not +/// undo the request itself, so problems are logged rather than returned. +async fn notify_device_approval_requested(user: &User, org_id: &OrganizationId, conn: &DbConn) { + if !CONFIG.mail_enabled() { + return; + } + + let Some(org) = Organization::find_by_uuid(org_id, conn).await else { + return; + }; + + let approvers = Membership::find_confirmed_by_org(org_id, conn) + .await + .into_iter() + .filter(|member| member.atype <= MembershipType::Admin as i32); + + for approver in approvers { + let Some(admin) = User::find_by_uuid(&approver.user_uuid, conn).await else { + continue; + }; + + if let Err(e) = mail::send_device_approval_requested(&admin.email, &org.name, &user.email, &user.name).await { + error!("Error sending device approval request email: {e:#?}"); + } + } } #[get("/auth-requests/")] @@ -1873,21 +1976,7 @@ async fn get_auth_request(auth_request_id: AuthRequestId, headers: Headers, conn err!("AuthRequest doesn't exist", "Record not found or user uuid does not match") }; - let response_date_utc = auth_request.response_date.map(|response_date| format_date(&response_date)); - - Ok(Json(json!({ - "id": &auth_request_id, - "publicKey": auth_request.public_key, - "requestDeviceType": DeviceType::from_i32(auth_request.device_type).to_string(), - "requestIpAddress": auth_request.request_ip, - "key": auth_request.enc_key, - "masterPasswordHash": auth_request.master_password_hash, - "creationDate": format_date(&auth_request.creation_date), - "responseDate": response_date_utc, - "requestApproved": auth_request.approved, - "origin": CONFIG.domain_origin(), - "object":"auth-request" - }))) + Ok(Json(auth_request_json(&auth_request))) } #[derive(Debug, Deserialize)] @@ -1914,6 +2003,13 @@ async fn put_auth_request( err!("AuthRequest doesn't exist", "Record not found or user uuid does not match") }; + // A request addressed to an administrator is answered through the organization, where the + // permission to do so can actually be checked. Letting the asking user answer it here would + // make the whole detour pointless. + if auth_request.is_admin_approval() { + err!("AuthRequest doesn't exist", "Admin approval requests are answered by the organization") + } + if headers.device.uuid != data.device_identifier { err!("AuthRequest doesn't exist", "Device verification failed") } @@ -1922,8 +2018,11 @@ async fn put_auth_request( err!("An authentication request with the same device already exists") } + if auth_request.is_expired() { + err!("AuthRequest doesn't exist", "Request has expired") + } + let response_date = Utc::now().naive_utc(); - let response_date_utc = format_date(&response_date); if data.request_approved { auth_request.approved = Some(data.request_approved); @@ -1957,19 +2056,7 @@ async fn put_auth_request( .await; } - Ok(Json(json!({ - "id": &auth_request_id, - "publicKey": auth_request.public_key, - "requestDeviceType": DeviceType::from_i32(auth_request.device_type).to_string(), - "requestIpAddress": auth_request.request_ip, - "key": auth_request.enc_key, - "masterPasswordHash": auth_request.master_password_hash, - "creationDate": format_date(&auth_request.creation_date), - "responseDate": response_date_utc, - "requestApproved": auth_request.approved, - "origin": CONFIG.domain_origin(), - "object":"auth-request" - }))) + Ok(Json(auth_request_json(&auth_request))) } #[get("/auth-requests//response?")] @@ -1990,21 +2077,11 @@ async fn get_auth_request_response( err!("AuthRequest doesn't exist", "Invalid device, IP or code") } - let response_date_utc = auth_request.response_date.map(|response_date| format_date(&response_date)); + if auth_request.is_expired() { + err!("AuthRequest doesn't exist", "Request has expired") + } - Ok(Json(json!({ - "id": &auth_request_id, - "publicKey": auth_request.public_key, - "requestDeviceType": DeviceType::from_i32(auth_request.device_type).to_string(), - "requestIpAddress": auth_request.request_ip, - "key": auth_request.enc_key, - "masterPasswordHash": auth_request.master_password_hash, - "creationDate": format_date(&auth_request.creation_date), - "responseDate": response_date_utc, - "requestApproved": auth_request.approved, - "origin": CONFIG.domain_origin(), - "object":"auth-request" - }))) + Ok(Json(auth_request_json(&auth_request))) } // Now unused but not yet removed diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 989ca47d..197480f3 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -1,5 +1,6 @@ use std::collections::{HashMap, HashSet}; +use chrono::Utc; use num_traits::FromPrimitive; use rocket::{Route, serde::json::Json}; use serde_json::Value; @@ -8,16 +9,17 @@ use crate::{ CONFIG, api::admin::FAKE_ADMIN_UUID, api::{ - EmptyResult, JsonResult, Notify, PasswordOrOtpData, UpdateType, + AnonymousNotify, EmptyResult, JsonResult, Notify, PasswordOrOtpData, UpdateType, core::{CipherSyncData, CipherSyncType, accept_org_invite, log_event, two_factor}, }, auth::{AdminHeaders, Headers, ManagerHeaders, ManagerHeadersLoose, OrgMemberHeaders, OwnerHeaders, decode_invite}, db::{ DbConn, models::{ - Cipher, CipherId, Collection, CollectionCipher, CollectionGroup, CollectionId, CollectionUser, EventType, - Group, GroupId, GroupUser, Invitation, Membership, MembershipId, MembershipStatus, MembershipType, - OrgPolicy, OrgPolicyType, Organization, OrganizationApiKey, OrganizationId, User, UserId, + AuthRequest, AuthRequestId, Cipher, CipherId, Collection, CollectionCipher, CollectionGroup, CollectionId, + CollectionUser, DeviceType, EventType, Group, GroupId, GroupUser, Invitation, Membership, MembershipId, + MembershipStatus, MembershipType, OrgPolicy, OrgPolicyType, Organization, OrganizationApiKey, + OrganizationId, User, UserId, }, }, mail, @@ -97,6 +99,10 @@ pub fn routes() -> Vec { get_reset_password_details, put_reset_password, put_recover_account, + get_organization_auth_requests, + deny_organization_auth_requests, + update_organization_auth_request, + update_many_organization_auth_requests, get_org_export, post_api_key, rotate_api_key, @@ -3153,7 +3159,14 @@ async fn put_reset_password_enrollment( err!("Reset password can't be withdrawn due to an enterprise policy"); } - if reset_password_key.is_some() { + // An account that unlocks with a trusted device has no master password to verify against, and + // the clients send nothing but the key when they enroll as part of that flow. Upstream carves + // out the same exception, keyed on the organization's SSO configuration rather than on a + // server-wide setting as here. + // https://github.com/bitwarden/server/blob/main/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs + let trusted_device_enrollment = CONFIG.sso_trusted_device_encryption() && headers.user.password_hash.is_empty(); + + if reset_password_key.is_some() && !trusted_device_enrollment { PasswordOrOtpData { master_password_hash: reset_request.master_password_hash, otp: reset_request.otp, @@ -3162,21 +3175,233 @@ async fn put_reset_password_enrollment( .await?; } - membership.reset_password_key = reset_password_key; - membership.save(&conn).await?; + let enrolled = reset_password_key.is_some(); + let membership_id = membership.uuid.clone(); - let event_type = if membership.reset_password_key.is_some() { + // Enrolling is where a member who was invited into a trusted device organization turns into a + // real one; upstream accepts the invitation at this point as well. Without it they would stay + // invited forever and no admin could ever confirm them. + if enrolled && membership.status == MembershipStatus::Invited as i32 { + accept_org_invite(&headers.user, membership, reset_password_key, &conn).await?; + } else { + membership.reset_password_key = reset_password_key; + membership.save(&conn).await?; + } + + let event_type = if enrolled { EventType::OrganizationUserResetPasswordEnroll as i32 } else { EventType::OrganizationUserResetPasswordWithdraw as i32 }; - log_event(event_type, &membership.uuid, &org_id, &headers.user.uuid, headers.device.atype, &headers.ip.ip, &conn) + log_event(event_type, &membership_id, &org_id, &headers.user.uuid, headers.device.atype, &headers.ip.ip, &conn) .await; Ok(()) } +// Device approvals. A member who unlocks with a trusted device and has no other device of their own +// left to ask can turn to the administrators of their organization instead. Answering means handing +// them their own user key, encrypted for the key pair of the asking device, which is only possible +// because the member enrolled into account recovery beforehand. +// https://github.com/bitwarden/server/blob/main/src/Api/AdminConsole/Controllers/OrganizationAuthRequestsController.cs + +/// The requests waiting for an answer in this organization. +#[get("/organizations//auth-requests")] +async fn get_organization_auth_requests(org_id: OrganizationId, headers: AdminHeaders, conn: DbConn) -> JsonResult { + if org_id != headers.org_id { + err!("Organization not found", "Organization id's do not match"); + } + + let mut requests = Vec::new(); + for auth_request in AuthRequest::find_pending_admin_approval_by_org(&org_id, &conn).await { + if auth_request.is_expired() { + continue; + } + + // A request whose asker is no longer a member of this organization is none of its business + // anymore, so it is quietly left out instead of being offered for approval. + let (Some(member), Some(user)) = ( + Membership::find_by_user_and_org(&auth_request.user_uuid, &org_id, &conn).await, + User::find_by_uuid(&auth_request.user_uuid, &conn).await, + ) else { + continue; + }; + + requests.push(auth_request.to_json_for_organization(&user.email, &member.uuid)); + } + + Ok(Json(json!({ + "data": requests, + "continuationToken": null, + "object": "list" + }))) +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct AdminAuthRequestUpdateData { + request_approved: bool, + encrypted_user_key: Option, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct BulkDenyAuthRequestData { + ids: Vec, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct OrganizationAuthRequestUpdateData { + id: AuthRequestId, + approved: bool, + key: Option, +} + +#[post("/organizations//auth-requests/", data = "", rank = 2)] +async fn update_organization_auth_request( + org_id: OrganizationId, + request_id: AuthRequestId, + data: Json, + headers: AdminHeaders, + conn: DbConn, + ant: AnonymousNotify<'_>, + nt: Notify<'_>, +) -> EmptyResult { + let data = data.into_inner(); + answer_organization_auth_request( + &org_id, + &request_id, + data.request_approved, + data.encrypted_user_key, + &headers, + &conn, + &ant, + &nt, + ) + .await +} + +#[post("/organizations//auth-requests/deny", data = "", rank = 1)] +async fn deny_organization_auth_requests( + org_id: OrganizationId, + data: Json, + headers: AdminHeaders, + conn: DbConn, + ant: AnonymousNotify<'_>, + nt: Notify<'_>, +) -> EmptyResult { + for request_id in data.into_inner().ids { + answer_organization_auth_request(&org_id, &request_id, false, None, &headers, &conn, &ant, &nt).await?; + } + + Ok(()) +} + +#[post("/organizations//auth-requests", data = "")] +async fn update_many_organization_auth_requests( + org_id: OrganizationId, + data: Json>, + headers: AdminHeaders, + conn: DbConn, + ant: AnonymousNotify<'_>, + nt: Notify<'_>, +) -> EmptyResult { + for update in data.into_inner() { + answer_organization_auth_request(&org_id, &update.id, update.approved, update.key, &headers, &conn, &ant, &nt) + .await?; + } + + Ok(()) +} + +#[expect(clippy::too_many_arguments, reason = "Rocket request guards have to be passed through")] +async fn answer_organization_auth_request( + org_id: &OrganizationId, + request_id: &AuthRequestId, + approved: bool, + encrypted_user_key: Option, + headers: &AdminHeaders, + conn: &DbConn, + ant: &AnonymousNotify<'_>, + nt: &Notify<'_>, +) -> EmptyResult { + if org_id != &headers.org_id { + err!("Organization not found", "Organization id's do not match"); + } + + // Only ever reachable through the organization it was addressed to, so an administrator cannot + // answer for an organization they have no say in. + let Some(mut auth_request) = AuthRequest::find_admin_approval_by_org_and_uuid(request_id, org_id, conn).await + else { + err!("AuthRequest doesn't exist", "Record not found or not addressed to this organization") + }; + + if auth_request.approved.is_some() { + err!("This request has already been answered") + } + + if auth_request.is_expired() { + err!("AuthRequest doesn't exist", "Request has expired") + } + + let Some(member) = Membership::find_by_user_and_org(&auth_request.user_uuid, org_id, conn).await else { + err!("AuthRequest doesn't exist", "The requesting user is no longer a member of this organization") + }; + + if approved { + // Without the wrapped user key the answer is worthless: it is the whole point of approving. + let Some(key) = encrypted_user_key.filter(|key| !key.is_empty()) else { + err!("An approved request needs the encrypted user key") + }; + auth_request.enc_key = Some(key); + } + + auth_request.approved = Some(approved); + auth_request.response_date = Some(Utc::now().naive_utc()); + auth_request.save(conn).await?; + + let event_type = if approved { + EventType::OrganizationUserApprovedAuthRequest as i32 + } else { + EventType::OrganizationUserRejectedAuthRequest as i32 + }; + log_event(event_type, &member.uuid, org_id, &headers.user.uuid, headers.device.atype, &headers.ip.ip, conn).await; + + // A denial is deliberately not announced. If the request came from somebody who is not the + // member, telling them that it was seen and refused is more than they should learn. + if !approved { + return Ok(()); + } + + ant.send_auth_response(&auth_request.user_uuid, &auth_request.uuid).await; + nt.send_auth_response(&auth_request.user_uuid, &auth_request.uuid, &headers.device, conn).await; + + if CONFIG.mail_enabled() + && let Some(user) = User::find_by_uuid(&auth_request.user_uuid, conn).await + && let Some(org) = Organization::find_by_uuid(org_id, conn).await + { + let device = + format!("{} - {}", DeviceType::from_i32(auth_request.device_type), auth_request.request_device_identifier); + let approved_at = auth_request.response_date.unwrap_or_else(|| Utc::now().naive_utc()); + + if let Err(e) = mail::send_trusted_device_admin_approval( + &user.email, + &org.name, + &approved_at, + &auth_request.request_ip, + &device, + ) + .await + { + error!("Error sending trusted device approval email: {e:#?}"); + } + } + + Ok(()) +} + // NOTE: It seems clients can't handle uppercase-first keys!! // We need to convert all keys so they have the first character to be a lowercase. // Else the export will be just an empty JSON file. @@ -3214,7 +3439,7 @@ async fn api_key( let org_api_key = if let Some(mut org_api_key) = OrganizationApiKey::find_by_org_uuid(org_id, &conn).await { if rotate { org_api_key.api_key = crate::crypto::generate_api_key(); - org_api_key.revision_date = chrono::Utc::now().naive_utc(); + org_api_key.revision_date = Utc::now().naive_utc(); org_api_key.save(&conn).await.expect("Error rotating organization API Key"); } org_api_key diff --git a/src/api/identity.rs b/src/api/identity.rs index a25c83b3..37cd52ec 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -30,9 +30,9 @@ use crate::{ db::{ DbConn, models::{ - AuthRequest, AuthRequestId, Device, DeviceId, DeviceType, EventType, Invitation, OIDCCodeResponseError, - OrganizationApiKey, OrganizationId, SendId, SsoAuth, SsoUser, TwoFactor, TwoFactorIncomplete, - TwoFactorType, User, UserId, + AuthRequest, AuthRequestId, Device, DeviceId, DeviceType, EventType, Invitation, Membership, + MembershipStatus, MembershipType, OIDCCodeResponseError, OrganizationApiKey, OrganizationId, SendId, + SsoAuth, SsoUser, TwoFactor, TwoFactorIncomplete, TwoFactorType, User, UserId, }, }, error::MapResult, @@ -507,12 +507,23 @@ async fn trusted_device_option(user: &User, device: &Device, conn: &DbConn) -> O .iter() .any(|other| other.uuid != device.uuid && DeviceType::from_i32(other.atype).can_approve_login_requests()); - // Approval by an organization admin is not implemented. Announcing it would leave the client - // waiting on a request that nobody here can answer. + let memberships = Membership::find_by_user(&user.uuid, conn).await; + + // An admin can only take over the approval once the member handed them a key to work with, + // which is what enrolling into account recovery does. + let has_admin_approval = + memberships.iter().any(|member| member.reset_password_key.as_ref().is_some_and(|key| !key.is_empty())); + + // Whether the user is on the answering side of that. The clients use it to push someone who + // could approve others, but has no master password themselves, into setting one. + let has_manage_reset_password_permission = memberships.iter().any(|member| { + member.status != MembershipStatus::Revoked as i32 && member.atype <= MembershipType::Admin as i32 + }); + Some(json!({ - "HasAdminApproval": false, + "HasAdminApproval": has_admin_approval, "HasLoginApprovingDevice": has_login_approving_device, - "HasManageResetPasswordPermission": false, + "HasManageResetPasswordPermission": has_manage_reset_password_permission, "IsTdeOffboarding": offboarding, "EncryptedPrivateKey": device.trusted_private_key(), "EncryptedUserKey": device.trusted_user_key(), diff --git a/src/config.rs b/src/config.rs index 72007f3e..09b72907 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1732,6 +1732,7 @@ where reg!("email/change_email_invited", ".html"); reg!("email/change_email", ".html"); reg!("email/delete_account", ".html"); + reg!("email/device_approval_requested", ".html"); reg!("email/emergency_access_invite_accepted", ".html"); reg!("email/emergency_access_invite_confirmed", ".html"); reg!("email/emergency_access_recovery_approved", ".html"); @@ -1753,6 +1754,7 @@ where reg!("email/send_single_org_removed_from_org", ".html"); reg!("email/smtp_test", ".html"); reg!("email/sso_change_email", ".html"); + reg!("email/trusted_device_admin_approval", ".html"); reg!("email/twofactor_email", ".html"); reg!("email/verify_email", ".html"); reg!("email/welcome_must_verify", ".html"); diff --git a/src/db/models/auth_request.rs b/src/db/models/auth_request.rs index a3876661..4a1a1124 100644 --- a/src/db/models/auth_request.rs +++ b/src/db/models/auth_request.rs @@ -1,4 +1,4 @@ -use chrono::{NaiveDateTime, Utc}; +use chrono::{NaiveDateTime, TimeDelta, Utc}; use derive_more::{AsRef, Deref, Display, From}; use diesel::prelude::*; use serde_json::Value; @@ -12,7 +12,7 @@ use crate::{ }; use macros::UuidFromParam; -use super::{DeviceId, OrganizationId, UserId}; +use super::{DeviceId, DeviceType, MembershipId, OrganizationId, UserId}; #[derive(Identifiable, Queryable, Insertable, AsChangeset, Deserialize, Serialize)] #[diesel(table_name = auth_requests)] @@ -22,6 +22,8 @@ pub struct AuthRequest { pub uuid: AuthRequestId, pub user_uuid: UserId, pub organization_uuid: Option, + /// See `AuthRequestType`. Decides who may answer the request and how long it stays open. + pub atype: i32, pub request_device_identifier: DeviceId, pub device_type: i32, // https://github.com/bitwarden/server/blob/9ebe16587175b1c0e9208f84397bb75d0d595510/src/Core/Enums/DeviceType.cs @@ -42,9 +44,50 @@ pub struct AuthRequest { pub authentication_date: Option, } +/// https://github.com/bitwarden/server/blob/main/src/Core/Auth/Enums/AuthRequestType.cs +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum AuthRequestType { + /// A new session asking one of the user's own devices to let it in. + AuthenticateAndUnlock = 0, + /// An existing session asking one of the user's own devices to unlock it. + Unlock = 1, + /// The user asking an administrator of their organization to let a device in, for when no + /// device of their own is around to ask. + AdminApproval = 2, +} + +impl AuthRequestType { + pub fn from_i32(value: i32) -> Option { + match value { + 0 => Some(AuthRequestType::AuthenticateAndUnlock), + 1 => Some(AuthRequestType::Unlock), + 2 => Some(AuthRequestType::AdminApproval), + _ => None, + } + } +} + impl AuthRequest { + /// A request between the user's own devices is short lived, an administrator gets a week to + /// answer, and their answer stays usable for half a day. Same windows as upstream. + /// https://github.com/bitwarden/server/blob/main/src/Core/Settings/GlobalSettings.cs + pub fn user_request_expiration() -> TimeDelta { + TimeDelta::try_minutes(15).unwrap() + } + + pub fn admin_request_expiration() -> TimeDelta { + TimeDelta::try_days(7).unwrap() + } + + pub fn after_admin_approval_expiration() -> TimeDelta { + TimeDelta::try_hours(12).unwrap() + } + + #[expect(clippy::too_many_arguments, reason = "Every field of the request is supplied by the caller")] pub fn new( user_uuid: UserId, + organization_uuid: Option, + atype: AuthRequestType, request_device_identifier: DeviceId, device_type: i32, request_ip: String, @@ -56,7 +99,8 @@ impl AuthRequest { Self { uuid: AuthRequestId(crate::util::get_uuid()), user_uuid, - organization_uuid: None, + organization_uuid, + atype: atype as i32, request_device_identifier, device_type, @@ -73,12 +117,50 @@ impl AuthRequest { } } + pub fn is_admin_approval(&self) -> bool { + self.atype == AuthRequestType::AdminApproval as i32 + } + + pub fn is_expired(&self) -> bool { + let now = Utc::now().naive_utc(); + + if self.is_admin_approval() { + // Once approved the clock restarts, so the user has time to come back and use it. + if let (Some(true), Some(response_date)) = (self.approved, self.response_date) { + return now > response_date + Self::after_admin_approval_expiration(); + } + return now > self.creation_date + Self::admin_request_expiration(); + } + + now > self.creation_date + Self::user_request_expiration() + } + pub fn to_json_for_pending_device(&self) -> Value { json!({ "id": self.uuid, "creationDate": format_date(&self.creation_date), }) } + + /// What an administrator gets to see about a request. Deliberately without the access code: + /// that one is the requesting device's proof, not something the answering side needs. + pub fn to_json_for_organization(&self, email: &str, member_id: &MembershipId) -> Value { + json!({ + "id": self.uuid, + "userId": self.user_uuid, + "organizationUserId": member_id, + "email": email, + "publicKey": self.public_key, + "requestDeviceIdentifier": self.request_device_identifier, + "requestDeviceType": DeviceType::from_i32(self.device_type).to_string(), + "requestIpAddress": self.request_ip, + "key": self.enc_key, + "creationDate": format_date(&self.creation_date), + "requestApproved": self.approved, + "responseDate": self.response_date.as_ref().map(format_date), + "object": "organizationAuthRequest", + }) + } } impl AuthRequest { @@ -155,6 +237,38 @@ impl AuthRequest { .await } + /// Everything an administrator of this organization still has to answer. + pub async fn find_pending_admin_approval_by_org(org_uuid: &OrganizationId, conn: &DbConn) -> Vec { + conn.run(move |conn| { + auth_requests::table + .filter(auth_requests::organization_uuid.eq(org_uuid)) + .filter(auth_requests::atype.eq(AuthRequestType::AdminApproval as i32)) + .filter(auth_requests::approved.is_null()) + .order_by(auth_requests::creation_date.desc()) + .load::(conn) + .expect("Error loading auth_requests") + }) + .await + } + + /// Bound to the organization on purpose: an administrator may only ever reach a request that + /// was addressed to their own organization. + pub async fn find_admin_approval_by_org_and_uuid( + uuid: &AuthRequestId, + org_uuid: &OrganizationId, + conn: &DbConn, + ) -> Option { + conn.run(move |conn| { + auth_requests::table + .filter(auth_requests::uuid.eq(uuid)) + .filter(auth_requests::organization_uuid.eq(org_uuid)) + .filter(auth_requests::atype.eq(AuthRequestType::AdminApproval as i32)) + .first::(conn) + .ok() + }) + .await + } + pub async fn find_created_before(dt: &NaiveDateTime, conn: &DbConn) -> Vec { conn.run(move |conn| { auth_requests::table @@ -179,11 +293,15 @@ impl AuthRequest { } pub async fn purge_expired_auth_requests(conn: &DbConn) { - // delete auth requests older than 15 minutes which is functionally equivalent to upstream: // https://github.com/bitwarden/server/blob/f8ee2270409f7a13125cd414c450740af605a175/src/Sql/dbo/Auth/Stored%20Procedures/AuthRequest_DeleteIfExpired.sql - let expiry_time = Utc::now().naive_utc() - chrono::TimeDelta::try_minutes(15).unwrap(); - for auth_request in Self::find_created_before(&expiry_time, conn).await { - auth_request.delete(conn).await.ok(); + // Nothing can be expired before the shortest window has passed, so that is the cheapest + // way to narrow the table down; which of them really are is decided per type afterwards, + // because a request waiting for an administrator lives a week rather than 15 minutes. + let candidates = Utc::now().naive_utc() - Self::user_request_expiration(); + for auth_request in Self::find_created_before(&candidates, conn).await { + if auth_request.is_expired() { + auth_request.delete(conn).await.ok(); + } } } } @@ -205,3 +323,70 @@ impl AuthRequest { UuidFromParam, )] pub struct AuthRequestId(String); + +#[cfg(test)] +mod tests { + use super::*; + + fn request(atype: AuthRequestType, age: TimeDelta) -> AuthRequest { + let mut auth_request = AuthRequest::new( + String::from("user").into(), + None, + atype, + String::from("device").into(), + 9, + String::from("127.0.0.1"), + String::from("code"), + String::from("2.public"), + ); + auth_request.creation_date = Utc::now().naive_utc() - age; + auth_request + } + + #[test] + fn a_request_between_the_users_own_devices_is_short_lived() { + assert!(!request(AuthRequestType::AuthenticateAndUnlock, TimeDelta::try_minutes(14).unwrap()).is_expired()); + assert!(request(AuthRequestType::AuthenticateAndUnlock, TimeDelta::try_minutes(16).unwrap()).is_expired()); + assert!(request(AuthRequestType::Unlock, TimeDelta::try_minutes(16).unwrap()).is_expired()); + } + + #[test] + fn an_administrator_gets_a_week_to_answer() { + assert!(!request(AuthRequestType::AdminApproval, TimeDelta::try_days(6).unwrap()).is_expired()); + assert!(request(AuthRequestType::AdminApproval, TimeDelta::try_days(8).unwrap()).is_expired()); + } + + #[test] + fn the_answer_of_an_administrator_starts_its_own_clock() { + // Answered right at the end of the week, so the request itself is long past its window. + let mut auth_request = request(AuthRequestType::AdminApproval, TimeDelta::try_days(7).unwrap()); + auth_request.approved = Some(true); + + auth_request.response_date = Some(Utc::now().naive_utc() - TimeDelta::try_hours(11).unwrap()); + assert!(!auth_request.is_expired(), "the user still has time to come back and use it"); + + auth_request.response_date = Some(Utc::now().naive_utc() - TimeDelta::try_hours(13).unwrap()); + assert!(auth_request.is_expired()); + + // A refusal does not extend anything, the request stays dead after its own window. + auth_request.approved = Some(false); + auth_request.response_date = Some(Utc::now().naive_utc()); + assert!(auth_request.is_expired()); + } + + #[test] + fn only_the_admin_approval_type_is_answered_by_an_organization() { + assert!(request(AuthRequestType::AdminApproval, TimeDelta::zero()).is_admin_approval()); + assert!(!request(AuthRequestType::Unlock, TimeDelta::zero()).is_admin_approval()); + assert!(!request(AuthRequestType::AuthenticateAndUnlock, TimeDelta::zero()).is_admin_approval()); + } + + #[test] + fn unknown_request_types_are_rejected() { + assert_eq!(AuthRequestType::from_i32(0), Some(AuthRequestType::AuthenticateAndUnlock)); + assert_eq!(AuthRequestType::from_i32(1), Some(AuthRequestType::Unlock)); + assert_eq!(AuthRequestType::from_i32(2), Some(AuthRequestType::AdminApproval)); + assert_eq!(AuthRequestType::from_i32(3), None); + assert_eq!(AuthRequestType::from_i32(-1), None); + } +} diff --git a/src/db/models/mod.rs b/src/db/models/mod.rs index 0ed8ef91..148a2105 100644 --- a/src/db/models/mod.rs +++ b/src/db/models/mod.rs @@ -20,7 +20,7 @@ mod user; pub use self::archive::Archive; pub use self::attachment::{Attachment, AttachmentId}; -pub use self::auth_request::{AuthRequest, AuthRequestId}; +pub use self::auth_request::{AuthRequest, AuthRequestId, AuthRequestType}; pub use self::cipher::{Cipher, CipherId, RepromptType}; pub use self::collection::{Collection, CollectionCipher, CollectionId, CollectionUser}; pub use self::device::{Device, DeviceId, DeviceType, DeviceWithAuthRequest, PushId}; diff --git a/src/db/schema.rs b/src/db/schema.rs index b1766270..66687ce6 100644 --- a/src/db/schema.rs +++ b/src/db/schema.rs @@ -331,6 +331,7 @@ table! { uuid -> Text, user_uuid -> Text, organization_uuid -> Nullable, + atype -> Integer, request_device_identifier -> Text, device_type -> Integer, request_ip -> Text, diff --git a/src/mail.rs b/src/mail.rs index a7e5e5ae..fe5f217a 100644 --- a/src/mail.rs +++ b/src/mail.rs @@ -531,6 +531,54 @@ pub async fn send_new_device_logged_in(address: &str, ip: &str, dt: &NaiveDateTi send_email(address, &subject, body_html, body_text).await } +/// Tells the administrators of an organization that one of their members is waiting to have a +/// device let in. Trusted device encryption falls back to this when the member has no other device +/// of their own left to ask. +pub async fn send_device_approval_requested( + address: &str, + org_name: &str, + user_email: &str, + user_name: &str, +) -> EmptyResult { + let (subject, body_html, body_text) = get_text( + "email/device_approval_requested", + json!({ + "url": CONFIG.domain(), + "img_src": CONFIG._smtp_img_src(), + "org_name": org_name, + "user_email": user_email, + "user_name": user_name, + }), + )?; + + send_email(address, &subject, body_html, body_text).await +} + +/// The other half of the above: the member learns that a device of theirs was let in, so an +/// approval they did not ask for does not pass unnoticed. +pub async fn send_trusted_device_admin_approval( + address: &str, + org_name: &str, + dt: &NaiveDateTime, + ip: &str, + device: &str, +) -> EmptyResult { + let fmt = "%A, %B %_d, %Y at %r %Z"; + let (subject, body_html, body_text) = get_text( + "email/trusted_device_admin_approval", + json!({ + "url": CONFIG.domain(), + "img_src": CONFIG._smtp_img_src(), + "org_name": org_name, + "datetime": crate::util::format_naive_datetime_local(dt, fmt), + "ip": ip, + "device": device, + }), + )?; + + send_email(address, &subject, body_html, body_text).await +} + pub async fn send_incomplete_2fa_login( address: &str, ip: &str, diff --git a/src/static/templates/email/device_approval_requested.hbs b/src/static/templates/email/device_approval_requested.hbs new file mode 100644 index 00000000..e436c01c --- /dev/null +++ b/src/static/templates/email/device_approval_requested.hbs @@ -0,0 +1,6 @@ +Device Approval Requested + +{{user_name}} ({{user_email}}) is asking to have a new device approved in your {{org_name}} organization. Until an administrator approves it, they cannot get into their vault on that device. + +Review the request in the organization administration of {{{url}}}. +{{> email/email_footer_text }} diff --git a/src/static/templates/email/device_approval_requested.html.hbs b/src/static/templates/email/device_approval_requested.html.hbs new file mode 100644 index 00000000..ee66c98d --- /dev/null +++ b/src/static/templates/email/device_approval_requested.html.hbs @@ -0,0 +1,16 @@ +Device Approval Requested + +{{> email/email_header }} + + + + + + + +
+ {{user_name}} ({{user_email}}) is asking to have a new device approved in your {{org_name}} organization. Until an administrator approves it, they cannot get into their vault on that device. +
+ Review the request in the organization administration of {{{url}}}. +
+{{> email/email_footer }} diff --git a/src/static/templates/email/trusted_device_admin_approval.hbs b/src/static/templates/email/trusted_device_admin_approval.hbs new file mode 100644 index 00000000..af5fbcf2 --- /dev/null +++ b/src/static/templates/email/trusted_device_admin_approval.hbs @@ -0,0 +1,9 @@ +Device Approved + +An administrator of your {{org_name}} organization approved a device for your account on {{datetime}}. + +Device: {{device}} +IP address: {{ip}} + +If this was not you, change your password and contact your administrator. +{{> email/email_footer_text }} diff --git a/src/static/templates/email/trusted_device_admin_approval.html.hbs b/src/static/templates/email/trusted_device_admin_approval.html.hbs new file mode 100644 index 00000000..535cd41a --- /dev/null +++ b/src/static/templates/email/trusted_device_admin_approval.html.hbs @@ -0,0 +1,22 @@ +Device Approved + +{{> email/email_header }} + + + + + + + + + + +
+ An administrator of your {{org_name}} organization approved a device for your account on {{datetime}}. +
+ Device: {{device}}
+ IP address: {{ip}} +
+ If this was not you, change your password and contact your administrator. +
+{{> email/email_footer }}