Rails - Nov 2008 - Securing Models From a Controller versus ActsAsRowSecured

So, I''m wondering, what do all of you think of ActsAsRowSecured? 
I''m
little wary because it appears to use a singleton UserContext and
jigger that back into a SecurityContext class under ActiveRecord.
i.e.

class UserContext
  include Singleton

  @values = {}

  class << self
    def get(key)
      @values[key]
    end

    def set(key, value)
      @values[key] = value
    end
  end
end

class AccessControlled < ActiveRecord::Base
  def self.inherited(subclass)
    subclass.send(:acts_as_row_secured, :conditions => { :user_id =>
SecurityContext.context_info(:user_id) })
  end

Isn''t it pure heresy to reference session data by proxy through
Singletons with an ActiveRecord override?  This seems like an
egregious violation of MVC to me.

Isn''t it better to simply apply a scope your AR models in
ApplicationController as follows?

class ApplicationController
  around_filter ScopedAccess::Filter.new(Posts, :accessible)

protected
  def accessible
    { :find   => {:conditions => ["user_id = ?",
session[:user_id]]},
      :create => {:user_id  => session[:user_id]} }
  end

# Adopted from http://www.caboo.se/articles/2006/2/22/nested-with_scope

Can someone confirm my suspicions or allay my fears regarding
acts_as_row_secure?  Or, am I being a religious zealot by insisting on
MVC purity here?  Better yet, is there another idiom that''s even more
ironclad than the caboo.se ScopedAccess one?

Tony

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

Tony Perrie wrote:
> Better yet, is there another idiom that''s even more
> ironclad than the caboo.se ScopedAccess one?

Maybe I''m misunderstanding the context, but why not just secure your 
model through the has-many association? It''s the most basic security 
idiom and is built-in.

class User < ActiveRecord::Base
  has_many :posts
end

class ApplicationController < ActionController::Base
  def ...
    current_user.posts.find ...
    current_user.posts.create ...
  end
end


BTW A pattern I seen if you really need to have the model access the 
current user is to use a class attribute accessor in a before filter.

class User < ActiveRecord::Base
  cattr_accessor :current_user
  ...
end

class ApplicationController
  before_filter :set_current_user

  protected

  def set_current_user
    User.current_user = current_user if logged_in?
  end
end

-- 
Posted via http://www.ruby-forum.com/.

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

On Nov 1, 9:44 am, David Lowenfels
<rails-mailing-l...-ARtvInVfO7ksV2N9l4h3zg@public.gmane.org>
wrote:
> Maybe I''m misunderstanding the context, but why not just secure
your
> model through the has-many association? It''s the most basic
security
> idiom and is built-in.
I suggested this to my team lead originally.  However, in our app
there are a lot of models, and multiple tiers of access.  So, it''s a
minor headache for us to manually secure everything that way.  But,
you''re right, associations and lambda''d named_scopes are the
least
surprising way to accomplish this.

Thankfully, the ScopedAccess stuff is built-in to Rails and does the
same thing as acts_as_row_secure in a standard way.  So, it''s just as
viable as an association and a sane compromise. This idiom basically
prevents us from having to type in accessible_by(@current_user) on
nearly every model in our controllers.
> BTW A pattern I seen if you really need to have the model access the
> current user is to use a class attribute accessor in a before filter.
That is a good point about using the User model cattr_accessor instead
of a library class for current_user.  Having the UserContext class
serve as a session proxy doesn''t really sit right with me either.
That UserContext business is only needed for acts_as_row_secured
because you can''t really access a defined superclass of ActiveRecord
when you''re defining a mixin for ActiveRecord itself.

This is a big reason _not_ to use acts_as_row_secured.  UserContext is
essentially a dirty hack to sneak session data references into the
ActiveRecord definition itself which violates MVC and forces you to
have to set global constants when using a model anywhere outside of a
controller.  This technique is used in all over the place in ad hoc
PHP apps which is why it seems wrong to me.

Tony
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

> This is a big reason _not_ to use acts_as_row_secured.  UserContext is
> essentially a dirty hack to sneak session data references into the
> ActiveRecord definition itself which violates MVC and forces you to
> have to set global constants when using a model anywhere outside of a
> controller.  This technique is used in all over the place in ad hoc
> PHP apps which is why it seems wrong to me.
Let me play devil''s advocate here and take the contrary position to
this, giving reasons why you might want to break MVC in this
particular way in this particular case:

First, by doing this with acts_as_row_secured you enforce that some
UserContext is available during any model access. Although this gets
in the way of some simple manual testing (requiring you to have
contexts set all the time), by the same token, it enforces that you
can''t ''fake'' your unit tests: You don''t get
the choice of wrapping a
UserContext all the time -- if access security is paramount to you,
this is a painful, but enforced way of making sure you have it at all
time (including in all your testing).

Second, although this does break "classical" MVC (making the M reach
out to the C for the UserContext, or, put the other way, invisibly
injecting the UserContext from C down into M), this can be thought of
very naturally if you just forget about UserContext altogether; just
forget UserContext exits -- it''s taken care of for you, by this hack,
and it (UserContext) becomes part of the metaobject protocol. So, if
you just forget that UserContext is a concept that you have to worry
about anylonger, then MVC is no longer broken.

Finally, even if we grant that this breaks MVC (that is, you can''t
take my proposed stance of just forgetting about UserContext), hacking
the metaobject protocol to take account of contextual factors is a
well-accepted pattern in many (non-PHP) programming paradigms. One
thing that RoR offers is a well encapsulated way of doing this; PHP
does not, so, you''re right that this is ad hoc and ugly in PHP, but
the same thing can be done in RoR in this conceptually clean way.
This is a very common, and very clean, pattern in Lisp engineering,
where working with the metaobject protocol is a well accepted meta-
pattern. This isn''t to bias the issue, just to point out that just
because a pattern is ugly in language A, it needn''t be ugly in
Language B, where B offers a clean way to do it.

''Jeff

[Full disclosures: a. I work with Tony, b. Although I''m not a highly
experienced Rails engineer, I *am* a highly experienced engineer in
many other paradigms OOP and classical paradigms, incl. Smalltalk and
Lisp.  I''m engaging in this discussion out of real constructive
interest in learning; thus the devil''s advocate stance.]

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

The problem is that acts_as_row_secured isn''t really conceptually
clean because you don''t know the user_id at ActiveRecord model class
creation time.  That''s why you have to create this mysterious context
stuff to punch a hole in the abstraction layer.

It''s a matter of pragmatism.  Using acts_as_row_secured this way
forces you to have to patch Rails itself and set a secret global
variable to use unit tests, script/console, rake db:migrate. After
that, you have to use a special disable flag to create a new user
because there is no UserContext during sign-up to the site.

Using ScopedAccess filters from the controller generates the exact
same SQL and doesn''t break any of the Rails infrastructure in the
process.  So, why violate the principle of least surprise, break MVC,
rewrite existing Rails features, and patch a bunch of infrastructure
when you can accomplish the exact same functionality with two lines of
code in ApplicationController?

Tony

On Nov 1, 1:35 pm, Jeff Shrager
<JShra...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
wrote:
> > This is a big reason _not_ to use acts_as_row_secured.  UserContext is
> > essentially a dirty hack to sneak session data references into the
> > ActiveRecord definition itself which violates MVC and forces you to
> > have to set global constants when using a model anywhere outside of a
> > controller.  This technique is used in all over the place in ad hoc
> > PHP apps which is why it seems wrong to me.
>
> Let me play devil''s advocate here and take the contrary position
to
> this, giving reasons why you might want to break MVC in this
> particular way in this particular case:
>
> First, by doing this with acts_as_row_secured you enforce that some
> UserContext is available during any model access. Although this gets
> in the way of some simple manual testing (requiring you to have
> contexts set all the time), by the same token, it enforces that you
> can''t ''fake'' your unit tests: You don''t
get the choice of wrapping a
> UserContext all the time -- if access security is paramount to you,
> this is a painful, but enforced way of making sure you have it at all
> time (including in all your testing).
>
> Second, although this does break "classical" MVC (making the M
reach
> out to the C for the UserContext, or, put the other way, invisibly
> injecting the UserContext from C down into M), this can be thought of
> very naturally if you just forget about UserContext altogether; just
> forget UserContext exits -- it''s taken care of for you, by this
hack,
> and it (UserContext) becomes part of the metaobject protocol. So, if
> you just forget that UserContext is a concept that you have to worry
> about anylonger, then MVC is no longer broken.
>
> Finally, even if we grant that this breaks MVC (that is, you can''t
> take my proposed stance of just forgetting about UserContext), hacking
> the metaobject protocol to take account of contextual factors is a
> well-accepted pattern in many (non-PHP) programming paradigms. One
> thing that RoR offers is a well encapsulated way of doing this; PHP
> does not, so, you''re right that this is ad hoc and ugly in PHP,
but
> the same thing can be done in RoR in this conceptually clean way.
> This is a very common, and very clean, pattern in Lisp engineering,
> where working with the metaobject protocol is a well accepted meta-
> pattern. This isn''t to bias the issue, just to point out that just
> because a pattern is ugly in language A, it needn''t be ugly in
> Language B, where B offers a clean way to do it.
>
> ''Jeff
>
> [Full disclosures: a. I work with Tony, b. Although I''m not a
highly
> experienced Rails engineer, I *am* a highly experienced engineer in
> many other paradigms OOP and classical paradigms, incl. Smalltalk and
> Lisp.  I''m engaging in this discussion out of real constructive
> interest in learning; thus the devil''s advocate stance.]
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

I found this post from dhh in 2006:

http://groups.google.com/group/rubyonrails-core/browse_thread/thread/a8e60f340a682977/91ecfff96ae35de1

He disagrees with even tacking on a with_scope at the controller
level.  So, is the Rails way of doing this to explicitly call model
functions or named_scopes from the controllers explicitly?

Tony

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

I got some more on this from the engineer who developed the other
approach. He writes:

---
The proposed method supports multi-tenancy with the explicit
requirement for role-based permissions for all data views. On the one
hand, the intellectual property of an individual or organization must
be guaranteed. On the other hand, it must be possible to share
information with an arbitrary granularity. All of this must be
accomplished with reasonable database performance. Our  model now
assumes that some role is always current.

Enforcing read permission cannot be accomplished as a post-query
filtering operation. Not only would such a solution not scale, but it
would potentially result in incorrect results being returned because
of inappropriate intermediate results being incorporated as part of a
join. Thus, for both efficiency and semantic correctness, we have
engineered a role-based view as part of the data model, affecting
nearly every query. This is most conveniently addressed in the Rails
framework by appending a WHERE clause to the ActiveRecord :find
operator (using the :conditions hook).

To designate the classes under access control, I define an
acts_as_access_controlled mixin based on a plugin downloaded from
RubyForge. This has the advantage of being declarative, making it
obvious which classes are under access control. At run time, a
UserContext (we could call it something different) is dynamically
discovered and used to bind parameters in the prepared SQL statement.
This causes access control to be on by default, which I consider to be
desirable because it is now part of the data model and should be
routinely incorporated into queries during development and testing. An
all-seeing (''root'') mode is also supported.

Tony has proposed an alternate mechanism to inject the same methods
(or perhaps methods with an SQL string not requiring parameter
binding)
into the models from the application controller. [See above for Tony''s
claims of the superiority of his method.]
---

Opinions?

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

The Rails way according to dhh is "don''t do that".  I agree. 
Neither
approach makes any sense as the main vector for MySQL vulnerabilities
is an injection attack.  Making the framework less general is both
dumb and counterproductive and provides no additional security.

I''d argue that we should just use finder methods and named_scopes in
the AR models rather than reinvent the wheel.

Tony

On Nov 5, 1:42 pm, Jeff Shrager
<JShra...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
wrote:
> I got some more on this from the engineer who developed the other
>
> approach. He writes:
>
> ---
> The proposed method supports multi-tenancy with the explicit
> requirement for role-based permissions for all data views. On the one
> hand, the intellectual property of an individual or organization must
> be guaranteed. On the other hand, it must be possible to share
> information with an arbitrary granularity. All of this must be
> accomplished with reasonable database performance. Our  model now
> assumes that some role is always current.
>
> Enforcing read permission cannot be accomplished as a post-query
> filtering operation. Not only would such a solution not scale, but it
> would potentially result in incorrect results being returned because
> of inappropriate intermediate results being incorporated as part of a
> join. Thus, for both efficiency and semantic correctness, we have
> engineered a role-based view as part of the data model, affecting
> nearly every query. This is most conveniently addressed in the Rails
> framework by appending a WHERE clause to the ActiveRecord :find
> operator (using the :conditions hook).
>
> To designate the classes under access control, I define an
> acts_as_access_controlled mixin based on a plugin downloaded from
> RubyForge. This has the advantage of being declarative, making it
> obvious which classes are under access control. At run time, a
> UserContext (we could call it something different) is dynamically
> discovered and used to bind parameters in the prepared SQL statement.
> This causes access control to be on by default, which I consider to be
> desirable because it is now part of the data model and should be
> routinely incorporated into queries during development and testing. An
> all-seeing (''root'') mode is also supported.
>
> Tonyhas proposed an alternate mechanism to inject the same methods
> (or perhaps methods with an SQL string not requiring parameter
> binding)
> into the models from the application controller. [See above
forTony''s
> claims of the superiority of his method.]
> ---
>
> Opinions?
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

Tony Perrie wrote:
> I''d argue that we should just use finder methods and named_scopes
in
> the AR models rather than reinvent the wheel.
> 
> Tony

earlier you said:
>> Maybe I''m misunderstanding the context, but why not just
secure your
>> model through the has-many association? It''s the most basic
security
>> idiom and is built-in.
> I suggested this to my team lead originally.  However, in our app
> there are a lot of models, and multiple tiers of access.  So, it''s
a
> minor headache for us to manually secure everything that way.  But,
> you''re right, associations and lambda''d named_scopes are
the least
> surprising way to accomplish this.
Minor headache is not a good excuse. You''ll only have to write that
sort
of code once, and then copy it around to your other controllers.
-- 
Posted via http://www.ruby-forum.com/.

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---