Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feature: Form Tokens #4509

Open
wants to merge 1 commit into
base: develop-next
Choose a base branch
from
Open

feature: Form Tokens #4509

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/ajax.tickets.php
View file
Open in desktop
Expand Up @@ -118,7 +118,7 @@ function acquireLock($tid) {

return $this->json_encode(array(
'id'=>$lock->getId(), 'time'=>$lock->getTime(),
'code' => $lock->getCode()
'code' => $lock->getCode(), 'token'=>$lock->getToken()
));
}

Expand Down
25 changes: 24 additions & 1 deletion include/class.lock.php
View file
Open in desktop
Expand Up @@ -20,6 +20,8 @@ class.lock.php

class Lock extends VerySimpleModel {

var $_token;

static $meta = array(
'table' => LOCK_TABLE,
'pk' => array('lock_id'),
Expand Down Expand Up @@ -79,6 +81,13 @@ function getCode() {
return $this->code;
}

function getToken() {
if (!$this->_token)
$this->_token = md5($this->lock_id.$this->code.strtotime($this->expire));

return $this->_token;
}

//Renew existing lock.
function renew($lockTime=0) {
global $cfg;
Expand All @@ -90,6 +99,7 @@ function renew($lockTime=0) {
SqlFunction::NOW(),
SqlInterval::MINUTE($lockTime)
);
$this->_token = null;
return $this->save(true);
}

Expand All @@ -111,7 +121,7 @@ static function lookup($id, $object=false) {
//Create a ticket lock...this function assumes the caller checked for access & validity of ticket & staff x-ship.
static function acquire($staffId, $lockTime) {

if (!$staffId or !$lockTime)
if (!is_int($staffId) or !$lockTime)
return null;

// Create the new lock.
Expand Down Expand Up @@ -146,5 +156,18 @@ static function cleanup() {
'expire__lt' => SqlFunction::NOW()
))->delete();
}

function validateToken($token=null, $code) {
if (!$code)
return false;

if (!($lock = Lock::lookup(array('code' => $code))))
return false;

if (strcmp($token, $lock->getToken()) !== 0)
return false;

return $lock->delete();
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure you want to delete the lock here - What about if the action (e.g posReply) errors? Re-acquiring the lock will change the token (due to new expire date - please confirm) - so I don't the need to delete here. Lock cleaning mechanism should handle deleting expired sessions.

}
}
?>
5 changes: 5 additions & 0 deletions include/client/open.inc.php
View file
Open in desktop
Expand Up @@ -30,11 +30,16 @@
}
}

// Form Token
if (!$mylock)
$mylock = Lock::acquire(0, $cfg->getLockTime() ?: 15);
?>
<h1><?php echo __('Open a New Ticket');?></h1>
<p><?php echo __('Please fill in the form below to open a new ticket.');?></p>
<form id="ticketForm" method="post" action="open.php" enctype="multipart/form-data">
<?php csrf_token(); ?>
<input type="hidden" name="lock_code" value="<?php echo $mylock ? $mylock->getCode() : ''; ?>">
<input type="hidden" name="form_token" value="<?php echo $mylock ? $mylock->getToken() : ''; ?>">
<input type="hidden" name="a" value="open">
<table width="800" cellpadding="1" cellspacing="0" border="0">
<tbody>
Expand Down
6 changes: 6 additions & 0 deletions include/client/view.inc.php
View file
Open in desktop
Expand Up @@ -8,6 +8,10 @@
if ($ticket->isClosed() && !$ticket->isReopenable())
$warn = sprintf(__('%s is marked as closed and cannot be reopened.'), __('This ticket'));

// Form Token
if (!$mylock)
$mylock = Lock::acquire(0, $cfg->getLockTime() ?: 15);

//Making sure we don't leak out internal dept names
if(!$dept || !$dept->isPublic())
$dept = $cfg->getDefaultDept();
Expand Down Expand Up @@ -158,6 +162,8 @@
<form id="reply" action="tickets.php?id=<?php echo $ticket->getId();
?>#reply" name="reply" method="post" enctype="multipart/form-data">
<?php csrf_token(); ?>
<input type="hidden" name="lock_code" value="<?php echo $mylock ? $mylock->getCode() : ''; ?>">
<input type="hidden" name="form_token" value="<?php echo $mylock ? $mylock->getToken() : ''; ?>">
<h2><?php echo __('Post a Reply');?></h2>
<input type="hidden" name="id" value="<?php echo $ticket->getId(); ?>">
<input type="hidden" name="a" value="reply">
Expand Down
6 changes: 6 additions & 0 deletions include/staff/ticket-open.inc.php
View file
Open in desktop
Expand Up @@ -54,11 +54,17 @@

if ($_POST)
$info['duedate'] = Format::date(strtotime($info['duedate']), false, false, 'UTC');

// Form Token
if (!$mylock)
$mylock = Lock::acquire($thisstaff->getId(), $cfg->getLockTime() ?: 15);
?>
<form action="tickets.php?a=open" method="post" class="save" enctype="multipart/form-data">
<?php csrf_token(); ?>
<input type="hidden" name="do" value="create">
<input type="hidden" name="a" value="open">
<input type="hidden" name="lockCode" value="<?php echo $mylock ? $mylock->getCode() : ''; ?>">
<input type="hidden" name="form_token" value="<?php echo $mylock ? $mylock->getToken() : ''; ?>">
<div style="margin-bottom:20px; padding-top:5px;">
<div class="pull-left flush-left">
<h2><?php echo __('Open a New Ticket');?></h2>
Expand Down
1 change: 1 addition & 0 deletions include/staff/ticket-view.inc.php
View file
Open in desktop
Expand Up @@ -715,6 +715,7 @@ class="icon-building icon-fixed-width"></i> <?php
<input type="hidden" name="msgId" value="<?php echo $msgId; ?>">
<input type="hidden" name="a" value="reply">
<input type="hidden" name="lockCode" value="<?php echo $mylock ? $mylock->getCode() : ''; ?>">
<input type="hidden" name="form_token" value="<?php echo $mylock ? $mylock->getToken() : ''; ?>">
<table style="width:100%" border="0" cellspacing="0" cellpadding="3">
<?php
if ($errors['reply']) {?>
Expand Down
9 changes: 9 additions & 0 deletions open.php
View file
Open in desktop
Expand Up @@ -19,6 +19,15 @@
$errors=array();
if ($_POST) {
$vars = $_POST;
// Validate Form Token to prevent form resubmissions
if (!$vars['form_token'] || !$vars['lock_code'])
$errors['err'] = __('Invalid Form Token');
if (!Lock::validateToken($vars['form_token'], $vars['lock_code'])) {
unset($_POST, $_REQUEST);
$errors['err'] = sprintf('%s %s',
__('Form Resubmission detected.'),
__('Action prohibited.'));
}
$vars['deptId']=$vars['emailId']=0; //Just Making sure we don't accept crap...only topicId is expected.
if ($thisclient) {
$vars['uid']=$thisclient->getId();
Expand Down
4 changes: 4 additions & 0 deletions scp/js/ticket.js
View file
Open in desktop
Expand Up @@ -178,6 +178,10 @@
// to the lock.code retrieved (if any)
if (lock.code)
$(this.options.lockInput, this.$element).val(lock.code);
// If there is an input with the name 'form_token', then set the value
// to the lock.signature retrieved (if any)
if (lock.token)
$('input[name=form_token]', this.$element).val(lock.token);

// Deadband renew to every 30 seconds
this.nextRenew = new Date().getTime() + 30000;
Expand Down
20 changes: 19 additions & 1 deletion scp/tickets.php
View file
Open in desktop
Expand Up @@ -149,6 +149,15 @@
$errors['err'] = __('Action denied. Contact admin for access');
} else {
$vars = $_POST;
// Validate Form Token to prevent form resubmissions
if (!$vars['form_token'] || !$vars['lockCode'])
$errors['err'] = __('Invalid Form Token');

if (!Lock::validateToken($vars['form_token'], $vars['lockCode']))
$errors['err'] = sprintf('%s %s',
__('Form Resubmission detected.'),
__('Action prohibited.'));

$vars['cannedattachments'] = $response_form->getField('attachments')->getClean();
$vars['response'] = ThreadEntryBody::clean($vars['response']);
if(!$vars['response'])
Expand Down Expand Up @@ -401,7 +410,16 @@
__('Contact admin for such access'));
} else {
$vars = $_POST;

// Validate Form Token to prevent form resubmissions
if (!$vars['form_token'] || !$vars['lockCode'])
$errors['err'] = __('Invalid Form Token');

if (!Lock::validateToken($vars['form_token'], $vars['lockCode'])) {
unset($_POST, $_REQUEST);
$errors['err'] = sprintf('%s %s',
__('Form Resubmission detected.'),
__('Action prohibited.'));
}
if ($vars['uid'] && !($user=User::lookup($vars['uid'])))
$vars['uid'] = 0;

Expand Down
9 changes: 9 additions & 0 deletions tickets.php
View file
Open in desktop
Expand Up @@ -77,6 +77,15 @@
if (!$_POST['message'])
$errors['message'] = __('Message required');

// Validate Form Token to prevent form resubmissions
if (!$_POST['form_token'] || !$_POST['lock_code'])
$errors['err'] = __('Invalid Form Token');

if (!Lock::validateToken($_POST['form_token'], $_POST['lock_code']))
$errors['err'] = sprintf('%s %s',
__('Form Resubmission detected.'),
__('Action prohibited.'));

if(!$errors) {
//Everything checked out...do the magic.
$vars = array(
Expand Down