From 0b39e4e854da85ea6fd59578e1623078012fcae2 Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Sun, 5 May 2019 18:26:43 +0200 Subject: [ticket/16008] Clean up phpBB OAuth system PHPBB3-16008 --- phpBB/config/default/container/services_auth.yml | 15 +- phpBB/language/en/common.php | 1 + phpBB/phpbb/auth/provider/base.php | 2 +- phpBB/phpbb/auth/provider/oauth/oauth.php | 974 ++++++++++++--------- phpBB/phpbb/auth/provider/oauth/service/base.php | 62 +- phpBB/phpbb/auth/provider/oauth/service/bitly.php | 107 ++- .../phpbb/auth/provider/oauth/service/facebook.php | 99 ++- phpBB/phpbb/auth/provider/oauth/service/google.php | 107 ++- .../provider/oauth/service/service_interface.php | 114 +-- .../phpbb/auth/provider/oauth/service/twitter.php | 113 +-- phpBB/phpbb/auth/provider/oauth/token_storage.php | 346 ++++---- phpBB/phpbb/auth/provider/provider_interface.php | 4 +- tests/functions/user_delete_test.php | 13 +- 13 files changed, 1089 insertions(+), 868 deletions(-) diff --git a/phpBB/config/default/container/services_auth.yml b/phpBB/config/default/container/services_auth.yml index ed8dc90a74..d1aeed01aa 100644 --- a/phpBB/config/default/container/services_auth.yml +++ b/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: diff --git a/phpBB/language/en/common.php b/phpBB/language/en/common.php index d050c1b109..8190bad83c 100644 --- a/phpBB/language/en/common.php +++ b/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', diff --git a/phpBB/phpbb/auth/provider/base.php b/phpBB/phpbb/auth/provider/base.php index dea27ccc25..30e0a0fe2d 100644 --- a/phpBB/phpbb/auth/provider/base.php +++ b/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} diff --git a/phpBB/phpbb/auth/provider/oauth/oauth.php b/phpBB/phpbb/auth/provider/oauth/oauth.php index e3f8394bba..a1538761f1 100644 --- a/phpBB/phpbb/auth/provider/oauth/oauth.php +++ b/phpBB/phpbb/auth/provider/oauth/oauth.php @@ -1,169 +1,134 @@ -* @license GNU General Public License, version 2 (GPL-2.0) -* -* For full copyright and license information, please see -* the docs/CREDITS.txt file. -* -*/ + * + * This file is part of the phpBB Forum Software package. + * + * @copyright (c) phpBB Limited + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ namespace phpbb\auth\provider\oauth; +use OAuth\ServiceFactory; use OAuth\Common\Consumer\Credentials; +use OAuth\Common\Service\ServiceInterface; +use OAuth\OAuth1\Service\AbstractService as OAuth1Service; +use OAuth\OAuth2\Service\AbstractService as OAuth2Service; +use Symfony\Component\DependencyInjection\ContainerInterface; +use phpbb\auth\provider\oauth\service\exception; /** -* OAuth authentication provider for phpBB3 -*/ + * OAuth authentication provider for phpBB3 + */ class oauth extends \phpbb\auth\provider\base { - /** - * Database driver - * - * @var \phpbb\db\driver\driver_interface - */ - protected $db; - - /** - * phpBB config - * - * @var \phpbb\config\config - */ + /** @var \phpbb\config\config */ protected $config; - /** - * phpBB passwords manager - * - * @var \phpbb\passwords\manager - */ - protected $passwords_manager; + /** @var ContainerInterface */ + protected $container; - /** - * phpBB request object - * - * @var \phpbb\request\request_interface - */ - protected $request; + /** @var \phpbb\db\driver\driver_interface */ + protected $db; - /** - * phpBB user - * - * @var \phpbb\user - */ - protected $user; + /** @var \phpbb\event\dispatcher */ + protected $dispatcher; - /** - * OAuth token table - * - * @var string - */ - protected $auth_provider_oauth_token_storage_table; + /** @var \phpbb\language\language */ + protected $language; - /** - * OAuth state table - * - * @var string - */ - protected $auth_provider_oauth_state_table; + /** @var \phpbb\passwords\manager */ + protected $passwords_manager; - /** - * OAuth account association table - * - * @var string - */ - protected $auth_provider_oauth_token_account_assoc; + /** @var \phpbb\request\request_interface */ + protected $request; - /** - * All OAuth service providers - * - * @var \phpbb\di\service_collection Contains \phpbb\auth\provider\oauth\service_interface - */ + /** @var \phpbb\di\service_collection */ protected $service_providers; - /** - * Users table - * - * @var string - */ - protected $users_table; + /** @var \phpbb\user */ + protected $user; - /** - * Cached current uri object - * - * @var \OAuth\Common\Http\Uri\UriInterface|null - */ - protected $current_uri; + /** @var string OAuth table: token storage */ + protected $oauth_token_table; - /** - * DI container - * - * @var \Symfony\Component\DependencyInjection\ContainerInterface - */ - protected $phpbb_container; + /** @var string OAuth table: state */ + protected $oauth_state_table; - /** - * phpBB event dispatcher - * - * @var \phpbb\event\dispatcher_interface - */ - protected $dispatcher; + /** @var string OAuth table: account association */ + protected $oauth_account_table; - /** - * phpBB root path - * - * @var string - */ - protected $phpbb_root_path; + /** @var string Users table */ + protected $users_table; - /** - * PHP file extension - * - * @var string - */ + /** @var string phpBB root path */ + protected $root_path; + + /** @var string php File extension */ protected $php_ext; /** - * OAuth Authentication Constructor - * - * @param \phpbb\db\driver\driver_interface $db - * @param \phpbb\config\config $config - * @param \phpbb\passwords\manager $passwords_manager - * @param \phpbb\request\request_interface $request - * @param \phpbb\user $user - * @param string $auth_provider_oauth_token_storage_table - * @param string $auth_provider_oauth_state_table - * @param string $auth_provider_oauth_token_account_assoc - * @param \phpbb\di\service_collection $service_providers Contains \phpbb\auth\provider\oauth\service_interface - * @param string $users_table - * @param \Symfony\Component\DependencyInjection\ContainerInterface $phpbb_container DI container - * @param \phpbb\event\dispatcher_interface $dispatcher phpBB event dispatcher - * @param string $phpbb_root_path - * @param string $php_ext - */ - public function __construct(\phpbb\db\driver\driver_interface $db, \phpbb\config\config $config, \phpbb\passwords\manager $passwords_manager, \phpbb\request\request_interface $request, \phpbb\user $user, $auth_provider_oauth_token_storage_table, $auth_provider_oauth_state_table, $auth_provider_oauth_token_account_assoc, \phpbb\di\service_collection $service_providers, $users_table, \Symfony\Component\DependencyInjection\ContainerInterface $phpbb_container, \phpbb\event\dispatcher_interface $dispatcher, $phpbb_root_path, $php_ext) + * Constructor. + * + * @param \phpbb\config\config $config Config object + * @param ContainerInterface $container Service container object + * @param \phpbb\db\driver\driver_interface $db Database object + * @param \phpbb\event\dispatcher $dispatcher Event dispatcher object + * @param \phpbb\language\language $language Language object + * @param \phpbb\passwords\manager $passwords_manager Password manager object + * @param \phpbb\request\request_interface $request Request object + * @param \phpbb\di\service_collection $service_providers OAuth providers service collection + * @param \phpbb\user $user User object + * @param string $oauth_token_table OAuth table: token storage + * @param string $oauth_state_table OAuth table: state + * @param string $oauth_account_table OAuth table: account association + * @param string $users_table User table + * @param string $root_path phpBB root path + * @param string $php_ext php File extension + */ + public function __construct( + \phpbb\config\config $config, + ContainerInterface $container, + \phpbb\db\driver\driver_interface $db, + \phpbb\event\dispatcher $dispatcher, + \phpbb\language\language $language, + \phpbb\passwords\manager $passwords_manager, + \phpbb\request\request_interface $request, + \phpbb\di\service_collection $service_providers, + \phpbb\user $user, + $oauth_token_table, + $oauth_state_table, + $oauth_account_table, + $users_table, + $root_path, + $php_ext + ) { - $this->db = $db; - $this->config = $config; - $this->passwords_manager = $passwords_manager; - $this->request = $request; - $this->user = $user; - $this->auth_provider_oauth_token_storage_table = $auth_provider_oauth_token_storage_table; - $this->auth_provider_oauth_state_table = $auth_provider_oauth_state_table; - $this->auth_provider_oauth_token_account_assoc = $auth_provider_oauth_token_account_assoc; - $this->service_providers = $service_providers; - $this->users_table = $users_table; - $this->phpbb_container = $phpbb_container; - $this->dispatcher = $dispatcher; - $this->phpbb_root_path = $phpbb_root_path; - $this->php_ext = $php_ext; + $this->config = $config; + $this->container = $container; + $this->db = $db; + $this->dispatcher = $dispatcher; + $this->passwords_manager = $passwords_manager; + $this->language = $language; + $this->service_providers = $service_providers; + $this->request = $request; + $this->user = $user; + + $this->oauth_token_table = $oauth_token_table; + $this->oauth_state_table = $oauth_state_table; + $this->oauth_account_table = $oauth_account_table; + $this->users_table = $users_table; + $this->root_path = $root_path; + $this->php_ext = $php_ext; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function init() { // This does not test whether or not the key and secret provided are valid. @@ -173,61 +138,91 @@ class oauth extends \phpbb\auth\provider\base if (($credentials['key'] && !$credentials['secret']) || (!$credentials['key'] && $credentials['secret'])) { - return $this->user->lang['AUTH_PROVIDER_OAUTH_ERROR_ELEMENT_MISSING']; + return $this->language->lang('AUTH_PROVIDER_OAUTH_ERROR_ELEMENT_MISSING'); } } + return false; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function login($username, $password) { // Temporary workaround for only having one authentication provider available if (!$this->request->is_set('oauth_service')) { - $provider = new \phpbb\auth\provider\db($this->db, $this->config, $this->passwords_manager, $this->request, $this->user, $this->phpbb_container, $this->phpbb_root_path, $this->php_ext); + $provider = new \phpbb\auth\provider\db( + $this->db, + $this->config, + $this->passwords_manager, + $this->request, + $this->user, + $this->container, + $this->root_path, + $this->php_ext + ); + return $provider->login($username, $password); } // Request the name of the OAuth service - $service_name_original = $this->request->variable('oauth_service', '', false); - $service_name = 'auth.provider.oauth.service.' . strtolower($service_name_original); - if ($service_name_original === '' || !array_key_exists($service_name, $this->service_providers)) + $provider = $this->request->variable('oauth_service', '', false); + $service_name = $this->get_service_name($provider); + + if ($provider === '' || !array_key_exists($service_name, $this->service_providers)) { - return array( + return [ 'status' => LOGIN_ERROR_EXTERNAL_AUTH, 'error_msg' => 'LOGIN_ERROR_OAUTH_SERVICE_DOES_NOT_EXIST', - 'user_row' => array('user_id' => ANONYMOUS), - ); + 'user_row' => ['user_id' => ANONYMOUS], + ]; } // Get the service credentials for the given service - $service_credentials = $this->service_providers[$service_name]->get_service_credentials(); + $storage = new token_storage($this->db, $this->user, $this->oauth_token_table, $this->oauth_state_table); + $query = 'mode=login&login=external&oauth_service=' . $provider; - $storage = new \phpbb\auth\provider\oauth\token_storage($this->db, $this->user, $this->auth_provider_oauth_token_storage_table, $this->auth_provider_oauth_state_table); - $query = 'mode=login&login=external&oauth_service=' . $service_name_original; - $service = $this->get_service($service_name_original, $storage, $service_credentials, $query, $this->service_providers[$service_name]->get_auth_scope()); + try + { + /** @var OAuth1Service|OAuth2Service $service */ + $service = $this->get_service($provider, $storage, $query); + } + catch (\Exception $e) + { + return [ + 'status' => LOGIN_ERROR_EXTERNAL_AUTH, + 'error_msg' => $e->getMessage(), + 'user_row' => ['user_id' => ANONYMOUS], + ]; + } - if (($service::OAUTH_VERSION === 2 && $this->request->is_set('code', \phpbb\request\request_interface::GET)) - || ($service::OAUTH_VERSION === 1 && $this->request->is_set('oauth_token', \phpbb\request\request_interface::GET))) + if ($this->is_set_code($service)) { $this->service_providers[$service_name]->set_external_service_provider($service); - $unique_id = $this->service_providers[$service_name]->perform_auth_login(); - /** - * Check to see if this provider is already associated with an account. - * - * Enforcing a data type to make data contains strings and not integers, - * so values are quoted in the SQL WHERE statement. - */ - $data = array( - 'provider' => (string) $service_name_original, + try + { + $unique_id = $this->service_providers[$service_name]->perform_auth_login(); + } + catch (exception $e) + { + return [ + 'status' => LOGIN_ERROR_EXTERNAL_AUTH, + 'error_msg' => $e->getMessage(), + 'user_row' => ['user_id' => ANONYMOUS], + ]; + } + + // Check to see if this provider is already associated with an account + $data = [ + 'provider' => (string) utf8_strtolower($provider), 'oauth_provider_id' => (string) $unique_id - ); + ]; - $sql = 'SELECT user_id FROM ' . $this->auth_provider_oauth_token_account_assoc . ' + $sql = 'SELECT user_id + FROM ' . $this->oauth_account_table . ' WHERE ' . $this->db->sql_build_array('SELECT', $data); $result = $this->db->sql_query($sql); $row = $this->db->sql_fetchrow($result); @@ -235,50 +230,79 @@ class oauth extends \phpbb\auth\provider\base $redirect_data = array( 'auth_provider' => 'oauth', - 'login_link_oauth_service' => $service_name_original, + 'login_link_oauth_service' => $provider, ); /** - * Event is triggered before check if provider is already associated with an account - * - * @event core.oauth_login_after_check_if_provider_id_has_match - * @var array row User row - * @var array data Provider data - * @var array redirect_data Data to be appended to the redirect url - * @var \OAuth\Common\Service\ServiceInterface service OAuth service - * @since 3.2.3-RC1 - * @changed 3.2.6-RC1 Added redirect_data - */ - $vars = array( + * Event is triggered before check if provider is already associated with an account + * + * @event core.oauth_login_after_check_if_provider_id_has_match + * @var array row User row + * @var array data Provider data + * @var array redirect_data Data to be appended to the redirect url + * @var ServiceInterface service OAuth service + * @since 3.2.3-RC1 + * @changed 3.2.6-RC1 Added redirect_data + */ + $vars = [ 'row', 'data', 'redirect_data', 'service', - ); + ]; extract($this->dispatcher->trigger_event('core.oauth_login_after_check_if_provider_id_has_match', compact($vars))); if (!$row) { // The user does not yet exist, ask to link or create profile - return array( + return [ 'status' => LOGIN_SUCCESS_LINK_PROFILE, 'error_msg' => 'LOGIN_OAUTH_ACCOUNT_NOT_LINKED', - 'user_row' => array(), + 'user_row' => [], 'redirect_data' => $redirect_data, - ); + ]; } // Retrieve the user's account $sql = 'SELECT user_id, username, user_password, user_passchg, user_email, user_ip, user_type, user_login_attempts FROM ' . $this->users_table . ' - WHERE user_id = ' . (int) $row['user_id']; + WHERE user_id = ' . (int) $row['user_id']; $result = $this->db->sql_query($sql); $row = $this->db->sql_fetchrow($result); $this->db->sql_freeresult($result); if (!$row) { - throw new \Exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_ENTRY'); + return [ + 'status' => LOGIN_ERROR_EXTERNAL_AUTH, + 'error_msg' => 'AUTH_PROVIDER_OAUTH_ERROR_INVALID_ENTRY', + 'user_row' => ['user_id' => ANONYMOUS], + ]; + } + + /** + * Check if the user is banned. + * The fourth parameter, return, has to be true, otherwise the OAuth login is still called and + * an uncaught exception is thrown as there is no token stored in the database. + */ + $ban = $this->user->check_ban($row['user_id'], $row['user_ip'], $row['user_email'], true); + + if (!empty($ban)) + { + $till_date = !empty($ban['ban_end']) ? $this->user->format_date($ban['ban_end']) : ''; + $message = !empty($ban['ban_end']) ? 'BOARD_BAN_TIME' : 'BOARD_BAN_PERM'; + + $contact_link = phpbb_get_board_contact_link($this->config, $this->root_path, $this->php_ext); + + $message = $this->language->lang($message, $till_date, '', ''); + $message .= !empty($ban['ban_give_reason']) ? '

' . $this->language->lang('BOARD_BAN_REASON', $ban['ban_give_reason']) : ''; + $message .= !empty($ban['ban_triggered_by']) ? '

' . $this->language->lang('BAN_TRIGGERED_BY_' . utf8_strtoupper($ban['ban_triggered_by'])) . '' : ''; + + return [ + 'status' => LOGIN_BREAK, + 'error_msg' => $message, + 'user_row' => $row, + ]; } /** @@ -310,129 +334,55 @@ class oauth extends \phpbb\auth\provider\base $storage->set_user_id($row['user_id']); /** - * Event is triggered after user is successfully logged in via OAuth. - * - * @event core.auth_oauth_login_after - * @var array row User row - * @since 3.1.11-RC1 - */ - $vars = array( + * Event is triggered after user is successfully logged in via OAuth. + * + * @event core.auth_oauth_login_after + * @var array row User row + * @since 3.1.11-RC1 + */ + $vars = [ 'row', - ); + ]; extract($this->dispatcher->trigger_event('core.auth_oauth_login_after', compact($vars))); // The user is now authenticated and can be logged in - return array( + return [ 'status' => LOGIN_SUCCESS, 'error_msg' => false, 'user_row' => $row, - ); + ]; } else { - if ($service::OAUTH_VERSION === 1) - { - $token = $service->requestRequestToken(); - $url = $service->getAuthorizationUri(array('oauth_token' => $token->getRequestToken())); - } - else - { - $url = $service->getAuthorizationUri(); - } - header('Location: ' . $url); - } - } - - /** - * Returns the cached current_uri object or creates and caches it if it is - * not already created. In each case the query string is updated based on - * the $query parameter. - * - * @param string $service_name The name of the service - * @param string $query The query string of the current_uri - * used in redirects - * @return \OAuth\Common\Http\Uri\UriInterface - */ - protected function get_current_uri($service_name, $query) - { - if ($this->current_uri) - { - $this->current_uri->setQuery($query); - return $this->current_uri; - } - - $uri_factory = new \OAuth\Common\Http\Uri\UriFactory(); - $super_globals = $this->request->get_super_global(\phpbb\request\request_interface::SERVER); - if (!empty($super_globals['HTTP_X_FORWARDED_PROTO']) && $super_globals['HTTP_X_FORWARDED_PROTO'] === 'https') - { - $super_globals['HTTPS'] = 'on'; - $super_globals['SERVER_PORT'] = 443; - } - $current_uri = $uri_factory->createFromSuperGlobalArray($super_globals); - $current_uri->setQuery($query); - - $this->current_uri = $current_uri; - return $current_uri; - } - - /** - * Returns a new service object - * - * @param string $service_name The name of the service - * @param \phpbb\auth\provider\oauth\token_storage $storage - * @param array $service_credentials {@see \phpbb\auth\provider\oauth\oauth::get_service_credentials} - * @param string $query The query string of the - * current_uri used in redirection - * @param array $scopes The scope of the request against - * the api. - * @return \OAuth\Common\Service\ServiceInterface - * @throws \Exception - */ - protected function get_service($service_name, \phpbb\auth\provider\oauth\token_storage $storage, array $service_credentials, $query, array $scopes = array()) - { - $current_uri = $this->get_current_uri($service_name, $query); - - // Setup the credentials for the requests - $credentials = new Credentials( - $service_credentials['key'], - $service_credentials['secret'], - $current_uri->getAbsoluteUri() - ); - - $service_factory = new \OAuth\ServiceFactory(); - $service = $service_factory->createService($service_name, $credentials, $storage, $scopes); - - if (!$service) - { - throw new \Exception('AUTH_PROVIDER_OAUTH_ERROR_SERVICE_NOT_CREATED'); + return $this->set_redirect($service); } - - return $service; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function get_login_data() { - $login_data = array( + $login_data = [ 'TEMPLATE_FILE' => 'login_body_oauth.html', 'BLOCK_VAR_NAME' => 'oauth', - 'BLOCK_VARS' => array(), - ); + 'BLOCK_VARS' => [], + ]; foreach ($this->service_providers as $service_name => $service_provider) { // Only include data if the credentials are set $credentials = $service_provider->get_service_credentials(); + if ($credentials['key'] && $credentials['secret']) { - $actual_name = str_replace('auth.provider.oauth.service.', '', $service_name); - $redirect_url = generate_board_url() . '/ucp.' . $this->php_ext . '?mode=login&login=external&oauth_service=' . $actual_name; - $login_data['BLOCK_VARS'][$service_name] = array( + $provider = $this->get_provider($service_name); + $redirect_url = generate_board_url() . '/ucp.' . $this->php_ext . '?mode=login&login=external&oauth_service=' . $provider; + + $login_data['BLOCK_VARS'][$service_name] = [ 'REDIRECT_URL' => redirect($redirect_url, true), - 'SERVICE_NAME' => $this->user->lang['AUTH_PROVIDER_OAUTH_SERVICE_' . strtoupper($actual_name)], - ); + 'SERVICE_NAME' => $this->get_provider_title($provider), + ]; } } @@ -440,51 +390,55 @@ class oauth extends \phpbb\auth\provider\base } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function acp() { - $ret = array(); + $ret = []; foreach ($this->service_providers as $service_name => $service_provider) { - $actual_name = str_replace('auth.provider.oauth.service.', '', $service_name); - $ret[] = 'auth_oauth_' . $actual_name . '_key'; - $ret[] = 'auth_oauth_' . $actual_name . '_secret'; + $provider = $this->get_provider($service_name); + + $provider = utf8_strtolower($provider); + + $ret[] = 'auth_oauth_' . $provider . '_key'; + $ret[] = 'auth_oauth_' . $provider . '_secret'; } return $ret; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function get_acp_template($new_config) { - $ret = array( + $ret = [ 'BLOCK_VAR_NAME' => 'oauth_services', - 'BLOCK_VARS' => array(), + 'BLOCK_VARS' => [], 'TEMPLATE_FILE' => 'auth_provider_oauth.html', - 'TEMPLATE_VARS' => array(), - ); + 'TEMPLATE_VARS' => [], + ]; foreach ($this->service_providers as $service_name => $service_provider) { - $actual_name = str_replace('auth.provider.oauth.service.', '', $service_name); - $ret['BLOCK_VARS'][$actual_name] = array( - 'ACTUAL_NAME' => $this->user->lang['AUTH_PROVIDER_OAUTH_SERVICE_' . strtoupper($actual_name)], - 'KEY' => $new_config['auth_oauth_' . $actual_name . '_key'], - 'NAME' => $actual_name, - 'SECRET' => $new_config['auth_oauth_' . $actual_name . '_secret'], - ); + $provider = $this->get_provider($service_name); + + $ret['BLOCK_VARS'][$provider] = [ + 'NAME' => $provider, + 'ACTUAL_NAME' => $this->get_provider_title($provider), + 'KEY' => $new_config['auth_oauth_' . utf8_strtolower($provider) . '_key'], + 'SECRET' => $new_config['auth_oauth_' . utf8_strtolower($provider) . '_secret'], + ]; } return $ret; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function login_link_has_necessary_data($login_link_data) { if (empty($login_link_data)) @@ -502,16 +456,13 @@ class oauth extends \phpbb\auth\provider\base } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function link_account(array $link_data) { // Check for a valid link method (auth_link or login_link) if (!array_key_exists('link_method', $link_data) || - !in_array($link_data['link_method'], array( - 'auth_link', - 'login_link', - ))) + !in_array($link_data['link_method'], ['auth_link', 'login_link'])) { return 'LOGIN_LINK_MISSING_DATA'; } @@ -527,7 +478,8 @@ class oauth extends \phpbb\auth\provider\base } } - $service_name = 'auth.provider.oauth.service.' . strtolower($link_data['oauth_service']); + $service_name = $this->get_service_name($link_data['oauth_service']); + if (!array_key_exists($service_name, $this->service_providers)) { return 'LOGIN_ERROR_OAUTH_SERVICE_DOES_NOT_EXIST'; @@ -539,21 +491,109 @@ class oauth extends \phpbb\auth\provider\base return $this->link_account_auth_link($link_data, $service_name); case 'login_link': return $this->link_account_login_link($link_data, $service_name); + default: + return 'LOGIN_LINK_MISSING_DATA'; } } /** - * Performs the account linking for login_link - * - * @param array $link_data The same variable given to {@see \phpbb\auth\provider\provider_interface::link_account} - * @param string $service_name The name of the service being used in - * linking. - * @return string|null Returns a language constant (string) if an error is - * encountered, or null on success. - */ + * {@inheritdoc} + */ + public function logout($data, $new_session) + { + // Clear all tokens belonging to the user + $storage = new token_storage($this->db, $this->user, $this->oauth_token_table, $this->oauth_state_table); + $storage->clearAllTokens(); + + return; + } + + /** + * {@inheritdoc} + */ + public function get_auth_link_data($user_id = 0) + { + $user_ids = []; + $block_vars = []; + + $sql = 'SELECT oauth_provider_id, provider + FROM ' . $this->oauth_account_table . ' + WHERE user_id = ' . ($user_id > 0 ? (int) $user_id : (int) $this->user->data['user_id']); + $result = $this->db->sql_query($sql); + while ($row = $this->db->sql_fetchrow($result)) + { + $user_ids[$row['provider']] = $row['oauth_provider_id']; + } + $this->db->sql_freeresult($result); + + foreach ($this->service_providers as $service_name => $service_provider) + { + // Only include data if the credentials are set + $credentials = $service_provider->get_service_credentials(); + + if ($credentials['key'] && $credentials['secret']) + { + $provider = $this->get_provider($service_name); + + $block_vars[$service_name] = [ + 'SERVICE_NAME' => $this->get_provider_title($provider), + 'UNIQUE_ID' => isset($user_ids[$provider]) ? $user_ids[$provider] : null, + 'HIDDEN_FIELDS' => [ + 'link' => !isset($user_ids[$provider]), + 'oauth_service' => $provider, + ], + ]; + } + } + + return [ + 'BLOCK_VAR_NAME' => 'oauth', + 'BLOCK_VARS' => $block_vars, + + 'TEMPLATE_FILE' => 'ucp_auth_link_oauth.html', + ]; + } + + /** + * {@inheritdoc} + */ + public function unlink_account(array $link_data) + { + if (!array_key_exists('oauth_service', $link_data) || !$link_data['oauth_service']) + { + return 'LOGIN_LINK_MISSING_DATA'; + } + + // Remove user specified in $link_data if possible + $user_id = isset($link_data['user_id']) ? $link_data['user_id'] : $this->user->data['user_id']; + + // Remove the link + $sql = 'DELETE FROM ' . $this->oauth_account_table . " + WHERE provider = '" . $this->db->sql_escape($link_data['oauth_service']) . "' + AND user_id = " . (int) $user_id; + $this->db->sql_query($sql); + + $service_name = $this->get_service_name($link_data['oauth_service']); + + // Clear all tokens belonging to the user on this service + $storage = new token_storage($this->db, $this->user, $this->oauth_token_table, $this->oauth_state_table); + $storage->clearToken($service_name); + + return false; + } + + /** + * Performs the account linking for login_link. + * + * @param array $link_data The same variable given to + * {@see \phpbb\auth\provider\provider_interface::link_account} + * @param string $service_name The name of the service being used in linking. + * @return string|false Returns a language key (string) if an error is encountered, + * or false on success. + */ protected function link_account_login_link(array $link_data, $service_name) { - $storage = new \phpbb\auth\provider\oauth\token_storage($this->db, $this->user, $this->auth_provider_oauth_token_storage_table, $this->auth_provider_oauth_state_table); + $storage = new token_storage($this->db, $this->user, $this->oauth_token_table, $this->oauth_state_table); // Check for an access token, they should have one if (!$storage->has_access_token_by_session($service_name)) @@ -561,87 +601,109 @@ class oauth extends \phpbb\auth\provider\base return 'LOGIN_LINK_ERROR_OAUTH_NO_ACCESS_TOKEN'; } - // Prepare the query string - $query = 'mode=login_link&login_link_oauth_service=' . strtolower($link_data['oauth_service']); - // Prepare for an authentication request - $service_credentials = $this->service_providers[$service_name]->get_service_credentials(); - $scopes = $this->service_providers[$service_name]->get_auth_scope(); - $service = $this->get_service(strtolower($link_data['oauth_service']), $storage, $service_credentials, $query, $scopes); + $query = 'mode=login_link&login_link_oauth_service=' . $link_data['oauth_service']; + + try + { + $service = $this->get_service($link_data['oauth_service'], $storage, $query); + } + catch (\Exception $e) + { + return $e->getMessage(); + } + $this->service_providers[$service_name]->set_external_service_provider($service); - // The user has already authenticated successfully, request to authenticate again - $unique_id = $this->service_providers[$service_name]->perform_token_auth(); + try + { + // The user has already authenticated successfully, request to authenticate again + $unique_id = $this->service_providers[$service_name]->perform_token_auth(); + } + catch (exception $e) + { + return $e->getMessage(); + } // Insert into table, they will be able to log in after this - $data = array( + $data = [ 'user_id' => $link_data['user_id'], - 'provider' => strtolower($link_data['oauth_service']), + 'provider' => utf8_strtolower($link_data['oauth_service']), 'oauth_provider_id' => $unique_id, - ); + ]; $this->link_account_perform_link($data); + // Update token storage to store the user_id $storage->set_user_id($link_data['user_id']); + + return false; } /** - * Performs the account linking for auth_link - * - * @param array $link_data The same variable given to {@see \phpbb\auth\provider\provider_interface::link_account} - * @param string $service_name The name of the service being used in - * linking. - * @return string|null Returns a language constant (string) if an error is - * encountered, or null on success. - */ + * Performs the account linking for auth_link. + * + * @param array $link_data The same variable given to + * {@see \phpbb\auth\provider\provider_interface::link_account} + * @param string $service_name The name of the service being used in linking. + * @return string|false Returns a language constant (string) if an error is encountered, + * or false on success. + */ protected function link_account_auth_link(array $link_data, $service_name) { - $storage = new \phpbb\auth\provider\oauth\token_storage($this->db, $this->user, $this->auth_provider_oauth_token_storage_table, $this->auth_provider_oauth_state_table); - $query = 'i=ucp_auth_link&mode=auth_link&link=1&oauth_service=' . strtolower($link_data['oauth_service']); - $service_credentials = $this->service_providers[$service_name]->get_service_credentials(); - $scopes = $this->service_providers[$service_name]->get_auth_scope(); - $service = $this->get_service(strtolower($link_data['oauth_service']), $storage, $service_credentials, $query, $scopes); + $storage = new token_storage($this->db, $this->user, $this->oauth_token_table, $this->oauth_state_table); + $query = 'i=ucp_auth_link&mode=auth_link&link=1&oauth_service=' . $link_data['oauth_service']; - if (($service::OAUTH_VERSION === 2 && $this->request->is_set('code', \phpbb\request\request_interface::GET)) - || ($service::OAUTH_VERSION === 1 && $this->request->is_set('oauth_token', \phpbb\request\request_interface::GET))) + try + { + /** @var OAuth1Service|OAuth2Service $service */ + $service = $this->get_service($link_data['oauth_service'], $storage, $query); + } + catch (\Exception $e) + { + return $e->getMessage(); + } + + if ($this->is_set_code($service)) { $this->service_providers[$service_name]->set_external_service_provider($service); - $unique_id = $this->service_providers[$service_name]->perform_auth_login(); + + try + { + $unique_id = $this->service_providers[$service_name]->perform_auth_login(); + } + catch (exception $e) + { + return $e->getMessage(); + } // Insert into table, they will be able to log in after this - $data = array( + $data = [ 'user_id' => $this->user->data['user_id'], - 'provider' => strtolower($link_data['oauth_service']), + 'provider' => utf8_strtolower($link_data['oauth_service']), 'oauth_provider_id' => $unique_id, - ); + ]; $this->link_account_perform_link($data); + + return false; } else { - if ($service::OAUTH_VERSION === 1) - { - $token = $service->requestRequestToken(); - $url = $service->getAuthorizationUri(array('oauth_token' => $token->getRequestToken())); - } - else - { - $url = $service->getAuthorizationUri(); - } - header('Location: ' . $url); + return $this->set_redirect($service); } } /** - * Performs the query that inserts an account link - * - * @param array $data This array is passed to db->sql_build_array - */ + * Performs the query that inserts an account link + * + * @param array $data This array is passed to db->sql_build_array + */ protected function link_account_perform_link(array $data) { // Check if the external account is already associated with other user $sql = 'SELECT user_id - FROM ' . $this->auth_provider_oauth_token_account_assoc . " + FROM ' . $this->oauth_account_table . " WHERE provider = '" . $this->db->sql_escape($data['provider']) . "' AND oauth_provider_id = '" . $this->db->sql_escape($data['oauth_provider_id']) . "'"; $result = $this->db->sql_query($sql); @@ -654,114 +716,172 @@ class oauth extends \phpbb\auth\provider\base } // Link account - $sql = 'INSERT INTO ' . $this->auth_provider_oauth_token_account_assoc . ' - ' . $this->db->sql_build_array('INSERT', $data); + $sql = 'INSERT INTO ' . $this->oauth_account_table . ' ' . $this->db->sql_build_array('INSERT', $data); $this->db->sql_query($sql); /** * Event is triggered after user links account. * * @event core.auth_oauth_link_after - * @var array data User row + * @var array data User row * @since 3.1.11-RC1 */ - $vars = array( + $vars = [ 'data', - ); + ]; extract($this->dispatcher->trigger_event('core.auth_oauth_link_after', compact($vars))); } /** - * {@inheritdoc} - */ - public function logout($data, $new_session) + * Returns a new service object. + * + * @param string $provider The name of the provider + * @param token_storage $storage Token storage object + * @param string $query The query string used for the redirect uri + * @return ServiceInterface + * @throws exception When OAuth service was not created + */ + protected function get_service($provider, token_storage $storage, $query) { - // Clear all tokens belonging to the user - $storage = new \phpbb\auth\provider\oauth\token_storage($this->db, $this->user, $this->auth_provider_oauth_token_storage_table, $this->auth_provider_oauth_state_table); - $storage->clearAllTokens(); + $service_name = $this->get_service_name($provider); - return; - } + /** @see \phpbb\auth\provider\oauth\service\service_interface::get_service_credentials */ + $service_credentials = $this->service_providers[$service_name]->get_service_credentials(); - /** - * {@inheritdoc} - */ - public function get_auth_link_data($user_id = 0) - { - $block_vars = array(); + /** @see \phpbb\auth\provider\oauth\service\service_interface::get_auth_scope */ + $scopes = $this->service_providers[$service_name]->get_auth_scope(); + + $callback = generate_board_url() . "/ucp.{$this->php_ext}?{$query}"; - // Get all external accounts tied to the current user - $data = array( - 'user_id' => ($user_id <= 0) ? (int) $this->user->data['user_id'] : (int) $user_id, + // Setup the credentials for the requests + $credentials = new Credentials( + $service_credentials['key'], + $service_credentials['secret'], + $callback ); - $sql = 'SELECT oauth_provider_id, provider FROM ' . $this->auth_provider_oauth_token_account_assoc . ' - WHERE ' . $this->db->sql_build_array('SELECT', $data); - $result = $this->db->sql_query($sql); - $rows = $this->db->sql_fetchrowset($result); - $this->db->sql_freeresult($result); - $oauth_user_ids = array(); + $service_factory = new ServiceFactory; - if ($rows !== false && count($rows)) + // Allow providers to register a custom class or override the provider name + if ($class = $this->service_providers[$service_name]->get_external_service_class()) { - foreach ($rows as $row) + if (class_exists($class)) { - $oauth_user_ids[$row['provider']] = $row['oauth_provider_id']; + try + { + $service_factory->registerService($provider, $class); + } + catch (\OAuth\Common\Exception\Exception $e) + { + throw new exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE'); + } + } + else + { + $provider = $class; } } - unset($rows); - foreach ($this->service_providers as $service_name => $service_provider) + $service = $service_factory->createService($provider, $credentials, $storage, $scopes); + + if (!$service) { - // Only include data if the credentials are set - $credentials = $service_provider->get_service_credentials(); - if ($credentials['key'] && $credentials['secret']) - { - $actual_name = str_replace('auth.provider.oauth.service.', '', $service_name); + throw new exception('AUTH_PROVIDER_OAUTH_ERROR_SERVICE_NOT_CREATED'); + } - $block_vars[$service_name] = array( - 'HIDDEN_FIELDS' => array( - 'link' => (!isset($oauth_user_ids[$actual_name])), - 'oauth_service' => $actual_name, - ), + return $service; + } - 'SERVICE_ID' => $actual_name, - 'SERVICE_NAME' => $this->user->lang['AUTH_PROVIDER_OAUTH_SERVICE_' . strtoupper($actual_name)], - 'UNIQUE_ID' => (isset($oauth_user_ids[$actual_name])) ? $oauth_user_ids[$actual_name] : null, - ); - } + /** + * Returns the service name for an OAuth provider name. + * + * @param string $provider The OAuth provider name + * @return string The service name + */ + protected function get_service_name($provider) + { + if (strpos($provider, 'auth.provider.oauth.service.') !== 0) + { + $provider = 'auth.provider.oauth.service.' . utf8_strtolower($provider); } - return array( - 'BLOCK_VAR_NAME' => 'oauth', - 'BLOCK_VARS' => $block_vars, + return $provider; + } - 'TEMPLATE_FILE' => 'ucp_auth_link_oauth.html', - ); + /** + * Returns the OAuth provider name from a service name. + * + * @param string $service_name The service name + * @return string The OAuth provider name + */ + protected function get_provider($service_name) + { + return str_replace('auth.provider.oauth.service.', '', $service_name); } /** - * {@inheritdoc} - */ - public function unlink_account(array $link_data) + * Returns the localized title for the OAuth provider. + * + * @param string $provider The OAuth provider name + * @return string The OAuth provider title + */ + protected function get_provider_title($provider) { - if (!array_key_exists('oauth_service', $link_data) || !$link_data['oauth_service']) + return $this->language->lang('AUTH_PROVIDER_OAUTH_SERVICE_' . utf8_strtoupper($provider)); + } + + /** + * Returns whether or not the authorization code is set. + * + * @param OAuth1Service|OAuth2Service $service The external OAuth service + * @return bool Whether or not the authorization code is set in the URL + * for the respective OAuth service's version + */ + protected function is_set_code($service) + { + switch ($service::OAUTH_VERSION) { - return 'LOGIN_LINK_MISSING_DATA'; + case 1: + return $this->request->is_set('oauth_token', \phpbb\request\request_interface::GET); + + case 2: + return $this->request->is_set('code', \phpbb\request\request_interface::GET); + + default: + return false; } + } - // Remove user specified in $link_data if possible - $user_id = isset($link_data['user_id']) ? $link_data['user_id'] : $this->user->data['user_id']; + /** + * Sets a redirect to the authorization uri. + * + * @param OAuth1Service|OAuth2Service $service The external OAuth service + * @return array|false Array if an error occurred, + * false on success + */ + protected function set_redirect($service) + { + $parameters = []; - // Remove the link - $sql = 'DELETE FROM ' . $this->auth_provider_oauth_token_account_assoc . " - WHERE provider = '" . $this->db->sql_escape($link_data['oauth_service']) . "' - AND user_id = " . (int) $user_id; - $this->db->sql_query($sql); + if ($service::OAUTH_VERSION === 1) + { + try + { + $token = $service->requestRequestToken(); + $parameters = ['oauth_token' => $token->getRequestToken()]; + } + catch (\OAuth\Common\Http\Exception\TokenResponseException $e) + { + return [ + 'status' => LOGIN_ERROR_EXTERNAL_AUTH, + 'error_msg' => $e->getMessage(), + 'user_row' => ['user_id' => ANONYMOUS], + ]; + } + } - // Clear all tokens belonging to the user on this service - $service_name = 'auth.provider.oauth.service.' . strtolower($link_data['oauth_service']); - $storage = new \phpbb\auth\provider\oauth\token_storage($this->db, $this->user, $this->auth_provider_oauth_token_storage_table, $this->auth_provider_oauth_state_table); - $storage->clearToken($service_name); + redirect($service->getAuthorizationUri($parameters), false, true); + + return false; } } diff --git a/phpBB/phpbb/auth/provider/oauth/service/base.php b/phpBB/phpbb/auth/provider/oauth/service/base.php index 6adf64aa30..566b77202a 100644 --- a/phpBB/phpbb/auth/provider/oauth/service/base.php +++ b/phpBB/phpbb/auth/provider/oauth/service/base.php @@ -1,51 +1,59 @@ -* @license GNU General Public License, version 2 (GPL-2.0) -* -* For full copyright and license information, please see -* the docs/CREDITS.txt file. -* -*/ + * + * This file is part of the phpBB Forum Software package. + * + * @copyright (c) phpBB Limited + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ namespace phpbb\auth\provider\oauth\service; /** -* Base OAuth abstract class that all OAuth services should implement -*/ -abstract class base implements \phpbb\auth\provider\oauth\service\service_interface + * Base OAuth abstract class that all OAuth services should implement + */ +abstract class base implements service_interface { /** - * External OAuth service provider - * - * @var \OAuth\Common\Service\ServiceInterface - */ + * External OAuth service provider + * + * @var \OAuth\Common\Service\ServiceInterface + */ protected $service_provider; /** - * {@inheritdoc} - */ - public function get_external_service_provider() + * {@inheritdoc} + */ + public function get_auth_scope() { - return $this->service_provider; + return []; } /** - * {@inheritdoc} - */ - public function get_auth_scope() + * {@inheritdoc} + */ + public function get_external_service_class() { - return array(); + return ''; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function set_external_service_provider(\OAuth\Common\Service\ServiceInterface $service_provider) { $this->service_provider = $service_provider; } + + /** + * {@inheritdoc} + */ + public function get_external_service_provider() + { + return $this->service_provider; + } } diff --git a/phpBB/phpbb/auth/provider/oauth/service/bitly.php b/phpBB/phpbb/auth/provider/oauth/service/bitly.php index 25e731a02c..ca131b2019 100644 --- a/phpBB/phpbb/auth/provider/oauth/service/bitly.php +++ b/phpBB/phpbb/auth/provider/oauth/service/bitly.php @@ -1,94 +1,107 @@ -* @license GNU General Public License, version 2 (GPL-2.0) -* -* For full copyright and license information, please see -* the docs/CREDITS.txt file. -* -*/ + * + * This file is part of the phpBB Forum Software package. + * + * @copyright (c) phpBB Limited + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ namespace phpbb\auth\provider\oauth\service; /** -* Bitly OAuth service -*/ -class bitly extends \phpbb\auth\provider\oauth\service\base + * Bitly OAuth service + */ +class bitly extends base { - /** - * phpBB config - * - * @var \phpbb\config\config - */ + /** @var \phpbb\config\config */ protected $config; - /** - * phpBB request - * - * @var \phpbb\request\request_interface - */ + /** @var \phpbb\request\request_interface */ protected $request; /** - * Constructor - * - * @param \phpbb\config\config $config - * @param \phpbb\request\request_interface $request - */ + * Constructor. + * + * @param \phpbb\config\config $config Config object + * @param \phpbb\request\request_interface $request Request object + */ public function __construct(\phpbb\config\config $config, \phpbb\request\request_interface $request) { - $this->config = $config; - $this->request = $request; + $this->config = $config; + $this->request = $request; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function get_service_credentials() { - return array( + return [ 'key' => $this->config['auth_oauth_bitly_key'], 'secret' => $this->config['auth_oauth_bitly_secret'], - ); + ]; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function perform_auth_login() { 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 - $this->service_provider->requestAccessToken($this->request->variable('code', '')); + 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'); + } - // Send a request with it - $result = json_decode($this->service_provider->request('user/info'), true); + try + { + // Send a request with it + $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']; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function perform_token_auth() { 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'); } - // Send a request with it - $result = json_decode($this->service_provider->request('user/info'), true); + try + { + // Send a request with it + $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']; } } diff --git a/phpBB/phpbb/auth/provider/oauth/service/facebook.php b/phpBB/phpbb/auth/provider/oauth/service/facebook.php index bb98835e07..f7dbe307eb 100644 --- a/phpBB/phpbb/auth/provider/oauth/service/facebook.php +++ b/phpBB/phpbb/auth/provider/oauth/service/facebook.php @@ -1,63 +1,55 @@ -* @license GNU General Public License, version 2 (GPL-2.0) -* -* For full copyright and license information, please see -* the docs/CREDITS.txt file. -* -*/ + * + * This file is part of the phpBB Forum Software package. + * + * @copyright (c) phpBB Limited + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ namespace phpbb\auth\provider\oauth\service; /** -* Facebook OAuth service -*/ + * Facebook OAuth service + */ class facebook extends base { - /** - * phpBB config - * - * @var \phpbb\config\config - */ + /** @var \phpbb\config\config */ protected $config; - /** - * phpBB request - * - * @var \phpbb\request\request_interface - */ + /** @var \phpbb\request\request_interface */ protected $request; /** - * Constructor - * - * @param \phpbb\config\config $config - * @param \phpbb\request\request_interface $request - */ + * Constructor. + * + * @param \phpbb\config\config $config Config object + * @param \phpbb\request\request_interface $request Request object + */ public function __construct(\phpbb\config\config $config, \phpbb\request\request_interface $request) { - $this->config = $config; - $this->request = $request; + $this->config = $config; + $this->request = $request; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function get_service_credentials() { - return array( + return [ 'key' => $this->config['auth_oauth_facebook_key'], 'secret' => $this->config['auth_oauth_facebook_secret'], - ); + ]; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function perform_auth_login() { if (!($this->service_provider instanceof \OAuth\OAuth2\Service\Facebook)) @@ -65,19 +57,33 @@ class facebook extends base 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', '')); + 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'); + } - // Send a request with it - $result = json_decode($this->service_provider->request('/me'), true); + try + { + // Send a request with it + $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']; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function perform_token_auth() { if (!($this->service_provider instanceof \OAuth\OAuth2\Service\Facebook)) @@ -85,8 +91,15 @@ class facebook extends base throw new exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE'); } - // Send a request with it - $result = json_decode($this->service_provider->request('/me'), true); + try + { + // Send a request with it + $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']; diff --git a/phpBB/phpbb/auth/provider/oauth/service/google.php b/phpBB/phpbb/auth/provider/oauth/service/google.php index cb9f83a94f..6e671ab13e 100644 --- a/phpBB/phpbb/auth/provider/oauth/service/google.php +++ b/phpBB/phpbb/auth/provider/oauth/service/google.php @@ -1,74 +1,66 @@ -* @license GNU General Public License, version 2 (GPL-2.0) -* -* For full copyright and license information, please see -* the docs/CREDITS.txt file. -* -*/ + * + * This file is part of the phpBB Forum Software package. + * + * @copyright (c) phpBB Limited + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ namespace phpbb\auth\provider\oauth\service; /** -* Google OAuth service -*/ + * Google OAuth service + */ class google extends base { - /** - * phpBB config - * - * @var \phpbb\config\config - */ + /** @var \phpbb\config\config */ protected $config; - /** - * phpBB request - * - * @var \phpbb\request\request_interface - */ + /** @var \phpbb\request\request_interface */ protected $request; /** - * Constructor - * - * @param \phpbb\config\config $config - * @param \phpbb\request\request_interface $request - */ + * Constructor. + * + * @param \phpbb\config\config $config Config object + * @param \phpbb\request\request_interface $request Request object + */ public function __construct(\phpbb\config\config $config, \phpbb\request\request_interface $request) { - $this->config = $config; - $this->request = $request; + $this->config = $config; + $this->request = $request; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function get_auth_scope() { - return array( + return [ 'userinfo_email', 'userinfo_profile', - ); + ]; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function get_service_credentials() { - return array( + return [ 'key' => $this->config['auth_oauth_google_key'], 'secret' => $this->config['auth_oauth_google_secret'], - ); + ]; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function perform_auth_login() { if (!($this->service_provider instanceof \OAuth\OAuth2\Service\Google)) @@ -76,19 +68,33 @@ class google extends base 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', '')); + 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'); + } - // Send a request with it - $result = json_decode($this->service_provider->request('https://www.googleapis.com/oauth2/v1/userinfo'), true); + try + { + // Send a request with it + $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']; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function perform_token_auth() { if (!($this->service_provider instanceof \OAuth\OAuth2\Service\Google)) @@ -96,8 +102,15 @@ class google extends base throw new exception('AUTH_PROVIDER_OAUTH_ERROR_INVALID_SERVICE_TYPE'); } - // Send a request with it - $result = json_decode($this->service_provider->request('https://www.googleapis.com/oauth2/v1/userinfo'), true); + try + { + // Send a request with it + $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']; diff --git a/phpBB/phpbb/auth/provider/oauth/service/service_interface.php b/phpBB/phpbb/auth/provider/oauth/service/service_interface.php index e84eb247b6..8d92a3725e 100644 --- a/phpBB/phpbb/auth/provider/oauth/service/service_interface.php +++ b/phpBB/phpbb/auth/provider/oauth/service/service_interface.php @@ -1,73 +1,87 @@ -* @license GNU General Public License, version 2 (GPL-2.0) -* -* For full copyright and license information, please see -* the docs/CREDITS.txt file. -* -*/ + * + * This file is part of the phpBB Forum Software package. + * + * @copyright (c) phpBB Limited + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ namespace phpbb\auth\provider\oauth\service; /** -* OAuth service interface -*/ + * OAuth service interface + */ interface service_interface { /** - * Returns an array of the scopes necessary for auth - * - * @return array An array of the required scopes - */ + * Returns an array of the scopes necessary for auth + * + * @return array An array of the required scopes + */ 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. - * - * @return array An array containing the 'key' and the 'secret' of the - * service in the form: - * array( - * 'key' => string - * 'secret' => string - * ) - */ + * Returns an array containing the service credentials belonging to requested + * service. + * + * @return array An array containing the 'key' and the 'secret' of the + * service in the form: + * array( + * 'key' => string + * 'secret' => string + * ) + */ public function get_service_credentials(); /** - * Returns the results of the authentication in json format - * - * @throws \phpbb\auth\provider\oauth\service\exception - * @return string The unique identifier returned by the service provider - * that is used to authenticate the user with phpBB. - */ + * Returns the results of the authentication in json format + * + * @throws \phpbb\auth\provider\oauth\service\exception + * @return string The unique identifier returned by the service provider + * that is used to authenticate the user with phpBB. + */ public function perform_auth_login(); /** - * Returns the results of the authentication in json format - * Use this function when the user already has an access token - * - * @throws \phpbb\auth\provider\oauth\service\exception - * @return string The unique identifier returned by the service provider - * that is used to authenticate the user with phpBB. - */ + * Returns the results of the authentication in json format + * Use this function when the user already has an access token + * + * @throws \phpbb\auth\provider\oauth\service\exception + * @return string The unique identifier returned by the service provider + * that is used to authenticate the user with phpBB. + */ public function perform_token_auth(); /** - * Sets the external library service provider - * - * @param \OAuth\Common\Service\ServiceInterface $service_provider - */ + * 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\ + * 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(); } diff --git a/phpBB/phpbb/auth/provider/oauth/service/twitter.php b/phpBB/phpbb/auth/provider/oauth/service/twitter.php index 06beac51e2..35cbc9e4f7 100644 --- a/phpBB/phpbb/auth/provider/oauth/service/twitter.php +++ b/phpBB/phpbb/auth/provider/oauth/service/twitter.php @@ -1,102 +1,111 @@ -* @license GNU General Public License, version 2 (GPL-2.0) -* -* For full copyright and license information, please see -* the docs/CREDITS.txt file. -* -*/ + * + * This file is part of the phpBB Forum Software package. + * + * @copyright (c) phpBB Limited + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ namespace phpbb\auth\provider\oauth\service; /** -* Twitter OAuth service -*/ -class twitter extends \phpbb\auth\provider\oauth\service\base + * Twitter OAuth service + */ +class twitter extends base { - /** - * phpBB config - * - * @var \phpbb\config\config - */ + /** @var \phpbb\config\config */ protected $config; - /** - * phpBB request - * - * @var \phpbb\request\request_interface - */ + /** @var \phpbb\request\request_interface */ protected $request; /** - * Constructor - * - * @param \phpbb\config\config $config - * @param \phpbb\request\request_interface $request - */ + * Constructor. + * + * @param \phpbb\config\config $config Config object + * @param \phpbb\request\request_interface $request Request object + */ public function __construct(\phpbb\config\config $config, \phpbb\request\request_interface $request) { - $this->config = $config; - $this->request = $request; + $this->config = $config; + $this->request = $request; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function get_service_credentials() { - return array( + return [ 'key' => $this->config['auth_oauth_twitter_key'], 'secret' => $this->config['auth_oauth_twitter_secret'], - ); + ]; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function perform_auth_login() { 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 - $this->service_provider->requestAccessToken( - $this->request->variable('oauth_token', ''), - $this->request->variable('oauth_verifier', ''), - $tokensecret - ); + 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', ''), + $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']; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function perform_token_auth() { 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']; } } diff --git a/phpBB/phpbb/auth/provider/oauth/token_storage.php b/phpBB/phpbb/auth/provider/oauth/token_storage.php index b0c2fd0d62..861b00f5cf 100644 --- a/phpBB/phpbb/auth/provider/oauth/token_storage.php +++ b/phpBB/phpbb/auth/provider/oauth/token_storage.php @@ -1,15 +1,15 @@ -* @license GNU General Public License, version 2 (GPL-2.0) -* -* For full copyright and license information, please see -* the docs/CREDITS.txt file. -* -*/ + * + * This file is part of the phpBB Forum Software package. + * + * @copyright (c) phpBB Limited + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ namespace phpbb\auth\provider\oauth; @@ -20,67 +20,48 @@ 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 { - /** - * Cache driver. - * - * @var \phpbb\db\driver\driver_interface - */ + /** @var \phpbb\db\driver\driver_interface */ protected $db; - /** - * phpBB user - * - * @var \phpbb\user - */ + /** @var \phpbb\user */ protected $user; - /** - * OAuth token table - * - * @var string - */ + /** @var string OAuth table: token storage */ protected $oauth_token_table; - /** - * OAuth state table - * - * @var string - */ + /** @var string OAuth table: state */ protected $oauth_state_table; - /** - * @var object|TokenInterface - */ + /** @var TokenInterface OAuth token */ protected $cachedToken; - /** - * @var string - */ + /** @var string OAuth state */ protected $cachedState; /** - * Creates token storage for phpBB. - * - * @param \phpbb\db\driver\driver_interface $db - * @param \phpbb\user $user - * @param string $oauth_token_table - * @param string $oauth_state_table - */ + * Constructor. + * + * @param \phpbb\db\driver\driver_interface $db Database object + * @param \phpbb\user $user User object + * @param string $oauth_token_table OAuth table: token storage + * @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->db = $db; + $this->user = $user; + $this->oauth_token_table = $oauth_token_table; $this->oauth_state_table = $oauth_state_table; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function retrieveAccessToken($service) { $service = $this->get_service_name_for_db($service); @@ -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) { @@ -104,33 +85,38 @@ class token_storage implements TokenStorageInterface } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function storeAccessToken($service, TokenInterface $token) { $service = $this->get_service_name_for_db($service); $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'] . ' - ' . ((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) . "'"; + SET ' . $this->db->sql_build_array('UPDATE', $data) . ' + WHERE user_id = ' . (int) $this->user->data['user_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); @@ -141,8 +127,8 @@ class token_storage implements TokenStorageInterface } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function hasAccessToken($service) { $service = $this->get_service_name_for_db($service); @@ -152,22 +138,22 @@ 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); } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function clearToken($service) { $service = $this->get_service_name_for_db($service); @@ -189,13 +175,13 @@ class token_storage implements TokenStorageInterface } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function clearAllTokens() { $this->cachedToken = null; - $sql = 'DELETE FROM ' . $this->oauth_token_table . ' + $sql = 'DELETE FROM ' . $this->oauth_token_table . ' WHERE user_id = ' . (int) $this->user->data['user_id']; if ((int) $this->user->data['user_id'] === ANONYMOUS) @@ -209,31 +195,30 @@ class token_storage implements TokenStorageInterface } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function storeAuthorizationState($service, $state) { $service = $this->get_service_name_for_db($service); $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 . ' - ' . $this->db->sql_build_array('INSERT', $data); + $sql = 'INSERT INTO ' . $this->oauth_state_table . ' ' . $this->db->sql_build_array('INSERT', $data); $this->db->sql_query($sql); return $this; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function hasAuthorizationState($service) { $service = $this->get_service_name_for_db($service); @@ -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) { @@ -257,8 +242,8 @@ class token_storage implements TokenStorageInterface } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function retrieveAuthorizationState($service) { $service = $this->get_service_name_for_db($service); @@ -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) { @@ -282,8 +267,8 @@ class token_storage implements TokenStorageInterface } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function clearAuthorizationState($service) { $service = $this->get_service_name_for_db($service); @@ -305,8 +290,8 @@ class token_storage implements TokenStorageInterface } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function clearAllAuthorizationStates() { $this->cachedState = null; @@ -325,10 +310,11 @@ class token_storage implements TokenStorageInterface } /** - * Updates the user_id field in the database assosciated with the token - * - * @param int $user_id - */ + * Updates the user_id field in the database associated with the token. + * + * @param int $user_id The user identifier + * @return void + */ public function set_user_id($user_id) { if (!$this->cachedToken) @@ -336,21 +322,24 @@ 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( - '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']) . "'"; + SET ' . $this->db->sql_build_array('UPDATE', $data) . ' + 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 - * - * @param string $service The name of the OAuth service - * @return bool true if they have token, false if they don't - */ + * Checks to see if an access token exists solely by the session_id of the user. + * + * @param string $service The OAuth service name + * @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) { $service = $this->get_service_name_for_db($service); @@ -360,20 +349,21 @@ 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 - * - * @param string $service The name of the OAuth service - * @return bool true if they have state, false if they don't - */ + * Checks to see if a state exists solely by the session_id of the user. + * + * @param string $service The OAuth service name + * @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) { $service = $this->get_service_name_for_db($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 - * - * @param array $data - * @return bool - */ - protected function _has_acess_token($data) + * A helper function that performs the query for has access token functions. + * + * @param array $data The SQL WHERE data + * @return bool true if the user's access token exists, + * false if the user's access token does not exist + */ + 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, - ); + '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,22 +434,22 @@ class token_storage implements TokenStorageInterface return $this->cachedState; } - $data = array( + $data = [ 'session_id' => $this->user->data['session_id'], - 'provider' => $service, - ); + 'provider' => $service, + ]; return $this->_retrieve_state($data); } /** - * A helper function that performs the query for retrieve access token functions - * Also checks if the token is a valid token - * - * @param array $data - * @return mixed - * @throws \OAuth\Common\Storage\Exception\TokenNotFoundException - */ + * A helper function that performs the query for retrieve access token functions. + * Also checks if the token is a valid token. + * + * @param array $data The SQL WHERE data + * @return TokenInterface + * @throws TokenNotFoundException + */ protected function _retrieve_access_token($data) { $row = $this->get_access_token_row($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 - * @return mixed - * @throws \OAuth\Common\Storage\Exception\AuthorizationStateNotFoundException + * @param array $data The SQL WHERE data + * @return string The OAuth state + * @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 - * - * @param array $data - * @return mixed - */ + * A helper function that performs the query for retrieving an access token. + * + * @param array $data The SQL WHERE data + * @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 - * @return mixed + * @param array $data The SQL WHERE data + * @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. - * - * @param string $service The name of the OAuth service - * @return string The name of the OAuth service as it needs to be stored - * in the database. - */ - protected function get_service_name_for_db($service) + * Returns the service name as it must be stored in the database. + * + * @param string $provider The OAuth provider name + * @return string The OAuth service name + */ + 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; } } diff --git a/phpBB/phpbb/auth/provider/provider_interface.php b/phpBB/phpbb/auth/provider/provider_interface.php index 463324ff46..21c73a33c5 100644 --- a/phpBB/phpbb/auth/provider/provider_interface.php +++ b/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. * diff --git a/tests/functions/user_delete_test.php b/tests/functions/user_delete_test.php index 89aecdefb9..83fda05542 100644 --- a/tests/functions/user_delete_test.php +++ b/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_dispatcher, - $this->phpbb_root_path, - $this->php_ext + $phpbb_root_path, + $phpEx ); $provider_collection->offsetSet('auth.provider.oauth', $oauth_provider); -- cgit v1.2.1 From 76b4a2faabf351e3a6ab71fd10ebf33045bf05a9 Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Sun, 5 May 2019 18:30:21 +0200 Subject: [ticket/16008] Unindent YML comments PHPBB3-16008 --- phpBB/config/default/container/services_auth.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/config/default/container/services_auth.yml b/phpBB/config/default/container/services_auth.yml index d1aeed01aa..5d8820842f 100644 --- a/phpBB/config/default/container/services_auth.yml +++ b/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: @@ -70,7 +70,7 @@ services: tags: - { name: auth.provider } - # ----- OAuth services providers ----- +# ----- OAuth services providers ----- auth.provider.oauth.service_collection: class: phpbb\di\service_collection arguments: -- cgit v1.2.1 From 85910fe5bc828743f39cf295e48f6d393390433b Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Sun, 5 May 2019 18:31:56 +0200 Subject: [ticket/16008] Add missing empty line PHPBB3-16008 --- phpBB/phpbb/auth/provider/provider_interface.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/auth/provider/provider_interface.php b/phpBB/phpbb/auth/provider/provider_interface.php index 21c73a33c5..6ad9b36f83 100644 --- a/phpBB/phpbb/auth/provider/provider_interface.php +++ b/phpBB/phpbb/auth/provider/provider_interface.php @@ -74,7 +74,7 @@ interface provider_interface * * @param \phpbb\config\config $new_config Contains the new configuration values * that have been set in acp_board. - * @return array|null Returns null if not implemented or an array with + * @return array|null Returns null if not implemented or an array withe * the template file name and an array of the vars * that the template needs that must conform to the * following example: -- cgit v1.2.1 From 95a696c4daa7f481c71362f87d9b6c6cae34cbf4 Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Sun, 5 May 2019 20:37:05 +0200 Subject: [ticket/16008] Properly order get_external_service_provider PHPBB3-16008 --- phpBB/phpbb/auth/provider/oauth/service/base.php | 8 ++++---- .../phpbb/auth/provider/oauth/service/service_interface.php | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/phpBB/phpbb/auth/provider/oauth/service/base.php b/phpBB/phpbb/auth/provider/oauth/service/base.php index 566b77202a..5ab426a0aa 100644 --- a/phpBB/phpbb/auth/provider/oauth/service/base.php +++ b/phpBB/phpbb/auth/provider/oauth/service/base.php @@ -44,16 +44,16 @@ abstract class base implements service_interface /** * {@inheritdoc} */ - public function set_external_service_provider(\OAuth\Common\Service\ServiceInterface $service_provider) + public function get_external_service_provider() { - $this->service_provider = $service_provider; + return $this->service_provider; } /** * {@inheritdoc} */ - public function get_external_service_provider() + public function set_external_service_provider(\OAuth\Common\Service\ServiceInterface $service_provider) { - return $this->service_provider; + $this->service_provider = $service_provider; } } diff --git a/phpBB/phpbb/auth/provider/oauth/service/service_interface.php b/phpBB/phpbb/auth/provider/oauth/service/service_interface.php index 8d92a3725e..ea9ef43788 100644 --- a/phpBB/phpbb/auth/provider/oauth/service/service_interface.php +++ b/phpBB/phpbb/auth/provider/oauth/service/service_interface.php @@ -72,16 +72,16 @@ interface service_interface public function get_external_service_class(); /** - * Sets the external library service provider + * Returns the external library service provider once it has been set * - * @param \OAuth\Common\Service\ServiceInterface $service_provider + * @param \OAuth\Common\Service\ServiceInterface|null */ - public function set_external_service_provider(\OAuth\Common\Service\ServiceInterface $service_provider); + public function get_external_service_provider(); /** - * Returns the external library service provider once it has been set + * Sets the external library service provider * - * @param \OAuth\Common\Service\ServiceInterface|null + * @param \OAuth\Common\Service\ServiceInterface $service_provider */ - public function get_external_service_provider(); + public function set_external_service_provider(\OAuth\Common\Service\ServiceInterface $service_provider); } -- cgit v1.2.1 From 219955fa6cf7de3fadf96f99292c4e411b3decfc Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Sun, 5 May 2019 21:41:06 +0200 Subject: [ticket/16008] Remove empty line, re-add @changed and remove duplicate code PHPBB3-16008 --- phpBB/phpbb/auth/provider/oauth/oauth.php | 29 ++--------------------------- 1 file changed, 2 insertions(+), 27 deletions(-) diff --git a/phpBB/phpbb/auth/provider/oauth/oauth.php b/phpBB/phpbb/auth/provider/oauth/oauth.php index a1538761f1..20810149dd 100644 --- a/phpBB/phpbb/auth/provider/oauth/oauth.php +++ b/phpBB/phpbb/auth/provider/oauth/oauth.php @@ -242,7 +242,7 @@ class oauth extends \phpbb\auth\provider\base * @var array redirect_data Data to be appended to the redirect url * @var ServiceInterface service OAuth service * @since 3.2.3-RC1 - * @changed 3.2.6-RC1 Added redirect_data + * @changed 3.2.6-RC1 Added redirect_data */ $vars = [ 'row', @@ -282,7 +282,7 @@ class oauth extends \phpbb\auth\provider\base /** * Check if the user is banned. - * The fourth parameter, return, has to be true, otherwise the OAuth login is still called and + * The fourth parameter (return) has to be true, otherwise the OAuth login is still called and * an uncaught exception is thrown as there is no token stored in the database. */ $ban = $this->user->check_ban($row['user_id'], $row['user_ip'], $row['user_email'], true); @@ -305,31 +305,6 @@ class oauth extends \phpbb\auth\provider\base ]; } - /** - * Check if the user is banned. - * The fourth parameter, return, has to be true, - * otherwise the OAuth login is still called and - * an uncaught exception is thrown as there is no - * token stored in the database. - */ - $ban = $this->user->check_ban($row['user_id'], $row['user_ip'], $row['user_email'], true); - if (!empty($ban)) - { - $till_date = !empty($ban['ban_end']) ? $this->user->format_date($ban['ban_end']) : ''; - $message = !empty($ban['ban_end']) ? 'BOARD_BAN_TIME' : 'BOARD_BAN_PERM'; - - $contact_link = phpbb_get_board_contact_link($this->config, $this->phpbb_root_path, $this->php_ext); - $message = $this->user->lang($message, $till_date, '', ''); - $message .= !empty($ban['ban_give_reason']) ? '

