Browse Source

improve admin invite (#5403)

* check for admin invite

* refactor the invitation logic

* cleanup check for undefined token

* prevent wrong user from accepting invitation
Stefan Melmuk 9 months ago
parent
commit
ef2695de0c
5 changed files with 76 additions and 95 deletions
  1. 7 2
      src/api/admin.rs
  2. 59 69
      src/api/core/organizations.rs
  3. 2 8
      src/api/core/public.rs
  4. 4 4
      src/auth.rs
  5. 4 12
      src/mail.rs

+ 7 - 2
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<InviteData>, _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(())
         }

+ 59 - 69
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<AcceptData>,
+    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)?;
 
+    // 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")
+    }
+
     // 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"),
+    if !claims.member_id.eq(&member_id) {
+        err!("Error accepting the invitation", "Claim does not match the member_id")
     }
 
-    match User::find_by_mail(&claims.email, &mut conn).await {
-        Some(user) => {
-            Invitation::take(&claims.email, &mut conn).await;
+    let member = &claims.member_id;
+    let org = &claims.org_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")
-                };
+    Invitation::take(&claims.email, &mut conn).await;
 
-                if member.status != MembershipStatus::Invited as i32 {
-                    err!("User already accepted the invitation")
-                }
+    // 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")
+        };
 
-                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.");
-                }
+        if member.status != MembershipStatus::Invited as i32 {
+            err!("User already accepted 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");
-                        }
+        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/<org_id>/policies/token?<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<Value> = policies.iter().map(OrgPolicy::to_json).collect();
@@ -2141,8 +2131,8 @@ async fn import(org_id: OrganizationId, data: Json<OrgImportData>, 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()),
                     )

+ 2 - 8
src/api/core/public.rs

@@ -119,14 +119,8 @@ async fn ldap_import(data: Json<OrgImportData>, 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 {

+ 4 - 4
src/auth.rs

@@ -194,16 +194,16 @@ pub struct InviteJwtClaims {
     pub sub: UserId,
 
     pub email: String,
-    pub org_id: Option<OrganizationId>,
-    pub member_id: Option<MembershipId>,
+    pub org_id: OrganizationId,
+    pub member_id: MembershipId,
     pub invited_by_email: Option<String>,
 }
 
 pub fn generate_invite_claims(
     user_id: UserId,
     email: String,
-    org_id: Option<OrganizationId>,
-    member_id: Option<MembershipId>,
+    org_id: OrganizationId,
+    member_id: MembershipId,
     invited_by_email: Option<String>,
 ) -> InviteJwtClaims {
     let time_now = Utc::now();

+ 4 - 12
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<OrganizationId>,
-    member_id: Option<MembershipId>,
+    org_id: OrganizationId,
+    member_id: MembershipId,
     org_name: &str,
     invited_by_email: Option<String>,
 ) -> 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");