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

Add more options for global URL parameter exclusions #22729

Open
wants to merge 61 commits into
base: 5.x-dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
3abdabe
Update global settings to support excluding common PII params
caddoo Oct 30, 2024
6ff6084
Clean up TS
caddoo Nov 4, 2024
baf1d9d
Add UI screenshots (with some commented)
caddoo Nov 4, 2024
d0fb658
Remove empty test
caddoo Nov 4, 2024
0f30e6d
Fix broken UI test
caddoo Nov 4, 2024
8efbeef
Fix broken UI test
caddoo Nov 4, 2024
8269536
Fix broken integration test
caddoo Nov 4, 2024
4d9cd4d
Merge branch '5.x-dev' into dev-18646-add-button-add-list-of-exclusio…
caddoo Nov 4, 2024
75db8c4
Build vue files
innocraft-automation Nov 4, 2024
639ddcc
Update UI to hide the list of common exclusions
caddoo Nov 4, 2024
cd39b1a
Build vue files
innocraft-automation Nov 4, 2024
84dfef1
Update UI screenshot
caddoo Nov 4, 2024
297f59e
Update UI screenshot
caddoo Nov 4, 2024
7a5b07a
Update API interface to keep data integrity
caddoo Nov 6, 2024
8949a69
Switch to using whitelist validator
caddoo Nov 6, 2024
a53ba24
Update exceptions to be translated, add missing integration test
caddoo Nov 6, 2024
21b31db
Restore commented out UI tests
caddoo Nov 6, 2024
e414625
Remove redundant switch statement
caddoo Nov 7, 2024
73c8350
Add return type
caddoo Nov 7, 2024
a6fec12
Move common PII exclusions to config
caddoo Nov 7, 2024
c8d080d
Translate button text
caddoo Nov 7, 2024
36161ad
Add common PII to config
caddoo Nov 7, 2024
da7bd72
Deprecate setGlobalExcludedQueryParameters
caddoo Nov 7, 2024
b623614
Fix CS issues
caddoo Nov 7, 2024
9288ac5
Build vue files
innocraft-automation Nov 7, 2024
4869ad7
Fix deprecated test and update changelog
caddoo Nov 7, 2024
6b93491
Use test config data for test, remove static copy of common PII
caddoo Nov 7, 2024
c5d3fad
Add missing translation key
caddoo Nov 7, 2024
9ea605a
Update UI screenshots
caddoo Nov 7, 2024
b541e1a
Fix tests
caddoo Nov 7, 2024
4c305c7
Merge branch '5.x-dev' into dev-18646-add-button-add-list-of-exclusio…
caddoo Nov 7, 2024
3af38c1
Update plugins/SitesManager/API.php
caddoo Nov 10, 2024
1476f50
Update plugins/SitesManager/API.php
caddoo Nov 10, 2024
d9236c8
Update plugins/SitesManager/API.php
caddoo Nov 10, 2024
69169d2
Update plugins/SitesManager/vue/src/ManageGlobalSettings/ManageGlobal…
caddoo Nov 10, 2024
31864a1
Reorder and remove duplicates for CommonPII params
caddoo Nov 10, 2024
b80ece9
Clear query parameters when custom_exclusions selected
caddoo Nov 11, 2024
2334c8d
Add missing prop type include
caddoo Nov 11, 2024
0998402
Make old API method more compatible with new methods
caddoo Nov 11, 2024
8200473
Merge branch '5.x-dev' into dev-18646-add-button-add-list-of-exclusio…
caddoo Nov 11, 2024
4f8e909
Fix syntax error
caddoo Nov 11, 2024
b4a9eb9
Fix UI screenshot
caddoo Nov 11, 2024
caa50ca
Build vue files
innocraft-automation Nov 11, 2024
643fa6f
Fix UI screenshot
caddoo Nov 11, 2024
bfca00e
Update plugins/SitesManager/API.php
caddoo Nov 12, 2024
dc7251b
Make sure URL params are cleaned before validation, and add tests
caddoo Nov 13, 2024
54ef856
Remove redundant check
caddoo Nov 13, 2024
a26b1c9
Update naming for the exclusion types
caddoo Nov 13, 2024
69dc3fa
Build vue files
innocraft-automation Nov 14, 2024
a121d18
Update wording once more
caddoo Nov 14, 2024
f973c0c
Update ui screenshots
caddoo Nov 14, 2024
4ab0b83
Update ui screenshots
caddoo Nov 14, 2024
14302c9
Update ui screenshots
caddoo Nov 14, 2024
f9aed0e
Update ui screenshots
caddoo Nov 14, 2024
3afa175
Update expected integration test
caddoo Nov 14, 2024
0787133
Update language again
caddoo Nov 14, 2024
4714d7e
Update integration test expected result
caddoo Nov 14, 2024
ca8ab48
Temp split test to get all screenshots
caddoo Nov 14, 2024
f278b42
Fix broken tests
caddoo Nov 14, 2024
cf3f647
Merge branch '5.x-dev' into dev-18646-add-button-add-list-of-exclusio…
caddoo Nov 14, 2024
f141197
Remove temp tests
caddoo Nov 14, 2024
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
50 changes: 49 additions & 1 deletion plugins/SitesManager/API.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class API extends \Piwik\Plugin\API
public const OPTION_EXCLUDED_USER_AGENTS_GLOBAL = 'SitesManager_ExcludedUserAgentsGlobal';
public const OPTION_EXCLUDED_REFERRERS_GLOBAL = 'SitesManager_ExcludedReferrersGlobal';
public const OPTION_KEEP_URL_FRAGMENTS_GLOBAL = 'SitesManager_KeepURLFragmentsGlobal';
public const OPTION_EXCLUDE_TYPE_QUERY_PARAMS_GLOBAL = 'SitesManager_ExcludeTypeQueryParamsGlobal';

