Crypt site credential password

15 files changed, 169 insertions, 9 deletions

diff --git a/.gitignore b/.gitignore
index 709f78cf..8fd762fa 100644
--- a/.gitignore
+++ b/.gitignore
@@ -56,3 +56,4 @@ app/Resources/build/
 # Test-generated files
 admin-export.json
 specialexport.json
+/data/site-credentials-secret-key.txt
diff --git a/app/DoctrineMigrations/Version20170501115751.php b/app/DoctrineMigrations/Version20170501115751.php
index 846a87b5..2597f1ff 100644
--- a/app/DoctrineMigrations/Version20170501115751.php
+++ b/app/DoctrineMigrations/Version20170501115751.php
@@ -39,7 +39,7 @@ class Version20170501115751 extends AbstractMigration implements ContainerAwareI
         $table->addColumn('user_id', 'integer');
         $table->addColumn('host', 'string', ['length' => 255]);
         $table->addColumn('username', 'string', ['length' => 255]);
-        $table->addColumn('password', 'string', ['length' => 255]);
+        $table->addColumn('password', 'text');
         $table->addColumn('createdAt', 'datetime');
         $table->addIndex(['user_id'], 'idx_user');
         $table->setPrimaryKey(['id']);
diff --git a/app/config/wallabag.yml b/app/config/wallabag.yml
index 51b7e4e3..b45934e4 100644
--- a/app/config/wallabag.yml
+++ b/app/config/wallabag.yml
@@ -26,6 +26,7 @@ wallabag_core:
     fetching_error_message: |
         wallabag can't retrieve contents for this article. Please <a href="http://doc.wallabag.org/en/user/errors_during_fetching.html#how-can-i-help-to-fix-that">troubleshoot this issue</a>.
     api_limit_mass_actions: 10
+    encryption_key_path: "%kernel.root_dir%/../data/site-credentials-secret-key.txt"
     default_internal_settings:
         -
             name: share_public
diff --git a/composer.json b/composer.json
index 0a170c16..7428f688 100644
--- a/composer.json
+++ b/composer.json
@@ -73,7 +73,7 @@
         "kphoen/rulerz-bundle": "~0.13",
         "guzzlehttp/guzzle": "^5.3.1",
         "doctrine/doctrine-migrations-bundle": "^1.0",
-        "paragonie/random_compat": "~1.0",
+        "paragonie/random_compat": "~2.0",
         "craue/config-bundle": "~2.0",
         "mnapoli/piwik-twig-extension": "^1.0",
         "ocramius/proxy-manager": "1.*",
@@ -83,7 +83,8 @@
         "javibravo/simpleue": "^1.0",
         "symfony/dom-crawler": "^3.1",
         "friendsofsymfony/jsrouting-bundle": "^1.6",
-        "bdunogier/guzzle-site-authenticator": "^1.0.0@dev"
+        "bdunogier/guzzle-site-authenticator": "^1.0.0@dev",
+        "defuse/php-encryption": "^2.1"
     },
     "require-dev": {
         "doctrine/doctrine-fixtures-bundle": "~2.2",
diff --git a/src/Wallabag/CoreBundle/Command/InstallCommand.php b/src/Wallabag/CoreBundle/Command/InstallCommand.php
index 0f119377..eb725a59 100644
--- a/src/Wallabag/CoreBundle/Command/InstallCommand.php
+++ b/src/Wallabag/CoreBundle/Command/InstallCommand.php
@@ -313,6 +313,8 @@ class InstallCommand extends ContainerAwareCommand
 
         $this
             ->runCommand('doctrine:migrations:migrate', ['--no-interaction' => true]);
+
+        return $this;
     }
 
     /**
diff --git a/src/Wallabag/CoreBundle/Controller/SiteCredentialController.php b/src/Wallabag/CoreBundle/Controller/SiteCredentialController.php
index dc8e723d..0bacafb7 100644
--- a/src/Wallabag/CoreBundle/Controller/SiteCredentialController.php
+++ b/src/Wallabag/CoreBundle/Controller/SiteCredentialController.php
@@ -45,6 +45,8 @@ class SiteCredentialController extends Controller
         $form->handleRequest($request);
 
         if ($form->isSubmitted() && $form->isValid()) {
+            $credential->setPassword($this->get('wallabag_core.helper.crypto_proxy')->crypt($credential->getPassword()));
+
             $em = $this->getDoctrine()->getManager();
             $em->persist($credential);
             $em->flush($credential);
diff --git a/src/Wallabag/CoreBundle/DependencyInjection/Configuration.php b/src/Wallabag/CoreBundle/DependencyInjection/Configuration.php
index 33df92d3..a9791f6b 100644
--- a/src/Wallabag/CoreBundle/DependencyInjection/Configuration.php
+++ b/src/Wallabag/CoreBundle/DependencyInjection/Configuration.php
@@ -63,6 +63,8 @@ class Configuration implements ConfigurationInterface
                         ->end()
                     ->end()
                 ->end()
+                ->scalarNode('encryption_key_path')
+                ->end()
             ->end()
         ;
 
diff --git a/src/Wallabag/CoreBundle/DependencyInjection/WallabagCoreExtension.php b/src/Wallabag/CoreBundle/DependencyInjection/WallabagCoreExtension.php
index b4d8a386..532ce238 100644
--- a/src/Wallabag/CoreBundle/DependencyInjection/WallabagCoreExtension.php
+++ b/src/Wallabag/CoreBundle/DependencyInjection/WallabagCoreExtension.php
@@ -29,6 +29,7 @@ class WallabagCoreExtension extends Extension
         $container->setParameter('wallabag_core.fetching_error_message_title', $config['fetching_error_message_title']);
         $container->setParameter('wallabag_core.api_limit_mass_actions', $config['api_limit_mass_actions']);
         $container->setParameter('wallabag_core.default_internal_settings', $config['default_internal_settings']);
+        $container->setParameter('wallabag_core.site_credentials.encryption_key_path', $config['encryption_key_path']);
 
         $loader = new Loader\YamlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));
         $loader->load('services.yml');
diff --git a/src/Wallabag/CoreBundle/Entity/SiteCredential.php b/src/Wallabag/CoreBundle/Entity/SiteCredential.php
index 85ee07d4..732d9506 100644
--- a/src/Wallabag/CoreBundle/Entity/SiteCredential.php
+++ b/src/Wallabag/CoreBundle/Entity/SiteCredential.php
@@ -46,8 +46,7 @@ class SiteCredential
      * @var string
      *
      * @Assert\NotBlank()
-     * @Assert\Length(max=255)
-     * @ORM\Column(name="password", type="string", length=255)
+     * @ORM\Column(name="password", type="text")
      */
     private $password;
 
diff --git a/src/Wallabag/CoreBundle/Helper/CryptoProxy.php b/src/Wallabag/CoreBundle/Helper/CryptoProxy.php
new file mode 100644
index 00000000..d0a9b85c
--- /dev/null
+++ b/src/Wallabag/CoreBundle/Helper/CryptoProxy.php
@@ -0,0 +1,86 @@
+<?php
+
+namespace Wallabag\CoreBundle\Helper;
+
+use Psr\Log\LoggerInterface;
+use Defuse\Crypto\Key;
+use Defuse\Crypto\Crypto;
+use Defuse\Crypto\Exception\WrongKeyOrModifiedCiphertextException;
+
+/**
+ * This is a proxy to crypt and decrypt password used by SiteCredential entity.
+ * BTW, It might be re-use for sth else.
+ */
+class CryptoProxy
+{
+    private $logger;
+    private $encryptionKey;
+
+    public function __construct($encryptionKeyPath, LoggerInterface $logger)
+    {
+        $this->logger = $logger;
+
+        if (!file_exists($encryptionKeyPath)) {
+            $key = Key::createNewRandomKey();
+
+            file_put_contents($encryptionKeyPath, $key->saveToAsciiSafeString());
+            chmod($encryptionKeyPath, 0600);
+        }
+
+        $this->encryptionKey = file_get_contents($encryptionKeyPath);
+    }
+
+    /**
+     * Ensure the given value will be crypted.
+     *
+     * @param string $secretValue Secret valye to crypt
+     *
+     * @return string
+     */
+    public function crypt($secretValue)
+    {
+        $this->logger->debug('Crypto: crypting value: '.$this->mask($secretValue));
+
+        return Crypto::encrypt($secretValue, $this->loadKey());
+    }
+
+    /**
+     * Ensure the given crypted value will be decrypted.
+     *
+     * @param string $cryptedValue The value to be decrypted
+     *
+     * @return string
+     */
+    public function decrypt($cryptedValue)
+    {
+        $this->logger->debug('Crypto: decrypting value: '.$this->mask($cryptedValue));
+
+        try {
+            return Crypto::decrypt($cryptedValue, $this->loadKey());
+        } catch (WrongKeyOrModifiedCiphertextException $e) {
+            throw new \RuntimeException('Decrypt fail: '.$e->getMessage());
+        }
+    }
+
+    /**
+     * Load the private key.
+     *
+     * @return string
+     */
+    private function loadKey()
+    {
+        return Key::loadFromAsciiSafeString($this->encryptionKey);
+    }
+
+    /**
+     * Keep first and last character and put some stars in between.
+     *
+     * @param string $value Value to mask
+     *
+     * @return string
+     */
+    private function mask($value)
+    {
+        return $value[0].'*****'.$value[strlen($value) - 1];
+    }
+}
diff --git a/src/Wallabag/CoreBundle/Repository/SiteCredentialRepository.php b/src/Wallabag/CoreBundle/Repository/SiteCredentialRepository.php
index 316ecc75..6f904f0a 100644
--- a/src/Wallabag/CoreBundle/Repository/SiteCredentialRepository.php
+++ b/src/Wallabag/CoreBundle/Repository/SiteCredentialRepository.php
@@ -2,11 +2,20 @@
 
 namespace Wallabag\CoreBundle\Repository;
 
+use Wallabag\CoreBundle\Helper\CryptoProxy;
+
 /**
  * SiteCredentialRepository.
  */
 class SiteCredentialRepository extends \Doctrine\ORM\EntityRepository
 {
+    private $cryptoProxy;
+
+    public function setCrypto(CryptoProxy $cryptoProxy)
+    {
+        $this->cryptoProxy = $cryptoProxy;
+    }
+
     /**
      * Retrieve one username/password for the given host and userId.
      *
@@ -17,12 +26,21 @@ class SiteCredentialRepository extends \Doctrine\ORM\EntityRepository
      */
     public function findOneByHostAndUser($host, $userId)
     {
-        return $this->createQueryBuilder('s')
+        $res = $this->createQueryBuilder('s')
             ->select('s.username', 's.password')
             ->where('s.host = :hostname')->setParameter('hostname', $host)
             ->andWhere('s.user = :userId')->setParameter('userId', $userId)
             ->setMaxResults(1)
             ->getQuery()
             ->getOneOrNullResult();
+
+        if (null === $res) {
+            return;
+        }
+
+        // decrypt password before returning it
+        $res['password'] = $this->cryptoProxy->decrypt($res['password']);
+
+        return $res;
     }
 }
diff --git a/src/Wallabag/CoreBundle/Resources/config/services.yml b/src/Wallabag/CoreBundle/Resources/config/services.yml
index 09bc77fe..e09b0f18 100644
--- a/src/Wallabag/CoreBundle/Resources/config/services.yml
+++ b/src/Wallabag/CoreBundle/Resources/config/services.yml
@@ -126,6 +126,8 @@ services:
         factory: [ "@doctrine.orm.default_entity_manager", getRepository ]
         arguments:
             - WallabagCoreBundle:SiteCredential
+        calls:
+            - [ setCrypto, [ "@wallabag_core.helper.crypto_proxy" ] ]
 
     wallabag_core.helper.entries_export:
         class: Wallabag\CoreBundle\Helper\EntriesExport
@@ -208,3 +210,9 @@ services:
 
     wallabag_core.entry.download_images.client:
         class: GuzzleHttp\Client
+
+    wallabag_core.helper.crypto_proxy:
+        class: Wallabag\CoreBundle\Helper\CryptoProxy
+        arguments:
+            - "%wallabag_core.site_credentials.encryption_key_path%"
+            - "@logger"
diff --git a/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php b/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php
index 104f60f1..80380685 100644
--- a/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php
+++ b/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php
@@ -1341,7 +1341,7 @@ class EntryControllerTest extends WallabagCoreTestCase
         $credential = new SiteCredential($user);
         $credential->setHost('monde-diplomatique.fr');
         $credential->setUsername('foo');
-        $credential->setPassword('bar');
+        $credential->setPassword($client->getContainer()->get('wallabag_core.helper.crypto_proxy')->crypt('bar'));
 
         $em->persist($credential);
         $em->flush();
diff --git a/tests/Wallabag/CoreBundle/GuzzleSiteAuthenticator/GrabySiteConfigBuilderTest.php b/tests/Wallabag/CoreBundle/GuzzleSiteAuthenticator/GrabySiteConfigBuilderTest.php
index 1e1e8989..b0c81e7b 100644
--- a/tests/Wallabag/CoreBundle/GuzzleSiteAuthenticator/GrabySiteConfigBuilderTest.php
+++ b/tests/Wallabag/CoreBundle/GuzzleSiteAuthenticator/GrabySiteConfigBuilderTest.php
@@ -6,12 +6,11 @@ use Monolog\Handler\TestHandler;
 use Monolog\Logger;
 use BD\GuzzleSiteAuthenticator\SiteConfig\SiteConfig;
 use Graby\SiteConfig\SiteConfig as GrabySiteConfig;
-use PHPUnit_Framework_TestCase;
 use Wallabag\CoreBundle\GuzzleSiteAuthenticator\GrabySiteConfigBuilder;
 use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
 
-class GrabySiteConfigBuilderTest extends PHPUnit_Framework_TestCase
+class GrabySiteConfigBuilderTest extends \PHPUnit_Framework_TestCase
 {
     /** @var \Wallabag\CoreBundle\GuzzleSiteAuthenticator\GrabySiteConfigBuilder */
     protected $builder;
diff --git a/tests/Wallabag/CoreBundle/Helper/CryptoProxyTest.php b/tests/Wallabag/CoreBundle/Helper/CryptoProxyTest.php
new file mode 100644
index 00000000..cede8696
--- /dev/null
+++ b/tests/Wallabag/CoreBundle/Helper/CryptoProxyTest.php
@@ -0,0 +1,40 @@
+<?php
+
+namespace Tests\Wallabag\CoreBundle\Helper;
+
+use Psr\Log\NullLogger;
+use Monolog\Logger;
+use Monolog\Handler\TestHandler;
+use Wallabag\CoreBundle\Helper\CryptoProxy;
+
+class CryptoProxyTest extends \PHPUnit_Framework_TestCase
+{
+    public function testCrypto()
+    {
+        $logHandler = new TestHandler();
+        $logger = new Logger('test', [$logHandler]);
+
+        $crypto = new CryptoProxy(sys_get_temp_dir().'/'.uniqid('', true).'.txt', $logger);
+        $crypted = $crypto->crypt('test');
+        $decrypted = $crypto->decrypt($crypted);
+
+        $this->assertSame('test', $decrypted);
+
+        $records = $logHandler->getRecords();
+        $this->assertCount(2, $records);
+        $this->assertContains('Crypto: crypting value', $records[0]['message']);
+        $this->assertContains('Crypto: decrypting value', $records[1]['message']);
+    }
+
+    /**
+     * @expectedException RuntimeException
+     * @expectedExceptionMessage Decrypt fail
+     *
+     * @return [type] [description]
+     */
+    public function testDecryptBadValue()
+    {
+        $crypto = new CryptoProxy(sys_get_temp_dir().'/'.uniqid('', true).'.txt', new NullLogger());
+        $crypto->decrypt('badvalue');
+    }
+}