Verify Serializer Should Honour Blacklist

[TeresaSiegmantel]

With this patch, the TokenVerifySerializer checks the blacklist if the app is installed.

See Issue #231.

[Andrew-Chen-Wang]

Thanks for your contribution! Left my review of your PR. Please resolve each comment as you make those changes. Sorry for taking so long @philipp-siegmantel Just getting exponentially busier with college comin' up.

I think from a security standpoint, it looks fine. You can see the description for the token type is:
https://github.com/SimpleJWT/django-rest-framework-simplejwt/blob/master/rest_framework_simplejwt/tokens.py#L299-L303

Untyped tokens do not verify the "token_type" claim.  This is useful
        when performing general validation of a token's signature and other
        properties which do not relate to the token's intended use.

I think only L158 is a concern. I'll leave another comment for that, but basically, you don't want to let someone know that it's blacklisted.

[Andrew-Chen-Wang]

Closes #231

Hopefully that links with the issue so that the issue automatically closes.

[TeresaSiegmantel]

I add your suggestions, thanks for them.

Now I have a problem I can't seem to fix. The test checks that the blacklisted token does not verify, fails. This is due to the RefreshToken constructor throwing a InvalidToken exception. I can't figure out how a freshly created Token for a valid user could not properly verify. Any hints?

[Andrew-Chen-Wang]

You might've used the wrong name for settings. @philipp-siegmantel

[Andrew-Chen-Wang]

Quick question: would it be better if we had Travis? Would probably make reviewing PRs and seeing where tests failed easier since it's embedded into the GH site.

[TeresaSiegmantel]

Ok, now all tests pass.

An attacker could still infer that a token is an access token, but only if it is blacklisted, so I see no problem.

[TeresaSiegmantel]

I think some form of CI would be great. Github has Actions, witch are pretty simple to set up.

EDIT: I see you use CircleCI.

[Andrew-Chen-Wang]

Github has Actions, witch are pretty simple to set up. I see you use CircleCI.

Yea, not a fan of CircleCI; that was here before I started using GitHub. I like GitHub actions for CI/CD when it comes to testing certain things and publishing/deployment (e.g. AWS services testing without Moto and AWS deployment). But Travis CI has a nicer interface, especially for open source projects because the builds are individualized via the TOXENV. Additionally, testing the configuration of .travis.yml is a little more smooth and visibly more appealing.

Some great things about GitHub actions are like if a certain path has a file change, then execute an action, whereas Travis doesn't support that.

An attacker could still infer that a token is an access token, but only if it is blacklisted, so I see no problem.

Perhaps with certain configurations. Not too big of a problem for most Django websites since an attacker is not going to know some random website.

[HodaSadatJafari]

Hi guys
When is this going to be merged?

[Andrew-Chen-Wang]

@auvipy do you mind reviewing this as well? I was waiting for David for a second opinion, but obviously he's absent now.

[auvipy]

docs/changelog should be updated as well

[auvipy]

this is testing migration but the PR is not showing any migration changes, or it was already done but not tested?

[Andrew-Chen-Wang]

Yea those migrations were already done and come from https://github.com/jazzband/django-rest-framework-simplejwt/tree/master/rest_framework_simplejwt/token_blacklist/migrations