From 7b5ad8bb74f66721d9c0742794f002272aec5b4d Mon Sep 17 00:00:00 2001 From: rxu Date: Mon, 14 Jun 2021 00:25:36 +0700 Subject: [PATCH 1/2] [ticket/16799] Fix OAuth external account linking PHP fatal error PHPBB3-16799 --- phpBB/phpbb/auth/provider/base.php | 2 +- phpBB/phpbb/auth/provider/oauth/oauth.php | 6 +- .../auth/provider/provider_interface.php | 2 +- tests/functional/auth_test.php | 59 +++++++++++++++++++ .../phpbb_functional_test_case.php | 2 +- 5 files changed, 65 insertions(+), 6 deletions(-) diff --git a/phpBB/phpbb/auth/provider/base.php b/phpBB/phpbb/auth/provider/base.php index 30e0a0fe2d..6aab3c2735 100644 --- a/phpBB/phpbb/auth/provider/base.php +++ b/phpBB/phpbb/auth/provider/base.php @@ -85,7 +85,7 @@ abstract class base implements provider_interface /** * {@inheritdoc} */ - public function login_link_has_necessary_data($login_link_data) + public function login_link_has_necessary_data(array $login_link_data) { return; } diff --git a/phpBB/phpbb/auth/provider/oauth/oauth.php b/phpBB/phpbb/auth/provider/oauth/oauth.php index 29ffe6d591..6f28d0f04f 100644 --- a/phpBB/phpbb/auth/provider/oauth/oauth.php +++ b/phpBB/phpbb/auth/provider/oauth/oauth.php @@ -163,7 +163,7 @@ class oauth extends base $provider = $this->request->variable('oauth_service', '', false); $service_name = $this->get_service_name($provider); - if ($provider === '' || !array_key_exists($service_name, $this->service_providers)) + if ($provider === '' || !array_key_exists($service_name, (array) $this->service_providers)) { return [ 'status' => LOGIN_ERROR_EXTERNAL_AUTH, @@ -411,7 +411,7 @@ class oauth extends base /** * {@inheritdoc} */ - public function login_link_has_necessary_data($login_link_data) + public function login_link_has_necessary_data(array $login_link_data) { if (empty($login_link_data)) { @@ -452,7 +452,7 @@ class oauth extends base $service_name = $this->get_service_name($link_data['oauth_service']); - if (!array_key_exists($service_name, $this->service_providers)) + if (!array_key_exists($service_name, (array) $this->service_providers)) { return 'LOGIN_ERROR_OAUTH_SERVICE_DOES_NOT_EXIST'; } diff --git a/phpBB/phpbb/auth/provider/provider_interface.php b/phpBB/phpbb/auth/provider/provider_interface.php index 21c73a33c5..389f6050f2 100644 --- a/phpBB/phpbb/auth/provider/provider_interface.php +++ b/phpBB/phpbb/auth/provider/provider_interface.php @@ -154,7 +154,7 @@ interface provider_interface * @return string|null Returns a string with a language constant if there * is data missing or null if there is no error. */ - public function login_link_has_necessary_data($login_link_data); + public function login_link_has_necessary_data(array $login_link_data); /** * Links an external account to a phpBB account. diff --git a/tests/functional/auth_test.php b/tests/functional/auth_test.php index 3e22a5b651..8bf3e61c1f 100644 --- a/tests/functional/auth_test.php +++ b/tests/functional/auth_test.php @@ -77,4 +77,63 @@ class phpbb_functional_auth_test extends phpbb_functional_test_case $crawler = self::request('GET', 'adm/index.php?sid=' . $this->sid); $this->assertStringContainsString($this->lang('ADMIN_PANEL'), $crawler->filter('h1')->text()); } + + public function test_board_auth_oauth_setting() + { + $this->login(); + $this->admin_login(); + $this->add_lang(['ucp', 'acp/board']); + + $crawler = self::request('GET', 'adm/index.php?i=acp_board&mode=auth&sid=' . $this->sid); + $this->assertStringContainsString($this->lang('AUTH_METHOD'), $crawler->filter('label[for="auth_method"]')->text()); + + // Set OAuth authentication method for Google with random keys + $form = $crawler->selectButton($this->lang('SUBMIT'))->form([ + 'config[auth_method]' => 'oauth', + 'config[auth_oauth_google_key]' => '123456', + 'config[auth_oauth_google_secret]' => '123456', + ]); + $crawler = self::submit($form); + $this->assertContainsLang('CONFIG_UPDATED', $crawler->filter('div[class="successbox"] > p')->text()); + + // Test OAuth linking via UCP + $crawler = self::request('GET', 'ucp.php?i=ucp_auth_link&mode=auth_link&sid=' . $this->sid); + $this->assertStringContainsString($this->lang('AUTH_PROVIDER_OAUTH_SERVICE_GOOGLE'), $crawler->filter('h3')->text()); + $form = $crawler->selectButton($this->lang('UCP_AUTH_LINK_LINK'))->form(); + $crawler = self::submit($form); + $this->assertStringContainsString('Google Accounts', $crawler->filter('title')->text()); + + // Test OAuth linking for registration + $this->logout(); + $crawler = self::request('GET', 'ucp.php?mode=register'); + $this->assertContainsLang('REGISTRATION', $crawler->filter('div.content h2')->text()); + $form = $crawler->selectButton('I agree to these terms')->form(); + $crawler = self::submit($form); + $this->assertContainsLang('AUTH_PROVIDER_OAUTH_SERVICE_GOOGLE', $crawler->filter('a[class="button2"]')->text()); + $crawler = self::request('GET', 'ucp.php?mode=login&login=external&oauth_service=google'); + $this->assertStringContainsString('Google Accounts', $crawler->filter('title')->text()); + + // Restore default auth method, but unset random keys first + // Restart webclient as we were redirected to external site before + self::$client->restart(); + + $this->login(); + $this->admin_login(); + + $crawler = self::request('GET', 'adm/index.php?i=acp_board&mode=auth&sid=' . $this->sid); + $this->assertStringContainsString($this->lang('AUTH_METHOD'), $crawler->filter('label[for="auth_method"]')->text()); + $form = $crawler->selectButton($this->lang('SUBMIT'))->form([ + 'config[auth_oauth_google_key]' => '', + 'config[auth_oauth_google_secret]' => '', + ]); + $crawler = self::submit($form); + $this->assertContainsLang('CONFIG_UPDATED', $crawler->filter('div[class="successbox"] > p')->text()); + $crawler = self::request('GET', 'adm/index.php?i=acp_board&mode=auth&sid=' . $this->sid); + $this->assertStringContainsString($this->lang('AUTH_METHOD'), $crawler->filter('label[for="auth_method"]')->text()); + $form = $crawler->selectButton($this->lang('SUBMIT'))->form([ + 'config[auth_method]' => 'db', + ]); + $crawler = self::submit($form); + $this->assertContainsLang('CONFIG_UPDATED', $crawler->filter('div[class="successbox"] > p')->text()); + } } diff --git a/tests/test_framework/phpbb_functional_test_case.php b/tests/test_framework/phpbb_functional_test_case.php index 75453cb5fa..cc10986fa2 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/tests/test_framework/phpbb_functional_test_case.php @@ -973,7 +973,7 @@ class phpbb_functional_test_case extends phpbb_test_case // Any output before the doc type means there was an error $content = self::get_content(); self::assertStringNotContainsString('[phpBB Debug]', $content); - self::assertStringStartsWith(' Date: Thu, 1 Jul 2021 03:04:38 +0700 Subject: [PATCH 2/2] [ticket/16799] Use offsetExists() method instead of array_key_exists() PHPBB3-16799 --- phpBB/phpbb/auth/provider/oauth/oauth.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/auth/provider/oauth/oauth.php b/phpBB/phpbb/auth/provider/oauth/oauth.php index 6f28d0f04f..cc84bf5dd7 100644 --- a/phpBB/phpbb/auth/provider/oauth/oauth.php +++ b/phpBB/phpbb/auth/provider/oauth/oauth.php @@ -163,7 +163,7 @@ class oauth extends base $provider = $this->request->variable('oauth_service', '', false); $service_name = $this->get_service_name($provider); - if ($provider === '' || !array_key_exists($service_name, (array) $this->service_providers)) + if ($provider === '' || !$this->service_providers->offsetExists($service_name)) { return [ 'status' => LOGIN_ERROR_EXTERNAL_AUTH, @@ -452,7 +452,7 @@ class oauth extends base $service_name = $this->get_service_name($link_data['oauth_service']); - if (!array_key_exists($service_name, (array) $this->service_providers)) + if (!$this->service_providers->offsetExists($service_name)) { return 'LOGIN_ERROR_OAUTH_SERVICE_DOES_NOT_EXIST'; }