Skip to content

Commit

Permalink
Add safety to prevent crafting queue.job records
Browse files Browse the repository at this point in the history
As we can delay a job on any method, and the queue.job model is
accessible from RPC (as any model), prevent to:

* create a queue.job using RPC
* write on protected fields (e.g. method name) using RPC

Admittedly, the risk is low since users need have Queue Job Manager
access to create/write on jobs, but it would allow these users to call
internal methods.

The check is done using a context key that must be equal to a sentinel
object, which is impossible to pass through RPC.
  • Loading branch information
guewen committed Nov 2, 2020
1 parent 417a466 commit 6fe6bf8
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 8 deletions.
9 changes: 7 additions & 2 deletions queue_job/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -553,9 +553,14 @@ def store(self):
if self.identity_key:
vals["identity_key"] = self.identity_key

job_model = self.env["queue.job"]
# The sentinel is used to prevent edition sensitive fields (such as
# method_name) from RPC methods.
edit_sentinel = job_model.EDIT_SENTINEL

db_record = self.db_record()
if db_record:
db_record.write(vals)
db_record.with_context(_job_edit_sentinel=edit_sentinel).write(vals)
else:
date_created = self.date_created
# The following values must never be modified after the
Expand All @@ -577,7 +582,7 @@ def store(self):
if self.channel:
vals.update({"channel": self.channel})

self.env[self.job_model_name].sudo().create(vals)
job_model.with_context(_job_edit_sentinel=edit_sentinel).sudo().create(vals)

def db_record(self):
return self.db_record_from_uuid(self.env, self.uuid)
Expand Down
49 changes: 43 additions & 6 deletions queue_job/models/queue_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,22 @@ class QueueJob(models.Model):
_removal_interval = 30 # days
_default_related_action = "related_action_open_record"

# This must be passed in a context key "_job_edit_sentinel" to write on
# protected fields. It protects against crafting "queue.job" records from
# RPC (e.g. on internal methods). When ``with_delay`` is used, the sentinel
# is set.
EDIT_SENTINEL = object()
_protected_fields = (
"uuid",
"name",
"date_created",
"model_name",
"method_name",
"record_ids",
"args",
"kwargs",
)

uuid = fields.Char(string="UUID", readonly=True, index=True, required=True)
user_id = fields.Many2one(comodel_name="res.users", string="User ID", required=True)
company_id = fields.Many2one(
Expand Down Expand Up @@ -126,6 +142,33 @@ def _compute_func_string(self):
all_args = ", ".join(args + kwargs)
record.func_string = "{}.{}({})".format(model, record.method_name, all_args)

@api.model_create_multi
def create(self, vals_list):
if self.env.context.get("_job_edit_sentinel") is not self.EDIT_SENTINEL:
# Prevent to create a queue.job record "raw" from RPC.
# ``with_delay()`` must be used.
raise exceptions.AccessError(
_("Queue jobs must created by calling 'with_delay()'.")
)
return super().create(vals_list)

def write(self, vals):
if self.env.context.get("_job_edit_sentinel") is not self.EDIT_SENTINEL:
write_on_protected_fields = [
fieldname in self._protected_fields for fieldname in vals
]
if write_on_protected_fields:
raise exceptions.AccessError(
_("Not allowed to change field(s): {}").format(
write_on_protected_fields
)
)

if vals.get("state") == "failed":
self._message_post_on_failure()

return super().write(vals)

def open_related_action(self):
"""Open the related action associated to the job"""
self.ensure_one()
Expand Down Expand Up @@ -171,12 +214,6 @@ def _message_post_on_failure(self):
if msg:
record.message_post(body=msg, subtype="queue_job.mt_job_failed")

def write(self, vals):
res = super(QueueJob, self).write(vals)
if vals.get("state") == "failed":
self._message_post_on_failure()
return res

def _subscribe_users_domain(self):
"""Subscribe all users having the 'Queue Job Manager' group"""
group = self.env.ref("queue_job.group_queue_job_manager")
Expand Down
1 change: 1 addition & 0 deletions queue_job/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
from . import test_runner_runner
from . import test_json_field
from . import test_model_job_channel
from . import test_queue_job_protected_write
25 changes: 25 additions & 0 deletions queue_job/tests/test_queue_job_protected_write.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# copyright 2020 Camptocamp
# license lgpl-3.0 or later (http://www.gnu.org/licenses/lgpl.html)

from odoo import exceptions
from odoo.tests import common


class TestJobWriteProtected(common.SavepointCase):
def test_create_error(self):
with self.assertRaises(exceptions.AccessError):
self.env["queue.job"].create(
{"uuid": "test", "model_name": "res.partner", "method_name": "write"}
)

def test_write_protected_field_error(self):
job_ = self.env["res.partner"].with_delay().create({"name": "test"})
db_job = job_.db_record()
with self.assertRaises(exceptions.AccessError):
db_job.method_name = "unlink"

def test_write_allow_no_protected_field_error(self):
job_ = self.env["res.partner"].with_delay().create({"name": "test"})
db_job = job_.db_record()
with self.assertRaises(exceptions.AccessError):
db_job.priority = 30

0 comments on commit 6fe6bf8

Please sign in to comment.