security(graduation): enforce manager-only assessment approval server-side

spp_graduation/README.rst

@@ -632,6 +632,18 @@ Test 10: Edge Cases
 Changelog
 =========
 
+19.0.2.0.2
+~~~~~~~~~~
+
+- fix(security): enforce manager-only approval of graduation assessments
+  server-side. ``action_approve`` / ``action_reject`` /
+  ``action_reset_draft`` now require the graduation manager group (the
+  view-level button gating was UI-only and bypassable via RPC), and
+  non-managers can no longer set the approval fields (``approved_by_id``
+  / ``approved_date`` / ``graduation_date``), move ``state`` beyond
+  ``draft → submitted``, or edit a submitted assessment's content.
+  Prevents a user from self-approving their own assessment.
+
 19.0.2.0.1
 ~~~~~~~~~~
 

spp_graduation/__manifest__.py

@@ -2,7 +2,7 @@
 {
     "name": "OpenSPP Graduation Management",
     "summary": "Manage graduation and exit from time-bound social protection programs",
-    "version": "19.0.2.0.1",
+    "version": "19.0.2.0.2",
     "category": "OpenSPP",
     "author": "OpenSPP.org",
     "website": "https://github.com/OpenSPP/OpenSPP2",

spp_graduation/models/graduation_assessment.py

@@ -1,7 +1,7 @@
 from dateutil.relativedelta import relativedelta
 
 from odoo import _, api, fields, models
-from odoo.exceptions import UserError, ValidationError
+from odoo.exceptions import AccessError, UserError, ValidationError
 
 
 class GraduationAssessment(models.Model):
@@ -136,13 +136,44 @@ def _compute_monitoring_end(self):
             else:
                 rec.monitoring_end_date = False
 
+    # Fields only a graduation manager may set — they record the approval
+    # outcome and must never be writable by the assessor via RPC.
+    _MANAGER_ONLY_FIELDS = ("approved_by_id", "approved_date", "graduation_date")
+
+    # Assessor-editable content. Once the assessment leaves draft it is awaiting
+    # the manager's decision, so a non-manager must not change what will be
+    # approved (e.g. flip recommendation to "graduate" after submitting). Only
+    # these business fields are frozen — technical/chatter writes are unaffected.
+    _LOCKED_CONTENT_FIELDS = (
+        "partner_id",
+        "pathway_id",
+        "assessment_date",
+        "recommendation",
+        "recommendation_notes",
+        "response_ids",
+    )
+
+    def _is_graduation_manager(self):
+        """True for graduation managers and for superuser/sudo (system) contexts.
+
+        Approve/reject/reset are manager-only. The superuser/sudo exemption keeps
+        programmatic and system flows working; real end users are always checked
+        against the manager group.
+        """
+        return self.env.su or self.env.user.has_group("spp_graduation.group_spp_graduation_manager")
+
+    def _ensure_graduation_manager(self):
+        if not self._is_graduation_manager():
+            raise AccessError(_("Only graduation managers can approve, reject, or reset graduation assessments."))
+
     def action_submit(self):
         for rec in self:
             if rec.state != "draft":
                 raise UserError(_("Only draft assessments can be submitted."))
             rec.state = "submitted"
 
     def action_approve(self):
+        self._ensure_graduation_manager()
         for rec in self:
             if rec.state != "submitted":
                 raise UserError(_("Only submitted assessments can be approved."))
@@ -157,17 +188,68 @@ def action_approve(self):
                 rec.graduation_date = fields.Date.today()
 
     def action_reject(self):
+        self._ensure_graduation_manager()
         for rec in self:
             if rec.state != "submitted":
                 raise UserError(_("Only submitted assessments can be rejected."))
             rec.state = "rejected"
 
     def action_reset_draft(self):
+        self._ensure_graduation_manager()
         for rec in self:
             if rec.state not in ("submitted", "rejected"):
                 raise UserError(_("Only submitted or rejected assessments can be reset to draft."))
             rec.state = "draft"
 
+    @api.model_create_multi
+    def create(self, vals_list):
+        # A non-manager may only create draft assessments and may not preset the
+        # approval outcome fields (blocks create({'state':'approved', ...})).
+        if not self._is_graduation_manager():
+            for vals in vals_list:
+                forbidden = [f for f in self._MANAGER_ONLY_FIELDS if vals.get(f)]
+                if forbidden:
+                    raise AccessError(
+                        _("Only graduation managers can set assessment approval fields: %s.") % ", ".join(forbidden)
+                    )
+                if vals.get("state", "draft") != "draft":
+                    raise AccessError(_("Only graduation managers can create an assessment in a non-draft state."))
+        return super().create(vals_list)
+
+    def write(self, vals):
+        # Enforce the approval boundary at the ORM so it cannot be bypassed via
+        # RPC (the view button groups are UI-only). Non-managers may not set the
+        # manager-only outcome fields, and may only move state draft -> submitted.
+        if not self._is_graduation_manager():
+            forbidden = [f for f in self._MANAGER_ONLY_FIELDS if f in vals]
+            if forbidden:
+                raise AccessError(
+                    _("Only graduation managers can set assessment approval fields: %s.") % ", ".join(forbidden)
+                )
+            # Content is frozen once the assessment leaves draft: the manager must
+            # approve exactly what was submitted (e.g. no flipping recommendation
+            # to "graduate" after submission). A manager can reset it to draft.
+            if any(f in vals for f in self._LOCKED_CONTENT_FIELDS):
+                for rec in self:
+                    if rec.state != "draft":
+                        raise AccessError(
+                            _(
+                                "A graduation assessment can only be edited while it is in draft. "
+                                "Ask a manager to reset it to draft to make changes."
+                            )
+                        )
+            if "state" in vals:
+                new_state = vals["state"]
+                for rec in self:
+                    if new_state != rec.state and (rec.state, new_state) != ("draft", "submitted"):
+                        raise AccessError(
+                            _(
+                                "Only graduation managers can change an assessment's state; "
+                                "you may only submit a draft for approval."
+                            )
+                        )
+        return super().write(vals)
+
 
 class GraduationCriteriaResponse(models.Model):
     _name = "spp.graduation.criteria.response"

spp_graduation/readme/HISTORY.md

@@ -1,3 +1,7 @@
+### 19.0.2.0.2
+
+- fix(security): enforce manager-only approval of graduation assessments server-side. `action_approve` / `action_reject` / `action_reset_draft` now require the graduation manager group (the view-level button gating was UI-only and bypassable via RPC), and non-managers can no longer set the approval fields (`approved_by_id` / `approved_date` / `graduation_date`), move `state` beyond `draft → submitted`, or edit a submitted assessment's content. Prevents a user from self-approving their own assessment.
+
 ### 19.0.2.0.1
 
 - fix(views): add a "Graduation Criteria" menu item directly under the Graduation root, plus a list/form/search view and action for `spp.graduation.criteria`. The model and ACL were already shipped, but no UI surface existed — criteria could only be edited indirectly through the pathway form. Visible to `group_spp_graduation_user` and above.

spp_graduation/static/description/index.html

@@ -1005,6 +1005,19 @@ <h2>Changelog</h2>
 </div>
 </div>
 <div class="section" id="section-1">
+<h1>19.0.2.0.2</h1>
+<ul class="simple">
+<li>fix(security): enforce manager-only approval of graduation assessments
+server-side. <tt class="docutils literal">action_approve</tt> / <tt class="docutils literal">action_reject</tt> /
+<tt class="docutils literal">action_reset_draft</tt> now require the graduation manager group (the
+view-level button gating was UI-only and bypassable via RPC), and
+non-managers can no longer set the approval fields (<tt class="docutils literal">approved_by_id</tt>
+/ <tt class="docutils literal">approved_date</tt> / <tt class="docutils literal">graduation_date</tt>), move <tt class="docutils literal">state</tt> beyond
+<tt class="docutils literal">draft → submitted</tt>, or edit a submitted assessment’s content.
+Prevents a user from self-approving their own assessment.</li>
+</ul>
+</div>
+<div class="section" id="section-2">
 <h1>19.0.2.0.1</h1>
 <ul class="simple">
 <li>fix(views): add a “Graduation Criteria” menu item directly under the
@@ -1019,7 +1032,7 @@ <h1>19.0.2.0.1</h1>
 the Settings → Users access-rights UI.</li>
 </ul>
 </div>
-<div class="section" id="section-2">
+<div class="section" id="section-3">
 <h1>19.0.2.0.0</h1>
 <ul class="simple">
 <li>Initial migration to OpenSPP2</li>

spp_graduation/tests/test_graduation_security.py

@@ -1,6 +1,6 @@
 """Security tests for graduation module."""
 
-from odoo import Command
+from odoo import Command, fields
 from odoo.exceptions import AccessError
 from odoo.tests import TransactionCase, tagged
 
@@ -190,3 +190,120 @@ def test_manager_can_reject(self):
         assessment.action_submit()
         assessment.with_user(self.manager).action_reject()
         self.assertEqual(assessment.state, "rejected")
+
+    # ──────────────────────────────────────────────────────────────────────────
+    # Approval boundary — a graduation USER must not approve (or otherwise drive
+    # the approval workflow of) their own assessment, via the action methods OR
+    # via direct RPC write/create of the workflow fields.
+    # ──────────────────────────────────────────────────────────────────────────
+
+    def _submitted_assessment_owned_by_user(self):
+        pathway = self.env["spp.graduation.pathway"].create({"name": "Test Pathway"})
+        assessment = (
+            self.env["spp.graduation.assessment"]
+            .with_user(self.user)
+            .create(
+                {
+                    "pathway_id": pathway.id,
+                    "partner_id": self.beneficiary.id,
+                    "recommendation": "graduate",
+                }
+            )
+        )
+        assessment.action_submit()
+        self.assertEqual(assessment.state, "submitted")
+        return assessment
+
+    def test_user_cannot_approve_own_assessment(self):
+        """Reported self-approval: a user must not approve their own assessment."""
+        assessment = self._submitted_assessment_owned_by_user()
+        with self.assertRaises(AccessError):
+            assessment.action_approve()
+        self.assertEqual(assessment.state, "submitted")
+        self.assertFalse(assessment.graduation_date)
+
+    def test_user_cannot_reject_or_reset_own_assessment(self):
+        assessment = self._submitted_assessment_owned_by_user()
+        with self.assertRaises(AccessError):
+            assessment.action_reject()
+        with self.assertRaises(AccessError):
+            assessment.action_reset_draft()
+        self.assertEqual(assessment.state, "submitted")
+
+    def test_user_cannot_write_approved_state_directly(self):
+        """Raw-RPC bypass of the action methods must also be blocked."""
+        assessment = self._submitted_assessment_owned_by_user()
+        with self.assertRaises(AccessError):
+            assessment.write({"state": "approved"})
+        self.assertEqual(assessment.state, "submitted")
+
+    def test_user_cannot_write_approval_fields_directly(self):
+        assessment = self._submitted_assessment_owned_by_user()
+        with self.assertRaises(AccessError):
+            assessment.write({"graduation_date": fields.Date.today()})
+        with self.assertRaises(AccessError):
+            assessment.write({"approved_by_id": self.user.id})
+
+    def test_user_cannot_create_non_draft_assessment(self):
+        """Creating an already-approved assessment via RPC must be blocked."""
+        pathway = self.env["spp.graduation.pathway"].create({"name": "Test Pathway"})
+        with self.assertRaises(AccessError):
+            self.env["spp.graduation.assessment"].with_user(self.user).create(
+                {
+                    "pathway_id": pathway.id,
+                    "partner_id": self.beneficiary.id,
+                    "state": "approved",
+                    "graduation_date": fields.Date.today(),
+                }
+            )
+
+    def test_manager_approve_sets_graduation_date(self):
+        """Regression: a manager can still approve, and graduation_date is set for
+        a 'graduate' recommendation."""
+        assessment = self._submitted_assessment_owned_by_user()
+        assessment.with_user(self.manager).action_approve()
+        self.assertEqual(assessment.state, "approved")
+        self.assertEqual(assessment.approved_by_id, self.manager)
+        self.assertTrue(assessment.graduation_date)
+
+    def test_user_can_edit_own_draft_content(self):
+        """The guard must not over-block: a user can still edit non-workflow
+        content on their own draft assessment."""
+        pathway = self.env["spp.graduation.pathway"].create({"name": "Test Pathway"})
+        assessment = (
+            self.env["spp.graduation.assessment"]
+            .with_user(self.user)
+            .create({"pathway_id": pathway.id, "partner_id": self.beneficiary.id})
+        )
+        assessment.write({"recommendation": "graduate", "recommendation_notes": "ready"})
+        self.assertEqual(assessment.recommendation, "graduate")
+
+    def test_user_cannot_modify_submitted_assessment_content(self):
+        """A user must not change assessment content once it is submitted — else
+        they could flip the recommendation the manager is about to approve."""
+        assessment = self._submitted_assessment_owned_by_user()
+        with self.assertRaises(AccessError):
+            assessment.write({"recommendation": "extend"})
+        with self.assertRaises(AccessError):
+            assessment.write({"recommendation_notes": "changed after submit"})
+        self.assertEqual(assessment.recommendation, "graduate")
+
+    def test_user_cannot_unsubmit_own_assessment(self):
+        """A user may only move draft -> submitted; un-submitting is manager-only."""
+        assessment = self._submitted_assessment_owned_by_user()
+        with self.assertRaises(AccessError):
+            assessment.write({"state": "draft"})
+        self.assertEqual(assessment.state, "submitted")
+
+    def test_user_cannot_create_assessment_for_another_assessor(self):
+        """A user cannot create an assessment attributed to a different assessor
+        (record-rule protection; documents the boundary)."""
+        pathway = self.env["spp.graduation.pathway"].create({"name": "Test Pathway"})
+        with self.assertRaises(AccessError):
+            self.env["spp.graduation.assessment"].with_user(self.user).create(
+                {
+                    "pathway_id": pathway.id,
+                    "partner_id": self.beneficiary.id,
+                    "assessor_id": self.manager.id,
+                }
+            )