gonzalo-bulnes / simple_token_authentication

Simple (and safe*) token authentication for Rails apps or API with Devise.
GNU General Public License v3.0
1.51k stars 238 forks source link

Grape integration? #138

Open gonzalomelov opened 9 years ago

gonzalomelov commented 9 years ago

Hi, I have a question.

How can I integrate this gem with Grape?

I think that I have to set _acts_as_token_authentication_handlerfor on the root class of Grape API, as it is used here

What do you think?

Thanks

gonzalo-bulnes commented 9 years ago

The article seems to be implementing same José Valim gist which this gem packages. The idea behind Simple Token Authentication is precisely to make the implementation easier and less prone to errors, and you idea sound very well to me.

In order to make possible to your API::Root to act as a token authentication handler, you'll need to ensure that Grape::API does extend the SimpleTokenAuthentication::ActsAsTokenAuthenticationHandler module. The usual way to do that (or to ensure that it is done automatically) is defining a dedicated adapter (see also where adapters are loaded). I released such an adapter an hour ago to add Rails::API support (v1.7.0); something similar can be done for Grape.

If you want to see how adapters are built, you can take a look at #131 (the Rails::API adapter) and #124 (where I described step-by-step the adapter implementation). If you want to move quickly, just do fork and I'll help you with the adapter definition, pulling back the commits afterwards is easy.

gonzalo-bulnes commented 9 years ago

Hi @gonzalomelov,

Would you give a try to a Grape adapter for Simple Token Authentication?

# Gemfile

gem 'simple_token_authentication', git: 'git@github.com:gonzalo-bulnes/simple_token_authentication.git', branch: 'spike-add-grape-support'

Hi @u2, for what I've seen you too may find that feature interesting ; )

Once the gem installed, you should be able to call acts_as_token_authentication_handler_for from any controller inheriting from Grape::API. Any feddback will be welcome, and if everything works fine, the feature is production ready.

Regards!

Edit: Fixed missing .git extension in Gemfile example.

ianchen06 commented 9 years ago

Hi @gonzalo-bulnes Thanks a lot for redirecting me to the right direction. I tried the branch with Grape support but I'm getting the following error

[38752] ! Unable to load application: NoMethodError: undefined method `before_filter' for V1::Users:Class
/Users/ian/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/bundler/gems/simple_token_authentication-43d085a9158a/lib/simple_token_authentication/token_authentication_handler.rb:144:in `set_token_authentication_hooks': undefined method `before_filter' for V1::Users:Class (NoMethodError)

My controller for Grape looks like the following

module V1
  class Users < Grape::API
    format :json
    version 'v1', using: :path
    acts_as_token_authentication_handler_for User
    resource :users do
      desc 'Get user info'
      get do
        user = User.all
      end
    end
  end
end

Am I missing something? Thanks a lot

gonzalo-bulnes commented 9 years ago

Hi @ianchen06,

Yes, indeed, the before_filter (called before_action in Rails 4) method is part of Rails' ActiveController... For what I've seen, there is no exact equivalent in Grape, and defining a method to call from a before block in the appropriate namespace seems to be the common usage.

First of all, I'll disable the before_filter unless before_filter is defined, so Grape users don't get bothered with that.

