Allow all printable ASCII characters in security group names
Thus far, the security group name was checked with Django's validate_slug. Consequently, some characters like spaces (bug #1224576) and @ (bug #1233501) were forbidden. This fix removes validate_slug and provides a less restrictive validation. Now, all printable ASCII characters -- i.e. from 0x20 to 0x7E -- are valid. Note that diacritics are disallowed. On top of that the length is now checked and limited to 255 characters in accordance with Nova. Security group forms have been factorized in one abstract base class: GroupBase. CreateGroup and UpdateGroup subclass GroupBase in order to be more DRY. Change-Id: Ifc8e5f75419a73a353b2138641773d36138ecea8 Closes-Bug: #1224576 Closes-Bug: #1233501
This commit is contained in:
Romain Hardouin
Romain Hardouin
parent
4a5af84adf
commit
80d9049fb6
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue