Fix infinite redirection of postless tags

.env

@@ -33,6 +33,7 @@ MAILER_DSN=smtp://localhost
 ###< symfony/mailer ###
 
 APP_MAIN_CONTACT=1
+APP_REQUIRE_2FA=true
 
 APP_MAIL_SENDER=user@example.org
 APP_LOG_RECIPIENT=user@example.org

config/services.yaml

@@ -21,11 +21,11 @@ services:
             - '../src/Kernel.php'
             - '../src/Tests/'
 
-    # controllers are imported separately to make sure services can be injected
-    # as action arguments even if you don't extend any base controller class
     App\Controller\:
         resource: '../src/Controller/'
         tags: ['controller.service_arguments']
+        arguments:
+            $requireTwoFactorAuth: '%env(bool:APP_REQUIRE_2FA)%'
 
     App\Settings:
         arguments:

docs/tags.rst

@@ -9,6 +9,10 @@ and on that page it has
 a title (which must be unique),
 and optional description (which uses the same syntax as Post bodies).
 
+A Tag's page has a list of Posts, which is paginated if there are more than ten posts.
+The first page has a URL of the form ``/Txx`` and subsequent pages ``/Txx/page-n`` (where ``n`` is 2 or greater).
+The layout of each post is the same as for the chronological listings of posts.
+
 Wikidata
 --------
 

phpunit.xml.dist

@@ -10,6 +10,7 @@
     <php>
         <ini name="error_reporting" value="-1" />
         <server name="APP_ENV" value="test" force="true" />
+        <server name="APP_REQUIRE_2FA" value="false" />
         <server name="SHELL_VERBOSITY" value="-1" />
         <server name="SYMFONY_PHPUNIT_REMOVE" value="" />
         <server name="SYMFONY_PHPUNIT_VERSION" value="7.5" />

src/Controller/ControllerBase.php

@@ -18,6 +18,9 @@ abstract class ControllerBase extends AbstractController
     /** @var string */
     protected const FLASH_SUCCESS = 'success';
 
+    /** @var bool */
+    private $requireTwoFactorAuth;
+
     /**
      * @return User|null
      */
@@ -26,13 +29,18 @@ protected function getUser()
         return parent::getUser();
     }
 
