[rails] An authorization question

Testing RSpec
1

I’ve been going back over some legacy code, backfilling tests, and I’m
encountering something that is causing no small amount of pain. This is
in
a mature Rails app, that’s lived and migrated from 1.1 through to 2.1,
so
there’s a lot of ancient cruft built up in the corners that I’ve been
trying
to clean up.

My question/pain point revolves around authorization. In at least two
different models in the system – areas that are core to the
functionality
– there are models that run through a state transition. Only certain
users
are allowed to make those transitions, however. You’re basic “only an
admin
can publish an article” kind of restrictions.

These models show up across most of the app – several different
controllers. As such, long, long ago, someone patched updated the site
authentication code to assign a User.current singleton inside the
login_required filter. This is then used by several models, sometimes
to
populate an updated_by stamp, sometimes it’s actually used within a
models
validations(!), and it’s definately used within some of the
state-transition
guards.

Now, this is really just a global variable by another name, and it’s
pretty
well embedded after two years. I’ve come upon a whole bunch of
different
pain points in trying to setup data (real data) within the cucumber
steps
I’ve been backfilling. Lacking any support of injection, I end up doing
a
lot of juggling of the User.current value, just to get some test data
built
and in the right set of states … and while I can bury the temporary
reassignments necessary inside a block, it still feels like it’s an
intractable mess.

I know why this was originally done – to avoid having to pass User
objects around all the time, and it does appear to keep the API clean

but the hidden dependancy isn’t really clean.

So, does anyone have any suggestions of how to easily manage model level
user authorization?

2

On Sat, Feb 28, 2009 at 11:52 AM, Chris F. [email protected]
wrote:

can publish an article" kind of restrictions.

These models show up across most of the app – several different
controllers. As such, long, long ago, someone patched updated the site
authentication code to assign a User.current singleton inside the
login_required filter.

Unless I’m missing something, this seems like the problem is wider
than testability.

Let’s say I log in. Right now I am User.current. Now you log in, and
become User.current. Now I got to view some resource that I am not
permitted to see, but I get to see it because you are permitted and
YOU are the User.current.

Am I missing something?

3

On Sat, Feb 28, 2009 at 1:38 PM, David C.
[email protected]wrote:

My question/pain point revolves around authorization. In at least two
authentication code to assign a User.current singleton inside the
Am I missing something?
The login filter is called at the beginning of every request, from
application controller. It resets the value, nilling it out if you’re
not
logged in, at the start of each request. So long as Rails remains
single
threaded, the scenario you describe isn’t possible. One process, one
request at a time. No bleed there.

Of course, they’re supposedly working on making it not-so single
threaded,
so that may eventually become a problem. All the more reason to find
something that doesn’t suck.

4

On Sat, Feb 28, 2009 at 12:51 PM, Chris F. [email protected]
wrote:

so
admin
Let’s say I log in. Right now I am User.current. Now you log in, and
request at a time. No bleed there.

Of course, they’re supposedly working on making it not-so single threaded,
so that may eventually become a problem. All the more reason to find
something that doesn’t suck.

:slight_smile:

different
pain points in trying to setup data (real data) within the cucumber
steps
I’ve been backfilling. Lacking any support of injection, I end up doing
a
lot of juggling of the User.current value

You can stub this in your code examples:

User.stub!(:current).and_return(mock_model(User, :authorized? =>
true))

or

User.stub!(:current).and_return(mock_model(User, :authorized? =>
false))

etc.

Replace “authorized? => true/false” with whatever is necessary for the
authorization to pass or fail as needed in each example.

Stubs are cleared out after each example, so you don’t have to use any
additional injection techniques.

HTH,
David

So, does anyone have any suggestions of how to easily manage model level
user authorization?

5

Chris F. wrote:

I’ve actually been okay with it at the unit testing / rspec level –
I’ve had it stubbed as you describe for a while.

The pain point came in as I was trying to setup data for Cucumber …
Which, listening to my tests, tells me that the current structure is
bad. I was more curious to see how others are handling that sort of
situation.

If you are seeing state from one scenario bleed over to the next I would
suggest something like this in your env.rb:

