[feature/oauth] Changes due to code review

PHPBB3-11673
author: Joseph Warner <hardolaf@hardolaf.com> 2013-08-24 22:00:16 -0400
committer: Joseph Warner <hardolaf@hardolaf.com> 2013-08-24 22:04:04 -0400
commit: a8ffbce99f9ea99bd1fdca0e009001026e2d6950 (patch)
tree: 1cae1d43fee4c417eedd8303f581eb3109d083ab /phpBB/phpbb/auth
parent: 310caec5d92d58453d1eee40e9b5a7f0157bd5ea (diff)
download: forums-a8ffbce99f9ea99bd1fdca0e009001026e2d6950.tar
forums-a8ffbce99f9ea99bd1fdca0e009001026e2d6950.tar.gz
forums-a8ffbce99f9ea99bd1fdca0e009001026e2d6950.tar.bz2
forums-a8ffbce99f9ea99bd1fdca0e009001026e2d6950.tar.xz
forums-a8ffbce99f9ea99bd1fdca0e009001026e2d6950.zip
6 files changed, 39 insertions, 43 deletions
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;
 		}
author	Joseph Warner <hardolaf@hardolaf.com>	2013-08-24 22:00:16 -0400
committer	Joseph Warner <hardolaf@hardolaf.com>	2013-08-24 22:04:04 -0400
commit	a8ffbce99f9ea99bd1fdca0e009001026e2d6950 (patch)
tree	1cae1d43fee4c417eedd8303f581eb3109d083ab /phpBB/phpbb/auth
parent	310caec5d92d58453d1eee40e9b5a7f0157bd5ea (diff)
download	forums-a8ffbce99f9ea99bd1fdca0e009001026e2d6950.tar forums-a8ffbce99f9ea99bd1fdca0e009001026e2d6950.tar.gz forums-a8ffbce99f9ea99bd1fdca0e009001026e2d6950.tar.bz2 forums-a8ffbce99f9ea99bd1fdca0e009001026e2d6950.tar.xz forums-a8ffbce99f9ea99bd1fdca0e009001026e2d6950.zip