Redmine API: Testing for the Authentication Key

I’m pausing the Redmine Refactoring for a few weeks so I can work on some features for the Redmine API. With the 1.1 release of Redmine coming out in two months, I need to get started on fixing and adding to the current API. I decided to use this blog to write about the changes I’m making so other people can see a little bit about how I work and how the Redmine API‘s internals are setup.

Before I start adding new APIs, I need to go back through the existing APIs to fix a few bugs that have been discovered. The first bug is that the Issues API doesn’t allow full key auth for all actions. There are several ways to authenticate against Redmine’s API as a user:

  1. HTTP Basic auth – http://login:password@redmine.org/issues.xml
  2. -HTTP Basic auth with API token and login – http://login:RANDOM_KEY@redmine.org/issues.xml– (not supported yet)
  3. HTTP Basic auth with API token – http://RANDOM_KEY:X@redmine.org/issues.xml
  4. Full token auth – http://redmine.org/issues.xml?key=RANDOM_KEY

Bug #6447 is about the fourth method. The first three methods work right away with Redmine but the fourth requires Redmine to explicitly allow key authentication (accept_key_auth). During the rush to get an Issues and Projects API for Redmine 1.0, this accept_key_auth was forgotten for a few actions which prevented that authentication from working.

It’s easy for me to just go and add accept_key_auth for all of the controller actions but I really want to make sure that Redmine’s API is well tested so it doesn’t break with a new version. So to start it off, I refactored the tests for the Full token auth so it uses shoulda macros.

Before

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60

require "#{File.dirname(__FILE__)}/../../test_helper"
 
class ApiTest::TokenAuthenticationTest  @user, :action => 'api')
          get "/news.xml?key=#{@token.value}"
        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 api token" do
        setup do
          @user = User.generate_with_protected!
          @token = Token.generate!(:user => @user, :action => 'feeds')
          get "/news.xml?key=#{@token.value}"
        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
    end
 
    context "in :json format" do
      context "with a valid api token" do
        setup do
          @user = User.generate_with_protected!
          @token = Token.generate!(:user => @user, :action => 'api')
          get "/news.json?key=#{@token.value}"
        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 api token" do
        setup do
          @user = User.generate_with_protected!
          @token = Token.generate!(:user => @user, :action => 'feeds')
          get "/news.json?key=#{@token.value}"
        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
    end
 
  end
end

After

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26

require "#{File.dirname(__FILE__)}/../../test_helper"
 
class ApiTest::TokenAuthenticationTest < ActionController::IntegrationTest
  fixtures :all
 
  def setup
    Setting.rest_api_enabled = '1'
    Setting.login_required = '1'
  end
 
  def teardown
    Setting.rest_api_enabled = '0'
    Setting.login_required = '0'
  end
 
  # Using the NewsController because it's a simple API.
  context "get /news" do
    context "in :xml format" do
      should_allow_key_based_auth(:get, "/news.xml")
    end
 
    context "in :json format" do
      should_allow_key_based_auth(:get, "/news.json")
    end
  end
end
 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55

# test_helper.rb
  # 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)
    context "should allow key based auth using key=X for #{url}" do
      context "with a valid api token" do
        setup do
          @user = User.generate_with_protected!
          @token = Token.generate!(:user => @user, :action => 'api')
          send(http_method, url + "?key=#{@token.value}")
        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 api token" do
        setup do
          @user = User.generate_with_protected!
          @token = Token.generate!(:user => @user, :action => 'feeds')
          send(http_method, url + "?key=#{@token.value}")
        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
 
  # Uses should_respond_with_content_type based on what's in the url:
  #
  # '/project/issues.xml' => should_respond_with_content_type :xml
  # '/project/issues.json' => should_respond_with_content_type :json
  #
  # @param [String] url Request
  def self.should_respond_with_content_type_based_on_url(url)
    case
    when url.match(/xml/i)
      should_respond_with_content_type :xml
    when url.match(/json/i)
      should_respond_with_content_type :json
    else
      raise "Unknown content type for should_respond_with_content_type_based_on_url: #{url}"
    end
 
  end

There are two massive benefits to this refactoring:

  1. I removed the amount of duplication in the actual test methods with a simple url check, since both the XML and JSON apis are 95% identical.
  2. The should_allow_key_based_auth macro is really easy to setup now and provides 6 complete tests each time it’s used.

Tomorrow I’ll show I use this new should_allow_key_based_auth macro to easily add tests for the Issues and Projects API that were part of Bug #6447.