2fa icon indicating copy to clipboard operation
2fa copied to clipboard

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

Open johnnoel opened this issue 8 months ago • 1 comments

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.

johnnoel avatar May 29 '24 19:05 johnnoel