Fix duplicate creation of acl options in acl_add_options() under certain conditions. (Bug #38385, #40225)

Add unique key to ACL options table to prevent duplicate permission options. (Bug #41835)


git-svn-id: file:///svn/phpbb/branches/phpBB-3_0_0@9400 89ea8834-ac86-4346-8a33-228a782c2dd0

phpBB/develop/create_schema_files.php

@@ -855,7 +855,7 @@ function get_schema_struct()
 		),
 		'PRIMARY_KEY'	=> 'auth_option_id',
 		'KEYS'			=> array(
-			'auth_option'		=> array('INDEX', 'auth_option'),
+			'auth_option'		=> array('UNIQUE', 'auth_option'),
 		),
 	);
 

phpBB/docs/CHANGELOG.html

@@ -126,6 +126,7 @@
 		<li>[Fix] Use FQDN for SMTP EHLO/HELO command. (Bug #41025)</li>
 		<li>[Fix] Mass Email works again for users with empty jabber address but notification set to 'both'. (Bug #39755)</li>
 		<li>[Fix] Fix race condition for updating post/topic/etc. counter. (reported by BartVB)</li>
+		<li>[Fix] Fix duplicate creation of acl options in acl_add_options() under certain conditions. (Bug #38385, #40225)</li>
 		<li>[Change] Allow download of conflicting file for later reference in automatic updater</li>
 		<li>[Change] Default difference view is now 'inline' instead of 'side by side'</li>
 		<li>[Change] Added new option for merging differences to conflicting files in automatic updater</li>
@@ -133,6 +134,7 @@
 		<li>[Change] Add link to user profile in the MCP for user notes and warn user.</li>
 		<li>[Change] Add IN_PHPBB check to generated cache files. (reported by bantu)</li>
 		<li>[Change] Add topic icons to prosilver UCP main and subscribed templates (Bug #42735 - Patch by Raimon)</li>
+		<li>[Change] Add unique key to ACL options table to prevent duplicate permission options. (Bug #41835)</li>
 		<li>[Feature] Allow translation of custom BBCode help messages. (Patch by bantu)</li>
 		<li>[Feature] db_tools now support create table and drop table.</li>
 		<li>[Feature] Database updater checks for incompatible db schema (MySQL 3.x/4.x against MySQL 4.1.x/5.x/6.x)</li>

phpBB/includes/acp/auth.php

@@ -696,6 +696,7 @@ class auth_admin extends auth
 
 		$cur_options = array();
 
+		// Determine current options
 		$sql = 'SELECT auth_option, is_global, is_local
 			FROM ' . ACL_OPTIONS_TABLE . '
 			ORDER BY auth_option_id';
@@ -703,15 +704,7 @@ class auth_admin extends auth
 
 		while ($row = $db->sql_fetchrow($result))
 		{
-			if ($row['is_global'])
+			$cur_options[$row['auth_option']] = ($row['is_global'] && $row['is_local']) ? 'both' : (($row['is_global']) ? 'global' : 'local');
-			{
-				$cur_options['global'][] = $row['auth_option'];
-			}
-
-			if ($row['is_local'])
-			{
-				$cur_options['local'][] = $row['auth_option'];
-			}
 		}
 		$db->sql_freeresult($result);
 
@@ -726,14 +719,11 @@ class auth_admin extends auth
 
 			foreach ($option_ary as $option_value)
 			{
-				if (!in_array($option_value, $cur_options[$type]))
+				$new_options[$type][] = $option_value;
-				{
-					$new_options[$type][] = $option_value;
-				}
 
 				$flag = substr($option_value, 0, strpos($option_value, '_') + 1);
 
-				if (!in_array($flag, $cur_options[$type]) && !in_array($flag, $new_options[$type]))
+				if (!in_array($flag, $new_options[$type]))
 				{
 					$new_options[$type][] = $flag;
 				}
@@ -744,23 +734,53 @@ class auth_admin extends auth
 		$options = array();
 		$options['local'] = array_diff($new_options['local'], $new_options['global']);
 		$options['global'] = array_diff($new_options['global'], $new_options['local']);
-		$options['local_global'] = array_intersect($new_options['local'], $new_options['global']);
+		$options['both'] = array_intersect($new_options['local'], $new_options['global']);
 
-		$sql_ary = array();
+		// Now check which options to add/update
+		$add_options = $update_options = array();
 
+		// First local ones...
 		foreach ($options as $type => $option_ary)
 		{
 			foreach ($option_ary as $option)
 			{
-				$sql_ary[] = array(
+				if (!isset($cur_options[$option]))
-					'auth_option'	=> (string) $option,
+				{
-					'is_global'		=> ($type == 'global' || $type == 'local_global') ? 1 : 0,
+					$add_options[] = array(
-					'is_local'		=> ($type == 'local' || $type == 'local_global') ? 1 : 0
+						'auth_option'	=> (string) $option,
-				);
+						'is_global'		=> ($type == 'global' || $type == 'both') ? 1 : 0,
+						'is_local'		=> ($type == 'local' || $type == 'both') ? 1 : 0
+					);
+
+					continue;
+				}
+
+				// Else, update existing entry if it is changed...
+				if ($type === $cur_options[$option])
+				{
+					continue;
+				}
+
+				// New type is always both:
+				// If is now both, we set both.
+				// If it was global the new one is local and we need to set it to both
+				// If it was local the new one is global and we need to set it to both
+				$update_options[] = $option;
 			}
 		}
 
-		$db->sql_multi_insert(ACL_OPTIONS_TABLE, $sql_ary);
+		if (!empty($add_options))
+		{
+			$db->sql_multi_insert(ACL_OPTIONS_TABLE, $add_options);
+		}
+
+		if (!empty($update_options))
+		{
+			$sql = 'UPDATE ' . ACL_OPTIONS_TABLE . '
+				SET is_global = 1, is_local = 1
+				WHERE ' . $db->sql_in_set('auth_option', $update_options);
+			$db->sql_query($sql);
+		}
 
 		$cache->destroy('_acl_options');
 		$this->acl_clear_prefetch();

phpBB/install/database_update.php

@@ -894,10 +894,10 @@ function change_database_data(&$no_updates, $version)
 			set_config('captcha_gd_wave', 0);
 			set_config('captcha_gd_3d_noise', 1);
 			set_config('captcha_gd_fonts', 1);
+
 			set_config('confirm_refresh', 1);
 
-
+			// Hash old MD5 passwords
-
 			$sql = 'SELECT user_id, user_password
 					FROM ' . USERS_TABLE . '
 					WHERE user_pass_convert = 1';
@@ -916,11 +916,73 @@ function change_database_data(&$no_updates, $version)
 			}
 			$db->sql_freeresult($result);
 
+			// Adjust bot entry
 			$sql = 'UPDATE ' . BOTS_TABLE . "
 				SET bot_agent = 'ichiro/'
 				WHERE bot_agent = 'ichiro/2'";
 			_sql($sql, $errored, $error_ary);
 
+			// Before we are able to add a unique key to auth_option, we need to remove duplicate entries
+
+			// We get duplicate entries first
+			$sql = 'SELECT auth_option
+				FROM ' . ACL_OPTIONS_TABLE . '
+				GROUP BY auth_option
+				HAVING COUNT(*) >= 1';
+			$result = $db->sql_query($sql);
+
+			$auth_options = array();
+			while ($row = $db->sql_fetchrow($result))
+			{
+				$auth_options[] = $row['auth_option'];
+			}
+			$db->sql_freeresult($result);
+
+			// Remove specific auth options
+			if (!empty($auth_options))
+			{
+				foreach ($auth_options as $option)
+				{
+					// Select auth_option_ids... the largest id will be preserved
+					$sql = 'SELECT auth_option_id
+						FROM ' . ACL_OPTIONS_TABLE . "
+						WHERE auth_option = '" . $db->sql_escape($option) . "'
+						ORDER BY auth_option_id DESC";
+					$result = $db->sql_query_limit($sql, 0, 1);
+
+					while ($row = $db->sql_fetchrow($result))
+					{
+						// Ok, remove this auth option...
+						_sql('DELETE FROM ' . ACL_OPTIONS_TABLE . ' WHERE auth_option_id = ' . $row['auth_option_id'], $errored, $error_ary);
+						_sql('DELETE FROM ' . ACL_ROLES_DATA_TABLE . ' WHERE auth_option_id = ' . $row['auth_option_id'], $errored, $error_ary);
+						_sql('DELETE FROM ' . ACL_GROUPS_TABLE . ' WHERE auth_option_id = ' . $row['auth_option_id'], $errored, $error_ary);
+						_sql('DELETE FROM ' . ACL_USERS_TABLE . ' WHERE auth_option_id = ' . $row['auth_option_id'], $errored, $error_ary);
+					}
+					$db->sql_freeresult($result);
+				}
+			}
+
+			// Now make auth_option UNIQUE, by dropping the old index and adding a UNIQUE one.
+			$changes = array(
+				'drop_keys'			=> array(
+					ACL_OPTIONS_TABLE		=> array('auth_option'),
+				),
+				'add_unique_index'	=> array(
+					ACL_OPTIONS_TABLE		=> array(
+						'auth_option'		=> array('auth_option'),
+					),
+				),
+			);
+
+			global $db_tools;
+
+			$statements = $db_tools->perform_schema_changes($changes);
+
+			foreach ($statements as $sql)
+			{
+				_sql($sql, $errored, $error_ary);
+			}
+
 			$no_updates = false;
 
 		break;

phpBB/install/schemas/firebird_schema.sql

@@ -65,7 +65,7 @@ CREATE TABLE phpbb_acl_options (
 
 ALTER TABLE phpbb_acl_options ADD PRIMARY KEY (auth_option_id);;
 
-CREATE INDEX phpbb_acl_options_auth_option ON phpbb_acl_options(auth_option);;
+CREATE UNIQUE INDEX phpbb_acl_options_auth_option ON phpbb_acl_options(auth_option);;
 
 CREATE GENERATOR phpbb_acl_options_gen;;
 SET GENERATOR phpbb_acl_options_gen TO 0;;

phpBB/install/schemas/mssql_schema.sql

@@ -93,7 +93,7 @@ ALTER TABLE [phpbb_acl_options] WITH NOCHECK ADD
 	)  ON [PRIMARY]
 GO
 
-CREATE  INDEX [auth_option] ON [phpbb_acl_options]([auth_option]) ON [PRIMARY]
+CREATE  UNIQUE  INDEX [auth_option] ON [phpbb_acl_options]([auth_option]) ON [PRIMARY]
 GO
 
 

phpBB/install/schemas/mysql_40_schema.sql

@@ -49,7 +49,7 @@ CREATE TABLE phpbb_acl_options (
 	is_local tinyint(1) UNSIGNED DEFAULT '0' NOT NULL,
 	founder_only tinyint(1) UNSIGNED DEFAULT '0' NOT NULL,
 	PRIMARY KEY (auth_option_id),
-	KEY auth_option (auth_option)
+	UNIQUE auth_option (auth_option)
 );
 
 

phpBB/install/schemas/mysql_41_schema.sql

@@ -49,7 +49,7 @@ CREATE TABLE phpbb_acl_options (
 	is_local tinyint(1) UNSIGNED DEFAULT '0' NOT NULL,
 	founder_only tinyint(1) UNSIGNED DEFAULT '0' NOT NULL,
 	PRIMARY KEY (auth_option_id),
-	KEY auth_option (auth_option)
+	UNIQUE auth_option (auth_option)
 ) CHARACTER SET `utf8` COLLATE `utf8_bin`;
 
 

phpBB/install/schemas/oracle_schema.sql

@@ -119,12 +119,11 @@ CREATE TABLE phpbb_acl_options (
 	is_global number(1) DEFAULT '0' NOT NULL,
 	is_local number(1) DEFAULT '0' NOT NULL,
 	founder_only number(1) DEFAULT '0' NOT NULL,
-	CONSTRAINT pk_phpbb_acl_options PRIMARY KEY (auth_option_id)
+	CONSTRAINT pk_phpbb_acl_options PRIMARY KEY (auth_option_id),
+	CONSTRAINT u_phpbb_auth_option UNIQUE (auth_option)
 )
 /
 
-CREATE INDEX phpbb_acl_options_auth_option ON phpbb_acl_options (auth_option)
-/
 
 CREATE SEQUENCE phpbb_acl_options_seq
 /

phpBB/install/schemas/postgres_schema.sql

@@ -140,7 +140,7 @@ CREATE TABLE phpbb_acl_options (
 	PRIMARY KEY (auth_option_id)
 );
 
-CREATE INDEX phpbb_acl_options_auth_option ON phpbb_acl_options (auth_option);
+CREATE UNIQUE INDEX phpbb_acl_options_auth_option ON phpbb_acl_options (auth_option);
 
 /*
 	Table: 'phpbb_acl_roles'

phpBB/install/schemas/sqlite_schema.sql

@@ -51,7 +51,7 @@ CREATE TABLE phpbb_acl_options (
 	founder_only INTEGER UNSIGNED NOT NULL DEFAULT '0'
 );
 
-CREATE INDEX phpbb_acl_options_auth_option ON phpbb_acl_options (auth_option);
+CREATE UNIQUE INDEX phpbb_acl_options_auth_option ON phpbb_acl_options (auth_option);
 
 # Table: 'phpbb_acl_roles'
 CREATE TABLE phpbb_acl_roles (