Merge branch 'release/5.0' into dev

app/controllers/application_controller.rb

@@ -172,7 +172,7 @@ class ApplicationController < ActionController::Base
       if (key = api_key_from_request) && accept_key_auth_actions.include?(params[:action])
         # Use API key
         User.find_by_api_key(key)
-      else
+      elsif OpenProject::Configuration.apiv2_enable_basic_auth?
         # HTTP Basic, either username/password or API key/random
         authenticate_with_http_basic do |username, password|
           User.try_to_login(username, password) || User.find_by_api_key(username)

app/models/work_package/pdf_export/ifpdf.rb

@@ -31,7 +31,6 @@ require 'rfpdf/fpdf'
 
 class WorkPackage::PdfExport::IFPDF < FPDF
   include Redmine::I18n
-  include Helpers
   attr_accessor :footer_date
 
   def initialize(lang)

app/models/work_package/pdf_export/to_pdf_helper.rb

@@ -27,12 +27,15 @@
 # See doc/COPYRIGHT.rdoc for more details.
 #++
 
+require 'work_package/pdf_export/ifpdf'
+require 'work_package/pdf_export/itcpdf'
+
 module WorkPackage::PdfExport::ToPdfHelper
   def get_pdf(language)
     if ['ko', 'ja', 'zh', 'zh-tw', 'th'].include? language.to_s.downcase
-      IFPDF.new(current_language)
+      ::WorkPackage::PdfExport::IFPDF.new(current_language)
     else
-      ITCPDF.new(current_language)
+      ::WorkPackage::PdfExport::ITCPDF.new(current_language)
     end
   end
 end

app/models/work_package/pdf_export/work_package_list_to_pdf.rb

@@ -32,7 +32,7 @@ class WorkPackage::PdfExport::WorkPackageListToPdf
   include ActionView::Helpers::TextHelper
   include ActionView::Helpers::NumberHelper
   include CustomFieldsHelper
-  include ToPdfHelper
+  include WorkPackage::PdfExport::ToPdfHelper
 
   attr_accessor :work_packages,
                 :pdf,
@@ -85,12 +85,16 @@ class WorkPackage::PdfExport::WorkPackageListToPdf
     # headers
     pdf.SetFontStyle('B', 8)
     pdf.SetFillColor(230, 230, 230)
+
+    base_x = pdf.get_x
+    base_y = pdf.get_y
+
     column_contents = query.columns.map(&:caption)
 
     max_height = calculate_max_height(column_contents, col_width)
 
     pdf.RDMCell Page.table_width, max_height, '', 1, 1, 'L', 1
-    pdf.SetXY(base_x, base_y)
+    pdf.set_y(base_y)
 
     write_cells(column_contents, col_width, Page.row_height)
     draw_borders(base_x, base_y, base_y + max_height, col_width)
@@ -131,7 +135,7 @@ class WorkPackage::PdfExport::WorkPackageListToPdf
         s.to_s
       }
 
-      max_height = calculate_max_height(column_contents, col_width)
+      max_height = calculate_max_height(col_values, col_width)
       description_height = if options[:show_descriptions]
                              calculate_max_height([work_package.description.to_s],
                                                   [Page.table_width / 2])
@@ -140,12 +144,13 @@ class WorkPackage::PdfExport::WorkPackageListToPdf
                            end
 
       # make new page if it doesn't fit on the current one
-      space_left = Page.height - base_y - Page.bottom_margin
+      space_left = Page.height - pdf.get_y - Page.bottom_margin
       if max_height + description_height > space_left
         pdf.AddPage('L')
+      end
+
       base_x = pdf.GetX
       base_y = pdf.GetY
-      end
 
       # write the cells on page
       write_cells(col_values, col_width, Page.row_height)

app/models/work_package/pdf_export/work_package_to_pdf.rb

@@ -32,7 +32,7 @@ class WorkPackage::PdfExport::WorkPackageToPdf
   include ActionView::Helpers::TextHelper
   include ActionView::Helpers::NumberHelper
   include CustomFieldsHelper
-  include ToPdfHelper
+  include WorkPackage::PdfExport::ToPdfHelper
 
   attr_accessor :work_package,
                 :pdf

config/configuration.yml.example

@@ -324,6 +324,11 @@ default:
   #     user: admin
   #     password: admin
 
+
+  # By default, the legacy APIv2 allows authentication through basic auth.
+  # Uncomment the following line to restrict APIv2 access to API tokens
+  # apiv2_enable_basic_auth: false
+
 # specific configuration options for production environment
 # that overrides the default ones
 # production:

doc/apiv2-documentation.md