/**
* @var SettingsProvider
Expand Down Expand Up @@ -1126,7 +1127,17 @@ public function getExcludedQueryParameters(int $idSite): array
public function getExcludedQueryParametersGlobal()
{
Piwik::checkUserHasSomeViewAccess();
return Option::get(self::OPTION_EXCLUDED_QUERY_PARAMETERS_GLOBAL);

switch ($this->getExclusionTypeForQueryParams()) {
case SitesManager::URL_PARAM_EXCLUSION_TYPE_NAME_NO_EXCLUSIONS:
return '';
case SitesManager::URL_PARAM_EXCLUSION_TYPE_NAME_COMMON_PII_EXCLUSIONS:
return implode(',', SitesManager::COMMON_URL_PARAMS_TO_EXCLUDE);
case SitesManager::URL_PARAM_EXCLUSION_TYPE_NAME_CUSTOM_EXCLUSIONS:
return Option::get(self::OPTION_EXCLUDED_QUERY_PARAMETERS_GLOBAL);
caddoo marked this conversation as resolved.
Show resolved Hide resolved
default:
return Option::get(self::OPTION_EXCLUDED_QUERY_PARAMETERS_GLOBAL);
}
}

/**
Expand Down Expand Up @@ -1346,6 +1357,43 @@ public function setDefaultTimezone($defaultTimezone)
return true;
}

public function setExclusionTypeForQueryParams(string $exclusionTypeForQueryParams): void
caddoo marked this conversation as resolved.
Show resolved Hide resolved
{
Piwik::checkUserHasSuperUserAccess();

if (!in_array($exclusionTypeForQueryParams, SitesManager::URL_PARAM_EXCLUSION_TYPES)) {
throw new Exception($this->translator->translate('SitesManager_ExceptionInvalidExclusionType'));
}
caddoo marked this conversation as resolved.
Show resolved Hide resolved

Option::set(self::OPTION_EXCLUDE_TYPE_QUERY_PARAMS_GLOBAL, $exclusionTypeForQueryParams);

Cache::deleteTrackerCache();
}

/**
* Gets the exclusion type, if the option is not present in the store then it infers the type based on if there are
* custom exclusions already defined.
*
* @return string
*/
public function getExclusionTypeForQueryParams(): string
{
Piwik::checkUserHasSomeViewAccess();

$result = Option::get(self::OPTION_EXCLUDE_TYPE_QUERY_PARAMS_GLOBAL);

if (!empty($result)) {
return $result;
}

$excludedQueryParamsGlobal = Option::get(self::OPTION_EXCLUDED_QUERY_PARAMETERS_GLOBAL);

if (empty($excludedQueryParamsGlobal)) {
return SitesManager::URL_PARAM_EXCLUSION_TYPE_NAME_NO_EXCLUSIONS;
}
return SitesManager::URL_PARAM_EXCLUSION_TYPE_NAME_CUSTOM_EXCLUSIONS;
}

