Allow key authentication when creating issues (with tests) #6447

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4365 e93f8b46-1217-0410-a6f0-8f06a7374b81

3 files changed, 84 insertions, 56 deletions

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index 67488b753..e8511bc60 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -27,7 +27,7 @@ class IssuesController < ApplicationController
   before_filter :find_optional_project, :only => [:index]
   before_filter :check_for_default_issue_status, :only => [:new, :create]
   before_filter :build_new_issue_from_params, :only => [:new, :create]
-  accept_key_auth :index, :show
+  accept_key_auth :index, :show, :create
 
   rescue_from Query::StatementInvalid, :with => :query_statement_invalid
   
diff --git a/test/integration/api_test/issues_test.rb b/test/integration/api_test/issues_test.rb
index 7e3b7aba3..2cee8c9be 100644
--- a/test/integration/api_test/issues_test.rb
+++ b/test/integration/api_test/issues_test.rb
@@ -91,69 +91,70 @@ class ApiTest::IssuesTest < ActionController::IntegrationTest
   end
 
   context "POST /issues.xml" do
-    setup do
-      @issue_count = Issue.count
-      @attributes = {:project_id => 1, :subject => 'API test', :tracker_id => 2, :status_id => 3}
-      post '/issues.xml', {:issue => @attributes}, :authorization => credentials('jsmith')
-    end
-
-    should_respond_with :created
-    should_respond_with_content_type 'application/xml'
+    should_allow_api_authentication(:post,
+                                    '/issues.xml',
+                                    {:issue => {:project_id => 1, :subject => 'API test', :tracker_id => 2, :status_id => 3}},
+                                    {:success_code => :created})
 
     should "create an issue with the attributes" do
-      assert_equal Issue.count, @issue_count + 1
-
-      issue = Issue.first(:order => 'id DESC')
-      @attributes.each do |attribute, value|
-        assert_equal value, issue.send(attribute)
+      assert_difference('Issue.count') do
+        post '/issues.xml', {:issue => {:project_id => 1, :subject => 'API test', :tracker_id => 2, :status_id => 3}}, :authorization => credentials('jsmith')
       end
+        
+      issue = Issue.first(:order => 'id DESC')
+      assert_equal 1, issue.project_id
+      assert_equal 2, issue.tracker_id
+      assert_equal 3, issue.status_id
+      assert_equal 'API test', issue.subject
     end
   end
   
   context "POST /issues.xml with failure" do
-    setup do
-      @attributes = {:project_id => 1}
-      post '/issues.xml', {:issue => @attributes}, :authorization => credentials('jsmith')
-    end
-
-    should_respond_with :unprocessable_entity
-    should_respond_with_content_type 'application/xml'
+    should_allow_api_authentication(:post,
+                                    '/issues.xml',
+                                    {:issue => {:project_id => 1}},
+                                    {:success_code => :unprocessable_entity})
 
     should "have an errors tag" do
+      assert_no_difference('Issue.count') do
+        post '/issues.xml', {:issue => {:project_id => 1}}, :authorization => credentials('jsmith')
+      end
+
       assert_tag :errors, :child => {:tag => 'error', :content => "Subject can't be blank"}
     end
   end
 
   context "POST /issues.json" do
-    setup do
-      @issue_count = Issue.count
-      @attributes = {:project_id => 1, :subject => 'API test', :tracker_id => 2, :status_id => 3}
-      post '/issues.json', {:issue => @attributes}, :authorization => credentials('jsmith')
-    end
-
-    should_respond_with :created
-    should_respond_with_content_type 'application/json'
+    should_allow_api_authentication(:post,
+                                    '/issues.json',
+                                    {:issue => {:project_id => 1, :subject => 'API test', :tracker_id => 2, :status_id => 3}},
+                                    {:success_code => :created})
 
     should "create an issue with the attributes" do
