Skip to content

Commit

Permalink
Cannot delete Agent not belonging to user-id
Browse files Browse the repository at this point in the history
  • Loading branch information
tianjing-li committed Aug 14, 2024
1 parent b05ce43 commit 4bf1aa7
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 17 deletions.
12 changes: 5 additions & 7 deletions src/backend/crud/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ def get_agent_by_id(db: Session, agent_id: str, user_id: str) -> Agent:
"""
agent = db.query(Agent).filter(Agent.id == agent_id).first()

# Cannot GET privates Agents not belonging to you
if agent and agent.is_private and agent.user_id != user_id:
return None

Expand Down Expand Up @@ -277,23 +278,20 @@ def update_agent(
@validate_transaction
def delete_agent(db: Session, agent_id: str, user_id: str) -> bool:
"""
Delete an agent by ID.
Anyone can delete a public agent, but only the owner can delete a private agent.
Delete an Agent by ID if the Agent was created by the user_id given.
Args:
db (Session): Database session.
agent_id (str): Agent ID.
Returns:
bool: True if the agent was deleted, False otherwise
bool: True if the Agent was deleted, False otherwise
"""
agent_query = db.query(Agent).filter(Agent.id == agent_id)
agent = agent_query.first()

if not agent:
return False

if agent and agent.is_private and agent.user_id != user_id:
# Can only delete Agent created by user_id
if agent.user_id != user_id:
return False

agent_query.delete()
Expand Down
9 changes: 5 additions & 4 deletions src/backend/routers/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -437,10 +437,11 @@ async def delete_agent(
ctx.with_agent(agent_schema)
ctx.with_metrics_agent(agent_to_metrics_agent(agent))

try:
agent_crud.delete_agent(session, agent_id, user_id)
except Exception as e:
raise HTTPException(status_code=500, detail=str(e))
deleted = agent_crud.delete_agent(session, agent_id, user_id)
if not deleted:
raise HTTPException(
status_code=401, detail="Cannot delete Agent not belonging to user."
)

return DeleteAgent()

Expand Down
58 changes: 52 additions & 6 deletions src/backend/tests/unit/routers/test_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from backend.tests.unit.factories import get_factory


async def test_create_agent_mertic(
async def test_create_agent_metric(
session_client: TestClient, session: Session
) -> None:
user = get_factory("User", session).create(fullname="John Doe")
Expand Down Expand Up @@ -301,11 +301,11 @@ def test_list_agents_with_pagination(


@pytest.mark.asyncio
async def test_get_agent_mertic(
async def test_get_agent_metric(
session_client: TestClient, session: Session, user
) -> None:
agent = get_factory("Agent", session).create(name="test agent", user_id=user.id)
agent_tool_metadata = get_factory("AgentToolMetadata", session).create(
_ = get_factory("AgentToolMetadata", session).create(
user_id=user.id,
agent_id=agent.id,
tool_name=ToolName.Google_Drive,
Expand Down Expand Up @@ -337,7 +337,7 @@ async def test_get_agent_mertic(


@pytest.mark.asyncio
async def test_get_default_agent_mertic(
async def test_get_default_agent_metric(
session_client: TestClient, session: Session, user
) -> None:

Expand Down Expand Up @@ -968,8 +968,24 @@ def test_delete_agent_metric(
assert m_args.assistant_id == agent.id


def test_delete_agent(session_client: TestClient, session: Session, user) -> None:
agent = get_factory("Agent", session).create(user=user)
def test_delete_public_agent(
session_client: TestClient, session: Session, user
) -> None:
agent = get_factory("Agent", session).create(user=user, is_private=False)
response = session_client.delete(
f"/v1/agents/{agent.id}", headers={"User-Id": user.id}
)
assert response.status_code == 200
assert response.json() == {}

agent = session.get(Agent, agent.id)
assert agent is None


def test_delete_private_agent(
session_client: TestClient, session: Session, user
) -> None:
agent = get_factory("Agent", session).create(user=user, is_private=True)
response = session_client.delete(
f"/v1/agents/{agent.id}", headers={"User-Id": user.id}
)
Expand All @@ -980,6 +996,36 @@ def test_delete_agent(session_client: TestClient, session: Session, user) -> Non
assert agent is None


def test_cannot_delete_private_agent_not_belonging_to_user_id(
session_client: TestClient, session: Session, user
) -> None:
agent = get_factory("Agent", session).create(user=user, is_private=True)
other_user = get_factory("User", session).create()
response = session_client.delete(
f"/v1/agents/{agent.id}", headers={"User-Id": other_user.id}
)
assert response.status_code == 404
assert response.json() == {"detail": f"Agent with ID {agent.id} not found."}

agent = session.get(Agent, agent.id)
assert agent is not None


def test_cannot_delete_public_agent_not_belonging_to_user_id(
session_client: TestClient, session: Session, user
) -> None:
agent = get_factory("Agent", session).create(user=user, is_private=False)
other_user = get_factory("User", session).create()
response = session_client.delete(
f"/v1/agents/{agent.id}", headers={"User-Id": other_user.id}
)
assert response.status_code == 401
assert response.json() == {"detail": "Cannot delete Agent not belonging to user."}

agent = session.get(Agent, agent.id)
assert agent is not None


def test_fail_delete_nonexistent_agent(
session_client: TestClient, session: Session, user
) -> None:
Expand Down

0 comments on commit 4bf1aa7

Please sign in to comment.