fix(tests): restrict On Hold/Resume to whichever team currently owns the test
canHold checked role OR role instead of matching the current phase, so a red_tech still saw (and could call) Hold/Resume on a test already sitting in blue_evaluating. Fixed on both frontend and backend — the API had the same gap, not just the button visibility.
This commit is contained in:
@@ -1310,6 +1310,21 @@ def assign_test_operators(
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _check_hold_permission(current_user: User, test) -> None:
|
||||
"""Only whichever team currently owns the test may hold/resume it.
|
||||
|
||||
``require_any_role_strict("red_tech", "blue_tech")`` alone lets either
|
||||
role through regardless of phase — a red_tech could otherwise hold (or
|
||||
resume) a test that has already moved into blue_evaluating.
|
||||
"""
|
||||
expected_role = "blue_tech" if test.state == TestState.blue_evaluating else "red_tech"
|
||||
if current_user.role != expected_role:
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
detail=f"Only {expected_role.replace('_', ' ')} can do this while the test is in '{test.state}'.",
|
||||
)
|
||||
|
||||
|
||||
@router.post("/{test_id}/hold", response_model=TestOut)
|
||||
def hold_test(
|
||||
test_id: uuid.UUID,
|
||||
@@ -1330,6 +1345,8 @@ def hold_test(
|
||||
detail=f"Cannot hold a test in state '{test.state}'. Only pre-validation states can be held.",
|
||||
)
|
||||
|
||||
_check_hold_permission(current_user, test)
|
||||
|
||||
if test.is_on_hold:
|
||||
raise HTTPException(status_code=400, detail="Test is already on hold")
|
||||
|
||||
@@ -1370,6 +1387,8 @@ def resume_test(
|
||||
if not test.is_on_hold:
|
||||
raise HTTPException(status_code=400, detail="Test is not on hold")
|
||||
|
||||
_check_hold_permission(current_user, test)
|
||||
|
||||
test.is_on_hold = False
|
||||
test.hold_reason = None
|
||||
test.held_at = None
|
||||
|
||||
@@ -31,6 +31,20 @@ def _seed_executing_test(db, technique, created_by) -> Test:
|
||||
return test
|
||||
|
||||
|
||||
def _seed_blue_evaluating_test(db, technique, created_by) -> Test:
|
||||
test = Test(
|
||||
technique_id=technique.id,
|
||||
name="Holdable test (blue)",
|
||||
created_by=created_by,
|
||||
state=TestState.blue_evaluating,
|
||||
blue_started_at=datetime.utcnow() - timedelta(minutes=10),
|
||||
)
|
||||
db.add(test)
|
||||
db.commit()
|
||||
db.refresh(test)
|
||||
return test
|
||||
|
||||
|
||||
def test_hold_pauses_the_running_timer(client, db, red_tech_headers, red_tech_user):
|
||||
technique = _seed_technique(db)
|
||||
test = _seed_executing_test(db, technique, red_tech_user.id)
|
||||
@@ -88,3 +102,49 @@ def test_hold_does_not_double_pause_an_already_paused_timer(client, db, red_tech
|
||||
|
||||
db.refresh(test)
|
||||
assert test.paused_at == manual_pause_time
|
||||
|
||||
|
||||
def test_red_tech_cannot_hold_a_test_in_blue_evaluating(
|
||||
client, db, red_tech_headers, red_tech_user,
|
||||
):
|
||||
"""Once a test has moved into Blue's queue, only blue_tech may hold it —
|
||||
a red_tech is no longer involved at this stage."""
|
||||
technique = _seed_technique(db)
|
||||
test = _seed_blue_evaluating_test(db, technique, red_tech_user.id)
|
||||
|
||||
resp = client.post(
|
||||
f"/api/v1/tests/{test.id}/hold",
|
||||
json={"reason": "waiting on lab access"},
|
||||
headers=red_tech_headers,
|
||||
)
|
||||
assert resp.status_code == 403
|
||||
|
||||
|
||||
def test_blue_tech_can_hold_a_test_in_blue_evaluating(
|
||||
client, db, blue_tech_headers, red_tech_user,
|
||||
):
|
||||
technique = _seed_technique(db)
|
||||
test = _seed_blue_evaluating_test(db, technique, red_tech_user.id)
|
||||
|
||||
resp = client.post(
|
||||
f"/api/v1/tests/{test.id}/hold",
|
||||
json={"reason": "waiting on EDR access"},
|
||||
headers=blue_tech_headers,
|
||||
)
|
||||
assert resp.status_code == 200, resp.text
|
||||
|
||||
|
||||
def test_blue_tech_cannot_resume_a_test_that_was_held_during_red_executing(
|
||||
client, db, api, red_tech_headers, blue_tech_headers, red_tech_user,
|
||||
):
|
||||
technique = _seed_technique(db)
|
||||
test = _seed_executing_test(db, technique, red_tech_user.id)
|
||||
|
||||
resp = api(
|
||||
"post", f"/api/v1/tests/{test.id}/hold", red_tech_headers,
|
||||
json={"reason": "waiting on lab access"},
|
||||
)
|
||||
assert resp.status_code == 200, resp.text
|
||||
|
||||
resp = api("post", f"/api/v1/tests/{test.id}/resume", blue_tech_headers)
|
||||
assert resp.status_code == 403
|
||||
|
||||
@@ -158,10 +158,13 @@ export default function TestDetailHeader({
|
||||
|
||||
// ── Contextual action buttons ────────────────────────────────────
|
||||
|
||||
const HOLDABLE_STATES: TestState[] = ["draft", "red_executing", "blue_evaluating"];
|
||||
// Whichever team currently "owns" the test is the only one who should see
|
||||
// Hold — a red_tech shouldn't be able to hold a test that's already moved
|
||||
// into blue_evaluating (and vice versa), even though both roles can hold
|
||||
// during their own phase.
|
||||
const canHold =
|
||||
HOLDABLE_STATES.includes(test.state) &&
|
||||
(role === "red_tech" || role === "blue_tech");
|
||||
(["draft", "red_executing"].includes(test.state) && role === "red_tech") ||
|
||||
(test.state === "blue_evaluating" && role === "blue_tech");
|
||||
|
||||
const renderActions = () => {
|
||||
const buttons: React.ReactNode[] = [];
|
||||
|
||||
Reference in New Issue
Block a user