Bug: Login Nonce fails for pages "cached" by bfcache

[renatonascalves]

Description of the bug

The Login Nonce feature introduces a bug for already active sites. To be fair, the problem is not with the Login Nonce per se but how it integrates with the bfcache-supported browsers.

Due to how browsers that support bfcache creates the snapshot of the login page, the addition of the Login Nonce feature creates a visible regression for the end user.

Steps To Reproduce

Before the login nonce feature is active. The login page has no nonce. The site also has no Cache-Control: no-store flag (recommended by Google).

• The user tries to log in;
• The page is "cached" by bfcache;
• The user logs in;
• The login nonce feature is activated;
• The user tries to login;
• It loads the "cached" page by bfcache;
• The plugin checks for the nonce;
• The check fails since it doesn’t exist.

Additional Information

See: https://web.dev/articles/bfcache For Alley devs, see https://l.alley.dev/5114f91814

[renatonascalves]

The current solution, workaround, is to enable the Cache-Control: no-store header before activating the Login Nonce feature. So that the login page is not "cached"/snapshotted by the browser.

[anubisthejackle]

While the Cache-Control: no-store solution works currently, that isn't a long term solution. Google are already looking at ways to ignore this header for bfcache.

Reading over this, and learning more about what bfcache actually is, I'm starting to think this isn't actually a problem worth solving. Specifically, this cache is intended to cache full page versions for use during a session. I.E. When a browser is closed the bfcache is cleared, so the only people who would be getting this bug are people who are attempting to login during the same session as the deploy.

On the broader topic of Cache-Control: no-store, I think that's a good thing to enable for the login page, despite the bfcache.

[renatonascalves]

When a browser is closed the bfcache is cleared, so the only people who would be getting this bug are people who are attempting to login during the same session as the deploy.

@anubisthejackle I think this is where the problem surfaced. A lot of the people who use the admin page, editors, don't close the browser.

The sad thing about this is that one can't resolve it for active sites. So if someone activates Alleyvate in a client, already active, site, the chance of running into this is high.

[renatonascalves]

Also, one doesn't need to be attempting to login during the same session as the deploy. They only need to have the login page bfcached. See the steps to reproduce the issue. It is an order of operation kind of problem. Adding the feature after the page is cached causes the issue.

Which is the case for active sites, instead of brand new ones.

[renatonascalves]

I'm reopening this issue since the lastest fix doesn't address the actual issue.

Google are already looking at ways to ignore this header for bfcache

Once this is fixed by Google or another solution is added, we can properly resolve it.

[james-tyner]

I'll add that at SF Standard, users in our newsroom are regularly getting this "Login attempt failed" issue and it's causing some confusion. So anything that can be done to make this experience more predictable and more readily understood by users would be beneficial, even if it's just better error messaging.

[anubisthejackle]

It looks like we might be able to do something client-side. There seems to be a pageshow event that we could listen to:

window.addEventListener('pageshow', (event) => {
  if (event.persisted) {
    console.log('This page was restored from the bfcache.');
  } else {
    console.log('This page was loaded normally.');
  }
});

If event.persisted we know that the page was loaded from bfcache. It looks like a reload of the page should fix the issue. Here is an example from https://web.dev/articles/bfcache

window.addEventListener('pageshow', (event) => {
  if (event.persisted && !document.cookie.match(/my-cookie/)) {
    // Force a reload if the user has logged out.
    location.reload();
  }
});

[renatonascalves]

I thought of that at first, but see this issue: https://github.com/alleyinteractive/elasticsearch-extensions/issues/52

I don't think this is something that ES Extensions can (or should) address, mainly because this plugin doesn't provide any client-side JS

The same issue we have there we do here. We don't provide any client-site JS by default. We can solve for the project manually. Or start bundling js client site.

Related.

[anubisthejackle]

I think this is slightly different in that we're already adding a meta refresh to the page for reloading in one case. This would just be an expansion of that for this additional case, and converting it to in-line JS. Its also limited to the login page.

[renatonascalves]

The other use case would be too, since it was related to a search page only. My main point here is that up until now, we haven't bundled client-side js, and some folks might question if we should.

But this is the only fix, for now.

[anubisthejackle]

Its probably worth a broader discussion, for sure.