Skip to content

Allow pending invitees to recover account access - #24911

Merged
chippison merged 3 commits into
matomo-org:5.x-devfrom
drewmt:fix/pending-invite-password-recovery
Jul 29, 2026
Merged

Allow pending invitees to recover account access#24911
chippison merged 3 commits into
matomo-org:5.x-devfrom
drewmt:fix/pending-invite-password-recovery

Conversation

@drewmt

@drewmt drewmt commented Jul 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Pending invitees currently reach the password recovery form because they have not set a password yet, but the flow treats them as invalid and sends no email.

This change lets an invitee with an unexpired pending invitation receive a fresh activation link from the recovery flow. The link points to acceptInvitation, preserves the existing invite_expired_at value, and shows the remaining validity in the email. Expired or malformed invitations still fail closed, while active users keep the existing password reset and cancellation flow.

Recovery now atomically replaces the pending user's existing invitation token instead of creating a second valid activation credential. The update succeeds only when the login, previous token, email, and unexpired pending state still match. If acceptance, expiry, or an email reassignment wins the race, no stale link is sent and the temporary reset state is removed.

Re-inviting a pending user or changing the invitation email also clears any previously generated recovery link, so the previous mailbox owner cannot complete an outdated invitation.

The existing recovery request limit remains in place.

Fixes #20297

Validation

  • Failing-first regressions for preserving the original token and for an old recovery link surviving pending-email reassignment.
  • Focused recovery, reassignment, acceptance/expiry race, and email-change race coverage: 5 tests, 28 assertions passed.
  • Full PasswordResetterTest: 17 tests, 77 assertions passed.
  • Full Login ControllerTest: 7 tests, 9 assertions passed.
  • Existing UsersManager email-change/re-invite integration test: 1 test, 4 assertions passed.
  • Full UserInviteTest, including local HTTP acceptance and decline routes: 2 tests, 9 assertions passed.
  • PHPCS passed for all changed PHP files.
  • Targeted PHPStan passed with no errors.
  • PHP syntax and git diff --check passed.

Checklist

  • [✔] I have understood, reviewed, and tested all AI outputs before use
  • [✔] All AI instructions respect security, IP, and privacy rules

@tzi tzi added this to the 6.0.0 milestone Jul 26, 2026
@tzi
tzi changed the base branch from 5.x-dev to 6.x-dev July 26, 2026 21:12
@tzi
tzi changed the base branch from 6.x-dev to 5.x-dev July 26, 2026 21:14
@tzi tzi modified the milestones: 6.0.0, 5.13.0 Jul 26, 2026
@tzi

tzi commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Could you please add a AI checklist to your PR description?
It's mandatory to pass our CI.

### Checklist

- [✔/✖/NA] I have understood, reviewed, and tested all AI outputs before use
- [✔/✖/NA] All AI instructions respect security, IP, and privacy rules

@chippison
chippison requested a review from a team July 26, 2026 23:38
chippison
chippison requested changes Jul 27, 2026
View reviewed changes

@chippison chippison left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @drewmt

Thanks for looking into this. 👍
There is 1 thing that needs fixing below, and also please update the ###AI Checklist above that we need. This is a requirement for our repo now especially when AI was used for this fix. You can just leave it NA if you did not use AI for this.

Also 2 other minor things I found:

  1. The initiatePasswordResetProcess docblock (and the class docblock) still say the email contains a password-reset token — that's no longer true for the invitation branch, so it's worth updating.
  2. The new fallback in getUserInformationForPasswordResetInitiation calls findUserByLoginOrEmail() directly instead of going through getUserInformation(), which plugins are meant to be able to override. Routing it through an overridable method would keep that extension point intact.

Comment thread plugins/UsersManager/Repository/UserRepository.php Outdated
chippison
chippison reviewed Jul 28, 2026
View reviewed changes

@chippison chippison left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @drewmt ,

Aside from below, can I also ask why we still need the old(original) email link to be not be invalidated (and the original invitation token is not invalidated.)?

Would it be more simple to just invalidate the original email token when a new one is created?

Comment thread plugins/Login/PasswordResetter.php Outdated
@drewmt

drewmt commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Yes, agreed — the updated flow now keeps a single current activation token. Recovery replaces the original invitation token instead of creating a second valid credential, while preserving the existing invitation expiry. I also updated the PR description and added coverage for reassignment and concurrent email-change cases.

chippison
chippison approved these changes Jul 28, 2026
View reviewed changes

@chippison chippison left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good now. 👍

Thanks again for this @drewmt

@chippison
chippison merged commit f9807a2 into matomo-org:5.x-dev Jul 29, 2026
172 of 173 checks passed
@tzi tzi mentioned this pull request Aug 5, 2026
Merged
chippison added a commit that referenced this pull request Aug 5, 2026
@chippison @drewmt @claude
Allow pending invitees to recover account access (forward port of #24911
783bf78 
) (#24944)

Co-authored-by: drewmt <idrew.mt@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chippison chippison chippison approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

5.13.0

Development

Successfully merging this pull request may close these issues.

Users | Pending invitees cannot resume account activation through “Lost your password?”

3 participants

@drewmt @tzi @chippison