Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 23 additions & 6 deletions analyser/test/marketing_consent.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,19 +36,23 @@ class marketing_consent implements test_interface
/** @var \phpbb\config\config */
protected $config;

/** @var \phpbb\extension\manager */
protected $extension_manager;

/**
* @param \phpbb\config\config $config Config object
* @param \phpbb\config\config $config Config object
* @param \phpbb\extension\manager $extension_manager Extension manager object
*/
public function __construct(\phpbb\config\config $config)
public function __construct(\phpbb\config\config $config, \phpbb\extension\manager $extension_manager)
{
$this->config = $config;
$this->extension_manager = $extension_manager;
}

/**
* {@inheritDoc}
*
* Recommend reviewing Require marketing consent when executable script tags
* are present and Consent Manager marketing is available.
* Recommend reviewing consent requirements when executable script tags are present.
*/
public function run($ad_code)
{
Expand Down Expand Up @@ -85,6 +89,7 @@ protected function get_recommendation_message($ad_code)
}

$google_consent_aware_sources = \phpbb\ads\ad\manager::get_google_consent_aware_script_sources($ad_code);
$consent_manager_available = $this->is_consent_manager_available();

foreach ($matches[1] as $index => $attributes)
{
Expand All @@ -101,15 +106,27 @@ protected function get_recommendation_message($ad_code)

if ($this->contains_marketing_host_hint($attributes, $content))
{
return 'MARKETING_CONSENT_VENDOR_RECOMMENDED';
return $consent_manager_available ? 'MARKETING_CONSENT_VENDOR_RECOMMENDED' : 'MARKETING_VENDOR_REVIEW_RECOMMENDED';
}

return 'MARKETING_CONSENT_RECOMMENDED';
return $consent_manager_available ? 'MARKETING_CONSENT_RECOMMENDED' : 'MARKETING_REVIEW_RECOMMENDED';
}

return false;
}

/**
* Check whether Consent Manager marketing controls are available.
*
* @return bool
*/
protected function is_consent_manager_available()
{
return $this->extension_manager->is_enabled('phpbb/consentmanager')
&& $this->config->offsetExists('consentmanager_marketing_enabled')
&& (bool) $this->config['consentmanager_marketing_enabled'];
}

/**
* Check for known advertising vendor hints inside script markup or content.
*
Expand Down
2 changes: 1 addition & 1 deletion config/analyser.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ services:
class: phpbb\ads\analyser\test\marketing_consent
arguments:
- '@config'
- '@ext.manager'
tags:
- { name: phpbb.ads.analyser.test }

Expand All @@ -47,4 +48,3 @@ services:
class: phpbb\ads\analyser\test\iframe
tags:
- { name: phpbb.ads.analyser.test }

1 change: 1 addition & 0 deletions config/services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ services:
- '@phpbb.ads.admin.input'
- '@phpbb.ads.helper'
- '@phpbb.ads.analyser.manager'
- '@ext.manager'
- '@controller.helper'
- '%core.root_path%'
- '%core.php_ext%'
Expand Down
10 changes: 8 additions & 2 deletions controller/admin_controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ class admin_controller
/** @var \phpbb\ads\analyser\manager */
protected $analyser;

/** @var \phpbb\extension\manager */
protected $extension_manager;

/** @var \phpbb\controller\helper */
protected $controller_helper;