@@ -9,6 +9,8 @@ __Be aware:__ The API v2 is marked as deprecated. Please read this [news article
 The API supports both *basic auth* and authentication via an *API access key*. The latter is transmitted either as one of the parameters, named `key`, for a request or in the request header `X-OpenProject-API-Key`.
 You can find a user's API key on their account page (/my/account).
 
+Authentication by basic auth is enabled by default, but can be disabled in the configuration (Set apiv2_enable_basic_auth to false)
+
 Example request:
 
 _GET_ `/api/v2/projects.xml?key=gh3g4h124grr871r8g`

lib/open_project/configuration.rb

@@ -80,7 +80,9 @@ module OpenProject
 
       'disabled_modules' => [], # allow to disable default modules
       'hidden_menu_items' => {},
-      'blacklisted_routes' => []
+      'blacklisted_routes' => [],
+
+      'apiv2_enable_basic_auth' => true,
     }
 
     @config = nil
@@ -318,6 +320,11 @@ module OpenProject
             define_method setting do
               self[setting]
             end
+
+            define_method "#{setting}?" do
+              ['true', true, '1'].include? self[setting]
+            end
+
           end unless respond_to? setting
         end
       end

lib/open_project/configuration/helpers.rb

@@ -33,19 +33,6 @@ module OpenProject
     # To be included into OpenProject::Configuration in order to provide
     # helper methods for easier access to certain configuration options.
     module Helpers
-      ##
-      # Activating this leaves omniauth as the only way to authenticate.
-      def disable_password_login?
-        true? self['disable_password_login']
-      end
-
-      ##
-      # If this is true a user's password cannot be chosen when editing a user.
-      # The only way to change the password is to generate a random one which is sent
-      # to the user who then has to change it immediately.
-      def disable_password_choice?
-        true? self['disable_password_choice']
-      end
 
       ##
       # Carrierwave storage type. Possible values are, among others, :file and :fog.
@@ -106,10 +93,6 @@ module OpenProject
           Array(value)
         end
       end
-
-      def true?(value)
-        ['true', true].include? value # check string to accommodate ENV override
-      end
     end
   end
 end

spec/requests/auth/api_v2_spec.rb

@@ -29,7 +29,13 @@
 require File.expand_path('../../../spec_helper', __FILE__)
 
 describe 'API v2', type: :request do
-  let(:admin) { FactoryGirl.create :admin }
+  let(:valid_password) { 'foobar!1234!foobar' }
+  let(:admin) {
+    FactoryGirl.create :admin,
+                       login: 'admin',
+                       password: valid_password,
+                       password_confirmation: valid_password
+  }
   let(:project) { FactoryGirl.create(:project) }
 
   before do
@@ -42,7 +48,7 @@ describe 'API v2', type: :request do
     User.current = nil # api key auth sets the current user, reset it to make test idempotent
   end
 
-  describe 'key authentication' do
+  describe 'API authentication' do
     let(:api_key) { admin.api_key }
 
     shared_examples_for 'API key access' do
@@ -59,34 +65,69 @@ describe 'API v2', type: :request do
       end
     end
 
+    shared_examples_for 'API Basic Auth access' do
+      let(:used_password) { valid_password }
+      let(:credentials) {
+        ActionController::HttpAuthentication::Basic.encode_credentials('admin', used_password)
+      }
+
+      before do
+        allow(OpenProject::Configuration).to receive(:apiv2_enable_basic_auth?).and_return(enabled)
+        get request_url, nil, { 'HTTP_AUTHORIZATION' => credentials }
+      end
+
+      context 'when enabled' do
+        let(:enabled) { true }
+
+        context 'valid' do
+          it { expect(response.status).to eq(200) }
+        end
+
+        context 'invalid' do
+          let(:used_password) { 'foobar' }
+          it { expect(response.status).to eq(401) }
+        end
+      end
+
+      context 'when disabled' do
+        let(:enabled) { false }
+        it { expect(response.status).to eq(401) }
+      end
+    end
+
     describe 'for planning element types' do
       let(:request_url) { "/api/v2/projects/#{project.id}/planning_element_types.json" }
 
       it_behaves_like 'API key access'
+      it_behaves_like 'API Basic Auth access'
     end
 
     describe 'for project associations' do
       let(:request_url) { "/api/v2/projects/#{project.id}/project_associations.xml" }
 
       it_behaves_like 'API key access'
+      it_behaves_like 'API Basic Auth access'
     end
 
     describe "for project associations' available projects" do
       let(:request_url) { "/api/v2/projects/#{project.id}/project_associations/available_projects.xml" }
 
       it_behaves_like 'API key access'
+      it_behaves_like 'API Basic Auth access'
     end
 
     describe 'for reportings' do
       let(:request_url) { "/api/v2/projects/#{project.id}/reportings.xml" }
 
       it_behaves_like 'API key access'
+      it_behaves_like 'API Basic Auth access'
     end
 
     describe "for reportings' available projects" do
       let(:request_url) { "/api/v2/projects/#{project.id}/reportings/available_projects.xml" }
 
       it_behaves_like 'API key access'
+      it_behaves_like 'API Basic Auth access'
     end
   end
 end