Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve admin invite #5403

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/api/admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(())
}
Expand Down
125 changes: 56 additions & 69 deletions src/api/core/organizations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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()),
)
Expand Down Expand Up @@ -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?;
Expand Down Expand Up @@ -1138,72 +1132,71 @@ async fn accept_invite(
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"),
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 {
Copy link
Contributor

@Timshel Timshel Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to retrieve the user from the Header guard instead and check that the correct user is making the call.

Made a PR in case you think it makes more sense to keep the change separate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, wow. I've never noticed. Yeah, sounds reasonable.

Some(user) => {
Invitation::take(&claims.email, &mut conn).await;
let Some(user) = User::find_by_mail(&claims.email, &mut conn).await else {
err!("Invited user not found")
};
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(&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?;
}
}
Expand Down Expand Up @@ -1825,23 +1818,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();
Expand Down Expand Up @@ -2141,8 +2128,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()),
)
Expand Down
10 changes: 2 additions & 8 deletions src/api/core/public.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
16 changes: 4 additions & 12 deletions src/mail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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");
Expand Down