' . $this->user->lang('BOARD_BAN_REASON', $ban['ban_give_reason']) : ''; - $message .= !empty($ban['ban_triggered_by']) ? '

' . $this->user->lang('BAN_TRIGGERED_BY_' . strtoupper($ban['ban_triggered_by'])) . '' : ''; - - return array( - 'status' => LOGIN_BREAK, - 'error_msg' => $message, - 'user_row' => $row, - ); - } - // Update token storage to store the user_id $storage->set_user_id($row['user_id']); -- cgit v1.2.1 From cbb5e6f765fbebc86980e5c72321fca79324aa34 Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Tue, 8 Oct 2019 20:18:20 +0200 Subject: [ticket/16008] Enforce string data type as per ticket/16181 PHPBB3-16008 --- phpBB/phpbb/auth/provider/oauth/oauth.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/phpBB/phpbb/auth/provider/oauth/oauth.php b/phpBB/phpbb/auth/provider/oauth/oauth.php index 20810149dd..fe82663799 100644 --- a/phpBB/phpbb/auth/provider/oauth/oauth.php +++ b/phpBB/phpbb/auth/provider/oauth/oauth.php @@ -215,7 +215,12 @@ class oauth extends \phpbb\auth\provider\base ]; } - // Check to see if this provider is already associated with an account + /** + * Check to see if this provider is already associated with an account. + * + * Enforcing a data type to make sure it are strings and not integers, + * so values are quoted in the SQL WHERE statement. + */ $data = [ 'provider' => (string) utf8_strtolower($provider), 'oauth_provider_id' => (string) $unique_id -- cgit v1.2.1 From 4679433ae19a12c79a6f568de4c85f4cf9cdf30b Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 11 Nov 2019 18:21:18 +0100 Subject: [ticket/16008] Adjust naming and remove typo PHPBB3-16008 --- phpBB/phpbb/auth/provider/oauth/token_storage.php | 8 ++++---- phpBB/phpbb/auth/provider/provider_interface.php | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/phpBB/phpbb/auth/provider/oauth/token_storage.php b/phpBB/phpbb/auth/provider/oauth/token_storage.php index 861b00f5cf..c0f585d7bb 100644 --- a/phpBB/phpbb/auth/provider/oauth/token_storage.php +++ b/phpBB/phpbb/auth/provider/oauth/token_storage.php @@ -20,7 +20,7 @@ use OAuth\Common\Storage\Exception\TokenNotFoundException; use OAuth\Common\Storage\Exception\AuthorizationStateNotFoundException; /** - * OAuth storage wrapper for phpBB3's cache + * OAuth storage wrapper for phpBB's cache */ class token_storage implements TokenStorageInterface { @@ -148,7 +148,7 @@ class token_storage implements TokenStorageInterface $data['session_id'] = $this->user->data['session_id']; } - return $this->_has_access_token($data); + return $this->has_access_token($data); } /** @@ -354,7 +354,7 @@ class token_storage implements TokenStorageInterface 'provider' => $service, ]; - return $this->_has_access_token($data); + return $this->has_access_token($data); } /** @@ -388,7 +388,7 @@ class token_storage implements TokenStorageInterface * @return bool true if the user's access token exists, * false if the user's access token does not exist */ - protected function _has_access_token($data) + protected function has_access_token($data) { return (bool) $this->get_access_token_row($data); } diff --git a/phpBB/phpbb/auth/provider/provider_interface.php b/phpBB/phpbb/auth/provider/provider_interface.php index 6ad9b36f83..21c73a33c5 100644 --- a/phpBB/phpbb/auth/provider/provider_interface.php +++ b/phpBB/phpbb/auth/provider/provider_interface.php @@ -74,7 +74,7 @@ interface provider_interface * * @param \phpbb\config\config $new_config Contains the new configuration values * that have been set in acp_board. - * @return array|null Returns null if not implemented or an array withe + * @return array|null Returns null if not implemented or an array with * the template file name and an array of the vars * that the template needs that must conform to the * following example: -- cgit v1.2.1