/**
* Update an existing website.
* If only one URL is specified then only the main url will be updated.
Expand Down
14 changes: 13 additions & 1 deletion plugins/SitesManager/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,12 @@ public function globalSettings()
{
Piwik::checkUserHasSuperUserAccess();

return $this->renderTemplate('globalSettings');
return $this->renderTemplate(
'globalSettings',
[
'commonSensitiveQueryParams' => SitesManager::COMMON_URL_PARAMS_TO_EXCLUDE
]
);
}

public function getGlobalSettings()
Expand All @@ -73,6 +78,11 @@ public function getGlobalSettings()
$globalSettings['excludedQueryParametersGlobal'] = API::getInstance()->getExcludedQueryParametersGlobal();
$globalSettings['excludedUserAgentsGlobal'] = API::getInstance()->getExcludedUserAgentsGlobal();
$globalSettings['excludedReferrersGlobal'] = API::getInstance()->getExcludedReferrersGlobal();
$globalSettings['exclusionTypeForQueryParams'] = API::getInstance()->getExclusionTypeForQueryParams();

if ($globalSettings['exclusionTypeForQueryParams'] !== 'custom_exclusions') {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if ($globalSettings['exclusionTypeForQueryParams'] !== 'custom_exclusions') {
if ($globalSettings['exclusionTypeForQueryParams'] !== SitesManager::URL_PARAM_EXCLUSION_TYPE_NAME_CUSTOM_EXCLUSIONS) {

We should prefer the constant if we have it.

Though I am wondering why we need that check here, and not also in another place where the query parameters are accessed:

public static function getTrackerExcludedQueryParameters($website)
{
$excludedQueryParameters = $website['excluded_parameters'];
$globalExcludedQueryParameters = API::getInstance()->getExcludedQueryParametersGlobal();
$excludedQueryParameters .= ',' . $globalExcludedQueryParameters;
return self::filterBlankFromCommaSepList($excludedQueryParameters);
}

Is there any special case where these two areas should differ in a way that cannot be managed inside the API methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, the controller should be dumb, so I've removed that check. The API handles this already. Take a look and let me know if that's what you are thinking, and you can resolve the thread.

$globalSettings['excludedQueryParametersGlobal'] = '';
}

return $response->getResponse($globalSettings);
}
Expand All @@ -95,6 +105,7 @@ public function setGlobalSettings()
$searchKeywordParameters = Common::getRequestVar('searchKeywordParameters', $default = "");
$searchCategoryParameters = Common::getRequestVar('searchCategoryParameters', $default = "");
$keepURLFragments = Common::getRequestVar('keepURLFragments', $default = 0);
$exclusionTypeForQueryParams = Common::getRequestVar('exclusionTypeForQueryParams', $default = "");

$api = API::getInstance();
$api->setDefaultTimezone($timezone);
Expand All @@ -105,6 +116,7 @@ public function setGlobalSettings()
$api->setGlobalExcludedReferrers($excludedReferrers);
$api->setGlobalSearchParameters($searchKeywordParameters, $searchCategoryParameters);
$api->setKeepURLFragmentsGlobal($keepURLFragments);
$api->setExclusionTypeForQueryParams($exclusionTypeForQueryParams);

$toReturn = $response->getResponse();
} catch (Exception $e) {
Expand Down
37 changes: 37 additions & 0 deletions plugins/SitesManager/SitesManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,36 @@ class SitesManager extends \Piwik\Plugin
public const KEEP_URL_FRAGMENT_YES = 1;
public const KEEP_URL_FRAGMENT_NO = 2;

public const COMMON_URL_PARAMS_TO_EXCLUDE = ['creditcardnumber', 'off', 'kreditkarte', 'debitcard', 'kreditkort',
caddoo marked this conversation as resolved.
Show resolved Hide resolved
'kredietkaart', ' kartakredytowa', 'cvv', 'cc', 'ccc', 'cccsc', 'cccvc', 'ccexpiry', 'ccexpyear', 'ccexpmonth',
'cccvv', 'cctype', 'cvc', 'exp', 'ccname', 'cardnumber', 'ccnumber', 'username', 'creditcard', 'name',
'fullname', 'familyname', 'firstname', 'vorname', 'nachname', 'lastname', 'nickname', 'surname', 'login',
'formlogin', 'konto', 'user', 'website', 'domain', 'gender', 'company', 'firma', 'geschlecht', 'email',
'emailaddress', 'emailadresse', 'mail', 'epos', 'ebost', 'epost', 'eposta', 'authpw', 'token_auth',
'tokenauth', 'token', 'pin', 'ibanaccountnum', 'ibanaccountnumber', 'account', 'accountnum', 'auth', 'age',
'alter', 'tel', 'city', 'cell', 'cellphone', 'bic', 'iban', 'swift', 'kontonummer', 'konto', 'kontonr',
'phone', 'mobile', 'mobiili', 'mobilne', 'handynummer', 'téléphone', 'telefono', 'ssn', 'socialsecuritynumber',
'socialsec', 'socsec', 'address', 'addressline1', 'addressline2','billingaddress', 'billingaddress1',
'billingaddress2','shippingaddress', 'shippingaddress1', 'shippingaddress2', 'vat', 'vatnumber', 'gst',
'gstnumber', 'tax', 'taxnumber', 'steuernummer', 'adresse', 'indirizzo', 'adres', 'dirección', 'osoite',
'address1', 'address2', 'address3', 'street', 'strasse', 'rue', 'via', 'ulica', 'calle', 'sokak', 'zip',
'zipcode', 'plz', 'postleitzahl', 'postalcode', 'postcode', 'dateofbirth', 'dob', 'telephone', 'telefon',
'telefonnr', 'telefonnummer', 'password', 'passwort', 'kennwort', 'wachtwoord', 'contraseña', 'passord',
'hasło', 'heslo', 'wagwoord', 'parole', 'contrasenya', 'heslo', 'clientid', 'identifier', 'id',
'consumersecret', 'webhooksecret', 'consumerkey', 'keyconsumersecret', 'keyconsumerkey', 'clientsecret',
'secret', 'secretq', 'secretquestion', 'privatekey', 'publickey', 'pw', 'pwd', 'pwrd', 'pword', 'paword',
'pasword', 'paswort', 'pass'];

public const URL_PARAM_EXCLUSION_TYPE_NAME_NO_EXCLUSIONS = 'no_exclusions';
public const URL_PARAM_EXCLUSION_TYPE_NAME_COMMON_PII_EXCLUSIONS = 'common_pii_exclusions';
public const URL_PARAM_EXCLUSION_TYPE_NAME_CUSTOM_EXCLUSIONS = 'custom_exclusions';

public const URL_PARAM_EXCLUSION_TYPES = [
self::URL_PARAM_EXCLUSION_TYPE_NAME_NO_EXCLUSIONS,
self::URL_PARAM_EXCLUSION_TYPE_NAME_COMMON_PII_EXCLUSIONS,
self::URL_PARAM_EXCLUSION_TYPE_NAME_CUSTOM_EXCLUSIONS
];

/**
* @see \Piwik\Plugin::registerEvents
*/
Expand Down Expand Up @@ -478,6 +508,13 @@ public function getClientSideTranslationKeys(&$translationKeys)
$translationKeys[] = 'SitesManager_SiteWithoutDataOtherInstallMethods';
$translationKeys[] = 'Mobile_NavigationBack';
$translationKeys[] = 'SitesManager_SiteWithoutDataInstallWithX';
$translationKeys[] = 'SitesManager_ExclusionTypeOptionNoExclusions';
$translationKeys[] = 'SitesManager_ExclusionTypeOptionCommonPIIExclusions';
$translationKeys[] = 'SitesManager_ExclusionTypeOptionCustomExclusions';
$translationKeys[] = 'SitesManager_ExclusionTypeDescriptionNoExclusions';
$translationKeys[] = 'SitesManager_ExclusionTypeDescriptionCommonPIIExclusions';
$translationKeys[] = 'SitesManager_ExclusionTypeDescriptionCustomExclusions';
$translationKeys[] = 'SitesManager_ExclusionViewListLink';
}

