Delete role assignments when locally-managed users / groups are deleted

nexus/db-queries/src/db/datastore/project.rs

@@ -296,6 +296,12 @@ impl DataStore {
                         db_project.id().into_untyped_uuid(),
                     )
                     .await?;
+
+                    self.role_assignment_delete_by_resource_on_connection(
+                        &conn,
+                        authz_project,
+                    )
+                    .await?;
                     Ok(())
                 }
             })

nexus/db-queries/src/db/datastore/role.rs

@@ -16,6 +16,7 @@ use crate::db::model::IdentityType;
 use crate::db::model::RoleAssignment;
 use async_bb8_diesel::AsyncRunQueryDsl;
 use diesel::prelude::*;
+use diesel::result::Error as DieselError;
 use nexus_db_errors::ErrorHandler;
 use nexus_db_errors::TransactionError;
 use nexus_db_errors::public_error_from_diesel;
@@ -25,6 +26,7 @@ use nexus_types::external_api::policy;
 use omicron_common::api::external::Error;
 use omicron_common::api::external::ListResultVec;
 use omicron_common::bail_unless;
+use uuid::Uuid;
 
 impl DataStore {
     /// Load role assignments for built-in users and built-in roles into the
@@ -103,7 +105,51 @@ impl DataStore {
             .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
     }
 
-    /// Removes all existing externally-visble role assignments on
+    /// Delete all role assignments held by the given identity, on any
+    /// resource. Used when deleting users and groups so that deleted
+    /// identities don't linger in policies.
+    pub(super) async fn role_assignment_delete_by_identity_on_connection(
+        &self,
+        conn: &async_bb8_diesel::Connection<DbConnection>,
+        identity_type: IdentityType,
+        identity_id: Uuid,
+    ) -> Result<(), DieselError> {
+        use nexus_db_schema::schema::role_assignment::dsl;
+
+        diesel::delete(dsl::role_assignment)
+            .filter(dsl::identity_type.eq(identity_type))
+            .filter(dsl::identity_id.eq(identity_id))
+            .execute_async(conn)
+            .await?;
+
+        Ok(())
+    }
+
+    /// Delete all role assignments on the given resource, held by any
+    /// identity. Used when deleting resources that have policies (silos,
+    /// projects) so their assignments don't outlive them.
+    pub(super) async fn role_assignment_delete_by_resource_on_connection<
+        T: authz::ApiResourceWithRoles,
+    >(
+        &self,
+        conn: &async_bb8_diesel::Connection<DbConnection>,
+        authz_resource: &T,
+    ) -> Result<(), DieselError> {
+        use nexus_db_schema::schema::role_assignment::dsl;
+
+        diesel::delete(dsl::role_assignment)
+            .filter(
+                dsl::resource_type
+                    .eq(authz_resource.resource_type().to_string()),
+            )
+            .filter(dsl::resource_id.eq(authz_resource.resource_id()))
+            .execute_async(conn)
+            .await?;
+
+        Ok(())
+    }
+
+    /// Removes all existing externally-visible role assignments on
     /// `authz_resource` and adds those specified by `new_assignments`
     ///
     /// Role assignments for internal identities (e.g., built-in users) are not

nexus/db-queries/src/db/datastore/scim_provider_store.rs

@@ -507,15 +507,13 @@ impl<'a> CrdbScimProviderStore<'a> {
         }
 
         // Cleanup role assignment records for the deleted user
-        {
-            use nexus_db_schema::schema::role_assignment::dsl;
-
-            diesel::delete(dsl::role_assignment)
-                .filter(dsl::identity_type.eq(IdentityType::SiloUser))
-                .filter(dsl::identity_id.eq(to_db_typed_uuid(user_id)))
-                .execute_async(conn)
-                .await?;
-        }
+        self.datastore
+            .role_assignment_delete_by_identity_on_connection(
+                conn,
+                IdentityType::SiloUser,
+                user_id.into_untyped_uuid(),
+            )
+            .await?;
 
         // Finally we should cleanup all auth related things like sessions and
         // tokens.
@@ -1217,15 +1215,13 @@ impl<'a> CrdbScimProviderStore<'a> {
         }
 
         // Cleanup role assignment records for the deleted group
-        {
-            use nexus_db_schema::schema::role_assignment::dsl;
-
-            diesel::delete(dsl::role_assignment)
-                .filter(dsl::identity_id.eq(to_db_typed_uuid(group_id)))
-                .filter(dsl::identity_type.eq(IdentityType::SiloGroup))
-                .execute_async(conn)
-                .await?;
-        }
+        self.datastore
+            .role_assignment_delete_by_identity_on_connection(
+                conn,
+                IdentityType::SiloGroup,
+                group_id.into_untyped_uuid(),
+            )
+            .await?;
 
         Ok(true)
     }

