Merge remote-tracking branch 'p/ticket/10758' into develop

* p/ticket/10758:
  [ticket/10758] Add return to the other compat function.
  [ticket/10758] Add periods.
  [ticket/10758] Yes, only one empty line.
  [ticket/10758] Add deprecated tags.
  [ticket/10758] Add compat functions.
  [ticket/10758] Admin is not working yet.
  [ticket/10758] Test moderator and admin permissions.
  [ticket/10758] Check that acl was changed in the test.
  [ticket/10758] Extract obtain_user_data for the benefit of tests.
  [ticket/10758] Functional test for changing a user's permission.
  [ticket/10758] Spelling fix.
  [ticket/10758] Add a test for acp login.
  [ticket/10758] Dependency inject parameters into update_foes.
  [ticket/10758] Dependency inject parameters into cache_moderators.

phpBB/includes/acp/acp_forums.php

@@ -206,7 +206,7 @@ class acp_forums
 							($action != 'edit' || empty($forum_id) || ($auth->acl_get('a_fauth') && $auth->acl_get('a_authusers') && $auth->acl_get('a_authgroups') && $auth->acl_get('a_mauth'))))
 						{
 							copy_forum_permissions($forum_perm_from, $forum_data['forum_id'], ($action == 'edit') ? true : false);
-							cache_moderators();
+							phpbb_cache_moderators($db, $cache, $auth);
 							$copied_permissions = true;
 						}
 /* Commented out because of questionable UI workflow - re-visit for 3.0.7
@@ -266,7 +266,7 @@ class acp_forums
 					add_log('admin', 'LOG_FORUM_' . strtoupper($action), $row['forum_name'], $move_forum_name);
 					$cache->destroy('sql', FORUMS_TABLE);
 				}
-				
+
 				if ($request->is_ajax())
 				{
 					$json_response = new phpbb_json_response;
@@ -768,7 +768,7 @@ class acp_forums
 				if (!empty($forum_perm_from) && $forum_perm_from != $forum_id)
 				{
 					copy_forum_permissions($forum_perm_from, $forum_id, true);
-					cache_moderators();
+					phpbb_cache_moderators($db, $cache, $auth);
 					$auth->acl_clear_prefetch();
 					$cache->destroy('sql', FORUMS_TABLE);
 

phpBB/includes/acp/acp_main.php

@@ -24,7 +24,7 @@ class acp_main
 
 	function main($id, $mode)
 	{
-		global $config, $db, $user, $auth, $template, $request;
+		global $config, $db, $cache, $user, $auth, $template, $request;
 		global $phpbb_root_path, $phpbb_admin_path, $phpEx;
 
 		// Show restore permissions notice
@@ -129,7 +129,7 @@ class acp_main
 						set_config('record_online_users', 1, true);
 						set_config('record_online_date', time(), true);
 						add_log('admin', 'LOG_RESET_ONLINE');
-						
+
 						if ($request->is_ajax())
 						{
 							trigger_error('RESET_ONLINE_SUCCESS');
@@ -184,7 +184,7 @@ class acp_main
 						update_last_username();
 
 						add_log('admin', 'LOG_RESYNC_STATS');
-						
+
 						if ($request->is_ajax())
 						{
 							trigger_error('RESYNC_STATS_SUCCESS');
@@ -251,7 +251,7 @@ class acp_main
 						}
 
 						add_log('admin', 'LOG_RESYNC_POSTCOUNTS');
-						
+
 						if ($request->is_ajax())
 						{
 							trigger_error('RESYNC_POSTCOUNTS_SUCCESS');
@@ -266,7 +266,7 @@ class acp_main
 
 						set_config('board_startdate', time() - 1);
 						add_log('admin', 'LOG_RESET_DATE');
-						
+
 						if ($request->is_ajax())
 						{
 							trigger_error('RESET_DATE_SUCCESS');
@@ -346,7 +346,7 @@ class acp_main
 						}
 
 						add_log('admin', 'LOG_RESYNC_POST_MARKING');
-						
+
 						if ($request->is_ajax())
 						{
 							trigger_error('RESYNC_POST_MARKING_SUCCESS');
@@ -359,10 +359,10 @@ class acp_main
 
 						// Clear permissions
 						$auth->acl_clear_prefetch();
-						cache_moderators();
+						phpbb_cache_moderators($db, $cache, $auth);
 
 						add_log('admin', 'LOG_PURGE_CACHE');
-						
+
 						if ($request->is_ajax())
 						{
 							trigger_error('PURGE_CACHE_SUCCESS');
@@ -413,7 +413,7 @@ class acp_main
 						$db->sql_query($sql);
 
 						add_log('admin', 'LOG_PURGE_SESSIONS');
-						
+
 						if ($request->is_ajax())
 						{
 							trigger_error('PURGE_SESSIONS_SUCCESS');

phpBB/includes/acp/acp_permissions.php

@@ -656,7 +656,7 @@ class acp_permissions
 	*/
 	function set_permissions($mode, $permission_type, &$auth_admin, &$user_id, &$group_id)
 	{
-		global $user, $auth;
+		global $db, $cache, $user, $auth;
 		global $request;
 
 		$psubmit = request_var('psubmit', array(0 => array(0 => 0)));
@@ -726,13 +726,13 @@ class acp_permissions
 		// Do we need to recache the moderator lists?
 		if ($permission_type == 'm_')
 		{
-			cache_moderators();
+			phpbb_cache_moderators($db, $cache, $auth);
 		}
 
 		// Remove users who are now moderators or admins from everyones foes list
 		if ($permission_type == 'm_' || $permission_type == 'a_')
 		{
-			update_foes($group_id, $user_id);
+			phpbb_update_foes($db, $auth, $group_id, $user_id);
 		}
 
 		$this->log_action($mode, 'add', $permission_type, $ug_type, $ug_id, $forum_id);
@@ -745,7 +745,7 @@ class acp_permissions
 	*/
 	function set_all_permissions($mode, $permission_type, &$auth_admin, &$user_id, &$group_id)
 	{
-		global $user, $auth;
+		global $db, $cache, $user, $auth;
 		global $request;
 
 		// User or group to be set?
@@ -794,13 +794,13 @@ class acp_permissions
 		// Do we need to recache the moderator lists?
 		if ($permission_type == 'm_')
 		{
-			cache_moderators();
+			phpbb_cache_moderators($db, $cache, $auth);
 		}
 
 		// Remove users who are now moderators or admins from everyones foes list
 		if ($permission_type == 'm_' || $permission_type == 'a_')
 		{
-			update_foes($group_id, $user_id);
+			phpbb_update_foes($db, $auth, $group_id, $user_id);
 		}
 
 		$this->log_action($mode, 'add', $permission_type, $ug_type, $ug_ids, $forum_ids);
@@ -858,7 +858,7 @@ class acp_permissions
 	*/
 	function remove_permissions($mode, $permission_type, &$auth_admin, &$user_id, &$group_id, &$forum_id)
 	{
-		global $user, $db, $auth;
+		global $user, $db, $cache, $auth;
 
 		// User or group to be set?
 		$ug_type = (sizeof($user_id)) ? 'user' : 'group';
@@ -874,7 +874,7 @@ class acp_permissions
 		// Do we need to recache the moderator lists?
 		if ($permission_type == 'm_')
 		{
-			cache_moderators();
+			phpbb_cache_moderators($db, $cache, $auth);
 		}
 
 		$this->log_action($mode, 'del', $permission_type, $ug_type, (($ug_type == 'user') ? $user_id : $group_id), (sizeof($forum_id) ? $forum_id : array(0 => 0)));
@@ -952,12 +952,7 @@ class acp_permissions
 
 		if ($user_id != $user->data['user_id'])
 		{
-			$sql = 'SELECT user_id, username, user_permissions, user_type
+			$userdata = $auth->obtain_user_data($user_id);
-				FROM ' . USERS_TABLE . '
-				WHERE user_id = ' . $user_id;
-			$result = $db->sql_query($sql);
-			$userdata = $db->sql_fetchrow($result);
-			$db->sql_freeresult($result);
 		}
 		else
 		{
@@ -1172,7 +1167,7 @@ class acp_permissions
 	*/
 	function copy_forum_permissions()
 	{
-		global $auth, $cache, $template, $user;
+		global $db, $auth, $cache, $template, $user;
 
 		$user->add_lang('acp/forums');
 
@@ -1187,7 +1182,7 @@ class acp_permissions
 			{
 				if (copy_forum_permissions($src, $dest))
 				{
-					cache_moderators();
+					phpbb_cache_moderators($db, $cache, $auth);
 
 					$auth->acl_clear_prefetch();
 					$cache->destroy('sql', FORUMS_TABLE);

phpBB/includes/acp/acp_styles.php

@@ -137,11 +137,13 @@ class acp_styles
 	*/
 	protected function action_cache()
 	{
+		global $db, $cache, $auth;
+
 		$this->cache->purge();
 
 		// Clear permissions
 		$this->auth->acl_clear_prefetch();
-		cache_moderators();
+		phpbb_cache_moderators($db, $cache, $auth);
 
 		add_log('admin', 'LOG_PURGE_CACHE');
 

phpBB/includes/auth/auth.php

@@ -102,6 +102,26 @@ class phpbb_auth
 		return;
 	}
 
+	/**
+	* Retrieves data wanted by acl function from the database for the
+	* specified user.
+	*
+	* @param int $user_id User ID
+	* @return array User attributes
+	*/
+	public function obtain_user_data($user_id)
+	{
+		global $db;
+
+		$sql = 'SELECT user_id, username, user_permissions, user_type
+			FROM ' . USERS_TABLE . '
+			WHERE user_id = ' . $user_id;
+		$result = $db->sql_query($sql);
+		$user_data = $db->sql_fetchrow($result);
+		$db->sql_freeresult($result);
+		return $user_data;
+	}
+
 	/**
 	* Fill ACL array with relevant bitstrings from user_permissions column
 	* @access private
@@ -191,7 +211,7 @@ class phpbb_auth
 
 	/**
 	* Get forums with the specified permission setting
-	* if the option is prefixed with !, then the result becomes nagated
+	* if the option is prefixed with !, then the result becomes negated
 	*
 	* @param bool $clean set to true if only values needs to be returned which are set/unset
 	*/

phpBB/includes/functions_admin.php

@@ -2292,13 +2292,17 @@ function auto_prune($forum_id, $prune_mode, $prune_flags, $prune_days, $prune_fr
 }
 
 /**
-* Cache moderators, called whenever permissions are changed via admin_permissions. Changes of username
+* Cache moderators. Called whenever permissions are changed
-* and group names must be carried through for the moderators table
+* via admin_permissions. Changes of usernames and group names
+* must be carried through for the moderators table.
+*
+* @param phpbb_db_driver $db Database connection
+* @param phpbb_cache_driver_interface Cache driver
+* @param phpbb_auth $auth Authentication object
+* @return null
 */
-function cache_moderators()
+function phpbb_cache_moderators($db, $cache, $auth)
 {
-	global $db, $cache, $auth, $phpbb_root_path, $phpEx;
-
 	// Remove cached sql results
 	$cache->destroy('sql', MODERATOR_CACHE_TABLE);
 
@@ -2468,6 +2472,20 @@ function cache_moderators()
 	$db->sql_multi_insert(MODERATOR_CACHE_TABLE, $sql_ary);
 }
 
+/**
+* Cache moderators. Called whenever permissions are changed
+* via admin_permissions. Changes of usernames and group names
+* must be carried through for the moderators table.
+*
+* @deprecated 3.1
+* @return null
+*/
+function cache_moderators()
+{
+	global $db, $cache, $auth;
+	return phpbb_cache_moderators($db, $cache, $auth);
+}
+
 /**
 * View log
 * If $log_count is set to false, we will skip counting all entries in the database.
@@ -2740,12 +2758,16 @@ function view_log($mode, &$log, &$log_count, $limit = 0, $offset = 0, $forum_id
 }
 
 /**
-* Update foes - remove moderators and administrators from foe lists...
+* Removes moderators and administrators from foe lists.
+*
+* @param phpbb_db_driver $db Database connection
+* @param phpbb_auth $auth Authentication object
+* @param array|bool $group_id If an array, remove all members of this group from foe lists, or false to ignore
+* @param array|bool $user_id If an array, remove this user from foe lists, or false to ignore
+* @return null
 */
-function update_foes($group_id = false, $user_id = false)
+function phpbb_update_foes($db, $auth, $group_id = false, $user_id = false)
 {
-	global $db, $auth;
-
 	// update foes for some user
 	if (is_array($user_id) && sizeof($user_id))
 	{
@@ -2854,6 +2876,20 @@ function update_foes($group_id = false, $user_id = false)
 	unset($perms);
 }
 
+/**
+* Removes moderators and administrators from foe lists.
+*
+* @deprecated 3.1
+* @param array|bool $group_id If an array, remove all members of this group from foe lists, or false to ignore
+* @param array|bool $user_id If an array, remove this user from foe lists, or false to ignore
+* @return null
+*/
+function update_foes($group_id = false, $user_id = false)
+{
+	global $db, $auth;
+	return phpbb_update_foes($db, $auth, $group_id, $user_id);
+}
+
 /**
 * Lists inactive users
 */

phpBB/includes/functions_user.php

@@ -2842,7 +2842,7 @@ function avatar_remove_db($avatar_name)
 */
 function group_delete($group_id, $group_name = false)
 {
-	global $db, $phpbb_root_path, $phpEx, $phpbb_dispatcher;
+	global $db, $cache, $auth, $phpbb_root_path, $phpEx, $phpbb_dispatcher;
 
 	if (!$group_name)
 	{
@@ -2913,12 +2913,12 @@ function group_delete($group_id, $group_name = false)
 	extract($phpbb_dispatcher->trigger_event('core.delete_group_after', compact($vars)));
 
 	// Re-cache moderators
-	if (!function_exists('cache_moderators'))
+	if (!function_exists('phpbb_cache_moderators'))
 	{
 		include($phpbb_root_path . 'includes/functions_admin.' . $phpEx);
 	}
 
-	cache_moderators();
+	phpbb_cache_moderators($db, $cache, $auth);
 
 	add_log('admin', 'LOG_GROUP_DELETE', $group_name);
 
@@ -3678,7 +3678,7 @@ function group_memberships($group_id_ary = false, $user_id_ary = false, $return_
 */
 function group_update_listings($group_id)
 {
-	global $auth;
+	global $db, $cache, $auth;
 
 	$hold_ary = $auth->acl_group_raw_data($group_id, array('a_', 'm_'));
 
@@ -3720,22 +3720,22 @@ function group_update_listings($group_id)
 
 	if ($mod_permissions)
 	{
-		if (!function_exists('cache_moderators'))
+		if (!function_exists('phpbb_cache_moderators'))
 		{
 			global $phpbb_root_path, $phpEx;
 			include($phpbb_root_path . 'includes/functions_admin.' . $phpEx);
 		}
-		cache_moderators();
+		phpbb_cache_moderators($db, $cache, $auth);
 	}
 
 	if ($mod_permissions || $admin_permissions)
 	{
-		if (!function_exists('update_foes'))
+		if (!function_exists('phpbb_update_foes'))
 		{
 			global $phpbb_root_path, $phpEx;
 			include($phpbb_root_path . 'includes/functions_admin.' . $phpEx);
 		}
-		update_foes(array($group_id));
+		phpbb_update_foes($db, $auth, array($group_id));
 	}
 }
 

phpBB/install/install_convert.php

@@ -1538,6 +1538,7 @@ class install_convert extends module
 	function finish_conversion()
 	{
 		global $db, $phpbb_root_path, $phpEx, $convert, $config, $language, $user, $template;
+		global $cache, $auth;
 
 		$db->sql_query('DELETE FROM ' . CONFIG_TABLE . "
 			WHERE config_name = 'convert_progress'
@@ -1547,7 +1548,7 @@ class install_convert extends module
 		$db->sql_query('DELETE FROM ' . SESSIONS_TABLE);
 
 		@unlink($phpbb_root_path . 'cache/data_global.' . $phpEx);
-		cache_moderators();
+		phpbb_cache_moderators($db, $cache, $auth);
 
 		// And finally, add a note to the log
 		add_log('admin', 'LOG_INSTALL_CONVERTED', $convert->convertor_data['forum_name'], $config['version']);

tests/functional/acp_permissions_test.php

@@ -0,0 +1,128 @@
+<?php
+/**
+*
+* @package testing
+* @copyright (c) 2012 phpBB Group
+* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
+*
+*/
+
+/**
+* @group functional
+*/
+class phpbb_functional_acp_permissions_test extends phpbb_functional_test_case
+{
+	public function setUp()
+	{
+		parent::setUp();
+
+		$this->login();
+		$this->admin_login();
+		$this->add_lang('acp/permissions');
+	}
+
+	public function test_permissions_tab()
+	{
+		// Permissions tab
+		// XXX hardcoded id
+		$crawler = $this->request('GET', 'adm/index.php?i=16&sid=' . $this->sid);
+		$this->assert_response_success();
+		// these language strings are html
+		$this->assertContains($this->lang('ACP_PERMISSIONS_EXPLAIN'), $this->client->getResponse()->getContent());
+	}
+
+	public function test_select_user()
+	{
+		// User permissions
+		$crawler = $this->request('GET', 'adm/index.php?i=acp_permissions&icat=16&mode=setting_user_global&sid=' . $this->sid);
+		$this->assert_response_success();
+		$this->assertContains($this->lang('ACP_USERS_PERMISSIONS_EXPLAIN'), $this->client->getResponse()->getContent());
+
+		// Select admin
+		$form = $crawler->selectButton($this->lang('SUBMIT'))->form();
+		$data = array('username[0]' => 'admin');
+		$form->setValues($data);
+		$crawler = $this->client->submit($form);
+		$this->assert_response_success();
+		$this->assertContains($this->lang('ACL_SET'), $crawler->filter('h1')->eq(1)->text());
+	}
+
+	public function permissions_data()
+	{
+		return array(
+			// description
+			// permission type
+			// permission name
+			// mode
+			// object name
+			// object id
+			array(
+				'user permission',
+				'u_',
+				'u_hideonline',
+				'setting_user_global',
+				'user_id',
+				2,
+			),
+			array(
+				'moderator permission',
+				'm_',
+				'm_ban',
+				'setting_mod_global',
+				'group_id',
+				4,
+			),
+			/* Admin does not work yet, probably because founder can do everything
+			array(
+				'admin permission',
+				'a_',
+				'a_forum',
+				'setting_admin_global',
+				'group_id',
+				5,
+			),
+			*/
+		);
+	}
+
+	/**
+	* @dataProvider permissions_data
+	*/
+	public function test_change_permission($description, $permission_type, $permission, $mode, $object_name, $object_id)
+	{
+		// Get the form
+		$crawler = $this->request('GET', "adm/index.php?i=acp_permissions&icat=16&mode=$mode&${object_name}[0]=$object_id&type=$permission_type&sid=" . $this->sid);
+		$this->assert_response_success();
+		$this->assertContains($this->lang('ACL_SET'), $crawler->filter('h1')->eq(1)->text());
+
+		// XXX globals for phpbb_auth, refactor it later
+		global $db, $cache;
+		$db = $this->get_db();
+		$cache = new phpbb_mock_null_cache;
+
+		$auth = new phpbb_auth;
+		// XXX hardcoded id
+		$user_data = $auth->obtain_user_data(2);
+		$auth->acl($user_data);
+		$this->assertEquals(1, $auth->acl_get($permission));
+
+		// Set u_hideonline to never
+		$form = $crawler->selectButton($this->lang('APPLY_PERMISSIONS'))->form();
+		// initially it should be a yes
+		$values = $form->getValues();
+		$this->assertEquals(1, $values["setting[$object_id][0][$permission]"]);
+		// set to never
+		$data = array("setting[$object_id][0][$permission]" => '0');
+		$form->setValues($data);
+		$crawler = $this->client->submit($form);
+		$this->assert_response_success();
+		$this->assertContains($this->lang('AUTH_UPDATED'), $crawler->text());
+
+		// check acl again
+		$auth = new phpbb_auth;
+		// XXX hardcoded id
+		$user_data = $auth->obtain_user_data(2);
+		$auth->acl($user_data);
+		$this->assertEquals(0, $auth->acl_get($permission));
+	}
+}

tests/functional/auth_test.php

@@ -49,4 +49,15 @@ class phpbb_functional_auth_test extends phpbb_functional_test_case
 		$this->assert_response_success();
 		$this->assertContains($this->lang('REGISTER'), $crawler->filter('.navbar')->text());
 	}
+
+	public function test_acp_login()
+	{
+		$this->login();
+		$this->admin_login();
+
+		// check that we are logged in
+		$crawler = $this->request('GET', 'adm/index.php?sid=' . $this->sid);
+		$this->assert_response_success();
+		$this->assertContains($this->lang('ADMIN_PANEL'), $crawler->filter('h1')->text());
+	}
 }

tests/test_framework/phpbb_functional_test_case.php

@@ -323,7 +323,7 @@ class phpbb_functional_test_case extends phpbb_test_case
 	* Login to the ACP
 	* You must run login() before calling this.
 	*/
-	protected function admin_login()
+	protected function admin_login($username = 'admin')
 	{
 		$this->add_lang('acp/common');
 
@@ -343,7 +343,9 @@ class phpbb_functional_test_case extends phpbb_test_case
 		{
 			if (strpos($field, 'password_') === 0)
 			{
-				$login = $this->client->submit($form, array('username' => 'admin', $field => 'admin'));
+				$crawler = $this->client->submit($form, array('username' => $username, $field => $username));
+				$this->assert_response_success();
+				$this->assertContains($this->lang('LOGIN_ADMIN_SUCCESS'), $crawler->filter('html')->text());
 
 				$cookies = $this->cookieJar->all();