From d63229a5b4b6be96f988cb48886a8915b5ab86e1 Mon Sep 17 00:00:00 2001 From: Jonas Heinrich Date: Mon, 11 Aug 2014 13:57:02 +0200 Subject: [PATCH 01/56] makes the workflow section more readable --- lib/redmine/default_data/loader.rb | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/lib/redmine/default_data/loader.rb b/lib/redmine/default_data/loader.rb index adad36b3d5f..b1657fca7be 100644 --- a/lib/redmine/default_data/loader.rb +++ b/lib/redmine/default_data/loader.rb @@ -212,21 +212,19 @@ module Redmine rejected = Status.create!(name: l(:default_status_rejected), is_closed: true, is_default: false, position: 9) closed = Status.create!(name: l(:default_status_closed), is_closed: true, is_default: false, position: 10) - # Workflow - statuses_for_task = [new, in_progress, on_hold, rejected, closed] - statuses_for_deliverable = [new, specified, in_progress, on_hold, rejected, closed] - statuses_for_none = [new, in_progress, rejected, closed] - statuses_for_milestone = [new, to_be_scheduled, scheduled, in_progress, on_hold, rejected, closed] - statuses_for_phase = statuses_for_milestone - statuses_for_bug = [new, confirmed, in_progress, tested, on_hold, rejected, closed] - statuses_for_feature = [new, specified, confirmed, in_progress, tested, on_hold, rejected, closed] - # Give each type its own workflow. Possible statuses are stored in one of the arrays above. - # Every status from the array gets a workflow to every other status from the array. - ['task', 'deliverable', 'none', 'milestone', 'phase', 'bug', 'feature'].each { |t| - (eval 'statuses_for_'.concat(t)).each { |os| - (eval 'statuses_for_'.concat(t)).each { |ns| + # Workflow - Each type has its own workflow + statuses = { task.name => [new, in_progress, on_hold, rejected, closed], + deliverable.name => [new, specified, in_progress, on_hold, rejected, closed], + none.name => [new, in_progress, rejected, closed], + milestone.name => [new, to_be_scheduled, scheduled, in_progress, on_hold, rejected, closed], + phase.name => [new, to_be_scheduled, scheduled, in_progress, on_hold, rejected, closed], + bug.name => [new, confirmed, in_progress, tested, on_hold, rejected, closed], + feature.name => [new, specified, confirmed, in_progress, tested, on_hold, rejected, closed] } + statuses.each { |t, statuses_for_t| + statuses_for_t.each { |os| + statuses_for_t.each { |ns| [manager.id, member.id].each { |role_id| - Workflow.create!(type_id: (eval t).id, role_id: role_id, old_status_id: os.id, new_status_id: ns.id) unless os == ns + Workflow.create!(:type_id => Type.where(:name => t).first.id, :role_id => role_id, :old_status_id => os.id, :new_status_id => ns.id) unless os == ns } } } From 6cb6953a07bd9e74e9f55b73aab979a91c6153d2 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Tue, 5 May 2015 18:51:30 +0100 Subject: [PATCH 02/56] basic auth for api v3 (WIP) --- Gemfile | 3 + app/models/user.rb | 2 +- config/initializers/warden.rb | 7 ++ lib/api/root.rb | 18 +++- lib/core_ext/hash/hash_navigation.rb | 35 +++++++ lib/open_project.rb | 2 + .../authentication/failure_app.rb | 23 +++++ lib/open_project/authentication/manager.rb | 42 ++++++++ lib/warden/strategies/global_basic_auth.rb | 32 +++++++ lib/warden/strategies/session.rb | 26 +++++ lib/warden/strategies/user_basic_auth.rb | 13 +++ spec/lib/api/v3/authentication_spec.rb | 96 +++++++++++++++++++ 12 files changed, 294 insertions(+), 5 deletions(-) create mode 100644 config/initializers/warden.rb create mode 100644 lib/core_ext/hash/hash_navigation.rb create mode 100644 lib/open_project/authentication/failure_app.rb create mode 100644 lib/open_project/authentication/manager.rb create mode 100644 lib/warden/strategies/global_basic_auth.rb create mode 100644 lib/warden/strategies/session.rb create mode 100644 lib/warden/strategies/user_basic_auth.rb create mode 100644 spec/lib/api/v3/authentication_spec.rb diff --git a/Gemfile b/Gemfile index b80a4b62296..2fd044066e7 100644 --- a/Gemfile +++ b/Gemfile @@ -38,6 +38,9 @@ gem 'omniauth' gem 'request_store', "~> 1.1.0" gem 'gravatar_image_tag', '~> 1.2.0' +gem 'warden', '~> 1.2' +gem 'warden-basic_auth', '~> 0.1.0' + # TODO: adds #auto_link which was deprecated in rails 3.1 gem 'rails_autolink', '~> 1.1.6' gem "will_paginate", '~> 3.0' diff --git a/app/models/user.rb b/app/models/user.rb index 872af182d4f..c7774e6e0b3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -738,7 +738,7 @@ class User < Principal end def self.system - system_user = SystemUser.find(:first) + system_user = SystemUser.first if system_user.nil? (system_user = SystemUser.new.tap do |u| u.lastname = 'System' diff --git a/config/initializers/warden.rb b/config/initializers/warden.rb new file mode 100644 index 00000000000..b6182a9bd1f --- /dev/null +++ b/config/initializers/warden.rb @@ -0,0 +1,7 @@ +require 'warden/strategies/global_basic_auth' +require 'warden/strategies/user_basic_auth' +require 'warden/strategies/session' + +Warden::Strategies.add :global_basic_auth, Warden::Strategies::GlobalBasicAuth +Warden::Strategies.add :user_basic_auth, Warden::Strategies::UserBasicAuth +Warden::Strategies.add :session, Warden::Strategies::Session diff --git a/lib/api/root.rb b/lib/api/root.rb index 1d49796106a..b5b5f96920d 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -31,6 +31,8 @@ # This is the place for all API wide configuration, helper methods, exceptions # rescuing, mounting of differnet API versions etc. +require 'open_project/authentication/manager' + module API class Root < Grape::API prefix :api @@ -59,15 +61,23 @@ module API parser :json, Parser.new + use OpenProject::Authentication::Manager + helpers do def current_user - return User.current if running_in_test_env? - user_id = env['rack.session']['user_id'] - User.current = user_id ? User.find(user_id) : User.anonymous + User.current + end + + def warden + env['warden'] end def authenticate - if Setting.login_required? && (current_user.nil? || current_user.anonymous?) + warden.authenticate! scope: :api_v3 + + User.current = warden.user scope: :api_v3 + + if Setting.login_required? && (current_user.nil? || (!current_user.admin? && current_user.anonymous?)) raise API::Errors::Unauthenticated end end diff --git a/lib/core_ext/hash/hash_navigation.rb b/lib/core_ext/hash/hash_navigation.rb new file mode 100644 index 00000000000..35ba32d3993 --- /dev/null +++ b/lib/core_ext/hash/hash_navigation.rb @@ -0,0 +1,35 @@ +module HashNavigation + def fetch?(*keys) + value = fetch_path keys, self + + if value + value + elsif block_given? + yield + else + nil + end + end + + module_function + + def fetch_path(keys, hash, default = nil) + if keys.size <= 1 + fetch_value keys.first, hash, default + else + head, *tail = keys + + fetch_path tail, fetch_hash(head, hash), default + end + end + + def fetch_hash(key, hash) + ActiveSupport::HashWithIndifferentAccess.new fetch_value(key, hash) + end + + def fetch_value(key, hash, default = nil) + hash[key] || default + end +end + +Hash.include HashNavigation diff --git a/lib/open_project.rb b/lib/open_project.rb index 691bf11802a..fe808cfb842 100644 --- a/lib/open_project.rb +++ b/lib/open_project.rb @@ -41,3 +41,5 @@ require 'redmine/notifiable' require 'csv' require 'globalize' + +require 'core_ext/hash/hash_navigation' diff --git a/lib/open_project/authentication/failure_app.rb b/lib/open_project/authentication/failure_app.rb new file mode 100644 index 00000000000..e7a17a69ffc --- /dev/null +++ b/lib/open_project/authentication/failure_app.rb @@ -0,0 +1,23 @@ +module OpenProject + module Authentication + class FailureApp + def call(env) + warden = env['warden'] + + if warden.present? && warden.result == :failure + wrong_credentials warden.message, headers: warden.headers + else + unauthorized + end + end + + def wrong_credentials(message, headers: {}) + [401, headers, [message]] + end + + def unauthorized + [401, {}, []] + end + end + end +end diff --git a/lib/open_project/authentication/manager.rb b/lib/open_project/authentication/manager.rb new file mode 100644 index 00000000000..d741ba96cf5 --- /dev/null +++ b/lib/open_project/authentication/manager.rb @@ -0,0 +1,42 @@ +module OpenProject + module Authentication + class Manager < Warden::Manager + def self.strategies + @strategies ||= begin + hash = { + api_v3: [:global_basic_auth, :user_basic_auth, :session] + } + + hash.tap do |h| + h.default = [] + end + end + end + + def self.register_strategy(name, clazz, scopes) + Warden::Strategies.add name, clazz + + scopes.each do |scope| + strategies[scope] << name + end + end + + def self.configure(config) + config.default_strategies :session + config.failure_app = OpenProject::Authentication::FailureApp + + config.scope_defaults :api_v3, strategies: strategies[:api_v3], store: false + end + + def initialize(app, options={}, &configure) + block = lambda do |config| + self.class.configure config + + configure.call config if configure + end + + super app, options, &block + end + end + end +end diff --git a/lib/warden/strategies/global_basic_auth.rb b/lib/warden/strategies/global_basic_auth.rb new file mode 100644 index 00000000000..743cd144fea --- /dev/null +++ b/lib/warden/strategies/global_basic_auth.rb @@ -0,0 +1,32 @@ +require 'warden/basic_auth' + +module Warden + module Strategies + class GlobalBasicAuth < BasicAuth + def self.configuration + config = Hash(OpenProject::Configuration['authentication']) + + user = config.fetch? 'global_basic_auth', 'user' + password = config.fetch? 'global_basic_auth', 'password' + + { user: user, password: password } if user && password + end + + ## + # Only valid if global basic auth is configured. + def valid? + self.class.configuration && super + end + + def authenticate_user(username, password) + config = self.class.configuration + + if username == config[:user] && password == config[:password] + User.system.tap do |user| + user.admin = true + end + end + end + end + end +end diff --git a/lib/warden/strategies/session.rb b/lib/warden/strategies/session.rb new file mode 100644 index 00000000000..4b65d656681 --- /dev/null +++ b/lib/warden/strategies/session.rb @@ -0,0 +1,26 @@ +module Warden + module Strategies + ## + # Temporary strategy necessary as long as the OpenProject authentication has not been unified + # in terms of Warden strategies and is only locally applied to the API v3. + class Session < Base + def valid? + session + end + + def authenticate! + user = user_id ? User.find(user_id) : User.anonymous + + success! user + end + + def user_id + Hash(session)['user_id'] + end + + def session + env['rack.session'] + end + end + end +end diff --git a/lib/warden/strategies/user_basic_auth.rb b/lib/warden/strategies/user_basic_auth.rb new file mode 100644 index 00000000000..7f896af6650 --- /dev/null +++ b/lib/warden/strategies/user_basic_auth.rb @@ -0,0 +1,13 @@ +require 'warden/basic_auth' + +module Warden + module Strategies + class UserBasicAuth < BasicAuth + def authenticate_user(username, password) + user = User.find_by_login username + + user if user && user.check_password?(password) + end + end + end +end diff --git a/spec/lib/api/v3/authentication_spec.rb b/spec/lib/api/v3/authentication_spec.rb new file mode 100644 index 00000000000..7713724cb33 --- /dev/null +++ b/spec/lib/api/v3/authentication_spec.rb @@ -0,0 +1,96 @@ +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2015 the OpenProject Foundation (OPF) +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See doc/COPYRIGHT.rdoc for more details. +#++ + +require 'spec_helper' + +describe API::V3, type: :request do + describe 'basic auth' do + let(:user) { FactoryGirl.create :user } + let(:resource) { "/api/v3/users/#{user.id}"} + + def basic_auth(user, password) + credentials = ActionController::HttpAuthentication::Basic.encode_credentials user, password + {'HTTP_AUTHORIZATION' => credentials} + end + + shared_examples 'it is basic auth protected' do + context 'without credentials' do + before do + get resource + end + + it 'should return 401 unauthorized' do + expect(response.status).to eq 401 + end + end + + context 'with invalid credentials' do + before do + get resource, {}, basic_auth(username, password.reverse) + end + + it 'should return 401 unauthorized' do + expect(response.status).to eq 401 + end + end + + context 'with valid credentials' do + before do + get resource, {}, basic_auth(username, password) + end + + it 'should return 200 OK' do + expect(response.status).to eq 200 + end + end + end + + context 'with global basic auth configured' do + let(:username) { 'root' } + let(:password) { 'toor' } + + before do + authentication = { + 'global_basic_auth' => { + 'user' => username, + 'password' => password + } + } + OpenProject::Configuration['authentication'] = authentication + end + + context 'with login required' do + before do + Setting.login_required = 1 + end + + it_behaves_like 'it is basic auth protected' + end + end + end +end From 0cf9668f6b23912321e1752f51953366690ccc22 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Wed, 6 May 2015 13:12:32 +0100 Subject: [PATCH 03/56] more specs; don't halt on failed global basic auth --- lib/warden/strategies/global_basic_auth.rb | 12 +- spec/lib/api/v3/authentication_spec.rb | 122 ++++++++++++++++++--- 2 files changed, 119 insertions(+), 15 deletions(-) diff --git a/lib/warden/strategies/global_basic_auth.rb b/lib/warden/strategies/global_basic_auth.rb index 743cd144fea..60be52d6da3 100644 --- a/lib/warden/strategies/global_basic_auth.rb +++ b/lib/warden/strategies/global_basic_auth.rb @@ -12,10 +12,18 @@ module Warden { user: user, password: password } if user && password end + def self.user + configuration[:user] + end + + def self.password + configuration[:password] + end + ## - # Only valid if global basic auth is configured. + # Only valid if global basic auth is configured and tried. def valid? - self.class.configuration && super + self.class.configuration && super && username == self.class.user end def authenticate_user(username, password) diff --git a/spec/lib/api/v3/authentication_spec.rb b/spec/lib/api/v3/authentication_spec.rb index 7713724cb33..17499b58a36 100644 --- a/spec/lib/api/v3/authentication_spec.rb +++ b/spec/lib/api/v3/authentication_spec.rb @@ -70,26 +70,122 @@ describe API::V3, type: :request do end end - context 'with global basic auth configured' do - let(:username) { 'root' } - let(:password) { 'toor' } - + context 'with login required' do before do - authentication = { - 'global_basic_auth' => { - 'user' => username, - 'password' => password - } - } - OpenProject::Configuration['authentication'] = authentication + Setting.login_required = 1 end - context 'with login required' do + context 'with global basic auth configured' do + let(:username) { 'root' } + let(:password) { 'toor' } + before do - Setting.login_required = 1 + authentication = { + 'global_basic_auth' => { + 'user' => 'root', + 'password' => 'toor' + } + } + OpenProject::Configuration['authentication'] = authentication end it_behaves_like 'it is basic auth protected' + + describe 'user basic auth' do + let(:username) { 'hans' } + let(:password) { 'bambidibam' } + + let!(:api_user) do + FactoryGirl.create :user, + login: username, + password: password, + password_confirmation: password + end + + # check that user basic auth is tried when global basic auth fails + it_behaves_like 'it is basic auth protected' + end + end + end + + context 'without login required' do + before do + Setting.login_required = 0 + end + + context 'with global and user basic auth enabled' do + let(:username) { 'hancholo' } + let(:password) { 'olooleol' } + + let!(:api_user) do + FactoryGirl.create :user, + login: 'user_account', + password: 'user_password', + password_confirmation: 'user_password' + end + + before do + authentication = { + 'global_basic_auth' => { + 'user' => 'global_account', + 'password' => 'global_password' + } + } + OpenProject::Configuration['authentication'] = authentication + end + + context 'without credentials' do + before do + get resource + end + + it 'should return 200 OK' do + expect(response.status).to eq 200 + end + + it 'should "login" the anonymous user' do + expect(User.current).to be_anonymous + end + end + + context 'with invalid credentials' do + before do + get resource, {}, basic_auth(username, password) + end + + it 'should return 401 unauthorized' do + expect(response.status).to eq 401 + end + end + + context 'with valid global credentials' do + before do + get resource, {}, basic_auth('global_account', 'global_password') + end + + it 'should return 200 OK' do + expect(response.status).to eq 200 + end + + it 'should login an admin system user' do + expect(User.current.is_a?(SystemUser)).to eq true + expect(User.current).to be_admin + end + end + + context 'with valid user credentials' do + before do + get resource, {}, basic_auth('user_account', 'user_password') + end + + it 'should return 200 OK' do + expect(response.status).to eq 200 + end + + it 'should login user' do + expect(User.current).to eq api_user + end + end end end end From 2b8848dc8b16d73df241544b1e8e5f32f58faf45 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Wed, 6 May 2015 13:16:38 +0100 Subject: [PATCH 04/56] don't use deprecated finder --- app/models/system_user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/system_user.rb b/app/models/system_user.rb index b2388eb6274..83da13de78f 100644 --- a/app/models/system_user.rb +++ b/app/models/system_user.rb @@ -62,7 +62,7 @@ class SystemUser < User # There should be only one SystemUser in the database def validate_unique_system_user - errors.add :base, 'A SystemUser already exists.' if SystemUser.find(:first) + errors.add :base, 'A SystemUser already exists.' if SystemUser.first end # Overrides a few properties From d4dfdb9dcc83a2310c7276694c5d25dbc39b840b Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Thu, 7 May 2015 11:51:20 +0100 Subject: [PATCH 05/56] use API keys for user basic auth; refactoring --- config/initializers/warden.rb | 21 ++++++++-- lib/open_project/authentication.rb | 46 +++++++++++++++++++++ lib/open_project/authentication/manager.rb | 45 ++++++++++---------- lib/warden/strategies/basic_auth_failure.rb | 12 ++++++ lib/warden/strategies/global_basic_auth.rb | 14 +++++++ lib/warden/strategies/user_basic_auth.rb | 23 +++++++++-- spec/factories/token_factory.rb | 13 +++++- spec/lib/api/v3/authentication_spec.rb | 21 +++------- 8 files changed, 151 insertions(+), 44 deletions(-) create mode 100644 lib/open_project/authentication.rb create mode 100644 lib/warden/strategies/basic_auth_failure.rb diff --git a/config/initializers/warden.rb b/config/initializers/warden.rb index b6182a9bd1f..99ffe612913 100644 --- a/config/initializers/warden.rb +++ b/config/initializers/warden.rb @@ -1,7 +1,22 @@ +# Strategies provided by OpenProject: +require 'warden/strategies/basic_auth_failure' require 'warden/strategies/global_basic_auth' require 'warden/strategies/user_basic_auth' require 'warden/strategies/session' -Warden::Strategies.add :global_basic_auth, Warden::Strategies::GlobalBasicAuth -Warden::Strategies.add :user_basic_auth, Warden::Strategies::UserBasicAuth -Warden::Strategies.add :session, Warden::Strategies::Session +strategies = [ + [:basic_auth_failure, Warden::Strategies::BasicAuthFailure], + [:global_basic_auth, Warden::Strategies::GlobalBasicAuth], + [:user_basic_auth, Warden::Strategies::UserBasicAuth], + [:session, Warden::Strategies::Session] +] + +strategies.each do |name, clazz| + Warden::Strategies.add name, clazz +end + +include OpenProject::Authentication::Scope + +OpenProject::Authentication.update_strategies(API_V3) do |_strategies| + [:global_basic_auth, :user_basic_auth, :basic_auth_failure, :session] +end diff --git a/lib/open_project/authentication.rb b/lib/open_project/authentication.rb new file mode 100644 index 00000000000..95e8eb71e8d --- /dev/null +++ b/lib/open_project/authentication.rb @@ -0,0 +1,46 @@ +require 'open_project/authentication/manager' + +module OpenProject + ## + # OpenProject uses Warden strategies for request authentication. + module Authentication + class << self + ## + # Updates the used warden strategies for a given scope. The strategies will be tried + # in the order they are set here. + # For available scopes please refer to `OpenProject::Authentication::Scope`. + # + # @param [Symbol] scope The scope for which to update the used warden strategies. + # @param [Boolean] store Indicates whether the user should be stored in the session for this scope. + # + # @yield [strategies] A block returning the strategies to be used for this scope. + # @yieldparam [Array] The strategies currently used by this scope. May be empty. + # @yieldreturn [Array] The strategies to be used by this scope. + def update_strategies(scope, store: nil, &block) + raise ArgumentError, "invalid scope: #{scope}" unless Scope.values.include? scope + + current_strategies = Array(Manager.scope_strategies[scope]) + + Manager.store_defaults[scope] = store unless store.nil? + Manager.scope_strategies[scope] = block.call current_strategies if block_given? + end + end + + ## + # This module is only there to declare all used scopes. Technically a scope can be an + # arbitrary symbol. But we declare them here not to lose sight of them. + # + # Plugins can declare new scopes by declaring new constants in this module. + module Scope + API_V3 = :api_v3 + + class << self + def values + constants.map do |name| + const_get name + end + end + end + end + end +end diff --git a/lib/open_project/authentication/manager.rb b/lib/open_project/authentication/manager.rb index d741ba96cf5..6cd1a9bb9e9 100644 --- a/lib/open_project/authentication/manager.rb +++ b/lib/open_project/authentication/manager.rb @@ -1,31 +1,13 @@ module OpenProject module Authentication class Manager < Warden::Manager - def self.strategies - @strategies ||= begin - hash = { - api_v3: [:global_basic_auth, :user_basic_auth, :session] - } - hash.tap do |h| - h.default = [] - end - end + serialize_into_session do |user| + user.id end - def self.register_strategy(name, clazz, scopes) - Warden::Strategies.add name, clazz - - scopes.each do |scope| - strategies[scope] << name - end - end - - def self.configure(config) - config.default_strategies :session - config.failure_app = OpenProject::Authentication::FailureApp - - config.scope_defaults :api_v3, strategies: strategies[:api_v3], store: false + serialize_from_session do |id| + User.find id end def initialize(app, options={}, &configure) @@ -37,6 +19,25 @@ module OpenProject super app, options, &block end + + class << self + def scope_strategies + @scope_strategies ||= {} + end + + def store_defaults + @store_defaults ||= Hash.new false + end + + def configure(config) + config.default_strategies :session + config.failure_app = OpenProject::Authentication::FailureApp + + scope_strategies.each do |scope, strategies| + config.scope_defaults scope, strategies: strategies, store: store_defaults[scope] + end + end + end end end end diff --git a/lib/warden/strategies/basic_auth_failure.rb b/lib/warden/strategies/basic_auth_failure.rb new file mode 100644 index 00000000000..c4d2a727503 --- /dev/null +++ b/lib/warden/strategies/basic_auth_failure.rb @@ -0,0 +1,12 @@ +module Warden + module Strategies + ## + # This strategy is inserted after optional basic auth strategies to + # indicate that invalid basic auth credentials were provided. + class BasicAuthFailure < BasicAuth + def authenticate_user(_username, _password) + nil # always fails + end + end + end +end diff --git a/lib/warden/strategies/global_basic_auth.rb b/lib/warden/strategies/global_basic_auth.rb index 60be52d6da3..c440bb289db 100644 --- a/lib/warden/strategies/global_basic_auth.rb +++ b/lib/warden/strategies/global_basic_auth.rb @@ -2,6 +2,20 @@ require 'warden/basic_auth' module Warden module Strategies + ## + # Allows authentication via a singular set of basic auth credentials for admin access. + # + # The credentials must be configured in `config/configuration.yml` like this: + # + # production: + # authentication: + # global_basic_auth: + # user: admin + # password: 123456 + # + # The strategy will only be triggered when the configured user name is sent. + # Meaning that this strategy is skipped if a basic auth attempt involving any + # other user name is made. class GlobalBasicAuth < BasicAuth def self.configuration config = Hash(OpenProject::Configuration['authentication']) diff --git a/lib/warden/strategies/user_basic_auth.rb b/lib/warden/strategies/user_basic_auth.rb index 7f896af6650..be3e81ffcb2 100644 --- a/lib/warden/strategies/user_basic_auth.rb +++ b/lib/warden/strategies/user_basic_auth.rb @@ -2,11 +2,28 @@ require 'warden/basic_auth' module Warden module Strategies + ## + # Allows users to authenticate using their API key via basic auth. + # Note that in order for a user to be able to generate one + # `Setting.rest_api_enabled` has to be `1`. + # + # The basic auth credentials are expected to contain the literal 'apikey' + # as the user name and the API key as the password. class UserBasicAuth < BasicAuth - def authenticate_user(username, password) - user = User.find_by_login username + def self.user + 'apikey' + end - user if user && user.check_password?(password) + def valid? + super && username == self.class.user + end + + def authenticate_user(_, api_key) + token(api_key).try(:user) + end + + def token(value) + Token.where(action: 'api', value: value).first end end end diff --git a/spec/factories/token_factory.rb b/spec/factories/token_factory.rb index 0a4fb136916..0981cc26f70 100644 --- a/spec/factories/token_factory.rb +++ b/spec/factories/token_factory.rb @@ -26,8 +26,19 @@ # See doc/COPYRIGHT.rdoc for more details. #++ +require 'securerandom' + FactoryGirl.define do factory :token do - # doesn't need anything + user + value { SecureRandom.hex(16) } + + factory :api_key do + action 'api' + end + + factory :rss_key do + action 'rss' + end end end diff --git a/spec/lib/api/v3/authentication_spec.rb b/spec/lib/api/v3/authentication_spec.rb index 17499b58a36..832680f2448 100644 --- a/spec/lib/api/v3/authentication_spec.rb +++ b/spec/lib/api/v3/authentication_spec.rb @@ -92,15 +92,10 @@ describe API::V3, type: :request do it_behaves_like 'it is basic auth protected' describe 'user basic auth' do - let(:username) { 'hans' } - let(:password) { 'bambidibam' } + let(:api_key) { FactoryGirl.create :api_key } - let!(:api_user) do - FactoryGirl.create :user, - login: username, - password: password, - password_confirmation: password - end + let(:username) { 'apikey' } + let(:password) { api_key.value } # check that user basic auth is tried when global basic auth fails it_behaves_like 'it is basic auth protected' @@ -117,12 +112,8 @@ describe API::V3, type: :request do let(:username) { 'hancholo' } let(:password) { 'olooleol' } - let!(:api_user) do - FactoryGirl.create :user, - login: 'user_account', - password: 'user_password', - password_confirmation: 'user_password' - end + let(:api_user) { FactoryGirl.create :user, login: 'user_account' } + let(:api_key) { FactoryGirl.create :api_key, user: api_user } before do authentication = { @@ -175,7 +166,7 @@ describe API::V3, type: :request do context 'with valid user credentials' do before do - get resource, {}, basic_auth('user_account', 'user_password') + get resource, {}, basic_auth('apikey', api_key.value) end it 'should return 200 OK' do From c2fc6dffd044afd6a3e914244bc8e24114f53e04 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Thu, 7 May 2015 11:58:46 +0100 Subject: [PATCH 06/56] hint for plugins --- lib/open_project/authentication.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/open_project/authentication.rb b/lib/open_project/authentication.rb index 95e8eb71e8d..4ab0a225049 100644 --- a/lib/open_project/authentication.rb +++ b/lib/open_project/authentication.rb @@ -7,7 +7,7 @@ module OpenProject class << self ## # Updates the used warden strategies for a given scope. The strategies will be tried - # in the order they are set here. + # in the order they are set here. Plugins can call this to add or remove strategies. # For available scopes please refer to `OpenProject::Authentication::Scope`. # # @param [Symbol] scope The scope for which to update the used warden strategies. From 6bbcc3b2696b7eab63aabb2b03e27f7f972cf404 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Thu, 7 May 2015 12:04:42 +0100 Subject: [PATCH 07/56] root.rb auth refactoring --- lib/api/root.rb | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/api/root.rb b/lib/api/root.rb index b5b5f96920d..63a8a4a9db2 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -31,10 +31,12 @@ # This is the place for all API wide configuration, helper methods, exceptions # rescuing, mounting of differnet API versions etc. -require 'open_project/authentication/manager' +require 'open_project/authentication' module API class Root < Grape::API + include OpenProject::Authentication::Scope + prefix :api class Formatter @@ -73,15 +75,20 @@ module API end def authenticate - warden.authenticate! scope: :api_v3 + warden.authenticate! scope: API_V3 - User.current = warden.user scope: :api_v3 + User.current = warden.user scope: API_V3 - if Setting.login_required? && (current_user.nil? || (!current_user.admin? && current_user.anonymous?)) + if Setting.login_required? && not_logged_in? raise API::Errors::Unauthenticated end end + def not_logged_in? + # An admin SystemUser is anonymous but still a valid user to be logged in. + current_user.nil? || (!current_user.admin? && current_user.anonymous?) + end + def authorize(permission, context: nil, global: false, user: current_user) is_authorized = AuthorizationService.new(permission, context: context, From c01902c45e81fc8e828316c3c3f6bec33cf2d762 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Thu, 7 May 2015 12:35:29 +0100 Subject: [PATCH 08/56] clean up --- lib/core_ext/hash/hash_navigation.rb | 35 ---------------------- lib/open_project.rb | 2 -- lib/open_project/authentication/manager.rb | 2 +- lib/warden/strategies/global_basic_auth.rb | 8 ++--- 4 files changed, 5 insertions(+), 42 deletions(-) delete mode 100644 lib/core_ext/hash/hash_navigation.rb diff --git a/lib/core_ext/hash/hash_navigation.rb b/lib/core_ext/hash/hash_navigation.rb deleted file mode 100644 index 35ba32d3993..00000000000 --- a/lib/core_ext/hash/hash_navigation.rb +++ /dev/null @@ -1,35 +0,0 @@ -module HashNavigation - def fetch?(*keys) - value = fetch_path keys, self - - if value - value - elsif block_given? - yield - else - nil - end - end - - module_function - - def fetch_path(keys, hash, default = nil) - if keys.size <= 1 - fetch_value keys.first, hash, default - else - head, *tail = keys - - fetch_path tail, fetch_hash(head, hash), default - end - end - - def fetch_hash(key, hash) - ActiveSupport::HashWithIndifferentAccess.new fetch_value(key, hash) - end - - def fetch_value(key, hash, default = nil) - hash[key] || default - end -end - -Hash.include HashNavigation diff --git a/lib/open_project.rb b/lib/open_project.rb index fe808cfb842..691bf11802a 100644 --- a/lib/open_project.rb +++ b/lib/open_project.rb @@ -41,5 +41,3 @@ require 'redmine/notifiable' require 'csv' require 'globalize' - -require 'core_ext/hash/hash_navigation' diff --git a/lib/open_project/authentication/manager.rb b/lib/open_project/authentication/manager.rb index 6cd1a9bb9e9..1aae1cb4b9c 100644 --- a/lib/open_project/authentication/manager.rb +++ b/lib/open_project/authentication/manager.rb @@ -10,7 +10,7 @@ module OpenProject User.find id end - def initialize(app, options={}, &configure) + def initialize(app, options = {}, &configure) block = lambda do |config| self.class.configure config diff --git a/lib/warden/strategies/global_basic_auth.rb b/lib/warden/strategies/global_basic_auth.rb index c440bb289db..9a6327704e2 100644 --- a/lib/warden/strategies/global_basic_auth.rb +++ b/lib/warden/strategies/global_basic_auth.rb @@ -18,10 +18,10 @@ module Warden # other user name is made. class GlobalBasicAuth < BasicAuth def self.configuration - config = Hash(OpenProject::Configuration['authentication']) - - user = config.fetch? 'global_basic_auth', 'user' - password = config.fetch? 'global_basic_auth', 'password' + path = %w(authentication global_basic_auth) + config = path.reduce(OpenProject::Configuration) { |acc, key| Hash(acc[key]) } + user = config['user'] + password = config['password'] { user: user, password: password } if user && password end From c60928d8c7408b0ca80c3f90ed9e05159856a281 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Thu, 7 May 2015 12:36:21 +0100 Subject: [PATCH 09/56] Gemfile.lock additions --- Gemfile.lock | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Gemfile.lock b/Gemfile.lock index d4d7cf75654..7b5eb3c3d92 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -461,6 +461,10 @@ GEM coercible (~> 1.0) descendants_tracker (~> 0.0, >= 0.0.3) equalizer (~> 0.0, >= 0.0.9) + warden (1.2.3) + rack (>= 1.0) + warden-basic_auth (0.1.0) + warden (~> 1.2) websocket (1.2.1) will_paginate (3.0.5) xpath (2.0.0) @@ -564,4 +568,6 @@ DEPENDENCIES timecop (~> 0.7.1) uglifier (>= 1.0.3) unicorn + warden (~> 1.2) + warden-basic_auth (~> 0.1.0) will_paginate (~> 3.0) From e4dbfeef3a044ee8d43cb3130b76fece2c8989e6 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Thu, 7 May 2015 12:39:48 +0100 Subject: [PATCH 10/56] hound --- lib/open_project/authentication.rb | 3 ++- lib/open_project/authentication/manager.rb | 1 - lib/warden/strategies/global_basic_auth.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/open_project/authentication.rb b/lib/open_project/authentication.rb index 4ab0a225049..06292e0a95b 100644 --- a/lib/open_project/authentication.rb +++ b/lib/open_project/authentication.rb @@ -11,7 +11,8 @@ module OpenProject # For available scopes please refer to `OpenProject::Authentication::Scope`. # # @param [Symbol] scope The scope for which to update the used warden strategies. - # @param [Boolean] store Indicates whether the user should be stored in the session for this scope. + # @param [Boolean] store Indicates whether the user should be stored in the session + # for this scope. # # @yield [strategies] A block returning the strategies to be used for this scope. # @yieldparam [Array] The strategies currently used by this scope. May be empty. diff --git a/lib/open_project/authentication/manager.rb b/lib/open_project/authentication/manager.rb index 1aae1cb4b9c..3fd7fd80ec0 100644 --- a/lib/open_project/authentication/manager.rb +++ b/lib/open_project/authentication/manager.rb @@ -1,7 +1,6 @@ module OpenProject module Authentication class Manager < Warden::Manager - serialize_into_session do |user| user.id end diff --git a/lib/warden/strategies/global_basic_auth.rb b/lib/warden/strategies/global_basic_auth.rb index 9a6327704e2..bf1e2bd3db1 100644 --- a/lib/warden/strategies/global_basic_auth.rb +++ b/lib/warden/strategies/global_basic_auth.rb @@ -19,7 +19,7 @@ module Warden class GlobalBasicAuth < BasicAuth def self.configuration path = %w(authentication global_basic_auth) - config = path.reduce(OpenProject::Configuration) { |acc, key| Hash(acc[key]) } + config = path.inject(OpenProject::Configuration) { |acc, key| Hash(acc[key]) } user = config['user'] password = config['password'] From 75ec64d94cf8d5606e13758c740d828a83c9a92a Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Thu, 7 May 2015 16:01:35 +0100 Subject: [PATCH 11/56] memoize config; forbid 'apikey' as global user --- lib/warden/strategies/global_basic_auth.rb | 28 ++++++++++++++-------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/lib/warden/strategies/global_basic_auth.rb b/lib/warden/strategies/global_basic_auth.rb index bf1e2bd3db1..9badc54f8eb 100644 --- a/lib/warden/strategies/global_basic_auth.rb +++ b/lib/warden/strategies/global_basic_auth.rb @@ -18,20 +18,30 @@ module Warden # other user name is made. class GlobalBasicAuth < BasicAuth def self.configuration - path = %w(authentication global_basic_auth) - config = path.inject(OpenProject::Configuration) { |acc, key| Hash(acc[key]) } - user = config['user'] - password = config['password'] + @configuration ||= configuration! + end - { user: user, password: password } if user && password + def self.configuration! + path = %w(authentication global_basic_auth) + @configuration = path.inject(OpenProject::Configuration) { |acc, key| Hash(acc[key]) } + + if user == UserBasicAuth.user + raise ArgumentError, "global user must not be '#{UserBasicAuth.user}'" + end + + @configuration + end + + def self.configuration? + user && password end def self.user - configuration[:user] + configuration['user'] end def self.password - configuration[:password] + configuration['password'] end ## @@ -41,9 +51,7 @@ module Warden end def authenticate_user(username, password) - config = self.class.configuration - - if username == config[:user] && password == config[:password] + if username == self.class.user && password == self.class.password User.system.tap do |user| user.admin = true end From 7e40c11ca40280d8b7102a15e4346a936c4956da Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Thu, 7 May 2015 23:39:57 +0100 Subject: [PATCH 12/56] moved strategies; refactoring --- config/initializers/warden.rb | 18 +++-- .../strategies/warden/basic_auth_failure.rb | 16 ++++ .../strategies/warden/global_basic_auth.rb | 78 +++++++++++++++++++ .../strategies/warden/session.rb | 30 +++++++ .../strategies/warden/user_basic_auth.rb | 34 ++++++++ lib/warden/strategies/basic_auth_failure.rb | 12 --- lib/warden/strategies/global_basic_auth.rb | 62 --------------- lib/warden/strategies/session.rb | 26 ------- lib/warden/strategies/user_basic_auth.rb | 30 ------- spec/lib/api/v3/authentication_spec.rb | 21 ++--- .../warden/global_basic_auth_spec.rb | 41 ++++++++++ 11 files changed, 215 insertions(+), 153 deletions(-) create mode 100644 lib/open_project/authentication/strategies/warden/basic_auth_failure.rb create mode 100644 lib/open_project/authentication/strategies/warden/global_basic_auth.rb create mode 100644 lib/open_project/authentication/strategies/warden/session.rb create mode 100644 lib/open_project/authentication/strategies/warden/user_basic_auth.rb delete mode 100644 lib/warden/strategies/basic_auth_failure.rb delete mode 100644 lib/warden/strategies/global_basic_auth.rb delete mode 100644 lib/warden/strategies/session.rb delete mode 100644 lib/warden/strategies/user_basic_auth.rb create mode 100644 spec/lib/open_project/authentication/strategies/warden/global_basic_auth_spec.rb diff --git a/config/initializers/warden.rb b/config/initializers/warden.rb index 99ffe612913..b28f24321f7 100644 --- a/config/initializers/warden.rb +++ b/config/initializers/warden.rb @@ -1,14 +1,16 @@ +require 'open_project/authentication' + # Strategies provided by OpenProject: -require 'warden/strategies/basic_auth_failure' -require 'warden/strategies/global_basic_auth' -require 'warden/strategies/user_basic_auth' -require 'warden/strategies/session' +require 'open_project/authentication/strategies/warden/basic_auth_failure' +require 'open_project/authentication/strategies/warden/global_basic_auth' +require 'open_project/authentication/strategies/warden/user_basic_auth' +require 'open_project/authentication/strategies/warden/session' strategies = [ - [:basic_auth_failure, Warden::Strategies::BasicAuthFailure], - [:global_basic_auth, Warden::Strategies::GlobalBasicAuth], - [:user_basic_auth, Warden::Strategies::UserBasicAuth], - [:session, Warden::Strategies::Session] + [:basic_auth_failure, OpenProject::Authentication::Strategies::Warden::BasicAuthFailure], + [:global_basic_auth, OpenProject::Authentication::Strategies::Warden::GlobalBasicAuth], + [:user_basic_auth, OpenProject::Authentication::Strategies::Warden::UserBasicAuth], + [:session, OpenProject::Authentication::Strategies::Warden::Session] ] strategies.each do |name, clazz| diff --git a/lib/open_project/authentication/strategies/warden/basic_auth_failure.rb b/lib/open_project/authentication/strategies/warden/basic_auth_failure.rb new file mode 100644 index 00000000000..05bc8053360 --- /dev/null +++ b/lib/open_project/authentication/strategies/warden/basic_auth_failure.rb @@ -0,0 +1,16 @@ +module OpenProject + module Authentication + module Strategies + module Warden + ## + # This strategy is inserted after optional basic auth strategies to + # indicate that invalid basic auth credentials were provided. + class BasicAuthFailure < ::Warden::Strategies::BasicAuth + def authenticate_user(_username, _password) + nil # always fails + end + end + end + end + end +end diff --git a/lib/open_project/authentication/strategies/warden/global_basic_auth.rb b/lib/open_project/authentication/strategies/warden/global_basic_auth.rb new file mode 100644 index 00000000000..8357725ab2f --- /dev/null +++ b/lib/open_project/authentication/strategies/warden/global_basic_auth.rb @@ -0,0 +1,78 @@ +require 'warden/basic_auth' + +module OpenProject + module Authentication + module Strategies + module Warden + ## + # Allows authentication via a singular set of basic auth credentials for admin access. + # + # The credentials must be configured in `config/configuration.yml` like this: + # + # production: + # authentication: + # global_basic_auth: + # user: admin + # password: 123456 + # + # The strategy will only be triggered when the configured user name is sent. + # Meaning that this strategy is skipped if a basic auth attempt involving any + # other user name is made. + class GlobalBasicAuth < ::Warden::Strategies::BasicAuth + def self.configuration + @configuration ||= configure! + end + + ## + # Updates the configuration for this strategy. It's usually called only once, at startup. + # + # @param [Hash] config The configuration to be used. Must contain :user and :password. + # @raise [ArgumentError] Raises an error if the configured user name collides with the + # user name used for UserBasicAuth (apikey). + def self.configure!(config = openproject_config) + if config[:user] == UserBasicAuth.user + raise ArgumentError, "global user must not be '#{UserBasicAuth.user}'" + end + + @configuration = config + end + + ## + # Reads the configuration for this strategy from OpenProject's `configuration.yml`. + def self.openproject_config + config = OpenProject::Configuration + %w(authentication global_basic_auth).inject(config) do |acc, key| + HashWithIndifferentAccess.new acc[key] + end + end + + def self.configuration? + user && password + end + + def self.user + configuration[:user] + end + + def self.password + configuration[:password] + end + + ## + # Only valid if global basic auth is configured and tried. + def valid? + self.class.configuration? && super && username == self.class.user + end + + def authenticate_user(username, password) + if username == self.class.user && password == self.class.password + User.system.tap do |user| + user.admin = true + end + end + end + end + end + end + end +end diff --git a/lib/open_project/authentication/strategies/warden/session.rb b/lib/open_project/authentication/strategies/warden/session.rb new file mode 100644 index 00000000000..1048b274503 --- /dev/null +++ b/lib/open_project/authentication/strategies/warden/session.rb @@ -0,0 +1,30 @@ +module OpenProject + module Authentication + module Strategies + module Warden + ## + # Temporary strategy necessary as long as the OpenProject authentication has not been unified + # in terms of Warden strategies and is only locally applied to the API v3. + class Session < ::Warden::Strategies::Base + def valid? + session + end + + def authenticate! + user = user_id ? User.find(user_id) : User.anonymous + + success! user + end + + def user_id + Hash(session)['user_id'] + end + + def session + env['rack.session'] + end + end + end + end + end +end diff --git a/lib/open_project/authentication/strategies/warden/user_basic_auth.rb b/lib/open_project/authentication/strategies/warden/user_basic_auth.rb new file mode 100644 index 00000000000..83ec5df0770 --- /dev/null +++ b/lib/open_project/authentication/strategies/warden/user_basic_auth.rb @@ -0,0 +1,34 @@ +require 'warden/basic_auth' + +module OpenProject + module Authentication + module Strategies + module Warden + ## + # Allows users to authenticate using their API key via basic auth. + # Note that in order for a user to be able to generate one + # `Setting.rest_api_enabled` has to be `1`. + # + # The basic auth credentials are expected to contain the literal 'apikey' + # as the user name and the API key as the password. + class UserBasicAuth < ::Warden::Strategies::BasicAuth + def self.user + 'apikey' + end + + def valid? + super && username == self.class.user + end + + def authenticate_user(_, api_key) + token(api_key).try(:user) + end + + def token(value) + Token.where(action: 'api', value: value).first + end + end + end + end + end +end diff --git a/lib/warden/strategies/basic_auth_failure.rb b/lib/warden/strategies/basic_auth_failure.rb deleted file mode 100644 index c4d2a727503..00000000000 --- a/lib/warden/strategies/basic_auth_failure.rb +++ /dev/null @@ -1,12 +0,0 @@ -module Warden - module Strategies - ## - # This strategy is inserted after optional basic auth strategies to - # indicate that invalid basic auth credentials were provided. - class BasicAuthFailure < BasicAuth - def authenticate_user(_username, _password) - nil # always fails - end - end - end -end diff --git a/lib/warden/strategies/global_basic_auth.rb b/lib/warden/strategies/global_basic_auth.rb deleted file mode 100644 index 9badc54f8eb..00000000000 --- a/lib/warden/strategies/global_basic_auth.rb +++ /dev/null @@ -1,62 +0,0 @@ -require 'warden/basic_auth' - -module Warden - module Strategies - ## - # Allows authentication via a singular set of basic auth credentials for admin access. - # - # The credentials must be configured in `config/configuration.yml` like this: - # - # production: - # authentication: - # global_basic_auth: - # user: admin - # password: 123456 - # - # The strategy will only be triggered when the configured user name is sent. - # Meaning that this strategy is skipped if a basic auth attempt involving any - # other user name is made. - class GlobalBasicAuth < BasicAuth - def self.configuration - @configuration ||= configuration! - end - - def self.configuration! - path = %w(authentication global_basic_auth) - @configuration = path.inject(OpenProject::Configuration) { |acc, key| Hash(acc[key]) } - - if user == UserBasicAuth.user - raise ArgumentError, "global user must not be '#{UserBasicAuth.user}'" - end - - @configuration - end - - def self.configuration? - user && password - end - - def self.user - configuration['user'] - end - - def self.password - configuration['password'] - end - - ## - # Only valid if global basic auth is configured and tried. - def valid? - self.class.configuration && super && username == self.class.user - end - - def authenticate_user(username, password) - if username == self.class.user && password == self.class.password - User.system.tap do |user| - user.admin = true - end - end - end - end - end -end diff --git a/lib/warden/strategies/session.rb b/lib/warden/strategies/session.rb deleted file mode 100644 index 4b65d656681..00000000000 --- a/lib/warden/strategies/session.rb +++ /dev/null @@ -1,26 +0,0 @@ -module Warden - module Strategies - ## - # Temporary strategy necessary as long as the OpenProject authentication has not been unified - # in terms of Warden strategies and is only locally applied to the API v3. - class Session < Base - def valid? - session - end - - def authenticate! - user = user_id ? User.find(user_id) : User.anonymous - - success! user - end - - def user_id - Hash(session)['user_id'] - end - - def session - env['rack.session'] - end - end - end -end diff --git a/lib/warden/strategies/user_basic_auth.rb b/lib/warden/strategies/user_basic_auth.rb deleted file mode 100644 index be3e81ffcb2..00000000000 --- a/lib/warden/strategies/user_basic_auth.rb +++ /dev/null @@ -1,30 +0,0 @@ -require 'warden/basic_auth' - -module Warden - module Strategies - ## - # Allows users to authenticate using their API key via basic auth. - # Note that in order for a user to be able to generate one - # `Setting.rest_api_enabled` has to be `1`. - # - # The basic auth credentials are expected to contain the literal 'apikey' - # as the user name and the API key as the password. - class UserBasicAuth < BasicAuth - def self.user - 'apikey' - end - - def valid? - super && username == self.class.user - end - - def authenticate_user(_, api_key) - token(api_key).try(:user) - end - - def token(value) - Token.where(action: 'api', value: value).first - end - end - end -end diff --git a/spec/lib/api/v3/authentication_spec.rb b/spec/lib/api/v3/authentication_spec.rb index 832680f2448..1e65ffc30c9 100644 --- a/spec/lib/api/v3/authentication_spec.rb +++ b/spec/lib/api/v3/authentication_spec.rb @@ -33,9 +33,11 @@ describe API::V3, type: :request do let(:user) { FactoryGirl.create :user } let(:resource) { "/api/v3/users/#{user.id}"} + Strategies = OpenProject::Authentication::Strategies::Warden + def basic_auth(user, password) credentials = ActionController::HttpAuthentication::Basic.encode_credentials user, password - {'HTTP_AUTHORIZATION' => credentials} + { 'HTTP_AUTHORIZATION' => credentials } end shared_examples 'it is basic auth protected' do @@ -80,13 +82,7 @@ describe API::V3, type: :request do let(:password) { 'toor' } before do - authentication = { - 'global_basic_auth' => { - 'user' => 'root', - 'password' => 'toor' - } - } - OpenProject::Configuration['authentication'] = authentication + Strategies::GlobalBasicAuth.configure! user: 'root', password: 'toor' end it_behaves_like 'it is basic auth protected' @@ -116,13 +112,8 @@ describe API::V3, type: :request do let(:api_key) { FactoryGirl.create :api_key, user: api_user } before do - authentication = { - 'global_basic_auth' => { - 'user' => 'global_account', - 'password' => 'global_password' - } - } - OpenProject::Configuration['authentication'] = authentication + config = { user: 'global_account', password: 'global_password' } + Strategies::GlobalBasicAuth.configure! config end context 'without credentials' do diff --git a/spec/lib/open_project/authentication/strategies/warden/global_basic_auth_spec.rb b/spec/lib/open_project/authentication/strategies/warden/global_basic_auth_spec.rb new file mode 100644 index 00000000000..ca789a87f26 --- /dev/null +++ b/spec/lib/open_project/authentication/strategies/warden/global_basic_auth_spec.rb @@ -0,0 +1,41 @@ +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2015 the OpenProject Foundation (OPF) +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See doc/COPYRIGHT.rdoc for more details. +#++ + +require 'spec_helper' + +describe OpenProject::Authentication::Strategies::Warden::GlobalBasicAuth do + it 'does not allow the UserBasicAuth user name' do + configuration = lambda do + user = OpenProject::Authentication::Strategies::Warden::UserBasicAuth.user + OpenProject::Authentication::Strategies::Warden::GlobalBasicAuth.configure!( + user: user, password: 'foo') + end + + expect(configuration).to raise_error(ArgumentError) + end +end From 8f7d8ef573d4ccd97cf0a5df2d0f1203cf3dd01d Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Thu, 7 May 2015 23:46:33 +0100 Subject: [PATCH 13/56] avoid unnecessary work --- app/models/system_user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/system_user.rb b/app/models/system_user.rb index 83da13de78f..ef540739866 100644 --- a/app/models/system_user.rb +++ b/app/models/system_user.rb @@ -62,7 +62,7 @@ class SystemUser < User # There should be only one SystemUser in the database def validate_unique_system_user - errors.add :base, 'A SystemUser already exists.' if SystemUser.first + errors.add :base, 'A SystemUser already exists.' if SystemUser.any? end # Overrides a few properties From 49577502709cc26e148025285978794ce1e62085 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Thu, 7 May 2015 23:46:49 +0100 Subject: [PATCH 14/56] wrapped comment --- lib/open_project/authentication/strategies/warden/session.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/open_project/authentication/strategies/warden/session.rb b/lib/open_project/authentication/strategies/warden/session.rb index 1048b274503..9cb47db1bac 100644 --- a/lib/open_project/authentication/strategies/warden/session.rb +++ b/lib/open_project/authentication/strategies/warden/session.rb @@ -3,8 +3,9 @@ module OpenProject module Strategies module Warden ## - # Temporary strategy necessary as long as the OpenProject authentication has not been unified - # in terms of Warden strategies and is only locally applied to the API v3. + # Temporary strategy necessary as long as the OpenProject authentication has + # not been unified in terms of Warden strategies and is only locally + # applied to the API v3. class Session < ::Warden::Strategies::Base def valid? session From 28c81dfad08269f83bf996d78f10ab7e9865fc16 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Fri, 8 May 2015 00:27:20 +0100 Subject: [PATCH 15/56] use existing method to find user via api key --- .../authentication/strategies/warden/user_basic_auth.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/open_project/authentication/strategies/warden/user_basic_auth.rb b/lib/open_project/authentication/strategies/warden/user_basic_auth.rb index 83ec5df0770..c6cb39e9a9b 100644 --- a/lib/open_project/authentication/strategies/warden/user_basic_auth.rb +++ b/lib/open_project/authentication/strategies/warden/user_basic_auth.rb @@ -21,11 +21,7 @@ module OpenProject end def authenticate_user(_, api_key) - token(api_key).try(:user) - end - - def token(value) - Token.where(action: 'api', value: value).first + User.find_by_api_key api_key end end end From 1814455eb7ad01bbacbdd6ea12dbdd7a5ef620c7 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Fri, 8 May 2015 17:33:29 +0100 Subject: [PATCH 16/56] failure_app has to be instance --- lib/open_project/authentication/manager.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/open_project/authentication/manager.rb b/lib/open_project/authentication/manager.rb index 3fd7fd80ec0..d83e3b74cfe 100644 --- a/lib/open_project/authentication/manager.rb +++ b/lib/open_project/authentication/manager.rb @@ -30,7 +30,7 @@ module OpenProject def configure(config) config.default_strategies :session - config.failure_app = OpenProject::Authentication::FailureApp + config.failure_app = OpenProject::Authentication::FailureApp.new scope_strategies.each do |scope, strategies| config.scope_defaults scope, strategies: strategies, store: store_defaults[scope] From d368aa85b630f6dee62754e6c6fb674a30243105 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Mon, 11 May 2015 10:12:34 +0100 Subject: [PATCH 17/56] be more positive --- lib/api/root.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/api/root.rb b/lib/api/root.rb index 63a8a4a9db2..080773f794f 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -79,14 +79,14 @@ module API User.current = warden.user scope: API_V3 - if Setting.login_required? && not_logged_in? + if Setting.login_required? and not logged_in? raise API::Errors::Unauthenticated end end - def not_logged_in? + def logged_in? # An admin SystemUser is anonymous but still a valid user to be logged in. - current_user.nil? || (!current_user.admin? && current_user.anonymous?) + current_user.present? && (current_user.admin? || !current_user.anonymous?) end def authorize(permission, context: nil, global: false, user: current_user) From 7b3df308d036661651fec563458925e52cf026f8 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Mon, 11 May 2015 10:15:05 +0100 Subject: [PATCH 18/56] removed obsolete code/comment --- lib/api/root.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/api/root.rb b/lib/api/root.rb index 080773f794f..94e7ab2c84f 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -163,12 +163,6 @@ module API # run authentication before each request before do - # Call current_user as it sets User.current. - # Not doing this might cause devs to use User.current without that value - # being set to the actually current user. That might result in standard - # users becoming admins and otherwise based on who called the ruby - # process last. - current_user authenticate end From 5ba15f3b5caa2760c0e042aaf7f9b0aec7042791 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Mon, 11 May 2015 10:40:45 +0100 Subject: [PATCH 19/56] removed unnecessary/wrong use of present? --- lib/api/root.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/root.rb b/lib/api/root.rb index 94e7ab2c84f..3f5332fe5d2 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -86,7 +86,7 @@ module API def logged_in? # An admin SystemUser is anonymous but still a valid user to be logged in. - current_user.present? && (current_user.admin? || !current_user.anonymous?) + current_user && (current_user.admin? || !current_user.anonymous?) end def authorize(permission, context: nil, global: false, user: current_user) From 12f9cdf21d10fdb1f51636507162b40e6e5aaeb1 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Mon, 11 May 2015 11:31:01 +0100 Subject: [PATCH 20/56] Clarified that `store` is optional. --- lib/open_project/authentication.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/open_project/authentication.rb b/lib/open_project/authentication.rb index 06292e0a95b..ac5597c0565 100644 --- a/lib/open_project/authentication.rb +++ b/lib/open_project/authentication.rb @@ -12,7 +12,8 @@ module OpenProject # # @param [Symbol] scope The scope for which to update the used warden strategies. # @param [Boolean] store Indicates whether the user should be stored in the session - # for this scope. + # for this scope. Optional. If not given, the current store flag + # for this strategy will remain unchanged what ever it is. # # @yield [strategies] A block returning the strategies to be used for this scope. # @yieldparam [Array] The strategies currently used by this scope. May be empty. From 6fc3469671ca2c58f2429a32b4635b815feecb7a Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Mon, 11 May 2015 11:31:42 +0100 Subject: [PATCH 21/56] use hash to save a bunch of brackets and commas --- config/initializers/warden.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/config/initializers/warden.rb b/config/initializers/warden.rb index b28f24321f7..85dc7990b8e 100644 --- a/config/initializers/warden.rb +++ b/config/initializers/warden.rb @@ -6,12 +6,12 @@ require 'open_project/authentication/strategies/warden/global_basic_auth' require 'open_project/authentication/strategies/warden/user_basic_auth' require 'open_project/authentication/strategies/warden/session' -strategies = [ - [:basic_auth_failure, OpenProject::Authentication::Strategies::Warden::BasicAuthFailure], - [:global_basic_auth, OpenProject::Authentication::Strategies::Warden::GlobalBasicAuth], - [:user_basic_auth, OpenProject::Authentication::Strategies::Warden::UserBasicAuth], - [:session, OpenProject::Authentication::Strategies::Warden::Session] -] +strategies = { + basic_auth_failure: OpenProject::Authentication::Strategies::Warden::BasicAuthFailure, + global_basic_auth: OpenProject::Authentication::Strategies::Warden::GlobalBasicAuth, + user_basic_auth: OpenProject::Authentication::Strategies::Warden::UserBasicAuth, + session: OpenProject::Authentication::Strategies::Warden::Session +} strategies.each do |name, clazz| Warden::Strategies.add name, clazz From 72c9759d8c305a1365012244d7daa42bcc454bb7 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Mon, 11 May 2015 14:01:25 +0100 Subject: [PATCH 22/56] whitespace --- spec/lib/api/v3/authentication_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/api/v3/authentication_spec.rb b/spec/lib/api/v3/authentication_spec.rb index 1e65ffc30c9..107c38f6e63 100644 --- a/spec/lib/api/v3/authentication_spec.rb +++ b/spec/lib/api/v3/authentication_spec.rb @@ -31,7 +31,7 @@ require 'spec_helper' describe API::V3, type: :request do describe 'basic auth' do let(:user) { FactoryGirl.create :user } - let(:resource) { "/api/v3/users/#{user.id}"} + let(:resource) { "/api/v3/users/#{user.id}" } Strategies = OpenProject::Authentication::Strategies::Warden From 7a7dff569c866987a2bc9aa8868228522e500945 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Mon, 11 May 2015 14:04:50 +0100 Subject: [PATCH 23/56] moved auth spec to other request specs --- spec/{lib => requests}/api/v3/authentication_spec.rb | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename spec/{lib => requests}/api/v3/authentication_spec.rb (100%) diff --git a/spec/lib/api/v3/authentication_spec.rb b/spec/requests/api/v3/authentication_spec.rb similarity index 100% rename from spec/lib/api/v3/authentication_spec.rb rename to spec/requests/api/v3/authentication_spec.rb From 312bd47ab8bb591335f27b178389669a9d0831eb Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Mon, 11 May 2015 14:17:20 +0100 Subject: [PATCH 24/56] test user basic auth on its own --- spec/requests/api/v3/authentication_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/requests/api/v3/authentication_spec.rb b/spec/requests/api/v3/authentication_spec.rb index 107c38f6e63..2e027d28c61 100644 --- a/spec/requests/api/v3/authentication_spec.rb +++ b/spec/requests/api/v3/authentication_spec.rb @@ -97,6 +97,16 @@ describe API::V3, type: :request do it_behaves_like 'it is basic auth protected' end end + + describe 'user basic auth' do + let(:api_key) { FactoryGirl.create :api_key } + + let(:username) { 'apikey' } + let(:password) { api_key.value } + + # check that user basic auth works on its own too + it_behaves_like 'it is basic auth protected' + end end context 'without login required' do From f7b978fe8ce24cd291f1192db2e9f9f90927d785 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Mon, 11 May 2015 14:31:35 +0100 Subject: [PATCH 25/56] whitespace --- spec/requests/api/v3/authentication_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/requests/api/v3/authentication_spec.rb b/spec/requests/api/v3/authentication_spec.rb index 2e027d28c61..ae2e9ad67e0 100644 --- a/spec/requests/api/v3/authentication_spec.rb +++ b/spec/requests/api/v3/authentication_spec.rb @@ -30,7 +30,7 @@ require 'spec_helper' describe API::V3, type: :request do describe 'basic auth' do - let(:user) { FactoryGirl.create :user } + let(:user) { FactoryGirl.create :user } let(:resource) { "/api/v3/users/#{user.id}" } Strategies = OpenProject::Authentication::Strategies::Warden @@ -88,7 +88,7 @@ describe API::V3, type: :request do it_behaves_like 'it is basic auth protected' describe 'user basic auth' do - let(:api_key) { FactoryGirl.create :api_key } + let(:api_key) { FactoryGirl.create :api_key } let(:username) { 'apikey' } let(:password) { api_key.value } @@ -99,7 +99,7 @@ describe API::V3, type: :request do end describe 'user basic auth' do - let(:api_key) { FactoryGirl.create :api_key } + let(:api_key) { FactoryGirl.create :api_key } let(:username) { 'apikey' } let(:password) { api_key.value } From 32a26429b796c78b5019a8b882deb3164d71ce83 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Mon, 11 May 2015 14:53:38 +0100 Subject: [PATCH 26/56] I don't even --- lib/api/root.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/root.rb b/lib/api/root.rb index 3f5332fe5d2..e39509f5847 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -80,7 +80,7 @@ module API User.current = warden.user scope: API_V3 if Setting.login_required? and not logged_in? - raise API::Errors::Unauthenticated + raise ::API::Errors::Unauthenticated end end From 7b15726e41dbb61d761132891602ced9d03dd17c Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Mon, 11 May 2015 17:39:29 +0100 Subject: [PATCH 27/56] updated broken warden basic auth --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 7b5eb3c3d92..d5befb6f918 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -463,7 +463,7 @@ GEM equalizer (~> 0.0, >= 0.0.9) warden (1.2.3) rack (>= 1.0) - warden-basic_auth (0.1.0) + warden-basic_auth (0.1.1) warden (~> 1.2) websocket (1.2.1) will_paginate (3.0.5) From 468357045c3b528d689f4a85d0fd9f1954148448 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Mon, 11 May 2015 18:57:42 +0100 Subject: [PATCH 28/56] spec against proper json response --- spec/requests/api/v3/authentication_spec.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/spec/requests/api/v3/authentication_spec.rb b/spec/requests/api/v3/authentication_spec.rb index ae2e9ad67e0..ca181601a6d 100644 --- a/spec/requests/api/v3/authentication_spec.rb +++ b/spec/requests/api/v3/authentication_spec.rb @@ -33,6 +33,15 @@ describe API::V3, type: :request do let(:user) { FactoryGirl.create :user } let(:resource) { "/api/v3/users/#{user.id}" } + let(:response_401) do + { + '_embedded' => {}, + '_type' => 'Error', + 'errorIdentifier' => 'urn:openproject-org:api:v3:errors:MissingPermission', + 'message' => 'You need to be authenticated to access this resource.' + } + end + Strategies = OpenProject::Authentication::Strategies::Warden def basic_auth(user, password) @@ -49,6 +58,10 @@ describe API::V3, type: :request do it 'should return 401 unauthorized' do expect(response.status).to eq 401 end + + it 'should return the correct JSON response' do + expect(JSON.parse(response.body)).to eq response_401 + end end context 'with invalid credentials' do @@ -59,6 +72,10 @@ describe API::V3, type: :request do it 'should return 401 unauthorized' do expect(response.status).to eq 401 end + + it 'should return the correct JSON response' do + expect(JSON.parse(response.body)).to eq response_401 + end end context 'with valid credentials' do From de1cad8863aad783a20afdaaba698a44fd79f94b Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Wed, 13 May 2015 17:05:31 +0100 Subject: [PATCH 29/56] updated warden-basic_auth --- Gemfile | 2 +- Gemfile.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 2fd044066e7..1911c04f508 100644 --- a/Gemfile +++ b/Gemfile @@ -39,7 +39,7 @@ gem 'request_store', "~> 1.1.0" gem 'gravatar_image_tag', '~> 1.2.0' gem 'warden', '~> 1.2' -gem 'warden-basic_auth', '~> 0.1.0' +gem 'warden-basic_auth', '~> 0.2.0' # TODO: adds #auto_link which was deprecated in rails 3.1 gem 'rails_autolink', '~> 1.1.6' diff --git a/Gemfile.lock b/Gemfile.lock index d5befb6f918..10618540412 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -463,7 +463,7 @@ GEM equalizer (~> 0.0, >= 0.0.9) warden (1.2.3) rack (>= 1.0) - warden-basic_auth (0.1.1) + warden-basic_auth (0.2.0) warden (~> 1.2) websocket (1.2.1) will_paginate (3.0.5) @@ -569,5 +569,5 @@ DEPENDENCIES uglifier (>= 1.0.3) unicorn warden (~> 1.2) - warden-basic_auth (~> 0.1.0) + warden-basic_auth (~> 0.2.0) will_paginate (~> 3.0) From 820ac0149316ffd2929c96c7ba3612c646b97ec6 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Wed, 13 May 2015 17:29:03 +0100 Subject: [PATCH 30/56] fixed doc for update_strategies --- lib/open_project/authentication.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/open_project/authentication.rb b/lib/open_project/authentication.rb index ac5597c0565..2de6d7083dd 100644 --- a/lib/open_project/authentication.rb +++ b/lib/open_project/authentication.rb @@ -16,7 +16,7 @@ module OpenProject # for this strategy will remain unchanged what ever it is. # # @yield [strategies] A block returning the strategies to be used for this scope. - # @yieldparam [Array] The strategies currently used by this scope. May be empty. + # @yieldparam [Array] strategies The strategies currently used by this scope. May be empty. # @yieldreturn [Array] The strategies to be used by this scope. def update_strategies(scope, store: nil, &block) raise ArgumentError, "invalid scope: #{scope}" unless Scope.values.include? scope From 27868f5c758cfb835508fc0720e9b48f5b0f6e24 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Wed, 13 May 2015 17:29:39 +0100 Subject: [PATCH 31/56] allow custom responses for authentication failures --- lib/open_project/authentication.rb | 16 +++++++ .../authentication/failure_app.rb | 43 ++++++++++++++++--- lib/open_project/authentication/manager.rb | 6 ++- 3 files changed, 57 insertions(+), 8 deletions(-) diff --git a/lib/open_project/authentication.rb b/lib/open_project/authentication.rb index 2de6d7083dd..fe87e6aea72 100644 --- a/lib/open_project/authentication.rb +++ b/lib/open_project/authentication.rb @@ -26,6 +26,22 @@ module OpenProject Manager.store_defaults[scope] = store unless store.nil? Manager.scope_strategies[scope] = block.call current_strategies if block_given? end + + ## + # Allows to handle an authentication failure with a custom response. + # + # @param [Symbol] scope The scope for which to set the custom failure handler. Optional. + # If omitted the default failure handler is set. + # + # @yield [failure_handler] A block returning a custom failure response. + # @yieldparam [Warden::Proxy] warden Warden instance giving access to the would-be + # result message and headers. + # @yieldparam [Hash] warden_options Warden options including the scope of the failed + # strategy and the attempted request path. + # @yieldreturn [Array] A rack standard response such as `[401, {}, ['unauthenticated']]`. + def handle_failure(scope: nil, &block) + Manager.failure_handlers[scope] = block + end end ## diff --git a/lib/open_project/authentication/failure_app.rb b/lib/open_project/authentication/failure_app.rb index e7a17a69ffc..8d7e71daff2 100644 --- a/lib/open_project/authentication/failure_app.rb +++ b/lib/open_project/authentication/failure_app.rb @@ -1,22 +1,51 @@ module OpenProject module Authentication class FailureApp - def call(env) - warden = env['warden'] + attr_reader :failure_handlers - if warden.present? && warden.result == :failure - wrong_credentials warden.message, headers: warden.headers + def initialize(failure_handlers) + @failure_handlers = failure_handlers + end + + def call(env) + warden = self.warden env + scope = self.scope env + + if warden && warden.result == :failure + handler = failure_handlers[scope] || default_failure_handler + + if handler + handler.call warden, warden_options(env) + else + handle_failure warden + end else unauthorized end end - def wrong_credentials(message, headers: {}) - [401, headers, [message]] + def default_failure_handler + failure_handlers[nil] + end + + def handle_failure(warden) + [warden.status || 401, warden.headers, [warden.message]] end def unauthorized - [401, {}, []] + [401, {}, ['unauthorized']] + end + + def warden(env) + env['warden'] + end + + def warden_options(env) + Hash(env['warden.options']) + end + + def scope(env) + warden_options(env)[:scope] end end end diff --git a/lib/open_project/authentication/manager.rb b/lib/open_project/authentication/manager.rb index d83e3b74cfe..57520cf7d5b 100644 --- a/lib/open_project/authentication/manager.rb +++ b/lib/open_project/authentication/manager.rb @@ -28,9 +28,13 @@ module OpenProject @store_defaults ||= Hash.new false end + def failure_handlers + @failure_handlers ||= {} + end + def configure(config) config.default_strategies :session - config.failure_app = OpenProject::Authentication::FailureApp.new + config.failure_app = OpenProject::Authentication::FailureApp.new failure_handlers scope_strategies.each do |scope, strategies| config.scope_defaults scope, strategies: strategies, store: store_defaults[scope] From 28d3dfd827853a2e4c5a2b7f00224b2368b57394 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Wed, 13 May 2015 17:30:14 +0100 Subject: [PATCH 32/56] set WWW-Authenticate header on authentication failure --- lib/api/root.rb | 29 +++++++++++++++++++++ lib/open_project/authentication.rb | 10 +++++++ spec/requests/api/v3/authentication_spec.rb | 4 +++ 3 files changed, 43 insertions(+) diff --git a/lib/api/root.rb b/lib/api/root.rb index e39509f5847..7fadeb54318 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -65,6 +65,27 @@ module API use OpenProject::Authentication::Manager + ## + # We need this to be able to use `Grape::Middleware::Error#error_response` + # outside of the Grape context. We use it outside of the Grape context because + # OpenProject authentication happens in a middleware upstream of Grape. + class GrapeError < Grape::Middleware::Error + def initialize(env) + @env = env + @options = {} + end + end + + ## + # Return JSON error response on authentication failure. + OpenProject::Authentication.handle_failure(scope: API_V3) do |warden, _opts| + e = GrapeError.new warden.env + representer = ::API::V3::Errors::ErrorRepresenter.new ::API::Errors::Unauthenticated.new + + warden.env['api.format'] = 'hal+json' + e.error_response(status: 401, message: representer.to_json, headers: warden.headers) + end + helpers do def current_user User.current @@ -155,6 +176,14 @@ module API error_response(status: api_error.code, message: representer.to_json) end + # Make sure the WWW-Authenticate header is set upon 401. + rescue_from ::API::Errors::Unauthenticated do |e| + headers = { 'WWW-Authenticate' => %(Basic realm="#{OpenProject::Authentication::Realm.realm}") } + representer = ::API::V3::Errors::ErrorRepresenter.new(e) + env['api.format'] = 'hal+json' + error_response(status: e.code, message: representer.to_json, headers: headers) + end + rescue_from ::API::Errors::ErrorBase, rescue_subclasses: true do |e| representer = ::API::V3::Errors::ErrorRepresenter.new(e) env['api.format'] = 'hal+json' diff --git a/lib/open_project/authentication.rb b/lib/open_project/authentication.rb index fe87e6aea72..75101000e43 100644 --- a/lib/open_project/authentication.rb +++ b/lib/open_project/authentication.rb @@ -60,5 +60,15 @@ module OpenProject end end end + + module Realm + module_function + + def realm + 'OpenProject' + end + end end end + +Warden::Strategies::BasicAuth.include OpenProject::Authentication::Realm diff --git a/spec/requests/api/v3/authentication_spec.rb b/spec/requests/api/v3/authentication_spec.rb index ca181601a6d..a53ea9f391c 100644 --- a/spec/requests/api/v3/authentication_spec.rb +++ b/spec/requests/api/v3/authentication_spec.rb @@ -62,6 +62,10 @@ describe API::V3, type: :request do it 'should return the correct JSON response' do expect(JSON.parse(response.body)).to eq response_401 end + + it 'should return the WWW-Authenticate header' do + expect(response.header['WWW-Authenticate']).to include 'Basic realm="OpenProject"' + end end context 'with invalid credentials' do From 5b05bfec904c4a78a1eaa118801e0052eb1463d5 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Wed, 13 May 2015 18:14:22 +0100 Subject: [PATCH 33/56] DRYing --- lib/api/root.rb | 58 ++++++++---------------------- lib/api/utilities/grape_helper.rb | 59 +++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 43 deletions(-) create mode 100644 lib/api/utilities/grape_helper.rb diff --git a/lib/api/root.rb b/lib/api/root.rb index 7fadeb54318..206c7494590 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -36,6 +36,7 @@ require 'open_project/authentication' module API class Root < Grape::API include OpenProject::Authentication::Scope + extend API::Utilities::GrapeHelper prefix :api @@ -65,27 +66,6 @@ module API use OpenProject::Authentication::Manager - ## - # We need this to be able to use `Grape::Middleware::Error#error_response` - # outside of the Grape context. We use it outside of the Grape context because - # OpenProject authentication happens in a middleware upstream of Grape. - class GrapeError < Grape::Middleware::Error - def initialize(env) - @env = env - @options = {} - end - end - - ## - # Return JSON error response on authentication failure. - OpenProject::Authentication.handle_failure(scope: API_V3) do |warden, _opts| - e = GrapeError.new warden.env - representer = ::API::V3::Errors::ErrorRepresenter.new ::API::Errors::Unauthenticated.new - - warden.env['api.format'] = 'hal+json' - e.error_response(status: 401, message: representer.to_json, headers: warden.headers) - end - helpers do def current_user User.current @@ -162,33 +142,25 @@ module API end end - rescue_from ActiveRecord::RecordNotFound do - api_error = ::API::Errors::NotFound.new - representer = ::API::V3::Errors::ErrorRepresenter.new(api_error) - env['api.format'] = 'hal+json' - error_response(status: api_error.code, message: representer.to_json) + def self.auth_headers + { 'WWW-Authenticate' => %(Basic realm="#{OpenProject::Authentication::Realm.realm}") } end - rescue_from ActiveRecord::StaleObjectError do - api_error = ::API::Errors::Conflict.new - representer = ::API::V3::Errors::ErrorRepresenter.new(api_error) - env['api.format'] = 'hal+json' - error_response(status: api_error.code, message: representer.to_json) + ## + # Return JSON error response on authentication failure. + OpenProject::Authentication.handle_failure(scope: API_V3) do |warden, _opts| + e = grape_error_for warden.env + representer = ::API::V3::Errors::ErrorRepresenter.new ::API::Errors::Unauthenticated.new + + warden.env['api.format'] = 'hal+json' + e.error_response status: 401, message: representer.to_json, headers: warden.headers end - # Make sure the WWW-Authenticate header is set upon 401. - rescue_from ::API::Errors::Unauthenticated do |e| - headers = { 'WWW-Authenticate' => %(Basic realm="#{OpenProject::Authentication::Realm.realm}") } - representer = ::API::V3::Errors::ErrorRepresenter.new(e) - env['api.format'] = 'hal+json' - error_response(status: e.code, message: representer.to_json, headers: headers) - end + error_response ::API::Errors::Unauthenticated, headers: auth_headers + error_response ::API::Errors::ErrorBase, rescue_subclasses: true - rescue_from ::API::Errors::ErrorBase, rescue_subclasses: true do |e| - representer = ::API::V3::Errors::ErrorRepresenter.new(e) - env['api.format'] = 'hal+json' - error_response(status: e.code, message: representer.to_json) - end + error_response ActiveRecord::RecordNotFound, ::API::Errors::NotFound.new + error_response ActiveRecord::StaleObjectError, ::API::Errors::Conflict.new # run authentication before each request before do diff --git a/lib/api/utilities/grape_helper.rb b/lib/api/utilities/grape_helper.rb new file mode 100644 index 00000000000..0998de5bc80 --- /dev/null +++ b/lib/api/utilities/grape_helper.rb @@ -0,0 +1,59 @@ +#-- encoding: UTF-8 +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2015 the OpenProject Foundation (OPF) +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See doc/COPYRIGHT.rdoc for more details. +#++ + +module API + module Utilities + module GrapeHelper + ## + # We need this to be able to use `Grape::Middleware::Error#error_response` + # outside of the Grape context. We use it outside of the Grape context because + # OpenProject authentication happens in a middleware upstream of Grape. + class GrapeError < Grape::Middleware::Error + def initialize(env) + @env = env + @options = {} + end + end + + def grape_error_for(env) + GrapeError.new env + end + + def error_response(rescue_from, error = nil, rescue_subclasses: nil, headers: {}) + rescue_from rescue_from, rescue_subclasses: rescue_subclasses do |e| + error ||= e + representer = ::API::V3::Errors::ErrorRepresenter.new error + env['api.format'] = 'hal+json' + + error_response status: error.code, message: representer.to_json, headers: headers + end + end + end + end +end From d990d5f2f7219cc03085399336a39af346f36a17 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Tue, 19 May 2015 09:56:30 +0100 Subject: [PATCH 34/56] restored order --- lib/api/root.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/api/root.rb b/lib/api/root.rb index 206c7494590..b1357d78f3d 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -156,12 +156,12 @@ module API e.error_response status: 401, message: representer.to_json, headers: warden.headers end - error_response ::API::Errors::Unauthenticated, headers: auth_headers - error_response ::API::Errors::ErrorBase, rescue_subclasses: true - error_response ActiveRecord::RecordNotFound, ::API::Errors::NotFound.new error_response ActiveRecord::StaleObjectError, ::API::Errors::Conflict.new + error_response ::API::Errors::Unauthenticated, headers: auth_headers + error_response ::API::Errors::ErrorBase, rescue_subclasses: true + # run authentication before each request before do authenticate From 45b40452557fba851dcd55cd654e2920009615ce Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Tue, 19 May 2015 09:57:56 +0100 Subject: [PATCH 35/56] fixed error_response helper --- lib/api/utilities/grape_helper.rb | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/api/utilities/grape_helper.rb b/lib/api/utilities/grape_helper.rb index 0998de5bc80..d5ad823c5fb 100644 --- a/lib/api/utilities/grape_helper.rb +++ b/lib/api/utilities/grape_helper.rb @@ -45,14 +45,24 @@ module API GrapeError.new env end - def error_response(rescue_from, error = nil, rescue_subclasses: nil, headers: {}) - rescue_from rescue_from, rescue_subclasses: rescue_subclasses do |e| - error ||= e - representer = ::API::V3::Errors::ErrorRepresenter.new error + def error_response(rescued_error, error = nil, rescue_subclasses: nil, headers: {}) + default_response = lambda do |e| + representer = ::API::V3::Errors::ErrorRepresenter.new e env['api.format'] = 'hal+json' - error_response status: error.code, message: representer.to_json, headers: headers + error_response status: e.code, message: representer.to_json, headers: headers end + + response = + if error.nil? + default_response + else + lambda { instance_exec error, &default_response } + end + + # We do this lambda business because #rescue_from behaves differently + # depending on the number of parameters the passed block accepts. + rescue_from rescued_error, rescue_subclasses: rescue_subclasses, &response end end end From c1e597c18615883b9407be92cc4e7b6821df769d Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Thu, 21 May 2015 16:00:36 +0100 Subject: [PATCH 36/56] fixed response headers (content type, realm) --- lib/api/root.rb | 3 +-- lib/api/utilities/grape_helper.rb | 7 +++++-- lib/open_project/authentication.rb | 2 +- spec/requests/api/v3/authentication_spec.rb | 8 ++++++++ 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/api/root.rb b/lib/api/root.rb index b1357d78f3d..345142179b8 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -149,10 +149,9 @@ module API ## # Return JSON error response on authentication failure. OpenProject::Authentication.handle_failure(scope: API_V3) do |warden, _opts| - e = grape_error_for warden.env + e = grape_error_for warden.env, self representer = ::API::V3::Errors::ErrorRepresenter.new ::API::Errors::Unauthenticated.new - warden.env['api.format'] = 'hal+json' e.error_response status: 401, message: representer.to_json, headers: warden.headers end diff --git a/lib/api/utilities/grape_helper.rb b/lib/api/utilities/grape_helper.rb index d5ad823c5fb..bb887d0fd03 100644 --- a/lib/api/utilities/grape_helper.rb +++ b/lib/api/utilities/grape_helper.rb @@ -41,8 +41,11 @@ module API end end - def grape_error_for(env) - GrapeError.new env + def grape_error_for(env, api) + GrapeError.new(env).tap do |e| + e.options[:content_types] = api.content_types + e.options[:format] = 'hal+json' + end end def error_response(rescued_error, error = nil, rescue_subclasses: nil, headers: {}) diff --git a/lib/open_project/authentication.rb b/lib/open_project/authentication.rb index 75101000e43..c0666454774 100644 --- a/lib/open_project/authentication.rb +++ b/lib/open_project/authentication.rb @@ -71,4 +71,4 @@ module OpenProject end end -Warden::Strategies::BasicAuth.include OpenProject::Authentication::Realm +Warden::Strategies::BasicAuth.prepend OpenProject::Authentication::Realm diff --git a/spec/requests/api/v3/authentication_spec.rb b/spec/requests/api/v3/authentication_spec.rb index a53ea9f391c..8315d7c420d 100644 --- a/spec/requests/api/v3/authentication_spec.rb +++ b/spec/requests/api/v3/authentication_spec.rb @@ -80,6 +80,14 @@ describe API::V3, type: :request do it 'should return the correct JSON response' do expect(JSON.parse(response.body)).to eq response_401 end + + it 'should return the correct content type header' do + expect(response.headers['Content-Type']).to eq 'application/hal+json; charset=utf-8' + end + + it 'should return the WWW-Authenticate header' do + expect(response.header['WWW-Authenticate']).to include 'Basic realm="OpenProject"' + end end context 'with valid credentials' do From 70fe41cb53b6ea829f993f63691ab86edece0af9 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Thu, 21 May 2015 17:54:14 +0100 Subject: [PATCH 37/56] use separate error code when unauthenticated also distinct error messages for missing or wrong credentials --- config/locales/de.yml | 1 + config/locales/en.yml | 1 + doc/apiv3-documentation.apib | 5 +++-- lib/api/errors/unauthenticated.rb | 4 ++-- lib/api/root.rb | 4 +++- lib/api/v3/errors/error_representer.rb | 4 +++- spec/requests/api/v3/authentication_spec.rb | 8 ++++++-- 7 files changed, 19 insertions(+), 8 deletions(-) diff --git a/config/locales/de.yml b/config/locales/de.yml index 0d24a22072f..412c8ed5e2e 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -1684,6 +1684,7 @@ de: lock_version: "Sperrversion" errors: code_401: "Sie müssen sich authentifizieren, um auf die Ressource zugreifen zu können." + code_401_wrong_credentials: "Die von Ihnen benutzten Zugangsdaten sind nicht korrekt." code_403: "Sie sind nicht berechtigt auf diese Ressource zuzugreifen." code_404: "Die angeforderte Ressource konnte nicht gefunden werden." code_409: "Die Ressource konnte wegen parallelen Zugriffs nicht aktualisiert werden." diff --git a/config/locales/en.yml b/config/locales/en.yml index 97afd0070c8..91e93c184a9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1676,6 +1676,7 @@ en: lock_version: "Lock Version" errors: code_401: "You need to be authenticated to access this resource." + code_401_wrong_credentials: "You did not provide the correct credentials." code_403: "You are not authorized to access this resource." code_404: "The requested resource could not be found." code_409: "Couldn\'t update the resource because of conflicting modifications." diff --git a/doc/apiv3-documentation.apib b/doc/apiv3-documentation.apib index 1c216357cfe..e15edc32ec4 100644 --- a/doc/apiv3-documentation.apib +++ b/doc/apiv3-documentation.apib @@ -207,8 +207,9 @@ embedded errors or simply state that multiple errors occured. * `urn:openproject-org:api:v3:errors:InvalidRequestBody` (**HTTP 400**) - The format of the request body did not match the servers expectation * `urn:openproject-org:api:v3:errors:InvalidRenderContext` (**HTTP 400**) - The client specified a rendering context that does not exist -* `urn:openproject-org:api:v3:errors:InvalidUserStatusTransition` (**HTTP 400**) The client used an invalid transition in the attempt to change the status of a user account. -* `urn:openproject-org:api:v3:errors:MissingPermission` (**HTTP 401** / **HTTP 403**) - The client does not have the needed permissions to perform the requested action +* `urn:openproject-org:api:v3:errors:InvalidUserStatusTransition` (**HTTP 400**) - The client used an invalid transition in the attempt to change the status of a user account. +* `urn:openproject-org:api:v3:errors:Unauthenticated` (**HTTP 401**) - The client has to authenticate to access the requested resource. +* `urn:openproject-org:api:v3:errors:MissingPermission` (**HTTP 403**) - The client does not have the needed permissions to perform the requested action * `urn:openproject-org:api:v3:errors:NotFound` (**HTTP 404**) - Default for HTTP 404 when no further information is available * `urn:openproject-org:api:v3:errors:UpdateConflict` (**HTTP 409**) - The resource changed between GET-ing it and performing an update on it * `urn:openproject-org:api:v3:errors:TypeNotSupported` (**HTTP 415**) - The request contained data in an unsupported media type (Content-Type) diff --git a/lib/api/errors/unauthenticated.rb b/lib/api/errors/unauthenticated.rb index 9742a289329..e098f577d62 100644 --- a/lib/api/errors/unauthenticated.rb +++ b/lib/api/errors/unauthenticated.rb @@ -30,8 +30,8 @@ module API module Errors class Unauthenticated < ErrorBase - def initialize - super 401, I18n.t('api_v3.errors.code_401') + def initialize(message = I18n.t('api_v3.errors.code_401')) + super 401, message end end end diff --git a/lib/api/root.rb b/lib/api/root.rb index 345142179b8..ea12449f095 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -150,7 +150,9 @@ module API # Return JSON error response on authentication failure. OpenProject::Authentication.handle_failure(scope: API_V3) do |warden, _opts| e = grape_error_for warden.env, self - representer = ::API::V3::Errors::ErrorRepresenter.new ::API::Errors::Unauthenticated.new + error_message = I18n.t('api_v3.errors.code_401_wrong_credentials') + api_error = ::API::Errors::Unauthenticated.new error_message + representer = ::API::V3::Errors::ErrorRepresenter.new api_error e.error_response status: 401, message: representer.to_json, headers: warden.headers end diff --git a/lib/api/v3/errors/error_representer.rb b/lib/api/v3/errors/error_representer.rb index f87f117d63a..08d6a451fe6 100644 --- a/lib/api/v3/errors/error_representer.rb +++ b/lib/api/v3/errors/error_representer.rb @@ -62,7 +62,9 @@ module API 'urn:openproject-org:api:v3:errors:UpdateConflict' when ::API::Errors::NotFound 'urn:openproject-org:api:v3:errors:NotFound' - when ::API::Errors::Unauthenticated, ::API::Errors::Unauthorized + when ::API::Errors::Unauthenticated + 'urn:openproject-org:api:v3:errors:Unauthenticated' + when ::API::Errors::Unauthorized 'urn:openproject-org:api:v3:errors:MissingPermission' when ::API::Errors::UnwritableProperty 'urn:openproject-org:api:v3:errors:PropertyIsReadOnly' diff --git a/spec/requests/api/v3/authentication_spec.rb b/spec/requests/api/v3/authentication_spec.rb index 8315d7c420d..15e98740a74 100644 --- a/spec/requests/api/v3/authentication_spec.rb +++ b/spec/requests/api/v3/authentication_spec.rb @@ -37,11 +37,13 @@ describe API::V3, type: :request do { '_embedded' => {}, '_type' => 'Error', - 'errorIdentifier' => 'urn:openproject-org:api:v3:errors:MissingPermission', - 'message' => 'You need to be authenticated to access this resource.' + 'errorIdentifier' => 'urn:openproject-org:api:v3:errors:Unauthenticated', + 'message' => expected_message } end + let(:expected_message) { 'You need to be authenticated to access this resource.' } + Strategies = OpenProject::Authentication::Strategies::Warden def basic_auth(user, password) @@ -69,6 +71,8 @@ describe API::V3, type: :request do end context 'with invalid credentials' do + let(:expected_message) { 'You did not provide the correct credentials.' } + before do get resource, {}, basic_auth(username, password.reverse) end From 5a0f4606ccbdb80f864a111ceb60cba866d9bb91 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Fri, 22 May 2015 08:47:11 +0100 Subject: [PATCH 38/56] updated error code expectation --- spec/requests/api/v3/support/response_examples.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/api/v3/support/response_examples.rb b/spec/requests/api/v3/support/response_examples.rb index 30cb6ca221f..6a2c652513e 100644 --- a/spec/requests/api/v3/support/response_examples.rb +++ b/spec/requests/api/v3/support/response_examples.rb @@ -92,7 +92,7 @@ end shared_examples_for 'unauthenticated access' do it_behaves_like 'error response', 401, - 'MissingPermission', + 'Unauthenticated', I18n.t('api_v3.errors.code_401') end From f8e0404c6f2688a490fbe509a4efdadace28ef1c Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Sat, 23 May 2015 02:48:11 +0200 Subject: [PATCH 39/56] remove link tag from non-links [ci skip] --- doc/QUICK_START.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/QUICK_START.md b/doc/QUICK_START.md index 5c853b1f17e..29137daabc2 100644 --- a/doc/QUICK_START.md +++ b/doc/QUICK_START.md @@ -43,8 +43,8 @@ These are generic (and condensed) installation instructions for the **current de * Git * Database (MySQL 5.x/PostgreSQL 8.x) * Ruby 2.1.x -* [Node.js] (version v0.10.x) -* [Bundler] (version 1.5.1 or higher required) +* Node.js (version v0.10.x) +* Bundler (version 1.5.1 or higher required) ### Install Dependencies From 7e8b95e1692cfba1b34911525a612ccfb303354d Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Mon, 25 May 2015 14:45:19 +0100 Subject: [PATCH 40/56] more documentation --- config/configuration.yml.example | 8 ++++++++ doc/CONFIGURATION.md | 18 +++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/config/configuration.yml.example b/config/configuration.yml.example index 0ceeede6cd0..05a7c361d41 100644 --- a/config/configuration.yml.example +++ b/config/configuration.yml.example @@ -277,6 +277,14 @@ default: # generated and sent to the user who then has to change their password upon login. disable_password_choice: false + # You can configure a global set of credentials to authenticate towards + # API v3 via basic auth. Note that this will grant admin access. Use with care + # and make sure the password does not leak. + # authentication: + # global_basic_auth: + # user: admin + # password: admin + # specific configuration options for production environment # that overrides the default ones # production: diff --git a/doc/CONFIGURATION.md b/doc/CONFIGURATION.md index 4bff000debf..f395cf32ae3 100644 --- a/doc/CONFIGURATION.md +++ b/doc/CONFIGURATION.md @@ -86,6 +86,7 @@ storage config above like this: * [`hidden_menu_items`](#hidden-menu-items) (default: {}) * [`disabled_modules`](#disabled-modules) (default: []) * [`blacklisted_routes`](#blacklisted-routes) (default: []) +* [`global_basic_auth`](#global-basic-auth) ### disable password login @@ -186,7 +187,7 @@ OPENPROJECT_HIDDEN__MENU__ITEMS_ADMIN__MENU='roles types' *default: []* -You can blacklist specific routes +You can blacklist specific routes The following example forbid all routes for above disabled menu: ``` @@ -237,6 +238,21 @@ The option to use a string is mostly relevant for when you want to override the OPENPROJECT_DISABLED__MODULES='backlogs meetings' ``` +### global basic auth + +*default: none* + +You can define a global set of credentials used to authenticate towards API v3. +Example section for `configuration.yml`: + +``` +default: + authentication: + global_basic_auth: + user: admin + password: admin +``` + ## Email configuration * `email_delivery_method`: The way emails should be delivered. Possible values: `smtp` or `sendmail` From 2a465785d1de28e2db71f47070a2e464ea1dc8ed Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Mon, 25 May 2015 14:45:40 +0100 Subject: [PATCH 41/56] allow digit-only but not empty passwords --- .../strategies/warden/global_basic_auth.rb | 6 ++- .../warden/global_basic_auth_spec.rb | 48 ++++++++++++++++--- 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/lib/open_project/authentication/strategies/warden/global_basic_auth.rb b/lib/open_project/authentication/strategies/warden/global_basic_auth.rb index 8357725ab2f..824eb1ba622 100644 --- a/lib/open_project/authentication/strategies/warden/global_basic_auth.rb +++ b/lib/open_project/authentication/strategies/warden/global_basic_auth.rb @@ -34,6 +34,10 @@ module OpenProject raise ArgumentError, "global user must not be '#{UserBasicAuth.user}'" end + if config[:password].blank? + raise ArgumentError, "password must not be empty" + end + @configuration = config end @@ -55,7 +59,7 @@ module OpenProject end def self.password - configuration[:password] + configuration[:password].to_s end ## diff --git a/spec/lib/open_project/authentication/strategies/warden/global_basic_auth_spec.rb b/spec/lib/open_project/authentication/strategies/warden/global_basic_auth_spec.rb index ca789a87f26..925ff4f2f7d 100644 --- a/spec/lib/open_project/authentication/strategies/warden/global_basic_auth_spec.rb +++ b/spec/lib/open_project/authentication/strategies/warden/global_basic_auth_spec.rb @@ -28,14 +28,48 @@ require 'spec_helper' -describe OpenProject::Authentication::Strategies::Warden::GlobalBasicAuth do - it 'does not allow the UserBasicAuth user name' do - configuration = lambda do - user = OpenProject::Authentication::Strategies::Warden::UserBasicAuth.user - OpenProject::Authentication::Strategies::Warden::GlobalBasicAuth.configure!( - user: user, password: 'foo') +Strategies = OpenProject::Authentication::Strategies::Warden + +describe Strategies::GlobalBasicAuth do + let(:user) { 'someuser' } + let(:password) { 'somepassword' } + + let(:config) do + lambda do + Strategies::GlobalBasicAuth.configure! user: user, password: password + end + end + + context "with UserBasicAuth's reserved username" do + let(:user) { Strategies::UserBasicAuth.user } + + before do + allow(Strategies::UserBasicAuth).to receive(:user).and_return('schluessel') end - expect(configuration).to raise_error(ArgumentError) + it "raises an error" do + expect(config).to raise_error("global user must not be 'schluessel'") + end + end + + context 'with an empty pasword' do + let(:password) { '' } + + it 'raises an error' do + expect(config).to raise_error('password must not be empty') + end + end + + context 'with digits-only password' do + let(:password) { 1234 } + let(:strategy) { Strategies::GlobalBasicAuth.new nil } + + before do + config.call + end + + it 'must authenticate successfully' do + expect(strategy.authenticate_user(user, '1234')).to be_truthy + end end end From be08d64991071837f36dfb7c95389d59f6866d18 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Tue, 26 May 2015 09:02:08 +0100 Subject: [PATCH 42/56] do nothing when configuration is missing altogether --- .../authentication/strategies/warden/global_basic_auth.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/open_project/authentication/strategies/warden/global_basic_auth.rb b/lib/open_project/authentication/strategies/warden/global_basic_auth.rb index 824eb1ba622..fd5b52268ba 100644 --- a/lib/open_project/authentication/strategies/warden/global_basic_auth.rb +++ b/lib/open_project/authentication/strategies/warden/global_basic_auth.rb @@ -30,6 +30,8 @@ module OpenProject # @raise [ArgumentError] Raises an error if the configured user name collides with the # user name used for UserBasicAuth (apikey). def self.configure!(config = openproject_config) + return {} if config.empty? + if config[:user] == UserBasicAuth.user raise ArgumentError, "global user must not be '#{UserBasicAuth.user}'" end From 0b07779e751baf918f25e73152be8f300ab9c210 Mon Sep 17 00:00:00 2001 From: Jonas Heinrich Date: Tue, 26 May 2015 15:35:16 +0200 Subject: [PATCH 43/56] Adapt version to corresponding OpenProject core version. --- lib/open_project/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/open_project/version.rb b/lib/open_project/version.rb index 1173a357fd6..5dad18c01b5 100644 --- a/lib/open_project/version.rb +++ b/lib/open_project/version.rb @@ -33,7 +33,7 @@ module OpenProject module VERSION #:nodoc: MAJOR = 4 MINOR = 1 - PATCH = 1 + PATCH = 2 TINY = PATCH # Redmine compat # Used by semver to define the special version (if any). From 87e759144d8443cc3a6e3b040a0ba9e686e18bb6 Mon Sep 17 00:00:00 2001 From: RobinWagner Date: Tue, 26 May 2015 15:50:41 +0200 Subject: [PATCH 44/56] Add wiki syntax for using acronyms Textile supports the use of acronyms. However, the wiki syntax details page currently does not include a description of this feature. --- app/views/help/wiki_syntax_detailed.html.erb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/views/help/wiki_syntax_detailed.html.erb b/app/views/help/wiki_syntax_detailed.html.erb index d2ea097c5b8..78d789643ce 100644 --- a/app/views/help/wiki_syntax_detailed.html.erb +++ b/app/views/help/wiki_syntax_detailed.html.erb @@ -236,6 +236,16 @@ To go live, all you need to add is a database and a web server. {{toc}} => left aligned toc {{>toc}} => right aligned toc +

