From aec79fad9670e9a421d10c37bee895ceac185610 Mon Sep 17 00:00:00 2001
From: MinetaS <skdty87@gmail.com>
Date: Thu, 4 Jul 2024 01:33:15 +0900
Subject: [PATCH] Prevent unrestrained unblocking of workers

This change prevents non-approver users from unblocking their workers
when they are not blocked by themselves.
---
 server/fishtest/rundb.py    |  4 ++--
 server/fishtest/schemas.py  |  2 +-
 server/fishtest/views.py    | 38 +++++++++++++++++++++++++++----------
 server/fishtest/workerdb.py |  8 ++++----
 4 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/server/fishtest/rundb.py b/server/fishtest/rundb.py
index 0e3c105ea..4e0b77ec2 100644
--- a/server/fishtest/rundb.py
+++ b/server/fishtest/rundb.py
@@ -1033,10 +1033,10 @@ def sync_request_task(self, worker_info):
         my_name = worker_name(worker_info, short=True)
         host_url = worker_info.get("host_url", "<host_url>")
         w = self.workerdb.get_worker(my_name)
-        if w["blocked"]:
+        if w["blocked_by"]:
             # updates last_updated
             self.workerdb.update_worker(
-                my_name, blocked=w["blocked"], message=w["message"]
+                my_name, blocked_by=w["blocked_by"], message=w["message"]
             )
             error = self.blocked_worker_message(my_name, w["message"], host_url)
             return {"task_waiting": False, "error": error}
diff --git a/server/fishtest/schemas.py b/server/fishtest/schemas.py
index 6dab8098e..cc9c4b02b 100644
--- a/server/fishtest/schemas.py
+++ b/server/fishtest/schemas.py
@@ -104,7 +104,7 @@ def size_is_length(x):
 worker_schema = {
     "_id?": ObjectId,
     "worker_name": short_worker_name,
-    "blocked": bool,
+    "blocked_by": str,
     "message": worker_message,
     "last_updated": datetime_utc,
 }
diff --git a/server/fishtest/views.py b/server/fishtest/views.py
index 21dc175f1..55dc0beee 100644
--- a/server/fishtest/views.py
+++ b/server/fishtest/views.py
@@ -160,7 +160,7 @@ def login(request):
 # limited.
 
 
-def worker_email(worker_name, blocker_name, message, host_url, blocked):
+def worker_email(worker_name, blocker_name, message, host_url, blocked_by):
     owner_name = worker_name.split("-")[0]
     body = f"""\
 Dear {owner_name},
@@ -204,7 +204,7 @@ def workers(request):
                 blocker_name,
                 w["message"],
                 request.host_url,
-                w["blocked"],
+                w["blocked_by"],
             )
             w["subject"] = f"Issue(s) with worker {w['worker_name']}"
 
@@ -247,26 +247,44 @@ def workers(request):
                 )
                 message = message[:max_chars]
             message = normalize_lf(message)
-            was_blocked = request.workerdb.get_worker(worker_name)["blocked"]
-            request.rundb.workerdb.update_worker(
-                worker_name, blocked=blocked, message=message
-            )
-            if blocked != was_blocked:
+            blocked_by = request.workerdb.get_worker(worker_name)["blocked_by"]
+            if blocked and not blocked_by:
+                request.rundb.workerdb.update_worker(
+                    worker_name, blocked_by=blocker_name, message=message
+                )
                 request.session.flash(
-                    f"Worker {worker_name} {'blocked' if blocked else 'unblocked'}!",
+                    f"Worker {worker_name} blocked!",
                 )
                 request.actiondb.block_worker(
                     username=blocker_name,
                     worker=worker_name,
-                    message="blocked" if blocked else "unblocked",
+                    message="blocked",
                 )
+            elif not blocked and blocked_by:
+                # Prevent users from abusing unblocking workers.
+                if not is_approver and blocked_by != blocker_name:
+                    request.session.flash(
+                        f"Contact approvers to unblock worker {worker_name}!",
+                    )
+                else:
+                    request.rundb.workerdb.update_worker(
+                        worker_name, blocked_by=None, message=message
+                    )
+                    request.session.flash(
+                        f"Worker {worker_name} unblocked!",
+                    )
+                    request.actiondb.block_worker(
+                        username=blocker_name,
+                        worker=worker_name,
+                        message="unblocked",
+                    )
         return HTTPFound(location=request.route_url("workers", worker_name="show"))
 
     w = request.rundb.workerdb.get_worker(worker_name)
     return {
         "show_admin": True,
         "worker_name": worker_name,
-        "blocked": w["blocked"],
+        "blocked": w["blocked_by"] is not None,
         "message": w["message"],
         "show_email": is_approver,
         "last_updated": w["last_updated"],
diff --git a/server/fishtest/workerdb.py b/server/fishtest/workerdb.py
index 2783871ee..4d2866407 100644
--- a/server/fishtest/workerdb.py
+++ b/server/fishtest/workerdb.py
@@ -20,17 +20,17 @@ def get_worker(
         if r is None:
             return {
                 "worker_name": worker_name,
-                "blocked": False,
+                "blocked_by": None,
                 "message": "",
                 "last_updated": None,
             }
         else:
             return r
 
-    def update_worker(self, worker_name, blocked=None, message=None):
+    def update_worker(self, worker_name, blocked_by=None, message=None):
         r = {
             "worker_name": worker_name,
-            "blocked": blocked,
+            "blocked_by": blocked_by,
             "message": message,
             "last_updated": datetime.now(timezone.utc),
         }
@@ -38,5 +38,5 @@ def update_worker(self, worker_name, blocked=None, message=None):
         self.workers.replace_one({"worker_name": worker_name}, r, upsert=True)
 
     def get_blocked_workers(self):
-        q = {"blocked": True}
+        q = {"blocked": {"$ne": None}}
         return list(self.workers.find(q))