verbb / knock-knock

Password protect your entire Craft website front-end with a single password
MIT License
18 stars 7 forks source link

Cached 302 redirect leading to problems in Chrome #63

Closed NicolajKN closed 1 year ago

NicolajKN commented 1 year ago

Describe the bug

When linking directly to a password-protected page, we get a 302 redirect to the login-page. The trouble is that Chrome seems to cache the redirect, meaning when the user returns to the page after logging in, they are again sent to the login-page as if they were logged out.

I can see from Developer Tools that the redirect is indeed served from Disk cache and the initial redirect includes a (in our case) cache-control: max-age=2592000 header.

I guess the fix for this would be to explicitly set a no-cache header?

Steps to reproduce

  1. Go directly to a password-protected page, triggering the redirect in Chrome.
  2. Log in.
  3. Go to the page again.

Craft CMS version

4.3.3

Plugin version

2.0.5

Multi-site?

No

Additional context

No response

engram-design commented 1 year ago

Hmm, we don't mess around with the headers specifically to apply a cache-control (it might be your server configuration), but we could likely just add that header to the redirect with no real issue.

NicolajKN commented 1 year ago

It's definitely some server configuration that sets the default headers.

Would it be possible to set a no-cache header on the redirect response? That way we can keep the configured caching for the rest of the site. I would make a PR for it, but I'm not well versed in Craft plugin development I'm afraid.

I think something like this would do the trick.

engram-design commented 1 year ago

Yep, that's what I was thinking. To get this early, run composer require verbb/knock-knock:"dev-craft-4 as 2.0.8".

NicolajKN commented 1 year ago

That's awesome! I will try this out as soon as possible.

Thank you very much.

NicolajKN commented 1 year ago

I have now tested this and unfortunately it is not enough to circumvent Chromes caching. The change correctly removes the cache headers, but Chrome (and apparently Safari, according to some reports) still choose to cache the 302 redirect.

I found this answer though, suggesting an explicit no-store cache header. Could this be an approach?

https://stackoverflow.com/a/31550450

engram-design commented 1 year ago

Ah okay, I was hoping that this would be enough (which is also what Craft uses for some similar things)

That does seem to include no-cache, no-store, must-revalidate

engram-design commented 1 year ago

Oh! Actually, I just re-read your original question, in that this is about when being redirected to the login page, not after the authentication, redirecting back to the original URL. Sorry for glossing over that.

I've just added the headers to the original redirect to Knock Knock's challenge page.

NicolajKN commented 1 year ago

I just tested with the newest commit and everything now works as intended. Thank you!

sg-modlab commented 1 year ago

I'm wondering if I am having this issue as well. In Safari, and getting too many redirects when get pushed to the welcome template for knock knock. Tried in private window and same thing. On staging not site not working at all. Not getting redirect so maybe something different.

engram-design commented 1 year ago

Updated in 2.0.9