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

BUG: config.enable_standard_devise_support = true returns wrong current_user #1060

Open
KelseyDH opened this issue Jan 8, 2018 · 5 comments

Comments

@KelseyDH
Copy link

KelseyDH commented Jan 8, 2018

  • devise_token_auth [0.1.42]
  • Ruby [2.4.2]
  • Rails [5.1.4]
  • relevant Gemfile.lock
    devise (4.3.0)
      bcrypt (~> 3.0)
      orm_adapter (~> 0.1)
      railties (>= 4.1.0, < 5.2)
      responders
      warden (~> 1.2.3)
    devise-i18n (1.5.0)
      devise (>= 3.4)
    devise_token_auth (0.1.42)
      devise (> 3.5.2, <= 4.3)
      rails (< 6)
  • Request and response headers:
    Described below.

  • Rails Stacktrace: this can be found in the log/development.log of your API.

  • Environmental Info: My app uses both devise and devise_token_auth, which I achieve by subclassing ApplicationController into ApiController and ApplicationController. One inheriting from ApplicationController::Base the other ApplicationController::API

  • Routes: I'm using both devise and devise_token_auth

  namespace :api do |api|
    mount_devise_token_auth_for 'User', at: 'auth'
  end

  as :user do
    # post 'users/password/change', to: 'users/passwords#change', as: 'change_user_password'
    patch 'users/password/change', to: 'users/passwords#change', as: 'change_user_password'
    get 'users/password/check-email', to: 'users/passwords#check_email', as: 'check_email_user_password'
    get 'users/password/reset-success', to: 'users/passwords#reset_success', as: 'reset_success_user_password'
  end


  devise_for :users, skip: :registrations, controllers: {
    confirmations:      'users/confirmations',
    passwords:          'users/passwords',
    # omniauth_callbacks: 'web/users/omniauth_callbacks',
    # registrations:      'users/registrations',
    sessions:           'users/sessions',
  }


  • Gems: Using devise web. Also using paperTrail gem, which writes a copy of the Users table over to UserVersion.

  • Custom Overrides: I'm (trying) to namespace both devise and devise_token_auth into separate controllers. But I have run into problems completely doing this (explained below).

  • Custom Frontend: iPhone & Android native clients, Postman for testing

Bug setup

In my app, my API consumers can switch between child and parent users, with the Bearer token uid / client / access-token being the mechanism for how my API consumers can switch between users.

However they were complaining about getting back incorrect data, with data from the wrong user getting sent back to them. After investigating I found their concerns to be true, with devise_token_auth returning back incorrect users based on the headers it is sent.

To replicate. I created two users with the same password:

user 1: [email protected]
user 2: [email protected]

First, log in with user 1:

GET api/auth/sign_in
{
	"email": "[email protected]", "password": "That1234"
}

receiving correctly back headers corresponding to uid = [email protected] / user 1:

access-token →z9fS0CuMUZk9_F1FpRY5Tw
cache-control →max-age=0, private, must-revalidate
client →AfYoRq09GaCmATnzAqjAgQ
content-type →application/json; charset=utf-8
etag →W/"1edac4bb16fda94888d87a447cbc0834"
expiry →1516659606
token-type →Bearer
transfer-encoding →chunked
uid →[email protected]
x-content-type-options →nosniff
x-frame-options →SAMEORIGIN
x-request-id →3b1f7b69-03aa-4d54-9911-0aca229d195a
x-runtime →0.217444
x-xss-protection →1; mode=block

Next, log in with user 2:

GET api/auth/sign_in
{
	"email": "[email protected]", "password": "That1234"
}

receiving correctly back headers corresponding to uid = [email protected] / user 2:

access-token →4jktXiC-jDG-yMRUboUL2A
cache-control →max-age=0, private, must-revalidate
client →GV1hagOF6CRWIZkvzBHtrA
content-type →application/json; charset=utf-8
etag →W/"aae490494e2e72440f4f0c8273f3dfaf"
expiry →1516660022
token-type →Bearer
transfer-encoding →chunked
uid →[email protected]
x-content-type-options →nosniff
x-frame-options →SAMEORIGIN
x-request-id →ee93f722-3a71-4f9b-bfdf-2408e1d9cec1
x-runtime →0.223127
x-xss-protection →1; mode=block

Bug

Now here's where the bug happens. When I copy the headers for these two separate users into two separate requests and hit an app endpoint requiring authentication. When config.enable_standard_devise_support is true the api returns back the incorrect current_user (confirmed with byebug), always defaulting to the first user that hit the endpoint (user 1), instead of user 2.

As proof, attached is a screenshot where you can see [email protected] (user 2) sending a token request and getting a response back with authentication headers meant for [email protected] (user 1):

screen shot 2018-01-08 at 3 20 48 pm

Restarting the server had no effect on the Postman requests.

Fix for now

