Problem/Motivation

When using the forgot password link there are two reset notices showing up:

  1. You have just used your one-time login link. It is no longer necessary to use this link to log in. Please set your password. - Drupal core
  2. You have just used your one-time login link. It is no longer necessary to use this link to log in. Please change your password. - Legal Module

Steps to reproduce

  • Install the module.
  • Create new T&C and make sure "Ask to accept T&Cs on every login" is NOT set.
  • Login as a non-admin user and accept the T&C.
  • Logout.
  • Reset the user password and login using the one time login link.
  • The T&C won't show up, as the user has already accepted them, and he will be directly forwarded to the user edit page instead.
  • The reset notice is showing up twice and after defining another password, it is still shown once.

Proposed resolution

Alter the text in the legal module to match the one found in core to prevent it showing up multiple times.

Issue fork legal-3414370

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:

Comments

3li created an issue. See original summary.

3li’s picture

Status: Active » Needs review
StatusFileSize
new753 bytes
luqmaan.essop’s picture

Status: Needs review » Reviewed & tested by the community

Patch #2 works well.
Matching string check allows the removal of the duplication notification messages.

anybody’s picture

Title: Reset notice is showing up twice » Reset notice is showing up twice (from core and legal module)
Priority: Normal » Major

I can confirm this, coming here as I was also searching for the reason for these weird duplicate messages.

These are the two messages shown:

  • You have just used your one-time login link. It is no longer necessary to use this link to log in. Please change your password.
  • You have just used your one-time login link. It is no longer necessary to use this link to log in. It is recommended that you set your password.

grepping for them returns:

  • modules/contrib/legal/legal.module: $reminder = t('You have just used your one-time login link. It is no longer necessary to use this link to log in. Please change your password.');
  • core/modules/user/src/Controller/UserController.php: $this->messenger()->addStatus($this->t('You have just used your one-time login link. It is no longer necessary to use this link to log in. It is recommended that you set your password.'));

This message appears twice after clicking the one time login link after registration and as it seems, that's even a different message?

Grepping for the message from the patch ("You have just used your one-time login link. It is no longer necessary to use this link to log in. Please set your password.") in core (10.3.x) returns no more results!
So it seems this patch needs to be updated again?

@3li could you check that please?

Here's the relevant line of code in core:
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/user/...

And indeed it was changed 8 months ago:
https://git.drupalcode.org/project/drupal/-/blame/11.x/core/modules/user...

anybody’s picture

Status: Reviewed & tested by the community » Needs review

I prepared an updated MR.

thomas.frobieter’s picture

Status: Needs review » Reviewed & tested by the community

Works for me!

anybody’s picture

Still MAJOR as it affects all modern installations and all their registered users.

anybody’s picture

StatusFileSize
new944 bytes

MR!16 as static patch attached. Hopefully the maintainer will fix this major issue soon ... :/

anybody’s picture

Okay after testing the MR!16 for a few days in production, we received feedback that the UX is still bad.

The reason is, that even after changing the password (after reset) the message is still shown.
This is because

$_REQUEST['pass-reset-token']

is always present in the URL even after changing the password. Feel free to try yourself. The URL parameters are kept in Drupal 8+ after form submit!

Looking at the blame you can see that this is legacy code from the Drupal 7 port and I'm pretty sure it should be removed, as core already handles everything we need.

That also safes us from having to update the hard-coded string from MR!16 / core again and again!

For that reason I'd suggest to hide MR!16 and instead merge MR!17

anybody’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new921 bytes

Back to needs review for the new approach. @Grevil will take a look tomorrow and try to align the tests accordingly.

anybody’s picture

Title: Reset notice is showing up twice (from core and legal module) » Reset notice is showing up twice (from core and legal module) and don't disappear after password change
anybody’s picture

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

grevil’s picture

Status: Needs review » Needs work

Changes are looking good and everything is working as expected! I'll provide a test to avoid regression in the future.

grevil’s picture

Ok tests need a bit of refactoring. Since the user in the tests never accepted the legal terms, they will appear, once we try to reset our password. BUT this leads to the message not even appearing on the reset password page, because the "password-reset-token" is removed. (That's also why resetting a password without having the legal terms accepted before, does not currently work. See #3074688: Password can not be reset, when user hasn't accepted the legal terms yet).

grevil’s picture

Status: Needs work » Needs review

Alright, finished refactoring the tests! Please review!

Anybody changed the visibility of the branch 3414370-reset-notice-is-shown-twice to hidden.

anybody’s picture

Status: Needs review » Reviewed & tested by the community

Great work @Grevil! Thank you for reworking the tests and adding GitLab CI to show it works as expected now!

RTBC!

robert castelo’s picture

Message is not shown at all when user needs to accept T&C.

Steps:

1. Create new T&C version
2. Use one time login link
3. User is asked to accept T&C - accept
4. User is shown password reset page - no message is shown

grevil’s picture

@Robert Castelo, my apologies, we should have properly adjusted the steps to reproduce before waiting for your review. The user needs to have already accepted the T&C, before resetting their password.
Here are the steps to reproduce (I will also add them in the issue summary):

  • Install the module.
  • Create new T&C and make sure "Ask to accept T&Cs on every login" is NOT set.
  • Login as a non-admin user and accept the T&C.
  • Logout.
  • Reset the user password and login using the one time login link.
  • The T&C won't show up, as the user has already accepted them, and he will be directly forwarded to the user edit page instead.
  • The reset notice is showing up twice and after defining another password, it is still shown once.

The reset notice showing up twice:
screenshot

The reset notice still showing once, after defining a new password:
screenshot2

I hope that helps!

anybody’s picture

Status: Reviewed & tested by the community » Needs work

@Grevil: Thanks! We should also check the case @Robert Castelo described and have tests for all possible cases, so we can be sure

  1. The message appears as expected
  2. The message does not appear multiple times

Both should be checked in the tests.

As a first step the case described by @Robert Castelo should also be checked manually.

grevil’s picture

Status: Needs work » Needs review

That are exactly the tests that we implemented here. ;)

One is currently commented out and will be implemented in the follow-up issue.

grevil’s picture

Status: Needs review » Fixed

Thanks, @Robert Castelo! Don't forget to credit all people involved!

grevil’s picture

Just a tiny heads up, the phpunit pipeline currently fails: https://git.drupalcode.org/project/legal/-/jobs/1593395

But not related to any tests done here, just some legacy tests defining a deprecated theme as their test theme.

grevil’s picture

robert castelo’s picture

Regression issue fixed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.