Skip to content

Commit

Permalink
Fixed issue #15350: Unable to delete saved reponse
Browse files Browse the repository at this point in the history
Fixed issue [security] : no CRSF control when delete saved response
Dev: move to LimeGridView
Dev: Add Permission check
  • Loading branch information
Shnoulle committed Oct 4, 2019
1 parent c825111 commit 391dba0
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 96 deletions.
93 changes: 55 additions & 38 deletions application/controllers/admin/saved.php
Expand Up @@ -3,7 +3,7 @@
}
/*
* LimeSurvey
* Copyright (C) 2007-2011 The LimeSurvey Project Team / Carsten Schmitz
* Copyright (C) 2007-2019 The LimeSurvey Project Team / Carsten Schmitz
* All rights reserved.
* License: GNU/GPL License v2 or later, see LICENSE.php
* LimeSurvey is free software. This version may have been modified pursuant
Expand All @@ -24,41 +24,79 @@
class saved extends Survey_Common_Action
{

/**
* Show the list of save response
* @param int $surveyid
* @return void
* @throw Exception
*/
public function view($iSurveyId)
{
$iSurveyId = sanitize_int($iSurveyId);
$aViewUrls = array();

if (!Permission::model()->hasSurveyPermission($iSurveyId, 'responses', 'read')) {
die();
throw new CHttpException(403, gT("You do not have permission to access this page."));
}

$aThisSurvey = getSurveyInfo($iSurveyId);
$aData['sSurveyName'] = $aThisSurvey['name'];
$aData['iSurveyId'] = $iSurveyId;
$aViewUrls[] = 'savedbar_view';
$aViewUrls['savedlist_view'][] = $this->_showSavedList($iSurveyId);

// saved.js bugs if table is empty
if (count($aViewUrls['savedlist_view'][0]['aResults'])) {
App()->getClientScript()->registerPackage('jquery-tablesorter');
App()->getClientScript()->registerScriptFile(App()->getConfig('adminscripts').'saved.js');
}


$aData['dataProvider'] = new CActiveDataProvider('SavedControl', array(
'criteria'=>array(
'condition'=>'sid=:sid',
'params'=>[':sid'=>$iSurveyId],
),
'countCriteria'=>array(
'condition'=>'sid=:sid',
'params'=>[':sid'=>$iSurveyId],
),
));
$aData['SavedControlModel'] = SavedControl::model();
$aViewUrls[] = 'savedlist_view';
$this->_renderWrappedTemplate('saved', $aViewUrls, $aData);
}

/**
* Function responsible to delete saved responses.
* @param int $surveyid
* @return void
* @throw Exception
*/
public function delete($iSurveyId, $iSurveyResponseId, $iSavedControlId)
public function actionDelete($surveyid)
{
$survey = Survey::model()->findByPk($iSurveyId);
SavedControl::model()->deleteAllByAttributes(array('scid' => $iSavedControlId, 'sid' => $iSurveyId)) or die(gT("Couldn't delete"));
Yii::app()->db->createCommand()->delete($survey->responsesTableName, 'id=:id', array('id' => $iSurveyResponseId)) or die(gT("Couldn't delete"));
if(!Permission::model()->hasSurveyPermission($surveyid, 'responses', 'delete')) {
throw new CHttpException(403, gT("You do not have permission to access this page."));
}
if(!Yii::app()->getRequest()->isPostRequest) {
throw new CHttpException(405, gT("Invalid action"));
}
Yii::import('application.helpers.admin.ajax_helper', true);

$this->getController()->redirect(array("admin/saved/sa/view/surveyid/{$iSurveyId}"));
$iScid = App()->getRequest()->getParam('scid');
$oSavedControl = SavedControl::model()->find('scid = :scid',array(':scid'=>$iScid));
if(empty($oSavedControl)) {
throw new CHttpException(401, gT("Saved response not found"));
}
if($oSavedControl->delete()) {
$oReponse = Response::model($surveyid)->findByPk($oSavedControl->srid);
if($oReponse) {
$oReponse->delete();
}
} else {
if(Yii::app()->getRequest()->isAjaxRequest) {
ls\ajax\AjaxHelper::outputError(gT('Unable to delete saved response.'));
Yii::app()->end();
}
Yii::app()->setFlashMessage(gT('Unable to delete saved response.'),'danger');
$this->getController()->redirect(array("admin/saved/sa/view/surveyid/{$surveyid}"));
}
if(Yii::app()->getRequest()->isAjaxRequest) {
ls\ajax\AjaxHelper::outputSuccess(gT('Saved response deleted.'));
Yii::app()->end();
}
Yii::app()->setFlashMessage(gT('Saved response deleted.'),'success');
$this->getController()->redirect(array("admin/saved/sa/view/surveyid/{$surveyid}"));
}

