Redmine API: Adding Key Authentication for Issues#create

Now that I’ve tested #index and #show for #6447, it’s time to see if #create is working.

Updating the Issues#create test

The first thing I need to do is to update the tests for #create to see if I can reproduce any authentication bugs. With a few tweaks to my shoulda macros, I added tests for the full authentication.

Before (only showing XML version)

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

  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 "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)
      end
    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 "have an errors tag" do
      assert_tag :errors, :child => {:tag => 'error', :content => "Subject can't be blank"}
    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
27
28
29
30
31
32
33

  context "POST /issues.xml" do
    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_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
    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

Updating the shoulda macros

In order to support the new :success_code => :created and :success_code => :unprocessable_entity I had to make a few changes to all of the shoulda macros to let me override the response codes. Using an options hash with a few default values was all that I needed:

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

  # 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)
  # @param [String] url the request url
  # @param [optional, Hash] parameters additional request 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
        # ...
        should_respond_with success_code
        # ...
      end
 
      context "with an invalid HTTP authentication" do
        # ...
        should_respond_with failure_code
        # ...
      end
 
      context "without credentials" do
        # ...
        should_respond_with failure_code
        # ...
      end
    end
 
  end

This will make it easy to override the macros to support all of the different response codes that Rails generates.

Running the new tests

Now that the tests are updated to use all of the authentications that the Redmine API supports, it’s time to run them and see if there is anything missing.

$ ruby test/integration/api_test/issues_test.rb
/usr/lib/ruby/gems/1.8/gems/rails-2.3.5/lib/rails/gem_dependency.rb:119:Warning: Gem::Dependency#version_requirements is deprecated and will be removed on or after August 2010.  Use #requirement
Loaded suite test/integration/api_test/issues_test
Started
..................................................................................................................................................................FF.F....................FF.F.....................F.F.....................F.F...........................
Finished in 51.680591 seconds.

  1) Failure:
test: POST /issues.json should allow key based auth using key=X for post /issues.json with a valid api token should be a valid JSON string. (ApiTest::IssuesTest)
    [/test/integration/api_test/../../test_helper.rb:405:in `__bind_1288975327_904252'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `call'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `test: POST /issues.json should allow key based auth using key=X for post /issues.json with a valid api token should be a valid JSON string. ']:
 is not true.

  2) Failure:
test: POST /issues.json should allow key based auth using key=X for post /issues.json with a valid api token should login as the user. (ApiTest::IssuesTest)
    [/test/integration/api_test/../../test_helper.rb:339:in `__bind_1288975328_194241'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `call'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `test: POST /issues.json should allow key based auth using key=X for post /issues.json with a valid api token should login as the user. ']:
<#> expected but was
<#>.

  3) Failure:
test: POST /issues.json should allow key based auth using key=X for post /issues.json with a valid api token should respond with created. (ApiTest::IssuesTest)
    [shoulda (2.10.3) lib/shoulda/assertions.rb:55:in `assert_accepts'
     shoulda (2.10.3) lib/shoulda/action_controller/macros.rb:124:in `__bind_1288975328_430126'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `call'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `test: POST /issues.json should allow key based auth using key=X for post /issues.json with a valid api token should respond with created. ']:
Expected response to be a 201, but was 401

  4) Failure:
test: POST /issues.json with failure should allow key based auth using key=X for post /issues.json with a valid api token should be a valid JSON string. (ApiTest::IssuesTest)
    [/test/integration/api_test/../../test_helper.rb:405:in `__bind_1288975332_83529'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `call'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `test: POST /issues.json with failure should allow key based auth using key=X for post /issues.json with a valid api token should be a valid JSON string. ']:
 is not true.

  5) Failure:
test: POST /issues.json with failure should allow key based auth using key=X for post /issues.json with a valid api token should login as the user. (ApiTest::IssuesTest)
    [/test/integration/api_test/../../test_helper.rb:339:in `__bind_1288975332_429683'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `call'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `test: POST /issues.json with failure should allow key based auth using key=X for post /issues.json with a valid api token should login as the user. ']:
<#> expected but was
<#>.

  6) Failure:
test: POST /issues.json with failure should allow key based auth using key=X for post /issues.json with a valid api token should respond with unprocessable_entity. (ApiTest::IssuesTest)
    [shoulda (2.10.3) lib/shoulda/assertions.rb:55:in `assert_accepts'
     shoulda (2.10.3) lib/shoulda/action_controller/macros.rb:124:in `__bind_1288975332_685249'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `call'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `test: POST /issues.json with failure should allow key based auth using key=X for post /issues.json with a valid api token should respond with unprocessable_entity. ']:
