From ef2695de0cb81feaa5cab8045f0bff71ab3e8c71 Mon Sep 17 00:00:00 2001 From: Stefan Melmuk <509385+stefan0xC@users.noreply.github.com> Date: Mon, 20 Jan 2025 20:21:44 +0100 Subject: [PATCH] improve admin invite (#5403) * check for admin invite * refactor the invitation logic * cleanup check for undefined token * prevent wrong user from accepting invitation --- src/api/admin.rs | 9 ++- src/api/core/organizations.rs | 130 ++++++++++++++++------------------ src/api/core/public.rs | 10 +-- src/auth.rs | 8 +-- src/mail.rs | 16 ++--- 5 files changed, 77 insertions(+), 96 deletions(-) diff --git a/src/api/admin.rs b/src/api/admin.rs index c215d987..31159816 100644 --- a/src/api/admin.rs +++ b/src/api/admin.rs @@ -99,6 +99,7 @@ const DT_FMT: &str = "%Y-%m-%d %H:%M:%S %Z"; const BASE_TEMPLATE: &str = "admin/base"; const ACTING_ADMIN_USER: &str = "vaultwarden-admin-00000-000000000000"; +pub const FAKE_ADMIN_UUID: &str = "00000000-0000-0000-0000-000000000000"; fn admin_path() -> String { format!("{}{}", CONFIG.domain_path(), ADMIN_PATH) @@ -299,7 +300,9 @@ async fn invite_user(data: Json, _token: AdminToken, mut conn: DbCon async fn _generate_invite(user: &User, conn: &mut DbConn) -> EmptyResult { if CONFIG.mail_enabled() { - mail::send_invite(user, None, None, &CONFIG.invitation_org_name(), None).await + let org_id: OrganizationId = FAKE_ADMIN_UUID.to_string().into(); + let member_id: MembershipId = FAKE_ADMIN_UUID.to_string().into(); + mail::send_invite(user, org_id, member_id, &CONFIG.invitation_org_name(), None).await } else { let invitation = Invitation::new(&user.email); invitation.save(conn).await @@ -475,7 +478,9 @@ async fn resend_user_invite(user_id: UserId, _token: AdminToken, mut conn: DbCon } if CONFIG.mail_enabled() { - mail::send_invite(&user, None, None, &CONFIG.invitation_org_name(), None).await + let org_id: OrganizationId = FAKE_ADMIN_UUID.to_string().into(); + let member_id: MembershipId = FAKE_ADMIN_UUID.to_string().into(); + mail::send_invite(&user, org_id, member_id, &CONFIG.invitation_org_name(), None).await } else { Ok(()) } diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 45e5e0a3..efaa4ac1 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -4,6 +4,7 @@ use rocket::Route; use serde_json::Value; use std::collections::{HashMap, HashSet}; +use crate::api::admin::FAKE_ADMIN_UUID; use crate::{ api::{ core::{log_event, two_factor, CipherSyncData, CipherSyncType}, @@ -971,8 +972,8 @@ async fn send_invite( if let Err(e) = mail::send_invite( &user, - Some(org_id.clone()), - Some(new_member.uuid.clone()), + org_id.clone(), + new_member.uuid.clone(), &org_name, Some(headers.user.email.clone()), ) @@ -1098,14 +1099,7 @@ async fn _reinvite_member( }; if CONFIG.mail_enabled() { - mail::send_invite( - &user, - Some(org_id.clone()), - Some(member.uuid), - &org_name, - Some(invited_by_email.to_string()), - ) - .await?; + mail::send_invite(&user, org_id.clone(), member.uuid, &org_name, Some(invited_by_email.to_string())).await?; } else if user.password_hash.is_empty() { let invitation = Invitation::new(&user.email); invitation.save(conn).await?; @@ -1131,79 +1125,81 @@ async fn accept_invite( org_id: OrganizationId, member_id: MembershipId, data: Json, + headers: Headers, mut conn: DbConn, ) -> EmptyResult { // The web-vault passes org_id and member_id in the URL, but we are just reading them from the JWT instead let data: AcceptData = data.into_inner(); let claims = decode_invite(&data.token)?; - // If a claim does not have a member_id or it does not match the one in from the URI, something is wrong. - match &claims.member_id { - Some(ou_id) if ou_id.eq(&member_id) => {} - _ => err!("Error accepting the invitation", "Claim does not match the member_id"), + // Don't allow other users from accepting an invitation. + if !claims.email.eq(&headers.user.email) { + err!("Invitation was issued to a different account", "Claim does not match user_id") } - match User::find_by_mail(&claims.email, &mut conn).await { - Some(user) => { - Invitation::take(&claims.email, &mut conn).await; + // If a claim does not have a member_id or it does not match the one in from the URI, something is wrong. + if !claims.member_id.eq(&member_id) { + err!("Error accepting the invitation", "Claim does not match the member_id") + } - if let (Some(member), Some(org)) = (&claims.member_id, &claims.org_id) { - let Some(mut member) = Membership::find_by_uuid_and_org(member, org, &mut conn).await else { - err!("Error accepting the invitation") - }; + let member = &claims.member_id; + let org = &claims.org_id; - if member.status != MembershipStatus::Invited as i32 { - err!("User already accepted the invitation") - } + Invitation::take(&claims.email, &mut conn).await; - let master_password_required = OrgPolicy::org_is_reset_password_auto_enroll(org, &mut conn).await; - if data.reset_password_key.is_none() && master_password_required { - err!("Reset password key is required, but not provided."); - } + // skip invitation logic when we were invited via the /admin panel + if **member != FAKE_ADMIN_UUID { + let Some(mut member) = Membership::find_by_uuid_and_org(member, org, &mut conn).await else { + err!("Error accepting the invitation") + }; - // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, admin::update_membership_type - // It returns different error messages per function. - if member.atype < MembershipType::Admin { - match OrgPolicy::is_user_allowed(&member.user_uuid, &org_id, false, &mut conn).await { - Ok(_) => {} - Err(OrgPolicyErr::TwoFactorMissing) => { - if CONFIG.email_2fa_auto_fallback() { - two_factor::email::activate_email_2fa(&user, &mut conn).await?; - } else { - err!("You cannot join this organization until you enable two-step login on your user account"); - } - } - Err(OrgPolicyErr::SingleOrgEnforced) => { - err!("You cannot join this organization because you are a member of an organization which forbids it"); - } + if member.status != MembershipStatus::Invited as i32 { + err!("User already accepted the invitation") + } + + let master_password_required = OrgPolicy::org_is_reset_password_auto_enroll(org, &mut conn).await; + if data.reset_password_key.is_none() && master_password_required { + err!("Reset password key is required, but not provided."); + } + + // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, admin::update_membership_type + // It returns different error messages per function. + if member.atype < MembershipType::Admin { + match OrgPolicy::is_user_allowed(&member.user_uuid, &org_id, false, &mut conn).await { + Ok(_) => {} + Err(OrgPolicyErr::TwoFactorMissing) => { + if CONFIG.email_2fa_auto_fallback() { + two_factor::email::activate_email_2fa(&headers.user, &mut conn).await?; + } else { + err!("You cannot join this organization until you enable two-step login on your user account"); } } - - member.status = MembershipStatus::Accepted as i32; - - if master_password_required { - member.reset_password_key = data.reset_password_key; + Err(OrgPolicyErr::SingleOrgEnforced) => { + err!("You cannot join this organization because you are a member of an organization which forbids it"); } - - member.save(&mut conn).await?; } } - None => err!("Invited user not found"), + + member.status = MembershipStatus::Accepted as i32; + + if master_password_required { + member.reset_password_key = data.reset_password_key; + } + + member.save(&mut conn).await?; } if CONFIG.mail_enabled() { - let mut org_name = CONFIG.invitation_org_name(); - if let Some(org_id) = &claims.org_id { - org_name = match Organization::find_by_uuid(org_id, &mut conn).await { + if let Some(invited_by_email) = &claims.invited_by_email { + let org_name = match Organization::find_by_uuid(&claims.org_id, &mut conn).await { Some(org) => org.name, None => err!("Organization not found."), }; - }; - if let Some(invited_by_email) = &claims.invited_by_email { // User was invited to an organization, so they must be confirmed manually after acceptance mail::send_invite_accepted(&claims.email, invited_by_email, &org_name).await?; } else { // User was invited from /admin, so they are automatically confirmed + let org_name = CONFIG.invitation_org_name(); mail::send_invite_confirmed(&claims.email, &org_name).await?; } } @@ -1825,23 +1821,17 @@ async fn list_policies(org_id: OrganizationId, _headers: AdminHeaders, mut conn: #[get("/organizations//policies/token?")] async fn list_policies_token(org_id: OrganizationId, token: &str, mut conn: DbConn) -> JsonResult { - // web-vault 2024.6.2 seems to send these values and cause logs to output errors - // Catch this and prevent errors in the logs - // TODO: CleanUp after 2024.6.x is not used anymore. - if org_id.as_ref() == "undefined" && token == "undefined" { - return Ok(Json(json!({}))); - } - let invite = decode_invite(token)?; - let Some(invite_org_id) = invite.org_id else { - err!("Invalid token") - }; - - if invite_org_id != org_id { + if invite.org_id != org_id { err!("Token doesn't match request organization"); } + // exit early when we have been invited via /admin panel + if org_id.as_ref() == FAKE_ADMIN_UUID { + return Ok(Json(json!({}))); + } + // TODO: We receive the invite token as ?token=<>, validate it contains the org id let policies = OrgPolicy::find_by_org(&org_id, &mut conn).await; let policies_json: Vec = policies.iter().map(OrgPolicy::to_json).collect(); @@ -2141,8 +2131,8 @@ async fn import(org_id: OrganizationId, data: Json, headers: Head mail::send_invite( &user, - Some(org_id.clone()), - Some(new_member.uuid.clone()), + org_id.clone(), + new_member.uuid.clone(), &org_name, Some(headers.user.email.clone()), ) diff --git a/src/api/core/public.rs b/src/api/core/public.rs index cd524db6..1c85ae1b 100644 --- a/src/api/core/public.rs +++ b/src/api/core/public.rs @@ -119,14 +119,8 @@ async fn ldap_import(data: Json, token: PublicToken, mut conn: Db None => err!("Error looking up organization"), }; - if let Err(e) = mail::send_invite( - &user, - Some(org_id.clone()), - Some(new_member.uuid.clone()), - &org_name, - Some(org_email), - ) - .await + if let Err(e) = + mail::send_invite(&user, org_id.clone(), new_member.uuid.clone(), &org_name, Some(org_email)).await { // Upon error delete the user, invite and org member records when needed if user_created { diff --git a/src/auth.rs b/src/auth.rs index 2d5f79fc..e1dd71f6 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -194,16 +194,16 @@ pub struct InviteJwtClaims { pub sub: UserId, pub email: String, - pub org_id: Option, - pub member_id: Option, + pub org_id: OrganizationId, + pub member_id: MembershipId, pub invited_by_email: Option, } pub fn generate_invite_claims( user_id: UserId, email: String, - org_id: Option, - member_id: Option, + org_id: OrganizationId, + member_id: MembershipId, invited_by_email: Option, ) -> InviteJwtClaims { let time_now = Utc::now(); diff --git a/src/mail.rs b/src/mail.rs index 68ab7413..410bac4b 100644 --- a/src/mail.rs +++ b/src/mail.rs @@ -259,8 +259,8 @@ pub async fn send_single_org_removed_from_org(address: &str, org_name: &str) -> pub async fn send_invite( user: &User, - org_id: Option, - member_id: Option, + org_id: OrganizationId, + member_id: MembershipId, org_name: &str, invited_by_email: Option, ) -> EmptyResult { @@ -272,22 +272,14 @@ pub async fn send_invite( invited_by_email, ); let invite_token = encode_jwt(&claims); - let org_id = match org_id { - Some(ref org_id) => org_id.as_ref(), - None => "_", - }; - let member_id = match member_id { - Some(ref member_id) => member_id.as_ref(), - None => "_", - }; let mut query = url::Url::parse("https://query.builder").unwrap(); { let mut query_params = query.query_pairs_mut(); query_params .append_pair("email", &user.email) .append_pair("organizationName", org_name) - .append_pair("organizationId", org_id) - .append_pair("organizationUserId", member_id) + .append_pair("organizationId", &org_id) + .append_pair("organizationUserId", &member_id) .append_pair("token", &invite_token); if user.private_key.is_some() { query_params.append_pair("orgUserHasExistingUser", "true");