+    public function __construct($requireTwoFactorAuth)
+    {
+        $this->requireTwoFactorAuth = $requireTwoFactorAuth;
+    }
+
     /**
      * Get authorization response (e.g. 403, redirect, etc.).
      * This is used in \App\EventListener\ControllerListener (which is why it's public).
      */
     public function getAuthResponse(): ?Response
     {
-        if (!$this->getUser() || $this->getUser()->getTwoFASecret()) {
+        if (!$this->requireTwoFactorAuth || !$this->getUser() || $this->getUser()->getTwoFASecret()) {
             return null;
         }
         $this->addFlash(self::FLASH_NOTICE, 'Please set up 2FA.');

src/Controller/ResetPasswordController.php

@@ -23,8 +23,9 @@ class ResetPasswordController extends ControllerBase
 
     private $resetPasswordHelper;
 
-    public function __construct(ResetPasswordHelperInterface $resetPasswordHelper)
+    public function __construct(bool $requireTwoFactorAuth, ResetPasswordHelperInterface $resetPasswordHelper)
     {
+        parent::__construct($requireTwoFactorAuth);
         $this->resetPasswordHelper = $resetPasswordHelper;
     }
 

src/Controller/TagController.php

@@ -40,7 +40,7 @@ public function viewTag(
         }
         $postCount = $tagRepository->countPosts($tag, $this->getUser());
         $pageCount = ceil($postCount / 10);
-        if ($pageNum > $pageCount) {
+        if ($pageCount > 0 && $pageNum > $pageCount) {
             // Redirect to last page.
             return $this->redirectToRoute('tag_view_page', ['id' => $tag->getId(), 'pageNum' => $pageCount]);
         }

src/EventListener/ControllerListener.php

@@ -20,7 +20,7 @@ public function onKernelController(ControllerEvent $event)
         /** @var ControllerBase $controller */
         $controller = $event->getController()[0];
 
-        // Some controllers are special and don't need interferring with.
+        // Some controllers are special and don't need interfering with.
         if (
             $controller instanceof SecurityController
             || $controller instanceof ResetPasswordController

templates/_macros.twig

@@ -1,20 +1,25 @@
 {% macro pagination(page_count, page_num, post_count, href_prev, href_next) %}
-    <nav class="pagination">
 
-        {% if page_num > 1 %}
-            <a href="{{ href_prev }}">&larr; Prev</a>
-        {% else %}
-            <span class="disabled">&larr; Prev</span>
-        {% endif %}
+    {% if page_count > 0 %}
 
-        | Page {{ page_num }} of {{ page_count }} ({{ post_count }} posts) |
+        <nav class="pagination">
 
-        {% if page_num < page_count %}
-            <a href="{{ href_next }}">Next &rarr;</a>
-        {% else %}
-            <span class="disabled">Next &rarr;</span>
-        {% endif %}
+            {% if page_num > 1 %}
+                <a href="{{ href_prev }}">&larr; Prev</a>
+            {% else %}
+                <span class="disabled">&larr; Prev</span>
+            {% endif %}
 
-    </nav>
+            | Page {{ page_num }} of {{ page_count }} ({{ post_count }} posts) |
+
+            {% if page_num < page_count %}
+                <a href="{{ href_next }}">Next &rarr;</a>
+            {% else %}
+                <span class="disabled">Next &rarr;</span>
+            {% endif %}
+
+        </nav>
+
+    {% endif %}
 
 {% endmacro %}

tests/Controller/TagControllerTest.php

@@ -0,0 +1,87 @@
+<?php
+
+namespace App\Tests\Controller;
+
+use App\Entity\Post;
+use Symfony\Bridge\PhpUnit\ClockMock;
+use Symfony\Bundle\FrameworkBundle\KernelBrowser;
+use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
+use Symfony\Component\DomCrawler\Form;
+
+class TagControllerTest extends WebTestCase
+{
+
+    public function setUp()
+    {
+        // Set a fake clock time of 2020-11-15 07:36:41 and register all our classes that use the time() function.
+        ClockMock::withClockMock(1605425801);
+        ClockMock::register(Post::class);
+    }
+
+    protected function createAccountAndLogIn(KernelBrowser $client)
+    {
+        $username = 'testuser-' . time();
+        $password = 'test123';
+        $client->request('GET', '/register');
+        $client->submitForm('Create account', [
+            'username' => $username,
+            'email' => 'test@example.org',
+            'password' => $password,
+            '_csrf_token' => '123',
+        ], 'POST');
+        $client->request('GET', '/login');
+        $client->submitForm('Log in', [
+            'username' => $username,
+            'password' => $password,
+        ]);
+        $client->request('GET', '/contacts');
+        // There will only be one edit link.
+        $client->clickLink('Edit');
+        //dump($client->getCrawler());
+        $client->submitForm('Save', ['new_group' => 'Private'], 'POST');
+        //dump($client->getCrawler());
+    }
+
+    public function testTagPage()
+    {
+        $client = static::createClient();
+        $this->createAccountAndLogIn($client);
+
+        // Create a new public post, with a tag.
+        $client->request('GET', '/post/new');
+        $crawler = $client->getCrawler();
+        $buttonNode = $crawler->selectButton('Save');
+        $form = new Form($buttonNode->getNode(0), $crawler->getUri(), 'POST', $crawler->getBaseHref());
+        // Disable validation, because the 'tag1' option doesn't exist in the HTML (it's added by JS).
+        $form->disableValidation();
+        $form->setValues(['tags' => ['tag1']]);
+        $client->submit($form);
+        $client->followRedirect();
+
+        // Now we're on the new post's page, go to the tag page.
+        $postUri = $client->getCrawler()->getUri();
+        $client->clickLink('tag1');
+        $this->assertSelectorTextContains('main h1', 'tag1');
+        $tagUri = $client->getCrawler()->getUri();
+
+        // Confirm that the post is listed.
+        $postLink = substr($postUri, strrpos($postUri, '/'));
+        $this->assertSelectorExists('a[href="' . $postLink . '"]');
+
+        // Edit the post to make it private.
+        $client->click($client->getCrawler()->filter('ol.post-list')->selectLink('Edit')->link());
+        $secondGroupId = $client->getCrawler()
+            ->filter('#view_group option')
+            ->getNode(1)
+            ->attributes
+            ->getNamedItem('value')
+            ->nodeValue;
+        $client->submitForm('Save', ['view_group' => $secondGroupId], 'POST');
+        $client->followRedirect();
+
+        // Log out, go back to the tag page, and find no posts.
+        $client->clickLink('Log out');
+        $client->request('GET', $tagUri);
+        $this->assertSelectorTextContains('main h1', 'tag1');
+    }
+}