From 7a0e6970b447b270c09e16fc7ee4098f736a7a12 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Thu, 21 Jan 2016 16:35:41 +0100 Subject: [PATCH 1/7] Use PHP7 random_bytes to generate RSS Token random_bytes is a PHP 7 function wich has been ported to PHP 5 using paragonie/random_compat --- composer.json | 3 ++- src/Wallabag/CoreBundle/Tools/Utils.php | 15 ++++----------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/composer.json b/composer.json index 78b32307a..d84e1f8b8 100644 --- a/composer.json +++ b/composer.json @@ -62,7 +62,8 @@ "wallabag/php-mobi": "~1.0.0", "kphoen/rulerz-bundle": "~0.10", "guzzlehttp/guzzle": "^5.2.0", - "doctrine/doctrine-migrations-bundle": "^1.0" + "doctrine/doctrine-migrations-bundle": "^1.0", + "paragonie/random_compat": "~1.0" }, "require-dev": { "doctrine/doctrine-fixtures-bundle": "~2.2", diff --git a/src/Wallabag/CoreBundle/Tools/Utils.php b/src/Wallabag/CoreBundle/Tools/Utils.php index a16baca97..71cbc490d 100644 --- a/src/Wallabag/CoreBundle/Tools/Utils.php +++ b/src/Wallabag/CoreBundle/Tools/Utils.php @@ -7,20 +7,13 @@ class Utils /** * Generate a token used for RSS. * + * @param integer $length Length of the token + * * @return string */ - public static function generateToken() + public static function generateToken($length = 15) { - if (ini_get('open_basedir') === '') { - if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') { - // alternative to /dev/urandom for Windows - $token = substr(base64_encode(uniqid(mt_rand(), true)), 0, 20); - } else { - $token = substr(base64_encode(file_get_contents('/dev/urandom', false, null, 0, 20)), 0, 15); - } - } else { - $token = substr(base64_encode(uniqid(mt_rand(), true)), 0, 20); - } + $token = substr(base64_encode(random_bytes($length)), 0, $length); // remove character which can broken the url return str_replace(array('+', '/'), '', $token); From 27ea492cf72657a7ba2deb3d45302923ddd289b8 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Thu, 21 Jan 2016 16:36:17 +0100 Subject: [PATCH 2/7] Add tests on TagAllCommand Some simple tests --- .../CoreBundle/Command/TagAllCommand.php | 2 +- .../Tests/Command/TagAllCommandTest.php | 60 +++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 src/Wallabag/CoreBundle/Tests/Command/TagAllCommandTest.php diff --git a/src/Wallabag/CoreBundle/Command/TagAllCommand.php b/src/Wallabag/CoreBundle/Command/TagAllCommand.php index 2cf3f8084..db1a9ab79 100644 --- a/src/Wallabag/CoreBundle/Command/TagAllCommand.php +++ b/src/Wallabag/CoreBundle/Command/TagAllCommand.php @@ -28,7 +28,7 @@ class TagAllCommand extends ContainerAwareCommand try { $user = $this->getUser($input->getArgument('username')); } catch (NoResultException $e) { - $output->writeln(sprintf('User %s not found.', $input->getArgument('username'))); + $output->writeln(sprintf('User "%s" not found.', $input->getArgument('username'))); return 1; } diff --git a/src/Wallabag/CoreBundle/Tests/Command/TagAllCommandTest.php b/src/Wallabag/CoreBundle/Tests/Command/TagAllCommandTest.php new file mode 100644 index 000000000..653c1a93e --- /dev/null +++ b/src/Wallabag/CoreBundle/Tests/Command/TagAllCommandTest.php @@ -0,0 +1,60 @@ +getClient()->getKernel()); + $application->add(new TagAllCommand()); + + $command = $application->find('wallabag:tag:all'); + + $tester = new CommandTester($command); + $tester->execute(array( + 'command' => $command->getName(), + )); + } + + public function testRunTagAllCommandWithBadUsername() + { + $application = new Application($this->getClient()->getKernel()); + $application->add(new TagAllCommand()); + + $command = $application->find('wallabag:tag:all'); + + $tester = new CommandTester($command); + $tester->execute(array( + 'command' => $command->getName(), + 'username' => 'unknown', + )); + + $this->assertContains('User "unknown" not found', $tester->getDisplay()); + } + + public function testRunTagAllCommand() + { + $application = new Application($this->getClient()->getKernel()); + $application->add(new TagAllCommand()); + + $command = $application->find('wallabag:tag:all'); + + $tester = new CommandTester($command); + $tester->execute(array( + 'command' => $command->getName(), + 'username' => 'admin', + )); + + $this->assertContains('Tagging entries for user « admin »... Done', $tester->getDisplay()); + } +} From e56983af1f7b74993784687e7b192a898422fe7f Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Thu, 21 Jan 2016 16:36:30 +0100 Subject: [PATCH 3/7] Fix bad redirection when adding a new user --- src/Wallabag/CoreBundle/Controller/ConfigController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Wallabag/CoreBundle/Controller/ConfigController.php b/src/Wallabag/CoreBundle/Controller/ConfigController.php index d0cf91def..6c375909c 100644 --- a/src/Wallabag/CoreBundle/Controller/ConfigController.php +++ b/src/Wallabag/CoreBundle/Controller/ConfigController.php @@ -125,7 +125,7 @@ class ConfigController extends Controller $newUser->setEnabled(true); $newUserForm = $this->createForm(NewUserType::class, $newUser, array( 'validation_groups' => array('Profile'), - 'action' => $this->generateUrl('config').'#set5', + 'action' => $this->generateUrl('config').'#set6', )); $newUserForm->handleRequest($request); From a0d6ccc5ca0dc0082467cc65b006150aff2488c4 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Thu, 21 Jan 2016 16:37:25 +0100 Subject: [PATCH 4/7] Fix bad type after using findByUrlAndUserId It returns an object since few commits this part of (untested) code still use an array. Also add test for that part of code. --- .../CoreBundle/Controller/EntryController.php | 4 +-- .../Tests/Controller/EntryControllerTest.php | 31 +++++++++++++++++-- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/Wallabag/CoreBundle/Controller/EntryController.php b/src/Wallabag/CoreBundle/Controller/EntryController.php index 9dd904f1a..3e1b512ff 100644 --- a/src/Wallabag/CoreBundle/Controller/EntryController.php +++ b/src/Wallabag/CoreBundle/Controller/EntryController.php @@ -54,10 +54,10 @@ class EntryController extends Controller if (false !== $existingEntry) { $this->get('session')->getFlashBag()->add( 'notice', - 'Entry already saved on '.$existingEntry['createdAt']->format('d-m-Y') + 'Entry already saved on '.$existingEntry->getCreatedAt()->format('d-m-Y') ); - return $this->redirect($this->generateUrl('view', array('id' => $existingEntry['id']))); + return $this->redirect($this->generateUrl('view', array('id' => $existingEntry->getId()))); } $this->updateEntry($entry); diff --git a/src/Wallabag/CoreBundle/Tests/Controller/EntryControllerTest.php b/src/Wallabag/CoreBundle/Tests/Controller/EntryControllerTest.php index 1d1620dc2..32d6a5753 100644 --- a/src/Wallabag/CoreBundle/Tests/Controller/EntryControllerTest.php +++ b/src/Wallabag/CoreBundle/Tests/Controller/EntryControllerTest.php @@ -127,10 +127,35 @@ class EntryControllerTest extends WallabagCoreTestCase $this->assertEquals(302, $client->getResponse()->getStatusCode()); - $crawler = $client->followRedirect(); + $content = $client->getContainer() + ->get('doctrine.orm.entity_manager') + ->getRepository('WallabagCoreBundle:Entry') + ->findByUrlAndUserId($this->url, $this->getLoggedInUserId()); - $this->assertGreaterThan(1, $alert = $crawler->filter('h2 a')->extract(array('_text'))); - $this->assertContains('Google', $alert[0]); + $this->assertInstanceOf('Wallabag\CoreBundle\Entity\Entry', $content); + $this->assertEquals($this->url, $content->getUrl()); + $this->assertContains('Google', $content->getTitle()); + } + + public function testPostNewOkUrlExist() + { + $this->logInAs('admin'); + $client = $this->getClient(); + + $crawler = $client->request('GET', '/new'); + + $this->assertEquals(200, $client->getResponse()->getStatusCode()); + + $form = $crawler->filter('button[type=submit]')->form(); + + $data = array( + 'entry[url]' => $this->url, + ); + + $client->submit($form, $data); + + $this->assertEquals(302, $client->getResponse()->getStatusCode()); + $this->assertContains('/view/', $client->getResponse()->getTargetUrl()); } /** From 0f0e8eb82a3374e20453fb1f8046325ee306b036 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Thu, 21 Jan 2016 16:39:13 +0100 Subject: [PATCH 5/7] Use FOSUserEvents instead of c/p a controller The `resetAction` was overriden to redirect user to the homepage instead of `fos_user_profile_show`. Instead of copying the whole method we can simply use FOSUserEvents to handle that. --- .../Controller/ResettingController.php | 75 ------------------- .../PasswordResettingListener.php | 41 ++++++++++ .../UserBundle/Resources/config/services.yml | 7 ++ 3 files changed, 48 insertions(+), 75 deletions(-) delete mode 100644 src/Wallabag/UserBundle/Controller/ResettingController.php create mode 100644 src/Wallabag/UserBundle/EventListener/PasswordResettingListener.php diff --git a/src/Wallabag/UserBundle/Controller/ResettingController.php b/src/Wallabag/UserBundle/Controller/ResettingController.php deleted file mode 100644 index 62e27d005..000000000 --- a/src/Wallabag/UserBundle/Controller/ResettingController.php +++ /dev/null @@ -1,75 +0,0 @@ -get('fos_user.resetting.form.factory'); - /** @var $userManager \FOS\UserBundle\Model\UserManagerInterface */ - $userManager = $this->get('fos_user.user_manager'); - /** @var $dispatcher \Symfony\Component\EventDispatcher\EventDispatcherInterface */ - $dispatcher = $this->get('event_dispatcher'); - - $user = $userManager->findUserByConfirmationToken($token); - - if (null === $user) { - throw new NotFoundHttpException(sprintf('The user with "confirmation token" does not exist for value "%s"', $token)); - } - - $event = new GetResponseUserEvent($user, $request); - $dispatcher->dispatch(FOSUserEvents::RESETTING_RESET_INITIALIZE, $event); - - if (null !== $event->getResponse()) { - return $event->getResponse(); - } - - $form = $formFactory->createForm(); - $form->setData($user); - - $form->handleRequest($request); - - if ($form->isValid()) { - $event = new FormEvent($form, $request); - $dispatcher->dispatch(FOSUserEvents::RESETTING_RESET_SUCCESS, $event); - - $userManager->updateUser($user); - - if (null === $response = $event->getResponse()) { - $this->get('session')->getFlashBag()->add( - 'notice', - 'Password updated' - ); - $url = $this->generateUrl('homepage'); - $response = new RedirectResponse($url); - } - - $dispatcher->dispatch(FOSUserEvents::RESETTING_RESET_COMPLETED, new FilterUserResponseEvent($user, $request, $response)); - - return $response; - } - - return $this->render('FOSUserBundle:Resetting:reset.html.twig', array( - 'token' => $token, - 'form' => $form->createView(), - )); - } -} diff --git a/src/Wallabag/UserBundle/EventListener/PasswordResettingListener.php b/src/Wallabag/UserBundle/EventListener/PasswordResettingListener.php new file mode 100644 index 000000000..2fdcb4419 --- /dev/null +++ b/src/Wallabag/UserBundle/EventListener/PasswordResettingListener.php @@ -0,0 +1,41 @@ +router = $router; + } + + /** + * {@inheritDoc} + */ + public static function getSubscribedEvents() + { + return array( + FOSUserEvents::RESETTING_RESET_SUCCESS => 'onPasswordResettingSuccess', + ); + } + + public function onPasswordResettingSuccess(FormEvent $event) + { + $url = $this->router->generate('homepage'); + + $event->setResponse(new RedirectResponse($url)); + } +} diff --git a/src/Wallabag/UserBundle/Resources/config/services.yml b/src/Wallabag/UserBundle/Resources/config/services.yml index 93e04d592..bf9e036ae 100644 --- a/src/Wallabag/UserBundle/Resources/config/services.yml +++ b/src/Wallabag/UserBundle/Resources/config/services.yml @@ -8,3 +8,10 @@ services: - "%scheb_two_factor.email.sender_name%" - "%wallabag_support_url%" - "%wallabag_url%" + + wallabag_user.password_resetting: + class: Wallabag\UserBundle\EventListener\PasswordResettingListener + arguments: + - "@router" + tags: + - { name: kernel.event_subscriber } From a3cac44c78a1d798e3282daa6b8c3e59e8ebc690 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Thu, 21 Jan 2016 18:06:10 +0100 Subject: [PATCH 6/7] Add for deleting rule from an other user --- .../Tests/Controller/ConfigControllerTest.php | 55 +++++++++++++++++-- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/src/Wallabag/CoreBundle/Tests/Controller/ConfigControllerTest.php b/src/Wallabag/CoreBundle/Tests/Controller/ConfigControllerTest.php index 89ca31e29..c8807425a 100644 --- a/src/Wallabag/CoreBundle/Tests/Controller/ConfigControllerTest.php +++ b/src/Wallabag/CoreBundle/Tests/Controller/ConfigControllerTest.php @@ -520,18 +520,61 @@ class ConfigControllerTest extends WallabagCoreTestCase return array( array( array( - 'rss_config[rule]' => 'unknownVar <= 3', - 'rss_config[tags]' => 'cool tag', + 'tagging_rule[rule]' => 'unknownVar <= 3', + 'tagging_rule[tags]' => 'cool tag', + ), + array( + 'The variable', + 'does not exist.', ), - 'The variable « unknownVar » does not exist.', ), array( array( - 'rss_config[rule]' => 'length(domainName) <= 42', - 'rss_config[tags]' => 'cool tag', + 'tagging_rule[rule]' => 'length(domainName) <= 42', + 'tagging_rule[tags]' => 'cool tag', + ), + array( + 'The operator', + 'does not exist.', ), - 'The operator « length » does not exist.', ), ); } + + /** + * @dataProvider dataForTaggingRuleFailed + */ + public function testTaggingRuleCreationFail($data, $messages) + { + $this->logInAs('admin'); + $client = $this->getClient(); + + $crawler = $client->request('GET', '/config'); + + $this->assertTrue($client->getResponse()->isSuccessful()); + + $form = $crawler->filter('button[id=tagging_rule_save]')->form(); + + $client->submit($form, $data); + + $this->assertEquals(200, $client->getResponse()->getStatusCode()); + + foreach ($messages as $message) { + $this->assertContains($message, $client->getResponse()->getContent()); + } + } + + public function testDeletingTaggingRuleFromAnOtherUser() + { + $this->logInAs('bob'); + $client = $this->getClient(); + + $rule = $client->getContainer()->get('doctrine.orm.entity_manager') + ->getRepository('WallabagCoreBundle:TaggingRule') + ->findAll()[0]; + + $client->request('GET', '/tagging-rule/delete/'.$rule->getId()); + $this->assertEquals(403, $client->getResponse()->getStatusCode()); + $this->assertContains('You can not access this tagging ryle', $client->getResponse()->getContent()); + } } From 23afdf3a70a5035cb58b76138a8627701ba55273 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Fri, 22 Jan 2016 08:01:32 +0100 Subject: [PATCH 7/7] CS --- src/Wallabag/CoreBundle/Tools/Utils.php | 2 +- .../UserBundle/EventListener/PasswordResettingListener.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Wallabag/CoreBundle/Tools/Utils.php b/src/Wallabag/CoreBundle/Tools/Utils.php index 71cbc490d..0c7831102 100644 --- a/src/Wallabag/CoreBundle/Tools/Utils.php +++ b/src/Wallabag/CoreBundle/Tools/Utils.php @@ -7,7 +7,7 @@ class Utils /** * Generate a token used for RSS. * - * @param integer $length Length of the token + * @param int $length Length of the token * * @return string */ diff --git a/src/Wallabag/UserBundle/EventListener/PasswordResettingListener.php b/src/Wallabag/UserBundle/EventListener/PasswordResettingListener.php index 2fdcb4419..128e85a42 100644 --- a/src/Wallabag/UserBundle/EventListener/PasswordResettingListener.php +++ b/src/Wallabag/UserBundle/EventListener/PasswordResettingListener.php @@ -9,7 +9,7 @@ use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; /** - * Listener responsible to change the redirection at the end of the password resetting + * Listener responsible to change the redirection at the end of the password resetting. * * @see http://symfony.com/doc/current/bundles/FOSUserBundle/controller_events.html */ @@ -23,7 +23,7 @@ class PasswordResettingListener implements EventSubscriberInterface } /** - * {@inheritDoc} + * {@inheritdoc} */ public static function getSubscribedEvents() {