jenkins-bot has submitted this change and it was merged.
Change subject: Refactor Special:GoogleLogin a bit
......................................................................
Refactor Special:GoogleLogin a bit
Change-Id: Ibfcb4b182aa3c5202449610c17d7f05ff336d674
Plus: Add a test case to cover the redirect from the login form.
---
M includes/GoogleLogin.hooks.php
M includes/specials/SpecialGoogleLogin.php
A tests/phpunit/specials/SpecialGoogleLoginTest.php
3 files changed, 228 insertions(+), 148 deletions(-)
Approvals:
Florianschmidtwelzow: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/GoogleLogin.hooks.php b/includes/GoogleLogin.hooks.php
index bcf15c0..0e463c0 100644
--- a/includes/GoogleLogin.hooks.php
+++ b/includes/GoogleLogin.hooks.php
@@ -6,7 +6,7 @@
$googleLogin = new GoogleLogin;
$request = $googleLogin->getRequest();
if ( $request->getSessionData( 'access_token' ) !== null ) {
- $request->setSessionData( 'access_token', '' );
+ $request->setSessionData( 'access_token', null );
}
}
diff --git a/includes/specials/SpecialGoogleLogin.php
b/includes/specials/SpecialGoogleLogin.php
index a029623..aa99f66 100644
--- a/includes/specials/SpecialGoogleLogin.php
+++ b/includes/specials/SpecialGoogleLogin.php
@@ -22,8 +22,10 @@
$request = $this->getRequest();
$out = $this->getOutput();
$config = $this->getConfig();
+ $client = $googleLogin->getClient();
$this->setHeaders();
+ $googleLogin->setLoginParameter( $request );
// every time enable OOUI on this special page
$out->enableOOUI();
@@ -37,48 +39,34 @@
wfSetupSession();
}
- // Check, if a user clicked the login button on the
login/create form
- if ( $request->getVal( 'googlelogin-submit' ) !== null ) {
- // ... and redirect them directly to the auth url from
google
- $googleLogin->setLoginParameter( $request );
- $client = $googleLogin->getClient();
- $authUrl = $client->createAuthUrl();
- $out->redirect( $authUrl );
- $out->output();
- return;
- }
+ $this->redirectFromLoginForm( $request, $client );
// first set our own handler for catchable fatal errors
set_error_handler( 'GoogleLogin::catchableFatalHandler',
E_RECOVERABLE_ERROR );
// if there is no subpage, use the value of action
$par = ( empty( $par ) ? $request->getVal( 'action' ) : $par );
- $googleLogin->setLoginParameter( $request );
- // initialize google api client and the client for google plus
api
- $client = $googleLogin->getClient();
+ // initialize the client for google plus api
$plus = $googleLogin->getPlus();
// if the user is redirected back from google, try to
authenticate
$authCode = $request->getVal( 'code' );
if ( $authCode !== null ) {
$this->tryAuthenticate( $authCode, $client, $plus );
- } elseif ( $request->getVal( 'error' ) !== null ) {
+ } elseif ( $request->getVal( 'error' ) !== null ) {
// if there was an error reported from google, show
this to the user
// FIXME: This should be a localized message!
$this->createError( 'Authentication failed' );
} else {
$access_token = $request->getSessionData(
'access_token' );
- if (
- $access_token !== null &&
- $access_token
- ) {
+ if ( $access_token !== null ) {
$client->setAccessToken( $access_token );
$request->setSessionData( 'access_token',
$client->getAccessToken() );
if ( !empty( $par ) ) {
$this->finishAction( $par, $client,
$plus );
} else {
- $this->showSummary( $client, $plus );
+ $this->showSummary( $plus );
}
} else {
$authUrl = $client->createAuthUrl();
@@ -91,7 +79,43 @@
}
/**
+ * Checks, if a user clicked the login with Google button on the
login/create form
+ * and redirects the user directly to the auth request of Google.
+ *
+ * @param WebRequest $request
+ * @param Google_Client $client
+ */
+ protected function redirectFromLoginForm( WebRequest $request,
Google_Client $client ) {
+ if ( $request->getBool( 'googlelogin-submit' ) ) {
+ $out = $this->getOutput();
+ $googleLogin = $this->mGoogleLogin;
+
+ // redirect them directly to the auth url from google
+ $authUrl = $client->createAuthUrl();
+ $out->redirect( $authUrl );
+ $out->output();
+ }
+ }
+
+ /**
+ * Tries to get the user information of the passed plus object and
+ * fails savely by adding an error message, if an Exception occurs.
+ *
+ * @param Google_Service_Plus $plus
+ * @return bool|array
+ */
+ private function getPlusUserInfo( Google_Service_Plus $plus ) {
+ try {
+ return $userInfo = $plus->people->get( "me" );
+ } catch ( Exception $e ) {
+ $this->createError( $e->getMessage() );
+ return false;
+ }
+ }
+
+ /**
* Helper function to authenticate a user against google plus api
+ *
* @param String $code The auth code to use
* @param Google_Client $client
* @param Google_Service_Plus $plus
@@ -101,13 +125,10 @@
try {
$client->authenticate( $authCode );
$request->setSessionData( 'access_token',
$client->getAccessToken() );
- try {
- $userInfo = $plus->people->get( "me" );
- } catch ( Exception $e ) {
- $this->createError( $e->getMessage() );
- return;
+ $userInfo = $this->getPlusUserInfo( $plus );
+ if ( $userInfo ) {
+ $this->createOrMerge( $userInfo,
User::newFromGoogleId( $userInfo['id'] ), true );
}
- $this->createOrMerge( $userInfo, true );
} catch ( Google_Auth_Exception $e ) {
$this->createError( $e->getMessage() );
}
@@ -115,19 +136,18 @@
/**
* Show a summary about the actual logged in google user.
- * @param Google_Client $client
+ *
* @param Google_Service_Plus $plus
*/
- private function showSummary( Google_Client $client,
Google_Service_Plus $plus ) {
+ private function showSummary( Google_Service_Plus $plus ) {
$request = $this->getRequest();
$out = $this->getOutput();
- try {
- $userInfo = $plus->people->get( "me" );
- } catch ( Exception $e ) {
- $this->createError( $e->getMessage() );
+ $userInfo = $this->getPlusUserInfo( $plus );
+ if ( !$userInfo ) {
return;
}
+
$user = User::newFromGoogleId( $userInfo['id'] );
// data that will be added to the account information box
@@ -169,7 +189,7 @@
);
$out->addHtml( $container );
- $this->createOrMerge( $userInfo, false, $user );
+ $this->createOrMerge( $userInfo, $user );
}
/**
@@ -183,24 +203,21 @@
/**
* Used to determine what is to do, merge, creata or login the
authenticated google user.
- * @param array $userInfo array of User information provided by Google
OAuth2 G+ Api
+ * @param object $userInfo Object of User information provided by
Google OAuth2 G+ Api
+ * @param GoogleLogin\\GoogleUser $gluser Google User object
* @param boolean $redirect If true, the function will redirect to
Special:GoogleLogin if
* nothing to display.
*/
- private function createOrMerge( $userInfo, $redirect = false, $gluser =
null ) {
+ private function createOrMerge( $userInfo, User $gluser, $redirect =
false ) {
$out = $this->getOutput();
$request = $this->getRequest();
$user = $this->getUser();
- if ( $gluser === null ) {
- $gluser = User::newFromGoogleId( $userInfo['id'] );
- }
$userId = $gluser->getId();
if (
- $this->mGoogleLogin->isValidDomain(
- $userInfo['emails'][0]['value']
- )
+ isset( $userInfo['emails'][0]['value'] ) &&
+ $this->mGoogleLogin->isValidDomain(
$userInfo['emails'][0]['value'] )
) {
if ( !$gluser->hasConnectedGoogleAccount() ) {
if ( !$user->isLoggedIn() ) {
@@ -228,14 +245,14 @@
}
}
} else {
- $loginUser =
$this->mGoogleLogin->loginGoogleUser(
+ $loginUserStatus =
$this->mGoogleLogin->loginGoogleUser(
$userId,
$userInfo['id']
);
- if ( $loginUser->isOk() ) {
- $out->redirect(
$loginUser->getValue() );
+ if ( $loginUserStatus->isOk() ) {
+ $out->redirect(
$loginUserStatus->getValue() );
} else {
- $out->addHtml(
$loginUser->getHTML() );
+ $out->addHtml(
$loginUserStatus->getHTML() );
}
}
}
@@ -252,11 +269,13 @@
/**
* Create and show a Form to for the user to create a new Wiki user
account. Called after
* Google user logged in and Google id not linked to an existing wiki
user.
- * @param array $userInfo An array of user information provided by
Google Plus API
+ * @param object $userInfo An array of user information provided by
Google Plus API
*/
private function createGoogleUserForm( $userInfo ) {
$request = $this->getRequest();
$this->getOutput()->setPageTitle( $this->msg(
'googlelogin-form-choosename-title' )->text() );
+
+ // create an array of possible usernames
$names = array(
$this->msg( 'googlelogin-login-already-registered'
)->text() => 'wpAlreadyRegistered'
);
@@ -266,14 +285,15 @@
if ( GoogleLogin::isValidUsername(
$userInfo['name']['givenName'] ) ) {
$names[$userInfo['name']['givenName']] = 'wpGivenName';
}
+ // "Choose own", so the user can pass it's own username
$names[$this->msg( 'googlelogin-form-chooseown' )->text() .
':'] = 'wpOwn';
- $defaultName = ( $request->getVal( 'wpChooseName' ) !== null ?
- $request->getVal( 'wpChooseName' ) : 'wpOwn' );
+
$formElements = array(
'ChooseName' => array(
'type' => 'radio',
'options' => $names,
- 'default' => $defaultName,
+ 'default' => ( $request->getVal( 'wpChooseName'
) !== null ?
+ $request->getVal( 'wpChooseName' ) :
'wpOwn' ),
),
'ChooseOwn' => array(
'class' => 'HTMLTextField',
@@ -281,6 +301,7 @@
'placeholder' => $this->msg(
'googlelogin-form-choosename-placeholder' )->text()
),
);
+
$htmlForm = HTMLForm::factory( 'ooui', $formElements,
$this->getContext(), 'googlelogin-form' );
$htmlForm->addHiddenField( 'action', 'Create' );
$htmlForm->addHiddenField( 'wpSecureHash',
$this->mGoogleLogin->getRequestToken() );
@@ -353,12 +374,11 @@
$request = $this->getRequest();
// get userinfos of google plus api result
- try {
- $userInfo = $plus->people->get( "me" );
- } catch ( Exception $e ) {
- $this->createError( $e->getMessage() );
+ $userInfo = $this->getPlusUserInfo( $plus );
+ if ( !$userInfo ) {
return;
}
+
$user = User::newFromGoogleId( $userInfo['id'] );
$isGoogleIdFree = User::isGoogleIdFree( $userInfo['id'] );
@@ -371,103 +391,12 @@
if ( !$this->mGoogleLogin->isValidRequest() ) {
$this->createError( 'Token failure' );
}
+
// Handles the creation of a new wikiuser, but
before: check, if no-one changed the username
// and is still valid
// Finish with the creation of connection
between user id and google id
if ( $this->mGoogleLogin->isCreateAllowed() ) {
- $userName = '';
- $chooseOwn = $request->getVal(
'wpChooseName' );
- if ( $chooseOwn === null ) {
- $this->createGoogleUserForm(
$userInfo );
- }
- // if the user selected the "i have an
account" option, redirect to the login page
- // with all required parameter
- if ( $chooseOwn ===
'wpAlreadyRegistered' ) {
- // target page query
- $query = array(
- // to redirect the user
to the link function directly after login
- 'action' => 'Merge',
- // with this parameter,
the user doesn't get an error message because of the
- // missing form token
- 'fromLogin' => 1
- );
-
- $out->redirect(
- // redirect to
Special:UserLogin
-
SpecialPage::getTitleFor( 'UserLogin' )->getFullUrl( array(
- 'returnto' =>
'Special:GoogleLogin',
- 'returntoquery'
=> wfArrayToCgi( $query ),
- // this
parameter disables the "Login with Google" option, it would be
- // pointless
here
- 'loginmerge' =>
1,
- // a custom
warning message, if you change the mesasge key, change it in
- //
GoogleLoginHooks::onLoginFormValidErrorMessages, too
- 'warning' =>
'googlelogin-login-merge-warning',
- ) )
- );
- }
- if ( $chooseOwn === 'wpOwn' ) {
- if ( $request->getVal(
'wpChooseOwn' ) === '' ) {
-
$this->createGoogleUserForm( $userInfo );
- } elseif(
- $request->getVal(
'wpChooseOwn' ) !== '' &&
-
!GoogleLogin::isValidUsername( $request->getVal( 'wpChooseOwn' ) )
- ) {
-
$this->createGoogleUserForm( $userInfo );
- } else {
- $userName =
$request->getVal( 'wpChooseOwn' );
- }
- }
- if ( $chooseOwn === 'wpDisplayName' ) {
- if (
GoogleLogin::isValidUsername( $userInfo['displayName'] ) ) {
- $userName =
$userInfo['displayName'];
- } else {
-
$this->createGoogleUserForm( $userInfo );
- }
- }
- if ( $chooseOwn === 'wpGivenName' ) {
- if (
GoogleLogin::isValidUsername( $userInfo['name']['givenName'] ) ) {
- $userName =
$userInfo['name']['givenName'];
- } else {
-
$this->createGoogleUserForm( $userInfo );
- }
- }
- if ( !empty( $userName ) ) {
- $out->setPageTitle( $this->msg(
'googlelogin-form-choosename-finish-title' )
- ->text() );
- $userParam = array(
- 'password' => md5(
Rand() ),
- 'email' =>
$userInfo['emails'][0]['value'],
- 'real_name' =>
$userInfo['name']['givenName']
- );
- if ( $isGoogleIdFree ) {
- // FIXME: Maybe report
upstream, that User shouldn't use hardcoded class name for
- // factory methods
- $mwuser =
MWUser::createNew( $userName, $userParam );
- $user =
User::newFromId( $mwuser->getId() );
- if ( !$user ) {
-
$this->createError( $this->msg( 'googlelogin-link-other' )->text() );
- } else {
- if (
$glConfig->get( 'GLNeedsConfirmEmail' ) ) {
-
$user->sendConfirmationMail();
- } else {
-
$user->confirmEmail();
-
$user->saveSettings();
- }
-
$user->setCookies();
- // create a log
entry for the created user - bug 67245
- if (
$glConfig->get( 'GLShowCreateReason' ) ) {
-
self::$performer = $userName;
- }
- $logEntry =
$user->addNewUserLogEntry( 'create' );
-
$user->connectWithGoogle( $userInfo['id'] );
-
$out->addWikiMsg( 'googlelogin-form-choosename-finish-body', $userName );
-
$this->addReturnTo();
- }
- } else {
- $this->createError(
$this->msg( 'googlelogin-link-other' )->text() );
- }
- }
+ $this->onActionCreate( $userInfo,
$isGoogleIdFree );
} else {
$this->createError( 'not allowed' );
}
@@ -487,7 +416,7 @@
}
}
} elseif ( $request->getBool( 'fromLogin' ) ===
true ) {
- $this->createOrMerge( $userInfo );
+ $this->createOrMerge( $userInfo,
User::newFromGoogleId( $userInfo['id'] ) );
} else {
$this->createError( 'Token failure' );
}
@@ -518,12 +447,135 @@
// When GoogleLogin replaces MW login,
Special:CreateAccount will
// redirect to Special:GoogleLogin/signup,
// handle this here correctly.
- $this->createOrMerge( $userInfo );
+ $this->createOrMerge( $userInfo,
User::newFromGoogleId( $userInfo['id'] ) );
break;
}
}
/**
+ * Get the username for the local MediaWiki account, based on the user
selection,
+ * and teh given Google Plus user info object.
+ *
+ * @param object $userInfo
+ * @return bool|string Returns the username or false, if something went
wrong.
+ */
+ private function getChooseName( $userInfo ) {
+ $userName = false;
+ $request = $this->getRequest();
+ $out = $this->getOutput();
+
+ switch ( $request->getVal( 'wpChooseName' ) ) {
+ default:
+ $this->createGoogleUserForm( $userInfo );
+ break;
+ case 'wpAlreadyRegistered':
+ // if the user selected the "i have an account"
option, redirect to the login page
+ // with all required parameter
+
+ // target page query
+ $query = array(
+ // to redirect the user to the link
function directly after login
+ 'action' => 'Merge',
+ // with this parameter, the user
doesn't get an error message because of the
+ // missing form token
+ 'fromLogin' => 1
+ );
+
+ $out->redirect(
+ // redirect to Special:UserLogin
+ SpecialPage::getTitleFor( 'UserLogin'
)->getFullUrl( array(
+ 'returnto' =>
'Special:GoogleLogin',
+ 'returntoquery' =>
wfArrayToCgi( $query ),
+ // this parameter disables the
"Login with Google" option, it would be
+ // pointless here
+ 'loginmerge' => 1,
+ // a custom warning message, if
you change the mesasge key, change it in
+ //
GoogleLoginHooks::onLoginFormValidErrorMessages, too
+ 'warning' =>
'googlelogin-login-merge-warning',
+ ) )
+ );
+ break;
+ case 'wpOwn':
+ if ( $request->getVal( 'wpChooseOwn' ) === '' )
{
+ $this->createGoogleUserForm( $userInfo
);
+ } elseif(
+ $request->getVal( 'wpChooseOwn' ) !==
'' &&
+ !GoogleLogin::isValidUsername(
$request->getVal( 'wpChooseOwn' ) )
+ ) {
+ $this->createGoogleUserForm( $userInfo
);
+ } else {
+ $userName = $request->getVal(
'wpChooseOwn' );
+ }
+ break;
+ case 'wpDisplayName':
+ if ( GoogleLogin::isValidUsername(
$userInfo['displayName'] ) ) {
+ $userName = $userInfo['displayName'];
+ } else {
+ $this->createGoogleUserForm( $userInfo
);
+ }
+ break;
+ case 'wpGivenName':
+ if ( GoogleLogin::isValidUsername(
$userInfo['name']['givenName'] ) ) {
+ $userName =
$userInfo['name']['givenName'];
+ } else {
+ $this->createGoogleUserForm( $userInfo
);
+ }
+ break;
+ }
+
+ return $userName;
+ }
+
+ /**
+ * "Create" handler for FinishAction.
+ *
+ * @param object $userInfo
+ * @param bool $isGoogleIdFree
+ */
+ private function onActionCreate( $userInfo, $isGoogleIdFree ) {
+ $out = $this->getOutput();
+ $glConfig = $this->mGoogleLogin->getGLConfig();
+ $userName = $this->getChooseName( $userInfo );
+
+ if ( $userName ) {
+ $out->setPageTitle( $this->msg(
'googlelogin-form-choosename-finish-title' )
+ ->text() );
+ $userParam = array(
+ 'password' => md5( Rand() ),
+ 'email' => $userInfo['emails'][0]['value'],
+ 'real_name' => $userInfo['name']['givenName']
+ );
+ if ( $isGoogleIdFree ) {
+ // FIXME: Maybe report upstream, that User
shouldn't use hardcoded class name for
+ // factory methods
+ $mwuser = MWUser::createNew( $userName,
$userParam );
+ $user = User::newFromId( $mwuser->getId() );
+ if ( !$user ) {
+ $this->createError( $this->msg(
'googlelogin-link-other' )->text() );
+ } else {
+ if ( $glConfig->get(
'GLNeedsConfirmEmail' ) ) {
+ $user->sendConfirmationMail();
+ } else {
+ $user->confirmEmail();
+ $user->saveSettings();
+ }
+ $user->setCookies();
+ // create a log entry for the created
user - bug 67245
+ if ( $glConfig->get(
'GLShowCreateReason' ) ) {
+ self::$performer = $userName;
+ }
+ $logEntry = $user->addNewUserLogEntry(
'create' );
+ $user->connectWithGoogle(
$userInfo['id'] );
+ $out->addWikiMsg(
'googlelogin-form-choosename-finish-body', $userName );
+ $this->addReturnTo();
+ }
+ } else {
+ $this->createError( $this->msg(
'googlelogin-link-other' )->text() );
+ }
+ }
+ }
+
+ /**
* If there is a return to target, add a backlink to it.
*/
private function addReturnTo() {
diff --git a/tests/phpunit/specials/SpecialGoogleLoginTest.php
b/tests/phpunit/specials/SpecialGoogleLoginTest.php
new file mode 100644
index 0000000..db6e48f
--- /dev/null
+++ b/tests/phpunit/specials/SpecialGoogleLoginTest.php
@@ -0,0 +1,28 @@
+<?php
+
+class SpecialGoogleLoginTest extends MediaWikiTestCase {
+ private function redirect( $val ) {
+ $request = new FauxRequest( array( 'googlelogin-submit' =>
$val, 'error' => '000' ) );
+ $context = new DerivativeContext( RequestContext::getMain() );
+ $output = new OutputPage( $context );
+ $output->disable();
+ $context->setOutput( $output );
+ $context->setRequest( $request );
+ $specialPage = new SpecialGoogleLogin();
+ $specialPage->setContext( $context );
+ $specialPage->execute( '' );
+
+ return $specialPage->getOutput();
+ }
+
+ public function testRedirectFromLoginForm() {
+ // test, if the special page redirects to the login form, if
the user comes
+ // from the login page
+ $output = $this->redirect( '1' );
+ $this->assertFalse( $output->getRedirect() === '' );
+
+ // test, if the special page does not redirect to the login page
+ $output = $this->redirect( '0' );
+ $this->assertTrue( $output->getRedirect() === '' );
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/249195
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibfcb4b182aa3c5202449610c17d7f05ff336d674
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/extensions/GoogleLogin
Gerrit-Branch: master
Gerrit-Owner: Florianschmidtwelzow <[email protected]>
Gerrit-Reviewer: Florianschmidtwelzow <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
