Skip to content

Commit a5eaa36

Browse files
LuisFigueroaGntkathole
LuisFigueroaG
authored and
ntkathole
committed
fix: Reject unknown resource types and actions in the REST permission API
apply_permission silently dropped any type or action name that was not in the name-to-enum maps. A typo or wrong casing in every type left the spec with an empty types list, which Permission treats as ALL_RESOURCE_TYPES, so a permission meant for one resource type ended up covering all of them. Partially invalid action lists were also narrowed without notice. Return a 400 listing the invalid names and the valid options instead. Signed-off-by: LuisFigueroaG <[email protected]>
1 parent 7087f1f commit a5eaa36

2 files changed

Lines changed: 75 additions & 6 deletions

File tree

‎sdk/python/feast/api/registry/rest/permissions.py‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
from typing import Dict, List, Optional
1+
from typing import Dict, List, Mapping, Optional, TypeVar
22

3-
from fastapi import APIRouter, Depends, Query
3+
from fastapi import APIRouter, Depends, HTTPException, Query
44
from fastapi.responses import JSONResponse
55
from pydantic import BaseModel
66

@@ -116,6 +116,21 @@ def _build_policy_proto(policy: PolicyModel) -> PolicyProto:
116116
}
117117

118118

119+
_EnumT = TypeVar("_EnumT")
120+
121+
122+
def _parse_enum_names(
123+
field: str, names: List[str], name_to_enum: Mapping[str, _EnumT]
124+
) -> List[_EnumT]:
125+
invalid = [name for name in names if name not in name_to_enum]
126+
if invalid:
127+
raise HTTPException(
128+
status_code=400,
129+
detail=f"Invalid {field}: {invalid}. Valid options are: {list(name_to_enum)}",
130+
)
131+
return [name_to_enum[name] for name in names]
132+
133+
119134
def get_permission_router(grpc_handler) -> APIRouter:
120135
router = APIRouter()
121136

@@ -179,10 +194,8 @@ def list_permissions(
179194

180195
@router.post("/permissions", status_code=201)
181196
def apply_permission(body: ApplyPermissionRequestBody):
182-
types = [_TYPE_NAME_TO_ENUM[t] for t in body.types if t in _TYPE_NAME_TO_ENUM]
183-
actions = [
184-
_ACTION_NAME_TO_ENUM[a] for a in body.actions if a in _ACTION_NAME_TO_ENUM
185-
]
197+
types = _parse_enum_names("types", body.types, _TYPE_NAME_TO_ENUM)
198+
actions = _parse_enum_names("actions", body.actions, _ACTION_NAME_TO_ENUM)
186199
policy_proto = _build_policy_proto(body.policy)
187200

188201
permission_spec = PermissionSpecProto(

‎sdk/python/tests/unit/api/test_api_rest_registry.py‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2274,6 +2274,62 @@ def test_apply_and_delete_feature_service_via_rest(fastapi_test_app):
22742274
assert response.status_code == 200
22752275

22762276

2277+
def _apply_permission_body(**overrides):
2278+
body = {
2279+
"name": "fv_reader",
2280+
"project": "demo_project",
2281+
"types": ["FEATURE_VIEW"],
2282+
"actions": ["DESCRIBE"],
2283+
"policy": {"role_based_policy": {"roles": ["reader"]}},
2284+
}
2285+
body.update(overrides)
2286+
return body
2287+
2288+
2289+
def test_apply_and_delete_permission_via_rest(fastapi_test_app):
2290+
"""Test POST /permissions and DELETE /permissions/{name} endpoints."""
2291+
response = fastapi_test_app.post("/permissions", json=_apply_permission_body())
2292+
assert response.status_code == 201
2293+
assert response.json()["status"] == "applied"
2294+
2295+
response = fastapi_test_app.get("/permissions/fv_reader?project=demo_project")
2296+
assert response.status_code == 200
2297+
spec = response.json()["spec"]
2298+
assert spec["types"] == ["FEATURE_VIEW"]
2299+
assert spec["actions"] == ["DESCRIBE"]
2300+
2301+
response = fastapi_test_app.delete("/permissions/fv_reader?project=demo_project")
2302+
assert response.status_code == 200
2303+
2304+
response = fastapi_test_app.get("/permissions/fv_reader?project=demo_project")
2305+
assert response.status_code == 404
2306+
2307+
2308+
@pytest.mark.parametrize(
2309+
"overrides, invalid_name",
2310+
[
2311+
({"types": ["feature_view"]}, "feature_view"),
2312+
({"types": ["FEATURE_VIEW", "FEATURE_VIWE"]}, "FEATURE_VIWE"),
2313+
({"actions": ["describe"]}, "describe"),
2314+
({"actions": ["DESCRIBE", "READ"]}, "READ"),
2315+
],
2316+
)
2317+
def test_apply_permission_rejects_unknown_names_via_rest(
2318+
fastapi_test_app, overrides, invalid_name
2319+
):
2320+
"""Unknown types or actions must be rejected instead of silently dropped."""
2321+
response = fastapi_test_app.post(
2322+
"/permissions", json=_apply_permission_body(**overrides)
2323+
)
2324+
assert response.status_code == 400
2325+
detail = response.json()["detail"]
2326+
assert invalid_name in detail
2327+
assert "Valid options are" in detail
2328+
2329+
response = fastapi_test_app.get("/permissions/fv_reader?project=demo_project")
2330+
assert response.status_code == 404
2331+
2332+
22772333
def test_metrics_resource_counts_nonexistent_project(fastapi_test_app):
22782334
"""Test /metrics/resource_counts with a non-existent project returns empty data."""
22792335
response = fastapi_test_app.get(

0 commit comments

Comments
 (0)