diff options
author | Jean-Philippe Lang <jp_lang@yahoo.fr> | 2012-03-03 15:09:20 +0000 |
---|---|---|
committer | Jean-Philippe Lang <jp_lang@yahoo.fr> | 2012-03-03 15:09:20 +0000 |
commit | b3866b05c14bd0f1fff4c54051f522e03e2bec36 (patch) | |
tree | c7248ef2b6cd4d22ec7bfaae11cf6815afdc8f64 | |
parent | bf8f8545461e5aeb1d2b1ddc1b4bf861ee5c2f9c (diff) | |
download | redmine-b3866b05c14bd0f1fff4c54051f522e03e2bec36.tar.gz redmine-b3866b05c14bd0f1fff4c54051f522e03e2bec36.zip |
Removes all #verify calls in controllers. Verification is handled at routing level now that the default route is removed.
git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@9061 e93f8b46-1217-0410-a6f0-8f06a7374b81
28 files changed, 56 insertions, 98 deletions
diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb index 2c79e8613..9d77993ab 100644 --- a/app/controllers/attachments_controller.rb +++ b/app/controllers/attachments_controller.rb @@ -82,7 +82,6 @@ class AttachmentsController < ApplicationController end end - verify :method => :delete, :only => :destroy def destroy # Make sure association callbacks are called @attachment.container.attachments.delete(@attachment) diff --git a/app/controllers/auth_sources_controller.rb b/app/controllers/auth_sources_controller.rb index 2a854ad48..b1150bf71 100644 --- a/app/controllers/auth_sources_controller.rb +++ b/app/controllers/auth_sources_controller.rb @@ -20,10 +20,6 @@ class AuthSourcesController < ApplicationController before_filter :require_admin - # GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html) - verify :method => :post, :only => [ :destroy, :create, :update ], - :redirect_to => { :template => :index } - def index @auth_source_pages, @auth_sources = paginate auth_source_class.name.tableize, :per_page => 10 render "auth_sources/index" diff --git a/app/controllers/boards_controller.rb b/app/controllers/boards_controller.rb index b5bce0c6a..e4d7749bc 100644 --- a/app/controllers/boards_controller.rb +++ b/app/controllers/boards_controller.rb @@ -63,7 +63,6 @@ class BoardsController < ApplicationController @board = @project.boards.build(params[:board]) end - verify :method => :post, :only => :create, :redirect_to => { :action => :index } def create @board = @project.boards.build(params[:board]) if @board.save @@ -77,7 +76,6 @@ class BoardsController < ApplicationController def edit end - verify :method => :put, :only => :update, :redirect_to => { :action => :index } def update if @board.update_attributes(params[:board]) redirect_to_settings_in_projects @@ -86,7 +84,6 @@ class BoardsController < ApplicationController end end - verify :method => :delete, :only => :destroy, :redirect_to => { :action => :index } def destroy @board.destroy redirect_to_settings_in_projects diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index e6cfab798..3a27b973a 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -22,7 +22,6 @@ class CommentsController < ApplicationController before_filter :find_project_from_association before_filter :authorize - verify :method => :post, :only => :create, :render => {:nothing => true, :status => :method_not_allowed } def create raise Unauthorized unless @news.commentable? @@ -35,7 +34,6 @@ class CommentsController < ApplicationController redirect_to :controller => 'news', :action => 'show', :id => @news end - verify :method => :delete, :only => :destroy, :render => {:nothing => true, :status => :method_not_allowed } def destroy @news.comments.find(params[:comment_id]).destroy redirect_to :controller => 'news', :action => 'show', :id => @news diff --git a/app/controllers/enumerations_controller.rb b/app/controllers/enumerations_controller.rb index f2777c3c6..c878afaa8 100644 --- a/app/controllers/enumerations_controller.rb +++ b/app/controllers/enumerations_controller.rb @@ -51,7 +51,6 @@ class EnumerationsController < ApplicationController end end - verify :method => :delete, :only => :destroy, :render => { :nothing => true, :status => :method_not_allowed } def destroy if !@enumeration.in_use? # No associated objects diff --git a/app/controllers/issue_categories_controller.rb b/app/controllers/issue_categories_controller.rb index 905ba66df..7b63bc62b 100644 --- a/app/controllers/issue_categories_controller.rb +++ b/app/controllers/issue_categories_controller.rb @@ -42,7 +42,6 @@ class IssueCategoriesController < ApplicationController @category = @project.issue_categories.build(params[:issue_category]) end - verify :method => :post, :only => :create def create @category = @project.issue_categories.build(params[:issue_category]) if @category.save @@ -73,7 +72,6 @@ class IssueCategoriesController < ApplicationController def edit end - verify :method => :put, :only => :update def update if @category.update_attributes(params[:issue_category]) respond_to do |format| @@ -91,7 +89,6 @@ class IssueCategoriesController < ApplicationController end end - verify :method => :delete, :only => :destroy def destroy @issue_count = @category.issues.size if @issue_count == 0 || params[:todo] || api_request? diff --git a/app/controllers/issue_relations_controller.rb b/app/controllers/issue_relations_controller.rb index bcd0362f0..ec87db727 100644 --- a/app/controllers/issue_relations_controller.rb +++ b/app/controllers/issue_relations_controller.rb @@ -39,7 +39,6 @@ class IssueRelationsController < ApplicationController end end - verify :method => :post, :only => :create, :render => {:nothing => true, :status => :method_not_allowed } def create @relation = IssueRelation.new(params[:relation]) @relation.issue_from = @issue @@ -70,7 +69,6 @@ class IssueRelationsController < ApplicationController end end - verify :method => :delete, :only => :destroy, :render => {:nothing => true, :status => :method_not_allowed } def destroy raise Unauthorized unless @relation.deletable? @relation.destroy diff --git a/app/controllers/issue_statuses_controller.rb b/app/controllers/issue_statuses_controller.rb index 8364074d4..62220d6c9 100644 --- a/app/controllers/issue_statuses_controller.rb +++ b/app/controllers/issue_statuses_controller.rb @@ -62,7 +62,6 @@ class IssueStatusesController < ApplicationController end end - verify :method => :delete, :only => :destroy, :redirect_to => { :action => :index } def destroy IssueStatus.find(params[:id]).destroy redirect_to :action => 'index' diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index b1a383c66..93a087361 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -53,10 +53,6 @@ class IssuesController < ApplicationController helper :gantt include Redmine::Export::PDF - verify :method => :post, :only => :create, :render => {:nothing => true, :status => :method_not_allowed } - verify :method => :post, :only => :bulk_update, :render => {:nothing => true, :status => :method_not_allowed } - verify :method => :put, :only => :update, :render => {:nothing => true, :status => :method_not_allowed } - def index retrieve_query sort_init(@query.sort_criteria.empty? ? [['id', 'desc']] : @query.sort_criteria) @@ -275,7 +271,6 @@ class IssuesController < ApplicationController end end - verify :method => :delete, :only => :destroy, :render => { :nothing => true, :status => :method_not_allowed } def destroy @hours = TimeEntry.sum(:hours, :conditions => ['issue_id IN (?)', @issues]).to_f if @hours > 0 diff --git a/app/controllers/mail_handler_controller.rb b/app/controllers/mail_handler_controller.rb index 9a2f429ea..73b2aa801 100644 --- a/app/controllers/mail_handler_controller.rb +++ b/app/controllers/mail_handler_controller.rb @@ -18,10 +18,6 @@ class MailHandlerController < ActionController::Base before_filter :check_credential - verify :method => :post, - :only => :index, - :render => { :nothing => true, :status => 405 } - # Submits an incoming email to MailHandler def index options = params.dup diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb index 2a5571bb1..6260cd1da 100644 --- a/app/controllers/messages_controller.rb +++ b/app/controllers/messages_controller.rb @@ -22,9 +22,6 @@ class MessagesController < ApplicationController before_filter :find_message, :except => [:new, :preview] before_filter :authorize, :except => [:preview, :edit, :destroy] - verify :method => :post, :only => [ :reply, :destroy ], :redirect_to => { :action => :show } - verify :xhr => true, :only => :quote - helper :watchers helper :attachments include AttachmentsHelper diff --git a/app/controllers/my_controller.rb b/app/controllers/my_controller.rb index a4a4b5131..cdf0182de 100644 --- a/app/controllers/my_controller.rb +++ b/app/controllers/my_controller.rb @@ -35,9 +35,6 @@ class MyController < ApplicationController 'right' => ['issuesreportedbyme'] }.freeze - verify :xhr => true, - :only => [:add_block, :remove_block, :order_blocks] - def index page render :action => 'page' diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index eea8aca14..279944ef6 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -69,7 +69,6 @@ class ProjectsController < ApplicationController @project = Project.new(params[:project]) end - verify :method => :post, :only => :create, :render => {:nothing => true, :status => :method_not_allowed } def create @issue_custom_fields = IssueCustomField.find(:all, :order => "#{CustomField.table_name}.position") @trackers = Tracker.all @@ -182,8 +181,6 @@ class ProjectsController < ApplicationController def edit end - # TODO: convert to PUT only - verify :method => [:post, :put], :only => :update, :render => {:nothing => true, :status => :method_not_allowed } def update @project.safe_attributes = params[:project] if validate_parent_id && @project.save @@ -206,7 +203,6 @@ class ProjectsController < ApplicationController end end - verify :method => :post, :only => :modules, :render => {:nothing => true, :status => :method_not_allowed } def modules @project.enabled_module_names = params[:enabled_module_names] flash[:notice] = l(:notice_successful_update) @@ -227,7 +223,6 @@ class ProjectsController < ApplicationController redirect_to(url_for(:controller => 'admin', :action => 'projects', :status => params[:status])) end - verify :method => :delete, :only => :destroy, :render => {:nothing => true, :status => :method_not_allowed } # Delete @project def destroy @project_to_destroy = @project diff --git a/app/controllers/queries_controller.rb b/app/controllers/queries_controller.rb index 241b5176c..3bc326fe1 100644 --- a/app/controllers/queries_controller.rb +++ b/app/controllers/queries_controller.rb @@ -50,7 +50,6 @@ class QueriesController < ApplicationController build_query_from_params end - verify :method => :post, :only => :create, :render => {:nothing => true, :status => :method_not_allowed } def create @query = Query.new(params[:query]) @query.user = User.current @@ -70,7 +69,6 @@ class QueriesController < ApplicationController def edit end - verify :method => :put, :only => :update, :render => {:nothing => true, :status => :method_not_allowed } def update @query.attributes = params[:query] @query.project = nil if params[:query_is_for_all] @@ -86,7 +84,6 @@ class QueriesController < ApplicationController end end - verify :method => :delete, :only => :destroy, :render => {:nothing => true, :status => :method_not_allowed } def destroy @query.destroy redirect_to :controller => 'issues', :action => 'index', :project_id => @project, :set_filter => 1 diff --git a/app/controllers/roles_controller.rb b/app/controllers/roles_controller.rb index 9c6c1652d..67acb545d 100644 --- a/app/controllers/roles_controller.rb +++ b/app/controllers/roles_controller.rb @@ -68,7 +68,6 @@ class RolesController < ApplicationController end end - verify :method => :delete, :only => :destroy, :redirect_to => { :action => :index } def destroy @role.destroy redirect_to :action => 'index' diff --git a/app/controllers/timelog_controller.rb b/app/controllers/timelog_controller.rb index dfa96d2d7..2feb129e2 100644 --- a/app/controllers/timelog_controller.rb +++ b/app/controllers/timelog_controller.rb @@ -121,7 +121,6 @@ class TimelogController < ApplicationController @time_entry.attributes = params[:time_entry] end - verify :method => :post, :only => :create, :render => {:nothing => true, :status => :method_not_allowed } def create @time_entry ||= TimeEntry.new(:project => @project, :issue => @issue, :user => User.current, :spent_on => User.current.today) @time_entry.attributes = params[:time_entry] @@ -156,7 +155,6 @@ class TimelogController < ApplicationController @time_entry.attributes = params[:time_entry] end - verify :method => :put, :only => :update, :render => {:nothing => true, :status => :method_not_allowed } def update @time_entry.attributes = params[:time_entry] @@ -200,7 +198,6 @@ class TimelogController < ApplicationController redirect_back_or_default({:controller => 'timelog', :action => 'index', :project_id => @projects.first}) end - verify :method => :delete, :only => :destroy, :render => {:nothing => true, :status => :method_not_allowed } def destroy @time_entries.each do |t| begin diff --git a/app/controllers/trackers_controller.rb b/app/controllers/trackers_controller.rb index 1a7022f37..58b10483f 100644 --- a/app/controllers/trackers_controller.rb +++ b/app/controllers/trackers_controller.rb @@ -71,7 +71,6 @@ class TrackersController < ApplicationController render :action => 'edit' end - verify :method => :delete, :only => :destroy, :redirect_to => { :action => :index } def destroy @tracker = Tracker.find(params[:id]) unless @tracker.issues.empty? diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 4f5e30451..b1c14c8bc 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -86,7 +86,6 @@ class UsersController < ApplicationController @auth_sources = AuthSource.find(:all) end - verify :method => :post, :only => :create, :render => {:nothing => true, :status => :method_not_allowed } def create @user = User.new(:language => Setting.default_language, :mail_notification => Setting.default_notification_option) @user.safe_attributes = params[:user] @@ -131,7 +130,6 @@ class UsersController < ApplicationController @membership ||= Member.new end - verify :method => :put, :only => :update, :render => {:nothing => true, :status => :method_not_allowed } def update @user.admin = params[:user][:admin] if params[:user][:admin] @user.login = params[:user][:login] if params[:user][:login] @@ -177,7 +175,6 @@ class UsersController < ApplicationController redirect_to :controller => 'users', :action => 'edit', :id => @user end - verify :method => :delete, :only => :destroy, :render => {:nothing => true, :status => :method_not_allowed } def destroy @user.destroy respond_to do |format| @@ -186,7 +183,6 @@ class UsersController < ApplicationController end end - verify :method => [:post, :put], :only => :edit_membership, :render => {:nothing => true, :status => :method_not_allowed } def edit_membership @membership = Member.edit_membership(params[:membership_id], params[:membership], @user) @membership.save @@ -209,7 +205,6 @@ class UsersController < ApplicationController end end - verify :method => :delete, :only => :destroy_membership, :render => {:nothing => true, :status => :method_not_allowed } def destroy_membership @membership = Member.find(params[:membership_id]) if @membership.deletable? diff --git a/app/controllers/versions_controller.rb b/app/controllers/versions_controller.rb index 34a5e2e34..ddbda38d3 100644 --- a/app/controllers/versions_controller.rb +++ b/app/controllers/versions_controller.rb @@ -160,7 +160,6 @@ class VersionsController < ApplicationController redirect_to :controller => 'projects', :action => 'settings', :tab => 'versions', :id => @project end - verify :method => :delete, :only => :destroy, :render => {:nothing => true, :status => :method_not_allowed } def destroy if @version.fixed_issues.empty? @version.destroy diff --git a/app/controllers/watchers_controller.rb b/app/controllers/watchers_controller.rb index 8a4ce5286..c51722b6a 100644 --- a/app/controllers/watchers_controller.rb +++ b/app/controllers/watchers_controller.rb @@ -20,10 +20,6 @@ class WatchersController < ApplicationController before_filter :require_login, :check_project_privacy, :only => [:watch, :unwatch] before_filter :authorize, :only => [:new, :destroy] - verify :method => :post, - :only => [ :watch, :unwatch ], - :render => { :nothing => true, :status => :method_not_allowed } - def watch if @watched.respond_to?(:visible?) && !@watched.visible?(User.current) render_403 diff --git a/app/controllers/wiki_controller.rb b/app/controllers/wiki_controller.rb index 28d0d44b1..2f851aec9 100644 --- a/app/controllers/wiki_controller.rb +++ b/app/controllers/wiki_controller.rb @@ -118,7 +118,6 @@ class WikiController < ApplicationController end end - verify :method => :put, :only => :update, :render => {:nothing => true, :status => :method_not_allowed } # Creates a new page or updates an existing one def update return render_403 unless editable? @@ -178,7 +177,6 @@ class WikiController < ApplicationController end end - verify :method => :post, :only => :protect, :redirect_to => { :action => :show } def protect @page.update_attribute :protected, params[:protected] redirect_to :action => 'show', :project_id => @project, :id => @page.title @@ -208,7 +206,6 @@ class WikiController < ApplicationController render_404 unless @annotate end - verify :method => :delete, :only => [:destroy], :redirect_to => { :action => :show } # Removes a wiki page and its history # Children can be either set as root pages, removed or reassigned to another parent page def destroy diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 585ee2fa9..13cd3679d 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -2138,20 +2138,6 @@ class IssuesControllerTest < ActionController::TestCase assert_equal 'This is the test_new issue', issue.subject end - def test_update_using_invalid_http_verbs - @request.session[:user_id] = 2 - subject = 'Updated by an invalid http verb' - - get :update, :id => 1, :issue => {:subject => subject} - assert_not_equal subject, Issue.find(1).subject - - post :update, :id => 1, :issue => {:subject => subject} - assert_not_equal subject, Issue.find(1).subject - - delete :update, :id => 1, :issue => {:subject => subject} - assert_not_equal subject, Issue.find(1).subject - end - def test_put_update_without_custom_fields_param @request.session[:user_id] = 2 ActionMailer::Base.deliveries.clear diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb index 6a788e6ec..9b7fca3c0 100644 --- a/test/functional/projects_controller_test.rb +++ b/test/functional/projects_controller_test.rb @@ -311,12 +311,6 @@ class ProjectsControllerTest < ActionController::TestCase end end - def test_create_should_not_accept_get - @request.session[:user_id] = 1 - get :create - assert_response :method_not_allowed - end - def test_show_by_id get :show, :id => 1 assert_response :success @@ -412,12 +406,6 @@ class ProjectsControllerTest < ActionController::TestCase assert_equal ['documents', 'issue_tracking', 'repository'], Project.find(1).enabled_module_names.sort end - def test_modules_should_not_allow_get - @request.session[:user_id] = 1 - get :modules, :id => 1 - assert_response :method_not_allowed - end - def test_destroy_without_confirmation @request.session[:user_id] = 1 # admin delete :destroy, :id => 1 diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 3fc224d01..6adfc2757 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -289,13 +289,6 @@ class UsersControllerTest < ActionController::TestCase assert_nil User.find_by_id(2) end - def test_destroy_should_not_accept_get_requests - assert_no_difference 'User.count' do - get :destroy, :id => 2 - end - assert_response 405 - end - def test_destroy_should_be_denied_for_non_admin_users @request.session[:user_id] = 3 diff --git a/test/functional/watchers_controller_test.rb b/test/functional/watchers_controller_test.rb index c8345a833..bfd93fe0d 100644 --- a/test/functional/watchers_controller_test.rb +++ b/test/functional/watchers_controller_test.rb @@ -32,12 +32,6 @@ class WatchersControllerTest < ActionController::TestCase User.current = nil end - def test_get_watch_should_be_invalid - @request.session[:user_id] = 3 - get :watch, :object_type => 'issue', :object_id => '1' - assert_response 405 - end - def test_watch @request.session[:user_id] = 3 assert_difference('Watcher.count') do diff --git a/test/integration/issues_test.rb b/test/integration/issues_test.rb index 4f72b9839..4f5503026 100644 --- a/test/integration/issues_test.rb +++ b/test/integration/issues_test.rb @@ -206,4 +206,23 @@ class IssuesTest < ActionController::IntegrationTest } } end + + def test_update_using_invalid_http_verbs + subject = 'Updated by an invalid http verb' + + get '/issues/update/1', {:issue => {:subject => subject}}, credentials('jsmith') + assert_response 404 + assert_not_equal subject, Issue.find(1).subject + + post '/issues/1', {:issue => {:subject => subject}}, credentials('jsmith') + assert_response 405 + assert_not_equal subject, Issue.find(1).subject + end + + def test_get_watch_should_be_invalid + assert_no_difference 'Watcher.count' do + get '/watchers/watch?object_type=issue&object_id=1', {}, credentials('jsmith') + assert_response 405 + end + end end diff --git a/test/integration/projects_test.rb b/test/integration/projects_test.rb index 890250c8a..1abe6ad96 100644 --- a/test/integration/projects_test.rb +++ b/test/integration/projects_test.rb @@ -18,7 +18,7 @@ require File.expand_path('../../test_helper', __FILE__) class ProjectsTest < ActionController::IntegrationTest - fixtures :projects, :users, :members + fixtures :projects, :users, :members, :enabled_modules def test_archive_project subproject = Project.find(1).children.first @@ -41,4 +41,11 @@ class ProjectsTest < ActionController::IntegrationTest get "projects/1" assert_response :success end + + def test_modules_should_not_allow_get + assert_no_difference 'EnabledModule.count' do + get '/projects/1/modules', {:enabled_module_names => ['']}, credentials('jsmith') + assert_response :method_not_allowed + end + end end diff --git a/test/integration/users_test.rb b/test/integration/users_test.rb new file mode 100644 index 000000000..a0c461b49 --- /dev/null +++ b/test/integration/users_test.rb @@ -0,0 +1,29 @@ +# Redmine - project management software +# Copyright (C) 2006-2012 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +require File.expand_path('../../test_helper', __FILE__) + +class USersTest < ActionController::IntegrationTest + fixtures :users + + def test_destroy_should_not_accept_get_requests + assert_no_difference 'User.count' do + get '/users/destroy/2', {}, credentials('admin') + assert_response 404 + end + end +end |