Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to protect the registrations controller through token authentication? #36

Closed
JoshuaNovak919 opened this issue Mar 2, 2014 · 10 comments
Labels
question When closed, this issue will become part of the FAQ.

Comments

@JoshuaNovak919
Copy link

I am trying to override the registration controller so I can respond with JSON. The issue I am having is that authenticate_scope! is called before [:edit, :update, :destroy] and I need the users to be able to modify their profiles. Is there a good way to handle this with a simple_token_authentication method or something?

@austinpray
Copy link

I have a registration controller that looks like this:

class API::RegistrationsController < Devise::RegistrationsController
  skip_before_filter :verify_authenticity_token, :if => Proc.new { |c| c.request.format == 'application/json' }
  skip_before_filter :authenticate_scope!, :only => [:update]
  respond_to :json

  def create
    build_resource(params[:user])
    if resource.save
      if(params[:facebook_id])
        resource.authentications.create(:provider => 'facebook', :uid => params[:facebook_id])
      end
      if resource.active_for_authentication?
        sign_in(resource)
        @success = true
        @user = resource
        return
      else
        expire_session_data_after_sign_in!
        return render :json => {:success => true}
      end
    else
      clean_up_passwords resource
      return render :status => 401, :json => {:errors => resource.errors}
    end
  end

  protected

  def configure_permitted_parameters
    devise_parameter_sanitizer.for(:sign_up) do |u|
      u.permit(:name, :heard_how,
        :email, :password, :password_confirmation)
    end
    devise_parameter_sanitizer.for(:account_update) do |u|
      u.permit(:name,
        :email, :password, :password_confirmation, :current_password)
    end
  end
end

Works great.

@gonzalo-bulnes
Copy link
Owner

Hi @JoshuaNovak919, thanks for editing your question, it's a lot clearer : )

@austinpray Your example allows users to register through JSON requests, but doesn't permit them to edit their registrations, am I wrong?

@JoshuaNovak919 I believe updating registrations using token authentication should be possible, yet probably not out-of-the-box.

Let's think aloud: in the original Devise::RegistrationsController, authenticate_scope! is called as a before_filter. You don't want it to be called. Did you try to move the :edit and :update actions to skip authentication?

class Devise::RegistrationsController < DeviseController
  prepend_before_filter :require_no_authentication, only: [ :new, :create, :edit, :update, :cancel ]
  prepend_before_filter :authenticate_scope!, only: [:destroy]

  # ...

end

(Note: It seems that the require_no_authentication filter does almost nothing if you don't request HTML, I'm not sure if :edit and :update should trigger it at all.)

Of course you don't want those action to be accessible without any authentication, but that's a first step.

The second step would be ensuring that your registrations controller acts_as_token_authentication_handler_for User, and that User is token_authenticatable.

class Devise::RegistrationsController < DeviseController

  prepend_before_filter :require_no_authentication, only: [ :new, :create, :edit, :update, :cancel ]
  prepend_before_filter :authenticate_scope!, only: [:destroy]
  acts_as_token_authentification_handler_for User # please ensure User is token authenticatable

  # ...

end

That should make the token authentication required for all the registrations controller actions. We're a bit closer but that may not be what you want, since it would prevent registration to be created without authenticating, right?