Expected response to be a 422, but was 401

  7) Failure:
test: POST /issues.xml should allow key based auth using key=X for post /issues.xml with a valid api token should login as the user. (ApiTest::IssuesTest)
    [/test/integration/api_test/../../test_helper.rb:339:in `__bind_1288975337_435557'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `call'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `test: POST /issues.xml should allow key based auth using key=X for post /issues.xml with a valid api token should login as the user. ']:
<#> expected but was
<#>.

  8) Failure:
test: POST /issues.xml should allow key based auth using key=X for post /issues.xml with a valid api token should respond with created. (ApiTest::IssuesTest)
    [shoulda (2.10.3) lib/shoulda/assertions.rb:55:in `assert_accepts'
     shoulda (2.10.3) lib/shoulda/action_controller/macros.rb:124:in `__bind_1288975337_674340'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `call'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `test: POST /issues.xml should allow key based auth using key=X for post /issues.xml with a valid api token should respond with created. ']:
Expected response to be a 201, but was 401

  9) Failure:
test: POST /issues.xml with failure should allow key based auth using key=X for post /issues.xml with a valid api token should login as the user. (ApiTest::IssuesTest)
    [/test/integration/api_test/../../test_helper.rb:339:in `__bind_1288975341_677792'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `call'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `test: POST /issues.xml with failure should allow key based auth using key=X for post /issues.xml with a valid api token should login as the user. ']:
<#> expected but was
<#>.

 10) Failure:
test: POST /issues.xml with failure should allow key based auth using key=X for post /issues.xml with a valid api token should respond with unprocessable_entity. (ApiTest::IssuesTest)
    [shoulda (2.10.3) lib/shoulda/assertions.rb:55:in `assert_accepts'
     shoulda (2.10.3) lib/shoulda/action_controller/macros.rb:124:in `__bind_1288975341_919793'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `call'
     shoulda (2.10.3) lib/shoulda/context.rb:362:in `test: POST /issues.xml with failure should allow key based auth using key=X for post /issues.xml with a valid api token should respond with unprocessable_entity. ']:
Expected response to be a 422, but was 401

265 tests, 277 assertions, 10 failures, 0 errors

Boom. 10 failures. All of them for the “key=X” authentication, just like what #6447 reported.

Fixing the key based authentication

Now that I have failing tests I can work on fixing the actual bug. I already know that Redmine uses a controller method called accept_key_auth to allow the key authentication so adding #create is a simple fix.

1
2
3
4

class IssuesController &lt; ApplicationController
-  accept_key_auth :index, :show
+  accept_key_auth :index, :show, :create
end
 
$ ruby test/integration/api_test/issues_test.rb
/usr/lib/ruby/gems/1.8/gems/rails-2.3.5/lib/rails/gem_dependency.rb:119:Warning: Gem::Dependency#version_requirements is deprecated and will be removed on or after August 2010.  Use #requirement
Loaded suite test/integration/api_test/issues_test
Started
.........................................................................................................................................................................................................................................................................
Finished in 53.92647 seconds.

265 tests, 277 assertions, 0 failures, 0 errors

Review

So now that I’ve fixed the #create method, it’s just a matter of repeating this for all of other API methods listed in #6447.

But why did I spent all week to add a whole bunch of tests for this when all it needed a single change?

My personal reason is because I see Redmine’s API becoming its major feature in 2011. I’ve worked on Redmine for dozens of companies now and every one of them has been wanting to do some deep integration of Redmine into their other systems (e.g. helpdesk, accounting, customer database). Having a functional and stable API for Redmine will make this easier. To have a stable API, Redmine needs to have a powerful set of tests to make sure the API works.

So for me, working on the tests for the API this week is to set me up for the work I’ll be doing all next year. And the work that I put into the API can be used by many other people to build on.

(Full commit)