Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Honeypot field appears when CSP is enabled #7029

Merged
merged 11 commits into from Jan 5, 2023
11 changes: 10 additions & 1 deletion app/Config/Honeypot.php
Expand Up @@ -24,10 +24,19 @@ class Honeypot extends BaseConfig
/**
* Honeypot HTML Template
*/
public string $template = '<label>{label}</label><input type="text" name="{name}" value=""/>';
public string $template = '<label>{label}</label><input type="text" name="{name}" value="">';

/**
* Honeypot container
*
* If you enables CSP, you can remove `style="display:none"`.
kenjis marked this conversation as resolved.
Show resolved Hide resolved
*/
public string $container = '<div style="display:none">{template}</div>';

/**
* The id attribute for Honeypot container tag
*
* Used when CSP is enabled.
*/
public string $containerId = 'hpc';
}
18 changes: 18 additions & 0 deletions system/Honeypot/Honeypot.php
Expand Up @@ -46,6 +46,8 @@ public function __construct(HoneypotConfig $config)
$this->config->container = '<div style="display:none">{template}</div>';
}

$this->config->containerId ??= 'hpc';

if ($this->config->template === '') {
throw HoneypotException::forNoTemplate();
}
Expand All @@ -70,10 +72,26 @@ public function hasContent(RequestInterface $request)
*/
public function attachHoneypot(ResponseInterface $response)
{
if ($response->getCSP()->enabled()) {
// Add id attribute to the container tag.
$this->config->container = str_ireplace(
'>{template}',
' id="' . $this->config->containerId . '">{template}',
$this->config->container
);
}

$prepField = $this->prepareTemplate($this->config->template);

$body = $response->getBody();
$body = str_ireplace('</form>', $prepField . '</form>', $body);

if ($response->getCSP()->enabled()) {
// Add style tag for the container tag in the head tag.
$style = '<style ' . csp_style_nonce() . '>#' . $this->config->containerId . ' { display:none }</style>';
$body = str_ireplace('</head>', $style . '</head>', $body);
}

$response->setBody($body);
}

Expand Down
40 changes: 32 additions & 8 deletions tests/system/Honeypot/HoneypotTest.php
Expand Up @@ -11,13 +11,16 @@

namespace CodeIgniter\Honeypot;

use CodeIgniter\Config\Factories;
use CodeIgniter\Config\Services;
use CodeIgniter\Filters\Filters;
use CodeIgniter\Honeypot\Exceptions\HoneypotException;
use CodeIgniter\HTTP\CLIRequest;
use CodeIgniter\HTTP\IncomingRequest;
use CodeIgniter\HTTP\Response;
use CodeIgniter\Test\CIUnitTestCase;
use Config\App;
use Config\Honeypot as HoneypotConfig;

