Implement RBAC message discarding filters

.github/workflows/build_cmake.yml

@@ -96,7 +96,7 @@
         shell: bash
         env:
           KEYCLOAK_URL: http://keycloak:8080
-          KEYCLOAK_REDIRECT_URI: http://localhost:8091
+          KEYCLOAK_REDIRECT_URI: http://localhost:8091/
           KEYCLOAK_ADMIN_PASSWORD: admin
         run: |
           if ! command -v jq >/dev/null 2>&1; then
@@ -111,7 +111,7 @@
         shell: bash
         env:
           KEYCLOAK_URL: http://keycloak:8080
-          KEYCLOAK_REDIRECT_URI: http://localhost:8091
+          KEYCLOAK_REDIRECT_URI: http://localhost:8091/
         # Execute tests defined by the CMake configuration. The coverage target runs the autodiscovered catch2 tests using
         # ctest and records the coverage using gcov
         # See https://cmake.org/cmake/help/latest/manual/ctest.1.html for more detail
@@ -123,7 +123,7 @@
         shell: bash
         env:
           KEYCLOAK_URL: http://keycloak:8080
-          KEYCLOAK_REDIRECT_URI: http://localhost:8091
+          KEYCLOAK_REDIRECT_URI: http://localhost:8091/
         # Execute tests defined by the CMake configuration. The coverage target runs the autodiscovered catch2 tests using
         # ctest and records the coverage using gcov
         # See https://cmake.org/cmake/help/latest/manual/ctest.1.html for more detail
@@ -138,7 +138,7 @@
         if: matrix.configurations.name == env.REFERENCE_CONFIG && matrix.cmake-build-type == 'Debug'
         working-directory: ${{ runner.workspace }}/build
         shell: bash
         run: bash <(curl -Ls https://coverage.codacy.com/get.sh) report -r coverage.xml --skip --project-token ${{ secrets.CODACY_PROJECT_TOKEN }}
 
       - name: Run sonar-scanner
         if: matrix.configurations.name == env.REFERENCE_CONFIG && matrix.cmake-build-type == 'Debug'

src/client/include/Client.hpp

@@ -244,7 +244,7 @@
     }
 
     void set(const URI<STRICT> & /*uri*/, std::string_view, const std::span<const std::byte> & /*request*/) override {
-        throw std::logic_error("get not implemented");
+        throw std::logic_error("set not implemented");
     }
 
     void subscribe(const URI<STRICT> &uri, std::string_view /*reqId*/) override {
@@ -399,18 +399,20 @@
                 _requests.erase(0); // todo: lookup correct subscription
             }
         }
-        sendCmd(cmd.topic, cmd.command, req_id, cmd.data);
+        sendCmd(cmd.topic, cmd.command, req_id, cmd.data, cmd.rbac);
     }
 
 private:
