[web/api] update group name validation check

can only contain letters, numbers, blank, hyphen or underscore

seahub/api2/views.py

@@ -50,7 +50,8 @@ from seahub.group.models import GroupMessage, MessageReply, MessageAttachment
 from seahub.group.signals import grpmsg_added, grpmsg_reply_added
 from seahub.group.views import group_check, remove_group_common, \
     rename_group_with_new_name
-from seahub.group.utils import BadGroupNameError, ConflictGroupNameError
+from seahub.group.utils import BadGroupNameError, ConflictGroupNameError, \
+    validate_group_name
 from seahub.thumbnail.utils import allow_generate_thumbnail, generate_thumbnail
 from seahub.message.models import UserMessage
 from seahub.notifications.models import UserNotification
@@ -3341,6 +3342,11 @@ class Groups(APIView):
                                     content_type=content_type)
 
         group_name = request.DATA.get('group_name', None)
+        group_name = group_name.strip()
+        if not validate_group_name(group_name):
+            result['error'] = 'Failed to rename group, group name can only contain letters, numbers, blank, hyphen or underscore.'
+            return HttpResponse(json.dumps(result), status=403,
+                                content_type=content_type)
 
         # Check whether group name is duplicated.
         if request.cloud_mode:

seahub/group/forms.py

@@ -32,8 +32,9 @@ class GroupAddForm(forms.Form):
             })
     def clean_group_name(self):
         group_name = self.cleaned_data['group_name']
+        group_name = group_name.strip()
         if not validate_group_name(group_name):
-            error_msg = _(u'Group name can only contain letters, numbers or underscore')
+            error_msg = _(u'Group name can only contain letters, numbers, blank, hyphen or underscore')
             raise forms.ValidationError(error_msg)
         else:
             return group_name

seahub/group/utils.py

@@ -15,5 +15,5 @@ def validate_group_name(group_name):
     """
     if len(group_name) > 255:
         return False
-    return re.match('^\w+$', group_name, re.U)
+    return re.match('^[\w\s-]+$', group_name, re.U)
 

seahub/group/views.py

@@ -318,13 +318,14 @@ def group_rename(request, group_id):
         raise Http404
 
     new_name = request.POST.get('new_name', '')
+    new_name = new_name.strip()
     next = request.META.get('HTTP_REFERER', SITE_ROOT)
     group_id = int(group_id)
 
     try:
         rename_group_with_new_name(request, group_id, new_name)
     except BadGroupNameError:
-        messages.error(request, _('Failed to rename group, group name can only contain letters, numbers or underscore'))
+        messages.error(request, _('Failed to rename group, group name can only contain letters, numbers, blank, hyphen or underscore'))
     except ConflictGroupNameError:
         messages.error(request, _('There is already a group with that name.'))
     else:

tests/api/test_groups.py

@@ -46,3 +46,45 @@ class GroupsApiTest(ApiTestBase):
         groups = self.get(GROUPS_URL).json()['groups']
         for group in groups:
             self.assertNotEqual(group['id'], group_id)
+
+    def test_add_remove_group_with_blank(self):
+        data = {'group_name': randstring(4) + ' ' + randstring(4)}
+        info = self.put(GROUPS_URL, data=data).json()
+        self.assertTrue(info['success'])
+        group_id = info['group_id']
+        self.assertGreater(group_id, 0)
+        url = urljoin(GROUPS_URL, str(group_id))
+        self.delete(url)
+
+        # check group is really removed
+        groups = self.get(GROUPS_URL).json()['groups']
+        for group in groups:
+
+            self.assertNotEqual(group['id'], group_id)
+    def test_add_remove_group_with_hyphen(self):
+        data = {'group_name': randstring(4) + '-' + randstring(4)}
+        info = self.put(GROUPS_URL, data=data).json()
+        self.assertTrue(info['success'])
+        group_id = info['group_id']
+        self.assertGreater(group_id, 0)
+        url = urljoin(GROUPS_URL, str(group_id))
+        self.delete(url)
+
+        # check group is really removed
+        groups = self.get(GROUPS_URL).json()['groups']
+        for group in groups:
+            self.assertNotEqual(group['id'], group_id)
+
+    def test_add_remove_group_with_blank_and_hyphen(self):
+        data = {'group_name': randstring(4) + '-' + randstring(4) + ' ' + randstring(4)}
+        info = self.put(GROUPS_URL, data=data).json()
+        self.assertTrue(info['success'])
+        group_id = info['group_id']
+        self.assertGreater(group_id, 0)
+        url = urljoin(GROUPS_URL, str(group_id))
+        self.delete(url)
+
+        # check group is really removed
+        groups = self.get(GROUPS_URL).json()['groups']
+        for group in groups:
+            self.assertNotEqual(group['id'], group_id)

tests/seahub/group/views/test_group.py

@@ -22,6 +22,50 @@ class GroupAddTest(TestCase, Fixtures):
         }, HTTP_X_REQUESTED_WITH='XMLHttpRequest')
         assert json.loads(resp.content)['success'] is True
 
+    def test_can_add_with_blank(self):
+        self.client.post(
+            reverse('auth_login'), {'username': self.user.username,
+                                    'password': 'secret'}
+        )
+
+        resp = self.client.post(reverse('group_add'), {
+            'group_name': 'test group %s' % randstring(6)
+        }, HTTP_X_REQUESTED_WITH='XMLHttpRequest')
+        assert json.loads(resp.content)['success'] is True
+
+    def test_can_add_with_hyphen(self):
+        self.client.post(
+            reverse('auth_login'), {'username': self.user.username,
+                                    'password': 'secret'}
+        )
+
+        resp = self.client.post(reverse('group_add'), {
+            'group_name': 'test-group-%s' % randstring(6)
+        }, HTTP_X_REQUESTED_WITH='XMLHttpRequest')
+        assert json.loads(resp.content)['success'] is True
+
+    def test_can_add_with_blank_and_hyphen(self):
+        self.client.post(
+            reverse('auth_login'), {'username': self.user.username,
+                                    'password': 'secret'}
+        )
+
+        resp = self.client.post(reverse('group_add'), {
+            'group_name': 'test-group %s' % randstring(6)
+        }, HTTP_X_REQUESTED_WITH='XMLHttpRequest')
+        assert json.loads(resp.content)['success'] is True
+
+    def test_can_not_add_with_invalid_name(self):
+        self.client.post(
+            reverse('auth_login'), {'username': self.user.username,
+                                    'password': 'secret'}
+        )
+
+        resp = self.client.post(reverse('group_add'), {
+            'group_name': 'test*group(name)'
+        }, HTTP_X_REQUESTED_WITH='XMLHttpRequest')
+        self.assertEqual(400, resp.status_code)
+
 class GroupDiscussTest(TestCase, Fixtures):
     def setUp(self):
         grp = self.group