From 2e11408240607f91bdef3319624fd2fa79bf3896 Mon Sep 17 00:00:00 2001 From: Uwe Schuster Date: Fri, 29 May 2026 12:49:03 +0200 Subject: [PATCH] #16 (audit H-1..H-5): fix the five high-severity findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - H-1 cert-DN spoofing: IRuntimeConfig::certAuthTrusted() now defaults to false (fail-closed). X-SSL-Client-DN is an ordinary request header; a loopback bind does not prove it came from a TLS-terminating proxy. Consumers must opt in explicitly behind a header-stripping proxy. - H-3 scope reparenting: ScopeGuardRepository::save() now also checks the EXISTING row's scope (via a new required entity-id accessor), so an actor can't claim an out-of-scope row by relabelling it in the request body. - H-2 IQueryable bypass: add ScopeGuardQueryable — filters query() results through the same predicate so the queryable surface can't escape the scope guard. - H-4 TemporalRepository TOCTOU: serialise the read-modify-write with a per-instance mutex (no more duplicate-live / lost-update under concurrent same-entity saves) and add an optional TxRunner so the close-then-insert pair can commit/rollback atomically. - H-5 SMTP header injection: reject CR/LF/NUL in `to`/`fromAddress` before building the envelope and From:/To: header lines. Tests: expand test_repository_decorators (reparenting + queryable filtering), add curl-guarded test_smtp_transport (base64 vectors + CRLF guard). All 15 ctest targets pass. README updated. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 4 +- .../oatpp-authkit/auth/AuthInterceptor.hpp | 5 +- include/oatpp-authkit/auth/IRuntimeConfig.hpp | 12 +- include/oatpp-authkit/mail/SmtpTransport.hpp | 16 +++ .../repo/ScopeGuardRepository.hpp | 104 +++++++++++++++- .../oatpp-authkit/repo/TemporalRepository.hpp | 41 ++++++- test/CMakeLists.txt | 9 ++ test/test_repository_decorators.cpp | 112 +++++++++++++++++- test/test_smtp_transport.cpp | 75 ++++++++++++ 9 files changed, 359 insertions(+), 19 deletions(-) create mode 100644 test/test_smtp_transport.cpp diff --git a/README.md b/README.md index ee86b01..d932b3b 100644 --- a/README.md +++ b/README.md @@ -15,8 +15,8 @@ hardened auth / security stack. Header-only, oatpp 1.3+, C++17. | `startup/RequireEncryptionKey.hpp` | `requireEncryptionKey(envVarName, encryptionEnabled, allowPlaintext)` — refuse startup without a symmetric key unless a dev flag overrides. | | `repo/Repository.hpp` + `IHistoryRepository.hpp` + `TemporalFieldTraits.hpp` + `TemporalAt.hpp` + `ActorContext.hpp` | Pure-abstract `Repository` interface set distilled from fewo-webapp's per-entity `*Db` clients. Mixed UUID allocation on `save`, separate `IHistoryRepository` for temporal versions, `TemporalFieldTraits` to map canonical (entity_id, valid_from, valid_until) onto whatever a DTO actually calls them, `ActorContext` placeholder for the scope-guard decorator. | | `repo/TemporalRepository.hpp` | Decorator that wraps any `Repository` and turns it into a temporally-versioned one. **Stable-live + historical-copy semantics (authkit#13):** the live row's `id` PK is preserved across updates; each prior version is captured as a fresh row with a new `id`. `softDelete` closes the live row in place; with `ON UPDATE CASCADE` on consumer-side composite child FKs, child rows follow automatically. `findByEntityIdAt(id, at)` returns the version live at a point in time; implements `IHistoryRepository`. Inner adapter is expected to expose all rows (live + historical) and treat `save` as upsert keyed by **`id`** (per-row PK). DTOs register their four temporal columns via `OATPP_AUTHKIT_REGISTER_TEMPORAL(Dto, id, entity_id, valid_from, valid_until)`. | -| `repo/ScopeGuardRepository.hpp` | Generic resource-scope decorator. Takes a `bool(ActorContext, TDto)` predicate at construction; gates every method on it. Throws `ScopeDeniedException` on deny (catchers translate to 403). Knows nothing about consumer-specific concepts like "property" or "tenant" — the predicate decides. | -| `repo/IQueryable.hpp` | Optional capability for repos that resolve a typed query AST. `field<&Dto::col>().eq(...)` style DSL composes via `&&` / `||` / `!`; `Query::toSql()` emits parameterised SQL plus a bind bag. Bounded surface — equality, range, IN, LIKE, NULL, ORDER BY, LIMIT/OFFSET. No joins, subqueries, or aggregates. Concrete repos opt in by deriving `IQueryable`. | +| `repo/ScopeGuardRepository.hpp` | Generic resource-scope decorator. Takes a `bool(ActorContext, TDto)` predicate, an actor accessor, and an `entity_id` accessor at construction; gates every method on the predicate. On `save` the predicate must pass on the incoming DTO **and**, for an update, on the row as it currently stands — so an actor can't reparent an out-of-scope row into its own scope by relabelling it in the request body. Throws `ScopeDeniedException` on deny (catchers translate to 403). Knows nothing about consumer-specific concepts like "property" or "tenant" — the predicate decides. **`ScopeGuardQueryable`** (same header) is the variant for `IQueryable` inners: it filters `query()` results through the predicate too, so the queryable surface can't bypass the guard. | +| `repo/IQueryable.hpp` | Optional capability for repos that resolve a typed query AST. `field<&Dto::col>().eq(...)` style DSL composes via `&&` / `||` / `!`; `Query::toSql()` emits parameterised SQL plus a bind bag. Bounded surface — equality, range, IN, LIKE, NULL, ORDER BY, LIMIT/OFFSET. No joins, subqueries, or aggregates. Concrete repos opt in by deriving `IQueryable`. Wrap a scope-guarded `IQueryable` with `ScopeGuardQueryable` (not the plain `ScopeGuardRepository`) so `query()` is scope-filtered. | | `repo/IAuditSink.hpp` + `repo/AuditLogRepository.hpp` | Cross-cutting audit-trail decorator. Emits an `AuditEvent` (actor, entity type/id, op, timestamp) per mutation through a consumer-supplied `IAuditSink`. Ops are `Create` / `Update` / `Delete` / `Read`; pre-write `findByEntityId` lookup distinguishes Create from Update. Configurable enabled-op set (default `{Create,Update,Delete}` — `Read` is opt-in, `list()` never audited). Sink failures are caught and swallowed unless a `bool(const std::exception&)` handler asks to rethrow. Stacks with `TemporalRepository` and `ScopeGuardRepository`. | | `repo/SchemaContract.hpp` | Declarative schema model for the decorator stack (authkit#14). Each decorator exposes a `static constexpr DecoratorSchema kSchema` listing the columns/indexes it contributes to the entity table plus any sidecar tables it owns. `SchemaBuilder::create(table, exec)` composes contributions into a single `CREATE TABLE` per entity table; sidecars emit separately. `SchemaContract::verify(table, probe)` is a runtime introspect-and-assert that throws `SchemaContractViolation` if any required column or sidecar is missing. Decorator code never runs ALTER at runtime — Atlas (atlasgo.io) owns evolution between deploys; the C++ side only declares desired state and checks it. | | `repo/RedactedFieldRepository.hpp` | Decorator that nulls out named fields on **historical** rows only (authkit#15). Sits below `TemporalRepository` and inspects each `save`: if `valid_until != SENTINEL`, the row is being closed as a historical version, so the configured fields (e.g. `passwordHash`, `tlsCertDn`) are set to null before persisting. The live row keeps its values intact. Built for the case where a credential rides a temporal row — every change creates a historical version with the prior secret preserved, and the redaction prevents a DB breach from yielding every credential a user has ever had. | diff --git a/include/oatpp-authkit/auth/AuthInterceptor.hpp b/include/oatpp-authkit/auth/AuthInterceptor.hpp index 2445cfb..052f933 100644 --- a/include/oatpp-authkit/auth/AuthInterceptor.hpp +++ b/include/oatpp-authkit/auth/AuthInterceptor.hpp @@ -231,8 +231,9 @@ public: } // TLS cert DN — only trusted when the runtime hook says so (#5). - // `certAuthTrusted()` defaults to `isLoopback()`; consumers can override - // it to gate more strictly (e.g. require an env-var or a TLS-only port). + // `certAuthTrusted()` defaults to `false` (fail closed); consumers must + // opt in explicitly and only behind a proxy that strips the inbound + // `X-SSL-Client-DN` header and re-sets it from a verified client cert. auto certDnH = request->getHeader("X-SSL-Client-DN"); if (m_runtime->certAuthTrusted() && certDnH && !certDnH->empty()) { if (auto p = m_backend->resolveByCertDn(std::string(*certDnH))) { diff --git a/include/oatpp-authkit/auth/IRuntimeConfig.hpp b/include/oatpp-authkit/auth/IRuntimeConfig.hpp index 81a4c4f..4862ea6 100644 --- a/include/oatpp-authkit/auth/IRuntimeConfig.hpp +++ b/include/oatpp-authkit/auth/IRuntimeConfig.hpp @@ -34,8 +34,14 @@ public: /** @brief Whether incoming `X-SSL-Client-DN` headers should be trusted (#5). * - * Default: `isLoopback()` — preserves the legacy behaviour for consumers - * that haven't overridden anything. Override to gate more strictly, e.g.: + * Default: `false` — **fail closed**. `X-SSL-Client-DN` is an ordinary + * request header; binding to loopback does NOT guarantee it originates + * from a TLS-terminating proxy. An SSH tunnel, a co-located process, or a + * reverse proxy that forwards the client-supplied header verbatim can all + * present an arbitrary DN to a loopback-bound service, so trusting it by + * default is an authentication-bypass primitive. Consumers must opt in + * explicitly, and only once the upstream proxy unconditionally strips the + * inbound header and re-sets it from a verified client certificate, e.g.: * * bool certAuthTrusted() override { * return isLoopback() && std::getenv("TRUST_CERT_DN") != nullptr; @@ -45,7 +51,7 @@ public: * `X-SSL-Client-DN` header and falls through to token / session auth. */ virtual bool certAuthTrusted() { - return isLoopback(); + return false; } }; diff --git a/include/oatpp-authkit/mail/SmtpTransport.hpp b/include/oatpp-authkit/mail/SmtpTransport.hpp index 129e25b..176101e 100644 --- a/include/oatpp-authkit/mail/SmtpTransport.hpp +++ b/include/oatpp-authkit/mail/SmtpTransport.hpp @@ -34,6 +34,15 @@ struct SmtpConfig { std::string password; }; +/** @brief True if `s` contains CR, LF or NUL — characters that would let a + * caller-influenced address smuggle extra SMTP/MIME headers (BCC injection, + * added recipients, body injection) when concatenated into a header line. */ +inline bool hasHeaderInjectionChars(const std::string& s) { + return s.find('\r') != std::string::npos + || s.find('\n') != std::string::npos + || s.find('\0') != std::string::npos; +} + /** @brief RFC 4648 Base64 encode — used for RFC 2047 Subject headers. */ inline std::string base64Encode(const std::string& data) { static const char* table = @@ -76,6 +85,13 @@ inline std::string send( if (cfg.host.empty()) return "SMTP not configured (no host)"; if (cfg.fromAddress.empty()) return "SMTP not configured (no from_address)"; + // Reject control characters in the addresses before they reach the envelope + // (MAIL FROM / RCPT TO) and the From:/To: header lines. The subject is safe + // — it is RFC 2047 base64 encoded-word wrapped below — but the addresses are + // concatenated raw, so a `\r\n` here would inject arbitrary headers. + if (hasHeaderInjectionChars(to)) return "invalid recipient address (control characters)"; + if (hasHeaderInjectionChars(cfg.fromAddress)) return "invalid from address (control characters)"; + CURL* curl = curl_easy_init(); if (!curl) return "curl_easy_init failed"; diff --git a/include/oatpp-authkit/repo/ScopeGuardRepository.hpp b/include/oatpp-authkit/repo/ScopeGuardRepository.hpp index 0e23b39..4b01793 100644 --- a/include/oatpp-authkit/repo/ScopeGuardRepository.hpp +++ b/include/oatpp-authkit/repo/ScopeGuardRepository.hpp @@ -4,6 +4,7 @@ #include "oatpp-authkit/repo/Repository.hpp" #include "oatpp-authkit/repo/ActorContext.hpp" #include "oatpp-authkit/repo/SchemaContract.hpp" +#include "oatpp-authkit/repo/IQueryable.hpp" #include "oatpp/core/Types.hpp" @@ -40,19 +41,33 @@ public: * error tradeoff: throwing is the safer default — callers that want to * silently 404 instead can catch and translate.) * - `list()`: load from inner; filter out rows the predicate denies. - * - `save(dto)`: predicate evaluated on the incoming dto; deny ⇒ throw. + * - `save(dto)`: predicate must pass on the incoming dto AND, for an update, + * on the row as it currently stands. Checking only the incoming dto would + * let an actor reparent an out-of-scope row into its own scope by setting + * the scope field in the request body (BOLA / set-your-own-scope). The + * existing-row lookup uses the constructor-injected `entityIdOf` accessor. * - `softDelete(id)`: load from inner; if denied, throw; otherwise delegate. * * The actor is provided via a constructor-injected accessor so a single * `ScopeGuardRepository` instance can serve many requests with different * actors (typically the accessor reads from the per-request authenticated * principal — fewo-webapp's `AuthInterceptor` populates one). + * + * @note `IQueryable` is a *separate* data-access surface. Wrapping an + * `IQueryable` repo in this decorator does NOT guard `query()` — a + * caller that obtains the inner queryable would bypass the scope + * predicate entirely. Use `ScopeGuardQueryable` (below) when the + * inner exposes the queryable capability. */ template class ScopeGuardRepository : public Repository { public: - using Predicate = std::function&)>; - using ActorAccess = std::function; + using Predicate = std::function&)>; + using ActorAccess = std::function; + /// Extracts the stable `entity_id` from a DTO (e.g. `[](auto& d){ return d->entity_id; }`). + /// Used to load the existing row on `save()` so an update can't reparent an + /// out-of-scope row. Returns null for a not-yet-allocated entity (fresh insert). + using EntityIdAccess = std::function&)>; /// Declarative schema contribution (authkit#14, D-replace). /// ScopeGuard touches no schema — empty contributions exposed so it @@ -66,10 +81,12 @@ public: ScopeGuardRepository(std::shared_ptr> inner, Predicate isAllowed, - ActorAccess currentActor) + ActorAccess currentActor, + EntityIdAccess entityIdOf) : m_inner(std::move(inner)) , m_isAllowed(std::move(isAllowed)) , m_currentActor(std::move(currentActor)) + , m_entityIdOf(std::move(entityIdOf)) {} oatpp::Object findByEntityId(const oatpp::String& entityId) override { @@ -92,8 +109,25 @@ public: } void save(const oatpp::Object& dto) override { - if (!m_isAllowed(m_currentActor(), dto)) { - throw ScopeDeniedException("scope guard denied save"); + const ActorContext actor = m_currentActor(); + // 1. The incoming DTO must be in scope — you can't write into a scope + // you don't own. + if (!m_isAllowed(actor, dto)) { + throw ScopeDeniedException("scope guard denied save (incoming)"); + } + // 2. If this is an update of an existing entity, the row as it stands + // NOW must also be in scope. Otherwise an actor scoped to A could + // take an entity currently owned by B and reparent it into A simply + // by setting the scope field in the body. A fresh insert has a null + // entity_id (or no matching row) and skips this check. + if (m_entityIdOf) { + auto eid = m_entityIdOf(dto); + if (eid) { + auto existing = m_inner->findByEntityId(eid); + if (existing && !m_isAllowed(actor, existing)) { + throw ScopeDeniedException("scope guard denied save (existing row out of scope)"); + } + } } m_inner->save(dto); } @@ -111,6 +145,64 @@ private: std::shared_ptr> m_inner; Predicate m_isAllowed; ActorAccess m_currentActor; + EntityIdAccess m_entityIdOf; +}; + +/** + * @brief `ScopeGuardRepository` for inners that also expose `IQueryable`. + * + * `ScopeGuardRepository` guards only the four `Repository` methods; the + * `IQueryable::query()` surface is separate, so a scope-guarded `IQueryable` + * repo would otherwise leak every row a raw query returns. This decorator + * closes that hole: it implements `IQueryable`, delegates the CRUD + * methods to an embedded `ScopeGuardRepository` (same predicate / actor / + * entity-id semantics, including the reparenting check), and post-filters + * `query()` results through the predicate exactly like `list()` does. + * + * Wire this — not the plain `ScopeGuardRepository` — whenever the concrete + * inner derives from `IQueryable`. + */ +template +class ScopeGuardQueryable : public IQueryable { +public: + using Predicate = typename ScopeGuardRepository::Predicate; + using ActorAccess = typename ScopeGuardRepository::ActorAccess; + using EntityIdAccess = typename ScopeGuardRepository::EntityIdAccess; + + ScopeGuardQueryable(std::shared_ptr> inner, + Predicate isAllowed, + ActorAccess currentActor, + EntityIdAccess entityIdOf) + : m_inner(std::move(inner)) + , m_isAllowed(isAllowed) + , m_currentActor(currentActor) + , m_guard(m_inner, std::move(isAllowed), std::move(currentActor), std::move(entityIdOf)) + {} + + oatpp::Object findByEntityId(const oatpp::String& entityId) override { + return m_guard.findByEntityId(entityId); + } + oatpp::Vector> list() override { return m_guard.list(); } + void save(const oatpp::Object& dto) override { m_guard.save(dto); } + void softDelete(const oatpp::String& entityId) override { m_guard.softDelete(entityId); } + + /** @brief Run the inner query, then drop every row the predicate denies. */ + oatpp::Vector> query(const Query& q) override { + auto rows = m_inner->query(q); + auto out = oatpp::Vector>::createShared(); + if (!rows) return out; + const ActorContext actor = m_currentActor(); + for (auto& row : *rows) { + if (m_isAllowed(actor, row)) out->push_back(row); + } + return out; + } + +private: + std::shared_ptr> m_inner; + Predicate m_isAllowed; + ActorAccess m_currentActor; + ScopeGuardRepository m_guard; }; } // namespace oatpp_authkit::repo diff --git a/include/oatpp-authkit/repo/TemporalRepository.hpp b/include/oatpp-authkit/repo/TemporalRepository.hpp index da16cba..9b347d2 100644 --- a/include/oatpp-authkit/repo/TemporalRepository.hpp +++ b/include/oatpp-authkit/repo/TemporalRepository.hpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -109,18 +110,31 @@ public: using Clock = std::function; ///< Returns milliseconds since epoch. using IdGen = std::function; + /// Runs a unit of work, ideally inside a DB transaction so the historical + /// insert + live update commit or roll back together. The default just + /// invokes the callback inline (no cross-statement atomicity); consumers + /// that have a connection/transaction handle should pass a runner that + /// wraps the callback in `BEGIN … COMMIT` / `ROLLBACK`. + using TxRunner = std::function&)>; /** * @param inner Concrete adapter that exposes all-rows-including-historical. * @param clock Optional injected clock for tests; default uses system_clock. * @param idgen Optional injected id generator for tests; default is a 32-char hex from mt19937_64. + * @param txRunner Optional transaction wrapper for the close-then-insert + * write pair; default runs the writes inline. A per-instance mutex + * already serialises the read-modify-write within this process so + * concurrent saves of the same entity can't produce two live rows; + * supply a real transaction runner for crash/rollback atomicity. */ explicit TemporalRepository(std::shared_ptr> inner, Clock clock = {}, - IdGen idgen = {}) + IdGen idgen = {}, + TxRunner txRunner = {}) : m_inner(std::move(inner)) , m_clock(clock ? std::move(clock) : defaultClock()) , m_idgen(idgen ? std::move(idgen) : defaultIdGen()) + , m_runTx(txRunner ? std::move(txRunner) : defaultTxRunner()) {} using F = TemporalFieldTraits; @@ -180,6 +194,12 @@ public: void save(const oatpp::Object& dto) override { if (!F::entityId(dto)) F::entityId(dto) = m_idgen(); + // Serialise the read-modify-write so two concurrent saves of the same + // entity can't both observe the same live row and each insert a new + // SENTINEL row (lost update / two live rows). In-process guard only; + // see TxRunner for cross-statement / crash atomicity. + std::lock_guard lock(m_writeMutex); + const int64_t nowMs = m_clock(); const std::string nowIso = isoFromMillis(nowMs); @@ -193,21 +213,26 @@ public: return; } - // Update path: insert a historical copy with a new PK, then - // update the live row in place by its existing PK. + // Update path: compute both rows, then commit the historical copy and + // the in-place live update as one unit of work so a failure between + // the two can't leave a closed-but-not-replaced or duplicate-live row. auto historical = cloneDto(live); F::id(historical) = m_idgen(); F::validUntil(historical) = oatpp::String(nowIso); - m_inner->save(historical); F::id(dto) = F::id(live); // preserve live PK F::validFrom(dto) = oatpp::String(nowIso); F::validUntil(dto) = oatpp::String(SENTINEL); - m_inner->save(dto); + + m_runTx([&] { + m_inner->save(historical); + m_inner->save(dto); + }); } /** @brief Close the live row without inserting a new version. */ void softDelete(const oatpp::String& entityId) override { + std::lock_guard lock(m_writeMutex); auto live = findByEntityId(entityId); if (!live) return; F::validUntil(live) = oatpp::String(isoFromMillis(m_clock())); @@ -247,6 +272,10 @@ private: }; } + static TxRunner defaultTxRunner() { + return [](const std::function& work) { work(); }; + } + /// Field-wise deep copy via oatpp's DTO reflection. Used to capture /// the live row's content as the historical copy before the live row /// is updated in place. @@ -287,6 +316,8 @@ private: std::shared_ptr> m_inner; Clock m_clock; IdGen m_idgen; + TxRunner m_runTx; + std::mutex m_writeMutex; }; } // namespace oatpp_authkit::repo diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e966e70..9d439e6 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -50,6 +50,15 @@ add_executable(test_redacted_field_repository test_redacted_field_repository.cpp target_link_libraries(test_redacted_field_repository PRIVATE oatpp::authkit oatpp::oatpp) add_test(NAME redacted_field_repository COMMAND test_redacted_field_repository) +# SmtpTransport.hpp pulls in and needs libcurl at link time. +# Guard the test so the suite still builds where curl dev headers are absent. +find_package(CURL QUIET) +if(CURL_FOUND) + add_executable(test_smtp_transport test_smtp_transport.cpp) + target_link_libraries(test_smtp_transport PRIVATE oatpp::authkit oatpp::oatpp CURL::libcurl) + add_test(NAME smtp_transport COMMAND test_smtp_transport) +endif() + # RoleTemplateDb pulls in oatpp-sqlite for its DbClient queries. Linking # the test against oatpp::oatpp-sqlite provides the QUERY codegen # definitions; the test itself doesn't open a real DB, only compiles diff --git a/test/test_repository_decorators.cpp b/test/test_repository_decorators.cpp index 88d3645..21f3a45 100644 --- a/test/test_repository_decorators.cpp +++ b/test/test_repository_decorators.cpp @@ -275,7 +275,8 @@ void test_scope_guard_denies_when_predicate_false() { for (auto& as : a.allowedScopes) if (as == s) return true; return false; }, - [actor]{ return actor; }); + [actor]{ return actor; }, + [](const oatpp::Object& d) { return d->entity_id; }); // list filters to allowed rows only. auto allowed = guarded.list(); @@ -310,6 +311,113 @@ void test_scope_guard_denies_when_predicate_false() { REQUIRE(threwOnDelete); } +// Scope predicate + entity-id accessor shared by the reparenting / queryable tests. +static bool scopeAllows(const oatpp_authkit::repo::ActorContext& a, + const oatpp::Object& d) { + if (!d || !d->scope) return false; + const std::string s = std::string(*d->scope); + for (auto& as : a.allowedScopes) if (as == s) return true; + return false; +} +static oatpp::String entityIdOf(const oatpp::Object& d) { return d->entity_id; } + +// An actor scoped to prop-A must NOT be able to reparent an existing prop-B row +// into prop-A by setting scope=prop-A in the body. save() must reject because the +// *existing* row is out of scope, even though the incoming dto looks in-scope. +void test_scope_guard_blocks_reparenting() { + using namespace oatpp_authkit::repo; + auto inner = std::make_shared(); + + // Seed an entity currently owned by prop-B. + auto seeded = MockTemporalDto::createShared(); + seeded->id = oatpp::String("pk-ent1"); + seeded->entity_id = oatpp::String("ent1"); + seeded->valid_from = oatpp::String("2020-01-01T00:00:00Z"); + seeded->valid_until = oatpp::String("9999-12-31T23:59:59Z"); + seeded->scope = oatpp::String("prop-B"); + inner->save(seeded); + + ActorContext actor; + actor.userId = "u1"; + actor.allowedScopes = {"prop-A"}; + + ScopeGuardRepository guarded( + inner, &scopeAllows, [actor]{ return actor; }, &entityIdOf); + + // Attempt to claim ent1 by relabelling it prop-A. + auto reparent = MockTemporalDto::createShared(); + reparent->entity_id = oatpp::String("ent1"); + reparent->scope = oatpp::String("prop-A"); // incoming looks in-scope... + bool blocked = false; + try { guarded.save(reparent); } + catch (const ScopeDeniedException&) { blocked = true; } // ...but existing row is prop-B + REQUIRE(blocked); + + // The stored row is untouched. + auto still = inner->findByEntityId(oatpp::String("ent1")); + REQUIRE(still); + REQUIRE(std::string(*still->scope) == "prop-B"); + + // A genuine insert into the actor's own scope still works (no existing row). + auto fresh = MockTemporalDto::createShared(); + fresh->id = oatpp::String("pk-ent2"); + fresh->entity_id = oatpp::String("ent2"); + fresh->scope = oatpp::String("prop-A"); + fresh->valid_until = oatpp::String("9999-12-31T23:59:59Z"); + bool ok = true; + try { guarded.save(fresh); } catch (const ScopeDeniedException&) { ok = false; } + REQUIRE(ok); +} + +// Minimal IQueryable inner whose query() returns every row, so the test can +// verify ScopeGuardQueryable post-filters results through the predicate. +class InMemoryQueryable : public oatpp_authkit::repo::IQueryable { + std::map> rows; +public: + oatpp::Object findByEntityId(const oatpp::String& id) override { + for (auto& kv : rows) + if (kv.second->entity_id && std::string(*kv.second->entity_id) == std::string(*id)) return kv.second; + return nullptr; + } + oatpp::Vector> list() override { + auto v = oatpp::Vector>::createShared(); + for (auto& kv : rows) v->push_back(kv.second); + return v; + } + void save(const oatpp::Object& dto) override { rows[std::string(*dto->id)] = dto; } + void softDelete(const oatpp::String&) override {} + oatpp::Vector> + query(const oatpp_authkit::repo::Query&) override { + return list(); // pretend the filter ran; the point is the guard filters scope + } +}; + +// query() through ScopeGuardQueryable must drop rows outside the actor's scope — +// otherwise the queryable surface bypasses the scope guard entirely. +void test_scope_guard_queryable_filters_query() { + using namespace oatpp_authkit::repo; + auto inner = std::make_shared(); + for (const char* sc : {"prop-A", "prop-B"}) { + auto dto = MockTemporalDto::createShared(); + dto->id = oatpp::String(std::string("pk-") + sc); + dto->entity_id = oatpp::String(sc); + dto->valid_until = oatpp::String("9999-12-31T23:59:59Z"); + dto->scope = oatpp::String(sc); + inner->save(dto); + } + + ActorContext actor; + actor.userId = "u1"; + actor.allowedScopes = {"prop-A"}; + + ScopeGuardQueryable guarded( + inner, &scopeAllows, [actor]{ return actor; }, &entityIdOf); + + auto result = guarded.query(Query{}); + REQUIRE(result->size() == 1); // prop-B filtered out + REQUIRE(std::string(*(*result)[0]->scope) == "prop-A"); +} + } // namespace int main() { @@ -319,6 +427,8 @@ int main() { test_history_returns_versions_in_order(); test_soft_delete_closes_live_without_new_version(); test_scope_guard_denies_when_predicate_false(); + test_scope_guard_blocks_reparenting(); + test_scope_guard_queryable_filters_query(); std::printf("%s (%d failures)\n", g_failures ? "FAIL" : "OK", g_failures); return g_failures ? 1 : 0; diff --git a/test/test_smtp_transport.cpp b/test/test_smtp_transport.cpp new file mode 100644 index 0000000..ccbe7c7 --- /dev/null +++ b/test/test_smtp_transport.cpp @@ -0,0 +1,75 @@ +// Tests for oatpp-authkit/mail/SmtpTransport.hpp. +// +// Covers the pure, network-free surface: +// - base64Encode against RFC 4648 vectors +// - hasHeaderInjectionChars +// - send() rejects CR/LF/NUL in recipient / from address BEFORE touching +// libcurl (the SMTP header-injection guard) — no live mail server needed, +// the validation short-circuits ahead of curl_easy_init / perform. + +#include "oatpp-authkit/mail/SmtpTransport.hpp" + +#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) + +using namespace oatpp_authkit::mail; + +void test_base64_rfc4648_vectors() { + REQUIRE(base64Encode("") == ""); + REQUIRE(base64Encode("f") == "Zg=="); + REQUIRE(base64Encode("fo") == "Zm8="); + REQUIRE(base64Encode("foo") == "Zm9v"); + REQUIRE(base64Encode("foob") == "Zm9vYg=="); + REQUIRE(base64Encode("fooba") == "Zm9vYmE="); + REQUIRE(base64Encode("foobar") == "Zm9vYmFy"); +} + +void test_header_injection_detector() { + REQUIRE(!hasHeaderInjectionChars("a@b.com")); + REQUIRE( hasHeaderInjectionChars("a@b.com\r\nBcc: evil@x.com")); + REQUIRE( hasHeaderInjectionChars("a@b.com\n")); + REQUIRE( hasHeaderInjectionChars("a@b.com\r")); + REQUIRE( hasHeaderInjectionChars(std::string("a@b.com\0x", 9))); // embedded NUL +} + +void test_send_rejects_crlf_in_addresses() { + SmtpConfig cfg; + cfg.host = "localhost"; + cfg.fromAddress = "noreply@example.com"; + + // CRLF in recipient → rejected with no network call. + std::string r1 = send("victim@example.com\r\nBcc: evil@x.com", + "subject", "

hi

", {}, cfg); + REQUIRE(r1.find("invalid recipient") != std::string::npos); + + // CRLF in from address → rejected. + SmtpConfig cfg2 = cfg; + cfg2.fromAddress = "noreply@example.com\r\nSubject: spoofed"; + std::string r2 = send("victim@example.com", "subject", "

hi

", {}, cfg2); + REQUIRE(r2.find("invalid from") != std::string::npos); + + // Empty-config guards still fire (and come before the address checks). + SmtpConfig empty; + REQUIRE(send("a@b.com", "s", "b", {}, empty).find("no host") != std::string::npos); +} + +} // namespace + +int main() { + test_base64_rfc4648_vectors(); + test_header_injection_detector(); + test_send_rejects_crlf_in_addresses(); + std::printf("%s (%d failures)\n", g_failures ? "FAIL" : "OK", g_failures); + return g_failures ? 1 : 0; +}