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

Give a dictionary to create_access_token #11

Closed
laranicolas opened this issue Oct 16, 2016 · 10 comments
Closed

Give a dictionary to create_access_token #11

laranicolas opened this issue Oct 16, 2016 · 10 comments

Comments

@laranicolas
Copy link

Hey, would be nice to pass a dictionary value when I'm creating token, so I can use in add_claims_to_access_token and I don't have to find again to database using identity.

LOGIN: Here I have to find on database.

@custom_api.route('/auth', methods=['POST'])
def login():
    options = {
        'body': None,
        'status_code': 200
    }

    username = request.json.get('username', None)
    password = request.json.get('password', None)

    controller = Controller().query.filter_by(uuid=username).first()
    if controller and not safe_str_cmp(controller.jwt.encode('utf-8'), password.encode('utf-8')):
        raise NotAuthorizedException()

    options['body'] = {'access_token': create_access_token(controller)}
    return __build_response_msg(options=options)

If not I have to make find twice:

@jwt.user_claims_loader
def add_claims_to_access_token(identity):
    controller = Controller().query.filter_by(uuid=identity).first()
    return {
        'id': controller.id,
        'role': controller.role
    }

Would be nice to do something like that so I can make a search less:

@jwt.user_claims_loader
def add_claims_to_access_token(identity):
    # I can use every parameter from identity dictionary
    return {
        'id': identity.id,
        'role': identity.role
    }
@vimalloc
Copy link
Owner

vimalloc commented Oct 16, 2016

The identity itself can be anything that is json serializable, so you wanted you could have the identity be:

identity = {
    'id': identity.id,
    'role': identity.role
}
access_token = create_access_token(identity=identity)

Although, that may be a little confusing as to what is going on at first glance. The problem with passing an object directly to the create_access_token is I'm not making any assumptions about what the object is (dict, sqlalchemy, string, etc) or what field should make up the identity (username, id, uuid, etc). I want to keep it this way, to not lock users into a single way of doing things, which caused a bunch of issues for flask-jwt. I suppose I could do something along the line of passing any object to the create_access_token method, pass that same object to the user_claims_loader endpoint, and supply an option function to the create access token method that tells it how to get the identity out of the current object. Perhaps something along the lines of:

# This syntax probably isn't valid, but hopefully you get the idea
controller = Controller().query.filter_by(uuid=identity).first()
create_access_token(identity_object=controller, identity_lookup=lambda c: c.username)

...

@jwt.user_claims_loader
def add_claims(identity_object):
return {
    'id': identity_object.id,
    'role': identity_object.role
}

Thoughts?

@laranicolas
Copy link
Author

Yes I think could be a good solution. So you will use identity_lookup to generate the access token and identity variable to use as a transition variable between create_access_token and add_claims method.

@vimalloc
Copy link
Owner

Yeah, exactly what I was thinking. Ok, good. I'll working on getting something like this setup soon. Thanks for the feedback :)

@laranicolas
Copy link
Author

Yes I would implementing as you suggest, considering identity_lookup as first parameter (obligatory) and identity_object as second parameter (optional).

Thanks!

@vimalloc
Copy link
Owner

I opted to leave it as just identity instead of identity_object, and added the identity_lookup as an optional kwarg to create_access_token(). Should be live with pip now on the new 0.0.7 release.

Documentation here: http://flask-jwt-extended.readthedocs.io/en/latest/tokens_from_complex_object.html

@laranicolas
Copy link
Author

@vimalloc great work and well documented!

I have a doubt regarding parameters, why identity_lookup it is optional, would not be obligatory? (I understand that is the one you use to create the token) and identity param would be the optional we can use as a transition parameter.

@vimalloc
Copy link
Owner

I opted to do it this way as I think it makes more sense if you aren't using the user_claims_loader. In this case, the identity can just be a username, userid, whatever, without needing to pass in a function to get the identity from that object (as the object itself is already the identity).

I wanted to use a single identity argument to keep the code simplier, instead of having to figure out what to do if someone passed in an identity and an identity_object. So just one place to pass in an identity, and then an optional argument to return data that is json serializable from that identity.

Perhaps the names for those could be better, but it is what made most sense to me as I was working on this. I'll think about making a more clear name in the future if this is confusing.

Cheers

@vimalloc vimalloc reopened this Oct 21, 2016
@vimalloc
Copy link
Owner

After using it this way, I am agreeing that I don't really like the API as it stands. I'm about to push 0.0.8 which will try this a bit differently. Instead of having additional options with the create_access_token function, I'm creating a new jwt loader function which lets you specify how it should act on a more universal level. Something like:

class UserObject:
    def __init__(self, username, roles):
        self.username = username
        self.roles = roles

@jwt.user_claims_loader
def add_claims_to_access_token(user):
    return {'roles': user.roles}

@jwt.user_identity_loader
def user_identity_lookup(user):
    return user.username

@app.route('/login', methods=['POST'])
def login():
    username = request.json.get('username', None)
    password = request.json.get('password', None)
    if username != 'test' and password != 'test':
        return jsonify({"msg": "Bad username or password"}), 401

    user = UserObject(username='test', roles=['foo', 'bar'])

    access_token = create_access_token(identity=user)
    ret = {'access_token': access_token}
    return jsonify(ret), 200

This will be going out soon, and will be a breaking change; the identity_lookup kwarg is no longer in the code. Sorry for the breakage there, but hopefully this will be better to use going forward. Let me know if you have any feedback with this approach.

Cheers

@laranicolas
Copy link
Author

I'm seeing as a complex way to do a simple thing, would be better to manage from one place, but you already took the decision.

Cheers!

@vimalloc
Copy link
Owner

Awesome, it sounds like we are on the same page then :) I'm going to go ahead and close this issue again, I think we should be good now. But if you have any feedback after using it this way for a bit let me know, I would love to hear it.

Thanks!

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

2 participants