Opened 4 months ago

Last modified 4 hours ago

#36226 assigned Bug

Only PBKDF2PasswordHasher supports str and bytes password

Reported by: Jason Held Owned by: Roelzkie
Component: contrib.auth Version: 5.1
Severity: Normal Keywords: auth
Cc: Roelzkie Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The other password hashers in their encode method all at some point call .encode on their password in some way. Thus, the other hasher classes, in the django source code, do not support the API and docstring of make_password.
This seems pretty easy to fix on the whole and I'd be happy to make a PR for it.

Change History (14)

comment:1 by Sarah Boyce, 4 months ago

Triage Stage: UnreviewedAccepted

Replicated thank you! PRs welcome

  • tests/auth_tests/test_hashers.py

    a b class TestUtilsHashPass(SimpleTestCase):  
    520520                    with self.assertRaisesMessage(ValueError, msg):
    521521                        hasher.encode("password", salt)
    522522
     523    def test_password_bytes(self):
     524        hasher_classes = [
     525            MD5PasswordHasher,
     526            PBKDF2PasswordHasher,
     527            PBKDF2SHA1PasswordHasher,
     528            ScryptPasswordHasher,
     529        ]
     530        for hasher_class in hasher_classes:
     531            hasher = hasher_class()
     532            with self.subTest(hasher_class.__name__):
     533                encoded = hasher.encode(b"password", hasher.salt())
     534                self.assertTrue(hasher.verify(b"password", encoded))
     535
    523536    def test_encode_password_required(self):
    524537        hasher_classes = [
    525538            MD5PasswordHasher,

comment:2 by Screamadelica, 4 months ago

Owner: set to Screamadelica
Status: newassigned

Hi, I'm new to this community and find this a decent good first issue. I will try to fix this bug :)

comment:3 by Antoliny, 4 months ago

Has patch: set

comment:4 by Screamadelica, 4 months ago

Just finished a pr and all checks have passed.
https://github.com/django/django/pull/19231

in reply to:  4 ; comment:5 by Antoliny, 4 months ago

Replying to Screamadelica:

Just finished a pr and all checks have passed.
https://github.com/django/django/pull/19231

It looks like you've submitted a PR, so I’ve set the "has patch" flag. Now, all that’s left is to wait for reviews from the fellows :)

in reply to:  5 comment:6 by Screamadelica, 4 months ago

Replying to Antoliny:

Replying to Screamadelica:

Just finished a pr and all checks have passed.
https://github.com/django/django/pull/19231

It looks like you've submitted a PR, so I’ve set the "has patch" flag. Now, all that’s left is to wait for reviews from the fellows :)

Thanks a lot, I will remember to change the flag after submitting PR later :)

comment:7 by Sarah Boyce, 3 months ago

Patch needs improvement: set

in reply to:  7 comment:8 by Roelzkie, 6 days ago

Replying to Sarah Boyce:

Hi Sarah, What's the status of this ticket? Can you re-assign this ticket to me? It seems like the owner is not actively improving the PR anymore.

comment:9 by Jacob Walls, 6 days ago

The prior owner closed their PR, so it's fair to set yourself in the owner field.

comment:10 by Roelzkie, 6 days ago

Please check this new PR. Thank you.

in reply to:  9 comment:11 by Roelzkie, 6 days ago

Owner: changed from Screamadelica to Roelzkie

Replying to Jacob Walls:

The prior owner closed their PR, so it's fair to set yourself in the owner field.

Thanks for the confirmation, Jacob. I set myself as the owner of this ticket and have created a PR.

comment:12 by Roelzkie, 6 days ago

Cc: Roelzkie added
Patch needs improvement: unset

comment:13 by Sarah Boyce, 6 hours ago

Patch needs improvement: set

comment:14 by Roelzkie, 4 hours ago

Keywords: auth added
Patch needs improvement: unset
Note: See TracTickets for help on using tickets.
Back to Top