Prevent unrestrained unblocking of workers

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}

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,
 }

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,46 @@ 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"]
+            new_worker_info = {
+                "worker_name": worker_name,
+                "blocked_by": blocked_by,
+                "message": message,
+            }
+            if blocked and not blocked_by:
+                new_worker_info["blocked_by"] = blocker_name
                 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:
+                    new_worker_info["blocked_by"] = None
+                    request.session.flash(
+                        f"Worker {worker_name} unblocked!",
+                    )
+                    request.actiondb.block_worker(
+                        username=blocker_name,
+                        worker=worker_name,
+                        message="unblocked",
+                    )
+            request.workerdb.update_worker(**new_worker_info)
         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"],

server/fishtest/workerdb.py

@@ -20,23 +20,23 @@ 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),
         }
         validate(worker_schema, r, "worker")  # may throw exception
         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))