-      assert_equal Issue.count, @issue_count + 1
-
-      issue = Issue.first(:order => 'id DESC')
-      @attributes.each do |attribute, value|
-        assert_equal value, issue.send(attribute)
+      assert_difference('Issue.count') do
+        post '/issues.json', {:issue => {:project_id => 1, :subject => 'API test', :tracker_id => 2, :status_id => 3}}, :authorization => credentials('jsmith')
       end
+        
+      issue = Issue.first(:order => 'id DESC')
+      assert_equal 1, issue.project_id
+      assert_equal 2, issue.tracker_id
+      assert_equal 3, issue.status_id
+      assert_equal 'API test', issue.subject
     end
+    
   end
   
   context "POST /issues.json with failure" do
-    setup do
-      @attributes = {:project_id => 1}
-      post '/issues.json', {:issue => @attributes}, :authorization => credentials('jsmith')
-    end
-
-    should_respond_with :unprocessable_entity
-    should_respond_with_content_type 'application/json'
+    should_allow_api_authentication(:post,
+                                    '/issues.json',
+                                    {:issue => {:project_id => 1}},
+                                    {:success_code => :unprocessable_entity})
 
     should "have an errors element" do
+      assert_no_difference('Issue.count') do
+        post '/issues.json', {:issue => {:project_id => 1}}, :authorization => credentials('jsmith')
+      end
+
       json = ActiveSupport::JSON.decode(response.body)
       assert_equal "can't be blank", json.first['subject']
     end
diff --git a/test/test_helper.rb b/test/test_helper.rb
index e956cdce5..09e600f27 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -195,10 +195,13 @@ class ActiveSupport::TestCase
   # @param [Symbol] http_method the HTTP method for request (:get, :post, :put, :delete)
   # @param [String] url the request url
   # @param [optional, Hash] parameters additional request parameters
-  def self.should_allow_api_authentication(http_method, url, parameters={})
-    should_allow_http_basic_auth_with_username_and_password(http_method, url, parameters)
-    should_allow_http_basic_auth_with_key(http_method, url, parameters)
-    should_allow_key_based_auth(http_method, url, parameters)
+  # @param [optional, Hash] options additional options
+  # @option options [Symbol] :success_code Successful response code (:success)
+  # @option options [Symbol] :failure_code Failure response code (:unauthorized)
+  def self.should_allow_api_authentication(http_method, url, parameters={}, options={})
+    should_allow_http_basic_auth_with_username_and_password(http_method, url, parameters, options)
+    should_allow_http_basic_auth_with_key(http_method, url, parameters, options)
+    should_allow_key_based_auth(http_method, url, parameters, options)
   end
 
   # Test that a request allows the username and password for HTTP BASIC
@@ -206,7 +209,13 @@ class ActiveSupport::TestCase
   # @param [Symbol] http_method the HTTP method for request (:get, :post, :put, :delete)
   # @param [String] url the request url
   # @param [optional, Hash] parameters additional request parameters
-  def self.should_allow_http_basic_auth_with_username_and_password(http_method, url, parameters={})
+  # @param [optional, Hash] options additional options
+  # @option options [Symbol] :success_code Successful response code (:success)
+  # @option options [Symbol] :failure_code Failure response code (:unauthorized)
+  def self.should_allow_http_basic_auth_with_username_and_password(http_method, url, parameters={}, options={})
+    success_code = options[:success_code] || :success
+    failure_code = options[:failure_code] || :unauthorized
+    
     context "should allow http basic auth using a username and password for #{http_method} #{url}" do
       context "with a valid HTTP authentication" do
         setup do
@@ -215,7 +224,7 @@ class ActiveSupport::TestCase
           send(http_method, url, parameters, {:authorization => @authorization})
         end
         
-        should_respond_with :success
+        should_respond_with success_code
         should_respond_with_content_type_based_on_url(url)
         should "login as the user" do
           assert_equal @user, User.current
@@ -229,7 +238,7 @@ class ActiveSupport::TestCase
           send(http_method, url, parameters, {:authorization => @authorization})
         end
         
