Add a built-in remove_user proposal

src/node/genesis_gen.h

@@ -215,6 +215,24 @@ namespace ccf
       return id;
     }
 
+    bool remove_user(UserId user_id)
+    {
+      auto [u, uc] = tx.get_view(tables.users, tables.user_certs);
+
+      auto user_info = u->get(user_id);
+      if (!user_info.has_value())
+      {
+        return false;
+      }
+
+      auto pem = tls::Pem(user_info.value().cert);
+      auto user_cert_der = tls::make_verifier(pem)->der_cert_data();
+
+      u->remove(user_id);
+      uc->remove(user_cert_der);
+      return true;
+    }
+
     auto add_node(const NodeInfo& node_info)
     {
       auto node_id =

src/node/rpc/member_frontend.h

@@ -176,7 +176,6 @@ namespace ccf
 
            return true;
          }},
-        // add a new user
         {"new_user",
          [this](ObjectId proposal_id, kv::Tx& tx, const nlohmann::json& args) {
            const auto pem_cert = args.get<tls::Pem>();
@@ -186,6 +185,20 @@ namespace ccf
 
            return true;
          }},
+        {"remove_user",
+         [this](ObjectId proposal_id, kv::Tx& tx, const nlohmann::json& args) {
+           const UserId user_id = args;
+
+           GenesisGenerator g(this->network, tx);
+           auto r = g.remove_user(user_id);
+           if (!r)
+           {
+             LOG_FAIL_FMT(
+               "Proposal {}: {} is not a valid user ID", proposal_id, user_id);
+           }
+
+           return r;
+         }},
         {"set_user_data",
          [this](ObjectId proposal_id, kv::Tx& tx, const nlohmann::json& args) {
            const auto parsed = args.get<SetUserData>();

src/node/rpc/test/member_voting_test.cpp

@@ -1186,7 +1186,7 @@ DOCTEST_TEST_CASE("Vetoed proposal gets rejected")
   }
 }
 
-DOCTEST_TEST_CASE("Add user via proposed call")
+DOCTEST_TEST_CASE("Add and remove user via proposed calls")
 {
   NetworkState network;
   network.tables->set_encryptor(encryptor);
@@ -1204,27 +1204,57 @@ DOCTEST_TEST_CASE("Add user via proposed call")
   MemberRpcFrontend frontend(network, node, share_manager);
   frontend.open();
 
-  Script proposal(R"xxx(
-    tables, user_cert = ...
-      return Calls:call("new_user", user_cert)
-    )xxx");
+  ccf::Cert user_der;
 
-  const auto propose =
-    create_signed_request(Propose::In{proposal, user_cert}, "propose", kp);
+  {
+    DOCTEST_INFO("Add user");
 
-  const auto r = parse_response_body<Propose::Out>(
-    frontend_process(frontend, propose, member_cert));
-  DOCTEST_CHECK(r.state == ProposalState::ACCEPTED);
-  DOCTEST_CHECK(r.proposal_id == 0);
+    Script proposal(R"xxx(
+      tables, user_cert = ...
+        return Calls:call("new_user", user_cert)
+      )xxx");
+
+    const vector<uint8_t> user_cert = kp->self_sign("CN=new user");
+    const auto propose =
+      create_signed_request(Propose::In{proposal, user_cert}, "propose", kp);
 
-  kv::Tx tx1;
-  const auto uid = tx1.get_view(network.values)->get(ValueIds::NEXT_USER_ID);
-  DOCTEST_CHECK(uid);
-  DOCTEST_CHECK(*uid == 1);
-  const auto uid1 = tx1.get_view(network.user_certs)
-                      ->get(tls::make_verifier(user_cert)->der_cert_data());
-  DOCTEST_CHECK(uid1);
-  DOCTEST_CHECK(*uid1 == 0);
+    const auto r = parse_response_body<Propose::Out>(
+      frontend_process(frontend, propose, member_cert));
+    DOCTEST_CHECK(r.state == ProposalState::ACCEPTED);
+    DOCTEST_CHECK(r.proposal_id == 0);
+
+    kv::Tx tx1;
+    const auto uid = tx1.get_view(network.values)->get(ValueIds::NEXT_USER_ID);
+    DOCTEST_CHECK(uid);
+    DOCTEST_CHECK(*uid == 1);
+    user_der = tls::make_verifier(user_cert)->der_cert_data();
+    const auto uid1 = tx1.get_view(network.user_certs)->get(user_der);
+    DOCTEST_CHECK(uid1);
+    DOCTEST_CHECK(*uid1 == 0);
+  }
+
+  {
+    DOCTEST_INFO("Remove user");
+
+    Script proposal(R"xxx(
+      tables, user_id = ...
+        return Calls:call("remove_user", user_id)
+      )xxx");
+
+    const auto propose =
+      create_signed_request(Propose::In{proposal, 0}, "propose", kp);
+
+    const auto r = parse_response_body<Propose::Out>(
+      frontend_process(frontend, propose, member_cert));
+    DOCTEST_CHECK(r.state == ProposalState::ACCEPTED);
+    DOCTEST_CHECK(r.proposal_id == 1);
+
+    kv::Tx tx1;
+    auto user = tx1.get_view(network.users)->get(0);
+    DOCTEST_CHECK(!user.has_value());
+    auto user_cert = tx1.get_view(network.user_certs)->get(user_der);
+    DOCTEST_CHECK(!user_cert.has_value());
+  }
 }
 
 DOCTEST_TEST_CASE("Passing members ballot with operator")

