diff --git a/CMakeLists.txt b/CMakeLists.txt index 805b301..8bee831 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,5 @@ cmake_minimum_required(VERSION 3.14) -project(oatpp-authkit VERSION 0.3.6 LANGUAGES CXX) +project(oatpp-authkit VERSION 0.4.0 LANGUAGES CXX) # Header-only interface library — no compilation, just an include path and # a CMake config package so consumers do: diff --git a/include/oatpp-authkit/auth/AuthInterceptor.hpp b/include/oatpp-authkit/auth/AuthInterceptor.hpp index a3a1eb7..2445cfb 100644 --- a/include/oatpp-authkit/auth/AuthInterceptor.hpp +++ b/include/oatpp-authkit/auth/AuthInterceptor.hpp @@ -10,11 +10,13 @@ #include "oatpp/web/protocol/http/outgoing/Response.hpp" #include "oatpp/web/protocol/http/outgoing/ResponseFactory.hpp" #include "oatpp/web/protocol/http/Http.hpp" +#include "oatpp/parser/json/mapping/ObjectMapper.hpp" #include "IAuthBackend.hpp" #include "IAuthPolicy.hpp" #include "IRuntimeConfig.hpp" #include "../util/TokenExtract.hpp" +#include "../dto/InternalDto.hpp" namespace oatpp_authkit { @@ -43,6 +45,7 @@ private: std::shared_ptr m_policy; std::shared_ptr m_runtime; TokenHasher m_hashToken; + std::shared_ptr m_mapper; using Status = oatpp::web::protocol::http::Status; using ResponseFactory = oatpp::web::protocol::http::outgoing::ResponseFactory; @@ -53,6 +56,22 @@ private: return r; } + /** @brief Build a JsonErrorDto-shaped body via ObjectMapper (#6) — escapes + * any user-supplied `msg` so a stray `"`/`\\`/control character doesn't + * break the JSON envelope. */ + std::shared_ptr makeJsonError(Status status, + const std::string& statusName, + const std::string& msg) { + auto dto = dto::JsonErrorDto::createShared(); + dto->status = oatpp::String(statusName); + dto->code = status.code; + if (!msg.empty()) dto->message = oatpp::String(msg); + oatpp::String json = m_mapper->writeToString(dto); + auto r = ResponseFactory::createResponse(status, json); + r->putHeader("Content-Type", "application/json"); + return r; + } + std::shared_ptr makeHtmlError(Status status, const std::string& title) { std::string body = "" + title + "

" + title + "