-    void sendCmd(const URI<STRICT> &uri, mdp::Command commandType, std::size_t req_id, IoBuffer data = {}) const {
+    void sendCmd(const URI<STRICT> &uri, mdp::Command commandType, std::size_t req_id, IoBuffer data = {}, IoBuffer rbac = {}) const {
         const bool        isSet = commandType == mdp::Command::Set;
         zmq::MessageFrame cmdType{ std::string{ static_cast<char>(commandType) } };
         cmdType.send(_control_socket_send, ZMQ_SNDMORE).assertSuccess();
         zmq::MessageFrame reqId{ std::to_string(req_id) };
         reqId.send(_control_socket_send, ZMQ_SNDMORE).assertSuccess();
         zmq::MessageFrame endpoint{ std::string(uri.str()) };
         endpoint.send(_control_socket_send, isSet ? ZMQ_SNDMORE : 0).assertSuccess();
+        zmq::MessageFrame rbacframe{ std::move(rbac) };
+        rbacframe.send(_control_socket_send, 0).assertSuccess();
         if (isSet) {
             zmq::MessageFrame dataframe{ std::move(data) };
             dataframe.send(_control_socket_send, 0).assertSuccess();
@@ -421,13 +423,19 @@
         zmq::MessageFrame cmd;
         zmq::MessageFrame reqId;
         zmq::MessageFrame endpoint;
+        zmq::MessageFrame rbac;
         while (cmd.receive(_control_socket_recv, ZMQ_DONTWAIT).isValid()) {
             if (!reqId.receive(_control_socket_recv, ZMQ_DONTWAIT).isValid()) {
                 throw std::logic_error("invalid request received: failure receiving message");
             }
             if (!endpoint.receive(_control_socket_recv, ZMQ_DONTWAIT).isValid()) {
                 throw std::logic_error("invalid request received: invalid message contents");
             }
+            if (!rbac.receive(_control_socket_recv, ZMQ_DONTWAIT).isValid()) {
+                throw std::logic_error("invalid request received: invalid rbac data");
+            }
+            auto        rbacdata = as_bytes(std::span(rbac.data().data(), rbac.data().size()));
+            IoBuffer    rbacData{ reinterpret_cast<const char *>(rbacdata.data()), rbacdata.size() };
             URI<STRICT> uri{ std::string(endpoint.data()) };
             auto       &client = getClient(uri);
             if (cmd.data().size() != 1) {

src/client/test/setup-keycloak.sh

@@ -7,7 +7,7 @@
 # For more information see: https://www.keycloak.org/getting-started/getting-started-docker
 
 URL="${KEYCLOAK_URL:-http://localhost:8090}"
-REDIRECT_URI="${KEYCLOAK_REDIRECT_URI:-http://localhost:8091}"
+REDIRECT_URI="${KEYCLOAK_REDIRECT_URI:-http://localhost:8091/}"
 ADMIN_PASSWORD="${KEYCLOAK_ADMIN_PASSWORD:-admin}"
 
 while ! curl -s "$URL" >/dev/null; do

src/majordomo/include/majordomo/Broker.hpp

@@ -280,6 +280,7 @@
     bool                                                    _connectedToDns    = false;
 
     const IoBuffer                                          _rbac              = IoBuffer("RBAC=ADMIN,abcdef12345");
+    std::function<void(BrokerMessage &)>                    _rbacHandler;
 
     std::atomic<bool>                                       _shutdownRequested = false;
 
@@ -422,6 +423,10 @@
         _subscriptionMatcher.addFilter<Filter>(key);
     }
 
+    void setRbacHandler(decltype(_rbacHandler) handler) {
+        _rbacHandler = std::move(handler);
+    }
+
     /**
      * Bind broker to endpoint, can call this multiple times. We use a single
      * socket for both clients and workers.
@@ -847,7 +852,7 @@
         }
     }
 
     void processClients() {
         for (auto &[senderId, client] : _clients) {
             if (client.requests.empty())
                 continue;
@@ -855,6 +860,10 @@
             auto clientMessage = std::move(client.requests.front());
             client.requests.pop_front();
 
+            if (_rbacHandler) {
+                _rbacHandler(clientMessage);
+            }
+
             if (auto service = bestMatchingService(clientMessage.serviceName)) {
                 if (service->internalHandler) {
                     auto reply = service->internalHandler(std::move(clientMessage));

src/majordomo/include/majordomo/Worker.hpp

@@ -98,6 +98,7 @@ class BasicWorker {
     std::shared_mutex                                        _notificationHandlersLock;
     const std::string                                        _notifyAddress;
     const IoBuffer                                           _defaultRbacToken = IoBuffer("RBAC=NONE,");
+    std::function<bool(const IoBuffer &rbac)>                _rbacPredicate;
 
 public:
     static constexpr std::string_view name = serviceName.data();
@@ -184,6 +185,10 @@ class BasicWorker {
         disconnect();
     }
 
+    void setRbacPredicate(decltype(_rbacPredicate) rbacPredicate) {
+        _rbacPredicate = std::move(rbacPredicate);
+    }
+
 protected:
     void setHandler(std::function<void(RequestContext &)> handler) {
         _handler = std::move(handler);
@@ -352,6 +357,12 @@ class BasicWorker {
         const auto clientRole = parse_rbac::role(request.rbac.asString());
         const auto permission = _permissionsByRole.at(clientRole, _defaultPermission);
 
+        if (_rbacPredicate && !_rbacPredicate(request.rbac)) {
+            auto errorReply  = replyFromRequest(request);
+            errorReply.error = "request rejected by RBAC handler";
+            return errorReply;
+        }
+
         if (request.command == mdp::Command::Get && !(permission == Permission::RW || permission == Permission::RO)) {
             auto errorReply  = replyFromRequest(request);
             errorReply.error = std::format("GET access denied to role '{}'", clientRole);

src/majordomo/test/rbac_tests.cpp

@@ -1,3 +1,5 @@
+#include <Client.hpp>
+#include <concepts/majordomo/helpers.hpp>
 #include <majordomo/Rbac.hpp>
 
 #include <catch2/catch.hpp>
@@ -30,3 +32,142 @@
     STATIC_REQUIRE2(parse_rbac::roleAndHash("RBAC=ADMIN,hash") == std::pair("ADMIN"sv, "hash"sv));
     STATIC_REQUIRE2(parse_rbac::hash("RBAC=ADMIN,hash,invalidHash") == "hash,invalidHash"); // TODO in java, this throws, should we throw/return "", too?
 }
+
+namespace opencmw {
+
+struct RbacContext {
+    int dummy;
+};
+
+struct RbacInput {
+    int dummy;
+};
+
+struct RbacOutput {
+    int i;
+};
+
+} // namespace opencmw
+
+ENABLE_REFLECTION_FOR(opencmw::RbacContext, dummy)
+ENABLE_REFLECTION_FOR(opencmw::RbacInput, dummy)
+ENABLE_REFLECTION_FOR(opencmw::RbacOutput, i)
+
+namespace {
+using RbacWorkerType = majordomo::Worker<"/rbac", opencmw::RbacContext, opencmw::RbacInput, opencmw::RbacOutput, majordomo::description<"Rbac test">>;
+};
+
+class RbacWorker : public RbacWorkerType {
+public:
+    template<typename BrokerType>
+    explicit RbacWorker(const BrokerType &broker) : RbacWorkerType(broker, {}) { init(); };
+
+private:
+    void init() {
+        RbacWorkerType::setCallback([this](const majordomo::RequestContext &rawCtx, const opencmw::RbacContext &context, const opencmw::RbacInput &in, opencmw::RbacContext &replyContext, opencmw::RbacOutput &out) {
+            out.i = 42;
+        });
+    };
+};
+
+TEST_CASE("RBAC basic predicate tests", "[rbac][predicate]") {
+    opencmw::majordomo::Broker<> broker{ "/Broker", {} };
+    RbacWorker                   worker{ broker };
+
+    worker.setRbacPredicate([](auto &) { return false; });
+
+    REQUIRE(broker.bind(opencmw::URI<>("mds://127.0.0.1:12345")));
+    REQUIRE(broker.bind(opencmw::URI<>("mdp://127.0.0.1:12346")));
+
+    RunInThread brokerThread(broker);
+    RunInThread workerThread(worker);
+
+    REQUIRE(waitUntilWorkerServiceAvailable(broker.context, worker));
+
+    std::vector<std::unique_ptr<opencmw::client::ClientBase>> clients;
+    clients.emplace_back(std::make_unique<opencmw::client::MDClientCtx>(broker.context, 20ms, ""));
+    opencmw::client::ClientContext client{ std::move(clients) };
+
+    opencmw::IoBuffer              inBuf;
+    opencmw::RbacInput             in;
+    opencmw::serialise<opencmw::YaS>(inBuf, in);
+
+    std::condition_variable cv;
+    std::mutex              m;
+    bool                    done = false;
+
+    client.set(opencmw::URI("mdp://127.0.0.1:12346/rbac"), [&](const mdp::Message &reply) {
+        REQUIRE(reply.error.size());
+
+        worker.setRbacPredicate([](auto& rb){
+                return true;});
+
+        opencmw::IoBuffer iBuf;
+        opencmw::serialise<opencmw::YaS>(iBuf, in);
+        client.set(opencmw::URI("mdp://127.0.0.1:12346/rbac"), [&](const mdp::Message &reply) {
+                auto outBuf = reply.data;
+                REQUIRE(reply.error.empty());
+                REQUIRE(outBuf.size());
+                opencmw::RbacOutput out;
+                opencmw::deserialise<opencmw::YaS, opencmw::ProtocolCheck::IGNORE>(outBuf, out);
+                REQUIRE(out.i == 42);
+
+                std::lock_guard lk(m);
+                done = true;
+                cv.notify_one();
+        }, std::move(iBuf)); }, std::move(inBuf));
+
+    std::unique_lock lk(m);
+    cv.wait(lk, [&] { return done; });
+}
+
+TEST_CASE("RBAC predicate tests", "[rbac][predicate]") {
+    opencmw::majordomo::Broker<> broker{ "/Broker", {} };
+    RbacWorker                   worker{ broker };
+
+    broker.setRbacHandler([](auto &msg) { msg.rbac.put("RBAC=USER,hash"); });
+    worker.setRbacPredicate([](auto &buf) { return buf.asString().contains("ADMIN"); });
+
+    REQUIRE(broker.bind(opencmw::URI<>("mds://127.0.0.1:12345")));
+    REQUIRE(broker.bind(opencmw::URI<>("mdp://127.0.0.1:12346")));
+
+    RunInThread brokerThread(broker);
+    RunInThread workerThread(worker);
+
+    REQUIRE(waitUntilWorkerServiceAvailable(broker.context, worker));
+
+    std::vector<std::unique_ptr<opencmw::client::ClientBase>> clients;
+    clients.emplace_back(std::make_unique<opencmw::client::MDClientCtx>(broker.context, 20ms, ""));
+    opencmw::client::ClientContext client{ std::move(clients) };
+
+    opencmw::IoBuffer              inBuf;
+    opencmw::RbacInput             in;
+    opencmw::serialise<opencmw::YaS>(inBuf, in);
+
+    std::condition_variable cv;
+    std::mutex              m;
+    bool                    done = false;
+
+    client.set(opencmw::URI("mdp://127.0.0.1:12346/rbac"), [&](const mdp::Message &reply) {
+        REQUIRE(reply.error.size());
+
+        broker.setRbacHandler([](auto& msg){msg.rbac.put("RBAC=ADMIN,hash");});
+
+        opencmw::IoBuffer iBuf;
+        opencmw::serialise<opencmw::YaS>(iBuf, in);
+        client.set(opencmw::URI("mdp://127.0.0.1:12346/rbac"), [&](const mdp::Message &reply) {
+                auto outBuf = reply.data;
+                REQUIRE_MESSAGE(reply.error.empty(), reply.error);
+                REQUIRE(outBuf.size());
+                opencmw::RbacOutput out;
+                opencmw::deserialise<opencmw::YaS, opencmw::ProtocolCheck::IGNORE>(outBuf, out);
+                REQUIRE(out.i == 42);
+
+                std::lock_guard lk(m);
+                done = true;
+                cv.notify_one();
+        }, std::move(iBuf)); }, std::move(inBuf));
+
+    std::unique_lock lk(m);
+    cv.wait(lk, [&] { return done; });
+}

src/services/include/services/KeyStore.hpp

@@ -56,6 +56,14 @@
         _keys[keyHash(roleKey.key)] = roleKey;
     }
 
+    void addRoles(const std::string &publicKey, const std::string &roles) {
+        const auto hash = keyHash(publicKey);
+        if (_keys.contains(hash)) {
+            auto &key = _keys.at(hash);
+            key.role  = roles;
+        }
+    }
+
 private:
     void init() {
         KeystoreWorkerType::setCallback([this](const majordomo::RequestContext &rawCtx, const KeystoreContext &context, const KeystoreInput &in, KeystoreContext &replyContext, KeystoreOutput &out) {

src/services/include/services/OAuthClient.hpp

@@ -324,6 +324,9 @@ class OAuthWorker : public OAuthWorkerType {
 
                 // get RBAC roles
                 out.roles = _client.getAssignedRoles(out.accessToken);
+                if (!in.publicKey.empty()) {
+                    _keystore.addRoles(in.publicKey, out.roles);
+                }
             }
         });
         _keystoreThread = std::thread([this] { _keystore.run(); });

src/services/test/OAuthClient_tests.cpp

@@ -6,6 +6,7 @@
 
 #include <Client.hpp>
 #include <concepts/majordomo/helpers.hpp>
+#include <Debug.hpp>
 #include <majordomo/Rbac.hpp>
 #include <services/OAuthClient.hpp>
 
@@ -72,17 +73,8 @@ bool loginAtUri(const opencmw::URI<opencmw::STRICT> &uri) {
     auto               redirectClient = opencmw::OAuthClient::getClient(redirecUri);
     relativeRef                       = redirecUri.relativeRef();
     REQUIRE(relativeRef);
-#if 0
-    // TODO: Why does this crash?
-    // In theory we should be able to use httplib to call the redirect URI
     res = redirectClient->Get(*relativeRef);
     REQUIRE(res);
-#else
-    // call the final redirect URI with curl as a workaround
-    const std::string curl = "curl '" + it->second + "'";
-    // flawfinder: ignore
-    res->status            = std::system(curl.c_str()) + 200;
-#endif
     REQUIRE(res->status == 200);
 
     return true;
@@ -100,7 +92,7 @@ TEST_CASE("Worker test", "[OAuth]") {
     };
 
     const std::string kcBase = envOr("KEYCLOAK_URL", "http://localhost:8090");
-    const std::string redirectBase = envOr("KEYCLOAK_REDIRECT_URI", "http://localhost:8091");
+    const std::string    redirectBase = envOr("KEYCLOAK_REDIRECT_URI", "http://localhost:8091/");
 
     opencmw::OAuthWorker oauthWorker{
         opencmw::URI(redirectBase),
@@ -137,6 +129,7 @@ TEST_CASE("Worker test", "[OAuth]") {
     client.set(opencmw::URI("mdp://127.0.0.1:12346/oauth"), [&](const mdp::Message &reply) {
         opencmw::OAuthOutput out;
         auto outBuf = reply.data;
+        REQUIRE_MESSAGE(reply.error.empty(), reply.error);
         opencmw::deserialise<opencmw::YaS, opencmw::ProtocolCheck::IGNORE>(outBuf, out);
 
         REQUIRE(out.authorizationUri.size());
@@ -149,6 +142,7 @@ TEST_CASE("Worker test", "[OAuth]") {
         client.set(opencmw::URI("mdp://127.0.0.1:12346/oauth"), [&](const mdp::Message& rep) {
                 opencmw::OAuthOutput tokenOut;
                 auto tokenOutBuf = rep.data;
+                REQUIRE_MESSAGE(rep.error.empty(), rep.error);
                 opencmw::deserialise<opencmw::YaS, opencmw::ProtocolCheck::IGNORE>(tokenOutBuf, tokenOut);
 
                 // we must have got an access token now
@@ -163,6 +157,7 @@ TEST_CASE("Worker test", "[OAuth]") {
                 client.set(opencmw::URI("mdp://127.0.0.1:12346/keystore"), [&](const mdp::Message &keyResp) {
                         opencmw::KeystoreOutput keyOut;
                         auto keyOutBuf = keyResp.data;
+                        REQUIRE_MESSAGE(keyResp.error.empty(), keyResp.error);
                         opencmw::deserialise<opencmw::YaS, opencmw::ProtocolCheck::IGNORE>(keyOutBuf, keyOut);
 
                         // the hashes should be the same