From: Tom Hughes <tom@compton.nu>
Date: Sun, 17 Nov 2013 21:52:04 +0000 (+0000)
Subject: Remove the _osm_username cookie and session validation logic
X-Git-Tag: live~4664
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/41e45bad51c90dd4e131068ba1cfc1dd1184155f

Remove the _osm_username cookie and session validation logic

This was a temporary hack to workaround issues with sessions getting
mixed up at the time of the rails 3.1 upgrade, but logs indicate that
whatever the original problem was it is no longer occurring.
---

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 12cdb15d4..fec202ca5 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -9,11 +9,7 @@ class ApplicationController < ActionController::Base
     if session[:user]
       @user = User.where(:id => session[:user]).where("status IN ('active', 'confirmed', 'suspended')").first
 
-      if @user.display_name != cookies["_osm_username"]
-        logger.info "Session user '#{@user.display_name}' does not match cookie user '#{cookies['_osm_username']}'"
-        reset_session
-        @user = nil
-      elsif @user.status == "suspended"
+    if @user.status == "suspended"
         session.delete(:user)
         session_expires_automatically
 
diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb
index a9006e82b..99777ca2d 100644
--- a/app/controllers/user_controller.rb
+++ b/app/controllers/user_controller.rb
@@ -7,6 +7,7 @@ class UserController < ApplicationController
   before_filter :authorize_web, :except => [:api_read, :api_details, :api_gpx_files]
   before_filter :set_locale, :except => [:api_read, :api_details, :api_gpx_files]
   before_filter :require_user, :only => [:account, :go_public, :make_friend, :remove_friend]
+  before_filter :require_self, :only => [:account]
   before_filter :check_database_readable, :except => [:login, :api_read, :api_details, :api_gpx_files]
   before_filter :check_database_writable, :only => [:new, :account, :confirm, :confirm_email, :lost_password, :reset_password, :go_public, :make_friend, :remove_friend]
   before_filter :check_api_readable, :only => [:api_read, :api_details, :api_gpx_files]
@@ -338,7 +339,6 @@ class UserController < ApplicationController
           token.destroy
 
           session[:user] = user.id
-          cookies.permanent["_osm_username"] = user.display_name
 
           redirect_to referer || welcome_path
         end
@@ -377,7 +377,6 @@ class UserController < ApplicationController
         end
         token.destroy
         session[:user] = @user.id
-        cookies.permanent["_osm_username"] = @user.display_name
         redirect_to :action => 'account', :display_name => @user.display_name
       else
         flash[:error] = t 'user.confirm_email.failure'
@@ -638,8 +637,6 @@ private
   ##
   # process a successful login
   def successful_login(user)
-    cookies.permanent["_osm_username"] = user.display_name
-
     session[:user] = user.id
     session_expires_after 28.days if session[:remember_me]
 
@@ -727,8 +724,6 @@ private
     if user.save
       set_locale
 
-      cookies.permanent["_osm_username"] = user.display_name
-
       if user.new_email.blank? or user.new_email == user.email
         flash.now[:notice] = t 'user.account.flash update success'
       else
@@ -769,6 +764,14 @@ private
     end
   end
 
+  ##
+  # require that the user in the URL is the logged in user
+  def require_self
+    if params[:display_name] != @user.display_name
+      render :text => "", :status => :forbidden
+    end
+  end
+
   ##
   # ensure that there is a "this_user" instance variable
   def lookup_user_by_id
diff --git a/test/functional/diary_entry_controller_test.rb b/test/functional/diary_entry_controller_test.rb
index 7e0f10790..8725def3d 100644
--- a/test/functional/diary_entry_controller_test.rb
+++ b/test/functional/diary_entry_controller_test.rb
@@ -86,8 +86,6 @@ class DiaryEntryControllerTest < ActionController::TestCase
   end
 
   def test_showing_new_diary_entry
-    @request.cookies["_osm_username"] = users(:normal_user).display_name
-
     get :new
     assert_response :redirect
     assert_redirected_to :controller => :user, :action => "login", :referer => "/diary/new"
@@ -125,7 +123,6 @@ class DiaryEntryControllerTest < ActionController::TestCase
   end
   
   def test_editing_diary_entry
-    @request.cookies["_osm_username"] = users(:normal_user).display_name
     entry = diary_entries(:normal_user_entry_1)
 
     # Make sure that you are redirected to the login page when you are 
@@ -217,8 +214,6 @@ class DiaryEntryControllerTest < ActionController::TestCase
       end
     end
 
-    @request.cookies["_osm_username"] = users(:public_user).display_name
-
     # and when not logged in as the user who wrote the entry
     get :view, {:display_name => entry.user.display_name, :id => entry.id}, {'user' => entry.user.id}
     assert_response :success
