[ticket/16008] Clean up phpBB OAuth system

PHPBB3-16008

phpBB/config/default/container/services_auth.yml

@@ -1,9 +1,9 @@
 services:
-# ----- Auth management -----
+    # ----- Auth management -----
     auth:
         class: phpbb\auth\auth
 
-# ----- Auth providers -----
+    # ----- Auth providers -----
     auth.provider_collection:
         class: phpbb\auth\provider_collection
         arguments:
@@ -52,24 +52,25 @@ services:
     auth.provider.oauth:
         class: phpbb\auth\provider\oauth\oauth
         arguments:
-            - '@dbal.conn'
             - '@config'
+            - '@service_container'
+            - '@dbal.conn'
+            - '@dispatcher'
+            - '@language'
             - '@passwords.manager'
             - '@request'
+            - '@auth.provider.oauth.service_collection'
             - '@user'
             - '%tables.auth_provider_oauth_token_storage%'
             - '%tables.auth_provider_oauth_states%'
             - '%tables.auth_provider_oauth_account_assoc%'
-            - '@auth.provider.oauth.service_collection'
             - '%tables.users%'
-            - '@service_container'
-            - '@dispatcher'
             - '%core.root_path%'
             - '%core.php_ext%'
         tags:
             - { name: auth.provider }
 
-# ----- OAuth services providers -----
+    # ----- OAuth services providers -----
     auth.provider.oauth.service_collection:
         class: phpbb\di\service_collection
         arguments:

phpBB/language/en/common.php

