Cache-control headers are set to private when using 2FA bundle

[johnnoel]

Bundle version: 7.3.0
Symfony version: 7.0.7
PHP version: 8.3.7

Description

I have a route /my-page for a plain content page where I set public HTTP cache control headers. I have a logged-in only area with all routes using the prefix /app/.

When using the 2FA bundle, regardless of whether I'm logged in or not, the cache control headers for /my-page are set to Cache-Control: max-age=0, must-revalidate, private as if I'm logged in.

To Reproduce

# security.yaml
security:
    firewalls:
        main:
            lazy: true
            # I use form_login, remember_me, logout and login_throttling 
            two_factor:
                auth_form_path: 2fa_login
                check_path: 2fa_login_check

    access_control:
        - { path: ^/logout, roles: PUBLIC_ACCESS }
        - { path: ^/2fa, roles: IS_AUTHENTICATED_2FA_IN_PROGRESS }
        - { path: ^/app/, roles: ROLE_USER }

// DefaultController.php
    #[Cache(maxage: 60 * 60, public: true, mustRevalidate: true)]
    #[Route('/my-page', name: 'my-page', methods: [ 'GET' ])]
    public function myPage(): Response
    {
        return $this->render('default/my-page.html.twig');
    }

Visit /my-page and look at the HTTP headers. Instead of seeing Cache-control: max-age=3600, must-revalidate, public as expected, see Cache-Control: max-age=0, must-revalidate, private.

Additional Context

When visiting /my-page, the cache control headers are set as if I'm logged in, however I'm not logged in and no session has been started.

After digging around I found that in Scheb\TwoFactorBundle\Security\Authorization::isPubliclyAccessAttribute() it considers a route with no access control to be non-public. The solution then is to add a fallback access control in security.yaml that looks like:

    access_control:
        # other rules
        - { path: ^/, roles: PUBLIC_ACCESS }

This solves the problem but feels kind of hacky as it doesn't feel like it should be needed.

I understand that the check was introduced after Symfony 5.2's lazy: true, and came up in issues #38 #39 and #40. Changing the isPubliclyAccessAttribute method to return true when there isn't any access control attribute also solves my problem, and the test suite passes except for one test which specifically tests that method's outcome though the app test suite is decidedly unhappy.

I'm happy enough to open up a PR with this change but I get the feeling that I'm not aware of the impact this would have.

[scheb]

We cannot change this line, because it changes the behavior of the bundle. If we change this, you're no longer redirected to the 2fa form after login. I tried locally with my test application and half of the integration test suite is going red 😅

The issue seems to be caused by this line

2fa/src/bundle/Security/Http/Firewall/TwoFactorAccessListener.php

Line 42 in 0649ea5

$token = $this->tokenStorage->getToken();

which initializes the token storage, which as a results sets the private cache headers on the response. Though the bundle has to make this call in order to decide if a 2fa process is currently ongoing, as this information is coming from the security token.

Only on routes, which are explicitly configured as "public", this check can be skipped, as these aren't protected by the firewall. That's why you see the cache header being set on routes that have a PUBLIC_ACCESS configured.

Configuring a "catch all" PUBLIC_ACCESS for all other routes seems to be the sensible approach. But you should be aware that 2fa will not respond to these routes. That means after login, you might get redirected to one of the public routes, instead of the 2fa form. Then you have the user in that "not yet fully authenticated" state until they hit a protected route, which will trigger 2fa and redirect them to the 2fa form.