Outlook.com now uses the BingPreview crawler to crawl links in emails.

This means that one-time login links send to outlook email addresses are marked as used/expired before the user gets the chance to use them, effectively locking them out of the site/their account.

BingPreview currently uses this user agent:
"Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/534+ (KHTML, like Gecko) BingPreview/1.0b"

Issue fork drupal-2828034

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Jelle_S created an issue. See original summary.

Jelle_S’s picture

Issue summary: View changes
Jelle_S’s picture

Patch returns access denied for BingPreview bot.

Jelle_S’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 3: 2828034-2-fix-bingpreview-one-time-login-mail.patch, failed testing.

daften’s picture

Maybe this can be enhanced with a deny for slack also? :)

Jelle_S’s picture

cilefen’s picture

Do these mail clients actually submit the confirmation form? I do not understand how execution could be in the code path that was patched.

Jelle_S’s picture

Hmm it seems we have altered that form to redirect straight to the /login path. Nevertheless it is still valid for slack and mail. Sometimes site admins send drush uli's to their devs or clients or whatnot. If you send in through mail or slack, the link will still get invalidated

gagarine’s picture

Version: 7.x-dev » 10.0.x-dev

It seem I stumble on this issue using https://www.drupal.org/project/passwordless/issues/3062482 on D8.

Kristen Pol’s picture

Issue tags: +Bug Smash Initiative

While this code may work as desired, IMO it feels like the wrong approach to hardcode these. I think we need a different approach so this is configurable instead. I'll check in with the bugsmash team for additional thoughts.

quietone’s picture

Version: 10.0.x-dev » 9.3.x-dev

I think this should be 9.3.x

gagarine’s picture

Another approach could be an intermediary page with a form with a button "reset my password"?

or is using <a> for links ==> <a href="[user:one-time-direct-login-url]">click here to reset your password</a>. Apparently outlook do not create link preview when they are proper HTML link.

Perhaps adding a nofollow to this link?

Are we sure robots.txt is configured correctly to handle one time login?

cilefen’s picture

cilefen’s picture

roderik’s picture

I welcome reviews to #3188375: Fix direct-login links (referer header leakage / UX) when already logged in but it won't solve this issue. It just fixes a confusion/security issue if user/reset/UID/TIMESTAMP/HASH/login is accessed while a user was already logged-in to the browser.

#3097238 may, though.

Are we sure robots.txt is configured correctly to handle one time login?

It doesn't; user/reset is not present in robots.txt. #3097238: Protect initial login link against abuse and username leaking fixes that plus one other thing. Please review.

Another approach could be an intermediary page with a form with a button "reset my password"?

Already exists.

  • user/reset/UID/TIMESTAMP/HASH has a form.
  • user/reset/UID/TIMESTAMP/HASH/login directly logs users in.

Core mails use a link to the form.

So, as far as I can tell right now, we might decide either of the following points:

  • We are sure that all 'prefetching clients' (like Slack) respect robots.txt: #3097238 is enough, and this issue is a duplicate.
  • We are not sure of that, and this is partly a "works as designed" because we can't do anything about 'prefetching clients' visiting .../login links.
  • We are not sure of that, and after fixing #3097238 this becomes a documentation issue where we want to tell people to never announce .../login links openly. (Core never uses .../login links. 'drush uli' does print .../login links on the terminal.)

As an illustration here's blunt stub example responses to #9 - but again, these are invalid if we know the robots.txt change fixes everything:

Hmm it seems we have altered that form to redirect straight to the /login path. Nevertheless it is still valid for slack and mail. Sometimes site admins send drush uli's to their devs or clients or whatnot. If you send in through mail or slack, the link will still get invalidated

We have two types of login links: "safe to spread" links, and "direct login (without button)" links. Don't conflate the two. Core only uses the first type in its mails.

Don't alter the form to log in directly. Invalidation is a direct result of that alteration. If you do, you're responsible for putting the "rel=nofollow" in mails (if that makes a difference).

Sending drush uli's through mail or slack should not be done. Chop off the "/login" part.

roderik’s picture

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

zcht’s picture

For me on the platform I tested a different approach, therefore also only now a feedback. Condition: over 4000 registered users with different email addresses, test period > 6 months, environment production, so far 100% worked with the created Shy One-Time module. The One Time Login links are no longer invalidated. Maybe you could implement this solution directly in Drupal itself. Test and feedback are welcome.

cilefen’s picture

Status: Needs review » Needs work

This is needs work for #11.

gagarine’s picture

I'm now convinced this should be marked has "work as designed". A link using a HTTP GET request should not modifies a resource (in that case, change user to active), period.

I think the route /user/reset/UID/TIMESTAMP/HASH/login should be deleted to avoid confusion. Even when you use drush you can click on a button. It's also a better UX.

Someone think HTTP GET request to modify a resource is a good idea? Is their a legitimate use case?

rgpublic’s picture

