Invalid JWT errors not thrown for decorated optional routes

[dmulter]

I'm migrating from flask-jwt and make extensive use of decorated required and optional routes. My tests also have extensive coverage for invalid JWT cases. I've confirmed that invalid JWTs are handled correctly for jwt_required() routes, but not fully for jwt_required(optional=True) routes. Note that I use "JWT" as my prefix instead of "Bearer". I've also seen that flask-jwt-extended test cases do not cover these types of cases for optional routes.

Specifically these routes are allowed access for these invalid tokens. It is obviously important that any type of error for a submitted token (in any location) is rejected even when it is optional.

• "JWT "
• "JWT xxxx"
• "JWT xxxx xxxx"
• "JXX"

[vimalloc-mavenlink]

You're doing something wrong, this extension absolutely handles optional situations correctly. Maybe you forgot to set the JWT_HEADER_TYPE to JWT instead of the default Bearer?

Here is an minimal example showing that non-valid tokens are still kicked out of an optionally protected endpoint:

from flask import Flask
from flask import jsonify

from flask_jwt_extended import create_access_token
from flask_jwt_extended import jwt_required
from flask_jwt_extended import JWTManager

app = Flask(__name__)
app.config["JWT_SECRET_KEY"] = "super-secret"
app.config["JWT_HEADER_TYPE"] = "JWT"
jwt = JWTManager(app)

@app.route("/login", methods=["POST"])
def login():
    return jsonify(access_token=create_access_token(identity='example'))


@app.route("/optional", methods=["GET"])
@jwt_required(optional=True)
def protected():
    return jsonify(foo='bar')


if __name__ == "__main__":
    app.run()

No token:

$ http GET :5000/optional
HTTP/1.0 200 OK
Content-Length: 14
Content-Type: application/json
Date: Fri, 30 Apr 2021 16:39:51 GMT
Server: Werkzeug/1.0.1 Python/3.8.8

{
    "foo": "bar"
}

A valid token:

± http GET :5000/optional Authorization:"JWT $VALID_JWT"
HTTP/1.0 200 OK
Content-Length: 14
Content-Type: application/json
Date: Fri, 30 Apr 2021 16:35:25 GMT
Server: Werkzeug/1.0.1 Python/3.8.8

{
    "foo": "bar"
}

An expired token:

$ http GET :5000/optional Authorization:"JWT $EXPIRED_JWT"
HTTP/1.0 401 UNAUTHORIZED
Content-Length: 28
Content-Type: application/json
Date: Fri, 30 Apr 2021 16:34:52 GMT
Server: Werkzeug/1.0.1 Python/3.8.8

{
    "msg": "Token has expired"
}

A token that has been tampered with:

$ http GET :5000/optional Authorization:"JWT $TAMPERED_JWT"
HTTP/1.0 422 UNPROCESSABLE ENTITY
Content-Length: 40
Content-Type: application/json
Date: Fri, 30 Apr 2021 16:33:33 GMT
Server: Werkzeug/1.0.1 Python/3.8.8

{
    "msg": "Signature verification failed"
}

Not a JWT:

± http GET :5000/optional Authorization:"JWT something_not_even_resembling_a_jwt"
HTTP/1.0 422 UNPROCESSABLE ENTITY
Content-Length: 30
Content-Type: application/json
Date: Fri, 30 Apr 2021 16:36:12 GMT
Server: Werkzeug/1.0.1 Python/3.8.8

{
    "msg": "Not enough segments"
}

[dmulter]

Please try the error cases I listed in the description. Note the result for one of them:

(venv) api-users (master) $ http GET :5000/optional Authorization:"JWT "
HTTP/1.0 200 OK
Content-Length: 14
Content-Type: application/json
Date: Fri, 30 Apr 2021 16:57:56 GMT
Server: Werkzeug/1.0.1 Python/3.8.5

{
    "foo": "bar"
}

[vimalloc-mavenlink]

I would argue that is the expected behavior. If you pass in an Authorizarion header with a null value, that's basically the same thing as not passing in a JWT at all and treated as such (as there is no token in the request).

If you want to make an argument that it shouldn't work that way I'm happy to hear it, but to me it feels like making that breaking change doesn't really provide much benefit.