Once that done, for Grape users, an additional step will be necessary to enable the token authentication handling feature for each endpoint (I don't know how the get '/' do; end blocks are called in Grape, endpoints?). That additional step will consist in calling a specific method from these endpoints. The method to call is already defined by Simple Token Authentication it is: authenticate_user_from_token! (assuming that User is your token authenticatable model).

I have some refactoring to do, however, to make sure the fallback option usage is straightforward, and I think it would be better if I created a new method called authenticate_user_from_token_or_fallback to be used in place of authenticate_user_from_token!. The name would be more explicit, and I could make that a backwards-compatible change. The corresponding usage, as I see it, would be:

module V1
  class Users < Grape::API
    format :json
    version 'v1', using: :path
    acts_as_token_authentication_handler_for User, fallback: :none

    desc 'Get all reports'
    get '/reports' do
      authenticate_user_from_token_or_fallback

      # At this point, if the users credentials (email, authentication_token) were correct,
      # then `current_user` is set.
      #
      # If the users credentials were missing or incorrect, the fallback was triggered:
      #  - with `fallback: :exception`, an exception was raised
      #  - with `fallback: :devise` (the default), Devise attempted to authenticate, and
      #    either `current_user` was set or an exception was raised by Devise
      #  - with `fallback: :none`, `current_user` is `nil`
      #
      # And you can do whatever you have to do with `current_user`.

      report = Report.where(author: current_user) # for example
    end
  end
end

(Note: I changed a little bit your example so we don't get confused by the different usages of User, :users, users, etc. and to avoid making errors with the resource syntax which is unfamiliar to me.)

What do you think? Does that sound reasonable? I mean: would that usage be something you would expect using Grape?

gonzalo-bulnes commented 9 years ago

Hi @ianchen06,

I updated the the code with the improvements described above. That's an exploratory move, but you can already use the brand new spike-add-grape-support-2 branch to try it:

# Gemfile

gem 'simple_token_authentication', git: 'git@github.com:gonzalo-bulnes/simple_token_authentication.git', branch: 'spike-add-grape-support-2'

I look forward to your feedback!

Edit: see the README in the spike-add-grape-support-2 branch for the draft documentation.

luiscarlos-gonzalez commented 9 years ago

Hi @gonzalo-bulnes I have a issue related with this thread, I got an undefined local variable or method authenticate_user_from_token_or_fallback' for #<Grape::Endpoint:0x00000008e57f10> error.

This is how I defined my Grapre Endpoint

module Pools
  class Data < Grape::API
    acts_as_token_authentication_handler_for User, fallback: :exception

    resource :pools do
      params do
        requires :id, type: Integer, desc: "Pool ID"
      end
      desc "Post payment to pool"
      post ":id/payments" do
        authenticate_user_from_token_or_fallback

        byebug
      end
    end
  end
end

This is my Gemfile

gem 'grape', '~> 0.10.0'
gem 'simple_token_authentication', github: 'gonzalo-bulnes/simple_token_authentication', branch: 'spike-add-grape-support-2'

Can you give me a hint about whats happening or there is a bug?

gonzalo-bulnes commented 8 years ago

Hi @Kentverger,

I think I know what's going on: Pools::Data inherits from Grape::API hence can authenticate_user_from_token_or_fallback (it acts as a token authentication handler), but Grape::Endpoint doesn't.

I'm not sure if the right thing to do is defining the method for Grape::Endpoint, or find another way to emulate the Rails before_filter behaviour.

Do you know what is the relation between a given instance of Grape::API and its endpoints? I mean, is there any helper to reference the Grape::API from your :pools resource? I'm thinking aloud, but maybe calling the Pools::Data instance (let's call it pools_data_api for example) authenticate_user_from_token_or_fallback method from the endpoint could be a step forward. I'm thinking in something like:

# ...

post ':id/payments' do

  pools_data_api.authenticate_user_from_token_or_fallback
end

# ...

What do you think?

luiscarlos-gonzalez commented 8 years ago

I don't know whats is the relation between Grape::API and Grape::Endpoint but I change my code for have Grape::API as super class and have authenticate_user_from_token_or_fallback method but I get the same error

module Pools
  class Data < Grape::API
    acts_as_token_authentication_handler_for User, fallback: :exception
    params do
      requires :id, type: Integer, desc: "Pool ID"
    end
    desc "Post payment to pool"
    post ":id/payments" do
      authenticate_user_from_token_or_fallback
    end
  end
end

What could be?

gonzalo-bulnes commented 8 years ago

Hi @Kentverger,

The Grape adapter is intended to be used just as you did. When I wrote it, I added the corresponding callbacks to Grape::API (and your Pools::Data instances themselves are able to authenticate_user_from_token_or_fallback). What I had no idea about, when I wrote the Grape adapter, is that the endpoint definition did in fact create a new instance of Grape::Endpoint.

(Note: The adapters role is to define which class should represent the token authentication handler in each framework, there is one adapter for Rails, another for Rails::API, and the one for Grape. Despite their big "adapter" name they are pretty simple pieces of code indeed.)

Specifically, what the Grape adapter does (and matters to us now) is defining that any Grape::API instance should be able to act as a _token authenticationhandler, that's a start, indeed, but it says nothing about Grape::Endpoint instances.

# ...

post ":id/payments" do
  # This piece of code defines a Grape::Endpoint instance.
  # That's to say anything in this block is executed in the scope of that instance,
  # not in the scope of the Grape::API instance,
  # and authenticate_user_from_token_or_fallback is not defined there...
end

# ...

I consider that as a bug in the adapter, I'm not sure how to handle it right now, but I'll explore it and keep you updated next week.

gonzalo-bulnes commented 8 years ago

TL;DR: If you're a Grape user and would like to use it with Simple Token Authentication, please let me know here. Some work has been done, but I need help defining an API which would make sense to Grape users, see #194.

Juko commented 7 years ago

Hey @gonzalo-bulnes, would love to get Grape Integration to work. I forked the gem and merged master to the grape branch https://github.com/Juko/simple_token_authentication/tree/spike-add-grape-support-2, but this has broken acts_as_token_authentication_handler_for when inheriting from Grape::API. I can't see anything that would have broken such a core part, and reading from the other discussions, this is one of the features that should work. Any help in direction would be appreciated! (couldn't just use the branch as I needed rails 5 support)

gonzalo-bulnes commented 7 years ago

Hello @Juko !

Merging master into the working branch has a downside: all the changes are gathered in the merge commit, which becomes pretty hard to read. To make things easier, I updated the spike in the spike-add-grape-support-3 branch.

The updated spike should make things easier to debug. Could you give it a try?

I'm definitely interested in progressing on the Grape support, and you help is very much appreciated. : )