Added a fourth lookup within the json body #173
Conversation
|
|
||
| try: | ||
| encoded_token = request.json.get(token_key, None) | ||
| assert encoded_token |
There was a problem hiding this comment.
I like the control flow here, but assertions can actually be disabled on the python project when running python with the -O flag, so I think this should section should be reworked without the use of assert.
| try: | ||
| encoded_token = request.json.get(token_key, None) | ||
| assert encoded_token | ||
| except (BadRequest, AssertionError): |
There was a problem hiding this comment.
Just curious, what case does the BadRequest get thrown here? I'm guessing it's when the application type is json but valid json could not actually be parsed from the request body?
There was a problem hiding this comment.
Exactly, that's when it would happen.
tests/test_config.py
Outdated
| assert config.jwt_in_query_string is True | ||
| assert config.jwt_in_cookies is True | ||
| assert config.jwt_in_headers is False | ||
| assert config.jwt_in_json is False |
There was a problem hiding this comment.
For the sake of checking all the config stuff here, could you add json to the JWT_TOKEN_LOCATION in the override configs and make that this is set to True here?
| return jsonify(foo='bar') | ||
|
|
||
| return app | ||
|
|
There was a problem hiding this comment.
Could we add one more unit test to this file, this working with the default json keys?
|
|
||
| response = test_client.post('/protected', headers=headers) | ||
| assert response.status_code == 201 | ||
| assert response.get_json() == {'foo': "bar"} |
There was a problem hiding this comment.
Could we add one more unit test in this file as well? https://github.com/vimalloc/flask-jwt-extended/blob/master/tests/test_multiple_token_locations.py
The test_no_jwt_in_request in that test was trying to encompass everything, but that is no longer going to be feasible, and honestly probably wasn't my greatest idea to begin with. I wouldn't worry too much about adding the json test there.
tests/test_json.py
Outdated
| assert response.status_code == 401 | ||
| assert response.get_json() == {'msg': 'Missing "access_token" key in json data.'} | ||
|
|
||
| # Test custom no headers response |
|
This looks great, thanks for working on it! I left some minor comments on a few things, if we can get those resolved I would love to get this merged. Thanks for contributing! 👍 |
|
Comments addressed, let me know of anything else. 👍 |
|
This looks great, thanks for contributing! I'll get a new released pushed out later tonight! 👍 |
|
Released in 3.12.0! |
|
Awesome! |
|
In my REST API, I always encrypt the whole JSON request and response. Is there any chance to make the token readable by Flask JWT Extended? Absolutely after the JSON was decrypted. Currently, I include the token inside the header, so I think it ain't really secure. Thanks. |
|
Why not just use the tried and true https and hsts? |
4 participants
In an application I did recently, a constraint was that the tokens should be part of the request body so I had to implement some stuff.
Thought it could be useful to have that option directly in
flask_jwt_extendedso pushing this to see if it's helpful.