/**
Expand All @@ -79,25 +117,4 @@ protected function _renderWrappedTemplate($sAction = 'saved', $aViewUrls = array
$aData['menu']['edition'] = false;
parent::_renderWrappedTemplate($sAction, $aViewUrls, $aData, $sRenderFile);
}

/**
* Load saved list.
* @param mixed $iSurveyId Survey id
* @return array
*/
private function _showSavedList($iSurveyId)
{
$aResults = SavedControl::model()->findAll(array(
'select' => array('scid', 'srid', 'identifier', 'ip', 'saved_date', 'email', 'access_code'),
'condition' => 'sid=:sid',
'order' => 'saved_date desc',
'params' => array(':sid' => $iSurveyId),
));

if (!empty($aResults)) {
return compact('aResults');
} else
{return array('aResults'=>array()); }
}

}
29 changes: 29 additions & 0 deletions application/models/SavedControl.php
Expand Up @@ -102,4 +102,33 @@ public function insertRecords($data)
return $this->db->insert('saved_control', $data);
}

public function getGridButtons($surveyid)
{
$gridButtons = array();
$gridButtons['editresponse'] = array(
'label'=>'<span class="sr-only">'.gT("Edit").'</span><span class="fa fa-list-alt" aria-hidden="true"></span>',
'imageUrl'=>false,
'url' => 'App()->createUrl("admin/dataentry/sa/editdata/subaction/edit",array("surveyid"=>$data->sid,"id"=>$data->srid));',
'options' => array(
'class'=>"btn btn-default btn-xs btn-edit",
'data-toggle'=>"tooltip",
'title'=>gT("Edit response")
),
'visible'=> 'boolval('.Permission::model()->hasSurveyPermission($surveyid, 'responses', 'update').')',
);
$gridButtons['delete'] = array(
'label'=>'<span class="sr-only">'.gT("Delete").'</span><span class="text-warning fa fa-trash" aria-hidden="true"></span>',
'imageUrl'=>false,
'icon'=>false,
'url' => 'App()->createUrl("admin/saved/sa/actionDelete",array("surveyid"=>$data->sid,"scid"=>$data->scid,"srid"=>$data->srid));',
'options' => array(
'class'=>"btn btn-default btn-xs btn-delete",
'data-toggle'=>"tooltip",
'title'=>gT("Delete this entry and related response")
),
'visible'=> 'boolval('.Permission::model()->hasSurveyPermission($surveyid, 'responses', 'delete').')',
'click' => 'function(event){ window.LS.gridButton.confirmGridAction(event,$(this)); }',
);
return $gridButtons;
}
}
91 changes: 43 additions & 48 deletions application/views/admin/saved/savedlist_view.php
@@ -1,54 +1,49 @@
<div class='side-body <?php echo getSideBodyClass(true); ?>'>
<h3>
<span style='font-weight:bold;'><?php eT('Saved responses'); ?></span>
<?php echo flattenText($sSurveyName) . ' ' . sprintf(gT('ID: %s'), $iSurveyId); ?>
<?php eT('Saved responses'); ?>
<small><?php echo flattenText($sSurveyName) . ' ' . sprintf(gT('ID: %s'), $iSurveyId); ?></small>
</h3>

<div class="row">
<div class="col-lg-12 content-right">
<div class="alert alert-info" role="alert">
<?php eT('Total:'); ?> <?php echo getSavedCount($iSurveyId); ?>
</div>

