Extensions for non-HTML formats

2 views
Skip to first unread message

Ian

unread,
May 10, 2009, 6:32:57 PM5/10/09
to Rails Authorization Plugin
Hi all,

I wrote a quick patch and submitted a pull request; Glenn requested I
repost here so that everyone can discuss.

I've used this plugin for most of my authorization needs for the last
2 1/2 years now I suppose, and every non-trivial project I've used it
on has at least needed a better way to handle "redirection" for failed
authorization on AJAX queries. Some projects I've worked on have also
had XML or JSON APIs, and they also needed different "redirection"
than a simple 302 to the login page.

I've previously just monkey-patched the code this way:

<pre>
module Authorization
module Base
module ControllerInstanceMethods
private
def handle_redirection_with_smarter_formats
respond_to do |format|
format.html { handle_redirection_without_smarter_formats }
format.js { render(:update) { |page| page.alert
"Permission denied." } }
format.xml { head :forbidden }
format.json { head :forbidden }
end
end
alias_method_chain :handle_redirection, :smarter_formats
end
end
end
</pre>

However, it strikes me that this must be a common need, and could be
written into the plugin's core. I did a quick translation of this
monkey patch into the plugin code, which you can see at this commit:
http://github.com/ianterrell/rails-authorization-plugin/commit/5eb7857e6955839f73ffda842f0877751bae4267

Of course, "redirection" is now a bit of a misnomer since it only
redirects in one case of the 4 supported, so ideally that would get
renamed. I was also unable to get the test project's test suite to
execute (only after about 3 minutes of trying), so I have not patched
the test project.

Thoughts?

Ian

Donald Ball

unread,
May 10, 2009, 9:51:40 PM5/10/09
to Rails Authorization Plugin
> module Authorization
>  module Base
>    module ControllerInstanceMethods
>      private
>      def handle_redirection_with_smarter_formats
>        respond_to do |format|
>          format.html { handle_redirection_without_smarter_formats }
>          format.js   { render(:update) { |page| page.alert
> "Permission denied." } }
>          format.xml  { head :forbidden }
>          format.json { head :forbidden }
>        end
>      end
>      alias_method_chain :handle_redirection, :smarter_formats
>    end
>  end
> end

Why don't you just send a :forbidden in the js case as well and let
the page decide what to do with it? It should be pretty easy with
jquery or prototype to register a global action to take in this case.

- donald

Glenn Rempe

unread,
May 10, 2009, 10:48:27 PM5/10/09
to Rails Authorization Plugin
My initial feedback would be:

- using respond_to is rails specific (and may change with rails3?).
While there may be other parts of the code that are rails specific it
would be a nice to have to make the plugin more framework agnostic,
and not the other way around.
- The format.js case is pretty application specific and I don't think
I would hard code a method to pop a page alert.
- All of this would optimally be configurable somehow so it could be
turned on or off depending on application needs. I don't think we can
assume that all xml, json, and js should be refused always. What
about if the app has basic auth or oauth and permits non-html
responses? Did I miss something?
- A passing test suite and new tests would be great. You should be
able to download the test suite and just manually replace the plugin
code with the code from your repo.

Even if this does not make it into core I think that some others out
there will find the monkey patch useful for their case.

What do others think?

Thanks.

Glenn
> monkey patch into the plugin code, which you can see at this commit:http://github.com/ianterrell/rails-authorization-plugin/commit/5eb785...

Ian

unread,
May 10, 2009, 11:27:52 PM5/10/09
to Rails Authorization Plugin
Sending the 403 to the JS case is definitely more "proper" default
behavior.

It would be nice to see it configurable to choose an alert or even a
custom error code snippet so that the error code can be centralized
per application and work transparently with link_to_remote,
form_for_remote, etc. From a pragmatic rather than purist point of
view, I'd hate to have to an error handling code to all of those calls
(which is how this monkey patch started!).

Ian

unread,
May 10, 2009, 11:28:11 PM5/10/09
to Rails Authorization Plugin
Hi Glenn,

Thanks much for the feedback.

Is being Rails specific really that much of an issue? The name of the
repo does say it's a Rails plugin. And with Merb merging with Rails,
that covers the vast majority of production uses, really about
everything but Sinatra, unless I'm missing a dark horse. And Rails 3
will definitely require plugin rework across the whole ecosystem.

I agree with you on the JS case; my earlier message to Donald just
states my pragmatic viewpoint on it. I'd like to see it all
configurable ideally.

