Skip to content

Commit

Permalink
Cleanup and remove broken change credentials (#35)
Browse files Browse the repository at this point in the history
  • Loading branch information
mohe2015 committed Oct 24, 2023
1 parent 4dfbb9d commit 67ae104
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 22 deletions.
3 changes: 3 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[*.php]
indent_style = tab
indent_size = 4
9 changes: 6 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@ Create accounts or login using OAuth

## Setup

1. install nodejs, npm, and PHP composer
3. `npm install`
1. install PHP composer
4. `composer install`

Once set up, running `npm test` and `composer test` will run automated code checks.
Once set up, running `composer test` will run automated code checks.

## Development

Add the extension do your development mediawiki instance and then open the top level mediawiki folder in Visual Studio Code. Then you should get more or less proper intellisense.

## Documentation

Expand Down
2 changes: 1 addition & 1 deletion i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@
"authmanageroauth-error": "Error: $1",
"authmanageroauth-account-already-exists": "Account already exists.",
"authmanageroauth-choose-username": "Choose a username",
"authmanageroauth-linked-accounts": "Linked accounts",
"authmanageroauth-linked-accounts": "Linked accounts and unlink",
"authmanageroauth-link-accounts": "Link accounts"
}
2 changes: 1 addition & 1 deletion i18n/qqq.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@
"authmanageroauth-error": "A generic OAuth error with $1 for the message.",
"authmanageroauth-account-already-exists": "Account already exists.",
"authmanageroauth-choose-username": "Choose a username",
"authmanageroauth-linked-accounts": "Linked accounts",
"authmanageroauth-linked-accounts": "Linked accounts and unlink",
"authmanageroauth-link-accounts": "Link accounts"
}
51 changes: 37 additions & 14 deletions src/AuthManagerOAuthPrimaryAuthenticationProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@
namespace MediaWiki\Extension\AuthManagerOAuth;

use League\OAuth2\Client\Provider\GenericProvider;
use LogicException;
use MediaWiki\Auth\AuthenticationRequest;
use MediaWiki\Auth\AuthenticationResponse;
use MediaWiki\MediaWikiServices;

