Skip to content

Commit 4dfbb9d

Browse files
authored
Yearly cleanup (#33)
Notable changes: Fix running with PostgreSQL
1 parent cc717d4 commit 4dfbb9d

18 files changed

+97
-85
lines changed

.github/workflows/php.yml

+11-14
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,12 @@ permissions:
1010
contents: read
1111

1212
jobs:
13-
build:
14-
13+
test:
1514
runs-on: ubuntu-latest
16-
1715
steps:
18-
- run: pwd
1916
- uses: actions/checkout@v3
20-
2117
- name: Validate composer.json and composer.lock
2218
run: composer validate --strict
23-
2419
- name: Cache Composer packages
2520
id: composer-cache
2621
uses: actions/cache@v3
@@ -29,15 +24,17 @@ jobs:
2924
key: ${{ runner.os }}-php-${{ hashFiles('**/composer.lock') }}
3025
restore-keys: |
3126
${{ runner.os }}-php-
32-
3327
- name: Install dependencies
34-
run: composer install --no-dev --prefer-dist --no-progress
35-
36-
# Add a test script to composer.json, for instance: "test": "vendor/bin/phpunit"
37-
# Docs: https://getcomposer.org/doc/articles/scripts.md
38-
39-
# - name: Run test suite
40-
# run: composer run-script test
28+
run: composer install
29+
- name: Run test suite
30+
run: composer run-script test
31+
publish:
32+
needs: test
33+
runs-on: ubuntu-latest
34+
steps:
35+
- uses: actions/checkout@v3
36+
- name: Install dependencies
37+
run: composer install --no-dev
4138
- uses: actions/upload-artifact@v3
4239
with:
4340
name: AuthManagerOAuth

.gitignore

-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,2 @@
11
/composer.lock
22
/vendor
3-
/node_modules
4-
.eslintcache

.gitreview

-6
This file was deleted.

.phan/config.php

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<?php
2+
3+
return require __DIR__ . '/../vendor/mediawiki/mediawiki-phan-config/src/config.php';

.phpcs.xml

+2
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,6 @@
22
<ruleset>
33
<rule ref="./vendor/mediawiki/mediawiki-codesniffer/MediaWiki" />
44
<file>.</file>
5+
<arg name="extensions" value="php"/>
6+
<arg name="encoding" value="UTF-8"/>
57
</ruleset>

.stylelintrc.json

-3
This file was deleted.

SECURITY.md

-7
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,6 @@ currently being supported with security updates.
1313

1414
Please report security vulnerabilites to [email protected].
1515

16-
You can use PGP encryption if you want.
17-
My public key is at https://keys.openpgp.org/search?q=Moritz.Hedtke%40t-online.de.
18-
19-
You can import it using: `gpg --keyserver keys.openpgp.org --recv-key 0x1248D3E11D114A8575C989346794D45A488C2EDE`
20-
21-
My fingerprint is: 1248 D3E1 1D11 4A85 75C9 8934 6794 D45A 488C 2EDE
22-
2316
You can expect me to respond within a few days but please be patient if it takes longer.
2417

2518
Please provide as much details as you can. If you have one a proof of concept would be great.

composer.json

+4-6
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,9 @@
22
"name": "mohe2015/authmanageroauth",
33
"description": "Create accounts or login using OAuth",
44
"license": "GPL-2.0-or-later",
5-
"config": {
6-
"autoloader-suffix": "authmanageroauth"
7-
},
85
"require-dev": {
9-
"mediawiki/mediawiki-codesniffer": "38.0.0",
6+
"mediawiki/mediawiki-codesniffer": "42.0.0",
7+
"mediawiki/mediawiki-phan-config": "0.13.0",
108
"mediawiki/minus-x": "1.1.1",
119
"php-parallel-lint/php-console-highlighter": "1.0.0",
1210
"php-parallel-lint/php-parallel-lint": "1.3.2"
@@ -21,10 +19,10 @@
2119
"minus-x fix .",
2220
"phpcbf"
2321
],
22+
"phan": "phan -d . --long-progress-bar",
2423
"phpcs": "phpcs -sp --cache"
2524
},
2625
"require": {
27-
"league/oauth2-client": "^2.6",
28-
"psr/container": "^1.0.0"
26+
"league/oauth2-client": "^2.7"
2927
}
3028
}