No, I also think this is bad design by Microsoft that this happens in the first place, but nevertheless, this problem is widespread enough to cause many people to think that the reset password feature doesn't work. We have now blocked these calls server-side but it's very difficult to get behind this and not everyone has the ability and experience to reconfigure their webserver. Many people were reporting that their one-time login links always seem to have already expired. The patch in #7 is a very simple solution with no likely side effects as it only kicks in when the UserAgent is BingPreview (and Slackbot). It should be considered to go in, IMHO.

rgpublic’s picture

Ah, perhaps I misunderstood you, @gagarine as I realize now. You mean we should remove the "/login" route altogether so you are always force to go via the form. Yes, if no one comes up with a valid use case, then why not? I wonder though: Why are you calling this solution "work as designed". It would still need a patch to remove that route, wouldn't it? IMO it'd be a valid fix and a change in the codebase.

gagarine’s picture

@rbpublic yes, I mean remove the "/login" route altogether so you are always force to go via the form. You are right, "work as designed" was certainly inappropriate in this situation (even if it does works has designed, but people use the direct login when they should not).

We should also inform module maintainers that using direct link to login is never ok. See #3298635: Bingpreview/Slack preview invalidates one time login links

gagarine’s picture

Status: Needs work » Needs review
FileSize
5.25 KB

A simple patch that remove the reset login route.

cilefen’s picture

Can we remove routes in minor releases?

gagarine’s picture

gagarine’s picture

Can we remove routes in minor releases?

In that case I would say yes, because it's a major bug that is not possible to fix properly otherwise. It's also a very easy change. But others will certainly disagree.

gagarine’s picture

Their was a much more test than anticipated...

I would be happy to clean all unused tests if the direction to remove the /login route is validated by the community and/or someone than can commit on the core.

zcht’s picture

i would be in favor of adding this function as an option. some modules use the one time login link for authentication in the system itself. or another technical solution must be worked out for this, so that a login is possible without reconfirming the login on the site itself.

often the link is generated by drush uli and sent by email in the plan text, but is then invalidated by the bingbot.

i am primarily thinking about usability and user-friendly handling of the login process.

removing the route is a hard intrusion into the core, which has been working in this form for years. this could cause various other problems.

gagarine’s picture

some modules use the one time login link for authentication in the system itself.

Do we have an example of this?

i am primarily thinking about usability and user-friendly handling of the login process.

In my experience people don't really understand the "auto login". It seem a much better UX to have a click on a "activate my account" button.

I agree it may be a bit hard for a minor version. But for Drupal 10 that would be a nice cleanup. So we mark the route and related function as deprecated already.

This issue is 7 years old now, someone with authority has to take a decision.

It's not only about a feature or not, it's about following the HTTP protocol or not => https://httpwg.org/specs/rfc7231.html#GET

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
FileSize
169 bytes

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.

Warped made their first commit to this issue’s fork.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

C-Logemann’s picture

Category: Bug report » Feature request

One ore more mail service providers decided to "read" mails and follow links inside. This is a "bug" of this mail providers especially when they made this behavior default and didn't provide a switch for this. On Drupal it's a feature request to deal with such "mail services".

gagarine’s picture

Category: Feature request » Bug report
Priority: Normal » Critical
Status: Needs work » Active

This issue is 7 years old now, someone with authority has to take a decision.

"this is a bug of the mail provider"

Seriously, Drupal community bring less and less value. Unfollow this issue, but I perhaps time for me to delete my D.O. account.

It's a critical issue that can lead to the impossibility for user to log-in. In the real world, nobody care if Microsft server "should" act differently.

But if we want to be theoretical: Drupal use a HTTP GET to change data witch is not how HTTP protocol is supposed to be work. A HTTP POST request should be used to change an account from blocked to active. It's a bug and a ugly one.

daften’s picture

Priority: Critical » Normal

Putting this back to normal priority (see https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-....). No data is altered, a blocked user can't use the one-time-login link. As can be seen in e.g. \Drupal\user\Controller\UserController::determineErrorRedirect

zcht’s picture

Small update from my side: we currently have a problem that some users who use Microsoft Office 365 do not receive emails. Even with our additional module, which works well, this is not remedied. It seems that MS Office 365 has introduced new crawlers or ways of accessing the link.

However, it does not affect all MS Office 365 users, at the moment only 4... nevertheless, of course, it's obvious that these users can't log in.

In my eyes it is also a problem of Drupal.... you cant tell the email providers how to define their own services to access a, in comparison, small Drupal site, which you have released yourself. I am absolutely with @gagarine, and a feature request is definitely NOT.

perspective, i think something should be done actively on the Drupal side. more and more services crawl all links, whether it's microsoft, slack or other services. That's the future right now and if we don't change something, you're going to exclude users from Drupal who can't log in.

zcht’s picture

A small update, the "Shy One Time" module has been updated to version 2.x. Via the Drupal state API you can now block unwanted user agents that access the route 'user.reset'. The users I mentioned in comment 40 can now log in without problems.