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

401 unauthorized errors for Session#destroy action #76

Closed
shedd opened this issue Jun 23, 2014 · 10 comments
Closed

401 unauthorized errors for Session#destroy action #76

shedd opened this issue Jun 23, 2014 · 10 comments
Labels
question When closed, this issue will become part of the FAQ.

Comments

@shedd
Copy link

shedd commented Jun 23, 2014

I'm running into an issue where Login via a JSON API using Simple Token Auth works, token authentication works, but I am always getting 401 authentication issues with the logout call.

I'm working on a couple of Rspec tests to demonstrate the issue. The authentication via token works every where else (test suite and in real life). It's just logout where I constantly get a 401.

I have Devise configured to accept logout via GET (rather than DELETE). Otherwise, my issue sounds similar to this recent, unanswered thread: http://stackoverflow.com/questions/24092791/ruby-on-rails-curl-delete-token-authentication

Does this sound like a Devise issue or a Simple Token Auth issue?

Two failing test cases are below.

Any and all thoughts would be greatly appreciated!

app/controllers/api/v1/api_controller.rb

class Api::V1::ApiController < ApplicationController
  acts_as_token_authentication_handler_for User, fallback_to_devise: false
  before_action :authenticate_user!
  skip_filter :authenticate_user!, only: [:connection_test]

  respond_to :json

  protect_from_forgery with: :null_session
end

app/controllers/api/v1/sessions_controller.rb

class Api::V1::SessionsController < Devise::RegistrationsController
  prepend_before_filter :require_no_authentication, only: [:create]
  skip_filter :authenticate_entity_from_token!, only: [:create]
  skip_filter :authenticate_user!, only: [:create]
  before_action :ensure_params_exist
  skip_before_action :verify_authenticity_token

  def create
    build_resource
    resource = User.find_for_database_authentication(
      email: params[:user][:email]
    )
    return invalid_login_attempt unless resource

    if resource.valid_password?(params[:user][:password])
      sign_in('user', resource)
      render json: {
        success: true,
        auth_token: resource.authentication_token,
        email: resource.email,
        user_id: resource.id,
        restaurant_id: resource.restaurant.blank? ? nil : resource.restaurant.id,
        driver_id: resource.driver.blank? ? nil : resource.driver.id
      }
      return
    end
    invalid_login_attempt
  end

  def destroy
    sign_out(resource_name)
  end

  protected

  def ensure_params_exist
    return unless params[:user].blank?
    render status: 422, json: {
      success: false,
      message: 'missing user parameter'
    }
  end

  def invalid_login_attempt
    warden.custom_failure!
    render status: 401, json: {
      success: false,
      message: 'Error with your login or password'
    }
  end
end

config/routes.rb

  devise_for :users, path: '', path_names: { sign_in: 'login', sign_out: 'logout', sign_up: 'register', edit: 'account' }

  namespace :api do
    namespace :v1, constraints: { format: 'json' }, defaults: { format: :json } do
      devise_for :users, path: 'users', path_names: { sign_in: 'login', sign_out: 'logout', sign_up: 'register', edit: 'account' }
      ...
    end
  end

spec/controllers/api/v1/sessions_controller_spec.rb

require 'spec_helper'

describe Api::V1::SessionsController do
  before(:each) { @request.env["devise.mapping"] = Devise.mappings[:user] }
  ...

  describe 'GET logout' do
    context 'for drivers' do
      it 'logs out drivers' do
        request.accept = 'application/json'

        resource = create(:driver)
        @user = resource.user

        # setup the api authentication headers
        request.headers['X-User-Email'] = @user.email
        request.headers['X-User-Token'] = @user.authentication_token

        get :destroy
        expect(response.status).to eq(200)
      end
    end

    context 'for restaurants' do
      it 'logs out restaurants' do
        request.accept = 'application/json'

        resource = create(:restaurant)
        @user = resource.user

        # setup the api authentication headers
        request.headers['X-User-Email'] = @user.email
        request.headers['X-User-Token'] = @user.authentication_token

        get :destroy
        expect(response.status).to eq(200)
      end
    end
  end
end
@gonzalo-bulnes
Copy link
Owner

Hi @shedd

About the question on SO that you referred to:

I had no SO account and I'm not authorized by SO to comment just after registering, but I wonder: isn't ukson forgetting to provide the user_email param? Token authentication can't be performed with the only auth_token. (In fact, the point of the José Valim's gist is to explain how to perform that e-mail cross-checking when implementing token authentication.)

You setup is more complex, and I'll take a closer look at it as soon as I can. If you can, try speaking with ukson!

@shedd
Copy link
Author

shedd commented Jun 24, 2014

@gonzalo-bulnes nice spot! Yep, I think you're right about the SO question. I replied giving your tip: http://stackoverflow.com/questions/24092791/ruby-on-rails-curl-delete-token-authentication/24394116#24394116

Let me know if you have any thoughts on what my issue is. I am passing the email and the token and it works everywhere else, just not Session#destroy

Thanks in advance!

@shedd
Copy link
Author

shedd commented Jul 11, 2014

Hey @gonzalo-bulnes - any thoughts on why this isn't working? Thanks so much!

@gonzalo-bulnes
Copy link
Owner

Hi @shedd, I'm sorry I haven't correctly looked at your issue. I'll make my best to do it tomorrow morning. Thanks for your patience!

@shedd
Copy link
Author

shedd commented Nov 4, 2014

@gonzalo-bulnes sorry to pull this back up again, but we're still having issues with this. Just wondering if you have any ideas?

@gonzalo-bulnes
Copy link
Owner

Sorry @shedd, I kind of forgot your issue : /

One question: why do you add the authenticate_entity_from_token! filter? Simple Token Authentication adds authenticate_user_from_token (no bang ! because the fallback is disabled) in you case.

See https://github.com/gonzalo-bulnes/simple_token_authentication/blob/v1.6.0/lib/simple_token_authentication/token_authentication_handler.rb#L118-L125

@gonzalo-bulnes
Copy link
Owner

The namespace could have introduced unexpected behaviour, but since you mention that token authentication is behaving well otherwise, I guess the devise_for path: users option handles it properly.

@shedd
Copy link
Author

shedd commented Nov 4, 2014

@gonzalo-bulnes hey, thanks for the response!

It looks like I had made some changes since when I originally posted this in June.

My API controller now only has:

  acts_as_token_authentication_handler_for User, fallback_to_devise: false
  before_action :authenticate_user!
  skip_filter :authenticate_user!, only: [:connection_test]

And I'm doing this filtering in the Sessions controller:

  prepend_before_filter :require_no_authentication, only: [:create]
  skip_filter :authenticate_entity_from_token!, only: [:create]
  skip_filter :authenticate_user!, only: [:create]
  before_action :ensure_params_exist
  skip_before_action :verify_authenticity_token

I've updated the sample code above.

So I think the confusion about the authenticate_entity_from_token! call should be resolved.

Still getting 401s from my tests, though.

And yes, the namespacing seems to be working fine.

@shedd
Copy link
Author

shedd commented Nov 4, 2014

Ok, I figured out what I was missing.

The sample code that I based this login/logout controller on had this class definition:

class Api::V1::SessionsController < Devise::RegistrationsController

When I was going through this again, I thought that seemed a bit off - using the Registrations controller for login and logout? Amazingly, this actually worked, though - for login anyway.

Looking at the Devise code, I decided to try this:

class Api::V1::SessionsController < Devise::SessionsController

As long as I did a skip_filter :verify_signed_out_user, only: [:destroy] this kind of worked. It no longer threw a 401 error.

But there was still no current_user in the scope of the destroy method.

Then it struck me - it should be class Api::V1::SessionsController < Api::V1::ApiController - inherit from the API controller, where the acts_as_token_authentication_handler_for is setup. No wonder it wasn't authenticated.

Changing that fixed the problem.

Now I have:

class Api::V1::SessionsController < Api::V1::ApiController
  skip_filter :authenticate_entity_from_token!, only: [:create]
  skip_filter :authenticate_user!, only: [:create]
  skip_before_action :verify_authenticity_token

  def create
    # remove the build_resource call here from my earlier example
    ...
  end

  def destroy
    sign_out(current_user)
    head :no_content
  end
end

Sorry, this was my mistake. Thanks for the help!

@shedd shedd closed this as completed Nov 4, 2014
@gonzalo-bulnes
Copy link
Owner

Nice, I'm glad the discussion gave you an occasion to take a different look at the code. Thanks for the details about your setup too! I'm planning to document some interesting use cases, and your experience is very helpful.

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

2 participants