extension.json

+7-7
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
"MediaWiki": ">= 1.35.0"
1212
},
1313
"AutoloadNamespaces": {
14-
"MediaWiki\\Extension\\AuthManagerOAuth\\": "includes/"
14+
"MediaWiki\\Extension\\AuthManagerOAuth\\": "src/"
1515
},
1616
"load_composer_autoloader": true,
1717
"config": {
@@ -39,12 +39,12 @@
3939
]
4040
},
4141
"AuthManagerAutoConfig": {
42-
"primaryauth": {
43-
"MediaWiki\\Extension\\AuthManagerOAuth\\AuthManagerOAuthPrimaryAuthenticationProvider": {
44-
"class": "MediaWiki\\Extension\\AuthManagerOAuth\\AuthManagerOAuthPrimaryAuthenticationProvider",
42+
"primaryauth": {
43+
"MediaWiki\\Extension\\AuthManagerOAuth\\AuthManagerOAuthPrimaryAuthenticationProvider": {
44+
"class": "MediaWiki\\Extension\\AuthManagerOAuth\\AuthManagerOAuthPrimaryAuthenticationProvider",
4545
"sort": 0
46-
}
47-
}
48-
},
46+
}
47+
}
48+
},
4949
"manifest_version": 2
5050
}

includes/AuthManagerOAuthPrimaryAuthenticationProvider.php renamed to src/AuthManagerOAuthPrimaryAuthenticationProvider.php

+69-39
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919

2020
namespace MediaWiki\Extension\AuthManagerOAuth;
2121

22+
use League\OAuth2\Client\Provider\GenericProvider;
23+
use MediaWiki\Auth\AuthenticationRequest;
24+
use MediaWiki\Auth\AuthenticationResponse;
2225
use MediaWiki\MediaWikiServices;
2326