public static function renderTrackingCodeEmail(int $idSite)
Expand Down
10 changes: 9 additions & 1 deletion plugins/SitesManager/lang/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,14 @@
"SiteWithoutDataSPADescription": "It is easy to start tracking your Single Page Application (SPA) or Progressive Web App (PWA) using Matomo Analytics. The easiest way to do this is using the Matomo Tag Manager (%1$slearn more%2$s) using the steps below, alternatively you can use the JavaScript Tracking code (%3$sfollowing this guide%4$s).",
"SiteWithoutDataSPAFollowStepCompleted": "%1$sCongratulations!%2$s You have successfully installed the Matomo Analytics tracking code via the Matomo Tag Manager. To verify that hits are being tracked, visit your SPA / PWA and check that this data is visible in your Matomo instance.",
"OtherWaysTabDescription": "Even if the solutions provided in the other tabs were not right for you, you can easily setup Matomo with one of the methods below. You may need the following information:",
"ImageTrackingDescription": "When a visitor has disabled JavaScript, or when JavaScript cannot be used, you can use an image tracking link to track visitors. For the whole list of options you can use with an image tracking link, see the %1$sTracking API Documentation%2$s."
"ImageTrackingDescription": "When a visitor has disabled JavaScript, or when JavaScript cannot be used, you can use an image tracking link to track visitors. For the whole list of options you can use with an image tracking link, see the %1$sTracking API Documentation%2$s.",
"ExclusionTypeOptionNoExclusions": "No exclusions",
"ExclusionTypeOptionCommonPIIExclusions": "Sensible PII exclusions",
"ExclusionTypeOptionCustomExclusions": "Custom exclusions",
"ExclusionTypeDescriptionNoExclusions": "Select this option to include all URL parameters in tracking and reports. No parameters will be excluded, providing a complete view of the URL data for each visit.",
"ExclusionTypeDescriptionCommonPIIExclusions": "Choose this option to exclude a common set of potentially sensitive URL parameters from tracking and reports. You can see these exclusions below",
"ExclusionTypeDescriptionCustomExclusions": "Enter your own list of URL parameters to exclude from tracking and reports. Customize further by pressing the button below the textarea to add a predefined set of common exclusions. You can adjust this list as needed to fit your privacy and tracking requirements.",
"ExclusionViewListLink": "View list of exclusions",
"ExceptionInvalidExclusionType": "The exclusion type you provided is invalid"
}
}
5 changes: 5 additions & 0 deletions plugins/SitesManager/stylesheets/SitesManager.less
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@
margin-bottom: 7px;
}
}