Acronyms

+

Display tooltip for acronym by entering tooltip in parantheses after upper case +acronym.

+
+WHO(World Health Organisation) => Displays "World Health Organisation" as
+tooltip of "WHO"
+
+

+ WHO +

Macros

OpenProject has the following builtin macros:

From 4a69d2505a743ddc77e05ec855b114d1df8380c1 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Wed, 27 May 2015 10:14:13 +0200 Subject: [PATCH 45/56] replace ternaries returning booleans - foo ? true : false was replaced where foo already was bool - foo ? true : false was kept, when the ternary did a bool conversion - foo ? false : true was replaced by either negation or replacing == with != Note: I intentionally left one occurence that is fixed in another running PR and would cause a conflict --- app/controllers/search_controller.rb | 2 +- app/controllers/work_packages_controller.rb | 2 +- app/controllers/workflows_controller.rb | 2 +- app/helpers/sort_helper.rb | 2 +- app/models/enumeration.rb | 2 +- app/models/mail_handler.rb | 2 +- extra/svn/reposman.rb | 2 +- features/step_definitions/custom_web_steps.rb | 2 +- features/step_definitions/general_steps.rb | 4 ++-- features/step_definitions/password_steps.rb | 2 +- features/step_definitions/web_steps.rb | 2 +- lib/open_project/text_formatting.rb | 2 +- 12 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 875db46fec4..0c3ea217784 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -33,7 +33,7 @@ class SearchController < ApplicationController def index @question = params[:q] || '' @question.strip! - @all_words = params[:all_words] || (params[:submit] ? false : true) + @all_words = params[:all_words] || !params[:submit] @titles_only = !params[:titles_only].nil? projects_to_search = diff --git a/app/controllers/work_packages_controller.rb b/app/controllers/work_packages_controller.rb index 5e620a5617f..22186c72c3d 100644 --- a/app/controllers/work_packages_controller.rb +++ b/app/controllers/work_packages_controller.rb @@ -425,7 +425,7 @@ class WorkPackagesController < ApplicationController end def send_notifications? - params[:send_notification] == '0' ? false : true + params[:send_notification] != '0' end def per_page_param diff --git a/app/controllers/workflows_controller.rb b/app/controllers/workflows_controller.rb index 368e5e0d430..2967ee95fd4 100644 --- a/app/controllers/workflows_controller.rb +++ b/app/controllers/workflows_controller.rb @@ -58,7 +58,7 @@ class WorkflowsController < ApplicationController end end - @used_statuses_only = (params[:used_statuses_only] == '0' ? false : true) + @used_statuses_only = params[:used_statuses_only] != '0' if @type && @used_statuses_only && @type.statuses.any? @statuses = @type.statuses end diff --git a/app/helpers/sort_helper.rb b/app/helpers/sort_helper.rb index 5d896bf14b2..3077faf3890 100644 --- a/app/helpers/sort_helper.rb +++ b/app/helpers/sort_helper.rb @@ -143,7 +143,7 @@ module SortHelper def normalize! @criteria ||= [] - @criteria = @criteria.map { |s| s = s.to_a; [s.first, (s.last == false || s.last == 'desc') ? false : true] } + @criteria = @criteria.map { |s| s = s.to_a; [s.first, !(s.last == false || s.last == 'desc')] } @criteria = @criteria.select { |k, _o| @available_criteria.has_key?(k) } if @available_criteria @criteria.slice!(3) self diff --git a/app/models/enumeration.rb b/app/models/enumeration.rb index a3d6ae7f92d..5586794e305 100644 --- a/app/models/enumeration.rb +++ b/app/models/enumeration.rb @@ -149,7 +149,7 @@ class Enumeration < ActiveRecord::Base # Are the new and previous fields equal? def self.same_active_state?(new, previous) - new = (new == '1' ? true : false) + new = new == '1' new == previous end diff --git a/app/models/mail_handler.rb b/app/models/mail_handler.rb index e1b657a121e..e8cf8f3ddfa 100644 --- a/app/models/mail_handler.rb +++ b/app/models/mail_handler.rb @@ -48,7 +48,7 @@ class MailHandler < ActionMailer::Base # Status overridable by default @@handler_options[:allow_override] << 'status' unless @@handler_options[:issue].has_key?(:status) - @@handler_options[:no_permission_check] = (@@handler_options[:no_permission_check].to_s == '1' ? true : false) + @@handler_options[:no_permission_check] = @@handler_options[:no_permission_check].to_s == '1' email.force_encoding('ASCII-8BIT') if email.respond_to?(:force_encoding) super email diff --git a/extra/svn/reposman.rb b/extra/svn/reposman.rb index 88b1b6d087d..ae8241ae149 100755 --- a/extra/svn/reposman.rb +++ b/extra/svn/reposman.rb @@ -205,7 +205,7 @@ def set_owner_and_rights(project, repos_path, &block) end def other_read_right?(file) - (File.stat(file).mode & 0007).zero? ? false : true + !(File.stat(file).mode & 0007).zero? end def owner_name(file) diff --git a/features/step_definitions/custom_web_steps.rb b/features/step_definitions/custom_web_steps.rb index 3a9561e96d5..984e820a039 100644 --- a/features/step_definitions/custom_web_steps.rb +++ b/features/step_definitions/custom_web_steps.rb @@ -60,7 +60,7 @@ end Then /^"([^"]*)" should (not )?be selectable from "([^"]*)"$/ do |value, negative, select_id| # more page.evaluate ugliness find(:xpath, '//body') - bool = negative ? false : true + bool = !negative (page.evaluate_script("$('#{select_id}').select('option[value=#{value}]').first.disabled") =~ /^#{bool}$/).should be_present end diff --git a/features/step_definitions/general_steps.rb b/features/step_definitions/general_steps.rb index f29bc672acb..f6a238d7e9f 100644 --- a/features/step_definitions/general_steps.rb +++ b/features/step_definitions/general_steps.rb @@ -214,8 +214,8 @@ Given /^there are the following issue status:$/ do |table| table.hashes.each_with_index do |t, i| status = Status.find_by_name(t['name']) status = Status.new name: t['name'] if status.nil? - status.is_closed = t['is_closed'] == 'true' ? true : false - status.is_default = t['is_default'] == 'true' ? true : false + status.is_closed = t['is_closed'] == 'true' + status.is_default = t['is_default'] == 'true' status.position = t['position'] ? t['position'] : i status.default_done_ratio = t['default_done_ratio'] status.save! diff --git a/features/step_definitions/password_steps.rb b/features/step_definitions/password_steps.rb index f9f8956d9ff..a56a0a977a8 100644 --- a/features/step_definitions/password_steps.rb +++ b/features/step_definitions/password_steps.rb @@ -143,7 +143,7 @@ def set_user_attribute(login, attribute, value) end Given /^the user "(.+)" is(not |) forced to change his password$/ do |login, disable| - set_user_attribute(login, :force_password_change, (disable == 'not ') ? false : true) + set_user_attribute(login, :force_password_change, disable != 'not ') end Given /^I use the first existing token to request a password reset$/ do diff --git a/features/step_definitions/web_steps.rb b/features/step_definitions/web_steps.rb index 90a9f7dbbad..742dfb5cec7 100644 --- a/features/step_definitions/web_steps.rb +++ b/features/step_definitions/web_steps.rb @@ -319,7 +319,7 @@ end Then /^there should be a( disabled)? "(.+)" field( visible| invisible)?$/ do |disabled, fieldname, visible| # Checking for a disabled field will only work for field with labels where the label # has a correctly filled "for" attribute - visibility = visible && visible.include?('invisible') ? false : true + visibility = visible && !visible.include?('invisible') if disabled # disabled fields can not be found via find_field diff --git a/lib/open_project/text_formatting.rb b/lib/open_project/text_formatting.rb index 55f8e133a60..1429b0cf952 100644 --- a/lib/open_project/text_formatting.rb +++ b/lib/open_project/text_formatting.rb @@ -80,7 +80,7 @@ module OpenProject # don't return html in edit mode when textile or text formatting is enabled return text if edit project = options[:project] || @project || (obj && obj.respond_to?(:project) ? obj.project : nil) - only_path = options.delete(:only_path) == false ? false : true + only_path = options.delete(:only_path) != false # offer 'plain' as readable version for 'no formatting' to callers options_format = options[:format] == 'plain' ? '' : options[:format] From a7c99913961192e41a80e1a8f0a0cbab7fec4623 Mon Sep 17 00:00:00 2001 From: Florian Kraft Date: Wed, 27 May 2015 11:15:28 +0200 Subject: [PATCH 46/56] adds a fix for syntax problem when generating json by hand This is a small fix for a issue that cna pop up when the default case is used for the first day of the week set in the datepicker via admin settings. It would have generated the wrong JSON structure breaking _everything_. Credit goes to @NobodysNightmare for finding this. Signed-off-by: Florian Kraft --- app/helpers/application_helper.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 68ece110647..df7747956fd 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -502,7 +502,10 @@ module ApplicationHelper when 6 '6' # Saturday else - '' # use language + # use language (pass a blank string into the JSON object, + # as the datepicker implementation checks for numbers in + # /frontend/app/misc/datepicker-defaults.js:34) + '""' end # FIXME: Get rid of this abomination js = "var CS = { lang: '#{current_language.to_s.downcase}', firstDay: #{start_of_week} };" From 1d0146dfda930aab29d825f0b7ef62e280e307c1 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Wed, 27 May 2015 10:26:34 +0100 Subject: [PATCH 47/56] amended comment --- .../authentication/strategies/warden/global_basic_auth.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/open_project/authentication/strategies/warden/global_basic_auth.rb b/lib/open_project/authentication/strategies/warden/global_basic_auth.rb index fd5b52268ba..97b9a93f0ec 100644 --- a/lib/open_project/authentication/strategies/warden/global_basic_auth.rb +++ b/lib/open_project/authentication/strategies/warden/global_basic_auth.rb @@ -28,7 +28,10 @@ module OpenProject # # @param [Hash] config The configuration to be used. Must contain :user and :password. # @raise [ArgumentError] Raises an error if the configured user name collides with the - # user name used for UserBasicAuth (apikey). + # user name used for UserBasicAuth (apikey) or if the + # provided password is empty. + # @return [Hash] The new hash set for the configuration or an empty hash if + # no configuration was provided. def self.configure!(config = openproject_config) return {} if config.empty? From 8d7bec598159b2f5ab2ac771682f3396901a4637 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Wed, 27 May 2015 10:27:08 +0100 Subject: [PATCH 48/56] always show api key for use with APIv3 --- app/views/my/_sidebar.html.erb | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/app/views/my/_sidebar.html.erb b/app/views/my/_sidebar.html.erb index dd08b56ebb7..d27a9953c77 100644 --- a/app/views/my/_sidebar.html.erb +++ b/app/views/my/_sidebar.html.erb @@ -44,19 +44,18 @@ See doc/COPYRIGHT.rdoc for more details. (<%= link_to l(:button_reset), {:action => 'reset_rss_key'}, :method => :post %>)

