Add FixMyTransport's strip_empty_sessions to not send a cookie if there's nothing in the session.

4 files changed, 101 insertions, 9 deletions

diff --git a/config/environment.rb b/config/environment.rb
index ec6a4096f..a40c2df4e 100644
--- a/config/environment.rb
+++ b/config/environment.rb
@@ -61,15 +61,6 @@ Rails::Initializer.run do |config|
   config.gem 'routing-filter'
   config.gem 'will_paginate', :version => '~> 2.3.11', :source => 'http://gemcutter.org'
   #GettextI18nRails.translations_are_html_safe = true
-  # Your secret key for verifying cookie session data integrity.
-  # If you change this key, all old sessions will become invalid!
-  # Make sure the secret is at least 30 characters and all random, 
-  # no regular words or you'll be exposed to dictionary attacks.
-  config.action_controller.session = {
-    :key => '_wdtk_cookie_session',
-    :secret => MySociety::Config.get("COOKIE_STORE_SESSION_SECRET", 'this default is insecure as code is open source, please override for live sites in config/general; this will do for local development')
-  }
-  config.action_controller.session_store = :cookie_store
 
   # Use SQL instead of Active Record's schema dumper when creating the test database.
   # This is necessary if your schema can't be completely dumped by the schema dumper, 
diff --git a/config/initializers/session_store.rb b/config/initializers/session_store.rb
new file mode 100644
index 000000000..9ef2dddc1
--- /dev/null
+++ b/config/initializers/session_store.rb
@@ -0,0 +1,17 @@
+# Be sure to restart your server when you modify this file.
+
+# Your secret key for verifying cookie session data integrity.
+# If you change this key, all old sessions will become invalid!
+# Make sure the secret is at least 30 characters and all random, 
+# no regular words or you'll be exposed to dictionary attacks.
+
+ActionController::Base.session = {
+  :key => '_wdtk_cookie_session',
+  :secret => MySociety::Config.get("COOKIE_STORE_SESSION_SECRET", 'this default is insecure as code is open source, please override for live sites in config/general; this will do for local development')
+}
+ActionController::Base.session_store = :cookie_store
+
+# Insert a bit of middleware code to prevent uneeded cookie setting.
+require "#{RAILS_ROOT}/lib/whatdotheyknow/strip_empty_sessions"
+ActionController::Dispatcher.middleware.insert_before ActionController::Base.session_store, WhatDoTheyKnow::StripEmptySessions, :key => '_wdtk_cookie_session', :path => "/", :httponly => true
+
diff --git a/lib/whatdotheyknow/strip_empty_sessions.rb b/lib/whatdotheyknow/strip_empty_sessions.rb
new file mode 100644
index 000000000..9c87a4bbc
--- /dev/null
+++ b/lib/whatdotheyknow/strip_empty_sessions.rb
@@ -0,0 +1,29 @@
+module WhatDoTheyKnow
+  
+  class StripEmptySessions
+    ENV_SESSION_KEY = "rack.session".freeze
+    HTTP_SET_COOKIE = "Set-Cookie".freeze
+    STRIPPABLE_KEYS = [:session_id, :_csrf_token]
+
+    def initialize(app, options = {})
+      @app = app
+      @options = options
+    end
+
+    def call(env)
+      status, headers, body = @app.call(env)
+      session_data = env[ENV_SESSION_KEY]
+      set_cookie = headers[HTTP_SET_COOKIE]
+      if session_data
+        if (session_data.keys - STRIPPABLE_KEYS).empty?
+          if set_cookie.is_a? Array
+            set_cookie.reject! {|c| c.match(/^\n?#{@options[:key]}=/)}
+          elsif set_cookie.is_a? String
+            headers[HTTP_SET_COOKIE].gsub!( /(^|\n)#{@options[:key]}=.*?(\n|$)/, "" )
+          end
+        end
+      end
+      [status, headers, body]
+    end
+  end
+end
diff --git a/spec/lib/whatdotheyknow/strip_empty_sessions_spec.rb b/spec/lib/whatdotheyknow/strip_empty_sessions_spec.rb
new file mode 100644
index 000000000..cbe1feea6
--- /dev/null
+++ b/spec/lib/whatdotheyknow/strip_empty_sessions_spec.rb
@@ -0,0 +1,55 @@
+require 'spec_helper'
+describe WhatDoTheyKnow::StripEmptySessions do
+  
+  def make_response(session_data, response_headers)
+    app = lambda do |env|
+      env['rack.session'] = session_data
+      return [200, response_headers, ['content']] 
+    end
+    strip_empty_sessions = WhatDoTheyKnow::StripEmptySessions
+    app = strip_empty_sessions.new(app, {:key => 'mykey', :path => '', :httponly => true})
+    response = Rack::MockRequest.new(app).get('/', 'HTTP_ACCEPT' => 'text/html')
+  end
+
+  
+  it 'should not prevent a cookie being set if there is data in the session' do 
+    session_data = { :some_real_data => 'important', 
+                     :session_id => 'my_session_id', 
+                     :_csrf_token => 'hi_there' }
+    application_response_headers = { 'Content-Type' => 'text/html', 
+                                     'Set-Cookie' => 'mykey=f274c61a35320c52d45e9f8d7d4e2649; path=/; HttpOnly'}
+    response = make_response(session_data, application_response_headers)
+    response.headers['Set-Cookie'].should == 'mykey=f274c61a35320c52d45e9f8d7d4e2649; path=/; HttpOnly'
+  end
+
+  describe 'if there is no meaningful data in the session' do 
+
+    before do 
+      @session_data = { :session_id => 'my_session_id', 
+                       :_csrf_token => 'hi_there' }
+    end
+    
+    it 'should not strip any other header' do 
+      application_response_headers = { 'Content-Type' => 'text/html',
+                                       'Set-Cookie' => 'mykey=f274c61a35320c52d45e9f8d7d4e2649; path=/; HttpOnly'}
+      response = make_response(@session_data, application_response_headers)
+      response.headers['Content-Type'].should == 'text/html'
+    end
+    
+    it 'should strip the session cookie setting header ' do 
+      application_response_headers = { 'Content-Type' => 'text/html', 
+                                       'Set-Cookie' => 'mykey=f274c61a35320c52d45e9f8d7d4e2649; path=/; HttpOnly'}
+      response = make_response(@session_data, application_response_headers)
+      response.headers['Set-Cookie'].should == ""
+    end
+  
+    it 'should strip the session cookie setting header (but no other cookie setting header) if there is more than one' do 
+      application_response_headers = { 'Content-Type' => 'text/html', 
+                                       'Set-Cookie' => ['mykey=f274c61a35320c52d45e9f8d7d4e2649; path=/; HttpOnly',
+                                                        'other=mydata']}
+      response = make_response(@session_data, application_response_headers)
+      response.headers['Set-Cookie'].should == ['other=mydata']
+    end
+    
+  end
+end