Add safety to prevent crafting queue.job records

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.

Author: Guewen Baconnier

queue_job/job.py

@@ -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
@@ -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)

queue_job/models/queue_job.py

@@ -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(
@@ -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()
@@ -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")

queue_job/tests/__init__.py

@@ -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

queue_job/tests/test_queue_job_protected_write.py

@@ -0,0 +1,32 @@
+# 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