Skip to content

Commit

Permalink
Refactor session token management
Browse files Browse the repository at this point in the history
Relates to shaarli#324

Added:
- `SessionManager` class to group session-related features
- unit tests

Changed:
- `getToken()` -> `SessionManager->generateToken()`
- `tokenOk()` -> `SessionManager->checkToken()`
- inject a `$token` parameter to `PageBuilder`'s constructor

Signed-off-by: VirtualTam <[email protected]>
  • Loading branch information
virtualtam committed Oct 22, 2017
1 parent e648f62 commit ebd650c
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 49 deletions.
6 changes: 4 additions & 2 deletions application/PageBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,14 @@ class PageBuilder
*
* @param ConfigManager $conf Configuration Manager instance (reference).
* @param LinkDB $linkDB instance.
* @param string $token Session token
*/
public function __construct(&$conf, $linkDB = null)
public function __construct(&$conf, $linkDB = null, $token = null)
{
$this->tpl = false;
$this->conf = $conf;
$this->linkDB = $linkDB;
$this->token = $token;
}

/**
Expand Down Expand Up @@ -92,7 +94,7 @@ private function initialize()
$this->tpl->assign('showatom', $this->conf->get('feed.show_atom', true));
$this->tpl->assign('feed_type', $this->conf->get('feed.show_atom', true) !== false ? 'atom' : 'rss');
$this->tpl->assign('hide_timestamps', $this->conf->get('privacy.hide_timestamps', false));
$this->tpl->assign('token', getToken($this->conf));
$this->tpl->assign('token', $this->token);

if ($this->linkDB !== null) {
$this->tpl->assign('tags', $this->linkDB->linksCountPerTag());
Expand Down
53 changes: 53 additions & 0 deletions application/SessionManager.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php
namespace Shaarli;

/**
* Manages the server-side session
*/
class SessionManager
{
protected $session = [];

/**
* Constructor
*
* @param array $session The $_SESSION array (reference)
* @param ConfigManager $conf ConfigManager instance (reference)
*/
public function __construct(& $session, & $conf)
{
$this->session = &$session;
$this->conf = &$conf;
}

/**
* Generates a session token
*
* @return string token
*/
public function generateToken()
{
$token = sha1(uniqid('', true) .'_'. mt_rand() . $this->conf->get('credentials.salt'));
$this->session['tokens'][$token] = 1;
return $token;
}

/**
* Checks the validity of a session token, and destroys it afterwards
*
* @param string $token The token to check
*
* @return bool true if the token is valid, else false
*/
public function checkToken($token)
{
if (! isset($this->session['tokens'][$token])) {
// the token is wrong, or has already been used
return false;
}

// destroy the token to prevent future use
unset($this->session['tokens'][$token]);
return true;
}
}
71 changes: 24 additions & 47 deletions index.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
use \Shaarli\Languages;
use \Shaarli\ThemeUtils;
use \Shaarli\Config\ConfigManager;
use \Shaarli\SessionManager;

// Ensure the PHP version is supported
try {
Expand Down Expand Up @@ -121,6 +122,7 @@
}

$conf = new ConfigManager();
$sessionManager = new SessionManager($_SESSION, $conf);

// Sniff browser language and set date format accordingly.
if (isset($_SERVER['HTTP_ACCEPT_LANGUAGE'])) {
Expand Down Expand Up @@ -165,7 +167,7 @@
}

// Display the installation form if no existing config is found
install($conf);
install($conf, $sessionManager);
}

// a token depending of deployment salt, user password, and the current ip
Expand Down Expand Up @@ -381,7 +383,7 @@ function ban_canLogin($conf)
{
if (!ban_canLogin($conf)) die(t('I said: NO. You are banned for the moment. Go away.'));
if (isset($_POST['password'])
&& tokenOk($_POST['token'])
&& $sessionManager->checkToken($_POST['token'])
&& (check_auth($_POST['login'], $_POST['password'], $conf))
) { // Login/password is OK.
ban_loginOk($conf);
Expand Down Expand Up @@ -454,32 +456,6 @@ function ban_canLogin($conf)
// Token should be used in any form which acts on data (create,update,delete,import...).
if (!isset($_SESSION['tokens'])) $_SESSION['tokens']=array(); // Token are attached to the session.

/**
* Returns a token.
*
* @param ConfigManager $conf Configuration Manager instance.
*
* @return string token.
*/
function getToken($conf)
{
$rnd = sha1(uniqid('', true) .'_'. mt_rand() . $conf->get('credentials.salt')); // We generate a random string.
$_SESSION['tokens'][$rnd]=1; // Store it on the server side.
return $rnd;
}

// Tells if a token is OK. Using this function will destroy the token.
// true=token is OK.
function tokenOk($token)
{
if (isset($_SESSION['tokens'][$token]))
{
unset($_SESSION['tokens'][$token]); // Token is used: destroy it.
return true; // Token is OK.
}
return false; // Wrong token, or already used.
}

/**
* Daily RSS feed: 1 RSS entry per day giving all the links on that day.
* Gives the last 7 days (which have links).
Expand Down Expand Up @@ -687,12 +663,13 @@ function showLinkList($PAGE, $LINKSDB, $conf, $pluginManager) {
/**
* Render HTML page (according to URL parameters and user rights)
*
* @param ConfigManager $conf Configuration Manager instance.
* @param PluginManager $pluginManager Plugin Manager instance,
* @param LinkDB $LINKSDB
* @param History $history instance
* @param ConfigManager $conf Configuration Manager instance.
* @param PluginManager $pluginManager Plugin Manager instance,
* @param LinkDB $LINKSDB
* @param History $history instance
* @param SessionManager $sessionManager SessionManager instance
*/
function renderPage($conf, $pluginManager, $LINKSDB, $history)
function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager)
{
$updater = new Updater(
read_updates_file($conf->get('resource.updates')),
Expand All @@ -713,7 +690,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history)
die($e->getMessage());
}

$PAGE = new PageBuilder($conf, $LINKSDB);
$PAGE = new PageBuilder($conf, $LINKSDB, $sessionManager->generateToken());
$PAGE->assign('linkcount', count($LINKSDB));
$PAGE->assign('privateLinkcount', count_private($LINKSDB));
$PAGE->assign('plugin_errors', $pluginManager->getErrors());
Expand Down Expand Up @@ -1109,13 +1086,13 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history)

if (!empty($_POST['setpassword']) && !empty($_POST['oldpassword']))
{
if (!tokenOk($_POST['token'])) die(t('Wrong token.')); // Go away!
if (!$sessionManager->checkToken($_POST['token'])) die(t('Wrong token.')); // Go away!

// Make sure old password is correct.
$oldhash = sha1($_POST['oldpassword'].$conf->get('credentials.login').$conf->get('credentials.salt'));
if ($oldhash!= $conf->get('credentials.hash')) {
echo '<script>alert("'. t('The old password is not correct.') .'");document.location=\'?do=changepasswd\';</script>';
exit;
exit;
}
// Save new password
// Salt renders rainbow-tables attacks useless.
Expand Down Expand Up @@ -1149,7 +1126,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history)
{
if (!empty($_POST['title']) )
{
if (!tokenOk($_POST['token'])) {
if (!$sessionManager->checkToken($_POST['token'])) {
die(t('Wrong token.')); // Go away!
}
$tz = 'UTC';
Expand Down Expand Up @@ -1225,7 +1202,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history)
exit;
}

if (!tokenOk($_POST['token'])) {
if (!$sessionManager->checkToken($_POST['token'])) {
die(t('Wrong token.'));
}

Expand Down Expand Up @@ -1255,7 +1232,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history)
if (isset($_POST['save_edit']))
{
// Go away!
if (! tokenOk($_POST['token'])) {
if (! $sessionManager->checkToken($_POST['token'])) {
die(t('Wrong token.'));
}

Expand Down Expand Up @@ -1355,7 +1332,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history)
// -------- User clicked the "Delete" button when editing a link: Delete link from database.
if ($targetPage == Router::$PAGE_DELETELINK)
{
if (! tokenOk($_GET['token'])) {
if (! $sessionManager->checkToken($_GET['token'])) {
die(t('Wrong token.'));
}

Expand Down Expand Up @@ -1572,7 +1549,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history)
echo '<script>alert("'. $msg .'");document.location=\'?do='.Router::$PAGE_IMPORT .'\';</script>';
exit;
}
if (! tokenOk($_POST['token'])) {
if (! $sessionManager->checkToken($_POST['token'])) {
die('Wrong token.');
}
$status = NetscapeBookmarkUtils::import(
Expand Down Expand Up @@ -1639,7 +1616,7 @@ function($a, $b) { return $a['order'] - $b['order']; }
// Get a fresh token
if ($targetPage == Router::$GET_TOKEN) {
header('Content-Type:text/plain');
echo getToken($conf);
echo $sessionManager->generateToken($conf);
exit;
}

Expand Down Expand Up @@ -1965,10 +1942,10 @@ function lazyThumbnail($conf, $url,$href=false)
* Installation
* This function should NEVER be called if the file data/config.php exists.
*
* @param ConfigManager $conf Configuration Manager instance.
* @param ConfigManager $conf Configuration Manager instance.
* @param SessionManager $sessionManager SessionManager instance
*/
function install($conf)
{
function install($conf, $sessionManager) {
// On free.fr host, make sure the /sessions directory exists, otherwise login will not work.
if (endsWith($_SERVER['HTTP_HOST'],'.free.fr') && !is_dir($_SERVER['DOCUMENT_ROOT'].'/sessions')) mkdir($_SERVER['DOCUMENT_ROOT'].'/sessions',0705);

Expand Down Expand Up @@ -2051,7 +2028,7 @@ function install($conf)
exit;
}

$PAGE = new PageBuilder($conf);
$PAGE = new PageBuilder($conf, null, $sessionManager->generateToken());
list($continents, $cities) = generateTimeZoneData(timezone_identifiers_list(), date_default_timezone_get());
$PAGE->assign('continents', $continents);
$PAGE->assign('cities', $cities);
Expand Down Expand Up @@ -2328,7 +2305,7 @@ function resizeImage($filepath)
if ($response->getStatusCode() == 404 && strpos($_SERVER['REQUEST_URI'], '/api/v1') === false) {
// We use UTF-8 for proper international characters handling.
header('Content-Type: text/html; charset=utf-8');
renderPage($conf, $pluginManager, $linkDb, $history);
renderPage($conf, $pluginManager, $linkDb, $history, $sessionManager);
} else {
$app->respond($response);
}
72 changes: 72 additions & 0 deletions tests/SessionManagerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
<?php
namespace Shaarli;

use \PHPUnit\Framework\TestCase;

/**
* Fake ConfigManager
*/
class FakeConfigManager
{
public static function get($key)
{
return $key;
}
}


/**
* Test coverage for SessionManager
*/
class SessionManagerTest extends TestCase
{
/**
* Generate a session token
*/
public function testGenerateToken()
{
$session = [];
$conf = new FakeConfigManager();
$sessionManager = new SessionManager($session, $conf);

$token = $sessionManager->generateToken();

$this->assertEquals(1, $session['tokens'][$token]);
$this->assertEquals(40, strlen($token));
}

/**
* Generate and check a session token
*/
public function testGenerateAndCheckToken()
{
$session = [];
$conf = new FakeConfigManager();
$sessionManager = new SessionManager($session, $conf);

$token = $sessionManager->generateToken();

// ensure a token has been generated
$this->assertEquals(1, $session['tokens'][$token]);
$this->assertEquals(40, strlen($token));

// check and destroy the token
$this->assertTrue($sessionManager->checkToken($token));
$this->assertFalse(isset($session['tokens'][$token]));

// ensure the token has been destroyed
$this->assertFalse($sessionManager->checkToken($token));
}

/**
* Check an invalid session token
*/
public function testCheckInvalidToken()
{
$session = [];
$conf = new FakeConfigManager();
$sessionManager = new SessionManager($session, $conf);

$this->assertFalse($sessionManager->checkToken('4dccc3a45ad9d03e5542b90c37d8db6d10f2b38b'));
}
}

0 comments on commit ebd650c

Please sign in to comment.