Skip to content

Make the login routes and the allowlist's reach settable - #5

Merged
JeroenDeDauw merged 21 commits into
masterfrom
feature/settable-login-routes
Aug 18, 2026
Merged

Make the login routes and the allowlist's reach settable#5
JeroenDeDauw merged 21 commits into
masterfrom
feature/settable-login-routes

Conversation

@JeroenDeDauw

@JeroenDeDauw JeroenDeDauw commented Aug 17, 2026

Copy link
Copy Markdown
Member

Follows-up to #4 (shares the login form and the captcha handling). Panel counterpart: https://github.com/ProfessionalWiki/ProAdmin/pull/1346, which makes both settings admin-editable.

Makes the login routes and the allowlist's reach settable instead of everything switching on together. No existing setting changed meaning; the defaults reproduce the behavior before this PR.

$wgMemberAccessCodeLoginallowlisted (default), open (anyone who can receive mail at the address they enter gets a read-only member account; their roster row has no group), or off (no button, no codes — held across the whole AuthManager flow, including logins already in progress). $wgMemberAccessApplyAllowlistToSsotrue by default; false leaves single sign-on entirely alone: nothing refused, nothing logged, and the accounts it creates are ordinary accounts that re-enabling does not reach, since accounts that are not members are exempt. Such accounts are absent from the roster and untouched by panel deactivation; an admin deals with them by hand (an ordinary block keeps them out). With the code route off and SSO left alone, the allowlist is manageable while governing nothing.

  • A group-less member is assigned their group at the next login once a matching entry exists, over either route; an existing group is never moved.
  • open is documented as an abuse surface: per-address limits bound one mailbox, and an address-varying attacker meets only the per-IP limits — meant for wikis behind another gate, such as an internal network.
  • Count login code requests against the member's own throttle #4's captcha-trigger relaxation now applies only while the code route exists.
  • The deactivated-member short-circuit, decoy codes, collision defence and password veto hold in every mode.

Verification: full suite and PHPStan level 9 (no baseline) green locally and on CI. The login form's new address field (from #4) has not been browser-checked on this branch — one click-through before undrafting.

Considered, omitted: a three-state SSO setting (off/allowlisted/open) that would also provision ungated SSO logins — the follow-up if the SSO-off cohort should be governed rather than documented; conditioning the other wiki-wide changes on the routes; deduplicating the code-login test helpers.

AI-authored — Claude Code, Fable 5 (max); decomposition directed by @JeroenDeDauw after his design questions, implementation and review by Opus 5 (max) subagents with findings adjudicated by Fable; diff not yet human-reviewed; TDD suite green locally and on CI, cross-PR adversarial review applied, browser check of the #4 form shape pending.

@JeroenDeDauw
JeroenDeDauw force-pushed the feature/settable-login-routes branch from 0bab83d to c1a32ca Compare August 17, 2026 22:49
@JeroenDeDauw JeroenDeDauw changed the title Make the login routes settable Make the login routes and the allowlist's reach settable Aug 17, 2026
JeroenDeDauw and others added 17 commits August 18, 2026 03:08
The roster column naming the group that admitted a member becomes nullable, so
that an account can be a member without any allowlist entry having matched it.
Nothing creates such a member yet.

Members without a group are counted overall and under no group, so the overall
count is now asked for on its own rather than added up from the per-group ones.

The table is changed in place, so the MediaWiki cache key in CI is bumped: a
restored install would otherwise keep the old table.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Loading the extension turned on one login route with one rule. $wgMemberAccessCodeLogin
now says which: "allowlisted", the previous behaviour and the default, admits the addresses
an allowlist entry matches; "open" admits every address, recording a member the allowlist
does not admit without a group, while an address an entry does match is still attributed to
that entry's group; "off" takes the route away, leaving the button off the login form and
every other way into the wiki as it was.

An open route keeps everything that is not the allowlist check: the throttles, the code
lifetime and attempt limit, one answer for every address, no mail for a deactivated member,
the audit trail, and the refusal to open an account the roster does not tie to the proven
address.

A setting nobody recognises is read as "allowlisted" and warned about once, so that a typo
leaves the wiki working the way the extension exists to work.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
$wgMemberAccessApplyAllowlistToSso says whether single sign-on logins are held to the
allowlist. It stays true, the previous behaviour: the address has to match an entry, and a
first login that does is provisioned as a member. Set to false, the route is left entirely
alone, refusing nobody and making nobody a member, which is what a wiki whose identity
provider already decides who gets in wants.

