diff --git a/seahub/api2/views.py b/seahub/api2/views.py index 1005db7036..2a17b8f7eb 100644 --- a/seahub/api2/views.py +++ b/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: diff --git a/seahub/group/forms.py b/seahub/group/forms.py index 23b9c836bd..f9e6d0c965 100644 --- a/seahub/group/forms.py +++ b/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 diff --git a/seahub/group/utils.py b/seahub/group/utils.py index 68eae57187..d29a5b8007 100644 --- a/seahub/group/utils.py +++ b/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) diff --git a/seahub/group/views.py b/seahub/group/views.py index 9e9c96f12d..038621114d 100644 --- a/seahub/group/views.py +++ b/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: diff --git a/tests/api/test_groups.py b/tests/api/test_groups.py index 36d29c247c..fcb199efb1 100644 --- a/tests/api/test_groups.py +++ b/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) diff --git a/tests/seahub/group/views/test_group.py b/tests/seahub/group/views/test_group.py index 613de01b5a..6e75062076 100644 --- a/tests/seahub/group/views/test_group.py +++ b/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