Regarding OAuth or another authorization solution -- doesn't this
imply that they wouldn't be using this plugin? Unless I'm missing
some key configuration, this plugin checks authorization no matter
what format is requested, but just always returns a redirect. If they
wanted to mix and match OAuth or something else with this plugin -- at
least, this plugin's declaritive controller filters -- they would need
two separate controllers. If they didn't use the controller filter
versions of "permit" (or if they did but separated formats via
routes), then they'd control where they tested for permissions with
the plugin, and their application would already handle the separate
cases.

I agree with you a test suite would be great. I try to follow TDD
almost to the letter, and teach all of my developers to, too. But the
test suite craps out on me pretty quick, which is an impediment for
any third party developers trying to improve the core. With Rails 2.3
it craps out on the application controller filename naturally, and
with 2.0.2 on my system it fails with the below error (after creating
and migrating the SQLite3 database without event). Could be my
environment, I'm not sure; I haven't tried to debug.

Anyway, thanks again for the feedback. I do hope at least the monkey
patch helps keep this plugin useful for more applications than just
plain HTML sites.

Cheers,
Ian

[16:52][ian@ian-terrells-macbook-pro:~/src/authorization/rails-
authorization-plugin-test(master)]$ rake test RAILS_GEM_VERSION=2.0.2
(in /Users/ian/src/authorization/rails-authorization-plugin-test)
/System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/bin/ruby -
I"lib:test" "/Library/Ruby/Gems/1.8/gems/rake-0.8.5/lib/rake/
rake_test_loader.rb" "test/unit/group_test.rb" "test/unit/
meeting_test.rb" "test/unit/role_test.rb" "test/unit/user_test.rb"
/Library/Ruby/Gems/1.8/gems/activesupport-2.0.2/lib/active_support/
core_ext/hash/keys.rb:49:in `assert_valid_keys': Unknown key(s):
readonly (ArgumentError)
from /Library/Ruby/Gems/1.8/gems/activerecord-2.0.2/lib/
active_record/associations.rb:1170:in `create_has_many_reflection'
from /Library/Ruby/Gems/1.8/gems/activerecord-2.0.2/lib/
active_record/associations.rb:667:in `has_many'
from /Users/ian/src/authorization/rails-authorization-plugin-test/
vendor/plugins/authorization/lib/publishare/object_roles_table.rb:
130:in `acts_as_authorizable'
from /Users/ian/src/authorization/rails-authorization-plugin-test/
app/models/user.rb:5
from /Library/Ruby/Site/1.8/rubygems/custom_require.rb:31:in
`gem_original_require'
from /Library/Ruby/Site/1.8/rubygems/custom_require.rb:31:in
`require'
from /Library/Ruby/Gems/1.8/gems/activesupport-2.0.2/lib/
active_support/dependencies.rb:496:in `require'
from /Library/Ruby/Gems/1.8/gems/activesupport-2.0.2/lib/
active_support/dependencies.rb:342:in `new_constants_in'
... 9 levels...
from /Library/Ruby/Gems/1.8/gems/rake-0.8.5/lib/rake/
rake_test_loader.rb:5:in `load'
from /Library/Ruby/Gems/1.8/gems/rake-0.8.5/lib/rake/
rake_test_loader.rb:5
from /Library/Ruby/Gems/1.8/gems/rake-0.8.5/lib/rake/
rake_test_loader.rb:5:in `each'
from /Library/Ruby/Gems/1.8/gems/rake-0.8.5/lib/rake/
rake_test_loader.rb:5
/System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/bin/ruby -
I"lib:test" "/Library/Ruby/Gems/1.8/gems/rake-0.8.5/lib/rake/
rake_test_loader.rb" "test/functional/account_controller_test.rb"
"test/functional/object_roles_controler_test.rb" "test/functional/
really_secure_controller_test.rb" "test/functional/
rest_controller_test.rb" "test/functional/secure_controller_test.rb"
/Library/Ruby/Gems/1.8/gems/activesupport-2.0.2/lib/active_support/
core_ext/hash/keys.rb:49:in `assert_valid_keys': Unknown key(s):
readonly (ArgumentError)
from /Library/Ruby/Gems/1.8/gems/activerecord-2.0.2/lib/
active_record/associations.rb:1170:in `create_has_many_reflection'
from /Library/Ruby/Gems/1.8/gems/activerecord-2.0.2/lib/
active_record/associations.rb:667:in `has_many'
from /Users/ian/src/authorization/rails-authorization-plugin-test/
vendor/plugins/authorization/lib/publishare/object_roles_table.rb:
130:in `acts_as_authorizable'
from /Users/ian/src/authorization/rails-authorization-plugin-test/
app/models/user.rb:5
from /Library/Ruby/Site/1.8/rubygems/custom_require.rb:31:in
`gem_original_require'
from /Library/Ruby/Site/1.8/rubygems/custom_require.rb:31:in
`require'
from /Library/Ruby/Gems/1.8/gems/activesupport-2.0.2/lib/
active_support/dependencies.rb:496:in `require'
from /Library/Ruby/Gems/1.8/gems/activesupport-2.0.2/lib/
active_support/dependencies.rb:342:in `new_constants_in'
... 12 levels...
from /Library/Ruby/Gems/1.8/gems/activesupport-2.0.2/lib/
active_support/dependencies.rb:489:in `load'
from /Library/Ruby/Gems/1.8/gems/rake-0.8.5/lib/rake/
rake_test_loader.rb:5
from /Library/Ruby/Gems/1.8/gems/rake-0.8.5/lib/rake/
rake_test_loader.rb:5:in `each'
from /Library/Ruby/Gems/1.8/gems/rake-0.8.5/lib/rake/
rake_test_loader.rb:5
/System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/bin/ruby -
I"lib:test" "/Library/Ruby/Gems/1.8/gems/rake-0.8.5/lib/rake/
rake_test_loader.rb" "test/integration/stories_test.rb"
/Library/Ruby/Gems/1.8/gems/activesupport-2.0.2/lib/active_support/
core_ext/hash/keys.rb:49:in `assert_valid_keys': Unknown key(s):
readonly (ArgumentError)
from /Library/Ruby/Gems/1.8/gems/activerecord-2.0.2/lib/
active_record/associations.rb:1170:in `create_has_many_reflection'
from /Library/Ruby/Gems/1.8/gems/activerecord-2.0.2/lib/
active_record/associations.rb:667:in `has_many'
from /Users/ian/src/authorization/rails-authorization-plugin-test/
vendor/plugins/authorization/lib/publishare/object_roles_table.rb:
130:in `acts_as_authorizable'
from /Users/ian/src/authorization/rails-authorization-plugin-test/
app/models/user.rb:5
from /Library/Ruby/Site/1.8/rubygems/custom_require.rb:31:in
`gem_original_require'
from /Library/Ruby/Site/1.8/rubygems/custom_require.rb:31:in
`require'
from /Library/Ruby/Gems/1.8/gems/activesupport-2.0.2/lib/
active_support/dependencies.rb:496:in `require'
from /Library/Ruby/Gems/1.8/gems/activesupport-2.0.2/lib/
active_support/dependencies.rb:342:in `new_constants_in'
... 9 levels...
from /Library/Ruby/Gems/1.8/gems/rake-0.8.5/lib/rake/
rake_test_loader.rb:5:in `load'
from /Library/Ruby/Gems/1.8/gems/rake-0.8.5/lib/rake/
rake_test_loader.rb:5
from /Library/Ruby/Gems/1.8/gems/rake-0.8.5/lib/rake/
rake_test_loader.rb:5:in `each'
from /Library/Ruby/Gems/1.8/gems/rake-0.8.5/lib/rake/
rake_test_loader.rb:5
Errors running test:units, test:functionals, and test:integration!