The allowlist itself is unaffected either way, so a wiki can manage it while it governs the
code login route only.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The README said loading the extension was the switch that turned members-only access on,
which is no longer what it is. A section now states what the two settings do, next to the
configuration table that names them.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The test for a member recorded without a group asserted only that the group was absent,
which an absent member row answers just as well.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Six of the seven protections an open route keeps already have a home earlier in the file,
where the route is described in full, so the section now says only that the allowlist check
is the one thing that changes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mutation testing found two holes. The test for a code entered while the route is off passed
whether or not the route was consulted, since the failure it asserted is also what a code
entry without a request in the session gets; it now asks for a code first, takes the route
away, and enters that code. And nothing held an open route to the defence that keeps a proven
mailbox from opening an account the roster does not tie to it, which is the route where
anybody can start a login.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A member the open code login route admitted has no group. An allowlist entry
added afterwards said which group admits them, but nothing ever wrote that
down, so the roster kept counting them as ungrouped for good.

Their next login now writes it, over either login route, where the matched
group is already in hand. The group a member already has is never moved: the
repository makes that its own condition, so two logins arriving together
cannot each read no group and then write a different one.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The reading of $wgMemberAccessCodeLogin is held on to so the warning about an
unrecognised value is said once rather than on every read. The extension
outlives the test that caused it, so a test asserting the warning was said
could be handed a reading another test had already made, and see nothing.

Nothing had broken yet only because the tests happened to use two different
unrecognised strings. They now share one, and the reading is dropped in
tearDown.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three gaps in what the tests hold on to:

The rule deciding whom a route admits was only ever reached through a login,
so which of its six answers changed was guesswork. It is now asserted directly.

The per-group totals test with a member outside every group asserted how many
groups the breakdown named, which is how many groups exist. It now runs two
groups against each other, so a breakdown lumping the ungrouped member in
anywhere is caught wherever it puts them.

The session data a login is provisioned from is written by two routes and
survives requests, and nothing covered it coming back as anything but what was
put there.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Turning the allowlist off single sign-on reads as a switch, and turning a
switch back is meant to undo it. It does not: the accounts made while it was
off are no members, and accounts that are no members are exempt, so they keep
their access whatever the switch is set to afterwards.

Also qualifies the sentence promising that narrowing a route ends access at
the next login. It holds for the whole code route, and on single sign-on for
the members alone.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The setting table said whom an open route admits without saying what that
costs: an account and a roster row for anyone with a working mailbox, and
throttles that bound what can be aimed at one address but not an attacker who
varies it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The test for session data that is no array passed a string, which reads a key
off as nothing, exactly as the guard does. It now passes an object, which
reading a key off is an error, so the guard is what the test needs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both were called Admits and assert a refusal.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Loading the extension turned ConfirmEdit's per-address bad-login trigger off wiki-wide, so that
whether a code request met a captcha could not tell an address the allowlist admits from one it
does not. A wiki with the code route off has no such request to protect, and paid for it with
ConfirmEdit's per-account escalation on every password login it does have. The trigger is left
alone there.

The list of what loading the extension changes no longer claims to hold whatever the routes are
set to, since this one item no longer does.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A text pass over this round's README sections, plus: the enable-time
claims in the intro and the route-narrowing sentence no longer
generalize over the SSO switch, the unknown-setting fallback is
documented, and the wiki-wide changes are stated not to depend on the
routes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Master's release notes state the allowlist over single sign-on as
absolute, which this branch makes a default, and list no route setting.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@JeroenDeDauw
JeroenDeDauw force-pushed the feature/settable-login-routes branch from f8def9f to 401a3e2 Compare August 18, 2026 01:10
@codecov-commenter

codecov-commenter commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.22%. Comparing base (87dadb0) to head (37ba174).

Files with missing lines Patch % Lines
src/Persistence/DatabaseRepository.php 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master       #5      +/-   ##
============================================
+ Coverage     82.63%   86.22%   +3.59%     
- Complexity      455      481      +26     
============================================
  Files            57       58       +1     
  Lines          1572     1626      +54     
============================================
+ Hits           1299     1402     +103     
+ Misses          273      224      -49     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

JeroenDeDauw and others added 4 commits August 18, 2026 03:22
Members no group admitted have to be in the overall count and in none of
the per-group ones. Excluding them from the grouped count and asking for
the overall one separately took a GET of the roster, of the groups, and
every group delete from two queries to four, one of them a full table
scan, since no index covers the deactivation column. Grouping already
puts them in a bucket of their own, which the total adds up and the
breakdown leaves out.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The reading was held on to so that an unknown value was warned about
once rather than twice. A request is a fresh process, so it never
stopped a misconfigured wiki from logging on every page view; what it
did buy was a public method to forget the reading again, without which
one test handed the next a warning it had already said.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Core sets a group permission for the duration of a test in one line,
and recording a member without a group is recording a member.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Read as a restatement of the rule the repository holds, it invites
removal. It is there to keep an ordinary login from writing at all.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@JeroenDeDauw
JeroenDeDauw marked this pull request as ready for review August 18, 2026 15:03
@JeroenDeDauw
JeroenDeDauw merged commit 69de93a into master Aug 18, 2026
6 checks passed
@JeroenDeDauw
JeroenDeDauw deleted the feature/settable-login-routes branch August 18, 2026 15:03
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.

2 participants