Fix reviews

Encrypt username too Redirect to list after saving credentials Fix typos Signed-off-by: Thomas Citharel <tcit@tcit.fr>
2024-12-14 17:35:17 +01:00 · 2017-06-14 15:02:34 +02:00 · 2017-06-14 15:02:34 +02:00 · bead8b42da
commit bead8b42da
parent 906424c1b6
11 changed files with 43 additions and 22 deletions
--- a/app/DoctrineMigrations/Version20170501115751.php
+++ b/app/DoctrineMigrations/Version20170501115751.php
@ -38,7 +38,7 @@ class Version20170501115751 extends AbstractMigration implements ContainerAwareI
        $table->addColumn('id', 'integer', ['autoincrement' => true]);
        $table->addColumn('user_id', 'integer');
        $table->addColumn('host', 'string', ['length' => 255]);
-        $table->addColumn('username', 'string', ['length' => 255]);
+        $table->addColumn('username', 'text');
        $table->addColumn('password', 'text');
        $table->addColumn('createdAt', 'datetime');
        $table->addIndex(['user_id'], 'idx_user');
--- a/src/Wallabag/CoreBundle/Controller/SiteCredentialController.php
+++ b/src/Wallabag/CoreBundle/Controller/SiteCredentialController.php
@ -26,9 +26,9 @@ class SiteCredentialController extends Controller
    {
        $credentials = $this->get('wallabag_core.site_credential_repository')->findByUser($this->getUser());

-        return $this->render('WallabagCoreBundle:SiteCredential:index.html.twig', array(
+        return $this->render('WallabagCoreBundle:SiteCredential:index.html.twig', [
            'credentials' => $credentials,
-        ));
+        ]);
    }

    /**
@ -36,6 +36,10 @@ class SiteCredentialController extends Controller
     *
     * @Route("/new", name="site_credentials_new")
     * @Method({"GET", "POST"})
+     *
+     * @param Request $request
+     *
+     * @return \Symfony\Component\HttpFoundation\Response
     */
    public function newAction(Request $request)
    {
@ -45,24 +49,25 @@ class SiteCredentialController extends Controller
        $form->handleRequest($request);

        if ($form->isSubmitted() && $form->isValid()) {
+            $credential->setUsername($this->get('wallabag_core.helper.crypto_proxy')->crypt($credential->getUsername()));
            $credential->setPassword($this->get('wallabag_core.helper.crypto_proxy')->crypt($credential->getPassword()));

            $em = $this->getDoctrine()->getManager();
            $em->persist($credential);
-            $em->flush($credential);
+            $em->flush();

            $this->get('session')->getFlashBag()->add(
                'notice',
                $this->get('translator')->trans('flashes.site_credential.notice.added', ['%host%' => $credential->getHost()])
            );

-            return $this->redirectToRoute('site_credentials_edit', array('id' => $credential->getId()));
+            return $this->redirectToRoute('site_credentials_index');
        }

-        return $this->render('WallabagCoreBundle:SiteCredential:new.html.twig', array(
+        return $this->render('WallabagCoreBundle:SiteCredential:new.html.twig', [
            'credential' => $credential,
            'form' => $form->createView(),
-        ));
+        ]);
    }

    /**
@ -70,6 +75,11 @@ class SiteCredentialController extends Controller
     *
     * @Route("/{id}/edit", name="site_credentials_edit")
     * @Method({"GET", "POST"})
+     *
+     * @param Request        $request
+     * @param SiteCredential $siteCredential
+     *
+     * @return \Symfony\Component\HttpFoundation\Response
     */
    public function editAction(Request $request, SiteCredential $siteCredential)
    {
@ -80,6 +90,9 @@ class SiteCredentialController extends Controller
        $editForm->handleRequest($request);

        if ($editForm->isSubmitted() && $editForm->isValid()) {
+            $siteCredential->setUsername($this->get('wallabag_core.helper.crypto_proxy')->crypt($siteCredential->getUsername()));
+            $siteCredential->setPassword($this->get('wallabag_core.helper.crypto_proxy')->crypt($siteCredential->getPassword()));
+
            $em = $this->getDoctrine()->getManager();
            $em->persist($siteCredential);
            $em->flush();
@ -89,14 +102,14 @@ class SiteCredentialController extends Controller
                $this->get('translator')->trans('flashes.site_credential.notice.updated', ['%host%' => $siteCredential->getHost()])
            );

-            return $this->redirectToRoute('site_credentials_edit', array('id' => $siteCredential->getId()));
+            return $this->redirectToRoute('site_credentials_index');
        }

