From 7dc7a49e3704b52c4a40909050bf831826b3c41b Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Tue, 20 Feb 2018 15:20:26 +0100 Subject: [permissionmanager] Ensure uniqueness in role_x_location table, consistent table naming, drop unused id field --- .../inc/getpermissiondata.inc.php | 6 ++-- .../inc/permissiondbupdate.inc.php | 22 +++++++------- .../permissionmanager/inc/permissionutil.inc.php | 12 ++++---- .../permissionmanager/install.inc.php | 34 +++++++++++++++++----- 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; -- cgit v1.2.3-55-g7522