Repository navigation
Conversation
jmnote
left a comment
There was a problem hiding this comment.
Reviewed the OAuth setup, the smoke test, and the lock changes. The direction looks good. My main concern is rollout safety; the rest are smaller.
- Unconditional load makes rollout order-sensitive (inline on
BaseExtensionSettings.php). Any environment whose keys or schema update are not ready yet still gets OAuth loaded, with empty keys. - The smoke test does not exercise this PR's settings (inline on
oauth-smoke-container.sh). hack/mediawiki-composer.lockalso upgrades unrelated packages. Besides the new OAuth dependencies (league/oauth2-serverdev-v9.0.0-alpha,lcobucci/jwt,firebase/php-jwt,defuse/php-encryption, …), it bumpsaws/aws-sdk-php3.399.1 → 3.403.1 and several Symfony components (7.4.18/7.4.20 → 7.4.22/7.4.23, 3.7.x → 3.8.1). These are probably harmless, but please mention them in the summary. Or regenerate with only the OAuth packages updated, so a regression can be traced to one change.- VERSION 0.9.81 → 0.9.83 assumes #482 ships 0.9.82 first. If this PR merges first, the numbering needs a fix-up.
- PR description: the rollout section names an internal deployment repository. Please reword it in generic terms (e.g. "deployment configuration"), since this repository is public.
| wfLoadExtension('CrawlerProtection'); | ||
| wfLoadExtension('EmbedVideo'); | ||
| wfLoadExtension('MsUpload'); | ||
| wfLoadExtension('OAuth'); |
There was a problem hiding this comment.
OAuth is loaded even when MW_OAUTH2_PRIVATE_KEY/MW_OAUTH2_PUBLIC_KEY are unset (they fall back to ''). Then any environment without keys, or one where the schema update has not run yet, exposes OAuth special pages and REST endpoints that fail at runtime (empty key at token issuance, missing oauth_* tables).
Suggestion: load OAuth only when both keys are present. Each environment then opts in once its keys are provisioned and the schema is updated, so the image can roll out before that coordinated step without breaking anything:
$oauth2PrivateKey = getenv('MW_OAUTH2_PRIVATE_KEY');
$oauth2PublicKey = getenv('MW_OAUTH2_PUBLIC_KEY');
if ($oauth2PrivateKey && $oauth2PublicKey) {
wfLoadExtension('OAuth');
$wgOAuth2PrivateKey = $oauth2PrivateKey;
$wgOAuth2PublicKey = $oauth2PublicKey;
$wgGroupPermissions['user']['mwoauthproposeconsumer'] = true;
$wgGroupPermissions['user']['mwoauthupdateownconsumer'] = true;
}The docs line in docs/config.md would then say OAuth is enabled when both keys are set.
|
|
||
| $wgOAuth2PrivateKey = getenv('MW_OAUTH2_PRIVATE_KEY') ?: ''; | ||
| $wgOAuth2PublicKey = getenv('MW_OAUTH2_PUBLIC_KEY') ?: ''; | ||
| $wgGroupPermissions['user']['mwoauthproposeconsumer'] = true; |
There was a problem hiding this comment.
With mwoauthproposeconsumer on user, users can also submit regular (non-owner-only) consumers. Those go to the approval queue, and no group here has mwoauthmanageconsumer, so the proposals would stay pending forever. If only owner-only tokens are intended, say so in the docs. Alternatively, grant mwoauthmanageconsumer to sysop so proposals can be approved or rejected.
| openssl rsa -in /tmp/oauth-private.pem -pubout -out /tmp/oauth-public.pem 2>/dev/null | ||
| export MW_OAUTH2_PRIVATE_KEY="$(cat /tmp/oauth-private.pem)" | ||
| export MW_OAUTH2_PUBLIC_KEY="$(cat /tmp/oauth-public.pem)" | ||
| cat >> LocalSettings.php <<'PHP' |
There was a problem hiding this comment.
The smoke test appends its own copy of the OAuth settings to a freshly installed LocalSettings.php instead of using mwz/settings/BaseExtensionSettings.php. So it checks upstream OAuth behaviour, but not this PR's configuration: the ?: '' fallback, the permission lines, or a future conditional load. If the base image makes it practical, include the repo's settings file (or at least the OAuth block from it) so the two cannot drift.
Also minor: the container runs with --rm and the install/update/server logs go to /tmp. When a step fails, set -e exits with no useful output. A trap 'tail -n 50 /tmp/*.log' ERR would make CI and local failures diagnosable.
Summary
Enable the MediaWiki OAuth extension so users can issue owner-only OAuth 2 tokens for API access.
REL1_43through the extension lock and configure OAuth 2 signing keys through environment variables.Verification
make extensions-lockandnode hack/extensions.mjs checkmake composer-lockhack/oauth-smoke.sh, including missing-key defaults and the temporary MediaWiki schema update.Rollout and dependency notes
make composer-lockupdates unrelated packages alongside OAuth dependencies, includingaws/aws-sdk-php3.399.1 → 3.403.1 and several Symfony components. These changes are part of this lock regeneration.