-        should_respond_with :unauthorized
+        should_respond_with failure_code
         should_respond_with_content_type_based_on_url(url)
         should "not login as the user" do
           assert_equal User.anonymous, User.current
@@ -241,7 +250,7 @@ class ActiveSupport::TestCase
           send(http_method, url, parameters, {:authorization => ''})
         end
 
-        should_respond_with :unauthorized
+        should_respond_with failure_code
         should_respond_with_content_type_based_on_url(url)
         should "include_www_authenticate_header" do
           assert @controller.response.headers.has_key?('WWW-Authenticate')
@@ -256,7 +265,13 @@ class ActiveSupport::TestCase
   # @param [Symbol] http_method the HTTP method for request (:get, :post, :put, :delete)
   # @param [String] url the request url
   # @param [optional, Hash] parameters additional request parameters
-  def self.should_allow_http_basic_auth_with_key(http_method, url, parameters={})
+  # @param [optional, Hash] options additional options
+  # @option options [Symbol] :success_code Successful response code (:success)
+  # @option options [Symbol] :failure_code Failure response code (:unauthorized)
+  def self.should_allow_http_basic_auth_with_key(http_method, url, parameters={}, options={})
+    success_code = options[:success_code] || :success
+    failure_code = options[:failure_code] || :unauthorized
+
     context "should allow http basic auth with a key for #{http_method} #{url}" do
       context "with a valid HTTP authentication using the API token" do
         setup do
@@ -266,7 +281,7 @@ class ActiveSupport::TestCase
           send(http_method, url, parameters, {:authorization => @authorization})
         end
         
-        should_respond_with :success
+        should_respond_with success_code
         should_respond_with_content_type_based_on_url(url)
         should_be_a_valid_response_string_based_on_url(url)
         should "login as the user" do
@@ -282,7 +297,7 @@ class ActiveSupport::TestCase
           send(http_method, url, parameters, {:authorization => @authorization})
         end
 
-        should_respond_with :unauthorized
+        should_respond_with failure_code
         should_respond_with_content_type_based_on_url(url)
         should "not login as the user" do
           assert_equal User.anonymous, User.current
@@ -296,7 +311,13 @@ class ActiveSupport::TestCase
   # @param [Symbol] http_method the HTTP method for request (:get, :post, :put, :delete)
   # @param [String] url the request url, without the key=ZXY parameter
   # @param [optional, Hash] parameters additional request parameters
-  def self.should_allow_key_based_auth(http_method, url, parameters={})
+  # @param [optional, Hash] options additional options
+  # @option options [Symbol] :success_code Successful response code (:success)
+  # @option options [Symbol] :failure_code Failure response code (:unauthorized)
+  def self.should_allow_key_based_auth(http_method, url, parameters={}, options={})
+    success_code = options[:success_code] || :success
+    failure_code = options[:failure_code] || :unauthorized
+
     context "should allow key based auth using key=X for #{http_method} #{url}" do
       context "with a valid api token" do
         setup do
@@ -311,7 +332,7 @@ class ActiveSupport::TestCase
           send(http_method, request_url, parameters)
         end
         
-        should_respond_with :success
+        should_respond_with success_code
         should_respond_with_content_type_based_on_url(url)
         should_be_a_valid_response_string_based_on_url(url)
         should "login as the user" do
@@ -323,10 +344,16 @@ class ActiveSupport::TestCase
         setup do
           @user = User.generate_with_protected!
           @token = Token.generate!(:user => @user, :action => 'feeds')
-          send(http_method, url + "?key=#{@token.value}")
+          # Simple url parse to add on ?key= or &key=
+          request_url = if url.match(/\?/)
+                          url + "&key=#{@token.value}"
+                        else
+                          url + "?key=#{@token.value}"
+                        end
+          send(http_method, request_url, parameters)
         end
         
-        should_respond_with :unauthorized
+        should_respond_with failure_code
         should_respond_with_content_type_based_on_url(url)
         should "not login as the user" do
           assert_equal User.anonymous, User.current