diff --git a/backend/README.md b/backend/README.md index dec4372..e91caec 100644 --- a/backend/README.md +++ b/backend/README.md @@ -126,19 +126,7 @@ or store it into an internal catalog (or both). If block is set to send data, an email with form data will be sent to the recipient set in block settings or (if not set) to the site address. -If there is an `attachments` field in the POST data, these files will be attached to the email sent. - -#### XML attachments - -An XML copy of the data can be optionally attached to the sent email by configuring the volto block's `attachXml` option. - -The sent XML follows the same format as the feature in [collective.easyform](https://github.com/collective/collective.easyform). An example is shown below: - -```xml -
My value
-``` - -The field names in the XML will utilise the Data ID Mapping feature if it is used. Read more about this feature in the following Store section of the documentation. +If there are file upload fields in the POST data, these files will be attached to the email sent. #### Acknowledgement email @@ -220,9 +208,9 @@ If honeypot dependency is available in the buildout, the honeypot validation is Default field name is `protected_1` and you can change it with an environment variable. See `collective.honeypot `\_ for details. -## Attachments upload limits +## File upload limits -Forms can have one or more attachment field to allow users to upload some files. +Forms can have one or more file upload fields to allow users to upload some files. These files will be sent via mail, so it could be a good idea setting a limit to them. For example if you use Gmail as mail server, you can't send messages with attachments > 25MB. @@ -237,8 +225,6 @@ There is an environment variable that you can use to set that limit (in MB): By default this is not set. -The upload limit is also passed to the frontend in the form data with the `attachments_limit` key. - ## Content-transfer-encoding It is possible to set the content-transfer-encoding for the email body, settings the environment diff --git a/backend/src/collective/volto/formsupport/interfaces.py b/backend/src/collective/volto/formsupport/interfaces.py index 52d72ba..0bf5e6d 100644 --- a/backend/src/collective/volto/formsupport/interfaces.py +++ b/backend/src/collective/volto/formsupport/interfaces.py @@ -56,9 +56,34 @@ class FormSubmissionContext: context: DexterityContent block: dict form_data: dict - attachments: dict request: BaseRequest + def get_records(self) -> list: + """ + Return field id, value, and label. + + Skips file upload fields. + """ + records = [] + for k, v in self.form_data.items(): + field = self.block["schema"]["properties"].get(k, {}) + if field.get("type") == "object": + continue + records.append({ + "field_id": k, + "value": v, + "label": field.get("title", k), + }) + return records + + def get_attachments(self) -> dict: + attachments = {} + for k, v in self.form_data.items(): + field = self.block["schema"]["properties"].get(k, {}) + if field.get("factory") == "File Upload": + attachments[k] = v + return attachments + class IFormSubmissionProcessor(Interface): """Subscriber which processes form data when it is submitted""" diff --git a/backend/src/collective/volto/formsupport/processors/__init__.py b/backend/src/collective/volto/formsupport/processors/__init__.py index 4151039..e69de29 100644 --- a/backend/src/collective/volto/formsupport/processors/__init__.py +++ b/backend/src/collective/volto/formsupport/processors/__init__.py @@ -1,12 +0,0 @@ -def filter_parameters(data, block): - """ - TODO do not send attachments fields. - """ - return [ - { - "field_id": k, - "value": v, - "label": block["schema"]["properties"].get(k, {}).get("title", k), - } - for k, v in data.items() - ] diff --git a/backend/src/collective/volto/formsupport/processors/email.py b/backend/src/collective/volto/formsupport/processors/email.py index 16883b2..ebc04a8 100644 --- a/backend/src/collective/volto/formsupport/processors/email.py +++ b/backend/src/collective/volto/formsupport/processors/email.py @@ -2,7 +2,6 @@ from collective.volto.formsupport import _ from collective.volto.formsupport.interfaces import FormSubmissionContext from collective.volto.formsupport.interfaces import IFormSubmissionProcessor -from collective.volto.formsupport.processors import filter_parameters from email import policy from email.message import EmailMessage from plone import api @@ -39,7 +38,8 @@ def __init__(self, context: FormSubmissionContext): self.request = context.request self.block = context.block self.form_data = context.form_data - self.attachments = context.attachments + self.records = context.get_records() + self.attachments = context.get_attachments() def __call__(self): if not self.block.get("send"): @@ -93,7 +93,7 @@ def __call__(self): if header_value: msg[header] = header_value - self.manage_attachments(msg=msg) + self.add_attachments(msg=msg) self.send_mail(msg=msg, charset=charset) @@ -128,7 +128,12 @@ def get_reply_to(self): return sender def get_subject(self): - subject = self.block.get("subject") or "${subject}" + subject = self.block.get("subject") + if not subject: + if "subject" in self.block["schema"].get("properties", {}): + subject = "${subject}" + else: + subject = self.block.get("title") or "Form Submission" subject = self.substitute_variables(subject) return subject @@ -168,7 +173,7 @@ def prepare_message(self): request=self.request, ) parameters = { - "parameters": filter_parameters(self.form_data, self.block), + "parameters": self.records, "url": self.context.absolute_url(), "title": self.context.Title(), "mail_header": mail_header, @@ -176,12 +181,10 @@ def prepare_message(self): } return message_template(**parameters) - def manage_attachments(self, msg): - attachments = self.attachments - - if not attachments: + def add_attachments(self, msg): + if not self.attachments: return [] - for _key, value in attachments.items(): + for _key, value in self.attachments.items(): content_type = "application/octet-stream" filename = None if isinstance(value, dict): diff --git a/backend/src/collective/volto/formsupport/processors/store.py b/backend/src/collective/volto/formsupport/processors/store.py index 37a4f07..1f8a667 100644 --- a/backend/src/collective/volto/formsupport/processors/store.py +++ b/backend/src/collective/volto/formsupport/processors/store.py @@ -1,7 +1,6 @@ from collective.volto.formsupport.interfaces import FormSubmissionContext from collective.volto.formsupport.interfaces import IFormDataStore from collective.volto.formsupport.interfaces import IFormSubmissionProcessor -from collective.volto.formsupport.processors import filter_parameters from zExceptions import BadRequest from zope.component import adapter from zope.component import getMultiAdapter @@ -20,12 +19,13 @@ def __init__(self, context): self.request = context.request self.block = context.block self.form_data = context.form_data + self.records = context.get_records() def __call__(self): if not self.block.get("store"): return store = getMultiAdapter((self.context, self.request), IFormDataStore) - res = store.add(data=filter_parameters(self.form_data, self.block)) + res = store.add(data=self.records) if not res: raise BadRequest("Unable to store data") diff --git a/backend/src/collective/volto/formsupport/restapi/services/submit_form/post.py b/backend/src/collective/volto/formsupport/restapi/services/submit_form/post.py index b9156d0..247efee 100644 --- a/backend/src/collective/volto/formsupport/restapi/services/submit_form/post.py +++ b/backend/src/collective/volto/formsupport/restapi/services/submit_form/post.py @@ -40,6 +40,12 @@ def reply(self): if self.block_id: self.block = self.get_block_data(block_id=self.block_id) self.form_data = self.cleanup_data() + self.form_submission_context = FormSubmissionContext( + context=self.context, + request=self.request, + block=self.block, + form_data=self.form_data, + ) self.validate_form() @@ -48,15 +54,8 @@ def reply(self): notify(PostEventService(self.context, self.body)) - form_submission_context = FormSubmissionContext( - context=self.context, - request=self.request, - block=self.block, - form_data=self.form_data, - attachments=self.body.get("attachments", {}), - ) for handler in sorted( - subscribers((form_submission_context,), IFormSubmissionProcessor), + subscribers((self.form_submission_context,), IFormSubmissionProcessor), key=lambda h: h.order, ): try: @@ -83,7 +82,8 @@ def cleanup_data(self): schema = self.block.get("schema", {}) form_data = self.body.get("data", {}) if not isinstance(form_data, dict): - raise BadRequest(translate( + raise BadRequest( + translate( _( "invalid_form_data", default="Invalid form data.", @@ -153,11 +153,10 @@ def validate_schema(self): raise BadRequest(json.dumps(errors)) def validate_attachments(self): - # TODO handle schemaForm attachments attachments_limit = os.environ.get("FORM_ATTACHMENTS_LIMIT", "") if not attachments_limit: return - attachments = self.body.get("attachments", {}) + attachments = self.form_submission_context.get_attachments() attachments_len = 0 for attachment in attachments.values(): data = attachment.get("data", "") diff --git a/backend/tests/functional/test_email_processor.py b/backend/tests/functional/test_email_processor.py index bc0f8c8..af74fbc 100644 --- a/backend/tests/functional/test_email_processor.py +++ b/backend/tests/functional/test_email_processor.py @@ -82,26 +82,11 @@ def test_email_not_sent_if_block_id_is_correct_but_form_data_missing( assert response.status_code == 400 assert res["message"] == "Empty form data." - def test_email_not_sent_if_block_id_is_correct_but_required_fields_missing( - self, submit_form - ): + def test_email_not_sent_if_all_fields_are_not_in_form_schema(self, submit_form): self.document.blocks = { "form-id": { "@type": "schemaForm", "send": True, - "schema": { - "fieldsets": [ - { - "id": "default", - "title": "Default", - "fields": ["xxx"], - }, - ], - "properties": { - "xxx": {}, - }, - "required": [], - }, }, } transaction.commit() @@ -116,13 +101,26 @@ def test_email_not_sent_if_block_id_is_correct_but_required_fields_missing( transaction.commit() res = response.json() assert response.status_code == 400 - assert res["message"] == "Missing required field: subject or from." + assert res["message"] == "Empty form data." - def test_email_not_sent_if_all_fields_are_not_in_form_schema(self, submit_form): + def test_email_sent_with_fallback_subject_and_sender(self, submit_form): self.document.blocks = { "form-id": { "@type": "schemaForm", "send": True, + "schema": { + "fieldsets": [ + { + "id": "default", + "title": "Default", + "fields": ["xxx"], + }, + ], + "properties": { + "xxx": {}, + }, + "required": [], + }, }, } transaction.commit() @@ -136,8 +134,15 @@ def test_email_not_sent_if_all_fields_are_not_in_form_schema(self, submit_form): ) transaction.commit() res = response.json() - assert response.status_code == 400 - assert res["message"] == "Empty form data." + assert response.status_code == 200 + assert res["data"] == {"xxx": "bar"} + + msg = self.mailhost.messages[0].decode("utf-8") + msg = re.sub(r"\s+", " ", msg) + assert "Subject: Form Submission" in msg + assert "From: site_addr@plone.com" in msg + assert "To: site_addr@plone.com" in msg + assert "Reply-To: site_addr@plone.com" in msg def test_email_sent_with_only_fields_from_schema(self, submit_form): self.document.blocks = { @@ -382,7 +387,7 @@ def test_email_with_sender_from_form_data(self, submit_form): "properties": { "message": {"title": "Message"}, "name": {"title": "Name"}, - "email": {} + "email": {}, }, "required": [], }, @@ -393,7 +398,11 @@ def test_email_with_sender_from_form_data(self, submit_form): response = submit_form( url=self.document_url, data={ - "data": {"message": "just want to say hi", "name": "Smith", "email": "smith@doe.com"}, + "data": { + "message": "just want to say hi", + "name": "Smith", + "email": "smith@doe.com", + }, "block_id": "form-id", }, ) @@ -426,7 +435,7 @@ def test_email_with_bcc_from_form_data(self, submit_form): "properties": { "message": {"title": "Message"}, "name": {"title": "Name"}, - "email": {} + "email": {}, }, "required": [], }, @@ -437,7 +446,11 @@ def test_email_with_bcc_from_form_data(self, submit_form): response = submit_form( url=self.document_url, data={ - "data": {"message": "just want to say hi", "name": "Smith", "email": "smith@doe.com"}, + "data": { + "message": "just want to say hi", + "name": "Smith", + "email": "smith@doe.com", + }, "block_id": "form-id", }, ) @@ -454,73 +467,65 @@ def test_email_with_bcc_from_form_data(self, submit_form): def test_send_attachment(self, submit_form, file_str): self.document.blocks = { - "text-id": {"@type": "text"}, "form-id": { - "@type": "form", - "default_subject": "block subject", - "default_from": "john@doe.com", - "send": ["recipient"], - "subblocks": [ - { - "field_id": "test", - "field_type": "text", - }, - { - "field_id": "message", - "field_type": "text", - }, - { - "field_id": "name", - "field_type": "text", + "@type": "schemaForm", + "send": True, + "subject": "test subject", + "schema": { + "fieldsets": [ + { + "id": "default", + "title": "Default", + "fields": ["attachment"], + }, + ], + "properties": { + "attachment": {"factory": "File Upload", "type": "object"}, }, - ], + "required": [], + }, }, } transaction.commit() response = submit_form( url=self.document_url, data={ - "data": [ - { - "field_id": "message", - "label": "Message", - "value": "just want to say hi", + "data": { + "attachment": { + "data": base64.b64encode(file_str), + "filename": "test.pdf", }, - {"field_id": "name", "label": "Name", "value": "Smith"}, - {"field_id": "test", "label": "Test", "value": "test text"}, - ], + }, "block_id": "form-id", - "attachments": {"foo": {"data": base64.b64encode(file_str)}}, }, ) transaction.commit() assert response.status_code == 200 assert len(self.mailhost.messages) == 1 + msg = self.mailhost.messages[0].decode("utf-8") + assert 'Content-Disposition: attachment; filename="test.pdf"' in msg def test_send_attachment_validate_size(self, monkeypatch, submit_form, file_str): monkeypatch.setenv("FORM_ATTACHMENTS_LIMIT", "1") self.document.blocks = { - "text-id": {"@type": "text"}, "form-id": { - "@type": "form", - "default_subject": "block subject", - "default_from": "john@doe.com", - "send": ["recipient"], - "subblocks": [ - { - "field_id": "test", - "field_type": "text", - }, - { - "field_id": "message", - "field_type": "text", - }, - { - "field_id": "name", - "field_type": "text", + "@type": "schemaForm", + "send": True, + "subject": "test subject", + "schema": { + "fieldsets": [ + { + "id": "default", + "title": "Default", + "fields": ["attachment"], + }, + ], + "properties": { + "attachment": {"factory": "File Upload", "type": "object"}, }, - ], + "required": [], + }, }, } transaction.commit() @@ -530,17 +535,13 @@ def test_send_attachment_validate_size(self, monkeypatch, submit_form, file_str) response = submit_form( url=self.document_url, data={ - "data": [ - { - "field_id": "message", - "label": "Message", - "value": "just want to say hi", + "data": { + "attachment": { + "data": base64.b64encode(file_str), + "filename": "test.pdf", }, - {"field_id": "name", "label": "Name", "value": "Smith"}, - {"field_id": "test", "label": "Test", "value": "test text"}, - ], + }, "block_id": "form-id", - "attachments": {"foo": {"data": base64.b64encode(file_str)}}, }, ) transaction.commit() @@ -797,292 +798,3 @@ def test_email_body_formatted_as_list(self, submit_form): assert "Reply-To: john@doe.com" in msg assert "Message: just want to say hi" in msg assert "Name: John" in msg - - def test_send_xml(self, submit_form): - self.document.blocks = { - "form-id": { - "@type": "form", - "send": True, - "attachXml": True, - "subblocks": [ - { - "field_id": "message", - "field_type": "text", - }, - { - "field_id": "name", - "field_type": "text", - }, - ], - }, - } - transaction.commit() - - form_data = [ - {"field_id": "message", "label": "Message", "value": "just want to say hi"}, - {"field_id": "name", "label": "Name", "value": "John"}, - ] - - response = submit_form( - url=self.document_url, - data={ - "from": "john@doe.com", - "data": form_data, - "subject": "test subject", - "block_id": "form-id", - }, - ) - transaction.commit() - assert response.status_code == 200 - msg = self.mailhost.messages[0].decode("utf-8") - - parsed_msgs = Parser().parsestr(msg) - # 1st index is the XML attachment - msg_contents = parsed_msgs.get_payload()[1].get_payload(decode=True) - - xml_tree = ET.fromstring(msg_contents) - for index, field in enumerate(xml_tree): - assert field.get("name") == form_data[index]["label"] - assert field.text == form_data[index]["value"] - - def test_submit_return_400_if_malformed_email_in_email_field(self, submit_form): - """ - email fields in frontend are set as "from" field_type - """ - self.document.blocks = { - "text-id": {"@type": "text"}, - "form-id": { - "@type": "form", - "default_subject": "block subject", - "default_from": "john@doe.com", - "send": ["recipient"], - "subblocks": [ - { - "field_id": "contact", - "field_type": "from", - }, - { - "field_id": "message", - "field_type": "text", - }, - { - "field_id": "name", - "field_type": "text", - }, - ], - }, - } - transaction.commit() - - response = submit_form( - url=self.document_url, - data={ - "data": [ - { - "field_id": "message", - "label": "Message", - "value": "just want to say hi", - }, - {"field_id": "name", "label": "Name", "value": "Smith"}, - {"field_id": "contact", "label": "Email", "value": "foo"}, - ], - "block_id": "form-id", - }, - ) - assert response.status_code == 400 - assert response.json()["message"] == 'Email not valid in "Email" field.' - - response = submit_form( - url=self.document_url, - data={ - "data": [ - { - "field_id": "message", - "label": "Message", - "value": "just want to say hi", - }, - {"field_id": "name", "label": "Name", "value": "Smith"}, - {"field_id": "contact", "label": "Email", "value": "foo@"}, - ], - "block_id": "form-id", - }, - ) - assert response.status_code == 400 - assert response.json()["message"] == 'Email not valid in "Email" field.' - - response = submit_form( - url=self.document_url, - data={ - "data": [ - { - "field_id": "message", - "label": "Message", - "value": "just want to say hi", - }, - {"field_id": "name", "label": "Name", "value": "Smith"}, - {"field_id": "contact", "label": "Email", "value": "foo@asd"}, - ], - "block_id": "form-id", - }, - ) - assert response.status_code == 400 - assert response.json()["message"] == 'Email not valid in "Email" field.' - - def test_submit_return_200_if_correct_email_in_email_field(self, submit_form): - """ - email fields in frontend are set as "from" field_type - """ - self.document.blocks = { - "text-id": {"@type": "text"}, - "form-id": { - "@type": "form", - "default_subject": "block subject", - "default_from": "john@doe.com", - "send": ["recipient"], - "subblocks": [ - { - "field_id": "contact", - "field_type": "from", - }, - { - "field_id": "message", - "field_type": "text", - }, - { - "field_id": "name", - "field_type": "text", - }, - ], - }, - } - transaction.commit() - - response = submit_form( - url=self.document_url, - data={ - "data": [ - { - "field_id": "message", - "label": "Message", - "value": "just want to say hi", - }, - {"field_id": "name", "label": "Name", "value": "Smith"}, - {"field_id": "contact", "label": "Email", "value": "foo@plone.org"}, - ], - "block_id": "form-id", - }, - ) - assert response.status_code == 200 - - def test_submit_return_200_with_submitted_data(self, submit_form): - """ - This is needed for confirm message - """ - self.document.blocks = { - "form-id": { - "@type": "form", - "send": ["recipient"], - "subblocks": [ - { - "field_id": "message", - "field_type": "text", - }, - { - "field_id": "name", - "field_type": "text", - }, - ], - }, - } - transaction.commit() - - response = submit_form( - url=self.document_url, - data={ - "from": "john@doe.com", - "data": [ - { - "field_id": "message", - "label": "Message", - "value": "just want to say hi", - }, - {"field_id": "name", "label": "Name", "value": "John"}, - ], - "subject": "test subject", - "block_id": "form-id", - }, - ) - transaction.commit() - assert response.status_code == 200 - msg = self.mailhost.messages[0].decode("utf-8") - msg = re.sub(r"\s+", " ", msg) - assert "Subject: test subject" in msg - assert "From: john@doe.com" in msg - assert "To: site_addr@plone.com" in msg - assert "Reply-To: john@doe.com" in msg - assert "Message: just want to say hi" in msg - assert "Name: John" in msg - - def test_cleanup_html_in_submitted_data(self, submit_form): - """ - This is needed for confirm message - """ - self.document.blocks = { - "form-id": { - "@type": "form", - "send": ["recipient"], - "subblocks": [ - { - "field_id": "message", - "field_type": "text", - }, - { - "field_id": "name", - "field_type": "text", - }, - ], - }, - } - transaction.commit() - - response = submit_form( - url=self.document_url, - data={ - "from": "john@doe.com", - "data": [ - { - "field_id": "message", - "label": "Message", - "value": "click here

keep tags

", # noqa: E501 - }, - { - "field_id": "name", - "label": "Name", - "value": " foo", - }, - ], - "subject": "test subject", - "block_id": "form-id", - }, - ) - transaction.commit() - assert response.status_code == 200 - res = response.json() - assert res == ({ - "data": [ - { - "field_id": "message", - "label": "Message", - "value": "click here keep tags", - }, - { - "field_id": "name", - "label": "Name", - "value": "alert('XSS') foo", - }, - ] - }) - transaction.commit() - msg = self.mailhost.messages[0].decode("utf-8") - msg = re.sub(r"\s+", " ", msg) - assert "Message: click here keep tags" in msg diff --git a/frontend/packages/volto-form-block/src/actions/index.js b/frontend/packages/volto-form-block/src/actions/index.js index 29dab5c..465de82 100644 --- a/frontend/packages/volto-form-block/src/actions/index.js +++ b/frontend/packages/volto-form-block/src/actions/index.js @@ -13,7 +13,7 @@ export const SUBMIT_FORM_ACTION = 'SUBMIT_FORM_ACTION'; * @param {Object} data * @returns {Object} attachments */ -export function submitForm(path = '', block_id, data, attachments, captcha) { +export function submitForm(path = '', block_id, data, captcha) { return { type: SUBMIT_FORM_ACTION, subrequest: block_id, @@ -23,7 +23,6 @@ export function submitForm(path = '', block_id, data, attachments, captcha) { data: { block_id, data, - attachments, captcha, }, }, diff --git a/frontend/packages/volto-form-block/src/schemaFormBlock/ViewSchemaForm.jsx b/frontend/packages/volto-form-block/src/schemaFormBlock/ViewSchemaForm.jsx index b5cf18a..6c12a01 100644 --- a/frontend/packages/volto-form-block/src/schemaFormBlock/ViewSchemaForm.jsx +++ b/frontend/packages/volto-form-block/src/schemaFormBlock/ViewSchemaForm.jsx @@ -28,7 +28,6 @@ const messages = defineMessages({ const FormBlockView = ({ data, id, properties, metadata, path }) => { const dispatch = useDispatch(); const intl = useIntl(); - let attachments = {}; const location = useLocation(); const propertyNames = keys(data.schema.properties); @@ -48,29 +47,27 @@ const FormBlockView = ({ data, id, properties, metadata, path }) => { delete formData.captchaToken; } - dispatch(submitForm(path, id, formData, attachments, captcha)).catch( - (err) => { - let message = - err?.response?.body?.error?.message || - err?.response?.body?.message || - err?.response?.text || - ''; - const errorsList = tryParseJSON(message); - let invariantErrors = []; - if (Array.isArray(errorsList)) { - invariantErrors = extractInvariantErrors(errorsList); - } - if (invariantErrors.length > 0) { - toast.error( - , - ); - } - }, - ); + dispatch(submitForm(path, id, formData, captcha)).catch((err) => { + let message = + err?.response?.body?.error?.message || + err?.response?.body?.message || + err?.response?.text || + ''; + const errorsList = tryParseJSON(message); + let invariantErrors = []; + if (Array.isArray(errorsList)) { + invariantErrors = extractInvariantErrors(errorsList); + } + if (invariantErrors.length > 0) { + toast.error( + , + ); + } + }); }; return ( diff --git a/frontend/packages/volto-form-block/src/schemaFormBlock/schema.js b/frontend/packages/volto-form-block/src/schemaFormBlock/schema.js index 5a5ef7f..9ffc991 100644 --- a/frontend/packages/volto-form-block/src/schemaFormBlock/schema.js +++ b/frontend/packages/volto-form-block/src/schemaFormBlock/schema.js @@ -279,6 +279,6 @@ export const schemaFormBlockSchema = ({ data, intl }) => { default: -1, }, }, - required: ['subject', ...conditional_required], + required: [...conditional_required], }; };