.limited-height-scrolling-textarea textarea {
max-height: 400px;
overflow-y: auto;
}
}

td.editable-site-field:hover {
Expand Down
4 changes: 1 addition & 3 deletions plugins/SitesManager/templates/globalSettings.twig
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,5 @@
{% set title %}{{ 'SitesManager_GlobalWebsitesSettings'|translate }}{% endset %}

{% block content %}

<div vue-entry="SitesManager.ManageGlobalSettings"></div>

<div vue-entry="SitesManager.ManageGlobalSettings" common-sensitive-query-params="{{ commonSensitiveQueryParams|json_encode }}"></div>
{% endblock %}
63 changes: 63 additions & 0 deletions plugins/SitesManager/tests/Integration/ApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Piwik\Plugin;
use Piwik\Plugins\IntranetMeasurable\Type as IntranetType;
use Piwik\Plugins\MobileAppMeasurable;
use Piwik\Plugins\SitesManager\SitesManager;
use Piwik\Plugins\WebsiteMeasurable\Type as WebsiteType;
use Piwik\Plugins\SitesManager\API;
use Piwik\Plugins\SitesManager\Model;
Expand Down Expand Up @@ -1614,7 +1615,69 @@ public function testSetGlobalExcludedReferrersWithValidValue()
$this->assertEquals('http://example.com/path', $excludedReferrers);
}

