Fix direct-login links (referer header leakage / UX) when already logged in

Created on 14 December 2020, over 3 years ago
Updated 30 January 2023, over 1 year ago

Problem/Motivation

One-time login links with "/login" appended are output by 'drush uli'. They return Access Denied if a user is already logged into the site on the browser where this link is followed. This:

History / situation description:

For regular (non-"/login") password reset links, this "unable to change password" situation was fixed in D8.0-dev, and for both types of links in D7, in #889772: Following a password reset link while logged in leaves users unable to change their password (May 2015)

"/login" links

  • were inadvertently removed from D8.0 in May 2014 ( #1998198: Convert user_pass_reset to a new-style Form object ). They were restored while fixing the "leaked link via referrer header" issue in D8.1.9 where they got their own separate route. Ironically, the "Access Denied" reintroduced the fixed issue on that new route, for already-logged-in users.
  • now serve as the "submit" route for the regular password reset form (that contains only a "Log in" button), as well as a way to bypass this form - just like in D7.
  • now return "Access Denied" but should behave the same as regular non-"/login" password reset links, when a user is already logged in. (See "Proposed resolution". In D7, they follow the exact same code path; the "/login" part of the path is just ignored for already logged-in users.)

Steps to reproduce

  • Be logged into Drupal.
  • Follow a link output by 'drush uli' (for either the same or a different user).
  • See "Access Denied", while still being logged in.
  • Check (or just know) that referrer headers include the password reset links, for assets loaded from this "Access Denied" page. See screenshots in #2515050: A valid one-time login link may be leaked by the referer header to 3rd parties .
  • Try the same link in a different browser that is not logged in. See that the link still works.

Proposed resolution

The user.reset.login route should always redirect, to prevent those leaked referrer headers - just like the user.reset route was modified in #2515050. I.e. it should never directly throw an AccessDeniedHttpException.

user.reset.login should behave (almost) the same as user.reset, when a user is already logged in. Both to resolve the previous point, and to make 'drush uli' output always functional / behave equally to D7. That is:

  • If the same user is logged in, then log out and redirect back to itself. (This will then skip the login form and go straight to the user edit screen in our case.)
  • If a different user is logged in, display a message and redirect to the front page.

Remaining tasks

Review.

User interface changes

API changes

Data model changes

Release notes snippet

One-time login links ending in "/login" do not leak URL info anymore, and enable the user to change their password, if they are used while the user was already logged in.

More

Below summary I made for at least myself to construct the above text, and to refer back to when I get confused about what happens on D7/8/9, with/without "/login" suffix, when already/not being logged in. Reading optional.

1)
URLs with paths like user/UID/reset/TIMESTAMP/HASH:

Provided by 'password reset' mails.

When not logged in:
Displays a form titled "Reset password" with just some data and a "Log in" button that takes you to your user edit screen to change your password. (This has been in place since Drupal < 7.)

When logged in already as the same user:

When logged in already as a different user:
Displays the front page and a warning message "Another user (USERNAME) is already logged into the site on this computer, ...". (This was introduced in D6 in #545230: One-time login link error message is incorrect and misleading and ported to D8 in #1998198: Convert user_pass_reset to a new-style Form object . Before D6, the user got a misleading message.)

2)
URLs with '/login' appended:

Output by drush uli.

When not logged in:

When logged in already (as the same or a different user):

  • In D7: same as without "/login". (Exact same code path.)
  • In D8.1.9: it now returns an "access denied" page.
🐛 Bug report
Status

Needs work

Version

10.1

Component
User system 

Last updated 2 days ago

Created by

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

Live updates comments and jobs are added and updated live.
  • Security

    It is used for security vulnerabilities which do not need a security advisory. For example, security issues in projects which do not have security advisory coverage, or forward-porting a change already disclosed in a security advisory. See Drupal’s security advisory policy for details. Be careful publicly disclosing security vulnerabilities! Use the “Report a security vulnerability” link in the project page’s sidebar. See how to report a security issue for details.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • The 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.

Production build 0.69.0 2024