Opened 23 months ago

Closed 23 months ago

Last modified 23 months ago

#26016 closed Bug (fixed)

Django 1.9 dropped compatibility with py-bcrypt (as an alternative to bcrypt)

Reported by: Raphaël Hertzog Owned by: Tim Graham
Component: contrib.auth Version: 1.9
Severity: Normal Keywords:
Cc: FunkyBob Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In Debian, our python-bcrypt package is currently based on py-bcrypt (https://pypi.python.org/pypi/py-bcrypt) and it always worked fine with Django. With version 1.9 however, the test suite now fails with errors like this:

======================================================================
ERROR: test_bcrypt (auth_tests.test_hashers.TestUtilsHashPass)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/adttmp.u2Wozt/django-tests/auth_tests/test_hashers.py", line 167, in test_bcrypt
    encoded = make_password('lètmein', hasher='bcrypt')
  File "/usr/lib/python3/dist-packages/django/contrib/auth/hashers.py", line 74, in make_password
    salt = hasher.salt()
  File "/usr/lib/python3/dist-packages/django/contrib/auth/hashers.py", line 289, in salt
    return bcrypt.gensalt(rounds=self.rounds)
TypeError: gensalt() got an unexpected keyword argument 'rounds'

This code works with bcrypt (https://pypi.python.org/pypi/bcrypt). I believe that keeping the compatibility with py-bcrypt is as easy as replacing the named parameter with the positional parameter that was used until commit 23529fb19594ffcc6ba6d716356b828157200288

The commit does not give a good reason why this change was actually needed... so I would suggest to revert it (and on the Debian side we will still upgrade our python-bcrypt to bcrypt instead of py-bcrypt but I wanted to file this to let you know and at least to document this regression for other users).

Change History (5)

comment:1 Changed 23 months ago by Tim Graham

Cc: FunkyBob added
Component: Uncategorizedcontrib.auth
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Curtis, any reason not to revert the change?

comment:2 Changed 23 months ago by Curtis Maloney

No, no reason... I believe the original motive was "explicit is better than implicit" ... a vague hope to protect ourselves against future prototype changes.

Well, we spotted the change... but didn't it didn't protect us.

comment:3 Changed 23 months ago by Tim Graham

Owner: changed from nobody to Tim Graham
Status: newassigned

comment:4 Changed 23 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In f0ad6416:

Fixed #26016 -- Restored contrib.auth hashers compatibility with py-bcrypt.

Reverted "Explicitly passed rounds as rounds to bcrypt.gensalt()"

This reverts commit 23529fb19594ffcc6ba6d716356b828157200288.

comment:5 Changed 23 months ago by Tim Graham <timograham@…>

In 2f4be21:

[1.9.x] Fixed #26016 -- Restored contrib.auth hashers compatibility with py-bcrypt.

Reverted "Explicitly passed rounds as rounds to bcrypt.gensalt()"

This reverts commit 23529fb19594ffcc6ba6d716356b828157200288.

Backport of f0ad641628a3ddc4e1c208e481b9cd0e9304dc3d from master

Note: See TracTickets for help on using tickets.
Back to Top