@@ -251,16 +246,12 @@ class DiaryEntryControllerTest < ActionController::TestCase
   end
   
   def test_edit_diary_entry_i18n
-    @request.cookies["_osm_username"] = users(:normal_user).display_name
-
     get :edit, {:display_name => users(:normal_user).display_name, :id => diary_entries(:normal_user_entry_1).id}, {'user' => users(:normal_user).id}
     assert_response :success
     assert_select "span[class=translation_missing]", false, "Missing translation in edit diary entry"
   end
   
   def test_create_diary_entry
-    @request.cookies["_osm_username"] = users(:normal_user).display_name
-
     # Make sure that you are redirected to the login page when you
     # are not logged in
     get :new
@@ -320,7 +311,6 @@ class DiaryEntryControllerTest < ActionController::TestCase
   end
   
   def test_creating_diary_comment
-    @request.cookies["_osm_username"] = users(:public_user).display_name
     entry = diary_entries(:normal_user_entry_1)
 
     # Make sure that you are denied when you are not logged in
@@ -472,16 +462,12 @@ class DiaryEntryControllerTest < ActionController::TestCase
     assert_response :forbidden
     assert_equal true, DiaryEntry.find(diary_entries(:normal_user_entry_1).id).visible
 
-    @request.cookies["_osm_username"] = users(:normal_user).display_name
-
     # Now try as a normal user
     post :hide, {:display_name => users(:normal_user).display_name, :id => diary_entries(:normal_user_entry_1).id}, {:user => users(:normal_user).id}
     assert_response :redirect
     assert_redirected_to :action => :view, :display_name => users(:normal_user).display_name, :id => diary_entries(:normal_user_entry_1).id
     assert_equal true, DiaryEntry.find(diary_entries(:normal_user_entry_1).id).visible
 
-    @request.cookies["_osm_username"] = users(:administrator_user).display_name
-
     # Finally try as an administrator
     post :hide, {:display_name => users(:normal_user).display_name, :id => diary_entries(:normal_user_entry_1).id}, {:user => users(:administrator_user).id}
     assert_response :redirect
@@ -495,16 +481,12 @@ class DiaryEntryControllerTest < ActionController::TestCase
     assert_response :forbidden
     assert_equal true, DiaryComment.find(diary_comments(:comment_for_geo_post).id).visible
 
-    @request.cookies["_osm_username"] = users(:normal_user).display_name
-
     # Now try as a normal user
     post :hidecomment, {:display_name => users(:normal_user).display_name, :id => diary_entries(:normal_user_geo_entry).id, :comment => diary_comments(:comment_for_geo_post).id}, {:user => users(:normal_user).id}
     assert_response :redirect
     assert_redirected_to :action => :view, :display_name => users(:normal_user).display_name, :id => diary_entries(:normal_user_geo_entry).id
     assert_equal true, DiaryComment.find(diary_comments(:comment_for_geo_post).id).visible
 
-    @request.cookies["_osm_username"] = users(:administrator_user).display_name
-
     # Finally try as an administrator
     post :hidecomment, {:display_name => users(:normal_user).display_name, :id => diary_entries(:normal_user_geo_entry).id, :comment => diary_comments(:comment_for_geo_post).id}, {:user => users(:administrator_user).id}
     assert_response :redirect
diff --git a/test/functional/message_controller_test.rb b/test/functional/message_controller_test.rb
index f015b1ec3..77fdfbeb9 100644
--- a/test/functional/message_controller_test.rb
+++ b/test/functional/message_controller_test.rb
@@ -53,7 +53,6 @@ class MessageControllerTest < ActionController::TestCase
 
     # Login as a normal user
     session[:user] = users(:normal_user).id
-    cookies["_osm_username"] = users(:normal_user).display_name
 
     # Check that the new message page loads
     get :new, :display_name => users(:public_user).display_name
@@ -106,7 +105,6 @@ class MessageControllerTest < ActionController::TestCase
 
     # Login as the wrong user
     session[:user] = users(:second_public_user).id
-    cookies["_osm_username"] = users(:second_public_user).display_name
 
     # Check that we can't reply to somebody else's message
     get :reply, :message_id => messages(:unread_message).id
@@ -115,7 +113,6 @@ class MessageControllerTest < ActionController::TestCase
 
     # Login as the right user
     session[:user] = users(:public_user).id
-    cookies["_osm_username"] = users(:public_user).display_name
 
     # Check that the message reply page loads
     get :reply, :message_id => messages(:unread_message).id
@@ -149,7 +146,6 @@ class MessageControllerTest < ActionController::TestCase
 
     # Login as the wrong user
     session[:user] = users(:second_public_user).id
-    cookies["_osm_username"] = users(:second_public_user).display_name
 
     # Check that we can't read the message
     get :read, :message_id => messages(:unread_message).id
