From 41e45bad51c90dd4e131068ba1cfc1dd1184155f Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Sun, 17 Nov 2013 21:52:04 +0000 Subject: [PATCH] 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. --- app/controllers/application_controller.rb | 6 +-- app/controllers/user_controller.rb | 15 +++--- .../functional/diary_entry_controller_test.rb | 18 ------- test/functional/message_controller_test.rb | 12 ----- test/functional/redactions_controller_test.rb | 9 ---- test/functional/site_controller_test.rb | 10 ---- test/functional/trace_controller_test.rb | 38 --------------- .../functional/user_blocks_controller_test.rb | 12 ----- test/functional/user_controller_test.rb | 48 +++++-------------- test/functional/user_roles_controller_test.rb | 4 -- 10 files changed, 21 insertions(+), 151 deletions(-) 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| -- 2.39.5