The third step should then be scoping the token authentication to the :update action (you don't need to :edit if using a JSON API, do you?). If skip_before_filter :authenticate_entity_from_token! is possible (point to be verified), then I would give a try to:

class Devise::RegistrationsController < DeviseController

  prepend_before_filter :require_no_authentication, only: [ :new, :create, :edit, :update, :cancel ]
  prepend_before_filter :authenticate_scope!, only: [:destroy]
  acts_as_token_authentification_handler_for User

  # Disable token authentication for all actions
  # See https://github.com/gonzalo-bulnes/simple_token_authentication/blob/master/lib/simple_token_authentication/acts_as_token_authentication_handler.rb#L12-L15
  skip_before_filter :authenticate_entity_from_token!
  skip_before_filter :authenticate_entity!

  # Enable token authentication only for the :update, :destroy actions
  before_filter :authenticate_entity_from_token!, :only => [:update, :destroy]
  before_filter :authenticate_entity!, :only => [:update, :destroy]

  # ...

end

Does anything of this make sense to you? If it does (I hope so), could you give it a try? If it doesn't, could you post some code so we can reproduce your setup?

Regards!

@gonzalo-bulnes
Copy link
Owner

By the way, the previous question about JSON registration is the issue #29. The key idea, I think, was that if you're using the Devise Registerable module, you don't have to worry about token authentication. All your token authenticatable users will see their authentication_tokens created and updated automatically. That makes easy for you to offer a mobile app access to users who registered through the web.

If you want them to register from the mobile app through a JSON API, you need to ensure the Devise::RegistrationsController responds to JSON, and that's what @austinpray did. That particular part has few to do with Simple Token Authentication because creating a new registration requires no authentication.

However, if you also want your users to be able to update their registrations through a JSON API, as @JoshuaNovak919 does (please say me if I'm wrong), making sure the RegistrationsController handles token authentication is necessary, and because some action should be protected and others should not, that's not trivial. That's IMHO the main point of this issue (#36).

@jamespeerless I have edited the title of #29 to make is more discoverable.

@JoshuaNovak919
Copy link
Author

@gonzalo-bulnes Thanks for all your help! You're awesome!! Seems to be working perfectly. 👍

As far as using the :edit action with a JSON API, I guess I could use :show instead, but I've been setting it up to mirror rails' rest setup and i'm quite new to JSON APIs so this may not be handling it the best way.

@gonzalo-bulnes
Copy link
Owner

@JoshuaNovak919 I'm glad my post helped you : D

Once you're sure it works --and if possible--, could you please create a gist with your registrations controller (example)? I'm sure that it would help others and could be a great start to write some documentation about it.

(Incidentally, we could also take a look at the :edit thing about which I'm quite curious.)

Regards!

@ryanjm
Copy link

ryanjm commented Jan 31, 2015

@gonzalo-bulnes Your solution above used to work for me. I'm working on upgrading my project to Rails 4.2.0 and I'm running into an error now. The only thing that has changed is going from version 1.2.0 to 1.7.0. I don't see a changelog so how would that affect this code?

Here is what the top part of my RegistrationController looks like:

class Api::RegistrationsController < Devise::RegistrationsController
  prepend_before_filter :require_no_authentication, only: [ :new, :create, :edit, :update, :cancel ]
  prepend_before_filter :authenticate_scope!, only: [:destroy]
  acts_as_token_authentication_handler_for User

  # Disable token authentication for all actions
  # See https://github.com/gonzalo-bulnes/simple_token_authentication/blob/master/lib/simple_token_authentication/acts_as_token_authentication_handler.rb#L12-L15
  skip_before_filter :authenticate_entity_from_token!
  skip_before_filter :authenticate_entity!

  # Enable token authentication only for the :update, :destroy actions
  before_filter :authenticate_entity_from_token!, :only => [:update, :destroy]
  before_filter :authenticate_entity!, :only => [:update, :destroy]

My Rspec test:

    it "updates attributes of the user" do
      u = create(:user, password: 'helloworld', password_confirmation: 'helloworld')
      binding.pry
      put "/api/users?user_email=#{u.email}&user_token=#{u.authentication_token}", user: { name: "Bob" }
      ...
    end

Without the binding.pry I get:

  1) Api::RegistrationsController update updates attributes of the user
     Failure/Error: put "/api/users?user_email=#{u.email}&user_token=#{u.authentication_token}", user: { name: "Bob" }
     ArgumentError:
       wrong number of arguments (0 for 1)
     # ./spec/requests/api/registrations_controller_spec.rb:32:in `block (3 levels) in <top (required)>'

When I stop inside the method and try to run the put myself I get:

> put "/api/users?user_email=#{u.email}&user_token=#{u.authentication_token}"
ArgumentError: wrong number of arguments (0 for 1)
from /Users/ryanjm/.rbenv/versions/2.2.0/lib/ruby/gems/2.2.0/gems/simple_token_authentication-1.7.0/lib/simple_token_authentication/token_authentication_handler.rb:29:in `authenticate_entity_from_token!'

Any ideas?

@ryanjm
Copy link

ryanjm commented Jan 31, 2015

Alright, looking through the readme it looks like my controller should be simplified to:

  prepend_before_filter :require_no_authentication, only: [ :new, :create, :edit, :update, :cancel ]
  prepend_before_filter :authenticate_scope!, only: [:destroy]
  acts_as_token_authentication_handler_for User, only: [:update, :destroy]

Which kind of works. It just doesn't throw the 401 I'm expecting when the user isn't found. But looking through the code for this gem, I'm not sure if that is what I should be expecting.

@ryanjm
Copy link

ryanjm commented Jan 31, 2015

Alright, for others running into this, I fixed it by adding this to my application controller:

  acts_as_token_authentication_handler_for User, fallback_to_devise: false
  before_filter :validate_user

  def validate_user
    user_signed_in? || throw(:warden, scope: :user)
  end

This comes from this method via @gonzalo-bulnes's comment.

Then in the RegistrationController I have:

  prepend_before_filter :require_no_authentication, only: [ :new, :create, :edit, :update, :cancel ]
  prepend_before_filter :authenticate_scope!, only: [:destroy]
  acts_as_token_authentication_handler_for User, only: [:update, :destroy]
  skip_before_filter :validate_user, only: [:create]

Hopefully this helps someone else.

@gonzalo-bulnes
Copy link
Owner

Hi @ryanjm,

First of all, thanks for your comments.

1. About the controllers behaviour when authentication fails (see those two comments)

It just doesn't throw the 401 I'm expecting when the user isn't found. But looking through the code for this gem, I'm not sure if that is what I should be expecting.

The core feature of the gem is providing an authentication mecanism, that's to say authenticating users when credentials (authentication token, identifier) are present. Beyond that, defining the controllers behaviour when authentication fails is IMO, out of scope. That can be discussed, and I find the @donbobka iniciative absolutely well founded. Besides, that's the way to go to ensure controllers do respond with the expected 401 HTTP errors, and the example you provided is both correct and helpful.

2. About the failing spec example (see this comment)

Yes, an idea: you should never call authenticate_entity_from_token! (nor any "entity" method, those are private methods).
Let's say your controller handles token authentication for User. Then two methods are defined automatically: authenticate_user_from_token and authenticate_user_from_token!. Depending on the value of the fallback option, one, or the other, is set as a callback.

It's that method that you should use when skipping or setting different callbacks (filters). In your example it would be the bang ! one, authenticate_user_from_token!. (Note that it takes no arguments, while private methods do.)

Best regards!

@gonzalo-bulnes
Copy link
Owner

3. About the only, except options

Yours was a good move. Those options were included to avoid the skiping then setting again the callbacks to scope them. In fact, the second point of my previous comment may be much less useful now than it was a few versions ago, but I think it's still good to know how things do work : )

Regards!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question When closed, this issue will become part of the FAQ.
Projects
None yet
Development

No branches or pull requests

4 participants