1
0
mirror of https://github.com/chylex/Lightning-Tracker.git synced 2025-07-25 20:59:01 +02:00

Add per-tracker milestone ID to avoid leaking global IDs

This commit is contained in:
chylex 2020-08-16 14:01:29 +02:00
parent c381c594fa
commit caa111fd59
12 changed files with 165 additions and 88 deletions

View File

@ -1,18 +1,18 @@
CREATE TABLE IF NOT EXISTS `issues` (
`tracker_id` INT NOT NULL,
`issue_id` INT NOT NULL,
`author_id` INT NULL,
`assignee_id` INT NULL,
`milestone_id` INT DEFAULT NULL,
`title` VARCHAR(128) NOT NULL,
`description` TEXT NOT NULL,
`type` ENUM ('feature', 'enhancement', 'bug', 'crash', 'task') NOT NULL,
`priority` ENUM ('low', 'medium', 'high') NOT NULL,
`scale` ENUM ('tiny', 'small', 'medium', 'large', 'massive') NOT NULL,
`status` ENUM ('open', 'in-progress', 'ready-to-test', 'blocked', 'finished', 'rejected') DEFAULT 'open' NOT NULL,
`progress` TINYINT DEFAULT 0 NOT NULL,
`date_created` DATETIME NOT NULL,
`date_updated` DATETIME NOT NULL,
`tracker_id` INT NOT NULL,
`issue_id` INT NOT NULL,
`author_id` INT NULL,
`assignee_id` INT NULL,
`milestone_gid` INT DEFAULT NULL,
`title` VARCHAR(128) NOT NULL,
`description` TEXT NOT NULL,
`type` ENUM ('feature', 'enhancement', 'bug', 'crash', 'task') NOT NULL,
`priority` ENUM ('low', 'medium', 'high') NOT NULL,
`scale` ENUM ('tiny', 'small', 'medium', 'large', 'massive') NOT NULL,
`status` ENUM ('open', 'in-progress', 'ready-to-test', 'blocked', 'finished', 'rejected') DEFAULT 'open' NOT NULL,
`progress` TINYINT DEFAULT 0 NOT NULL,
`date_created` DATETIME NOT NULL,
`date_updated` DATETIME NOT NULL,
PRIMARY KEY (`tracker_id`, `issue_id`),
FOREIGN KEY (`tracker_id`)
REFERENCES `trackers` (`id`)
@ -27,13 +27,13 @@ CREATE TABLE IF NOT EXISTS `issues` (
REFERENCES `users` (`id`)
ON UPDATE CASCADE
ON DELETE SET NULL,
FOREIGN KEY (`milestone_id`)
REFERENCES `milestones` (`id`)
FOREIGN KEY (`milestone_gid`)
REFERENCES `milestones` (`gid`)
ON UPDATE CASCADE
ON DELETE SET NULL,
FOREIGN KEY (`milestone_id`, `tracker_id`)
FOREIGN KEY (`milestone_gid`, `tracker_id`)
# Ensures the milestone-tracker pair is always valid.
REFERENCES `milestones` (`id`, `tracker_id`)
REFERENCES `milestones` (`gid`, `tracker_id`)
ON UPDATE NO ACTION
ON DELETE NO ACTION,
FOREIGN KEY (`scale`)

View File

@ -1,10 +1,11 @@
CREATE TABLE IF NOT EXISTS `milestones` (
`id` INT NOT NULL AUTO_INCREMENT,
`tracker_id` INT NOT NULL,
`ordering` INT NOT NULL,
`title` VARCHAR(64) NOT NULL,
PRIMARY KEY (`id`),
KEY (`id`, `tracker_id`), # Needed for milestone-tracker pair checks.
`gid` INT NOT NULL AUTO_INCREMENT,
`milestone_id` INT NOT NULL,
`tracker_id` INT NOT NULL,
`ordering` INT NOT NULL,
`title` VARCHAR(64) NOT NULL,
PRIMARY KEY (`tracker_id`, `milestone_id`),
KEY (`gid`, `tracker_id`), # Needed for milestone-tracker pair checks.
FOREIGN KEY (`tracker_id`)
REFERENCES `trackers` (`id`)
ON UPDATE CASCADE

View File

@ -12,11 +12,11 @@ CREATE TABLE IF NOT EXISTS `tracker_user_settings` (
ON UPDATE CASCADE
ON DELETE CASCADE,
FOREIGN KEY (`active_milestone`)
REFERENCES `milestones` (`id`)
REFERENCES `milestones` (`gid`)
ON UPDATE CASCADE
ON DELETE SET NULL,
FOREIGN KEY (`active_milestone`, `tracker_id`) # Ensures the milestone-tracker pair is always valid.
REFERENCES `milestones` (`id`, `tracker_id`)
REFERENCES `milestones` (`gid`, `tracker_id`)
ON UPDATE NO ACTION
ON DELETE NO ACTION
) ENGINE = InnoDB

View File

@ -40,6 +40,8 @@ final class IssueFilter extends AbstractTrackerIdFilter{
return new FieldOneOf($field, $value);
case 'milestone':
return new FieldOneOf($field.'_id', $value, 'm');
case 'author':
case 'assignee':
return new FieldOneOf($field.'_id', $value);

View File

@ -6,7 +6,7 @@ namespace Database\Objects;
use function Database\protect;
final class MilestoneInfo{
private int $id;
private int $milestone_id;
private string $title;
private int $closed_issues;
@ -14,8 +14,8 @@ final class MilestoneInfo{
private ?int $percentage_done;
private ?string $date_updated;
public function __construct(int $id, string $title, int $closed_issues, int $total_issues, ?int $percentage_done, ?string $date_updated){
$this->id = $id;
public function __construct(int $milestone_id, string $title, int $closed_issues, int $total_issues, ?int $percentage_done, ?string $date_updated){
$this->milestone_id = $milestone_id;
$this->title = $title;
$this->closed_issues = $closed_issues;
$this->total_issues = $total_issues;
@ -23,8 +23,8 @@ final class MilestoneInfo{
$this->date_updated = $date_updated;
}
public function getId(): int{
return $this->id;
public function getMilestoneId(): int{
return $this->milestone_id;
}
public function getTitleSafe(): string{

View File

@ -32,7 +32,7 @@ final class IssueTable extends AbstractTrackerTable{
IssueScale $scale,
IssueStatus $status,
int $progress,
?int $milestone_id,
?int $milestone_gid,
?int $assignee_id
): int{
$this->db->beginTransaction();
@ -49,16 +49,16 @@ final class IssueTable extends AbstractTrackerTable{
}
$stmt = $this->db->prepare(<<<SQL
INSERT INTO issues (tracker_id, issue_id, author_id, assignee_id, milestone_id, title, description, type, priority, scale, status, progress, date_created, date_updated)
VALUES (:tracker_id, :issue_id, :author_id, :assignee_id, :milestone_id, :title, :description, :type, :priority, :scale, :status, :progress, NOW(), NOW())
SQL
INSERT INTO issues (tracker_id, issue_id, author_id, assignee_id, milestone_gid, title, description, type, priority, scale, status, progress, date_created, date_updated)
VALUES (:tracker_id, :issue_id, :author_id, :assignee_id, :milestone_gid, :title, :description, :type, :priority, :scale, :status, :progress, NOW(), NOW())
SQL
);
$stmt->bindValue('tracker_id', $this->getTrackerId(), PDO::PARAM_INT);
$stmt->bindValue('issue_id', $next_id, PDO::PARAM_INT);
$stmt->bindValue('author_id', $author->getId(), PDO::PARAM_INT);
$stmt->bindValue('assignee_id', $assignee_id, $assignee_id === null ? PDO::PARAM_NULL : PDO::PARAM_INT);
$stmt->bindValue('milestone_id', $milestone_id, $milestone_id === null ? PDO::PARAM_NULL : PDO::PARAM_INT);
$stmt->bindValue('milestone_gid', $milestone_gid, $milestone_gid === null ? PDO::PARAM_NULL : PDO::PARAM_INT);
$stmt->bindValue('title', $title);
$stmt->bindValue('description', $description);
$stmt->bindValue('type', $type->getId());
@ -84,7 +84,7 @@ final class IssueTable extends AbstractTrackerTable{
IssueScale $scale,
IssueStatus $status,
int $progress,
?int $milestone_id,
?int $milestone_gid,
?int $assignee_id
): void{
$stmt = $this->db->prepare(<<<SQL
@ -97,7 +97,7 @@ SET title = :title,
status = :status,
progress = :progress,
assignee_id = :assignee_id,
milestone_id = :milestone_id,
milestone_gid = :milestone_gid,
date_updated = NOW()
WHERE issue_id = :issue_id AND tracker_id = :tracker_id
SQL
@ -106,7 +106,7 @@ SQL
$stmt->bindValue('tracker_id', $this->getTrackerId(), PDO::PARAM_INT);
$stmt->bindValue('issue_id', $id, PDO::PARAM_INT);
$stmt->bindValue('assignee_id', $assignee_id, $assignee_id === null ? PDO::PARAM_NULL : PDO::PARAM_INT);
$stmt->bindValue('milestone_id', $milestone_id, $milestone_id === null ? PDO::PARAM_NULL : PDO::PARAM_INT);
$stmt->bindValue('milestone_gid', $milestone_gid, $milestone_gid === null ? PDO::PARAM_NULL : PDO::PARAM_INT);
$stmt->bindValue('title', $title);
$stmt->bindValue('description', $description);
$stmt->bindValue('type', $type->getId());
@ -145,9 +145,15 @@ SQL
}
public function countIssues(?IssueFilter $filter = null): ?int{
$filter = $this->prepareFilter($filter ?? IssueFilter::empty());
$filter = $this->prepareFilter($filter ?? IssueFilter::empty(), 'i');
if ($filter->isEmpty()){
$stmt = $filter->prepare($this->db, 'SELECT COUNT(*) FROM issues i', AbstractFilter::STMT_COUNT);
}
else{
$stmt = $filter->prepare($this->db, 'SELECT COUNT(*) FROM issues i LEFT JOIN milestones m ON i.milestone_gid = m.gid', AbstractFilter::STMT_COUNT);
}
$stmt = $filter->prepare($this->db, 'SELECT COUNT(*) FROM issues', AbstractFilter::STMT_COUNT);
$stmt->execute();
$count = $this->fetchOneColumn($stmt);
@ -159,9 +165,23 @@ SQL
* @return IssueInfo[]
*/
public function listIssues(?IssueFilter $filter = null): array{
$filter = $this->prepareFilter($filter ?? IssueFilter::empty());
$filter = $this->prepareFilter($filter ?? IssueFilter::empty(), 'i');
$stmt = $filter->prepare($this->db, <<<SQL
SELECT i.issue_id AS id,
i.title,
i.type,
i.priority,
i.scale,
i.status,
i.progress,
i.date_created,
i.date_updated
FROM issues i
LEFT JOIN milestones m ON i.milestone_gid = m.gid
SQL
);
$stmt = $filter->prepare($this->db, 'SELECT issue_id AS id, title, type, priority, scale, status, progress, date_created, date_updated FROM issues');
$stmt->execute();
$results = [];
@ -183,25 +203,25 @@ SQL
public function getIssueDetail(int $id): ?IssueDetail{
$stmt = $this->db->prepare(<<<SQL
SELECT issues.title AS title,
issues.description AS description,
issues.type AS type,
issues.priority AS priority,
issues.scale AS scale,
issues.status AS status,
issues.progress AS progress,
issues.date_created AS date_created,
issues.date_updated AS date_updated,
issues.author_id AS author_id,
author.name AS author_name,
issues.assignee_id AS assignee_id,
assignee.name AS assignee_name,
milestone.id AS milestone_id,
milestone.title AS milestone_title
SELECT issues.title AS title,
issues.description AS description,
issues.type AS type,
issues.priority AS priority,
issues.scale AS scale,
issues.status AS status,
issues.progress AS progress,
issues.date_created AS date_created,
issues.date_updated AS date_updated,
issues.author_id AS author_id,
author.name AS author_name,
issues.assignee_id AS assignee_id,
assignee.name AS assignee_name,
milestone.milestone_id AS milestone_id,
milestone.title AS milestone_title
FROM issues
LEFT JOIN users author ON issues.author_id = author.id
LEFT JOIN users assignee ON issues.assignee_id = assignee.id
LEFT JOIN milestones milestone ON issues.milestone_id = milestone.id
LEFT JOIN milestones milestone ON issues.milestone_gid = milestone.gid
WHERE issues.issue_id = :issue_id AND issues.tracker_id = :tracker_id
SQL
);

View File

@ -21,21 +21,29 @@ final class MilestoneTable extends AbstractTrackerTable{
$this->db->beginTransaction();
try{
$stmt = $this->db->prepare('SELECT IFNULL(MAX(ordering) + 1, 1) FROM milestones WHERE tracker_id = ?');
$stmt = $this->db->prepare(<<<SQL
SELECT IFNULL(MAX(milestone_id) + 1, 1) AS id,
IFNULL(MAX(ordering) + 1, 1) AS ordering
FROM milestones
WHERE tracker_id = ?
SQL
);
$stmt->bindValue(1, $this->getTrackerId(), PDO::PARAM_INT);
$stmt->execute();
$order = $this->fetchOneColumn($stmt);
$next = $this->fetchOne($stmt);
if ($order === false){
if ($next === false){
$this->db->rollBack();
throw new LogicException('Error calculating milestone order.');
throw new LogicException('Error calculating next milestone ID.');
}
$stmt = $this->db->prepare('INSERT INTO milestones (tracker_id, ordering, title) VALUES (?, ?, ?)');
$stmt->bindValue(1, $this->getTrackerId(), PDO::PARAM_INT);
$stmt->bindValue(2, $order, PDO::PARAM_INT);
$stmt->bindValue(3, $title);
$stmt = $this->db->prepare('INSERT INTO milestones (milestone_id, tracker_id, ordering, title) VALUES (?, ?, ?, ?)');
$stmt->bindValue(1, $next['id'], PDO::PARAM_INT);
$stmt->bindValue(2, $this->getTrackerId(), PDO::PARAM_INT);
$stmt->bindValue(3, $next['ordering'], PDO::PARAM_INT);
$stmt->bindValue(4, $title);
$stmt->execute();
$this->db->commit();
@ -98,7 +106,7 @@ final class MilestoneTable extends AbstractTrackerTable{
$stmt->bindValue(3, $this->getTrackerId(), PDO::PARAM_INT);
$stmt->execute();
$stmt = $this->db->prepare('UPDATE milestones SET ordering = ? WHERE id = ? AND tracker_id = ?');
$stmt = $this->db->prepare('UPDATE milestones SET ordering = ? WHERE milestone_id = ? AND tracker_id = ?');
$stmt->bindValue(1, $other_ordering, PDO::PARAM_INT);
$stmt->bindValue(2, $id, PDO::PARAM_INT);
$stmt->bindValue(3, $this->getTrackerId(), PDO::PARAM_INT);
@ -106,7 +114,7 @@ final class MilestoneTable extends AbstractTrackerTable{
}
private function getMilestoneOrdering(int $id): ?int{
$stmt = $this->db->prepare('SELECT ordering FROM milestones WHERE id = ? AND tracker_id = ?');
$stmt = $this->db->prepare('SELECT ordering FROM milestones WHERE milestone_id = ? AND tracker_id = ?');
$stmt->bindValue(1, $id, PDO::PARAM_INT);
$stmt->bindValue(2, $this->getTrackerId(), PDO::PARAM_INT);
$stmt->execute();
@ -133,17 +141,17 @@ final class MilestoneTable extends AbstractTrackerTable{
$filter = $this->prepareFilter($filter ?? MilestoneFilter::empty(), 'm');
$sql = <<<SQL
SELECT m.id AS id,
SELECT m.milestone_id AS milestone_id,
m.title AS title,
COUNT(CASE WHEN i.status IN ('finished', 'rejected') THEN 1 END) AS closed_issues,
COUNT(i.issue_id) AS total_issues,
FLOOR(SUM(i.progress * iw.contribution) / SUM(iw.contribution)) AS progress,
MAX(i.date_updated) AS date_updated
FROM milestones m
LEFT JOIN issues i ON m.id = i.milestone_id
LEFT JOIN issues i ON m.gid = i.milestone_gid
LEFT JOIN issue_weights iw ON i.scale = iw.scale
# WHERE
GROUP BY m.id, m.title
GROUP BY m.gid, m.title
# ORDER
# LIMIT
SQL;
@ -154,7 +162,7 @@ SQL;
$results = [];
while(($res = $this->fetchNext($stmt)) !== false){
$results[] = new MilestoneInfo($res['id'],
$results[] = new MilestoneInfo($res['milestone_id'],
$res['title'],
$res['closed_issues'],
$res['total_issues'],
@ -165,6 +173,16 @@ SQL;
return $results;
}
public function findGlobalId(int $id): ?int{
$stmt = $this->db->prepare('SELECT gid FROM milestones WHERE milestone_id = ? AND tracker_id = ?');
$stmt->bindValue(1, $id, PDO::PARAM_INT);
$stmt->bindValue(2, $this->getTrackerId(), PDO::PARAM_INT);
$stmt->execute();
$gid = $this->fetchOneColumn($stmt);
return $gid === false ? null : (int)$gid;
}
public function deleteById(int $id): void{
$this->db->beginTransaction();
@ -181,7 +199,7 @@ SQL;
$stmt->bindValue(2, $this->getTrackerId(), PDO::PARAM_INT);
$stmt->execute();
$stmt = $this->db->prepare('DELETE FROM milestones WHERE id = ? AND tracker_id = ?');
$stmt = $this->db->prepare('DELETE FROM milestones WHERE milestone_id = ? AND tracker_id = ?');
$stmt->bindValue(1, $id, PDO::PARAM_INT);
$stmt->bindValue(2, $this->getTrackerId(), PDO::PARAM_INT);
$stmt->execute();

View File

@ -14,7 +14,7 @@ final class TrackerUserSettingsTable extends AbstractTrackerTable{
parent::__construct($db, $tracker);
}
public function toggleActiveMilestone(UserProfile $user, int $milestone_id): void{
public function toggleActiveMilestone(UserProfile $user, int $milestone_gid): void{
$stmt = $this->db->prepare(<<<SQL
INSERT INTO tracker_user_settings (tracker_id, user_id, active_milestone)
VALUES (?, ?, ?)
@ -24,21 +24,21 @@ SQL
$stmt->bindValue(1, $this->getTrackerId(), PDO::PARAM_INT);
$stmt->bindValue(2, $user->getId(), PDO::PARAM_INT);
$stmt->bindValue(3, $milestone_id, PDO::PARAM_INT);
$stmt->bindValue(3, $milestone_gid, PDO::PARAM_INT);
$stmt->execute();
}
public function getActiveMilestoneProgress(UserProfile $user): ?MilestoneProgress{
$stmt = $this->db->prepare(<<<SQL
SELECT m.id AS id,
SELECT m.milestone_id AS id,
m.title AS title,
FLOOR(SUM(i.progress * iw.contribution) / SUM(iw.contribution)) AS percentage_done
FROM tracker_user_settings tus
JOIN milestones m ON m.id = tus.active_milestone
LEFT JOIN issues i ON m.id = i.milestone_id
JOIN milestones m ON m.gid = tus.active_milestone
LEFT JOIN issues i ON m.gid = i.milestone_gid
LEFT JOIN issue_weights iw ON i.scale = iw.scale
WHERE tus.user_id = ? AND tus.tracker_id = ?
GROUP BY m.id
GROUP BY m.gid
SQL
);

View File

@ -116,7 +116,7 @@ HTML
$select_assignee = $this->form->addSelect('Assignee')->addOption('', '(None)')->dropdown();
foreach((new MilestoneTable(DB::get(), $tracker))->listMilestones() as $milestone){
$select_milestone->addOption(strval($milestone->getId()), $milestone->getTitleSafe());
$select_milestone->addOption(strval($milestone->getMilestoneId()), $milestone->getTitleSafe());
}
if ($perms->checkTracker($tracker, MembersModel::PERM_LIST)){
@ -236,8 +236,21 @@ HTML
return null;
}
if ($milestone === null){
$milestone_gid = null;
}
else{
$milestones = new MilestoneTable(DB::get(), $tracker);
$milestone_gid = $milestones->findGlobalId($milestone);
if ($milestone_gid === null){
$this->form->invalidateField('Milestone', 'Invalid milestone.');
return null;
}
}
if ($this->issue_id === null){
$id = $issues->addIssue($new_issue_author, $title, $description, $type, $priority, $scale, $status, $progress, $milestone, $assignee);
$id = $issues->addIssue($new_issue_author, $title, $description, $type, $priority, $scale, $status, $progress, $milestone_gid, $assignee);
}
else{
if ($progress === $this->issue->getProgress()){
@ -268,7 +281,7 @@ HTML
$assignee = $prev_assignee === null ? null : $prev_assignee->getId();
}
$issues->editIssue($this->issue_id, $title, $description, $type, $priority, $scale, $status, $progress, $milestone, $assignee);
$issues->editIssue($this->issue_id, $title, $description, $type, $priority, $scale, $status, $progress, $milestone_gid, $assignee);
$id = $this->issue_id;
}

View File

@ -109,7 +109,7 @@ class IssuesModel extends BasicTrackerPageModel{
$filtering_milestone->addOption('', '<span class="missing">(No Milestone)</span>');
foreach((new MilestoneTable(DB::get(), $tracker))->listMilestones() as $milestone){
$filtering_milestone->addOption(strval($milestone->getId()), $milestone->getTitleSafe());
$filtering_milestone->addOption(strval($milestone->getMilestoneId()), $milestone->getTitleSafe());
}
// TODO get rid of IDs and allow filtering by manually typing username (either add a field, or just in the URL & add the options if the user cannot see everyone)

View File

@ -83,7 +83,7 @@ class MilestonesModel extends BasicTrackerPageModel{
$active_milestone_id = $active_milestone === null ? null : $active_milestone->getId();
foreach($milestones->listMilestones($filter) as $milestone){
$milestone_id = $milestone->getId();
$milestone_id = $milestone->getMilestoneId();
$milestone_id_str = strval($milestone_id);
$update_date = $milestone->getLastUpdateDate();
@ -193,8 +193,18 @@ class MilestonesModel extends BasicTrackerPageModel{
return false;
}
$settings = new TrackerUserSettingsTable(DB::get(), $this->getTracker());
$settings->toggleActiveMilestone($logon_user, (int)$data['Milestone']);
$db = DB::get();
$tracker = $this->getTracker();
$milestones = new MilestoneTable($db, $tracker);
$milestone_gid = $milestones->findGlobalId((int)$data['Milestone']);
if ($milestone_gid === null){
return false;
}
$settings = new TrackerUserSettingsTable($db, $tracker);
$settings->toggleActiveMilestone($logon_user, $milestone_gid);
return true;
}

View File

@ -21,6 +21,13 @@ try{
$db->query('ALTER TABLE system_roles ADD special BOOL DEFAULT FALSE NOT NULL');
$db->query('ALTER TABLE tracker_roles ADD special BOOL DEFAULT FALSE NOT NULL');
/** @noinspection SqlResolve */
$db->query('ALTER TABLE milestones CHANGE id gid INT NOT NULL AUTO_INCREMENT');
$db->query('ALTER TABLE milestones ADD milestone_id INT NOT NULL DEFAULT 0 AFTER gid');
/** @noinspection SqlResolve */
$db->query('ALTER TABLE issues CHANGE milestone_id milestone_gid INT NULL');
begin_transaction($db);
$db->query(<<<SQL
@ -30,7 +37,7 @@ FROM tracker_roles
GROUP BY tracker_id
SQL
);
$db->query(<<<SQL
INSERT INTO tracker_members (tracker_id, user_id, role_id)
SELECT t.id AS tracker_id, t.owner_id AS user_id, tr.id AS role_id
@ -38,6 +45,12 @@ FROM trackers t
JOIN tracker_roles tr ON t.id = tr.tracker_id AND tr.title = 'Owner' AND tr.special = TRUE
SQL
);
/** @noinspection SqlWithoutWhere */
$db->query('UPDATE milestones SET milestone_id = ordering');
$db->query('ALTER TABLE milestones MODIFY milestone_id INT NOT NULL AFTER gid');
$db->query('ALTER TABLE milestones DROP PRIMARY KEY');
$db->query('ALTER TABLE milestones ADD PRIMARY KEY (tracker_id, milestone_id)');
}
if (!file_put_contents(CONFIG_FILE, SystemConfig::fromCurrentInstallation()->generate(), LOCK_EX)){