Newy
Newy

Reputation: 40137

What are the security issues of using User.current_user

This post - http://www.ruby-forum.com/topic/51782 - seems to suggest a way of setting User.current_user in a before_filter in a controller and accessing User.current_user in models affected by that action. Is this perfectly thread-safe or are there security issues here? Seems like the correct approach would be to always pass in @current_user into any model that needs it, but that gets messy.

Upvotes: 2

Views: 333

Answers (2)

Jacob
Jacob

Reputation: 23225

Using a global before_filter in your ApplicationController should be thread-safe (if you are using thread local storage). This is from the excellent declarative_authorization gem's documentation:

If you’d like to use model security, add a before_filter that sets the user globally to your ApplicationController. This is thread-safe.

before_filter :set_current_user 
protected  def set_current_user   
  Authorization.current_user = current_user
end

Update:

The actual implementation of Authorization.current_user looks like this. It uses thread-local storage, which is what makes it thread-safe.

Upvotes: 2

Samuel
Samuel

Reputation: 38356

That solution is not thread safe and when processing two requests (A and B), the later one will change the earlier one's current user mid request (which would be almost impossible to debug and extremely confusing to the user).

Store the user (or the user id) in the current thread's storage.

class User < ActiveRecord::Base
  class << self
    def current_user
      Thread.current[:current_user]
    end

    def current_user=(user)
      Thread.current[:current_user] = user
    end
  end
end

before_filter :set_current_user
private
  def set_current_user
    User.current_user = User.find(session[:user_id])
  end

Upvotes: 5

Related Questions