Item14281: Cookie related changes. Inconsistent use of the domain and secure flags.

In preparing for the new blog.foswiki.org domain, noticed that not all of our cookies are honoring the {CookieRealm} setting. Also, in an https environment, not all cookies are marked secure.

Below patches have all been pushed to the Item14281 branch

Proposed patch:

(This appears incomplete. The FOSWIKIPREFS cookie is still using the default domain, and is not flagged as secure on https sites. The FOSWIKI_UPDATESPLUGIN is also using the wrong domain / security flag. I think that these cookies are set by javascript rather than CGI. Looks like System/JavascriptFiles/foswikiPref.js needs to have access to the cookie domain, and https secure status. It might be able to pick up secure status by examining JQUERYPLUGIN::FOSWIKI::PREFERENCES, but the domain is not provided that I can find. It can't pick up the domain from the session cookie, as that is http only.)

diff --git a/UnitTestContrib/test/unit/RequestTests.pm b/UnitTestContrib/test/unit/RequestTests.pm
index d7309d5..110787a 100644
--- a/UnitTestContrib/test/unit/RequestTests.pm
+++ b/UnitTestContrib/test/unit/RequestTests.pm
@@ -12,6 +12,7 @@ sub set_up {
     my $this = shift;
     $this->SUPER::set_up(@_);
     $Foswiki::cfg{ScriptUrlPath} = '/fatwilly/bin';
+    $Foswiki::cfg{Sessions}{CookieRealm} = 'weebles.wobble';
     delete $Foswiki::cfg{ScriptUrlPaths};
 }
 
@@ -522,11 +523,13 @@ sub test_cookies {
     );
     $result[1] = new CGI::Cookie(
         -name    => 'c3',
+        -domain  => 'weebles.wobble',
         -value   => 'value3',
         -path    => '/test',
         -expires => '1234',
         -secure  => 1
     );
+
     $this->assert_deep_equals( $result[0], $result[1],
         'Wrong returned cookie' );
 }
diff --git a/core/lib/Foswiki/Request.pm b/core/lib/Foswiki/Request.pm
index 91c362e..ad60804 100644
--- a/core/lib/Foswiki/Request.pm
+++ b/core/lib/Foswiki/Request.pm
@@ -521,7 +521,8 @@ sub cookie {
         -value => $value,
         -path  => $path || '/',
         -secure  => $secure  || $this->secure,
-        -expires => $expires || abs( $Foswiki::cfg{Sessions}{ExpireAfter} )
+        -expires => $expires || abs( $Foswiki::cfg{Sessions}{ExpireAfter} ),
+        -domain   => $Foswiki::cfg{Sessions}{CookieRealm} || '',
     );
 }
 
diff --git a/core/lib/Foswiki/Validation.pm b/core/lib/Foswiki/Validation.pm
index c4ae7c4..0a40633 100644
--- a/core/lib/Foswiki/Validation.pm
+++ b/core/lib/Foswiki/Validation.pm
@@ -203,6 +203,8 @@ sub getCookie {
         -value => $secret,
         -path  => '/',
         -httponly => 0,    # we *want* JS to be able to read it!
+        -domain   => $Foswiki::cfg{Sessions}{CookieRealm} || '',
+        -secure   => $Foswiki::Plugins::SESSION->{request}->secure,
     );
 
     return $cookie;

-- GeorgeClark - 18 Jan 2017

The following fixes seem to correct the FOSWIKIPREFS cookie:

diff --git a/JQueryPlugin/lib/Foswiki/Plugins/JQueryPlugin/FOSWIKI.pm b/JQueryPlugin/lib/Foswiki/Plugins/JQueryPlugin/FOSWIKI.pm
index 24e459e..ee15b24 100644
--- a/JQueryPlugin/lib/Foswiki/Plugins/JQueryPlugin/FOSWIKI.pm
+++ b/JQueryPlugin/lib/Foswiki/Plugins/JQueryPlugin/FOSWIKI.pm
@@ -93,6 +93,11 @@ sub init {
     if ( defined $Foswiki::cfg{ScriptUrlPaths} ) {
         %{ $prefs{"SCRIPTURLPATHS"} } = %{ $Foswiki::cfg{ScriptUrlPaths} };
     }
+
+    # add {Sessions}{CookieRealm}
+    if ( defined $Foswiki::cfg{Sessions}{CookieRealm} ) {
+        $prefs{"COOKIEREALM"} = $Foswiki::cfg{Sessions}{CookieRealm};
+    }
     $prefs{"URLHOST"} = Foswiki::Func::getUrlHost();
 
     my $text =
diff --git a/PatternSkin/pub/System/JavascriptFiles/foswikiPref_src.js b/PatternSkin/pub/System/JavascriptFiles/foswikiPref_src.js
index 873797c..29ff821 100644
--- a/PatternSkin/pub/System/JavascriptFiles/foswikiPref_src.js
+++ b/PatternSkin/pub/System/JavascriptFiles/foswikiPref_src.js
@@ -260,12 +260,14 @@ foswiki.Pref = {
                var cookieString = (inValues != null)
         ? inValues.join(foswiki.Pref.COOKIE_PREF_SEPARATOR) : '';
                var expiryDate = new Date ();
+        var cookieDomain = foswiki.getPreference('COOKIEREALM');
+        var cookieSecure = foswiki.getPreference('URLHOST').startsWith("https://");
         // Correct for Mac date bug - call only once for given Date object!
                foswiki.Pref._fixCookieDate (expiryDate);
                expiryDate.setTime (expiryDate.getTime()
                             + foswiki.Pref.COOKIE_EXPIRY_TIME);
                foswiki.Pref.setCookie(foswiki.Pref.FOSWIKI_PREF_COOKIE_NAME,
-                               cookieString, expiryDate, '/');
+                               cookieString, expiryDate, '/', cookieDomain, cookieSecure);
        },
        
        /**

And one more patch against UpdatesPlugin. Also as this is "javascript by george..." it needs careful review.

diff --git a/UpdatesPlugin/pub/System/UpdatesPlugin/jquery.updates.uncompressed.js b/UpdatesPlugin/pub/System/UpdatesPlugin/jquery.updates.uncompressed.js
index 90fe375..3576ee4 100644
--- a/UpdatesPlugin/pub/System/UpdatesPlugin/jquery.updates.uncompressed.js
+++ b/UpdatesPlugin/pub/System/UpdatesPlugin/jquery.updates.uncompressed.js
@@ -16,7 +16,10 @@
     delay: 1000, // number of seconds to delay contacting f.o.
     timeout: 5000, // number of seconds a jsonp call is considered failure
     cookieName: "FOSWIKI_UPDATESPLUGIN", // name of the cookie
-    cookieExpires: 7 // number of days the cookie takes to expire
+    cookieExpires: 7, // number of days the cookie takes to expire
+    cookieSecure: '0', // If secure cookies are needed (https)
+    cookieDomain: ''   // Override domain if requested.
+
   }, foswikiUpdates; // singleton
 
   // class constructor
@@ -47,6 +50,9 @@
       self.options.endpointUrl = foswiki.getScriptUrl("rest", "UpdatesPlugin", "check");
     }
 
+    self.options.cookieDomain = foswiki.getPreference('COOKIEREALM'); // Allow domain override
+    self.options.cookieSecure = (foswiki.getPreference('URLHOST').startsWith('https://')) ? '1' : '0';
+
     // events
     $(document).bind("refresh.foswikiUpdates", function() {
       //console.log("BIND refresh.foswikiUpdates calling loadPluginInfo.");
@@ -55,7 +61,12 @@
 
     $(document).bind("forceRefresh.foswikiUpdates", function() {
       //console.log("BIND forceRefresh.foswikiUpdates calling loadPluginInfo.");
-      $.cookie(self.options.cookieName, null, {expires: -1, path:'/'});
+      $.cookie(self.options.cookieName, null, {
+        expires: -1,
+        path:'/',
+        domain:self.options.cookieDomain,
+        secure:self.options.cookieSecure
+      });
       self.loadPluginInfo(1);
     });
 
@@ -69,7 +80,9 @@
       //console.log("BIND click entered ");
       $.cookie(self.options.cookieName, 0, {
         expires: self.options.cookieExpires, 
-        path: "/"
+        path: "/",
+        domain:self.options.cookieDomain,
+        secure:self.options.cookieSecure
       });
       $(".foswikiUpdatesMessage").fadeOut();
       return false;
@@ -100,7 +113,9 @@
             // zero explicitly can either mean: everything up-to-date or ignore pending updates
             $.cookie(self.options.cookieName, self.numberOutdatedPlugins, {
               expires: self.options.cookieExpires, 
-              path: "/"
+              path: "/",
+              domain:self.options.cookieDomain,
+              secure:self.options.cookieSecure
             });
 
             //console.log("Forced: " + forced);
@@ -113,7 +128,9 @@
             // remember the error state
             $.cookie(self.options.cookieName, -1, {
               expires: self.options.cookieExpires, 
-              path: "/"
+              path: "/",
+              domain:self.options.cookieDomain,
+              secure:self.options.cookieSecure
             });
           }
         });

-- GeorgeClark - 19 Jan 2017

Checking JQueryCookie hosted upstream at https://github.com/carhartl/jquery-cookie. This lib needs to be replaced by https://github.com/js-cookie/js-cookie. Not a task for this bug item. Just mentioning as I checked the code.

-- MichaelDaum - 19 Jan 2017

I agree with your change for the FOSWIKIPREFS cookie - I don't think we ever gave security of that cookie much thought before. I can't comment on the updates one.

-- Main.CrawfordCurrie - 21 Jan 2017 - 08:56

Commenting is disabled while not logged in