<p>
<table class='browsetable table' style='margin:0 auto; width:60%'>
<thead>
<tr>
<th><?php eT('ID'); ?></th>
<th><?php eT('Actions'); ?></th>
<th><?php eT('Identifier'); ?></th>
<th><?php eT('IP address'); ?></th>
<th><?php eT('Date saved'); ?></th>
<th><?php eT('Email address'); ?></th>
</tr>
</thead>

<tbody>
<?php foreach($aResults as $oResult)
{ ?>
<tr>
<td><?php echo $oResult->scid; ?></td>
<td align='center'>

<?php if (Permission::model()->hasSurveyPermission($iSurveyId,'responses','update'))
{ ?>
<span onclick="window.open('<?php echo $this->createUrl("admin/dataentry/sa/editdata/subaction/edit/surveyid/{$iSurveyId}/id/{$oResult->srid}"); ?>', '_top')" title='<?php eT('Edit entry'); ?>' class="fa fa-pencil text-success"></span>
<?php }
if (Permission::model()->hasSurveyPermission($iSurveyId,'responses','delete'))
{ ?>
<span class="fa fa-trash text-warning" title='<?php eT('Delete entry'); ?>' onclick="if (confirm('<?php eT('Are you sure you want to delete this entry?', 'js'); ?>')) { window.open('<?php echo $this->createUrl("admin/saved/delete/surveyid/{$iSurveyId}/srid/{$oResult->srid}/scid/{$oResult->scid}"); ?>', '_top'); }" ></span>
<?php } ?>
</td>

<td><?php echo htmlspecialchars($oResult->identifier); ?></td>
<td><?php echo $oResult->ip; ?></td>
<td><?php echo $oResult->saved_date; ?></td>
<td><?php echo CHtml::link(htmlspecialchars($oResult->email),'mailto:'.htmlspecialchars($oResult->email)); ?></td>

</tr>
<?php } ?>
</tbody>
</table>
<br />&nbsp;
</p>
</div></div></div>
<?php
$this->widget('ext.LimeGridView.LimeGridView', array(
'id' => 'saved-grid',
'ajaxUpdate' => 'saved-grid',
'dataProvider' => $dataProvider,
'ajaxType' => 'POST',
'template' => "{items}\n<div class='row'><div class='col-sm-4 col-md-offset-4'>{pager}</div><div class='col-sm-4'>{summary}</div></div>",
'columns' => array(
array(
'header' => gT("ID"),
'name' => 'scid',
),
array(
'class'=>'bootstrap.widgets.TbButtonColumn',
'template'=>'{editresponse}{delete}',
//~ 'htmlOptions' => array('class' => 'text-left response-buttons'),
'buttons'=> $SavedControlModel->getGridButtons($iSurveyId),
),
array(
'header' => gT("Identifier"),
'name' => 'identifier',
),
array(
'header' => gT("IP address"),
'name' => 'ip',
),
array(
'header' => gT("Date saved"),
'name' => 'saved_date',
),
array(
'header' => gT("Email address"),
'name' => 'email',
),
),
),

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Oct 8, 2019

Contributor

Syntax error here. Please fix.

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Oct 8, 2019

Contributor

No comma allowed after last argument in function.

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Oct 8, 2019

Contributor

Also maybe test after fix ;)

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Oct 8, 2019

Contributor

Wait, maybe this is allowed in PHP 7.3?

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Oct 8, 2019

Author Collaborator

It's an array, not a function call. Array params.
In master : no syntax error are shown (on PHP Version 7.3.10) , PHP version ?

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Oct 8, 2019

Contributor

Fixed in 3.19.1.

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Oct 8, 2019

Contributor

It's not inside the array, read closely.

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Oct 8, 2019

Contributor

image

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Oct 8, 2019

Contributor

Confirmed that PHP 7.3 is different: https://3v4l.org/LVXFa

);
?>
</div>
</div>
</div>
10 changes: 0 additions & 10 deletions assets/scripts/admin/saved.js

This file was deleted.

2 comments on commit 391dba0

@olleharstedt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge conflict with develop branch. Please merge (sorry).

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argl, i know there are update to do (call of /ls/ but didn't know there are merge issue …)
I look :) thanks for reminder

Please sign in to comment.