diff --git a/.agents/skills/espocrm-guard/SKILL.md b/.agents/skills/espocrm-guard/SKILL.md index 15980d4..f317d33 100644 --- a/.agents/skills/espocrm-guard/SKILL.md +++ b/.agents/skills/espocrm-guard/SKILL.md @@ -8,15 +8,15 @@ description: Use when an agent works in Guard-Core/espocrm-guard or touches its ## Quick Reference - EspoCRM extension "Guard Core", module `GuardCore`, namespace `Espo\Modules\GuardCore\Api`. EspoCRM `>=9.0.0`, PHP `>=8.2`. -- Registers `Espo\Modules\GuardCore\Api\Middleware` as a global API middleware. It does nothing until `guardEnabled` is true in `data/config.php`. -- `guard` is the settings map: every engine option is a `SecurityConfig` argument, every route option a `RouteConfig` argument. Unknown keys throw. Closures and objects cannot be configured. -- `guard` and `guardEnabled` are level `system` in `Resources/metadata/app/config.json`, so the Settings API never returns them. `src/scripts/AfterUninstall.php` removes them on uninstall. +- Registers `Espo\Modules\GuardCore\Api\Middleware` as a global API middleware. It does nothing until `guardCore.enabled` is true in `data/config-internal.php`. +- `guardCore` is the settings map (`enabled` plus every setting): every engine option is a `SecurityConfig` argument, every route option a `RouteConfig` argument. Unknown keys throw. Closures and objects cannot be configured. +- `guardCore`, `guard` and `guardEnabled` are level `system` in `Resources/metadata/app/config.json`, so the Settings API never returns them. `src/scripts/AfterUninstall.php` removes them on uninstall. `guardEnabled` and `guard` are the deprecated 1.0.0 keys: they still work with a warning, and `src/scripts/AfterInstall.php` moves them under `guardCore` on upgrade. Both at once while enabled is an error. - The extension holds no security logic. Every verdict comes from guard-core-php through psr15-guard. - Commands: `make prepare-test`, `make lint`, `make local-test` (Redis on 127.0.0.1:6420), `make test` (Docker), `make extension`, `make bump-version VERSION=x.y.z`. ## Default policy -With only `guardEnabled` set: passive mode, attack screening on, no body scan on `POST /api/v1/Attachment/chunk/`, a ban after more than 10 responses with status 401 in 5 minutes on `GET /api/v1/App/user` and `/api/v1/`, rate limits on `POST /api/v1/LeadCapture/` (10 per minute) and the two password routes (5 per 5 minutes). Operator keys override key by key, a `null` route entry switches a default route off. +With only `guardCore.enabled` set: passive mode, attack screening on, no body scan on `POST /api/v1/Attachment/chunk/`, a ban after more than 10 responses with status 401 in 5 minutes on `GET /api/v1/App/user` and `/api/v1/`, rate limits on `POST /api/v1/LeadCapture/` (10 per minute) and the two password routes (5 per 5 minutes). Operator keys override key by key, a `null` route entry switches a default route off. ## Gotchas diff --git a/.github/ISSUE_TEMPLATE/bug.yml b/.github/ISSUE_TEMPLATE/bug.yml index 6ad016e..d8c5d14 100644 --- a/.github/ISSUE_TEMPLATE/bug.yml +++ b/.github/ISSUE_TEMPLATE/bug.yml @@ -23,7 +23,7 @@ body: attributes: label: Steps to reproduce placeholder: | - 1. Set the 'guard' config to '...' + 1. Set the 'guardCore' config to '...' 2. Send a request to '...' 3. See error validations: @@ -40,11 +40,11 @@ body: id: config attributes: label: Configuration - description: The `guard` array from data/config.php with secrets (Redis URL, agent API key) masked. + description: The `guardCore` array from data/config-internal.php with secrets (Redis URL, agent API key) masked. render: php placeholder: | - 'guardEnabled' => true, - 'guard' => [ + 'guardCore' => [ + 'enabled' => true, 'passiveMode' => false, ], - type: textarea diff --git a/.github/scripts/smoke-lib.sh b/.github/scripts/smoke-lib.sh index d70a4c9..b5b9db1 100755 --- a/.github/scripts/smoke-lib.sh +++ b/.github/scripts/smoke-lib.sh @@ -25,14 +25,14 @@ expect() { echo "ok: $name -> $code" } -# settings_hide_guard : GET /api/v1/Settings must not carry guard or guardEnabled, for the admin. +# settings_hide_guard : GET /api/v1/Settings must not carry guardCore, guard or guardEnabled, for the admin. settings_hide_guard() { expect "GET /api/v1/Settings answers" 200 -u "$SMOKE_USER:$SMOKE_PASSWORD" "$1/api/v1/Settings" - if grep -Eq '"guard(Enabled)?"' "$SMOKE_BODY"; then - echo "FAIL: the Settings API exposes guard or guardEnabled" + if grep -Eq '"guard(Core|Enabled)?"' "$SMOKE_BODY"; then + echo "FAIL: the Settings API exposes guardCore, guard or guardEnabled" exit 1 fi - echo "ok: guard and guardEnabled are absent from GET /api/v1/Settings" + echo "ok: guardCore, guard and guardEnabled are absent from GET /api/v1/Settings" } # log_contains -- : greps EspoCRM's log files in the espocrm service. diff --git a/.github/workflows/greetings.yml b/.github/workflows/greetings.yml index 07bb5fc..1169f5e 100644 --- a/.github/workflows/greetings.yml +++ b/.github/workflows/greetings.yml @@ -18,7 +18,7 @@ jobs: Thanks for opening your first issue on **espocrm-guard**! To help us triage quickly, please make sure your report includes: - - A clear reproduction (the `guard` config and the request you sent are ideal) + - A clear reproduction (the `guardCore` config and the request you sent are ideal) - Output of `php -v` (the extension requires PHP 8.2 or later) and your EspoCRM version - The extension version and the `rennf93/psr15-guard` and `rennf93/guard-core-php` versions in the module's `vendor/` - Whether EspoCRM runs behind a reverse proxy, and the `guard.trustedProxies` you set diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml index 32e69d2..2e1194e 100644 --- a/.github/workflows/stale.yml +++ b/.github/workflows/stale.yml @@ -38,7 +38,7 @@ jobs: If it's still relevant, please leave a comment, otherwise it will be closed in 14 days. Things that help keep an issue moving: - - A current reproduction (the `guard` config and the request) + - A current reproduction (the `guardCore` config and the request) - Output of `php -v` and your EspoCRM and extension versions - Any new findings since the last activity diff --git a/AGENTS.md b/AGENTS.md index 225db2d..18a7631 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -3,10 +3,10 @@ Guidance for AI agents (including Claude Code) working in this repository. ## Project Overview -Guard-Core/espocrm-guard (https://github.com/Guard-Core/espocrm-guard) is an EspoCRM extension, "Guard Core for EspoCRM". It registers the psr15-guard middleware as a global API middleware (`globalMiddlewareClassNameList`) so the guard-core-php engine screens `public/api/v1/` and the portal API. It is off until `guardEnabled` is true, and the keys `guardEnabled` and `guard` are hidden from the Settings API (level `system`). +Guard-Core/espocrm-guard (https://github.com/Guard-Core/espocrm-guard) is an EspoCRM extension, "Guard Core for EspoCRM". It registers the psr15-guard middleware as a global API middleware (`globalMiddlewareClassNameList`) so the guard-core-php engine screens `public/api/v1/` and the portal API. It is off until `guardCore.enabled` is true. The single root key `guardCore` (a map: `enabled` plus every setting) is hidden from the Settings API (level `system`), like the deprecated 1.0.0 keys `guardEnabled` and `guard`, which still work, log a deprecation warning and are migrated automatically on upgrade by `src/scripts/AfterInstall.php`. - The extension is distributed as a zip (`build/guard-core-.zip`, built by `npm run extension`) that an administrator uploads under Administration > Extensions. It is not on Packagist or any registry. The module `composer.json` is named `rennf93/espocrm-guard-core` and only bundles `rennf93/psr15-guard` and, transitively, `rennf93/guard-core-php`. -- This repository contains NO security logic. Detection, rate limiting, bans, headers and responses live in guard-core-php. The module reads the `guard` config, merges it with a default policy, builds the engine objects and wires them to EspoCRM's logger and config. +- This repository contains NO security logic. Detection, rate limiting, bans, headers and responses live in guard-core-php. The module reads the `guardCore` config, merges it with a default policy, builds the engine objects and wires them to EspoCRM's logger and config. - EspoCRM `>=9.0.0` (`extension.json` `acceptableVersions`) and PHP `>=8.2`. The version lives in `package.json` (and `package-lock.json`); `extension.json` carries none, and the zip name takes it from `package.json`. - Releases are tag driven. `master` is protected: never push to it, never merge into it, never create tags or releases as part of agent work. Releases are immutable, so `release.yml` attaches the zip and its `.sha256` to a DRAFT release that the maintainer publishes. - Docs site: MkDocs Material in `docs/`, versioned with mike (`.mike.yml`), deployed by `docs.yml`. `mkdocs.yml` sets `site_dir: docs_site` because `site/` is the EspoCRM checkout: a default `mkdocs build` would wipe it. @@ -15,7 +15,7 @@ Guard-Core/espocrm-guard (https://github.com/Guard-Core/espocrm-guard) is an Esp - `rennf93/guard-core-php` (https://github.com/Guard-Core/guard-core-php) is the engine. It owns `SecurityConfig`, `RouteConfig`, `GuardEngine`, `RedisHandler`, `BehaviorRule` and every verdict. - `rennf93/psr15-guard` (https://github.com/Guard-Core/psr15-guard) is the PSR-15 adapter this module composes: `GuardMiddleware` takes the engine, PSR-17 factories, the route map and an optional agent handler. The module floors it at `^1.6.1`. -- This repository is the EspoCRM adapter layer: `Api\Middleware` is the EspoCRM middleware, `Api\GuardMiddlewareBuilder` wires logger, Redis, country database and agent, `Api\SecurityConfigBuilder` and `Api\RouteConfigBuilder` map the `guard` array onto `SecurityConfig` and `RouteConfig`, `Api\DefaultPolicy` is the policy applied when `guardEnabled` is true, `Api\GuardAgentFactory` creates the optional agent. +- This repository is the EspoCRM adapter layer: `Api\Middleware` is the EspoCRM middleware, `Api\GuardMiddlewareBuilder` wires logger, Redis, country database and agent, `Api\SecurityConfigBuilder` and `Api\RouteConfigBuilder` map the `guardCore` settings onto `SecurityConfig` and `RouteConfig`, `Api\DefaultPolicy` is the policy applied when the guard is enabled, `Api\GuardAgentFactory` creates the optional agent. - `rennf93/guard-agent-php` is the optional telemetry agent. It is resolved by name and is not a dependency; the operator runs `composer require rennf93/guard-agent-php` in the EspoCRM root. - `psr/http-*` and Slim come from EspoCRM itself. The module `composer.json` `replace`s the four `psr/http-*` packages so none ends up in the zip, and `GuardMiddlewareBuilder` uses EspoCRM's `Slim\Psr7` factories. @@ -24,9 +24,9 @@ Guard-Core/espocrm-guard (https://github.com/Guard-Core/espocrm-guard) is an Esp The extension wires; the engine decides. - MUST NOT implement detection rules, penetration signatures, rate limiting, IP banning, security headers or CORS. The extension only translates config into engine objects and copies nothing but the engine's verdicts. -- MUST stay opt-in: `Middleware::process()` returns `$handler->handle($request)` and builds nothing unless `guardEnabled` is true. -- MUST fail loudly on bad config: an unknown `guard` key, an unknown route option, an invalid behavior rule, an unsupported Redis URL or an `agent` map without the agent package throws instead of being ignored. -- MUST keep `guard` and `guardEnabled` out of every API response: `Resources/metadata/app/config.json` declares both at level `system`, and `src/scripts/AfterUninstall.php` removes both from the config files when the module is uninstalled. +- MUST stay opt-in: `Middleware::process()` returns `$handler->handle($request)` and builds nothing unless `guardCore.enabled` (or the deprecated `guardEnabled`) is true. +- MUST fail loudly on bad config: an unknown `guardCore` key, `guardCore` that is not a map, settings split between `guardCore` and the old keys, an unknown route option, an invalid behavior rule, an unsupported Redis URL or an `agent` map without the agent package throws instead of being ignored. +- MUST keep `guardCore`, `guard` and `guardEnabled` out of every API response: `Resources/metadata/app/config.json` declares all three at level `system`, and `src/scripts/AfterUninstall.php` removes all three from the config files when the module is uninstalled. - MUST NOT bundle `psr/*` or Slim in the zip. `.github/scripts/check-extension-zip.sh` checks the zip contents and size on every CI build and release. - MUST NOT log or store anything beyond the engine verdict lines (check name, reason, client IP, method, path, status) it writes through EspoCRM's logger. @@ -59,9 +59,10 @@ The extension wires; the engine decides. src/files/custom/Espo/Modules/GuardCore/Api/ the module classes src/files/custom/Espo/Modules/GuardCore/Resources/ module.json, autoload.json, metadata/app/{api,config}.json src/files/custom/Espo/Modules/GuardCore/composer.json, composer.lock module libraries (the lock is what ships) -src/scripts/AfterUninstall.php removes guardEnabled and guard on uninstall +src/scripts/AfterInstall.php migrates guardEnabled and guard into guardCore on install and upgrade +src/scripts/AfterUninstall.php removes guardCore, guardEnabled and guard on uninstall .agents/skills/espocrm-guard/SKILL.md agent skill (outside src/, so it is not in the zip) -tests/unit/Espo/Modules/GuardCore/ PHPUnit suite (MiddlewareTest, GuardAgentFactoryTest, AfterUninstallTest) +tests/unit/Espo/Modules/GuardCore/ PHPUnit suite (MiddlewareTest, GuardAgentFactoryTest, AfterInstallTest, AfterUninstallTest) php_scripts/, build.js, config-default.json espo-extension-tools helpers and build examples/simple_app, examples/advanced_app Docker Compose stacks with a real EspoCRM and the built zip docs/, mkdocs.yml, .mike.yml docs site diff --git a/CHANGELOG.md b/CHANGELOG.md index 525ecc6..2482901 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,36 @@ Release Notes ___ +v1.1.0 (2026-10-31) +------------------- + +One extension-owned config key, `guardCore`, with an automatic migration (v1.1.0) +--------------------------------------------------------------------------------- + +### Added + +- **One root config key, `guardCore`.** It is a map that holds `enabled` plus every setting that lived in `guard`, Redis settings included, so nothing collides with the config keys of other extensions. It is declared at level `system`, so the Settings API never returns it, and EspoCRM stores it in `data/config-internal.php`. +- **An automatic migration on upgrade.** A new `AfterInstall` script moves `guardEnabled` and `guard` into `guardCore` (`enabled` first, then the settings) and removes the old keys. It changes nothing on a fresh install, when `guardCore` is already set, or when `guard` is not a map (the middleware reports that one loudly). Running it twice changes nothing. +- **A docs page on route policies** (#15): how a request finds its route, what switching a default route off does, and every route option and behavior rule key with its unit. + +### Changed + +- **`guardEnabled` and `guard` are deprecated.** They keep working, and the guard logs one warning when it is built, pointing to docs/configuration.md. Setting `guardCore` and an old key at the same time while the guard is on throws an error that asks to move everything under `guardCore`. When both say the guard is off, nothing happens. +- **The uninstall also removes `guardCore`.** An upgrade does not run the old version's uninstall script, so the migration is what moves the old keys. +- **The docs, the examples and the live smoke checks use `guardCore`.** The configuration docs say to put it in `data/config-internal.php`, because EspoCRM stores level `system` parameters there and rewrites them there on any config save. + +### Fixed + +- **Uninstall and the migration now clear the old keys from both config files.** On EspoCRM 9.0 to 9.2, removing a level `system` parameter only touched `data/config-internal.php`, so uninstalling left `guardEnabled` and `guard` in `data/config.php`, where they lose their `system` level and the Settings API can return them. The migration had the same gap and would have left them next to `guardCore`. Both scripts now remove them from both files. +- **The migration leaves old keys in an override file alone.** EspoCRM never writes `data/config-override.php` or `data/config-internal-override.php`, so when either holds `guardEnabled` or `guard` nothing is migrated and the old keys keep working with the deprecation warning. + +### Test + +- New unit tests run both scripts against EspoCRM's real `Config` and `ConfigWriter` and assert the file contents (keys in `config.php`, in `config-internal.php`, split across both, in an override file, a fresh install, a second run and an uninstall). +- New unit tests cover the `guardCore` resolution (enabled, disabled, missing `enabled`, not a bool, not a map, split settings), the deprecation warning once per build, every case of the migration including a second run, and the three-key uninstall. Line coverage of the module stays at 100%. + +___ + v1.0.0 (2026-10-20) ------------------- diff --git a/CLAUDE.md b/CLAUDE.md index 225db2d..18a7631 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -3,10 +3,10 @@ Guidance for AI agents (including Claude Code) working in this repository. ## Project Overview -Guard-Core/espocrm-guard (https://github.com/Guard-Core/espocrm-guard) is an EspoCRM extension, "Guard Core for EspoCRM". It registers the psr15-guard middleware as a global API middleware (`globalMiddlewareClassNameList`) so the guard-core-php engine screens `public/api/v1/` and the portal API. It is off until `guardEnabled` is true, and the keys `guardEnabled` and `guard` are hidden from the Settings API (level `system`). +Guard-Core/espocrm-guard (https://github.com/Guard-Core/espocrm-guard) is an EspoCRM extension, "Guard Core for EspoCRM". It registers the psr15-guard middleware as a global API middleware (`globalMiddlewareClassNameList`) so the guard-core-php engine screens `public/api/v1/` and the portal API. It is off until `guardCore.enabled` is true. The single root key `guardCore` (a map: `enabled` plus every setting) is hidden from the Settings API (level `system`), like the deprecated 1.0.0 keys `guardEnabled` and `guard`, which still work, log a deprecation warning and are migrated automatically on upgrade by `src/scripts/AfterInstall.php`. - The extension is distributed as a zip (`build/guard-core-.zip`, built by `npm run extension`) that an administrator uploads under Administration > Extensions. It is not on Packagist or any registry. The module `composer.json` is named `rennf93/espocrm-guard-core` and only bundles `rennf93/psr15-guard` and, transitively, `rennf93/guard-core-php`. -- This repository contains NO security logic. Detection, rate limiting, bans, headers and responses live in guard-core-php. The module reads the `guard` config, merges it with a default policy, builds the engine objects and wires them to EspoCRM's logger and config. +- This repository contains NO security logic. Detection, rate limiting, bans, headers and responses live in guard-core-php. The module reads the `guardCore` config, merges it with a default policy, builds the engine objects and wires them to EspoCRM's logger and config. - EspoCRM `>=9.0.0` (`extension.json` `acceptableVersions`) and PHP `>=8.2`. The version lives in `package.json` (and `package-lock.json`); `extension.json` carries none, and the zip name takes it from `package.json`. - Releases are tag driven. `master` is protected: never push to it, never merge into it, never create tags or releases as part of agent work. Releases are immutable, so `release.yml` attaches the zip and its `.sha256` to a DRAFT release that the maintainer publishes. - Docs site: MkDocs Material in `docs/`, versioned with mike (`.mike.yml`), deployed by `docs.yml`. `mkdocs.yml` sets `site_dir: docs_site` because `site/` is the EspoCRM checkout: a default `mkdocs build` would wipe it. @@ -15,7 +15,7 @@ Guard-Core/espocrm-guard (https://github.com/Guard-Core/espocrm-guard) is an Esp - `rennf93/guard-core-php` (https://github.com/Guard-Core/guard-core-php) is the engine. It owns `SecurityConfig`, `RouteConfig`, `GuardEngine`, `RedisHandler`, `BehaviorRule` and every verdict. - `rennf93/psr15-guard` (https://github.com/Guard-Core/psr15-guard) is the PSR-15 adapter this module composes: `GuardMiddleware` takes the engine, PSR-17 factories, the route map and an optional agent handler. The module floors it at `^1.6.1`. -- This repository is the EspoCRM adapter layer: `Api\Middleware` is the EspoCRM middleware, `Api\GuardMiddlewareBuilder` wires logger, Redis, country database and agent, `Api\SecurityConfigBuilder` and `Api\RouteConfigBuilder` map the `guard` array onto `SecurityConfig` and `RouteConfig`, `Api\DefaultPolicy` is the policy applied when `guardEnabled` is true, `Api\GuardAgentFactory` creates the optional agent. +- This repository is the EspoCRM adapter layer: `Api\Middleware` is the EspoCRM middleware, `Api\GuardMiddlewareBuilder` wires logger, Redis, country database and agent, `Api\SecurityConfigBuilder` and `Api\RouteConfigBuilder` map the `guardCore` settings onto `SecurityConfig` and `RouteConfig`, `Api\DefaultPolicy` is the policy applied when the guard is enabled, `Api\GuardAgentFactory` creates the optional agent. - `rennf93/guard-agent-php` is the optional telemetry agent. It is resolved by name and is not a dependency; the operator runs `composer require rennf93/guard-agent-php` in the EspoCRM root. - `psr/http-*` and Slim come from EspoCRM itself. The module `composer.json` `replace`s the four `psr/http-*` packages so none ends up in the zip, and `GuardMiddlewareBuilder` uses EspoCRM's `Slim\Psr7` factories. @@ -24,9 +24,9 @@ Guard-Core/espocrm-guard (https://github.com/Guard-Core/espocrm-guard) is an Esp The extension wires; the engine decides. - MUST NOT implement detection rules, penetration signatures, rate limiting, IP banning, security headers or CORS. The extension only translates config into engine objects and copies nothing but the engine's verdicts. -- MUST stay opt-in: `Middleware::process()` returns `$handler->handle($request)` and builds nothing unless `guardEnabled` is true. -- MUST fail loudly on bad config: an unknown `guard` key, an unknown route option, an invalid behavior rule, an unsupported Redis URL or an `agent` map without the agent package throws instead of being ignored. -- MUST keep `guard` and `guardEnabled` out of every API response: `Resources/metadata/app/config.json` declares both at level `system`, and `src/scripts/AfterUninstall.php` removes both from the config files when the module is uninstalled. +- MUST stay opt-in: `Middleware::process()` returns `$handler->handle($request)` and builds nothing unless `guardCore.enabled` (or the deprecated `guardEnabled`) is true. +- MUST fail loudly on bad config: an unknown `guardCore` key, `guardCore` that is not a map, settings split between `guardCore` and the old keys, an unknown route option, an invalid behavior rule, an unsupported Redis URL or an `agent` map without the agent package throws instead of being ignored. +- MUST keep `guardCore`, `guard` and `guardEnabled` out of every API response: `Resources/metadata/app/config.json` declares all three at level `system`, and `src/scripts/AfterUninstall.php` removes all three from the config files when the module is uninstalled. - MUST NOT bundle `psr/*` or Slim in the zip. `.github/scripts/check-extension-zip.sh` checks the zip contents and size on every CI build and release. - MUST NOT log or store anything beyond the engine verdict lines (check name, reason, client IP, method, path, status) it writes through EspoCRM's logger. @@ -59,9 +59,10 @@ The extension wires; the engine decides. src/files/custom/Espo/Modules/GuardCore/Api/ the module classes src/files/custom/Espo/Modules/GuardCore/Resources/ module.json, autoload.json, metadata/app/{api,config}.json src/files/custom/Espo/Modules/GuardCore/composer.json, composer.lock module libraries (the lock is what ships) -src/scripts/AfterUninstall.php removes guardEnabled and guard on uninstall +src/scripts/AfterInstall.php migrates guardEnabled and guard into guardCore on install and upgrade +src/scripts/AfterUninstall.php removes guardCore, guardEnabled and guard on uninstall .agents/skills/espocrm-guard/SKILL.md agent skill (outside src/, so it is not in the zip) -tests/unit/Espo/Modules/GuardCore/ PHPUnit suite (MiddlewareTest, GuardAgentFactoryTest, AfterUninstallTest) +tests/unit/Espo/Modules/GuardCore/ PHPUnit suite (MiddlewareTest, GuardAgentFactoryTest, AfterInstallTest, AfterUninstallTest) php_scripts/, build.js, config-default.json espo-extension-tools helpers and build examples/simple_app, examples/advanced_app Docker Compose stacks with a real EspoCRM and the built zip docs/, mkdocs.yml, .mike.yml docs site diff --git a/README.md b/README.md index 78b0d5a..45810d9 100644 --- a/README.md +++ b/README.md @@ -169,15 +169,15 @@ The extension registers one global API middleware (`globalMiddlewareClassNameLis 1. Download `guard-core-.zip` from the [latest release](https://github.com/Guard-Core/espocrm-guard/releases/latest) (or build it, see Testing). 2. In EspoCRM open Administration > Extensions, upload the zip and click Install. -Installing changes nothing: `guardEnabled` is absent, so the middleware passes every request straight through. Uninstalling removes the middleware and also deletes `guardEnabled` and `guard` from the config files. That is on purpose: the extension hides `guard` (it can hold a Redis URL and an agent API key) from the Settings API, and once it is gone nothing would hide those values from logged-in users. Keep a copy of your `guard` block if you plan to reinstall. +Installing changes nothing: `guardCore` is absent, so the middleware passes every request straight through. Upgrading from 1.0.0 moves `guardEnabled` and `guard` under `guardCore` automatically. Uninstalling removes the middleware and also deletes `guardCore` (and the old `guardEnabled` and `guard`) from the config files. That is on purpose: the extension hides `guardCore` (it can hold a Redis URL and an agent API key) from the Settings API, and once it is gone nothing would hide those values from logged-in users. Keep a copy of your `guardCore` block if you plan to reinstall. ## Usage -Add the keys to `data/config.php` (or `data/config-internal.php`). They are not editable in the admin UI and are never sent to the front end. +Add one key, `guardCore`, to `data/config-internal.php` (EspoCRM stores level `system` parameters there and rewrites them there on any config save). It is not editable in the admin UI and is never sent to the front end. ```php -'guardEnabled' => true, -'guard' => [ +'guardCore' => [ + 'enabled' => true, 'enableRedis' => true, 'redisUrl' => 'redis://127.0.0.1:6379', 'redisPrefix' => 'espocrm_guard:', @@ -190,7 +190,7 @@ Add the keys to `data/config.php` (or `data/config-internal.php`). They are not Clear the cache after changing the config (`php command.php clear-cache`). -With only `guardEnabled` set to true you get the default policy: +With only `'enabled' => true` set you get the default policy: - Passive mode: what would be blocked is logged, nothing is blocked. - Attack screening of paths, query strings, headers and JSON or form bodies is on. @@ -198,7 +198,7 @@ With only `guardEnabled` set to true you get the default policy: - A ban rule after more than 10 responses with status 401 in 5 minutes on `GET /api/v1/App/user` and on every other API path (counted per path). - Rate limits on `POST /api/v1/LeadCapture/` (10 per minute) and on the two password routes (5 per 5 minutes). -Start in passive mode, watch `data/logs` for `Guard would block by ...` lines, then set `'passiveMode' => false`. Settings you give override the defaults key by key. A route key is an optional method plus a path, and a `null` route entry switches a default route off. Every engine option is a key of `guard` (the named arguments of the engine's `SecurityConfig`), and every route option a key of a route (`RouteConfig`). An unknown key or an invalid route option throws instead of being ignored. Options that need a closure or an object (`onBlock`, `authVerifier`, `customRequestCheck` and similar) cannot be expressed in a config file and are rejected. +Start in passive mode, watch `data/logs` for `Guard would block by ...` lines, then set `'passiveMode' => false`. Settings you give override the defaults key by key. A route key is an optional method plus a path, and a `null` route entry switches a default route off. Every other engine option is a key of `guardCore` (the named arguments of the engine's `SecurityConfig`), and every route option a key of a route (`RouteConfig`). An unknown key or an invalid route option throws instead of being ignored. Options that need a closure or an object (`onBlock`, `authVerifier`, `customRequestCheck` and similar) cannot be expressed in a config file and are rejected. ### Redis @@ -206,7 +206,7 @@ Redis is off by default. Without it, rate limits, behavior rules and bans keep t ### Trusted proxies and `ipAddressServerParam` -The guard identifies the client by `REMOTE_ADDR`. EspoCRM's `ipAddressServerParam` is not read. If you set it to something else (for example `HTTP_X_FORWARDED_FOR`), the guard logs a warning, and behind a proxy you must set `guard.trustedProxies` (and `guard.trustedProxyDepth`) so the client is resolved from the forwarded header. Without that every client behind the proxy looks like the proxy. +The guard identifies the client by `REMOTE_ADDR`. EspoCRM's `ipAddressServerParam` is not read. If you set it to something else (for example `HTTP_X_FORWARDED_FOR`), the guard logs a warning, and behind a proxy you must set `guardCore.trustedProxies` (and `guardCore.trustedProxyDepth`) so the client is resolved from the forwarded header. Without that every client behind the proxy looks like the proxy. ### Examples @@ -220,7 +220,7 @@ The [guard-agent-php](https://packagist.org/packages/rennf93/guard-agent-php) ag composer require rennf93/guard-agent-php ``` -then add an `agent` map to `guard`. The extension loads the agent only when the package is present, and fails with a clear message when `agent` is configured without it. An EspoCRM upgrade replaces `vendor/`, so run the command again after upgrading. +then add an `agent` map to `guardCore`. The extension loads the agent only when the package is present, and fails with a clear message when `agent` is configured without it. An EspoCRM upgrade replaces `vendor/`, so run the command again after upgrading. ## What it covers diff --git a/SECURITY.md b/SECURITY.md index 7e534b2..d4fc5ca 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -35,4 +35,4 @@ I will acknowledge your report within 48 hours and send a detailed response with Guard Core for EspoCRM is a security extension: report anything that weakens detection, bypasses rate limiting or bans, leaks secrets into logs or responses, or trusts unvalidated input. -The `guard` config key can hold a Redis URL and a telemetry agent API key. The extension declares `guard` and `guardEnabled` at level `system` so EspoCRM does not return them from `GET /api/v1/Settings`, for any user. Uninstalling the extension deletes both keys from the config files for the same reason: once the extension is gone, nothing would hide them. A report that shows `guard` or `guardEnabled` reaching any API response or the front end is a vulnerability. +The `guardCore` config key can hold a Redis URL and a telemetry agent API key. The extension declares `guardCore` (and the deprecated `guardEnabled` and `guard`) at level `system` so EspoCRM does not return them from `GET /api/v1/Settings`, for any user. Uninstalling the extension deletes the keys from the config files for the same reason: once the extension is gone, nothing would hide them. A report that shows `guardCore`, `guardEnabled` or `guard` reaching any API response or the front end is a vulnerability. diff --git a/docs/configuration.md b/docs/configuration.md index fc99b5c..095727a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1,10 +1,10 @@ # Configuration -Add the keys to `data/config.php` (or `data/config-internal.php`). They are not editable in the admin UI and are never sent to the front end. +Add one key, `guardCore`, to `data/config-internal.php`. EspoCRM stores level `system` config parameters there and rewrites them there on any config save, so a copy kept in `data/config.php` can end up shadowed or moved. The key is not editable in the admin UI and is never sent to the front end. It is the only root key the extension owns, so nothing collides with other extensions. ```php -'guardEnabled' => true, -'guard' => [ +'guardCore' => [ + 'enabled' => true, 'enableRedis' => true, 'redisUrl' => 'redis://127.0.0.1:6379', 'redisPrefix' => 'espocrm_guard:', @@ -17,7 +17,16 @@ Add the keys to `data/config.php` (or `data/config-internal.php`). They are not Clear the cache after changing the config (`php command.php clear-cache`). -Settings you give override the defaults key by key. A route key is an optional method plus a path, and a `null` route entry switches a default route off. Every engine option is a key of `guard` (the named arguments of the engine's `SecurityConfig`), and every route option a key of a route (`RouteConfig`). An unknown key or an invalid route option throws instead of being ignored. Options that need a closure or an object (`onBlock`, `authVerifier`, `customRequestCheck` and similar) cannot be expressed in a config file and are rejected. +Settings you give override the defaults key by key. A route key is an optional method plus a path, and a `null` route entry switches a default route off. `enabled` switches the guard on (the default is off). Every other engine option is a key of `guardCore` (the named arguments of the engine's `SecurityConfig`), and every route option a key of a route (`RouteConfig`). An unknown key or an invalid route option throws instead of being ignored. Options that need a closure or an object (`onBlock`, `authVerifier`, `customRequestCheck` and similar) cannot be expressed in a config file and are rejected. + +## Upgrading from 1.0.0 + +Version 1.0.0 used two root keys, `guardEnabled` and `guard`. Upgrading the extension moves them automatically when they are in `data/config.php` or `data/config-internal.php`, which is where the 1.0.0 docs said to put them: the upgrade runs a script that writes `guardCore` into `data/config-internal.php` with `enabled` set from `guardEnabled` and every setting of `guard`, then removes the old keys from both files. + +- Old keys in `data/config-override.php` or `data/config-internal-override.php` are left alone, because EspoCRM never writes those files. The upgrade changes nothing while either one holds `guardEnabled` or `guard`, and the old keys keep working with the deprecation warning until you move them under `guardCore` yourself. +- Old keys that you add by hand later still work. The guard logs one deprecation warning when it starts, naming this page. +- Setting `guardCore` and one of the old keys at the same time while the guard is on is an error: the middleware throws and says to move everything under `guardCore` and remove the old keys. When both say the guard is off, nothing happens. +- If `guard` is not a map, the upgrade leaves the config as it is, and the middleware reports it loudly when the guard is on. ## Redis @@ -25,7 +34,7 @@ Redis is off by default. Without it, rate limits, behavior rules and bans keep t ## Trusted proxies and `ipAddressServerParam` -The guard identifies the client by `REMOTE_ADDR`. EspoCRM's `ipAddressServerParam` is not read. If you set it to something else (for example `HTTP_X_FORWARDED_FOR`), the guard logs a warning, and behind a proxy you must set `guard.trustedProxies` (and `guard.trustedProxyDepth`) so the client is resolved from the forwarded header. Without that every client behind the proxy looks like the proxy. +The guard identifies the client by `REMOTE_ADDR`. EspoCRM's `ipAddressServerParam` is not read. If you set it to something else (for example `HTTP_X_FORWARDED_FOR`), the guard logs a warning, and behind a proxy you must set `guardCore.trustedProxies` (and `guardCore.trustedProxyDepth`) so the client is resolved from the forwarded header. Without that every client behind the proxy looks like the proxy. ## Country filtering @@ -33,4 +42,4 @@ The optional `ipinfo` map (`token`, and optionally `dbPath` and `maxAge`) turns ## Secrets -`guard` can hold a Redis URL and an agent API key. The extension declares both config keys at level `system`, so EspoCRM never returns them from `GET /api/v1/Settings`, for any user. +`guardCore` can hold a Redis URL and an agent API key. The extension declares it (and the deprecated `guardEnabled` and `guard`) at level `system`, so EspoCRM never returns them from `GET /api/v1/Settings`, for any user. diff --git a/docs/index.md b/docs/index.md index 6be48e3..b622fbf 100644 --- a/docs/index.md +++ b/docs/index.md @@ -6,7 +6,7 @@ It screens API requests for attacks, bans clients that misbehave, applies per-ro The extension registers one global API middleware (`globalMiddlewareClassNameList`), so it covers `public/api/v1/` and the portal API. The guard runs inside the application, so it sees what a perimeter device cannot: the parsed JSON body, the route and the response status. It complements a reverse proxy or WAF, it does not replace one. -This extension holds no security logic. Detection, rate limiting, bans, headers and responses live in the engine. The extension reads the `guard` config, wires the engine to EspoCRM's logger and config, and registers the middleware. +This extension holds no security logic. Detection, rate limiting, bans, headers and responses live in the engine. The extension reads the `guardCore` config, wires the engine to EspoCRM's logger and config, and registers the middleware. ## Requirements @@ -17,8 +17,8 @@ This extension holds no security logic. Detection, rate limiting, bans, headers ## Where to go next - [Installation](installation.md): the zip, the upload and what uninstalling does. -- [Configuration](configuration.md): the `guard` keys, Redis, trusted proxies. -- [Usage and default policy](usage.md): what you get with only `guardEnabled` set, and how to roll out from passive mode. +- [Configuration](configuration.md): the `guardCore` keys, Redis, trusted proxies. +- [Usage and default policy](usage.md): what you get with only `guardCore.enabled` set, and how to roll out from passive mode. - [Route policies](routes.md): how routes match, every route option, behavior rules (types, patterns, units, actions). - [Telemetry agent](telemetry-agent.md): the optional agent. - [What it covers and does not](coverage.md): the entry points and the known limits. diff --git a/docs/installation.md b/docs/installation.md index acc266a..8d9bca4 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -6,11 +6,15 @@ From the command line the same install is `php command.php extension --file=guard-core-.zip`, run in the EspoCRM root directory. -Installing changes nothing: `guardEnabled` is absent, so the middleware passes every request straight through. See [Usage and default policy](usage.md) to turn it on. +Installing changes nothing: `guardCore` is absent, so the middleware passes every request straight through. See [Usage and default policy](usage.md) to turn it on. ## Uninstall -Uninstalling removes the middleware and also deletes `guardEnabled` and `guard` from the config files. That is on purpose: the extension hides `guard` (it can hold a Redis URL and an agent API key) from the Settings API, and once it is gone nothing would hide those values from logged-in users. Keep a copy of your `guard` block if you plan to reinstall. +Uninstalling removes the middleware and also deletes `guardCore` (and the old `guardEnabled` and `guard`) from the config files. That is on purpose: the extension hides `guardCore` (it can hold a Redis URL and an agent API key) from the Settings API, and once it is gone nothing would hide those values from logged-in users. Keep a copy of your `guardCore` block if you plan to reinstall. + +## Upgrade + +Upgrading from 1.0.0 moves `guardEnabled` and `guard` under `guardCore` automatically. See [Configuration](configuration.md#upgrading-from-100). ## Build from source diff --git a/docs/routes.md b/docs/routes.md index 4c0e484..d482dc1 100644 --- a/docs/routes.md +++ b/docs/routes.md @@ -1,16 +1,17 @@ # Route policies -A route policy is a set of extra rules for one API path: a rate limit, a ban rule, a size limit, a scan exclusion. You add them under `guard.routes` in `data/config.php` and clear the cache (`php command.php clear-cache`). +A route policy is a set of extra rules for one API path: a rate limit, a ban rule, a size limit, a scan exclusion. You add them under `guardCore.routes` in `data/config-internal.php` and clear the cache (`php command.php clear-cache`). ```php -'guard' => [ +'guardCore' => [ + 'enabled' => true, 'routes' => [ 'POST /api/v1/LeadCapture/' => ['rateLimit' => 3, 'rateLimitWindow' => 60], ], ], ``` -Requests that match no route policy still get everything set at the `guard` level (attack screening, bans, the global rate limit). +Requests that match no route policy still get everything set at the `guardCore` level (attack screening, bans, the global rate limit). ## How a request finds its policy @@ -21,7 +22,7 @@ Requests that match no route policy still get everything set at the `guard` leve ## Default routes -With only `guardEnabled` set, these routes are active: +With only `guardCore.enabled` set, these routes are active: | Route | What it does | | --- | --- | @@ -38,7 +39,8 @@ With only `guardEnabled` set, these routes are active: - A route set to `null` switches the default route off: the extension removes it, and requests to that path fall back to the next matching route, usually `/api/v1/`. ```php -'guard' => [ +'guardCore' => [ + 'enabled' => true, 'routes' => [ // Lead capture: 3 requests per minute instead of 10. 'POST /api/v1/LeadCapture/' => ['rateLimit' => 3, 'rateLimitWindow' => 60], @@ -82,7 +84,7 @@ Patterns for `return_pattern`: | `regex:...` | A response body that matches the regular expression, case insensitive | | any other text | A response body that contains the text, case insensitive | -Only `status:` patterns work out of the box. The other three read the response body, so they need `'behaviorScanResponseBody' => true` at the `guard` level; without it a route rule with such a pattern never matches. The body is read up to 262144 bytes (`behaviorMaxResponseBodyInspectBytes`). +Only `status:` patterns work out of the box. The other three read the response body, so they need `'behaviorScanResponseBody' => true` at the `guardCore` level; without it a route rule with such a pattern never matches. The body is read up to 262144 bytes (`behaviorMaxResponseBodyInspectBytes`). Actions: @@ -99,7 +101,7 @@ Things to know: - The rule acts after the request that crosses the threshold is answered: that request goes through, the ban applies from the next one. - In passive mode (the default) nothing is banned. You get a `[PASSIVE MODE] Would ban IP ...` line instead. - Without Redis the counts start from zero on every request under PHP-FPM, so a rule never fires. Set `enableRedis` and `redisUrl` (see [Configuration](configuration.md#redis)). -- `correlate_with_detection` is accepted but only changes global rules (`globalBehaviorRules` at the `guard` level), not route rules. +- `correlate_with_detection` is accepted but only changes global rules (`globalBehaviorRules` at the `guardCore` level), not route rules. ## All route options @@ -129,7 +131,7 @@ Every key below goes inside a route, next to `behaviorRules`. An unknown key mak | `blockedUserAgents` | `['sqlmap', 'python-requests']` | A `User-Agent` that matches one of these regular expressions (case insensitive, anywhere in the text) gets 403. Adds to the global list | | `timeRestrictions` | `['start' => '09:00', 'end' => '18:00', 'timezone' => 'Europe/Rome']` | Outside the window, 403. 24 hour `HH:MM` with leading zeros, timezone optional (default UTC). A start after the end is an overnight window | | `requireReferrer` | `['example.com']` | A request without `Referer` or with a `Referer` from another domain gets 403. Subdomains count | -| `requireHttps` | `true` | A plain HTTP request gets a 301 to the https URL. Behind a TLS proxy set `trustXForwardedProto` and `trustedProxies` at the `guard` level, or every request looks like HTTP | +| `requireHttps` | `true` | A plain HTTP request gets a 301 to the https URL. Behind a TLS proxy set `trustXForwardedProto` and `trustedProxies` at the `guardCore` level, or every request looks like HTTP | Client IPs come from `REMOTE_ADDR` unless `trustedProxies` is set (see [Configuration](configuration.md#trusted-proxies-and-ipaddressserverparam)). diff --git a/docs/telemetry-agent.md b/docs/telemetry-agent.md index 3761535..06b4cf9 100644 --- a/docs/telemetry-agent.md +++ b/docs/telemetry-agent.md @@ -6,10 +6,11 @@ The [guard-agent-php](https://packagist.org/packages/rennf93/guard-agent-php) ag composer require rennf93/guard-agent-php ``` -then add an `agent` map to `guard`: +then add an `agent` map to `guardCore`: ```php -'guard' => [ +'guardCore' => [ + 'enabled' => true, 'agent' => ['apiKey' => 'your-project-api-key'], ], ``` @@ -18,4 +19,4 @@ The extension loads the agent only when the package is present, and fails with a An EspoCRM upgrade replaces `vendor/`, so run the `composer require` command again after upgrading. -The agent API key lives in `guard`, which is hidden from the Settings API. +The agent API key lives in `guardCore`, which is hidden from the Settings API. diff --git a/docs/usage.md b/docs/usage.md index 18d0835..0d22c55 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -1,6 +1,6 @@ # Usage and default policy -With only `guardEnabled` set to true you get the default policy: +With only `'enabled' => true` under `guardCore` you get the default policy: - Passive mode: what would be blocked is logged, nothing is blocked. - Attack screening of paths, query strings, headers and JSON or form bodies is on. @@ -10,7 +10,7 @@ With only `guardEnabled` set to true you get the default policy: ## Rolling out -1. Set `'guardEnabled' => true` and clear the cache. +1. Set `'guardCore' => ['enabled' => true]` and clear the cache. 2. Watch `data/logs` for `Guard would block by ...` lines. Each line names the check, the reason and the client, and the request still passes. 3. Fix or exclude what is a false positive, for example with a route policy. 4. Set `'passiveMode' => false`. Blocked requests now log `Guard blocked by ...` and get the engine's response (400 for a detected attack, 403 for a banned client, 429 for a rate limit). @@ -22,7 +22,8 @@ Redis matters for steps 3 and 4: without it the rate limits and bans do not hold [Route policies](routes.md) explains how a request finds its route, what switching a default route off does, and every option and behavior rule key with its unit. ```php -'guard' => [ +'guardCore' => [ + 'enabled' => true, 'routes' => [ // A stricter lead capture limit. 'POST /api/v1/LeadCapture/' => ['rateLimit' => 3, 'rateLimitWindow' => 60], diff --git a/examples/advanced_app/apply-config.php b/examples/advanced_app/apply-config.php index 46d1d2c..6c88226 100644 --- a/examples/advanced_app/apply-config.php +++ b/examples/advanced_app/apply-config.php @@ -1,16 +1,16 @@ -// Merges the keys the file returns (guardEnabled and guard) into data/config.php. The file is rewritten in place +// Merges the keys the file returns (guardCore) into data/config-internal.php, where EspoCRM keeps level system params. The file is rewritten in place // because EspoCRM's ConfigWriter drops keys it does not know yet. -$path = '/var/www/html/data/config.php'; +$path = '/var/www/html/data/config-internal.php'; $config = include $path; $override = include $argv[1]; $config = array_replace($config, $override); -$config['microtime'] = microtime(true); +$config['microtimeInternal'] = microtime(true); file_put_contents($path, " true, - 'guard' => [ + 'guardCore' => [ + 'enabled' => true, 'passiveMode' => false, 'enableRedis' => true, 'redisUrl' => 'redis://redis:6379', diff --git a/examples/simple_app/README.md b/examples/simple_app/README.md index 0483b22..024593a 100644 --- a/examples/simple_app/README.md +++ b/examples/simple_app/README.md @@ -1,6 +1,6 @@ # simple_app -A real EspoCRM (the official `espocrm/espocrm` image) with MariaDB, and the Guard Core extension installed from the zip this repository builds. `guardEnabled` is true and nothing else is set, so the guard runs the default policy in passive mode: attacks are logged, nothing is blocked. +A real EspoCRM (the official `espocrm/espocrm` image) with MariaDB, and the Guard Core extension installed from the zip this repository builds. `guardCore` holds only `'enabled' => true`, so the guard runs the default policy in passive mode: attacks are logged, nothing is blocked. The official image is used because it installs EspoCRM on first start from environment variables, and its `custom/`, `data/` and `client/custom/` volumes are what an extension install needs. The extension is installed through EspoCRM's own `command.php extension` once the installer has finished (`guard-entrypoint.sh`, `guard-setup.sh`), and the keys in `guard-config.php` are written to `data/config.php`. diff --git a/examples/simple_app/apply-config.php b/examples/simple_app/apply-config.php index 46d1d2c..6c88226 100644 --- a/examples/simple_app/apply-config.php +++ b/examples/simple_app/apply-config.php @@ -1,16 +1,16 @@ -// Merges the keys the file returns (guardEnabled and guard) into data/config.php. The file is rewritten in place +// Merges the keys the file returns (guardCore) into data/config-internal.php, where EspoCRM keeps level system params. The file is rewritten in place // because EspoCRM's ConfigWriter drops keys it does not know yet. -$path = '/var/www/html/data/config.php'; +$path = '/var/www/html/data/config-internal.php'; $config = include $path; $override = include $argv[1]; $config = array_replace($config, $override); -$config['microtime'] = microtime(true); +$config['microtimeInternal'] = microtime(true); file_put_contents($path, " true, + 'guardCore' => [ + 'enabled' => true, + ], ]; diff --git a/package-lock.json b/package-lock.json index 94c1edd..193be41 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "espocrm-ext-guard-core", - "version": "1.0.0", + "version": "1.1.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "espocrm-ext-guard-core", - "version": "1.0.0", + "version": "1.1.0", "license": "MIT", "dependencies": { "espo-extension-tools": "github:espocrm/extension-tools#0.4.6", diff --git a/package.json b/package.json index 7c65138..244915e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "espocrm-ext-guard-core", - "version": "1.0.0", + "version": "1.1.0", "description": "Guard Core application-layer security for the EspoCRM API.", "scripts": { "sa": "vendor/bin/phpstan", diff --git a/src/files/custom/Espo/Modules/GuardCore/Api/DefaultPolicy.php b/src/files/custom/Espo/Modules/GuardCore/Api/DefaultPolicy.php index e675da1..c59e19c 100644 --- a/src/files/custom/Espo/Modules/GuardCore/Api/DefaultPolicy.php +++ b/src/files/custom/Espo/Modules/GuardCore/Api/DefaultPolicy.php @@ -3,7 +3,7 @@ namespace Espo\Modules\GuardCore\Api; /** - * The policy applied when `guardEnabled` is true: passive mode, screening on, and route policies for the + * The policy applied when the guard is enabled: passive mode, screening on, and route policies for the * endpoints that are internet facing or credential related. Operator config overrides it key by key. * * Redis is off by default, and without it route rate limits, behavior rules and auto-bans keep state per request only, diff --git a/src/files/custom/Espo/Modules/GuardCore/Api/GuardMiddlewareBuilder.php b/src/files/custom/Espo/Modules/GuardCore/Api/GuardMiddlewareBuilder.php index 169c821..9334c28 100644 --- a/src/files/custom/Espo/Modules/GuardCore/Api/GuardMiddlewareBuilder.php +++ b/src/files/custom/Espo/Modules/GuardCore/Api/GuardMiddlewareBuilder.php @@ -76,7 +76,7 @@ private function warnWhenClientAddressIsNotRemoteAddr(): void $this->log->warning( "Guard: ipAddressServerParam is '$param', but the guard identifies the client by REMOTE_ADDR only. " . - "Set guard.trustedProxies (and guard.trustedProxyDepth) to resolve the client behind a proxy." + "Set guardCore.trustedProxies (and guardCore.trustedProxyDepth) to resolve the client behind a proxy." ); } diff --git a/src/files/custom/Espo/Modules/GuardCore/Api/Middleware.php b/src/files/custom/Espo/Modules/GuardCore/Api/Middleware.php index 38a7b83..353aae8 100644 --- a/src/files/custom/Espo/Modules/GuardCore/Api/Middleware.php +++ b/src/files/custom/Espo/Modules/GuardCore/Api/Middleware.php @@ -3,6 +3,7 @@ namespace Espo\Modules\GuardCore\Api; use Espo\Core\Utils\Config; +use Espo\Core\Utils\Log; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as ServerRequest; use Psr\Http\Server\MiddlewareInterface; @@ -10,7 +11,8 @@ use RenzoFranceschini\GuardCorePsr15\GuardMiddleware; /** - * Opt-in application layer security (Guard Core). Does nothing, and builds nothing, unless `guardEnabled` is true. + * Opt-in application layer security (Guard Core). Does nothing, and builds nothing, unless `guardCore.enabled` (or + * the deprecated `guardEnabled`) is true. */ class Middleware implements MiddlewareInterface { @@ -20,30 +22,79 @@ public function __construct( private Config $config, private DefaultPolicy $defaultPolicy, private GuardMiddlewareBuilder $builder, + private Log $log, ) {} public function process(ServerRequest $request, RequestHandler $handler): Response { - if (!$this->config->get('guardEnabled')) { + $settings = $this->resolveOperatorSettings(); + + if ($settings === null) { return $handler->handle($request); } - $this->guard ??= $this->builder->build($this->defaultPolicy->merge($this->getOperatorSettings())); + $this->guard ??= $this->builder->build($this->defaultPolicy->merge($settings)); return $this->guard->process($request, $handler); } /** - * @return array + * @return array|null Null when the guard is off. + */ + private function resolveOperatorSettings(): ?array + { + $guardCore = $this->config->get('guardCore'); + + if ($guardCore === null) { + return $this->resolveLegacySettings(); + } + + if (!is_array($guardCore)) { + throw new \InvalidArgumentException("Config parameter 'guardCore' must be a map of settings."); + } + + $enabled = $guardCore['enabled'] ?? false; + + if (!is_bool($enabled)) { + throw new \InvalidArgumentException("Config parameter 'guardCore.enabled' must be true or false."); + } + + unset($guardCore['enabled']); + + $hasLegacyKeys = $this->config->get('guardEnabled') !== null || $this->config->get('guard') !== null; + + if (($enabled || $this->config->get('guardEnabled')) && $hasLegacyKeys) { + throw new \InvalidArgumentException( + "Guard settings are split between 'guardCore' and the old 'guardEnabled'/'guard'. " + . "Move everything under 'guardCore' and remove the old keys." + ); + } + + return $enabled ? $guardCore : null; + } + + /** + * @return array|null */ - private function getOperatorSettings(): array + private function resolveLegacySettings(): ?array { + if (!$this->config->get('guardEnabled')) { + return null; + } + $settings = $this->config->get('guard') ?? []; if (!is_array($settings)) { throw new \InvalidArgumentException("Config parameter 'guard' must be a map of settings."); } + if ($this->guard === null) { + $this->log->warning( + "Config parameters 'guardEnabled' and 'guard' are deprecated. " + . "Move them under 'guardCore' (see docs/configuration.md)." + ); + } + return $settings; } } diff --git a/src/files/custom/Espo/Modules/GuardCore/Api/SecurityConfigBuilder.php b/src/files/custom/Espo/Modules/GuardCore/Api/SecurityConfigBuilder.php index 06693fa..bdd2e0d 100644 --- a/src/files/custom/Espo/Modules/GuardCore/Api/SecurityConfigBuilder.php +++ b/src/files/custom/Espo/Modules/GuardCore/Api/SecurityConfigBuilder.php @@ -9,8 +9,8 @@ use RenzoFranceschini\GuardCore\Config\SecurityConfig; /** - * Maps the `guard` settings onto the named arguments of the engine's `SecurityConfig`, so every engine option is - * available from `data/config.php`. An unknown key fails loudly. + * Maps the `guardCore` settings onto the named arguments of the engine's `SecurityConfig`, so every engine option is + * available from the config. An unknown key fails loudly. */ class SecurityConfigBuilder { diff --git a/src/files/custom/Espo/Modules/GuardCore/Resources/metadata/app/config.json b/src/files/custom/Espo/Modules/GuardCore/Resources/metadata/app/config.json index 00fa419..8c74f58 100644 --- a/src/files/custom/Espo/Modules/GuardCore/Resources/metadata/app/config.json +++ b/src/files/custom/Espo/Modules/GuardCore/Resources/metadata/app/config.json @@ -1,5 +1,8 @@ { "params": { + "guardCore": { + "level": "system" + }, "guardEnabled": { "level": "system" }, diff --git a/src/scripts/AfterInstall.php b/src/scripts/AfterInstall.php new file mode 100644 index 0000000..68dfeaf --- /dev/null +++ b/src/scripts/AfterInstall.php @@ -0,0 +1,64 @@ +getByClass(Config::class); + + $guardCore = $config->get('guardCore'); + $guardEnabled = $config->get('guardEnabled'); + $guard = $config->get('guard'); + + if ($guardCore !== null || ($guardEnabled === null && $guard === null)) { + return; + } + + if ($guard !== null && !is_array($guard)) { + return; + } + + if ($this->overrideFileHoldsAnOldKey('data/config-override.php') + || $this->overrideFileHoldsAnOldKey('data/config-internal-override.php')) { + return; + } + + $configWriter = $container->getByClass(InjectableFactory::class)->create(ConfigWriter::class); + + $configWriter->set('guardCore', ['enabled' => (bool) $guardEnabled] + ($guard ?? [])); + + foreach (['guardEnabled', 'guard'] as $oldKey) { + // Before EspoCRM 9.3 `remove` drops a level system param from config-internal.php only, so a key the admin + // kept in config.php (the 1.0.0 docs) survived (ConfigWriter::save at 9.0.0, lines 143-153). `set` moves a + // system param out of config.php into config-internal.php (lines 130-141), and `remove` then drops it + // there. From 9.3 `remove` unsets the key from every file, so the pair is harmless there. + $configWriter->set($oldKey, null); + $configWriter->remove($oldKey); + } + + $configWriter->save(); + } + + private function overrideFileHoldsAnOldKey(string $path): bool + { + if (!is_file($path)) { + return false; + } + + $data = include $path; + + return is_array($data) && (array_key_exists('guardEnabled', $data) || array_key_exists('guard', $data)); + } +} diff --git a/src/scripts/AfterUninstall.php b/src/scripts/AfterUninstall.php index 69696ce..b289b7c 100644 --- a/src/scripts/AfterUninstall.php +++ b/src/scripts/AfterUninstall.php @@ -5,8 +5,8 @@ use Espo\Core\Utils\Config\ConfigWriter; /** - * Once the module is gone nothing hides `guard` (Redis URL, agent API key) from the Settings API any more, so the - * keys are removed from the config files. + * Once the module is gone nothing hides `guardCore` (Redis URL, agent API key) from the Settings API any more, so the + * keys (and the old `guardEnabled` and `guard`) are removed from the config files. */ class AfterUninstall { @@ -14,8 +14,15 @@ public function run(Container $container): void { $configWriter = $container->getByClass(InjectableFactory::class)->create(ConfigWriter::class); - $configWriter->remove('guardEnabled'); - $configWriter->remove('guard'); + foreach (['guardCore', 'guardEnabled', 'guard'] as $key) { + // Before EspoCRM 9.3 `remove` drops a level system param from config-internal.php only, so a key kept in + // config.php survived and, with this module gone, lost its system level (ConfigWriter::save at 9.0.0, + // lines 143-153). `set` moves a system param out of config.php into config-internal.php (lines 130-141), + // and `remove` then drops it there. From 9.3 `remove` unsets the key from every file (lines 185-197). + $configWriter->set($key, null); + $configWriter->remove($key); + } + $configWriter->save(); } } diff --git a/tests/unit/Espo/Modules/GuardCore/AfterInstallConfigFilesTest.php b/tests/unit/Espo/Modules/GuardCore/AfterInstallConfigFilesTest.php new file mode 100644 index 0000000..749e40f --- /dev/null +++ b/tests/unit/Espo/Modules/GuardCore/AfterInstallConfigFilesTest.php @@ -0,0 +1,154 @@ + false, 'routes' => ['/x/' => ['rateLimit' => 2]]]; + + private const MIGRATED = ['enabled' => true, 'passiveMode' => false, 'routes' => ['/x/' => ['rateLimit' => 2]]]; + + public function testOldKeysInConfigPhpOnlyMoveIntoConfigInternal(): void + { + $this->writeConfigFile('config.php', ['siteUrl' => 'https://crm.test', 'guardEnabled' => true, 'guard' => self::GUARD]); + $this->writeConfigFile('config-internal.php', ['passwordSalt' => 'salt']); + + $this->migrate(); + + $this->assertMigrated(); + $this->assertSame('https://crm.test', $this->readConfigFile('config.php')['siteUrl']); + $this->assertSame('salt', $this->readConfigFile('config-internal.php')['passwordSalt']); + } + + public function testOldKeysInConfigInternalOnlyAreReplacedByGuardCore(): void + { + $this->writeConfigFile('config.php', ['siteUrl' => 'https://crm.test']); + $this->writeConfigFile('config-internal.php', ['guardEnabled' => true, 'guard' => self::GUARD]); + + $this->migrate(); + + $this->assertMigrated(); + } + + public function testOldKeysSplitAcrossBothFilesAreMigrated(): void + { + $this->writeConfigFile('config.php', ['guardEnabled' => true]); + $this->writeConfigFile('config-internal.php', ['guard' => self::GUARD]); + + $this->migrate(); + + $this->assertMigrated(); + } + + public function testOldKeysInConfigPhpAndConfigInternalAtOnceAreMigrated(): void + { + $this->writeConfigFile('config.php', ['guardEnabled' => true, 'guard' => ['passiveMode' => true]]); + $this->writeConfigFile('config-internal.php', ['guardEnabled' => true, 'guard' => self::GUARD]); + + $this->migrate(); + + $this->assertMigrated(); + } + + /** + * @return array}> + */ + public static function overrideProvider(): array + { + return [ + 'guardEnabled in config-override' => ['config-override.php', ['guardEnabled' => true]], + 'guard in config-override' => ['config-override.php', ['guard' => self::GUARD]], + 'guardEnabled in config-internal-override' => ['config-internal-override.php', ['guardEnabled' => true]], + 'guard in config-internal-override' => ['config-internal-override.php', ['guard' => self::GUARD]], + ]; + } + + /** + * @param array $override + */ + #[DataProvider('overrideProvider')] + public function testOldKeysInAnOverrideFileChangeNothing(string $overrideFile, array $override): void + { + $this->writeConfigFile('config.php', ['siteUrl' => 'https://crm.test', 'guardEnabled' => false]); + $this->writeConfigFile('config-internal.php', ['passwordSalt' => 'salt']); + $this->writeConfigFile($overrideFile, $override); + $before = $this->allConfigContents(); + + $this->migrate(); + + $this->assertSame($before, $this->allConfigContents()); + } + + public function testAnOverrideFileWithoutOldKeysDoesNotBlockTheMigration(): void + { + $this->writeConfigFile('config.php', ['guardEnabled' => true, 'guard' => self::GUARD]); + $this->writeConfigFile('config-internal.php', []); + $this->writeConfigFile('config-override.php', ['siteUrl' => 'https://override.test']); + + $this->migrate(); + + $this->assertMigrated(); + } + + public function testFreshInstallWritesNeitherFile(): void + { + $this->writeConfigFile('config.php', ['siteUrl' => 'https://crm.test']); + $this->writeConfigFile('config-internal.php', ['passwordSalt' => 'salt']); + $before = $this->allConfigContents(); + + $this->migrate(); + + $this->assertSame($before, $this->allConfigContents()); + } + + public function testASecondRunChangesNothing(): void + { + $this->writeConfigFile('config.php', ['guardEnabled' => true, 'guard' => self::GUARD]); + $this->writeConfigFile('config-internal.php', []); + + $this->migrate(); + $afterFirstRun = $this->allConfigContents(); + + $this->migrate(); + + $this->assertSame($afterFirstRun, $this->allConfigContents()); + } + + private function migrate(): void + { + (new \AfterInstall())->run($this->container()); + } + + /** + * @return array + */ + private function allConfigContents(): array + { + $contents = []; + + foreach (['config.php', 'config-internal.php', 'config-override.php', 'config-internal-override.php'] as $file) { + $contents[$file] = $this->configFileExists($file) ? $this->configFileContents($file) : ''; + } + + return $contents; + } + + private function assertMigrated(): void + { + $this->assertSame(self::MIGRATED, $this->readConfigFile('config-internal.php')['guardCore']); + $this->assertSame(['enabled', 'passiveMode', 'routes'], array_keys($this->readConfigFile('config-internal.php')['guardCore'])); + $this->assertArrayNotHasKey('guardCore', $this->readConfigFile('config.php')); + + foreach (['config.php', 'config-internal.php'] as $file) { + foreach (self::OLD_KEYS as $oldKey) { + $this->assertArrayNotHasKey($oldKey, $this->readConfigFile($file), "$oldKey left in $file"); + } + } + } +} diff --git a/tests/unit/Espo/Modules/GuardCore/AfterInstallTest.php b/tests/unit/Espo/Modules/GuardCore/AfterInstallTest.php new file mode 100644 index 0000000..d10a1c6 --- /dev/null +++ b/tests/unit/Espo/Modules/GuardCore/AfterInstallTest.php @@ -0,0 +1,158 @@ + */ + private array $set = []; + + /** @var string[] */ + private array $removed = []; + + /** @var string[] */ + private array $nulled = []; + + private int $saves = 0; + + public function testMovesBothOldKeysUnderGuardCoreWithEnabledFirst(): void + { + $guard = ['passiveMode' => false, 'routes' => ['/x/' => ['rateLimit' => 2]]]; + + $this->runWith(['guardEnabled' => true, 'guard' => $guard]); + + $this->assertSame(['guardCore' => ['enabled' => true, ...$guard]], $this->set); + $this->assertSame(['enabled', 'passiveMode', 'routes'], array_keys($this->set['guardCore'])); + $this->assertSame(['guardEnabled', 'guard'], $this->removed); + $this->assertSame(['guardEnabled', 'guard'], $this->nulled); + $this->assertSame(1, $this->saves); + } + + public function testGuardEnabledAloneBecomesEnabledOnly(): void + { + $this->runWith(['guardEnabled' => true]); + + $this->assertSame(['guardCore' => ['enabled' => true]], $this->set); + $this->assertSame(1, $this->saves); + } + + public function testDisabledGuardEnabledKeepsTheGuardMapDisabled(): void + { + $this->runWith(['guardEnabled' => false, 'guard' => ['passiveMode' => false]]); + + $this->assertSame(['guardCore' => ['enabled' => false, 'passiveMode' => false]], $this->set); + } + + public function testGuardMapAloneBecomesDisabled(): void + { + $this->runWith(['guard' => ['passiveMode' => false]]); + + $this->assertSame(['guardCore' => ['enabled' => false, 'passiveMode' => false]], $this->set); + } + + public function testGuardThatIsNotAMapChangesNothing(): void + { + $this->runWith(['guardEnabled' => true, 'guard' => 'nope']); + + $this->assertNothingWritten(); + } + + public function testGuardCoreAlreadySetChangesNothing(): void + { + $this->runWith(['guardCore' => ['enabled' => true], 'guardEnabled' => true, 'guard' => []]); + + $this->assertNothingWritten(); + } + + public function testFreshInstallWritesNothing(): void + { + $this->runWith([]); + + $this->assertNothingWritten(); + } + + /** + * @return array}> + */ + public static function oldConfigProvider(): array + { + return [ + 'both keys' => [['guardEnabled' => true, 'guard' => ['passiveMode' => false]]], + 'guardEnabled only' => [['guardEnabled' => true]], + 'guard only' => [['guard' => ['passiveMode' => false]]], + ]; + } + + /** + * @param array $old + */ + #[DataProvider('oldConfigProvider')] + public function testASecondRunChangesNothing(array $old): void + { + $this->runWith($old); + + $migrated = $this->set; + $this->assertSame(1, $this->saves); + + $this->runWith($migrated); + + $this->assertSame(1, $this->saves, 'the second run must not save again'); + } + + /** + * @param array $values + */ + private function runWith(array $values): void + { + $this->set = []; + $this->removed = []; + $this->nulled = []; + + $config = $this->createStub(Config::class); + $config->method('get')->willReturnCallback(fn (string $name, $default = null) => $values[$name] ?? $default); + + $configWriter = $this->createStub(ConfigWriter::class); + $configWriter->method('set')->willReturnCallback(function (string $name, $value): void { + if ($value === null) { + $this->nulled[] = $name; + + return; + } + + $this->set[$name] = $value; + }); + $configWriter->method('remove')->willReturnCallback(function (string $name): void { + $this->removed[] = $name; + }); + $configWriter->method('save')->willReturnCallback(function (): void { + $this->saves++; + }); + + $factory = $this->createStub(InjectableFactory::class); + $factory->method('create')->willReturn($configWriter); + + $container = $this->createStub(Container::class); + $container->method('getByClass')->willReturnCallback( + fn (string $class) => $class === Config::class ? $config : $factory + ); + + (new \AfterInstall())->run($container); + } + + private function assertNothingWritten(): void + { + $this->assertSame([], $this->set); + $this->assertSame([], $this->removed); + $this->assertSame([], $this->nulled); + $this->assertSame(0, $this->saves); + } +} diff --git a/tests/unit/Espo/Modules/GuardCore/AfterUninstallTest.php b/tests/unit/Espo/Modules/GuardCore/AfterUninstallTest.php index 95c30f0..d3446e7 100644 --- a/tests/unit/Espo/Modules/GuardCore/AfterUninstallTest.php +++ b/tests/unit/Espo/Modules/GuardCore/AfterUninstallTest.php @@ -2,35 +2,36 @@ namespace tests\unit\Espo\Modules\GuardCore; -use Espo\Core\Container; -use Espo\Core\InjectableFactory; -use Espo\Core\Utils\Config\ConfigWriter; -use PHPUnit\Framework\TestCase; - require_once __DIR__ . '/../../../../../src/scripts/AfterUninstall.php'; -class AfterUninstallTest extends TestCase +class AfterUninstallTest extends RealConfigFilesTestCase { - public function testRemovesTheGuardKeysFromTheConfigFilesAndSaves(): void + public function testRemovesEveryGuardKeyFromBothConfigFilesAndKeepsTheRest(): void { - $removed = []; + $this->writeConfigFile('config.php', ['siteUrl' => 'https://crm.test', 'guardEnabled' => true, 'guard' => []]); + $this->writeConfigFile('config-internal.php', [ + 'passwordSalt' => 'salt', + 'guardCore' => ['enabled' => true, 'redisUrl' => 'redis://:secret@redis:6379'], + 'guard' => ['passiveMode' => false], + ]); + + (new \AfterUninstall())->run($this->container()); - $configWriter = $this->createMock(ConfigWriter::class); - $configWriter->expects(self::exactly(2)) - ->method('remove') - ->willReturnCallback(function (string $name) use (&$removed): void { - $removed[] = $name; - }); - $configWriter->expects(self::once())->method('save'); + foreach (['config.php', 'config-internal.php'] as $file) { + $this->assertSame([], array_intersect(['guardCore', 'guardEnabled', 'guard'], array_keys($this->readConfigFile($file))), $file); + } - $factory = $this->createMock(InjectableFactory::class); - $factory->expects(self::once())->method('create')->with(ConfigWriter::class)->willReturn($configWriter); + $this->assertSame('https://crm.test', $this->readConfigFile('config.php')['siteUrl']); + $this->assertSame('salt', $this->readConfigFile('config-internal.php')['passwordSalt']); + } - $container = $this->createMock(Container::class); - $container->expects(self::once())->method('getByClass')->with(InjectableFactory::class)->willReturn($factory); + public function testRemovesTheKeysThatLiveInConfigPhpOnly(): void + { + $this->writeConfigFile('config.php', ['guardCore' => ['enabled' => true], 'guardEnabled' => true, 'guard' => []]); - (new \AfterUninstall())->run($container); + (new \AfterUninstall())->run($this->container()); - $this->assertSame(['guardEnabled', 'guard'], $removed); + $this->assertSame([], array_intersect(['guardCore', 'guardEnabled', 'guard'], array_keys($this->readConfigFile('config.php')))); + $this->assertSame([], array_intersect(['guardCore', 'guardEnabled', 'guard'], array_keys($this->readConfigFile('config-internal.php')))); } } diff --git a/tests/unit/Espo/Modules/GuardCore/Api/MiddlewareTest.php b/tests/unit/Espo/Modules/GuardCore/Api/MiddlewareTest.php index dce63f9..cde682b 100644 --- a/tests/unit/Espo/Modules/GuardCore/Api/MiddlewareTest.php +++ b/tests/unit/Espo/Modules/GuardCore/Api/MiddlewareTest.php @@ -43,7 +43,7 @@ public function testDisabledPassesThroughAndBuildsNothing(): void $builder = $this->createMock(GuardMiddlewareBuilder::class); $builder->expects(self::never())->method('build'); - $middleware = new Middleware($this->createConfig(['guardEnabled' => false]), new DefaultPolicy(), $builder); + $middleware = new Middleware($this->createConfig(['guardEnabled' => false]), new DefaultPolicy(), $builder, $this->createLog()); $request = $this->createRequest('GET', "/api/v1/App/user?q=1' OR '1'='1' UNION SELECT 1--"); @@ -58,7 +58,7 @@ public function testMissingGuardEnabledParamIsDisabled(): void $builder = $this->createMock(GuardMiddlewareBuilder::class); $builder->expects(self::never())->method('build'); - $middleware = new Middleware($this->createConfig([]), new DefaultPolicy(), $builder); + $middleware = new Middleware($this->createConfig([]), new DefaultPolicy(), $builder, $this->createLog()); $response = $middleware->process($this->createRequest('GET', '/'), $this->createHandler()); @@ -435,6 +435,142 @@ public function testInvalidBehaviorRuleFailsLoudly(): void ->process($this->createRequest('GET', '/'), $this->createHandler()); } + public function testGuardCoreEnabledBuildsTheGuardWithoutPassingEnabledToTheEngine(): void + { + $middleware = $this->createMiddleware(['passiveMode' => false]); + + $response = $middleware->process( + $this->createRequest('GET', "/api/v1/Account?where=1' OR '1'='1' UNION SELECT password FROM user--"), + $this->createHandler() + ); + + $this->assertSame(400, $response->getStatusCode()); + $this->assertSame(0, $this->countDeprecationWarnings()); + } + + public function testGuardCoreDisabledPassesThroughAndBuildsNothing(): void + { + $this->assertDisabledBuildsNothing(['guardCore' => ['enabled' => false, 'passiveMode' => false]]); + } + + public function testGuardCoreWithoutEnabledIsDisabled(): void + { + $this->assertDisabledBuildsNothing(['guardCore' => ['passiveMode' => false]]); + } + + public function testEmptyGuardCoreMapIsDisabled(): void + { + $this->assertDisabledBuildsNothing(['guardCore' => []]); + } + + public function testGuardCoreEnabledThatIsNotABoolFailsLoudly(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage("Config parameter 'guardCore.enabled' must be true or false."); + + $this->createMiddlewareFromConfig(['guardCore' => ['enabled' => 'yes']]) + ->process($this->createRequest('GET', '/'), $this->createHandler()); + } + + public function testGuardCoreThatIsNotAMapFailsLoudly(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage("Config parameter 'guardCore' must be a map of settings."); + + $this->createMiddlewareFromConfig(['guardCore' => 'nope']) + ->process($this->createRequest('GET', '/'), $this->createHandler()); + } + + /** + * @return array}> + */ + public static function splitSettingsProvider(): array + { + $enabled = ['enabled' => true]; + + return [ + 'guardCore enabled and old guard set' => [['guardCore' => $enabled, 'guard' => ['passiveMode' => false]]], + 'guardCore enabled and old guardEnabled set' => [['guardCore' => $enabled, 'guardEnabled' => true]], + 'guardCore enabled and old guardEnabled false' => [['guardCore' => $enabled, 'guardEnabled' => false]], + 'guardCore disabled and old guardEnabled true' => [['guardCore' => ['enabled' => false], 'guardEnabled' => true]], + 'guardCore without enabled and old guardEnabled true' => [['guardCore' => [], 'guardEnabled' => true]], + ]; + } + + /** + * @param array $values + */ + #[DataProvider('splitSettingsProvider')] + public function testSettingsSplitBetweenGuardCoreAndTheOldKeysFailLoudly(array $values): void + { + $builder = $this->createMock(GuardMiddlewareBuilder::class); + $builder->expects(self::never())->method('build'); + + $middleware = new Middleware($this->createConfig($values), new DefaultPolicy(), $builder, $this->createLog()); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage("split between 'guardCore' and the old 'guardEnabled'/'guard'"); + + $middleware->process($this->createRequest('GET', '/'), $this->createHandler()); + } + + public function testGuardCoreDisabledWithOldKeysDisabledPassesThrough(): void + { + $this->assertDisabledBuildsNothing([ + 'guardCore' => ['enabled' => false], + 'guardEnabled' => false, + 'guard' => ['passiveMode' => false], + ]); + } + + public function testLegacyKeysStillWorkAndLogTheDeprecationOncePerBuild(): void + { + $middleware = $this->createMiddlewareFromConfig([ + 'guardEnabled' => true, + 'guard' => ['passiveMode' => false], + ]); + + $request = $this->createRequest('GET', "/api/v1/Account?where=1' OR '1'='1' UNION SELECT password FROM user--"); + + $this->assertSame(400, $middleware->process($request, $this->createHandler())->getStatusCode()); + $this->assertSame(400, $middleware->process($request, $this->createHandler())->getStatusCode()); + + $this->assertSame(1, $this->countDeprecationWarnings()); + $this->assertTrue($this->logHandler->hasWarningThatContains('docs/configuration.md')); + $this->assertTrue($this->logHandler->hasWarningThatContains("'guardCore'")); + } + + public function testLegacyGuardEnabledAloneUsesTheDefaultPolicy(): void + { + $middleware = $this->createMiddlewareFromConfig(['guardEnabled' => true]); + + $this->assertSame(200, $middleware->process($this->createRequest('GET', '/'), $this->createHandler())->getStatusCode()); + $this->assertSame(1, $this->countDeprecationWarnings()); + } + + public function testDisabledLegacyLogsNothing(): void + { + $this->assertDisabledBuildsNothing(['guardEnabled' => false, 'guard' => ['passiveMode' => false]]); + $this->assertSame([], $this->logHandler->getRecords()); + } + + /** + * @param array $values + */ + private function assertDisabledBuildsNothing(array $values): void + { + $builder = $this->createMock(GuardMiddlewareBuilder::class); + $builder->expects(self::never())->method('build'); + + $middleware = new Middleware($this->createConfig($values), new DefaultPolicy(), $builder, $this->createLog()); + + $request = $this->createRequest('GET', "/api/v1/App/user?q=1' OR '1'='1' UNION SELECT 1--"); + + $this->assertSame(200, $middleware->process($request, $this->createHandler())->getStatusCode()); + $this->assertSame(1, $this->handlerCalls); + $this->assertSame([], $this->logHandler->getRecords()); + } + public function testGuardConfigThatIsNotAMapFailsLoudly(): void { $builder = $this->createMock(GuardMiddlewareBuilder::class); @@ -443,7 +579,8 @@ public function testGuardConfigThatIsNotAMapFailsLoudly(): void $middleware = new Middleware( $this->createConfig(['guardEnabled' => true, 'guard' => 'nope']), new DefaultPolicy(), - $builder + $builder, + $this->createLog() ); $this->expectException(InvalidArgumentException::class); @@ -525,14 +662,14 @@ public function testWarnsWhenClientAddressParamIsNotRemoteAddr(): void $this->createMiddleware([], ['ipAddressServerParam' => 'HTTP_X_FORWARDED_FOR']) ->process($this->createRequest('GET', '/'), $this->createHandler()); - $this->assertTrue($this->logHandler->hasWarningThatContains('guard.trustedProxies')); + $this->assertTrue($this->logHandler->hasWarningThatContains('guardCore.trustedProxies')); } public function testNoWarningWithDefaultClientAddressParam(): void { $this->createMiddleware([])->process($this->createRequest('GET', '/'), $this->createHandler()); - $this->assertFalse($this->logHandler->hasWarningThatContains('guard.trustedProxies')); + $this->assertFalse($this->logHandler->hasWarningThatContains('guardCore.trustedProxies')); } private function getRedisHost(): string @@ -557,17 +694,41 @@ public function recordHandled(ServerRequestInterface $request): void */ private function createMiddleware(array $guard, array $extraConfig = []): Middleware { - $config = $this->createConfig(['guardEnabled' => true, 'guard' => $guard, ...$extraConfig]); + return $this->createMiddlewareFromConfig( + ['guardCore' => ['enabled' => true, ...$guard], ...$extraConfig] + ); + } + + /** + * @param array $values + */ + private function createMiddlewareFromConfig(array $values): Middleware + { + $config = $this->createConfig($values); + $log = $this->createLog(); $builder = new GuardMiddlewareBuilder( $config, - new Log('test', [$this->logHandler]), + $log, new SecurityConfigBuilder(), new RouteConfigBuilder(), new GuardAgentFactory(), ); - return new Middleware($config, new DefaultPolicy(), $builder); + return new Middleware($config, new DefaultPolicy(), $builder, $log); + } + + private function createLog(): Log + { + return new Log('test', [$this->logHandler]); + } + + private function countDeprecationWarnings(): int + { + return count(array_filter( + $this->logHandler->getRecords(), + fn ($record) => str_contains((string) $record['message'], 'are deprecated') + )); } /** diff --git a/tests/unit/Espo/Modules/GuardCore/RealConfigFilesTestCase.php b/tests/unit/Espo/Modules/GuardCore/RealConfigFilesTestCase.php new file mode 100644 index 0000000..a756263 --- /dev/null +++ b/tests/unit/Espo/Modules/GuardCore/RealConfigFilesTestCase.php @@ -0,0 +1,121 @@ +originalWorkingDirectory = (string) getcwd(); + $this->root = sys_get_temp_dir() . '/espocrm-guard-' . uniqid('', true); + + mkdir($this->root . '/data', 0777, true); + mkdir($this->root . '/application/Espo/Resources/defaults', 0777, true); + copy( + $this->originalWorkingDirectory . '/site/application/Espo/Resources/defaults/systemConfig.php', + $this->root . '/application/Espo/Resources/defaults/systemConfig.php' + ); + + chdir($this->root); + } + + protected function tearDown(): void + { + chdir($this->originalWorkingDirectory); + + // ConfigWriter also leaves scratch files under data/ (tmp/), so the whole scratch root goes, and only it. + $entries = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($this->root, \FilesystemIterator::SKIP_DOTS), + \RecursiveIteratorIterator::CHILD_FIRST + ); + + foreach ($entries as $entry) { + $entry->isDir() && !$entry->isLink() ? rmdir($entry->getPathname()) : unlink($entry->getPathname()); + } + + rmdir($this->root); + } + + /** + * @param array $values + */ + protected function writeConfigFile(string $name, array $values): void + { + file_put_contents($this->root . '/data/' . $name, ' + */ + protected function readConfigFile(string $name): array + { + $values = include $this->root . '/data/' . $name; + + return is_array($values) ? $values : []; + } + + protected function configFileContents(string $name): string + { + return (string) file_get_contents($this->root . '/data/' . $name); + } + + protected function configFileExists(string $name): bool + { + return is_file($this->root . '/data/' . $name); + } + + /** + * What EspoCRM's container resolves: the real Config, and a real ConfigWriter whose InternalConfigHelper learns + * the level of each key from the module's own metadata file. + */ + protected function container(): Container + { + $config = new Config(new ConfigFileManager()); + + $metadata = $this->createStub(Metadata::class); + $metadata->method('get')->willReturnCallback(function ($key, $default = null) { + $params = json_decode((string) file_get_contents(self::METADATA_CONFIG), true)['params']; + + return $params[$key[3] ?? '']['level'] ?? $default; + }); + + $configWriter = new ConfigWriter( + $config, + new ConfigWriterFileManager($config), + new ConfigWriterHelper(), + new InternalConfigHelper($config, $metadata) + ); + + $factory = $this->createStub(InjectableFactory::class); + $factory->method('create')->willReturn($configWriter); + + $container = $this->createStub(Container::class); + $container->method('getByClass')->willReturnCallback( + fn (string $class) => $class === Config::class ? $config : $factory + ); + + return $container; + } +}