Problem/Motivation
The user_pass_rehash function generates a HMAC keyed by user credentials. Its primary purpose is to generate tamper-proof password-reset links and ensure that they can only be used once.
A nice effect of using the hashed password as the HMAC key is that existing authentication tokens automatically become invalid if the password is changed. This is a feature which is interesting for other use-cases as well, e.g. #2472535: Remove SessionManager::delete in favor of a portable mechanism to invalid sessions of authenticated users.
Proposed resolution
Extract account_token service (AccountTokenGenerator) from user_pass_rehash().
Remaining tasks
Write tests.
Review, commit.
User interface changes
None.
API changes
Deprecates user_pass_rehash().
Beta phase evaluation
| Issue category | Task |
|---|---|
| Issue priority | Not critical |
| Prioritized changes | The main goal of this issue is improving DX for better security |
| Disruption | Disruptive for live sites because this will render existing password-reset / account-cancel links invalid when updating |
| Comment | File | Size | Author |
|---|---|---|---|
| #23 | 2486415-nr-bot.txt | 155 bytes | needs-review-queue-bot |
| #3 | interdiff.txt | 4.37 KB | znerol |
| #3 | rework_user_pass_rehash-2486415-3.patch | 18.99 KB | znerol |
| #1 | rework_user_pass_rehash-2486415-1.patch | 18.73 KB | znerol |
Comments
Comment #1
znerol commentedComment #3
znerol commentedFixed tests. Also added a paragraph to the class documentation about the necessity to either limit the validity time of tokens or supply a nonce in order to prevent against replay attacks. I wonder whether we should add a
$nonceparameter toget()/validate()?Comment #4
cpj commented@znerol - I really like this abstraction of the user_pass_rehash to a service. Very useful.
Comment #5
andypostwhat's left here?
Comment #6
znerol commentedComment #7
znerol commentedAdded beta evaluation. Still needs tests.
Comment #14
jibranComment #15
andypostIt is api addition
Comment #16
avpadernoAPI changes are still done before Drupal 9.
Comment #23
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.