Skip to content

[3.0] Lets something other than a password log a member in - #9381

Open
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:3.0/auth-foundation
Open

[3.0] Lets something other than a password log a member in#9381
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:3.0/auth-foundation

Conversation

@albertlast

Copy link
Copy Markdown
Collaborator

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:

  1. The steps after a successful check are unreachable. Login2::DoLogin() calls
    integrate_login, sets the cookie, resets the flood counter, enforces bans, writes
    member_logins and redirects — but it is protected and reads $this->member, so
    anything 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.
  2. Two factor authentication is tfa_secret. Every place that asks "does this member
    have a second factor" reads that column. Added User::getSecondFactors(), which
    reports 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() runs
    before setProperties() has populated anything.
  3. Every account is assumed to have a password. Added
    User::hasUsablePassword(). The login form now refuses such an account before
    checkPasswordFallbacks() compares the submitted password against an empty hash, and
    validateSession() offers integrate_reauthenticate so a member who signs in some
    other 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_auth table 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 checks tfa_mode as
well as the member, which User::verifyTfa() already did. Previously a member with a
tfa_secret on a forum with TFA switched off was redirected to ?action=logintfa, where
nothing 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_mode off, 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 central
hook 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 passwd being refused — passes identically on this branch and on
unmodified 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_secret at
all.

Noted while testing, not touched here: ?action=logintfa is already broken on
release-3.0 independently of this. LoginTFA::execute() does
User::load(..., dataset: User::$me->dataset), but verifyTfa() has just reset the member
to a guest, whose dataset is null, so it dies with "Cannot assign null to property
SMF\User::$dataset". Reproduced identically with and without this branch. Worth its own PR.

Issues References (Fixes|Related|Closes)

  1. Groundwork for OpenID Connect and passkey support

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>
@albertlast

Copy link
Copy Markdown
Collaborator Author

@Sesquipedalian you mention you would like to see passkeys to work this would be the base of this idea,
some addiotnal pr will be in the pipeline

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant