Make the login routes and the allowlist's reach settable - #5
Merged
Conversation
JeroenDeDauw
force-pushed
the
feature/settable-login-routes
branch
from
August 17, 2026 22:49
0bab83d to
c1a32ca
Compare
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
force-pushed
the
feature/settable-login-routes
branch
from
August 18, 2026 01:10
f8def9f to
401a3e2
Compare
Codecov Report❌ Patch coverage is
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. 🚀 New features to boost your workflow:
|
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>
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.
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.
$wgMemberAccessCodeLogin—allowlisted(default),open(anyone who can receive mail at the address they enter gets a read-only member account; their roster row has no group), oroff(no button, no codes — held across the whole AuthManager flow, including logins already in progress).$wgMemberAccessApplyAllowlistToSso—trueby default;falseleaves 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.openis 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.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.