From 0d960599450314473f5907f9f0b6e4d1bb226dc0 Mon Sep 17 00:00:00 2001
From: Frederic Guillot <fred@miniflux.net>
Date: Wed, 17 Aug 2016 22:05:11 -0400
Subject: [PATCH] Move csrf functions to Helper namespace

---
 composer.json                       |  1 +
 controllers/config.php              | 26 ++++++++++-----------
 controllers/feed.php                |  6 ++---
 controllers/user.php                |  6 ++---
 helpers/csrf.php                    | 35 +++++++++++++++++++++++++++++
 models/config.php                   | 35 -----------------------------
 tests/unit/BaseTest.php             |  4 +++-
 tests/unit/HelperTest.php           | 29 +++++++++++++++++++++++-
 tests/unit/RequestTest.php          |  2 +-
 vendor/composer/autoload_files.php  |  1 +
 vendor/composer/autoload_static.php |  1 +
 11 files changed, 89 insertions(+), 57 deletions(-)
 create mode 100644 helpers/csrf.php

diff --git a/composer.json b/composer.json
index df986cb..2a10b92 100644
--- a/composer.json
+++ b/composer.json
@@ -26,6 +26,7 @@
     "autoload": {
         "files": [
             "lib/helpers.php",
+            "helpers/csrf.php",
             "lib/Translator.php",
             "lib/Request.php",
             "lib/Response.php",
diff --git a/controllers/config.php b/controllers/config.php
index b48cd57..8a9f770 100644
--- a/controllers/config.php
+++ b/controllers/config.php
@@ -8,7 +8,7 @@ Router\get_action('new-db', function () {
         Response\html(Template\layout('new_db', array(
             'errors' => array(),
             'values' => array(
-                'csrf' => Model\Config\generate_csrf(),
+                'csrf' => Helper\generate_csrf(),
             ),
             'nb_unread_items' => Model\Item\count_by_status('unread'),
             'menu' => 'config',
@@ -23,7 +23,7 @@ Router\get_action('new-db', function () {
 Router\post_action('new-db', function () {
     if (ENABLE_MULTIPLE_DB) {
         $values = Request\values();
-        Model\Config\check_csrf_values($values);
+        Helper\check_csrf_values($values);
         list($valid, $errors) = Model\Database\validate($values);
 
         if ($valid) {
@@ -38,7 +38,7 @@ Router\post_action('new-db', function () {
 
         Response\html(Template\layout('new_db', array(
             'errors' => $errors,
-            'values' => $values + array('csrf' => Model\Config\generate_csrf()),
+            'values' => $values + array('csrf' => Helper\generate_csrf()),
             'nb_unread_items' => Model\Item\count_by_status('unread'),
             'menu' => 'config',
             'title' => t('New database')
@@ -72,7 +72,7 @@ Router\get_action('auto-update', function () {
 
 // Re-generate tokens
 Router\get_action('generate-tokens', function () {
-    if (Model\Config\check_csrf(Request\param('csrf'))) {
+    if (Helper\check_csrf(Request\param('csrf'))) {
         Model\Config\new_tokens();
     }
 
@@ -81,7 +81,7 @@ Router\get_action('generate-tokens', function () {
 
 // Optimize the database manually
 Router\get_action('optimize-db', function () {
-    if (Model\Config\check_csrf(Request\param('csrf'))) {
+    if (Helper\check_csrf(Request\param('csrf'))) {
         Database::getInstance('db')->getConnection()->exec('VACUUM');
     }
 
@@ -90,7 +90,7 @@ Router\get_action('optimize-db', function () {
 
 // Download the compressed database
 Router\get_action('download-db', function () {
-    if (Model\Config\check_csrf(Request\param('csrf'))) {
+    if (Helper\check_csrf(Request\param('csrf'))) {
         Response\force_download('db.sqlite.gz');
         Response\binary(gzencode(file_get_contents(Model\Database\get_path())));
     }
@@ -100,7 +100,7 @@ Router\get_action('download-db', function () {
 Router\get_action('config', function () {
     Response\html(Template\layout('config', array(
         'errors' => array(),
-        'values' => Model\Config\get_all() + array('csrf' => Model\Config\generate_csrf()),
+        'values' => Model\Config\get_all() + array('csrf' => Helper\generate_csrf()),
         'languages' => Model\Config\get_languages(),
         'timezones' => Model\Config\get_timezones(),
         'autoflush_read_options' => Model\Config\get_autoflush_read_options(),
@@ -120,7 +120,7 @@ Router\get_action('config', function () {
 // Update preferences
 Router\post_action('config', function () {
     $values = Request\values() + array('nocontent' => 0, 'image_proxy' => 0, 'favicons' => 0, 'debug_mode' => 0, 'original_marks_read' => 0);
-    Model\Config\check_csrf_values($values);
+    Helper\check_csrf_values($values);
     list($valid, $errors) = Model\Config\validate_modification($values);
 
     if ($valid) {
@@ -135,7 +135,7 @@ Router\post_action('config', function () {
 
     Response\html(Template\layout('config', array(
         'errors' => $errors,
-        'values' => Model\Config\get_all() + array('csrf' => Model\Config\generate_csrf()),
+        'values' => Model\Config\get_all() + array('csrf' => Helper\generate_csrf()),
         'languages' => Model\Config\get_languages(),
         'timezones' => Model\Config\get_timezones(),
         'autoflush_read_options' => Model\Config\get_autoflush_read_options(),
@@ -181,7 +181,7 @@ Router\get_action('help', function () {
 // Display about page
 Router\get_action('about', function () {
     Response\html(Template\layout('about', array(
-        'csrf' => Model\Config\generate_csrf(),
+        'csrf' => Helper\generate_csrf(),
         'config' => Model\Config\get_all(),
         'db_name' => Model\Database\select(),
         'nb_unread_items' => Model\Item\count_by_status('unread'),
@@ -193,7 +193,7 @@ Router\get_action('about', function () {
 // Display database page
 Router\get_action('database', function () {
     Response\html(Template\layout('database', array(
-        'csrf' => Model\Config\generate_csrf(),
+        'csrf' => Helper\generate_csrf(),
         'config' => Model\Config\get_all(),
         'db_size' => filesize(\Model\Database\get_path()),
         'nb_unread_items' => Model\Item\count_by_status('unread'),
@@ -216,7 +216,7 @@ Router\get_action('api', function () {
 Router\get_action('services', function () {
     Response\html(Template\layout('services', array(
         'errors' => array(),
-        'values' => Model\Config\get_all() + array('csrf' => Model\Config\generate_csrf()),
+        'values' => Model\Config\get_all() + array('csrf' => Helper\generate_csrf()),
         'menu' => 'config',
         'title' => t('Preferences')
     )));
@@ -225,7 +225,7 @@ Router\get_action('services', function () {
 // Update bookmark services
 Router\post_action('services', function () {
     $values = Request\values() + array('pinboard_enabled' => 0, 'instapaper_enabled' => 0);
-    Model\Config\check_csrf_values($values);
+    Helper\check_csrf_values($values);
 
     if (Model\Config\save($values)) {
         Session\flash(t('Your preferences are updated.'));
diff --git a/controllers/feed.php b/controllers/feed.php
index 640844b..86494fc 100644
--- a/controllers/feed.php
+++ b/controllers/feed.php
@@ -138,7 +138,7 @@ Router\get_action('add', function () {
     );
 
     Response\html(Template\layout('add', array(
-        'values' => $values + array('csrf' => Model\Config\generate_csrf()),
+        'values' => $values + array('csrf' => Helper\generate_csrf()),
         'errors' => array(),
         'nb_unread_items' => Model\Item\count_by_status('unread'),
         'groups' => Model\Group\get_all(),
@@ -151,7 +151,7 @@ Router\get_action('add', function () {
 Router\action('subscribe', function () {
     if (Request\is_post()) {
         $values = Request\values();
-        Model\Config\check_csrf_values($values);
+        Helper\check_csrf_values($values);
         $url = isset($values['url']) ? $values['url'] : '';
     } else {
         $values = array();
@@ -215,7 +215,7 @@ Router\action('subscribe', function () {
     }
 
     Response\html(Template\layout('add', array(
-        'values' => $values + array('csrf' => Model\Config\generate_csrf()),
+        'values' => $values + array('csrf' => Helper\generate_csrf()),
         'nb_unread_items' => Model\Item\count_by_status('unread'),
         'groups' => Model\Group\get_all(),
         'menu' => 'feeds',
diff --git a/controllers/user.php b/controllers/user.php
index 0c087ed..1530334 100644
--- a/controllers/user.php
+++ b/controllers/user.php
@@ -15,7 +15,7 @@ Router\get_action('login', function () {
     Response\html(Template\load('login', array(
         'errors' => array(),
         'values' => array(
-            'csrf' => Model\Config\generate_csrf(),
+            'csrf' => Helper\generate_csrf(),
         ),
         'databases' => Model\Database\get_list(),
         'current_database' => Model\Database\select()
@@ -25,7 +25,7 @@ Router\get_action('login', function () {
 // Check credentials and redirect to unread items
 Router\post_action('login', function () {
     $values = Request\values();
-    Model\Config\check_csrf_values($values);
+    Helper\check_csrf_values($values);
     list($valid, $errors) = Model\User\validate_login($values);
 
     if ($valid) {
@@ -34,7 +34,7 @@ Router\post_action('login', function () {
 
     Response\html(Template\load('login', array(
         'errors' => $errors,
-        'values' => $values + array('csrf' => Model\Config\generate_csrf()),
+        'values' => $values + array('csrf' => Helper\generate_csrf()),
         'databases' => Model\Database\get_list(),
         'current_database' => Model\Database\select()
     )));
diff --git a/helpers/csrf.php b/helpers/csrf.php
new file mode 100644
index 0000000..c2ac113
--- /dev/null
+++ b/helpers/csrf.php
@@ -0,0 +1,35 @@
+<?php
+
+namespace Helper;
+
+function generate_csrf()
+{
+    if (! isset($_SESSION['csrf'])) {
+        $_SESSION['csrf'] = array();
+    }
+
+    $token = generate_token();
+    $_SESSION['csrf'][$token] = true;
+
+    return $token;
+}
+
+function check_csrf_values(array &$values)
+{
+    if (empty($values['csrf']) || ! isset($_SESSION['csrf'][$values['csrf']])) {
+        $values = array();
+    } else {
+        unset($_SESSION['csrf'][$values['csrf']]);
+        unset($values['csrf']);
+    }
+}
+
+function check_csrf($token)
+{
+    if (isset($_SESSION['csrf'][$token])) {
+        unset($_SESSION['csrf'][$token]);
+        return true;
+    }
+
+    return false;
+}
diff --git a/models/config.php b/models/config.php
index e0fa6de..7530f03 100644
--- a/models/config.php
+++ b/models/config.php
@@ -208,41 +208,6 @@ function get_nothing_to_read_redirections()
     );
 }
 
-// Create a CSRF token
-function generate_csrf()
-{
-    if (empty($_SESSION['csrf'])) {
-        $_SESSION['csrf'] = array();
-    }
-
-    $token = Helper\generate_token();
-    $_SESSION['csrf'][$token] = true;
-
-    return $token;
-}
-
-// Check CSRF token (form values)
-function check_csrf_values(array &$values)
-{
-    if (empty($values['csrf']) || ! isset($_SESSION['csrf'][$values['csrf']])) {
-        $values = array();
-    } else {
-        unset($_SESSION['csrf'][$values['csrf']]);
-        unset($values['csrf']);
-    }
-}
-
-// Check CSRF token
-function check_csrf($token)
-{
-    if (isset($_SESSION['csrf'][$token])) {
-        unset($_SESSION['csrf'][$token]);
-        return true;
-    }
-
-    return false;
-}
-
 
 // Regenerate tokens for the API and bookmark feed
 function new_tokens()
diff --git a/tests/unit/BaseTest.php b/tests/unit/BaseTest.php
index a2ef938..89605c4 100644
--- a/tests/unit/BaseTest.php
+++ b/tests/unit/BaseTest.php
@@ -4,5 +4,7 @@ require_once __DIR__.'/../../vendor/autoload.php';
 
 abstract class BaseTest extends PHPUnit_Framework_TestCase
 {
-
+    public function setUp()
+    {
+    }
 }
diff --git a/tests/unit/HelperTest.php b/tests/unit/HelperTest.php
index bc08d5a..62ee43e 100644
--- a/tests/unit/HelperTest.php
+++ b/tests/unit/HelperTest.php
@@ -1,6 +1,6 @@
 <?php
 
-class HelperTest extends PHPUnit_Framework_TestCase
+class HelperTest extends BaseTest
 {
     public function testGenerateToken()
     {
@@ -8,4 +8,31 @@ class HelperTest extends PHPUnit_Framework_TestCase
         $token2 = Helper\generate_token();
         $this->assertNotEquals($token1, $token2);
     }
+
+    public function testGenerateCsrf()
+    {
+        $_SESSION = array();
+
+        $token1 = Helper\generate_csrf();
+        $token2 = Helper\generate_csrf();
+        $this->assertNotEquals($token1, $token2);
+    }
+
+    public function testCheckCsrf()
+    {
+        $token = Helper\generate_csrf();
+        $this->assertTrue(Helper\check_csrf($token));
+        $this->assertFalse(Helper\check_csrf('test'));
+    }
+
+    public function testCheckCsrfValues()
+    {
+        $values = array('field' => 'value');
+        Helper\check_csrf_values($values);
+        $this->assertEmpty($values);
+
+        $values = array('field' => 'value', 'csrf' => Helper\generate_csrf());
+        Helper\check_csrf_values($values);
+        $this->assertEquals(array('field' => 'value'), $values);
+    }
 }
diff --git a/tests/unit/RequestTest.php b/tests/unit/RequestTest.php
index fdd21ea..970f7fd 100644
--- a/tests/unit/RequestTest.php
+++ b/tests/unit/RequestTest.php
@@ -1,6 +1,6 @@
 <?php
 
-class RequestTest extends PHPUnit_Framework_TestCase
+class RequestTest extends BaseTest
 {
     public function testGetIpAddress()
     {
diff --git a/vendor/composer/autoload_files.php b/vendor/composer/autoload_files.php
index 938cb4d..d769aaa 100644
--- a/vendor/composer/autoload_files.php
+++ b/vendor/composer/autoload_files.php
@@ -7,6 +7,7 @@ $baseDir = dirname($vendorDir);
 
 return array(
     '441b53696b2c1c13da1210b9b5d22213' => $baseDir . '/lib/helpers.php',
+    '4b6f1c38c1cab2809f0444d3a253f8f7' => $baseDir . '/helpers/csrf.php',
     '2ba60f191527015eb45c05a71d95b69f' => $baseDir . '/lib/Translator.php',
     '1d58cdba7ce052ff0ce0219a932c284a' => $baseDir . '/lib/Request.php',
     '8e1ed5229092ce48fdcef0a911fd739d' => $baseDir . '/lib/Response.php',
diff --git a/vendor/composer/autoload_static.php b/vendor/composer/autoload_static.php
index 6b118af..db80028 100644
--- a/vendor/composer/autoload_static.php
+++ b/vendor/composer/autoload_static.php
@@ -8,6 +8,7 @@ class ComposerStaticInitfd7e8d436e1dc450edc3153ac8bc31b4
 {
     public static $files = array (
         '441b53696b2c1c13da1210b9b5d22213' => __DIR__ . '/../..' . '/lib/helpers.php',
+        '4b6f1c38c1cab2809f0444d3a253f8f7' => __DIR__ . '/../..' . '/helpers/csrf.php',
         '2ba60f191527015eb45c05a71d95b69f' => __DIR__ . '/../..' . '/lib/Translator.php',
         '1d58cdba7ce052ff0ce0219a932c284a' => __DIR__ . '/../..' . '/lib/Request.php',
         '8e1ed5229092ce48fdcef0a911fd739d' => __DIR__ . '/../..' . '/lib/Response.php',