From 045e06abee70f8362735e7b911c147d87a11eadf Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 4 Nov 2019 13:57:52 -0500 Subject: [PATCH 1/3] When validating that envs have names, make sure that names containing only strings are not valid --- js/components/forms/new_application/environments.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/js/components/forms/new_application/environments.js b/js/components/forms/new_application/environments.js index 655fd817..e2a49c5e 100644 --- a/js/components/forms/new_application/environments.js +++ b/js/components/forms/new_application/environments.js @@ -91,7 +91,9 @@ export default { if (this.environments.length > 1) { // only want to display this error if we have multiple envs and one or // more do not have names - return this.environments.every(e => e.name !== '') + return this.environments.every( + e => !e.name.match(/^\s+$/) && e.name !== '' + ) } else { return true } From ab9b62f54b6c27c992f32242d43727f69de99a97 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 7 Nov 2019 11:25:18 -0500 Subject: [PATCH 2/3] Update validators and filter to remove strings that contain only whitespace The validator ListItemRequired() was only checking for None and an empty string, not for strings that were multiple whitespace characters. This fixes this issue by checking each item with regex to make sure it contains non whitespace characters The filter remove_empty_string() also was not checking for strings that were multiple whitespace characters. This was also fixed by using regex tomake sure that the string contains non whitespace characters, and also clips any trailing whitespace. --- .secrets.baseline | 11 +---------- atst/forms/application.py | 15 +++++++++++---- atst/forms/forms.py | 7 ++++++- atst/forms/portfolio.py | 2 -- atst/forms/validators.py | 6 ++++-- tests/forms/test_base_form.py | 22 +++++++++++++++++++++- tests/forms/test_validators.py | 17 +++++++++++++++++ 7 files changed, 60 insertions(+), 20 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 9e94b782..1060e869 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -3,7 +3,7 @@ "files": "^.secrets.baseline$|^.*pgsslrootcert.yml$", "lines": null }, - "generated_at": "2019-11-01T18:50:54Z", + "generated_at": "2019-11-06T21:35:23Z", "plugins_used": [ { "base64_limit": 4.5, @@ -170,15 +170,6 @@ "type": "Private Key" } ], - "tests/forms/test_validators.py": [ - { - "hashed_secret": "260408f687da9094705a841acda9b029563053ee", - "is_secret": false, - "is_verified": false, - "line_number": 30, - "type": "Hex High Entropy String" - } - ], "tests/routes/task_orders/test_new.py": [ { "hashed_secret": "e4f14805dfd1e6af030359090c535e149e6b4207", diff --git a/atst/forms/application.py b/atst/forms/application.py index 16db3827..ea71d1e3 100644 --- a/atst/forms/application.py +++ b/atst/forms/application.py @@ -7,24 +7,31 @@ from atst.utils.localization import translate class EditEnvironmentForm(BaseForm): name = StringField( - label=translate("forms.environments.name_label"), validators=[Required()] + label=translate("forms.environments.name_label"), + validators=[Required()], + filters=[BaseForm.remove_empty_string], ) class NameAndDescriptionForm(BaseForm): name = StringField( - label=translate("forms.application.name_label"), validators=[Required()] + label=translate("forms.application.name_label"), + validators=[Required()], + filters=[BaseForm.remove_empty_string], ) description = TextAreaField( label=translate("forms.application.description_label"), validators=[Optional()], - filters=[lambda x: x or None], + filters=[BaseForm.remove_empty_string], ) class EnvironmentsForm(BaseForm): environment_names = FieldList( - StringField(label=translate("forms.application.environment_names_label")), + StringField( + label=translate("forms.application.environment_names_label"), + filters=[BaseForm.remove_empty_string], + ), validators=[ ListItemRequired( message=translate( diff --git a/atst/forms/forms.py b/atst/forms/forms.py index 4287c81c..36bb7994 100644 --- a/atst/forms/forms.py +++ b/atst/forms/forms.py @@ -1,5 +1,6 @@ from flask_wtf import FlaskForm from flask import current_app, request as http_request +import re from atst.utils.flash import formatted_flash as flash @@ -38,4 +39,8 @@ class BaseForm(FlaskForm): @classmethod def remove_empty_string(cls, value): - return value or None + # only return strings that contain non whitespace characters + if value and re.search(r"\S", value): + return value.strip() + else: + return None diff --git a/atst/forms/portfolio.py b/atst/forms/portfolio.py index 6e807876..d9991471 100644 --- a/atst/forms/portfolio.py +++ b/atst/forms/portfolio.py @@ -78,7 +78,6 @@ class PortfolioCreationForm(BaseForm): description=translate("forms.task_order.complexity.description"), choices=APPLICATION_COMPLEXITY, default=None, - filters=[BaseForm.remove_empty_string], widget=ListWidget(prefix_label=False), option_widget=CheckboxInput(), ) @@ -94,7 +93,6 @@ class PortfolioCreationForm(BaseForm): description=translate("forms.task_order.dev_team.description"), choices=DEV_TEAM, default=None, - filters=[BaseForm.remove_empty_string], widget=ListWidget(prefix_label=False), option_widget=CheckboxInput(), ) diff --git a/atst/forms/validators.py b/atst/forms/validators.py index cac21e3e..a16bf4f5 100644 --- a/atst/forms/validators.py +++ b/atst/forms/validators.py @@ -65,10 +65,12 @@ def Name(message=translate("forms.validators.name_message")): def ListItemRequired( message=translate("forms.validators.list_item_required_message"), - empty_values=("", None), + empty_values=[None], ): def _list_item_required(form, field): - non_empty_values = [v for v in field.data if v not in empty_values] + non_empty_values = [ + v for v in field.data if (v not in empty_values and re.search(r"\S", v)) + ] if len(non_empty_values) == 0: raise ValidationError(message) diff --git a/tests/forms/test_base_form.py b/tests/forms/test_base_form.py index fe6456e7..5587649b 100644 --- a/tests/forms/test_base_form.py +++ b/tests/forms/test_base_form.py @@ -1,5 +1,5 @@ import pytest -from wtforms.fields import RadioField +from wtforms.fields import RadioField, FieldList, StringField from werkzeug.datastructures import ImmutableMultiDict from atst.forms.forms import BaseForm @@ -16,6 +16,12 @@ class FormWithChoices(BaseForm): ) +class FormWithList(BaseForm): + list = FieldList( + StringField("a very fancy list", filters=[BaseForm.remove_empty_string]) + ) + + class TestBaseForm: class Foo: person = {"force_side": None} @@ -34,3 +40,17 @@ class TestBaseForm: form_data_3 = ImmutableMultiDict({"force_side": "dark"}) form_3 = FormWithChoices(form_data_3, obj=self.obj) assert form_3.data["force_side"] is "dark" + + @pytest.mark.parametrize( + "form_data", + [["testing", "", "QA"], ["testing", " ", "QA"], ["testing", None, "QA"]], + ) + def test_blank_list_items_removed(self, form_data): + form = FormWithList(list=form_data) + assert form.validate(flash_invalid=False) + assert not form.data == ["testing", "QA"] + + def test_remove_empty_string_clips_whitespace(self): + form = FormWithList(list=[" QA", " testing "]) + assert form.validate(flash_invalid=False) + assert form.list.data == ["QA", "testing"] diff --git a/tests/forms/test_validators.py b/tests/forms/test_validators.py index f19d06cb..bac06f47 100644 --- a/tests/forms/test_validators.py +++ b/tests/forms/test_validators.py @@ -85,3 +85,20 @@ class TestFileLength: dummy_field.data = "random string" assert validator(dummy_form, dummy_field) + + +class TestListItemRequired: + @pytest.mark.parametrize("valid", [[" a", ""], ["a ", ""], ["a", ""]]) + def test_ListItemRequired(self, valid, dummy_form, dummy_field): + validator = ListItemRequired() + dummy_field.data = valid + validator(dummy_form, dummy_field) + + @pytest.mark.parametrize("invalid", [[""], [" "], [None], []]) + def test_ListItemRequired_rejects_blank_names( + self, invalid, dummy_form, dummy_field + ): + validator = ListItemRequired() + dummy_field.data = invalid + with pytest.raises(ValidationError): + validator(dummy_form, dummy_field) From 9037c44498335a4898b331e70f337ae930fb5e0c Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 12 Nov 2019 13:07:35 -0500 Subject: [PATCH 3/3] Move filter out of class definition and change name of form field --- atst/forms/application.py | 10 +++++----- atst/forms/forms.py | 16 ++++++++-------- atst/forms/portfolio.py | 8 ++++---- tests/forms/test_base_form.py | 12 ++++++------ 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/atst/forms/application.py b/atst/forms/application.py index ea71d1e3..de6294cd 100644 --- a/atst/forms/application.py +++ b/atst/forms/application.py @@ -1,4 +1,4 @@ -from .forms import BaseForm +from .forms import BaseForm, remove_empty_string from wtforms.fields import StringField, TextAreaField, FieldList from wtforms.validators import Required, Optional from atst.forms.validators import ListItemRequired, ListItemsUnique @@ -9,7 +9,7 @@ class EditEnvironmentForm(BaseForm): name = StringField( label=translate("forms.environments.name_label"), validators=[Required()], - filters=[BaseForm.remove_empty_string], + filters=[remove_empty_string], ) @@ -17,12 +17,12 @@ class NameAndDescriptionForm(BaseForm): name = StringField( label=translate("forms.application.name_label"), validators=[Required()], - filters=[BaseForm.remove_empty_string], + filters=[remove_empty_string], ) description = TextAreaField( label=translate("forms.application.description_label"), validators=[Optional()], - filters=[BaseForm.remove_empty_string], + filters=[remove_empty_string], ) @@ -30,7 +30,7 @@ class EnvironmentsForm(BaseForm): environment_names = FieldList( StringField( label=translate("forms.application.environment_names_label"), - filters=[BaseForm.remove_empty_string], + filters=[remove_empty_string], ), validators=[ ListItemRequired( diff --git a/atst/forms/forms.py b/atst/forms/forms.py index 36bb7994..cd63b278 100644 --- a/atst/forms/forms.py +++ b/atst/forms/forms.py @@ -7,6 +7,14 @@ from atst.utils.flash import formatted_flash as flash EMPTY_LIST_FIELD = ["", None] +def remove_empty_string(value): + # only return strings that contain non whitespace characters + if value and re.search(r"\S", value): + return value.strip() + else: + return None + + class BaseForm(FlaskForm): def __init__(self, formdata=None, **kwargs): # initialize the form with data from the cache @@ -36,11 +44,3 @@ class BaseForm(FlaskForm): if not valid and flash_invalid: flash("form_errors") return valid - - @classmethod - def remove_empty_string(cls, value): - # only return strings that contain non whitespace characters - if value and re.search(r"\S", value): - return value.strip() - else: - return None diff --git a/atst/forms/portfolio.py b/atst/forms/portfolio.py index d9991471..307f463a 100644 --- a/atst/forms/portfolio.py +++ b/atst/forms/portfolio.py @@ -8,7 +8,7 @@ from wtforms.fields import ( from wtforms.validators import Length, Optional from wtforms.widgets import ListWidget, CheckboxInput -from .forms import BaseForm +from .forms import BaseForm, remove_empty_string from atst.utils.localization import translate from .data import ( @@ -49,7 +49,7 @@ class PortfolioCreationForm(BaseForm): translate("forms.task_order.defense_component_label"), choices=SERVICE_BRANCHES, default="", - filters=[BaseForm.remove_empty_string], + filters=[remove_empty_string], ) description = TextAreaField( @@ -85,7 +85,7 @@ class PortfolioCreationForm(BaseForm): complexity_other = StringField( translate("forms.task_order.complexity_other_label"), default=None, - filters=[BaseForm.remove_empty_string], + filters=[remove_empty_string], ) dev_team = SelectMultipleField( @@ -100,7 +100,7 @@ class PortfolioCreationForm(BaseForm): dev_team_other = StringField( translate("forms.task_order.dev_team_other_label"), default=None, - filters=[BaseForm.remove_empty_string], + filters=[remove_empty_string], ) team_experience = RadioField( diff --git a/tests/forms/test_base_form.py b/tests/forms/test_base_form.py index 5587649b..8f0c000e 100644 --- a/tests/forms/test_base_form.py +++ b/tests/forms/test_base_form.py @@ -2,7 +2,7 @@ import pytest from wtforms.fields import RadioField, FieldList, StringField from werkzeug.datastructures import ImmutableMultiDict -from atst.forms.forms import BaseForm +from atst.forms.forms import BaseForm, remove_empty_string class FormWithChoices(BaseForm): @@ -17,8 +17,8 @@ class FormWithChoices(BaseForm): class FormWithList(BaseForm): - list = FieldList( - StringField("a very fancy list", filters=[BaseForm.remove_empty_string]) + fancy_list = FieldList( + StringField("a very fancy list", filters=[remove_empty_string]) ) @@ -46,11 +46,11 @@ class TestBaseForm: [["testing", "", "QA"], ["testing", " ", "QA"], ["testing", None, "QA"]], ) def test_blank_list_items_removed(self, form_data): - form = FormWithList(list=form_data) + form = FormWithList(fancy_list=form_data) assert form.validate(flash_invalid=False) assert not form.data == ["testing", "QA"] def test_remove_empty_string_clips_whitespace(self): - form = FormWithList(list=[" QA", " testing "]) + form = FormWithList(fancy_list=[" QA", " testing "]) assert form.validate(flash_invalid=False) - assert form.list.data == ["QA", "testing"] + assert form.fancy_list.data == ["QA", "testing"]