jazzband / django-rest-knox

Authentication Module for django rest auth
MIT License
1.17k stars 213 forks source link

AttributeError: 'SHA512' object has no attribute 'update' #364

Closed giovannicimolin closed 3 months ago

giovannicimolin commented 3 months ago

When migrating my project to version 5.x, I get the following error when running tests:

    response = handler(request, *args, **kwargs)
  File "/code/custom_auth/views.py", line 165, in post
    _, token = AuthToken.objects.create(
  File "/opt/venv/lib/python3.10/site-packages/knox/models.py", line 24, in create
    digest = crypto.hash_token(token)
  File "/opt/venv/lib/python3.10/site-packages/knox/crypto.py", line 30, in hash_token
    digest.update(make_hex_compatible(token))
AttributeError: 'SHA512' object has no attribute 'update'

Related code: https://github.com/jazzband/django-rest-knox/blob/1e379b48f5281b2df9224ff85f24e9f59647abe1/knox/crypto.py#L15

Investigation ongoing.

giovannicimolin commented 3 months ago

I think this was caused by this switch alongside the code changes linked above: https://github.com/jazzband/django-rest-knox/pull/230

johnraz commented 3 months ago

The related code is actually https://github.com/jazzband/django-rest-knox/blob/1e379b48f5281b2df9224ff85f24e9f59647abe1/knox/crypto.py#L30

and digest is from

https://github.com/jazzband/django-rest-knox/blob/1e379b48f5281b2df9224ff85f24e9f59647abe1/knox/crypto.py#L6

This is weird because the interface should have the update method according to the Python doc

https://docs.python.org/3/library/hashlib.html#hash-algorithms

What do you have in your settings ? Could you have a bad import there ?

Also the Python version might be a factor here

which version are you using? edit: it’s clearly 3.10 from the traceback

I recommend we improve the test suite of knox to cover all the hash functions we support.

Cheers !

giovannicimolin commented 3 months ago

@johnraz Yup, that's excatly the place having issues.

I'm using the old default cryptography.hazmat.primitives.hashes.SHA512, but it looks like it's broken now. I'm investigating to figure out why this happened and why it was working before.

I recommend we improve the test suite of knox to cover all the hash functions we support.

:+1: to this. It was already in my plans but I guess I'll have to work on it earlier than expected.

giovannicimolin commented 3 months ago

In 4.2.0 the code was like this:

    digest = hashes.Hash(sha(), backend=default_backend())
    digest.update(binascii.unhexlify(token))
    return binascii.hexlify(digest.finalize()).decode()

With the signature change the cryptography functions don't work anymore.

I wanted to keep old tokens working with https://github.com/jazzband/django-rest-knox/pull/362, but now I'm not 100% sure it'll work as is currently.

giovannicimolin commented 3 months ago

I'll close this now since the issue was me trying to use a now incompatible hashing method.

If anyone is looking for a solution when bumping into this issue: just remove the line below and let the library use the default from hashlib.

    "SECURE_HASH_ALGORITHM": "cryptography.hazmat.primitives.hashes.SHA512",

I think we'll need to do a better job at documentation when changes like this come in. I'll be more attentive when reviewing PRs related to this now.