@@ -158,7 +154,6 @@ class MessageControllerTest < ActionController::TestCase
 
     # Login as the message sender
     session[:user] = users(:normal_user).id
-    cookies["_osm_username"] = users(:normal_user).display_name
 
     # Check that the message sender can read the message
     get :read, :message_id => messages(:unread_message).id
@@ -168,7 +163,6 @@ class MessageControllerTest < ActionController::TestCase
 
     # Login as the message recipient
     session[:user] = users(:public_user).id
-    cookies["_osm_username"] = users(:public_user).display_name
 
     # Check that the message recipient can read the message
     get :read, :message_id => messages(:unread_message).id
@@ -196,7 +190,6 @@ class MessageControllerTest < ActionController::TestCase
 
     # Login
     session[:user] = users(:normal_user).id
-    cookies["_osm_username"] = users(:normal_user).display_name
 
     # Check that we can view our inbox when logged in
     get :inbox, :display_name => users(:normal_user).display_name
@@ -221,7 +214,6 @@ class MessageControllerTest < ActionController::TestCase
 
     # Login
     session[:user] = users(:normal_user).id
-    cookies["_osm_username"] = users(:normal_user).display_name
 
     # Check that we can view our outbox when logged in
     get :outbox, :display_name => users(:normal_user).display_name
@@ -246,7 +238,6 @@ class MessageControllerTest < ActionController::TestCase
 
     # Login as a user with no messages
     session[:user] = users(:second_public_user).id
-    cookies["_osm_username"] = users(:second_public_user).display_name
 
     # Check that marking a message we didn't send or receive fails
     post :mark, :message_id => messages(:read_message).id
@@ -255,7 +246,6 @@ class MessageControllerTest < ActionController::TestCase
 
     # Login as the message recipient
     session[:user] = users(:public_user).id
-    cookies["_osm_username"] = users(:public_user).display_name
 
     # Check that the marking a message read works
     post :mark, :message_id => messages(:unread_message).id, :mark => "read"
@@ -299,7 +289,6 @@ class MessageControllerTest < ActionController::TestCase
 
     # Login as a user with no messages
     session[:user] = users(:second_public_user).id
-    cookies["_osm_username"] = users(:second_public_user).display_name
 
     # Check that deleting a message we didn't send or receive fails
     post :delete, :message_id => messages(:read_message).id
@@ -308,7 +297,6 @@ class MessageControllerTest < ActionController::TestCase
 
     # Login as the message recipient
     session[:user] = users(:normal_user).id
-    cookies["_osm_username"] = users(:normal_user).display_name
 
     # Check that the deleting a received message works
     post :delete, :message_id => messages(:read_message).id
diff --git a/test/functional/redactions_controller_test.rb b/test/functional/redactions_controller_test.rb
index a2bd75b10..83bb3adc2 100644
--- a/test/functional/redactions_controller_test.rb
+++ b/test/functional/redactions_controller_test.rb
@@ -39,7 +39,6 @@ class RedactionsControllerTest < ActionController::TestCase
 
   def test_moderators_can_create
     session[:user] = users(:moderator_user).id
-    cookies["_osm_username"] = users(:moderator_user).display_name
 
     post :create, :redaction => { :title => "Foo", :description => "Description here." }
     assert_response :redirect
@@ -48,7 +47,6 @@ class RedactionsControllerTest < ActionController::TestCase
 
   def test_non_moderators_cant_create
     session[:user] = users(:public_user).id
-    cookies["_osm_username"] = users(:public_user).display_name
 
     post :create, :redaction => { :title => "Foo", :description => "Description here." }
     assert_response :forbidden
@@ -56,7 +54,6 @@ class RedactionsControllerTest < ActionController::TestCase
 
   def test_moderators_can_delete_empty
     session[:user] = users(:moderator_user).id
-    cookies["_osm_username"] = users(:moderator_user).display_name
 
     # remove all elements from the redaction
     redaction = redactions(:example)
@@ -71,7 +68,6 @@ class RedactionsControllerTest < ActionController::TestCase
 
   def test_moderators_cant_delete_nonempty
     session[:user] = users(:moderator_user).id
-    cookies["_osm_username"] = users(:moderator_user).display_name
 
     # leave elements in the redaction
     redaction = redactions(:example)
@@ -84,7 +80,6 @@ class RedactionsControllerTest < ActionController::TestCase
 
   def test_non_moderators_cant_delete
     session[:user] = users(:public_user).id
-    cookies["_osm_username"] = users(:public_user).display_name
 
     delete :destroy, :id => redactions(:example).id
     assert_response :forbidden
@@ -92,7 +87,6 @@ class RedactionsControllerTest < ActionController::TestCase
 
   def test_moderators_can_edit
     session[:user] = users(:moderator_user).id