tests/governance.py

@@ -9,54 +9,93 @@
 import infra.notification
 import infra.net
 import infra.e2e_args
+import suite.test_requirements as reqs
+import infra.logging_app as app
 
 from loguru import logger as LOG
 
 
+@reqs.description("Test quotes")
+@reqs.supports_methods("quote", "quotes")
+def test_quote(network, args, notifications_queue=None, verify=True):
+    primary, _ = network.find_nodes()
+    with primary.client() as c:
+        oed = subprocess.run(
+            [
+                args.oesign,
+                "dump",
+                "-e",
+                infra.path.build_lib_path(args.package, args.enclave_type),
+            ],
+            capture_output=True,
+            check=True,
+        )
+        lines = [
+            line
+            for line in oed.stdout.decode().split(os.linesep)
+            if line.startswith("mrenclave=")
+        ]
+        expected_mrenclave = lines[0].strip().split("=")[1]
+
+        r = c.get("/node/quote")
+        quotes = r.result["quotes"]
+        assert len(quotes) == 1
+        primary_quote = quotes[0]
+        assert primary_quote["node_id"] == 0
+        primary_mrenclave = primary_quote["mrenclave"]
+        assert primary_mrenclave == expected_mrenclave, (
+            primary_mrenclave,
+            expected_mrenclave,
+        )
+
+        r = c.get("/node/quotes")
+        quotes = r.result["quotes"]
+        assert len(quotes) == len(network.find_nodes())
+        for quote in quotes:
+            mrenclave = quote["mrenclave"]
+            assert mrenclave == expected_mrenclave, (mrenclave, expected_mrenclave)
+
+    return network
+
+
+@reqs.description("Add user, remove user, add user back")
+@reqs.supports_methods("log/private")
+def test_user(network, args, notifications_queue=None, verify=True):
+    primary, _ = network.find_nodes()
+    new_user_id = 3
+    network.create_users([new_user_id], args.participants_curve)
+    network.consortium.add_user(primary, new_user_id)
+    txs = app.LoggingTxs(notifications_queue=notifications_queue, user_id=3)
+    txs.issue(
+        network=network, number_txs=1, consensus=args.consensus,
+    )
+    if verify:
+        txs.verify(network)
+    network.consortium.remove_user(primary, new_user_id)
+    with primary.client(f"user{new_user_id}") as c:
+        r = c.get("/app/log/private")
+        assert r.status == 403
+    return network
+
+
 def run(args):
-    hosts = ["localhost"] * (4 if args.consensus == "pbft" else 2)
-
-    with infra.ccf.network(
-        hosts, args.binary_dir, args.debug_nodes, args.perf_nodes, pdb=args.pdb
-    ) as network:
-        network.start_and_join(args)
-        primary, _ = network.find_nodes()
-
-        with primary.client() as mc:
-            oed = subprocess.run(
-                [
-                    args.oesign,
-                    "dump",
-                    "-e",
-                    infra.path.build_lib_path(args.package, args.enclave_type),
-                ],
-                capture_output=True,
-                check=True,
-            )
-            lines = [
-                line
-                for line in oed.stdout.decode().split(os.linesep)
-                if line.startswith("mrenclave=")
-            ]
-            expected_mrenclave = lines[0].strip().split("=")[1]
-
-            r = mc.get("/node/quote")
-            quotes = r.result["quotes"]
-            assert len(quotes) == 1
-            primary_quote = quotes[0]
-            assert primary_quote["node_id"] == 0
-            primary_mrenclave = primary_quote["mrenclave"]
-            assert primary_mrenclave == expected_mrenclave, (
-                primary_mrenclave,
-                expected_mrenclave,
-            )
-
-            r = mc.get("/node/quotes")
-            quotes = r.result["quotes"]
-            assert len(quotes) == len(hosts)
-            for quote in quotes:
-                mrenclave = quote["mrenclave"]
-                assert mrenclave == expected_mrenclave, (mrenclave, expected_mrenclave)
+    hosts = ["localhost"] * (3 if args.consensus == "pbft" else 2)
+
+    with infra.notification.notification_server(args.notify_server) as notifications:
+        # Lua apps do not support notifications
+        # https://github.com/microsoft/CCF/issues/415
+        notifications_queue = (
+            notifications.get_queue()
+            if (args.package == "liblogging" and args.consensus == "raft")
+            else None
+        )
+
+        with infra.ccf.network(
+            hosts, args.binary_dir, args.debug_nodes, args.perf_nodes, pdb=args.pdb
+        ) as network:
+            network.start_and_join(args)
+            network = test_quote(network, args, notifications_queue)
+            network = test_user(network, args, notifications_queue)
 
 
 if __name__ == "__main__":
