[ticket/12574] Remove passwords manager dependency from ldap

Also started to implement tests for ldap provider. PHPBB3-12574
2025-06-07 20:08:53 +00:00 · 2019-11-01 11:52:44 +01:00 · 2019-11-01 11:52:44 +01:00 · 2c3a24b678
commit 2c3a24b678
parent 51bb05f286
4 changed files with 272 additions and 28 deletions
--- 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 }
--- a/phpBB/phpbb/auth/provider/ldap.php
+++ b/phpBB/phpbb/auth/provider/ldap.php
@ -1,4 +1,5 @@
 <?php
+
 /**
 *
 * This file is part of the phpBB Forum Software package.
@ -13,32 +14,42 @@

 namespace phpbb\auth\provider;

+use phpbb\config\config;
+use phpbb\db\driver\driver_interface;
+use phpbb\language\language;
+use phpbb\user;
+
 /**
 * Database authentication provider for phpBB3
 * This is for authentication via the integrated user table
 */
-class ldap extends \phpbb\auth\provider\base
+class ldap extends base
 {
-	/**
-	* phpBB passwords manager
-	*
-	* @var \phpbb\passwords\manager
-	*/
-	protected $passwords_manager;
+	/** @var config phpBB config */
+	protected $config;
+
+	/** @var driver_interface DBAL driver interface */
+	protected $db;
+
+	/** @var language phpBB language class */
+	protected $language;
+
+	/** @var user phpBB user */
+	protected $user;

 	/**
 	 * LDAP 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\user			$user		User object
+	 * @param	driver_interface	$db			DBAL driver interface
+	 * @param	config				$config		Config object
+	 * @param	language			$language	Language object
+	 * @param	user				$user		User object
 	 */
-	public function __construct(\phpbb\db\driver\driver_interface $db, \phpbb\config\config $config, \phpbb\passwords\manager $passwords_manager, \phpbb\user $user)
+	public function __construct(config $config, driver_interface $db, language $language, user $user)
 	{
-		$this->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,
--- a/tests/auth/provider_ldap_test.php
+++ b/tests/auth/provider_ldap_test.php
@ -0,0 +1,197 @@
+<?php
+/**
+ *
+ * This file is part of the phpBB Forum Software package.
+ *
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @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));
+	}
+*/
+}
--- 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