Merge remote-tracking branch 'upstream/release/4.2' into dev

Conflicts:
	Gemfile.lock

app/controllers/application_controller.rb

@@ -426,58 +426,15 @@ class ApplicationController < ActionController::Base
     params[:back_url] || request.env['HTTP_REFERER']
   end
 
-  def redirect_back_or_default(default, escape = true, use_escaped = true)
+  def redirect_back_or_default(default, use_escaped = true)
-    escaped_back_url = if escape
+    policy = RedirectPolicy.new(
-                         URI.escape(CGI.unescape(params[:back_url].to_s))
+      params[:back_url],
-                       else
+      hostname: request.host,
-                         params[:back_url]
+      default: default,
-               end
+      return_escaped: use_escaped,
-
+    )
-    # if we have a back_url it must not contain two consecutive dots
+
-    if escaped_back_url.present? && !escaped_back_url.match(/\.\./)
+    redirect_to policy.redirect_url
-      begin
-        uri = URI.parse(escaped_back_url)
-
-        # do not redirect user to another host (even protocol relative urls have the host set)
-        # whenever a host is set it must match the request's host
-        uri_local_to_host = uri.host.nil? || uri.host == request.host
-
-        # do not redirect user to the login or register page
-        uri_path_allowed  = !uri.path.match(ignored_back_url_regex)
-
-        # do not redirect to another subdirectory
-        uri_subdir_allowed = relative_url_root.blank? || uri.path.match(/\A#{relative_url_root}/)
-
-        if uri_local_to_host && uri_path_allowed && uri_subdir_allowed
-          if use_escaped
-            redirect_to(escaped_back_url)
-          else
-            redirect_to(back_url)
-          end
-          return
-        end
-      rescue URI::InvalidURIError
-        # redirect to default
-      end
-    end
-    redirect_to default
-    false
-  end
-
-  ##
-  # URLs that match the returned regex must be ignored when they are the back url.
-  def ignored_back_url_regex
-    %r{/(
-      # Ignore login since redirect to back url is result of successful login.
-      login |
-      # When signing out with a direct login provider enabled you will be left at the logout
-      # page with a message indicating that you were logged out. Logging in from there would
-      # normally cause you to be redirected to this page. As it is the logout page, however,
-      # this would log you right out again after a successful login.
-      logout |
-      # TODO explain reasoning for this
-      account/register
-      )}x # ignore whitespace
   end
 
   def render_400(options = {})

app/controllers/work_packages/bulk_controller.rb

@@ -64,7 +64,7 @@ class WorkPackages::BulkController < ApplicationController
       end
     end
     set_flash_from_bulk_save(@work_packages, unsaved_work_package_ids)
-    redirect_back_or_default({ controller: '/work_packages', action: :index, project_id: @project }, false)
+    redirect_back_or_default(controller: '/work_packages', action: :index, project_id: @project)
   end
 
   def destroy

app/controllers/work_packages_controller.rb

@@ -189,7 +189,7 @@ class WorkPackagesController < ApplicationController
 
       flash[:notice] = l(:notice_successful_update)
 
-      redirect_back_or_default(work_package_path(work_package), true, false)
+      redirect_back_or_default(work_package_path(work_package), false)
     else
       edit
     end

app/policies/redirect_policy.rb

@@ -0,0 +1,149 @@
+#-- 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 'uri'
+require 'cgi'
+
+# This capsulates the validation of a requested redirect URL.
+#
+class RedirectPolicy
+  attr_reader :validated_redirect_url, :request
+
+  def initialize(requested_url, hostname:, default:, return_escaped: true)
+    @current_host = hostname
+    @return_escaped = return_escaped
+
+    @requested_url = preprocess(requested_url)
+    @default_url = default
+  end
+
+  ##
+  # Performs all validations for the requested URL
+  def valid?
+    return false if @requested_url.nil?
+
+    [
+      # back_url must not contain two consecutive dots
+      :no_upper_levels,
+      # Require the path to begin with a slash
+      :path_has_slash,
+      # do not redirect user to another host
+      :same_host,
+      # do not redirect user to the login or register page
+      :path_not_blacklisted,
+      # do not redirect to another subdirectory
+      :matches_relative_root
+    ].all? { |check| send(check) }
+  end
+
+  ##
+  # Return a valid redirect URI.
+  # If the validation check on the current back URL apply
+  def redirect_url
+    if valid?
+      postprocess(@requested_url)
+    else
+      @default_url
+    end
+  end
+
+  private
+
+  ##
+  # Preprocesses the requested redirect URL.
+  # - Escapes it when necessary
+  # - Tries to parse it
+  # - Escapes the redirect URL when requested so.
+  def preprocess(requested)
+    url = URI.escape(CGI.unescape(requested.to_s))
+    URI.parse(url)
+  rescue URI::InvalidURIError => e
+    Rails.logger.warn("Encountered invalid redirect URL '#{requested}': #{e.message}")
+    nil
+  end
+
+  ##
+  # Postprocesses the validated URL
+  def postprocess(redirect_url)
+    # Remove basic auth credentials
+    redirect_url.userinfo = ''
+
+    if @return_escaped
+      redirect_url.to_s
+    else
+      URI.unescape(redirect_url.to_s)
+    end
+  end
+
+  ##
+  # Avoid paths with references to parent paths
+  def no_upper_levels
+    !@requested_url.path.include? '../'
+  end
+
+  ##
+  # Require URLs to contain a path slash.
+  # This will always be the case for parsed URLs unless
+  # +URI.parse('@foo.bar')+ or a non-root relative URL  +URI.parse('foo')+
+  def path_has_slash
+    @requested_url.path =~ %r{\A/([^/]|\z)}
+  end
+
+  ##
+  # do not redirect user to another host (even protocol relative urls have the host set)
+  # whenever a host is set it must match the request's host
+  def same_host
+    @requested_url.host.nil? || @requested_url.host == @current_host
+  end
+
+  ##
+  # Avoid redirect URLs to specific locations, such as login page
+  def path_not_blacklisted
+    !@requested_url.path.match(
+      %r{/(
+      # Ignore login since redirect to back url is result of successful login.
+      login |
+      # When signing out with a direct login provider enabled you will be left at the logout
+      # page with a message indicating that you were logged out. Logging in from there would
+      # normally cause you to be redirected to this page. As it is the logout page, however,
+      # this would log you right out again after a successful login.
+      logout |
+      # Avoid sending users to the register form. The exact reasoning behind
+      # this is unclear, but grown from tradition.
+      account/register
+      )}x # ignore whitespace
+    )
+  end
+
+  ##
+  # Requires the redirect URL to reside inside the relative root, when given.
+  def matches_relative_root
+    relative_root = OpenProject::Configuration['rails_relative_url_root']
+    relative_root.blank? || @requested_url.path.starts_with?(relative_root)
+  end
+end

app/views/help/wiki_syntax_detailed.html.erb

@@ -259,6 +259,15 @@ tooltip of "WHO"
       <p>Include a wiki page. Example:</p>
       <pre><code>{{include(Foo)}}</code></pre>
     </dd>
+    <dt><code>child_pages</code></dt>
+    <dd>
+      <p>Displays a list of child pages. With no argument, it displays the child pages of the current wiki page. Examples:</p>
+      <pre><code>
+        {{child_pages}} -- can be used from a wiki page only
+        {{child_pages(Foo)}} -- lists all children of page Foo
+        {{child_pages(Foo, parent=1)}} -- same as above with a link to page Foo
+      </code></pre>
+    </dd>
     <dt><code>macro_list</code></dt>
     <dd>
       <p>Displays a list of all available macros, including description if available.</p>

doc/operation_guides/manual/installation-guide.md

@@ -248,10 +248,13 @@ Then, we prepare apache and passenger:
 
 Now, the Passenger gem is installed and integrated into apache.
 
+```bash
 [root@ubuntu] su openproject --login
 [openproject@ubuntu] cd ~/openproject
 [openproject@ubuntu] gem install passenger
 [openproject@ubuntu] passenger-install-apache2-module
+```
+
 Follow the instructions passenger provides.
 The passenger installer will ask you the question in "Which languages are you
 interested in?". We are interested only in ruby.

doc/operation_guides/system_requirements.md

@@ -17,7 +17,7 @@ provide any official support for them.
 | :------------------------------ | :----------- | :---------- |
 | Ubuntu 14.04 Trusty             | ubuntu-14.04 | upstart     |
 | Debian 8 Jessie                 | debian-8     | systemd     |
-| Debian 7 Wheezy                 | debiani-7    | sysvinit    |
+| Debian 7 Wheezy                 | debian-7    | sysvinit    |
 | CentOS/RHEL 7.x                 | centos-7     | systemd     |
 | CentOS/RHEL 6.x                 | centos-6     | upstart     |
 | Fedora 20                       | fedora-20    | sysvinit    |

spec/controllers/account_controller_spec.rb

@@ -90,12 +90,12 @@ describe AccountController, type: :controller do
 
       context 'with a relative url root' do
         before do
-          @old_relative_url_root = ApplicationController.relative_url_root
+          @old_relative_url_root = OpenProject::Configuration['rails_relative_url_root']
-          ApplicationController.relative_url_root = '/openproject'
+          OpenProject::Configuration['rails_relative_url_root'] = '/openproject'
         end
 
         after do
-          ApplicationController.relative_url_root = @old_relative_url_root
+          OpenProject::Configuration['rails_relative_url_root'] = @old_relative_url_root
         end
 
         it 'should redirect to the same subdirectory with an absolute path' do

spec/policies/redirect_policy_spec.rb

@@ -0,0 +1,126 @@
+#-- 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 File.expand_path('../../spec_helper', __FILE__)
+
+describe RedirectPolicy, type: :controller do
+  let(:host) { 'test.host' }
+
+  let(:return_escaped) { true }
+  let(:default) { 'http://test.foo/default' }
+
+  let(:policy) {
+    described_class.new(
+      back_url,
+      default: default,
+      hostname: host,
+      return_escaped: return_escaped
+    )
+  }
+  let(:subject) { policy.redirect_url }
+
+  shared_examples 'redirects to default' do |url|
+    let(:back_url) { url }
+    it "#{url} redirects to the default URL" do
+      expect(subject).to eq(default)
+    end
+  end
+
+  shared_examples 'valid redirect URL' do |url|
+    let(:back_url) { url }
+    it "#{url} is valid" do
+      expect(subject).to eq(url)
+    end
+  end
+
+  shared_examples 'valid redirect, escaped URL' do |input, output|
+    let(:back_url) { input }
+    it "#{input} is valid, but escaped to #{output}" do
+      expect(subject).to eq(output)
+    end
+  end
+
+  shared_examples 'ignores invalid URLs' do
+    uris = %w(
+      //test.foo/fake
+      //bar@test.foo
+      //test.foo
+      ////test.foo
+      @test.foo
+      fake@test.foo
+      //foo:bar@test.foo
+      /../somedir
+      /work_packages/../../secret
+    )
+
+    uris.each do |uri|
+      it_behaves_like 'redirects to default', uri
+    end
+  end
+
+  it_behaves_like 'ignores invalid URLs'
+  it_behaves_like 'valid redirect URL', '/work_packages/1234?filter=[foo,bar]'
+
+  it_behaves_like 'valid redirect, escaped URL',
+                  'http://test.host/?a=\11\15',
+                  'http://test.host/?a=%5C11%5C15'
+
+  context 'without escaped return URLs' do
+    let(:return_escaped) { false }
+    it_behaves_like 'valid redirect URL', '/work_packages/1234?filter=[foo,bar]'
+    it_behaves_like 'valid redirect URL', 'http://test.host/?a=\11\15'
+  end
+
+  context 'with relative root' do
+    let(:relative_root) { '/mysubdir' }
+
+    before do
+      allow(OpenProject::Configuration)
+        .to receive(:[]).with('rails_relative_url_root')
+        .and_return(relative_root)
+    end
+
+    it_behaves_like 'valid redirect URL', '/mysubdir/work_packages/1234'
+    it_behaves_like 'valid redirect URL', '/mysubdir'
+    it_behaves_like 'redirects to default', '/'
+    it_behaves_like 'redirects to default', '/foobar'
+    it_behaves_like 'redirects to default', '/mysubdir/../foobar'
+    it_behaves_like 'redirects to default', '/mysubdir/%2E%2E/secret/etc/passwd'
+    it_behaves_like 'redirects to default', '/%2E%2E/secret/etc/passwd'
+    it_behaves_like 'redirects to default', '/foobar/%2E%2E/secret/etc/passwd'
+    it_behaves_like 'redirects to default', 'wusdus/%2E%2E/%2E%2E/secret/etc/passwd'
+  end
+
+  describe 'auth credentials' do
+    let(:back_url) { 'http://user:pass@test.host/work_packages/123' }
+
+    it 'removes the credentials' do
+      expect(subject).to eq('http://test.host/work_packages/123')
+    end
+  end
+end