Merge remote branch 'edavis/master' into master-journalized

test/integration/api_test/http_basic_login_with_api_token_test.rb

@@ -17,68 +17,11 @@ class ApiTest::HttpBasicLoginWithApiTokenTest < ActionController::IntegrationTes
   context "get /news" do
 
     context "in :xml format" do
-      context "with a valid HTTP authentication using the API token" do
-        setup do
-          @user = User.generate_with_protected!
-          @token = Token.generate!(:user => @user, :action => 'api')
-          @authorization = ActionController::HttpAuthentication::Basic.encode_credentials(@token.value, 'X')
-          get "/news.xml", nil, :authorization => @authorization
-        end
-        
-        should_respond_with :success
-        should_respond_with_content_type :xml
-        should "login as the user" do
-          assert_equal @user, User.current
-        end
-      end
-
-      context "with an invalid HTTP authentication" do
-        setup do
-          @user = User.generate_with_protected!
-          @token = Token.generate!(:user => @user, :action => 'feeds')
-          @authorization = ActionController::HttpAuthentication::Basic.encode_credentials(@token.value, 'X')
-          get "/news.xml", nil, :authorization => @authorization
-        end
-
-        should_respond_with :unauthorized
-        should_respond_with_content_type :xml
-        should "not login as the user" do
-          assert_equal User.anonymous, User.current
-        end
-      end
+      should_allow_http_basic_auth_with_key(:get, "/news.xml")
     end
 
     context "in :json format" do
-      context "with a valid HTTP authentication" do
-        setup do
-          @user = User.generate_with_protected!
-          @token = Token.generate!(:user => @user, :action => 'api')
-          @authorization = ActionController::HttpAuthentication::Basic.encode_credentials(@token.value, 'DoesNotMatter')
-          get "/news.json", nil, :authorization => @authorization
-        end
-        
-        should_respond_with :success
-        should_respond_with_content_type :json
-        should "login as the user" do
-          assert_equal @user, User.current
-        end
-      end
-
-      context "with an invalid HTTP authentication" do
-        setup do
-          @user = User.generate_with_protected!
-          @token = Token.generate!(:user => @user, :action => 'feeds')
-          @authorization = ActionController::HttpAuthentication::Basic.encode_credentials(@token.value, 'DoesNotMatter')
-          get "/news.json", nil, :authorization => @authorization
-        end
-        
-        should_respond_with :unauthorized
-        should_respond_with_content_type :json
-        should "not login as the user" do
-          assert_equal User.anonymous, User.current
-        end
-      end
+      should_allow_http_basic_auth_with_key(:get, "/news.json")
     end
-    
   end
 end

test/integration/api_test/issues_test.rb

@@ -45,36 +45,21 @@ class ApiTest::IssuesTest < ActionController::IntegrationTest
     Setting.rest_api_enabled = '1'
   end
 
+  # Use a private project to make sure auth is really working and not just
+  # only showing public issues.
   context "/index.xml" do
-    setup do
-      get '/issues.xml'
-    end
-
-    should_respond_with :success
-    should_respond_with_content_type 'application/xml'
+    should_allow_api_authentication(:get, "/projects/private-child/issues.xml")
   end
 
   context "/index.json" do
-    setup do
-      get '/issues.json'
-    end
-
-    should_respond_with :success
-    should_respond_with_content_type 'application/json'
-
-    should 'return a valid JSON string' do
-      assert ActiveSupport::JSON.decode(response.body)
-    end
+    should_allow_api_authentication(:get, "/projects/private-child/issues.json")
   end
 
   context "/index.xml with filter" do
-    setup do
-      get '/issues.xml?status_id=5'
-    end
-    
-    should_respond_with :success
-    should_respond_with_content_type 'application/xml'
+    should_allow_api_authentication(:get, "/projects/private-child/issues.xml?status_id=5")
+
     should "show only issues with the status_id" do
+      get '/issues.xml?status_id=5'
       assert_tag :tag => 'issues',
                  :children => { :count => Issue.visible.count(:conditions => {:status_id => 5}), 
                                 :only => { :tag => 'issue' } }
@@ -82,18 +67,11 @@ class ApiTest::IssuesTest < ActionController::IntegrationTest
   end
 
   context "/index.json with filter" do
-    setup do
-      get '/issues.json?status_id=5'
-    end
-
-    should_respond_with :success
-    should_respond_with_content_type 'application/json'
-
-    should 'return a valid JSON string' do
-      assert ActiveSupport::JSON.decode(response.body)
-    end
+    should_allow_api_authentication(:get, "/projects/private-child/issues.json?status_id=5")
 
     should "show only issues with the status_id" do