-        return $this->render('WallabagCoreBundle:SiteCredential:edit.html.twig', array(
+        return $this->render('WallabagCoreBundle:SiteCredential:edit.html.twig', [
            'credential' => $siteCredential,
            'edit_form' => $editForm->createView(),
            'delete_form' => $deleteForm->createView(),
-        ));
+        ]);
    }

    /**
@ -104,6 +117,11 @@ class SiteCredentialController extends Controller
     *
     * @Route("/{id}", name="site_credentials_delete")
     * @Method("DELETE")
+     *
+     * @param Request        $request
+     * @param SiteCredential $siteCredential
+     *
+     * @return \Symfony\Component\HttpFoundation\RedirectResponse
     */
    public function deleteAction(Request $request, SiteCredential $siteCredential)
    {
@ -136,7 +154,7 @@ class SiteCredentialController extends Controller
    private function createDeleteForm(SiteCredential $siteCredential)
    {
        return $this->createFormBuilder()
-            ->setAction($this->generateUrl('site_credentials_delete', array('id' => $siteCredential->getId())))
+            ->setAction($this->generateUrl('site_credentials_delete', ['id' => $siteCredential->getId()]))
            ->setMethod('DELETE')
            ->getForm()
        ;
--- a/src/Wallabag/CoreBundle/Entity/SiteCredential.php
+++ b/src/Wallabag/CoreBundle/Entity/SiteCredential.php
@ -37,8 +37,7 @@ class SiteCredential
     * @var string
     *
     * @Assert\NotBlank()
-     * @Assert\Length(max=255)
-     * @ORM\Column(name="username", type="string", length=255)
+     * @ORM\Column(name="username", type="text")
     */
    private $username;

--- a/src/Wallabag/CoreBundle/Form/Type/SiteCredentialType.php
+++ b/src/Wallabag/CoreBundle/Form/Type/SiteCredentialType.php
@ -19,6 +19,7 @@ class SiteCredentialType extends AbstractType
            ])
            ->add('username', TextType::class, [
                'label' => 'site_credential.form.username_label',
+                'data' => '',
            ])
            ->add('password', PasswordType::class, [
                'label' => 'site_credential.form.password_label',
--- a/src/Wallabag/CoreBundle/GuzzleSiteAuthenticator/GrabySiteConfigBuilder.php
+++ b/src/Wallabag/CoreBundle/GuzzleSiteAuthenticator/GrabySiteConfigBuilder.php
@ -87,7 +87,8 @@ class GrabySiteConfigBuilder implements SiteConfigBuilder

        $config = new SiteConfig($parameters);

-        // do not leak password in log
+        // do not leak usernames and passwords in log
+        $parameters['username'] = '**masked**';
        $parameters['password'] = '**masked**';

        $this->logger->debug('Auth: add parameters.', ['host' => $host, 'parameters' => $parameters]);
--- a/src/Wallabag/CoreBundle/Helper/CryptoProxy.php
+++ b/src/Wallabag/CoreBundle/Helper/CryptoProxy.php
@ -65,7 +65,7 @@ class CryptoProxy
    /**
     * Load the private key.
     *
-     * @return string
+     * @return Key
     */
    private function loadKey()
    {
@ -81,6 +81,6 @@ class CryptoProxy
     */
    private function mask($value)
    {
-        return $value[0].'*****'.$value[strlen($value) - 1];
+        return strlen($value) > 0 ? $value[0].'*****'.$value[strlen($value) - 1] : 'Empty value';
    }
 }
--- a/src/Wallabag/CoreBundle/Repository/SiteCredentialRepository.php
+++ b/src/Wallabag/CoreBundle/Repository/SiteCredentialRepository.php
@ -38,7 +38,8 @@ class SiteCredentialRepository extends \Doctrine\ORM\EntityRepository
            return;
        }

-        // decrypt password before returning it
+        // decrypt user & password before returning them
+        $res['username'] = $this->cryptoProxy->decrypt($res['username']);
        $res['password'] = $this->cryptoProxy->decrypt($res['password']);

        return $res;
--- a/src/Wallabag/CoreBundle/Resources/translations/messages.fr.yml
+++ b/src/Wallabag/CoreBundle/Resources/translations/messages.fr.yml
@ -515,7 +515,7 @@ user:
        twofactor_label: "Double authentification"
        save: "Sauvegarder"
        delete: "Supprimer"
-        delete_confirm: "Êtes-vous sur ?"
+        delete_confirm: "Êtes-vous sûr ?"
        back_to_list: "Revenir à la liste"
    search:
        placeholder: "Filtrer par nom d’utilisateur ou email"
@ -537,7 +537,7 @@ site_credential:
        password_label: 'Mot de passe'
        save: "Sauvegarder"
        delete: "Supprimer"
-        delete_confirm: "Êtes-vous sur ?"
+        delete_confirm: "Êtes-vous sûr ?"
        back_to_list: "Revenir à la liste"

 error:
--- a/src/Wallabag/CoreBundle/Resources/views/themes/material/SiteCredential/index.html.twig
+++ b/src/Wallabag/CoreBundle/Resources/views/themes/material/SiteCredential/index.html.twig
@ -16,6 +16,7 @@
                                <tr>
                                    <th>{{ 'site_credential.form.host_label'|trans }}</th>
                                    <th>{{ 'site_credential.form.username_label'|trans }}</th>
+                                    <th>{{ 'site_credential.form.password_label'|trans }}</th>
                                    <th>{{ 'site_credential.list.actions'|trans }}</th>
                                </tr>
                            </thead>
@ -23,7 +24,8 @@
                            {% for credential in credentials %}
                                <tr>
                                    <td>{{ credential.host }}</td>
-                                    <td>{{ credential.username }}</td>
+                                    <td>*****</td>
+                                    <td>*****</td>
                                    <td>
                                        <a href="{{ path('site_credentials_edit', { 'id': credential.id }) }}">{{ 'site_credential.list.edit_action'|trans }}</a>
                                    </td>
--- a/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php
+++ b/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php
@ -1340,7 +1340,7 @@ class EntryControllerTest extends WallabagCoreTestCase
        $user = $client->getContainer()->get('security.token_storage')->getToken()->getUser();
        $credential = new SiteCredential($user);
        $credential->setHost('monde-diplomatique.fr');
-        $credential->setUsername('foo');
+        $credential->setUsername($client->getContainer()->get('wallabag_core.helper.crypto_proxy')->crypt('foo'));
        $credential->setPassword($client->getContainer()->get('wallabag_core.helper.crypto_proxy')->crypt('bar'));

        $em->persist($credential);
--- a/tests/Wallabag/CoreBundle/Controller/SiteCredentialControllerTest.php
+++ b/tests/Wallabag/CoreBundle/Controller/SiteCredentialControllerTest.php
@ -85,7 +85,6 @@ class SiteCredentialControllerTest extends WallabagCoreTestCase
        $crawler = $client->followRedirect();

        $this->assertContains('flashes.site_credential.notice.updated', $crawler->filter('body')->extract(['_text'])[0]);
-        $this->assertContains('larry', $crawler->filter('input[id=site_credential_username]')->attr('value'));
    }

    public function testEditFromADifferentUserSiteCredential()