CODEWITHSUNDEEP
ruby-on-railsruby-on-rails-3logging
mofeeta
mofeeta

Reputation: 1189

Setting current_user in pg_audit_log

I'd like to use pg_audit_log for logging in a rails app. The audit log must not only show the columns that have changed, but also the user who made those changes. The docs don't show how to do this, but after looking through the pg_audit_log source (postgresql_adapter.rb) I see it reads user information from a thread local variable, ala:

current_user = Thread.current[:current_user]

I've considered setting/unsetting this in before and after filters like so:

Thread.current[:current_user] = current_user

(using the current_user helper method in the controller to get the currently logged in user), but that seems dangerous. I'm now spending time trying to understand how the rails request cycle and threads interact, to get a better feel for just how dangerous. In the mean time, I was curious if any SO users currently using pg_audit_log have solved the need to log the user_id and user_unique_name to the log tables each time the user makes a change to a record.

Upvotes: 3

Views: 344

Answers (2)

Giuseppe
Giuseppe

Reputation: 5348

Relying on the Thread.current hash to provide model-level access to objects managed by the controller is indeed controversial. For example, see the following:

Safety of Thread.current[] usage in rails

It is worrisome that this particular feature is undocumented in the pg_audit_log gem.

Suppose you had not actively explored the gem's source code, and suppose you had independently decided to define Thread.current[:current_user] = something in your own application, for your own purpose. In that case, pg_audit_log would audit that object, without your knowledge.

Granted, the name current_user is so universally accepted to mean the currently logged-on user as defined by authentication routines that it's difficult to imagine this potential bug as a concrete problem, but from a design standpoint? Ouch.

On the other hand, since you know what you are doing, ensuring that Thread.current[:current_user] is set/unset at the beginning/end of each and every response cycle should make the process safe. At least that's what I gather from reading lots of posts on the topic.

Cheers, Giuseppe

Upvotes: 1

Alex Ghiculescu
Alex Ghiculescu

Reputation: 7540

Setting the current user the way you describe is a common way to do it. See, for example, http://rails-bestpractices.com/posts/47-fetch-current-user-in-models

Some example code could look like:

# in your model
class User < ActiveRecord::Base
  def self.current
    Thread.current[:current_user]
  end
  def self.current=(user)
    Thread.current[:current_user] = user if user.nil? || user.is_a?(User)
  end
end

# in your controller
class ApplicationController < ActionController::Base
    before_filter :set_current_user
    def set_current_user
      User.current = user_signed_in? ? current_user : nil
    end
end

Upvotes: 4

Related Questions