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

Rename post_only to secure_only and allow for future enhancements to use bearer tokens #21180

Merged
merged 11 commits into from
Aug 29, 2023
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ The Product Changelog at **[matomo.org/changelog](https://matomo.org/changelog)*

### Usage of authentication tokens
* By default, new authentication tokens will be restricted to be used in POST requests only. This is recommended for improved security. This option can be unselected when creating a new token. Existing tokens will continue to work with both, POST and GET requests.
* A new config setting `only_allow_posted_auth_tokens`, defaulting to `0`, has been added. Enabling this option will prevent any use of tokens in GET API requests.
* A new config setting `only_allow_secure_auth_tokens`, defaulting to `0`, has been added. Enabling this option will prevent any use of tokens in GET API requests.

## Matomo 4.14.0

Expand Down
6 changes: 3 additions & 3 deletions config/global.ini.php
Original file line number Diff line number Diff line change
Expand Up @@ -535,11 +535,11 @@
; information view the FAQ: https://matomo.org/faq/troubleshooting/faq_147/
enable_framed_allow_write_admin_token_auth = 0

; Set to 1 to only allow tokens to be used in POST requests. This will completely prevent using
; Set to 1 to only allow tokens to be used in a secure way (e.g. via POST requests). This will completely prevent using
; token_auth as URL parameter in GET requests. When enabled all new authentication tokens
; will be created as POST only. Previously created tokens will only be accepted in POST requests.
; will be created for Secure use only, and previously created tokens will only be accepted in a secure way (POST requests).
; Recommended for best security.
only_allow_posted_auth_tokens = 0
only_allow_secure_auth_tokens = 0
michalkleiner marked this conversation as resolved.
Show resolved Hide resolved

; language cookie name for session
language_cookie_name = matomo_lang
Expand Down
8 changes: 5 additions & 3 deletions core/API/Request.php
Original file line number Diff line number Diff line change
Expand Up @@ -517,11 +517,13 @@ public static function shouldReloadAuthUsingTokenAuth($request)
}

/**
* Returns true if a token_auth parameter was supplied via a POST request and is not present as a URL parameter
* Returns true if a token_auth parameter was supplied via a secure mechanism and is not present as a URL parameter
* At the moment POST requests are checked, but in future other mechanism such as Authorisation HTTP header
* and bearer tokens might be used as well.
*
* @return bool True if token supplied via POST request
* @return bool True if token was supplied in a secure way
*/
public static function isTokenAuthPosted(): bool
public static function isTokenAuthProvidedSecurely(): bool
{
return (\Piwik\Request::fromGet()->getStringParameter('token_auth', '') === '' &&
\Piwik\Request::fromPost()->getStringParameter('token_auth', '') !== '');
Expand Down
2 changes: 1 addition & 1 deletion core/Db/Schema/Mysql.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public function getTablesCreateSql()
last_used DATETIME NULL,
date_created DATETIME NOT NULL,
date_expired DATETIME NULL,
post_only TINYINT(2) unsigned NOT NULL DEFAULT '0',
secure_only TINYINT(2) unsigned NOT NULL DEFAULT '0',
PRIMARY KEY(idusertokenauth),
UNIQUE KEY uniq_password(password)
) ENGINE=$engine DEFAULT CHARSET=$charset
Expand Down
45 changes: 45 additions & 0 deletions core/Updates/5.0.0-rc2.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php

/**
* Matomo - free/libre analytics platform
*
* @link https://matomo.org
* @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
*
*/

namespace Piwik\Updates;

use Piwik\Updater;
use Piwik\Updates as PiwikUpdates;
use Piwik\Updater\Migration\Factory as MigrationFactory;

/**
* Update for version 5.0.0-rc2
*/
class Updates_5_0_0_rc2 extends PiwikUpdates
{
/**
* @var MigrationFactory
*/
private $migration;

public function __construct(MigrationFactory $factory)
{
$this->migration = $factory;
}

public function getMigrations(Updater $updater)
{
$migration1 = $this->migration->db->changeColumn('user_token_auth', 'post_only', 'secure_only', "TINYINT(2) UNSIGNED NOT NULL DEFAULT '0'");

return [
$migration1,
];
}

public function doUpdate(Updater $updater)
{
$updater->executeMigrations(__FILE__, $this->getMigrations($updater));
}
}
2 changes: 1 addition & 1 deletion core/Version.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ final class Version
* The current Matomo version.
* @var string
*/
const VERSION = '5.0.0-rc1';
const VERSION = '5.0.0-rc2';

const MAJOR_VERSION = 5;

Expand Down
2 changes: 1 addition & 1 deletion plugins/Login/Login.php
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public function onFailedAPILogin()
if ($this->isModuleIsAPI()) {

// Throw an exception if a token was provided but it was invalid
if (Request::isTokenAuthPosted()) {
if (Request::isTokenAuthProvidedSecurely()) {
throw new NoAccessException('Unable to authenticate with the provided token. It is either invalid or expired.');
} else {
throw new NoAccessException('Unable to authenticate with the provided token. It is either invalid, expired or is required to be sent as a POST parameter.');
Expand Down
6 changes: 3 additions & 3 deletions plugins/UsersManager/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -384,13 +384,13 @@ public function addNewToken()
Nonce::checkNonce(self::NONCE_ADD_AUTH_TOKEN);

$description = \Piwik\Request::fromRequest()->getStringParameter('description', '');
$postOnly = \Piwik\Request::fromRequest()->getBoolParameter('post_only', false);
$secureOnly = \Piwik\Request::fromRequest()->getBoolParameter('secure_only', false);

$login = Piwik::getCurrentUserLogin();

$generatedToken = $this->userModel->generateRandomTokenAuth();

$this->userModel->addTokenAuth($login, $generatedToken, $description, Date::now()->getDatetime(), null, false, $postOnly);
$this->userModel->addTokenAuth($login, $generatedToken, $description, Date::now()->getDatetime(), null, false, $secureOnly);

$container = StaticContainer::getContainer();
$email = $container->make(TokenAuthCreatedEmail::class, [
Expand All @@ -408,7 +408,7 @@ public function addNewToken()
return $this->renderTemplate('addNewToken', [
'nonce' => Nonce::getNonce(self::NONCE_ADD_AUTH_TOKEN),
'noDescription' => $noDescription,
'forcePostOnly' => GeneralConfig::getConfigValue('only_allow_posted_auth_tokens')
'forceSecureOnly' => GeneralConfig::getConfigValue('only_allow_secure_auth_tokens')
]);
}

Expand Down
25 changes: 13 additions & 12 deletions plugins/UsersManager/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ private function generateTokenAuth()
* @param $dateCreated
* @param null $dateExpired
* @param false $isSystemToken
* @param bool $postOnly True if this token can only be used in POST requests, default false
* @param bool $secureOnly True if this token can only be used in a secure way (e.g. POST requests), default false
*
* @return int Primary key of the new token auth
* @throws \Piwik\Tracker\Db\DbException
Expand All @@ -336,7 +336,7 @@ public function addTokenAuth(
$dateCreated,
$dateExpired = null,
$isSystemToken = false,
bool $postOnly = false
bool $secureOnly = false
) {
if (!$this->getUser($login)) {
throw new \Exception('User ' . $login . ' does not exist');
Expand All @@ -351,13 +351,13 @@ public function addTokenAuth(

$isSystemToken = (int)$isSystemToken;

$insertSql = "INSERT INTO " . $this->tokenTable . ' (login, description, password, date_created, date_expired, system_token, hash_algo, post_only) VALUES (?, ?, ?, ?, ?, ?, ?, ?)';
$insertSql = "INSERT INTO " . $this->tokenTable . ' (login, description, password, date_created, date_expired, system_token, hash_algo, secure_only) VALUES (?, ?, ?, ?, ?, ?, ?, ?)';

$tokenAuth = $this->hashTokenAuth($tokenAuth);

$db = $this->getDb();
$db->query($insertSql,
[$login, $description, $tokenAuth, $dateCreated, $dateExpired, $isSystemToken, self::TOKEN_HASH_ALGO, $postOnly]);
[$login, $description, $tokenAuth, $dateCreated, $dateExpired, $isSystemToken, self::TOKEN_HASH_ALGO, $secureOnly]);

return $db->lastInsertId();
}
Expand Down Expand Up @@ -392,15 +392,16 @@ private function getQueryNotExpiredToken()
* Attempt to load a valid auth token
*
* @param string|null $tokenAuth The token auth string
* @param bool $isTokenPosted True if the token was sent via a POST request
* @param bool $isTokenSecured True if the token was sent via a secure mechanism (POST request, Auth header)
*
* @return array|bool An array representing the token record, or null if not found
* @throws \Exception
*/
private function getTokenByTokenAuthIfNotExpired(?string $tokenAuth, bool $isTokenPosted)
private function getTokenByTokenAuthIfNotExpired(?string $tokenAuth, bool $isTokenSecured)
{
// If the token wasn't posted and use of posted tokens is enforced globally then don't attempt to find the token
if (GeneralConfig::getConfigValue('only_allow_posted_auth_tokens') && !$isTokenPosted) {
// If the token wasn't provided via a secure mechanism and use of secure tokens is enforced globally
// then don't attempt to find the token
if (GeneralConfig::getConfigValue('only_allow_secure_auth_tokens') && !$isTokenSecured) {
return false;
}

Expand All @@ -412,9 +413,9 @@ private function getTokenByTokenAuthIfNotExpired(?string $tokenAuth, bool $isTok

$sql = "SELECT * FROM " . $this->tokenTable . " WHERE `password` = ? AND " . $expired['sql'];

// If the token was not send via a POST request then exclude post_only tokens
if (!$isTokenPosted) {
$sql .= " AND post_only = 0";
// If the token was not send via a secure mechanism then exclude secure_only tokens
if (!$isTokenSecured) {
$sql .= " AND secure_only = 0";
}

$token = $db->fetchRow($sql, $bind);
Expand Down Expand Up @@ -558,7 +559,7 @@ public function getUserByTokenAuth(?string $tokenAuth): ?array
return (is_array($row) ? $row : null);
}

$token = $this->getTokenByTokenAuthIfNotExpired($tokenAuth, \Piwik\API\Request::isTokenAuthPosted());
$token = $this->getTokenByTokenAuthIfNotExpired($tokenAuth, \Piwik\API\Request::isTokenAuthProvidedSecurely());
if (!empty($token)) {
$db = $this->getDb();
$row = $db->fetchRow("SELECT * FROM " . $this->userTable . " WHERE `login` = ?", $token['login']);
Expand Down
8 changes: 4 additions & 4 deletions plugins/UsersManager/UsersManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,8 @@ public function getClientSideTranslationKeys(&$translationKeys)
$translationKeys[] = 'UsersManager_AreYouSureAddCapability';
$translationKeys[] = 'UsersManager_AreYouSureChangeDetails';
$translationKeys[] = 'UsersManager_AreYouSureRemoveCapability';
$translationKeys[] = 'UsersManager_AuthTokenPostOnlyHelp';
$translationKeys[] = 'UsersManager_AuthTokenPostOnlyHelpForced';
$translationKeys[] = 'UsersManager_AuthTokenSecureOnlyHelp';
$translationKeys[] = 'UsersManager_AuthTokenSecureOnlyHelpForced';
$translationKeys[] = 'UsersManager_AuthTokenPurpose';
$translationKeys[] = 'UsersManager_AuthTokens';
$translationKeys[] = 'UsersManager_BackToUser';
Expand Down Expand Up @@ -356,8 +356,7 @@ public function getClientSideTranslationKeys(&$translationKeys)
$translationKeys[] = 'UsersManager_PrivAdmin';
$translationKeys[] = 'UsersManager_PrivNone';
$translationKeys[] = 'UsersManager_PrivView';
$translationKeys[] = 'UsersManager_OnlyAllowPostRequests';
$translationKeys[] = 'UsersManager_PostOnly';
$translationKeys[] = 'UsersManager_OnlyAllowSecureRequests';
$translationKeys[] = 'UsersManager_RemovePermissions';
$translationKeys[] = 'UsersManager_RemoveSuperuserAccessConfirm';
$translationKeys[] = 'UsersManager_RemoveUserAccess';
Expand All @@ -372,6 +371,7 @@ public function getClientSideTranslationKeys(&$translationKeys)
$translationKeys[] = 'UsersManager_RoleFor';
$translationKeys[] = 'UsersManager_RolesHelp';
$translationKeys[] = 'UsersManager_SaveBasicInfo';
$translationKeys[] = 'UsersManager_SecureUseOnly';
$translationKeys[] = 'UsersManager_SendInvite';
$translationKeys[] = 'UsersManager_SetPermission';
$translationKeys[] = 'UsersManager_Status';
Expand Down
8 changes: 4 additions & 4 deletions plugins/UsersManager/lang/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,9 @@
"InviteActionNotes": "Please notes that resending an invite or copy invite link will extend the time limit for previous invites by %1$s days.",
"CopyDenied": "The request is not allowed due to your browser's settings.",
"CopyDeniedHints": "Please try again by either switching browsers or copying and sharing this link directly instead: %1$s",
"AuthTokenPostOnlyHelp": "Enable this option to only allow this token to be used for POST requests, this is recommended as a best security practice. The token will then not be valid as a URL parameter in GET requests.",
"AuthTokenPostOnlyHelpForced": "The system administrator has configured Matomo to only allow tokens to be created for use in POST requests, you cannot change this token option.",
"OnlyAllowPostRequests": "Only allow POST requests",
"PostOnly": "POST only"
"AuthTokenSecureOnlyHelp": "Enable this option to only allow this token to be used in a secure way (e.g. POST requests), this is recommended as a best security practice. The token will then not be valid as a URL parameter in GET requests.",
"AuthTokenSecureOnlyHelpForced": "The system administrator has configured Matomo to only allow tokens to be created for use in secure way (e.g. via POST requests), you cannot change this token option.",
"OnlyAllowSecureRequests": "Only allow secure requests",
"SecureUseOnly": "Secure use only"
}
}
2 changes: 1 addition & 1 deletion plugins/UsersManager/templates/addNewToken.twig
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
vue-entry="UsersManager.AddNewToken"
no-description="{{ noDescription|json_encode }}"
form-nonce="{{ nonce|json_encode }}"
force-post-only="{{ forcePostOnly|json_encode }}"
force-secure-only="{{ forceSecureOnly|json_encode }}"
></div>

{% endblock %}
6 changes: 3 additions & 3 deletions plugins/UsersManager/tests/Integration/ModelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public function test_addTokenAuth_minimal()
'last_used' => null,
'date_created' => '2020-01-02 03:04:05',
'date_expired' => null,
'post_only' => '0'
'secure_only' => '0'
)), $tokens);
}

Expand All @@ -148,7 +148,7 @@ public function test_addTokenAuth_expire()
'last_used' => null,
'date_created' => '2020-01-02 03:04:05',
'date_expired' => '2030-01-05 03:04:05',
'post_only' => '0'
'secure_only' => '0'
)), $tokens);
}