2427
class AuthManagerOAuthPrimaryAuthenticationProvider extends \MediaWiki\Auth\AbstractPrimaryAuthenticationProvider {
@@ -31,7 +34,9 @@ class AuthManagerOAuthPrimaryAuthenticationProvider extends \MediaWiki\Auth\Abst
3134
*/
3235
public function getAuthenticationRequests( $action, array $options ) {
3336
wfDebugLog( 'AuthManagerOAuth getAuthenticationRequests', var_export( $action, true ) );
34-
if ( $action === \MediaWiki\Auth\AuthManager::ACTION_LOGIN || $action === \MediaWiki\Auth\AuthManager::ACTION_CREATE || $action === \MediaWiki\Auth\AuthManager::ACTION_LINK ) {
37+
if ( $action === \MediaWiki\Auth\AuthManager::ACTION_LOGIN
38+
|| $action === \MediaWiki\Auth\AuthManager::ACTION_CREATE
39+
|| $action === \MediaWiki\Auth\AuthManager::ACTION_LINK ) {
3540
$config = MediaWikiServices::getInstance()->getConfigFactory()->makeConfig( 'authmanageroauth' );
3641
$reqs = [];
3742
foreach ( $config->get( 'AuthManagerOAuthConfig' ) as $amoa_provider => $provider ) {
@@ -70,10 +75,11 @@ public function testUserExists( $username, $flags = User::READ_NORMAL ) {
7075
/**
7176
* @inheritDoc
7277
*/
73-
public function providerAllowsAuthenticationDataChange( \MediaWiki\Auth\AuthenticationRequest $req, $checkData = true ) {
78+
public function providerAllowsAuthenticationDataChange( AuthenticationRequest $req, $checkData = true ) {
7479
wfDebugLog( 'AuthManagerOAuth providerAllowsAuthenticationDataChange', var_export( $req, true ) );
75-
if ( get_class( $req ) === UnlinkOAuthAccountRequest::class &&
76-
( $req->action === \MediaWiki\Auth\AuthManager::ACTION_REMOVE || $req->action === \MediaWiki\Auth\AuthManager::ACTION_CHANGE ) ) {
80+
if ( get_class( $req ) === UnlinkOAuthAccountRequest::class
81+
&& ( $req->action === \MediaWiki\Auth\AuthManager::ACTION_REMOVE
82+
|| $req->action === \MediaWiki\Auth\AuthManager::ACTION_CHANGE ) ) {
7783
return \StatusValue::newGood();
7884
}
7985
return \StatusValue::newGood( 'ignored' );
@@ -82,10 +88,11 @@ public function providerAllowsAuthenticationDataChange( \MediaWiki\Auth\Authenti
8288
/**
8389
* @inheritDoc
8490
*/
85-
public function providerChangeAuthenticationData( \MediaWiki\Auth\AuthenticationRequest $req ) {
91+
public function providerChangeAuthenticationData( AuthenticationRequest $req ) {
8692
wfDebugLog( 'AuthManagerOAuth providerChangeAuthenticationData', var_export( $req, true ) );
87-
if ( get_class( $req ) === UnlinkOAuthAccountRequest::class &&
88-
( $req->action === \MediaWiki\Auth\AuthManager::ACTION_REMOVE || $req->action === \MediaWiki\Auth\AuthManager::ACTION_CHANGE ) ) {
93+
if ( get_class( $req ) === UnlinkOAuthAccountRequest::class
94+
&& ( $req->action === \MediaWiki\Auth\AuthManager::ACTION_REMOVE
95+
|| $req->action === \MediaWiki\Auth\AuthManager::ACTION_CHANGE ) ) {
8996
$user = \User::newFromName( $req->username );
9097
$lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
9198
$dbr = $lb->getConnectionRef( DB_PRIMARY );
@@ -112,23 +119,30 @@ public function accountCreationType() {
112119
/**
113120
* This starts primary authentication/creation/linking by redirecting to the OAuth provider.
114121
* @param array $reqs The original requests.
115-
* @return \MediaWiki\Auth\AuthenticationResponse the response for redirecting or abstaining.
122+
* @return AuthenticationResponse the response for redirecting or abstaining.
116123
*/
117124
private function beginPrimary( array $reqs ) {
118125
wfDebugLog( 'AuthManagerOAuth beginPrimary*', var_export( $reqs, true ) );
119-
$req = \MediaWiki\Auth\AuthenticationRequest::getRequestByClass( $reqs, ChooseOAuthProviderRequest::class );
126+
$req = AuthenticationRequest::getRequestByClass( $reqs, ChooseOAuthProviderRequest::class );
120127
if ( $req !== null ) {
121128
$config = MediaWikiServices::getInstance()->getConfigFactory()->makeConfig( 'authmanageroauth' );
122-
$provider = new \League\OAuth2\Client\Provider\GenericProvider( $config->get( 'AuthManagerOAuthConfig' )[$req->amoa_provider] );
129+
$provider = new GenericProvider( $config->get( 'AuthManagerOAuthConfig' )[$req->amoa_provider] );
123130
$authorizationUrl = $provider->getAuthorizationUrl( [
124131
'redirect_uri' => $req->returnToUrl
125132
] );
126133

127-
$this->manager->setAuthenticationSessionData( self::AUTHENTICATION_SESSION_DATA_STATE, $provider->getState() );
134+
$this->manager->setAuthenticationSessionData(
135+
self::AUTHENTICATION_SESSION_DATA_STATE,
136+
$provider->getState()
137+
);
128138

129-
return \MediaWiki\Auth\AuthenticationResponse::newRedirect( [ new OAuthProviderAuthenticationRequest( $req->amoa_provider ) ], $authorizationUrl, null );
139+
return AuthenticationResponse::newRedirect(
140+
[ new OAuthProviderAuthenticationRequest( $req->amoa_provider ) ],
141+
$authorizationUrl,
142+
null
143+
);
130144
} else {
131-
return \MediaWiki\Auth\AuthenticationResponse::newAbstain();
145+
return AuthenticationResponse::newAbstain();
132146
}
133147
}
134148

@@ -157,19 +171,21 @@ public function beginPrimaryAccountLink( $user, array $reqs ) {
157171
}
158172

159173
/**
160-
* Convert the response of an OAuth redirect to the identity it represents for further use. This asks the OAuth provider to verify the the login and gets the remote username and id.
174+
* Convert the response of an OAuth redirect to the identity it represents for further use.
175+
* This asks the OAuth provider to verify the the login and gets the remote username and id.
161176
* @param OAuthProviderAuthenticationRequest $req
162177
* @return OAuthIdentityRequest
163178
*/
164179
private function convertOAuthProviderAuthenticationRequestToOAuthIdentityRequest( $req ) {
165180
$config = MediaWikiServices::getInstance()->getConfigFactory()->makeConfig( 'authmanageroauth' );
166-
$provider = new \League\OAuth2\Client\Provider\GenericProvider( $config->get( 'AuthManagerOAuthConfig' )[$req->amoa_provider] );
181+
$provider = new GenericProvider( $config->get( 'AuthManagerOAuthConfig' )[$req->amoa_provider] );
167182
try {
168-
// TODO do we even need this authentication data or can we just store this in the authentication request. ensure again that both of it can't be manipulated
183+
// TODO do we even need this authentication data or can we just store this in the authentication request.
184+
// ensure again that both of it can't be manipulated
169185
$state = $this->manager->getAuthenticationSessionData( self::AUTHENTICATION_SESSION_DATA_STATE );
170186
$this->manager->removeAuthenticationSessionData( self::AUTHENTICATION_SESSION_DATA_STATE );
171187
if ( ( !$state ) || $state !== $req->state ) {
172-
return \MediaWiki\Auth\AuthenticationResponse::newFail( wfMessage( 'authmanageroauth-state-mismatch' ) );
188+
return AuthenticationResponse::newFail( wfMessage( 'authmanageroauth-state-mismatch' ) );
173189
}
174190

175191
$accessToken = $provider->getAccessToken( 'authorization_code', [
@@ -178,15 +194,20 @@ private function convertOAuthProviderAuthenticationRequestToOAuthIdentityRequest
178194

179195
$resourceOwner = $provider->getResourceOwner( $accessToken );
180196

181-
$req = new OAuthIdentityRequest( $req->amoa_provider, $resourceOwner->getId(), $resourceOwner->toArray()['login'] ); // TODO FIXME provider dependent path
197+
// TODO FIXME provider dependent path
198+
$req = new OAuthIdentityRequest(
199+
$req->amoa_provider,
200+
strval( $resourceOwner->getId() ),
201+
$resourceOwner->toArray()['login']
202+
);
182203

183-
$response = \MediaWiki\Auth\AuthenticationResponse::newPass();
204+
$response = AuthenticationResponse::newPass();
184205
$response->createRequest = $req;
185206
$response->linkRequest = $req;
186207
$response->loginRequest = $req;
187208
return $response;
188209
} catch ( \League\OAuth2\Client\Provider\Exception\IdentityProviderException $e ) {
189-
return \MediaWiki\Auth\AuthenticationResponse::newFail( wfMessage( 'authmanageroauth-error', $e->getMessage() ) );
210+
return AuthenticationResponse::newFail( wfMessage( 'authmanageroauth-error', $e->getMessage() ) );
190211
}
191212
}
192213

@@ -195,11 +216,11 @@ private function convertOAuthProviderAuthenticationRequestToOAuthIdentityRequest
195216
*/
196217
public function continuePrimaryAccountCreation( $user, $creator, array $reqs ) {
197218
wfDebugLog( 'AuthManagerOAuth continuePrimaryAccountCreation', var_export( $reqs, true ) );
198-
$req = \MediaWiki\Auth\AuthenticationRequest::getRequestByClass( $reqs, OAuthProviderAuthenticationRequest::class );
219+
$req = AuthenticationRequest::getRequestByClass( $reqs, OAuthProviderAuthenticationRequest::class );
199220
if ( $req !== null ) {
200221
return $this->convertOAuthProviderAuthenticationRequestToOAuthIdentityRequest( $req );
201222
} else {
202-
return \MediaWiki\Auth\AuthenticationResponse::newAbstain();
223+
return AuthenticationResponse::newAbstain();
203224
}
204225
}
205226

@@ -209,28 +230,37 @@ public function continuePrimaryAccountCreation( $user, $creator, array $reqs ) {
209230
public function continuePrimaryAuthentication( array $reqs ) {
210231
wfDebugLog( 'AuthManagerOAuth continuePrimaryAuthentication', var_export( $reqs, true ) );
211232

212-
$identity_req = \MediaWiki\Auth\AuthenticationRequest::getRequestByClass( $reqs, OAuthIdentityRequest::class );
213-
if ( $identity_req !== null ) { // Already authenticated with OAuth provider
214-
$choose_local_account_req = \MediaWiki\Auth\AuthenticationRequest::getRequestByClass( $reqs, ChooseLocalAccountRequest::class );
233+
$identity_req = AuthenticationRequest::getRequestByClass( $reqs, OAuthIdentityRequest::class );
234+
if ( $identity_req !== null ) {
235+
// Already authenticated with OAuth provider
236+
237+
$choose_local_account_req = AuthenticationRequest::getRequestByClass(
238+
$reqs,
239+
ChooseLocalAccountRequest::class
240+
);
215241
if ( $choose_local_account_req !== null ) {
216-
return \MediaWiki\Auth\AuthenticationResponse::newPass( $choose_local_account_req->username );
242+
return AuthenticationResponse::newPass( $choose_local_account_req->username );
217243
}
218244

219-
$choose_local_username_req = \MediaWiki\Auth\AuthenticationRequest::getRequestByClass( $reqs, LocalUsernameInputRequest::class );
245+
$choose_local_username_req = AuthenticationRequest::getRequestByClass(
246+
$reqs,
247+
LocalUsernameInputRequest::class
248+
);
220249
if ( $choose_local_username_req !== null ) {
221250
$user = \User::newFromName( $choose_local_username_req->local_username );
222-
if ( !$user->isRegistered() ) { // TODO FIXME query on primary race condition but that's just how it is https://phabricator.wikimedia.org/T138678#3911381
223-
return \MediaWiki\Auth\AuthenticationResponse::newPass( $choose_local_username_req->local_username );
251+
// TODO FIXME query on primary race condition https://phabricator.wikimedia.org/T138678#3911381
252+
if ( !$user->isRegistered() ) {
253+
return AuthenticationResponse::newPass( $choose_local_username_req->local_username );
224254
} else {
225-
return \MediaWiki\Auth\AuthenticationResponse::newFail( wfMessage( 'authmanageroauth-account-already-exists' ) );
255+
return AuthenticationResponse::newFail( wfMessage( 'authmanageroauth-account-already-exists' ) );
226256
}
227257
}
228258
}
229259

230-
$req = \MediaWiki\Auth\AuthenticationRequest::getRequestByClass( $reqs, OAuthProviderAuthenticationRequest::class );
260+
$req = AuthenticationRequest::getRequestByClass( $reqs, OAuthProviderAuthenticationRequest::class );
231261
if ( $req !== null ) {
232262
$resp = $this->convertOAuthProviderAuthenticationRequestToOAuthIdentityRequest( $req );
233-
if ( $resp->status !== \MediaWiki\Auth\AuthenticationResponse::PASS ) {
263+
if ( $resp->status !== AuthenticationResponse::PASS ) {
234264
return $resp;
235265
}
236266

@@ -256,24 +286,24 @@ public function continuePrimaryAuthentication( array $reqs ) {
256286
] );
257287
if ( count( $reqs ) === 2 ) {
258288
// There are no previous linked accounts
259-
return \MediaWiki\Auth\AuthenticationResponse::newUI( $reqs, wfMessage( 'authmanageroauth-choose-username' ) );
289+
return AuthenticationResponse::newUI( $reqs, wfMessage( 'authmanageroauth-choose-username' ) );
260290
} else {
261291
// There are already accounts linked
262-
return \MediaWiki\Auth\AuthenticationResponse::newUI( $reqs, wfMessage( 'authmanageroauth-choose-message' ) );
292+
return AuthenticationResponse::newUI( $reqs, wfMessage( 'authmanageroauth-choose-message' ) );
263293
}
264294
}
265-
return \MediaWiki\Auth\AuthenticationResponse::newAbstain();
295+
return AuthenticationResponse::newAbstain();
266296
}
267297

268298
/**
269299
* @inheritDoc
270300
*/
271301
public function continuePrimaryAccountLink( $user, array $reqs ) {
272302
wfDebugLog( 'AuthManagerOAuth continuePrimaryAccountLink', var_export( $reqs, true ) );
273-
$req = \MediaWiki\Auth\AuthenticationRequest::getRequestByClass( $reqs, OAuthProviderAuthenticationRequest::class );
303+
$req = AuthenticationRequest::getRequestByClass( $reqs, OAuthProviderAuthenticationRequest::class );
274304
if ( $req !== null ) {
275305
$resp = $this->convertOAuthProviderAuthenticationRequestToOAuthIdentityRequest( $req );
276-
if ( $resp->status !== \MediaWiki\Auth\AuthenticationResponse::PASS ) {
306+
if ( $resp->status !== AuthenticationResponse::PASS ) {
277307
return $resp;
278308
}
279309

@@ -295,7 +325,7 @@ public function continuePrimaryAccountLink( $user, array $reqs ) {
295325
return $resp;
296326
} else {
297327
// TODO FIXME maybe we can put this in the common method so this is even less duplication
298-
return \MediaWiki\Auth\AuthenticationResponse::newAbstain();
328+
return AuthenticationResponse::newAbstain();
299329
}
300330
}
301331

@@ -321,7 +351,7 @@ public function autoCreatedAccount( $user, $source ) {
321351
/**
322352
* @inheritDoc
323353
*/
324-
public function finishAccountCreation( $user, $creator, \MediaWiki\Auth\AuthenticationResponse $response ) {
354+
public function finishAccountCreation( $user, $creator, AuthenticationResponse $response ) {
325355
wfDebugLog( 'AuthManagerOAuth finishAccountCreation', var_export( $response, true ) );
326356
$req = $response->createRequest;
327357
$lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
File renamed without changes.
File renamed without changes.

includes/sql/authmanageroauth_linked_accounts.sql renamed to src/sql/authmanageroauth_linked_accounts.sql

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ CREATE TABLE /*_*/authmanageroauth_linked_accounts(
33
amoa_provider VARCHAR(255) NOT NULL,
44

55
-- the local user id
6-
amoa_local_user INTEGER UNSIGNED NOT NULL,
6+
amoa_local_user BIGINT NOT NULL,
77

88
-- the remote user identifier
99
amoa_remote_user VARCHAR(255) NOT NULL,

0 commit comments

Comments
 (0)