From 81977274fd4b01db79cec3da2dc5fd361a02d56b Mon Sep 17 00:00:00 2001 From: Thomas NOEL Date: Mon, 26 Mar 2018 17:02:41 +0200 Subject: [PATCH] add fault tolerance on convert_value_from_anything usage (#22793) ... and make None a legitimate value. --- tests/test_hobo_notify.py | 33 +++++++++++++++++++++++++ tests/test_workflows.py | 59 ++++++++++++++++++++++++++++++++++++--------- wcs/ctl/hobo_notify.py | 8 ++++-- wcs/fields.py | 7 +++--- wcs/qommon/saml2.py | 8 ++++-- wcs/wf/backoffice_fields.py | 6 ++--- wcs/wf/profile.py | 10 ++++++-- 7 files changed, 108 insertions(+), 23 deletions(-) diff --git a/tests/test_hobo_notify.py b/tests/test_hobo_notify.py index dad592c8..03cb9c64 100644 --- a/tests/test_hobo_notify.py +++ b/tests/test_hobo_notify.py @@ -544,6 +544,39 @@ def test_process_notification_user_provision(pub): assert user.is_admin is True assert set(user.roles) == set([old_role.id]) + for birthdate in ('baddate', '', None): + notification = { + u'@type': u'provision', + u'issuer': 'http://idp.example.net/idp/saml/metadata', + u'audience': [u'test'], + u'objects': { + u'@type': 'user', + u'data': [ + { + u'uuid': u'a' * 32, + u'first_name': u'John', + u'last_name': u'Doe', + u'email': u'john.doe@example.net', + u'zipcode': u'13600', + u'birthdate': birthdate, + u'is_superuser': True, + u'roles': [ + { + u'uuid': u'xyz', + u'name': u'Service état civil', + u'description': u'etc.', + }, + ], + } + ] + } + } + CmdHoboNotify.process_notification(notification) + assert User.count() == 1 + if birthdate is not None: # wrong value : no nothing + assert User.select()[0].form_data['_birthdate'].tm_year == 2000 + else: # empty value : empty field + assert User.select()[0].form_data['_birthdate'] is None def notify_of_exception(exc_info, context): raise Exception(exc_info) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index f579f85c..be4e9bf9 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -2893,6 +2893,26 @@ def test_profile(two_pubs): assert http_patch_request.call_count == 1 assert http_patch_request.call_args[0][1] == '{"bar": "2018-03-20"}' + user.form_data['4'] = datetime.datetime.now().timetuple() + user.store() + year = User.get(user.id).form_data.get('4').tm_year + for date_value in ('baddate', '', {}, [], None): + item.fields = [{'field_id': 'bar', 'value': date_value}] + item.perform(formdata) + if date_value is not None: # bad value : do nothing + assert User.get(user.id).form_data.get('4').tm_year == year + else: # empty value : empty field + assert User.get(user.id).form_data.get('4') == None + + with mock.patch('wcs.wf.profile.http_patch_request') as http_patch_request: + http_patch_request.return_value = (None, 200, '', None) + get_response().process_after_jobs() + assert http_patch_request.call_count == 1 + if date_value is not None: # bad value : do nothing + assert http_patch_request.call_args[0][1] == '{}' + else: # empty value : null field + assert http_patch_request.call_args[0][1] == '{"bar": null}' + def test_set_backoffice_field(http_requests, two_pubs): Workflow.wipe() FormDef.wipe() @@ -3054,18 +3074,23 @@ def test_set_backoffice_field_file(http_requests, two_pubs): assert formdata.data['bo1'].base_filename == 'hello.txt' assert formdata.data['bo1'].get_content() == 'HELLO WORLD' - # check wrong value - del formdata.data['bo1'] - formdata.store() - assert not 'bo1' in formdata.data + hello_world = formdata.data['bo1'] + # check wrong value, or None (no file) + for value in ('="HELLO"', '', 'BAD', '={}', '=[]', None): + formdata.data['bo1'] = hello_world + formdata.store() - item = SetBackofficeFieldsWorkflowStatusItem() - item.parent = st1 - item.fields = [{'field_id': 'bo1', 'value': '="HELLO"'}] - item.perform(formdata) + item = SetBackofficeFieldsWorkflowStatusItem() + item.parent = st1 + item.fields = [{'field_id': 'bo1', 'value': value}] + item.perform(formdata) - formdata = formdef.data_class().get(formdata.id) - assert formdata.data.get('bo1') is None + formdata = formdef.data_class().get(formdata.id) + if value is not None: # wrong value : do nothing + assert formdata.data['bo1'].base_filename == 'hello.txt' + assert formdata.data['bo1'].get_content() == 'HELLO WORLD' + else: # empty value : remove field + assert formdata.data['bo1'] is None # check wrong field item = SetBackofficeFieldsWorkflowStatusItem() @@ -3230,14 +3255,26 @@ def test_set_backoffice_field_date(two_pubs): formdata = formdef.data_class().get(formdata.id) assert datetime.date(*formdata.data['bo1'][:3]) == datetime.date(2017, 3, 23) + # invalid values => do nothing + for value in ('plop', '', {}, [], '{{ blah }}'): + item = SetBackofficeFieldsWorkflowStatusItem() + item.parent = st1 + item.fields = [{'field_id': 'bo1', 'value': value}] + + item.perform(formdata) + formdata = formdef.data_class().get(formdata.id) + assert datetime.date(*formdata.data['bo1'][:3]) == datetime.date(2017, 3, 23) + + # None : empty date item = SetBackofficeFieldsWorkflowStatusItem() item.parent = st1 - item.fields = [{'field_id': 'bo1', 'value': "plop"}] + item.fields = [{'field_id': 'bo1', 'value': None}] item.perform(formdata) formdata = formdef.data_class().get(formdata.id) assert formdata.data['bo1'] is None + def test_set_backoffice_field_immediate_use(http_requests, two_pubs): Workflow.wipe() FormDef.wipe() diff --git a/wcs/ctl/hobo_notify.py b/wcs/ctl/hobo_notify.py index 66a3f28e..30d15355 100644 --- a/wcs/ctl/hobo_notify.py +++ b/wcs/ctl/hobo_notify.py @@ -189,8 +189,12 @@ class CmdHoboNotify(Command): if not field.id.startswith('_'): continue field_value = o.get(field.id[1:]) - if field_value and field.convert_value_from_anything: - field_value = field.convert_value_from_anything(field_value) + if field.convert_value_from_anything: + try: + field_value = field.convert_value_from_anything(field_value) + except ValueError: + publisher.notify_of_exception(sys.exc_info(), context='[PROVISIONNING]') + continue user.form_data[field.id] = field_value user.name_identifiers = [uuid] role_uuids = [role['uuid'] for role in o['roles']] diff --git a/wcs/fields.py b/wcs/fields.py index a58a9a0f..bc1815a0 100644 --- a/wcs/fields.py +++ b/wcs/fields.py @@ -788,6 +788,8 @@ class FileField(WidgetField): @classmethod def convert_value_from_anything(cls, value): + if value is None: + return None if hasattr(value, 'base_filename'): upload = PicklableUpload(value.base_filename, value.content_type or 'application/octet-stream') @@ -972,10 +974,9 @@ class DateField(WidgetField): @classmethod def convert_value_from_anything(cls, value): - try: - date_value = evalutils.make_date(value).timetuple() - except ValueError: + if value is None: return None + date_value = evalutils.make_date(value).timetuple() # could raise ValueError return date_value def convert_value_from_str(self, value): diff --git a/wcs/qommon/saml2.py b/wcs/qommon/saml2.py index 7c0fe696..9df89765 100644 --- a/wcs/qommon/saml2.py +++ b/wcs/qommon/saml2.py @@ -493,8 +493,12 @@ class Saml2Directory(Directory): continue field_value = d[key] field = dict_fields.get(field_id) - if field and field_value and field.convert_value_from_anything: - field_value = field.convert_value_from_anything(field_value) + if field and field.convert_value_from_anything: + try: + field_value = field.convert_value_from_anything(field_value) + except ValueError: + publisher.notify_of_exception(sys.exc_info(), context='[SAML]') + continue if user.form_data.get(field_id) != field_value: user.form_data[field_id] = field_value logger.info('setting field %s of user %s to value %r', field_id, user.id, field_value) diff --git a/wcs/wf/backoffice_fields.py b/wcs/wf/backoffice_fields.py index 6653edb6..44569472 100644 --- a/wcs/wf/backoffice_fields.py +++ b/wcs/wf/backoffice_fields.py @@ -97,14 +97,14 @@ class SetBackofficeFieldsWorkflowStatusItem(WorkflowStatusItem): try: new_value = self.compute(field['value'], raises=True) except: - get_publisher().notify_of_exception(sys.exc_info()) + get_publisher().notify_of_exception(sys.exc_info(), context='[BO_FIELDS]') continue - if new_value and formdef_field.convert_value_from_anything: + if formdef_field.convert_value_from_anything: try: new_value = formdef_field.convert_value_from_anything(new_value) except ValueError: - get_publisher().notify_of_exception(sys.exc_info()) + get_publisher().notify_of_exception(sys.exc_info(), context='[BO_FIELDS]') continue formdata.data['%s' % field['field_id']] = new_value diff --git a/wcs/wf/profile.py b/wcs/wf/profile.py index cd6a3ee5..dd0b61f0 100644 --- a/wcs/wf/profile.py +++ b/wcs/wf/profile.py @@ -156,8 +156,14 @@ class UpdateUserProfileStatusItem(WorkflowStatusItem): for field in user_formdef.fields: if field.varname in new_data: field_value = new_data.get(field.varname) - if field and field_value and field.convert_value_from_anything: - field_value = field.convert_value_from_anything(field_value) + if field and field.convert_value_from_anything: + try: + field_value = field.convert_value_from_anything(field_value) + except ValueError: + get_publisher().notify_of_exception(sys.exc_info(), context='[PROFILE]') + # invalid attribute, do not update it + del new_data[field.varname] + continue new_user_data[field.id] = field_value # also change initial value to the converted one, as the # initial dictionary is used when sending the profile changes -- 2.16.2