Glenn Rempe

unread,
May 10, 2009, 11:58:11 PM5/10/09
to Rails Authorization Plugin
Hi Ian. Comments below.

On May 10, 8:28 pm, Ian <ian.terr...@gmail.com> wrote:
> Hi Glenn,
>
> Thanks much for the feedback.
>
> Is being Rails specific really that much of an issue?  The name of the
> repo does say it's a Rails plugin.  And with Merb merging with Rails,
> that covers the vast majority of production uses, really about
> everything but Sinatra, unless I'm missing a dark horse.  And Rails 3
> will definitely require plugin rework across the whole ecosystem.

Probably not a major issue. The plugin is definitely targeted at
rails. Just throwing that out there as a nice to have.

>
> I agree with you on the JS case; my earlier message to Donald just
> states my pragmatic viewpoint on it.  I'd like to see it all
> configurable ideally.

Agreed.

>
> Regarding OAuth or another authorization solution -- doesn't this
> imply that they wouldn't be using this plugin?  Unless I'm missing
> some key configuration, this plugin checks authorization no matter
> what format is requested, but just always returns a redirect.  If they
> wanted to mix and match OAuth or something else with this plugin -- at
> least, this plugin's declaritive controller filters -- they would need
> two separate controllers.  If they didn't use the controller filter
> versions of "permit" (or if they did but separated formats via
> routes), then they'd control where they tested for permissions with
> the plugin, and their application would already handle the separate
> cases.

