diff --git a/file.php b/file.php index fda45974ed942b2e342a9a1efc8c0eaa06ce99c1..d62d588eb5942b0c0c1f5639ba22cf511f3a8884 100644 --- a/file.php +++ b/file.php @@ -26,6 +26,21 @@ if (!$_GET['key'] Http::response(404, __('Unknown or invalid file')); } +// Enforce security settings +if ($cfg->isAuthRequiredForFiles() && !$thisclient) { + if (!($U = StaffAuthenticationBackend::getUser())) { + // Try and determine if a staff is viewing this page + if (strpos($_SERVER['HTTP_REFERRER'], ROOT_PATH . 'scp/') !== false) { + $_SESSION['_staff']['auth']['dest'] = + '/' . ltrim($_SERVER['REQUEST_URI'], '/'); + Http::redirect(ROOT_PATH.'scp/login.php'); + } + else { + require 'secure.inc.php'; + } + } +} + // Validate session access hash - we want to make sure the link is FRESH! // and the user has access to the parent ticket!! if ($file->verifySignature($_GET['signature'], $_GET['expires'])) { diff --git a/include/ajax.sequence.php b/include/ajax.sequence.php index 299e8c3223d7a1d8262cc40c8919ac44fbde534f..ed292c51cc5e3fba7fd80d7f089d60fd27b82210 100644 --- a/include/ajax.sequence.php +++ b/include/ajax.sequence.php @@ -67,7 +67,7 @@ class SequenceAjaxAPI extends AjaxController { foreach ($_POST['seq'] as $id=>$info) { if (strpos($id, 'new-') === 0) { unset($info['id']); - $sequences[] = Sequence::create($info); + $sequences[] = new Sequence($info); } else { foreach ($sequences as $s) { diff --git a/include/class.auth.php b/include/class.auth.php index afda9c0e9a2b9610e44a5d363a8b58989fbf5424..1dc5a9ccf8f05172bd081929dfcb4d50c4104fa8 100644 --- a/include/class.auth.php +++ b/include/class.auth.php @@ -1061,7 +1061,7 @@ class AuthTokenAuthentication extends UserAuthenticationBackend { if (($ticket = Ticket::lookupByNumber($_GET['t'], $_GET['e'])) // Using old ticket auth code algo - hardcoded here because it // will be removed in ticket class in the upcoming rewrite - && !strcasecmp($_GET['a'], md5($ticket->getId() . $_GET['e'] . SECRET_SALT)) + && !strcasecmp($_GET['a'], md5($ticket->getId() . strtolower($_GET['e']) . SECRET_SALT)) && ($owner = $ticket->getOwner())) $user = new ClientSession($owner); } diff --git a/include/class.config.php b/include/class.config.php index 1cbb1407ca5bf9b5529af919f052eed68c16ea74..277741525d9bc3760b784052a362b53c53f6483a 100644 --- a/include/class.config.php +++ b/include/class.config.php @@ -213,6 +213,7 @@ class OsticketConfig extends Config { 'agent_avatar' => 'gravatar.mm', 'ticket_lock' => 2, // Lock on activity 'max_open_tickets' => 0, + 'files_req_auth' => 1, ); function __construct($section=null) { @@ -1156,6 +1157,7 @@ class OsticketConfig extends Config { 'autolock_minutes' => $vars['autolock_minutes'], 'enable_avatars' => isset($vars['enable_avatars']) ? 1 : 0, 'enable_richtext' => isset($vars['enable_richtext']) ? 1 : 0, + 'files_req_auth' => isset($vars['files_req_auth']) ? 1 : 0, )); } @@ -1399,6 +1401,10 @@ class OsticketConfig extends Config { return ($id) ? AttachmentFile::lookup((int) $id) : null; } + function isAuthRequiredForFiles() { + return $this->get('files_req_auth'); + } + function updatePagesSettings($vars, &$errors) { global $ost; diff --git a/include/class.dept.php b/include/class.dept.php index 397df89c480b7fafa20ba09a51cd6dfa41fca78c..93f92d754ad613b3afe4ad7b3b9655ba211b6d15 100644 --- a/include/class.dept.php +++ b/include/class.dept.php @@ -51,6 +51,9 @@ implements TemplateVariable { ); var $_members; + var $_primary_members; + var $_extended_members; + var $_groupids; var $config; @@ -182,16 +185,8 @@ implements TemplateVariable { 'onvacation' => 0, )); } - switch ($cfg->getAgentNameFormat()) { - case 'last': - case 'lastfirst': - case 'legal': - $members->order_by('lastname', 'firstname'); - break; - - default: - $members->order_by('firstname', 'lastname'); - } + + $members = Staff::nsort($members); if ($criteria) return $members; @@ -205,6 +200,42 @@ implements TemplateVariable { return $this->getMembers(array('available'=>1)); } + function getPrimaryMembers() { + + if (!isset($this->_primary_members)) { + $members = clone $this->getMembers(); + $members->filter(array('dept_id' =>$this->getId())); + $members = Staff::nsort($members); + $this->_primary_members = $members->all(); + } + + return $this->_primary_members; + } + + function getExtendedMembers() { + + if (!isset($this->_exended_members)) { + // We need a query set so we can sort the names + $members = StaffDeptAccess::objects(); + $members->filter(array('dept_id' => $this->getId())); + $members = Staff::nsort($members, 'staff__'); + $extended = array(); + foreach($members->all() as $member) { + if (!$member->staff) continue; + // Annoted the staff model with alerts and role + $extended[] = new AnnotatedModel($member->staff, array( + 'alerts' => $member->isAlertsEnabled(), + 'role_id' => $member->role_id, + ) + ); + } + + $this->_extended_members = $extended; + } + + return $this->_extended_members; + } + // Get members eligible members only function getAssignees() { @@ -305,10 +336,10 @@ implements TemplateVariable { if (is_object($staff)) $staff = $staff->getId(); - // Members are indexed by ID - $members = $this->getMembers(); + $members = $this->getMembers() ?: $this->members; - return ($members && isset($members[$staff])); + return ($members->findFirst(array( + 'staff_id' => $staff))); } function isPublic() { @@ -657,11 +688,21 @@ implements TemplateVariable { unset($dropped[$staff_id]); if (!$role_id || !Role::lookup($role_id)) $errors['members'][$staff_id] = __('Select a valid role'); - if (!$staff_id || !Staff::lookup($staff_id)) + if (!$staff_id || !($staff=Staff::lookup($staff_id))) $errors['members'][$staff_id] = __('No such agent'); + + if ($staff->dept_id == $this->id) { + + // If primary member then simply update the role. + if (($m = $this->members->findFirst(array( + 'staff_id' => $staff_id)))) + $m->role_id = $role_id; + continue; + } + $da = $this->extended->findFirst(array('staff_id' => $staff_id)); if (!isset($da)) { - $da = StaffDeptAccess::create(array( + $da = new StaffDeptAccess(array( 'staff_id' => $staff_id, 'role_id' => $role_id )); $this->extended->add($da); @@ -670,14 +711,24 @@ implements TemplateVariable { $da->role_id = $role_id; } $da->setAlerts($alerts); + } - if (!$errors && $dropped) { + + if ($errors) + return false; + + if ($dropped) { + $this->extended->saveAll(); $this->extended ->filter(array('staff_id__in' => array_keys($dropped))) ->delete(); $this->extended->reset(); } - return !$errors; + + // Save any role change. + $this->members->saveAll(); + + return true; } } diff --git a/include/class.dynamic_forms.php b/include/class.dynamic_forms.php index 513f014c95c71bdbab77ef2993eb7c2e9a2c9a69..6131a294b747e7328cfe3a1dcfd88cb7ae8a7673 100644 --- a/include/class.dynamic_forms.php +++ b/include/class.dynamic_forms.php @@ -1414,7 +1414,7 @@ class DynamicFormEntryAnswer extends VerySimpleModel { return true; } - function save($refetch) { + function save($refetch=false) { if ($this->dirty) unset($this->_value); return parent::save($refetch); diff --git a/include/class.email.php b/include/class.email.php index e0bdb64bd38fe3ffb82a15139d222cebffed18df..2b5f5e033973b65f0b9a5862e57af2311f6488db 100644 --- a/include/class.email.php +++ b/include/class.email.php @@ -216,7 +216,10 @@ class Email extends VerySimpleModel { /******* Static functions ************/ static function getIdByEmail($email) { - $qs = static::objects()->filter(array('email' => $email)) + $qs = static::objects()->filter(Q::any(array( + 'email' => $email, + 'userid' => $email + ))) ->values_flat('email_id'); $row = $qs->first(); diff --git a/include/class.faq.php b/include/class.faq.php index 027b25ef09815a24ab775ba55ed9e639393f3eb1..4eb1c2d83df46c4953ed0712c1cb03c5a8d913ba 100644 --- a/include/class.faq.php +++ b/include/class.faq.php @@ -297,23 +297,6 @@ class FAQ extends VerySimpleModel { return $att; } - function getAttachmentsLinks($separator=' ',$target='') { - - $str=''; - if ($attachments = $this->getLocalAttachments()->all()) { - foreach($attachments as $attachment ) { - /* The h key must match validation in file.php */ - if($attachment['size']) - $size=sprintf(' <small>(<i>%s</i>)</small>',Format::file_size($attachment['size'])); - - $str.=sprintf('<a class="Icon file no-pjax" href="%s" target="%s">%s</a>%s %s', - $attachment['download_url'], $target, Format::htmlchars($attachment['name']), $size, $separator); - - } - } - return $str; - } - function delete() { try { parent::delete(); diff --git a/include/class.forms.php b/include/class.forms.php index 6618220058e7be217ee3526e1359651d8dd7de80..627cd5a72c1cbfc99a826abd0a89503bbc61fd6f 100644 --- a/include/class.forms.php +++ b/include/class.forms.php @@ -3668,17 +3668,19 @@ class FileUploadWidget extends Widget { $maxfilesize = ($config['size'] ?: 1048576) / 1048576; $files = $F = array(); $new = array_fill_keys($this->field->getClean(), 1); - foreach ($attachments as $f) { - $F[] = $f->file; - unset($new[$f->id]); + foreach ($attachments as $a) { + $F[] = $a->file; + unset($new[$a->file_id]); } // Add in newly added files not yet saved (if redisplaying after an // error) if ($new) { $F = array_merge($F, AttachmentFile::objects() ->filter(array('id__in' => array_keys($new))) - ->all()); + ->all() + ); } + foreach ($F as $file) { $files[] = array( 'id' => $file->getId(), diff --git a/include/class.mailer.php b/include/class.mailer.php index cffaec5ed6f5c1b54923bb63c168e0c291f1980f..b124ffd65f9893b7cace527c4dc2f44e24124dd1 100644 --- a/include/class.mailer.php +++ b/include/class.mailer.php @@ -560,7 +560,8 @@ class Mailer { } //No SMTP or it failed....use php's native mail function. - $mail = mail::factory('mail'); + $args = array('-f '.$this->getEmail()->getEmail()); + $mail = mail::factory('mail', $args); // Ensure the To: header is properly encoded. $to = $headers['To']; $result = $mail->send($to, $headers, $body); diff --git a/include/class.mailfetch.php b/include/class.mailfetch.php index 325300f7b6e09e310324e6002d2f72844b75552e..7333992a47726d49b4cbd60eb64e682b3cea9287 100644 --- a/include/class.mailfetch.php +++ b/include/class.mailfetch.php @@ -757,7 +757,8 @@ class MailFetcher { // Validate and save immediately try { - $file['id'] = $fileField->uploadAttachment($file); + if ($f = $fileField->uploadAttachment($file)) + $file['id'] = $f->getId(); } catch (FileUploadError $ex) { $file['error'] = $file['name'] . ': ' . $ex->getMessage(); diff --git a/include/class.orm.php b/include/class.orm.php index c71f0e1e60f4e8e5e0aac99a8bfca98d89fcba8b..b784b6341e71df002a2f8bc136f12da32d7f5f0d 100644 --- a/include/class.orm.php +++ b/include/class.orm.php @@ -229,6 +229,17 @@ class ModelMeta implements ArrayAccess { return $this->fields; } + function getByPath($path) { + if (is_string($path)) + $path = explode('__', $path); + $root = $this; + foreach ($path as $P) { + list($root, ) = $root['joins'][$P]['fkey']; + $root = $root::getMeta(); + } + return $root; + } + /** * Create a new instance of the model, optionally hydrating it with the * given hash table. The constructor is not called, which leaves the @@ -429,11 +440,7 @@ class VerySimpleModel { } // Capture the object under the object's field name $this->ht[$field] = $value; - if ($value->__new__) - // save() will be performed when saving this object - $value = null; - else - $value = $value->get($j['fkey'][1]); + $value = $value->get($j['fkey'][1]); // Fall through to the standard logic below } // Capture the foreign key id value @@ -1138,7 +1145,7 @@ class QuerySet implements IteratorAggregate, ArrayAccess, Serializable, Countabl } function all() { - return $this->getIterator(); + return $this->getIterator()->asArray(); } function first() { @@ -1897,14 +1904,13 @@ extends ModelResultSet { } /** - * Slight edit to the standard ::next() iteration method which will skip - * deleted items. + * Slight edit to the standard iteration method which will skip deleted + * items. */ - function next() { - do { - parent::next(); - } - while ($this->valid() && $this->current()->__deleted__); + function getIterator() { + return new CallbackFilterIterator(parent::getIterator(), + function($i) { return !$i->__deleted__; } + ); } /** @@ -2115,13 +2121,20 @@ class SqlCompiler { // Call pushJoin for each segment in the join path. A new JOIN // fragment will need to be emitted and/or cached $joins = array(); - $push = function($p, $model) use (&$joins, &$path) { + $null = false; + $push = function($p, $model) use (&$joins, &$path, &$null) { $J = $model::getMeta('joins'); if (!($info = $J[$p])) { throw new OrmException(sprintf( 'Model `%s` does not have a relation called `%s`', $model, $p)); } + // Propogate LEFT joins through other joins. That is, if a + // multi-join expression is used, the first LEFT join should + // result in further joins also being LEFT + if (isset($info['null'])) + $null = $null || $info['null']; + $info['null'] = $null; $crumb = $path; $path = ($path) ? "{$path}__{$p}" : $p; $joins[] = array($crumb, $path, $model, $info); @@ -2246,9 +2259,22 @@ class SqlCompiler { // Handle relationship comparisons with model objects elseif ($value instanceof VerySimpleModel) { $criteria = array(); - foreach ($value->pk as $f=>$v) { - $f = $field . '__' . $f; - $criteria[$f] = $v; + // Avoid a join if possible. Use the local side of the + // relationship + if (count($value->pk) === 1) { + $path = explode('__', $field); + $relationship = array_pop($path); + $lmodel = $model::getMeta()->getByPath($path); + $local = $lmodel['joins'][$relationship]['local']; + $path = $path ? (implode('__', $path) . '__') : ''; + foreach ($value->pk as $v) { + $criteria["{$path}{$local}"] = $v; + } + } + else { + foreach ($value->pk as $f=>$v) { + $criteria["{$field}__{$f}"] = $v; + } } $filter[] = $this->compileQ(new Q($criteria), $model, $slot); } @@ -2269,7 +2295,7 @@ class SqlCompiler { elseif (is_callable($op)) $filter[] = call_user_func($op, $field, $value, $model); else - $filter[] = sprintf($op, $field, $this->input($value, $slot)); + $filter[] = sprintf($op, $field, $this->input($value)); } } $glue = $Q->isOred() ? ' OR ' : ' AND '; @@ -2536,7 +2562,7 @@ class MySqlCompiler extends SqlCompiler { * (string) token to be placed into the compiled SQL statement. This * is a colon followed by a number */ - function input($what, $slot=false, $model=false) { + function input($what, $model=false) { if ($what instanceof QuerySet) { $q = $what->getQuery(array('nosort'=>!($what->limit || $what->offset))); // Rewrite the parameter numbers so they fit the parameter numbers @@ -2868,7 +2894,7 @@ class MySqlCompiler extends SqlCompiler { $table = $model::getMeta('table'); $set = array(); foreach ($what as $field=>$value) - $set[] = sprintf('%s = %s', $this->quote($field), $this->input($value, false, $model)); + $set[] = sprintf('%s = %s', $this->quote($field), $this->input($value, $model)); $set = implode(', ', $set); list($where, $having) = $this->getWhereHavingClause($queryset); $joins = $this->getJoins($queryset); diff --git a/include/class.ostsession.php b/include/class.ostsession.php index 8581965eb5f538e343875a0e0ea2df94a02748c2..dbb5cf6312551abc4e24e4eb905eca53e0a74188 100644 --- a/include/class.ostsession.php +++ b/include/class.ostsession.php @@ -174,6 +174,8 @@ extends VerySimpleModel { class DbSessionBackend extends SessionBackend { + var $data = null; + function read($id) { try { $this->data = SessionData::objects()->filter([ @@ -200,7 +202,9 @@ extends SessionBackend { $ttl = $this && method_exists($this, 'getTTL') ? $this->getTTL() : SESSION_TTL; - assert($this->data->session_id == $id); + // Create a session data obj if not loaded. + if (!isset($this->data)) + $this->data = new SessionData(['session_id' => $id]); $this->data->session_data = $data; $this->data->session_expire = diff --git a/include/class.staff.php b/include/class.staff.php index 1241e58fcd144ca6fbfc3dd125e27c6c2dcee437..7f627982c4f1ab8f98f98a47df3817e38c182450 100644 --- a/include/class.staff.php +++ b/include/class.staff.php @@ -782,16 +782,7 @@ implements AuthenticatedUser, EmailContact, TemplateVariable { )); } - switch ($cfg->getAgentNameFormat()) { - case 'last': - case 'lastfirst': - case 'legal': - $members->order_by('lastname', 'firstname'); - break; - - default: - $members->order_by('firstname', 'lastname'); - } + $members = self::nsort($members); $users=array(); foreach ($members as $M) { @@ -805,6 +796,23 @@ implements AuthenticatedUser, EmailContact, TemplateVariable { return self::getStaffMembers(array('available'=>true)); } + static function nsort(QuerySet $qs, $path='', $format=null) { + global $cfg; + + $format = $format ?: $cfg->getAgentNameFormat(); + switch ($format) { + case 'last': + case 'lastfirst': + case 'legal': + $qs->order_by("{$path}lastname", "{$path}firstname"); + break; + default: + $qs->order_by("${path}firstname", "${path}lastname"); + } + + return $qs; + } + static function getIdByUsername($username) { $row = static::objects()->filter(array('username' => $username)) ->values_flat('staff_id')->first(); @@ -1143,10 +1151,6 @@ class StaffDeptAccess extends VerySimpleModel { 'joins' => array( 'dept' => array( 'constraint' => array('dept_id' => 'Dept.id'), - // FIXME: The ORM needs a way to support - // staff__dept_access__dept performing a LEFT join b/c - // staff__dept_access is LEFT - 'null' => true, ), 'staff' => array( 'constraint' => array('staff_id' => 'Staff.staff_id'), @@ -1460,6 +1464,12 @@ extends AbstractForm { list($clean['username'],) = preg_split('/[^\w.-]/u', $clean['email'], 2); if (mb_strlen($clean['username']) < 3 || Staff::lookup($clean['username'])) $clean['username'] = mb_strtolower($clean['firstname']); + + + // Inherit default dept's role as primary role + $clean['assign_use_pri_role'] = true; + + // Default permissions $clean['perms'] = array( User::PERM_CREATE, User::PERM_EDIT, diff --git a/include/class.task.php b/include/class.task.php index 62113a499ccef833e9b163a7cb5333e4272893bb..aee64f44c2620add950ae332fcdeac93a8f53c20 100644 --- a/include/class.task.php +++ b/include/class.task.php @@ -846,8 +846,8 @@ class Task extends TaskModel implements RestrictedAccess, Threadable { } } elseif ($cfg->alertDeptMembersONTaskTransfer() && !$this->isAssigned()) { // Only alerts dept members if the task is NOT assigned. - if ($members = $dept->getMembersForAlerts()->all()) - $recipients = array_merge($recipients, $members); + foreach ($dept->getMembersForAlerts() as $M) + $recipients[] = $M; } // Always alert dept manager?? diff --git a/include/class.thread.php b/include/class.thread.php index 07073b11cacd338c4379599f862abc09db661ec8..31b2c5c3df702e43a2855bf736c7d93141a6b24d 100644 --- a/include/class.thread.php +++ b/include/class.thread.php @@ -881,98 +881,77 @@ implements TemplateVariable { return $this->hasFlag(self::FLAG_SYSTEM); } - //Web uploads - caller is expected to format, validate and set any errors. - function uploadFiles($files) { - - if(!$files || !is_array($files)) - return false; + protected function normalizeFileInfo($files, $add_error=true) { + static $error_descriptions = array( + UPLOAD_ERR_INI_SIZE => /* @trans */ 'File is too large', + UPLOAD_ERR_FORM_SIZE => /* @trans */ 'File is too large', + UPLOAD_ERR_PARTIAL => 'The uploaded file was only partially uploaded.', + UPLOAD_ERR_NO_TMP_DIR => 'Missing a temporary folder.', + UPLOAD_ERR_CANT_WRITE => 'Failed to write file to disk.', + UPLOAD_ERR_EXTENSION => 'A PHP extension stopped the file upload.', + ); - $uploaded=array(); - foreach($files as $file) { - if($file['error'] && $file['error']==UPLOAD_ERR_NO_FILE) + if (!is_array($files)) + $files = array($files); + + $ids = array(); + foreach ($files as $name => $file) { + $F = array('inline' => is_array($file) && @$file['inline']); + + if (is_numeric($file)) + $fileId = $file; + elseif ($file instanceof AttachmentFile) + $fileId = $file->getId(); + elseif (is_array($file) && isset($file['id'])) + $fileId = $file['id']; + elseif ($AF = AttachmentFile::create($file)) + $fileId = $AF->getId(); + elseif ($add_error) { + $error = $file['error'] + ?: sprintf(_S('Unable to import attachment - %s'), + $name ?: $file['name']); + if (is_numeric($error) && isset($error_descriptions[$error])) { + $error = sprintf(_S('Error #%1$d: %2$s'), $error, + _S($error_descriptions[$error])); + } + // No need to log the missing-file error number + if ($error != UPLOAD_ERR_NO_FILE) + $this->getThread()->getObject()->logNote( + _S('File Import Error'), $error, _S('SYSTEM'), false); continue; - - if(!$file['error'] - && ($F=AttachmentFile::upload($file)) - && $this->saveAttachment($F)) - $uploaded[]= $F->getId(); - else { - if(!$file['error']) - $error = sprintf(__('Unable to upload file - %s'),$file['name']); - elseif(is_numeric($file['error'])) - $error ='Error #'.$file['error']; //TODO: Transplate to string. - else - $error = $file['error']; - /* - Log the error as an internal note. - XXX: We're doing it here because it will eventually become a thread post comment (hint: comments coming!) - XXX: logNote must watch for possible loops - */ - $this->getThread()->getObject()->logNote(__('File Upload Error'), $error, 'SYSTEM', false); } - } - - return $uploaded; - } - - function importAttachments(&$attachments) { + $F['id'] = $fileId; - if(!$attachments || !is_array($attachments)) - return null; - - $files = array(); - foreach($attachments as &$attachment) - if(($id=$this->importAttachment($attachment))) - $files[] = $id; + if (is_string($name)) + $F['name'] = $name; + if (isset($AF)) + $F['file'] = $AF; - return $files; - } + // Add things like the `key` field, but don't change current + // keys of the file array + if (is_array($file)) + $F += $file; - /* Emailed & API attachments handler */ - function importAttachment(&$attachment) { - - if(!$attachment || !is_array($attachment)) - return null; + // Key is required for CID rewriting in the body + if (!isset($F['key']) && ($AF = AttachmentFile::lookup($F['id']))) + $F['key'] = $AF->key; - $A=null; - if ($attachment['error'] || !($A=$this->saveAttachment($attachment))) { - $error = $attachment['error']; - if(!$error) - $error = sprintf(_S('Unable to import attachment - %s'), - $attachment['name']); - //FIXME: logComment here - $this->getThread()->getObject()->logNote( - _S('File Import Error'), $error, _S('SYSTEM'), false); + $ids[] = $F; } - - return $A; + return $ids; } /* Save attachment to the DB. @file is a mixed var - can be ID or file hashtable. */ - function saveAttachment(&$file, $name=false) { - - $inline = is_array($file) && @$file['inline']; - - if (is_numeric($file)) - $fileId = $file; - elseif ($file instanceof AttachmentFile) - $fileId = $file->getId(); - elseif ($F = AttachmentFile::create($file)) - $fileId = $F->getId(); - elseif (is_array($file) && isset($file['id'])) - $fileId = $file['id']; - else - return false; - + function createAttachment($file, $name=false) { $att = new Attachment(array( 'type' => 'H', 'object_id' => $this->getId(), - 'file_id' => $fileId, - 'inline' => $inline ? 1 : 0, + 'file_id' => $file['id'], + 'inline' => $file['inline'] ? 1 : 0, )); // Record varying file names in the attachment record @@ -984,7 +963,7 @@ implements TemplateVariable { } if ($filename) { // This should be a noop since the ORM caches on PK - $F = $F ?: AttachmentFile::lookup($fileId); + $F = @$file['file'] ?: AttachmentFile::lookup($file['id']); // XXX: This is not Unicode safe if ($F && 0 !== strcasecmp($F->name, $filename)) $att->name = $filename; @@ -995,13 +974,12 @@ implements TemplateVariable { return $att; } - function saveAttachments($files) { + function createAttachments(array $files) { $attachments = array(); - foreach ($files as $name=>$file) { - if (($A = $this->saveAttachment($file, $name))) + foreach ($files as $info) { + if ($A = $this->createAttachment($info, @$info['name'] ?: false)) $attachments[] = $A; } - return $attachments; } @@ -1326,29 +1304,6 @@ implements TemplateVariable { $vars['body'] = new TextThreadEntryBody($vars['body']); } - // Drop stripped images - if ($vars['attachments']) { - foreach ($vars['body']->getStrippedImages() as $cid) { - foreach ($vars['attachments'] as $i=>$a) { - if (@$a['cid'] && $a['cid'] == $cid) { - // Inline referenced attachment was stripped - unset($vars['attachments'][$i]); - } - } - } - } - - // Handle extracted embedded images (<img src="data:base64,..." />). - // The extraction has already been performed in the ThreadEntryBody - // class. Here they should simply be added to the attachments list - if ($atts = $vars['body']->getEmbeddedHtmlImages()) { - if (!is_array($vars['attachments'])) - $vars['attachments'] = array(); - foreach ($atts as $info) { - $vars['attachments'][] = $info; - } - } - if (!($body = $vars['body']->getClean())) $body = '-'; //Special tag used to signify empty message as stored. @@ -1377,11 +1332,6 @@ implements TemplateVariable { if (!($vars['staffId'] || $vars['userId'])) $entry->flags |= self::FLAG_SYSTEM; - if (!isset($vars['attachments']) || !$vars['attachments']) - // Otherwise, body will be configured in a block below (after - // inline attachments are saved and updated in the database) - $entry->body = $body; - if (isset($vars['pid'])) $entry->pid = $vars['pid']; // Check if 'reply_to' is in the $vars as the previous ThreadEntry @@ -1394,51 +1344,80 @@ implements TemplateVariable { if ($vars['ip_address']) $entry->ip_address = $vars['ip_address']; - if (!$entry->save()) - return false; - /************* ATTACHMENTS *****************/ + // Drop stripped email inline images + if ($vars['attachments']) { + foreach ($vars['body']->getStrippedImages() as $cid) { + foreach ($vars['attachments'] as $i=>$a) { + if (@$a['cid'] && $a['cid'] == $cid) { + // Inline referenced attachment was stripped + unset($vars['attachments'][$i]); + } + } + } + } - //Upload/save attachments IF ANY - if($vars['files']) //expects well formatted and VALIDATED files array. - $entry->uploadFiles($vars['files']); - - //Canned attachments... - if($vars['cannedattachments'] && is_array($vars['cannedattachments'])) - $entry->saveAttachments($vars['cannedattachments']); - - //Emailed or API attachments - if (isset($vars['attachments']) && $vars['attachments']) { - foreach ($vars['attachments'] as &$a) - if (isset($a['cid']) && $a['cid'] - && strpos($body, 'cid:'.$a['cid']) !== false) - $a['inline'] = true; - unset($a); - - $entry->importAttachments($vars['attachments']); - foreach ($vars['attachments'] as $a) { - // Change <img src="cid:"> inside the message to point to - // a unique hash-code for the attachment. Since the - // content-id will be discarded, only the unique hash-code - // will be available to retrieve the image later - if ($a['cid'] && $a['key']) { - $body = preg_replace('/src=("|\'|\b)(?:cid:)?' - . preg_quote($a['cid'], '/').'\1/i', - 'src="cid:'.$a['key'].'"', $body); + // Handle extracted embedded images (<img src="data:base64,..." />). + // The extraction has already been performed in the ThreadEntryBody + // class. Here they should simply be added to the attachments list + if ($atts = $vars['body']->getEmbeddedHtmlImages()) { + if (!is_array($vars['attachments'])) + $vars['attachments'] = array(); + foreach ($atts as $info) { + $vars['attachments'][] = $info; + } + } + + $attached_files = array(); + foreach (array( + // Web uploads and canned attachments + $vars['files'], $vars['cannedattachments'], + // Emailed or API attachments + $vars['attachments'], + // Inline images (attached to the draft) + Draft::getAttachmentIds($body), + ) as $files + ) { + if (is_array($files)) { + // Detect *inline* email attachments + foreach ($files as $i=>$a) { + if (isset($a['cid']) && $a['cid'] + && strpos($body, 'cid:'.$a['cid']) !== false) + $files[$i]['inline'] = true; + } + foreach ($entry->normalizeFileInfo($files) as $F) { + // Deduplicate on the `key` attribute. The key is + // necessary for the CID rewrite below + $attached_files[$F['key']] = $F; } } + } - $entry->body = $body; - if (!$entry->save()) - return false; + // Change <img src="cid:"> inside the message to point to a unique + // hash-code for the attachment. Since the content-id will be + // discarded, only the unique hash-code (key) will be available to + // retrieve the image later + foreach ($attached_files as $key => $a) { + if (isset($a['cid']) && $a['cid']) { + $body = preg_replace('/src=("|\'|\b)(?:cid:)?' + . preg_quote($a['cid'], '/').'\1/i', + 'src="cid:'.$key.'"', $body); + } } + // Set body here after it was rewritten to capture the stored file + // keys (above) + $entry->body = $body; + + if (!$entry->save()) + return false; + + // Associate the attached files with this new entry + $entry->createAttachments($attached_files); + // Save mail message id, if available $entry->saveEmailInfo($vars); - // Inline images (attached to the draft) - $entry->saveAttachments(Draft::getAttachmentIds($body)); - Signal::send('threadentry.created', $entry); return $entry; diff --git a/include/class.ticket.php b/include/class.ticket.php index a97aae045e88f2696d2bc9d78c1fe7bda41e9e64..8b75335a76874ac6e9f6b91b16b361573f8b2ffe 100644 --- a/include/class.ticket.php +++ b/include/class.ticket.php @@ -1818,8 +1818,8 @@ implements RestrictedAccess, Threadable { } elseif ($cfg->alertDeptMembersONOverdueTicket() && !$this->isAssigned()) { // Only alerts dept members if the ticket is NOT assigned. - if ($members = $dept->getMembersForAlerts()->all()) - $recipients = array_merge($recipients, $members); + foreach ($dept->getMembersForAlerts() as $M) + $recipients[] = $M; } // Always alert dept manager?? if ($cfg->alertDeptManagerONOverdueTicket() @@ -2082,8 +2082,8 @@ implements RestrictedAccess, Threadable { } elseif ($cfg->alertDeptMembersONTransfer() && !$this->isAssigned()) { // Only alerts dept members if the ticket is NOT assigned. - if ($members = $dept->getMembersForAlerts()->all()) - $recipients = array_merge($recipients, $members); + foreach ($dept->getMembersForAlerts() as $M) + $recipients[] = $M; } // Always alert dept manager?? @@ -3245,7 +3245,9 @@ implements RestrictedAccess, Threadable { } } - // Any error above is fatal. + + + // Any errors above are fatal. if ($errors) return 0; @@ -3427,38 +3429,39 @@ implements RestrictedAccess, Threadable { // Configure service-level-agreement for this ticket $ticket->selectSLAId($vars['slaId']); - // Assign ticket to staff or team (new ticket by staff) - if ($vars['assignId']) { - $asnform = $ticket->getAssignmentForm(array( - 'assignee' => $vars['assignId'], - 'comments' => $vars['note']) - ); - $e = array(); - $ticket->assign($asnform, $e); - } - else { - // Auto assign staff or team - auto assignment based on filter - // rules. Both team and staff can be assigned - if ($vars['staffId']) - $ticket->assignToStaff($vars['staffId'], false); - if ($vars['teamId']) - // No team alert if also assigned to an individual agent - $ticket->assignToTeam($vars['teamId'], false, !$vars['staffId']); + // Set status + $status = TicketStatus::lookup($statusId); + if (!$status || !$ticket->setStatus($status, false, $errors, + !strcasecmp($origin, 'staff'))) { + // Tickets _must_ have a status. Forceably set one here + $ticket->setStatusId($cfg->getDefaultTicketStatusId()); + } + + // Only do assignment if the ticket is in an open state + if ($ticket->isOpen()) { + // Assign ticket to staff or team (new ticket by staff) + if ($vars['assignId']) { + $asnform = $ticket->getAssignmentForm(array( + 'assignee' => $vars['assignId'], + 'comments' => $vars['note']) + ); + $e = array(); + $ticket->assign($asnform, $e); + } + else { + // Auto assign staff or team - auto assignment based on filter + // rules. Both team and staff can be assigned + if ($vars['staffId']) + $ticket->assignToStaff($vars['staffId'], false); + if ($vars['teamId']) + // No team alert if also assigned to an individual agent + $ticket->assignToTeam($vars['teamId'], false, !$vars['staffId']); + } } // Update the estimated due date in the database $ticket->updateEstDueDate(); - // Apply requested status — this should be done AFTER assignment, - // because if it is requested to be closed, it should not cause the - // ticket to be reopened for assignment. - if ($statusId) { - if (!$ticket->setStatus($statusId, false, $errors, false)) { - // Tickets _must_ have a status. Forceably set one here - $ticket->setStatusId($cfg->getDefaultTicketStatusId()); - } - } - /********** double check auto-response ************/ //Override auto responder if the FROM email is one of the internal emails...loop control. if($autorespond && (Email::getIdByEmail($ticket->getEmail()))) diff --git a/include/class.translation.php b/include/class.translation.php index 2d503baf654697a0be78d8371b7a614cd095a00f..2e864b39383fd0dc26b4244eb4f68e304eb532d8 100644 --- a/include/class.translation.php +++ b/include/class.translation.php @@ -1033,7 +1033,7 @@ class CustomDataTranslation extends VerySimpleModel { $criteria['lang'] = $lang; try { - return static::objects()->filter($criteria)->all()->asArray(); + return static::objects()->filter($criteria)->all(); } catch (OrmException $e) { // Translation table might not exist yet — happens on the upgrader diff --git a/include/client/templates/sidebar.tmpl.php b/include/client/templates/sidebar.tmpl.php index 682f88d7bf9ee8e5ac10070049630291eba1c07d..09f9bd689c80c7be183b95bde2de14084c3ce031 100644 --- a/include/client/templates/sidebar.tmpl.php +++ b/include/client/templates/sidebar.tmpl.php @@ -20,7 +20,7 @@ $BUTTONS = isset($BUTTONS) ? $BUTTONS : true; <?php } ?> <div class="content"><?php $faqs = FAQ::getFeatured()->select_related('category')->limit(5); - if (count($faqs)) { ?> + if ($faqs->all()) { ?> <section><div class="header"><?php echo __('Featured Questions'); ?></div> <?php foreach ($faqs as $F) { ?> <div><a href="<?php echo ROOT_PATH; ?>kb/faq.php?id=<?php diff --git a/include/client/templates/thread-entries.tmpl.php b/include/client/templates/thread-entries.tmpl.php index 1f2c1eed4e3144259475b76b1dd9054dbeded451..edd71b4a8a0e927a34d067537522b1b8f487f72c 100644 --- a/include/client/templates/thread-entries.tmpl.php +++ b/include/client/templates/thread-entries.tmpl.php @@ -2,7 +2,7 @@ $events = $events ->filter(array('state__in' => array('created', 'closed', 'reopened', 'edited', 'collab'))) ->order_by('id'); -$events = new IteratorIterator($events->all()); +$events = new IteratorIterator($events->getIterator()); $events->rewind(); $event = $events->current(); diff --git a/include/i18n/en_US/help/tips/settings.system.yaml b/include/i18n/en_US/help/tips/settings.system.yaml index 53ed8c65e029731ba2ec0f9857de8894d479cd53..28b069775ba6e2a8b8eafc4c93ae94fc96a002e6 100644 --- a/include/i18n/en_US/help/tips/settings.system.yaml +++ b/include/i18n/en_US/help/tips/settings.system.yaml @@ -142,7 +142,7 @@ default_storage_bk: title: File Storage Backend content: > Choose how attachments are stored. - <br<br> + <br><br> Additional storage backends can be added by installing storage plugins max_file_size: @@ -155,3 +155,14 @@ max_file_size: links: - title: PHP ini settings href: "http://php.net/manual/en/ini.core.php#ini.upload-max-filesize" + +files_req_auth: + title: Require Login + content: > + Enable this setting to forbid serving attachments to unauthenticated + users. That is, users must sign into the system (both end users and + agents), in order to view attachments. + <br><br> + From a security perspective, be aware that the user's browser may + retain previously-viewed files in its cache. Furthermore, all file + links on your helpdesk automatically expire after about 24 hours. diff --git a/include/staff/department.inc.php b/include/staff/department.inc.php index a3822f68b9e729400bdf1104ecf6960321b6172d..4d2cbd5be9522cd4f25069af42e326f3eaa4543e 100644 --- a/include/staff/department.inc.php +++ b/include/staff/department.inc.php @@ -318,7 +318,7 @@ $info = Format::htmlchars(($errors && $_POST) ? $_POST : $info); <div id="access" class="hidden tab_content"> <table class="two-column table" width="100%"> <tbody> - <tr class="header"> + <tr class="header" id="primary-members"> <td colspan="2"> <?php echo __('Department Members'); ?> <div><small> @@ -326,6 +326,23 @@ $info = Format::htmlchars(($errors && $_POST) ? $_POST : $info); </small></div> </td> </tr> + <?php + if (!count($dept->members)) { ?> + <tr><td colspan=2><em><?php + echo __('Department does not have primary members'); ?> + </em> </td> + </tr> + <?php + } ?> + </tbody> + <tbody> + <tr class="header" id="extended-access-members"> + <td colspan="2"> + <div><small> + <?php echo __('Agents who have extended access to this department'); ?> + </small></div> + </td> + </tr> <?php $agents = Staff::getStaffMembers(); foreach ($dept->getMembers() as $member) { @@ -387,26 +404,26 @@ foreach ($dept->getMembers() as $member) { <script type="text/javascript"> var addAccess = function(staffid, name, role, alerts, primary, error) { + if (!staffid) return; var copy = $('#member_template').clone(); - + var target = (primary) ? 'extended-access-members' : 'add_extended_access'; copy.find('td:first').append(document.createTextNode(name)); if (primary) { - copy.find('td:first').append($('<span class="faded">').text(primary)); - copy.find('td:last').empty(); + copy.find('a.drop-membership').remove(); } - else { copy.find('[data-name^=member_alerts]') .attr('name', 'member_alerts['+staffid+']') - .prop('checked', alerts); + .prop('disabled', (primary)) + .prop('checked', primary || alerts); copy.find('[data-name^=member_role]') .attr('name', 'member_role['+staffid+']') .val(role || 0); copy.find('[data-name=members\\[\\]]') .attr('name', 'members[]') .val(staffid); - } - copy.attr('id', '').show().insertBefore($('#add_extended_access')); + + copy.attr('id', '').show().insertBefore($('#'+target)); copy.removeClass('hidden') if (error) $('<div class="error">').text(error).appendTo(copy.find('td:last')); @@ -433,25 +450,27 @@ $('#add_extended_access').find('button').on('click', function() { <?php if ($dept) { - $members = $dept->members->all(); - foreach ($dept->extended as $x) { - if (!$x->staff) - continue; - $members[] = new AnnotatedModel($x->staff, array( - 'alerts' => $x->isAlertsEnabled(), - 'role_id' => $x->role_id, - )); + // Primary members + foreach ($dept->getPrimaryMembers() as $member) { + $primary = $member->dept_id == $info['id']; + echo sprintf('addAccess(%d, %s, %d, %d, %d, %s);', + $member->getId(), + JsonDataEncoder::encode((string) $member->getName()), + $member->role_id, + $member->get('alerts', 0), + ($member->dept_id == $info['id']) ? 1 : 0, + JsonDataEncoder::encode($errors['members'][$member->staff_id]) + ); } - usort($members, function($a, $b) { return strcmp($a->getName(), $b->getName()); }); - foreach ($members as $member) { - $primary = $member->dept_id == $info['id']; - echo sprintf('addAccess(%d, %s, %d, %d, %s, %s);', + // Extended members. + foreach ($dept->getExtendedMembers() as $member) { + echo sprintf('addAccess(%d, %s, %d, %d, %d, %s);', $member->getId(), JsonDataEncoder::encode((string) $member->getName()), $member->role_id, $member->get('alerts', 0), - JsonDataEncoder::encode($primary ? ' — '.__('Primary') : ''), + 0, JsonDataEncoder::encode($errors['members'][$member->staff_id]) ); } diff --git a/include/staff/dynamic-form.inc.php b/include/staff/dynamic-form.inc.php index f1bba83968aa5b116f28084aef6ba9ca0d9fb02c..775e058792ccb28790e1b21b55ba479734879105 100644 --- a/include/staff/dynamic-form.inc.php +++ b/include/staff/dynamic-form.inc.php @@ -114,14 +114,16 @@ if ($form && count($langs) > 1) { ?> </tbody> </table> <table class="form_table" width="940" border="0" cellspacing="0" cellpadding="2"> - <?php if ($form && $form->get('type') == 'T') { ?> + <?php if ($form && $form->get('type') == 'T') { + $uform = UserForm::objects()->one(); + ?> <thead> <tr> <th colspan="7"> <em><strong><?php echo __('User Information Fields'); ?></strong> <?php echo sprintf(__('(These fields are requested for new tickets via the %s form)'), - UserForm::objects()->one()->get('title')); ?></em> + $uform->get('title')); ?></em> </th> </tr> <tr> @@ -135,9 +137,8 @@ if ($form && count($langs) > 1) { ?> </thead> <tbody> <?php - $uform = UserForm::objects()->all(); $ftypes = FormField::allTypes(); - foreach ($uform[0]->getFields() as $f) { + foreach ($uform->getFields() as $f) { if (!$f->isVisibleToUsers()) continue; ?> <tr> diff --git a/include/staff/helptopics.inc.php b/include/staff/helptopics.inc.php index 3116e5016be5744958619a0e3240fcc80c0c3a8c..d356542c4a0d1f1165f76cab2a29e0ae31c955ab 100644 --- a/include/staff/helptopics.inc.php +++ b/include/staff/helptopics.inc.php @@ -100,7 +100,8 @@ $order_by = 'sort'; $names[$topic->getId()] = $topic->getFullName(); $topics[$topic->getId()] = $topic; } - $names = Internationalization::sortKeyedList($names); + if ($cfg->getTopicSortMode() != 'm') + $names = Internationalization::sortKeyedList($names); $defaultDept = $cfg->getDefaultDept(); $defaultPriority = $cfg->getDefaultPriority(); diff --git a/include/staff/settings-system.inc.php b/include/staff/settings-system.inc.php index 6560e7c2074266aa4d69bb47c421cc9594759f42..6d03642201a509181a3d623691e88806298213ae 100644 --- a/include/staff/settings-system.inc.php +++ b/include/staff/settings-system.inc.php @@ -359,6 +359,16 @@ $gmtime = Misc::gmtime(); <div class="error"><?php echo $errors['max_file_size']; ?></div> </td> </tr> + <tr> + <td width="180"><?php echo __('Login required');?>:</td> + <td> + <input type="checkbox" name="files_req_auth" <?php + if ($config['files_req_auth']) echo 'checked="checked"'; + ?> /> + <?php echo __('Require login to view any attachments'); ?> + <i class="help-tip icon-question-sign" href="#files_req_auth"></i> + </td> + </tr> </tbody> </table> <p style="text-align:center;"> diff --git a/include/staff/staff.inc.php b/include/staff/staff.inc.php index bf2f925f4f522571e9b8d7f5125a048e28e4c2fb..c063948b75a68ed6e7ae6c9c4cac1adc8f1f410c 100644 --- a/include/staff/staff.inc.php +++ b/include/staff/staff.inc.php @@ -214,12 +214,13 @@ if (count($bks) > 1) { <tbody> <tr class="header"> <th colspan="3"> - <?php echo __('Primary Department and Role'); ?> - <span class="error">*</span> + <?php echo __('Access'); ?> <div><small><?php echo __( - "Select the departments the agent is allowed to access and optionally select an effective role." + "Select the departments the agent is allowed to access and the corresponding effective role." ); ?> - </small></div> + </small></div><br> + <div><?php echo __('Primary Department'); ?> <span + class="error">*</span></div> </th> </tr> <tr> @@ -256,7 +257,7 @@ if (count($bks) > 1) { if ($staff->usePrimaryRoleOnAssignment()) echo 'checked="checked"'; ?> /> - <?php echo __('Fall back to primary role on assigned tickets'); ?> + <?php echo __('Fall back to primary role on assignments'); ?> <i class="icon-question-sign help-tip" href="#primary_role_on_assign"></i> </label> diff --git a/include/staff/templates/thread-entries.tmpl.php b/include/staff/templates/thread-entries.tmpl.php index 8786472e5f16b52f59eb3e59857254ee3c8e1a3f..0d15aa0c9f15dfd410a9c60914a5c17ef67d3ab5 100644 --- a/include/staff/templates/thread-entries.tmpl.php +++ b/include/staff/templates/thread-entries.tmpl.php @@ -10,7 +10,7 @@ $cmp = function ($a, $b) use ($sort) { }; $events = $events->order_by($sort); -$events = new IteratorIterator($events->all()); +$events = new IteratorIterator($events->getIterator()); $events->rewind(); $event = $events->current(); $htmlId = $options['html-id'] ?: ('thread-'.$this->getId()); diff --git a/include/staff/ticket-view.inc.php b/include/staff/ticket-view.inc.php index ea85e04f357c6f31e7f9aebd5d7912fa75ad913f..fc37d5912a39b5bd6f8254a64772a8d9840e1f16 100644 --- a/include/staff/ticket-view.inc.php +++ b/include/staff/ticket-view.inc.php @@ -310,7 +310,7 @@ if($ticket->isOverdue()) <span id="user-<?php echo $ticket->getOwnerId(); ?>-email"><?php echo $ticket->getEmail(); ?></span> </td> </tr> -<?php if ($user->getOrgId()) { ?> +<?php if ($user->getOrganization()) { ?> <tr> <th><?php echo __('Organization'); ?>:</th> <td><i class="icon-building"></i> diff --git a/include/upgrader/streams/core/ed60ba20-934954de.patch.sql b/include/upgrader/streams/core/ed60ba20-934954de.patch.sql index 39c210f4d1d7d9ffa65150243cb4cf81db96d552..9d3428177b9215d38992920c45b1ad6c6f548efe 100644 --- a/include/upgrader/streams/core/ed60ba20-934954de.patch.sql +++ b/include/upgrader/streams/core/ed60ba20-934954de.patch.sql @@ -18,10 +18,12 @@ UPDATE `%TABLE_PREFIX%filter_rule` -- Previously there was no primary key on the %ticket_email_info table, so -- clean up any junk records before adding one ALTER TABLE `%TABLE_PREFIX%ticket_email_info` - CHANGE `message_id` `thread_id` int(11) unsigned NOT NULL, DROP INDEX `message_id`, ADD INDEX `email_mid` (`email_mid`); +ALTER TABLE `%TABLE_PREFIX%ticket_email_info` + CHANGE `message_id` `thread_id` int(11) unsigned NOT NULL; + -- [#386](https://github.com/osTicket/osTicket-1.8/issues/386) UPDATE `%TABLE_PREFIX%email_template` SET `body` = REPLACE(`body`, '%{recipient}', '%{recipient.name}'); diff --git a/setup/test/tests/class.php_analyze.php b/setup/test/tests/class.php_analyze.php index 53e0a85ab342851a322b102ecde9899a6361cee0..1ed22fe1a90dbac3303953f1ab01dbd9f53c866c 100644 --- a/setup/test/tests/class.php_analyze.php +++ b/setup/test/tests/class.php_analyze.php @@ -201,11 +201,10 @@ class SourceAnalyzer extends Test { // PHP does not automatically nest scopes. Variables // available inside the closure must be explictly defined. // Therefore, there is no need to pass the current scope. - // However, $this is not allowed inside inline functions - // unless declared in the use () parameters. + // As of PHP 5.4, $this is added to the closure automatically $this->traverseFunction( array($token[2], $function['file']), - array('allow_this'=>false)); + array('allow_this'=>true)); break; case T_STATIC: $c = current($this->tokens);