/**
* @backupGlobals enabled
Expand All @@ -28,7 +31,7 @@
*/
final class HoneypotTest extends CIUnitTestCase
{
private \Config\Honeypot $config;
private HoneypotConfig $config;
private Honeypot $honeypot;

/**
Expand All @@ -41,14 +44,16 @@ final class HoneypotTest extends CIUnitTestCase
protected function setUp(): void
{
parent::setUp();
$this->config = new \Config\Honeypot();

$this->config = new HoneypotConfig();
$this->honeypot = new Honeypot($this->config);

unset($_POST[$this->config->name]);
$_SERVER['REQUEST_METHOD'] = 'POST';
$_POST[$this->config->name] = 'hey';
$this->request = Services::request(null, false);
$this->response = Services::response();

$this->request = Services::request(null, false);
$this->response = Services::response();
}

public function testAttachHoneypot()
Expand All @@ -66,16 +71,35 @@ public function testAttachHoneypotAndContainer()
{
$this->response->setBody('<form></form>');
$this->honeypot->attachHoneypot($this->response);
$expected = '<form><div style="display:none"><label>Fill This Field</label><input type="text" name="honeypot" value=""/></div></form>';
$expected = '<form><div style="display:none"><label>Fill This Field</label><input type="text" name="honeypot" value=""></div></form>';
$this->assertSame($expected, $this->response->getBody());

$this->config->container = '<div class="hidden">{template}</div>';
$this->response->setBody('<form></form>');
$this->honeypot->attachHoneypot($this->response);
$expected = '<form><div class="hidden"><label>Fill This Field</label><input type="text" name="honeypot" value=""/></div></form>';
$expected = '<form><div class="hidden"><label>Fill This Field</label><input type="text" name="honeypot" value=""></div></form>';
$this->assertSame($expected, $this->response->getBody());
}

public function testAttachHoneypotAndContainerWithCSP()
{
$this->resetServices();

$config = new App();
$config->CSPEnabled = true;
Factories::injectMock('config', 'App', $config);
Copy link
Member Author

@kenjis kenjis Dec 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because CodeIgniter\HTTP\ContentSecurityPolicy depends on config('App')
(and Response pulls Services::csp()), we must inject Config\App into the Factories.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation! We can also just use the factory version here:

config('App')->CSPEnabled = true;

But nothing wrong with the explicit version.

Copy link
Member Author

@kenjis kenjis Dec 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Factories are reset each time during tests, so there is probably no problem with that way.
However, I do not think it is healthy that most framework classes depend on Factories (config()).

It's not easy to get rid of this dependency now, but I would like to reduce it if possible.

$this->response = Services::response($config, false);

$this->config = new HoneypotConfig();
$this->honeypot = new Honeypot($this->config);

$this->response->setBody('<head></head><body><form></form></body>');
$this->honeypot->attachHoneypot($this->response);

$regex = '!<head><style nonce="[0-9a-f]+">#hpc { display:none }</style></head><body><form><div style="display:none" id="hpc"><label>Fill This Field</label><input type="text" name="honeypot" value=""></div></form></body>!u';
$this->assertMatchesRegularExpression($regex, $this->response->getBody());
}

public function testHasntContent()
{
unset($_POST[$this->config->name]);
Expand Down Expand Up @@ -147,7 +171,7 @@ public function testHoneypotFilterAfter()

public function testEmptyConfigContainer()
{
$config = new \Config\Honeypot();
$config = new HoneypotConfig();
$config->container = '';
$honeypot = new Honeypot($config);

Expand All @@ -159,7 +183,7 @@ public function testEmptyConfigContainer()

public function testNoTemplateConfigContainer()
{
$config = new \Config\Honeypot();
$config = new HoneypotConfig();
$config->container = '<div></div>';
$honeypot = new Honeypot($config);

Expand Down
2 changes: 2 additions & 0 deletions user_guide_src/source/changelogs/v4.3.0.rst
Expand Up @@ -291,6 +291,7 @@ The following items are affected:

- Typography class: Creation of ``br`` tag
- View Parser: The ``nl2br`` filter
- Honeypot: ``input`` tag
- Form helper
- HTML helper
- Common Functions
Expand Down Expand Up @@ -364,3 +365,4 @@ Bugs Fixed
- Fixed a bug when all types of ``Prepared Queries`` were returning a ``Result`` object instead of a bool value for write-type queries.
- Fixed a bug with variable filtering in JSON requests using with ``IncomingRequest::getVar()`` or ``IncomingRequest::getJsonVar()`` methods.
- Fixed a bug when variable type may be changed when using a specified index with ``IncomingRequest::getVar()`` or ``IncomingRequest::getJsonVar()`` methods.
- Fixed a bug that Honeypot field appears when CSP is enabled. See also :ref:`upgrade-430-honeypot-and-csp`.
14 changes: 14 additions & 0 deletions user_guide_src/source/installation/upgrade_430.rst
Expand Up @@ -216,6 +216,16 @@ Database
- The ``Model::update()`` method now raises a ``DatabaseException`` if it generates an SQL
statement without a WHERE clause. If you need to update all records in a table, use Query Builder instead. E.g., ``$model->builder()->update($data)``.

.. _upgrade-430-honeypot-and-csp:

Honeypot and CSP
================

When CSP is enabled, id attribute ``id="hpc"`` will be injected into the container tag
for the Honeypot field to hide the field. If the id is already used in your views, you need to change it
with ``Config\Honeypot::$containerId``.
And you can remove ``style="display:none"`` in ``Config\Honeypot::$container``.

Others
======

Expand Down Expand Up @@ -260,6 +270,10 @@ Config
- app/Config/Exceptions.php
- Two additional public properties were added: ``$logDeprecations`` and ``$deprecationLogLevel``.
See See :ref:`logging_deprecation_warnings` for details.
- app/Config/Honeypot.php
- The new property ``$containerId`` is added to set id attribute value for the container tag
when CSP is enabled.
- The ``input`` tag in the property ``$template`` value has been changed to HTML5 compatible.
- app/Config/Logger.php
- The property ``$threshold`` has been changed to ``9`` in other than ``production``
environment.
Expand Down
27 changes: 16 additions & 11 deletions user_guide_src/source/libraries/honeypot.rst
@@ -1,35 +1,40 @@
=====================
##############
Honeypot Class
=====================
##############

The Honeypot Class makes it possible to determine when a Bot makes a request to a CodeIgniter4 application,
if it's enabled in ``Application\Config\Filters.php`` file. This is done by attaching form fields to any form,
if it's enabled in **app\Config\Filters.php** file. This is done by attaching form fields to any form,
and this form field is hidden from a human but accessible to a Bot. When data is entered into the field, it's
assumed the request is coming from a Bot, and you can throw a ``HoneypotException``.

.. contents::
:local:
:depth: 2

*****************
Enabling Honeypot
=====================
*****************

To enable a Honeypot, changes have to be made to the **app/Config/Filters.php**. Just uncomment honeypot
from the ``$globals`` array, like:

.. literalinclude:: honeypot/001.php

A sample Honeypot filter is bundled, as ``system/Filters/Honeypot.php``.
If it is not suitable, make your own at ``app/Filters/Honeypot.php``,
A sample Honeypot filter is bundled, as **system/Filters/Honeypot.php**.
If it is not suitable, make your own at **app/Filters/Honeypot.php**,
and modify the ``$aliases`` in the configuration appropriately.

********************
Customizing Honeypot
=====================
********************

Honeypot can be customized. The fields below can be set either in
**app/Config/Honeypot.php** or in **.env**.

* ``hidden`` - true|false to control visibility of the honeypot field; default is ``true``
* ``label`` - HTML label for the honeypot field, default is 'Fill This Field'
* ``name`` - name of the HTML form field used for the template; default is 'honeypot'
* ``template`` - form field template used for the honeypot; default is '<label>{label}</label><input type="text" name="{name}" value=""/>'
* ``$hidden`` - ``true`` or ``false`` to control visibility of the honeypot field; default is ``true``
* ``$label`` - HTML label for the honeypot field, default is ``'Fill This Field'``
* ``$name`` - name of the HTML form field used for the template; default is ``'honeypot'``
* ``$template`` - form field template used for the honeypot; default is ``'<label>{label}</label><input type="text" name="{name}" value="">'``
* ``$container`` - container tag for the template; default is ``'<div style="display:none">{template}</div>'``.
If you enables CSP, you can remove ``style="display:none"``.
* ``$containerId`` - [Since v4.3.0] this setting is used only when you enables CSP. You can change the id attribute for the container tag; default is ``'hpc'``
4 changes: 4 additions & 0 deletions user_guide_src/source/libraries/honeypot/001.php
Expand Up @@ -6,14 +6,18 @@

class Filters extends BaseConfig
{
// ...

public $globals = [
'before' => [
'honeypot',
// 'csrf',
// 'invalidchars',
],
'after' => [
'toolbar',
'honeypot',
// 'secureheaders',
],
];

Expand Down