From 6f5f0d6d8d5d3afcabccaa9da7c64108af5d4ab7 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 31 May 2014 22:43:07 +0200 Subject: [PATCH] [ticket/12352] Use custom provider collection for auth providers Using this custom provider collection, we can properly check whether the configured auth provider does exist. The method get_provider() has been added for returning the default auth provider or the standard db auth provider if the specified one does not exist. Additionally, the method get_provider() will throw an RuntimeException if none of the above exist. PHPBB3-12352 --- phpBB/config/auth_providers.yml | 3 +- phpBB/includes/functions.php | 10 +--- phpBB/phpbb/auth/provider_collection.php | 63 ++++++++++++++++++++++++ phpBB/phpbb/session.php | 25 ++-------- tests/session/testable_factory.php | 4 +- 5 files changed, 73 insertions(+), 32 deletions(-) create mode 100644 phpBB/phpbb/auth/provider_collection.php diff --git a/phpBB/config/auth_providers.yml b/phpBB/config/auth_providers.yml index dac8b9d252..d2f22ec477 100644 --- a/phpBB/config/auth_providers.yml +++ b/phpBB/config/auth_providers.yml @@ -1,8 +1,9 @@ services: auth.provider_collection: - class: phpbb\di\service_collection + class: phpbb\auth\provider_collection arguments: - @service_container + - @config tags: - { name: service_collection, tag: auth.provider } auth.provider.db: diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 31a6246d34..0d0bc4e6f6 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2855,15 +2855,7 @@ function login_box($redirect = '', $l_explain = '', $l_success = '', $admin = fa } $provider_collection = $phpbb_container->get('auth.provider_collection'); - $auth_method = $config['auth_method']; - - // Revert to db auth provider if selected method does not exist - if (!isset($provider_collection['auth.provider.' . $config['auth_method']])) - { - $auth_method = 'db'; - } - - $auth_provider = $provider_collection['auth.provider.' . $auth_method]; + $auth_provider = $provider_collection->get_provider(); $auth_provider_data = $auth_provider->get_login_data(); if ($auth_provider_data) diff --git a/phpBB/phpbb/auth/provider_collection.php b/phpBB/phpbb/auth/provider_collection.php new file mode 100644 index 0000000000..bef1dd2c50 --- /dev/null +++ b/phpBB/phpbb/auth/provider_collection.php @@ -0,0 +1,63 @@ +container = $container; + $this->config = $config; + } + + /** + * Get an auth provider. + * + * @return object Default auth provider selected in config if it + * does exist. Otherwise the standard db auth + * provider. + * @throws \RuntimeException If neither the auth provider that + * is specified by the phpBB config nor the db + * auth provider exist. The db auth provider + * should always exist in a phpBB installation. + */ + public function get_provider() + { + if ($this->offsetExists('auth.provider.' . basename(trim($this->config['auth_method'])))) + { + return $this->offsetGet('auth.provider.' . basename(trim($this->config['auth_method']))); + } + // Revert to db auth provider if selected method does not exist + elseif ($this->offsetExists('auth.provider.db')) + { + return $this->offsetGet('auth.provider.db'); + } + else + { + throw new \RuntimeException(sprintf('The authentication provider for the authentication method "%1$s" does not exist. It was not possible to recover from this by reverting to the database authentication provider.', $this->config['auth_method'])); + } + } +} diff --git a/phpBB/phpbb/session.php b/phpBB/phpbb/session.php index c663977882..ad6759a3e2 100644 --- a/phpBB/phpbb/session.php +++ b/phpBB/phpbb/session.php @@ -408,16 +408,8 @@ class session $session_expired = false; // Check whether the session is still valid if we have one - $method = basename(trim($config['auth_method'])); - $provider_collection = $phpbb_container->get('auth.provider_collection'); - - // Revert to db auth provider if selected method does not exist - if (!isset($provider_collection['auth.provider.' . $method])) - { - $method = 'db'; - } - $provider = $provider_collection['auth.provider.' . $method]; + $provider = $provider_collection->get_provider(); if (!($provider instanceof \phpbb\auth\provider\provider_interface)) { @@ -584,16 +576,8 @@ class session } } - $method = basename(trim($config['auth_method'])); - $provider_collection = $phpbb_container->get('auth.provider_collection'); - - // Revert to db auth provider if selected method does not exist - if (!isset($provider_collection['auth.provider.' . $method])) - { - $method = 'db'; - } - $provider = $provider_collection['auth.provider.' . $method]; + $provider = $provider_collection->get_provider(); $this->data = $provider->autologin(); if (sizeof($this->data)) @@ -912,9 +896,8 @@ class session $db->sql_query($sql); // Allow connecting logout with external auth method logout - $method = basename(trim($config['auth_method'])); - - $provider = $phpbb_container->get('auth.provider.' . $method); + $provider_collection = $phpbb_container->get('auth.provider_collection'); + $provider = $provider_collection->get_provider(); $provider->logout($this->data, $new_session); if ($this->data['user_id'] != ANONYMOUS) diff --git a/tests/session/testable_factory.php b/tests/session/testable_factory.php index 4bd7fa1366..3e25286480 100644 --- a/tests/session/testable_factory.php +++ b/tests/session/testable_factory.php @@ -96,9 +96,11 @@ class phpbb_session_testable_factory 'auth.provider.db', new phpbb_mock_auth_provider() ); + $provider_collection = new \phpbb\auth\provider_collection($phpbb_container, $config); + $provider_collection->add('auth.provider.db'); $phpbb_container->set( 'auth.provider_collection', - array('auth.provider.db' => $phpbb_container->get('auth.provider.db')) + $provider_collection ); $session = new phpbb_mock_session_testable;