[3.0] Lets something other than a password log a member in - #9381
Open
albertlast wants to merge 1 commit into
Open
[3.0] Lets something other than a password log a member in#9381albertlast wants to merge 1 commit into
albertlast wants to merge 1 commit into
Conversation
Everything about signing in assumes the password form did it. The steps that follow a successful check live in Login2::DoLogin(), which is protected and reads its member from a private property, so nothing else can reuse them; two factor authentication is looked up by reading the tfa_secret column wherever the question comes up; and every account is assumed to have a password worth asking for. None of that is a problem until something else can vouch for a member, at which point each one has to be worked around rather than used. So: Moves the body of DoLogin() to Login2::completeLogin(), taking the member and the cookie lifetime as arguments. DoLogin() now just calls it, so the password path is unchanged, and anything else that authenticates a member can finish the job the same way instead of setting the cookie by hand and missing the ban check or the login history. Adds User::getSecondFactors(), which reports the factors a member has and lets a mod add its own, and asks it instead of reading tfa_secret. It reads the loaded profile rather than object properties because verifyTfa() runs before setProperties() does. Checking it in Login2::checkCookie() now also checks tfa_mode, as verifyTfa() already did; without that a member could be sent to ?action=logintfa when nothing was going to ask them for a code, which ends in "You are not allowed to access this section" rather than a login. Adds User::hasUsablePassword() for accounts that have no password to give. The login form refuses them before the legacy hash fallbacks get to compare anything against an empty string, and validateSession() offers integrate_reauthenticate so such a member is not simply locked out of the admin areas. Nothing here creates such an account yet. Adds a member_auth table for whatever credentials those accounts sign in with, dropped along with the member, and a login form slot that renders the methods registered through integrate_authentication_methods. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Collaborator
Author
|
@Sesquipedalian you mention you would like to see passkeys to work this would be the base of this idea, |
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.
Description
Groundwork so that something other than the password form can log a member in. No new
feature here, and nothing user visible on a stock install — this is the part that OpenID
Connect and passkeys would both otherwise have to work around, split out so it can be
reviewed on its own.
Three things currently assume the password form:
Login2::DoLogin()callsintegrate_login, sets the cookie, resets the flood counter, enforces bans, writesmember_loginsand redirects — but it isprotectedand reads$this->member, soanything else authenticating a member has to reimplement it and will quietly miss a
step. Moved to
Login2::completeLogin($member, $stay_logged_in, $redirect);DoLogin()is now a one line caller, so the password path is byte for byte the same.tfa_secret. Every place that asks "does this memberhave a second factor" reads that column. Added
User::getSecondFactors(), whichreports what they have and lets a mod register its own via
integrate_second_factors.It reads the loaded profile rather than object properties, because
verifyTfa()runsbefore
setProperties()has populated anything.User::hasUsablePassword(). The login form now refuses such an account beforecheckPasswordFallbacks()compares the submitted password against an empty hash, andvalidateSession()offersintegrate_reauthenticateso a member who signs in someother way is not simply locked out of the admin and moderation areas. Nothing in SMF
creates such an account yet; this is the seam for whatever does.
Also adds a
member_authtable for the credentials such accounts would sign in with(deleted along with the member), and a slot on the login form that renders whatever
registers through
integrate_authentication_methods.One behaviour change worth flagging.
Login2::checkCookie()now checkstfa_modeaswell as the member, which
User::verifyTfa()already did. Previously a member with atfa_secreton a forum with TFA switched off was redirected to?action=logintfa, wherenothing was going to ask them for a code — they got "You are not allowed to access this
section" instead of being logged in. They now log in normally. This is not a weakening:
with
tfa_modeoff,verifyTfa()was not enforcing anything either way.New hooks:
integrate_second_factors,integrate_authentication_methods,integrate_reauthenticate. Each is documented at its call site, since there is no centralhook list to add them to.
Testing. A 14 check regression run over the login paths this touches — form renders,
correct and incorrect password, remember me and its cookie lifetime, logout, the admin
password re-prompt and passing it, a member with TOTP being held at the second factor, and
an account with an empty
passwdbeing refused — passes identically on this branch and onunmodified
release-3.0, which is the point for a change that is meant to alter nothing.Run on both MySQL and PostgreSQL. The new table was checked on both engines, created by
the installer on a fresh install (73 tables) and by the migration on an existing one. The
two new extension points were exercised with a throwaway mod: a registered method renders
on the login form, and a registered factor holds back a member who has no
tfa_secretatall.
Noted while testing, not touched here:
?action=logintfais already broken onrelease-3.0independently of this.LoginTFA::execute()doesUser::load(..., dataset: User::$me->dataset), butverifyTfa()has just reset the memberto a guest, whose
datasetis null, so it dies with "Cannot assign null to propertySMF\User::$dataset". Reproduced identically with and without this branch. Worth its own PR.
Issues References (Fixes|Related|Closes)