+      get '/issues.json?status_id=5'
+
       json = ActiveSupport::JSON.decode(response.body)
       status_ids_used = json.collect {|j| j['status_id'] }
       assert_equal 3, status_ids_used.length
@@ -102,26 +80,13 @@ class ApiTest::IssuesTest < ActionController::IntegrationTest
 
   end
 
-  context "/issues/1.xml" do
-    setup do
-      get '/issues/1.xml'
-    end
-    
-    should_respond_with :success
-    should_respond_with_content_type 'application/xml'
+  # Issue 6 is on a private project
+  context "/issues/6.xml" do
+    should_allow_api_authentication(:get, "/issues/6.xml")
   end
 
-  context "/issues/1.json" do
-    setup do
-      get '/issues/1.json'
-    end
-    
-    should_respond_with :success
-    should_respond_with_content_type 'application/json'
-
-    should 'return a valid JSON string' do
-      assert ActiveSupport::JSON.decode(response.body)
-    end
+  context "/issues/6.json" do
+    should_allow_api_authentication(:get, "/issues/6.json")
   end
 
   context "POST /issues.xml" do

test/test_helper.rb

@@ -182,6 +182,21 @@ class ActiveSupport::TestCase
     end
   end
 
+  # Test that a request allows the three types of API authentication
+  #
+  # * HTTP Basic with username and password
+  # * HTTP Basic with an api key for the username
+  # * Key based with the key=X parameter
+  #
+  # @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)
+  end
+
   # Test that a request allows the username and password for HTTP BASIC
   #
   # @param [Symbol] http_method the HTTP method for request (:get, :post, :put, :delete)
@@ -232,21 +247,69 @@ class ActiveSupport::TestCase
 
   end
 
+  # Test that a request allows the API key with HTTP BASIC
+  #
+  # @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={})
+    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
+          @user = User.generate_with_protected!(:admin => true)
+          @token = Token.generate!(:user => @user, :action => 'api')
+          @authorization = ActionController::HttpAuthentication::Basic.encode_credentials(@token.value, 'X')
+          send(http_method, url, parameters, {:authorization => @authorization})
+        end
+        
+        should_respond_with :success
+        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
+          assert_equal @user, User.current
+        end
+      end
+
+      context "with an invalid HTTP authentication" do
+        setup do
+          @user = User.generate_with_protected!
+          @token = Token.generate!(:user => @user, :action => 'feeds')
+          @authorization = ActionController::HttpAuthentication::Basic.encode_credentials(@token.value, 'X')
+          send(http_method, url, parameters, {:authorization => @authorization})
+        end
+
+        should_respond_with :unauthorized
+        should_respond_with_content_type_based_on_url(url)
+        should "not login as the user" do
+          assert_equal User.anonymous, User.current
+        end
+      end
+    end
+  end
+  
   # Test that a request allows full key authentication
   #
   # @param [Symbol] http_method the HTTP method for request (:get, :post, :put, :delete)
   # @param [String] url the request url, without the key=ZXY parameter
-  def self.should_allow_key_based_auth(http_method, url)
+  # @param [optional, Hash] parameters additional request parameters
+  def self.should_allow_key_based_auth(http_method, url, parameters={})
     context "should allow key based auth using key=X for #{http_method} #{url}" do
       context "with a valid api token" do
         setup do
-          @user = User.generate_with_protected!
+          @user = User.generate_with_protected!(:admin => true)
           @token = Token.generate!(:user => @user, :action => 'api')
-          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 :success
         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
           assert_equal @user, User.current
         end
@@ -286,6 +349,39 @@ class ActiveSupport::TestCase
     end
     
   end
+
+  # Uses the url to assert which format the response should be in
+  #
+  # '/project/issues.xml' => should_be_a_valid_xml_string
+  # '/project/issues.json' => should_be_a_valid_json_string
+  #
+  # @param [String] url Request
+  def self.should_be_a_valid_response_string_based_on_url(url)
+    case
+    when url.match(/xml/i)
+      should_be_a_valid_xml_string
+    when url.match(/json/i)
+      should_be_a_valid_json_string
+    else
+      raise "Unknown content type for should_be_a_valid_response_based_on_url: #{url}"
+    end
+    
+  end
+  
+  # Checks that the response is a valid JSON string
+  def self.should_be_a_valid_json_string
+    should "be a valid JSON string" do
+      assert ActiveSupport::JSON.decode(response.body)
+    end
+  end
+
+  # Checks that the response is a valid XML string
+  def self.should_be_a_valid_xml_string
+    should "be a valid XML string" do
+      assert REXML::Document.new(response.body)
+    end
+  end
+  
 end
 
 # Simple module to "namespace" all of the API tests