// https://doc.wikimedia.org/mediawiki-core/master/php/classMediaWiki_1_1Auth_1_1AbstractPrimaryAuthenticationProvider.html
class AuthManagerOAuthPrimaryAuthenticationProvider extends \MediaWiki\Auth\AbstractPrimaryAuthenticationProvider {

private const AUTHENTICATION_SESSION_DATA_STATE = 'authmanageroauth:state';
Expand All @@ -44,8 +46,9 @@ public function getAuthenticationRequests( $action, array $options ) {
}
return $reqs;
}
if ( $options['username'] !== null && ( $action === \MediaWiki\Auth\AuthManager::ACTION_REMOVE ||
$action === \MediaWiki\Auth\AuthManager::ACTION_CHANGE ) ) {
if ( $options['username'] !== null
&& ( $action === \MediaWiki\Auth\AuthManager::ACTION_REMOVE
|| $action === \MediaWiki\Auth\AuthManager::ACTION_UNLINK ) ) {
$user = \User::newFromName( $options['username'] );
$lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
$dbr = $lb->getConnectionRef( DB_REPLICA );
Expand All @@ -68,7 +71,7 @@ public function getAuthenticationRequests( $action, array $options ) {
* All our users need to also be created locally so always return false here.
* @inheritDoc
*/
public function testUserExists( $username, $flags = User::READ_NORMAL ) {
public function testUserExists( $username, $flags = \User::READ_NORMAL ) {
return false;
}

Expand All @@ -77,9 +80,9 @@ public function testUserExists( $username, $flags = User::READ_NORMAL ) {
*/
public function providerAllowsAuthenticationDataChange( AuthenticationRequest $req, $checkData = true ) {
wfDebugLog( 'AuthManagerOAuth providerAllowsAuthenticationDataChange', var_export( $req, true ) );
if ( get_class( $req ) === UnlinkOAuthAccountRequest::class
if ( $req instanceof UnlinkOAuthAccountRequest
&& ( $req->action === \MediaWiki\Auth\AuthManager::ACTION_REMOVE
|| $req->action === \MediaWiki\Auth\AuthManager::ACTION_CHANGE ) ) {
|| $req->action === \MediaWiki\Auth\AuthManager::ACTION_UNLINK ) ) {
return \StatusValue::newGood();
}
return \StatusValue::newGood( 'ignored' );
Expand All @@ -90,9 +93,9 @@ public function providerAllowsAuthenticationDataChange( AuthenticationRequest $r
*/
public function providerChangeAuthenticationData( AuthenticationRequest $req ) {
wfDebugLog( 'AuthManagerOAuth providerChangeAuthenticationData', var_export( $req, true ) );
if ( get_class( $req ) === UnlinkOAuthAccountRequest::class
if ( $req instanceof UnlinkOAuthAccountRequest
&& ( $req->action === \MediaWiki\Auth\AuthManager::ACTION_REMOVE
|| $req->action === \MediaWiki\Auth\AuthManager::ACTION_CHANGE ) ) {
|| $req->action === \MediaWiki\Auth\AuthManager::ACTION_UNLINK ) ) {
$user = \User::newFromName( $req->username );
$lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
$dbr = $lb->getConnectionRef( DB_PRIMARY );
Expand All @@ -105,6 +108,8 @@ public function providerChangeAuthenticationData( AuthenticationRequest $req ) {
],
__METHOD__,
);
} else {
throw new LogicException( "Unexpected unhandled request" );
}
}

Expand All @@ -124,7 +129,7 @@ public function accountCreationType() {
private function beginPrimary( array $reqs ) {
wfDebugLog( 'AuthManagerOAuth beginPrimary*', var_export( $reqs, true ) );
$req = AuthenticationRequest::getRequestByClass( $reqs, ChooseOAuthProviderRequest::class );
if ( $req !== null ) {
if ( $req !== null && $req instanceof ChooseOAuthProviderRequest ) {
$config = MediaWikiServices::getInstance()->getConfigFactory()->makeConfig( 'authmanageroauth' );
$provider = new GenericProvider( $config->get( 'AuthManagerOAuthConfig' )[$req->amoa_provider] );
$authorizationUrl = $provider->getAuthorizationUrl( [
Expand Down Expand Up @@ -174,7 +179,7 @@ public function beginPrimaryAccountLink( $user, array $reqs ) {
* 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.
* @param OAuthProviderAuthenticationRequest $req
* @return OAuthIdentityRequest
* @return AuthenticationResponse
*/
private function convertOAuthProviderAuthenticationRequestToOAuthIdentityRequest( $req ) {
$config = MediaWikiServices::getInstance()->getConfigFactory()->makeConfig( 'authmanageroauth' );
Expand Down Expand Up @@ -231,22 +236,24 @@ public function continuePrimaryAuthentication( array $reqs ) {
wfDebugLog( 'AuthManagerOAuth continuePrimaryAuthentication', var_export( $reqs, true ) );

$identity_req = AuthenticationRequest::getRequestByClass( $reqs, OAuthIdentityRequest::class );
if ( $identity_req !== null ) {
if ( $identity_req !== null && $identity_req instanceof OAuthIdentityRequest ) {
// Already authenticated with OAuth provider

$choose_local_account_req = AuthenticationRequest::getRequestByClass(
$reqs,
ChooseLocalAccountRequest::class
);
if ( $choose_local_account_req !== null ) {
if ( $choose_local_account_req !== null
&& $choose_local_account_req instanceof ChooseLocalAccountRequest ) {
return AuthenticationResponse::newPass( $choose_local_account_req->username );
}

$choose_local_username_req = AuthenticationRequest::getRequestByClass(
$reqs,
LocalUsernameInputRequest::class
);
if ( $choose_local_username_req !== null ) {
if ( $choose_local_username_req !== null
&& $choose_local_username_req instanceof LocalUsernameInputRequest ) {
$user = \User::newFromName( $choose_local_username_req->local_username );
// TODO FIXME query on primary race condition https://phabricator.wikimedia.org/T138678#3911381
if ( !$user->isRegistered() ) {
Expand All @@ -258,11 +265,16 @@ public function continuePrimaryAuthentication( array $reqs ) {
}

$req = AuthenticationRequest::getRequestByClass( $reqs, OAuthProviderAuthenticationRequest::class );
if ( $req !== null ) {
if ( $req !== null && $req instanceof OAuthProviderAuthenticationRequest ) {
$resp = $this->convertOAuthProviderAuthenticationRequestToOAuthIdentityRequest( $req );
if ( $resp->status !== AuthenticationResponse::PASS ) {
return $resp;
}
if ( !( $resp->linkRequest instanceof OAuthIdentityRequest ) ) {
throw new LogicException(
"Unexpected createRequest type {${get_class($req)}}. This should never happen."
);
}

$lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
$dbr = $lb->getConnectionRef( DB_REPLICA );
Expand Down Expand Up @@ -301,11 +313,16 @@ public function continuePrimaryAuthentication( array $reqs ) {
public function continuePrimaryAccountLink( $user, array $reqs ) {
wfDebugLog( 'AuthManagerOAuth continuePrimaryAccountLink', var_export( $reqs, true ) );
$req = AuthenticationRequest::getRequestByClass( $reqs, OAuthProviderAuthenticationRequest::class );
if ( $req !== null ) {
if ( $req !== null && $req instanceof OAuthProviderAuthenticationRequest ) {
$resp = $this->convertOAuthProviderAuthenticationRequestToOAuthIdentityRequest( $req );
if ( $resp->status !== AuthenticationResponse::PASS ) {
return $resp;
}
if ( !( $resp->linkRequest instanceof OAuthIdentityRequest ) ) {
throw new LogicException(
"Unexpected createRequest type {${get_class($req)}}. This should never happen."
);
}

$lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
$dbr = $lb->getConnectionRef( DB_PRIMARY );
Expand Down Expand Up @@ -354,6 +371,9 @@ public function autoCreatedAccount( $user, $source ) {
public function finishAccountCreation( $user, $creator, AuthenticationResponse $response ) {
wfDebugLog( 'AuthManagerOAuth finishAccountCreation', var_export( $response, true ) );
$req = $response->createRequest;
if ( !( $req instanceof OAuthIdentityRequest ) ) {
throw new LogicException( "Unexpected createRequest type {${get_class($req)}}. This should never happen." );
}
$lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
$dbr = $lb->getConnectionRef( DB_PRIMARY );
$result = $dbr->insert(
Expand All @@ -366,4 +386,7 @@ public function finishAccountCreation( $user, $creator, AuthenticationResponse $
__METHOD__,
);
}

// TODO providerNormalizeUsername()
// TODO providerRevokeAccessForUser
}
2 changes: 1 addition & 1 deletion src/Hooks.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public static function onGetPreferences( \User $user, array &$preferences ) {
'type' => 'info',
'raw' => true,
'default' => (string)( new \OOUI\ButtonWidget( [
'href' => \SpecialPage::getTitleFor( 'ChangeCredentials' )->getLinkURL(),
'href' => \SpecialPage::getTitleFor( 'UnlinkAccounts' )->getLinkURL(),
'label' => wfMessage( 'authmanageroauth-linked-accounts' )->plain()
] ) )
];
Expand Down
3 changes: 3 additions & 0 deletions src/OAuthProviderAuthenticationRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@

class OAuthProviderAuthenticationRequest extends AuthenticationRequest {

/** @var string The OAuth provider name */
public $accessToken;

/** @var string The OAuth state */
public $state;

Expand Down
4 changes: 2 additions & 2 deletions src/UnlinkOAuthAccountRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ public function __construct( $amoa_provider, $amoa_remote_user ) {

public function describeCredentials() {
return [
"provider" => new \RawMessage( '$1 OAuth', [ $this->amoa_provider ] ),
"account" => new \RawMessage( '$1', [ $this->amoa_remote_user ] )
"provider" => new \MediaWiki\Language\RawMessage( '$1 OAuth', [ $this->amoa_provider ] ),
"account" => new \MediaWiki\Language\RawMessage( '$1', [ $this->amoa_remote_user ] )
];
}
}

0 comments on commit 67ae104

Please sign in to comment.