Expand All @@ -68,11 +71,12 @@ class admin_controller
* @param \phpbb\ads\controller\admin_input $input Admin input object
* @param \phpbb\ads\controller\helper $helper Helper object
* @param \phpbb\ads\analyser\manager $analyser Ad code analyser object
* @param \phpbb\extension\manager $extension_manager Extension manager object
* @param \phpbb\controller\helper $controller_helper Controller helper object
* @param string $root_path phpBB root path
* @param string $php_ext PHP extension
*/
public function __construct(\phpbb\template\template $template, \phpbb\language\language $language, \phpbb\request\request $request, \phpbb\ads\ad\manager $manager, \phpbb\config\db_text $config_text, \phpbb\config\config $config, \phpbb\ads\controller\admin_input $input, \phpbb\ads\controller\helper $helper, \phpbb\ads\analyser\manager $analyser, \phpbb\controller\helper $controller_helper, $root_path, $php_ext)
public function __construct(\phpbb\template\template $template, \phpbb\language\language $language, \phpbb\request\request $request, \phpbb\ads\ad\manager $manager, \phpbb\config\db_text $config_text, \phpbb\config\config $config, \phpbb\ads\controller\admin_input $input, \phpbb\ads\controller\helper $helper, \phpbb\ads\analyser\manager $analyser, \phpbb\extension\manager $extension_manager, \phpbb\controller\helper $controller_helper, $root_path, $php_ext)
{
$this->template = $template;
$this->language = $language;
Expand All @@ -83,6 +87,7 @@ public function __construct(\phpbb\template\template $template, \phpbb\language\
$this->input = $input;
$this->helper = $helper;
$this->analyser = $analyser;
$this->extension_manager = $extension_manager;
$this->controller_helper = $controller_helper;

$this->language->add_lang('posting'); // Used by banner_upload() file errors
Expand Down Expand Up @@ -537,7 +542,8 @@ protected function toggle_permission($user_id)
*/
protected function is_consent_manager_available()
{
return $this->config->offsetExists('consentmanager_marketing_enabled')
return $this->extension_manager->is_enabled('phpbb/consentmanager')
&& $this->config->offsetExists('consentmanager_marketing_enabled')
&& (bool) $this->config['consentmanager_marketing_enabled'];
}
}
2 changes: 2 additions & 0 deletions language/en/acp.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@
'SCRIPT_WITHOUT_ASYNC' => '<strong>Non-asynchronous javascript</strong><br>This advertisement code loads JavaScript code in a non-asynchronous way. This means it will block any other JavaScript from loading until it has completed loading, which can affect page load performance. Use of the <samp>async</samp> attribute can speed up the page load.',
'MARKETING_CONSENT_RECOMMENDED' => '<strong>Require marketing consent</strong><br>This advertisement contains executable <samp>&lt;script&gt;</samp> tags. If this ad loads marketing, tracking, cookies, or other consent-controlled resources, ensure <strong>Require marketing consent</strong> is enabled below for this ad so its scripts are deferred until the visitor allows marketing in Privacy Settings.',
'MARKETING_CONSENT_VENDOR_RECOMMENDED' => '<strong>Known ad vendor detected</strong><br>This advertisement contains executable <samp>&lt;script&gt;</samp> tags from a known advertising or marketing vendor. Ensure <strong>Require marketing consent</strong> is enabled below for this ad so its scripts are deferred until the visitor allows marketing in Privacy Settings.',
'MARKETING_REVIEW_RECOMMENDED' => '<strong>Review consent requirements</strong><br>This advertisement contains executable <samp>&lt;script&gt;</samp> tags. If this ad loads marketing, tracking, cookies, or other consent-controlled resources, review the advertisement code and your privacy settings to ensure it complies with your user privacy policies.',
'MARKETING_VENDOR_REVIEW_RECOMMENDED' => '<strong>Known ad vendor detected</strong><br>This advertisement contains executable <samp>&lt;script&gt;</samp> tags from a known advertising or marketing vendor. Review the advertisement code and your privacy settings to ensure it complies with your user privacy policies.',
'ALERT_USAGE' => '<strong>Usage of <samp>alert()</samp></strong><br>Your code uses the <samp>alert()</samp> function which is not a good practice and can distract users. Some browsers may also block page load and display additional warnings to the user.',
'LOCATION_CHANGE' => '<strong>Redirection</strong><br>Your code appears it can redirect a user to another page or site. Redirects can sometimes send users to unintended, often malicious, destinations. Please verify the integrity of your advertisement code’s redirection destination.',
'IFRAME_USAGE' => '<strong>Usage of <samp>&lt;iframe&gt;</samp></strong><br>Your code contains HTML-encoded <samp>&lt;iframe&gt;</samp> tags. Because iframes can introduce third-party tracking or data collection, please review this advertisement snippet to ensure it complies with your user privacy policies.',
Expand Down
14 changes: 13 additions & 1 deletion tests/analyser/analyser_base.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ class analyser_base extends \phpbb_test_case
/** @var \phpbb\config\config */
protected $config;

/** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\extension\manager */
protected $extension_manager;

protected static function setup_extensions()
{
return array('phpbb/ads');
Expand All @@ -53,6 +56,15 @@ protected function setUp(): void
$this->config = new \phpbb\config\config(array(
'consentmanager_marketing_enabled' => 0,
));
$this->extension_manager = $this->getMockBuilder('\phpbb\extension\manager')
->disableOriginalConstructor()
->getMock();
$this->extension_manager->method('is_enabled')
->with('phpbb/consentmanager')
->willReturnCallback(function ()
{
return (bool) $this->config['consentmanager_extension_enabled'];
});

// Tests
$tests = array(
Expand All @@ -73,7 +85,7 @@ protected function setUp(): void
}
else if ($test === 'marketing_consent')
{
$analyser_tests['phpbb.ads.analyser.test.' . $test] = new $class($this->config);
$analyser_tests['phpbb.ads.analyser.test.' . $test] = new $class($this->config, $this->extension_manager);
}
else
{
Expand Down
30 changes: 30 additions & 0 deletions tests/analyser/run_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ public function run_data()
)),
'allows consent-aware iframe placeholder' => array('&lt;iframe data-consent-src=&quot;https://some.url&quot; width=&quot;640&quot; height=&quot;360&quot; allowfullscreen&gt;&lt;/iframe&gt;', false, array(), array()),
'recommends marketing consent for generic ad script' => array('<script src="https://ads.example.com/tag.js" async></script>', false, array(
'consentmanager_extension_enabled' => 1,
'consentmanager_marketing_enabled' => 1,
), array(
array(
Expand All @@ -111,6 +112,7 @@ public function run_data()
),
)),
'recommends marketing consent for inline cookie script' => array('<script>document.cookie = "ad=1";</script>', false, array(
'consentmanager_extension_enabled' => 1,
'consentmanager_marketing_enabled' => 1,
), array(
array(
Expand All @@ -119,6 +121,7 @@ public function run_data()
),
)),
'recommends marketing consent for known non-Google vendor script' => array('<script src="https://cdn.taboola.com/tag.js" async></script>', false, array(
'consentmanager_extension_enabled' => 1,
'consentmanager_marketing_enabled' => 1,
), array(
array(
Expand All @@ -127,21 +130,27 @@ public function run_data()
),
)),
'allows AdSense loader under Google Consent Mode' => array('<script async src="https://pagead2.googlesyndication.com/pagead/js/adsbygoogle.js?client=ca-pub-123"></script>', false, array(
'consentmanager_extension_enabled' => 1,
'consentmanager_marketing_enabled' => 1,
), array()),
'allows full AdSense snippet under Google Consent Mode' => array('<script async src="https://pagead2.googlesyndication.com/pagead/js/adsbygoogle.js?client=ca-pub-123" crossorigin="anonymous"></script><ins class="adsbygoogle" style="display:block" data-ad-client="ca-pub-123" data-ad-slot="456" data-ad-format="auto" data-full-width-responsive="true"></ins><script>(adsbygoogle = window.adsbygoogle || []).push({});</script>', false, array(
'consentmanager_extension_enabled' => 1,
'consentmanager_marketing_enabled' => 1,
), array()),
'allows GPT loader under Google Consent Mode' => array('<script async src="//securepubads.g.doubleclick.net/tag/js/gpt.js"></script>', false, array(
'consentmanager_extension_enabled' => 1,
'consentmanager_marketing_enabled' => 1,
), array()),
'allows non-executable json script' => array('<script type="application/ld+json">{"@context":"https://schema.org"}</script>', false, array(
'consentmanager_extension_enabled' => 1,
'consentmanager_marketing_enabled' => 1,
), array()),
'allows Google ad iframe because marketing consent analyser only handles scripts' => array('<iframe src="https://googleads.g.doubleclick.net/pagead/ads"></iframe>', false, array(
'consentmanager_extension_enabled' => 1,
'consentmanager_marketing_enabled' => 1,
), array()),
'recommends marketing consent regardless of ad consent form value' => array('<script src="https://ads.example.com/tag.js" async></script>', false, array(
'consentmanager_extension_enabled' => 1,
'consentmanager_marketing_enabled' => 1,
), array(
array(
Expand All @@ -150,9 +159,29 @@ public function run_data()
),
)),
'allows generic ad script when Consent Manager marketing category is disabled' => array('<script src="https://ads.example.com/tag.js" async></script>', false, array(
'consentmanager_extension_enabled' => 1,
'consentmanager_marketing_enabled' => 0,
), array()),
'recommends review when Consent Manager extension is disabled' => array('<script src="https://ads.example.com/tag.js" async></script>', false, array(
'consentmanager_extension_enabled' => 0,
'consentmanager_marketing_enabled' => 1,
), array(
array(
'severity' => 'notice',
'lang_key' => 'MARKETING_REVIEW_RECOMMENDED',
),
)),
'recommends vendor review when Consent Manager extension is disabled' => array('<script src="https://cdn.taboola.com/tag.js" async></script>', false, array(
'consentmanager_extension_enabled' => 0,
'consentmanager_marketing_enabled' => 1,
), array(
array(
'severity' => 'notice',
'lang_key' => 'MARKETING_VENDOR_REVIEW_RECOMMENDED',
),
)),
'allows already consent-tagged script' => array('<script type="text/plain" data-consent-category="marketing" src="https://ads.example.com/tag.js"></script>', false, array(
'consentmanager_extension_enabled' => 1,
'consentmanager_marketing_enabled' => 1,
), array()),
);
Expand All @@ -166,6 +195,7 @@ public function run_data()
public function test_run($ad_code, $is_https, $config, $expected)
{
$manager = $this->get_manager();
$this->config['consentmanager_extension_enabled'] = $config['consentmanager_extension_enabled'] ?? 0;
$this->config['consentmanager_marketing_enabled'] = $config['consentmanager_marketing_enabled'] ?? 0;

$this->request
Expand Down
56 changes: 56 additions & 0 deletions tests/controller/admin_controller_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ class admin_controller_test extends \phpbb_database_test_case
/** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\ads\analyser\manager */
protected $analyser;

/** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\extension\manager */
protected $extension_manager;

/** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\controller\helper */
protected $controller_helper;

Expand Down Expand Up @@ -114,6 +117,13 @@ protected function setUp(): void
$this->analyser = $this->getMockBuilder('\phpbb\ads\analyser\manager')
->disableOriginalConstructor()
->getMock();
$this->extension_manager = $this->getMockBuilder('\phpbb\extension\manager')
->disableOriginalConstructor()
->getMock();
$this->extension_manager
->method('is_enabled')
->with('phpbb/consentmanager')
->willReturn(false);
$this->controller_helper = $this->getMockBuilder('\phpbb\controller\helper')
->disableOriginalConstructor()
->getMock();
Expand Down Expand Up @@ -145,6 +155,7 @@ public function get_controller()
$this->input,
$this->helper,
$this->analyser,
$this->extension_manager,
$this->controller_helper,
$this->root_path,
$this->php_ext
Expand All @@ -154,6 +165,50 @@ public function get_controller()
return $controller;
}

