Problems with including in controllers that extend ActionController::Metal about doorkeeper

[felipeelias]

I didn't test this code but this might help:

class MyController < ActionController::Metal
  include AbstractController::Callbacks
  include Doorkeeper::Helpers::Filter

  doorkeeper_for :all
end

You have to include this two modules manually because when we started doorkeeper, we didn't build with Metal in mind.

This is something that will have to change in this next version, since makes much more sense to work with Metal controllers for API.

[felipeelias]

Let me know whether this works or not, then I'll add it to the wiki.

Thanks!

[r3ap3r2004]

Thanks for responding!

I actually tried something similar to that earlier, although I hadn't tried with the AbstractController::Callbacks entry. Trying your suggestion throws the following error when trying to start the server.

api/v1/users_controller.rb:8:in `<class:UsersController>': uninitialized constant Doorkeeper::Helpers (NameError)

I've tried adding a bunch of require statements to pull everything in too, but that didn't seem to work either. It also didn't make sense since I didn't actually need any require statements when using ActionController::Base

I assume that it's something fairly simple, but I'm at a loss for how to make it work.

Thanks again,
Chris

[felipeelias]

@r3ap3r2004 are you working from master? because this helpers were introduced yesterday.

[r3ap3r2004]

I'm using 0.4.0

[felipeelias]

Ah ok. Would you mind to try this from master? It might break some stuff though, sorry.

I'll check a best way to do this tonight and release a 0.4.1 with this improvement.

[r3ap3r2004]

Using master and adding include Doorkeeper::Helpers::Filter seems to have worked. I'm not sure if I needed to also add include AbstractController::Callbacks since it looks like I was already including that one.

Thanks for your help, I really appreciate it!

Chris

[r3ap3r2004]

Well it looks like master breaks the applications controller (or at least the routes have changed somehow).

I get the following error:

No route matches [GET] "/oauth/applications/1"

It works fine with 0.4.0.

[felipeelias]

@r3ap3r2004 yes, it's because the routes were changed. You have to use use_doorkeeper instead of mounting the app with mount.

This will be the defaults on 0.5

[r3ap3r2004]

That worked great! Thanks again.

[r3ap3r2004]

There still seems to be an issue.

I'm now running into problems where the system can't find my rabl view templates for some reason when accessing info through the api. I'll need to debug that one in the morning, but it appears there are still some issues with using ActionController::Metal.

It's doorkeeper related because I can hit them just fine if I leave doorkeeper out of the equation, or if I use ActionController::Base instead. Too tired to debug it tonight though. :-(

Thanks again for your help.

[felipeelias]

Post the error when you find it out, maybe I can help.

But, if you try to access the API with an invalid token, doorkeeper might try to render the error response, which might be your case.

[r3ap3r2004]

You can see the output here: https://gist.github.com/2821603

I have a node.js app that calls into my rails app. Everything works perfectly if I change the rails class to extend from ActionController::Metal to ActionController::Base which tells me that my tokens, etc are all working as expected.

I can also access the template just fine using ActionController::Metal if I remove these lines from the class.

        include Doorkeeper::Helpers::Filter
        doorkeeper_for :all

That tells me that it is definitely Doorkeeper that is causing the problem, I'm just not exactly sure why it is causing a problem.

Currently my ActionController::Metal class is including the following modules:

          include AbstractController::Rendering
          include AbstractController::ViewPaths
          include AbstractController::Callbacks
          include AbstractController::Helpers          
          include ActiveSupport::Rescuable          
          include ActionController::Rendering
          include ActionController::ImplicitRender
          include ActionController::Rescue
          include ActionController::MimeResponds
          include CanCan::ControllerAdditions

Any help you can give me would be greatly appreciated.

Thanks again,
Chris

[felipeelias]

Thanks!

It's very weird behaviour, I still couldn't find why this happens. Going to investigate it today and let you know.

[r3ap3r2004]

Thanks. I look forward to seeing what you find out.

[felipeelias]

I just tested and this works:

class MetalController < ActionController::Metal
  include AbstractController::Callbacks
  include ActionController::Head
  include Doorkeeper::Helpers::Filter

  doorkeeper_for :all

  def index
    self.response_body = { :ok => true }.to_json
  end
end

This is the bare minimun to get an :unauthorized response or, if the access token passes, you'll get the response body.

You mentioned that you're using rabl, right? Judging by the gist you posted, it seems an issue finding view paths for your app which has nothing to do with doorkeeper.

Also, where did you include all those modules? If you are allowed, could you share the controller/action you're testing (the one from /api/users/1.json) ?

[r3ap3r2004]

I'm extending Spree. You can get the base code I'm using here:
https://github.com/spree/spree

You should be able to use any of the controllers in:
/api/app/controllers/spree/api/v1

They have their own authentication method, but I need it to tie in with some other pieces that I'm doing that I need an OAuth2 solution for.

The base_controller.rb file is the controller that each individual api controller uses for it's base class, and that extends from ActionController::Metal.

The modules are pulled in via the line:

include Spree::Api::ControllerSetup

which is found in /api/lib/spree/api/controller_setup.rb

I believe this is a doorkeeper problem because the view_path works as expected as long as I leave these lines out of the controller

 include Doorkeeper::Helpers::Filter
 doorkeeper_for :all

Alternatively, it works fine with those two lines from Doorkeeper if I change the base_controller.rb file/class to extend from ActionController::Base instead of Metal.

Note: In order to authorize with Doorkeeper you will need to change the following methods found in base_controller.rb.

        def check_for_api_key
          render "spree/api/v1/errors/must_specify_api_key", :status => 401 and return if api_key.blank?
        end

        def authenticate_user
          unless @current_api_user = User.find_by_api_key(api_key)
            render "spree/api/v1/errors/invalid_api_key", :status => 401 and return
          end
        end

You should be able to just do this:

def check_for_api_key
end

def authenticate_user
  unless @current_api_user = User.find(doorkeeper_token.resource_owner_id) if doorkeeper_token
  render "spree/api/v1/errors/invalid_api_key", :status => 401 and return
  end
end

I really appreciate the help. Unfortunately I won't be anywhere that I will be able to test your code suggestion for at least another 10 hours.

Thanks again,
Chris

[felipeelias]

I had a similar issue yesterday on another project without even including doorkeeper. Still have no idea why...

[felipeelias]

@r3ap3r2004 just released 0.4.1 with the doorkeeper filter module.

There's also some docs related to this. https://github.com/applicake/doorkeeper/wiki/ActionController::Metal-with-doorkeeper

[felipeelias]

@r3ap3r2004 any news on this?

[r3ap3r2004]

Sorry for the lack of update. I've been buried getting some other pieces in place and haven't had a chance to test it yet. Although when I tried the master version adding the "include ActionController::Head" section right after you put up that example it still didn't work. I haven't tried with the 0.4.1 release yet. Hopefully I'll have time to look into it later today or tomorrow. I'll be sure and post back here and let you know.

Thanks again,
Chris

[felipeelias]

I'm closing this. If anybody else run into the same issue, please reopen.

Thanks!