Actually, this plugin is for 'authorization' and does not handle
'authentication' which would be handled by another plugin/gem such as
clearance or restful-authentication.

http://giantrobots.thoughtbot.com/2009/2/9/clearance-rails-authentication-for-developers-who-write-tests
http://github.com/technoweenie/restful-authentication/tree/master

I could envision application authentication using OAuth/basic auth/API
keys and then use this rails authorization plugin for access control
within an app for requests that represent authenticated users. So I
guess I am saying that if code like this is to be implemented it needs
to remain agnostic to what mechanism users authenticate with.

http://railstips.org/2009/3/29/oauth-explained-and-what-it-is-good-for


>
> I agree with you a test suite would be great.  I try to follow TDD
> almost to the letter, and teach all of my developers to, too.  But the
> test suite craps out on me pretty quick, which is an impediment for
> any third party developers trying to improve the core.  With Rails 2.3
> it craps out on the application controller filename naturally, and
> with 2.0.2 on my system it fails with the below error (after creating
> and migrating the SQLite3 database without event).  Could be my
> environment, I'm not sure; I haven't tried to debug.

I have not touched the test suite in a while as I have not been very
active on this code except for helping out with managing some of the
patches coming in. It SHOULD be brought up to date. I'd love some
help with that. Optimally it would be best if the test suite was
integrated into the plugin code. I just don't have time to tackle
that one myself. If anyone would like to take that on I'm sure we
would all appreciate it greatly.

>
> Anyway, thanks again for the feedback.  I do hope at least the monkey
> patch helps keep this plugin useful for more applications than just
> plain HTML sites.

I'm sure it will help. I think this is a good conversation to have.

Cheers,

Glenn

Ian

unread,
May 11, 2009, 11:41:21 AM5/11/09
to Rails Authorization Plugin
> Actually, this plugin is for 'authorization' and does not handle
> 'authentication' which would be handled by another plugin/gem such as
> clearance or restful-authentication.
> ...
> I could envision application authentication using OAuth/basic auth/API
> keys and then use this rails authorization plugin for access control
> within an app for requests that represent authenticated users. So I
> guess I am saying that if code like this is to be implemented it needs
> to remain agnostic to what mechanism users authenticate with.

You know, I've yet to use OAuth, and I was unclear yesterday as to
whether the "auth" part stood for authentication or authorization. To
make sure I didn't sound too ignorant, and since your lumping it in
with HTTP Basic made me assume authentication, I peeked at their
website oauth.net to be sure. However, it proudly claims "An open
protocol to allow secure API authorization." Unfortunately, it
appears they really meant authentication, but I didn't investigate
past that, figuring of course they'd be using proper terminology. :)

But as to the substance: Given that the handle_redirection code is
only executed after the authorization has been checked -- which
requires authentication already -- this solution is already
authentication-agnostic. (In fact, my live projects use it both with
HTTP Basic Auth and RestfulAuthentication.)

To rephrase, it does not change the dependencies that already exist in
the codebase.

Cheers,
Ian
> http://giantrobots.thoughtbot.com/2009/2/9/clearance-rails-authentica...http://github.com/technoweenie/restful-authentication/tree/master

Glenn Rempe

unread,
May 11, 2009, 6:00:31 PM5/11/09
to Rails Authorization Plugin
Thanks Ian :-)

OK. So today I have upgraded the test suite to run with Rails 2.3.2.
In fact I have frozen rails within vendor/. We can update it later as
needed.

This should ensure more of a level playing ground for testing the
plugin. Thanks for pointing out how broken it was with the upgrade to
Rails 2.3.x

This leaves all tests passing with the exception of one unit test
which is the result of an earlier commit. I'll contact the author of
that commit to see if he can fix it up.

Here is the test run now on my machine.

http://pastie.org/474972

Anyone doing dev on the plugin should make sure that the tests are all
passing with any changes they make (and of course add new tests to
reflect their changes.)

Ian, maybe you can try out your patch with the updated tests and see
how we do. We can figure out the best way to apply your code after
that unless any issues are raised.

Cheers,

Glenn
Reply all
Reply to author
Forward
0 new messages