nexus/db-queries/src/db/datastore/silo.rs

@@ -452,6 +452,7 @@ impl DataStore {
         opctx.authorize(authz::Action::Delete, authz_silo).await?;
 
         use nexus_db_schema::schema::project;
+        use nexus_db_schema::schema::role_assignment;
         use nexus_db_schema::schema::silo;
         use nexus_db_schema::schema::silo_group;
         use nexus_db_schema::schema::silo_group_membership;
@@ -511,6 +512,14 @@ impl DataStore {
                 self.silo_auth_settings_delete(opctx, &conn, &authz_silo)
                     .await?;
 
+                // Delete all role assignments on the silo, including those
+                // held by users from other silos (typically fleet admins).
+                // The count is bounded by MAX_ROLE_ASSIGNMENTS_PER_RESOURCE.
+                self.role_assignment_delete_by_resource_on_connection(
+                    &conn, authz_silo,
+                )
+                .await?;
+
                 self.virtual_provisioning_collection_delete_on_connection(
                     &opctx.log,
                     &conn,
@@ -561,6 +570,41 @@ impl DataStore {
             "deleted {} password hashes for silo {}", updated_rows, id
         );
 
+        // delete role assignments held by the silo's users
+        let updated_rows =
+            diesel::delete(role_assignment::dsl::role_assignment)
+                .filter(
+                    role_assignment::dsl::identity_type
+                        .eq(db::model::IdentityType::SiloUser),
+                )
+                .filter(
+                    role_assignment::dsl::identity_id.eq_any(
+                        silo_user::dsl::silo_user
+                            .filter(
+                                silo_user::dsl::silo_id
+                                    .eq(to_db_typed_uuid(id)),
+                            )
+                            .filter(silo_user::dsl::time_deleted.is_null())
+                            .filter(
+                                silo_user::dsl::user_provision_type
+                                    .eq(db_silo.user_provision_type),
+                            )
+                            .select(silo_user::dsl::id),
+                    ),
+                )
+                .execute_async(&*conn)
+                .await
+                .map_err(|e| {
+                    public_error_from_diesel(e, ErrorHandler::Server)
+                })?;
+
+        debug!(
+            opctx.log,
+            "deleted {} silo user role assignments for silo {}",
+            updated_rows,
+            id
+        );
+
         let updated_rows = diesel::update(silo_user::dsl::silo_user)
             .filter(silo_user::dsl::silo_id.eq(to_db_typed_uuid(id)))
             .filter(silo_user::dsl::time_deleted.is_null())
@@ -607,6 +651,41 @@ impl DataStore {
             "deleted {} silo group memberships for silo {}", updated_rows, id
         );
 
+        // delete role assignments held by the silo's groups
+        let updated_rows =
+            diesel::delete(role_assignment::dsl::role_assignment)
+                .filter(
+                    role_assignment::dsl::identity_type
+                        .eq(db::model::IdentityType::SiloGroup),
+                )
+                .filter(
+                    role_assignment::dsl::identity_id.eq_any(
+                        silo_group::dsl::silo_group
+                            .filter(
+                                silo_group::dsl::silo_id
+                                    .eq(to_db_typed_uuid(id)),
+                            )
+                            .filter(silo_group::dsl::time_deleted.is_null())
+                            .filter(
+                                silo_group::dsl::user_provision_type
+                                    .eq(db_silo.user_provision_type),
+                            )
+                            .select(silo_group::dsl::id),
+                    ),
+                )
+                .execute_async(&*conn)
+                .await
+                .map_err(|e| {
+                    public_error_from_diesel(e, ErrorHandler::Server)
+                })?;
+
+        debug!(
+            opctx.log,
+            "deleted {} silo group role assignments for silo {}",
+            updated_rows,
+            id
+        );
+
         // delete all silo groups
         let updated_rows = diesel::update(silo_group::dsl::silo_group)
             .filter(silo_group::dsl::silo_id.eq(to_db_typed_uuid(id)))

nexus/db-queries/src/db/datastore/silo_group.rs

@@ -20,13 +20,11 @@ use chrono::DateTime;
 use chrono::Utc;
 use diesel::prelude::*;
 use nexus_db_errors::ErrorHandler;
-use nexus_db_errors::TransactionError;
 use nexus_db_errors::public_error_from_diesel;
 use nexus_types::external_api::user;
 use nexus_types::identity::Asset;
 use omicron_common::api::external::CreateResult;
 use omicron_common::api::external::DataPageParams;
-use omicron_common::api::external::DeleteResult;
 use omicron_common::api::external::Error;
 use omicron_common::api::external::InternalContext;
 use omicron_common::api::external::ListResultVec;
@@ -680,71 +678,6 @@ impl DataStore {
             .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
     }
 
-    pub async fn silo_group_delete(
-        &self,
-        opctx: &OpContext,
-        authz_silo_group: &authz::SiloGroup,
-    ) -> DeleteResult {
-        opctx.authorize(authz::Action::Delete, authz_silo_group).await?;
-
-        #[derive(Debug, thiserror::Error)]
-        enum SiloDeleteError {
-            #[error("group {0} still has memberships")]
-            GroupStillHasMemberships(SiloGroupUuid),
-        }
-        type TxnError = TransactionError<SiloDeleteError>;
-
-        let group_id = authz_silo_group.id();
-        let conn = self.pool_connection_authorized(opctx).await?;
-
-        // Prefer to use "transaction_retry_wrapper"
-        self.transaction_non_retry_wrapper("silo_group_delete")
-            .transaction(&conn, |conn| async move {
-                use nexus_db_schema::schema::silo_group_membership;
-
-                // Don't delete groups that still have memberships
-                let group_memberships =
-                    silo_group_membership::dsl::silo_group_membership
-                        .filter(
-                            silo_group_membership::dsl::silo_group_id
-                                .eq(to_db_typed_uuid(group_id)),
-                        )
-                        .select(SiloGroupMembership::as_returning())
-                        .limit(1)
-                        .load_async(&conn)
-                        .await?;
-
-                if !group_memberships.is_empty() {
-                    return Err(TxnError::CustomError(
-                        SiloDeleteError::GroupStillHasMemberships(group_id),
-                    ));
-                }
-
-                // Delete silo group
-                use nexus_db_schema::schema::silo_group::dsl;
-                diesel::update(dsl::silo_group)
-                    .filter(dsl::id.eq(to_db_typed_uuid(group_id)))
-                    .filter(dsl::time_deleted.is_null())
-                    .set(dsl::time_deleted.eq(Utc::now()))
-                    .execute_async(&conn)
-                    .await?;
-
-                Ok(())
-            })
-            .await
-            .map_err(|e| match e {
-                TxnError::CustomError(
-                    SiloDeleteError::GroupStillHasMemberships(id),
-                ) => Error::invalid_request(&format!(
-                    "group {0} still has memberships",
-                    id
-                )),
-                TxnError::Database(error) => {
-                    public_error_from_diesel(error, ErrorHandler::Server)
-                }
-            })
-    }
-
     pub async fn silo_groups_list_by_id(
         &self,
         opctx: &OpContext,

nexus/db-queries/src/db/datastore/silo_user.rs

@@ -550,6 +550,14 @@ impl DataStore {
                         .await?;
                 }
 
+                // Delete role assignments.
+                self.role_assignment_delete_by_identity_on_connection(
+                    &conn,
+                    model::IdentityType::SiloUser,
+                    authz_silo_user_id.into_untyped_uuid(),
+                )
+                .await?;
+
                 // Delete ssh keys.
                 {
                     use nexus_db_schema::schema::ssh_key::dsl;

nexus/db-schema/src/schema.rs

@@ -921,6 +921,8 @@ allow_tables_to_appear_in_same_query!(
 allow_tables_to_appear_in_same_query!(silo_group, silo);
 allow_tables_to_appear_in_same_query!(silo_user, silo);
 allow_tables_to_appear_in_same_query!(role_assignment, silo_group_membership);
+allow_tables_to_appear_in_same_query!(role_assignment, silo_user);
+allow_tables_to_appear_in_same_query!(role_assignment, silo_group);
 
 table! {
     identity_provider (silo_id, id) {