Can't use 'authentication-token' generated by custom login code

[booherbg]

Hi everyone. I've lost most of my day to this bug and it's driving me crazy. I've finally nailed down what does and doesn't work, but I can't make heads or tails of it.

In short, I have an endpoint that logs a user in via an api request. This endpoint isn't anything fancy, but it exists so that I can still have CSRF enabled globally, but have a single endpoint that ignores CSRF for the sole purpose of logging in and getting an authentication-token. Once we have the token, we can use it for other API endpoints without triggering CSRF violations. I like this approach because I can still keep CSRF enabled but provide a JSON api as well.

The issue seems to be that any authentication token returned from my custom login function doesn't actually work. However, any authentication token generated from the flask shell (user.get_auth_token()) or from a regular session login work just fine.

I'm confused because at the end of the day, my code is literally just calling user.get_auth_token() and returning it, but the seemingly valid token gets rejected by future api requests. It's really weird.

Here's my custom login code (that works other than returning a broken auth token):

from flask import request, Blueprint, jsonify
from flask_security.utils import verify_and_update_password, login_user
from ..models.User import User
from ..schemas.UserSchema import UserSchema # Marshmallow Schema

# ... blueprint boilerplate ...

# A login just for json -- requires no CSRF token. This is great because
# it allows us to login with just username/password to get an auth token,
# but still keep CSRF support for the rest of the site!
@bp.route('/api/login', methods=['POST'])
def login():
   content = request.json
   email = content.get('email', None)
   password = content.get('password', None)

   # Fail before DB lookup if either field is missing
   if ((email is None) or (password is None) or (len(email) < 3)):
      return (
         jsonify({
            'error': 'Email and Password fields are required'
            }), 400)

   # Find the user (try both email and username fields)
   user = User.query.filter_by(email=email).first()
   if (user is None):
      user = User.query.filter_by(username=email).first()

   if (user and verify_and_update_password(password, user)):
      login_user(user) # NOTE: disabling this does not fix the bug

      # auth token is dangerous so it doesnt live in schema... this is
      # the only time we ever expose it to the client
      authentication_token = user.get_auth_token()

      schema = UserSchema(only=('id', 'username', 'authentication_token', 'email'))
      data = schema.dump(user).data
      data['authentication_token'] = authentication_token
      return jsonify(data)
   else:
      return (jsonify({
         'error': 'Email/Username or Password are not correct'
         }), 401)

Now that I've reviewed this, maybe it has to do with the verify_and_update_password call? I'm using that to verify that the password provided is correct. If this is the culprit, maybe there's a better way? This is the only function I've found that easily lets me verify that a password is correct, but perhaps a side effect of this is somehow invalidating the auth token (even though it's generated immediately after the function call, so you would think that it would generate a valid token!).

[booherbg]

Confirmed: Removing the verify_and_update_password call results in a valid token. Also, what's weird is that if this function is corrupting calls to get_auth_token() I would expect it to corrupt all future calls, not just the immediate next call. It's also worth noting that all previous valid tokens work just fine. In other words, the call to verify_and_update_password seems to corrupt ONLY the token generated in my login code. Opening up flask shell and running get_auth_token() after the request is complete works just fine (generates a valid token that works with the api).

This is really goofy.

[booherbg]

Futher confirmed. The following is broken:

$ flask shell
>>> from flask_security.utils import verify_and_update_password
>>> user = User.query.first()
>>> verify_and_update_password('password', user)
True
>>> user.get_auth_token()
'... token ...' <------- this thing is broken!
>>> user.get_auth_token() 
'... token ...' <------- this thing is broken too!
>>> exit()
$ flask shell
>>> user = User.query.first()
>>> user.get_auth_token()
' ... token ... ' <------- this token works just fine!

I'm going to investigate other ways of confirming the password (like using hash_password) but for now I think this is definitely a bug. Maybe something to do with SQLAlchemy sessions?

[jwag956]

First - what version are you on 3.0.0? or directly from develop branch?

so verify_and_update_password will only modify the user/password if the password was hashed with a 'obsolete' hash. It calls _datastore.put(user) to store that new password. However, it isn't clear what would commit that change. Try adding a datastore.commit() after returning from verify_and_update_password() (and you might want to verify that in fact it is changing the password - it should just do that once unless some other config is wonky).

[booherbg]

Hi @jwag956 -- thanks for the quick response. I just confirmed that if I input a 'wrong' password, get_auth_token does not return a broken commit. Yes, using 3.0.0.

Great tip! I can confirm that Session.commit() does indeed unbork the token generation.

I imagine that switching to a simple verify_password('password', u.password) should suffice for now (since I don't actually need to update the password, just verify it). That's probably what I should have done instead anyway. I can confirm that verify_password does not corrupt the session's ability to generate tokens.

[booherbg]

Maybe this is a separate discussion: But what's the best way to verify that a password is correct? Is verify_password the best practice?

[booherbg]

@jwag956 -- some more info. While playing around with verify_password I realized that my User.password field was still stored in plaintext. I had created these test users with create_user which (according to other documentation that I've found @ #136 does not hash by default) but did not hash the password. I was apparently not saving hashed passwords for these test users... but after hashing the password on creationit seems to have fixed the issue. In other words, now that I'm storing hashed passwords with the user on creation, it seems that verify_and_update_password does not break the token generation any more.

I guess this is good for future documentation and for now I'm able to move forward. In Summary:

from flask_security.utils import hash_password
app.UserDataStore.create_user(email='user@example.com', password=hash_password('password'))
app.Session.commit()

# Now... the following will work:
from flask_security.utils import verify_and_update_password
u = User.query.filter_by(email='user@example.com').first()
verify_and_update_password('password', u)
# > True
token = u.get_auth_token() # This now returns a valid token!

In the end I'll just use verify_password but I'm satisfied with the closure here.

[booherbg]

For anyone who is googling this later: the following code works great as long as the passwords are hashed (which is true with register_user but not necessarily create_user). It's a great way to keep CSRF enabled but providing a login mechanism via a JSON API, Flask, and Flask-Security. All of the other solutions that I've found involve either throwing out Flask-Security in favor of JWT or disabling CSRF protection all together (which I find unacceptable). The reason this works is because once you have an authentication-token you can use the Flask-Security decorators without CSRF fields, but in order to get the authentication-token you need to hit the Flask-Security login endpoint, which still requires CSRF fields in the request. This custom login endpoint fixes that chicken-and-the-egg situation while keeping all security features intact.

from flask import request, Blueprint, jsonify
from flask_security.utils import verify_password, login_user
from ..models.User import User
from ..schemas.UserSchema import UserSchema # Marshmallow Schema

# ... blueprint boilerplate ...

# A login just for json -- requires no CSRF token. This is great because
# it allows us to login with just username/password to get an auth token,
# but still keep CSRF support for the rest of the site!
@bp.route('/api/login', methods=['POST'])
def login():
   content = request.json
   email = content.get('email', None)
   password = content.get('password', None)

   # Fail before DB lookup if either field is missing
   if ((email is None) or (password is None) or (len(email) < 3)):
      return (
         jsonify({
            'error': 'Email and Password fields are required'
            }), 400)

   # Find the user (try both email and username fields)
   user = User.query.filter_by(email=email).first()
   if (user is None):
      user = User.query.filter_by(username=email).first()

   if (user and verify_password(password, user.password)):
      login_user(user) # updates trackable fields

      # auth token is dangerous so it doesnt live in schema... this is
      # the only time we ever expose it to the client
      authentication_token = user.get_auth_token()

      schema = UserSchema(only=('id', 'username', 'authentication_token', 'email'))
      data = schema.dump(user).data
      data['authentication_token'] = authentication_token
      return jsonify(data)
   else:
      return (jsonify({
         'error': 'Email/Username or Password are not correct'
         }), 401)

[booherbg]

Closing this issue because, while technically it is a bug, it's an extreme corner case that can be mitigated either by using verify_password OR just making sure you hash your passwords (as you should be) for manually created users.

[jwag956]

Thanks for the detailed analysis - glade you worked through things - makes sense now that you created a user and stored an unencrypted password - so the first time you tried to use it - it tried to 'update' it to a real hashed password - but w/o a DB commit the new password didn't stick. I see a few 'bugs' here - first - create_user really should create a hashed password using the configured hash algorithms. Second, verify_and_update_password should document that a DB commit might be required. (If you look at the normal login() view - it does in fact call after_this_request(_commit)