Page MenuHomePhabricator

Long-lived Conpherence tabs occasionally cause "invalid CSRF token" errors
Open, Needs TriagePublic

Description

Especially after my laptop goes to sleep (or I close the lid), if I attempt to send a message in a Conpherence tab that I've left open, I'll get an exception about having an invalid CSRF token and a suggestion to clear my cookies. Refreshing the page solves the problem. I also permanently lose whatever message I was composing (probably unavoidable unless we store them client-side).

Event Timeline

I believe that something in this vein is likely the fix:

diff --git a/webroot/rsrc/js/core/behavior-refresh-csrf.js b/webroot/rsrc/js/core/behavior-refresh-csrf.js
index 27c7a9d8ee..b21d2804f3 100644
--- a/webroot/rsrc/js/core/behavior-refresh-csrf.js
+++ b/webroot/rsrc/js/core/behavior-refresh-csrf.js
@@ -48,6 +48,28 @@ JX.behavior('refresh-csrf', function(config) {
   // Refresh the CSRF tokens every 55 minutes.
   setInterval(refresh_csrf, 1000 * 60 * 55);
 
+
+  // If the user walks away from their machine for a long time and it goes to
+  // sleep, we may end up in a situation where many hours have elapsed but
+  // the main refresh callback hasn't been invoked. Once a second, check if
+  // we lost some time and refresh immediately if we did.
+  var last_update = null;
+  var detect_sleep = function() {
+    var now = new Date().getTime();
+    if (last_update) {
+      // If it has been more than a minute since the last update, we lost
+      // some time, probably because the machine went to sleep. Fire the
+      // CSRF update event immediately.
+      var delta = (now - last_update);
+      if (delta > (1000 * 60)) {
+        refresh_csrf();
+      }
+    }
+    last_update = now;
+  };
+  setInterval(every_second, 1000);
+
+
   // Additionally, add the CSRF token as an HTTP header to every AJAX request.
   JX.Request.listen('open', function(r) {
     var via = JX.$U(window.location).getRelativeURI();

One way to test this would be:

  • Add a "console.log()" or "JX.log()" to refresh_csrf().
  • Close your laptop.
  • Wait a couple minutes.
  • Open it back up.
  • See if refresh_csrf() fires immediately.

I can turn that into a real revision at some point if no one gets there first, but testing it convincingly requires a bunch of sitting around with a closed laptop in different browsers so I'm not going to dive into it right now.

This comment was removed by richardvanvelzen.

This will also fix the important use case where someone is running their browser inside a VM that they frequently suspend.

We could also rip out all CSRF cycling code completely; Rails uses infinite-duration tokens with no cycling. However, the current code hasn't really caused any issues other than this for a number of years, I believe, and there are some practical-if-obscure attacks this can partially mitigate. An example is:

  • You take a screenshot of a page with the web inspector open to report a bug.
  • You don't notice or censor the CSRF token from the document.
  • With cycling: you're only vulnerable to CSRF attacks for the next 7 hours.
  • Without cycling: you're vulnerable to CSRF attacks for a much longer time / forever.

I have an extra laptop I can use to test this.