Skip to content

Commit 467d1ff

Browse files
committed
fix(validation): return proper HTTP status codes for institution errors
- Change ValueError to HTTPException (404) when institution not found in validation_helper - Fix test_validate_edvise_unauthorized to test actual unauthorized access instead of non-existent institution - Ensures proper HTTP status codes are returned to API clients
1 parent 8bfd945 commit 467d1ff

File tree

2 files changed

+47
-9
lines changed

2 files changed

+47
-9
lines changed

src/webapp/routers/data.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1627,7 +1627,10 @@ def validation_helper(
16271627
select(InstTable).where(InstTable.id == str_to_uuid(inst_id))
16281628
).scalar_one_or_none()
16291629
if inst is None:
1630-
raise ValueError(f"Institution {inst_id} not found")
1630+
raise HTTPException(
1631+
status_code=status.HTTP_404_NOT_FOUND,
1632+
detail=f"Institution {inst_id} not found",
1633+
)
16311634

16321635
bucket = get_external_bucket_name(inst_id)
16331636
# --- choose / prepare extension schema (try to avoid heavy path)

src/webapp/routers/data_test.py

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1241,15 +1241,50 @@ def test_validate_sftp_with_edvise_schema(edvise_client: TestClient) -> None:
12411241
assert MOCK_STORAGE.validate_file.called
12421242

12431243

1244-
def test_validate_edvise_unauthorized(edvise_client: TestClient) -> None:
1244+
def test_validate_edvise_unauthorized(
1245+
edvise_session: sqlalchemy.orm.Session, monkeypatch: Any
1246+
) -> None:
12451247
"""Test validation endpoint with unauthorized access."""
1246-
response = edvise_client.post(
1247-
"/institutions/"
1248-
+ uuid_to_str(UUID_INVALID) # Institution user doesn't have access to
1249-
+ "/input/validate-upload/test_student.csv",
1250-
)
1251-
assert response.status_code == 401
1252-
assert "Not authorized" in response.json()["detail"]
1248+
monkeypatch.setenv("SST_SKIP_EXT_GEN", "1")
1249+
1250+
# Create a test client with a MODEL_OWNER user who only has access to EDVISE_INST_UUID
1251+
# This user should NOT have access to EDVISE_INST_2_UUID
1252+
def get_session_override():
1253+
return edvise_session
1254+
1255+
def get_current_active_user_override():
1256+
# User belongs to EDVISE_INST_UUID, not DATAKINDER, so access is restricted
1257+
from ..utilities import AccessType, BaseUser
1258+
1259+
return BaseUser(
1260+
uuid_to_str(USER_UUID),
1261+
uuid_to_str(EDVISE_INST_UUID), # User belongs to this institution
1262+
AccessType.MODEL_OWNER, # Not DATAKINDER, so access is restricted
1263+
"abc@example.com",
1264+
)
1265+
1266+
def storage_control_override():
1267+
return MOCK_STORAGE
1268+
1269+
app.include_router(router)
1270+
app.dependency_overrides[get_session] = get_session_override
1271+
app.dependency_overrides[get_current_active_user] = get_current_active_user_override
1272+
app.dependency_overrides[StorageControl] = storage_control_override
1273+
1274+
client = TestClient(app)
1275+
try:
1276+
# Try to access EDVISE_INST_2_UUID which exists but user doesn't have access to
1277+
response = client.post(
1278+
"/institutions/"
1279+
+ uuid_to_str(
1280+
EDVISE_INST_2_UUID
1281+
) # Institution exists but user is unauthorized
1282+
+ "/input/validate-upload/test_student.csv",
1283+
)
1284+
assert response.status_code == 401
1285+
assert "Not authorized" in response.json()["detail"]
1286+
finally:
1287+
app.dependency_overrides.clear()
12531288

12541289

12551290
def test_validate_edvise_invalid_filename(edvise_client: TestClient) -> None:

0 commit comments

Comments
 (0)