From 1dac41313005c9035a5f8d3c9db83dcad7883f24 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Fri, 13 Jul 2018 14:39:17 +0000 Subject: [PATCH] [FIX] website_forum: limit portal/user rights on forum.post.vote Create and write rights are granted on portal and user. We need to improve the security. task-1934286 task-1862656 opw-1862218 opw-1862111 opw-1857912 opw-1861426 Github-24986 forum-135070 closes odoo/odoo#25671 --- addons/website_forum/models/forum.py | 82 ++++++++++------ addons/website_forum/tests/test_forum.py | 114 +++++++++++++++++++++++ 2 files changed, 167 insertions(+), 29 deletions(-) diff --git a/addons/website_forum/models/forum.py b/addons/website_forum/models/forum.py index 245a054dac4..a35b8a18f3b 100644 --- a/addons/website_forum/models/forum.py +++ b/addons/website_forum/models/forum.py @@ -355,7 +355,7 @@ class Post(models.Model): @api.multi def _get_post_karma_rights(self): user = self.env.user - is_admin = user.id == SUPERUSER_ID + is_admin = user._is_admin() # sudoed recordset instead of individual posts so values can be # prefetched in bulk for post, post_sudo in zip(self, self.sudo()): @@ -850,6 +850,10 @@ class Vote(models.Model): forum_id = fields.Many2one('forum.forum', string='Forum', related="post_id.forum_id", store=True, readonly=False) recipient_id = fields.Many2one('res.users', string='To', related="post_id.create_uid", store=True, readonly=False) + _sql_constraints = [ + ('vote_uniq', 'unique (post_id, user_id)', "Vote already exists !"), + ] + def _get_karma_value(self, old_vote, new_vote, up_karma, down_karma): _karma_upd = { '-1': {'-1': 0, '0': -1 * down_karma, '1': -1 * down_karma + up_karma}, @@ -860,46 +864,66 @@ class Vote(models.Model): @api.model def create(self, vals): + # can't modify owner of a vote + if not self.env.user._is_admin(): + vals.pop('user_id', None) + vote = super(Vote, self).create(vals) - # own post check - if vote.user_id.id == vote.post_id.create_uid.id: - raise UserError(_('It is not allowed to vote for its own post.')) - # karma check - if vote.vote == '1' and not vote.post_id.can_upvote: - raise KarmaError('You don\'t have enough karma toupvote.') - elif vote.vote == '-1' and not vote.post_id.can_downvote: - raise KarmaError('You don\'t have enough karma to downvote.') + vote._check_general_rights() + vote._check_karma_rights(vote.vote == '1') - if vote.post_id.parent_id: - karma_value = self._get_karma_value('0', vote.vote, vote.forum_id.karma_gen_answer_upvote, vote.forum_id.karma_gen_answer_downvote) - else: - karma_value = self._get_karma_value('0', vote.vote, vote.forum_id.karma_gen_question_upvote, vote.forum_id.karma_gen_question_downvote) - vote.recipient_id.sudo().add_karma(karma_value) + # karma update + vote._vote_update_karma('0', vote.vote) return vote @api.multi def write(self, values): - if 'vote' in values: - for vote in self: - # own post check - if vote.user_id.id == vote.post_id.create_uid.id: - raise UserError(_('It is not allowed to vote for its own post.')) - # karma check - if (values['vote'] == '1' or vote.vote == '-1' and values['vote'] == '0') and not vote.post_id.can_upvote: - raise KarmaError('You don\'t have enough karma to upvote.') - elif (values['vote'] == '-1' or vote.vote == '1' and values['vote'] == '0') and not vote.post_id.can_downvote: - raise KarmaError('You don\'t have enough karma to downvote.') + # can't modify owner of a vote + if not self.env.user._is_admin(): + values.pop('user_id', None) + + for vote in self: + self._check_general_rights(values) + if 'vote' in values: + if (values['vote'] == '1' or vote.vote == '-1' and values['vote'] == '0'): + upvote = True + elif (values['vote'] == '-1' or vote.vote == '1' and values['vote'] == '0'): + upvote = False + self._check_karma_rights(upvote) # karma update - if vote.post_id.parent_id: - karma_value = self._get_karma_value(vote.vote, values['vote'], vote.forum_id.karma_gen_answer_upvote, vote.forum_id.karma_gen_answer_downvote) - else: - karma_value = self._get_karma_value(vote.vote, values['vote'], vote.forum_id.karma_gen_question_upvote, vote.forum_id.karma_gen_question_downvote) - vote.recipient_id.sudo().add_karma(karma_value) + self._vote_update_karma(vote.vote, values['vote']) + res = super(Vote, self).write(values) return res + def _check_general_rights(self, vals={}): + post = self.post_id + if vals.get('post_id'): + post = self.env['forum.post'].browse(vals.get('post_id')) + if not self.env.user._is_admin(): + # own post check + if self._uid == post.create_uid.id: + raise UserError(_('It is not allowed to vote for its own post.')) + # own vote check + if self._uid != self.user_id.id: + raise UserError(_('It is not allowed to modify someone else\'s vote.')) + + def _check_karma_rights(self, upvote=None): + # karma check + if upvote and not self.post_id.can_upvote: + raise KarmaError('You don\'t have enough karma to upvote.') + elif not upvote and not self.post_id.can_downvote: + raise KarmaError('You don\'t have enough karma to downvote.') + + def _vote_update_karma(self, old_vote, new_vote): + if self.post_id.parent_id: + karma_value = self._get_karma_value(old_vote, new_vote, self.forum_id.karma_gen_answer_upvote, self.forum_id.karma_gen_answer_downvote) + else: + karma_value = self._get_karma_value(old_vote, new_vote, self.forum_id.karma_gen_question_upvote, self.forum_id.karma_gen_question_downvote) + self.recipient_id.sudo().add_karma(karma_value) + class Tags(models.Model): _name = "forum.tag" diff --git a/addons/website_forum/tests/test_forum.py b/addons/website_forum/tests/test_forum.py index 9f60b237bd4..4037dc3b783 100644 --- a/addons/website_forum/tests/test_forum.py +++ b/addons/website_forum/tests/test_forum.py @@ -5,10 +5,124 @@ from .common import KARMA, TestForumCommon from ..models.forum import KarmaError from odoo.exceptions import UserError, AccessError from odoo.tools import mute_logger +from psycopg2 import IntegrityError class TestForum(TestForumCommon): + def test_crud_rights(self): + Post = self.env['forum.post'] + Vote = self.env['forum.post.vote'] + self.user_portal.karma = 500 + self.user_employee.karma = 500 + + # create some posts + self.admin_post = self.post + self.portal_post = Post.sudo(self.user_portal).create({ + 'name': 'Post from Portal User', + 'content': 'I am not a bird.', + 'forum_id': self.forum.id, + }) + self.employee_post = Post.sudo(self.user_employee).create({ + 'name': 'Post from Employee User', + 'content': 'I am not a bird.', + 'forum_id': self.forum.id, + }) + + # vote on some posts + self.employee_vote_on_admin_post = Vote.sudo(self.user_employee).create({ + 'post_id': self.admin_post.id, + 'vote': '1', + }) + self.portal_vote_on_admin_post = Vote.sudo(self.user_portal).create({ + 'post_id': self.admin_post.id, + 'vote': '1', + }) + self.admin_vote_on_portal_post = Vote.create({ + 'post_id': self.portal_post.id, + 'vote': '1', + }) + self.admin_vote_on_employee_post = Vote.create({ + 'post_id': self.employee_post.id, + 'vote': '1', + }) + + # One should not be able to modify someone else's vote + with self.assertRaises(UserError): + self.admin_vote_on_portal_post.sudo(self.user_employee).write({ + 'vote': '-1', + }) + with self.assertRaises(UserError): + self.admin_vote_on_employee_post.sudo(self.user_portal).write({ + 'vote': '-1', + }) + + # One should not be able to give his vote to someone else + self.employee_vote_on_admin_post.sudo(self.user_employee).write({ + 'user_id': 1, + }) + self.assertEqual(self.employee_vote_on_admin_post.user_id, self.user_employee, 'User employee should not be able to give its vote ownership to someone else') + # One should not be able to change his vote's post to a post of his own (would be self voting) + with self.assertRaises(UserError): + self.employee_vote_on_admin_post.sudo(self.user_employee).write({ + 'post_id': self.employee_post.id, + }) + + # One should not be able to give his vote to someone else + self.portal_vote_on_admin_post.sudo(self.user_portal).write({ + 'user_id': 1, + }) + self.assertEqual(self.portal_vote_on_admin_post.user_id, self.user_portal, 'User portal should not be able to give its vote ownership to someone else') + # One should not be able to change his vote's post to a post of his own (would be self voting) + with self.assertRaises(UserError): + self.portal_vote_on_admin_post.sudo(self.user_portal).write({ + 'post_id': self.portal_post.id, + }) + + # One should not be able to vote for its own post + with self.assertRaises(UserError): + Vote.sudo(self.user_employee).create({ + 'post_id': self.employee_post.id, + 'vote': '1', + }) + # One should not be able to vote for its own post + with self.assertRaises(UserError): + Vote.sudo(self.user_portal).create({ + 'post_id': self.portal_post.id, + 'vote': '1', + }) + + with mute_logger('odoo.sql_db'): + with self.assertRaises(IntegrityError): + with self.cr.savepoint(): + # One should not be able to vote more than once on a same post + Vote.sudo(self.user_employee).create({ + 'post_id': self.admin_post.id, + 'vote': '1', + }) + with self.assertRaises(IntegrityError): + with self.cr.savepoint(): + # One should not be able to vote more than once on a same post + Vote.sudo(self.user_employee).create({ + 'post_id': self.admin_post.id, + 'vote': '1', + }) + + # One should not be able to create a vote for someone else + new_employee_vote = Vote.sudo(self.user_employee).create({ + 'post_id': self.portal_post.id, + 'user_id': 1, + 'vote': '1', + }) + self.assertEqual(new_employee_vote.user_id, self.user_employee, 'Creating a vote for someone else should not be allowed. It should create it for yourself instead') + # One should not be able to create a vote for someone else + new_portal_vote = Vote.sudo(self.user_portal).create({ + 'post_id': self.employee_post.id, + 'user_id': 1, + 'vote': '1', + }) + self.assertEqual(new_portal_vote.user_id, self.user_portal, 'Creating a vote for someone else should not be allowed. It should create it for yourself instead') + @mute_logger('odoo.addons.base.models.ir_model', 'odoo.models') def test_ask(self): Post = self.env['forum.post']