Yr code shenanigans always make me laugh.

“methods.each do |shenanigan| ” nearly made me cry. That’s frickin’ awesome.

Haha :) Good job Pratik, you’re teaching them crap :P

For your next article I propose: ‘How to query the database directly within .erb’

Or you could do something like this:

http://scie.nti.st/2007/2/1/let-activerecord-observers-see-controller-context

Since the behavior of an observer may depend in which context it was invoked, I’ve used code like that on a couple of occasions.

LOL thank u ive been looking 4 this 4 ages…... do you know y doesnt this works? Can u help me???

<%= link_to_remote ‘Read article’, :controller => ‘articles’, :class => ‘ajax’, :action => ‘get_article_js’, :onclick => ‘return false’, :update => $(‘article_#{article.id}’).innerHTML %>

Im making an AJAX desktop!!

Somebody on #ruby-lang actually mentioned a plugin called Goldberg, which made it still simpler and you would be surprised by its ingenuity. They had a class called Goldberg and before_filter in application.rb and in the callback, they used to store session as a class attribute in Goldberg class and now in models they used to do this:

 class Book < ActiveRecord::Base
    def self.my_books
       # use Goldberg.user to have access to current_user object which was copied from session and saved as a class attributed
     end
 end

I literally pulled my hair out and asked myself why?

I enjoy cache sweepers. Look at this:

class AuthorStampSweeper < ActionController::Caching::Sweeper
  observe Task, Project, User, Log, UrMom

  def before_create(record)
    record.creator = current_user
  end

  def before_update(record)
    record.updater = current_user
  end
end

Yay! And you have to call it from the controller, so everything makes a lot of sense. I prefer this method instead of passing the user id to some accessor in the model or something.

@ August Lilleaas. Except that passing the user’s id to the model is the right [and elegant] way to do this.

i don’t don’t see anything wrong with accessing the sessionid in a model to help prevent against csrf attacks.

cheers, tom

HeresTomWithTheWeather – U-hum..could you please shed some more light on that ?

suppose we add a hidden :action_token field in a form_for. the value of the field is derived from the session id (for simplicity, let’s assume it’s just the session id), so let’s add attr_accessor :action_token to the model.

how will we set the session id to the action token field for new and edit? we could do it in the controller for new():

@request = Request.new @request.action_token = session.session_id

and again for edit():

@request = Request.find(params[:Id]) @request.action_token = session.session_id

and when we process the submitted action token, we could do this in create():

@request = Request.new(params[:request]) process_forgery() if @request.action_token != session.session_id

and again for update():

@request = Request.find(params[:id]) process_forgery() if @request.action_token != session.session_id

or we could just explicitly add methods to the model so we don’t have to add all this stuff to the controller.

in theory, we could use Garry’s observer idea. i had trouble getting after_initialize() to work with STI, though.

-tom

Probably you should check out Rails built in CSRF protection mechanism.

now that 2.0 is out, i can!

http://weblog.rubyonrails.org/2007/12/7/rails-2-0-it-s-done

thanks, tom

Ok i can see how breaking MVC is bad. But how would you solve the issue of recording modified_by and created_by where most tables have those columns and you are looking for a auto pain free way of taking the current_user and putting there id or login into those fields???? That is a very real problem and i don’t feel like having Model.modified_by = current_user all over the place be rather prefer it to work like updated_at and created_at

How about the simple task of Auditing changes? On a website where Users make changes to all kinds of objects I want to store the changes in a SystemLogs table.

A model with no reference to a User object whatsoever (or atleast not directly) gets changed by a user. Now whenever Model.save is called in the controller I want to create a SysLog record/instance with the changes and the user_id of the calling user.

To do this from the controller violates the DRY principle, as every .save / .update call from different controllers need to be followed by a specific creation of a SysLog object.

in my specific case auditing causes the DRY principle to conflict with the MVC model

I have examples where models writing to a session is the only thing that makes any real sense from an API and DRY standpoint as well. IMO, a session is not solely the domain of the controller. It is a system wide bucket for storing state during a visit. It should be available to the entire framework. Can it be abused? Sure, so can every tenet of programming. Considering the wide opinions on just about every facet of OOP details, it cannot be claimed to be absolutely wrong for both controllers and models to make use of a session store.

Because Rails doesn’t allow it, there’s some ugly APIs that result for those cases where automated handling of tasks by the framework or a service layer just on top of it are handled. the two examples above are legit IMO, and I have at least one other.

class ApplicationController < ActionController::Base
before_filter :set_current_user
private
def set_current_user
  UserActionObserver.current_user = current_user
end
end

class UserActionObserver < ActiveRecord::Observer
  # assuming these all have created_by_id and updated_by_id
  observe Product, Thing, OtherThing

  cattr_accessor :current_user

  def before_create(model)
    model.created_by = @@current_user
  end

  def before_update(model)
    model.updated_by = @@current_user
  end
end

Pratik, you are being a royal cock about this.

Surely, adding your own custom logging to a rails app, WITHOUT having to go mess with the controller methods that call save on your model in order to pass the username stored in the session to the model by just making the session data available to the model is WAAAAY more DRY and elegant than going back and fucking around with controller methods.

You dumbass.

I don’t understand the appeal of the observer examples. What is wrong with going through the current_user’s association proxies to build and create new records in the controller? It makes the code too clear? The logic isn’t split up into enough files and classes with implicit behavior?

I guess I don’t understand who audits by “modified_by” on every model. That just seems dumb because you have no transaction history. It’s useless. You modify it and change something important… then I come along and do a normal update. When the books don’t balance, I’m to blame cos I was the ‘last to change it’.

It seems to me that the best approach is to use the cache_sweeper approach to write everything to a transaction table,

record_type
record_id
user_id
previous_values
new_values
created_at
ip_address

Anything less than that and you’re wasting your time with auditing. Throwing the session at your models isn’t going to give you this in a DRY way. I’d like to see people solve the real problem. I think there needs to be another observer that does have that stuff, or maybe just a global around filter. I just know that session in model isn’t the right way.

So if I don’t get it please explain or link to somewhere that does, rather than just calling me stupid.

Rails 2.x is all about restful designs, thin controllers and the suggestion that business logic should be in models isn’t it? Are we really supposed to believe that the session will never contain information that affects business logic!

Rails is based on the Model-View-Controller pattern.

Model: data; business rules to manipulate data. View: user interface. Controller: manage communication to model of user actions.

In which of those does session data belong? If there is a business rule that says data XYZ is only accessible to a user with role ABC, doesn’t the model need information from the session?

Implementation is a separate issue.

I recommend using observers if you want to audit changes. The code is similar to the cache_sweeper mechanism. Just include the Auditing module to your controller and your observer should inherit Auditing::Observer

module Auditing def self.included(base) ActiveRecord::Base.observers.each do |observer| observer = if observer.respond_to?(:to_sym) observer.to_s.camelize.constantize.instance elsif observer.respond_to?(:instance) observer.instance else raise ArgumentError, ”#{observer} is an invalid calss name” end base.around_filter(observer) if observer.is_a?(Auditing::Observer) end end

class Observer < ActiveRecord::Observer
  attr_accessor :controller
end

def before(controller)
  self.controller = controller
end

def after(controller)
  self.controller = nil
end

end

when i try to run that code i get the following error

ActionController::ActionControllerError

Filter object must respond to both before and after

I’m running rails 1.1.6. Don’t ask me why. I just need to be able to access the session in the model directly.

after using the code i got the session in my model, but while running the test cases for that model it gave me an error

NameError: undefined local variable or method `session’ for #<order:0x38b2b14>

H’mmm… I found this discussion because I was rushing to get something done late on a Friday (today) and tried to take the easy path of using session data in a model. I didn’t even think about it… it was just the easy, ‘logical’ thing to do. The error message I got brought me up short, and I had to go looking for a reason why.

In my case, I want to construct a file path for the user who is currently using the site… i.e., each user will have his own directory for files. Something like ”/path/to/user_id”

The user_id is in the session, and the ‘business logic’ that constructs this path is in a model. Voila, you have the temptation to use the session data.

I’m glad Rails stopped me from doing that, but actually, the controller that needs this path doesn’t call this method directly… it goes through several levels of method calls. It is not exactly ‘elegant’ to pass this value from the controller through a bunch of methods that don’t really need the value, simply so it can get to the method that constructs the path.

There is something about that that smells bad.

How about saving the client_id as a Class variable, @@client_id. Then any method that needs it can call Client.client_id, and get it. This seems like a clean way of solving this problem.

Anything I’m missing? (Yes, I’m ready to be called a stupid moron, if necessary :-)

—John

Pratik, got to agree with you whole heartedly on this issue. I personally think that hacking through and violating the MVC architecture is silly. My preferred way is to add in a reference to the user in the model. It does have to be stored and can quite simply just be added using attr_accessor. I recently wrote an article giving my solution to the problem. It can be found here – http://www.david-mcnally.co.uk/2009/01/31/how-to-reference-the-current-user-from-an-observer/

What really amazes me is that something like this can make someone so angry. That’s beyond geek. But you definitely got the message across.

You forgot to tag your rant “sanctimonious”.

Well, I’m confused actually. With the Restful concept, a model DOES have a specific URL that it lives at. Why can’t I get to it from the model?

I could also use this to create a dynamic default_scope on all my models that belong to the current_user sitting in the session, without having to use a lambda and remembering to pass the current_user from the controller.

So it’s definitely not all that completely/totally/f**g wrong!

Rails breaks MVC by allowing you to do whatever you want with ERB.

It’s also not completely wrong if you want to read that data outside of the controller if you want to do something with it from something in /lib.

I get here by searching for something very similar to the article heading. I’m aware it is pretty nasty to want to do this, but hear me out! :)

I want to do some basic auditing on my models. When they are CRUDed in various ways they should save an instance of a simple AuditRecord model.

Now I thought it would be cool to have request.remote_ip in the record for identification purposes. (Or blank if the operation wasn’t caused by an external request.)

My first idea was to just log stuff at the controller level but doing that I would miss modifications to my model objects that happen outside of a user’s direct actions with my controllers. Doubling up auditing at the controller and the model is a possibility but I’d be nice just to have an optional remote_ip field.

The only other thing I can think of is passing the request or ip address to the model when I do much of anything with it so the model can log the operation, ip included. That seems much uglier than letting models have default access to the current originating request.

Would this solution really be that heinous? If so how would you suggest solving a situation like this?

We have very tight security on our databases and implement all accesses via views and stored procedures. These views and procedures ensure at a database level (the correct place for this sort of security) that users are limited to only that data that they have access to.

The Rails method of global access it simply terrible. It’s shockingly bad security and a barn door to all sorts of security problems, from bugs to obtaining global DB access maliciously.

The correct way to implement security on a DB is to use the DB for security, not rely on the application to add the right “where” clause.

As such, you have to record the user’s logins, you cannot use the database.yml global method of access. User logins must == database logins.

And this means that in your models you need to use establish_connection. And to do that, you have to have the login information.

The best place (and correct place) for this is in the model. Something like:

class UserLoginTable < ActiveRecord::Base $config = ActiveRecord::Base.configurations[‘app’] $config.username=session[:dbuser]||’fail’ $config.password=session[:dbpass]||’fail’ end

establish_connection($config)

Then you subclass your models from UserLoginTable and voila, you have proper security implemented.

Hi guys,

I’ve implemented some observers for my models, but their purpose wasn’t just to register loggs. I wanna register what was done on the respective model, like actions of my controllers. The callback methods seem to be insuficient to do that, because sometimes my observe must know some business rules to know what was done on that “update” or “create”. Is it possible to create methods in my observers to “listen” do specific methods of the model, not necessarily the rails CRUD methods? For example: class ModelObserver > ActiveRecord::Observer{

‘do_something’ is a method of my model

def after_do_somenthing(model){ ... }

}

thanks, guilherme