Close the emergency access gaps of the auto confirm policy

Confirming a member into an organization which confirms automatically now drops
the emergency access of that member. Enabling the policy only cleaned up the
members present at that time, so anybody who joined afterwards kept a grantee
which could take over the account of a member nobody ever vetted and reach the
organization vault through it. Bitwarden drops these on every confirmation,
manual ones included, so this does the same.

Enabling the policy no longer touches invited members. An invitation is created
by an admin alone, without any consent of the invited user, so it must not be
able to delete the emergency access of an account that never joined the
organization.

The validation and the cleanup now only run on the step from disabled to
enabled. The web vault saves a policy on every edit and re-running the cleanup
kept wiping emergency access that members created in the meantime. The cleanup
also moved behind the save of the policy so that a failed save can no longer
destroy data for nothing.

While here: only notify members which may actually confirm, `AdminHeaders`
rejects the managers that `find_confirmed_and_manage_all_by_org` also returns,
and stop unwrapping the client supplied id in the manual bulk confirm.

The referenced Bitwarden sources were pinned to a commit which predates the
feature and returned 404, they now point at one which contains them.

src/api/core/mod.rs

@@ -293,7 +293,13 @@ pub async fn notify_pending_auto_confirm(member: &Membership, conn: &DbConn, nt:
         return;
     }
 
-    for admin in Membership::find_confirmed_and_manage_all_by_org(&member.org_uuid, conn).await {
+    // Confirming requires `AdminHeaders`, so skip the managers this also returns. They could not act on
+    // the notification anyway and would only run into a rejected request.
+    for admin in Membership::find_confirmed_and_manage_all_by_org(&member.org_uuid, conn)
+        .await
+        .into_iter()
+        .filter(|m| m.atype >= MembershipType::Admin)
+    {
         nt.send_auto_confirm_member(&admin.user_uuid, &member.org_uuid, &member.uuid, &member.user_uuid).await;
     }
 }

src/api/core/organizations.rs