-    cookies["_osm_username"] = users(:moderator_user).display_name
 
     get :edit, :id => redactions(:example).id
     assert_response :success
@@ -100,7 +94,6 @@ class RedactionsControllerTest < ActionController::TestCase
 
   def test_non_moderators_cant_edit
     session[:user] = users(:public_user).id
-    cookies["_osm_username"] = users(:public_user).display_name
 
     get :edit, :id => redactions(:example).id
     assert_response :redirect
@@ -109,7 +102,6 @@ class RedactionsControllerTest < ActionController::TestCase
 
   def test_moderators_can_update
     session[:user] = users(:moderator_user).id
-    cookies["_osm_username"] = users(:moderator_user).display_name
 
     redaction = redactions(:example)
 
@@ -120,7 +112,6 @@ class RedactionsControllerTest < ActionController::TestCase
 
   def test_non_moderators_cant_update
     session[:user] = users(:public_user).id
-    cookies["_osm_username"] = users(:public_user).display_name
 
     redaction = redactions(:example)
 
diff --git a/test/functional/site_controller_test.rb b/test/functional/site_controller_test.rb
index 00a184ba8..2db756ad1 100644
--- a/test/functional/site_controller_test.rb
+++ b/test/functional/site_controller_test.rb
@@ -132,8 +132,6 @@ class SiteControllerTest < ActionController::TestCase
 
   # test the right editor gets used when the user hasn't set a preference
   def test_edit_without_preference
-    @request.cookies["_osm_username"] = users(:public_user).display_name
-
     get(:edit, nil, { 'user' => users(:public_user).id })
     assert_response :success
     assert_template :partial => "_#{DEFAULT_EDITOR}", :count => 1
@@ -141,8 +139,6 @@ class SiteControllerTest < ActionController::TestCase
 
   # and when they have...
   def test_edit_with_preference
-    @request.cookies["_osm_username"] = users(:public_user).display_name
-
     user = users(:public_user)
     user.preferred_editor = "potlatch"
     user.save!
@@ -161,8 +157,6 @@ class SiteControllerTest < ActionController::TestCase
   end
 
   def test_edit_with_node
-    @request.cookies["_osm_username"] = users(:public_user).display_name
-
     user = users(:public_user)
     node = current_nodes(:visible_node)
 
@@ -172,8 +166,6 @@ class SiteControllerTest < ActionController::TestCase
   end
 
   def test_edit_with_way
-    @request.cookies["_osm_username"] = users(:public_user).display_name
-
     user = users(:public_user)
     way  = current_ways(:visible_way)
 
@@ -183,8 +175,6 @@ class SiteControllerTest < ActionController::TestCase
   end
 
   def test_edit_with_gpx
-    @request.cookies["_osm_username"] = users(:public_user).display_name
-
     user = users(:public_user)
     gpx  = gpx_files(:public_trace_file)
 
diff --git a/test/functional/trace_controller_test.rb b/test/functional/trace_controller_test.rb
index 57d6f4916..1a074082b 100644
--- a/test/functional/trace_controller_test.rb
+++ b/test/functional/trace_controller_test.rb
@@ -171,8 +171,6 @@ class TraceControllerTest < ActionController::TestCase
 
   # Check that I can get mine
   def test_list_mine
-    @request.cookies["_osm_username"] = users(:public_user).display_name
-
     # First try to get it when not logged in
     get :mine
     assert_redirected_to :controller => 'user', :action => 'login', :referer => '/traces/mine'
@@ -196,14 +194,10 @@ class TraceControllerTest < ActionController::TestCase
     get :list, :display_name => users(:public_user).display_name
     check_trace_list users(:public_user).traces.public
 
-    @request.cookies["_osm_username"] = users(:normal_user).display_name
-
     # Should still see only public ones when authenticated as another user
     get :list, {:display_name => users(:public_user).display_name}, {:user => users(:normal_user).id}
     check_trace_list users(:public_user).traces.public
 
-    @request.cookies["_osm_username"] = users(:public_user).display_name
-
     # Should see all traces when authenticated as the target user
     get :list, {:display_name => users(:public_user).display_name}, {:user => users(:public_user).id}
     check_trace_list users(:public_user).traces
@@ -234,14 +228,10 @@ class TraceControllerTest < ActionController::TestCase
     get :view, {:display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id}
     check_trace_view gpx_files(:public_trace_file)
 
-    @request.cookies["_osm_username"] = users(:public_user).display_name
-
     # Now with some other user, which should work since the trace is public
     get :view, {:display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id}, {:user => users(:public_user).id}
     check_trace_view gpx_files(:public_trace_file)
 
-    @request.cookies["_osm_username"] = users(:normal_user).display_name
-
     # And finally we should be able to do it with the owner of the trace
     get :view, {:display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id}, {:user => users(:normal_user).id}
     check_trace_view gpx_files(:public_trace_file)
