From 0d6912b0b7d5e7e1873f156d9487e94482ad91e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20Gro=C3=9F?= Date: Thu, 2 Apr 2026 13:20:44 +0200 Subject: [PATCH 1/4] Cryptography: Fix ODR violations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Magnus Groß --- src/majordomo/include/majordomo/Cryptography.hpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/majordomo/include/majordomo/Cryptography.hpp b/src/majordomo/include/majordomo/Cryptography.hpp index 6c42bf4d..6eac5ae6 100644 --- a/src/majordomo/include/majordomo/Cryptography.hpp +++ b/src/majordomo/include/majordomo/Cryptography.hpp @@ -23,19 +23,19 @@ class KeyHash { unsigned char hash[crypto_generichash_KEYBYTES]; }; -std::pair generateKeyPair() { +inline std::pair generateKeyPair() { std::pair result; crypto_sign_keypair(result.first.key, result.second.key); return result; } -KeyHash publicKeyHash(const PublicKey &key) { +inline KeyHash publicKeyHash(const PublicKey &key) { KeyHash result; crypto_generichash(result.hash, sizeof result.hash, key.key, crypto_sign_PUBLICKEYBYTES, nullptr, 0); return result; } -std::string messageHash(const mdp::Message &msg) { +inline std::string messageHash(const mdp::Message &msg) { unsigned char buf[crypto_generichash_KEYBYTES]; for (size_t i = 0; i < crypto_generichash_KEYBYTES; ++i) { buf[i] = msg.rbac[i]; @@ -43,7 +43,7 @@ std::string messageHash(const mdp::Message &msg) { return std::string(buf, buf + crypto_generichash_KEYBYTES); } -void sign(mdp::Message &msg, const PrivateKey &key, const KeyHash &hash) { +inline void sign(mdp::Message &msg, const PrivateKey &key, const KeyHash &hash) { // write key hash followed by signature unsigned char buf[crypto_generichash_KEYBYTES + crypto_sign_BYTES]; std::copy(hash.hash, hash.hash + crypto_generichash_KEYBYTES, buf); @@ -53,7 +53,7 @@ void sign(mdp::Message &msg, const PrivateKey &key, const KeyHash &hash) { msg.rbac = rbac; } -bool verify(mdp::Message &msg, const PublicKey &key) { +inline bool verify(const mdp::Message &msg, const PublicKey &key) { auto data = msg.data; unsigned char sig[crypto_sign_BYTES]; for (size_t i = 0; i < crypto_sign_BYTES; ++i) { @@ -62,7 +62,6 @@ bool verify(mdp::Message &msg, const PublicKey &key) { } return !crypto_sign_verify_detached(sig, data.data(), data.size(), key.key); } - } } // namespace opencmw::majordomo::cryptography From 74ca7768dd85f042bcf1c9c5033fe5d7ddbaf2b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20Gro=C3=9F?= Date: Thu, 2 Apr 2026 13:23:06 +0200 Subject: [PATCH 2/4] OAuth: Provide better error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Magnus Groß --- src/services/include/services/OAuthClient.hpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/services/include/services/OAuthClient.hpp b/src/services/include/services/OAuthClient.hpp index 5f784c18..49d1716a 100644 --- a/src/services/include/services/OAuthClient.hpp +++ b/src/services/include/services/OAuthClient.hpp @@ -99,18 +99,24 @@ class OAuthClient { explicit OAuthClient(StrictUri redirectUri, StrictUri endpoint, StrictUri tokenEndpoint) : _redirectUri(redirectUri), _endpoint(endpoint), _tokenEndpoint(tokenEndpoint) { _srv.Get("/", [&](const httplib::Request req, httplib::Response &res) { - std::string code, state; + std::string code, state, error; // response as per https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2 for (const auto &[k, v] : req.params) { if (k == "code") { code = v; } else if (k == "state") { state = v; + } else if (k == "error_description") { + error = v; } } if (code.empty()) { - res.set_content("Did not receive an RFC 6749-compliant response.", "text/plain"); res.status = 401; // Unauthorized + if (error.empty()) { + res.set_content("Did not receive an RFC 6749-compliant response.", "text/plain"); + } else { + res.set_content("Error: " + error, "text/plain"); + } } else { res.set_content("Authorization complete. You can close this browser window now.\n", "text/plain"); if (_endpointCallback) { @@ -119,7 +125,9 @@ class OAuthClient { } }); _thread = std::make_unique([this]() { - _srv.listen(*_redirectUri.hostName(), *_redirectUri.port()); + if (!_srv.listen(*_redirectUri.hostName(), *_redirectUri.port())) { + std::println("Failed to listen on port {}", *_redirectUri.port()); + }; }); } From 558abda8e1c7bffd910431325524057ed1db1cd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20Gro=C3=9F?= Date: Thu, 2 Apr 2026 13:23:48 +0200 Subject: [PATCH 3/4] Keycloak: Allow retrieving roles in userinfo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Magnus Groß --- src/client/test/setup-keycloak.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/client/test/setup-keycloak.sh b/src/client/test/setup-keycloak.sh index 11b65a39..877dabc1 100755 --- a/src/client/test/setup-keycloak.sh +++ b/src/client/test/setup-keycloak.sh @@ -2,7 +2,7 @@ # This script requires keycloak to be already running and configures it to be compatible with the unit tests by setting up a realm, a user etc... # To start a keycloak instance in the first place, the following is sufficient: -# docker run -p 8090:8080 -e KEYCLOAK_ADMIN=admin -e KEYCLOAK_ADMIN_PASSWORD=admin quay.io/keycloak/keycloak:25.0.4 start-dev +# docker run -p 127.0.0.1:8090:8080 -e KEYCLOAK_ADMIN=admin -e KEYCLOAK_ADMIN_PASSWORD=admin quay.io/keycloak/keycloak:25.0.4 start-dev # # For more information see: https://www.keycloak.org/getting-started/getting-started-docker @@ -15,6 +15,8 @@ while ! curl -s "$URL" >/dev/null; do sleep 1 done +# For more information about admin endpoints refer to https://www.keycloak.org/docs-api/latest/rest-api/index.html + echo 'Getting access token' AUTH="Authorization: Bearer $(curl -s -X POST "$URL/realms/master/protocol/openid-connect/token" -H 'Accept: application/json' -H 'Content-Type: application/x-www-form-urlencoded' -d 'grant_type=password&username=admin&password='"$ADMIN_PASSWORD"'&client_id=admin-cli' | jq -r '.access_token')" @@ -27,5 +29,11 @@ curl -s -X POST "$URL/admin/realms/testrealm/users" -H 'Content-Type: applicatio echo 'Adding a client with client id "testclientid"' curl -s -X POST "$URL/admin/realms/testrealm/clients" -H 'Content-Type: application/json' -H "$AUTH" -d '{"clientId":"testclientid","enabled":true,"redirectUris":["'"$REDIRECT_URI"'"],"publicClient":true}' +echo 'Updating client to return role names in userinfo for openid scope' +ROLES="$(curl -s -X GET "$URL/admin/realms/testrealm/client-scopes" -H 'Content-Type: application/json' -H "$AUTH" | jq '.[] | select(.name=="roles")')" +CLIENTSCOPEID="$(echo "$ROLES" | jq -r '.id')" +MAPPERID="$(echo "$ROLES" | jq -r '.protocolMappers[] | select(.name == "client roles") | .id')" +curl -s -X PUT "$URL/admin/realms/testrealm/client-scopes/$CLIENTSCOPEID/protocol-mappers/models/$MAPPERID" -H 'Content-Type: application/json' -H "$AUTH" -d '{"id":"'"$MAPPERID"'","protocol":"openid-connect","protocolMapper":"oidc-usermodel-client-role-mapper","name":"client roles","consentRequired":false,"config":{"introspection.token.claim":true,"userinfo.token.claim":true,"claim.name":"resource_access.${client_id}.roles", "jsonType.label":"String","multivalued":true,"userinfo.token.claim":true}}' # it is intended that ${client_id} should not shell expand here + echo 'Adding a confidential client with client id "confidentialclientid" and secret "secret00000000000000000000000000"' curl -s -X POST "$URL/admin/realms/testrealm/clients" -H 'Content-Type: application/json' -H "$AUTH" -d '{"clientId":"confidentialclientid","enabled":true,"redirectUris":["'"$REDIRECT_URI"'"],"publicClient":false,"clientAuthenticatorType":"client-secret","directAccessGrantsEnabled":true,"secret":"secret00000000000000000000000000"}' From 88bb8a0caa8673fe9e2886ec73ec4695408b8f31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20Gro=C3=9F?= Date: Thu, 2 Apr 2026 13:24:41 +0200 Subject: [PATCH 4/4] OAuth: Return assigned roles as part of the OAuth output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Magnus Groß --- src/services/include/services/OAuthClient.hpp | 46 ++++++++++++++++--- src/services/test/OAuthClient_tests.cpp | 3 +- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/services/include/services/OAuthClient.hpp b/src/services/include/services/OAuthClient.hpp index 49d1716a..d37ebaa9 100644 --- a/src/services/include/services/OAuthClient.hpp +++ b/src/services/include/services/OAuthClient.hpp @@ -27,12 +27,13 @@ struct OAuthOutput { std::string secret; std::string accessToken; std::string refreshToken; + std::string roles; }; } // namespace opencmw ENABLE_REFLECTION_FOR(opencmw::OAuthContext, secretToken) ENABLE_REFLECTION_FOR(opencmw::OAuthInput, scope, clientId, clientSecret, secret, publicKey) -ENABLE_REFLECTION_FOR(opencmw::OAuthOutput, authorizationUri, secret, accessToken, refreshToken) +ENABLE_REFLECTION_FOR(opencmw::OAuthOutput, authorizationUri, secret, accessToken, refreshToken, roles) namespace opencmw { @@ -91,13 +92,14 @@ class OAuthClient { StrictUri _redirectUri; StrictUri _endpoint; StrictUri _tokenEndpoint; + StrictUri _userinfoEndpoint; httplib::Server _srv; std::unique_ptr _thread; std::function _endpointCallback; public: - explicit OAuthClient(StrictUri redirectUri, StrictUri endpoint, StrictUri tokenEndpoint) - : _redirectUri(redirectUri), _endpoint(endpoint), _tokenEndpoint(tokenEndpoint) { + explicit OAuthClient(StrictUri redirectUri, StrictUri endpoint, StrictUri tokenEndpoint, StrictUri userinfoEndpoint = StrictUri("")) + : _redirectUri(redirectUri), _endpoint(endpoint), _tokenEndpoint(tokenEndpoint), _userinfoEndpoint(userinfoEndpoint) { _srv.Get("/", [&](const httplib::Request req, httplib::Response &res) { std::string code, state, error; // response as per https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2 @@ -213,6 +215,33 @@ class OAuthClient { return getAccessToken({ { "grant_type", "refresh_token" }, { "refresh_token", refreshToken }, { "client_id", clientId } }); } + std::string getAssignedRoles(const std::string &accessToken) { + auto client = getClient(_userinfoEndpoint); + if (!client) { + return {}; + } + + httplib::Headers headers{ { { "Authorization", "Bearer " + accessToken } } }; + auto res = client->Get(*_userinfoEndpoint.path(), headers); + if (!res || res->status != 200) { + return {}; + } + std::string s{ res->body.c_str() }; + const std::string marker{ "\"account\":{\"roles\":[" }; + auto start = s.find(marker); + if (start == std::string::npos) { + return {}; + } + start += marker.size(); + const auto end = s.find("]", start); + if (end == std::string::npos) { + return {}; + } + std::string roles = s.substr(start, end - start); + std::erase(roles, '\"'); + return roles; + } + void stop() { _srv.stop(); _thread->join(); @@ -246,11 +275,11 @@ using OAuthWorkerType = majordomo::Worker<"/oauth", OAuthContext, OAuthInput, OA class OAuthWorker : public OAuthWorkerType { public: - explicit OAuthWorker(StrictUri redirectUri, StrictUri endpoint, StrictUri tokenEndpoint, StrictUri brokerAddress, const zmq::Context &context, majordomo::Settings settings = {}) - : OAuthWorkerType(brokerAddress, {}, context, settings), _client(redirectUri, endpoint, tokenEndpoint), _keystore(brokerAddress, context, settings) { init(); }; + explicit OAuthWorker(StrictUri redirectUri, StrictUri endpoint, StrictUri tokenEndpoint, StrictUri userinfoEndpoint, StrictUri brokerAddress, const zmq::Context &context, majordomo::Settings settings = {}) + : OAuthWorkerType(brokerAddress, {}, context, settings), _client(redirectUri, endpoint, tokenEndpoint, userinfoEndpoint), _keystore(brokerAddress, context, settings) { init(); }; template - explicit OAuthWorker(StrictUri redirecturi, StrictUri endpoint, StrictUri tokenEndpoint, const BrokerType &broker) - : OAuthWorkerType(broker, {}), _client(redirecturi, endpoint, tokenEndpoint), _keystore(broker) { init(); }; + explicit OAuthWorker(StrictUri redirecturi, StrictUri endpoint, StrictUri tokenEndpoint, StrictUri userinfoEndpoint, const BrokerType &broker) + : OAuthWorkerType(broker, {}), _client(redirecturi, endpoint, tokenEndpoint, userinfoEndpoint), _keystore(broker) { init(); }; ~OAuthWorker() { _keystore.shutdown(); _keystoreThread.join(); @@ -292,6 +321,9 @@ class OAuthWorker : public OAuthWorkerType { const auto access = _client.requestToken(code, in.clientId); out.accessToken = access.accessToken._token; out.refreshToken = access.refreshToken._token; + + // get RBAC roles + out.roles = _client.getAssignedRoles(out.accessToken); } }); _keystoreThread = std::thread([this] { _keystore.run(); }); diff --git a/src/services/test/OAuthClient_tests.cpp b/src/services/test/OAuthClient_tests.cpp index 8fbca2c8..569825de 100644 --- a/src/services/test/OAuthClient_tests.cpp +++ b/src/services/test/OAuthClient_tests.cpp @@ -106,7 +106,8 @@ TEST_CASE("Worker test", "[OAuth]") { opencmw::URI(redirectBase), opencmw::URI(kcBase + "/realms/testrealm/protocol/openid-connect/auth"), opencmw::URI(kcBase + "/realms/testrealm/protocol/openid-connect/token"), - broker + opencmw::URI(kcBase + "/realms/testrealm/protocol/openid-connect/userinfo"), + broker }; REQUIRE(broker.bind(opencmw::URI<>("mds://127.0.0.1:12345")));