@@ -1388,7 +1388,11 @@ async fn bulk_confirm_invite(
     match data.keys {
         Some(keys) => {
             for invite in keys {
-                let member_id = invite.id.unwrap();
+                // Never unwrap the id, this is client supplied and a missing one must not take the request down
+                let Some(member_id) = invite.id else {
+                    error!("Ignoring a bulk confirm entry without a member id");
+                    continue;
+                };
                 let user_key = invite.key.unwrap_or_default();
                 let err_msg = match confirm_invite_impl(&org_id, &member_id, &user_key, &headers, &conn, &nt).await {
                     Ok(()) => String::new(),
@@ -1475,6 +1479,16 @@ async fn confirm_member(
     // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, admin::update_membership_type
     OrgPolicy::check_user_allowed(&member_to_confirm, "confirm", conn).await?;
 
+    // An organization which confirms its members automatically does not tolerate emergency access: the
+    // grantee could take over the account of a member that nobody ever vetted and reach the organization
+    // vault through it. Enabling the policy drops the grants of the members present at that time, this
+    // covers the member which brings one along when it joins afterwards. Bitwarden does the same, and
+    // like there it applies to the manual confirmation as well.
+    // https://github.com/bitwarden/server/blob/b3d1eb9a7854322f106efa55c191c1a4da9f8645/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs
+    if OrgPolicy::is_auto_confirm_enabled(&org_id, conn).await {
+        EmergencyAccess::delete_all_by_user(&member_to_confirm.user_uuid, conn).await?;
+    }
+
     log_event(
         EventType::OrganizationUserConfirmed as i32,
         &member_to_confirm.uuid,
@@ -2280,8 +2294,14 @@ async fn put_policy(
 
     // The automatic user confirmation policy hands out organization access without anybody looking at it,
     // so it needs to be allowed by the server first and it requires the Single Org policy on top.
-    // https://github.com/bitwarden/server/blob/9ebe16587175b1c0e9208f84397bb75d0d595510/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/AutomaticUserConfirmationPolicyEventHandler.cs
+    // https://github.com/bitwarden/server/blob/b3d1eb9a7854322f106efa55c191c1a4da9f8645/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/AutomaticUserConfirmationPolicyEventHandler.cs
-    if pol_type_enum == OrgPolicyType::AutomaticUserConfirmation && data.enabled {
+    let auto_confirm_turned_on = if pol_type_enum == OrgPolicyType::AutomaticUserConfirmation
+        && data.enabled
+        // Only the step from disabled to enabled validates and has side effects. The web vault saves a
+        // policy on every edit, and re-running the below on an already enabled policy would keep wiping
+        // emergency access that members created in the meantime. Bitwarden guards this the same way.
+        && !OrgPolicy::is_auto_confirm_enabled(&org_id, &conn).await
+    {
         if !CONFIG.org_auto_confirm_enabled() {
             err!("Automatic user confirmation is not enabled on this server.")
         }
@@ -2299,8 +2319,7 @@ async fn put_policy(
         // Every member has to be compliant already. Contrary to the Single Org policy below we do not revoke
         // the members that are not, because this policy also applies to owners and admins and revoking those
         // could lock the organization out of itself.
-        let members = Membership::find_by_org(&org_id, &conn).await;
+        for member in Membership::find_by_org(&org_id, &conn).await {
-        for member in &members {
             if member.status != MembershipStatus::Invited as i32
                 && Membership::count_accepted_and_confirmed_by_user(&member.user_uuid, &org_id, &conn).await > 0
             {
@@ -2308,14 +2327,10 @@ async fn put_policy(
             }
         }
 
-        // Emergency access would hand the account of a member to somebody outside of the control of this
+        true
-        // organization, which defeats the point of vetting members. Bitwarden drops these grants when the
+    } else {
-        // policy is turned on, and blocks new ones while it is on (see `emergency_access.rs`).
+        false
-        for member in &members {
+    };
-            info!("Removing emergency access of {} because automatic user confirmation was enabled", member.user_uuid);
-            EmergencyAccess::delete_all_by_user(&member.user_uuid, &conn).await?;
-        }
-    }
 
     // Also prevent the Single Org policy to be disabled while automatic user confirmation depends on it
     if pol_type_enum == OrgPolicyType::SingleOrg
@@ -2381,6 +2396,25 @@ async fn put_policy(
     policy.data = serde_json::to_string(&data.data)?;
     policy.save(&conn).await?;
 
+    // Emergency access would hand the account of a member to somebody outside of the control of this
+    // organization, which defeats the point of vetting members. Bitwarden drops these grants when the
+    // policy is turned on, and blocks new ones while it is on (see `emergency_access.rs`).
+    // This runs after the policy is stored so that a failed save can not destroy data for nothing, and it
+    // skips invited members on purpose: an invitation is created by an admin without any consent of the
+    // invited user, so it must never be able to delete data of an account that never joined.
+    if auto_confirm_turned_on {
+        for member in Membership::find_by_org(&org_id, &conn).await {
+            if member.status == MembershipStatus::Invited as i32 {
+                continue;
+            }
+            info!(
+                "Removing emergency access of {} because automatic user confirmation was enabled for {org_id}",
+                member.user_uuid
+            );
+            EmergencyAccess::delete_all_by_user(&member.user_uuid, &conn).await?;
+        }
+    }
+
     log_event(
         EventType::PolicyUpdated as i32,
         policy.uuid.as_ref(),

src/db/models/org_policy.rs

@@ -283,7 +283,7 @@ impl OrgPolicy {
     /// Returns true if the user is a member of an organization other than `exclude_org_uuid` which has the
     /// automatic user confirmation policy enabled. Contrary to `is_applicable_to_user` this does not exempt
     /// owners and admins, the policy applies to every role and every status.
-    /// https://github.com/bitwarden/server/blob/9ebe16587175b1c0e9208f84397bb75d0d595510/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/AutomaticUserConfirmationPolicyRequirement.cs
+    /// https://github.com/bitwarden/server/blob/b3d1eb9a7854322f106efa55c191c1a4da9f8645/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/AutomaticUserConfirmationPolicyRequirement.cs
     pub async fn auto_confirm_enabled_for_other_org(
         user_uuid: &UserId,
         exclude_org_uuid: &OrganizationId,
@@ -360,7 +360,7 @@ impl OrgPolicy {
         // The automatic user confirmation policy is a stricter variant of the SingleOrg policy, it does not
         // exempt owners and admins and it applies to every status. Therefore it is checked outside of the
         // block above.
-        // https://github.com/bitwarden/server/blob/9ebe16587175b1c0e9208f84397bb75d0d595510/src/Core/AdminConsole/OrganizationFeatures/Policies/Enforcement/AutoConfirm/AutomaticUserConfirmationPolicyEnforcementHandler.cs
+        // https://github.com/bitwarden/server/blob/b3d1eb9a7854322f106efa55c191c1a4da9f8645/src/Core/AdminConsole/OrganizationFeatures/Policies/Enforcement/AutoConfirm/AutomaticUserConfirmationPolicyEnforcementHandler.cs
         if Self::auto_confirm_enabled_for_other_org(&m.user_uuid, &m.org_uuid, conn).await {
             err!(format!(
                 "Cannot {} because another organization confirms its members automatically and forbids other memberships (membership {})",