From b8acaae1c52365ed1b38e50e72d1daef6ab79655 Mon Sep 17 00:00:00 2001 From: nathan Date: Mon, 16 Aug 2021 13:07:17 -0600 Subject: [PATCH] * Api: Fix changes in history log had a hash instead of user if the change was made after a share was opened. --- api/src/Storage/History.php | 204 ++++++++++++++++++++++-------------- 1 file changed, 123 insertions(+), 81 deletions(-) diff --git a/api/src/Storage/History.php b/api/src/Storage/History.php index 180aa9c49e..25a9aa5171 100644 --- a/api/src/Storage/History.php +++ b/api/src/Storage/History.php @@ -57,12 +57,12 @@ class History * * @param string $appname app name this instance operates on */ - function __construct($appname='',$user=null) + function __construct($appname = '', $user = null) { $this->appname = $appname ? $appname : $GLOBALS['egw_info']['flags']['currentapp']; $this->user = !is_null($user) ? $user : $GLOBALS['egw_info']['user']['account_id']; - if (is_object($GLOBALS['egw_setup']->db)) + if(is_object($GLOBALS['egw_setup']->db)) { $this->db = $GLOBALS['egw_setup']->db; } @@ -82,11 +82,11 @@ class History { $where = array('history_appname' => $this->appname); - if (is_array($record_id) || is_numeric($record_id)) + if(is_array($record_id) || is_numeric($record_id)) { $where['history_record_id'] = $record_id; } - $this->db->delete(self::TABLE,$where,__LINE__,__FILE__); + $this->db->delete(self::TABLE, $where, __LINE__, __FILE__); return $this->db->affected_rows(); } @@ -100,14 +100,14 @@ class History { $where = array( 'history_appname' => $this->appname, - 'history_status' => $status + 'history_status' => $status ); - if (is_array($record_id) || is_numeric($record_id)) + if(is_array($record_id) || is_numeric($record_id)) { $where['history_record_id'] = $record_id; } - $this->db->delete(self::TABLE,$where,__LINE__,__FILE__); + $this->db->delete(self::TABLE, $where, __LINE__, __FILE__); return $this->db->affected_rows(); } @@ -120,16 +120,13 @@ class History * @param string $new_value new value * @param string $old_value old value */ - function add($status,$record_id,$new_value,$old_value) + function add($status, $record_id, $new_value, $old_value) { - if ($new_value != $old_value) + if($new_value != $old_value) { - $share_with = ''; - foreach(isset($GLOBALS['egw']->sharing) ? $GLOBALS['egw']->sharing : [] as $token => $share_obj) - { - $share_with .= $share_obj->get_share_with(); - } - $this->db->insert(self::TABLE,array( + $share_with = static::get_share_with($this->appname, $record_id); + + $this->db->insert(self::TABLE, array( 'history_record_id' => $record_id, 'history_appname' => $this->appname, 'history_owner' => $this->user, @@ -137,9 +134,9 @@ class History 'history_new_value' => $new_value, 'history_old_value' => $old_value, 'history_timestamp' => time(), - 'sessionid' => $GLOBALS['egw']->session->sessionid_access_log, + 'sessionid' => $GLOBALS['egw']->session->sessionid_access_log, 'share_email' => $share_with, - ),false,__LINE__,__FILE__); + ), false, __LINE__, __FILE__); } } @@ -148,14 +145,10 @@ class History */ public static function static_add($appname, $id, $user, $field_code, $new_value, $old_value = '') { - if ($new_value != $old_value) + if($new_value != $old_value) { - $share_with = ''; - foreach(isset($GLOBALS['egw']->sharing) ? $GLOBALS['egw']->sharing : [] as $token => $share_obj) - { - $share_with .= $share_obj->get_share_with(); - } - $GLOBALS['egw']->db->insert(self::TABLE,array( + $share_with = static::get_share_with($appname, $id); + $GLOBALS['egw']->db->insert(self::TABLE, array( 'history_record_id' => $id, 'history_appname' => $appname, 'history_owner' => (int)$user, @@ -163,12 +156,38 @@ class History 'history_new_value' => $new_value, 'history_old_value' => $old_value, 'history_timestamp' => time(), - 'sessionid' => $GLOBALS['egw']->session->sessionid_access_log, + 'sessionid' => $GLOBALS['egw']->session->sessionid_access_log, 'share_email' => $share_with, - ),false,__LINE__,__FILE__); + ), false, __LINE__, __FILE__); } } + /** + * If a record was accessed via a share, we want to record who the entry was shared with, rather than the current + * user. Since multiple shares can be active at once, and they might not be for the current entry, we check to + * see if the given entry was accessed via a share, and which share was used. + * The share's share_with is recorded into the history for some hope of tracking who made the change. + * share_with is a list of email addresses, and may be empty. + * + * @param $appname + * @param $id + * + * @return string + */ + static function get_share_with($appname, $id) + { + $share_with = ''; + foreach(isset($GLOBALS['egw']->sharing) ? $GLOBALS['egw']->sharing : [] as $token => $share_obj) + { + // Make sure share is of the correct type to access an entry, and it is the correct entry + if($share_obj instanceof Api\Link\Sharing && "$appname::$id" === $share_obj['share_path']) + { + $share_with .= $share_obj->get_share_with(); + } + } + return $share_with; + } + /** * Search history-log * @@ -177,13 +196,16 @@ class History * @param string $sort ='DESC' * @param int $limit =null only return this many entries * @return array of arrays with keys id, record_id, appname, owner (account_id), status, new_value, old_value, - * timestamp (Y-m-d H:i:s in servertime), user_ts (timestamp in user-time) + * timestamp (Y-m-d H:i:s in servertime), user_ts (timestamp in user-time) */ - function search($filter,$order='history_id',$sort='DESC',$limit=null) + function search($filter, $order = 'history_id', $sort = 'DESC', $limit = null) { - if (!is_array($filter)) $filter = is_numeric($filter) ? array('history_record_id' => $filter) : array(); + if(!is_array($filter)) + { + $filter = is_numeric($filter) ? array('history_record_id' => $filter) : array(); + } - if (!$order || !preg_match('/^[a-z0-9_]+$/i',$order) || !preg_match('/^(asc|desc)?$/i',$sort)) + if(!$order || !preg_match('/^[a-z0-9_]+$/i', $order) || !preg_match('/^(asc|desc)?$/i', $sort)) { $orderby = 'ORDER BY history_id DESC'; } @@ -193,23 +215,30 @@ class History } foreach($filter as $col => $value) { - if (!is_numeric($col) && substr($col,0,8) != 'history_') + if(!is_numeric($col) && substr($col, 0, 8) != 'history_') { - $filter['history_'.$col] = $value; + $filter['history_' . $col] = $value; unset($filter[$col]); } } - if (!isset($filter['history_appname'])) $filter['history_appname'] = $this->appname; + if(!isset($filter['history_appname'])) + { + $filter['history_appname'] = $this->appname; + } // do not try to read all history entries of an app - if (!$filter['history_record_id']) return array(); + if(!$filter['history_record_id']) + { + return array(); + } $rows = array(); foreach($this->db->select(self::TABLE, '*', $filter, __LINE__, __FILE__, - isset($limit) ? 0 : false, $orderby, 'phpgwapi', $limit) as $row) + isset($limit) ? 0 : false, $orderby, 'phpgwapi', $limit + ) as $row) { $row['user_ts'] = $this->db->from_timestamp($row['history_timestamp']) + 3600 * $GLOBALS['egw_info']['user']['preferences']['common']['tz_offset']; - $rows[] = Api\Db::strip_array_keys($row,'history_'); + $rows[] = Api\Db::strip_array_keys($row, 'history_'); } return $rows; } @@ -227,54 +256,58 @@ class History $rows = array(); $filter['history_appname'] = $query['appname']; $filter['history_record_id'] = $query['record_id']; - if(is_array($query['colfilter'])) { - foreach($query['colfilter'] as $column => $value) { + if(is_array($query['colfilter'])) + { + foreach($query['colfilter'] as $column => $value) + { $filter[$column] = $value; } } // filter out private (or no longer defined) custom fields - if ($filter['history_appname']) + if($filter['history_appname']) { $to_or[] = "history_status NOT LIKE '#%'"; // explicitly allow "##" used to store iCal/vCard X-attributes - if (in_array($filter['history_appname'], array('calendar','infolog','addressbook'))) + if(in_array($filter['history_appname'], array('calendar', 'infolog', 'addressbook'))) { $to_or[] = "history_status LIKE '##%'"; } - if (($cfs = Customfields::get($filter['history_appname']))) + if(($cfs = Customfields::get($filter['history_appname']))) { - $to_or[] = 'history_status IN ('.implode(',', array_map(function($str) - { - return $GLOBALS['egw']->db->quote('#'.$str); - }, array_keys($cfs))).')'; + $to_or[] = 'history_status IN (' . implode(',', array_map(function ($str) + { + return $GLOBALS['egw']->db->quote('#' . $str); + }, array_keys($cfs)) + ) . ')'; } - $filter[] = '('.implode(' OR ', $to_or).')'; + $filter[] = '(' . implode(' OR ', $to_or) . ')'; } $_query = array(array( - 'table' => self::TABLE, - 'cols' => array( - 'history_id', - 'history_record_id', - 'history_appname', - 'history_owner', - 'history_status', - 'history_new_value', - 'history_timestamp', - 'history_old_value', - 'share_email' - ), - 'where' => $filter, - )); + 'table' => self::TABLE, + 'cols' => array( + 'history_id', + 'history_record_id', + 'history_appname', + 'history_owner', + 'history_status', + 'history_new_value', + 'history_timestamp', + 'history_old_value', + 'share_email' + ), + 'where' => $filter, + )); // Add in files, if possible if($GLOBALS['egw_info']['user']['apps']['filemanager'] && ($sqlfs_sw = new Api\Vfs\Sqlfs\StreamWrapper()) && - ($file = $sqlfs_sw->url_stat("/apps/{$query['appname']}/{$query['record_id']}",STREAM_URL_STAT_LINK))) + ($file = $sqlfs_sw->url_stat("/apps/{$query['appname']}/{$query['record_id']}", STREAM_URL_STAT_LINK))) { $_query[] = array( 'table' => Api\Vfs\Sqlfs\StreamWrapper::TABLE, - 'cols' =>array('fs_id', 'fs_dir', "'filemanager'",'COALESCE(fs_modifier,fs_creator)',"'~file~'",'fs_name','fs_modified', 'fs_mime', '"" AS share_email'), + 'cols' => array('fs_id', 'fs_dir', "'filemanager'", 'COALESCE(fs_modifier,fs_creator)', "'~file~'", + 'fs_name', 'fs_modified', 'fs_mime', '"" AS share_email'), 'where' => array('fs_dir' => $file['ino']) ); } @@ -290,20 +323,21 @@ class History $row['user_ts'] = $GLOBALS['egw']->db->from_timestamp($row['history_timestamp']) + 3600 * $GLOBALS['egw_info']['user']['preferences']['common']['tz_offset']; // Explode multi-part values - foreach(array('history_new_value','history_old_value') as $field) + foreach(array('history_new_value', 'history_old_value') as $field) { - if(strpos($row[$field],Tracking::ONE2N_SEPERATOR) !== false) + if(strpos($row[$field], Tracking::ONE2N_SEPERATOR) !== false) { - $row[$field] = explode(Tracking::ONE2N_SEPERATOR,$row[$field]); + $row[$field] = explode(Tracking::ONE2N_SEPERATOR, $row[$field]); } } - if ($row['history_old_value'] !== Tracking::DIFF_MARKER && ( - static::needs_diff($row['history_status'], $row['history_old_value']) || - static::needs_diff($row['history_status'], $row['history_old_value']) - )) + if($row['history_old_value'] !== Tracking::DIFF_MARKER && ( + static::needs_diff($row['history_status'], $row['history_old_value']) || + static::needs_diff($row['history_status'], $row['history_old_value']) + )) { // Larger text stored with full old / new value - calculate diff and just send that - $diff = new \Horde_Text_Diff('auto', array(explode("\n",$row['history_old_value']), explode("\n",$row['history_new_value']))); + $diff = new \Horde_Text_Diff('auto', array(explode("\n", $row['history_old_value']), + explode("\n", $row['history_new_value']))); $renderer = new \Horde_Text_Diff_Renderer_Unified(); $row['history_new_value'] = $renderer->render($diff); $row['history_old_value'] = Tracking::DIFF_MARKER; @@ -330,18 +364,26 @@ class History $rows[$new_version]['old_value'] = $row['history_new_value']; } } - $rows[] = Api\Db::strip_array_keys($row,'history_'); + + // TODO: This is just here to hide bad values before we clean them with an update. If you're here, remove this IF block + // Clear invalid share_email values + if($row['share_email'] && stripos($row['share_email'], '@') === false) + { + $row['share_email'] = ''; + } + + $rows[] = Api\Db::strip_array_keys($row, 'history_'); } - $total = $GLOBALS['egw']->db->union($_query,__LINE__,__FILE__)->NumRows(); + $total = $GLOBALS['egw']->db->union($_query, __LINE__, __FILE__)->NumRows(); // allow to hook into get_rows of other apps Api\Hooks::process(array( - 'hook_location' => 'etemplate2_history_get_rows', - 'get_rows' => __METHOD__, - 'value' => &$query, - 'rows' => &$rows, - 'total' => &$total, - ), array(), true); // true = no permission check + 'hook_location' => 'etemplate2_history_get_rows', + 'get_rows' => __METHOD__, + 'value' => &$query, + 'rows' => &$rows, + 'total' => &$total, + ), array(), true); // true = no permission check return $total; } @@ -364,9 +406,9 @@ class History { return false; } - return $name == 'note' || // Addressbook - strpos($name, 'description') !== false || // Calendar, Records, Timesheet, ProjectManager, Resources - $name == 'De' || // Tracker, InfoLog + return $name == 'note' || // Addressbook + strpos($name, 'description') !== false || // Calendar, Records, Timesheet, ProjectManager, Resources + $name == 'De' || // Tracker, InfoLog ($value && (strlen($value) > 200 || strstr($value, "\n") !== FALSE)); } }