diff --git a/bt5/erp5_ui_test/PortalTypePropertySheetTemplateItem/property_sheet_list.xml b/bt5/erp5_ui_test/PortalTypePropertySheetTemplateItem/property_sheet_list.xml index 2d573edc0e4f1fc9310dd6f94b27d705795a0f28..4ecce9d4931d928c108e396c2718d43c50115770 100644 --- a/bt5/erp5_ui_test/PortalTypePropertySheetTemplateItem/property_sheet_list.xml +++ b/bt5/erp5_ui_test/PortalTypePropertySheetTemplateItem/property_sheet_list.xml @@ -5,5 +5,6 @@ Amount DublinCore + Foo \ No newline at end of file diff --git a/bt5/erp5_ui_test/PropertySheetTemplateItem/portal_property_sheets/Foo.xml b/bt5/erp5_ui_test/PropertySheetTemplateItem/portal_property_sheets/Foo.xml new file mode 100644 index 0000000000000000000000000000000000000000..f3d2c9c7be29736019920fcb1cb0d9dc78dfe111 --- /dev/null +++ b/bt5/erp5_ui_test/PropertySheetTemplateItem/portal_property_sheets/Foo.xml @@ -0,0 +1,66 @@ + + + + + + + + + + _count + + AAAAAAAAAAI= + + + + _mt_index + + AAAAAAAAAAM= + + + + _tree + + AAAAAAAAAAQ= + + + + description + + + + + + id + Foo + + + portal_type + Property Sheet + + + + + + + + + 0 + + + + + + + + + + + + + + + + + + diff --git a/bt5/erp5_ui_test/PropertySheetTemplateItem/portal_property_sheets/Foo/protected_property_property.xml b/bt5/erp5_ui_test/PropertySheetTemplateItem/portal_property_sheets/Foo/protected_property_property.xml new file mode 100644 index 0000000000000000000000000000000000000000..6aa6a962cf62a8dec199a41aa046f8e88cc054eb --- /dev/null +++ b/bt5/erp5_ui_test/PropertySheetTemplateItem/portal_property_sheets/Foo/protected_property_property.xml @@ -0,0 +1,40 @@ + + + + + + + + + + categories + + + elementary_type/string + + + + + description + A protected property for UI test. This is a property that only managers should be able to see and modify + + + id + protected_property_property + + + portal_type + Standard Property + + + read_permission + Manage portal + + + write_permission + Manage portal + + + + + diff --git a/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Foo_viewSecurity.xml b/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Foo_viewSecurity.xml new file mode 100644 index 0000000000000000000000000000000000000000..11de402b2694f584593136940756ee2931de76a9 --- /dev/null +++ b/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Foo_viewSecurity.xml @@ -0,0 +1,153 @@ + + + + + + + + + + _bind_names + + + + + + + + + + _asgns + + + + + + + + + + + _objects + + + + + + action + Base_edit + + + description + View with some restricted properties + + + edit_order + + + + + + encoding + UTF-8 + + + enctype + + + + group_list + + + left + right + center + bottom + hidden + + + + + groups + + + + bottom + + + + + + center + + + + + + hidden + + + + + + left + + + my_protected_property + my_foo_category_title + + + + + right + + + + + + + + + id + Foo_viewSecurity + + + method + POST + + + name + Foo_view + + + pt + form_view + + + row_length + 4 + + + stored_encoding + UTF-8 + + + title + Foo Security + + + unicode_mode + 0 + + + update_action + + + + update_action_title + + + + + + diff --git a/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Foo_viewSecurity/my_foo_category_title.xml b/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Foo_viewSecurity/my_foo_category_title.xml new file mode 100644 index 0000000000000000000000000000000000000000..b9cb5ce0bc0061ff51fc0bc065b27384dadbd7a2 --- /dev/null +++ b/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Foo_viewSecurity/my_foo_category_title.xml @@ -0,0 +1,540 @@ + + + + + + + + + + id + my_foo_category_title + + + message_values + + + + external_validator_failed + The input failed the external validator. + + + line_too_long + A line was too long. + + + relation_result_ambiguous + Select appropriate document in the list. + + + relation_result_empty + No such document was found. + + + relation_result_too_long + Too many documents were found. + + + required_not_found + Input is required but no input given. + + + too_long + You entered too many characters. + + + too_many_lines + You entered too many lines. + + + + + + overrides + + + + allow_creation + + + + allow_jump + + + + alternate_name + + + + base_category + + + + catalog_index + + + + columns + + + + container_getter_id + + + + css_class + + + + default + + + + description + + + + display_maxwidth + + + + display_width + + + + editable + + + + enabled + + + + external_validator + + + + extra + + + + extra_item + + + + first_item + + + + hidden + + + + items + + + + jump_method + + + + list_method + + + + max_length + + + + max_linelength + + + + max_lines + + + + parameter_list + + + + portal_type + + + + relation_setter_id + + + + required + + + + size + + + + sort + + + + title + + + + truncate + + + + unicode + + + + update_method + + + + whitespace_preserve + + + + + + + tales + + + + allow_creation + + + + allow_jump + + + + alternate_name + + + + base_category + + + + catalog_index + + + + columns + + + + container_getter_id + + + + css_class + + + + default + + + + description + + + + display_maxwidth + + + + display_width + + + + editable + + + + enabled + + + + external_validator + + + + extra + + + + extra_item + + + + first_item + + + + hidden + + + + items + + + + jump_method + + + + list_method + + + + max_length + + + + max_linelength + + + + max_lines + + + + parameter_list + + + + portal_type + + + + relation_setter_id + + + + required + + + + size + + + + sort + + + + title + + + + truncate + + + + unicode + + + + update_method + + + + whitespace_preserve + + + + + + + values + + + + allow_creation + 0 + + + allow_jump + 1 + + + alternate_name + + + + base_category + foo_category + + + catalog_index + title + + + columns + + + + title + Title + + + int_index + Sort Index + + + relative_url + Relative URL + + + + + + container_getter_id + + + + css_class + + + + default + + + + description + + + + display_maxwidth + + + + display_width + 20 + + + editable + 1 + + + enabled + 1 + + + external_validator + + + + extra + + + + extra_item + + + + first_item + 0 + + + hidden + 0 + + + items + + + + + + jump_method + Base_jumpToRelatedDocument + + + list_method + + + + max_length + + + + max_linelength + + + + max_lines + + + + parameter_list + + + + + + portal_type + + + + Category + Category + + + + + + relation_form_id + Base_viewCustomRelatedObjectList + + + relation_setter_id + + + + required + 0 + + + size + 1 + + + sort + + + + + + title + Foo Category + + + truncate + 0 + + + unicode + 0 + + + update_method + Base_validateRelation + + + whitespace_preserve + 1 + + + + + + + + diff --git a/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Foo_viewSecurity/my_protected_property.xml b/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Foo_viewSecurity/my_protected_property.xml new file mode 100644 index 0000000000000000000000000000000000000000..7f03fa8e72cedc1e90863c00fc9b228754b1d770 --- /dev/null +++ b/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Foo_viewSecurity/my_protected_property.xml @@ -0,0 +1,264 @@ + + + + + + + + + + id + my_protected_property + + + message_values + + + + external_validator_failed + The input failed the external validator. + + + required_not_found + Input is required but no input given. + + + too_long + Too much input was given. + + + + + + overrides + + + + alternate_name + + + + css_class + + + + default + + + + description + + + + display_maxwidth + + + + display_width + + + + editable + + + + enabled + + + + external_validator + + + + extra + + + + hidden + + + + max_length + + + + required + + + + title + + + + truncate + + + + unicode + + + + whitespace_preserve + + + + + + + tales + + + + alternate_name + + + + css_class + + + + default + + + + description + + + + display_maxwidth + + + + display_width + + + + editable + + + + enabled + + + + external_validator + + + + extra + + + + hidden + + + + max_length + + + + required + + + + title + + + + truncate + + + + unicode + + + + whitespace_preserve + + + + + + + values + + + + alternate_name + + + + css_class + + + + default + + + + description + + + + display_maxwidth + + + + display_width + 20 + + + editable + 1 + + + enabled + 1 + + + external_validator + + + + extra + + + + hidden + 0 + + + input_type + text + + + max_length + + + + required + 0 + + + title + Protected Property + + + truncate + 0 + + + unicode + 0 + + + whitespace_preserve + 0 + + + + + + + + diff --git a/bt5/erp5_ui_test/bt/template_portal_type_property_sheet_list b/bt5/erp5_ui_test/bt/template_portal_type_property_sheet_list index 18e04ea6dea4f53df7e8c1b85924360d970e63dc..181a41749cb4fec156f85a89994ee8ff2d889e98 100644 --- a/bt5/erp5_ui_test/bt/template_portal_type_property_sheet_list +++ b/bt5/erp5_ui_test/bt/template_portal_type_property_sheet_list @@ -1,3 +1,4 @@ Bar | Reference Foo | Amount -Foo | DublinCore \ No newline at end of file +Foo | DublinCore +Foo | Foo \ No newline at end of file diff --git a/bt5/erp5_ui_test/bt/template_property_sheet_id_list b/bt5/erp5_ui_test/bt/template_property_sheet_id_list new file mode 100644 index 0000000000000000000000000000000000000000..9f26b637f0172389994613cc1cc28361857354b6 --- /dev/null +++ b/bt5/erp5_ui_test/bt/template_property_sheet_id_list @@ -0,0 +1 @@ +Foo \ No newline at end of file diff --git a/product/ERP5/Document/ParentDeliveryPropertyMovementGroup.py b/product/ERP5/Document/ParentDeliveryPropertyMovementGroup.py index 65cb6e6473b0680f6b44f8d9a96f7e0aa571c06c..fbf5bf6385e2fe609076eadc2d8f03811450c0df 100644 --- a/product/ERP5/Document/ParentDeliveryPropertyMovementGroup.py +++ b/product/ERP5/Document/ParentDeliveryPropertyMovementGroup.py @@ -45,7 +45,7 @@ class ParentDeliveryPropertyMovementGroup(PropertyMovementGroup): parent_delivery = self._getParentDelivery(movement) if parent_delivery is not None: for prop in self.getTestedPropertyList(): - property_dict['_%s' % prop] = self._getProperty(parent_delivery, prop) + property_dict['_%s' % prop] = self._getPropertyFromDocument(parent_delivery, prop) return property_dict def test(self, document, property_dict, property_list=None, **kw): @@ -64,7 +64,7 @@ class ParentDeliveryPropertyMovementGroup(PropertyMovementGroup): return False, {} document = self._getParentDelivery(movement) for prop in target_property_list: - if property_dict['_%s' % prop] != self._getProperty(document, prop): + if property_dict['_%s' % prop] != self._getPropertyFromDocument(document, prop): return False, {} return True, {} @@ -78,7 +78,7 @@ class ParentDeliveryPropertyMovementGroup(PropertyMovementGroup): delivery = movement.getDeliveryValue() return delivery - def _getProperty(self, document, property_id): + def _getPropertyFromDocument(self, document, property_id): if document is None: return None # XXX here we don't use Base.getProperty() but try to call accessor diff --git a/product/ERP5/ERP5Site.py b/product/ERP5/ERP5Site.py index 77e33e7e8a5dc3066aae77c8bcead059109797a1..421192bd48bdb1109ad80935d98e3e9ba64444a3 100644 --- a/product/ERP5/ERP5Site.py +++ b/product/ERP5/ERP5Site.py @@ -565,6 +565,12 @@ class ERP5Site(FolderMixIn, CMFSite, CacheCookieMixin): return self._v_version_priority_name_list + # Make sure ERP5Site follow same API as Products.ERP5Type.Base + + # _getProperty is missing, but since there are no protected properties + # on an ERP5 Site, we can just use getProperty instead. + _getProperty = CMFSite.getProperty + security.declareProtected(Permissions.AccessContentsInformation, 'getUid') def getUid(self): """ diff --git a/product/ERP5Catalog/tests/testERP5Catalog.py b/product/ERP5Catalog/tests/testERP5Catalog.py index aac6a29554f779ed24084232cb1eecc536fcc195..d06edebb1addf3f53bcb07c8c795e8810ed99d9d 100644 --- a/product/ERP5Catalog/tests/testERP5Catalog.py +++ b/product/ERP5Catalog/tests/testERP5Catalog.py @@ -33,6 +33,7 @@ import unittest import httplib from AccessControl import getSecurityManager from AccessControl.SecurityManagement import newSecurityManager +from Acquisition import aq_base from DateTime import DateTime from _mysql_exceptions import ProgrammingError from OFS.ObjectManager import ObjectManager @@ -44,6 +45,8 @@ from Testing import ZopeTestCase from zLOG import LOG class IndexableDocument(ObjectManager): + # This tests uses a simple ObjectManager, but ERP5Catalog only + # support classes inherting from ERP5Type.Base. # this property is required for dummy providesIMovement __allow_access_to_unprotected_subobjects__ = 1 @@ -66,6 +69,11 @@ class IndexableDocument(ObjectManager): return lambda: 0 raise AttributeError, name + def getProperty(self, prop, default=None): + return getattr(aq_base(self), prop, default) + + _getProperty = getProperty + def getPath(self): return self._path diff --git a/product/ERP5Form/tests/testGUIwithSecurity.py b/product/ERP5Form/tests/testGUIwithSecurity.py index 91ec75b1ad002ddf6ef3cb9eb6998beccfe409d6..0389824647e419b9575bf5436cbbd60943bb8493 100644 --- a/product/ERP5Form/tests/testGUIwithSecurity.py +++ b/product/ERP5Form/tests/testGUIwithSecurity.py @@ -28,21 +28,14 @@ ############################################################################## -import unittest - from Products.ERP5Type.tests.ERP5TypeTestCase import ERP5TypeTestCase from AccessControl.SecurityManagement import newSecurityManager -from zLOG import LOG from Products.ERP5Type.tests.Sequence import SequenceList -from Testing import ZopeTestCase -from DateTime import DateTime class TestGUISecurity(ERP5TypeTestCase): """ """ - quiet = 0 - run_all_test = 1 def getBusinessTemplateList(self): return ('erp5_ui_test', 'erp5_base') @@ -50,15 +43,6 @@ class TestGUISecurity(ERP5TypeTestCase): def getTitle(self): return "Security Issues in GUI" - def afterSetUp(self): - self.login() - - def login(self): - uf = self.getPortal().acl_users - uf._doAddUser('seb', '', ['Manager'], []) - user = uf.getUserById('seb').__of__(uf) - newSecurityManager(None, user) - def loginAs(self, id='user'): uf = self.getPortal().acl_users user = uf.getUser(id).__of__(uf) @@ -66,35 +50,51 @@ class TestGUISecurity(ERP5TypeTestCase): def stepCreateObjects(self, sequence = None, sequence_list = None, **kw): # Make sure that the status is clean. - portal = self.getPortal() - portal.ListBoxZuite_reset() - message = portal.foo_module.FooModule_createObjects() + self.portal.ListBoxZuite_reset() + message = self.portal.foo_module.FooModule_createObjects() self.assertTrue('Created Successfully' in message) - if not hasattr(portal.person_module, 'user'): - user = portal.person_module.newContent(portal_type='Person', id='user', reference='user') + if not hasattr(self.portal.person_module, 'user'): + user = self.portal.person_module.newContent(portal_type='Person', id='user', reference='user') user.newContent(portal_type='ERP5 Login', reference='user').validate() - asg = user.newContent(portal_type='Assignment') - asg.setStartDate(DateTime() - 100) - asg.setStopDate(DateTime() + 100) - asg.open() - self.commit() + user.newContent(portal_type='Assignment').open() def stepCreateTestFoo(self, sequence = None, sequence_list = None, **kw): - foo_module = self.getPortal().foo_module - foo_module.newContent(portal_type='Foo', id='foo', foo_category='a') + foo_module = self.portal.foo_module + foo_module.newContent(portal_type='Foo', id='foo', foo_category='a', protected_property='Protected Property') # allow Member to view foo_module in a hard coded way as it is not required to setup complex # security for this test (by default only 5A roles + Manager can view default modules) - args = (('Manager', 'Member', 'Assignor', 'Assignee', 'Auditor', 'Associate' ), 0) - foo_module.manage_permission('Access contents information', *args) - foo_module.manage_permission('View', *args) - self.commit() + for permission in ('Access contents information', 'View'): + foo_module.manage_permission( + permission, + ('Manager', 'Member', 'Assignor', 'Assignee', 'Auditor', 'Associate' ), + 0) - def stepAccessFoo(self, sequence = None, sequence_list = None, **kw): + def stepAccessFooDoesNotRaise(self, sequence = None, sequence_list = None, **kw): """ Try to view the Foo_view form, make sure Unauthorized is not raised. """ self.loginAs() - self.getPortal().foo_module.foo.Foo_view() + self.portal.foo_module.foo.Foo_view() + self.login() + + def stepAccessFooDisplaysCategoryName(self, sequence = None, sequence_list = None, **kw): + """ + Try to view the Foo_view form, make sure our category name is displayed + """ + self.loginAs() + self.assertIn( + self.category_field_markup, + self.portal.foo_module.foo.Foo_view()) + self.login() + + def stepAccessFooDoesNotDisplayCategoryName(self, sequence = None, sequence_list = None, **kw): + """ + Try to view the Foo_view form, make sure our category name is not displayed + """ + self.loginAs() + self.assertNotIn( + self.category_field_markup, + self.portal.foo_module.foo.Foo_view()) self.login() def stepChangeCategorySecurity(self, sequence = None, sequence_list = None, **kw): @@ -102,30 +102,23 @@ class TestGUISecurity(ERP5TypeTestCase): here we change security of a category to which the "Foo" is related and which is displayed in the Foo's RelationStringField """ - category = self.getPortal().portal_categories.foo_category.a - args = (('Manager',), 0) - category.manage_permission('Access contents information', *args) - category.manage_permission('View', *args) - self.tic() + category = self.portal.portal_categories.foo_category.a + for permission in ('Access contents information', 'View'): + category.manage_permission(permission, ('Manager',), 0 ) def stepResetCategorySecurity(self, sequence = None, sequence_list = None, **kw): """ reset it back """ - category = self.getPortal().portal_categories.foo_category.a - args = ((), 1) - category.manage_permission('Access contents information', *args) - category.manage_permission('View', *args) - self.tic() + category = self.portal.portal_categories.foo_category.a + for permission in ('Access contents information', 'View'): + category.manage_permission(permission, ('Manager',), 1) - def test_01_relationFieldToInaccessibleObject(self, quiet=quiet, run=run_all_test): + def test_01_relationFieldToInaccessibleObject(self): """ This test checks if a form can be viewed when it contains a RelationStringField which links to an object the user is not authorized to view. - This fails for now. A proposed patch solving this problem is here: - http://svn.erp5.org/experimental/FSPatch/Products/ERP5Form/ERP5Form_safeRelationField.diff?view=markup - This problem can happen for example in the following situation: - a user is a member of a project P team, so he can view P - the user creates a project-related document and leaves it in "draft" state @@ -133,29 +126,39 @@ class TestGUISecurity(ERP5TypeTestCase): Then the user can not view the project, but still can view his document as he is the owner. An attempt to view the document form would raise Unauthorized. """ - self.login() - if not run: return - if not quiet: - message = 'test_01_relationFieldToInaccessibleObject' - ZopeTestCase._print('\n%s ' % message) - LOG('Testing... ', 0, message) + # this really depends on the generated markup + self.category_field_markup = ' 0.001725 DO_TEST = 1 +# Profiler support. # set 1 to get profiler's result (unit_test/tests/) -PROFILE=0 +PROFILE = 0 +# set this to 'pprofile' to profile with pprofile ( https://github.com/vpelletier/pprofile ) +# instad of python's standard library profiler ( https://docs.python.org/2/library/profile.html ) +PROFILER = 'pprofile' + class TestPerformanceMixin(ERP5TypeTestCase, LogInterceptor): @@ -147,22 +153,34 @@ class TestPerformanceMixin(ERP5TypeTestCase, LogInterceptor): """ return self.portal['bar_module'] - def profile(self, func, suffix=''): + def profile(self, func, suffix='', args=(), kw=None): + """Profile `func(*args, **kw)` with selected profiler, + and dump output in a file called `func.__name__ + suffix` + """ + if not kw: + kw = {} + + if PROFILER == 'pprofile': + import pprofile + prof = pprofile.Profile() + else: from cProfile import Profile - prof_file = '%s%s' % (func.__name__, suffix) - try: - os.unlink(prof_file) - except OSError: - pass prof = Profile() - prof.runcall(func) - prof.dump_stats(prof_file) + + prof_file = '%s%s' % (func.__name__, suffix) + try: + os.unlink(prof_file) + except OSError: + pass + prof.runcall(func, *args, **kw) + prof.dump_stats(prof_file) def beforeTearDown(self): # Re-enable gc at teardown. gc.enable() self.abort() + class TestPerformance(TestPerformanceMixin): def getTitle(self): @@ -368,3 +386,75 @@ class TestPerformance(TestPerformanceMixin): MIN_OBJECT_MANY_LINES_VIEW, req_time, MAX_OBJECT_MANY_LINES_VIEW)) + + +class TestPropertyPerformance(TestPerformanceMixin): + def afterSetUp(self): + super(TestPerformanceMixin, self).afterSetUp() + self.foo = self.portal.foo_module.newContent( + portal_type='Foo', + title='Foo Test', + protected_property='Restricted Property', + ) + + # we will run the test as anonymous user. Setup permissions so that anymous can + # get and set all properties, except `protected_property` (unless test change to another user) + for permission in ('View', 'Access contents information', 'Modify portal content'): + self.foo.manage_permission(permission, ['Anonymous'], 1) + + self.tic() + self.logout() + + def _benchmark(self, nb_iterations, min_time, max_time): + def decorated(f): + before = time() + for i in xrange(nb_iterations): + f(i) + after = time() + total_time = (after - before) / 100. + + print ("time %s.%s %.4f < %.4f < %.4f\n" % \ + ( self.id(), + f.__doc__ or f.__name__, + min_time, + total_time, + max_time )) + if PROFILE: + self.profile(f, args=(i, )) + if DO_TEST: + self.assertTrue( + min_time < total_time < max_time, + '%.4f < %.4f < %.4f' % + (min_time, total_time, max_time)) + return decorated + + def test_getProperty_protected_property_refused(self): + getProperty = self.foo.getProperty + self.assertRaises(Unauthorized, getProperty, 'protected_property') + + @self._benchmark(100000, 0.0001, 1) + def getPropertyWithRestrictedPropertyRefused(_): + getProperty('protected_property', checked_permission='Access contents information') + + def test_getProperty_protected_property_allowed(self): + getProperty = self.foo.getProperty + self.login() + @self._benchmark(100000, 0.0001, 1) + def getPropertyWithRestrictedPropertyAllowed(_): + getProperty('protected_property', checked_permission='Access contents information') + + def test_getProperty_simple_property(self): + getProperty = self.foo.getProperty + @self._benchmark(100000, 0.0001, 1) + def getPropertyWithSimpleProperty(_): + getProperty('title', checked_permission='Access contents information') + + def test_edit_restricted_properties(self): + _edit = self.foo.edit + self.login() + @self._benchmark(10000, 0.0001, 1) + def edit(i): + _edit( + title=str(i), + protected_property=str(i) + ) diff --git a/product/ZSQLCatalog/SQLCatalog.py b/product/ZSQLCatalog/SQLCatalog.py index 86ebd4c95fe97a1a4bfd570a57673b84546020da..f46948283eb71e49e2df8b3de2acce56ae6c62e7 100644 --- a/product/ZSQLCatalog/SQLCatalog.py +++ b/product/ZSQLCatalog/SQLCatalog.py @@ -1395,7 +1395,7 @@ class Catalog(Folder, # objects but we could also use over multiple transactions # if this can improve performance significantly # ZZZ - we could find a way to compute this once only - cache_key = tuple(object.getProperty(key) for key + cache_key = tuple(object._getProperty(key) for key in expression_cache_key_list) try: if expression_result_cache[cache_key]: