GIANT ROBOTS SMASHING INTO OTHER GIANT ROBOTS

Written by thoughtbot

Using Wrapper Classes with ActiveRecord to Enforce Security Rules

(This article is also posted here)

Here’s the model in question:

class Article < ActiveRecord::Bas
  def self.search(query)
    ...
  end

  def title
    ...
  end

  # Columns include submitted and accepted
end

Articles can be submitted by users, and accepted by editors. We have controller namespaces for the public (when not logged in), users (when logged in), and editors (when logged in as an editor). And there are some privacy considerations to go along with all that. Namely:

  • The public should only see accepted articles
  • The editors should only see accepted and submitted articles
  • Users should only see accepted articles or unsubmitted articles that they’re still working on (owned by them).

The super-naive (and seriously painful) approach would be to have separate Article methods for each security domain. For example:

class Article < ActiveRecord::Base
  def search_for_admin(query)
    ...
  end

  def search_for_member(query)
    ...
  end

  def search_for_guest(query)
    ...
  end
end

..and you’d have to do this for all methods the controller may call.

Another approach would be to use the before_filter/with_scope magic described on HABTM. But the fact that DHH condemned it (to the point of declaring that with_scope will soon be declared private), and the way you have to go under the hood a bit with the @klass.scoped_methods call effectively dissuaded us.

Instead, we decided to go with a couple of wrapper classes. We still use with_scope, but within the model, and in a much more opaque way. We have three wrappers (one for each security context). Here’s the one for the administrator controllers:

Then, we replace all calls to Article in the admin controllers to AdminArticle. This ensures that any records returned to our admin controllers are submitted articles, and we added an incredibly small amount of code. We did the same thing for MemberArticles and PublicArticles, and it’s all worked fine. There are two changes that I’m thinking about making:

  1. If we have AdminArticle inherit from Article (without adding a type column to the table), we should be able to set all methods on Article as protected, ensuring that no errant code is looking at the wrong class.
  2. We could conceivably combine these wrappers into one WrappedArticle class, but I’m not sure if it would remain readable. It’s right on the edge of what I’d consider obfuscated as it is.

If you have any comments or suggestions on how to clean this up further, I’d love to hear them.