<% end %> -<% if Setting.rest_api_enabled? %> -

<%= l(:label_api_access_key) %>

-
- <%= link_to_function(l(:button_show), "$('api-access-key').toggle();")%> - 
<%= h(@user.api_key) %>
-
- <%= javascript_tag("$('api-access-key').hide();") %> -

- <% if @user.api_token %> - <%= l(:label_api_access_key_created_on, distance_of_time_in_words(Time.now, @user.api_token.created_on)) %> - <% else %> - <%= l(:label_missing_api_access_key) %> - <% end %> - (<%= link_to l(:button_reset), {:action => 'reset_api_key'}, :method => :post %>) -

-<% end %> + +

<%= l(:label_api_access_key) %>

+
+ <%= link_to_function(l(:button_show), "$('api-access-key').toggle();")%> + 
<%= h(@user.api_key) %>
+
+<%= javascript_tag("$('api-access-key').hide();") %> +

+ <% if @user.api_token %> + <%= l(:label_api_access_key_created_on, distance_of_time_in_words(Time.now, @user.api_token.created_on)) %> + <% else %> + <%= l(:label_missing_api_access_key) %> + <% end %> + (<%= link_to l(:button_reset), {:action => 'reset_api_key'}, :method => :post %>) +

From 197945c6c91436d43cb31d8b513d7607077efc92 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Wed, 27 May 2015 12:05:40 +0200 Subject: [PATCH 49/56] style [ci skip] --- app/helpers/sort_helper.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/app/helpers/sort_helper.rb b/app/helpers/sort_helper.rb index 3077faf3890..031e638212f 100644 --- a/app/helpers/sort_helper.rb +++ b/app/helpers/sort_helper.rb @@ -143,8 +143,15 @@ module SortHelper def normalize! @criteria ||= [] - @criteria = @criteria.map { |s| s = s.to_a; [s.first, !(s.last == false || s.last == 'desc')] } - @criteria = @criteria.select { |k, _o| @available_criteria.has_key?(k) } if @available_criteria + @criteria = @criteria.map { |s| + s = s.to_a + [s.first, !(s.last == false || s.last == 'desc')] + } + + if @available_criteria + @criteria = @criteria.select { |k, _o| @available_criteria.has_key?(k) } + end + @criteria.slice!(3) self end From 09e86e3cd8317994dafa18faf3c7a0772ecfd7d7 Mon Sep 17 00:00:00 2001 From: Jonas Heinrich Date: Wed, 27 May 2015 14:56:22 +0200 Subject: [PATCH 50/56] Bump version to 4.3.0-alpha --- lib/open_project/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/open_project/version.rb b/lib/open_project/version.rb index afe177aabbf..d57d77e06f9 100644 --- a/lib/open_project/version.rb +++ b/lib/open_project/version.rb @@ -32,7 +32,7 @@ require 'rexml/document' module OpenProject module VERSION #:nodoc: MAJOR = 4 - MINOR = 2 + MINOR = 3 PATCH = 0 TINY = PATCH # Redmine compat From fdf5288f25a2e9699fd30bb825730bf9feba29ba Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Wed, 27 May 2015 15:01:38 +0200 Subject: [PATCH 51/56] set line length limit to 100 99 is really a stupid limit. We live in a decimal world, we should have decimal limits... --- .rubocop.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index 151ba2e17e9..5ae61c918bb 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -125,7 +125,7 @@ LineEndConcatenation: Enabled: false LineLength: - Max: 99 + Max: 100 MethodLength: Enabled: false From 3814c9bb2bda5ae582f384a5f3dca419aa403912 Mon Sep 17 00:00:00 2001 From: Jonas Heinrich Date: Wed, 27 May 2015 15:44:29 +0000 Subject: [PATCH 52/56] Makes the workflow section even more readable --- lib/redmine/default_data/loader.rb | 75 +++++++++++++++++++++--------- 1 file changed, 54 insertions(+), 21 deletions(-) diff --git a/lib/redmine/default_data/loader.rb b/lib/redmine/default_data/loader.rb index b1657fca7be..8e6aac7806e 100644 --- a/lib/redmine/default_data/loader.rb +++ b/lib/redmine/default_data/loader.rb @@ -201,30 +201,63 @@ module Redmine none = Type.standard_type # Issue statuses - new = Status.create!(name: l(:default_status_new), is_closed: false, is_default: true, position: 1) - specified = Status.create!(name: l(:default_status_specified), is_closed: false, is_default: false, position: 2) - confirmed = Status.create!(name: l(:default_status_confirmed), is_closed: false, is_default: false, position: 3) - to_be_scheduled = Status.create!(name: l(:default_status_to_be_scheduled), is_closed: false, is_default: false, position: 4) - scheduled = Status.create!(name: l(:default_status_scheduled), is_closed: false, is_default: false, position: 5) - in_progress = Status.create!(name: l(:default_status_in_progress), is_closed: false, is_default: false, position: 6) - tested = Status.create!(name: l(:default_status_tested), is_closed: false, is_default: false, position: 7) - on_hold = Status.create!(name: l(:default_status_on_hold), is_closed: false, is_default: false, position: 8) - rejected = Status.create!(name: l(:default_status_rejected), is_closed: true, is_default: false, position: 9) - closed = Status.create!(name: l(:default_status_closed), is_closed: true, is_default: false, position: 10) + new = Status.create!(name: l(:default_status_new), + is_closed: false, + is_default: true, + position: 1) + specified = Status.create!(name: l(:default_status_specified), + is_closed: false, + is_default: false, + position: 2) + confirmed = Status.create!(name: l(:default_status_confirmed), + is_closed: false, + is_default: false, + position: 3) + to_be_scheduled = Status.create!(name: l(:default_status_to_be_scheduled), + is_closed: false, + is_default: false, + position: 4) + scheduled = Status.create!(name: l(:default_status_scheduled), + is_closed: false, + is_default: false, + position: 5) + in_progress = Status.create!(name: l(:default_status_in_progress), + is_closed: false, + is_default: false, + position: 6) + tested = Status.create!(name: l(:default_status_tested), + is_closed: false, + is_default: false, + position: 7) + on_hold = Status.create!(name: l(:default_status_on_hold), + is_closed: false, + is_default: false, + position: 8) + rejected = Status.create!(name: l(:default_status_rejected), + is_closed: true, + is_default: false, + position: 9) + closed = Status.create!(name: l(:default_status_closed), + is_closed: true, + is_default: false, + position: 10) # Workflow - Each type has its own workflow - statuses = { task.name => [new, in_progress, on_hold, rejected, closed], - deliverable.name => [new, specified, in_progress, on_hold, rejected, closed], - none.name => [new, in_progress, rejected, closed], - milestone.name => [new, to_be_scheduled, scheduled, in_progress, on_hold, rejected, closed], - phase.name => [new, to_be_scheduled, scheduled, in_progress, on_hold, rejected, closed], - bug.name => [new, confirmed, in_progress, tested, on_hold, rejected, closed], - feature.name => [new, specified, confirmed, in_progress, tested, on_hold, rejected, closed] } - statuses.each { |t, statuses_for_t| - statuses_for_t.each { |os| - statuses_for_t.each { |ns| + workflows = { task.id => [new, in_progress, on_hold, rejected, closed], + deliverable.id => [new, specified, in_progress, on_hold, rejected, closed], + none.id => [new, in_progress, rejected, closed], + milestone.id => [new, to_be_scheduled, scheduled, in_progress, on_hold, rejected, closed], + phase.id => [new, to_be_scheduled, scheduled, in_progress, on_hold, rejected, closed], + bug.id => [new, confirmed, in_progress, tested, on_hold, rejected, closed], + feature.id => [new, specified, confirmed, in_progress, tested, on_hold, rejected, closed] } + workflows.each { |type_id, statuses_for_type| + statuses_for_type.each { |old_status| + statuses_for_type.each { |new_status| [manager.id, member.id].each { |role_id| - Workflow.create!(:type_id => Type.where(:name => t).first.id, :role_id => role_id, :old_status_id => os.id, :new_status_id => ns.id) unless os == ns + Workflow.create!(type_id: type_id, + role_id: role_id, + old_status_id: old_status.id, + new_status_id: new_status.id) } } } From acf791ae29d950d26ab4a0a9f59f50dfc3a7bf2b Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Thu, 28 May 2015 10:15:06 +0200 Subject: [PATCH 53/56] rename "Properties" heading Properties has become a MSON keyword and therefore produces compile warnings when used. We could either enclose it in backticks or rename it. I went for renaming, because that makes the distinction between "linked" and "non-linked" properties more apparent --- doc/apiv3-documentation.apib | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/doc/apiv3-documentation.apib b/doc/apiv3-documentation.apib index b9ac4dda163..0f1930f3216 100644 --- a/doc/apiv3-documentation.apib +++ b/doc/apiv3-documentation.apib @@ -59,7 +59,7 @@ However for the future we plan to add authentication modes that are more suitabl Links to other resources in the API are represented uniformly by so called link objects. -## Properties +## Local Properties | Property | Description | Type | Required | Nullable | Default | |:---------:| ------------------------------------------------------------------------ | ------- |:--------:|:--------:| ------- | @@ -261,7 +261,7 @@ pagination can be found below the links table. A collection also carries meta information like the total count of elements in the collection or - in case of a paginated collection - the amount of elements returned in this response and action links to retrieve the remaining elements. -## Properties +## Local Properties | Property | Description | Type | Availability | |:--------:| --------------------------------------------------------------- | ------- | --------------------------- | @@ -401,7 +401,7 @@ Cursor based pagination is therefore less suited for use cases where you want to # Group Activities -## Properties: +## Local Properties: | Property | Description | Type | Constraints | Supported operations | | :---------: | ------------- | ---- | ----------- | -------------------- | | id | Activity id | Integer | x > 0 | READ | @@ -517,7 +517,7 @@ Updates an activity's comment and, on success, returns the updated activity. # Group Attachments -## Properties: +## Local Properties: | Property | Description | Type | Constraints | Supported operations | | :---------: | ------------- | ---- | ----------- | -------------------- | | id | Attachment's id | Integer | x > 0 | READ | @@ -579,7 +579,7 @@ Updates an activity's comment and, on success, returns the updated activity. | project | The project of this category | Project | not null | READ | | defaultAssignee | Default assignee for work packages of this category | User | | READ | -## Properties +## Local Properties | Property | Description | Type | Constraints | Supported operations | | :--------: | ------------- | ------- | ----------- | -------------------- | | id | Category id | Integer | x > 0 | READ | @@ -1041,7 +1041,7 @@ The request body is the actual string that shall be rendered as HTML string. |:---------:|-------------------------------------------- | ------------- | --------------------- | -------------------- | | self | This priority | Priority | not null | READ | -## Properties +## Local Properties | Property | Description | Type | Constraints | Supported operations | | :---------: | ------------------------------------------- | ---------- | ----------- | -------------------- | | id | Priority id | Integer | x > 0 | READ | @@ -1197,7 +1197,7 @@ The request body is the actual string that shall be rendered as HTML string. | types | Types available in this project | Types | not null | READ | | versions | Versions available in this project | Versions | not null | READ | -## Properties: +## Local Properties: | Property | Description | Type | Constraints | Supported operations | | :---------: | ------------- | ---- | ----------- | -------------------- | | id | Projects's id | Integer | x > 0 | READ | @@ -1317,7 +1317,7 @@ but are also limited to the projects that the current user is allowed to see. # Group Queries -## Properties: +## Local Properties: | Property | Description | Type | Constraints | Supported operations | | :---------: | ------------- | ---- | ----------- | -------------------- | | id | Query id | Integer | x > 0 | READ | @@ -1581,7 +1581,7 @@ The `allowedValues` can either contain a list of canonical links or just a singl This is an optimization to allow efficient handling of both small resource lists (that can be enumerated inline) and large resource lists (requiring one or more separate requests). -## Properties: +## Local Properties: | Property | Description | Type | Default | |:-----------------:| ---------------------------------------------------------------------------------- | -------- | ------- | @@ -1677,7 +1677,7 @@ This is an example of how a schema might look like. Note that this endpoint does |:-------------:|-------------------------- | ------------- | ----------- | -------------------- | | self | This status | Status | not null | READ | -## Properties +## Local Properties | Property | Description | Type | Constraints | Supported operations | | :--------: | ------------- | ------- | ----------- | -------------------- | | id | Status id | Integer | x > 0 | READ | @@ -1878,7 +1878,7 @@ This is an example of how a schema might look like. Note that this endpoint does |:-------------:|-------------------------- | ------------- | ----------- | -------------------- | | self | This string object | StringObject | not null | READ | -## Properties +## Local Properties | Property | Description | Type | Constraints | Supported operations | |:----------------:| ---------------------------------------------- | -------- | ----------- | -------------------- | | value | The value of the string | String | | READ | @@ -1915,7 +1915,7 @@ e.g. to be able to provide `allowedValues` for a string typed property. |:-------------:|-------------------------- | ------------- | ----------- | -------------------- | | self | This type | Type | not null | READ | -## Properties +## Local Properties | Property | Description | Type | Constraints | Supported operations | |:----------------:| ---------------------------------------------- | -------- | ----------- | -------------------- | | id | Type id | Integer | x > 0 | READ | @@ -2151,7 +2151,7 @@ This endpoint lists the types that are *available* in a given project. |:---------:|-------------------------------------------- | ------------- | --------------------- | -------------------- | | self | This user | User | not null | READ | -## Properties: +## Local Properties: | Property | Description | Type | Constraints | Supported operations | Condition | | :---------: | ------------- | ---- | ----------- | -------------------- | ------------------------- | | id | User's id | Integer | x > 0 | READ | | @@ -2378,7 +2378,7 @@ Permanently deletes the specified user account. | definingProject | The project to which the version belongs | Project | only present if the project is visible for the current user | READ | | availableInProjects | Projects where this version can be used | Projects | not null | READ | -## Properties +## Local Properties | Property | Description | Type | Constraints | Supported operations | | :---------: | --------------------------------------------- | ----------- | ----------- | -------------------- | | id | Version id | Integer | x > 0 | READ | @@ -2574,7 +2574,7 @@ Note that due to sharing this might be more than the versions *defined* by that | type | The type of the work package | Type | not null | READ / WRITE | | | version | The version associated to the work package | Version | | READ / WRITE | | -## Properties: +## Local Properties: | Property | Description | Type | Constraints | Supported operations | Condition | | :--------------: | ------------------------------------------------------ | ----------- | ------------------------------------------------------------------------------------------------------ | -------------------- | -------------------------------- | From 847686a81dba1b434a82437d7d08db47ceb8088b Mon Sep 17 00:00:00 2001 From: Jonas Heinrich Date: Mon, 1 Jun 2015 10:11:49 +0200 Subject: [PATCH 54/56] Updates README.md --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 25c6137c3f8..67b97c25441 100644 --- a/README.md +++ b/README.md @@ -38,10 +38,10 @@ The [OpenProject Foundation (OPF)](https://www.openproject.org/projects/openproj ## Repository -This repository contains two main branches: +This repository contains several main branches: * `dev`: The main development branch. We try to keep it stable in the sense of all tests are passing, but we don't recommend it for production systems. -* `stable`: Contains the latest stable release that we recommend for production use. Use this if you always want the latest version of OpenProject. +* `stable/`: Contains the latest stable release for a specific version. We recommend to use this for production use. Example: `stable/4.1`. ## License From d4f2eec37e8f5f6bf1ec859a5b9136e034c247b9 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Tue, 2 Jun 2015 11:59:40 +0200 Subject: [PATCH 55/56] remove dom font reduction --- app/views/boards/index.html.erb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/app/views/boards/index.html.erb b/app/views/boards/index.html.erb index 7b63e3435e6..61b6555b64d 100644 --- a/app/views/boards/index.html.erb +++ b/app/views/boards/index.html.erb @@ -47,12 +47,10 @@ See doc/COPYRIGHT.rdoc for more details. <%= board.topics_count %> <%= board.messages_count %> - - <% if board.last_message %> - <%= authoring board.last_message.created_on, board.last_message.author %>
- <%= link_to_message board.last_message %> - <% end %> - + <% if board.last_message %> + <%= authoring board.last_message.created_on, board.last_message.author %>
+ <%= link_to_message board.last_message %> + <% end %> <% end %> From ff7a9cd6594728c56c250af252d80e86f88b28c6 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Tue, 2 Jun 2015 16:36:55 +0200 Subject: [PATCH 56/56] correctly type button --- frontend/app/ui_components/wiki-toolbar-directive.js | 1 + lib/redmine/wiki_formatting/textile/helper.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/frontend/app/ui_components/wiki-toolbar-directive.js b/frontend/app/ui_components/wiki-toolbar-directive.js index e2081522937..f504ec80cf5 100644 --- a/frontend/app/ui_components/wiki-toolbar-directive.js +++ b/frontend/app/ui_components/wiki-toolbar-directive.js @@ -32,6 +32,7 @@ module.exports = function() { 'menubar=no, status=no, scrollbars=yes"); return false;', HELP_LINK_HTML = jQuery('