@@ -94,6 +94,7 @@ $lang = array_merge($lang, array(
 	'AUTH_PROVIDER_OAUTH_ERROR_ALREADY_LINKED'				=> 'This external service is already associated with another board account.',
 	'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_REQUEST'						=> 'Something went wrong when processing your OAuth request.',
 	'AUTH_PROVIDER_OAUTH_ERROR_SERVICE_NOT_CREATED'			=> 'OAuth service not created',
 	'AUTH_PROVIDER_OAUTH_SERVICE_BITLY'						=> 'Bitly',
 	'AUTH_PROVIDER_OAUTH_SERVICE_FACEBOOK'					=> 'Facebook',

phpBB/phpbb/auth/provider/base.php

@@ -16,7 +16,7 @@ namespace phpbb\auth\provider;
 /**
 * Base authentication provider class that all other providers should implement
 */
-abstract class base implements \phpbb\auth\provider\provider_interface
+abstract class base implements provider_interface
 {
 	/**
 	* {@inheritdoc}

phpBB/phpbb/auth/provider/oauth/service/base.php

@@ -1,22 +1,22 @@
 <?php
 /**
-*
+ *
-* This file is part of the phpBB Forum Software package.
+ * This file is part of the phpBB Forum Software package.
-*
+ *
-* @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
-* @license GNU General Public License, version 2 (GPL-2.0)
+ * @license GNU General Public License, version 2 (GPL-2.0)
-*
+ *
-* For full copyright and license information, please see
+ * For full copyright and license information, please see
-* the docs/CREDITS.txt file.
+ * the docs/CREDITS.txt file.
-*
+ *
-*/
+ */
 
 namespace phpbb\auth\provider\oauth\service;
 
 /**
-* Base OAuth abstract class that all OAuth services should implement
+ * Base OAuth abstract class that all OAuth services should implement
-*/
+ */
-abstract class base implements \phpbb\auth\provider\oauth\service\service_interface
+abstract class base implements service_interface
 {
 	/**
 	 * External OAuth service provider
@@ -28,17 +28,17 @@ abstract class base implements \phpbb\auth\provider\oauth\service\service_interf
 	/**
 	 * {@inheritdoc}
 	 */
-	public function get_external_service_provider()
+	public function get_auth_scope()
 	{
-		return $this->service_provider;
+		return [];
 	}
 
 	/**
 	 * {@inheritdoc}
 	 */
-	public function get_auth_scope()
+	public function get_external_service_class()
 	{
-		return array();
+		return '';
 	}
 
 	/**
@@ -48,4 +48,12 @@ abstract class base implements \phpbb\auth\provider\oauth\service\service_interf
 	{
 		$this->service_provider = $service_provider;
 	}
+
+	/**
+	 * {@inheritdoc}
+	 */
+	public function get_external_service_provider()
+	{
+		return $this->service_provider;
+	}
 }

phpBB/phpbb/auth/provider/oauth/service/bitly.php

@@ -1,42 +1,34 @@
 <?php
 /**
-*
+ *
-* This file is part of the phpBB Forum Software package.
+ * This file is part of the phpBB Forum Software package.
-*
+ *
-* @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
-* @license GNU General Public License, version 2 (GPL-2.0)
+ * @license GNU General Public License, version 2 (GPL-2.0)
-*
+ *
-* For full copyright and license information, please see
+ * For full copyright and license information, please see
-* the docs/CREDITS.txt file.
+ * the docs/CREDITS.txt file.
-*
+ *
-*/
+ */
 
 namespace phpbb\auth\provider\oauth\service;
 
 /**
-* Bitly OAuth service
+ * Bitly OAuth service
-*/
-class bitly extends \phpbb\auth\provider\oauth\service\base
-{
-	/**
-	* phpBB config
-	*
-	* @var \phpbb\config\config
  */
+class bitly extends base
+{
+	/** @var \phpbb\config\config */
 	protected $config;
 
-	/**
+	/** @var \phpbb\request\request_interface */
-	* phpBB request
-	*
-	* @var \phpbb\request\request_interface
-	*/
 	protected $request;
 
 	/**
-	* Constructor
+	 * Constructor.
 	 *
-	* @param	\phpbb\config\config				$config
+	 * @param \phpbb\config\config				$config		Config object
-	* @param	\phpbb\request\request_interface	$request
+	 * @param \phpbb\request\request_interface	$request	Request object
 	 */
 	public function __construct(\phpbb\config\config $config, \phpbb\request\request_interface $request)
 	{
@@ -49,10 +41,10 @@ class bitly extends \phpbb\auth\provider\oauth\service\base
 	 */
 	public function get_service_credentials()
 	{
-		return array(
+		return [
 			'key'		=> $this->config['auth_oauth_bitly_key'],
 			'secret'	=> $this->config['auth_oauth_bitly_secret'],
-		);
+		];
 	}
 
 	/**
@@ -62,14 +54,28 @@ class bitly extends \phpbb\auth\provider\oauth\service\base
 	{
 		if (!($this->service_provider instanceof \OAuth\OAuth2\Service\Bitly))
 		{
-			throw new \phpbb\auth\provider\oauth\service\exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE');
+			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE');
 		}
 
-		// This was a callback request from bitly, get the token
+		try
+		{
+			// This was a callback request, get the token
 			$this->service_provider->requestAccessToken($this->request->variable('code', ''));
+		}
+		catch (\OAuth\Common\Http\Exception\TokenResponseException $e)
+		{
+			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_REQUEST');
+		}
 
+		try
+		{
 			// Send a request with it
-		$result = json_decode($this->service_provider->request('user/info'), true);
+			$result = (array) json_decode($this->service_provider->request('user/info'), true);
+		}
+		catch (\OAuth\Common\Exception\Exception $e)
+		{
+			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_REQUEST');
+		}
 
 		// Return the unique identifier returned from bitly
 		return $result['data']['login'];
@@ -82,13 +88,20 @@ class bitly extends \phpbb\auth\provider\oauth\service\base
 	{
 		if (!($this->service_provider instanceof \OAuth\OAuth2\Service\Bitly))
 		{
-			throw new \phpbb\auth\provider\oauth\service\exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE');
+			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE');
 		}
 
+		try
+		{
 			// Send a request with it
-		$result = json_decode($this->service_provider->request('user/info'), true);
+			$result = (array) json_decode($this->service_provider->request('user/info'), true);
+		}
+		catch (\OAuth\Common\Exception\Exception $e)
+		{
+			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_REQUEST');
+		}
 
-		// Return the unique identifier returned from bitly
+		// Return the unique identifier
 		return $result['data']['login'];
 	}
 }

phpBB/phpbb/auth/provider/oauth/service/facebook.php

@@ -1,42 +1,34 @@
 <?php
 /**
-*
+ *
-* This file is part of the phpBB Forum Software package.
+ * This file is part of the phpBB Forum Software package.
-*
+ *
-* @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
-* @license GNU General Public License, version 2 (GPL-2.0)
+ * @license GNU General Public License, version 2 (GPL-2.0)
-*
+ *
-* For full copyright and license information, please see
+ * For full copyright and license information, please see
-* the docs/CREDITS.txt file.
+ * the docs/CREDITS.txt file.
-*
+ *
-*/
+ */
 
 namespace phpbb\auth\provider\oauth\service;
 
 /**
-* Facebook OAuth service
+ * Facebook OAuth service
-*/
+ */
 class facebook extends base
 {
-	/**
+	/** @var \phpbb\config\config */
-	* phpBB config
-	*
-	* @var \phpbb\config\config
-	*/
 	protected $config;
 
-	/**
+	/** @var \phpbb\request\request_interface */
-	* phpBB request
-	*
-	* @var \phpbb\request\request_interface
-	*/
 	protected $request;
 
 	/**
-	* Constructor
+	 * Constructor.
 	 *
-	* @param	\phpbb\config\config				$config
+	 * @param \phpbb\config\config				$config		Config object
-	* @param	\phpbb\request\request_interface 	$request
+	 * @param \phpbb\request\request_interface	$request	Request object
 	 */
 	public function __construct(\phpbb\config\config $config, \phpbb\request\request_interface $request)
 	{
@@ -49,10 +41,10 @@ class facebook extends base
 	 */
 	public function get_service_credentials()
 	{
-		return array(
+		return [
 			'key'		=> $this->config['auth_oauth_facebook_key'],
 			'secret'	=> $this->config['auth_oauth_facebook_secret'],
-		);
+		];
 	}
 
 	/**
@@ -65,11 +57,25 @@ class facebook extends base
 			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE');
 		}
 
+		try
+		{
 			// This was a callback request, get the token
 			$this->service_provider->requestAccessToken($this->request->variable('code', ''));
+		}
+		catch (\OAuth\Common\Http\Exception\TokenResponseException $e)
+		{
+			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_REQUEST');
+		}
 
+		try
+		{
 			// Send a request with it
-		$result = json_decode($this->service_provider->request('/me'), true);
+			$result = (array) json_decode($this->service_provider->request('/me'), true);
+		}
+		catch (\OAuth\Common\Exception\Exception $e)
+		{
+			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_REQUEST');
+		}
 
 		// Return the unique identifier
 		return $result['id'];
@@ -85,8 +91,15 @@ class facebook extends base
 			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE');
 		}
 
+		try
+		{
 			// Send a request with it
-		$result = json_decode($this->service_provider->request('/me'), true);
+			$result = (array) json_decode($this->service_provider->request('/me'), true);
+		}
+		catch (\OAuth\Common\Exception\Exception $e)
+		{
+			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_REQUEST');
+		}
 
 		// Return the unique identifier
 		return $result['id'];

phpBB/phpbb/auth/provider/oauth/service/google.php

@@ -1,42 +1,34 @@
 <?php
 /**
-*
+ *
-* This file is part of the phpBB Forum Software package.
+ * This file is part of the phpBB Forum Software package.
-*
+ *
-* @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
-* @license GNU General Public License, version 2 (GPL-2.0)
+ * @license GNU General Public License, version 2 (GPL-2.0)
-*
+ *
-* For full copyright and license information, please see
+ * For full copyright and license information, please see
-* the docs/CREDITS.txt file.
+ * the docs/CREDITS.txt file.
-*
+ *
-*/
+ */
 
 namespace phpbb\auth\provider\oauth\service;
 
 /**
-* Google OAuth service
+ * Google OAuth service
-*/
+ */
 class google extends base
 {
-	/**
+	/** @var \phpbb\config\config */
-	* phpBB config
-	*
-	* @var \phpbb\config\config
-	*/
 	protected $config;
 
-	/**
+	/** @var \phpbb\request\request_interface */
-	* phpBB request
-	*
-	* @var \phpbb\request\request_interface
-	*/
 	protected $request;
 
 	/**
-	* Constructor
+	 * Constructor.
 	 *
-	* @param	\phpbb\config\config				$config
+	 * @param \phpbb\config\config				$config		Config object
-	* @param	\phpbb\request\request_interface 	$request
+	 * @param \phpbb\request\request_interface	$request	Request object
 	 */
 	public function __construct(\phpbb\config\config $config, \phpbb\request\request_interface $request)
 	{
@@ -49,10 +41,10 @@ class google extends base
 	 */
 	public function get_auth_scope()
 	{
-		return array(
+		return [
 			'userinfo_email',
 			'userinfo_profile',
-		);
+		];
 	}
 
 	/**
@@ -60,10 +52,10 @@ class google extends base
 	 */
 	public function get_service_credentials()
 	{
-		return array(
+		return [
 			'key'		=> $this->config['auth_oauth_google_key'],
 			'secret'	=> $this->config['auth_oauth_google_secret'],
-		);
+		];
 	}
 
 	/**
@@ -76,11 +68,25 @@ class google extends base
 			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE');
 		}
 
+		try
+		{
 			// This was a callback request, get the token
 			$this->service_provider->requestAccessToken($this->request->variable('code', ''));
+		}
+		catch (\OAuth\Common\Http\Exception\TokenResponseException $e)
+		{
+			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_REQUEST');
+		}
 
+		try
+		{
 			// Send a request with it
-		$result = json_decode($this->service_provider->request('https://www.googleapis.com/oauth2/v1/userinfo'), true);
+			$result = (array) json_decode($this->service_provider->request('https://www.googleapis.com/oauth2/v1/userinfo'), true);
+		}
+		catch (\OAuth\Common\Exception\Exception $e)
+		{
+			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_REQUEST');
+		}
 
 		// Return the unique identifier
 		return $result['id'];
@@ -96,8 +102,15 @@ class google extends base
 			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE');
 		}
 
+		try
+		{
 			// Send a request with it
-		$result = json_decode($this->service_provider->request('https://www.googleapis.com/oauth2/v1/userinfo'), true);
+			$result = (array) json_decode($this->service_provider->request('https://www.googleapis.com/oauth2/v1/userinfo'), true);
+		}
+		catch (\OAuth\Common\Exception\Exception $e)
+		{
+			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_REQUEST');
+		}
 
 		// Return the unique identifier
 		return $result['id'];

phpBB/phpbb/auth/provider/oauth/service/service_interface.php

@@ -1,21 +1,21 @@
 <?php
 /**
-*
+ *
-* This file is part of the phpBB Forum Software package.
+ * This file is part of the phpBB Forum Software package.
-*
+ *
-* @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
-* @license GNU General Public License, version 2 (GPL-2.0)
+ * @license GNU General Public License, version 2 (GPL-2.0)
-*
+ *
-* For full copyright and license information, please see
+ * For full copyright and license information, please see
-* the docs/CREDITS.txt file.
+ * the docs/CREDITS.txt file.
-*
+ *
-*/
+ */
 
 namespace phpbb\auth\provider\oauth\service;
 
 /**
-* OAuth service interface
+ * OAuth service interface
-*/
+ */
 interface service_interface
 {
 	/**
@@ -25,13 +25,6 @@ interface service_interface
 	 */
 	public function get_auth_scope();
 
-	/**
-	* Returns the external library service provider once it has been set
-	*
-	* @param	\OAuth\Common\Service\ServiceInterface|null
-	*/
-	public function get_external_service_provider();
-
 	/**
 	 * Returns an array containing the service credentials belonging to requested
 	 * service.
@@ -64,10 +57,31 @@ interface service_interface
 	 */
 	public function perform_token_auth();
 
+	/**
+	 * Returns the class of external library service provider that has to be used.
+	 *
+	 * @return string	If the string is a class, it will register the provided string as a class,
+	 *						which later will be generated as the OAuth external service provider.
+	 * 					If the string is not a class, it will use this string,
+	 * 						trying to generate a service for the version 2 and 1 respectively:
+	 * 						\OAuth\OAuth2\Service\<string>
+	 * 					If the string is empty, it will default to OAuth's standard service classes,
+	 * 						trying to generate a service for the version 2 and 1 respectively:
+	 * 						\OAuth\OAuth2\Service\Facebook
+	 */
+	public function get_external_service_class();
+
 	/**
 	 * Sets the external library service provider
 	 *
 	 * @param \OAuth\Common\Service\ServiceInterface	$service_provider
 	 */
 	public function set_external_service_provider(\OAuth\Common\Service\ServiceInterface $service_provider);
+
+	/**
+	 * Returns the external library service provider once it has been set
+	 *
+	 * @param \OAuth\Common\Service\ServiceInterface|null
+	 */
+	public function get_external_service_provider();
 }

phpBB/phpbb/auth/provider/oauth/service/twitter.php

@@ -1,42 +1,34 @@
 <?php
 /**
-*
+ *
-* This file is part of the phpBB Forum Software package.
+ * This file is part of the phpBB Forum Software package.
-*
+ *
-* @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
-* @license GNU General Public License, version 2 (GPL-2.0)
+ * @license GNU General Public License, version 2 (GPL-2.0)
-*
+ *
-* For full copyright and license information, please see
+ * For full copyright and license information, please see
-* the docs/CREDITS.txt file.
+ * the docs/CREDITS.txt file.
-*
+ *
-*/
+ */
 
 namespace phpbb\auth\provider\oauth\service;
 
 /**
-* Twitter OAuth service
+ * Twitter OAuth service
-*/
-class twitter extends \phpbb\auth\provider\oauth\service\base
-{
-	/**
-	* phpBB config
-	*
-	* @var \phpbb\config\config
  */
+class twitter extends base
+{
+	/** @var \phpbb\config\config */
 	protected $config;
 
-	/**
+	/** @var \phpbb\request\request_interface */
-	* phpBB request
-	*
-	* @var \phpbb\request\request_interface
-	*/
 	protected $request;
 
 	/**
-	* Constructor
+	 * Constructor.
 	 *
-	* @param	\phpbb\config\config				$config
+	 * @param \phpbb\config\config				$config		Config object
-	* @param	\phpbb\request\request_interface	$request
+	 * @param \phpbb\request\request_interface	$request	Request object
 	 */
 	public function __construct(\phpbb\config\config $config, \phpbb\request\request_interface $request)
 	{
@@ -49,10 +41,10 @@ class twitter extends \phpbb\auth\provider\oauth\service\base
 	 */
 	public function get_service_credentials()
 	{
-		return array(
+		return [
 			'key'		=> $this->config['auth_oauth_twitter_key'],
 			'secret'	=> $this->config['auth_oauth_twitter_secret'],
-		);
+		];
 	}
 
 	/**
@@ -62,24 +54,41 @@ class twitter extends \phpbb\auth\provider\oauth\service\base
 	{
 		if (!($this->service_provider instanceof \OAuth\OAuth1\Service\Twitter))
 		{
-			throw new \phpbb\auth\provider\oauth\service\exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE');
+			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE');
 		}
 
 		$storage = $this->service_provider->getStorage();
-		$token = $storage->retrieveAccessToken('Twitter');
-		$tokensecret = $token->getRequestTokenSecret();
 
-		// This was a callback request from twitter, get the token
+		try
+		{
+			/** @var \OAuth\OAuth1\Token\TokenInterface $token */
+			$token = $storage->retrieveAccessToken('Twitter');
+		}
+		catch (\OAuth\Common\Storage\Exception\TokenNotFoundException $e)
+		{
+			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_REQUEST');
+		}
+
+		$secret = $token->getRequestTokenSecret();
+
+		try
+		{
+			// This was a callback request, get the token
 			$this->service_provider->requestAccessToken(
 				$this->request->variable('oauth_token', ''),
 				$this->request->variable('oauth_verifier', ''),
-			$tokensecret
+				$secret
 			);
+		}
+		catch (\OAuth\Common\Http\Exception\TokenResponseException $e)
+		{
+			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_REQUEST');
+		}
 
 		// Send a request with it
-		$result = json_decode($this->service_provider->request('account/verify_credentials.json'), true);
+		$result = (array) json_decode($this->service_provider->request('account/verify_credentials.json'), true);
 
-		// Return the unique identifier returned from twitter
+		// Return the unique identifier
 		return $result['id'];
 	}
 
@@ -90,13 +99,13 @@ class twitter extends \phpbb\auth\provider\oauth\service\base
 	{
 		if (!($this->service_provider instanceof \OAuth\OAuth1\Service\Twitter))
 		{
-			throw new \phpbb\auth\provider\oauth\service\exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE');
+			throw new exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE');
 		}
 
 		// Send a request with it
-		$result = json_decode($this->service_provider->request('account/verify_credentials.json'), true);
+		$result = (array) json_decode($this->service_provider->request('account/verify_credentials.json'), true);
 
-		// Return the unique identifier returned from twitter
+		// Return the unique identifier
 		return $result['id'];
 	}
 }

phpBB/phpbb/auth/provider/oauth/token_storage.php

@@ -1,15 +1,15 @@
 <?php
 /**
-*
+ *
-* This file is part of the phpBB Forum Software package.
+ * This file is part of the phpBB Forum Software package.
-*
+ *
-* @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
-* @license GNU General Public License, version 2 (GPL-2.0)
+ * @license GNU General Public License, version 2 (GPL-2.0)
-*
+ *
-* For full copyright and license information, please see
+ * For full copyright and license information, please see
-* the docs/CREDITS.txt file.
+ * the docs/CREDITS.txt file.
-*
+ *
-*/
+ */
 
 namespace phpbb\auth\provider\oauth;
 
@@ -20,60 +20,41 @@ use OAuth\Common\Storage\Exception\TokenNotFoundException;
 use OAuth\Common\Storage\Exception\AuthorizationStateNotFoundException;
 
 /**
-* OAuth storage wrapper for phpbb's cache
+ * OAuth storage wrapper for phpBB3's cache
-*/
+ */
 class token_storage implements TokenStorageInterface
 {
-	/**
+	/** @var \phpbb\db\driver\driver_interface */
-	* Cache driver.
-	*
-	* @var \phpbb\db\driver\driver_interface
-	*/
 	protected $db;
 
-	/**
+	/** @var \phpbb\user */
-	* phpBB user
-	*
-	* @var \phpbb\user
-	*/
 	protected $user;
 
-	/**
+	/** @var string OAuth table: token storage */
-	* OAuth token table
-	*
-	* @var string
-	*/
 	protected $oauth_token_table;
 
-	/**
+	/** @var string OAuth table: state */
-	* OAuth state table
-	*
-	* @var string
-	*/
 	protected $oauth_state_table;
 
-	/**
+	/** @var TokenInterface OAuth token */
-	* @var object|TokenInterface
-	*/
 	protected $cachedToken;
 
-	/**
+	/** @var string OAuth state */
-	* @var string
-	*/
 	protected $cachedState;
 
 	/**
-	* Creates token storage for phpBB.
+	 * Constructor.
 	 *
-	* @param	\phpbb\db\driver\driver_interface	$db
+	 * @param \phpbb\db\driver\driver_interface	$db					Database object
-	* @param	\phpbb\user		$user
+	 * @param \phpbb\user						$user				User object
-	* @param	string			$oauth_token_table
+	 * @param string							$oauth_token_table	OAuth table: token storage
-	* @param	string			$oauth_state_table
+	 * @param string							$oauth_state_table	OAuth table: state
 	 */
 	public function __construct(\phpbb\db\driver\driver_interface $db, \phpbb\user $user, $oauth_token_table, $oauth_state_table)
 	{
 		$this->db	= $db;
 		$this->user	= $user;
+
 		$this->oauth_token_table = $oauth_token_table;
 		$this->oauth_state_table = $oauth_state_table;
 	}
@@ -90,10 +71,10 @@ class token_storage implements TokenStorageInterface
 			return $this->cachedToken;
 		}
 
-		$data = array(
+		$data = [
 			'user_id'	=> (int) $this->user->data['user_id'],
 			'provider'	=> $service,
-		);
+		];
 
 		if ((int) $this->user->data['user_id'] === ANONYMOUS)
 		{
@@ -112,25 +93,30 @@ class token_storage implements TokenStorageInterface
 
 		$this->cachedToken = $token;
 
-		$data = array(
+		$data = [
 			'oauth_token'	=> $this->json_encode_token($token),
-		);
+		];
 
 		$sql = 'UPDATE ' . $this->oauth_token_table . '
 			SET ' . $this->db->sql_build_array('UPDATE', $data) . '
-				WHERE user_id = ' . (int) $this->user->data['user_id'] . '
+			WHERE user_id = ' . (int) $this->user->data['user_id'] . "
-					' . ((int) $this->user->data['user_id'] === ANONYMOUS ? "AND session_id = '" . $this->db->sql_escape($this->user->data['session_id']) . "'" : '') . "
 				AND provider = '" . $this->db->sql_escape($service) . "'";
+
+		if ((int) $this->user->data['user_id'] === ANONYMOUS)
+		{
+			$sql .= " AND session_id = '" . $this->db->sql_escape($this->user->data['session_id']) . "'";
+		}
+
 		$this->db->sql_query($sql);
 
 		if (!$this->db->sql_affectedrows())
 		{
-			$data = array(
+			$data = [
 				'user_id'		=> (int) $this->user->data['user_id'],
 				'provider'		=> $service,
 				'oauth_token'	=> $this->json_encode_token($token),
 				'session_id'	=> $this->user->data['session_id'],
-			);
+			];
 
 			$sql = 'INSERT INTO ' . $this->oauth_token_table . $this->db->sql_build_array('INSERT', $data);
 
@@ -152,17 +138,17 @@ class token_storage implements TokenStorageInterface
 			return true;
 		}
 
-		$data = array(
+		$data = [
 			'user_id'	=> (int) $this->user->data['user_id'],
 			'provider'	=> $service,
-		);
+		];
 
 		if ((int) $this->user->data['user_id'] === ANONYMOUS)
 		{
 			$data['session_id']	= $this->user->data['session_id'];
 		}
 
-		return $this->_has_acess_token($data);
+		return $this->_has_access_token($data);
 	}
 
 	/**
@@ -217,15 +203,14 @@ class token_storage implements TokenStorageInterface
 
 		$this->cachedState = $state;
 
-		$data = array(
+		$data = [
 			'user_id'		=> (int) $this->user->data['user_id'],
 			'provider'		=> $service,
 			'oauth_state'	=> $state,
 			'session_id'	=> $this->user->data['session_id'],
-		);
+		];
 
-		$sql = 'INSERT INTO ' . $this->oauth_state_table . '
+		$sql = 'INSERT INTO ' . $this->oauth_state_table . ' ' . $this->db->sql_build_array('INSERT', $data);
-			' . $this->db->sql_build_array('INSERT', $data);
 		$this->db->sql_query($sql);
 
 		return $this;
@@ -243,10 +228,10 @@ class token_storage implements TokenStorageInterface
 			return true;
 		}
 
-		$data = array(
+		$data = [
 			'user_id'	=> (int) $this->user->data['user_id'],
 			'provider'	=> $service,
-		);
+		];
 
 		if ((int) $this->user->data['user_id'] === ANONYMOUS)
 		{
@@ -268,10 +253,10 @@ class token_storage implements TokenStorageInterface
 			return $this->cachedState;
 		}
 
-		$data = array(
+		$data = [
 			'user_id'	=> (int) $this->user->data['user_id'],
 			'provider'	=> $service,
-		);
+		];
 
 		if ((int) $this->user->data['user_id'] === ANONYMOUS)
 		{
@@ -325,9 +310,10 @@ class token_storage implements TokenStorageInterface
 	}
 
 	/**
-	* Updates the user_id field in the database assosciated with the token
+	 * Updates the user_id field in the database associated with the token.
 	 *
-	* @param	int	$user_id
+	 * @param int		$user_id	The user identifier
+	 * @return void
 	 */
 	public function set_user_id($user_id)
 	{
@@ -336,20 +322,23 @@ class token_storage implements TokenStorageInterface
 			return;
 		}
 
+		$data = [
+			'user_id' => (int) $user_id,
+		];
+
 		$sql = 'UPDATE ' . $this->oauth_token_table . '
-			SET ' . $this->db->sql_build_array('UPDATE', array(
+			SET ' . $this->db->sql_build_array('UPDATE', $data) . '
-					'user_id' => (int) $user_id
-				)) . '
 			WHERE user_id = ' . (int) $this->user->data['user_id'] . "
 				AND session_id = '" . $this->db->sql_escape($this->user->data['session_id']) . "'";
 		$this->db->sql_query($sql);
 	}
 
 	/**
-	* Checks to see if an access token exists solely by the session_id of the user
+	 * Checks to see if an access token exists solely by the session_id of the user.
 	 *
-	* @param	string	$service	The name of the OAuth service
+	 * @param string	$service	The OAuth service name
-	* @return	bool	true if they have token, false if they don't
+	 * @return bool					true if the user's access token exists,
+	 * 								false if the user's access token does not exist
 	 */
 	public function has_access_token_by_session($service)
 	{
@@ -360,19 +349,20 @@ class token_storage implements TokenStorageInterface
 			return true;
 		}
 
-		$data = array(
+		$data = [
 			'session_id'	=> $this->user->data['session_id'],
 			'provider'		=> $service,
-		);
+		];
 
-		return $this->_has_acess_token($data);
+		return $this->_has_access_token($data);
 	}
 
 	/**
-	* Checks to see if a state exists solely by the session_id of the user
+	 * Checks to see if a state exists solely by the session_id of the user.
 	 *
-	* @param	string	$service	The name of the OAuth service
+	 * @param string	$service	The OAuth service name
-	* @return	bool	true if they have state, false if they don't
+	 * @return bool					true if the user's state exists,
+	 * 								false if the user's state does not exist
 	 */
 	public function has_state_by_session($service)
 	{
@@ -383,25 +373,34 @@ class token_storage implements TokenStorageInterface
 			return true;
 		}
 
-		$data = array(
+		$data = [
 			'session_id'	=> $this->user->data['session_id'],
 			'provider'		=> $service,
-		);
+		];
 
 		return (bool) $this->get_state_row($data);
 	}
 
 	/**
-	* A helper function that performs the query for has access token functions
+	 * A helper function that performs the query for has access token functions.
 	 *
-	* @param	array	$data
+	 * @param array		$data		The SQL WHERE data
-	* @return	bool
+	 * @return bool					true if the user's access token exists,
+	 * 								false if the user's access token does not exist
 	 */
-	protected function _has_acess_token($data)
+	protected function _has_access_token($data)
 	{
 		return (bool) $this->get_access_token_row($data);
 	}
 
+	/**
+	 * A helper function that performs the query for retrieving access token functions by session.
+	 * Also checks if the token is a valid token.
+	 *
+	 * @param string	$service	The OAuth service provider name
+	 * @return TokenInterface
+	 * @throws TokenNotFoundException
+	 */
 	public function retrieve_access_token_by_session($service)
 	{
 		$service = $this->get_service_name_for_db($service);
@@ -411,14 +410,21 @@ class token_storage implements TokenStorageInterface
 			return $this->cachedToken;
 		}
 
-		$data = array(
+		$data = [
 			'session_id'	=> $this->user->data['session_id'],
 			'provider'		=> $service,
-		);
+		];
 
 		return $this->_retrieve_access_token($data);
 	}
 
+	/**
+	 * A helper function that performs the query for retrieving state functions by session.
+	 *
+	 * @param string	$service	The OAuth service provider name
+	 * @return string				The OAuth state
+	 * @throws AuthorizationStateNotFoundException
+	 */
 	public function retrieve_state_by_session($service)
 	{
 		$service = $this->get_service_name_for_db($service);
@@ -428,21 +434,21 @@ class token_storage implements TokenStorageInterface
 			return $this->cachedState;
 		}
 
-		$data = array(
+		$data = [
 			'session_id'	=> $this->user->data['session_id'],
 			'provider'		=> $service,
-		);
+		];
 
 		return $this->_retrieve_state($data);
 	}
 
 	/**
-	* A helper function that performs the query for retrieve access token functions
+	 * A helper function that performs the query for retrieve access token functions.
-	* Also checks if the token is a valid token
+	 * Also checks if the token is a valid token.
 	 *
-	* @param	array	$data
+	 * @param array		$data		The SQL WHERE data
-	* @return	mixed
+	 * @return TokenInterface
-	* @throws \OAuth\Common\Storage\Exception\TokenNotFoundException
+	 * @throws TokenNotFoundException
 	 */
 	protected function _retrieve_access_token($data)
 	{
@@ -459,19 +465,21 @@ class token_storage implements TokenStorageInterface
 		if (!($token instanceof TokenInterface))
 		{
 			$this->clearToken($data['provider']);
+
 			throw new TokenNotFoundException('AUTH_PROVIDER_OAUTH_TOKEN_ERROR_INCORRECTLY_STORED');
 		}
 
 		$this->cachedToken = $token;
+
 		return $token;
 	}
 
 	/**
-	 * A helper function that performs the query for retrieve state functions
+	 * A helper function that performs the query for retrieve state functions.
 	 *
-	 * @param	array	$data
+	 * @param array		$data		The SQL WHERE data
-	 * @return	mixed
+	 * @return string				The OAuth state
-	 * @throws \OAuth\Common\Storage\Exception\AuthorizationStateNotFoundException
+	 * @throws AuthorizationStateNotFoundException
 	 */
 	protected function _retrieve_state($data)
 	{
@@ -483,18 +491,21 @@ class token_storage implements TokenStorageInterface
 		}
 
 		$this->cachedState = $row['oauth_state'];
+
 		return $this->cachedState;
 	}
 
 	/**
-	* A helper function that performs the query for retrieving an access token
+	 * A helper function that performs the query for retrieving an access token.
 	 *
-	* @param	array	$data
+	 * @param array		$data		The SQL WHERE data
-	* @return	mixed
+	 * @return array|false			array with the OAuth token row,
+	 *                       		false if the token does not exist
 	 */
 	protected function get_access_token_row($data)
 	{
-		$sql = 'SELECT oauth_token FROM ' . $this->oauth_token_table . '
+		$sql = 'SELECT oauth_token 
+			FROM ' . $this->oauth_token_table . '
 			WHERE ' . $this->db->sql_build_array('SELECT', $data);
 		$result = $this->db->sql_query($sql);
 		$row = $this->db->sql_fetchrow($result);
@@ -504,14 +515,16 @@ class token_storage implements TokenStorageInterface
 	}
 
 	/**
-	 * A helper function that performs the query for retrieving a state
+	 * A helper function that performs the query for retrieving a state.
 	 *
-	 * @param	array	$data
+	 * @param array		$data		The SQL WHERE data
-	 * @return	mixed
+	 * @return array|false			array with the OAuth state row,
+	 *                       		false if the state does not exist
 	 */
 	protected function get_state_row($data)
 	{
-		$sql = 'SELECT oauth_state FROM ' . $this->oauth_state_table . '
+		$sql = 'SELECT oauth_state 
+			FROM ' . $this->oauth_state_table . '
 			WHERE ' . $this->db->sql_build_array('SELECT', $data);
 		$result = $this->db->sql_query($sql);
 		$row = $this->db->sql_fetchrow($result);
@@ -520,16 +533,22 @@ class token_storage implements TokenStorageInterface
 		return $row;
 	}
 
+	/**
+	 * A helper function that JSON encodes a TokenInterface's data.
+	 *
+	 * @param TokenInterface	$token
+	 * @return string					The json encoded TokenInterface's data
+	 */
 	public function json_encode_token(TokenInterface $token)
 	{
-		$members = array(
+		$members = [
 			'accessToken'	=> $token->getAccessToken(),
 			'endOfLife'		=> $token->getEndOfLife(),
 			'extraParams'	=> $token->getExtraParams(),
 			'refreshToken'	=> $token->getRefreshToken(),
 
 			'token_class'	=> get_class($token),
-		);
+		];
 
 		// Handle additional data needed for OAuth1 tokens
 		if ($token instanceof StdOAuth1Token)
@@ -542,6 +561,13 @@ class token_storage implements TokenStorageInterface
 		return json_encode($members);
 	}
 
+	/**
+	 * A helper function that JSON decodes a data string and creates a TokenInterface.
+	 *
+	 * @param string	$json			The json encoded TokenInterface's data
+	 * @return TokenInterface
+	 * @throws TokenNotFoundException
+	 */
 	public function json_decode_token($json)
 	{
 		$token_data = json_decode($json, true);
@@ -557,7 +583,10 @@ class token_storage implements TokenStorageInterface
 		$endOfLife		= $token_data['endOfLife'];
 		$extra_params	= $token_data['extraParams'];
 
-		// Create the token
+		/**
+		 * Create the token
+		 * @var TokenInterface	$token
+		 */
 		$token = new $token_class($access_token, $refresh_token, TokenInterface::EOL_NEVER_EXPIRES, $extra_params);
 		$token->setEndOfLife($endOfLife);
 
@@ -573,20 +602,19 @@ class token_storage implements TokenStorageInterface
 	}
 
 	/**
-	* Returns the name of the service as it must be stored in the database.
+	 * Returns the service name as it must be stored in the database.
 	 *
-	* @param	string	$service	The name of the OAuth service
+	 * @param string	$provider	The OAuth provider name
-	* @return	string	The name of the OAuth service as it needs to be stored
+	 * @return string				The OAuth service name
-	*					in the database.
 	 */
-	protected function get_service_name_for_db($service)
+	protected function get_service_name_for_db($provider)
 	{
 		// Enforce the naming convention for oauth services
-		if (strpos($service, 'auth.provider.oauth.service.') !== 0)
+		if (strpos($provider, 'auth.provider.oauth.service.') !== 0)
 		{
-			$service = 'auth.provider.oauth.service.' . strtolower($service);
+			$provider = 'auth.provider.oauth.service.' . strtolower($provider);
 		}
 
-		return $service;
+		return $provider;
 	}
 }

phpBB/phpbb/auth/provider/provider_interface.php

@@ -53,7 +53,7 @@ interface provider_interface
 	 * Autologin function
 	 *
 	 * @return 	array|null	containing the user row, empty if no auto login
-	 * 						should take place, or null if not impletmented.
+	 * 						should take place, or null if not implemented.
 	 */
 	public function autologin();
 
@@ -68,7 +68,7 @@ interface provider_interface
 
 	/**
 	 * This function updates the template with variables related to the acp
-	 * options with whatever configuraton values are passed to it as an array.
+	 * options with whatever configuration values are passed to it as an array.
 	 * It then returns the name of the acp file related to this authentication
 	 * provider.
 	 *

tests/functions/user_delete_test.php

@@ -61,20 +61,21 @@ class phpbb_functions_user_delete_test extends phpbb_database_test_case
 		$passwords_manager = new \phpbb\passwords\manager($config, $passwords_drivers, $passwords_helper, array_keys($passwords_drivers));
 
 		$oauth_provider = new \phpbb\auth\provider\oauth\oauth(
-			$db,
 			$config,
+			$phpbb_container,
+			$db,
+			$phpbb_dispatcher,
+			$lang,
 			$passwords_manager,
 			$request,
+			$oauth_provider_collection,
 			$user,
 			'phpbb_oauth_tokens',
 			'phpbb_oauth_states',
 			'phpbb_oauth_accounts',
-			$oauth_provider_collection,
 			'phpbb_users',
-			$phpbb_container,
+			$phpbb_root_path,
-			$phpbb_dispatcher,
+			$phpEx
-			$this->phpbb_root_path,
-			$this->php_ext
 		);
 		$provider_collection->offsetSet('auth.provider.oauth', $oauth_provider);