@@ -72,5 +111,12 @@ def add(parser):
         LOG.warning("This test can only run in real enclaves, skipping")
         sys.exit(0)
 
+    notify_server_host = "localhost"
+    args.notify_server = (
+        notify_server_host
+        + ":"
+        + str(infra.net.probably_free_local_port(notify_server_host))
+    )
+
     args.package = "liblogging"
     run(args)

tests/infra/consortium.py

@@ -254,15 +254,24 @@ def update_recovery_shares(self, remote_node):
         proposal = self.get_any_active_member().propose(remote_node, proposal_body)
         return self.vote_using_majority(remote_node, proposal)
 
+    def add_user(self, remote_node, user_id):
+        user_cert = []
+        proposal, vote = infra.proposal_generator.new_user(
+            os.path.join(self.common_dir, f"user{user_id}_cert.pem")
+        )
+
+        proposal = self.get_any_active_member().propose(remote_node, proposal)
+        return self.vote_using_majority(remote_node, proposal)
+
     def add_users(self, remote_node, users):
         for u in users:
-            user_cert = []
-            proposal, vote = infra.proposal_generator.new_user(
-                os.path.join(self.common_dir, f"user{u}_cert.pem")
-            )
+            self.add_user(remote_node, u)
+
+    def remove_user(self, remote_node, user_id):
+        proposal, vote = infra.proposal_generator.remove_user(user_id)
 
-            proposal = self.get_any_active_member().propose(remote_node, proposal)
-            self.vote_using_majority(remote_node, proposal)
+        proposal = self.get_any_active_member().propose(remote_node, proposal)
+        self.vote_using_majority(remote_node, proposal)
 
     def set_lua_app(self, remote_node, app_script_path):
         proposal_body, vote = infra.proposal_generator.set_lua_app(app_script_path)

tests/infra/logging_app.py

@@ -44,14 +44,13 @@ def test_run_txs(
 
 
 class LoggingTxs:
-    def __init__(
-        self, notifications_queue=None,
-    ):
+    def __init__(self, notifications_queue=None, user_id=0):
         self.pub = {}
         self.priv = {}
         self.next_pub_index = 1
         self.next_priv_index = 1
         self.notifications_queue = notifications_queue
+        self.user = f"user{user_id}"
 
     def issue(
         self,
@@ -92,7 +91,7 @@ def issue_on_node(
             check_commit = infra.checker.Checker(mc)
             check_commit_n = infra.checker.Checker(mc, self.notifications_queue)
 
-            with remote_node.client("user0") as uc:
+            with remote_node.client(self.user) as uc:
                 for _ in range(number_txs):
                     end_time = time.time() + timeout
                     while time.time() < end_time:
@@ -160,7 +159,7 @@ def _verify_tx(self, node, idx, priv=True, timeout=5):
 
         end_time = time.time() + timeout
         while time.time() < end_time:
-            with node.client("user0") as uc:
+            with node.client(self.user) as uc:
                 rep = uc.get(cmd, {"id": idx})
                 if rep.status == 404:
                     LOG.warning("User frontend is not yet opened")

tests/infra/proposal_generator.py

@@ -193,6 +193,11 @@ def new_user(user_cert_path, **kwargs):
     return build_proposal("new_user", user_cert, **kwargs)
 
 
+@cli_proposal
+def remove_user(user_id, **kwargs):
+    return build_proposal("remove_user", user_id, **kwargs)
+
+
 @cli_proposal
 def set_user_data(user_id, user_data, **kwargs):
     proposal_args = {"user_id": user_id, "user_data": user_data}