public function consent_manager_available_data()
{
return array(
'extension disabled, config enabled' => array(false, true, true, false),
'extension enabled, config enabled' => array(true, true, true, true),
'extension enabled, config disabled' => array(true, true, false, false),
'extension enabled, config missing' => array(true, false, false, false),
);
}

/**
* @dataProvider consent_manager_available_data
*/
public function test_consent_manager_available_flag($extension_enabled, $config_exists, $config_enabled, $expected)
{
if ($config_exists)
{
$this->config['consentmanager_marketing_enabled'] = $config_enabled ? '1' : '0';
}
$this->config->method('offsetExists')
->with('consentmanager_marketing_enabled')
->willReturn($config_exists);
$this->config->method('offsetGet')
->with('consentmanager_marketing_enabled')
->willReturn($config_enabled ? '1' : '0');

$this->extension_manager = $this->getMockBuilder('\phpbb\extension\manager')
->disableOriginalConstructor()
->getMock();
$this->extension_manager->expects(self::once())
->method('is_enabled')
->with('phpbb/consentmanager')
->willReturn($extension_enabled);

$this->template->expects(self::once())
->method('assign_vars')
->with(array(
'S_PHPBB_ADS' => true,
'S_ADS_CONSENTMANAGER_AVAILABLE' => $expected,
));

$this->get_controller();
}

/**
* Test mode_settings()
*/
Expand Down Expand Up @@ -281,6 +336,7 @@ public function test_mode_manage($action, $expected)
$this->input,
$this->helper,
$this->analyser,
$this->extension_manager,
$this->controller_helper,
$this->root_path,
$this->php_ext
Expand Down