Adapt authorizations controller to be scope agnostic, refs #23

[BRMatt]

Just starting a pull request to keep track of this (not ready to merge just yet).

Currently doesn't include tests (noticed that the routing for the authorization controller is failing in routing tests so not sure if integration test is possible) or documentation (currently need to wrap the mount in a devise_scope)

  devise_scope :member do
    mount Devise::Oauth2Providable::Engine => '/oauth2'
  end

This is currently a breaking change, need to make it backwards compatible.

[wireframe]

+1

I'm absolutely in favor of decoupling the gem from requiring a User model. if you can add some tests to verify behavior I'll pull it in.

re: routing specs...they are failing due to a rails bug loading the routes for tests. i'm not too concerned with it at the moment, but let me know if you see any patches.

[wireframe]

can you apply this change to the tokens_controller as well?

[BRMatt]

can you apply this change to the tokens_controller as well?

Hehe whoops, forgot to do them! I've done a little bit of refactoring locally and will hopefully be able to push them up at some point. There are some other things related to this that are a bit annoying (e.g. all the relationships between models also hardcode User)

re: routing specs...they are failing due to a rails bug loading the routes for tests. i'm not too concerned with it at the moment, but let me know if you see any patches.

Ok, do you happen to know what the issue number for that bug is?

[wireframe]

here's a blog post that details the issues testing routes with rails engines: http://www.builtfromsource.com/2011/09/21/testing-routes-with-rails-3-1-engines

and here is the monkey patch i've applied to the engine routing specs: https://github.com/socialcast/devise_oauth2_providable/blob/master/spec/support/inject_engine_routes_into_application.rb

[BRMatt]

I've been running into some problems with routing etc., just thinking through options and was wondering if you had anything against dropping the rails engine mounting and using a devise-esque routing helper such as devise_oauth_for?

For a start it'd make it easier to view routes with the rake routes task and should solve the problems with spec routing not working. It'd also make it easier to use more than one oauth endpoint (e.g. if you have two different devise scopes) and if you need to customise behaviour (e.g. in our app only a few private apps are allowed, so if the client's credentials are valid we don't want to ask the user to authorize the app)

[BRMatt]

Ok, the middle commit basically rewrote the routes to be initialized in the same way as devise's, using devise_oauth_for.

Still not quite decoupled the User model, that's the next thing I'm going to hit.

[JeanMertz]

Is this something that is still being worked on. This seemed like a promissing change for someone who doesn't use a User model...

[BRMatt]

Everything started off fairly simply, but I ended up rewriting a fair amount of this gem's "core code" (mainly so that it could be tested without throwing off a few hundred errors) so I doubt it'll get merged in.

We're currently using our semi-rewrite in production, the modified version of the gem is available in jestro/devise_oauth2_providable (see jestro/master and 23-scope), however we haven't [yet] backported recent changes to this gem.

[JeanMertz]

I see, thanks for the heads-up. I'll go check out your fork and see if I can get it to work.

update: I've ported over some easy-to-understand commits to your version of the gem, but I haven't tested this yet. Other changes/commits where a bit larger in scope and seemed to change things you already modified in your fork. I hope we can get this ball rolling again and get the different forks merged together in the near future.

[scashin133]

@BRMatt are you still interested in getting this merged in? If so can you please re-pull master and I'll take a look.

[BRMatt]

It's been a long time since I looked at this code, so it will probably require a cleanup in some areas before it can be merged. We also moved the repository a few months ago, so unfortunately this PR is out of date.

Unfortunately life's a bit busy atm, so it may be a few weeks before I can get time to check it.