From 51bb05f2860ed086ea8ef502eb4120c952e110dd Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 30 Oct 2019 18:08:17 +0100 Subject: [PATCH 01/12] [ticket/12574] Introduce files for LDAP testing on travis PHPBB3-12574 --- .travis.yml | 1 + travis/ldap/base.ldif | 5 +++++ travis/ldap/slapd.conf | 17 +++++++++++++++++ travis/setup-ldap.sh | 24 ++++++++++++++++++++++++ 4 files changed, 47 insertions(+) create mode 100644 travis/ldap/base.ldif create mode 100644 travis/ldap/slapd.conf create mode 100755 travis/setup-ldap.sh diff --git a/.travis.yml b/.travis.yml index 346d067240..d6fc46c585 100644 --- a/.travis.yml +++ b/.travis.yml @@ -37,6 +37,7 @@ install: before_script: - travis/setup-database.sh $DB $TRAVIS_PHP_VERSION $NOTESTS + - travis/setup-ldap.sh $SLOWTESTS - phantomjs --webdriver=8910 > /dev/null & script: diff --git a/travis/ldap/base.ldif b/travis/ldap/base.ldif new file mode 100644 index 0000000000..26535b9e72 --- /dev/null +++ b/travis/ldap/base.ldif @@ -0,0 +1,5 @@ +dn:dc=example,dc=com +objectClass:dcObject +objectClass:organizationalUnit +dc:example +ou:foo diff --git a/travis/ldap/slapd.conf b/travis/ldap/slapd.conf new file mode 100644 index 0000000000..5fce95cee2 --- /dev/null +++ b/travis/ldap/slapd.conf @@ -0,0 +1,17 @@ +# See slapd.conf(5) for details on configuration options. +include /etc/ldap/schema/core.schema +include /etc/ldap/schema/cosine.schema +include /etc/ldap/schema/inetorgperson.schema +include /etc/ldap/schema/nis.schema + +pidfile /tmp/slapd/slapd.pid +argsfile /tmp/slapd/slapd.args + +modulepath /usr/lib/openldap + +database ldif +directory /tmp/slapd + +suffix "dc=example,dc=com" +rootdn "cn=admin,dc=example,dc=com" +rootpw adminadmin diff --git a/travis/setup-ldap.sh b/travis/setup-ldap.sh new file mode 100755 index 0000000000..a8f4fddc49 --- /dev/null +++ b/travis/setup-ldap.sh @@ -0,0 +1,24 @@ +#!/bin/bash +# +# 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. +# +set -e +set -x + +SLOWTESTS=$1 + +if [ "$SLOWTESTS" == '1' ] +then + sudo apt-get -y install ldap-utils slapd + echo "extension = ldap.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini + mkdir /tmp/slapd + slapd -f travis/ldap/slapd.conf -h ldap://localhost:3389 & + sleep 3 + ldapadd -h localhost:3389 -D "cn=admin,dc=example,dc=com" -w adminadmin -f travis/ldap/base.ldif +fi From 2c3a24b678fa5070ffacd4a6bb3cfc26f648c07e Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 1 Nov 2019 11:52:44 +0100 Subject: [PATCH 02/12] [ticket/12574] Remove passwords manager dependency from ldap Also started to implement tests for ldap provider. PHPBB3-12574 --- .../default/container/services_auth.yml | 4 +- phpBB/phpbb/auth/provider/ldap.php | 53 +++-- tests/auth/provider_ldap_test.php | 197 ++++++++++++++++++ travis/ldap/base.ldif | 46 +++- 4 files changed, 272 insertions(+), 28 deletions(-) create mode 100644 tests/auth/provider_ldap_test.php diff --git a/phpBB/config/default/container/services_auth.yml b/phpBB/config/default/container/services_auth.yml index 5d8820842f..f8270400a5 100644 --- a/phpBB/config/default/container/services_auth.yml +++ b/phpBB/config/default/container/services_auth.yml @@ -42,9 +42,9 @@ services: auth.provider.ldap: class: phpbb\auth\provider\ldap arguments: - - '@dbal.conn' - '@config' - - '@passwords.manager' + - '@dbal.conn' + - '@language' - '@user' tags: - { name: auth.provider } diff --git a/phpBB/phpbb/auth/provider/ldap.php b/phpBB/phpbb/auth/provider/ldap.php index 0789a6234d..6a78136e5f 100644 --- a/phpBB/phpbb/auth/provider/ldap.php +++ b/phpBB/phpbb/auth/provider/ldap.php @@ -1,4 +1,5 @@ db = $db; $this->config = $config; - $this->passwords_manager = $passwords_manager; + $this->db = $db; + $this->language = $language; $this->user = $user; } @@ -49,7 +60,7 @@ class ldap extends \phpbb\auth\provider\base { if (!@extension_loaded('ldap')) { - return $this->user->lang['LDAP_NO_LDAP_EXTENSION']; + return $this->language->lang('LDAP_NO_LDAP_EXTENSION'); } $this->config['ldap_port'] = (int) $this->config['ldap_port']; @@ -64,7 +75,7 @@ class ldap extends \phpbb\auth\provider\base if (!$ldap) { - return $this->user->lang['LDAP_NO_SERVER_CONNECTION']; + return $this->language->lang('LDAP_NO_SERVER_CONNECTION'); } @ldap_set_option($ldap, LDAP_OPT_PROTOCOL_VERSION, 3); @@ -74,7 +85,7 @@ class ldap extends \phpbb\auth\provider\base { if (!@ldap_bind($ldap, htmlspecialchars_decode($this->config['ldap_user']), htmlspecialchars_decode($this->config['ldap_password']))) { - return $this->user->lang['LDAP_INCORRECT_USER_PASSWORD']; + return $this->language->lang('LDAP_INCORRECT_USER_PASSWORD'); } } @@ -92,7 +103,7 @@ class ldap extends \phpbb\auth\provider\base if ($search === false) { - return $this->user->lang['LDAP_SEARCH_FAILED']; + return $this->language->lang('LDAP_SEARCH_FAILED'); } $result = @ldap_get_entries($ldap, $search); @@ -101,12 +112,12 @@ class ldap extends \phpbb\auth\provider\base if (!is_array($result) || count($result) < 2) { - return sprintf($this->user->lang['LDAP_NO_IDENTITY'], $this->user->data['username']); + return $this->language->lang('LDAP_NO_IDENTITY', $this->user->data['username']); } if (!empty($this->config['ldap_email']) && !isset($result[0][htmlspecialchars_decode($this->config['ldap_email'])])) { - return $this->user->lang['LDAP_NO_EMAIL']; + return $this->language->lang('LDAP_NO_EMAIL'); } return false; @@ -245,7 +256,7 @@ class ldap extends \phpbb\auth\provider\base // generate user account data $ldap_user_row = array( 'username' => $username, - 'user_password' => $this->passwords_manager->hash($password), + 'user_password' => '', 'user_email' => (!empty($this->config['ldap_email'])) ? utf8_htmlspecialchars($ldap_result[0][htmlspecialchars_decode($this->config['ldap_email'])][0]) : '', 'group_id' => (int) $row['group_id'], 'user_type' => USER_NORMAL, diff --git a/tests/auth/provider_ldap_test.php b/tests/auth/provider_ldap_test.php new file mode 100644 index 0000000000..2c4b36ffd8 --- /dev/null +++ b/tests/auth/provider_ldap_test.php @@ -0,0 +1,197 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +class phpbb_auth_provider_ldap_test extends phpbb_database_test_case +{ + /** @var \phpbb\auth\provider\ldap */ + protected $provider; + + protected $user; + + protected function setup() + { + parent::setUp(); + + global $phpbb_root_path, $phpEx; + + $db = $this->new_dbal(); + $config = new \phpbb\config\config([ + 'ldap_server' => 'localhost', + 'ldap_port' => 3389, + 'ldap_dn' => 'dc=example,dc=com', + 'ldap_uid' => 'uid', + 'ldap_email' => 'mail', + ]); + $lang_loader = new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx); + $lang = new \phpbb\language\language($lang_loader); + $this->user = new \phpbb\user($lang, '\phpbb\datetime'); + + $this->provider = new \phpbb\auth\provider\ldap($config, $db, $lang, $this->user); + } + + public function getDataSet() + { + return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/user.xml'); + } + + /** + * Test to see if a user is identified to Apache. Expects false if they are. + */ + public function test_init() + { + $this->assertFalse($this->provider->init()); + } +/* + public function test_login() + { + $username = 'foobar'; + $password = 'example'; + + $this->request->expects($this->once()) + ->method('is_set') + ->with('PHP_AUTH_USER', + \phpbb\request\request_interface::SERVER) + ->will($this->returnValue(true)); + $this->request->expects($this->at(1)) + ->method('server') + ->with('PHP_AUTH_USER') + ->will($this->returnValue('foobar')); + $this->request->expects($this->at(2)) + ->method('server') + ->with('PHP_AUTH_PW') + ->will($this->returnValue('example')); + + $expected = array( + 'status' => LOGIN_SUCCESS, + 'error_msg' => false, + 'user_row' => array( + 'user_id' => '1', + 'username' => 'foobar', + 'user_password' => $this->password_hash, + 'user_passchg' => '0', + 'user_email' => 'example@example.com', + 'user_type' => '0', + ), + ); + + $this->assertEquals($expected, $this->provider->login($username, $password)); + } + + public function test_autologin() + { + $this->request->expects($this->once()) + ->method('is_set') + ->with('PHP_AUTH_USER', + \phpbb\request\request_interface::SERVER) + ->will($this->returnValue(true)); + $this->request->expects($this->at(1)) + ->method('server') + ->with('PHP_AUTH_USER') + ->will($this->returnValue('foobar')); + $this->request->expects($this->at(2)) + ->method('server') + ->with('PHP_AUTH_PW') + ->will($this->returnValue('example')); + + $expected = array( + 'user_id' => '1', + 'user_type' => '0', + 'group_id' => '3', + 'user_permissions' => '', + 'user_perm_from' => '0', + 'user_ip' => '', + 'user_regdate' => '0', + 'username' => 'foobar', + 'username_clean' => 'foobar', + 'user_password' => $this->password_hash, + 'user_passchg' => '0', + 'user_email' => 'example@example.com', + 'user_email_hash' => '0', + 'user_birthday' => '', + 'user_lastvisit' => '0', + 'user_lastmark' => '0', + 'user_lastpost_time' => '0', + 'user_lastpage' => '', + 'user_last_confirm_key' => '', + 'user_last_search' => '0', + 'user_warnings' => '0', + 'user_last_warning' => '0', + 'user_login_attempts' => '0', + 'user_inactive_reason' => '0', + 'user_inactive_time' => '0', + 'user_posts' => '0', + 'user_lang' => '', + 'user_timezone' => '', + 'user_dateformat' => 'd M Y H:i', + 'user_style' => '0', + 'user_rank' => '0', + 'user_colour' => '', + 'user_new_privmsg' => '0', + 'user_unread_privmsg' => '0', + 'user_last_privmsg' => '0', + 'user_message_rules' => '0', + 'user_full_folder' => '-3', + 'user_emailtime' => '0', + 'user_topic_show_days' => '0', + 'user_topic_sortby_type' => 't', + 'user_topic_sortby_dir' => 'd', + 'user_post_show_days' => '0', + 'user_post_sortby_type' => 't', + 'user_post_sortby_dir' => 'a', + 'user_notify' => '0', + 'user_notify_pm' => '1', + 'user_notify_type' => '0', + 'user_allow_pm' => '1', + 'user_allow_viewonline' => '1', + 'user_allow_viewemail' => '1', + 'user_allow_massemail' => '1', + 'user_options' => '230271', + 'user_avatar' => '', + 'user_avatar_type' => '', + 'user_avatar_width' => '0', + 'user_avatar_height' => '0', + 'user_sig' => '', + 'user_sig_bbcode_uid' => '', + 'user_sig_bbcode_bitfield' => '', + 'user_jabber' => '', + 'user_actkey' => '', + 'user_newpasswd' => '', + 'user_form_salt' => '', + 'user_new' => '1', + 'user_reminded' => '0', + 'user_reminded_time' => '0', + ); + + $this->assertEquals($expected, $this->provider->autologin()); + } + + public function test_validate_session() + { + $user = array( + 'username' => 'foobar', + 'user_type' + ); + $this->request->expects($this->once()) + ->method('is_set') + ->with('PHP_AUTH_USER', + \phpbb\request\request_interface::SERVER) + ->will($this->returnValue(true)); + $this->request->expects($this->once()) + ->method('server') + ->with('PHP_AUTH_USER') + ->will($this->returnValue('foobar')); + + $this->assertTrue($this->provider->validate_session($user)); + } +*/ +} diff --git a/travis/ldap/base.ldif b/travis/ldap/base.ldif index 26535b9e72..09fe7cecc6 100644 --- a/travis/ldap/base.ldif +++ b/travis/ldap/base.ldif @@ -1,5 +1,41 @@ -dn:dc=example,dc=com -objectClass:dcObject -objectClass:organizationalUnit -dc:example -ou:foo +dn: dc=example,dc=com +objectClass: top +objectClass: dcObject +objectClass: organization +o: example +dc: example + +dn: ou=foo,dc=example,dc=com +objectClass: organizationalUnit +ou: foo + +dn: cn=admin,dc=example,dc=com +objectClass: simpleSecurityObject +objectClass: organizationalRole +cn: admin +description: LDAP administrator +userPassword:: e1NTSEF9NytMR2gveUxTMzdsc3RRd1V1dENZSVA0TWdYdm9SdDY= + +dn: ou=group,dc=example,dc=com +objectClass: organizationalUnit +ou: group + +dn: cn=admin,ou=foo,dc=example,dc=com +objectClass: posixAccount +objectClass: inetOrgPerson +objectClass: organizationalPerson +objectClass: person +loginShell: /bin/bash +homeDirectory: /home/admin +uid: admin +cn: admin +uidNumber: 10000 +gidNumber: 10000 +sn: admin +mail: admin@example.com +userPassword:: e1NTSEF9WHpueGZURHZZc21JSkl6czdMVXBjdCtWYTA1dlMzVlQ= + +dn: cn=admin,ou=group,dc=example,dc=com +objectClass: posixGroup +gidNumber: 10000 +cn: admin From 0cd7033baad12acc2abcee38b023031322623dc4 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 1 Nov 2019 12:03:45 +0100 Subject: [PATCH 03/12] [ticket/12574] Run LDAP test as slow test and install ldap extension PHPBB3-12574 --- tests/auth/provider_ldap_test.php | 3 +++ travis/setup-ldap.sh | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/auth/provider_ldap_test.php b/tests/auth/provider_ldap_test.php index 2c4b36ffd8..2b3c377d9e 100644 --- a/tests/auth/provider_ldap_test.php +++ b/tests/auth/provider_ldap_test.php @@ -11,6 +11,9 @@ * */ +/** + * @group slow + */ class phpbb_auth_provider_ldap_test extends phpbb_database_test_case { /** @var \phpbb\auth\provider\ldap */ diff --git a/travis/setup-ldap.sh b/travis/setup-ldap.sh index a8f4fddc49..699bb9eae8 100755 --- a/travis/setup-ldap.sh +++ b/travis/setup-ldap.sh @@ -15,7 +15,7 @@ SLOWTESTS=$1 if [ "$SLOWTESTS" == '1' ] then - sudo apt-get -y install ldap-utils slapd + sudo apt-get -y install ldap-utils slapd php5-ldap echo "extension = ldap.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini mkdir /tmp/slapd slapd -f travis/ldap/slapd.conf -h ldap://localhost:3389 & From 9aee50968e0ecb8b88000815b273d0395418e1ff Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 1 Nov 2019 12:16:56 +0100 Subject: [PATCH 04/12] [ticket/12574] Don't add extension to php config & set default username PHPBB3-12574 --- tests/auth/provider_ldap_test.php | 1 + travis/setup-ldap.sh | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/auth/provider_ldap_test.php b/tests/auth/provider_ldap_test.php index 2b3c377d9e..211966875e 100644 --- a/tests/auth/provider_ldap_test.php +++ b/tests/auth/provider_ldap_test.php @@ -38,6 +38,7 @@ class phpbb_auth_provider_ldap_test extends phpbb_database_test_case $lang_loader = new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx); $lang = new \phpbb\language\language($lang_loader); $this->user = new \phpbb\user($lang, '\phpbb\datetime'); + $this->user->data['username'] = 'admin'; $this->provider = new \phpbb\auth\provider\ldap($config, $db, $lang, $this->user); } diff --git a/travis/setup-ldap.sh b/travis/setup-ldap.sh index 699bb9eae8..14b32d4048 100755 --- a/travis/setup-ldap.sh +++ b/travis/setup-ldap.sh @@ -16,7 +16,6 @@ SLOWTESTS=$1 if [ "$SLOWTESTS" == '1' ] then sudo apt-get -y install ldap-utils slapd php5-ldap - echo "extension = ldap.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini mkdir /tmp/slapd slapd -f travis/ldap/slapd.conf -h ldap://localhost:3389 & sleep 3 From c75502e09c9b997779a109b132298dd16b16e6d9 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 1 Nov 2019 12:40:52 +0100 Subject: [PATCH 05/12] [ticket/12574] Use correct config entry for connecting to ldap PHPBB3-12574 --- tests/auth/provider_ldap_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/auth/provider_ldap_test.php b/tests/auth/provider_ldap_test.php index 211966875e..5482f964bf 100644 --- a/tests/auth/provider_ldap_test.php +++ b/tests/auth/provider_ldap_test.php @@ -31,7 +31,7 @@ class phpbb_auth_provider_ldap_test extends phpbb_database_test_case $config = new \phpbb\config\config([ 'ldap_server' => 'localhost', 'ldap_port' => 3389, - 'ldap_dn' => 'dc=example,dc=com', + 'ldap_base_dn' => 'dc=example,dc=com', 'ldap_uid' => 'uid', 'ldap_email' => 'mail', ]); From 149df2d7b674e601f8f1197fe705a2a23d2a4229 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 1 Nov 2019 13:03:50 +0100 Subject: [PATCH 06/12] [ticket/12574] Adjust login tests for ldap PHPBB3-12574 --- tests/auth/fixtures/user.xml | 12 +++ tests/auth/provider_ldap_test.php | 134 ++++-------------------------- 2 files changed, 26 insertions(+), 120 deletions(-) diff --git a/tests/auth/fixtures/user.xml b/tests/auth/fixtures/user.xml index 1e0eb6ee49..33f69a9067 100644 --- a/tests/auth/fixtures/user.xml +++ b/tests/auth/fixtures/user.xml @@ -1,5 +1,17 @@ + + group_id + group_name + group_type + group_desc + + 1 + REGISTERED + 3 + foobar + +
user_idusername diff --git a/tests/auth/provider_ldap_test.php b/tests/auth/provider_ldap_test.php index 5482f964bf..7d95f476bc 100644 --- a/tests/auth/provider_ldap_test.php +++ b/tests/auth/provider_ldap_test.php @@ -55,36 +55,23 @@ class phpbb_auth_provider_ldap_test extends phpbb_database_test_case { $this->assertFalse($this->provider->init()); } -/* + public function test_login() { - $username = 'foobar'; - $password = 'example'; - - $this->request->expects($this->once()) - ->method('is_set') - ->with('PHP_AUTH_USER', - \phpbb\request\request_interface::SERVER) - ->will($this->returnValue(true)); - $this->request->expects($this->at(1)) - ->method('server') - ->with('PHP_AUTH_USER') - ->will($this->returnValue('foobar')); - $this->request->expects($this->at(2)) - ->method('server') - ->with('PHP_AUTH_PW') - ->will($this->returnValue('example')); + $username = 'admin'; + $password = 'adminadmin'; $expected = array( - 'status' => LOGIN_SUCCESS, + 'status' => LOGIN_SUCCESS_CREATE_PROFILE, // successful login and user created 'error_msg' => false, 'user_row' => array( - 'user_id' => '1', - 'username' => 'foobar', - 'user_password' => $this->password_hash, - 'user_passchg' => '0', - 'user_email' => 'example@example.com', - 'user_type' => '0', + 'username' => 'admin', + 'user_password' => '', + 'user_email' => 'admin@example.com', + 'user_type' => 0, + 'group_id' => 1, + 'user_new' => 0, + 'user_ip' => '', ), ); @@ -93,109 +80,16 @@ class phpbb_auth_provider_ldap_test extends phpbb_database_test_case public function test_autologin() { - $this->request->expects($this->once()) - ->method('is_set') - ->with('PHP_AUTH_USER', - \phpbb\request\request_interface::SERVER) - ->will($this->returnValue(true)); - $this->request->expects($this->at(1)) - ->method('server') - ->with('PHP_AUTH_USER') - ->will($this->returnValue('foobar')); - $this->request->expects($this->at(2)) - ->method('server') - ->with('PHP_AUTH_PW') - ->will($this->returnValue('example')); - - $expected = array( - 'user_id' => '1', - 'user_type' => '0', - 'group_id' => '3', - 'user_permissions' => '', - 'user_perm_from' => '0', - 'user_ip' => '', - 'user_regdate' => '0', - 'username' => 'foobar', - 'username_clean' => 'foobar', - 'user_password' => $this->password_hash, - 'user_passchg' => '0', - 'user_email' => 'example@example.com', - 'user_email_hash' => '0', - 'user_birthday' => '', - 'user_lastvisit' => '0', - 'user_lastmark' => '0', - 'user_lastpost_time' => '0', - 'user_lastpage' => '', - 'user_last_confirm_key' => '', - 'user_last_search' => '0', - 'user_warnings' => '0', - 'user_last_warning' => '0', - 'user_login_attempts' => '0', - 'user_inactive_reason' => '0', - 'user_inactive_time' => '0', - 'user_posts' => '0', - 'user_lang' => '', - 'user_timezone' => '', - 'user_dateformat' => 'd M Y H:i', - 'user_style' => '0', - 'user_rank' => '0', - 'user_colour' => '', - 'user_new_privmsg' => '0', - 'user_unread_privmsg' => '0', - 'user_last_privmsg' => '0', - 'user_message_rules' => '0', - 'user_full_folder' => '-3', - 'user_emailtime' => '0', - 'user_topic_show_days' => '0', - 'user_topic_sortby_type' => 't', - 'user_topic_sortby_dir' => 'd', - 'user_post_show_days' => '0', - 'user_post_sortby_type' => 't', - 'user_post_sortby_dir' => 'a', - 'user_notify' => '0', - 'user_notify_pm' => '1', - 'user_notify_type' => '0', - 'user_allow_pm' => '1', - 'user_allow_viewonline' => '1', - 'user_allow_viewemail' => '1', - 'user_allow_massemail' => '1', - 'user_options' => '230271', - 'user_avatar' => '', - 'user_avatar_type' => '', - 'user_avatar_width' => '0', - 'user_avatar_height' => '0', - 'user_sig' => '', - 'user_sig_bbcode_uid' => '', - 'user_sig_bbcode_bitfield' => '', - 'user_jabber' => '', - 'user_actkey' => '', - 'user_newpasswd' => '', - 'user_form_salt' => '', - 'user_new' => '1', - 'user_reminded' => '0', - 'user_reminded_time' => '0', - ); - - $this->assertEquals($expected, $this->provider->autologin()); + $this->assertNull($this->provider->autologin()); } public function test_validate_session() { $user = array( - 'username' => 'foobar', + 'username' => 'admin', 'user_type' ); - $this->request->expects($this->once()) - ->method('is_set') - ->with('PHP_AUTH_USER', - \phpbb\request\request_interface::SERVER) - ->will($this->returnValue(true)); - $this->request->expects($this->once()) - ->method('server') - ->with('PHP_AUTH_USER') - ->will($this->returnValue('foobar')); - $this->assertTrue($this->provider->validate_session($user)); + $this->assertNull($this->provider->validate_session($user)); } -*/ } From 0d668fee3686d1df8a0415572cebb01025419cd6 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 1 Nov 2019 13:38:09 +0100 Subject: [PATCH 07/12] [ticket/12574] Fix incorrect setup() method declaration & ldap extension PHPBB3-12574 --- tests/auth/provider_ldap_test.php | 2 +- travis/setup-ldap.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/auth/provider_ldap_test.php b/tests/auth/provider_ldap_test.php index 7d95f476bc..63cd63eee2 100644 --- a/tests/auth/provider_ldap_test.php +++ b/tests/auth/provider_ldap_test.php @@ -21,7 +21,7 @@ class phpbb_auth_provider_ldap_test extends phpbb_database_test_case protected $user; - protected function setup() + protected function setup() : void { parent::setUp(); diff --git a/travis/setup-ldap.sh b/travis/setup-ldap.sh index 14b32d4048..9be816d77d 100755 --- a/travis/setup-ldap.sh +++ b/travis/setup-ldap.sh @@ -15,7 +15,7 @@ SLOWTESTS=$1 if [ "$SLOWTESTS" == '1' ] then - sudo apt-get -y install ldap-utils slapd php5-ldap + sudo apt-get -y install ldap-utils slapd php-ldap mkdir /tmp/slapd slapd -f travis/ldap/slapd.conf -h ldap://localhost:3389 & sleep 3 From 9e0c3fc81ec0c3006acc5aa3074cedb98f11e680 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 1 Nov 2019 14:30:27 +0100 Subject: [PATCH 08/12] [ticket/12574] Stop using passwords manager in apache provider PHPBB3-12574 --- .../default/container/services_auth.yml | 4 +- phpBB/phpbb/auth/provider/apache.php | 75 ++++++++++++------- tests/auth/provider_apache_test.php | 37 ++------- 3 files changed, 55 insertions(+), 61 deletions(-) diff --git a/phpBB/config/default/container/services_auth.yml b/phpBB/config/default/container/services_auth.yml index f8270400a5..dfe3ab8be5 100644 --- a/phpBB/config/default/container/services_auth.yml +++ b/phpBB/config/default/container/services_auth.yml @@ -29,9 +29,9 @@ services: auth.provider.apache: class: phpbb\auth\provider\apache arguments: - - '@dbal.conn' - '@config' - - '@passwords.manager' + - '@dbal.conn' + - '@language' - '@request' - '@user' - '%core.root_path%' diff --git a/phpBB/phpbb/auth/provider/apache.php b/phpBB/phpbb/auth/provider/apache.php index aa5bf64335..a713674657 100644 --- a/phpBB/phpbb/auth/provider/apache.php +++ b/phpBB/phpbb/auth/provider/apache.php @@ -13,34 +13,55 @@ namespace phpbb\auth\provider; +use phpbb\config\config; +use phpbb\db\driver\driver_interface; +use phpbb\language\language; +use phpbb\request\request_interface; +use phpbb\request\type_cast_helper; +use phpbb\user; + /** * Apache authentication provider for phpBB3 */ -class apache extends \phpbb\auth\provider\base +class apache extends base { - /** - * phpBB passwords manager - * - * @var \phpbb\passwords\manager - */ - protected $passwords_manager; + /** @var config phpBB config */ + protected $config; + + /** @var driver_interface Database object */ + protected $db; + + /** @var language Language object */ + protected $language; + + /** @var request_interface Request object */ + protected $request; + + /** @var user User object */ + protected $user; + + /** @var string Relative path to phpBB root */ + protected $phpbb_root_path; + + /** @var string PHP file extension */ + protected $php_ext; /** * Apache Authentication Constructor * - * @param \phpbb\db\driver\driver_interface $db Database object - * @param \phpbb\config\config $config Config object - * @param \phpbb\passwords\manager $passwords_manager Passwords Manager object - * @param \phpbb\request\request $request Request object - * @param \phpbb\user $user User object + * @param config $config Config object + * @param driver_interface $db Database object + * @param language $language Language object + * @param request_interface $request Request object + * @param user $user User object * @param string $phpbb_root_path Relative path to phpBB root * @param string $php_ext PHP file extension */ - public function __construct(\phpbb\db\driver\driver_interface $db, \phpbb\config\config $config, \phpbb\passwords\manager $passwords_manager, \phpbb\request\request $request, \phpbb\user $user, $phpbb_root_path, $php_ext) + public function __construct(config $config, driver_interface $db, language $language, request_interface $request, user $user, $phpbb_root_path, $php_ext) { - $this->db = $db; $this->config = $config; - $this->passwords_manager = $passwords_manager; + $this->db = $db; + $this->language = $language; $this->request = $request; $this->user = $user; $this->phpbb_root_path = $phpbb_root_path; @@ -52,9 +73,9 @@ class apache extends \phpbb\auth\provider\base */ public function init() { - if (!$this->request->is_set('PHP_AUTH_USER', \phpbb\request\request_interface::SERVER) || $this->user->data['username'] !== htmlspecialchars_decode($this->request->server('PHP_AUTH_USER'))) + if (!$this->request->is_set('PHP_AUTH_USER', request_interface::SERVER) || $this->user->data['username'] !== htmlspecialchars_decode($this->request->server('PHP_AUTH_USER'))) { - return $this->user->lang['APACHE_SETUP_BEFORE_USE']; + return $this->language->lang('APACHE_SETUP_BEFORE_USE'); } return false; } @@ -83,7 +104,7 @@ class apache extends \phpbb\auth\provider\base ); } - if (!$this->request->is_set('PHP_AUTH_USER', \phpbb\request\request_interface::SERVER)) + if (!$this->request->is_set('PHP_AUTH_USER', request_interface::SERVER)) { return array( 'status' => LOGIN_ERROR_EXTERNAL_AUTH, @@ -137,7 +158,7 @@ class apache extends \phpbb\auth\provider\base return array( 'status' => LOGIN_SUCCESS_CREATE_PROFILE, 'error_msg' => false, - 'user_row' => $this->user_row($php_auth_user, $php_auth_pw), + 'user_row' => $this->user_row($php_auth_user), ); } @@ -154,7 +175,7 @@ class apache extends \phpbb\auth\provider\base */ public function autologin() { - if (!$this->request->is_set('PHP_AUTH_USER', \phpbb\request\request_interface::SERVER)) + if (!$this->request->is_set('PHP_AUTH_USER', request_interface::SERVER)) { return array(); } @@ -164,8 +185,8 @@ class apache extends \phpbb\auth\provider\base if (!empty($php_auth_user) && !empty($php_auth_pw)) { - set_var($php_auth_user, $php_auth_user, 'string', true); - set_var($php_auth_pw, $php_auth_pw, 'string', true); + $type_cast_helper = new type_cast_helper(); + $type_cast_helper->set_var($php_auth_user, $php_auth_user, 'string', true); $sql = 'SELECT * FROM ' . USERS_TABLE . " @@ -185,7 +206,7 @@ class apache extends \phpbb\auth\provider\base } // create the user if he does not exist yet - user_add($this->user_row($php_auth_user, $php_auth_pw)); + user_add($this->user_row($php_auth_user)); $sql = 'SELECT * FROM ' . USERS_TABLE . " @@ -208,11 +229,11 @@ class apache extends \phpbb\auth\provider\base * function in order to create a user * * @param string $username The username of the new user. - * @param string $password The password of the new user. + * * @return array Contains data that can be passed directly to * the user_add function. */ - private function user_row($username, $password) + private function user_row($username) { // first retrieve default group id $sql = 'SELECT group_id @@ -231,7 +252,7 @@ class apache extends \phpbb\auth\provider\base // generate user account data return array( 'username' => $username, - 'user_password' => $this->passwords_manager->hash($password), + 'user_password' => '', 'user_email' => '', 'group_id' => (int) $row['group_id'], 'user_type' => USER_NORMAL, @@ -246,7 +267,7 @@ class apache extends \phpbb\auth\provider\base public function validate_session($user) { // Check if PHP_AUTH_USER is set and handle this case - if ($this->request->is_set('PHP_AUTH_USER', \phpbb\request\request_interface::SERVER)) + if ($this->request->is_set('PHP_AUTH_USER', request_interface::SERVER)) { $php_auth_user = $this->request->server('PHP_AUTH_USER'); diff --git a/tests/auth/provider_apache_test.php b/tests/auth/provider_apache_test.php index 58d6354228..b1c84d47b6 100644 --- a/tests/auth/provider_apache_test.php +++ b/tests/auth/provider_apache_test.php @@ -28,41 +28,14 @@ class phpbb_auth_provider_apache_test extends phpbb_database_test_case $lang_loader = new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx); $lang = new \phpbb\language\language($lang_loader); $this->request = $this->createMock('\phpbb\request\request'); - $this->user = new \phpbb\user($lang, '\phpbb\datetime'); - $driver_helper = new \phpbb\passwords\driver\helper($config); - $passwords_drivers = array( - 'passwords.driver.bcrypt_2y' => new \phpbb\passwords\driver\bcrypt_2y($config, $driver_helper), - 'passwords.driver.bcrypt' => new \phpbb\passwords\driver\bcrypt($config, $driver_helper), - 'passwords.driver.salted_md5' => new \phpbb\passwords\driver\salted_md5($config, $driver_helper), - 'passwords.driver.phpass' => new \phpbb\passwords\driver\phpass($config, $driver_helper), - ); + $this->user = new \phpbb\user($lang, '\phpbb\datetime');; - $passwords_helper = new \phpbb\passwords\helper; - // Set up passwords manager - $passwords_manager = new \phpbb\passwords\manager($config, $passwords_drivers, $passwords_helper, array_keys($passwords_drivers)); - - if (version_compare(PHP_VERSION, '5.3.7', '<')) - { - $this->password_hash = '$2a$10$e01Syh9PbJjUkio66eFuUu4FhCE2nRgG7QPc1JACalsPXcIuG2bbi'; - } - else - { - $this->password_hash = '$2y$10$4RmpyVu2y8Yf/lP3.yQBquKvE54TCUuEDEBJYY6FDDFN3LcbCGz9i'; - } - - $this->provider = new \phpbb\auth\provider\apache($db, $config, $passwords_manager, $this->request, $this->user, $phpbb_root_path, $phpEx); + $this->provider = new \phpbb\auth\provider\apache($config, $db, $lang, $this->request, $this->user, $phpbb_root_path, $phpEx); } public function getDataSet() { - if ((version_compare(PHP_VERSION, '5.3.7', '<'))) - { - return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/user_533.xml'); - } - else - { - return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/user.xml'); - } + return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/user.xml'); } /** @@ -109,7 +82,7 @@ class phpbb_auth_provider_apache_test extends phpbb_database_test_case 'user_row' => array( 'user_id' => '1', 'username' => 'foobar', - 'user_password' => $this->password_hash, + 'user_password' => '$2y$10$4RmpyVu2y8Yf/lP3.yQBquKvE54TCUuEDEBJYY6FDDFN3LcbCGz9i', 'user_passchg' => '0', 'user_email' => 'example@example.com', 'user_type' => '0', @@ -145,7 +118,7 @@ class phpbb_auth_provider_apache_test extends phpbb_database_test_case 'user_regdate' => '0', 'username' => 'foobar', 'username_clean' => 'foobar', - 'user_password' => $this->password_hash, + 'user_password' => '$2y$10$4RmpyVu2y8Yf/lP3.yQBquKvE54TCUuEDEBJYY6FDDFN3LcbCGz9i', 'user_passchg' => '0', 'user_email' => 'example@example.com', 'user_email_hash' => '0', From a00b8c29204e4517ada908cdc0bc6eacf93d7305 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 1 Nov 2019 14:37:27 +0100 Subject: [PATCH 09/12] [ticket/12574] Clean up dependencies of db auth provider PHPBB3-12574 --- .../default/container/services_auth.yml | 4 +- phpBB/phpbb/auth/provider/db.php | 61 ++++++++++++------- tests/auth/provider_db_test.php | 14 ++++- 3 files changed, 55 insertions(+), 24 deletions(-) diff --git a/phpBB/config/default/container/services_auth.yml b/phpBB/config/default/container/services_auth.yml index dfe3ab8be5..bace1bb6cd 100644 --- a/phpBB/config/default/container/services_auth.yml +++ b/phpBB/config/default/container/services_auth.yml @@ -15,12 +15,12 @@ services: auth.provider.db: class: phpbb\auth\provider\db arguments: - - '@dbal.conn' + - '@captcha.factory' - '@config' + - '@dbal.conn' - '@passwords.manager' - '@request' - '@user' - - '@service_container' - '%core.root_path%' - '%core.php_ext%' tags: diff --git a/phpBB/phpbb/auth/provider/db.php b/phpBB/phpbb/auth/provider/db.php index 1adf85ee05..a70734fcbe 100644 --- a/phpBB/phpbb/auth/provider/db.php +++ b/phpBB/phpbb/auth/provider/db.php @@ -13,48 +13,69 @@ namespace phpbb\auth\provider; +use phpbb\captcha\factory; +use phpbb\config\config; +use phpbb\db\driver\driver_interface; +use phpbb\passwords\manager; +use phpbb\request\request_interface; +use phpbb\user; + /** * Database authentication provider for phpBB3 * This is for authentication via the integrated user table */ -class db extends \phpbb\auth\provider\base +class db extends base { + /** @var factory CAPTCHA factory */ + protected $captcha_factory; + + /** @var config phpBB config */ + protected $config; + + /** @var driver_interface DBAL driver instance */ + protected $db; + + /** @var request_interface Request object */ + protected $request; + + /** @var user User object */ + protected $user; + + /** @var string phpBB root path */ + protected $phpbb_root_path; + + /** @var string PHP file extension */ + protected $php_ext; + /** * phpBB passwords manager * - * @var \phpbb\passwords\manager + * @var manager */ protected $passwords_manager; - /** - * DI container - * - * @var \Symfony\Component\DependencyInjection\ContainerInterface - */ - protected $phpbb_container; - /** * Database Authentication Constructor * - * @param \phpbb\db\driver\driver_interface $db - * @param \phpbb\config\config $config - * @param \phpbb\passwords\manager $passwords_manager - * @param \phpbb\request\request $request - * @param \phpbb\user $user - * @param \Symfony\Component\DependencyInjection\ContainerInterface $phpbb_container DI container + * @param factory $captcha_factory + * @param config $config + * @param driver_interface $db + * @param manager $passwords_manager + * @param request_interface $request + * @param user $user * @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 $request, \phpbb\user $user, \Symfony\Component\DependencyInjection\ContainerInterface $phpbb_container, $phpbb_root_path, $php_ext) + public function __construct(factory $captcha_factory, config $config, driver_interface $db, manager $passwords_manager, request_interface $request, user $user, $phpbb_root_path, $php_ext) { - $this->db = $db; + $this->captcha_factory = $captcha_factory; $this->config = $config; + $this->db = $db; $this->passwords_manager = $passwords_manager; $this->request = $request; $this->user = $user; $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; - $this->phpbb_container = $phpbb_container; } /** @@ -155,9 +176,7 @@ class db extends \phpbb\auth\provider\base // Every auth module is able to define what to do by itself... if ($show_captcha) { - /* @var $captcha_factory \phpbb\captcha\factory */ - $captcha_factory = $this->phpbb_container->get('captcha.factory'); - $captcha = $captcha_factory->get_instance($this->config['captcha_plugin']); + $captcha = $this->captcha_factory->get_instance($this->config['captcha_plugin']); $captcha->init(CONFIRM_LOGIN); $vc_response = $captcha->validate($row); if ($vc_response) diff --git a/tests/auth/provider_db_test.php b/tests/auth/provider_db_test.php index b7d94ed046..2c467518f5 100644 --- a/tests/auth/provider_db_test.php +++ b/tests/auth/provider_db_test.php @@ -52,8 +52,20 @@ class phpbb_auth_provider_db_test extends phpbb_database_test_case $passwords_manager = new \phpbb\passwords\manager($config, $passwords_drivers, $passwords_helper, array_keys($passwords_drivers)); $phpbb_container = new phpbb_mock_container_builder(); + $plugins = new \phpbb\di\service_collection($phpbb_container); + $plugins->add('core.captcha.plugins.nogd'); + $phpbb_container->set( + 'captcha.factory', + new \phpbb\captcha\factory($phpbb_container, $plugins) + ); + $phpbb_container->set( + 'core.captcha.plugins.nogd', + new \phpbb\captcha\plugins\nogd() + ); + /** @var \phpbb\captcha\factory $captcha_factory */ + $captcha_factory = $phpbb_container->get('captcha.factory'); - $provider = new \phpbb\auth\provider\db($db, $config, $passwords_manager, $request, $user, $phpbb_container, $phpbb_root_path, $phpEx); + $provider = new \phpbb\auth\provider\db($captcha_factory, $config, $db, $passwords_manager, $request, $user, $phpbb_root_path, $phpEx); if (version_compare(PHP_VERSION, '5.3.7', '<')) { $password_hash = '$2a$10$e01Syh9PbJjUkio66eFuUu4FhCE2nRgG7QPc1JACalsPXcIuG2bbi'; From 0cbe05faadfade06737f3f057fb1b9aa44a78693 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 1 Nov 2019 16:15:25 +0100 Subject: [PATCH 10/12] [ticket/12574] Remove special cases for PHP < 5.3.7 PHPBB3-12574 --- tests/auth/fixtures/user_533.xml | 39 -------- tests/auth/provider_db_test.php | 21 +---- tests/passwords/manager_test.php | 154 ++++++++++--------------------- 3 files changed, 50 insertions(+), 164 deletions(-) delete mode 100644 tests/auth/fixtures/user_533.xml diff --git a/tests/auth/fixtures/user_533.xml b/tests/auth/fixtures/user_533.xml deleted file mode 100644 index 9731e4db4a..0000000000 --- a/tests/auth/fixtures/user_533.xml +++ /dev/null @@ -1,39 +0,0 @@ - - -
- user_id - username - username_clean - user_password - user_passchg - user_email - user_type - user_login_attempts - user_permissions - user_sig - - 1 - foobar - foobar - $2a$10$e01Syh9PbJjUkio66eFuUu4FhCE2nRgG7QPc1JACalsPXcIuG2bbi - 0 - example@example.com - 0 - 0 - - - - - 2 - foobar2 - foobar2 - $H$9E45lK6J8nLTSm9oJE5aNCSTFK9wqa/ - 0 - example@example.com - 0 - 0 - - - -
- diff --git a/tests/auth/provider_db_test.php b/tests/auth/provider_db_test.php index 2c467518f5..8305e7caa4 100644 --- a/tests/auth/provider_db_test.php +++ b/tests/auth/provider_db_test.php @@ -15,14 +15,7 @@ class phpbb_auth_provider_db_test extends phpbb_database_test_case { public function getDataSet() { - if ((version_compare(PHP_VERSION, '5.3.7', '<'))) - { - return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/user_533.xml'); - } - else - { - return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/user.xml'); - } + return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/user.xml'); } public function test_login() @@ -66,14 +59,7 @@ class phpbb_auth_provider_db_test extends phpbb_database_test_case $captcha_factory = $phpbb_container->get('captcha.factory'); $provider = new \phpbb\auth\provider\db($captcha_factory, $config, $db, $passwords_manager, $request, $user, $phpbb_root_path, $phpEx); - if (version_compare(PHP_VERSION, '5.3.7', '<')) - { - $password_hash = '$2a$10$e01Syh9PbJjUkio66eFuUu4FhCE2nRgG7QPc1JACalsPXcIuG2bbi'; - } - else - { - $password_hash = '$2y$10$4RmpyVu2y8Yf/lP3.yQBquKvE54TCUuEDEBJYY6FDDFN3LcbCGz9i'; - } + $password_hash = '$2y$10$4RmpyVu2y8Yf/lP3.yQBquKvE54TCUuEDEBJYY6FDDFN3LcbCGz9i'; $expected = array( 'status' => LOGIN_SUCCESS, @@ -100,7 +86,6 @@ class phpbb_auth_provider_db_test extends phpbb_database_test_case // Check if convert works $login_return = $provider->login('foobar2', 'example'); - $password_start = (version_compare(PHP_VERSION, '5.3.7', '<')) ? '$2a$10$' : '$2y$10$'; - $this->assertStringStartsWith($password_start, $login_return['user_row']['user_password']); + $this->assertStringStartsWith('$2y$10$', $login_return['user_row']['user_password']); } } diff --git a/tests/passwords/manager_test.php b/tests/passwords/manager_test.php index dc5c539316..90dbb47f46 100644 --- a/tests/passwords/manager_test.php +++ b/tests/passwords/manager_test.php @@ -51,26 +51,13 @@ class phpbb_passwords_manager_test extends \phpbb_test_case public function hash_password_data() { - if (version_compare(PHP_VERSION, '5.3.7', '<')) - { - return array( - array('', '2a', 60), - array('passwords.driver.bcrypt_2y', '2a', 60), - array('passwords.driver.bcrypt', '2a', 60), - array('passwords.driver.salted_md5', 'H', 34), - array('passwords.driver.foobar', '', false), - ); - } - else - { - return array( - array('', '2y', 60), - array('passwords.driver.bcrypt_2y', '2y', 60), - array('passwords.driver.bcrypt', '2a', 60), - array('passwords.driver.salted_md5', 'H', 34), - array('passwords.driver.foobar', '', false), - ); - } + return array( + array('', '2y', 60), + array('passwords.driver.bcrypt_2y', '2y', 60), + array('passwords.driver.bcrypt', '2a', 60), + array('passwords.driver.salted_md5', 'H', 34), + array('passwords.driver.foobar', '', false), + ); } /** @@ -100,23 +87,12 @@ class phpbb_passwords_manager_test extends \phpbb_test_case public function check_password_data() { - if (version_compare(PHP_VERSION, '5.3.7', '<')) - { - return array( - array('passwords.driver.bcrypt'), - array('passwords.driver.salted_md5'), - array('passwords.driver.phpass'), - ); - } - else - { - return array( - array('passwords.driver.bcrypt_2y'), - array('passwords.driver.bcrypt'), - array('passwords.driver.salted_md5'), - array('passwords.driver.phpass'), - ); - } + return array( + array('passwords.driver.bcrypt_2y'), + array('passwords.driver.bcrypt'), + array('passwords.driver.salted_md5'), + array('passwords.driver.phpass'), + ); } /** @@ -136,7 +112,7 @@ class phpbb_passwords_manager_test extends \phpbb_test_case } // Check if convert_flag is correctly set - $default_type = (version_compare(PHP_VERSION, '5.3.7', '<')) ? 'passwords.driver.bcrypt' : 'passwords.driver.bcrypt_2y'; + $default_type = 'passwords.driver.bcrypt_2y'; $this->assertEquals(($hash_type !== $default_type), $this->manager->convert_flag); } @@ -200,79 +176,43 @@ class phpbb_passwords_manager_test extends \phpbb_test_case public function test_hash_password_8bit_bcrypt() { $this->assertEquals(false, $this->manager->hash('foobarš¯„˛', 'passwords.driver.bcrypt')); - if (version_compare(PHP_VERSION, '5.3.7', '<')) - { - $this->assertEquals(false, $this->manager->hash('foobarš¯„˛', 'passwords.driver.bcrypt_2y')); - } - else - { - $this->assertNotEquals(false, $this->manager->hash('foobarš¯„˛', 'passwords.driver.bcrypt_2y')); - } + $this->assertNotEquals(false, $this->manager->hash('foobarš¯„˛', 'passwords.driver.bcrypt_2y')); } public function combined_hash_data() { - if (version_compare(PHP_VERSION, '5.3.7', '<')) - { - return array( - array( - 'passwords.driver.salted_md5', - array('passwords.driver.bcrypt'), - ), - array( - 'passwords.driver.phpass', - array('passwords.driver.salted_md5'), - ), - array( - 'passwords.driver.salted_md5', - array('passwords.driver.phpass', 'passwords.driver.bcrypt'), - ), - array( - 'passwords.driver.salted_md5', - array('passwords.driver.salted_md5'), - false, - ), - array( - '$H$', - array('$2a$'), - ), - ); - } - else - { - return array( - array( - 'passwords.driver.salted_md5', - array('passwords.driver.bcrypt_2y'), - ), - array( - 'passwords.driver.salted_md5', - array('passwords.driver.bcrypt'), - ), - array( - 'passwords.driver.phpass', - array('passwords.driver.salted_md5'), - ), - array( - 'passwords.driver.salted_md5', - array('passwords.driver.bcrypt_2y', 'passwords.driver.bcrypt'), - ), - array( - 'passwords.driver.salted_md5', - array('passwords.driver.salted_md5'), - false, - ), - array( - 'passwords.driver.bcrypt_2y', - array('passwords.driver.salted_md4'), - false, - ), - array( - '$H$', - array('$2y$'), - ), - ); - } + return array( + array( + 'passwords.driver.salted_md5', + array('passwords.driver.bcrypt_2y'), + ), + array( + 'passwords.driver.salted_md5', + array('passwords.driver.bcrypt'), + ), + array( + 'passwords.driver.phpass', + array('passwords.driver.salted_md5'), + ), + array( + 'passwords.driver.salted_md5', + array('passwords.driver.bcrypt_2y', 'passwords.driver.bcrypt'), + ), + array( + 'passwords.driver.salted_md5', + array('passwords.driver.salted_md5'), + false, + ), + array( + 'passwords.driver.bcrypt_2y', + array('passwords.driver.salted_md4'), + false, + ), + array( + '$H$', + array('$2y$'), + ), + ); } /** From f460194379ece85a0f53e737dbe736a1bdfe359c Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 16 Nov 2019 23:05:36 +0100 Subject: [PATCH 11/12] [ticket/12574] Remove passwords manager & container requirement from oauth PHPBB3-12574 --- .../default/container/services_auth.yml | 3 +- phpBB/phpbb/auth/provider/oauth/oauth.php | 92 +++++++++---------- tests/functions/user_delete_test.php | 25 ++++- 3 files changed, 66 insertions(+), 54 deletions(-) diff --git a/phpBB/config/default/container/services_auth.yml b/phpBB/config/default/container/services_auth.yml index bace1bb6cd..1540bea97f 100644 --- a/phpBB/config/default/container/services_auth.yml +++ b/phpBB/config/default/container/services_auth.yml @@ -53,11 +53,10 @@ services: class: phpbb\auth\provider\oauth\oauth arguments: - '@config' - - '@service_container' - '@dbal.conn' + - '@auth.provider.db' - '@dispatcher' - '@language' - - '@passwords.manager' - '@request' - '@auth.provider.oauth.service_collection' - '@user' diff --git a/phpBB/phpbb/auth/provider/oauth/oauth.php b/phpBB/phpbb/auth/provider/oauth/oauth.php index fe82663799..29ffe6d591 100644 --- a/phpBB/phpbb/auth/provider/oauth/oauth.php +++ b/phpBB/phpbb/auth/provider/oauth/oauth.php @@ -13,44 +13,50 @@ namespace phpbb\auth\provider\oauth; +use OAuth\Common\Http\Exception\TokenResponseException; 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\base; +use phpbb\auth\provider\db; use phpbb\auth\provider\oauth\service\exception; +use phpbb\config\config; +use phpbb\db\driver\driver_interface; +use phpbb\di\service_collection; +use phpbb\event\dispatcher; +use phpbb\language\language; +use phpbb\request\request_interface; +use phpbb\user; /** * OAuth authentication provider for phpBB3 */ -class oauth extends \phpbb\auth\provider\base +class oauth extends base { - /** @var \phpbb\config\config */ + /** @var config */ protected $config; - /** @var ContainerInterface */ - protected $container; - - /** @var \phpbb\db\driver\driver_interface */ + /** @var driver_interface */ protected $db; - /** @var \phpbb\event\dispatcher */ + /** @var db */ + protected $db_auth; + + /** @var dispatcher */ protected $dispatcher; - /** @var \phpbb\language\language */ + /** @var language */ protected $language; - /** @var \phpbb\passwords\manager */ - protected $passwords_manager; - - /** @var \phpbb\request\request_interface */ + /** @var request_interface */ protected $request; - /** @var \phpbb\di\service_collection */ + /** @var service_collection */ protected $service_providers; - /** @var \phpbb\user */ + /** @var user */ protected $user; /** @var string OAuth table: token storage */ @@ -74,15 +80,14 @@ class oauth extends \phpbb\auth\provider\base /** * 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 config $config Config object + * @param driver_interface $db Database object + * @param db $db_auth DB auth provider + * @param dispatcher $dispatcher Event dispatcher object + * @param language $language Language object + * @param request_interface $request Request object + * @param service_collection $service_providers OAuth providers service collection + * @param 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 @@ -91,15 +96,14 @@ class oauth extends \phpbb\auth\provider\base * @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, + config $config, + driver_interface $db, + db $db_auth, + dispatcher $dispatcher, + language $language, + request_interface $request, + service_collection $service_providers, + user $user, $oauth_token_table, $oauth_state_table, $oauth_account_table, @@ -109,10 +113,9 @@ class oauth extends \phpbb\auth\provider\base ) { $this->config = $config; - $this->container = $container; $this->db = $db; + $this->db_auth = $db_auth; $this->dispatcher = $dispatcher; - $this->passwords_manager = $passwords_manager; $this->language = $language; $this->service_providers = $service_providers; $this->request = $request; @@ -153,18 +156,7 @@ class oauth extends \phpbb\auth\provider\base // 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->container, - $this->root_path, - $this->php_ext - ); - - return $provider->login($username, $password); + return $this->db_auth->login($username, $password); } // Request the name of the OAuth service @@ -822,10 +814,10 @@ class oauth extends \phpbb\auth\provider\base switch ($service::OAUTH_VERSION) { case 1: - return $this->request->is_set('oauth_token', \phpbb\request\request_interface::GET); + return $this->request->is_set('oauth_token', request_interface::GET); case 2: - return $this->request->is_set('code', \phpbb\request\request_interface::GET); + return $this->request->is_set('code', request_interface::GET); default: return false; @@ -850,7 +842,7 @@ class oauth extends \phpbb\auth\provider\base $token = $service->requestRequestToken(); $parameters = ['oauth_token' => $token->getRequestToken()]; } - catch (\OAuth\Common\Http\Exception\TokenResponseException $e) + catch (TokenResponseException $e) { return [ 'status' => LOGIN_ERROR_EXTERNAL_AUTH, diff --git a/tests/functions/user_delete_test.php b/tests/functions/user_delete_test.php index 83fda05542..f4ea5696b9 100644 --- a/tests/functions/user_delete_test.php +++ b/tests/functions/user_delete_test.php @@ -60,13 +60,34 @@ class phpbb_functions_user_delete_test extends phpbb_database_test_case // Set up passwords manager $passwords_manager = new \phpbb\passwords\manager($config, $passwords_drivers, $passwords_helper, array_keys($passwords_drivers)); + $plugins = new \phpbb\di\service_collection($phpbb_container); + $plugins->add('core.captcha.plugins.nogd'); + $phpbb_container->set( + 'captcha.factory', + new \phpbb\captcha\factory($phpbb_container, $plugins) + ); + $phpbb_container->set( + 'core.captcha.plugins.nogd', + new \phpbb\captcha\plugins\nogd() + ); + // Set up passwords manager + $db_auth_provider = new \phpbb\auth\provider\db( + new \phpbb\captcha\factory($phpbb_container, $plugins), + $config, + $db, + $passwords_manager, + $request, + $user, + $phpbb_root_path, + $phpEx + ); + $oauth_provider = new \phpbb\auth\provider\oauth\oauth( $config, - $phpbb_container, $db, + $db_auth_provider, $phpbb_dispatcher, $lang, - $passwords_manager, $request, $oauth_provider_collection, $user, From c11dbffbac8789d9532234ed88a1e47026632363 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 17 Nov 2019 09:42:27 +0100 Subject: [PATCH 12/12] [ticket/12574] Remove not used user_type PHPBB3-12574 --- tests/auth/provider_ldap_test.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/auth/provider_ldap_test.php b/tests/auth/provider_ldap_test.php index 63cd63eee2..0bc9961f52 100644 --- a/tests/auth/provider_ldap_test.php +++ b/tests/auth/provider_ldap_test.php @@ -87,7 +87,6 @@ class phpbb_auth_provider_ldap_test extends phpbb_database_test_case { $user = array( 'username' => 'admin', - 'user_type' ); $this->assertNull($this->provider->validate_session($user));