Documentation and authenticate_user!

[huttarichard]

I just came across one issue, I have both authenticate_user!, acts_as_token_authentication_handler_for User since I used devise before

 before_action :authenticate_user!
 acts_as_token_authentication_handler_for User, fallback: :exception, as: :user

and this will not work due to same before_filter handlers....
I think this should be mentioned in documentation... or you don't think so?

[gonzalo-bulnes]

Hello @huttarichard!

Integration with other authentication methods (namely Devise strategies) is documented in its own section of the README, maybe that documentation could be improved, however.

If you ask Devise to authenticate_user!, and the user fails at authenticationg with the Devise strategies, then Devise will raise an exception and you will have no opportunity to try to authenticate the user from any authentication token because you defined the token-authentication-related callbacks after the Devise callbacks.

By attempting token authentication before the Devise authentication, however, you are able to authenticate users from their authentication token, or if token authentication credentials were missing or incorrect, by any of the Devise strategies. If these do also fail, then Devise raises an exception an the access is denied, which is what you wanted.

# Deny access if all authentication methods fail:
acts_as_token_authentication_handler_for User, fallback: :none
before_action :authenticate_user!

# That's to say: attempt to authenticate users from their token, then fallback to
# the usual Devise behaviour if token authentication fails.
# That's what the fallback option was created in the first place:
acts_as_token_authentication_handler_for User, fallback: :devise

# And fallback: :devise is in fact the default _Simple Token Authentication_ behaviour.
# The previous configuration could be simplified further:
acts_as_token_authentication_handler_for User

Also, when the token authenticatable model you defined is User, the default behaviour is looking for the user credentials (user_email and user_token). Explicitely setting the as: :user option is therefore not needed.

I'd love to hear how you think the documentation could be improved. Writing documentation requires to find a balance between precise and concise explanations, and sometimes we get it definitely wrong! I've been trying to attend the need for detailed examples in #96 for a while, if you feel to jump in and start discussing some ideas, that could be a great contribution indeed!

I hope the explanation helped : )
Regards!

[feliperaul]

@gonzalo-bulnes I also think that the documentation needs to be improved on this point, and it took me a long time until I found this post :)

Your explanation here is already a great start for an improvement !

Anyways, If I understood correctly:

1. Preferred method is to replace a line like before_action :authenticate_user (or before_filter :authenticate_user, as they are the same) with acts_as_token_authentication_handler_for User, since it defaults a fallback to devise, so it doesn't need to be called in the controller anymore.
2. If you want, you can leave both in the controller, as long as (2.1) you declare acts_as_token_authentication_handler_for User before the line where you run your before_filter :authenticate_user and (2.2) you need to pass the fallback: none, as to not create any duplication (because Devise will be called already since it's in the controller).

Even having understood that, I'm having issues. Take a look at my example controller:

class DashboardController < ApplicationController
  layout 'new'
  acts_as_token_authentication_handler_for User, fallback: :devise  

def index 
....

If I try to access /dashboard without the token params, it prompts me for password just like before, which shows that devise fallback is being triggered (I guess).

But if I pass the params in the URL, like /dashboard?user_email=user@app.com&user_token=GNHuhDQzhYmYyszdYZEx, it starts rendering all sorts of weird errors, like undefined method each_with_index' for nil:NilClass`in a line that has the following code (the line lives in the layout template):

<% @last_contacts.each_with_index do |contact, index| %>

This instance variable @last_contacts is generated from a method that is inside ApplicationController, from which, as you saw above, DashboardController derives from:

class ApplicationController < ActionController::Base
  protect_from_forgery
  before_filter :set_locale
  before_filter :set_authorization_user
  layout 'omega'
  before_filter :redirect_to_http
  before_filter :populate_last_contacts_for_menu 

private
  def populate_last_contacts_for_menu
      @last_contacts = Contact.where("user_id" => current_user.id).where('blocked != ? or blocked is null', true).last(10).reverse
  end

Sorry for pasting all this code, but if I remove the @last_contacts loop from the layout, the authentication works fine. This should be an issue tough, since I shouldn't have to modify my views and layouts just for authentication to work properly if with Devise this was not being required.

My versions;
simple_token_authentication (1.10.1)
devise (3.4.1) (which as far as I can see in the gemfile.lock is higher then the dependency).

[feliperaul]

@gonzalo-bulnes The exception is happening because current_user is not set. I read #86 but it didn't help.

I can log in with Devise fine, it's a Rails app running in production for more then one year.

[feliperaul]

@gonzalo-bulnes
Just a followup (because I think as the gem curator you are interested in knowing bugs that happen :)

My colleague, checking out at the exactly same branch as I am, is not even being able to try to debug our issue.

As soon as he tries to open any view, he gets an exception from within a view that is deprecated and shouldn't even be called.

This is so bizarre I had him to open a VNC session with me so I could believe he was not on a different branch.

--- Edit: this was our fault. We were actually using 2 different controllers, and one of the controllers had fallback: none and didn't call Devise.

Anyways, the bug persists: current_user is defined within the Dashboard Controller and also it's layout, but inside ApplicationController it is nil!

[feliperaul]

Update: this might be related (I know it's another gem, but it's a gem that supposedly does that same as this without being simple at all), but I still couldn't solve it (I tried setting devise_for :users) outside any scope in routes.rb

lynndylanhurley/devise_token_auth#28

[gonzalo-bulnes]

Hi @feliperaul,

Starting by your first comment about the authenticate_user callback:

1. Preferred method is to replace a line like before_action :authenticate_user (or before_filter :authenticate_user, as they are the same) with acts_as_token_authentication_handler_for User, since it defaults a fallback to devise, so it doesn't need to be called in the controller anymore.
2. If you want, you can leave both in the controller, as long as (2.1) you declare acts_as_token_authentication_handler_for User before the line where you run your before_filter :authenticate_user and (2.2) you need to pass the fallback: none, as to not create any duplication (because Devise will be called already since it's in the controller).

1. Yes.
2. Yes, it's probably pointless because that's why the fallback option is for, but you can do it.

Now to discuss the current_user issue - which is definitely an interesting one - let's move to #86 to keep the topics separated. (I'll quote your comments there.)