Add option to log user for all requests

Also remove before filter that worked around some session cookie bug
in Rails. This workaround wasn't effective anyway, since our session
cookie names don't contain chiliproject any more.

app/controllers/application_controller.rb

@@ -46,18 +46,6 @@ class ApplicationController < ActionController::Base
     cookies.delete(:autologin)
   end
 
-  # Remove broken cookie after upgrade from 0.8.x (#4292)
-  # See https://rails.lighthouseapp.com/projects/8994/tickets/3360
-  # TODO: remove it when Rails is fixed
-  before_filter :delete_broken_cookies
-  def delete_broken_cookies
-    if cookies['_chiliproject_session'] && cookies['_chiliproject_session'] !~ /--/
-      cookies.delete '_chiliproject_session'
-      redirect_to home_path
-      return false
-    end
-  end
-
   # FIXME: Remove this when all of Rack and Rails have learned how to
   # properly use encodings
   before_filter :params_filter
@@ -76,7 +64,12 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  before_filter :user_setup, :check_if_login_required, :reset_i18n_fallbacks, :set_localization, :check_session_lifetime
+  before_filter :user_setup,
+                :check_if_login_required,
+                :log_requesting_user,
+                :reset_i18n_fallbacks,
+                :set_localization,
+                :check_session_lifetime
 
   rescue_from ActionController::InvalidAuthenticityToken, :with => :invalid_authenticity_token
 
@@ -154,6 +147,21 @@ class ApplicationController < ActionController::Base
     require_login if Setting.login_required?
   end
 
+  def log_requesting_user
+    return unless Setting.log_requesting_user?
+    login_and_mail = " (#{escape_for_logging(User.current.login)} ID: #{User.current.id} " +
+                     "<#{escape_for_logging(User.current.mail)}>)" unless User.current.anonymous?
+    logger.info "OpenProject User: #{escape_for_logging(User.current.name)}#{login_and_mail}"
+  end
+
+  # Escape string to prevent log injection
+  # e.g. setting the user name to contain \r allows overwriting a log line on console
+  # replaces all invalid characters with #
+  def escape_for_logging(string)
+    # only allow numbers, ASCII letters, space and the following characters: @.-"'!?=/
+    string.gsub(/[^0-9a-zA-Z@._\-"\'!\?=\/ ]{1}/, '#')
+  end
+
   def reset_i18n_fallbacks
     return if I18n.fallbacks.defaults == (fallbacks = [I18n.default_locale] + Setting.available_languages.map(&:to_sym))
     I18n.fallbacks = nil

app/views/settings/_authentication.html.erb

@@ -62,6 +62,7 @@ See doc/COPYRIGHT.rdoc for more details.
       <legend><%= I18n.t(:other, :scope => [:settings]) %></legend>
       <p><%= setting_check_box :openid, :disabled => !Object.const_defined?(:OpenID) %></p>
 
+      <p><%= setting_check_box :log_requesting_user %></p>
       <p><%= setting_check_box :rest_api_enabled %></p>
     </fieldset>
   </div>

config/locales/de.yml

@@ -1158,6 +1158,7 @@ de:
   setting_work_package_properties: "Arbeitspaket-Eigenschaften"
   setting_work_package_startdate_is_adddate: "Neue Arbeitspakete haben \"Heute\" als Anfangsdatum"
   setting_work_package_export_limit: "Max. Anzahl Arbeitspakete bei CSV/PDF-Export"
+  setting_log_requesting_user: "Logge Benutzer Login, Name und Mailadresse für alle Anfragen"
   setting_login_required: "Authentifizierung erforderlich"
   setting_mail_from: "E-Mail-Absender"
   setting_mail_handler_api_enabled: "Abruf eingehender E-Mails aktivieren"

config/locales/en.yml

@@ -1137,6 +1137,7 @@ en:
   setting_work_package_properties: "Work package properties"
   setting_work_package_startdate_is_adddate: "Use current date as start date for new work packages"
   setting_work_packages_export_limit: "Work packages export limit"
+  setting_log_requesting_user: "Log user login, name, and mail address for all requests"
   setting_login_required: "Authentication required"
   setting_mail_from: "Emission email address"
   setting_mail_handler_api_enabled: "Enable WS for incoming emails"

config/settings.yml

@@ -13,6 +13,9 @@ brute_force_block_after_failed_logins:
   format: int
 welcome_text:
   default:
+log_requesting_user:
+  default: 0
+  format: int
 login_required:
   default: 0
 self_registration:

doc/CHANGELOG.md

@@ -1,5 +1,7 @@
 # Changelog
 
+* `#1808` Add option to log user for each request
+
 ## 3.0.0pre14
 
 * `#825` Migrate Duration

spec/controllers/application_controller_spec.rb

@@ -0,0 +1,79 @@
+#-- copyright
+# OpenProject is a project management system.
+#
+# Copyright (C) 2012-2013 the OpenProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# See doc/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+
+describe ApplicationController do
+  let(:user) { FactoryGirl.create(:user, :lastname => "Crazy! Name with \r\n Newline") }
+
+  # Fake controller to test calling an action
+  controller do
+    def index
+      # just do anything that doesn't require an extra template
+      render_404
+    end
+  end
+
+  describe 'with log_requesting_user enabled' do
+    before do
+      Setting.stub(:log_requesting_user?).and_return(true)
+    end
+
+    it 'should log the current user' do
+      messages = []
+      Rails.logger.should_receive(:info).at_least(:once) do |message|
+        messages << message
+      end
+
+      as_logged_in_user(user) do
+        get(:index)
+      end
+
+      filtered_messages = messages.select { |message| message.start_with? 'OpenProject User' }
+      filtered_messages.length.should == 1
+      filtered_messages[0].should == "OpenProject User: #{user.firstname} Crazy! Name with \#\# " +
+                                     "Newline (#{user.login} ID: #{user.id} <#{user.mail}>)"
+    end
+
+    it 'should log an anonymous user' do
+      messages = []
+      Rails.logger.should_receive(:info).at_least(:once) do |message|
+        messages << message
+      end
+
+      # no login, so this is done as Anonymous
+      get(:index)
+
+      filtered_messages = messages.select { |message| message.start_with? 'OpenProject User' }
+      filtered_messages.length.should == 1
+      filtered_messages[0].should == "OpenProject User: Anonymous"
+    end
+  end
+  describe 'with log_requesting_user disabled' do
+    before do
+      Setting.stub(:log_requesting_user?).and_return(false)
+    end
+
+    it 'should not log the current user' do
+      messages = []
+      Rails.logger.stub(:info) do |message|
+        messages << message
+      end
+
+      as_logged_in_user(user) do
+        get(:index)
+      end
+
+      filtered_messages = messages.select { |message| message.start_with? 'OpenProject User' }
+      filtered_messages.length.should == 0
+    end
+  end
+end