[dmulter]

Also

(venv) api-users (master) $ http GET :5000/optional Authorization:"JWT xxxx xxxx"
HTTP/1.0 200 OK
Content-Length: 14
Content-Type: application/json
Date: Fri, 30 Apr 2021 17:19:59 GMT
Server: Werkzeug/1.0.1 Python/3.8.5

{
    "foo": "bar"
}

(venv) api-users (master) $ http GET :5000/optional Authorization:"JXX"
HTTP/1.0 200 OK
Content-Length: 14
Content-Type: application/json
Date: Fri, 30 Apr 2021 17:00:00 GMT
Server: Werkzeug/1.0.1 Python/3.8.5

{
    "foo": "bar"
}

[dmulter]

The following request fails as expected, but if you change the optional route to use locations=["query_string"] then it does not fail:

(venv) api-users (master) $ http GET :5000/optional Authorization=="JWT xxxx"
HTTP/1.0 200 OK
Content-Length: 14
Content-Type: application/json
Date: Fri, 30 Apr 2021 18:37:46 GMT
Server: Werkzeug/1.0.1 Python/3.8.5

{
    "foo": "bar"
}

[dmulter]

The last three are pretty clearly bugs. I would also suggest that lack of the token portion is not an indication of not supplying authentication. It seems clear that passing in the Authorization header at all means that the package should validate the header for authorization. If the first problem was the only bug then I might agree with you, but considering the others as well, please consider all four as actual bugs.

[vimalloc]

The last three are pretty clearly bugs.

The only bug is the Authorization:"JWT xxxx xxxx". I do agree that is not currently working as intended, and it should return something like a not enough segments error.

As for the other ones, they are behaving as intended. If you pass in a different authorization header then flask-jwt-extended is configured to look for, of course it's going to be ignored. It's entirely possible to have different authorization schemas in your application for different endpoints, that's the point of being able to specify them as Bearer/JWT/basic/etc.

Even more so for the other example, if you tell an endpoint to only look for a JWT in the query string in the request, it's going to look for a JWT in the query string in the request. Why would passing in a header when you are explicitly saying "ignore headers" be cause for an error? That's just an example of someone using the API incorrectly.

[dmulter]

I see your point on "JXX" and I agree. For the last one, I am passing the authorization as a query param by using == instead of : in httpie. Am I missing something?

[vimalloc-mavenlink]

Oh thanks for clarifying that, I missed that entirely. I think that example is failing because by default the query string looks for the key jwt to find the token. If I do this instead, I see the expected error:

$ http GET :5000/optional jwt=="xxx"
HTTP/1.0 422 UNPROCESSABLE ENTITY
Content-Length: 30
Content-Type: application/json
Date: Fri, 30 Apr 2021 20:22:53 GMT
Server: Werkzeug/1.0.1 Python/3.8.8

{
    "msg": "Not enough segments"
}

[dmulter]

So in a query string param does it expect jwt="JWT token string" or jwt="token string"? In my code I've used authorization="JWT token string". Do you support my approach via some config?

[dmulter]

One last thing I'll point out again for the first issue I raised. You said

I would argue that is the expected behavior. If you pass in an Authorizarion header with a null value, that's basically the same thing as not passing in a JWT at all and treated as such (as there is no token in the request).

Note that the Authorization header is not null, it is the valid string "JWT ". This is why I think it should be parsed and rejected because no JWT token is actually specified for authorization.

I leave it to you now to decide.

[vimalloc-mavenlink]

So in a query string param does it expect jwt="JWT token string" or jwt="token string"? In my code I've used authorization="JWT token string". Do you support my approach via some config?

Currently no, it only supports <key>=<token>, but I have no problem adding a configuration option that would make something like <key>=<type> <token> work. There are a couple other PRs in the pipeline already, so I could probably add it sometime this weekend and get a new release cut.

Note that the Authorization header is not null, it is the valid string "JWT ". This is why I think it should be parsed and rejected because no JWT token is actually specified for authorization.

I can see the argument for that. Let me consider it and I'll figure out exactly what I want to do while working on the other enhancements this weekend.

Cheers 👍

[dmulter]

Thanks for considering both! Would love to see the first one as a new feature if you can.