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

App specific token_auths #15410

Merged
merged 70 commits into from Mar 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
eb45511
some initial work
tsteur Jan 9, 2020
a0814b0
add security page
tsteur Jan 9, 2020
4bfd018
backing up some code
tsteur Jan 10, 2020
08cc438
more functionality
tsteur Jan 10, 2020
d7fa33c
adjust more UI parts
tsteur Jan 10, 2020
c212987
adjust more code
tsteur Jan 10, 2020
ca8bdab
more tweaks
tsteur Jan 10, 2020
ea2339f
add todo note
tsteur Jan 10, 2020
1800e2e
few tweaks
tsteur Jan 10, 2020
d709b22
make sure date is in right format
tsteur Jan 10, 2020
1f9ce60
fix not existing column
tsteur Jan 10, 2020
b176c12
few fixes
tsteur Jan 10, 2020
a08aa7d
available hashes
tsteur Jan 10, 2020
220cc33
use different hash algo so tests run on php 5
tsteur Jan 12, 2020
7e7978f
fix name of aglorithm
tsteur Jan 12, 2020
e5182e0
trying to fix some tests
tsteur Jan 13, 2020
7ff807f
another try to fix some tests
tsteur Jan 13, 2020
a60b09d
more fixes
tsteur Jan 13, 2020
1816b51
more fixes
tsteur Jan 13, 2020
360a013
few fixes
tsteur Jan 13, 2020
63dc654
update template
tsteur Jan 13, 2020
370f8f1
Merge branch '4.x-dev' into 6559
tsteur Jan 13, 2020
c2d1078
fix some tests
tsteur Jan 13, 2020
d4c6597
fix test
tsteur Jan 14, 2020
dfe6105
fixing some tests
tsteur Jan 14, 2020
f7671d8
various test fixes
tsteur Jan 14, 2020
99e87dc
more fixes
tsteur Jan 14, 2020
0db3f81
few more tests
tsteur Jan 14, 2020
4afd872
more tests
tsteur Jan 14, 2020
4ab8368
various tweaks
tsteur Jan 15, 2020
924d383
add translations
tsteur Jan 15, 2020
e9032e1
add some ui tests
tsteur Jan 15, 2020
6dfe810
fix selector
tsteur Jan 15, 2020
871532b
tweaks
tsteur Jan 16, 2020
67eb22a
Merge branch '4.x-dev' into 6559
tsteur Jan 16, 2020
4369b5b
trying to fix some ui tests
tsteur Jan 16, 2020
690a4f2
fallback to regular authentication if needed
tsteur Jan 16, 2020
2ec880f
fix call authenticate on null
tsteur Jan 17, 2020
bac6357
fix user settings
tsteur Jan 17, 2020
e5bfa31
Merge branch '4.x-dev' into 6559
tsteur Jan 17, 2020
ab0ebe0
fix some tests
tsteur Jan 17, 2020
f42e9fb
few fixes
tsteur Jan 17, 2020
7df5a0f
fix more ui tests
tsteur Jan 17, 2020
6be4fe2
update schema
tsteur Jan 20, 2020
083d9e6
Update plugins/CoreHome/angularjs/widget-loader/widgetloader.directiv…
tsteur Jan 20, 2020
84dca33
Merge branch '4.x-dev' into 6559
tsteur Jan 20, 2020
888384d
fix maps are not showing data
tsteur Jan 23, 2020
78d8a28
Merge branch '6559' of github.com:matomo-org/matomo into 6559
tsteur Jan 23, 2020
fae64eb
trying to fix some tests
tsteur Jan 24, 2020
faefb00
set correct token
tsteur Jan 24, 2020
0e0cdcf
trying to fix tracking failure
tsteur Jan 26, 2020
c0bb59f
minor tweaks and fixes
tsteur Jan 26, 2020
0534cb0
Merge branch '4.x-dev' into 6559
tsteur Jan 26, 2020
60180bc
fix more tests
tsteur Jan 26, 2020
ba4ad0d
fix screenshot test
tsteur Jan 26, 2020
1500628
Merge branch '4.x-dev' into 6559
tsteur Jan 26, 2020
6ef19d7
trigger event so brute force logic is executed
tsteur Feb 9, 2020
dfde586
test no fallback to actual authentication
tsteur Feb 9, 2020
47095a9
allow fallback
tsteur Feb 10, 2020
0e6a68a
Merge branch '4.x-dev' into 6559
tsteur Mar 3, 2020
c437d74
apply review feedback
tsteur Mar 3, 2020
be0160c
fix some tests
tsteur Mar 3, 2020
d44fd51
fix tests
tsteur Mar 3, 2020
ffb8651
Merge branch '4.x-dev' into 6559
diosmosis Mar 9, 2020
5a85558
Merge branch '4.x-dev' into 6559
diosmosis Mar 15, 2020
065c0ed
make sure location values from query params are limited properly befo…
diosmosis Mar 16, 2020
d10fc52
make sure plugin uninstall migration reloads plugins, make sure 4.0.0…
diosmosis Mar 16, 2020
1b42ee5
Merge branch '4.x-dev' into 6559
diosmosis Mar 17, 2020
19e24b0
Fix UI tests.
diosmosis Mar 17, 2020
e9438d7
update expected screenshot
diosmosis Mar 18, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 0 additions & 9 deletions .gitmodules
Expand Up @@ -57,15 +57,6 @@
[submodule "plugins/DeviceDetectorCache"]
path = plugins/DeviceDetectorCache
url = https://github.com/matomo-org/plugin-DeviceDetectorCache.git
[submodule "plugins/Provider"]
path = plugins/Provider
url = https://github.com/matomo-org/plugin-Provider.git

# Add new Plugin submodule above this line ^^
#
# Note: when you add a submodule that SHOULD be left in the packaged release such as the few submodules below,
# then you MUST add these submodules names in the SUBMODULES_PACKAGED_WITH_CORE variable in:
# https://github.com/matomo-org/matomo-package/blob/master/scripts/build-package.sh
[submodule "misc/log-analytics"]
path = misc/log-analytics
url = https://github.com/matomo-org/matomo-log-analytics.git
Expand Down
6 changes: 5 additions & 1 deletion CHANGELOG.md
Expand Up @@ -6,8 +6,12 @@ The Product Changelog at **[matomo.org/changelog](https://matomo.org/changelog)*

## Matomo 4.0.0

### Breaking Changes
### New API
* A new API `UsersManager.createAppSpecificTokenAuth` has been added to create an app specific token for a user.

### Breaking changes
* The API `UsersManager.getTokenAuth` has been removed. Instead you need to use `UsersManager.createAppSpecificTokenAuth` and store this token in your application.
* The API `UsersManager.createTokenAuth` has been removed. Instead you need to use `UsersManager.createAppSpecificTokenAuth` and store this token in your application.
* Deprecated `piwik` font was removed. Use `matomo` font instead
* The JavaScript AjaxHelper does not longer support synchronous requests. All requests will be sent async instead.
* The deprecated Platform API method `\Piwik\Plugin::getListHooksRegistered()` has been removed. Use `\Piwik\Plugin::registerEvents()` instead
Expand Down
27 changes: 26 additions & 1 deletion core/Access.php
Expand Up @@ -15,6 +15,7 @@
use Piwik\Container\StaticContainer;
use Piwik\Exception\InvalidRequestParameterException;
use Piwik\Plugins\SitesManager\API as SitesManagerApi;
use Piwik\Session\SessionAuth;

/**
* Singleton that manages user access to Piwik resources.
Expand Down Expand Up @@ -155,8 +156,32 @@ public function reloadAccess(Auth $auth = null)
return false;
}

$result = null;

$forceApiSession = Common::getRequestVar('force_api_session', 0, 'int', $_POST);
if ($forceApiSession && Piwik::getModule() === 'API' && (Piwik::getAction() === 'index' || !Piwik::getAction())) {
$tokenAuth = Common::getRequestVar('token_auth', '', 'string', $_POST);
if (!empty($tokenAuth)) {
Session::start();
$auth = StaticContainer::get(SessionAuth::class);
$auth->setTokenAuth($tokenAuth);
$result = $auth->authenticate();
if (!$result->wasAuthenticationSuccessful()) {
/**
* Ensures brute force logic to be executed
* @ignore
* @internal
*/
Piwik::postEvent('API.Request.authenticate.failed');
}
// if not successful, we will fallback to regular auth
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Is this for using the session during API requests? I remember when changing to sessions we didn't want to do this for some reason, do you remember why? Wondering if it's still relevant. It might've been for performance.

Copy link
Member Author

Choose a reason for hiding this comment

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

@diosmosis this is only for requests initiated by the UI. A standard API request will still not use the session. Was like the only workaround I could think of that doesn't break too much. It's actually not even so much of a tokenAuth here but more of an nonce.

Note to myself: Add BruteForce logic here.


// access = array ( idsite => accessIdSite, idsite2 => accessIdSite2)
$result = $this->auth->authenticate();
if (!$result || !$result->wasAuthenticationSuccessful()) {
$result = $this->auth->authenticate();
}

if (!$result->wasAuthenticationSuccessful()) {
return false;
Expand Down
16 changes: 3 additions & 13 deletions core/CliMulti.php
Expand Up @@ -365,8 +365,7 @@ private function executeNotAsyncHttp($url, Output $output)
}

if ($this->runAsSuperUser) {
$tokenAuths = self::getSuperUserTokenAuths();
$tokenAuth = reset($tokenAuths);
$tokenAuth = self::getSuperUserTokenAuth();

if (strpos($url, '?') === false) {
$url .= '?';
Expand Down Expand Up @@ -433,18 +432,9 @@ private function requestUrls(array $piwikUrls)
return $results;
}

private static function getSuperUserTokenAuths()
private static function getSuperUserTokenAuth()
{
$tokens = array();

/**
* Used to be in CronArchive, moved to CliMulti.
*
* @ignore
*/
Piwik::postEvent('CronArchive.getTokenAuth', array(&$tokens));

return $tokens;
return Piwik::requestTemporarySystemAuthToken('CliMultiNonAsyncArchive', 36);
}

public function setUrlToPiwik($urlToPiwik)
Expand Down
26 changes: 21 additions & 5 deletions core/Db/Schema/Mysql.php
Expand Up @@ -16,6 +16,7 @@
use Piwik\Db;
use Piwik\DbHelper;
use Piwik\Option;
use Piwik\Plugins\UsersManager\Model;
use Piwik\Version;

/**
Expand Down Expand Up @@ -45,12 +46,24 @@ public function getTablesCreateSql()
alias VARCHAR(45) NOT NULL,
email VARCHAR(100) NOT NULL,
twofactor_secret VARCHAR(40) NOT NULL DEFAULT '',
token_auth CHAR(32) NOT NULL,
superuser_access TINYINT(2) unsigned NOT NULL DEFAULT '0',
date_registered TIMESTAMP NULL,
ts_password_modified TIMESTAMP NULL,
PRIMARY KEY(login),
UNIQUE KEY uniq_keytoken(token_auth)
PRIMARY KEY(login)
) ENGINE=$engine DEFAULT CHARSET=utf8
",
'user_token_auth' => "CREATE TABLE {$prefixTables}user_token_auth (
idusertokenauth BIGINT UNSIGNED NOT NULL AUTO_INCREMENT,
login VARCHAR(100) NOT NULL,
description VARCHAR(".Model::MAX_LENGTH_TOKEN_DESCRIPTION.") NOT NULL,
password VARCHAR(255) NOT NULL,
hash_algo VARCHAR(30) NOT NULL,
system_token TINYINT(1) NOT NULL DEFAULT 0,
last_used DATETIME NULL,
date_created DATETIME NOT NULL,
date_expired DATETIME NULL,
PRIMARY KEY(idusertokenauth),
UNIQUE KEY uniq_password(password)
) ENGINE=$engine DEFAULT CHARSET=utf8
",

Expand Down Expand Up @@ -504,12 +517,15 @@ public function createTables()
public function createAnonymousUser()
{
$now = Date::factory('now')->getDatetime();

// The anonymous user is the user that is assigned by default
// note that the token_auth value is anonymous, which is assigned by default as well in the Login plugin
$db = $this->getDb();
$db->query("INSERT IGNORE INTO " . Common::prefixTable("user") . "
VALUES ( 'anonymous', '', 'anonymous', 'anonymous@example.org', '', 'anonymous', 0, '$now', '$now' );");
(`login`, `password`, `alias`, `email`, `twofactor_secret`, `superuser_access`, `date_registered`, `ts_password_modified`)
VALUES ( 'anonymous', '', 'anonymous', 'anonymous@example.org', '', 0, '$now', '$now' );");

$model = new Model();
$model->addTokenAuth('anonymous', 'anonymous', 'anonymous default token', $now);
}

/**
Expand Down
46 changes: 46 additions & 0 deletions core/Piwik.php
Expand Up @@ -16,6 +16,7 @@
use Piwik\Period\Week;
use Piwik\Period\Year;
use Piwik\Plugins\UsersManager\API as APIUsersManager;
use Piwik\Plugins\UsersManager\Model;
use Piwik\Translation\Translator;

/**
Expand Down Expand Up @@ -257,6 +258,51 @@ public static function checkUserHasSuperUserAccessOrIsTheUser($theUser)
}
}

/**
* Request a token auth to authenticate in a request.
*
* Note: During one request the token is only being requested once and used throughout the request. So you want to make
* sure the token is valid for enough time for the whole request to finish.
*
* @param string $reason some short string/text explaining the reason for the token generation, eg "CliMultiAsyncHttpArchiving"
* @param int $validForHours For how many hours the token should be valid. Should not be valid for more than 14 days.
* @return mixed
*/
public static function requestTemporarySystemAuthToken($reason, $validForHours)
{
static $token = array();
Copy link
Member

Choose a reason for hiding this comment

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

Would this ever need to be reset for tests? If so, might be good to keep it as a class static variable.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it's needed for tests. If so, we can refactor it for sure 👍


if (isset($token[$reason])) {
// note: For now we do not increase the expire time when it is already requested
return $token[$reason];
}

$twoWeeksInHours = 14 * 24;
if ($validForHours > $twoWeeksInHours) {
throw new Exception('The token cannot be valid for so many hours: ' . $validForHours);
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we just set $validForHours to $twoWeeksInHours in this case? Might avoid some fatal errors in the future. (I can't imagine a case where someone would do this on purpose in a plugin, so I assume it would always be a strange bug, if it occurs at all.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure @diosmosis personally, would prefer to throw an exception. It be only mostly under development anyway if someone calls it wrongly. Otherwise they might pause 4 weeks and don't notice it won't work because it's changed silently which might not be expected.

I could throw the exception though if needed only if Dev Mode is enabled if that helps? And fallback to 2 weeks if dev mode is disabled but then it might be hard to debug some issues.

Copy link
Member

Choose a reason for hiding this comment

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

We can always remove the exception later if it becomes an issue, so probably not an issue.


$model = new Model();
$users = $model->getUsersHavingSuperUserAccess();
if (!empty($users)) {
$user = reset($users);
$expireDate = Date::now()->addHour($validForHours)->getDatetime();

$token[$reason] = $model->generateRandomTokenAuth();

$model->addTokenAuth(
$user['login'],
$token[$reason],
'System generated ' . $reason,
Date::now()->getDatetime(),
$expireDate,
true);

return $token[$reason];
}

}

/**
* Check whether the given user has superuser access.
*
Expand Down
25 changes: 20 additions & 5 deletions core/Session/SessionAuth.php
Expand Up @@ -11,8 +11,10 @@

use Piwik\Auth;
use Piwik\AuthResult;
use Piwik\Common;
use Piwik\Config;
use Piwik\Date;
use Piwik\Plugins\UsersManager\Model;
use Piwik\Plugins\UsersManager\Model as UsersModel;
use Piwik\Session;

Expand Down Expand Up @@ -42,6 +44,8 @@ class SessionAuth implements Auth
*/
private $user;

private $tokenAuth;

public function __construct(UsersModel $userModel = null, $shouldDestroySession = true)
{
$this->userModel = $userModel ?: new UsersModel();
Expand All @@ -55,7 +59,7 @@ public function getName()

public function setTokenAuth($token_auth)
{
// empty
$this->tokenAuth = $token_auth;
}

public function getLogin()
Expand Down Expand Up @@ -113,7 +117,17 @@ public function authenticate()

$this->updateSessionExpireTime($sessionFingerprint);

return $this->makeAuthSuccess($user);
if (!empty($this->tokenAuth) && $this->tokenAuth !== $sessionFingerprint->getSessionTokenAuth()) {
return $this->makeAuthFailure();
}
tsteur marked this conversation as resolved.
Show resolved Hide resolved

if ($sessionFingerprint->getSessionTokenAuth()) {
$tokenAuth = $sessionFingerprint->getSessionTokenAuth();
} else {
$tokenAuth = $this->userModel->generateRandomTokenAuth();
}

return $this->makeAuthSuccess($user, $tokenAuth);
}

private function isSessionStartedBeforePasswordChange(SessionFingerprint $sessionFingerprint, $tsPasswordModified)
Expand All @@ -137,14 +151,15 @@ private function makeAuthFailure()
return new AuthResult(AuthResult::FAILURE, null, null);
}

private function makeAuthSuccess($user)
private function makeAuthSuccess($user, $tokenAuth)
{
$this->user = $user;
$this->tokenAuth = $tokenAuth;

$isSuperUser = (int) $user['superuser_access'];
$code = $isSuperUser ? AuthResult::SUCCESS_SUPERUSER_AUTH_CODE : AuthResult::SUCCESS;

return new AuthResult($code, $user['login'], $user['token_auth']);
return new AuthResult($code, $user['login'], $tokenAuth);
}

protected function initNewBlankSession(SessionFingerprint $sessionFingerprint)
Expand Down Expand Up @@ -178,7 +193,7 @@ protected function destroyCurrentSession(SessionFingerprint $sessionFingerprint)

public function getTokenAuth()
{
return $this->user['token_auth'];
return $this->tokenAuth;
}

private function updateSessionExpireTime(SessionFingerprint $sessionFingerprint)
Expand Down
18 changes: 17 additions & 1 deletion core/Session/SessionFingerprint.php
Expand Up @@ -9,6 +9,7 @@

namespace Piwik\Session;

use Piwik\Common;
use Piwik\Config;
use Piwik\Date;

Expand Down Expand Up @@ -42,6 +43,7 @@ class SessionFingerprint
const USER_NAME_SESSION_VAR_NAME = 'user.name';
const SESSION_INFO_SESSION_VAR_NAME = 'session.info';
const SESSION_INFO_TWO_FACTOR_AUTH_VERIFIED = 'twofactorauth.verified';
const SESSION_INFO_TEMP_TOKEN_AUTH = 'user.token_auth_temp';

public function getUser()
{
Expand All @@ -61,6 +63,15 @@ public function getUserInfo()
return null;
}

public function getSessionTokenAuth()
{
if (!empty($_SESSION[self::SESSION_INFO_TEMP_TOKEN_AUTH])) {
return $_SESSION[self::SESSION_INFO_TEMP_TOKEN_AUTH];
}

return null;
}

public function hasVerifiedTwoFactor()
{
if (isset($_SESSION[self::SESSION_INFO_TWO_FACTOR_AUTH_VERIFIED])) {
Expand All @@ -75,11 +86,12 @@ public function setTwoFactorAuthenticationVerified()
$_SESSION[self::SESSION_INFO_TWO_FACTOR_AUTH_VERIFIED] = 1;
}

public function initialize($userName, $isRemembered = false, $time = null)
public function initialize($userName, $tokenAuth, $isRemembered = false, $time = null)
{
$time = $time ?: Date::now()->getTimestampUTC();
$_SESSION[self::USER_NAME_SESSION_VAR_NAME] = $userName;
$_SESSION[self::SESSION_INFO_TWO_FACTOR_AUTH_VERIFIED] = 0;
$_SESSION[self::SESSION_INFO_TEMP_TOKEN_AUTH] = $tokenAuth;
$_SESSION[self::SESSION_INFO_SESSION_VAR_NAME] = [
'ts' => $time,
'remembered' => $isRemembered,
Expand All @@ -100,6 +112,10 @@ public function clear()
if (isset($_SESSION[self::SESSION_INFO_TWO_FACTOR_AUTH_VERIFIED])) { // may not be available during tests
unset($_SESSION[self::SESSION_INFO_TWO_FACTOR_AUTH_VERIFIED]);
}

if (isset($_SESSION[self::SESSION_INFO_TEMP_TOKEN_AUTH])) { // may not be available during tests
unset($_SESSION[self::SESSION_INFO_TEMP_TOKEN_AUTH]);
}
}

public function getSessionStartTime()
Expand Down
2 changes: 1 addition & 1 deletion core/Session/SessionInitializer.php
Expand Up @@ -83,7 +83,7 @@ protected function processFailedSession()
protected function processSuccessfulSession(AuthResult $authResult)
{
$sessionIdentifier = new SessionFingerprint();
$sessionIdentifier->initialize($authResult->getIdentity(), $this->isRemembered());
$sessionIdentifier->initialize($authResult->getIdentity(), $authResult->getTokenAuth(), $this->isRemembered());

/**
* @ignore
Expand Down
2 changes: 2 additions & 0 deletions core/Tracker/Request.php
Expand Up @@ -208,6 +208,8 @@ public static function authenticateSuperUserOrAdminOrWrite($tokenAuth, $idSite)
// Now checking the list of admin token_auth cached in the Tracker config file
if (!empty($idSite) && $idSite > 0) {
$website = Cache::getCacheWebsiteAttributes($idSite);
$userModel = new \Piwik\Plugins\UsersManager\Model();
$tokenAuth = $userModel->hashTokenAuth($tokenAuth);
$hashedToken = UsersManager::hashTrackingToken((string) $tokenAuth, $idSite);

if (array_key_exists('tracking_token_auth', $website)
Expand Down
4 changes: 4 additions & 0 deletions core/Updater/Migration/Plugin/Uninstall.php
Expand Up @@ -48,6 +48,10 @@ public function shouldIgnoreError($exception)
public function exec()
{
$this->pluginManager->uninstallPlugin($this->pluginName);

// uninstallPlugin() loads all plugins in the filesystem, which we don't want for the rest of the updates
$this->pluginManager->unloadPlugins();
$this->pluginManager->loadActivatedPlugins();
}

}
4 changes: 2 additions & 2 deletions core/Updates/2.0.4-b5.php
Expand Up @@ -36,7 +36,7 @@ public function __construct(MigrationFactory $factory)
public function getMigrations(Updater $updater)
{
return array(
$this->migration->db->addColumn('user', 'superuser_access', "TINYINT(2) UNSIGNED NOT NULL DEFAULT '0'", 'token_auth')
$this->migration->db->addColumn('user', 'superuser_access', "TINYINT(2) UNSIGNED NOT NULL DEFAULT '0'")
);
}

Expand Down Expand Up @@ -82,7 +82,7 @@ private static function migrateConfigSuperUserToDb()
'password' => $superUser['password'],
'alias' => $superUser['login'],
'email' => $superUser['email'],
'token_auth' => $userApi->getTokenAuth($superUser['login'], $superUser['password']),
'token_auth' => md5(Common::getRandomString(32)),
'date_registered' => Date::now()->getDatetime(),
'superuser_access' => 1
)
Expand Down