Repository navigation
fix(security): mail password resets to the stored admin address - #77
Merged
Merged
Conversation
Cherry-pick of upstream sbpp#1607 (91b69b9). Send reset mail to the address stored on the admin row, update the token by aid, and compare the reset token byte-exact with hash_equals. `:prefix_admins.email` and `.validate` use a case- and accent- insensitive collation, so a request for `ADMIN@EXAMPLE.TEST` (or an accent variant) matched the stored row and the reset mail went to the typed variant instead of the stored address. Fork adaptations: - keep `Host::panelUrl()` (#72) for the reset link and home URL; - the lostpassword E2E seeder keeps the dedicated `lostpw-happy` admin as its default (completing a reset revokes the target admin's sessions since #63) and gains upstream's per-address argument; - AuthTest decodes HTML entities in the mail body, since template values are HTML-escaped here (#65). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-pick of upstream sbpp#1607.
Problem
:prefix_admins.emailand.validateuse a case- and accent-insensitive collation. A password-reset request forADMIN@EXAMPLE.TEST(orádmin@example.test) matched the stored row, and the reset mail, then the new password, went to the address typed in the form instead of the stored one. Anyone who could receive mail at a collation-equal variant could take over the account. The token check also ran through the same collation, so it wasn't byte-exact.Fix
auth.lost_password: select the storedemail, roll the token byaid, mail the stored address.?p=lostpassword: narrow by email in SQL, compare the token withhash_equals, send the new password to the stored address.Mailer::overrideTransportForTests()test seam + new AuthTest coverage.Fork adaptations
Host::panelUrl()(fix(mail): keep the install subfolder in emailed panel links #72) for the links.lostpw-happyadmin as default (a reset revokes the admin's sessions since fix(auth): expire and revoke browser sessions server-side #63) and gains upstream's per-address argument.Tests
AuthTest|LostPassword|Mailer: 44 tests OK (sbpp-nide stack). E2E specs not run locally (no mailpit in that stack).CHANGELOG: 2.2.1-srcdslab.