runtime: Unregister user if not an org member

CHANGELOG.md

@@ -6,6 +6,7 @@ Upcoming
 
 ### Breaking changes
 
+* Only unregistered a user if not a member of any org
 * Tx author needs to have an associated registered user to operate on Orgs
 * `Org::members` is now `Vec<Id>`
 * client: `Client::new_emulator()` now returns a pair of a `Client` and

runtime-tests/tests/user_registration.rs

@@ -129,35 +129,93 @@ async fn unregister_user() {
 }
 
 #[async_std::test]
-async fn unregister_user_with_invalid_sender() {
+async fn unregister_user_member_of_an_org() {
     let (client, _) = Client::new_emulator();
-    let alice = key_pair_from_string("Alice");
-    let register_user_message = random_register_user_message();
+    let (author, user_id) = key_pair_with_associated_user(&client).await;
 
-    // Reistration.
-    let tx_included = submit_ok(&client, &alice, register_user_message.clone()).await;
-    assert!(tx_included
-        .events
-        .contains(&RegistryEvent::UserRegistered(register_user_message.user_id.clone()).into()));
-    assert!(tx_included.result.is_ok());
+    // Have user registering an org, which sets the associated user as its single member.
+    let register_org = random_register_org_message();
+    submit_ok(&client, &author, register_org.clone()).await;
+    let org = client.get_org(register_org.org_id).await.unwrap().unwrap();
+    assert_eq!(org.members, vec![user_id.clone()]);
+
+    // Unregistration.
+    let initial_balance = client.free_balance(&author.public()).await.unwrap();
+
+    let unregister_user_message = message::UnregisterUser {
+        user_id: user_id.clone(),
+    };
+    let random_fee = random_balance();
+    let tx_unregister_applied = submit_ok_with_fee(
+        &client,
+        &author,
+        unregister_user_message.clone(),
+        random_fee,
+    )
+    .await;
+    assert_eq!(
+        tx_unregister_applied.result,
+        Err(RegistryError::UnregisterableUser.into())
+    );
     assert!(
-        user_exists(&client, register_user_message.user_id.clone()).await,
-        "User not found in users list",
+        user_exists(&client, unregister_user_message.user_id.clone()).await,
+        "The user was expected to still exist"
+    );
+    assert_eq!(
+        client.free_balance(&author.public()).await.unwrap(),
+        initial_balance - random_fee,
+        "The tx fee was not charged properly."
     );
+}
 
-    // Invalid unregistration.
-    let bad_actor = key_pair_from_string("BadActor");
-    // The bad actor needs funds to submit transactions.
-    transfer(&client, &alice, bad_actor.public(), 1000).await;
+#[async_std::test]
+async fn unregister_user_with_invalid_sender() {
+    let (client, _) = Client::new_emulator();
+    let (_, user_id) = key_pair_with_associated_user(&client).await;
 
+    // Invalid unregistration.
+    let (bad_actor, _) = key_pair_with_associated_user(&client).await;
     let unregister_user_message = message::UnregisterUser {
-        user_id: register_user_message.user_id.clone(),
+        user_id: user_id.clone(),
     };
     let tx_unregister_applied =
         submit_ok(&client, &bad_actor, unregister_user_message.clone()).await;
-    assert!(tx_unregister_applied.result.is_err());
+
+    assert_eq!(
+        tx_unregister_applied.result,
+        Err(RegistryError::InsufficientSenderPermissions.into())
+    );
     assert!(
-        user_exists(&client, register_user_message.user_id.clone()).await,
+        user_exists(&client, user_id.clone()).await,
         "The user was expected to exist"
     );
 }
+
+#[async_std::test]
+async fn unregister_user_with_no_associated_user() {
+    let (client, _) = Client::new_emulator();
+    let alice = key_pair_from_string("Alice");
+    let initial_balance = client.free_balance(&alice.public()).await.unwrap();
+    let unregister_user_message = message::UnregisterUser {
+        user_id: random_id(),
+    };
+
+    assert!(
+        !user_exists(&client, unregister_user_message.user_id.clone()).await,
+        "User should not exist",
+    );
+
+    let random_fee = random_balance();
+    let tx_unregister_applied =
+        submit_ok_with_fee(&client, &alice, unregister_user_message.clone(), random_fee).await;
+    assert_eq!(
+        tx_unregister_applied.result,
+        Err(RegistryError::InexistentUser.into())
+    );
+
+    assert_eq!(
+        client.free_balance(&alice.public()).await.unwrap(),
+        initial_balance - random_fee,
+        "The tx fee was not charged properly."
+    );
+}

runtime/src/registry.rs

@@ -274,25 +274,19 @@ decl_module! {
 
         #[weight = SimpleDispatchInfo::InsecureFreeNormal]
         pub fn unregister_user(origin, message: message::UnregisterUser) -> DispatchResult {
-            fn can_be_unregistered(user: state::User, sender: AccountId) -> bool {
-                user.account_id == sender && user.projects.is_empty()
-            }
-
             let sender = ensure_signed(origin)?;
+            let sender_user = get_user_with_account(sender).ok_or(RegistryError::InexistentUser)?;
 
-            match store::Users::get(message.user_id.clone()) {
-                None => Err(RegistryError::InexistentUser.into()),
-                Some(user) => {
-                    if can_be_unregistered(user, sender) {
-                        store::Users::remove(message.user_id.clone());
-                        Self::deposit_event(Event::UserUnregistered(message.user_id));
-                        Ok(())
-                    }
-                    else {
-                        Err(RegistryError::UnregisterableUser.into())
-                    }
-                }
+            if sender_user.id != message.user_id {
+                return Err(RegistryError::InsufficientSenderPermissions.into());
+            }
+            if find_org(|org| org.members.contains(&sender_user.id)).is_some() {
+                return Err(RegistryError::UnregisterableUser.into());
             }
+
+            store::Users::remove(message.user_id.clone());
+            Self::deposit_event(Event::UserUnregistered(message.user_id));
+            Ok(())
         }
 
         #[weight = SimpleDispatchInfo::InsecureFreeNormal]
@@ -424,6 +418,12 @@ pub fn get_user_with_account(account_id: AccountId) -> Option<User> {
         .map(|(id, user)| User::new(id, user))
 }
 
+pub fn find_org(predicate: impl Fn(&state::Org) -> bool) -> Option<state::Org> {
+    store::Orgs::iter()
+        .find(|(_, org)| predicate(org))
+        .map(|(_, org)| org)
+}
+
 /// Check whether the user associated with the given account_id is a member of the given org.
 /// Return false if the account doesn't have an associated user or if said user is not a member
 /// of the org.