After do
User.reset_current
end

I want to get away from the global variable, I’m just not entirely
sure what the target should be. There doesn’t seem to be a whole lot
of talk about actual implementation specifics around model level
authorization.

I generally have a current_user method defined of my controller to
return the logged in user. Assuming that your app is only using
User.current in your controllers you could try to move towards something
like that… If you have models accessing User.current then it truly is
being used as a global. :confused: The user will then have some permissions
methods that may take other objects or symbols. The method will simply
return a boolean telling if the user is authorized or not. That logic
usually is based on the role(s) of that user or relationship with the
passed in object. Having this logic in the user could be viewed as a
responsibility issue- should the user really be responsible for telling
if it is authorized for everything? In general I do this for most
simple cases. Only when it starts to get complex do I move it out into
a Manager-like object.

HTH,
Ben

6

I’ve actually been okay with it at the unit testing / rspec level –
I’ve
had it stubbed as you describe for a while.

The pain point came in as I was trying to setup data for Cucumber …
Which,
listening to my tests, tells me that the current structure is bad. I
was
more curious to see how others are handling that sort of situation.

I want to get away from the global variable, I’m just not entirely
sure
what the target should be. There doesn’t seem to be a whole lot of talk
about actual implementation specifics around model level authorization.

7

On Sat, Feb 28, 2009 at 2:54 PM, Chris F. [email protected] wrote:

I’ve actually been okay with it at the unit testing / rspec level – I’ve
had it stubbed as you describe for a while.

The pain point came in as I was trying to setup data for Cucumber … Which,
listening to my tests, tells me that the current structure is bad. I was
more curious to see how others are handling that sort of situation.

I want to get away from the global variable, I’m just not entirely sure
what the target should be. There doesn’t seem to be a whole lot of talk
about actual implementation specifics around model level authorization.

Disclaimer, this entire post has to deal with model level
authorization and a technique I’ve been using and evolving in Rails
projects. It has nothing to do with your issue of global state for
what User is logged in. Onto the fun stuff IMO…

I’ve been wanting to write a thorough blog post on thoughts and
reflections on a very related topic, but since you’ve already kicked
off the conservation I’ll try to add to the discussion. :slight_smile:

For nearly a year I’ve been utilizing the concepts of Roles and also
Privileges. They are close at heart, but different needs have proven
both of them to be successful at different times.

Here’s an example of being reliant on a role:

raises if the user cannot fulfill the role

supervisor = user.in_role(:supervisor)
supervisor.do_supervisory_thing

And the supervisor role looks like:

class Roles::Supervisor < Roles::Base
def do_supervisory_thing
# inside the implementation you
# can refer to the source of the role
# using the method #source
end
end

The privilege functionality is similar:

this will raise access denied if user doesn’t have the privilege

payer = user.with_privilege(:make_payment)
payer.pay!(invoice)

And its implementation looks like:

class Privileges::MakePayment < Privileges::Base
def pay!(invoice)
Payment.create! :invoice => invoice
end
end

This abstracts out the behaviour surrounding privileges and roles out
into their own component. In most cases the methods are very small,
and they merely create real models, or do other things. The big win
for me has been that they create a concrete representation of
something tied to a role or a privilege. I don’t know how much more
intention-revealing I can get. :slight_smile:

Right now I am leaving things a little verbose in the controller
actions, e.g. user.in_role(…) or user.with_privilege(…), but I
like it because no one has to guess what is being used. The less
verbose route would be to omit #in_role or #with_privilege and just
say:

user.pay!(invoice)

The user would be in charge of searching its roles/privileges for one
that responded to #pay!. I am considering moving to this route, there
are some edge cases that would need to be solved.

If you’re interested, the roles project is on github:

I just through up a wiki page for using them with Rails if you want to
test it out: http://wiki.github.com/zdennis/roles/rails

wrote:

so
users

The login filter is called at the beginning of every request, from
something that doesn’t suck.

Now, this is really just a global variable by another name, and it’s
You can stub this in your code examples:
authorization to pass or fail as needed in each example.
reassignments necessary inside a block, it still feels like it’s an
user authorization?

