From 30dc4fec998183f8dc077a6503fed993e9e08b9e Mon Sep 17 00:00:00 2001 From: Eric Davis Date: Wed, 3 Nov 2010 16:48:23 +0000 Subject: [PATCH 1/2] Refactor: convert API key tests using HTTP Basic to a shoulda macro git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4363 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- .../http_basic_login_with_api_token_test.rb | 61 +------------------ test/test_helper.rb | 39 ++++++++++++ 2 files changed, 41 insertions(+), 59 deletions(-) diff --git a/test/integration/api_test/http_basic_login_with_api_token_test.rb b/test/integration/api_test/http_basic_login_with_api_token_test.rb index 0d0d5bd0cd..42c0be2876 100644 --- a/test/integration/api_test/http_basic_login_with_api_token_test.rb +++ b/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 diff --git a/test/test_helper.rb b/test/test_helper.rb index 285ef2fd94..18d8fd8f14 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -236,6 +236,45 @@ 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! + @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 "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) From c967899b145619080197d32d801f99484b485ed7 Mon Sep 17 00:00:00 2001 From: Eric Davis Date: Thu, 4 Nov 2010 16:22:47 +0000 Subject: [PATCH 2/2] Refactor: Convert the tests for Issues#index and #show APIs to shoulda. #6447 git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4364 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- test/integration/api_test/issues_test.rb | 65 ++++++------------------ test/test_helper.rb | 65 ++++++++++++++++++++++-- 2 files changed, 76 insertions(+), 54 deletions(-) diff --git a/test/integration/api_test/issues_test.rb b/test/integration/api_test/issues_test.rb index c51438bbaf..7e3b7aba37 100644 --- a/test/integration/api_test/issues_test.rb +++ b/test/integration/api_test/issues_test.rb @@ -46,36 +46,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' } } @@ -83,18 +68,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 @@ -103,26 +81,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 diff --git a/test/test_helper.rb b/test/test_helper.rb index 18d8fd8f14..e956cdce5e 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -186,6 +186,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) @@ -245,7 +260,7 @@ class ActiveSupport::TestCase 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! + @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}) @@ -253,6 +268,7 @@ class ActiveSupport::TestCase 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 @@ -279,17 +295,25 @@ 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 - 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 @@ -329,6 +353,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