Commit 63dcec59 authored by Jérome Perrin's avatar Jérome Perrin

Security of tester accessors

The first commit of this merge request was introduced for zope4py3 branch but it revealed issues on py2 as well, i.e. we don't have roles in auto-generated tester methods.

```
======================================================================
FAIL: test_method_protection (testSecurity.TestSecurity)
This test will list all implicitly Public methods in any objects in ZODB.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/(SR)/parts/erp5/Products/ERP5/tests/testSecurity.py", line 113, in test_method_protection
    self.fail(message)
AssertionError: 
The following 7 methods have a docstring but have no security assertions.
	/(SR)/parts/erp5/product/ERP5Form/PreferenceTool.py:65 isPreferredVcsPushMode
	/srv/slapgrid/slappart19/t/eiy/soft/8a7759fd7b65b20d9f87713605745d05/parts/erp5/product/ERP5Type/Accessor/AcquiredProperty.py:217 hasTelephoneValidationState
	/(SR)/parts/erp5/product/ERP5Type/Accessor/Base.py:219 hasViewFormIdList
	/(SR)/parts/erp5/product/ERP5Type/Accessor/Constant.py:94 isWebDocumentType
	/(SR)/parts/erp5/product/ERP5Type/Accessor/Content.py:224 hasImage
	/(SR)/parts/erp5/product/ERP5Type/Accessor/ContentProperty.py:283 hasImageWidthList
	/(SR)/parts/erp5/product/ERP5Type/Accessor/Translation.py:233 hasFrTranslatedTitle

----------------------------------------------------------------------
```

