diff options
15 files changed, 169 insertions, 9 deletions
@@ -56,3 +56,4 @@ app/Resources/build/ | |||
56 | # Test-generated files | 56 | # Test-generated files |
57 | admin-export.json | 57 | admin-export.json |
58 | specialexport.json | 58 | specialexport.json |
59 | /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 | |||
39 | $table->addColumn('user_id', 'integer'); | 39 | $table->addColumn('user_id', 'integer'); |
40 | $table->addColumn('host', 'string', ['length' => 255]); | 40 | $table->addColumn('host', 'string', ['length' => 255]); |
41 | $table->addColumn('username', 'string', ['length' => 255]); | 41 | $table->addColumn('username', 'string', ['length' => 255]); |
42 | $table->addColumn('password', 'string', ['length' => 255]); | 42 | $table->addColumn('password', 'text'); |
43 | $table->addColumn('createdAt', 'datetime'); | 43 | $table->addColumn('createdAt', 'datetime'); |
44 | $table->addIndex(['user_id'], 'idx_user'); | 44 | $table->addIndex(['user_id'], 'idx_user'); |
45 | $table->setPrimaryKey(['id']); | 45 | $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: | |||
26 | fetching_error_message: | | 26 | fetching_error_message: | |
27 | 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>. | 27 | 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>. |
28 | api_limit_mass_actions: 10 | 28 | api_limit_mass_actions: 10 |
29 | encryption_key_path: "%kernel.root_dir%/../data/site-credentials-secret-key.txt" | ||
29 | default_internal_settings: | 30 | default_internal_settings: |
30 | - | 31 | - |
31 | name: share_public | 32 | 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 @@ | |||
73 | "kphoen/rulerz-bundle": "~0.13", | 73 | "kphoen/rulerz-bundle": "~0.13", |
74 | "guzzlehttp/guzzle": "^5.3.1", | 74 | "guzzlehttp/guzzle": "^5.3.1", |
75 | "doctrine/doctrine-migrations-bundle": "^1.0", | 75 | "doctrine/doctrine-migrations-bundle": "^1.0", |
76 | "paragonie/random_compat": "~1.0", | 76 | "paragonie/random_compat": "~2.0", |
77 | "craue/config-bundle": "~2.0", | 77 | "craue/config-bundle": "~2.0", |
78 | "mnapoli/piwik-twig-extension": "^1.0", | 78 | "mnapoli/piwik-twig-extension": "^1.0", |
79 | "ocramius/proxy-manager": "1.*", | 79 | "ocramius/proxy-manager": "1.*", |
@@ -83,7 +83,8 @@ | |||
83 | "javibravo/simpleue": "^1.0", | 83 | "javibravo/simpleue": "^1.0", |
84 | "symfony/dom-crawler": "^3.1", | 84 | "symfony/dom-crawler": "^3.1", |
85 | "friendsofsymfony/jsrouting-bundle": "^1.6", | 85 | "friendsofsymfony/jsrouting-bundle": "^1.6", |
86 | "bdunogier/guzzle-site-authenticator": "^1.0.0@dev" | 86 | "bdunogier/guzzle-site-authenticator": "^1.0.0@dev", |
87 | "defuse/php-encryption": "^2.1" | ||
87 | }, | 88 | }, |
88 | "require-dev": { | 89 | "require-dev": { |
89 | "doctrine/doctrine-fixtures-bundle": "~2.2", | 90 | "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 | |||
313 | 313 | ||
314 | $this | 314 | $this |
315 | ->runCommand('doctrine:migrations:migrate', ['--no-interaction' => true]); | 315 | ->runCommand('doctrine:migrations:migrate', ['--no-interaction' => true]); |
316 | |||
317 | return $this; | ||
316 | } | 318 | } |
317 | 319 | ||
318 | /** | 320 | /** |
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 | |||
45 | $form->handleRequest($request); | 45 | $form->handleRequest($request); |
46 | 46 | ||
47 | if ($form->isSubmitted() && $form->isValid()) { | 47 | if ($form->isSubmitted() && $form->isValid()) { |
48 | $credential->setPassword($this->get('wallabag_core.helper.crypto_proxy')->crypt($credential->getPassword())); | ||
49 | |||
48 | $em = $this->getDoctrine()->getManager(); | 50 | $em = $this->getDoctrine()->getManager(); |
49 | $em->persist($credential); | 51 | $em->persist($credential); |
50 | $em->flush($credential); | 52 | $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 | |||
63 | ->end() | 63 | ->end() |
64 | ->end() | 64 | ->end() |
65 | ->end() | 65 | ->end() |
66 | ->scalarNode('encryption_key_path') | ||
67 | ->end() | ||
66 | ->end() | 68 | ->end() |
67 | ; | 69 | ; |
68 | 70 | ||
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 | |||
29 | $container->setParameter('wallabag_core.fetching_error_message_title', $config['fetching_error_message_title']); | 29 | $container->setParameter('wallabag_core.fetching_error_message_title', $config['fetching_error_message_title']); |
30 | $container->setParameter('wallabag_core.api_limit_mass_actions', $config['api_limit_mass_actions']); | 30 | $container->setParameter('wallabag_core.api_limit_mass_actions', $config['api_limit_mass_actions']); |
31 | $container->setParameter('wallabag_core.default_internal_settings', $config['default_internal_settings']); | 31 | $container->setParameter('wallabag_core.default_internal_settings', $config['default_internal_settings']); |
32 | $container->setParameter('wallabag_core.site_credentials.encryption_key_path', $config['encryption_key_path']); | ||
32 | 33 | ||
33 | $loader = new Loader\YamlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config')); | 34 | $loader = new Loader\YamlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config')); |
34 | $loader->load('services.yml'); | 35 | $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 | |||
46 | * @var string | 46 | * @var string |
47 | * | 47 | * |
48 | * @Assert\NotBlank() | 48 | * @Assert\NotBlank() |
49 | * @Assert\Length(max=255) | 49 | * @ORM\Column(name="password", type="text") |
50 | * @ORM\Column(name="password", type="string", length=255) | ||
51 | */ | 50 | */ |
52 | private $password; | 51 | private $password; |
53 | 52 | ||
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 @@ | |||
1 | <?php | ||
2 | |||
3 | namespace Wallabag\CoreBundle\Helper; | ||
4 | |||
5 | use Psr\Log\LoggerInterface; | ||
6 | use Defuse\Crypto\Key; | ||
7 | use Defuse\Crypto\Crypto; | ||
8 | use Defuse\Crypto\Exception\WrongKeyOrModifiedCiphertextException; | ||
9 | |||
10 | /** | ||
11 | * This is a proxy to crypt and decrypt password used by SiteCredential entity. | ||
12 | * BTW, It might be re-use for sth else. | ||
13 | */ | ||
14 | class CryptoProxy | ||
15 | { | ||
16 | private $logger; | ||
17 | private $encryptionKey; | ||
18 | |||
19 | public function __construct($encryptionKeyPath, LoggerInterface $logger) | ||
20 | { | ||
21 | $this->logger = $logger; | ||
22 | |||
23 | if (!file_exists($encryptionKeyPath)) { | ||
24 | $key = Key::createNewRandomKey(); | ||
25 | |||
26 | file_put_contents($encryptionKeyPath, $key->saveToAsciiSafeString()); | ||
27 | chmod($encryptionKeyPath, 0600); | ||
28 | } | ||
29 | |||
30 | $this->encryptionKey = file_get_contents($encryptionKeyPath); | ||
31 | } | ||
32 | |||
33 | /** | ||
34 | * Ensure the given value will be crypted. | ||
35 | * | ||
36 | * @param string $secretValue Secret valye to crypt | ||
37 | * | ||
38 | * @return string | ||
39 | */ | ||
40 | public function crypt($secretValue) | ||
41 | { | ||
42 | $this->logger->debug('Crypto: crypting value: '.$this->mask($secretValue)); | ||
43 | |||
44 | return Crypto::encrypt($secretValue, $this->loadKey()); | ||
45 | } | ||
46 | |||
47 | /** | ||
48 | * Ensure the given crypted value will be decrypted. | ||
49 | * | ||
50 | * @param string $cryptedValue The value to be decrypted | ||
51 | * | ||
52 | * @return string | ||
53 | */ | ||
54 | public function decrypt($cryptedValue) | ||
55 | { | ||
56 | $this->logger->debug('Crypto: decrypting value: '.$this->mask($cryptedValue)); | ||
57 | |||
58 | try { | ||
59 | return Crypto::decrypt($cryptedValue, $this->loadKey()); | ||
60 | } catch (WrongKeyOrModifiedCiphertextException $e) { | ||
61 | throw new \RuntimeException('Decrypt fail: '.$e->getMessage()); | ||
62 | } | ||
63 | } | ||
64 | |||
65 | /** | ||
66 | * Load the private key. | ||
67 | * | ||
68 | * @return string | ||
69 | */ | ||
70 | private function loadKey() | ||
71 | { | ||
72 | return Key::loadFromAsciiSafeString($this->encryptionKey); | ||
73 | } | ||
74 | |||
75 | /** | ||
76 | * Keep first and last character and put some stars in between. | ||
77 | * | ||
78 | * @param string $value Value to mask | ||
79 | * | ||
80 | * @return string | ||
81 | */ | ||
82 | private function mask($value) | ||
83 | { | ||
84 | return $value[0].'*****'.$value[strlen($value) - 1]; | ||
85 | } | ||
86 | } | ||
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 @@ | |||
2 | 2 | ||
3 | namespace Wallabag\CoreBundle\Repository; | 3 | namespace Wallabag\CoreBundle\Repository; |
4 | 4 | ||
5 | use Wallabag\CoreBundle\Helper\CryptoProxy; | ||
6 | |||
5 | /** | 7 | /** |
6 | * SiteCredentialRepository. | 8 | * SiteCredentialRepository. |
7 | */ | 9 | */ |
8 | class SiteCredentialRepository extends \Doctrine\ORM\EntityRepository | 10 | class SiteCredentialRepository extends \Doctrine\ORM\EntityRepository |
9 | { | 11 | { |
12 | private $cryptoProxy; | ||
13 | |||
14 | public function setCrypto(CryptoProxy $cryptoProxy) | ||
15 | { | ||
16 | $this->cryptoProxy = $cryptoProxy; | ||
17 | } | ||
18 | |||
10 | /** | 19 | /** |
11 | * Retrieve one username/password for the given host and userId. | 20 | * Retrieve one username/password for the given host and userId. |
12 | * | 21 | * |
@@ -17,12 +26,21 @@ class SiteCredentialRepository extends \Doctrine\ORM\EntityRepository | |||
17 | */ | 26 | */ |
18 | public function findOneByHostAndUser($host, $userId) | 27 | public function findOneByHostAndUser($host, $userId) |
19 | { | 28 | { |
20 | return $this->createQueryBuilder('s') | 29 | $res = $this->createQueryBuilder('s') |
21 | ->select('s.username', 's.password') | 30 | ->select('s.username', 's.password') |
22 | ->where('s.host = :hostname')->setParameter('hostname', $host) | 31 | ->where('s.host = :hostname')->setParameter('hostname', $host) |
23 | ->andWhere('s.user = :userId')->setParameter('userId', $userId) | 32 | ->andWhere('s.user = :userId')->setParameter('userId', $userId) |
24 | ->setMaxResults(1) | 33 | ->setMaxResults(1) |
25 | ->getQuery() | 34 | ->getQuery() |
26 | ->getOneOrNullResult(); | 35 | ->getOneOrNullResult(); |
36 | |||
37 | if (null === $res) { | ||
38 | return; | ||
39 | } | ||
40 | |||
41 | // decrypt password before returning it | ||
42 | $res['password'] = $this->cryptoProxy->decrypt($res['password']); | ||
43 | |||
44 | return $res; | ||
27 | } | 45 | } |
28 | } | 46 | } |
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: | |||
126 | factory: [ "@doctrine.orm.default_entity_manager", getRepository ] | 126 | factory: [ "@doctrine.orm.default_entity_manager", getRepository ] |
127 | arguments: | 127 | arguments: |
128 | - WallabagCoreBundle:SiteCredential | 128 | - WallabagCoreBundle:SiteCredential |
129 | calls: | ||
130 | - [ setCrypto, [ "@wallabag_core.helper.crypto_proxy" ] ] | ||
129 | 131 | ||
130 | wallabag_core.helper.entries_export: | 132 | wallabag_core.helper.entries_export: |
131 | class: Wallabag\CoreBundle\Helper\EntriesExport | 133 | class: Wallabag\CoreBundle\Helper\EntriesExport |
@@ -208,3 +210,9 @@ services: | |||
208 | 210 | ||
209 | wallabag_core.entry.download_images.client: | 211 | wallabag_core.entry.download_images.client: |
210 | class: GuzzleHttp\Client | 212 | class: GuzzleHttp\Client |
213 | |||
214 | wallabag_core.helper.crypto_proxy: | ||
215 | class: Wallabag\CoreBundle\Helper\CryptoProxy | ||
216 | arguments: | ||
217 | - "%wallabag_core.site_credentials.encryption_key_path%" | ||
218 | - "@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 | |||
1341 | $credential = new SiteCredential($user); | 1341 | $credential = new SiteCredential($user); |
1342 | $credential->setHost('monde-diplomatique.fr'); | 1342 | $credential->setHost('monde-diplomatique.fr'); |
1343 | $credential->setUsername('foo'); | 1343 | $credential->setUsername('foo'); |
1344 | $credential->setPassword('bar'); | 1344 | $credential->setPassword($client->getContainer()->get('wallabag_core.helper.crypto_proxy')->crypt('bar')); |
1345 | 1345 | ||
1346 | $em->persist($credential); | 1346 | $em->persist($credential); |
1347 | $em->flush(); | 1347 | $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; | |||
6 | use Monolog\Logger; | 6 | use Monolog\Logger; |
7 | use BD\GuzzleSiteAuthenticator\SiteConfig\SiteConfig; | 7 | use BD\GuzzleSiteAuthenticator\SiteConfig\SiteConfig; |
8 | use Graby\SiteConfig\SiteConfig as GrabySiteConfig; | 8 | use Graby\SiteConfig\SiteConfig as GrabySiteConfig; |
9 | use PHPUnit_Framework_TestCase; | ||
10 | use Wallabag\CoreBundle\GuzzleSiteAuthenticator\GrabySiteConfigBuilder; | 9 | use Wallabag\CoreBundle\GuzzleSiteAuthenticator\GrabySiteConfigBuilder; |
11 | use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; | 10 | use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; |
12 | use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; | 11 | use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; |
13 | 12 | ||
14 | class GrabySiteConfigBuilderTest extends PHPUnit_Framework_TestCase | 13 | class GrabySiteConfigBuilderTest extends \PHPUnit_Framework_TestCase |
15 | { | 14 | { |
16 | /** @var \Wallabag\CoreBundle\GuzzleSiteAuthenticator\GrabySiteConfigBuilder */ | 15 | /** @var \Wallabag\CoreBundle\GuzzleSiteAuthenticator\GrabySiteConfigBuilder */ |
17 | protected $builder; | 16 | 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 @@ | |||
1 | <?php | ||
2 | |||
3 | namespace Tests\Wallabag\CoreBundle\Helper; | ||
4 | |||
5 | use Psr\Log\NullLogger; | ||
6 | use Monolog\Logger; | ||
7 | use Monolog\Handler\TestHandler; | ||
8 | use Wallabag\CoreBundle\Helper\CryptoProxy; | ||
9 | |||
10 | class CryptoProxyTest extends \PHPUnit_Framework_TestCase | ||
11 | { | ||
12 | public function testCrypto() | ||
13 | { | ||
14 | $logHandler = new TestHandler(); | ||
15 | $logger = new Logger('test', [$logHandler]); | ||
16 | |||
17 | $crypto = new CryptoProxy(sys_get_temp_dir().'/'.uniqid('', true).'.txt', $logger); | ||
18 | $crypted = $crypto->crypt('test'); | ||
19 | $decrypted = $crypto->decrypt($crypted); | ||
20 | |||
21 | $this->assertSame('test', $decrypted); | ||
22 | |||
23 | $records = $logHandler->getRecords(); | ||
24 | $this->assertCount(2, $records); | ||
25 | $this->assertContains('Crypto: crypting value', $records[0]['message']); | ||
26 | $this->assertContains('Crypto: decrypting value', $records[1]['message']); | ||
27 | } | ||
28 | |||
29 | /** | ||
30 | * @expectedException RuntimeException | ||
31 | * @expectedExceptionMessage Decrypt fail | ||
32 | * | ||
33 | * @return [type] [description] | ||
34 | */ | ||
35 | public function testDecryptBadValue() | ||
36 | { | ||
37 | $crypto = new CryptoProxy(sys_get_temp_dir().'/'.uniqid('', true).'.txt', new NullLogger()); | ||
38 | $crypto->decrypt('badvalue'); | ||
39 | } | ||
40 | } | ||