/**
* @dataProvider getExclusionTypesAndExpectedResults
*/
public function testGetExcludedQueryParametersGlobalShowsCorrectParamsDependingOnExclusionType($exclusionType, $expected): void
{
Option::set(API::OPTION_EXCLUDE_TYPE_QUERY_PARAMS_GLOBAL, $exclusionType);
Option::set(API::OPTION_EXCLUDED_QUERY_PARAMETERS_GLOBAL, 'one,two');

$this->assertEquals(
$expected,
API::getInstance()->getExcludedQueryParametersGlobal()
);
}

public function getExclusionTypesAndExpectedResults(): \Generator
{
yield 'no exclusions' => ['no_exclusions', ''];
yield 'common PII exclusions' => ['common_pii_exclusions', implode(',', SitesManager::COMMON_URL_PARAMS_TO_EXCLUDE)];
yield 'custom exclusions' => ['custom_exclusions', 'one,two'];
yield 'empty exclusion type' => ['', 'one,two'];
yield 'false exclusion type' => [false, 'one,two'];
}

/**
* @dataProvider getExclusionTypesWithUrlParamsAndExpectedResults
*/
public function testGetExclusionTypeForQueryParamsReturnsCorrectType($exclusionTypeSetting, string $excludedQueryParamsGlobal, string $expectedType): void
{
if ($exclusionTypeSetting === null) {
Option::delete(API::OPTION_EXCLUDE_TYPE_QUERY_PARAMS_GLOBAL);
} else {
Option::set(API::OPTION_EXCLUDE_TYPE_QUERY_PARAMS_GLOBAL, $exclusionTypeSetting);
}

Option::set(API::OPTION_EXCLUDED_QUERY_PARAMETERS_GLOBAL, $excludedQueryParamsGlobal);

$this->assertEquals(
$expectedType,
API::getInstance()->getExclusionTypeForQueryParams()
);
}

public function testSetExclusionTypeForQueryParamsThrowsExceptionIfInvalidValueProvided(): void
{
$this->expectExceptionMessage('ExceptionInvalidExclusionType');
API::getInstance()->setExclusionTypeForQueryParams('bad_value');
}

public function testSetExclusionTypeForQueryParamsSetsTypeCorrectly(): void
{
API::getInstance()->setExclusionTypeForQueryParams('no_exclusions');
$this->assertEquals(
'no_exclusions',
API::getInstance()->getExclusionTypeForQueryParams()
);
}

public function getExclusionTypesWithUrlParamsAndExpectedResults(): \Generator
{
yield 'option exists already in options store' => ['no_exclusions', '', 'no_exclusions'];
yield 'option doesnt exist and excluded query parameters has data' => [null, 'myapp_name,myapp_email', 'custom_exclusions'];
yield 'option doesnt exist and excluded query parameters has no data' => [null, '', 'no_exclusions'];
}

public function provideContainerConfig()
{
Expand Down
Loading
Loading