Skip to content

Apply the activating wiki changes only while a login route needs them - #11

Merged
JeroenDeDauw merged 1 commit into
masterfrom
feature/route-following-side-effects
Aug 18, 2026
Merged

Apply the activating wiki changes only while a login route needs them#11
JeroenDeDauw merged 1 commit into
masterfrom
feature/route-following-side-effects

Conversation

@JeroenDeDauw

@JeroenDeDauw JeroenDeDauw commented Aug 18, 2026

Copy link
Copy Markdown
Member

Follows-up to #5. Panel counterpart: https://github.com/ProfessionalWiki/ProAdmin/pull/1356, which makes loading follow the wiki's plan — this PR is what makes that safe.

Splits what loading the extension changes into two groups, so that a loaded extension with no route turned on changes nothing an admin has to think about:

  • Always applied while loaded, because they protect members and data that may already exist: the reader-group revocations, $wgBlockDisablesLogin, the newusers/block/renameuser log restrictions, and the @-in-usernames allowance with the user-rights delimiter change.
  • Route-following: the ConfirmEdit captcha-trigger relaxation exists only while the code route does; autocreateaccount for anonymous visitors and the session-duration override exist while any route can log a member in — the code route offered, or the allowlist applied to single sign-on.

The shipped defaults flip to match: $wgMemberAccessCodeLogin defaults to off and $wgMemberAccessApplyAllowlistToSso to false, so activation is always an explicit setting. Only an explicit true gates single sign-on, and an unrecognised or empty route value now reads as off (warning for junk, none for empty) — in a default-off world a typo must not activate a login route. Deployments therefore set both values deliberately; the installation section shows the line, and the dev environment seeds them (https://github.com/ProfessionalWiki/prowiki-docker/pull/1958).

From review: the first cut gated autocreateaccount on the code route alone, reasoning that PluggableAuth grants the right itself. It does not — the requirement is core's AuthManager, identical for both member routes — so a wiki whose only member route was single sign-on could not create its first member account. The grant now follows the member routes, like the session duration; a wiki running single sign-on outside the allowlist is running plain PluggableAuth and grants what that needs itself. The README's always-applied list now also names the password refusal and the closed account-listing modules, and the outgoing-email requirement follows the code route.

Rebased over the removal endpoint (#12): the renameuser restriction joins the always-applied group, and the removal tests pin the code route on, as every other login test now does.

Verification: 660 tests green locally in both runs — as configured, and with both globals unset so the shipped defaults are actually exercised, which is how three dozen tests silently riding the old defaults were found and pinned. The route-state matrix covers each of off/allowlisted/open/junk/empty × SSO on/off, asserting both what is applied and what is not.

Considered, omitted: a "no button by default" browser assertion (any wiki that configures the route falsifies it — the manifest default test plus the explicit-off test compose the claim).

AI-authored — Claude Code, Fable 5 (max); design directed by @JeroenDeDauw in review discussion, implementation by Opus 5 (max) subagents, defaults flip and fallback change directed mid-round; the review pass that platform degradation previously prevented has since run (Fable 5 (max) orchestrating Opus 5 (max) checks) — its one blocking finding, the single-sign-on autocreation gap, is fixed here; mutation-checked, 660 green in default and configured runs, phpcs and phpstan clean; diff not yet human-reviewed.

@codecov-commenter

codecov-commenter commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.83%. Comparing base (12cd570) to head (f6fbd80).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master      #11      +/-   ##
============================================
+ Coverage     85.77%   85.83%   +0.06%     
- Complexity      512      519       +7     
============================================
  Files            61       61              
  Lines          1785     1793       +8     
============================================
+ Hits           1531     1539       +8     
  Misses          254      254              

☔ 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 JeroenDeDauw changed the title Hold the wiki-wide changes to the routes that need them Apply the activating wiki changes only while a login route needs them Aug 18, 2026
Follows-up to #5. Panel counterpart: ProfessionalWiki/ProAdmin#1356, which makes loading follow the wiki's plan — this PR is what makes that safe.

Splits what loading the extension changes into two groups, so that a loaded extension with no route turned on changes nothing an admin has to think about:

* **Always applied while loaded**, because they protect members and data that may already exist: the reader-group revocations, `$wgBlockDisablesLogin`, the `newusers`/`block`/`renameuser` log restrictions, and the `@`-in-usernames allowance with the user-rights delimiter change.
* **Route-following**: the ConfirmEdit captcha-trigger relaxation exists only while the code route does; `autocreateaccount` for anonymous visitors and the session-duration override exist while any route can log a member in — the code route offered, or the allowlist applied to single sign-on.

The shipped defaults flip to match: `$wgMemberAccessCodeLogin` defaults to `off` and `$wgMemberAccessApplyAllowlistToSso` to `false`, so activation is always an explicit setting. Only an explicit `true` gates single sign-on, and an unrecognised or empty route value now reads as `off` (warning for junk, none for empty) — in a default-off world a typo must not activate a login route. Deployments therefore set both values deliberately; the installation section shows the line, and the dev environment seeds them (ProfessionalWiki/prowiki-docker#1958).

From review: the first cut gated `autocreateaccount` on the code route alone, reasoning that PluggableAuth grants the right itself. It does not — the requirement is core's `AuthManager`, identical for both member routes — so a wiki whose only member route was single sign-on could not create its first member account. The grant now follows the member routes, like the session duration; a wiki running single sign-on outside the allowlist is running plain PluggableAuth and grants what that needs itself. The README's always-applied list now also names the password refusal and the closed account-listing modules, and the outgoing-email requirement follows the code route.

Rebased over the removal endpoint (#12): the `renameuser` restriction joins the always-applied group, and the removal tests pin the code route on, as every other login test now does.

Verification: 660 tests green locally in both runs — as configured, and with both globals unset so the shipped defaults are actually exercised, which is how three dozen tests silently riding the old defaults were found and pinned. The route-state matrix covers each of `off`/`allowlisted`/`open`/junk/empty × SSO on/off, asserting both what is applied and what is not.

Considered, omitted: a "no button by default" browser assertion (any wiki that configures the route falsifies it — the manifest default test plus the explicit-`off` test compose the claim).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@JeroenDeDauw
JeroenDeDauw force-pushed the feature/route-following-side-effects branch from a08307b to f6fbd80 Compare August 18, 2026 20:21
@JeroenDeDauw
JeroenDeDauw marked this pull request as ready for review August 18, 2026 20:22
@JeroenDeDauw
JeroenDeDauw merged commit d1d538a into master Aug 18, 2026
6 checks passed
@JeroenDeDauw
JeroenDeDauw deleted the feature/route-following-side-effects branch August 18, 2026 20:22
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