fix User.current leak (#4533)

app/middleware/reset_current_user.rb

@@ -0,0 +1,53 @@
+#-- 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.
+#++
+
+##
+# Resets `User.current` between requests. This is to make sure that
+# no data leaks occur. One case specifically where this did occur before
+# is when a user is trying to authorize through OmniAuth.
+# During the login an account may be created on the fly.
+# If this failed due to some reason, e.g. an already taken email address,
+# `User.current` still had the value of the last processed request.
+# Through this users were able to see random other user's full names
+# in the header.
+class ResetCurrentUser
+  attr_reader :app
+
+  def initialize(app)
+    @app = app
+  end
+
+  def call(env)
+    reset_current_user!
+    app.call env
+  end
+
+  def reset_current_user!
+    User.current = nil
+  end
+end

config/application.rb

@@ -89,6 +89,7 @@ module OpenProject
                            }
 
     config.middleware.use Rack::Attack
+    config.middleware.use 'ResetCurrentUser'
 
     ##
     # Support XML requests as params for APIv2

spec/requests/current_user_spec.rb

@@ -0,0 +1,73 @@
+#-- 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'
+
+module InsertUserSetupCallback
+  def user_setup
+    current_user_before User.current
+
+    super
+  end
+
+  def current_user_before(user)
+    ResetCurrentUserCallback.current_user_before(user)
+  end
+end
+
+module ResetCurrentUserCallback
+  class << self
+    def current_user_before(user)
+      user
+    end
+  end
+end
+
+describe ResetCurrentUser, type: :request do
+  let!(:user) { FactoryGirl.create :user }
+
+  before do
+    ApplicationController.prepend InsertUserSetupCallback
+
+    allow_any_instance_of(ApplicationController)
+      .to receive(:find_current_user).and_return(user)
+  end
+
+  it 'resets User.current between requests' do
+    expect(ResetCurrentUserCallback).to receive(:current_user_before).with(User.anonymous)
+    get '/my/page'
+    expect(response.body).to include (user.name)
+
+    # without the ResetCurrentUser middleware the following expectation
+    # fails as User.current still has the value from the last request
+
+    expect(ResetCurrentUserCallback).to receive(:current_user_before).with(User.anonymous)
+    get '/my/page'
+    expect(response.body).to include (user.name)
+  end
+end