fix(ical): only accept known columns as the calendar start date
The calendar_project_tasks and calendar_user_tasks settings were used as a column name in the iCal query, and escapeIdentifier() leaves values with a space or a dot untouched. A stored value like "1 OR 1=1 OR 1" made the public project feed return tasks from every project. Only date_creation and date_started are accepted now, anything else falls back to date_started. The currency form also stops saving keys other than application_currency, which is how the value could be written in the first place. Fixes #5904
This commit is contained in:
1 parent
a0b55ff348
commit
23bbaa8c60
4 files changed
+142
-3
No files matched your search
@@ -91,7 +91,7 @@ class CurrencyController extends BaseController
|
|||||||
*/
|
*/
|
||||||
public function update()
|
public function update()
|
||||||
{
|
{
|
||||||
$values = $this->request->getValues();
|
$values = array_intersect_key($this->request->getValues(), array('application_currency' => ''));
|
||||||
|
|
||||||
if ($this->configModel->save($values)) {
|
if ($this->configModel->save($values)) {
|
||||||
$this->flash->success(t('Settings saved successfully.'));
|
$this->flash->success(t('Settings saved successfully.'));
|
||||||
|
|||||||
@@ -31,7 +31,7 @@ class ICalendarController extends BaseController
|
|||||||
$startRange = strtotime('-2 months');
|
$startRange = strtotime('-2 months');
|
||||||
$endRange = strtotime('+6 months');
|
$endRange = strtotime('+6 months');
|
||||||
|
|
||||||
$startColumn = $this->configModel->get('calendar_user_tasks', 'date_started');
|
$startColumn = $this->getStartColumn('calendar_user_tasks');
|
||||||
|
|
||||||
$calendar = new iCalendar('Kanboard');
|
$calendar = new iCalendar('Kanboard');
|
||||||
$calendar->setName($user['name'] ?: $user['username']);
|
$calendar->setName($user['name'] ?: $user['username']);
|
||||||
@@ -71,7 +71,7 @@ class ICalendarController extends BaseController
|
|||||||
$startRange = strtotime('-2 months');
|
$startRange = strtotime('-2 months');
|
||||||
$endRange = strtotime('+6 months');
|
$endRange = strtotime('+6 months');
|
||||||
|
|
||||||
$startColumn = $this->configModel->get('calendar_project_tasks', 'date_started');
|
$startColumn = $this->getStartColumn('calendar_project_tasks');
|
||||||
|
|
||||||
$calendar = new iCalendar('Kanboard');
|
$calendar = new iCalendar('Kanboard');
|
||||||
$calendar->setName($project['name']);
|
$calendar->setName($project['name']);
|
||||||
@@ -99,6 +99,12 @@ class ICalendarController extends BaseController
|
|||||||
->format());
|
->format());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
protected function getStartColumn($option)
|
||||||
|
{
|
||||||
|
$column = $this->configModel->get($option, 'date_started');
|
||||||
|
return in_array($column, array('date_creation', 'date_started'), true) ? $column : 'date_started';
|
||||||
|
}
|
||||||
|
|
||||||
protected function getConditionForTasksWithStartAndDueDate($start_time, $end_time, $start_column, $end_column)
|
protected function getConditionForTasksWithStartAndDueDate($start_time, $end_time, $start_column, $end_column)
|
||||||
{
|
{
|
||||||
$start_time = (int) $start_time;
|
$start_time = (int) $start_time;
|
||||||
|
|||||||
@@ -0,0 +1,41 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
namespace KanboardTests\units\Controller;
|
||||||
|
|
||||||
|
use Kanboard\Controller\CurrencyController;
|
||||||
|
use Kanboard\Core\Http\Request;
|
||||||
|
use Kanboard\Core\Http\Response;
|
||||||
|
use KanboardTests\units\Base;
|
||||||
|
|
||||||
|
class CurrencyControllerTest extends Base
|
||||||
|
{
|
||||||
|
public function testChangingTheCurrencyDoesNotSaveOtherSettings()
|
||||||
|
{
|
||||||
|
$this->container['request'] = new Request(
|
||||||
|
$this->container,
|
||||||
|
array('REQUEST_METHOD' => 'POST'),
|
||||||
|
array(),
|
||||||
|
array(
|
||||||
|
'csrf_token' => $this->container['token']->getCSRFToken(),
|
||||||
|
'application_currency' => 'EUR',
|
||||||
|
'calendar_project_tasks' => '1 OR 1=1 OR 1',
|
||||||
|
),
|
||||||
|
array(),
|
||||||
|
array()
|
||||||
|
);
|
||||||
|
|
||||||
|
$this->container['response'] = $this->getMockBuilder(Response::class)
|
||||||
|
->setConstructorArgs(array($this->container))
|
||||||
|
->onlyMethods(array('redirect'))
|
||||||
|
->getMock();
|
||||||
|
|
||||||
|
$this->container['response']
|
||||||
|
->expects($this->once())
|
||||||
|
->method('redirect');
|
||||||
|
|
||||||
|
(new CurrencyController($this->container))->update();
|
||||||
|
|
||||||
|
$this->assertEquals('EUR', $this->container['configModel']->getOption('application_currency'));
|
||||||
|
$this->assertEquals('date_started', $this->container['configModel']->getOption('calendar_project_tasks'));
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,92 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
namespace KanboardTests\units\Controller;
|
||||||
|
|
||||||
|
use Kanboard\Controller\ICalendarController;
|
||||||
|
use Kanboard\Core\Http\Request;
|
||||||
|
use Kanboard\Core\Http\Response;
|
||||||
|
use KanboardTests\units\Base;
|
||||||
|
|
||||||
|
class ICalendarControllerTest extends Base
|
||||||
|
{
|
||||||
|
private $calendar = '';
|
||||||
|
|
||||||
|
public function testProjectCalendarIgnoresUnknownStartColumn()
|
||||||
|
{
|
||||||
|
$this->container['configModel']->save(array('calendar_project_tasks' => '1 OR 1=1 OR 1'));
|
||||||
|
$this->createFixtures();
|
||||||
|
$project = $this->container['projectModel']->getById(1);
|
||||||
|
|
||||||
|
$this->renderCalendar(array('token' => $project['token']))->project();
|
||||||
|
|
||||||
|
$this->assertStringNotContainsString('Private task', $this->calendar);
|
||||||
|
$this->assertStringContainsString('Public task', $this->calendar);
|
||||||
|
$this->assertStringContainsString('task-#1-date_started-date_due', $this->calendar);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testUserCalendarIgnoresUnknownStartColumn()
|
||||||
|
{
|
||||||
|
$this->container['configModel']->save(array('calendar_user_tasks' => '1 OR 1=1 OR 1'));
|
||||||
|
$this->createFixtures();
|
||||||
|
$user = $this->container['userModel']->getById(2);
|
||||||
|
|
||||||
|
$this->renderCalendar(array('token' => $user['token']))->user();
|
||||||
|
|
||||||
|
$this->assertStringNotContainsString('Private task', $this->calendar);
|
||||||
|
$this->assertStringContainsString('Public task', $this->calendar);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testProjectCalendarUsesCreationDateWhenConfigured()
|
||||||
|
{
|
||||||
|
$this->container['configModel']->save(array('calendar_project_tasks' => 'date_creation'));
|
||||||
|
$this->createFixtures();
|
||||||
|
$project = $this->container['projectModel']->getById(1);
|
||||||
|
|
||||||
|
$this->renderCalendar(array('token' => $project['token']))->project();
|
||||||
|
|
||||||
|
$this->assertStringContainsString('task-#1-date_creation-date_due', $this->calendar);
|
||||||
|
}
|
||||||
|
|
||||||
|
private function createFixtures()
|
||||||
|
{
|
||||||
|
$this->assertEquals(2, $this->container['userModel']->create(array('username' => 'user1')));
|
||||||
|
$this->assertTrue($this->container['userModel']->enablePublicAccess(2));
|
||||||
|
|
||||||
|
$this->assertEquals(1, $this->container['projectModel']->create(array('name' => 'Public project')));
|
||||||
|
$this->assertTrue($this->container['projectModel']->enablePublicAccess(1));
|
||||||
|
$this->assertEquals(2, $this->container['projectModel']->create(array('name' => 'Private project')));
|
||||||
|
|
||||||
|
$this->assertEquals(1, $this->createTask(1, 'Public task', 2));
|
||||||
|
$this->assertEquals(2, $this->createTask(2, 'Private task', 1));
|
||||||
|
}
|
||||||
|
|
||||||
|
private function createTask($projectId, $title, $ownerId)
|
||||||
|
{
|
||||||
|
return $this->container['taskCreationModel']->create(array(
|
||||||
|
'project_id' => $projectId,
|
||||||
|
'title' => $title,
|
||||||
|
'owner_id' => $ownerId,
|
||||||
|
'date_started' => strtotime('-1 day'),
|
||||||
|
'date_due' => strtotime('+1 day'),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
private function renderCalendar(array $params)
|
||||||
|
{
|
||||||
|
$this->container['request'] = new Request($this->container, array('REQUEST_METHOD' => 'GET'), $params);
|
||||||
|
|
||||||
|
$this->container['response'] = $this->getMockBuilder(Response::class)
|
||||||
|
->setConstructorArgs(array($this->container))
|
||||||
|
->onlyMethods(array('ical'))
|
||||||
|
->getMock();
|
||||||
|
|
||||||
|
$this->container['response']
|
||||||
|
->expects($this->once())
|
||||||
|
->method('ical')
|
||||||
|
->willReturnCallback(function ($calendar) {
|
||||||
|
$this->calendar = $calendar;
|
||||||
|
});
|
||||||
|
|
||||||
|
return new ICalendarController($this->container);
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in new issue
Block a user