fix(): Support for PyJWT v1.7.1 or later

[belal-bh]

Hi @CuriousLearner

I was facing the same issue #51
I found that the main problem is the dependency package PyJWT version change.
I have tried to fix this to support PyJWT v1.7.1 or later. It worked on python 3.8, 3.9.
Could you please check the changes?

And thanks for the great package.

[CuriousLearner]

Why are we catching AttributeError here? In what case does it cause an AttributeError?

[belal-bh]

Why are we catching AttributeError here? In what case does it cause an AttributeError?

Thanks for the review.
It was a mistake, at first the if condition was different in my local (before commit), but later I updated the if conditional block.

encoded_str = jwt.encode(data, django_settings.SECRET_KEY)
if type(encoded_str) is not str:
    # jwt v2.0.0 or greater: `jwt.encode()` no longer return <class 'bytes'> 
    # instead it return <class 'str'> and that's why `jwt.encode().decode()` fails
    # To support jwt v1.7.1 or greater: if `jwt.encode()` return <class 'bytes'>
    # convert it to <class 'str'>
    # Details: https://github.com/jpadilla/pyjwt
    try:
        encoded_str = str(encoded_str, encoding='utf-8')
    except TypeError:
        try:
            encoded_str = encoded_str.decode()
        except AttributeError:
            pass
return encoded_str

In that case PyJWT v2.0.1 can through AttributeError because str object does not have decode(). But I noticed It will not happen. But I have forgotten to remove that line.

It should be removed.

By the way, it was my first pull request. I am not good at testing. But it worked for me. Please apologize for my mistake.
I am updating this.

[CuriousLearner]

Thank you for coming forward to contribute to this library. I really appreciate it. I've mentioned some comments and suggestions inline. Please have a look at them.

[CuriousLearner]

If you've the link to the issue, that this change was made in, that would be helpful.

[belal-bh]

If you've the link to the issue, that this change was made in, that would be helpful.

Sorry for late. I think you wanted to know about PyJWT issue.

At first, I found it from the documentation https://pyjwt.readthedocs.io/en/stable/changelog.html.

PyJWT Github issue: jwt.encode returns str instead of bytes in 2.0.0a1 #529

And PyJWT Github PR:

• Return tokens as string not bytes #513
• Update return type for jwt.encode #521
• Enforce .encode() return type using mypy #551

[CuriousLearner]

Thanks for these. I'll modify the comment in the changes to reflect these and merge when the other PR on tox and other dependency upgrade is ready.

[belal-bh]

Thanks! 😊

[CuriousLearner]

Hi @belal-bh

I've mentioned my comments inline. Once you take care of those and ensure that the code is working fine, we'd ideally also want to include the new version of Python/Django in the .tox file so that we are testing this library on the most recent versions.

Please have a look at the file here: https://github.com/CuriousLearner/django-phone-verify/blob/master/tox.ini

Ideally we'd like to test Python 3.8 and Python 3.9 version.

Once you're done, please ensure that CI is passing and we can then merge it in.

Thank you once again for your contribution. If you need any help, please let me know.

[CuriousLearner]

Thanks!

I'll work on the tox stuff and merge this when those changes are ready.

[belal-bh]

Thanks!

I'll work on the tox stuff and merge this when those changes are ready.

Thanks a lot.
I want to learn more about it. Actually, I am very new to CI and testing.
I have mistakenly closed the pull request! But I have reopened it.
I've given the PyJWT issue and PR links the previous comment. Please check it.

And thanks a lot for helping me and contributing this great package!

[CuriousLearner]

I'm working on upgrades in this PR: #58

It is almost done. I'll test the comments that we added in this PR and then merge your changes in.

Many thanks!

[CuriousLearner]

I have added the changes to #58 and will merge it once the CI passes.

I'm closing this one.