v0.10.0.rc5 interferes with devise_token_auth #1667
Comments
|
hello, I also use devise (with ember! (ember-simple-auth)) this is how I handle sign ins: class Api::Users::SessionsController < Devise::SessionsController
clear_respond_to
respond_to :json
prepend_before_filter :require_no_authentication, only: [:create]
# We have to write our own login, cause devise isn't
# designed for API Authentication
#
# instead of returning the current user,
# we want to return the auth token.
# the auth token and email will be passed on every request,
# and the user will be authorized based on the validitity
# of the token and email pair.
#
# HTTPS required :-)
def create
resource = User.find_by_email(params[:email])
return invalid_login_attempt unless resource
success = resource.valid_password?(params[:password])
if success
sign_in("user", resource)
return render_success(resource)
end
invalid_login_attempt
end
protected
def invalid_login_attempt
# have to declare custom failure,
# cause warden likes to redirect
warden.custom_failure!
render json: {
error: I18n.t('devise.failure.not_found_in_database')
}, status: 401
end
def render_success(user)
data = {
token: user.authentication_token,
email: user.email,
id: user.id
}
render json: data, status: 201
end
endhere is how I handle getting the current user with each request: def authenticate_user_from_token!
http_authorization = request.env['HTTP_AUTHORIZATION']
return false unless http_authorization
matches = http_authorization.match(/Bearer (.+)\z/)
return false unless matches
token = matches[1]
user = User.find_by_authentication_token(token)
if user
if !user.confirmed?
user.errors.add(:base, I18n.t('devise.failure.unconfirmed'))
else
sign_in user, store: false
end
end
user
endfeel free to poke around here if you want to know more about my setup: https://github.com/NullVoxPopuli/aeonvera/ |
|
@JamesChevalier What's Also, devise isn't very friendly to APIs. :( |
|
Ok, apparently I introduced that serializer in ec5dc49 But that's a logging concern.. weird |
|
In the second block of code within the 'Expected behavior vs actual behavior' section, this is in the log output: I can agree with you on the devise/API note. This devise_token_auth gem that I'm using is supposed to help with that, but - obviously - things aren't going very well. :-P What's odd is that everything works great while using active_model_serializers v0.10.0.rc4 |
|
@JamesChevalier for what it's worth, I'm using master, if you want to borrow my code. :-) |
@bf4 you're the best 😂 |
|
@JamesChevalier what's the status on this? |
|
@NullVoxPopuli Nothing has changed on my end & I haven't received a response to the Issue that I filed with devise_token_auth. I appreciate you sharing your code with me, but (and hopefully I'm not misunderstanding this) I'm pretty tied in to using devise_token_auth at this point & can't take the time to rewrite my authentication strategy. |
|
@JamesChevalier My guess is that it's a change in how the serialization_context or serialization_scope are being used. I see in the source code some instance variable stuff which boils down to: client_name = DeviseTokenAuth.headers_names[:'client']
@client_id = request.headers[client_name] || params[client_name]Maybe you can paste that somewhere or use that to debug? refs:
Or maybe it's this insane line: def is_json_api
return false unless defined?(ActiveModel::Serializer)
return ActiveModel::Serializer.config.adapter == :json_api
end |
|
Thanks for the detective work, @bf4 ! Their code on master isn't what's in their latest gem, though. :-/ This is their released version. The released version of your first reference doesn't use The problem is that the It looks like After a bit of research, I've found that the sign_up flow goes like this:
When using ASM v0.10.0.rc4, the When using ASM v0.10.0.rc5, the The only thing that I have to point to is that self.as_json line in the token_validation_response method. Based on my research & tests so far, it looks like the two versions of ASM behave differently when that line is called. Now I'm about to dive in to see if I can find out what that is... |
|
If you clone ams into a local dir and set that as the path in the gemfile, you can run a git bisect on ams and check for the failure at each commit... Might help |
|
The failing line in |
|
The released version of 0.10.0 does not resolve this issue. |
|
@JamesChevalier, please take a look at this repo: https://github.com/NullVoxPopuli/aeonvera I have AMS + super basic token auth working with devise. |
|
Thanks, @NullVoxPopuli but the issue is between active_model_serializers & devise_token_auth. |
|
Do you need devise_token_auth? Devise token auth doesn't provide much and it seems like it's just preventing you from upgrading AMS |
|
I'm not sure what else we can do-- we've all put time into it-- without prioritizing this over features or performance or bugs with failing testa B mobile phone
|
|
I have another update... The short version is that this actually is working as expected with the latest 0.10.0 version. The longer version is: I read back through the issue, and I was reminded that there's unreleased devise_token_auth code. I decided to try out the latest AMS against different DTA git refs to see if I could get a functional environment that way... I tried the latest commit first, and that succeeded. Since that worked so quickly, I decided I could spend a few minutes trying to pin down which commit fixed things. I tried a number of versions by specifying different refs in the Gemfile, and I couldn't get it to fail again - going all the way back to the released commit. So, I'm guessing that there was something wrong with my initial test of active_model_serializers v0.10.0 which was cleared up in subsequent tests along the way. As of now, this is working. Here are the versions in use for this functional environment:
Sorry for the trouble this morning & thanks for all your help along the way! |
|
glad it works now!!! yay!!! 💯 |
|
So much awesome that you shared this back! B mobile phone
|
|
I spoke too soon. With active_model_serializers version 0.10.0, the API response body is correct but the headers lack the The only other difference that I've spotted so far is that v0.10.0 includes The headers are allowed: headers for a successful signin requestheaders for a failed signin requestI'm not sure that this new discovery warrants reopening the issue, though. The API request into devise_token_auth's space only includes I just wanted to pass along the details here, in case anyone has the same problem & comes across this issue in their initial debugging process... |
|
are you wanting the access token to be included in every response? or every request? |
|
Yeah There are two places in the docs that mention this behavior:
|
|
have you thought about using JWT? it sounds like you may want to use that? along with knock? https://github.com/nsarno/knock |
|
Having exactly the same issue. I had to downgrade AMS to |
|
@mustela I believe this is resolved #1667 (comment) |
|
It may also be worth keeping in mind that https://github.com/lynndylanhurley/devise_token_auth is regularly having issues with AMS due the gem's design... might be worth considering improving the code there or using different gem |
|
it's much easier to write your own token auth (As I have pasted in an earlier comment). It's easier to understand, customize, and upgrade. |
|
FYI, for anyone who ends up here on this issue: |
|
@JamesChevalier Any configuration options required? I've got a ng-token-auth authenticating with devise_token_auth, and it always gives: |
|
@alfmatos |
|
@bf4 It seems I was getting a hash whenever the credentials failed (invalid). It created an empty session object and tried to serialise that. Not sure if that's an expected behaviour. This was related wit My problem was having confirmable active. When I added the confirmed_at, it started working. Since then I've added a namespace defaulting to json to make sure it processes the requests correctly: |
|
I was having a similar issue and after many tweaks I found the problem that was affecting my project. Just like many of you have said, my response headers didn't have the access-token and client The server was rolling back the transaction because a required attribute was missing. After I sort that out, everything worked as expected. I felt kind of stupid after realizing it was such a minor thing. Currently using: |
|
Did anyone experienced that the API completely hangs util the request timeout after adding |
|
I'd recommend just stop using `devise_token_auth`. You can probably do a
better job yourself.
|
|
@bf4 yeah I downgraded I realized that after started using I'm not saying the author is a bad coder, I'm just warning future adopters to consider this. |
Expected behavior vs actual behavior
With version 0.10.0.rc4 ASM is not serializing the response from devise_token_auth, so sign_in succeeds:
With version 0.10.0.rc5 ASM is attempting to serialize the response from devise_token_auth, so sign_in fails:
Steps to reproduce
(e.g., detailed walkthrough, runnable script, example application)
The environment is a Rails app that uses devise_token_auth for authentication, and active_model_serializers to serialize responses. The error occurs when the rc5 version of active_model_serializers is in place, and it does not appear when the rc4 version is in place.
I'm not sure if the fix will ultimately reside in devise_token_auth or here.
I've included more information at the bottom.
Environment
ActiveModelSerializers Version (commit ref if not on tag):
0.10.0.rc5
Output of
ruby -e "puts RUBY_DESCRIPTION":ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-darwin14]
OS Type & Version:
Mac OS X 10.10.5
Integrated application and version (e.g., Rails, Grape, etc):
Rails 4.2.6
devise 3.5.6
devise_token_auth 0.1.37 (code)
Backtrace
(e.g., provide any applicable backtraces from your application)
Additonal helpful information
(e.g., Gemfile.lock, configurations, PR containing a failing test, git bisect results)
I do have a serializer for User, but it looks like devise_token_auth isn't using it. The rc5 output at the top of this issue included this line:
[active_model_serializers] Rendered ActiveModel::Serializer::Null with Hash (0.14ms)... TheNullin there seems odd.With rc4, the
client_idis making it into the build_auth_header method. With rc5, it is not (it exists asdefault).I've been trying to trace back that
client_idvalue, but I haven't been successful in finding where it gets unset. I'm going to file an issue with devise_token_auth to see if they can explain it.If the fix ends up being that devise_token_auth has to avoid running its response through active_model_serializers, is there a way to do that?
The text was updated successfully, but these errors were encountered: