Opened 12 years ago

Closed 11 years ago

#17944 closed Bug (fixed)

Unable to access to User record in the admin if the user has a unmanageable password

Reported by: Stefano Apostolico Owned by: Claude Paroz
Component: contrib.auth Version: 1.4-rc-2
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If the user has a unmanageable password (ie no hashers found) ReadOnlyPasswordHashWidget reraise ValueError up to the UI, this prevents to access to the user record if the form contains ReadOnlyPasswordHashWidget ( admin ), this happen also for blank password. If the system use a custom backend with external authentication there is no reason for django to known the password algorithm, but should be still possible to access to the user record.
The patch simply surround the involved lined into the ReadOnlyPasswordHashWidget.render() and display a 'Unknown password hashing algorithm' message instead.
Not sure if the message could be improved/changed.

Attachments (3)

ReadOnlyPasswordHashWidget.patch (1.3 KB ) - added by Stefano Apostolico 12 years ago.
ReadOnlyPasswordHashWidget_0.1.patch (3.9 KB ) - added by Stefano Apostolico 12 years ago.
17944.3.patch (7.6 KB ) - added by Aymeric Augustin 12 years ago.

Download all attachments as: .zip

Change History (17)

by Stefano Apostolico, 12 years ago

comment:1 by Stefano Apostolico, 12 years ago

Type: UncategorizedBug

comment:2 by Carl Meyer, 12 years ago

Needs tests: set
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

This is definitely a release-blocking regression, thanks for the report.

Patch looks good to me except that the "except" clause captures the exception when it has no use for it (and uses the old syntax to do so); the ",e" should just be removed.

And it needs a test.

This violates the string freeze, but I don't see any good alternative. Having this fixed with an untranslated message in 1.4 is better than not having it fixed.

by Stefano Apostolico, 12 years ago

comment:3 by Stefano Apostolico, 12 years ago

Needs tests: unset

Added ReadOnlyPasswordHashWidget_0.1.patch

  • added tests both for empty and unparseable password
  • removed exception capturing

comment:4 by Stefano Apostolico, 12 years ago

Owner: changed from nobody to Stefano Apostolico

comment:5 by Stefano Apostolico, 12 years ago

Version: 1.4-beta-11.4-rc-2

comment:6 by Aymeric Augustin, 12 years ago

Triage Stage: AcceptedReady for checkin

I've updated the patch slightly to make the try/except clause as specific as possible and add one more test.

I agree with Carl that an untranslated string is less of a problem than an uncaught exception.

(There's a little bit of noise in the patch because my editor strips trailing spaces, sorry.)

by Aymeric Augustin, 12 years ago

Attachment: 17944.3.patch added

comment:7 by Aymeric Augustin, 12 years ago

Resolution: fixed
Status: newclosed

In [17775]:

Fixed #17944 -- Prevented an error in the user change page of the admin when the content of the password field doesn't match the expected format. Thanks saxix for the report and initial patch.

comment:8 by Jannis Leidel, 12 years ago

In [17779]:

Reverted the introduction of a translation string in r17775 as it happened after string freeze. Refs #17944.

comment:9 by Aymeric Augustin, 12 years ago

Has patch: unset
Resolution: fixed
Status: closedreopened
Triage Stage: Ready for checkinAccepted

I'm re-opening the bug so we remember to re-introduce the ugettext call after 1.4.

comment:10 by Aymeric Augustin, 12 years ago

Owner: changed from Stefano Apostolico to Aymeric Augustin
Status: reopenednew

comment:11 by Aymeric Augustin, 12 years ago

Owner: changed from Aymeric Augustin to nobody
Severity: Release blockerNormal

comment:12 by Aymeric Augustin, 11 years ago

Owner: changed from nobody to Aymeric Augustin
Severity: NormalRelease blocker

Oops, let's not forget this before 1.5.

comment:13 by Claude Paroz, 11 years ago

Owner: changed from Aymeric Augustin to Claude Paroz

comment:14 by Claude Paroz, 11 years ago

Resolution: fixed
Status: newclosed

In fact, already fixed in [703c266682be39f7153498ad0d8031231f12ee79].

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