From a46fd5fc9f9439190dcc3fdd524e6451e4a4e5ef Mon Sep 17 00:00:00 2001 From: Simounet Date: Tue, 26 Sep 2023 18:02:46 +0200 Subject: [PATCH 1/6] Fix deprecated null parameter passed to explode() --- src/Wallabag/CoreBundle/Controller/TagController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Wallabag/CoreBundle/Controller/TagController.php b/src/Wallabag/CoreBundle/Controller/TagController.php index 6e147df98..d0e5a5aa6 100644 --- a/src/Wallabag/CoreBundle/Controller/TagController.php +++ b/src/Wallabag/CoreBundle/Controller/TagController.php @@ -45,7 +45,7 @@ class TagController extends AbstractController $form = $this->createForm(NewTagType::class, new Tag()); $form->handleRequest($request); - $tags = $form->get('label')->getData(); + $tags = $form->get('label')->getData() ?? ''; $tagsExploded = explode(',', $tags); // avoid too much tag to be added From 9bc026f34300b887aa72e66cd2f5b708690fcfd5 Mon Sep 17 00:00:00 2001 From: Simounet Date: Wed, 27 Sep 2023 19:25:16 +0200 Subject: [PATCH 2/6] Fix #6971 - Full clickable card on mass action --- .../static/themes/material/css/entries.scss | 22 +++++++++++-------- .../views/Entry/Card/_mass_checkbox.html.twig | 4 ++-- .../views/Entry/_card_preview.html.twig | 2 +- web/wallassets/material.css | 2 +- web/wallassets/material.css.map | 2 +- 5 files changed, 18 insertions(+), 14 deletions(-) diff --git a/app/Resources/static/themes/material/css/entries.scss b/app/Resources/static/themes/material/css/entries.scss index b79a201da..0db39d7c0 100644 --- a/app/Resources/static/themes/material/css/entries.scss +++ b/app/Resources/static/themes/material/css/entries.scss @@ -38,25 +38,29 @@ border-radius: 2px 0 0 2px; } -.entry-checkbox { +.card-stacked .entry-checkbox { margin: 10px 15px 10px 5px; +} - .card & { - float: right; - margin-right: 0; - padding: 10px; - } +.card .entry-checkbox { + position: absolute; + display: flex; + padding: 10px; + inset: 0; + justify-content: flex-end; + align-items: start; + background-color: rgb(0 172 193 / 20%); + cursor: pointer; + z-index: 10; } .entries .entry-checkbox-input, .mass-action .entry-checkbox-input { position: relative; - left: 0; width: 20px; min-height: 25px; - height: 100%; - vertical-align: middle; opacity: initial; + cursor: pointer; z-index: 10; } diff --git a/src/Wallabag/CoreBundle/Resources/views/Entry/Card/_mass_checkbox.html.twig b/src/Wallabag/CoreBundle/Resources/views/Entry/Card/_mass_checkbox.html.twig index 503be8ae2..5e4fe8f6d 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Entry/Card/_mass_checkbox.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Entry/Card/_mass_checkbox.html.twig @@ -1,3 +1,3 @@ -
+
+ diff --git a/src/Wallabag/CoreBundle/Resources/views/Entry/_card_preview.html.twig b/src/Wallabag/CoreBundle/Resources/views/Entry/_card_preview.html.twig index 82fde9c9b..5fba482fc 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Entry/_card_preview.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Entry/_card_preview.html.twig @@ -1,7 +1,7 @@
+ {% include "@WallabagCore/Entry/Card/_mass_checkbox.html.twig" with {'entry': entry} only %}
- {% include "@WallabagCore/Entry/Card/_mass_checkbox.html.twig" with {'entry': entry} only %}
diff --git a/src/Wallabag/CoreBundle/Resources/views/Config/otp_app.html.twig b/src/Wallabag/CoreBundle/Resources/views/Config/otp_app.html.twig index 3beaf241f..f042fd500 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Config/otp_app.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Config/otp_app.html.twig @@ -40,6 +40,7 @@ {% endfor %}
+
diff --git a/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php b/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php index 9174daf85..46f5c232e 100644 --- a/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php +++ b/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php @@ -1174,14 +1174,13 @@ class ConfigControllerTest extends WallabagCoreTestCase $this->logInAs('admin'); $client = $this->getTestClient(); - $crawler = $client->request('GET', '/config/otp/email'); + $crawler = $client->request('GET', '/config'); + + $form = $crawler->filter('form[name=config_otp_email]')->form(); + $client->submit($form); $this->assertSame(302, $client->getResponse()->getStatusCode()); - - $crawler = $client->followRedirect(); - - $this->assertGreaterThan(1, $alert = $crawler->filter('body')->extract(['_text'])); - $this->assertStringContainsString('flashes.config.notice.otp_enabled', $alert[0]); + $this->assertStringContainsString('flashes.config.notice.otp_enabled', $client->getContainer()->get(SessionInterface::class)->getFlashBag()->get('notice')[0]); // restore user $em = $this->getEntityManager(); @@ -1201,14 +1200,23 @@ class ConfigControllerTest extends WallabagCoreTestCase $this->logInAs('admin'); $client = $this->getTestClient(); - $crawler = $client->request('GET', '/config/otp/email/disable'); + $em = $this->getEntityManager(); + $user = $em + ->getRepository(User::class) + ->findOneByUsername('admin'); + + $user->setEmailTwoFactor(true); + $em->persist($user); + $em->flush(); + + $crawler = $client->request('GET', '/config'); + + $form = $crawler->filter('form[name=disable_otp_email]')->form(); + $client->submit($form); $this->assertSame(302, $client->getResponse()->getStatusCode()); - - $crawler = $client->followRedirect(); - - $this->assertGreaterThan(1, $alert = $crawler->filter('body')->extract(['_text'])); - $this->assertStringContainsString('flashes.config.notice.otp_disabled', $alert[0]); + + $this->assertStringContainsString('flashes.config.notice.otp_disabled', $client->getContainer()->get(SessionInterface::class)->getFlashBag()->get('notice')[0]); // restore user $em = $this->getEntityManager(); @@ -1224,7 +1232,10 @@ class ConfigControllerTest extends WallabagCoreTestCase $this->logInAs('admin'); $client = $this->getTestClient(); - $crawler = $client->request('GET', '/config/otp/app'); + $crawler = $client->request('GET', '/config'); + + $form = $crawler->filter('form[name=config_otp_app]')->form(); + $client->submit($form); $this->assertSame(200, $client->getResponse()->getStatusCode()); @@ -1278,14 +1289,23 @@ class ConfigControllerTest extends WallabagCoreTestCase $this->logInAs('admin'); $client = $this->getTestClient(); - $crawler = $client->request('GET', '/config/otp/app/disable'); + $em = $this->getEntityManager(); + $user = $em + ->getRepository(User::class) + ->findOneByUsername('admin'); + + $user->setGoogleAuthenticatorSecret("Google2FA"); + $em->persist($user); + $em->flush(); + + $crawler = $client->request('GET', '/config'); + + $form = $crawler->filter('form[name=disable_otp_app]')->form(); + $client->submit($form); $this->assertSame(302, $client->getResponse()->getStatusCode()); - - $crawler = $client->followRedirect(); - - $this->assertGreaterThan(1, $alert = $crawler->filter('body')->extract(['_text'])); - $this->assertStringContainsString('flashes.config.notice.otp_disabled', $alert[0]); + + $this->assertStringContainsString('flashes.config.notice.otp_disabled', $client->getContainer()->get(SessionInterface::class)->getFlashBag()->get('notice')[0]); // restore user $em = $this->getEntityManager(); From aa06e8328eaac5cbdc7ac02d57a6d3a9f7c63b18 Mon Sep 17 00:00:00 2001 From: Kevin Decherf Date: Sat, 30 Sep 2023 00:46:19 +0200 Subject: [PATCH 5/6] ConfigController: remove 2fa cancel step This change annoys me, however this endpoint was anyway problematic: - it was vulnerable to a CSRF attack, see GHSA-56fm-hfp3-x3w3 - it is useless as we don't really handle a two-steps validation Still, if you send an incorrect code during the "activation" phase a flash error will pop up but the 2fa will stay enabled. This need rework when possible. Signed-off-by: Kevin Decherf --- .../Controller/ConfigController.php | 13 ++++++-- .../Resources/views/Config/otp_app.html.twig | 3 -- .../Controller/ConfigControllerTest.php | 30 ------------------- 3 files changed, 10 insertions(+), 36 deletions(-) diff --git a/src/Wallabag/CoreBundle/Controller/ConfigController.php b/src/Wallabag/CoreBundle/Controller/ConfigController.php index db318dddf..cfe992f59 100644 --- a/src/Wallabag/CoreBundle/Controller/ConfigController.php +++ b/src/Wallabag/CoreBundle/Controller/ConfigController.php @@ -373,8 +373,10 @@ class ConfigController extends AbstractController * Cancelling 2FA using OTP app. * * @Route("/config/otp/app/cancel", name="config_otp_app_cancel") + * + * XXX: commented until we rewrite 2fa with a real two-steps activation */ - public function otpAppCancelAction() + /*public function otpAppCancelAction() { $user = $this->getUser(); $user->setGoogleAuthenticatorSecret(null); @@ -383,7 +385,7 @@ class ConfigController extends AbstractController $this->userManager->updateUser($user, true); return $this->redirect($this->generateUrl('config') . '#set3'); - } + }*/ /** * Validate OTP code. @@ -415,7 +417,12 @@ class ConfigController extends AbstractController 'scheb_two_factor.code_invalid' ); - return $this->redirect($this->generateUrl('config_otp_app')); + $this->addFlash( + 'notice', + 'scheb_two_factor.code_invalid' + ); + + return $this->redirect($this->generateUrl('config') . '#set3'); } /** diff --git a/src/Wallabag/CoreBundle/Resources/views/Config/otp_app.html.twig b/src/Wallabag/CoreBundle/Resources/views/Config/otp_app.html.twig index f042fd500..6d5d402b1 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Config/otp_app.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Config/otp_app.html.twig @@ -50,9 +50,6 @@
- - {{ 'config.otp.app.cancel'|trans }} - + + {% if app.user.isEmailTwoFactor %} +
+ + + +
+ {% endif %} + + + + {{ 'config.form_user.two_factor.googleTwoFactor_label'|trans }} + {% if app.user.isGoogleTwoFactor %}{{ 'config.form_user.two_factor.state_enabled'|trans }}{% else %}{{ 'config.form_user.two_factor.state_disabled'|trans }}{% endif %} + +
+ + + +
+ {% if app.user.isGoogleTwoFactor %} +
+ + + +
+ {% endif %} + + + + +
diff --git a/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php b/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php index c0d138dfa..6c049e1dc 100644 --- a/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php +++ b/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php @@ -1215,7 +1215,7 @@ class ConfigControllerTest extends WallabagCoreTestCase $client->submit($form); $this->assertSame(302, $client->getResponse()->getStatusCode()); - + $this->assertStringContainsString('flashes.config.notice.otp_disabled', $client->getContainer()->get(SessionInterface::class)->getFlashBag()->get('notice')[0]); // restore user @@ -1264,7 +1264,7 @@ class ConfigControllerTest extends WallabagCoreTestCase ->getRepository(User::class) ->findOneByUsername('admin'); - $user->setGoogleAuthenticatorSecret("Google2FA"); + $user->setGoogleAuthenticatorSecret('Google2FA'); $em->persist($user); $em->flush(); @@ -1274,7 +1274,7 @@ class ConfigControllerTest extends WallabagCoreTestCase $client->submit($form); $this->assertSame(302, $client->getResponse()->getStatusCode()); - + $this->assertStringContainsString('flashes.config.notice.otp_disabled', $client->getContainer()->get(SessionInterface::class)->getFlashBag()->get('notice')[0]); // restore user