@@ -254,15 +244,11 @@ class TraceControllerTest < ActionController::TestCase
     assert_response :redirect
     assert_redirected_to :action => :list
 
-    @request.cookies["_osm_username"] = users(:normal_user).display_name
-
     # Now with some other user, which should work since the trace is anon
     get :view, {:display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id}, {:user => users(:normal_user).id}
     assert_response :redirect
     assert_redirected_to :action => :list
 
-    @request.cookies["_osm_username"] = users(:public_user).display_name
-
     # And finally we should be able to do it with the owner of the trace
     get :view, {:display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id}, {:user => users(:public_user).id}
     check_trace_view gpx_files(:anon_trace_file)
@@ -275,8 +261,6 @@ class TraceControllerTest < ActionController::TestCase
     assert_response :redirect
     assert_redirected_to :action => :list
 
-    @request.cookies["_osm_username"] = users(:public_user).display_name
-
     # Now with some other user, which should work since the trace is public
     get :view, {:display_name => users(:public_user).display_name, :id => 0}, {:user => users(:public_user).id}
     assert_response :redirect
@@ -294,14 +278,10 @@ class TraceControllerTest < ActionController::TestCase
     get :data, {:display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id}
     check_trace_data gpx_files(:public_trace_file)
 
-    @request.cookies["_osm_username"] = users(:public_user).display_name
-
     # Now with some other user, which should work since the trace is public
     get :data, {:display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id}, {:user => users(:public_user).id}
     check_trace_data gpx_files(:public_trace_file)
 
-    @request.cookies["_osm_username"] = users(:normal_user).display_name
-
     # And finally we should be able to do it with the owner of the trace
     get :data, {:display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id}, {:user => users(:normal_user).id}
     check_trace_data gpx_files(:public_trace_file)
@@ -328,14 +308,10 @@ class TraceControllerTest < ActionController::TestCase
     get :data, {:display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id}
     assert_response :not_found
 
-    @request.cookies["_osm_username"] = users(:normal_user).display_name
-
     # Now with some other user, which should work since the trace is anon
     get :data, {:display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id}, {:user => users(:normal_user).id}
     assert_response :not_found
 
-    @request.cookies["_osm_username"] = users(:public_user).display_name
-
     # And finally we should be able to do it with the owner of the trace
     get :data, {:display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id}, {:user => users(:public_user).id}
     check_trace_data gpx_files(:anon_trace_file)
@@ -347,8 +323,6 @@ class TraceControllerTest < ActionController::TestCase
     get :data, {:display_name => users(:public_user).display_name, :id => 0}
     assert_response :not_found
 
-    @request.cookies["_osm_username"] = users(:public_user).display_name
-
     # Now with some other user, which should work since the trace is public
     get :data, {:display_name => users(:public_user).display_name, :id => 0}, {:user => users(:public_user).id}
     assert_response :not_found
@@ -365,8 +339,6 @@ class TraceControllerTest < ActionController::TestCase
     assert_response :redirect
     assert_redirected_to :controller => :user, :action => :login, :referer => trace_edit_path(:display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id)
 
-    @request.cookies["_osm_username"] = users(:public_user).display_name
-
     # Now with some other user, which should fail
     get :edit, {:display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id}, {:user => users(:public_user).id}
     assert_response :forbidden
@@ -379,8 +351,6 @@ class TraceControllerTest < ActionController::TestCase
     get :edit, {:display_name => users(:public_user).display_name, :id => gpx_files(:deleted_trace_file).id}, {:user => users(:public_user).id}
     assert_response :not_found
 
-    @request.cookies["_osm_username"] = users(:normal_user).display_name
-
     # Finally with a trace that we are allowed to edit
     get :edit, {:display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id}, {:user => users(:normal_user).id}
     assert_response :success
@@ -395,8 +365,6 @@ class TraceControllerTest < ActionController::TestCase
     post :edit, {:display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id, :trace => new_details}
     assert_response :forbidden
 
-    @request.cookies["_osm_username"] = users(:public_user).display_name
-
     # Now with some other user, which should fail
     post :edit, {:display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id, :trace => new_details}, {:user => users(:public_user).id}
     assert_response :forbidden
@@ -409,8 +377,6 @@ class TraceControllerTest < ActionController::TestCase
     post :edit, {:display_name => users(:public_user).display_name, :id => gpx_files(:deleted_trace_file).id, :trace => new_details}, {:user => users(:public_user).id}
     assert_response :not_found
 
-    @request.cookies["_osm_username"] = users(:normal_user).display_name
-
     # Finally with a trace that we are allowed to edit
     post :edit, {:display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id, :trace => new_details}, {:user => users(:normal_user).id}
     assert_response :redirect