See merge request !1911
parents 2ca25775 3992822a
Pipeline #34649 failed with stage
in 0 seconds
...@@ -67,10 +67,10 @@ class TestRenderViewAPI(ERP5TypeTestCase): ...@@ -67,10 +67,10 @@ class TestRenderViewAPI(ERP5TypeTestCase):
def test_signature(self): def test_signature(self):
for field in FieldRegistry.get_field_classes().itervalues(): # pylint: disable=no-value-for-parameter for field in FieldRegistry.get_field_classes().itervalues(): # pylint: disable=no-value-for-parameter
self.assertEqual(('self', 'value', 'REQUEST', 'render_prefix'), self.assertEqual(('self', 'value', 'REQUEST', 'render_prefix'),
field.render_view.__func__.func_code.co_varnames) field.render_view.__code__.co_varnames)
if field is not ProxyField: if field is not ProxyField:
self.assertEqual(('self', 'field', 'value', 'REQUEST'), self.assertEqual(('self', 'field', 'value', 'REQUEST'),
field.widget.render_view.__func__.func_code.co_varnames[:4], '%s %s' % (field.widget, field.widget.render_view.__func__.func_code.co_varnames[:4])) field.widget.render_view.__code__.co_varnames[:4], '%s %s' % (field.widget, field.widget.render_view.__code__.co_varnames[:4]))
class TestFloatField(ERP5TypeTestCase): class TestFloatField(ERP5TypeTestCase):
......
...@@ -126,8 +126,8 @@ class TestFacebookLogin(ERP5TypeTestCase): ...@@ -126,8 +126,8 @@ class TestFacebookLogin(ERP5TypeTestCase):
'erp5.component.extension.FacebookLoginUtility.getUserEntry', 'erp5.component.extension.FacebookLoginUtility.getUserEntry',
side_effect=getUserEntry side_effect=getUserEntry
) as getUserEntry_mock: ) as getUserEntry_mock:
getAccessTokenFromCode_mock.func_code = getAccessTokenFromCode.func_code getAccessTokenFromCode_mock.__code__ = getAccessTokenFromCode.__code__
getUserEntry_mock.func_code = getUserEntry.func_code getUserEntry_mock.__code__ = getUserEntry.__code__
self.portal.ERP5Site_callbackFacebookLogin(code=CODE) self.portal.ERP5Site_callbackFacebookLogin(code=CODE)
getAccessTokenFromCode_mock.assert_called_once() getAccessTokenFromCode_mock.assert_called_once()
getUserEntry_mock.assert_called_once() getUserEntry_mock.assert_called_once()
...@@ -166,8 +166,8 @@ class TestFacebookLogin(ERP5TypeTestCase): ...@@ -166,8 +166,8 @@ class TestFacebookLogin(ERP5TypeTestCase):
'erp5.component.extension.FacebookLoginUtility.getUserEntry', 'erp5.component.extension.FacebookLoginUtility.getUserEntry',
side_effect=getUserEntry side_effect=getUserEntry
) as getUserEntry_mock: ) as getUserEntry_mock:
getAccessTokenFromCode_mock.func_code = getAccessTokenFromCode.func_code getAccessTokenFromCode_mock.__code__ = getAccessTokenFromCode.__code__
getUserEntry_mock.func_code = getUserEntry.func_code getUserEntry_mock.__code__ = getUserEntry.__code__
self.portal.ERP5Site_callbackFacebookLogin(code=CODE) self.portal.ERP5Site_callbackFacebookLogin(code=CODE)
ac_cookie, = [v for (k, v) in response.listHeaders() if k.lower() == 'set-cookie' and '__ac_facebook_hash=' in v] ac_cookie, = [v for (k, v) in response.listHeaders() if k.lower() == 'set-cookie' and '__ac_facebook_hash=' in v]
...@@ -232,8 +232,8 @@ return credential_request ...@@ -232,8 +232,8 @@ return credential_request
'erp5.component.extension.FacebookLoginUtility.getUserEntry', 'erp5.component.extension.FacebookLoginUtility.getUserEntry',
side_effect=getUserEntry side_effect=getUserEntry
) as getUserEntry_mock: ) as getUserEntry_mock:
getAccessTokenFromCode_mock.func_code = getAccessTokenFromCode.func_code getAccessTokenFromCode_mock.__code__ = getAccessTokenFromCode.__code__
getUserEntry_mock.func_code = getUserEntry.func_code getUserEntry_mock.__code__ = getUserEntry.__code__
response = self.portal.ERP5Site_callbackFacebookLogin(code=CODE) response = self.portal.ERP5Site_callbackFacebookLogin(code=CODE)
facebook_hash = self.portal.REQUEST.RESPONSE.cookies.get("__ac_facebook_hash")["value"] facebook_hash = self.portal.REQUEST.RESPONSE.cookies.get("__ac_facebook_hash")["value"]
self.assertEqual("8cec04e21e927f1023f4f4980ec11a77", facebook_hash) self.assertEqual("8cec04e21e927f1023f4f4980ec11a77", facebook_hash)
......
...@@ -163,8 +163,8 @@ class TestGoogleLogin(GoogleLoginTestCase): ...@@ -163,8 +163,8 @@ class TestGoogleLogin(GoogleLoginTestCase):
'erp5.component.extension.GoogleLoginUtility.getUserEntry', 'erp5.component.extension.GoogleLoginUtility.getUserEntry',
side_effect=getUserEntry side_effect=getUserEntry
) as getUserEntry_mock: ) as getUserEntry_mock:
getAccessTokenFromCode_mock.func_code = getAccessTokenFromCode.func_code getAccessTokenFromCode_mock.__code__ = getAccessTokenFromCode.__code__
getUserEntry_mock.func_code = getUserEntry.func_code getUserEntry_mock.__code__ = getUserEntry.__code__
self.portal.ERP5Site_receiveGoogleCallback(code=CODE) self.portal.ERP5Site_receiveGoogleCallback(code=CODE)
getAccessTokenFromCode_mock.assert_called_once() getAccessTokenFromCode_mock.assert_called_once()
getUserEntry_mock.assert_called_once() getUserEntry_mock.assert_called_once()
...@@ -207,8 +207,8 @@ class TestGoogleLogin(GoogleLoginTestCase): ...@@ -207,8 +207,8 @@ class TestGoogleLogin(GoogleLoginTestCase):
'erp5.component.extension.GoogleLoginUtility.getUserEntry', 'erp5.component.extension.GoogleLoginUtility.getUserEntry',
side_effect=getUserEntry side_effect=getUserEntry
) as getUserEntry_mock: ) as getUserEntry_mock:
getAccessTokenFromCode_mock.func_code = getAccessTokenFromCode.func_code getAccessTokenFromCode_mock.__code__ = getAccessTokenFromCode.__code__
getUserEntry_mock.func_code = getUserEntry.func_code getUserEntry_mock.__code__ = getUserEntry.__code__
self.portal.ERP5Site_receiveGoogleCallback(code=CODE) self.portal.ERP5Site_receiveGoogleCallback(code=CODE)
getAccessTokenFromCode_mock.assert_called_once() getAccessTokenFromCode_mock.assert_called_once()
...@@ -278,8 +278,8 @@ return credential_request ...@@ -278,8 +278,8 @@ return credential_request
'erp5.component.extension.GoogleLoginUtility.getUserEntry', 'erp5.component.extension.GoogleLoginUtility.getUserEntry',
side_effect=getUserEntry side_effect=getUserEntry
) as getUserEntry_mock: ) as getUserEntry_mock:
getAccessTokenFromCode_mock.func_code = getAccessTokenFromCode.func_code getAccessTokenFromCode_mock.__code__ = getAccessTokenFromCode.__code__
getUserEntry_mock.func_code = getUserEntry.func_code getUserEntry_mock.__code__ = getUserEntry.__code__
response = self.portal.ERP5Site_receiveGoogleCallback(code=CODE) response = self.portal.ERP5Site_receiveGoogleCallback(code=CODE)
getAccessTokenFromCode_mock.assert_called_once() getAccessTokenFromCode_mock.assert_called_once()
getUserEntry_mock.assert_called_once() getUserEntry_mock.assert_called_once()
......
...@@ -126,8 +126,8 @@ class TestOpenIdConnectLogin(OpenIdConnectLoginTestCase): ...@@ -126,8 +126,8 @@ class TestOpenIdConnectLogin(OpenIdConnectLoginTestCase):
'erp5.component.extension.OpenIdConnectLoginUtility.getUserEntry', 'erp5.component.extension.OpenIdConnectLoginUtility.getUserEntry',
side_effect=getUserEntry side_effect=getUserEntry
) as getUserEntry_mock: ) as getUserEntry_mock:
getAccessTokenFromCode_mock.func_code = getAccessTokenFromCode.func_code getAccessTokenFromCode_mock.__code__ = getAccessTokenFromCode.__code__
getUserEntry_mock.func_code = getUserEntry.func_code getUserEntry_mock.__code__ = getUserEntry.__code__
self.portal.ERP5Site_receiveOpenIdCallback(code=CODE, state=state) self.portal.ERP5Site_receiveOpenIdCallback(code=CODE, state=state)
getAccessTokenFromCode_mock.assert_called_once() getAccessTokenFromCode_mock.assert_called_once()
...@@ -165,8 +165,8 @@ class TestOpenIdConnectLogin(OpenIdConnectLoginTestCase): ...@@ -165,8 +165,8 @@ class TestOpenIdConnectLogin(OpenIdConnectLoginTestCase):
'erp5.component.extension.OpenIdConnectLoginUtility.getUserEntry', 'erp5.component.extension.OpenIdConnectLoginUtility.getUserEntry',
side_effect=getUserEntry side_effect=getUserEntry
) as getUserEntry_mock: ) as getUserEntry_mock:
getAccessTokenFromCode_mock.func_code = getAccessTokenFromCode.func_code getAccessTokenFromCode_mock.__code__ = getAccessTokenFromCode.__code__
getUserEntry_mock.func_code = getUserEntry.func_code getUserEntry_mock.__code__ = getUserEntry.__code__
self.portal.ERP5Site_receiveOpenIdCallback(code=CODE, state=state) self.portal.ERP5Site_receiveOpenIdCallback(code=CODE, state=state)
getAccessTokenFromCode_mock.assert_called_once() getAccessTokenFromCode_mock.assert_called_once()
getUserEntry_mock.assert_called_once() getUserEntry_mock.assert_called_once()
...@@ -260,8 +260,8 @@ return credential_request ...@@ -260,8 +260,8 @@ return credential_request
'erp5.component.extension.OpenIdConnectLoginUtility.getUserEntry', 'erp5.component.extension.OpenIdConnectLoginUtility.getUserEntry',
side_effect=getUserEntry side_effect=getUserEntry
) as getUserEntry_mock: ) as getUserEntry_mock:
getAccessTokenFromCode_mock.func_code = getAccessTokenFromCode.func_code getAccessTokenFromCode_mock.__code__ = getAccessTokenFromCode.__code__
getUserEntry_mock.func_code = getUserEntry.func_code getUserEntry_mock.__code__ = getUserEntry.__code__
self.portal.ERP5Site_receiveOpenIdCallback(code=CODE, state=state) self.portal.ERP5Site_receiveOpenIdCallback(code=CODE, state=state)
getAccessTokenFromCode_mock.assert_called_once() getAccessTokenFromCode_mock.assert_called_once()
getUserEntry_mock.assert_called_once() getUserEntry_mock.assert_called_once()
......
...@@ -75,7 +75,7 @@ class TestSecurityMixin(ERP5TypeTestCase): ...@@ -75,7 +75,7 @@ class TestSecurityMixin(ERP5TypeTestCase):
allowed_method_id_list = ['om_icons',] allowed_method_id_list = ['om_icons',]
app = self.portal.aq_parent app = self.portal.aq_parent
meta_type_set = set([None]) meta_type_set = set([None])
error_set = set() error_dict = {}
for _, obj in app.ZopeFind(app, search_sub=1): for _, obj in app.ZopeFind(app, search_sub=1):
meta_type = getattr(obj, 'meta_type', None) meta_type = getattr(obj, 'meta_type', None)
if meta_type in meta_type_set: if meta_type in meta_type_set:
...@@ -88,17 +88,20 @@ class TestSecurityMixin(ERP5TypeTestCase): ...@@ -88,17 +88,20 @@ class TestSecurityMixin(ERP5TypeTestCase):
continue continue
method = getattr(obj, method_id) method = getattr(obj, method_id)
if isinstance(method, MethodType) and \ if isinstance(method, MethodType) and \
getattr(method, 'func_name', None) is not None and \ getattr(method, '__name__', None) is not None and \
method.__doc__ and \ method.__doc__ and \
not hasattr(obj, '%s__roles__' % method_id) and \ not hasattr(obj, '%s__roles__' % method_id) and \
not hasattr(method, '__roles__') and \
method.__module__: method.__module__:
if method.__module__ == 'Products.ERP5Type.Accessor.WorkflowState' and method.func_code.co_name == 'serialize': if method.__module__ == 'Products.ERP5Type.Accessor.WorkflowState' and method.__code__.co_name == 'serialize':
continue continue
func_code = method.__code__ func_code = method.__code__
error_set.add((func_code.co_filename, func_code.co_firstlineno, method_id)) if not hasattr(func_code, 'co_filename'): # ERP5 Accessor
func_code = method.__func__.__class__.__init__.__code__
error_dict[(func_code.co_filename, func_code.co_firstlineno)] = method_id
error_list = [] error_list = []
for filename, lineno, method_id in sorted(error_set): for (filename, lineno), method_id in sorted(error_dict.items()):
# ignore security problems with non ERP5 documents, unless running in debug mode. # ignore security problems with non ERP5 documents, unless running in debug mode.
if os.environ.get('erp5_debug_mode') or '/erp5/' in filename or '<portal_components' in filename: if os.environ.get('erp5_debug_mode') or '/erp5/' in filename or '<portal_components' in filename:
error_list.append('%s:%s %s' % (filename, lineno, method_id)) error_list.append('%s:%s %s' % (filename, lineno, method_id))
......
...@@ -31,7 +31,7 @@ from AccessControl import ClassSecurityInfo ...@@ -31,7 +31,7 @@ from AccessControl import ClassSecurityInfo
from AccessControl.SecurityManagement import getSecurityManager,\ from AccessControl.SecurityManagement import getSecurityManager,\
setSecurityManager, newSecurityManager setSecurityManager, newSecurityManager
from AccessControl.ZopeGuards import guarded_getattr from AccessControl.ZopeGuards import guarded_getattr
from MethodObject import Method from Products.ERP5Type.Accessor.Base import Getter
from Products.ERP5Type.Globals import InitializeClass, DTMLFile from Products.ERP5Type.Globals import InitializeClass, DTMLFile
from zLOG import LOG, PROBLEM from zLOG import LOG, PROBLEM
...@@ -54,7 +54,7 @@ class Priority: ...@@ -54,7 +54,7 @@ class Priority:
class func_code: pass class func_code: pass
class PreferenceMethod(Method): class PreferenceMethod(Getter):
""" A method object that lookup the attribute on preferences. """ """ A method object that lookup the attribute on preferences. """
# This is required to call the method form the Web # This is required to call the method form the Web
__code__ = func_code = func_code() __code__ = func_code = func_code()
......
...@@ -82,7 +82,7 @@ class Dummy(Reindex): ...@@ -82,7 +82,7 @@ class Dummy(Reindex):
self._id = id self._id = id
self.__name__ = id self.__name__ = id
self._accessor_id = accessor_id self._accessor_id = accessor_id
# self.__code__ = func_code = getattr(instance, self._accessor_id).func_code # self.__code__ = func_code = getattr(instance, self._accessor_id).__code__
def __call__(self, instance, *args, **kw): def __call__(self, instance, *args, **kw):
method = getattr(instance, self._accessor_id) method = getattr(instance, self._accessor_id)
......
...@@ -203,19 +203,10 @@ class Getter(Method): ...@@ -203,19 +203,10 @@ class Getter(Method):
return getattr(roles, '__of__', lambda aq_parent: roles)(self) return getattr(roles, '__of__', lambda aq_parent: roles)(self)
class Tester(Method): class Tester(Getter):
""" """
Tests if an attribute value exists Tests if an attribute value exists
""" """
_need__name__=1
# Generic Definition of Method Object
# This is required to call the method form the Web
__code__ = func_code = func_code()
__code__.co_varnames = ('self',)
__code__.co_argcount = 1
__defaults__ = func_defaults = ()
def __init__(self, id, key, property_type, storage_id=None): def __init__(self, id, key, property_type, storage_id=None):
self._id = id self._id = id
self.__name__ = id self.__name__ = id
......
...@@ -96,6 +96,8 @@ class Getter(Accessor): ...@@ -96,6 +96,8 @@ class Getter(Accessor):
self._key = key self._key = key
self.__name__ = id self.__name__ = id
self.value = value self.value = value
# Do not publish deprecated Constant getter
self.__doc__ = None
def __call__(self, instance): def __call__(self, instance):
return self.value return self.value
...@@ -208,7 +208,7 @@ class ListGetter(Base.Getter): ...@@ -208,7 +208,7 @@ class ListGetter(Base.Getter):
DefaultGetter = Getter DefaultGetter = Getter
class Tester(Method): class Tester(Base.Tester):
""" """
Tests if an attribute value exists Tests if an attribute value exists
""" """
......
...@@ -267,7 +267,7 @@ class ListGetter(Base.Getter): ...@@ -267,7 +267,7 @@ class ListGetter(Base.Getter):
DefaultGetter = Getter DefaultGetter = Getter
class Tester(Method): class Tester(Base.Tester):
""" """
Tests if an attribute value exists Tests if an attribute value exists
""" """
......
...@@ -31,7 +31,7 @@ from zLOG import LOG ...@@ -31,7 +31,7 @@ from zLOG import LOG
from Products.ERP5Type.PsycoWrapper import psyco from Products.ERP5Type.PsycoWrapper import psyco
from Acquisition import aq_base from Acquisition import aq_base
from Products.ERP5Type.Accessor.Base import func_code, ATTRIBUTE_PREFIX, evaluateTales, Getter as BaseGetter, Method from Products.ERP5Type.Accessor.Base import func_code, ATTRIBUTE_PREFIX, evaluateTales, Getter as BaseGetter, Setter as BaseSetter, Tester as BaseTester
from Products.ERP5Type.Accessor import Accessor, AcquiredProperty from Products.ERP5Type.Accessor import Accessor, AcquiredProperty
from Products.ERP5Type.Accessor.TypeDefinition import type_definition from Products.ERP5Type.Accessor.TypeDefinition import type_definition
from Products.ERP5Type.Utils import unicode2str from Products.ERP5Type.Utils import unicode2str
...@@ -218,7 +218,7 @@ class AcquiredPropertyGetter(AcquiredProperty.Getter): ...@@ -218,7 +218,7 @@ class AcquiredPropertyGetter(AcquiredProperty.Getter):
else: else:
return default return default
class TranslatedPropertyTester(Method): class TranslatedPropertyTester(BaseTester):
""" """
Tests if an attribute value exists Tests if an attribute value exists
""" """
......
...@@ -353,7 +353,7 @@ def transactional_cached(key_method=_default_key_method): ...@@ -353,7 +353,7 @@ def transactional_cached(key_method=_default_key_method):
def decorator(function): def decorator(function):
# Unfornately, we can only check functions (not other callable like class). # Unfornately, we can only check functions (not other callable like class).
assert (key_method is not _default_key_method or assert (key_method is not _default_key_method or
not getattr(function, 'func_defaults', None)), ( not getattr(function, '__defaults__', None)), (
"default 'key_method' of 'transactional_cached' does not work with" "default 'key_method' of 'transactional_cached' does not work with"
" functions having default values for parameters") " functions having default values for parameters")
key = repr(function) key = repr(function)
......
...@@ -83,14 +83,18 @@ class _(PatchClass(ExternalMethod)): ...@@ -83,14 +83,18 @@ class _(PatchClass(ExternalMethod)):
return _f return _f
except AttributeError: except AttributeError:
pass pass
code = f.func_code code = f.__code__
argument_object = getargs(code) argument_object = getargs(code)
# reconstruct back the original names # reconstruct back the original names
arg_list = argument_object.args[:] arg_list = argument_object.args[:]
if argument_object.varargs: if argument_object.varargs:
arg_list.append('*' + argument_object.varargs) arg_list.append('*' + argument_object.varargs)
if argument_object.keywords: if six.PY2:
arg_list.append('**' + argument_object.keywords) if argument_object.keywords:
arg_list.append('**' + argument_object.keywords)
else:
if argument_object.varkw:
arg_list.append('**' + argument_object.varkw)
i = isinstance(f, MethodType) i = isinstance(f, MethodType)
ff = six.get_unbound_function(f) if i else f ff = six.get_unbound_function(f) if i else f
...@@ -98,7 +102,10 @@ class _(PatchClass(ExternalMethod)): ...@@ -98,7 +102,10 @@ class _(PatchClass(ExternalMethod)):
i += has_self i += has_self
if i: if i:
code = FuncCode(ff, i) code = FuncCode(ff, i)
self._v_f = _f = (f, f.func_defaults, code, has_self, arg_list) try: # This fails with mock function
self._v_f = _f = (f, f.__defaults__, code, has_self, arg_list)
except AttributeError:
self._v_f = _f = (f, f.func_defaults, code, has_self, arg_list)
return _f return _f
def __call__(self, *args, **kw): def __call__(self, *args, **kw):
......
...@@ -59,4 +59,4 @@ try: ...@@ -59,4 +59,4 @@ try:
except TypeError: except TypeError:
# We need to monkey patch in-place, as it is a top-level function and # We need to monkey patch in-place, as it is a top-level function and
# already imported in other places. # already imported in other places.
convertToUnicode.func_code = patched_convertToUnicode.func_code convertToUnicode.__code__ = patched_convertToUnicode.__code__
...@@ -57,7 +57,7 @@ def special_extract_tb(tb, limit = None): ...@@ -57,7 +57,7 @@ def special_extract_tb(tb, limit = None):
else: line = None else: line = None
# display where we failed in the sequence # display where we failed in the sequence
if co == Sequence.play.func_code: if co == Sequence.play.__code__:
if line is None: if line is None:
line = '' line = ''
sequence = f.f_locals['self'] sequence = f.f_locals['self']
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment