Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Ajax Session loss Feature for 401 Standard Conformity #209

Merged
merged 8 commits into from
Jul 4, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion app/assets/javascripts/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -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-Authentication-Scheme', "Session");
});
// ajaxStop gets called when ALL Requests finish, so we won't need a counter as in PT
$(document).ajaxStop(function () {
Expand Down
12 changes: 11 additions & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,17 @@ 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" }

authentication_scheme = if request.headers["X-Authentication-Scheme"] == "Session"
'Session'
else
'Basic'
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming you didn't remove the "Reason" => "login needed" on purpose, I'd suggest this code

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"' }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I changed the code.

format.any(:xml, :js, :json) {
head :unauthorized,
"Reason" => "login needed",
'WWW-Authenticate' => authentication_scheme + ' realm="OpenProject API"'
}
end
return false
end
Expand Down
2 changes: 1 addition & 1 deletion doc/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
43 changes: 43 additions & 0 deletions test/integration/api_test/http_accept_auth_test.rb
Original file line number Diff line number Diff line change
@@ -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_send_correct_authentication_scheme_when_header_authentication_scheme_is_session(:get, "/api/v1/projects/onlinestore/news.xml")
end

context "in :json format" do
should_send_correct_authentication_scheme_when_header_authentication_scheme_is_session(:get, "/api/v1/projects/onlinestore/news.json")
end
end
end
48 changes: 48 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,43 @@ 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_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
send(http_method, url, parameters, { "X-Authentication-Scheme" => "Session" })
end

should respond_with failure_code
should_respond_with_content_type_based_on_url(url)
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_equal 'Session realm="OpenProject API"', @controller.response.headers['Www-Authenticate']
else
assert @controller.response.headers.has_key?('WWW-Authenticate')
assert_equal 'Session realm="OpenProject API"', @controller.response.headers['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)
Expand Down Expand Up @@ -333,6 +370,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

Expand Down