@@ -427,8 +393,6 @@ class TraceControllerTest < ActionController::TestCase
     post :delete, {:display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id,}
     assert_response :forbidden
 
-    @request.cookies["_osm_username"] = users(:public_user).display_name
-
     # Now with some other user, which should fail
     post :delete, {:display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id}, {:user => users(:public_user).id}
     assert_response :forbidden
@@ -441,8 +405,6 @@ class TraceControllerTest < ActionController::TestCase
     post :delete, {:display_name => users(:public_user).display_name, :id => gpx_files(:deleted_trace_file).id}, {:user => users(:public_user).id}
     assert_response :not_found
 
-    @request.cookies["_osm_username"] = users(:normal_user).display_name
-
     # Finally with a trace that we are allowed to delete
     post :delete, {:display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id}, {:user => users(:normal_user).id}
     assert_response :redirect
diff --git a/test/functional/user_blocks_controller_test.rb b/test/functional/user_blocks_controller_test.rb
index 39ebfd28e..49fb6552f 100644
--- a/test/functional/user_blocks_controller_test.rb
+++ b/test/functional/user_blocks_controller_test.rb
@@ -101,7 +101,6 @@ class UserBlocksControllerTest < ActionController::TestCase
 
     # Login as the blocked user
     session[:user] = users(:blocked_user).id
-    cookies["_osm_username"] = users(:blocked_user).display_name
 
     # Now viewing it should mark it as seen
     get :show, :id => user_blocks(:active_block)
@@ -118,7 +117,6 @@ class UserBlocksControllerTest < ActionController::TestCase
 
     # Login as a normal user
     session[:user] = users(:public_user).id
-    cookies["_osm_username"] = users(:public_user).display_name
 
     # Check that normal users can't load the block creation page
     get :new, :display_name => users(:normal_user).display_name
@@ -127,7 +125,6 @@ class UserBlocksControllerTest < ActionController::TestCase
 
     # Login as a moderator
     session[:user] = users(:moderator_user).id
-    cookies["_osm_username"] = users(:moderator_user).display_name
 
     # Check that the block creation page loads for moderators
     get :new, :display_name => users(:normal_user).display_name
@@ -162,7 +159,6 @@ class UserBlocksControllerTest < ActionController::TestCase
 
     # Login as a normal user
     session[:user] = users(:public_user).id
-    cookies["_osm_username"] = users(:public_user).display_name
 
     # Check that normal users can't load the block edit page
     get :edit, :id => user_blocks(:active_block).id
@@ -171,7 +167,6 @@ class UserBlocksControllerTest < ActionController::TestCase
 
     # Login as a moderator
     session[:user] = users(:moderator_user).id
-    cookies["_osm_username"] = users(:moderator_user).display_name
 
     # Check that the block edit page loads for moderators
     get :edit, :id => user_blocks(:active_block).id
@@ -204,7 +199,6 @@ class UserBlocksControllerTest < ActionController::TestCase
 
     # Login as a normal user
     session[:user] = users(:public_user).id
-    cookies["_osm_username"] = users(:public_user).display_name
 
     # Check that normal users can't create blocks
     post :create
@@ -212,7 +206,6 @@ class UserBlocksControllerTest < ActionController::TestCase
 
     # Login as a moderator
     session[:user] = users(:moderator_user).id
-    cookies["_osm_username"] = users(:moderator_user).display_name
 
     # A bogus block period should result in an error
     assert_no_difference "UserBlock.count" do
@@ -263,7 +256,6 @@ class UserBlocksControllerTest < ActionController::TestCase
 
     # Login as a normal user
     session[:user] = users(:public_user).id
-    cookies["_osm_username"] = users(:public_user).display_name
 
     # Check that normal users can't update blocks
     put :update, :id => user_blocks(:active_block).id
@@ -271,7 +263,6 @@ class UserBlocksControllerTest < ActionController::TestCase
 
     # Login as the wrong moderator
     session[:user] = users(:second_moderator_user).id
-    cookies["_osm_username"] = users(:second_moderator_user).display_name
 
     # Check that only the person who created a block can update it
     assert_no_difference "UserBlock.count" do
@@ -285,7 +276,6 @@ class UserBlocksControllerTest < ActionController::TestCase
 
     # Login as the correct moderator
     session[:user] = users(:moderator_user).id
-    cookies["_osm_username"] = users(:moderator_user).display_name
 
     # A bogus block period should result in an error
     assert_no_difference "UserBlock.count" do
@@ -331,7 +321,6 @@ class UserBlocksControllerTest < ActionController::TestCase
 
     # Login as a normal user
     session[:user] = users(:public_user).id
-    cookies["_osm_username"] = users(:public_user).display_name
 
     # Check that normal users can't load the block revoke page
     get :revoke, :id => user_blocks(:active_block).id