"; @@ -129,7 +148,7 @@ private: const std::shared_ptr& req, const std::string& path) { if (requestWantsJson(req, path)) - return makeJsonError(Status::CODE_401, "{\"status\":\"Unauthorized\"}"); + return makeJsonError(Status::CODE_401, "Unauthorized", ""); if (auto loc = m_policy->unauthenticatedRedirect(path)) return makeRedirect(*loc); return makeHtmlError(Status::CODE_401, "Unauthorized"); @@ -140,9 +159,10 @@ private: const std::string& msg = "") { if (requestWantsJson(req, path)) { - if (msg.empty()) return makeJsonError(Status::CODE_403, "{\"status\":\"Forbidden\"}"); - return makeJsonError(Status::CODE_403, - "{\"status\":\"Forbidden\",\"message\":\"" + msg + "\"}"); + // #6: route through ObjectMapper so any caller-supplied `msg` + // containing `"`/`\\`/control chars is escaped instead of breaking + // the response envelope. + return makeJsonError(Status::CODE_403, "Forbidden", msg); } if (auto loc = m_policy->unauthenticatedRedirect(path)) return makeRedirect(*loc); @@ -172,11 +192,13 @@ public: AuthInterceptor(std::shared_ptr backend, std::shared_ptr policy, std::shared_ptr runtime, - TokenHasher hashToken) + TokenHasher hashToken, + std::shared_ptr mapper = nullptr) : m_backend(std::move(backend)) , m_policy(std::move(policy)) , m_runtime(std::move(runtime)) - , m_hashToken(std::move(hashToken)) {} + , m_hashToken(std::move(hashToken)) + , m_mapper(mapper ? mapper : oatpp::parser::json::mapping::ObjectMapper::createShared()) {} std::shared_ptr intercept( const std::shared_ptr& request) override diff --git a/include/oatpp-authkit/dto/InternalDto.hpp b/include/oatpp-authkit/dto/InternalDto.hpp new file mode 100644 index 0000000..2bdff4e --- /dev/null +++ b/include/oatpp-authkit/dto/InternalDto.hpp @@ -0,0 +1,73 @@ +#ifndef OATPP_AUTHKIT_DTO_INTERNAL_DTO_HPP +#define OATPP_AUTHKIT_DTO_INTERNAL_DTO_HPP + +#include "oatpp/codegen/dto/base_define.hpp" +#include "oatpp/core/macro/codegen.hpp" +#include "oatpp/core/Types.hpp" + +#include OATPP_CODEGEN_BEGIN(DTO) + +namespace oatpp_authkit::dto { + +/** + * @brief Body shape emitted by JsonErrorHandler and AuthInterceptor::makeJsonError (#6). + * + * Replaces ad-hoc string concatenation. Going through ObjectMapper + * guarantees the embedded `status` / `message` strings are properly + * escaped — the previous hand-rolled `JsonErrorHandler::handleError` + * embedded `status.description` raw, which would emit invalid JSON for + * any `Status{…, "I'm a \"teapot\""}` description. + */ +class JsonErrorDto : public oatpp::DTO { + DTO_INIT(JsonErrorDto, DTO) + + DTO_FIELD(String, status); + DTO_FIELD(Int32, code); + DTO_FIELD(String, message); +}; + +/** + * @brief Outbound WS broadcast for booking/person lifecycle events (#6). + * + * {"type":"booking_updated","id":"42"} + */ +class WsEntityEventDto : public oatpp::DTO { + DTO_INIT(WsEntityEventDto, DTO) + + DTO_FIELD(String, type); + DTO_FIELD(String, id); +}; + +/** + * @brief Outbound WS broadcast for presence updates (#6). + * + * {"type":"presence_update","booking_id":"42","users":["alice","bob"]} + */ +class WsPresenceUpdateDto : public oatpp::DTO { + DTO_INIT(WsPresenceUpdateDto, DTO) + + DTO_FIELD(String, type); + DTO_FIELD(String, booking_id); + DTO_FIELD(List, users); +}; + +/** + * @brief Inbound WS message envelope (#6) — replaces `Listener::jsonStr/jsonInt`. + * + * The toy regex parsers in the previous implementation mishandled escaped + * quotes and nested objects; routing through `ObjectMapper` rejects + * malformed inbound payloads cleanly. + */ +class WsClientMsgDto : public oatpp::DTO { + DTO_INIT(WsClientMsgDto, DTO) + + DTO_FIELD(String, type); + DTO_FIELD(String, booking_id); + DTO_FIELD(String, user); +}; + +} // namespace oatpp_authkit::dto + +#include OATPP_CODEGEN_END(DTO) + +#endif diff --git a/include/oatpp-authkit/handler/JsonErrorHandler.hpp b/include/oatpp-authkit/handler/JsonErrorHandler.hpp index d2ce7f1..35c604f 100644 --- a/include/oatpp-authkit/handler/JsonErrorHandler.hpp +++ b/include/oatpp-authkit/handler/JsonErrorHandler.hpp @@ -3,6 +3,9 @@ #include "oatpp/web/server/handler/ErrorHandler.hpp" #include "oatpp/web/protocol/http/outgoing/ResponseFactory.hpp" +#include "oatpp/parser/json/mapping/ObjectMapper.hpp" + +#include "../dto/InternalDto.hpp" namespace oatpp_authkit { @@ -10,23 +13,36 @@ namespace oatpp_authkit { * @brief Custom error handler that returns JSON error responses. * * Replaces oatpp's default plain-text error handler so that - * OATPP_ASSERT_HTTP errors are returned as JSON objects matching - * the StatusDto schema: {"status": "...", "code": N, "message": "..."}. - * This allows the frontend's coreFetch to parse error details reliably. + * OATPP_ASSERT_HTTP errors are returned as JSON objects matching the + * `JsonErrorDto` schema: `{"status": "...", "code": N, "message": "..."}`. + * + * Routing through `ObjectMapper` (DI'd) replaces the previous hand-rolled + * concatenation that embedded `status.description` raw — see #6. */ class JsonErrorHandler : public oatpp::web::server::handler::ErrorHandler { +private: + std::shared_ptr m_mapper; + public: + /** + * @param mapper Shared JSON object mapper. Pass nullptr for a + * handler-owned default mapper (back-compat path). + */ + explicit JsonErrorHandler(std::shared_ptr mapper = nullptr) + : m_mapper(mapper ? mapper : oatpp::parser::json::mapping::ObjectMapper::createShared()) {} + std::shared_ptr handleError(const oatpp::web::protocol::http::Status& status, const oatpp::String& message, const Headers& headers) override { - auto json = oatpp::String( - "{\"status\":\"" + std::string(status.description) + - "\",\"code\":" + std::to_string(status.code) + - ",\"message\":\"" + escapeJson(message ? message->c_str() : "") + "\"}" - ); + auto dto = dto::JsonErrorDto::createShared(); + dto->status = oatpp::String(std::string(status.description)); + dto->code = status.code; + dto->message = message ? message : oatpp::String(""); + + oatpp::String json = m_mapper->writeToString(dto); auto response = oatpp::web::protocol::http::outgoing::ResponseFactory::createResponse( status, json @@ -39,30 +55,6 @@ public: return response; } - -private: - - static std::string escapeJson(const char* s) { - std::string out; - for (; *s; ++s) { - switch (*s) { - case '"': out += "\\\""; break; - case '\\': out += "\\\\"; break; - case '\n': out += "\\n"; break; - case '\r': out += "\\r"; break; - case '\t': out += "\\t"; break; - default: - if (static_cast(*s) < 0x20) { - char buf[8]; - snprintf(buf, sizeof(buf), "\\u%04x", static_cast(*s)); - out += buf; - } else { - out += *s; - } - } - } - return out; - } }; } // namespace oatpp_authkit diff --git a/include/oatpp-authkit/ws/Hub.hpp b/include/oatpp-authkit/ws/Hub.hpp index 6c70e22..f867f16 100644 --- a/include/oatpp-authkit/ws/Hub.hpp +++ b/include/oatpp-authkit/ws/Hub.hpp @@ -12,7 +12,9 @@ #include "oatpp-websocket/ConnectionHandler.hpp" #include "oatpp-websocket/WebSocket.hpp" +#include "oatpp/parser/json/mapping/ObjectMapper.hpp" #include "Listener.hpp" +#include "../dto/InternalDto.hpp" namespace oatpp_authkit::ws { @@ -102,21 +104,50 @@ private: static std::map> s_socketPresence; /** - * @brief Serialise a presence-update notification as a JSON string. + * @brief Process-wide ObjectMapper for outbound WS frames (#6). + * + * Lazy-initialised on first use; consumers can override via + * `setObjectMapper()` to share the same mapper instance with the rest of + * the app. Mapper is thread-safe for concurrent `writeToString` use. + */ + static std::shared_ptr& sharedMapper() { + static std::shared_ptr m + = oatpp::parser::json::mapping::ObjectMapper::createShared(); + return m; + } + + /** + * @brief Serialise a presence-update notification as a JSON string (#6). + * + * Routes through ObjectMapper so usernames / booking IDs containing `"`, + * `\\`, or control characters are escaped properly. The previous hand- + * rolled concatenation produced invalid JSON for any such input. */ static std::string buildPresenceMsg(const std::string& bookingId, const std::set& users) { - std::string list = "["; - bool first = true; - for (const auto& u : users) { - if (!first) list += ","; - list += "\"" + u + "\""; - first = false; - } - list += "]"; - return "{\"type\":\"presence_update\",\"booking_id\":\"" - + bookingId + "\",\"users\":" + list + "}"; + auto dto = dto::WsPresenceUpdateDto::createShared(); + dto->type = oatpp::String("presence_update"); + dto->booking_id = oatpp::String(bookingId); + dto->users = {}; + for (const auto& u : users) dto->users->push_back(oatpp::String(u)); + return std::string(*sharedMapper()->writeToString(dto)); } + /** @brief Build a `{type,id}` event envelope via ObjectMapper (#6). */ + static std::string buildEntityEvent(const char* type, const std::string& id) { + auto dto = dto::WsEntityEventDto::createShared(); + dto->type = oatpp::String(type); + dto->id = oatpp::String(id); + return std::string(*sharedMapper()->writeToString(dto)); + } + +public: + /** @brief Replace the process-wide ObjectMapper (#6). Call once at startup + * if the host application has its own mapper instance to share. */ + static void setObjectMapper(std::shared_ptr m) { + if (m) sharedMapper() = std::move(m); + } +private: + /** * @brief Check whether a socket has access to a given property. * @@ -232,9 +263,7 @@ public: * @param propertyId The property this booking belongs to (empty = broadcast to all). */ static void notifyBooking(const char* type, const std::string& id, const std::string& propertyId) { - broadcastToProperty( - std::string("{\"type\":\"") + type + "\",\"id\":\"" + id + "\"}", - propertyId); + broadcastToProperty(buildEntityEvent(type, id), propertyId); } /** @@ -244,8 +273,7 @@ public: * available. Sends to all authenticated sockets. */ static void notifyBooking(const char* type, const std::string& id) { - broadcast(std::string("{\"type\":\"") + type - + "\",\"id\":\"" + id + "\"}"); + broadcast(buildEntityEvent(type, id)); } /** @@ -255,8 +283,7 @@ public: * notifications are not property-scoped. */ static void notifyPerson(const char* type, const std::string& id) { - broadcast(std::string("{\"type\":\"") + type - + "\",\"id\":\"" + id + "\"}"); + broadcast(buildEntityEvent(type, id)); } // --- Presence --- @@ -376,11 +403,27 @@ inline HubHousekeeper s_wsHubHousekeeper; inline void Listener::touchActivity(const WebSocket* socket) { Hub::touchSocket(socket); } -// Listener::handleMessage defined here (after Hub) to break the circular dependency +// Listener::handleMessage defined here (after Hub) to break the circular dependency. +// +// #6: parse inbound WS frames via ObjectMapper instead of the toy +// jsonStr/jsonInt regex parsers — those mishandled escaped quotes, +// nested objects, and unicode escapes. ObjectMapper rejects malformed +// payloads cleanly (caught here so a bad client frame is just dropped, +// never an unhandled exception). inline void Listener::handleMessage(const WebSocket& socket, const std::string& text) { Hub::touchSocket(&socket); // #439: record activity to suppress idle close - std::string type = jsonStr(text, "type"); - std::string bookingId = jsonStr(text, "booking_id"); + + oatpp::Object msg; + try { + msg = Hub::sharedMapper()->readFromString>( + oatpp::String(text)); + } catch (...) { + return; // malformed JSON — drop frame + } + if (!msg || !msg->type || !msg->booking_id) return; + + const std::string type = std::string(*msg->type); + const std::string bookingId = std::string(*msg->booking_id); if (bookingId.empty()) return; if (type == "presence_open") { diff --git a/include/oatpp-authkit/ws/Listener.hpp b/include/oatpp-authkit/ws/Listener.hpp index b8c054e..5dd2279 100644 --- a/include/oatpp-authkit/ws/Listener.hpp +++ b/include/oatpp-authkit/ws/Listener.hpp @@ -35,41 +35,6 @@ private: oatpp::data::stream::BufferOutputStream m_buffer{256}; ///< Accumulates frame payloads until end-of-message. bool m_overflowed = false; ///< Set when kMaxMessageBytes was exceeded; drop remainder of the current message. - /** - * @brief Extract a JSON string value for the given key from a JSON object. - * @param json The raw JSON text. - * @param key The field name to look up. - * @return The string value, or an empty string if the key is absent. - */ - static std::string jsonStr(const std::string& json, const std::string& key) { - auto kpos = json.find("\"" + key + "\""); - if (kpos == std::string::npos) return ""; - auto cpos = json.find(':', kpos + key.size() + 2); - if (cpos == std::string::npos) return ""; - auto qpos = json.find('"', cpos + 1); - if (qpos == std::string::npos) return ""; - auto epos = json.find('"', qpos + 1); - if (epos == std::string::npos) return ""; - return json.substr(qpos + 1, epos - qpos - 1); - } - - /** - * @brief Extract a JSON integer value for the given key from a JSON object. - * @param json The raw JSON text. - * @param key The field name to look up. - * @return The integer value, or -1 if the key is absent or not a digit sequence. - */ - static int jsonInt(const std::string& json, const std::string& key) { - auto kpos = json.find("\"" + key + "\""); - if (kpos == std::string::npos) return -1; - auto cpos = json.find(':', kpos + key.size() + 2); - if (cpos == std::string::npos) return -1; - cpos++; - while (cpos < json.size() && (json[cpos] == ' ' || json[cpos] == '\t')) cpos++; - if (cpos >= json.size() || !std::isdigit((unsigned char)json[cpos])) return -1; - return std::stoi(json.substr(cpos)); - } - /** * @brief Dispatch a fully received text-frame message to Hub presence handlers. * diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 80eb07f..f880486 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -17,3 +17,7 @@ add_test(NAME body_size_limit COMMAND test_body_size_limit) add_executable(test_security_headers test_security_headers.cpp) target_link_libraries(test_security_headers PRIVATE oatpp::authkit oatpp::oatpp) add_test(NAME security_headers COMMAND test_security_headers) + +add_executable(test_json_serialization test_json_serialization.cpp) +target_link_libraries(test_json_serialization PRIVATE oatpp::authkit oatpp::oatpp) +add_test(NAME json_serialization COMMAND test_json_serialization) diff --git a/test/test_json_serialization.cpp b/test/test_json_serialization.cpp new file mode 100644 index 0000000..e6e37dc --- /dev/null +++ b/test/test_json_serialization.cpp @@ -0,0 +1,97 @@ +// Tests for the #6 ObjectMapper migration — verifies that the JSON envelopes +// produced by JsonErrorHandler / Hub::buildEntityEvent / Hub::buildPresenceMsg +// escape special characters instead of emitting raw text. The previous +// hand-rolled concatenations broke when given a `"`/`\\`/control-char string. + +// Avoid pulling Hub.hpp here — it includes oatpp-websocket, which is a +// separate optional dependency not necessarily on the test target's include +// path. The escaping behaviour we care about is purely a property of +// ObjectMapper round-tripping the InternalDto types, so we exercise the +// DTOs directly. +#include "oatpp-authkit/handler/JsonErrorHandler.hpp" +#include "oatpp-authkit/dto/InternalDto.hpp" + +#include "oatpp/parser/json/mapping/ObjectMapper.hpp" +#include "oatpp/web/protocol/http/Http.hpp" + +#include +#include +#include + +namespace { + +int g_failures = 0; + +#define REQUIRE(expr) do { \ + if (!(expr)) { \ + std::fprintf(stderr, "FAIL %s:%d %s\n", __FILE__, __LINE__, #expr); \ + ++g_failures; \ + } \ +} while (0) + +void test_presence_dto_round_trips_special_chars() { + auto m = oatpp::parser::json::mapping::ObjectMapper::createShared(); + auto dto = oatpp_authkit::dto::WsPresenceUpdateDto::createShared(); + dto->type = oatpp::String("presence_update"); + dto->booking_id = oatpp::String("id-with-\"-quote"); + dto->users = {}; + dto->users->push_back(oatpp::String("al\"ice")); + dto->users->push_back(oatpp::String("bo\\b")); + auto json = m->writeToString(dto); + + auto rt = m->readFromString>(json); + REQUIRE(rt); + REQUIRE(std::string(*rt->booking_id) == "id-with-\"-quote"); + REQUIRE(rt->users->size() == 2); + auto it = rt->users->begin(); + REQUIRE(std::string(**it++) == "al\"ice"); + REQUIRE(std::string(**it) == "bo\\b"); +} + +void test_entity_event_dto_round_trip() { + auto m = oatpp::parser::json::mapping::ObjectMapper::createShared(); + auto dto = oatpp_authkit::dto::WsEntityEventDto::createShared(); + dto->type = oatpp::String("booking_updated"); + dto->id = oatpp::String("id-with-\"-and-\\"); + auto json = m->writeToString(dto); + auto rt = m->readFromString>(json); + REQUIRE(rt); + REQUIRE(std::string(*rt->id) == "id-with-\"-and-\\"); +} + +void test_client_msg_dto_rejects_malformed() { + auto m = oatpp::parser::json::mapping::ObjectMapper::createShared(); + bool threw = false; + try { + m->readFromString>( + oatpp::String("{not json")); + } catch (...) { threw = true; } + REQUIRE(threw); +} + +void test_json_error_dto_round_trip() { + auto m = oatpp::parser::json::mapping::ObjectMapper::createShared(); + auto dto = oatpp_authkit::dto::JsonErrorDto::createShared(); + dto->status = oatpp::String("I'm a \"teapot\""); + dto->code = 418; + dto->message = oatpp::String("brew\nfailure"); + auto json = m->writeToString(dto); + + auto rt = m->readFromString>(json); + REQUIRE(rt); + REQUIRE(std::string(*rt->status) == "I'm a \"teapot\""); + REQUIRE(rt->code == 418); + REQUIRE(std::string(*rt->message) == "brew\nfailure"); +} + +} // namespace + +int main() { + test_presence_dto_round_trips_special_chars(); + test_entity_event_dto_round_trip(); + test_client_msg_dto_rejects_malformed(); + test_json_error_dto_round_trip(); + + std::printf("%s (%d failures)\n", g_failures ? "FAIL" : "OK", g_failures); + return g_failures ? 1 : 0; +}