–
Zach D.
http://www.continuousthinking.com

8

On Sat, Feb 28, 2009 at 4:34 PM, Zach D. [email protected]
wrote:

Disclaimer, this entire post has to deal with model level
authorization and a technique I’ve been using and evolving in Rails
projects. It has nothing to do with your issue of global state for
what User is logged in. Onto the fun stuff IMO…

It’s actually a lot closer to what I’m trying to get to the bottom of
than
the global variable bit is.

Here’s an example of being reliant on a role:

raises if the user cannot fulfill the role

supervisor = user.in_role(:supervisor)
supervisor.do_supervisory_thing

Okay. There actually is something fairly close to this.

Take a group blog with an editor/supervisor. Anyone can write up and
draft
a document, but only the supervisor can publish it.

Right now, I do have a User#is_supervisor_of?(group) method. Good
enough
there.

Problem is, where does that check happen ? At the model level? Or in
the
controller?

You can externalize the request in the controller:

if current_user.is_supervisor_of?( document.group)
document.published_by = current_user
document.publish!
else
# flag some errors
end

But if it’s the sort of action that finds itself in a couple of
different
controllers, there’s been a worry about repetition – forgetting logic
and
such. Which is why the code is really more like this:

in the model

def publish!
if User.current.is_supervisor_of?(group)
self.published_by = User.current
self.state = ‘published’
else
raise NotASupervisorError
end
end

Only it’s actually a bit worse than that, because I think it’s wormed
it’s
way into the validations…

No controller can forget to check the authorization. However, it makes
setting up test data – and even working from the console – painful.
Among
other things, there a business rule in place that you can’t approve your
own
document.

 Payment.create! :invoice => invoice

Right now I am leaving things a little verbose in the controller

If you’re interested, the roles project is on github:
GitHub - zdennis/roles: Roles is a library for using role-based authorization and object modification.

I just through up a wiki page for using them with Rails if you want to
test it out: http://wiki.github.com/zdennis/roles/rails

I’ll give it a read, see what I can learn

9

On 28 Feb 2009, at 22:26, Chris F. wrote:

different locations.
your model needs not only to know about Users in general, it needs a
specific user. You have less chance of someone doing Something They
Shouldn’t due to a forgotten check in a controller, but the test
setup seems to suffer for it.

One way or the other, the global User.current is going away –
soon. It’s just a question of what to replace it with, and where.

I was only skim-reading this topic so I may be misunderstanding what
you’re after but I think that maybe what you’re looking for is
something like

, a Rails plugin that allows you to specify the authorisation in a
single place for both controllers and at the model level. I’ve just
started using it for a project and so far it seems a good fit, though
I’m trying to keep the whole app as restful resources which makes
things a little easier.

It also has a few test helper methods which make it really easy to use
with Cucumber and RSpec.

Andy

10

Personally I think this should be in the model layer. As you’ve pointed
it
you get alot of repetition if its in the controller layer. But putting
business logic in the controller layer goes against the fundamentals of
MVC,
and is a bit of a Rails anti-pattern (fat controller).

So given that the rules should be in the model then the question is
which
part of the model should have this responsibility and how do you call
it.
Three choices come to mind here

  1. Place the rule inside User

  2. Place the rule inside the affected model

  3. Create a new model(s) to encapsulate this functionality. It might be
    helpful to think of this as a service which models can use

  4. Pollutes User with additional responsibility, but you can live with
    this
    so long as things remain very simple

  5. Isn’t DRY if the rules apply to many different models

  6. Is more complex

It sounds like you have 1) and should consider moving to 3). However you
might get some change out of improving 1) first if you can, by using
standard refactorings

Have you considered creating a spike and doing a bit of BDD on the
spike.
i.e write the stories you want to write, implement simple code for them
and
try and work out the interactions you need from there. Even if you don’t
use
the spike code in your application this might help you get a better set
of
features for this piece of functionality, (not tainted by the
implementation
of the existing code. These features then can be applied to your legacy
project and perhaps other projects as well.

HTH

Andrew

2009/2/28 Chris F. [email protected]

11

Andrew P. wrote:

So given that the rules should be in the model then the question is
which part of the model should have this responsibility and how do
you call it.
Three choices come to mind here

  1. Place the rule inside User

  2. Place the rule inside the affected model

  3. Create a new model(s) to encapsulate this functionality. It might be
    helpful to think of this as a service which models can use

  4. Pollutes User with additional responsibility, but you can live with
    this so long as things remain very simple

I am not sure that this is really “pollution”. One of the things that
was pointed out to me on the Ruby list when I first began transitioning
to OO was the mantra “ask” don’t “tell”. It seems to me that in an OO
authorization scheme one might properly ask the user instance (model)
whether or not they are permitted to do “something” (controller) rather
than have the “something” test to see if that user is permitted.

12

On Sat, Feb 28, 2009 at 3:42 PM, Ben M. [email protected] wrote:

If you are seeing state from one scenario bleed over to the next I would
suggest something like this in your env.rb:

After do
User.reset_current
end

Yep got that. The tests are actually working, it’s just that the
setup
has gotten painful.

I want to get away from the global variable, I’m just not entirely sure
what the target should be. There doesn’t seem to be a whole lot of talk
about actual implementation specifics around model level authorization.

I generally have a current_user method defined of my controller to return
the logged in user.

Same. Authentication was originally generated using
acts_as_authenticated,
so the standard conventions at the controller level are in place.

Assuming that your app is only using User.current in your controllers you
could try to move towards something like that… If you have models
accessing User.current then it truly is being used as a global. :confused:

It truly is being used as a global.

The user will then have some permissions methods that may take other
objects or symbols. The method will simply return a boolean telling if the
user is authorized or not. That logic usually is based on the role(s) of
that user or relationship with the passed in object. Having this logic in
the user could be viewed as a responsibility issue- should the user really
be responsible for telling if it is authorized for everything? In general I
do this for most simple cases. Only when it starts to get complex do I move
it out into a Manager-like object.

Yes! This is what I was trying (poorly) to get at.

Responsibility issues might be a large part of why it got factored this
way
to begin with. The global is bad. Really bad, which is why I’m trying
to
figure out something that works better. But I believe it was put in
place
so that a model can be responsible for it’s own authorization. Some of
the
models are used and updated from several different controllers, so any
authorization logic external to the model would have had to be repeated
in
several different locations.

The concern with that might be an over-enthusiastic embrace of DRY.
However
some of the authorization stuff is Really Really Important, so embedding
the
authorization logic in the model itself was seen as a way to ensure it’s
not
forgotten about.

Half of my problem right now is that I’m not even sure what layer to
put
model specific authentication! If it’s in the controller layer, it’s
repeated logic in every controller that touches the model in question.
If
it’s in the model, the logic is centralized, but now your model needs
not
only to know about Users in general, it needs a specific user. You have
less chance of someone doing Something They Shouldn’t due to a forgotten
check in a controller, but the test setup seems to suffer for it.

One way or the other, the global User.current is going away – soon.
It’s
just a question of what to replace it with, and where.

13

James B. wrote:

I am not sure that this is really “pollution”. One of the things that
was pointed out to me on the Ruby list when I first began transitioning
to OO was the mantra “ask” don’t “tell”. It seems to me that in an OO
authorization scheme one might properly ask the user instance (model)
whether or not they are permitted to do “something” (controller) rather
than have the “something” test to see if that user is permitted.

Unless I have misunderstood your intent and by your third choice you are
referring to an external role based model while your first choice refers
to putting the actual rules inside the user model. In which case I
agree with you.

My comments refer to the idea that the user model makes the calls to the
role model and returns whether or not they were authorized to the
request.

14

On Mon, Mar 2, 2009 at 10:39 AM, James B. [email protected]
wrote:

I am not sure that this is really “pollution”. One of the things that
was pointed out to me on the Ruby list when I first began transitioning
to OO was the mantra “ask” don’t “tell”.

Actually, it’s the other way around.

///ark

15

Mark W. wrote:

Actually, it’s the other way around.

Pragmatic Bookshelf: By Developers, For Developers

Sigh…

16

James B. wrote:

Is this a semantic confusion on my part? Should I consider that what I
do with x.exists? is tell the object to answer a question?

Consider…

if x.exists?
x.important_method()
else
# nothing!
end

Now lets upgrade the variable x. Sometimes it points to a thing that
exists, and
sometimes it points to a stub object whose only job is to return ‘false’
from
exists?, and do nothing inside important_method():

if x.exists?
x.important_method()
else
x.important_method()
end

Now you can take out the if!

x.important_method()

We are no longer asking x if it exists. We are telling it what to do,
and
letting it decide how to do it. That is the heart of OO programming.

(There is still an ‘if’, somewhere - the one that populates x. But -
hopefully -
it’s only in one place.)

–
Phlip
http://www.zeroplayer.com/

17

----- Original Message ----

From: James B. [email protected]

Mark W. wrote:

Actually, it’s the other way around.

Pragmatic Bookshelf: By Developers, For Developers

I have read this article and it leaves me rather more confused than not.

That’s the danger of oversimplification. Another way to phrase it is,
don’t rely on objects for things they know. Rely on them for things
they know how to do.

Now it boils down to who’s responsible. The door’s lock is responsible
for reading the key, and the bolt is responsible for unlocking the door.
The door is only responsible for letting me in, along with some cold
air. The key is a role here, the lock is the controller, and the
bolt… okay, the analogy breaks down again. But consider zones of
responibility.

sam.authorized?(controller_or_model, action)?

I’ll suggest that it’s the controllers who are responsible for telling
what role or other requirements need to be satisfied to get their
services, and that it’s the job of the user object (maybe by delegating
to some role class or objects) to provide the information as to whether
those requirements are met.

I smell something when I think about individual models specifying their
requirements. Front-gate access through the controller actions smells
more correct to me.

If your actions can’t be boiled down that atomically, I ask the
question, “is there something else wrong?”.

Randy

18

unknown wrote:

sam.authorized?(controller_or_model, action)?

I’ll suggest that it’s the controllers who are responsible for telling
what role or other requirements need to be satisfied to get their
services, and that it’s the job of the user object (maybe by delegating
to some role class or objects) to provide the information as to whether
those requirements are met.

That is what I thought that I was doing. The Controller sends the
message to the User Instance telling it to answer the question: are you
authorized to perform “controller + action”?; or role, or whatever the
controller sends as the criteria to be met. It seems to me necessary
that the User model receive the context of the authorization call. Now
the actual check on whether user x is authorized to perform the create
method of the PaymentReceivedController is done in the #authorized?
method of User.

Is this what should be done or is there a different way?

19

Mark W. wrote:

Actually, it’s the other way around.

Pragmatic Bookshelf: By Developers, For Developers

I have read this article and it leaves me rather more confused than not.
I gather that I am missing something fundamental. Consider that when I
write x.to_s I am telling the object what to give back. But then what
am I doing when I write x.exists? or x.is_a?(y) or
sam.authorized?(controller_or_model, action)?

Is this a semantic confusion on my part? Should I consider that what I
do with x.exists? is tell the object to answer a question?

20

On Mon, Mar 2, 2009 at 3:59 PM, James B. [email protected]
wrote:

Is this what should be done or is there a different way?

Today, my pair and I hit a scenario which I think maybe a good example
to clarify.

Let’s say that you have an “admin” and “associate” role. Each role can
access invoices in the system, but each role can access different
subsets of invoices.

In our controller, we could have something like:

def index
if user.has_role?(“admin”)
Invoice.all
elsif user.has_role?(“associate”)
Invoice.by_area(current_user.area)
else
raise AccessDenied
end
end

We ended up not doing this because the lines “Invoice.all” and
“Invoice.by_area” is behaviour which is tied specifically to a certain
role in the system. We don’t want behaviour for these roles to be
scattered throughout controller actions. We want it in one single
location.

So we create a FiscalAdmin role object and a FiscalAssociate role
object. As these roles become filled out we leave the role-specific
behaviour in one location, creating a well-defined cohesive class for
each role.

For me it’s a little win, that results in big ways. :slight_smile:

–
Zach D.
http://www.continuousthinking.com