diff --git a/kallithea/lib/auth.py b/kallithea/lib/auth.py --- a/kallithea/lib/auth.py +++ b/kallithea/lib/auth.py @@ -760,8 +760,13 @@ class LoginRequired(object): log.warning('API access to %s is not allowed', loc) return abort(403) - # CSRF protection - POSTs with session auth must contain correct token - if request.POST and user.is_authenticated: + # CSRF protection: Whenever a request has ambient authority (whether + # through a session cookie or its origin IP address), it must include + # the correct token, unless the HTTP method is GET or HEAD (and thus + # guaranteed to be side effect free. + # Note that the 'is_authenticated' flag is True for anonymous users too, + # but not when the user is authenticated by API key. + if user.is_authenticated and request.method not in ['GET', 'HEAD']: token = request.POST.get(secure_form.token_key) if not token or token != secure_form.authentication_token(): log.error('CSRF check failed') diff --git a/kallithea/tests/functional/test_admin_defaults.py b/kallithea/tests/functional/test_admin_defaults.py --- a/kallithea/tests/functional/test_admin_defaults.py +++ b/kallithea/tests/functional/test_admin_defaults.py @@ -16,7 +16,8 @@ class TestDefaultsController(TestControl response = self.app.get(url('formatted_defaults', format='xml')) def test_create(self): - response = self.app.post(url('defaults')) + response = self.app.post(url('defaults'), + {'_authentication_token': self.authentication_token()}) def test_new(self): response = self.app.get(url('new_default')) @@ -62,7 +63,8 @@ class TestDefaultsController(TestControl response = self.app.post(url('default', id=1), params=dict(_method='put', _authentication_token=self.authentication_token())) def test_delete(self): - response = self.app.delete(url('default', id=1)) + # Not possible due to CSRF protection. + response = self.app.delete(url('default', id=1), status=403) def test_delete_browser_fakeout(self): response = self.app.post(url('default', id=1), params=dict(_method='delete', _authentication_token=self.authentication_token())) diff --git a/kallithea/tests/functional/test_admin_gists.py b/kallithea/tests/functional/test_admin_gists.py --- a/kallithea/tests/functional/test_admin_gists.py +++ b/kallithea/tests/functional/test_admin_gists.py @@ -136,19 +136,20 @@ class TestGistsController(TestController def test_delete(self): self.log_user() gist = _create_gist('delete-me') - response = self.app.delete(url('gist', gist_id=gist.gist_id)) - self.checkSessionFlash(response, 'Deleted gist %s' % gist.gist_id) + response = self.app.post(url('gist', gist_id=gist.gist_id), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) def test_delete_normal_user_his_gist(self): self.log_user(TEST_USER_REGULAR_LOGIN, TEST_USER_REGULAR_PASS) gist = _create_gist('delete-me', owner=TEST_USER_REGULAR_LOGIN) - response = self.app.delete(url('gist', gist_id=gist.gist_id)) - self.checkSessionFlash(response, 'Deleted gist %s' % gist.gist_id) + response = self.app.post(url('gist', gist_id=gist.gist_id), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) def test_delete_normal_user_not_his_own_gist(self): self.log_user(TEST_USER_REGULAR_LOGIN, TEST_USER_REGULAR_PASS) gist = _create_gist('delete-me') - response = self.app.delete(url('gist', gist_id=gist.gist_id), status=403) + response = self.app.post(url('gist', gist_id=gist.gist_id), status=403, + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) def test_show(self): gist = _create_gist('gist-show-me') diff --git a/kallithea/tests/functional/test_admin_notifications.py b/kallithea/tests/functional/test_admin_notifications.py --- a/kallithea/tests/functional/test_admin_notifications.py +++ b/kallithea/tests/functional/test_admin_notifications.py @@ -56,9 +56,9 @@ class TestNotificationsController(TestCo self.assertEqual(get_notif(u2.notifications), [notification]) cur_usr_id = cur_user.user_id - response = self.app.delete(url('notification', - notification_id= - notification.notification_id)) + response = self.app.post( + url('notification', notification_id=notification.notification_id), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) self.assertEqual(response.body, 'ok') cur_user = User.get(cur_usr_id) diff --git a/kallithea/tests/functional/test_admin_repos.py b/kallithea/tests/functional/test_admin_repos.py --- a/kallithea/tests/functional/test_admin_repos.py +++ b/kallithea/tests/functional/test_admin_repos.py @@ -398,7 +398,8 @@ class _BaseTest(object): except vcs.exceptions.VCSError: self.fail('no repo %s in filesystem' % repo_name) - response = self.app.delete(url('delete_repo', repo_name=repo_name)) + response = self.app.post(url('delete_repo', repo_name=repo_name), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) self.checkSessionFlash(response, 'Deleted repository %s' % (repo_name)) @@ -450,7 +451,8 @@ class _BaseTest(object): except vcs.exceptions.VCSError: self.fail('no repo %s in filesystem' % repo_name) - response = self.app.delete(url('delete_repo', repo_name=repo_name)) + response = self.app.post(url('delete_repo', repo_name=repo_name), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) self.checkSessionFlash(response, 'Deleted repository %s' % (repo_name_unicode)) response.follow() diff --git a/kallithea/tests/functional/test_admin_user_groups.py b/kallithea/tests/functional/test_admin_user_groups.py --- a/kallithea/tests/functional/test_admin_user_groups.py +++ b/kallithea/tests/functional/test_admin_user_groups.py @@ -32,7 +32,7 @@ class TestAdminUsersGroupsController(Tes response = self.app.get(url('new_users_group')) def test_update(self): - response = self.app.put(url('users_group', id=1)) + response = self.app.put(url('users_group', id=1), status=403) def test_update_browser_fakeout(self): response = self.app.post(url('users_group', id=1), @@ -54,7 +54,8 @@ class TestAdminUsersGroupsController(Tes gr = Session().query(UserGroup)\ .filter(UserGroup.users_group_name == users_group_name).one() - response = self.app.delete(url('users_group', id=gr.users_group_id)) + response = self.app.post(url('users_group', id=gr.users_group_id), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) gr = Session().query(UserGroup)\ .filter(UserGroup.users_group_name == users_group_name).scalar() @@ -97,7 +98,8 @@ class TestAdminUsersGroupsController(Tes ## DISABLE REPO CREATE ON A GROUP response = self.app.put( - url('edit_user_group_default_perms', id=ug.users_group_id), {}) + url('edit_user_group_default_perms', id=ug.users_group_id), + params={'_authentication_token': self.authentication_token()}) response.follow() ug = UserGroup.get_by_group_name(users_group_name) @@ -119,7 +121,8 @@ class TestAdminUsersGroupsController(Tes # DELETE ! ug = UserGroup.get_by_group_name(users_group_name) ugid = ug.users_group_id - response = self.app.delete(url('users_group', id=ug.users_group_id)) + response = self.app.post(url('users_group', id=ug.users_group_id), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) response = response.follow() gr = Session().query(UserGroup)\ .filter(UserGroup.users_group_name == users_group_name).scalar() @@ -167,8 +170,8 @@ class TestAdminUsersGroupsController(Tes [ug.users_group_id, p3.permission_id]])) ## DISABLE REPO CREATE ON A GROUP - response = self.app.put( - url('edit_user_group_default_perms', id=ug.users_group_id), {}) + response = self.app.put(url('edit_user_group_default_perms', id=ug.users_group_id), + params={'_authentication_token': self.authentication_token()}) response.follow() ug = UserGroup.get_by_group_name(users_group_name) @@ -189,7 +192,8 @@ class TestAdminUsersGroupsController(Tes # DELETE ! ug = UserGroup.get_by_group_name(users_group_name) ugid = ug.users_group_id - response = self.app.delete(url('users_group', id=ug.users_group_id)) + response = self.app.post(url('users_group', id=ug.users_group_id), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) response = response.follow() gr = Session().query(UserGroup)\ .filter(UserGroup.users_group_name == diff --git a/kallithea/tests/functional/test_admin_users.py b/kallithea/tests/functional/test_admin_users.py --- a/kallithea/tests/functional/test_admin_users.py +++ b/kallithea/tests/functional/test_admin_users.py @@ -167,7 +167,8 @@ class TestAdminUsersController(TestContr new_user = Session().query(User)\ .filter(User.username == username).one() - response = self.app.delete(url('user', id=new_user.user_id)) + response = self.app.post(url('user', id=new_user.user_id), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) self.checkSessionFlash(response, 'Successfully deleted user') @@ -181,16 +182,19 @@ class TestAdminUsersController(TestContr new_user = Session().query(User)\ .filter(User.username == username).one() - response = self.app.delete(url('user', id=new_user.user_id)) + response = self.app.post(url('user', id=new_user.user_id), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) self.checkSessionFlash(response, 'User "%s" still ' 'owns 1 repositories and cannot be removed. ' 'Switch owners or remove those repositories: ' '%s' % (username, reponame)) - response = self.app.delete(url('delete_repo', repo_name=reponame)) + response = self.app.post(url('delete_repo', repo_name=reponame), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) self.checkSessionFlash(response, 'Deleted repository %s' % reponame) - response = self.app.delete(url('user', id=new_user.user_id)) + response = self.app.post(url('user', id=new_user.user_id), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) self.checkSessionFlash(response, 'Successfully deleted user') def test_delete_repo_group_err(self): @@ -203,7 +207,8 @@ class TestAdminUsersController(TestContr new_user = Session().query(User)\ .filter(User.username == username).one() - response = self.app.delete(url('user', id=new_user.user_id)) + response = self.app.post(url('user', id=new_user.user_id), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) self.checkSessionFlash(response, 'User "%s" still ' 'owns 1 repository groups and cannot be removed. ' 'Switch owners or remove those repository groups: ' @@ -213,10 +218,12 @@ class TestAdminUsersController(TestContr # rg = RepoGroup.get_by_group_name(group_name=groupname) # response = self.app.get(url('repos_groups', id=rg.group_id)) - response = self.app.delete(url('delete_repo_group', group_name=groupname)) + response = self.app.post(url('delete_repo_group', group_name=groupname), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) self.checkSessionFlash(response, 'Removed repository group %s' % groupname) - response = self.app.delete(url('user', id=new_user.user_id)) + response = self.app.post(url('user', id=new_user.user_id), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) self.checkSessionFlash(response, 'Successfully deleted user') def test_delete_user_group_err(self): @@ -229,7 +236,8 @@ class TestAdminUsersController(TestContr new_user = Session().query(User)\ .filter(User.username == username).one() - response = self.app.delete(url('user', id=new_user.user_id)) + response = self.app.post(url('user', id=new_user.user_id), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) self.checkSessionFlash(response, 'User "%s" still ' 'owns 1 user groups and cannot be removed. ' 'Switch owners or remove those user groups: ' @@ -241,7 +249,8 @@ class TestAdminUsersController(TestContr fixture.destroy_user_group(ug.users_group_id) - response = self.app.delete(url('user', id=new_user.user_id)) + response = self.app.post(url('user', id=new_user.user_id), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) self.checkSessionFlash(response, 'Successfully deleted user') def test_show(self): diff --git a/kallithea/tests/functional/test_changeset_comments.py b/kallithea/tests/functional/test_changeset_comments.py --- a/kallithea/tests/functional/test_changeset_comments.py +++ b/kallithea/tests/functional/test_changeset_comments.py @@ -132,10 +132,11 @@ class TestChangeSetCommentsController(Te self.assertEqual(len(comments), 1) comment_id = comments[0].comment_id - self.app.delete(url(controller='changeset', + self.app.post(url(controller='changeset', action='delete_comment', repo_name=HG_REPO, - comment_id=comment_id)) + comment_id=comment_id), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) comments = ChangesetComment.query().all() self.assertEqual(len(comments), 0) diff --git a/kallithea/tests/functional/test_forks.py b/kallithea/tests/functional/test_forks.py --- a/kallithea/tests/functional/test_forks.py +++ b/kallithea/tests/functional/test_forks.py @@ -96,7 +96,8 @@ class _BaseTestCase(object): ) # remove this fork - response = self.app.delete(url('delete_repo', repo_name=fork_name)) + response = self.app.post(url('delete_repo', repo_name=fork_name), + params={'_method': 'delete', '_authentication_token': self.authentication_token()}) def test_fork_create_into_group(self): self.log_user() diff --git a/kallithea/tests/functional/test_my_account.py b/kallithea/tests/functional/test_my_account.py --- a/kallithea/tests/functional/test_my_account.py +++ b/kallithea/tests/functional/test_my_account.py @@ -57,7 +57,8 @@ class TestMyAccountController(TestContro self.log_user() response = self.app.get(url('my_account_emails')) response.mustcontain('No additional emails specified') - response = self.app.post(url('my_account_emails'),) + response = self.app.post(url('my_account_emails'), + {'_authentication_token': self.authentication_token()}) self.checkSessionFlash(response, 'Please enter an email address') def test_my_account_my_emails_add_remove(self):