@@ -340,7 +329,6 @@ class UserBlocksControllerTest < ActionController::TestCase
 
     # Login as a moderator
     session[:user] = users(:moderator_user).id
-    cookies["_osm_username"] = users(:moderator_user).display_name
 
     # Check that the block revoke page loads for moderators
     get :revoke, :id => user_blocks(:active_block).id
diff --git a/test/functional/user_controller_test.rb b/test/functional/user_controller_test.rb
index cba27f852..66fcef785 100644
--- a/test/functional/user_controller_test.rb
+++ b/test/functional/user_controller_test.rb
@@ -351,17 +351,12 @@ class UserControllerTest < ActionController::TestCase
   def test_user_terms_seen
     user = users(:normal_user)
 
-    # Set the username cookie
-    @request.cookies["_osm_username"] = user.display_name
-
     get :terms, {}, { "user" => user }
     assert_response :redirect
     assert_redirected_to :action => :account, :display_name => user.display_name
   end
 
   def test_user_go_public
-    @request.cookies["_osm_username"] = users(:normal_user).display_name
-
     post :go_public, {}, { :user => users(:normal_user) }
     assert_response :redirect
     assert_redirected_to :action => :account, :display_name => users(:normal_user).display_name
@@ -460,20 +455,15 @@ class UserControllerTest < ActionController::TestCase
     # validation errors being reported
     user = users(:normal_user)
 
-    # Set the username cookie
-    @request.cookies["_osm_username"] = user.display_name
-
     # Make sure that you are redirected to the login page when
     # you are not logged in
     get :account, { :display_name => user.display_name }
     assert_response :redirect
     assert_redirected_to :controller => :user, :action => "login", :referer => "/user/test/account"
 
-    # Make sure that you are redirected to the login page when
-    # you are not logged in as the right user
+    # Make sure that you are blocked when not logged in as the right user
     get :account, { :display_name => user.display_name }, { "user" => users(:public_user).id }
-    assert_response :redirect
-    assert_redirected_to :controller => :user, :action => "login", :referer => "/user/test/account"
+    assert_response :forbidden
 
     # Make sure we get the page when we are logged in as the right user
     get :account, { :display_name => user.display_name }, { "user" => user }
@@ -490,8 +480,8 @@ class UserControllerTest < ActionController::TestCase
     assert_select "form#accountForm > fieldset > div.form-row > div#user_description_container > div#user_description_content > textarea#user_description", user.description
 
     # Changing name to one that exists should fail
-    user.display_name = users(:public_user).display_name
-    post :account, { :display_name => user.display_name, :user => user.attributes }, { "user" => user.id }
+    new_attributes = user.attributes.dup.merge(:display_name => users(:public_user).display_name)
+    post :account, { :display_name => user.display_name, :user => new_attributes }, { "user" => user.id }
     assert_response :success
     assert_template :account
     assert_select "div#notice", false
@@ -499,8 +489,8 @@ class UserControllerTest < ActionController::TestCase
     assert_select "form#accountForm > fieldset > div.form-row > div.field_with_errors > input#user_display_name"
 
     # Changing name to one that exists should fail, regardless of case
-    user.display_name = users(:public_user).display_name.upcase
-    post :account, { :display_name => user.display_name, :user => user.attributes }, { "user" => user.id }
+    new_attributes = user.attributes.dup.merge(:display_name => users(:public_user).display_name.upcase)
+    post :account, { :display_name => user.display_name, :user => new_attributes }, { "user" => user.id }
     assert_response :success
     assert_template :account
     assert_select "div#notice", false
@@ -508,16 +498,16 @@ class UserControllerTest < ActionController::TestCase
     assert_select "form#accountForm > fieldset > div.form-row > div.field_with_errors > input#user_display_name"
 
     # Changing name to one that doesn't exist should work
-    user.display_name = "new tester"
-    post :account, { :display_name => user.display_name, :user => user.attributes }, { "user" => user.id }
+    new_attributes = user.attributes.dup.merge(:display_name => "new tester")
+    post :account, { :display_name => user.display_name, :user => new_attributes }, { "user" => user.id }
     assert_response :success
     assert_template :account
     assert_select "div#errorExplanation", false
     assert_select "div#notice", /^User information updated successfully/
-    assert_select "form#accountForm > fieldset > div.form-row > input#user_display_name[value=?]", user.display_name
+    assert_select "form#accountForm > fieldset > div.form-row > input#user_display_name[value=?]", "new tester"
 
-    # Need to update cookies now to stay valid
-    @request.cookies["_osm_username"] = user.display_name
+    # Record the change of name
+    user.display_name = "new tester"
 
     # Changing email to one that exists should fail
     user.new_email = users(:public_user).email
