[permissionmanager] Ensure uniqueness in role_x_location table, consistent table naming, drop unused id field

4 files changed, 47 insertions, 27 deletions

diff --git a/modules-available/permissionmanager/inc/getpermissiondata.inc.php b/modules-available/permissionmanager/inc/getpermissiondata.inc.php
index 496c8224..fc18de99 100644
--- a/modules-available/permissionmanager/inc/getpermissiondata.inc.php
+++ b/modules-available/permissionmanager/inc/getpermissiondata.inc.php
@@ -15,8 +15,8 @@ class GetPermissionData
 	{
 		$res = Database::simpleQuery("SELECT user.userid AS userid, user.login AS login, role.rolename AS rolename, role.roleid AS roleid
 												FROM user
-												LEFT JOIN user_x_role ON user.userid = user_x_role.userid
-												LEFT JOIN role ON user_x_role.roleid = role.roleid
+												LEFT JOIN role_x_user ON user.userid = role_x_user.userid
+												LEFT JOIN role ON role_x_user.roleid = role.roleid
 												");
 		$userdata = array();
 		while ($row = $res->fetch(PDO::FETCH_ASSOC)) {
@@ -75,7 +75,7 @@ class GetPermissionData
 		$cols = $joins = '';
 		if ($flags & self::WITH_USER_COUNT) {
 			$cols .= ', Count(DISTINCT rxu.userid) AS users';
-			$joins .= ' LEFT JOIN user_x_role rxu ON (r.roleid = rxu.roleid)';
+			$joins .= ' LEFT JOIN role_x_user rxu ON (r.roleid = rxu.roleid)';
 		}
 		if ($flags & self::WITH_LOCATION_COUNT) {
 			$cols .= ', Count(DISTINCT rxl.locationid) AS locations';
diff --git a/modules-available/permissionmanager/inc/permissiondbupdate.inc.php b/modules-available/permissionmanager/inc/permissiondbupdate.inc.php
index 8a67bf24..1f56f4ea 100644
--- a/modules-available/permissionmanager/inc/permissiondbupdate.inc.php
+++ b/modules-available/permissionmanager/inc/permissiondbupdate.inc.php
@@ -4,10 +4,10 @@ class PermissionDbUpdate
 {
 
 	/**
-	 * Insert all user/role combinations into the user_x_role table.
+	 * Insert all user/role combinations into the role_x_user table.
 	 *
-	 * @param array $users userids
-	 * @param array $roles roleids
+	 * @param int[] $users userids
+	 * @param string[] $roles roleids
 	 */
 	public static function addRoleToUser($users, $roles)
 	{
@@ -17,19 +17,19 @@ class PermissionDbUpdate
 				$arg[] = compact('userid', 'roleid');
 			}
 		}
-		Database::exec("INSERT IGNORE INTO user_x_role (userid, roleid) VALUES :arg",
+		Database::exec("INSERT IGNORE INTO role_x_user (userid, roleid) VALUES :arg",
 			['arg' => $arg]);
 	}
 
 	/**
-	 * Remove all user/role combinations from the user_x_role table.
+	 * Remove all user/role combinations from the role_x_user table.
 	 *
-	 * @param array $users userids
-	 * @param array $roles roleids
+	 * @param int[] $users userids
+	 * @param string[] $roles roleids
 	 */
 	public static function removeRoleFromUser($users, $roles)
 	{
-		$query = "DELETE FROM user_x_role WHERE userid IN (:users) AND roleid IN (:roles)";
+		$query = "DELETE FROM role_x_user WHERE userid IN (:users) AND roleid IN (:roles)";
 		Database::exec($query, array("users" => $users, "roles" => $roles));
 	}
 
@@ -47,8 +47,8 @@ class PermissionDbUpdate
 	 * Save changes to a role or create a new one.
 	 *
 	 * @param string $rolename rolename
-	 * @param array $locations array of locations
-	 * @param array $permissions array of permissions
+	 * @param int[] $locations array of locations
+	 * @param string[] $permissions array of permissions
 	 * @param string|null $roleid roleid or null if the role does not exist yet
 	 */
 	public static function saveRole($rolename, $locations, $permissions, $roleid = null)
@@ -61,7 +61,7 @@ class PermissionDbUpdate
 			Database::exec("UPDATE role SET rolename = :rolename WHERE roleid = :roleid",
 				array("rolename" => $rolename, "roleid" => $roleid));
 			Database::exec("DELETE FROM role_x_location
-					WHERE roleid = :roleid AND locationid NOT IN (:locations)",
+					WHERE roleid = :roleid AND (locationid NOT IN (:locations) OR locationid IS NULL)",
 				array("roleid" => $roleid, 'locations' => $locations));
 			Database::exec("DELETE FROM role_x_permission
 					WHERE roleid = :roleid AND permissionid NOT IN (:permissions)",
diff --git a/modules-available/permissionmanager/inc/permissionutil.inc.php b/modules-available/permissionmanager/inc/permissionutil.inc.php
index bc42c5a0..29663ed9 100644
--- a/modules-available/permissionmanager/inc/permissionutil.inc.php
+++ b/modules-available/permissionmanager/inc/permissionutil.inc.php
@@ -60,8 +60,8 @@ class PermissionUtil
 		$prefix = $parts[0] . '.%';
 		if (is_null($locationid)) {
 			$res = Database::simpleQuery("SELECT permissionid FROM role_x_permission
-					INNER JOIN user_x_role USING (roleid)
-					WHERE user_x_role.userid = :userid AND (permissionid LIKE :prefix OR permissionid LIKE '*')",
+					INNER JOIN role_x_user USING (roleid)
+					WHERE role_x_user.userid = :userid AND (permissionid LIKE :prefix OR permissionid LIKE '*')",
 				compact('userid', 'prefix'));
 		} else {
 			if ($locationid === 0) {
@@ -73,9 +73,9 @@ class PermissionUtil
 				}
 			}
 			$res = Database::simpleQuery("SELECT permissionid FROM role_x_permission
-					INNER JOIN user_x_role USING (roleid)
+					INNER JOIN role_x_user USING (roleid)
 					INNER JOIN role_x_location USING (roleid)
-					WHERE user_x_role.userid = :userid AND (permissionid LIKE :prefix OR permissionid LIKE '*')
+					WHERE role_x_user.userid = :userid AND (permissionid LIKE :prefix OR permissionid LIKE '*')
 						AND (locationid IN (:locations) OR locationid IS NULL)",
 				compact('userid', 'prefix', 'locations'));
 		}
@@ -113,9 +113,9 @@ class PermissionUtil
 			// Limit query to first part of permissionid, which is always the module id
 			$prefix = $parts[0] . '.%';
 			$res = Database::simpleQuery("SELECT permissionid, locationid FROM role_x_permission
-						INNER JOIN user_x_role USING (roleid)
+						INNER JOIN role_x_user USING (roleid)
 						INNER JOIN role_x_location USING (roleid)
-						WHERE user_x_role.userid = :userid AND (permissionid LIKE :prefix OR permissionid LIKE '*')",
+						WHERE role_x_user.userid = :userid AND (permissionid LIKE :prefix OR permissionid LIKE '*')",
 				compact('userid', 'prefix'));
 
 			// Gather locationid from relevant rows
diff --git a/modules-available/permissionmanager/install.inc.php b/modules-available/permissionmanager/install.inc.php
index 71ee7a1e..afa5dd7e 100644
--- a/modules-available/permissionmanager/install.inc.php
+++ b/modules-available/permissionmanager/install.inc.php
@@ -8,17 +8,23 @@ $res[] = tableCreate('role', "
 	PRIMARY KEY (roleid)
 ");
 
-$res[] = tableCreate('user_x_role', "
+if (tableExists('user_x_role')) {
+	if (tableExists('role_x_user')) {
+		Database::exec('DROP TABLE user_x_role');
+	} else {
+		$res[] = tableRename('user_x_role', 'role_x_user');
+	}
+}
+$res[] = tableCreate('role_x_user', "
 	userid int(10) unsigned NOT NULL,
 	roleid int(10) unsigned NOT NULL,
 	PRIMARY KEY (userid, roleid)
 ");
 
 $res[] = tableCreate('role_x_location', "
-	id int(10) unsigned NOT NULL AUTO_INCREMENT,
 	roleid int(10) unsigned NOT NULL,
 	locationid int(11),
-	PRIMARY KEY (id)
+	CONSTRAINT role_loc UNIQUE (roleid, locationid) 
 ");
 
 $res[] = tableCreate('role_x_permission', "
@@ -27,24 +33,38 @@ $res[] = tableCreate('role_x_permission', "
 	PRIMARY KEY (roleid, permissionid)
 ");
 
+if (tableHasColumn('role_x_location', 'id')) {
+	$cnt = Database::exec('DELETE a FROM role_x_location a, role_x_location b
+			WHERE a.roleid = b.roleid AND (a.locationid = b.locationid OR (a.locationid IS NULL AND b.locationid IS NULL))
+			AND a.id > b.id');
+	$ret = Database::exec('ALTER TABLE role_x_location DROP COLUMN id,
+			ADD CONSTRAINT role_loc UNIQUE (roleid, locationid)');
+	if ($ret === false) {
+		$res[] = UPDATE_NOOP;
+	} else {
+		$res[] = UPDATE_DONE;
+	}
+
+}
+
 if (!tableExists('user') || !tableExists('location')) {
 	finalResponse(UPDATE_RETRY, 'Cannot add constraint yet. Please retry.');
 } else {
-	$c = tableGetContraints('user_x_role', 'userid', 'user', 'userid');
+	$c = tableGetContraints('role_x_user', 'userid', 'user', 'userid');
 	if ($c === false)
 		finalResponse(UPDATE_FAILED, 'Cannot get constraints of user table: ' . Database::lastError());
 	if (empty($c)) {
-		$alter = Database::exec('ALTER TABLE user_x_role ADD FOREIGN KEY (userid) REFERENCES user (userid) ON DELETE CASCADE ON UPDATE CASCADE');
+		$alter = Database::exec('ALTER TABLE role_x_user ADD FOREIGN KEY (userid) REFERENCES user (userid) ON DELETE CASCADE ON UPDATE CASCADE');
 		if ($alter === false)
 			finalResponse(UPDATE_FAILED, 'Cannot add userid constraint referencing user table: ' . Database::lastError());
 		$res[] = UPDATE_DONE;
 	}
 
-	$c = tableGetContraints('user_x_role', 'roleid', 'role', 'roleid');
+	$c = tableGetContraints('role_x_user', 'roleid', 'role', 'roleid');
 	if ($c === false)
 		finalResponse(UPDATE_FAILED, 'Cannot get constraints of role table: ' . Database::lastError());
 	if (empty($c)) {
-		$alter = Database::exec('ALTER TABLE user_x_role ADD FOREIGN KEY (roleid) REFERENCES role (roleid) ON DELETE CASCADE ON UPDATE CASCADE');
+		$alter = Database::exec('ALTER TABLE role_x_user ADD FOREIGN KEY (roleid) REFERENCES role (roleid) ON DELETE CASCADE ON UPDATE CASCADE');
 		if ($alter === false)
 			finalResponse(UPDATE_FAILED, 'Cannot add roleid constraint referencing role table: ' . Database::lastError());
 		$res[] = UPDATE_DONE;