aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJoseph Warner <hardolaf@hardolaf.com>2013-08-24 22:00:16 -0400
committerJoseph Warner <hardolaf@hardolaf.com>2013-08-24 22:04:04 -0400
commita8ffbce99f9ea99bd1fdca0e009001026e2d6950 (patch)
tree1cae1d43fee4c417eedd8303f581eb3109d083ab
parent310caec5d92d58453d1eee40e9b5a7f0157bd5ea (diff)
downloadforums-a8ffbce99f9ea99bd1fdca0e009001026e2d6950.tar
forums-a8ffbce99f9ea99bd1fdca0e009001026e2d6950.tar.gz
forums-a8ffbce99f9ea99bd1fdca0e009001026e2d6950.tar.bz2
forums-a8ffbce99f9ea99bd1fdca0e009001026e2d6950.tar.xz
forums-a8ffbce99f9ea99bd1fdca0e009001026e2d6950.zip
[feature/oauth] Changes due to code review
PHPBB3-11673
-rw-r--r--phpBB/includes/ucp/ucp_auth_link.php32
-rw-r--r--phpBB/includes/ucp/ucp_login_link.php4
-rw-r--r--phpBB/language/en/common.php1
-rw-r--r--phpBB/language/en/ucp.php10
-rw-r--r--phpBB/phpbb/auth/provider/interface.php8
-rw-r--r--phpBB/phpbb/auth/provider/oauth/oauth.php17
-rw-r--r--phpBB/phpbb/auth/provider/oauth/service/bitly.php6
-rw-r--r--phpBB/phpbb/auth/provider/oauth/service/facebook.php10
-rw-r--r--phpBB/phpbb/auth/provider/oauth/service/google.php6
-rw-r--r--phpBB/phpbb/auth/provider/oauth/token_storage.php35
10 files changed, 62 insertions, 67 deletions
diff --git a/phpBB/includes/ucp/ucp_auth_link.php b/phpBB/includes/ucp/ucp_auth_link.php
index e2bf369984..4fa984c9e7 100644
--- a/phpBB/includes/ucp/ucp_auth_link.php
+++ b/phpBB/includes/ucp/ucp_auth_link.php
@@ -17,8 +17,17 @@ if (!defined('IN_PHPBB'))
class ucp_auth_link
{
+ /**
+ * @var string
+ */
public $u_action;
+ /**
+ * Generates the ucp_auth_link page and handles the auth link process
+ *
+ * @param int $id
+ * @param string $mode
+ */
public function main($id, $mode)
{
global $config, $request, $template, $phpbb_container, $user;
@@ -72,7 +81,7 @@ class ucp_auth_link
}
}
- // In some cases, an request to an external server may be required in
+ // In some cases, a request to an external server may be required. In
// these cases, the GET parameter 'link' should exist and should be true
if ($request->variable('link', false))
{
@@ -114,8 +123,11 @@ class ucp_auth_link
$s_hidden_fields = build_hidden_fields($s_hidden_fields);
+ // Replace "error" strings with their real, localised form
+ $error = array_map(array($user, 'lang'), $error);
+
$template->assign_vars(array(
- 'ERROR' => $this->build_error_text($error),
+ 'ERROR' => $error,
'PROVIDER_TEMPLATE_FILE' => $provider_data['TEMPLATE_FILE'],
@@ -126,20 +138,4 @@ class ucp_auth_link
$this->tpl_name = 'ucp_auth_link';
$this->page_title = 'UCP_AUTH_LINK';
}
-
- private function build_error_text(array $errors)
- {
- global $user;
-
- // Replace all errors that are language constants
- foreach ($errors as $key => $error)
- {
- if (isset($user->lang[$error]))
- {
- $errors[$key] = $user->lang($error);
- }
- }
-
- return implode('<br />', $errors);
- }
}
diff --git a/phpBB/includes/ucp/ucp_login_link.php b/phpBB/includes/ucp/ucp_login_link.php
index 2ed6a985d5..4173c54c42 100644
--- a/phpBB/includes/ucp/ucp_login_link.php
+++ b/phpBB/includes/ucp/ucp_login_link.php
@@ -91,7 +91,9 @@ class ucp_login_link
if ($result)
{
$login_link_error = $user->lang[$result];
- } else {
+ }
+ else
+ {
// Finish login
$result = $user->session_create($login_result['user_row']['user_id'], false, false, true);
diff --git a/phpBB/language/en/common.php b/phpBB/language/en/common.php
index 8c36edec58..5c7df2c5fd 100644
--- a/phpBB/language/en/common.php
+++ b/phpBB/language/en/common.php
@@ -90,6 +90,7 @@ $lang = array_merge($lang, array(
'AUTH_NO_PROFILE_CREATED' => 'The creation of a user profile was unsuccessful.',
'AUTH_PROVIDER_OAUTH_ERROR_INVALID_ENTRY' => 'Invalid database entry.',
'AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE' => 'Invalid service type provided to OAuth service handler.',
+ 'AUTH_PROVIDER_OAUTH_ERROR_SERVICE_NOT_CREATED' => 'OAuth service not created',
'AUTH_PROVIDER_OAUTH_SERVICE_BITLY' => 'Bitly',
'AUTH_PROVIDER_OAUTH_SERVICE_FACEBOOK' => 'Facebook',
'AUTH_PROVIDER_OAUTH_SERVICE_GOOGLE' => 'Google',
diff --git a/phpBB/language/en/ucp.php b/phpBB/language/en/ucp.php
index 19892ce194..03a14336b5 100644
--- a/phpBB/language/en/ucp.php
+++ b/phpBB/language/en/ucp.php
@@ -270,7 +270,7 @@ $lang = array_merge($lang, array(
'LINK_REMOTE_SIZE' => 'Avatar dimensions',
'LINK_REMOTE_SIZE_EXPLAIN' => 'Specify the width and height of the avatar, leave blank to attempt automatic verification.',
'LOGIN_EXPLAIN_UCP' => 'Please login in order to access the User Control Panel.',
- 'LOGIN_LINK' => 'Link or Register Your External Account with This Board',
+ 'LOGIN_LINK' => 'Link or register your account on an external service with your board account',
'LOGIN_LINK_EXPLAIN' => 'You have attempted to login with an external service that is not yet connected to an account on this board. You must now either link this account to an existing account or create a new account.',
'LOGIN_LINK_MISSING_DATA' => 'Data that is necessary to link your account with an external service is not available. Please restart the login process.',
'LOGIN_LINK_NO_DATA_PROVIDED' => 'No data has been provided to this page to link an external account to a forum account. Please contact the board administrator if you continue to experience problems.',
@@ -480,18 +480,18 @@ $lang = array_merge($lang, array(
'UCP_AIM' => 'AOL Instant Messenger',
'UCP_ATTACHMENTS' => 'Attachments',
'UCP_AUTH_LINK' => 'External accounts',
- 'UCP_AUTH_LINK_ASK' => 'You currently have no account tied to this external service. Click the button below to link your board account to an account with this external service.',
+ 'UCP_AUTH_LINK_ASK' => 'You currently have no account associated with this external service. Click the button below to link your board account to an account with this external service.',
'UCP_AUTH_LINK_ID' => 'Unique identifier',
'UCP_AUTH_LINK_LINK' => 'Link',
- 'UCP_AUTH_LINK_MANAGE' => 'Manage external accounts',
- 'UCP_AUTH_LINK_TITLE' => 'Manage your external accounts',
+ 'UCP_AUTH_LINK_MANAGE' => 'Manage external account associations',
+ 'UCP_AUTH_LINK_TITLE' => 'Manage your external account associations',
'UCP_AUTH_LINK_UNLINK' => 'Unlink',
'UCP_COPPA_BEFORE' => 'Before %s',
'UCP_COPPA_ON_AFTER' => 'On or after %s',
'UCP_EMAIL_ACTIVATE' => 'Please note that you will need to enter a valid email address before your account is activated. You will receive an email at the address you provide that contains an account activation link.',
'UCP_ICQ' => 'ICQ number',
'UCP_JABBER' => 'Jabber address',
- 'UCP_LOGIN_LINK' => 'Set up an external account',
+ 'UCP_LOGIN_LINK' => 'Set up an external account association',
'UCP_MAIN' => 'Overview',
'UCP_MAIN_ATTACHMENTS' => 'Manage attachments',
diff --git a/phpBB/phpbb/auth/provider/interface.php b/phpBB/phpbb/auth/provider/interface.php
index 480ee4301b..eadd5f01d1 100644
--- a/phpBB/phpbb/auth/provider/interface.php
+++ b/phpBB/phpbb/auth/provider/interface.php
@@ -45,9 +45,9 @@ interface phpbb_auth_provider_interface
* 'error_msg' => string
* 'user_row' => array
* )
- * A fourth key of the array may be present 'redirect_data'
- * This key is only used when 'status' is equal to
- * LOGIN_SUCCESS_LINK_PROFILE and it's value is an
+ * A fourth key of the array may be present:
+ * 'redirect_data' This key is only used when 'status' is
+ * equal to LOGIN_SUCCESS_LINK_PROFILE and its value is an
* associative array that is turned into GET variables on
* the redirect url.
*/
@@ -89,7 +89,7 @@ interface phpbb_auth_provider_interface
* array: 'BLOCK_VAR_NAME'. If this is present,
* then its value should be a string that is used
* to designate the name of the loop used in the
- * ACP template file. In addition to this, an
+ * ACP template file. When this is present, an
* additional key named 'BLOCK_VARS' is required.
* This must be an array containing at least one
* array of variables that will be assigned during
diff --git a/phpBB/phpbb/auth/provider/oauth/oauth.php b/phpBB/phpbb/auth/provider/oauth/oauth.php
index b427ca4e72..c1c27c979f 100644
--- a/phpBB/phpbb/auth/provider/oauth/oauth.php
+++ b/phpBB/phpbb/auth/provider/oauth/oauth.php
@@ -211,8 +211,8 @@ class phpbb_auth_provider_oauth extends phpbb_auth_provider_base
// Retrieve the user's account
$sql = 'SELECT user_id, username, user_password, user_passchg, user_pass_convert, user_email, user_type, user_login_attempts
- FROM ' . $this->users_table . "
- WHERE user_id = '" . $this->db->sql_escape($row['user_id']) . "'";
+ FROM ' . $this->users_table . '
+ WHERE user_id = ' . (int) $row['user_id'];
$result = $this->db->sql_query($sql);
$row = $this->db->sql_fetchrow($result);
$this->db->sql_freeresult($result);
@@ -231,7 +231,9 @@ class phpbb_auth_provider_oauth extends phpbb_auth_provider_base
'error_msg' => false,
'user_row' => $row,
);
- } else {
+ }
+ else
+ {
$url = $service->getAuthorizationUri();
header('Location: ' . $url);
}
@@ -291,8 +293,7 @@ class phpbb_auth_provider_oauth extends phpbb_auth_provider_base
if (!$service)
{
- // Update to an actual error message
- throw new Exception('Service not created: ' . $service_name);
+ throw new Exception('AUTH_PROVIDER_OAUTH_ERROR_SERVICE_NOT_CREATED');
}
return $service;
@@ -474,7 +475,7 @@ class phpbb_auth_provider_oauth extends phpbb_auth_provider_base
}
/**
- * Performs the account linking for login_link
+ * Performs the account linking for auth_link
*
* @param array $link_data The same variable given to {@see phpbb_auth_provider_interface::link_account}
* @param string $service_name The name of the service being used in
@@ -503,7 +504,9 @@ class phpbb_auth_provider_oauth extends phpbb_auth_provider_base
);
$this->link_account_perform_link($data);
- } else {
+ }
+ else
+ {
$url = $service->getAuthorizationUri();
header('Location: ' . $url);
}
diff --git a/phpBB/phpbb/auth/provider/oauth/service/bitly.php b/phpBB/phpbb/auth/provider/oauth/service/bitly.php
index 0918f577ec..59e66c7c34 100644
--- a/phpBB/phpbb/auth/provider/oauth/service/bitly.php
+++ b/phpBB/phpbb/auth/provider/oauth/service/bitly.php
@@ -71,10 +71,10 @@ class phpbb_auth_provider_oauth_service_bitly extends phpbb_auth_provider_oauth_
}
// This was a callback request from bitly, get the token
- $this->service_provider->requestAccessToken( $this->request->variable('code', '') );
+ $this->service_provider->requestAccessToken($this->request->variable('code', ''));
// Send a request with it
- $result = json_decode( $this->service_provider->request('user/info'), true );
+ $result = json_decode($this->service_provider->request('user/info'), true);
// Return the unique identifier returned from bitly
return $result['data']['login'];
@@ -91,7 +91,7 @@ class phpbb_auth_provider_oauth_service_bitly extends phpbb_auth_provider_oauth_
}
// Send a request with it
- $result = json_decode( $this->service_provider->request('user/info'), true );
+ $result = json_decode($this->service_provider->request('user/info'), true);
// Return the unique identifier returned from bitly
return $result['data']['login'];
diff --git a/phpBB/phpbb/auth/provider/oauth/service/facebook.php b/phpBB/phpbb/auth/provider/oauth/service/facebook.php
index 836e4ee052..b853c8c8a5 100644
--- a/phpBB/phpbb/auth/provider/oauth/service/facebook.php
+++ b/phpBB/phpbb/auth/provider/oauth/service/facebook.php
@@ -66,15 +66,14 @@ class phpbb_auth_provider_oauth_service_facebook extends phpbb_auth_provider_oau
{
if (!($this->service_provider instanceof \OAuth\OAuth2\Service\Facebook))
{
- // TODO: make exception class and use language constant
throw new Exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE');
}
// This was a callback request, get the token
- $this->service_provider->requestAccessToken( $this->request->variable('code', '') );
+ $this->service_provider->requestAccessToken($this->request->variable('code', ''));
// Send a request with it
- $result = json_decode( $this->service_provider->request('/me'), true );
+ $result = json_decode($this->service_provider->request('/me'), true);
// Return the unique identifier
return $result['id'];
@@ -87,12 +86,11 @@ class phpbb_auth_provider_oauth_service_facebook extends phpbb_auth_provider_oau
{
if (!($this->service_provider instanceof \OAuth\OAuth2\Service\Facebook))
{
- // TODO: make exception class and use language constant
- throw new Exception('Invalid service provider type');
+ throw new Exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE');
}
// Send a request with it
- $result = json_decode( $this->service_provider->request('/me'), true );
+ $result = json_decode($this->service_provider->request('/me'), true);
// Return the unique identifier
return $result['id'];
diff --git a/phpBB/phpbb/auth/provider/oauth/service/google.php b/phpBB/phpbb/auth/provider/oauth/service/google.php
index 9c782bcaa0..eb4ad6317a 100644
--- a/phpBB/phpbb/auth/provider/oauth/service/google.php
+++ b/phpBB/phpbb/auth/provider/oauth/service/google.php
@@ -81,10 +81,10 @@ class phpbb_auth_provider_oauth_service_google extends phpbb_auth_provider_oauth
}
// This was a callback request, get the token
- $this->service_provider->requestAccessToken( $this->request->variable('code', '') );
+ $this->service_provider->requestAccessToken($this->request->variable('code', ''));
// Send a request with it
- $result = json_decode( $this->service_provider->request('https://www.googleapis.com/oauth2/v1/userinfo'), true );
+ $result = json_decode($this->service_provider->request('https://www.googleapis.com/oauth2/v1/userinfo'), true);
// Return the unique identifier
return $result['id'];
@@ -101,7 +101,7 @@ class phpbb_auth_provider_oauth_service_google extends phpbb_auth_provider_oauth
}
// Send a request with it
- $result = json_decode( $this->service_provider->request('https://www.googleapis.com/oauth2/v1/userinfo'), true );
+ $result = json_decode($this->service_provider->request('https://www.googleapis.com/oauth2/v1/userinfo'), true);
// Return the unique identifier
return $result['id'];
diff --git a/phpBB/phpbb/auth/provider/oauth/token_storage.php b/phpBB/phpbb/auth/provider/oauth/token_storage.php
index b31ffcd1ab..05e308d192 100644
--- a/phpBB/phpbb/auth/provider/oauth/token_storage.php
+++ b/phpBB/phpbb/auth/provider/oauth/token_storage.php
@@ -83,7 +83,8 @@ class phpbb_auth_provider_oauth_token_storage implements TokenStorageInterface
*/
public function retrieveAccessToken()
{
- if( $this->cachedToken instanceOf TokenInterface ) {
+ if ($this->cachedToken instanceOf TokenInterface)
+ {
return $this->cachedToken;
}
@@ -92,7 +93,7 @@ class phpbb_auth_provider_oauth_token_storage implements TokenStorageInterface
'provider' => $this->service_name,
);
- if ($this->user->data['user_id'] == ANONYMOUS)
+ if ($this->user->data['user_id'] === ANONYMOUS)
{
$data['session_id'] = $this->user->data['session_id'];
}
@@ -124,7 +125,7 @@ class phpbb_auth_provider_oauth_token_storage implements TokenStorageInterface
*/
public function hasAccessToken()
{
- if( $this->cachedToken ) {
+ if ($this->cachedToken) {
return true;
}
@@ -133,7 +134,7 @@ class phpbb_auth_provider_oauth_token_storage implements TokenStorageInterface
'provider' => $this->service_name,
);
- if ($this->user->data['user_id'] == ANONYMOUS)
+ if ($this->user->data['user_id'] === ANONYMOUS)
{
$data['session_id'] = $this->user->data['session_id'];
}
@@ -149,12 +150,12 @@ class phpbb_auth_provider_oauth_token_storage implements TokenStorageInterface
$this->cachedToken = null;
$sql = 'DELETE FROM ' . $this->auth_provider_oauth_table . '
- WHERE user_id = ' . $this->user->data['user_id'] . '
- AND provider = \'' . $this->db->sql_escape($this->service_name) . '\'';
+ WHERE user_id = ' . $this->user->data['user_id'] . "
+ AND provider = '" . $this->db->sql_escape($this->service_name) . "'";
- if ($this->user->data['user_id'] == ANONYMOUS)
+ if ($this->user->data['user_id'] === ANONYMOUS)
{
- $sql .= ' AND session_id = \'' . $this->user->data['session_id'] . '\'';
+ $sql .= " AND session_id = '" . $this->user->data['session_id'] . "'";
}
$this->db->sql_query($sql);
@@ -176,8 +177,8 @@ class phpbb_auth_provider_oauth_token_storage implements TokenStorageInterface
SET ' . $this->db->sql_build_array('UPDATE', array(
'user_id' => (int) $user_id
)) . '
- WHERE user_id = ' . $this->user->data['user_id'] . '
- AND session_id = \'' . $this->user->data['session_id'] . '\'';
+ WHERE user_id = ' . $this->user->data['user_id'] . "
+ AND session_id = '" . $this->user->data['session_id'] . "'";
$this->db->sql_query($sql);
}
@@ -188,7 +189,8 @@ class phpbb_auth_provider_oauth_token_storage implements TokenStorageInterface
*/
public function has_access_token_by_session()
{
- if( $this->cachedToken ) {
+ if ($this->cachedToken)
+ {
return true;
}
@@ -208,19 +210,12 @@ class phpbb_auth_provider_oauth_token_storage implements TokenStorageInterface
*/
protected function _has_acess_token($data)
{
- $row = $this->get_access_token_row($data);
-
- if (!$row)
- {
- return false;
- }
-
- return true;
+ return (bool) $this->get_access_token_row($data);
}
public function retrieve_access_token_by_session()
{
- if( $this->cachedToken instanceOf TokenInterface ) {
+ if ($this->cachedToken instanceOf TokenInterface) {
return $this->cachedToken;
}