fix(assign): exclude admin from assignable operators; sync Jira on assign even for first-time users
Aegis CI / lint-and-test (push) Has been cancelled
Snyk Security Scan / Python vulnerabilities (backend) (push) Has been cancelled
Snyk Security Scan / npm vulnerabilities (frontend) (push) Has been cancelled
Snyk Security Scan / Docker image vulnerabilities (backend) (push) Has been cancelled
Aegis CI / lint-and-test (push) Has been cancelled
Snyk Security Scan / Python vulnerabilities (backend) (push) Has been cancelled
Snyk Security Scan / npm vulnerabilities (frontend) (push) Has been cancelled
Snyk Security Scan / Docker image vulnerabilities (backend) (push) Has been cancelled
Admin can no longer be picked as a red_tech/blue_tech assignee — it administers the site, it doesn't operate tests. Applied to the eligible-assignee list on both the picker UI and the assign endpoint's validation, and dropped from the GET /users/operators pool. push_assignee_update() now falls back to a fresh Jira account-id lookup when the assignee hasn't logged in yet (so jira_account_id is still empty), instead of silently no-op'ing — assignment now syncs to Jira immediately regardless of whether the assignee has ever logged into Aegis before.
This commit is contained in:
@@ -1286,14 +1286,14 @@ def assign_test_operators(
|
||||
|
||||
if payload.red_tech_assignee is not None:
|
||||
u = db.query(User).filter(User.id == payload.red_tech_assignee).first()
|
||||
if not u or u.role not in ("red_tech", "red_lead", "admin"):
|
||||
if not u or u.role not in ("red_tech", "red_lead"):
|
||||
raise HTTPException(status_code=400, detail="Invalid red tech assignee")
|
||||
test.red_tech_assignee = payload.red_tech_assignee
|
||||
newly_assigned = u
|
||||
|
||||
if payload.blue_tech_assignee is not None:
|
||||
u = db.query(User).filter(User.id == payload.blue_tech_assignee).first()
|
||||
if not u or u.role not in ("blue_tech", "blue_lead", "admin"):
|
||||
if not u or u.role not in ("blue_tech", "blue_lead"):
|
||||
raise HTTPException(status_code=400, detail="Invalid blue tech assignee")
|
||||
test.blue_tech_assignee = payload.blue_tech_assignee
|
||||
newly_assigned = u
|
||||
|
||||
@@ -161,13 +161,14 @@ def list_operators_route(
|
||||
"""Return active red/blue operators and leads, for lead/manager assignment pickers.
|
||||
|
||||
Not reachable by admin — admin administers the site, leads/managers
|
||||
coordinate operators. Returns only id/username/role — no emails or
|
||||
coordinate operators, and admin cannot itself be assigned as an
|
||||
operator either. Returns only id/username/role — no emails or
|
||||
tokens — since this is reachable by non-admin leads.
|
||||
"""
|
||||
return (
|
||||
db.query(User)
|
||||
.filter(
|
||||
User.role.in_(["red_tech", "red_lead", "blue_tech", "blue_lead", "admin"]),
|
||||
User.role.in_(["red_tech", "red_lead", "blue_tech", "blue_lead"]),
|
||||
User.is_active.is_(True),
|
||||
)
|
||||
.order_by(User.username)
|
||||
|
||||
@@ -891,6 +891,14 @@ def push_assignee_update(db: Session, test: Test, assignee: User) -> None:
|
||||
if not link:
|
||||
return
|
||||
|
||||
jira_account_id = getattr(assignee, "jira_account_id", None)
|
||||
if not jira_account_id:
|
||||
# Normally populated on login (see lookup_user_jira_account_id) —
|
||||
# but a lead may assign someone who has never logged in yet. Try
|
||||
# a fresh lookup now instead of silently giving up, so assignment
|
||||
# still syncs to Jira immediately.
|
||||
lookup_user_jira_account_id(db, assignee)
|
||||
db.flush()
|
||||
jira_account_id = getattr(assignee, "jira_account_id", None)
|
||||
if not jira_account_id:
|
||||
return
|
||||
|
||||
@@ -55,6 +55,19 @@ def test_assign_rejects_wrong_role_for_side(client, db, red_lead_headers, red_le
|
||||
assert resp.status_code == 400
|
||||
|
||||
|
||||
def test_admin_cannot_be_assigned_as_operator(client, db, red_lead_headers, red_lead_user, admin_user):
|
||||
"""Admin administers the site and can't be assigned as an operator either."""
|
||||
technique = _seed_technique(db)
|
||||
test = _seed_test(db, technique, red_lead_user.id)
|
||||
|
||||
resp = client.post(
|
||||
f"/api/v1/tests/{test.id}/assign",
|
||||
json={"red_tech_assignee": str(admin_user.id)},
|
||||
headers=red_lead_headers,
|
||||
)
|
||||
assert resp.status_code == 400
|
||||
|
||||
|
||||
def test_red_tech_cannot_assign(client, db, red_tech_headers, red_tech_user):
|
||||
technique = _seed_technique(db)
|
||||
test = _seed_test(db, technique, red_tech_user.id)
|
||||
|
||||
@@ -488,6 +488,7 @@ def test_push_assignee_update_assigns_jira_issue(mock_get_client, mock_configure
|
||||
@patch("app.services.jira_service.get_admin_jira_client")
|
||||
def test_push_assignee_update_noop_without_jira_account_id(mock_get_client, mock_configured, db):
|
||||
mock_jira = MagicMock()
|
||||
mock_jira.user_find_by_user_string.return_value = [] # fallback lookup finds nobody either
|
||||
mock_get_client.return_value = mock_jira
|
||||
test = _make_test()
|
||||
link = _make_link(test.id)
|
||||
@@ -500,6 +501,29 @@ def test_push_assignee_update_noop_without_jira_account_id(mock_get_client, mock
|
||||
mock_jira.assign_issue.assert_not_called()
|
||||
|
||||
|
||||
@patch("app.services.jira_service.has_admin_jira_configured", return_value=True)
|
||||
@patch("app.services.jira_service.get_admin_jira_client")
|
||||
def test_push_assignee_update_looks_up_missing_account_id(mock_get_client, mock_configured, db):
|
||||
"""Regression: a lead can assign someone who has never logged in, so
|
||||
jira_account_id isn't populated yet. Must try a fresh lookup instead of
|
||||
silently giving up, so the assignment still syncs immediately."""
|
||||
mock_jira = MagicMock()
|
||||
mock_jira.user_find_by_user_string.return_value = [
|
||||
{"emailAddress": "gerardo.ruiz@kaseya.com", "accountId": "freshly-found-id"}
|
||||
]
|
||||
mock_get_client.return_value = mock_jira
|
||||
test = _make_test()
|
||||
link = _make_link(test.id)
|
||||
db.add(link)
|
||||
db.commit()
|
||||
assignee = _make_user(jira_account_id=None)
|
||||
|
||||
jira_service.push_assignee_update(db, test, assignee)
|
||||
|
||||
mock_jira.assign_issue.assert_called_once_with(link.jira_issue_key, account_id="freshly-found-id")
|
||||
assert assignee.jira_account_id == "freshly-found-id"
|
||||
|
||||
|
||||
@patch("app.services.jira_service.has_admin_jira_configured", return_value=True)
|
||||
@patch("app.services.jira_service.get_admin_jira_client")
|
||||
def test_push_assignee_update_noop_without_link(mock_get_client, mock_configured, db):
|
||||
|
||||
@@ -44,3 +44,11 @@ def test_manager_can_list_operators(client, manager_headers, red_tech_user):
|
||||
assert resp.status_code == 200, resp.text
|
||||
usernames = {u["username"] for u in resp.json()}
|
||||
assert "redtech" in usernames
|
||||
|
||||
|
||||
def test_operators_list_excludes_admin(client, manager_headers, admin_user):
|
||||
"""Admin can't be assigned as an operator, so it shouldn't appear in the picker."""
|
||||
resp = client.get("/api/v1/users/operators", headers=manager_headers)
|
||||
assert resp.status_code == 200
|
||||
usernames = {u["username"] for u in resp.json()}
|
||||
assert admin_user.username not in usernames
|
||||
|
||||
@@ -19,8 +19,8 @@ const SIDE_STYLE = {
|
||||
};
|
||||
|
||||
const SIDE_ROLES: Record<"red" | "blue", string[]> = {
|
||||
red: ["red_tech", "red_lead", "admin"],
|
||||
blue: ["blue_tech", "blue_lead", "admin"],
|
||||
red: ["red_tech", "red_lead"],
|
||||
blue: ["blue_tech", "blue_lead"],
|
||||
};
|
||||
|
||||
/** Lead/manager picker for the red_tech_assignee / blue_tech_assignee fields. */
|
||||
|
||||
Reference in New Issue
Block a user