When I changed config.enable_standard_devise_support = false the problem went away. Consequently I strongly suspect there is an issue with DeviseTokenAuth.enable_standard_devise_support, such as in the way it gets invoked within set_user_by_token:

  def set_user_by_token(mapping=nil)
    # determine target authentication class
    rc = resource_class(mapping)

    # no default user defined
    return unless rc

    # gets the headers names, which was set in the initialize file
    uid_name = DeviseTokenAuth.headers_names[:'uid']
    access_token_name = DeviseTokenAuth.headers_names[:'access-token']
    client_name = DeviseTokenAuth.headers_names[:'client']

    # parse header for values necessary for authentication
    uid        = request.headers[uid_name] || params[uid_name]
    @token     ||= request.headers[access_token_name] || params[access_token_name]
    @client_id ||= request.headers[client_name] || params[client_name]

    # client_id isn't required, set to 'default' if absent
    @client_id ||= 'default'

    # check for an existing user, authenticated via warden/devise, if enabled
    if DeviseTokenAuth.enable_standard_devise_support
      devise_warden_user = warden.user(rc.to_s.underscore.to_sym)
      if devise_warden_user && devise_warden_user.tokens[@client_id].nil?
        @used_auth_by_token = false
        @resource = devise_warden_user
        @resource.create_new_auth_token
      end
    end

...
@zachfeldman
Copy link
Contributor

@KelseyDH thanks so much for your report! Seems like a pretty complicated setup. Do you think you could solve this through code/a pull request? If so we'd welcome it!

@KelseyDH
Copy link
Author

KelseyDH commented Jan 10, 2018

@zachfeldman I definitely have a hacky setup for my routes, which reflects trying to support a classic devise web app endpoint page for password resets, within a Rails Api-only app using devise_token_auth for everything else. It's not an arrangement I'm all that happy with.

Getting the two to play together has been a tremendous challenge, one that I haven't still completely resolved. E.g. one current problem with my setup is that I haven't been completely successful in subclassing ApplicationController into separate Rails web and Rails Api endpoints for devise and devise_token_auth. (I.e. so that ApplicationController < ApplicationController::Base for web uses vanilla devise, while ApiController < ApplicationController::Api for Rails API uses only devise_token_auth). Currently my devise_token_auth controller /auth route endpoints still hit ApplicationController::Base (meaning it engages with Rails session, cookies and CSRF protections intended for my devise password reset web app), rather than these routes hitting my desired Rails Api-specific controller of ApiController < ApplicationController::Api.

(Any advice on how to properly subclass devise_token_auth to point to my ApiController much appreciated!)

This quirk I suspect, which is a combination of devise_token_auth hitting ActionController::Base with CSRF/sessions/cookies, along with DeviseTokenAuth.enable_standard_devise_support = true invoking warden methods that were built with traditional devise web apps with session/cookies in mind, could be behind what's causing this bug if I had to speculate.

@maltem-za
Copy link

@KelseyDH You may want to check out this issue (see my comment at the bottom), its related issues, and also this one.

We had the same symptoms that you are describing, and turning off config.enable_standard_devise_support also "fixed" it for us, but it wasn't the root cause.

@sujaysudheenda
Copy link

sujaysudheenda commented Oct 6, 2018

I recently started setting up devise_token_auth (latest version as of 2018-10-05) for an existing application which has devise. I can confirm what @KelseyDH has mentioned in above comment still exists. I am not using any other devise_token_auth UI libraries.

Consequently I strongly suspect there is an issue with DeviseTokenAuth.enable_standard_devise_support, such as in the way it gets invoked within set_user_by_token

In addition to wrong current_user thingy, When I try to create/destroy sessions via web (which should go through devise), I see following error (whenever a user tries to sign-in or sign-out):

ArgumentError in Devise::SessionsController#destroy

devise (4.3.0) app/controllers/devise_controller.rb:47:in `resource_class'
devise_token_auth (0.2.0) app/controllers/devise_token_auth/concerns/set_user_by_token.rb:41:in `set_user_by_token'

The line 41 in set_user_by_token, corresponds to the invocation of resource_class by providing the mapping argument.

https://github.com/plataformatec/devise/blob/715192a7709a4c02127afb067e66230061b82cf2/app/controllers/devise_controller.rb#L49

which actually doesn't take any argument.

Update
using different controllers for api & web solved few of these issues but the caching of current_user is not resolved WRT API requests, Although the web side of things seems to work without any such problem. Its only for when I try to hit via postman I get back data based on cached current_user

Update 2

Working more on this, I found that the issue might be because of session being persisted when a api request is submitted. Due to which the next time even if I don't pass client, uid, access-token it still authenticates the user from the existing session


Side note: Thank you for developing this gem.

@aguynamedben
Copy link

aguynamedben commented Mar 17, 2021

I have seen a similar thing. I use Paw to make HTTP requests and it has these settings that help prove what's going on:
image

If I remove the 5 devise_token_auth HTTP headers and leave "Automatically Send Cookies", the user's session is still used to authenticate them. This was surprising to me as I assumed if I didn't have the headers, devise_token_auth would barf. If I uncheck "Automatically Send Cookies", the 5 devise_token_auth HTTP headers are required.

I found a similar behavior in one of my clients that by default stores cookies. Even if tokens became invalidated, the session would be used to "revive" the client.

I'm also seeing a really annoying issue with config.enable_standard_devise_support = true where session-based XHR request undesirably rotate devise_token_auth tokens on each request. I have config.max_number_of_devices = 25 so after 25 session-based XHR requests, the token another client is using may be rotated out to be invalid. So making 25 API requests using session from one client cause other clients to become logged out.

Fun stuff!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants