diff --git a/horizon/utils/validators.py b/horizon/utils/validators.py index 27d7f1d78a..d3baa96ef1 100644 --- a/horizon/utils/validators.py +++ b/horizon/utils/validators.py @@ -12,7 +12,10 @@ # License for the specific language governing permissions and limitations # under the License. +import re + from django.core.exceptions import ValidationError # noqa +from django.core import validators # noqa from django.utils.translation import ugettext_lazy as _ from horizon import conf @@ -47,3 +50,11 @@ def validate_port_or_colon_separated_port_range(port_range): raise ValidationError(_("Not a valid port number")) except ValueError: raise ValidationError(_("Port number must be integer")) + +# Same as POSIX [:print:]. Accordingly, diacritics are disallowed. +PRINT_REGEX = re.compile(r'^[\x20-\x7E]*$') + +validate_printable_ascii = validators.RegexValidator( + PRINT_REGEX, + _("The string may only contain ASCII printable characters."), + "invalid_characters") diff --git a/openstack_dashboard/dashboards/project/access_and_security/security_groups/forms.py b/openstack_dashboard/dashboards/project/access_and_security/security_groups/forms.py index 08e80ce90c..0e39e33d9b 100644 --- a/openstack_dashboard/dashboards/project/access_and_security/security_groups/forms.py +++ b/openstack_dashboard/dashboards/project/access_and_security/security_groups/forms.py @@ -20,7 +20,6 @@ import netaddr from django.conf import settings from django.core.urlresolvers import reverse -from django.core import validators from django.forms import ValidationError # noqa from django.utils.translation import ugettext_lazy as _ @@ -33,62 +32,68 @@ from openstack_dashboard import api from openstack_dashboard.utils import filters -class CreateGroup(forms.SelfHandlingForm): +class GroupBase(forms.SelfHandlingForm): + """Base class to handle creation and update of security groups. + + Children classes must define two attributes: + + .. attribute:: success_message + + A success message containing the placeholder %s, + which will be replaced by the group name. + + .. attribute:: error_message + + An error message containing the placeholder %s, + which will be replaced by the error message. + """ name = forms.CharField(label=_("Name"), max_length=255, - error_messages={ - 'required': _('This field is required.'), - 'invalid': _("The string may only contain" - " ASCII characters and numbers.")}, - validators=[validators.validate_slug]) + validators=[ + utils_validators.validate_printable_ascii]) description = forms.CharField(label=_("Description"), required=False, widget=forms.Textarea(attrs={'rows': 4})) + def _call_network_api(self, request, data): + """Call the underlying network API: Nova-network or Neutron. + + Used in children classes to create or update a group. + """ + raise NotImplementedError() + def handle(self, request, data): try: - sg = api.network.security_group_create(request, - data['name'], - data['description']) - messages.success(request, - _('Successfully created security group: %s') - % data['name']) + sg = self._call_network_api(request, data) + messages.success(request, self.success_message % sg.name) return sg - except Exception: + except Exception as e: redirect = reverse("horizon:project:access_and_security:index") - exceptions.handle(request, - _('Unable to create security group.'), - redirect=redirect) + error_msg = self.error_message % e + exceptions.handle(request, error_msg, redirect=redirect) -class UpdateGroup(forms.SelfHandlingForm): +class CreateGroup(GroupBase): + success_message = _('Successfully created security group: %s') + error_message = _('Unable to create security group: %s') + + def _call_network_api(self, request, data): + return api.network.security_group_create(request, + data['name'], + data['description']) + + +class UpdateGroup(GroupBase): + success_message = _('Successfully updated security group: %s') + error_message = _('Unable to update security group: %s') + id = forms.CharField(widget=forms.HiddenInput()) - name = forms.CharField(label=_("Name"), - max_length=255, - error_messages={ - 'required': _('This field is required.'), - 'invalid': _("The string may only contain" - " ASCII characters and numbers.")}, - validators=[validators.validate_slug]) - description = forms.CharField(label=_("Description"), - required=False, - widget=forms.Textarea(attrs={'rows': 4})) - def handle(self, request, data): - try: - sg = api.network.security_group_update(request, - data['id'], - data['name'], - data['description']) - messages.success(request, - _('Successfully updated security group: %s') - % data['name']) - return sg - except Exception: - redirect = reverse("horizon:project:access_and_security:index") - exceptions.handle(request, - _('Unable to update security group.'), - redirect=redirect) + def _call_network_api(self, request, data): + return api.network.security_group_update(request, + data['id'], + data['name'], + data['description']) class AddRule(forms.SelfHandlingForm): diff --git a/openstack_dashboard/dashboards/project/access_and_security/security_groups/tests.py b/openstack_dashboard/dashboards/project/access_and_security/security_groups/tests.py index 7713668395..395dbd2a9b 100644 --- a/openstack_dashboard/dashboards/project/access_and_security/security_groups/tests.py +++ b/openstack_dashboard/dashboards/project/access_and_security/security_groups/tests.py @@ -35,6 +35,16 @@ INDEX_URL = reverse('horizon:project:access_and_security:index') SG_CREATE_URL = reverse('horizon:project:access_and_security:' 'security_groups:create') +SG_VIEW_PATH = 'horizon:project:access_and_security:security_groups:%s' +SG_DETAIL_VIEW = SG_VIEW_PATH % 'detail' +SG_UPDATE_VIEW = SG_VIEW_PATH % 'update' +SG_ADD_RULE_VIEW = SG_VIEW_PATH % 'add_rule' + +SG_TEMPLATE_PATH = 'project/access_and_security/security_groups/%s' +SG_DETAIL_TEMPLATE = SG_TEMPLATE_PATH % 'detail.html' +SG_CREATE_TEMPLATE = SG_TEMPLATE_PATH % 'create.html' +SG_UPDATE_TEMPLATE = SG_TEMPLATE_PATH % '_update.html' + def strip_absolute_base(uri): return uri.split(settings.TESTSERVER, 1)[-1] @@ -46,12 +56,9 @@ class SecurityGroupsViewTests(test.TestCase): def setUp(self): super(SecurityGroupsViewTests, self).setUp() sec_group = self.security_groups.first() - self.detail_url = reverse('horizon:project:access_and_security:' - 'security_groups:detail', - args=[sec_group.id]) - self.edit_url = reverse('horizon:project:access_and_security:' - 'security_groups:add_rule', - args=[sec_group.id]) + self.detail_url = reverse(SG_DETAIL_VIEW, args=[sec_group.id]) + self.edit_url = reverse(SG_ADD_RULE_VIEW, args=[sec_group.id]) + self.update_url = reverse(SG_UPDATE_VIEW, args=[sec_group.id]) @test.create_stubs({api.network: ('security_group_rule_create', 'security_group_list', @@ -83,66 +90,77 @@ class SecurityGroupsViewTests(test.TestCase): api.network.security_group_get(IsA(http.HttpRequest), sec_group.id).AndReturn(sec_group) self.mox.ReplayAll() - - res = self.client.get(reverse('horizon:project:access_and_security:' - 'security_groups:update', - args=[sec_group.id])) - self.assertTemplateUsed( - res, 'project/access_and_security/security_groups/_update.html') + res = self.client.get(self.update_url) + self.assertTemplateUsed(res, SG_UPDATE_TEMPLATE) self.assertEqual(res.context['security_group'].name, sec_group.name) @test.create_stubs({api.network: ('security_group_update', 'security_group_get')}) def test_update_security_groups_post(self): - sec_group = self.security_groups.get(name="other_group") - api.network.security_group_update(IsA(http.HttpRequest), - str(sec_group.id), - sec_group.name, - sec_group.description) \ - .AndReturn(sec_group) - api.network.security_group_get(IsA(http.HttpRequest), - sec_group.id).AndReturn(sec_group) + """Ensure that we can change a group name. + + The name must not be restricted to alphanumeric characters. + bug #1233501 Security group names cannot contain at characters + bug #1224576 Security group names cannot contain spaces + """ + sec_group = self.security_groups.first() + sec_group.name = "@new name" + api.network.security_group_update( + IsA(http.HttpRequest), + str(sec_group.id), + sec_group.name, + sec_group.description).AndReturn(sec_group) + api.network.security_group_get( + IsA(http.HttpRequest), sec_group.id).AndReturn(sec_group) self.mox.ReplayAll() - - formData = {'method': 'UpdateGroup', - 'id': sec_group.id, - 'name': sec_group.name, - 'description': sec_group.description} - - update_url = reverse('horizon:project:access_and_security:' - 'security_groups:update', - args=[sec_group.id]) - res = self.client.post(update_url, formData) + form_data = {'method': 'UpdateGroup', + 'id': sec_group.id, + 'name': sec_group.name, + 'description': sec_group.description} + res = self.client.post(self.update_url, form_data) self.assertRedirectsNoFollow(res, INDEX_URL) def test_create_security_groups_get(self): res = self.client.get(SG_CREATE_URL) - self.assertTemplateUsed( - res, 'project/access_and_security/security_groups/create.html') + self.assertTemplateUsed(res, SG_CREATE_TEMPLATE) - @test.create_stubs({api.network: ('security_group_create',)}) def test_create_security_groups_post(self): sec_group = self.security_groups.first() - api.network.security_group_create(IsA(http.HttpRequest), - sec_group.name, - sec_group.description) \ - .AndReturn(sec_group) + self._create_security_group(sec_group) + + def test_create_security_groups_special_chars(self): + """Ensure that a group name is not restricted to alphanumeric + characters. + + bug #1233501 Security group names cannot contain at characters + bug #1224576 Security group names cannot contain spaces + """ + sec_group = self.security_groups.first() + sec_group.name = '@group name' + self._create_security_group(sec_group) + + @test.create_stubs({api.network: ('security_group_create',)}) + def _create_security_group(self, sec_group): + api.network.security_group_create( + IsA(http.HttpRequest), + sec_group.name, + sec_group.description).AndReturn(sec_group) self.mox.ReplayAll() - formData = {'method': 'CreateGroup', - 'name': sec_group.name, - 'description': sec_group.description} - res = self.client.post(SG_CREATE_URL, formData) + form_data = {'method': 'CreateGroup', + 'name': sec_group.name, + 'description': sec_group.description} + res = self.client.post(SG_CREATE_URL, form_data) self.assertRedirectsNoFollow(res, INDEX_URL) @test.create_stubs({api.network: ('security_group_create',)}) def test_create_security_groups_post_exception(self): sec_group = self.security_groups.first() - api.network.security_group_create(IsA(http.HttpRequest), - sec_group.name, - sec_group.description) \ - .AndRaise(self.exceptions.nova) + api.network.security_group_create( + IsA(http.HttpRequest), + sec_group.name, + sec_group.description).AndRaise(self.exceptions.nova) self.mox.ReplayAll() formData = {'method': 'CreateGroup', @@ -153,17 +171,22 @@ class SecurityGroupsViewTests(test.TestCase): self.assertRedirectsNoFollow(res, INDEX_URL) @test.create_stubs({api.network: ('security_group_create',)}) - def test_create_security_groups_post_wrong_name(self): + def test_create_security_groups_non_printable(self): + """Ensure that group names can only contain printable + ASCII characters. + + Only 95 characters are allowed: from 0x20 (space) to 0x7E (~). + """ sec_group = self.security_groups.first() - fail_name = sec_group.name + ' invalid' + # 0x7F is a control character (DELETE) + fail_name = sec_group.name + ' \x7F' self.mox.ReplayAll() - formData = {'method': 'CreateGroup', - 'name': fail_name, - 'description': sec_group.description} - res = self.client.post(SG_CREATE_URL, formData) - self.assertTemplateUsed( - res, 'project/access_and_security/security_groups/create.html') + form_data = {'method': 'CreateGroup', + 'name': fail_name, + 'description': sec_group.description} + res = self.client.post(SG_CREATE_URL, form_data) + self.assertTemplateUsed(res, SG_CREATE_TEMPLATE) self.assertContains(res, "ASCII") @test.create_stubs({api.network: ('security_group_get',)}) @@ -174,17 +197,15 @@ class SecurityGroupsViewTests(test.TestCase): sec_group.id).AndReturn(sec_group) self.mox.ReplayAll() res = self.client.get(self.detail_url) - self.assertTemplateUsed( - res, 'project/access_and_security/security_groups/detail.html') + self.assertTemplateUsed(res, SG_DETAIL_TEMPLATE) @test.create_stubs({api.network: ('security_group_get',)}) def test_detail_get_exception(self): sec_group = self.security_groups.first() - api.network.security_group_get(IsA(http.HttpRequest), - sec_group.id) \ - .AndRaise(self.exceptions.nova) - + api.network.security_group_get( + IsA(http.HttpRequest), + sec_group.id).AndRaise(self.exceptions.nova) self.mox.ReplayAll() res = self.client.get(self.detail_url) @@ -630,12 +651,9 @@ class SecurityGroupsNovaNeutronDriverTests(SecurityGroupsViewTests): self.security_group_rules = self.security_group_rules_uuid sec_group = self.security_groups.first() - self.detail_url = reverse('horizon:project:access_and_security:' - 'security_groups:detail', - args=[sec_group.id]) - self.edit_url = reverse('horizon:project:access_and_security:' - 'security_groups:add_rule', - args=[sec_group.id]) + self.detail_url = reverse(SG_DETAIL_VIEW, args=[sec_group.id]) + self.edit_url = reverse(SG_ADD_RULE_VIEW, args=[sec_group.id]) + self.update_url = reverse(SG_UPDATE_VIEW, args=[sec_group.id]) def tearDown(self): self.security_groups = self._sec_groups_orig @@ -656,12 +674,9 @@ class SecurityGroupsNeutronTests(SecurityGroupsViewTests): self.security_group_rules = self.q_secgroup_rules sec_group = self.security_groups.first() - self.detail_url = reverse('horizon:project:access_and_security:' - 'security_groups:detail', - args=[sec_group.id]) - self.edit_url = reverse('horizon:project:access_and_security:' - 'security_groups:add_rule', - args=[sec_group.id]) + self.detail_url = reverse(SG_DETAIL_VIEW, args=[sec_group.id]) + self.edit_url = reverse(SG_ADD_RULE_VIEW, args=[sec_group.id]) + self.update_url = reverse(SG_UPDATE_VIEW, args=[sec_group.id]) def tearDown(self): self.security_groups = self._sec_groups_orig