holm/egroupware
1
1
Fork 0
forked from extern/egroupware
Code Issues Pull Requests Packages Projects Releases Wiki Activity

* Allow private custom fields in notifications, with filtering according to set permissions. Private custom fields are removed from notifications to non-users.

Browse Source
This commit is contained in:
nathangray 2018-01-10 10:42:28 -07:00
parent b7ffb65644
commit 3709d773d9
7 changed files with 321 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
api/src/Contacts/Tracking.php
View File

@ -227,8 +227,6 @@ class Tracking extends Api\Storage\Tracking
*/
function get_details($data,$receiver=null)
{
unset($receiver); // not used, but required by function signature
foreach($this->contacts->contact_fields as $name => $label)
{
if (!$data[$name] && $name != 'owner') continue;
@ -288,7 +286,7 @@ class Tracking extends Api\Storage\Tracking
}
}
// add custom fields for given type
$details += $this->get_customfields($data, $data['tid']);
$details += $this->get_customfields($data, $data['tid'], $receiver);
return $details;
}

46
api/src/Storage/Customfields.php
View File

@ -38,11 +38,11 @@ class Customfields implements \IteratorAggregate
protected $app;
/**
* should all the private fields be returned too, default no
* User account to filter custom field private
*
* @var boolean
* @var int
*/
protected $all_private_too=false;
protected $account=false;
/**
* Iterator initialised for custom fields
@ -55,25 +55,29 @@ class Customfields implements \IteratorAggregate
* Constructor
*
* @param string $app
* @param boolean $all_private_too =false should all the private fields be returned too, default no
* @param int|boolean $account =false Filter private for given account_id,
* false for current user or true for all the private fields be returned too, default current user
* @param string $only_type2 =null if given only return fields of type2 == $only_type2
* @param int $start =0
* @param int $num_rows =null
* @param Api\Db $db =null reference to database instance to use
* @return array with customfields
*/
function __construct($app, $all_private_too=false, $only_type2=null, $start=0, $num_rows=null, Api\Db $db=null)
function __construct($app, $account=false, $only_type2=null, $start=0, $num_rows=null, Api\Db $db=null)
{
$this->app = $app;
$this->all_private_too = $all_private_too;
// If $account is true, no filtering otherwise use current user
$this->account = $account === true ? false :
is_numeric($account) ? (int)$account : $GLOBALS['egw_info']['user']['account_id'];
$query = array(
'cf_app' => $app,
);
if (!$all_private_too)
if ($this->account)
{
$memberships = $GLOBALS['egw']->accounts->memberships($GLOBALS['egw_info']['user']['account_id'], true);
$memberships[] = $GLOBALS['egw_info']['user']['account_id'];
$memberships = $GLOBALS['egw']->accounts->memberships($this->account, true);
$memberships[] = $this->account;
$query[] = $this->commasep_match('cf_private', $memberships);
}
if ($only_type2)
@ -127,19 +131,24 @@ class Customfields implements \IteratorAggregate
* Get customfield array of an application
*
* @param string $app
* @param boolean $all_private_too =false should all the private fields be returned too, default no
* @param int|boolean $account =false Filter private for given account_id,
* false for current user or true for all the private fields be returned too, default current user
* @param string $only_type2 =null if given only return fields of type2 == $only_type2
* @param Api\Db $db =null reference to database to use
* @return array with customfields
*/
public static function get($app, $all_private_too=false, $only_type2=null, Api\Db $db=null)
public static function get($app, $account=false, $only_type2=null, Api\Db $db=null)
{
$cache_key = $app.':'.($all_private_too?'all':$GLOBALS['egw_info']['user']['account_id']).':'.$only_type2;
$account = $account === true ? 'all' :
$account === false ? $GLOBALS['egw_info']['user']['account_id'] :
(int)$account;
$cache_key = $app.':'.$account.':'.$only_type2;
$cfs = Api\Cache::getInstance(__CLASS__, $cache_key);
if (!isset($cfs))
{
$cfs = iterator_to_array(new Customfields($app, $all_private_too, $only_type2, 0, null, $db));
$cfs = iterator_to_array(new Customfields($app, $account, $only_type2, 0, null, $db));
Api\Cache::setInstance(__CLASS__, $cache_key, $cfs);
$cached = Api\Cache::getInstance(__CLASS__, $app);
@ -149,7 +158,7 @@ class Customfields implements \IteratorAggregate
Api\Cache::setInstance(__CLASS__, $app, $cached);
}
}
//error_log(__METHOD__."('$app', $all_private_too, '$only_type2') returning fields: ".implode(', ', array_keys($cfs)));
//error_log(__METHOD__."('$app', $account, '$only_type2') returning fields: ".implode(', ', array_keys($cfs)));
return $cfs;
}
@ -157,13 +166,14 @@ class Customfields implements \IteratorAggregate
* Check if any customfield uses html (type == 'htmlarea')
*
* @param string $app
* @param boolean $all_private_too =false should all the private fields be returned too, default no
* @param int|boolean $account =false Filter private for given account_id,
* false for current user or true for all the private fields be returned too, default current user
* @param string $only_type2 =null if given only return fields of type2 == $only_type2
* @return boolen true: if there is a custom field useing html, false if not
*/
public static function use_html($app, $all_private_too=false, $only_type2=null)
public static function use_html($app, $account=false, $only_type2=null)
{
foreach(self::get($app, $all_private_too, $only_type2) as $data)
foreach(self::get($app, $account, $only_type2) as $data)
{
if ($data['type'] == 'htmlarea') return true;
}
@ -544,7 +554,7 @@ class Customfields implements \IteratorAggregate
self::$db = $GLOBALS['egw_setup']->db;
}
}
/**
* Handle any uploaded files that weren't dealt with immediately when uploaded.
* This usually happens for new entries, where we don't have the entry's ID

68
api/src/Storage/Tracking.php
View File

@ -242,13 +242,20 @@ abstract class Tracking
*
* @param array|object $data
* @param string $only_type2 = null if given only return fields of type2 == $only_type2
* @param int $user = false Use this user for custom field permissions, or false
* to strip all private custom fields
*
* @return array of details as array with values for keys 'label','value','type'
*/
function get_customfields($data, $only_type2=null)
function get_customfields($data, $only_type2=null, $user = false)
{
$details = array();
if(!is_numeric($user))
{
$user = false;
}
if (($cfs = Customfields::get($this->app, $all_private_too=false, $only_type2)))
if (($cfs = Customfields::get($this->app, $user, $only_type2)))
{
$header_done = false;
foreach($cfs as $name => $field)
@ -256,8 +263,8 @@ abstract class Tracking
if (in_array($field['type'], Customfields::$non_printable_fields)) continue;
// Sometimes cached customfields let private fields the user can access
// leak through. Make sure we don't expose them.
if ($field['private']) continue;
// leak through. If no specific user provided, make sure we don't expose them.
if ($user === false && $field['private']) continue;
if (!$header_done)
{
@ -701,6 +708,7 @@ abstract class Tracking
$save_user = $GLOBALS['egw_info']['user'];
$do_notify = true;
$can_cache = false;
if (is_numeric($user_or_lang)) // user --> read everything from his prefs
{
@ -717,6 +725,7 @@ abstract class Tracking
{
$do_notify = false; // no notification requested / necessary
}
$can_cache = (Customfields::get($this->app, true) == Customfields::get($this->app, $user_or_lang));
}
else
{
@ -741,8 +750,11 @@ abstract class Tracking
$date_format = $GLOBALS['egw_info']['user']['preferences']['common']['dateformat'] .
$GLOBALS['egw_info']['user']['preferences']['common']['timeformat'];
// Cache text body
$body_cache =& $this->body_cache[$data[$this->id_field]][$lang][$date_format];
// Cache text body, if there's no private custom fields we might reveal
if($can_cache)
{
$body_cache =& $this->body_cache[$data[$this->id_field]][$lang][$date_format];
}
if(empty($data[$this->id_field]) || !isset($body_cache['text']))
{
$body_cache['text'] = $this->get_body(false,$data,$old,false,$receiver);
@ -1015,7 +1027,7 @@ abstract class Tracking
$body = '';
if($this->get_config(self::CUSTOM_NOTIFICATION, $data, $old))
{
$body = $this->get_custom_message($data,$old);
$body = $this->get_custom_message($data,$old,null,$receiver);
if(($sig = $this->get_signature($data,$old,$receiver)))
{
$body .= ($html_email ? '<br />':'') . "\n$sig";
@ -1201,7 +1213,7 @@ abstract class Tracking
* Get a custom notification message to be used instead of the standard one.
* It can use merge print placeholders to include data.
*/
protected function get_custom_message($data, $old, $merge_class = null)
protected function get_custom_message($data, $old, $merge_class = null, $receiver = false)
{
$message = $this->get_config(self::CUSTOM_NOTIFICATION, $data, $old);
if(!$message)
@ -1209,6 +1221,9 @@ abstract class Tracking
return '';
}
// Check if there's any custom field privacy issues, and try to remove them
$message = $this->sanitize_custom_message($message, $receiver);
// Automatically set merge class from naming conventions
if($merge_class == null)
{
@ -1236,4 +1251,41 @@ abstract class Tracking
throw new Api\Exception\WrongParameter("Invalid merge class '$merge_class' for {$this->app} custom notification");
}
}
/**
* Check to see if the message would expose any custom fields that are
* not visible to the receiver, and try to remove them from the message.
*
* @param string $message
* @param string|int $receiver Account ID or email address
*/
protected function sanitize_custom_message($message, $receiver)
{
if(!is_numeric($receiver))
{
$receiver = false;
}
$cfs = Customfields::get($this->app, $receiver);
$all_cfs = Customfields::get($this->app, true);
// If we have a specific user and they're the same then there are
// no private fields so nothing needs to be done
if($receiver && $all_cfs == $cfs)
{
return $message;
}
// Replace any placeholders that use the private field, or any sub-keys
// of the field
foreach($all_cfs as $name => $field)
{
if ($receiver === false && $field['private'] || !$cfs[$name])
{
// {{field}} or {{field/subfield}} or $$field$$ or $$field/subfield$$
$message = preg_replace('/(\{\{|\$\$)#'.$name.'(\/[^\}\$]+)?(\}\}|\$\$)/', '', $message);
}
}
return $message;
}
}

121
api/tests/Storage/CustomfieldsTest.php Normal file
View File

@ -0,0 +1,121 @@
<?php
/**
* Tests for customfields
*
* @package api
* @subpackage tests
* @link http://www.egroupware.org
* @author Nathan Gray
* @copyright 2018 Nathan Gray
* @license http://opensource.org/licenses/gpl-license.php GPL - GNU General Public License
*/
namespace EGroupware\Api\Storage;
use EGroupware\Api;
use EGroupware\Api\LoggedInTest as LoggedInTest;
class CustomfieldsTest extends LoggedInTest
{
const APP = 'test';
protected $customfields = null;
protected $simple_field = array(
'app' => self::APP,
'name' => 'test_field',
'label' => 'Custom field',
'type' => 'text',
'type2' => array(),
'help' => 'Custom field created for automated testing by CustomfieldsTest',
'values' => null,
'len' => null,
'rows' => null,
'order' => null,
'needed' => null,
'private' => array()
);
public function assertPreConditions()
{
parent::assertPreConditions();
$tables = $GLOBALS['egw']->db->table_names(true);
$this->assertContains('egw_test', $tables, 'Could not find DB table "egw_test", make sure test app is installed');
}
/**
* Check to make sure we can create a custom field
*/
public function testCreateField()
{
// Create
$field = $this->simple_field;
Customfields::update($field);
// Check
$fields = Customfields::get(self::APP);
$this->assertArrayHasKey($field['name'], $fields);
$saved_field = $fields[$field['name']];
foreach(array('app','label','type','type2','help','values','len','rows','needed','private') as $key)
{
$this->assertEquals($field[$key], $saved_field[$key], "Load of $key did not match save");
}
// Clean
unset($fields[$field['name']]);
Customfields::save(self::APP, $fields);
}
/**
* Test the access control on private custom fields
*/
public function testPrivate()
{
// Create field
$field = array_merge(
$this->simple_field,
array(
'private' => array($GLOBALS['egw_info']['user']['account_id'])
)
);
Customfields::update($field);
$fields = Customfields::get(self::APP);
// Get another user
$accounts = $GLOBALS['egw']->accounts->search(array(
'type' => 'accounts'
));
unset($accounts[$GLOBALS['egw_info']['user']['account_id']]);
if(count($accounts) == 0)
{
$this->markTestSkipped('Need more than one user to check private');
}
$other_account = key($accounts);
// Try to read - should not be there
$fields = Customfields::get(self::APP,$other_account);
$this->assertArrayNotHasKey($field['name'], $fields);
// Give access & check again
$field['private'][] = $other_account;
Customfields::update($field);
$fields = Customfields::get(self::APP,$other_account);
$this->assertArrayHasKey($field['name'], $fields);
// Remove access, check its gone
$field['private'] = array($GLOBALS['egw_info']['user']['account_id']);
Customfields::update($field);
$fields = Customfields::get(self::APP,$other_account);
$this->assertArrayNotHasKey($field['name'], $fields);
// Clean up
unset($fields[$field['name']]);
Customfields::save(self::APP, $fields);
}
}

32
api/tests/Storage/TestTracking.php Normal file
View File

@ -0,0 +1,32 @@
<?php
/**
* Concrete implementation of tracking class for testing
*
* @link http://www.egroupware.org
* @author Nathan Gray
* @package api
* @subpackage tests
* @copyright (c) 2018 Nathan Gray
* @license http://opensource.org/licenses/gpl-license.php GPL - GNU General Public License
*/
namespace EGroupware\Api\Storage;
class TestTracking extends Tracking {
var $app = 'test';
/**
* Expose protected parent method so it can be tested
* @param string $message
* @param string|int $receiver
* @return string
*/
public function sanitize_custom_message($message, $receiver)
{
return parent::sanitize_custom_message($message, $receiver);
}
}

78
api/tests/Storage/TrackingTest.php Normal file
View File

@ -0,0 +1,78 @@
<?php
/**
* Tests for Tracking
*
* @package api
* @subpackage tests
* @link http://www.egroupware.org
* @author Nathan Gray
* @copyright 2018 Nathan Gray
* @license http://opensource.org/licenses/gpl-license.php GPL - GNU General Public License
*/
namespace EGroupware\Api\Storage;
require_once __DIR__ . '/TestTracking.php';
use EGroupware\Api;
use EGroupware\Api\LoggedInTest as LoggedInTest;
class TrackingTest extends LoggedInTest
{
const APP = 'test';
protected $simple_field = array(
'app' => self::APP,
'name' => 'test_field',
'label' => 'Custom field',
'type' => 'text',
'type2' => array(),
'help' => 'Custom field created for automated testing by CustomfieldsTest',
'values' => null,
'len' => null,
'rows' => null,
'order' => null,
'needed' => null,
'private' => array()
);
/**
* Test the access control on private custom fields
*/
public function testSanitizeCustomMessage()
{
// Create field
$field = array_merge(
$this->simple_field,
array(
'private' => array($GLOBALS['egw_info']['user']['account_id'])
)
);
Customfields::update($field);
$fields = Customfields::get(self::APP);
$dirty_message = "This custom message contains {{#test_field}}, which only user {$GLOBALS['egw_info']['user']['account_id']} can access.";
$clean_message = "This custom message contains , which only user {$GLOBALS['egw_info']['user']['account_id']} can access.";
// Get another user
$accounts = $GLOBALS['egw']->accounts->search(array(
'type' => 'accounts'
));
unset($accounts[$GLOBALS['egw_info']['user']['account_id']]);
if(count($accounts) == 0)
{
$this->markTestSkipped('Need more than one user to check private');
}
$other_account = key($accounts);
$tracking = new TestTracking(self::APP);
$cleaned = $tracking->sanitize_custom_message($dirty_message, $other_account);
$this->assertEquals($clean_message, $cleaned);
// Clean up
unset($fields[$field['name']]);
Customfields::save(self::APP, $fields);
}
}

3
infolog/inc/class.infolog_tracking.inc.php
View File

@ -199,7 +199,6 @@ class infolog_tracking extends Api\Storage\Tracking
*/
function get_details($data,$receiver=null)
{
unset($receiver); // not used, but required function signature
//error_log(__METHOD__.__LINE__.' Data:'.array2string($data));
$responsible = array();
if ($data['info_responsible'])
@ -257,7 +256,7 @@ class infolog_tracking extends Api\Storage\Tracking
'type' => 'multiline',
);
// add custom fields for given type
$details += $this->get_customfields($data, $data['info_type']);
$details += $this->get_customfields($data, $data['info_type'], $receiver);
return $details;
}