Password can not be reset, when user hasn't accepted the legal terms yet

I created a new MR for 3.0.x.

The patch in #28 no longer applies when using 3.0.1 and the older issue is still occurring - if there is a T&C change to accept when a user also needs to do a password reset, this user will see "You are not authorized to access this page. " and the logs will show:

Path: /legal_accept?destination=/user/6393/edit&token=Y4TDAQZgW2GfBznyr3WLYHSzLF-7cHrU6OmrNRC6SuQ. Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException: in Drupal\Core\Routing\AccessAwareRouter->checkAccess() (line 121 of /var/www/html/drupal/web/core/lib/Drupal/Core/Routing/AccessAwareRouter.php).

Curious though, I have been unable to reproduce this now, with different users. Maybe was a browser cache issue. Fresh browser, fresh user and i am able to reset the password and accept a new T&C.

I've created a patch from the MR in #37 for Legal 3.0.1

Could someone, who is running into this issue, please add clear steps to the issue summary how to reproduce this in 3.0.x?
In which cases does this happen?
Does the switch in #31 affect this issue?

This should definitely be fixed and I think it's major if it still happens as described, but the issue summary is poor for reproduction.

I updated the issue summary to provide clear steps on how to reproduce this issue in 3.0.x.

Does the switch in #31 affect this issue?

It doesn't, but it generally seems, that this issue is not as bad as before, since when the user already accepted the legal terms, the issue won't come up.

"3074688-password-reset-links-3.0.x" doesn't look finished yet. I can't find the time to have a detailed look into this, but I can tell, that this still needs work.

The old MR 7 can be closed I guess.

It doesn't, but it generally seems, that this issue is not as bad as before, since when the user already accepted the legal terms, the issue won't come up.

Ok, I didn't think about changing legal terms, which need to get accepted once again. As this is quite common and will trigger this issue, I'll set this issue to major.

Ok, I added "testPasswordResetWithoutLegalAccepted()", "testPasswordResetWithLegalAccepted()" and generally refactored the tests in 🐛 Reset notice is showing up twice (from core and legal module) and don't disappear after password change RTBC , because that issue is also indirectly affected by the problem described inside this issue.

Test-wise, we should wait for that issue to get merged, before we touch the tests here.

Alright, the provided patch works great!

Let's wait for 🐛 Reset notice is showing up twice (from core and legal module) and don't disappear after password change RTBC to get merged, so we can start with the remaining test and check if this MR introduces any regressions.

POSTPONED on 🐛 Reset notice is showing up twice (from core and legal module) and don't disappear after password change RTBC

Grevil → changed the visibility of the branch 3074688-password-reset-links to hidden.

@Grevil I guess you mean MR!11?

@Anybody when exactly?

The old MR 7 can be closed I guess.

?

I meant MR 7 there.

@Grevil not sure I understand this issue....manually testing I wasn't able to reproduce the issue at all.

Am I right in thinking that this is now just an issue with automated tests?

@Ron Collins which version of the module did you test? Did you test with any other patches / MR's applied?

Am I right in thinking that this is now just an issue with automated tests?

I don't think so. This issue exists in 3.0.x. We'll add clear steps to reproduce this. Like in 🐛 Reset notice is showing up twice (from core and legal module) and don't disappear after password change RTBC we have to ensure we describe all possible paths and have tests for them.

@Robert Castelo, I can provide more specific steps to reproduce if needed!

Steps to reproduce:

• Install the module.
• Create new T&C.
• Reset the password of a non-admin user who has NOT accepted the T&C yet.
• Use the one time login link, and you will be forwarded to the Password reset page:

• After pressing Login, the T&C need to be accepted first, as the user hasn't accepted them yet:

• Accept the terms, and you will now be forwarded to the user edit page. BUT it is required for you to enter your current password to change the password. As the user obviously forgot it (hence he is trying to reset his password), he can not change it and needs to get a new one time login link, if he wants to log in ever again:

@anybody 3.0.1 is installed.

There was one other patch applied: https://www.drupal.org/files/issues/2023-01-23/3283807-8-deprecated-func... →

@Robert Castelo any reason this was merged without any status change at all?

This issue was still missing test implementations. Also, no credits were given yet. @danielspeicher, @jurgenhaas and others contributed quite a bit here!

I'll create a follow-up issue for fixing / adding the missing tests.

@Grevil I've merged but still need to make some changes before I consider this issue resolved, which I'm currently working on.

I've given attribution when I merged, maybe that commit is still local and I need to push to origin.

Commit:
e99a8547c3f9d5cee4845be049b7f2acabec9274 [e99a854]

Issue #3074688 by johne, jurgenhaas, ankithashetty, danielspeicher, tobiasb, Grevil, JayDarnell, Ron Collins, mrinalini9, fjgarlin: [PP-1] Password can not be reset, when user hasn't accepted the legal terms yet

Ok.

Credit through commit isn't enough anymore, you need to additionally credit users through the "Credit & commiting" section when adding a new comment in the issue.

@Robert Castelo I finished the tests in 🐛 Fix and add remaining test Needs work now.

@Grevil I fixed the tests and also an issue with the one time login message not appearing on the password reset page.

I already credited everyone who contributed code in the "Credit & committing" section when merging the issue through the issue page, let me know if there's anything else that needs to be done to give credit.

Everyone seems to be properly credited now, thanks! :)

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