From bd3599856756d0bd957ff96fd4acec0a4b031ea7 Mon Sep 17 00:00:00 2001 From: Nils Kenneweg Date: Thu, 27 Jun 2013 16:21:54 +0200 Subject: [PATCH 1/8] Revert "removed test which is not valid anymore." This reverts commit 9ede533f443e37772e645b15691bbc88d5070e36. --- test/test_helper.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/test_helper.rb b/test/test_helper.rb index 9c67c0da63de..09d5370dbb97 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -333,6 +333,17 @@ def self.should_allow_http_basic_auth_with_username_and_password(http_method, ur should respond_with failure_code should_respond_with_content_type_based_on_url(url) + should "include_www_authenticate_header" do + # the 3.0.9 implementation of head leads to Www as the method capitalizes each + # word split by a hyphen. + # this is fixed in 3.1.0 http://apidock.com/rails/v3.1.0/ActionController/Head/head + # remove this switch once on 3.1.0 + if ::Rails::VERSION::MAJOR == 3 && ::Rails::VERSION::MINOR == 0 + assert @controller.response.headers.has_key?('Www-Authenticate') + else + assert @controller.response.headers.has_key?('WWW-Authenticate') + end + end end end From 22664cb7c91115cf6262ab4bf1f1c09cd604ab03 Mon Sep 17 00:00:00 2001 From: Nils Kenneweg Date: Thu, 27 Jun 2013 16:37:40 +0200 Subject: [PATCH 2/8] 401 standard conformity --- app/assets/javascripts/application.js | 4 +++- app/controllers/application_controller.rb | 7 ++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 38059333c1e0..1e94b7ddaae7 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -574,8 +574,10 @@ jQuery(document).ready(function($) { var header = 'X-CSRF-Token', token = csrf_meta_tag.attr('content'); - request.setRequestHeader[header] = token; + request.setRequestHeader(header, token); } + + request.setRequestHeader('X-ACCEPT-AUTH', "Session"); }); // ajaxStop gets called when ALL Requests finish, so we won't need a counter as in PT $(document).ajaxStop(function () { diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index d28ab13110fb..d5a05671d32a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -186,7 +186,12 @@ def require_login end respond_to do |format| format.any(:html, :atom) { redirect_to signin_path(:back_url => url) } - format.any(:xml, :js, :json) { head :unauthorized, "Reason" => "login needed" } + + if request.headers["X-ACCEPT-AUTH"] == "Session" + format.any(:xml, :js, :json) { head :unauthorized, "Reason" => "login needed", 'WWW-Authenticate' => 'Session realm="OpenProject API"' } + else + format.any(:xml, :js, :json) { head :unauthorized, 'WWW-Authenticate' => 'Basic realm="OpenProject API"' } + end end return false end From 05003493e254885a829c622c787677ed0d426394 Mon Sep 17 00:00:00 2001 From: Nils Kenneweg Date: Thu, 27 Jun 2013 17:26:34 +0200 Subject: [PATCH 3/8] added reason to api. --- app/controllers/application_controller.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index d5a05671d32a..f3628c134dba 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -187,11 +187,12 @@ def require_login respond_to do |format| format.any(:html, :atom) { redirect_to signin_path(:back_url => url) } - if request.headers["X-ACCEPT-AUTH"] == "Session" - format.any(:xml, :js, :json) { head :unauthorized, "Reason" => "login needed", 'WWW-Authenticate' => 'Session realm="OpenProject API"' } - else - format.any(:xml, :js, :json) { head :unauthorized, 'WWW-Authenticate' => 'Basic realm="OpenProject API"' } + authentication_scheme = if request.headers["X-ACCEPT-AUTH"] == "Session" + 'Session' else 'Basic' end + format.any(:xml, :js, :json) { head :unauthorized, + "Reason" => "login needed", + 'WWW-Authenticate' => authentication_scheme + ' realm="OpenProject API"' } end return false end From 0c220c38726831a10f78b1d5eb69abd3a3866b8d Mon Sep 17 00:00:00 2001 From: Nils Kenneweg Date: Thu, 27 Jun 2013 17:36:31 +0200 Subject: [PATCH 4/8] test for the correct response. --- .../api_test/http_accept_auth_test.rb | 43 +++++++++++++++++++ test/test_helper.rb | 36 ++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 test/integration/api_test/http_accept_auth_test.rb diff --git a/test/integration/api_test/http_accept_auth_test.rb b/test/integration/api_test/http_accept_auth_test.rb new file mode 100644 index 000000000000..c2d3157800fa --- /dev/null +++ b/test/integration/api_test/http_accept_auth_test.rb @@ -0,0 +1,43 @@ +#-- encoding: UTF-8 +#-- copyright +# OpenProject is a project management system. +# +# Copyright (C) 2012-2013 the OpenProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# See doc/COPYRIGHT.rdoc for more details. +#++ + +require File.expand_path('../../../test_helper', __FILE__) + +class ApiTest::HttpAcceptAuthTest < ActionDispatch::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 + setup do + project = Project.find('onlinestore') + EnabledModule.create(:project => project, :name => 'news') + end + + context "in :xml format" do + should_not_send_www_authenticate_when_header_accept_auth_is_session(:get, "/api/v1/projects/onlinestore/news.xml") + end + + context "in :json format" do + should_not_send_www_authenticate_when_header_accept_auth_is_session(:get, "/api/v1/projects/onlinestore/news.json") + end + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 09d5370dbb97..5a30b1b6bdf2 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -285,6 +285,42 @@ def self.should_allow_api_authentication(http_method, url, parameters={}, option should_allow_key_based_auth(http_method, url, parameters, options) 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) + # @param [String] url the request url + # @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_not_send_www_authenticate_when_header_accept_auth_is_session(http_method, url, options) + success_code = options[:success_code] || :success + failure_code = options[:failure_code] || :unauthorized + + context "should not send www authenticate when header accept auth is session #{http_method} #{url}" do + context "without credentials" do + setup do + parameters = { "X-ACCEPT-AUTH" => "Session" } + send(http_method, url, parameters) + end + + should respond_with failure_code + should_respond_with_content_type_based_on_url(url) + should "not include_www_authenticate_header" do + # the 3.0.9 implementation of head leads to Www as the method capitalizes each + # word split by a hyphen. + # this is fixed in 3.1.0 http://apidock.com/rails/v3.1.0/ActionController/Head/head + # remove this switch once on 3.1.0 + if ::Rails::VERSION::MAJOR == 3 && ::Rails::VERSION::MINOR == 0 + assert !@controller.response.headers.has_key?('Www-Authenticate') + else + assert !@controller.response.headers.has_key?('WWW-Authenticate') + end + end + end + end + + 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) From 78c1bd075c0a8e0c9a4f581793b049a880e749d8 Mon Sep 17 00:00:00 2001 From: Nils Kenneweg Date: Fri, 28 Jun 2013 09:42:53 +0200 Subject: [PATCH 5/8] rename X-ACCEPT-AUTH=X-Authentication-Scheme, test --- app/assets/javascripts/application.js | 2 +- app/controllers/application_controller.rb | 2 +- test/test_helper.rb | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 1e94b7ddaae7..138e35d3b0bb 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -577,7 +577,7 @@ jQuery(document).ready(function($) { request.setRequestHeader(header, token); } - request.setRequestHeader('X-ACCEPT-AUTH', "Session"); + request.setRequestHeader('X-Authentication-Scheme', "Session"); }); // ajaxStop gets called when ALL Requests finish, so we won't need a counter as in PT $(document).ajaxStop(function () { diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index f3628c134dba..22881f289fed 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -187,7 +187,7 @@ def require_login respond_to do |format| format.any(:html, :atom) { redirect_to signin_path(:back_url => url) } - authentication_scheme = if request.headers["X-ACCEPT-AUTH"] == "Session" + authentication_scheme = if request.headers["X-Authentication-Scheme"] == "Session" 'Session' else 'Basic' end format.any(:xml, :js, :json) { head :unauthorized, diff --git a/test/test_helper.rb b/test/test_helper.rb index 5a30b1b6bdf2..d55f1616799e 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -292,14 +292,14 @@ def self.should_allow_api_authentication(http_method, url, parameters={}, option # @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_not_send_www_authenticate_when_header_accept_auth_is_session(http_method, url, options) + def self.should_not_send_www_authenticate_when_header_accept_auth_is_session(http_method, url, options = {}) success_code = options[:success_code] || :success failure_code = options[:failure_code] || :unauthorized context "should not send www authenticate when header accept auth is session #{http_method} #{url}" do context "without credentials" do setup do - parameters = { "X-ACCEPT-AUTH" => "Session" } + parameters = { "X-Authentication-Scheme" => "Session" } send(http_method, url, parameters) end From b04fab00fedfa72cba32b475dff06cf89b2f5b2f Mon Sep 17 00:00:00 2001 From: Nils Kenneweg Date: Fri, 28 Jun 2013 12:01:42 +0200 Subject: [PATCH 6/8] test bugfix. --- test/test_helper.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/test_helper.rb b/test/test_helper.rb index d55f1616799e..3ea0591d0535 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -292,7 +292,7 @@ def self.should_allow_api_authentication(http_method, url, parameters={}, option # @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_not_send_www_authenticate_when_header_accept_auth_is_session(http_method, url, options = {}) + def self.should_send_correct_authentication_scheme_when_header_authentication_scheme_is_session(http_method, url, options = {}) success_code = options[:success_code] || :success failure_code = options[:failure_code] || :unauthorized @@ -305,15 +305,17 @@ def self.should_not_send_www_authenticate_when_header_accept_auth_is_session(htt should respond_with failure_code should_respond_with_content_type_based_on_url(url) - should "not include_www_authenticate_header" do + should "include correct www_authenticate_header" do # the 3.0.9 implementation of head leads to Www as the method capitalizes each # word split by a hyphen. # this is fixed in 3.1.0 http://apidock.com/rails/v3.1.0/ActionController/Head/head # remove this switch once on 3.1.0 if ::Rails::VERSION::MAJOR == 3 && ::Rails::VERSION::MINOR == 0 - assert !@controller.response.headers.has_key?('Www-Authenticate') + assert @controller.response.headers.has_key?('Www-Authenticate') + assert_equal @controller.response.headers['Www-Authenticate'], 'Session realm="OpenProject API"' else - assert !@controller.response.headers.has_key?('WWW-Authenticate') + assert @controller.response.headers.has_key?('WWW-Authenticate') + assert_equal @controller.response.headers['WWW-Authenticate'], 'Session realm="OpenProject API"' end end end From 16d694a39e92d9ab09eacf10c46583e3c984c401 Mon Sep 17 00:00:00 2001 From: Nils Kenneweg Date: Thu, 4 Jul 2013 10:42:30 +0200 Subject: [PATCH 7/8] fixed tests. --- test/integration/api_test/http_accept_auth_test.rb | 4 ++-- test/test_helper.rb | 9 ++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/test/integration/api_test/http_accept_auth_test.rb b/test/integration/api_test/http_accept_auth_test.rb index c2d3157800fa..097a42407537 100644 --- a/test/integration/api_test/http_accept_auth_test.rb +++ b/test/integration/api_test/http_accept_auth_test.rb @@ -33,11 +33,11 @@ def teardown end context "in :xml format" do - should_not_send_www_authenticate_when_header_accept_auth_is_session(:get, "/api/v1/projects/onlinestore/news.xml") + should_send_correct_authentication_scheme_when_header_authentication_scheme_is_session(:get, "/api/v1/projects/onlinestore/news.xml") end context "in :json format" do - should_not_send_www_authenticate_when_header_accept_auth_is_session(:get, "/api/v1/projects/onlinestore/news.json") + should_send_correct_authentication_scheme_when_header_authentication_scheme_is_session(:get, "/api/v1/projects/onlinestore/news.json") end end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 3ea0591d0535..25ebf60ccdd3 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -292,15 +292,14 @@ def self.should_allow_api_authentication(http_method, url, parameters={}, option # @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_send_correct_authentication_scheme_when_header_authentication_scheme_is_session(http_method, url, options = {}) + def self.should_send_correct_authentication_scheme_when_header_authentication_scheme_is_session(http_method, url, options = {}, parameters = {}) success_code = options[:success_code] || :success failure_code = options[:failure_code] || :unauthorized context "should not send www authenticate when header accept auth is session #{http_method} #{url}" do context "without credentials" do setup do - parameters = { "X-Authentication-Scheme" => "Session" } - send(http_method, url, parameters) + send(http_method, url, parameters, { "X-Authentication-Scheme" => "Session" }) end should respond_with failure_code @@ -312,10 +311,10 @@ def self.should_send_correct_authentication_scheme_when_header_authentication_sc # remove this switch once on 3.1.0 if ::Rails::VERSION::MAJOR == 3 && ::Rails::VERSION::MINOR == 0 assert @controller.response.headers.has_key?('Www-Authenticate') - assert_equal @controller.response.headers['Www-Authenticate'], 'Session realm="OpenProject API"' + assert_equal 'Session realm="OpenProject API"', @controller.response.headers['Www-Authenticate'] else assert @controller.response.headers.has_key?('WWW-Authenticate') - assert_equal @controller.response.headers['WWW-Authenticate'], 'Session realm="OpenProject API"' + assert_equal 'Session realm="OpenProject API"', @controller.response.headers['WWW-Authenticate'] end end end From 45534694e04e418478729ed289e08323c9b8114a Mon Sep 17 00:00:00 2001 From: Nils Kenneweg Date: Thu, 4 Jul 2013 13:57:13 +0200 Subject: [PATCH 8/8] intendation + changelog. --- app/controllers/application_controller.rb | 10 +++++++--- doc/CHANGELOG.md | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 22881f289fed..56e49671bbce 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -188,11 +188,15 @@ def require_login format.any(:html, :atom) { redirect_to signin_path(:back_url => url) } authentication_scheme = if request.headers["X-Authentication-Scheme"] == "Session" - 'Session' else 'Basic' + 'Session' + else + 'Basic' end - format.any(:xml, :js, :json) { head :unauthorized, + format.any(:xml, :js, :json) { + head :unauthorized, "Reason" => "login needed", - 'WWW-Authenticate' => authentication_scheme + ' realm="OpenProject API"' } + 'WWW-Authenticate' => authentication_scheme + ' realm="OpenProject API"' + } end return false end diff --git a/doc/CHANGELOG.md b/doc/CHANGELOG.md index f4d8f6d88f41..e5c4a1398a55 100644 --- a/doc/CHANGELOG.md +++ b/doc/CHANGELOG.md @@ -2,4 +2,4 @@ * `#1209` Fix adding watcher to issue * `#1034` Create changelog and document format - +* `#1301` Ajax call when logged out should open a popup window