@@ -598,7 +588,6 @@ class UserControllerTest < ActionController::TestCase
 
     # Login as a normal user
     session[:user] = users(:normal_user).id
-    cookies["_osm_username"] = users(:normal_user).display_name
 
     # Test the normal user
     get :view, {:display_name => "test"}
@@ -616,7 +605,6 @@ class UserControllerTest < ActionController::TestCase
 
     # Login as a moderator
     session[:user] = users(:moderator_user).id
-    cookies["_osm_username"] = users(:moderator_user).display_name
 
     # Test the normal user
     get :view, {:display_name => "test"}
@@ -734,9 +722,6 @@ class UserControllerTest < ActionController::TestCase
     # Check that the users aren't already friends
     assert_nil Friend.where(:user_id => user.id, :friend_user_id => friend.id).first
 
-    # Set the username cookie
-    @request.cookies["_osm_username"] = user.display_name
-
     # When not logged in a GET should ask us to login
     get :make_friend, {:display_name => friend.display_name}
     assert_redirected_to :controller => :user, :action => "login", :referer => make_friend_path(:display_name => friend.display_name)
@@ -787,9 +772,6 @@ class UserControllerTest < ActionController::TestCase
     # Check that the users are friends
     assert Friend.where(:user_id => user.id, :friend_user_id => friend.id).first
 
-    # Set the username cookie
-    @request.cookies["_osm_username"] = user.display_name
-
     # When not logged in a GET should ask us to login
     get :remove_friend, {:display_name => friend.display_name}
     assert_redirected_to :controller => :user, :action => "login", :referer => remove_friend_path(:display_name => friend.display_name)
@@ -838,15 +820,11 @@ class UserControllerTest < ActionController::TestCase
     assert_response :redirect
     assert_redirected_to :action => :login, :referer => set_status_user_path(:status => "suspended")
 
-    @request.cookies["_osm_username"] = users(:normal_user).display_name
-
     # Now try as a normal user
     get :set_status, {:display_name => users(:normal_user).display_name, :status => "suspended"}, {:user => users(:normal_user).id}
     assert_response :redirect
     assert_redirected_to :action => :view, :display_name => users(:normal_user).display_name
 
-    @request.cookies["_osm_username"] = users(:administrator_user).display_name
-
     # Finally try as an administrator
     get :set_status, {:display_name => users(:normal_user).display_name, :status => "suspended"}, {:user => users(:administrator_user).id}
     assert_response :redirect
@@ -860,15 +838,11 @@ class UserControllerTest < ActionController::TestCase
     assert_response :redirect
     assert_redirected_to :action => :login, :referer => delete_user_path(:status => "suspended")
 
-    @request.cookies["_osm_username"] = users(:normal_user).display_name
-
     # Now try as a normal user
     get :delete, {:display_name => users(:normal_user).display_name, :status => "suspended"}, {:user => users(:normal_user).id}
     assert_response :redirect
     assert_redirected_to :action => :view, :display_name => users(:normal_user).display_name
 
-    @request.cookies["_osm_username"] = users(:administrator_user).display_name
-
     # Finally try as an administrator
     get :delete, {:display_name => users(:normal_user).display_name, :status => "suspended"}, {:user => users(:administrator_user).id}
     assert_response :redirect
diff --git a/test/functional/user_roles_controller_test.rb b/test/functional/user_roles_controller_test.rb
index ed5d1a17d..e942cd0fa 100644
--- a/test/functional/user_roles_controller_test.rb
+++ b/test/functional/user_roles_controller_test.rb
@@ -25,7 +25,6 @@ class UserRolesControllerTest < ActionController::TestCase
 
     # Login as an unprivileged user
     session[:user] = users(:public_user).id
-    cookies["_osm_username"] = users(:public_user).display_name
 
     # Granting should still fail
     post :grant, :display_name => users(:normal_user).display_name, :role => "moderator"
@@ -34,7 +33,6 @@ class UserRolesControllerTest < ActionController::TestCase
 
     # Login as an administrator
     session[:user] = users(:administrator_user).id
-    cookies["_osm_username"] = users(:administrator_user).display_name
 
     UserRole::ALL_ROLES.each do |role|
 
@@ -85,7 +83,6 @@ class UserRolesControllerTest < ActionController::TestCase
 
     # Login as an unprivileged user
     session[:user] = users(:public_user).id
-    cookies["_osm_username"] = users(:public_user).display_name
 
     # Revoking should still fail
     post :revoke, :display_name => users(:normal_user).display_name, :role => "moderator"
@@ -94,7 +91,6 @@ class UserRolesControllerTest < ActionController::TestCase
 
     # Login as an administrator
     session[:user] = users(:administrator_user).id
-    cookies["_osm_username"] = users(:administrator_user).display_name
 
     UserRole::ALL_ROLES.each do |role|