Expand Down Expand Up @@ -210,7 +210,7 @@ public function test_getAllNonSystemTokensForLogin_returnsNotExpiredToken()
'last_used' => null,
'date_created' => '2020-01-02 03:04:05',
'date_expired' => '2030-01-05 03:04:05',
'post_only' => '0'
'secure_only' => '0'
)), $tokens);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@

/**
* @group UsersManager
* @group TokenPostOnlyTest
* @group TokenSecureOnlyTest
*/
class TokenPostOnlyTest extends IntegrationTestCase
class TokenSecureOnlyTest extends IntegrationTestCase
{
protected static $tokenPostOnly = 'f3fa8c38fd277a9af0fab7e35f9736fe';
protected static $tokenSecureOnly = 'f3fa8c38fd277a9af0fab7e35f9736fe';

public static function beforeTableDataCached()
{
Expand All @@ -36,20 +36,20 @@ private static function createUserAndTokens()
UsersManagerAPI::getInstance()->setUserAccess('user1', 'view', [1]);

$userModel = new UsersManagerModel();
$userModel->addTokenAuth('user1', self::$tokenPostOnly, 'Post Only', '2020-01-02 03:04:05',
$userModel->addTokenAuth('user1', self::$tokenSecureOnly, 'Secure Only', '2020-01-02 03:04:05',
null, false, true);
}
}

/**
* Post Only tokens should return a 401 code if used in a GET request
* Secure only tokens should return a 401 code if used in a GET request
*/
public function test_postOnlyToken_accessDeniedIfGet()
public function test_secureOnlyToken_accessDeniedIfGet()
{
$url = Fixture::getTestRootUrl().'?'.http_build_query([
'module' => 'API',
'method' => 'API.getMatomoVersion',
'token_auth' => self::$tokenPostOnly,
'token_auth' => self::$tokenSecureOnly,
]);

$ch = curl_init();
Expand All @@ -64,17 +64,17 @@ public function test_postOnlyToken_accessDeniedIfGet()
}

/**
* Post only tokens should return a 200 code if used in a POST request
* Secure only tokens should return a 200 code if used in a POST request
*/
public function test_postOnlyToken_accessGrantedIfPost()
public function test_secureOnlyToken_accessGrantedIfPost()
{
$url = Fixture::getTestRootUrl().'?'.http_build_query([
'module' => 'API',
'method' => 'API.getMatomoVersion'
]);

$ch = curl_init();
curl_setopt($ch, CURLOPT_POSTFIELDS, ['token_auth' => self::$tokenPostOnly]);
curl_setopt($ch, CURLOPT_POSTFIELDS, ['token_auth' => self::$tokenSecureOnly]);
curl_setopt($ch, CURLOPT_URL, $url);
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
curl_exec($ch);
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Loading