Skip to content

Commit b784dba

Browse files
authored
Merge branch 'main' into sort_bug
2 parents 1342301 + 1eacd2b commit b784dba

File tree

5 files changed

+360
-10
lines changed

5 files changed

+360
-10
lines changed

tests/model_serving/maas_billing/maas_subscription/conftest.py

Lines changed: 83 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,25 @@
2424
from tests.model_serving.maas_billing.maas_subscription.utils import (
2525
MAAS_DB_NAMESPACE,
2626
MAAS_SUBSCRIPTION_NAMESPACE,
27+
create_and_yield_api_key_id,
2728
create_api_key,
2829
create_maas_subscription,
2930
get_maas_postgres_resources,
3031
patch_llmisvc_with_maas_router_and_tiers,
32+
resolve_api_key_username,
3133
revoke_api_key,
34+
wait_for_auth_ready,
3235
wait_for_postgres_connection_log,
3336
wait_for_postgres_deployment_ready,
3437
)
3538
from tests.model_serving.maas_billing.utils import build_maas_headers
3639
from utilities.constants import DscComponents
3740
from utilities.general import generate_random_name
38-
from utilities.infra import create_inference_token, create_ns, login_with_user_password
41+
from utilities.infra import create_inference_token, create_ns, get_openshift_token, login_with_user_password
3942
from utilities.llmd_constants import ContainerImages, ModelStorage
4043
from utilities.llmd_utils import create_llmisvc
4144
from utilities.plugins.constant import OpenAIEnpoints
45+
from utilities.resources.auth import Auth
4246

4347
LOGGER = get_logger(name=__name__)
4448

@@ -662,22 +666,91 @@ def active_api_key_id(
662666
"""
663667
Create a single active API key and return its ID for revoke tests.
664668
"""
665-
key_name = f"e2e-fixture-key-{generate_random_name()}"
666-
_, body = create_api_key(
669+
yield from create_and_yield_api_key_id(
670+
request_session_http=request_session_http,
667671
base_url=base_url,
668672
ocp_user_token=ocp_token_for_actor,
669-
request_session_http=request_session_http,
670-
api_key_name=key_name,
673+
key_name_prefix="e2e-fixture-key",
671674
)
672-
LOGGER.info(f"active_api_key_id: created key id={body['id']}")
673-
yield body["id"]
674-
LOGGER.info(f"Fixture teardown: revoking key {body['id']}")
675-
revoke_api_key(
675+
676+
677+
@pytest.fixture(scope="function")
678+
def free_user_username(
679+
request_session_http: requests.Session,
680+
base_url: str,
681+
ocp_token_for_actor: str,
682+
active_api_key_id: str,
683+
) -> str:
684+
"""Resolve and return the free (non-admin) actor's username from their active API key."""
685+
username = resolve_api_key_username(
676686
request_session_http=request_session_http,
677687
base_url=base_url,
678-
key_id=body["id"],
688+
key_id=active_api_key_id,
679689
ocp_user_token=ocp_token_for_actor,
680690
)
691+
LOGGER.info(f"free_user_username: resolved username from key id={active_api_key_id}")
692+
return username
693+
694+
695+
@pytest.fixture(scope="function")
696+
def admin_username(
697+
request_session_http: requests.Session,
698+
base_url: str,
699+
admin_ocp_token: str,
700+
admin_active_api_key_id: str,
701+
) -> str:
702+
"""Resolve and return the admin actor's username from their active API key."""
703+
username = resolve_api_key_username(
704+
request_session_http=request_session_http,
705+
base_url=base_url,
706+
key_id=admin_active_api_key_id,
707+
ocp_user_token=admin_ocp_token,
708+
)
709+
LOGGER.info(f"admin_username: resolved username from key id={admin_active_api_key_id}")
710+
return username
711+
712+
713+
@pytest.fixture(scope="function")
714+
def admin_active_api_key_id(
715+
request_session_http: requests.Session,
716+
base_url: str,
717+
admin_ocp_token: str,
718+
) -> Generator[str, Any, Any]:
719+
"""Create an active API key as the admin user, yield its ID, and revoke on teardown."""
720+
yield from create_and_yield_api_key_id(
721+
request_session_http=request_session_http,
722+
base_url=base_url,
723+
ocp_user_token=admin_ocp_token,
724+
key_name_prefix="e2e-authz-admin",
725+
)
726+
727+
728+
@pytest.fixture(scope="class")
729+
def admin_ocp_token(admin_client: DynamicClient) -> Generator[str, Any, Any]:
730+
"""Temporarily adds dedicated-admins to Auth CR adminGroups so the admin token is recognised by MaaS."""
731+
auth = Auth(client=admin_client, name="auth")
732+
current_groups: list[str] = list(auth.instance.spec.adminGroups or [])
733+
patched_groups = list(set(current_groups + ["dedicated-admins"]))
734+
735+
auth_conditions = (auth.instance.status or {}).get("conditions") or []
736+
ready_before = next(
737+
(condition for condition in auth_conditions if condition.get("type") == "Ready"),
738+
{},
739+
)
740+
baseline_time: str = ready_before.get("lastTransitionTime", "")
741+
742+
LOGGER.info(f"admin_ocp_token: patching Auth CR adminGroups to {patched_groups}")
743+
with ResourceEditor(patches={auth: {"spec": {"adminGroups": patched_groups}}}):
744+
wait_for_auth_ready(auth=auth, baseline_time=baseline_time)
745+
auth_conditions_after = (auth.instance.status or {}).get("conditions") or []
746+
ready_after = next(
747+
(condition for condition in auth_conditions_after if condition.get("type") == "Ready"),
748+
{},
749+
)
750+
cleanup_baseline_time: str = ready_after.get("lastTransitionTime", "")
751+
yield get_openshift_token(client=admin_client)
752+
753+
wait_for_auth_ready(auth=auth, baseline_time=cleanup_baseline_time)
681754

682755

683756
@pytest.fixture(scope="function")
Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
from __future__ import annotations
2+
3+
import pytest
4+
import requests
5+
from simple_logger.logger import get_logger
6+
7+
from tests.model_serving.maas_billing.maas_subscription.utils import (
8+
get_api_key,
9+
list_api_keys,
10+
revoke_api_key,
11+
)
12+
13+
LOGGER = get_logger(name=__name__)
14+
15+
16+
@pytest.mark.parametrize("ocp_token_for_actor", [{"type": "free"}], indirect=True)
17+
@pytest.mark.usefixtures(
18+
"maas_subscription_controller_enabled_latest",
19+
"maas_gateway_api",
20+
"maas_api_gateway_reachable",
21+
)
22+
class TestAPIKeyAuthorization:
23+
"""Tests for MaaS API key admin and non-admin access control."""
24+
25+
@pytest.mark.tier1
26+
def test_admin_manage_other_users_keys(
27+
self,
28+
request_session_http: requests.Session,
29+
base_url: str,
30+
admin_ocp_token: str,
31+
active_api_key_id: str,
32+
free_user_username: str,
33+
) -> None:
34+
"""Verify an admin can search for another user's keys by username and revoke them."""
35+
list_resp, list_body = list_api_keys(
36+
request_session_http=request_session_http,
37+
base_url=base_url,
38+
ocp_user_token=admin_ocp_token,
39+
filters={"username": free_user_username, "status": ["active"]},
40+
sort={"by": "created_at", "order": "desc"},
41+
pagination={"limit": 50, "offset": 0},
42+
)
43+
assert list_resp.status_code == 200, (
44+
f"Expected 200 on admin search by username, got {list_resp.status_code}: {list_resp.text[:200]}"
45+
)
46+
items: list[dict] = list_body.get("items") or list_body.get("data") or []
47+
key_ids = [item["id"] for item in items]
48+
assert active_api_key_id in key_ids, (
49+
f"Expected free user's key id={active_api_key_id} in admin search results, found ids={key_ids}"
50+
)
51+
LOGGER.info(f"[authz] Admin found {len(items)} active key(s) for the free user")
52+
53+
revoke_resp, revoke_body = revoke_api_key(
54+
request_session_http=request_session_http,
55+
base_url=base_url,
56+
key_id=active_api_key_id,
57+
ocp_user_token=admin_ocp_token,
58+
)
59+
assert revoke_resp.status_code == 200, (
60+
f"Expected 200 on admin DELETE /v1/api-keys/{active_api_key_id}, "
61+
f"got {revoke_resp.status_code}: {revoke_resp.text[:200]}"
62+
)
63+
assert revoke_body.get("status") == "revoked", (
64+
f"Expected status='revoked' in admin revoke response, got: {revoke_body}"
65+
)
66+
LOGGER.info(f"[authz] Admin successfully revoked free user's key id={active_api_key_id}")
67+
68+
@pytest.mark.tier1
69+
def test_non_admin_cannot_access_other_users_keys(
70+
self,
71+
request_session_http: requests.Session,
72+
base_url: str,
73+
ocp_token_for_actor: str,
74+
admin_active_api_key_id: str,
75+
) -> None:
76+
"""Verify a non-admin user gets 404 when accessing another user's API key."""
77+
get_resp, _ = get_api_key(
78+
request_session_http=request_session_http,
79+
base_url=base_url,
80+
key_id=admin_active_api_key_id,
81+
ocp_user_token=ocp_token_for_actor,
82+
)
83+
assert get_resp.status_code == 404, (
84+
f"Expected 404 (IDOR protection) on free user GET of admin's key id={admin_active_api_key_id}, "
85+
f"got {get_resp.status_code}: {get_resp.text[:200]}"
86+
)
87+
LOGGER.info(f"[authz] Free user correctly received 404 on GET of admin's key id={admin_active_api_key_id}")
88+
89+
revoke_resp, _ = revoke_api_key(
90+
request_session_http=request_session_http,
91+
base_url=base_url,
92+
key_id=admin_active_api_key_id,
93+
ocp_user_token=ocp_token_for_actor,
94+
)
95+
assert revoke_resp.status_code == 404, (
96+
f"Expected 404 (IDOR protection) on free user DELETE of admin's key id={admin_active_api_key_id}, "
97+
f"got {revoke_resp.status_code}: {revoke_resp.text[:200]}"
98+
)
99+
LOGGER.info(f"[authz] Free user correctly received 404 on DELETE of admin's key id={admin_active_api_key_id}")
100+
101+
@pytest.mark.tier2
102+
def test_non_admin_search_only_returns_own_keys(
103+
self,
104+
request_session_http: requests.Session,
105+
base_url: str,
106+
ocp_token_for_actor: str,
107+
active_api_key_id: str,
108+
admin_active_api_key_id: str,
109+
) -> None:
110+
"""Verify a non-admin user's search results contain only their own keys."""
111+
list_resp, list_body = list_api_keys(
112+
request_session_http=request_session_http,
113+
base_url=base_url,
114+
ocp_user_token=ocp_token_for_actor,
115+
filters={"status": ["active"]},
116+
sort={"by": "created_at", "order": "desc"},
117+
pagination={"limit": 50, "offset": 0},
118+
)
119+
assert list_resp.status_code == 200, (
120+
f"Expected 200 on free user search, got {list_resp.status_code}: {list_resp.text[:200]}"
121+
)
122+
items: list[dict] = list_body.get("items") or list_body.get("data") or []
123+
key_ids = [item["id"] for item in items]
124+
125+
assert active_api_key_id in key_ids, (
126+
f"Expected free user's own key id={active_api_key_id} in results, found ids={key_ids}"
127+
)
128+
assert admin_active_api_key_id not in key_ids, (
129+
f"Admin's key id={admin_active_api_key_id} must NOT appear in free user's search results"
130+
)
131+
LOGGER.info(
132+
f"[authz] Free user search returned {len(items)} key(s) — own key present, admin's key correctly excluded"
133+
)
134+
135+
@pytest.mark.tier2
136+
def test_non_admin_cannot_search_by_other_username(
137+
self,
138+
request_session_http: requests.Session,
139+
base_url: str,
140+
ocp_token_for_actor: str,
141+
admin_username: str,
142+
) -> None:
143+
"""Verify a non-admin user gets 403 when searching with another user's username as a filter."""
144+
list_resp, _ = list_api_keys(
145+
request_session_http=request_session_http,
146+
base_url=base_url,
147+
ocp_user_token=ocp_token_for_actor,
148+
filters={"username": admin_username, "status": ["active"]},
149+
sort={"by": "created_at", "order": "desc"},
150+
pagination={"limit": 50, "offset": 0},
151+
)
152+
assert list_resp.status_code == 403, (
153+
f"Expected 403 when free user searches by another user's username, "
154+
f"got {list_resp.status_code}: {list_resp.text[:200]}"
155+
)
156+
LOGGER.info("[authz] Free user correctly received 403 when searching by another user's username")

tests/model_serving/maas_billing/maas_subscription/utils.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
MAAS_GATEWAY_NAMESPACE,
2626
ApiGroups,
2727
)
28+
from utilities.general import generate_random_name
29+
from utilities.resources.auth import Auth
2830

2931
LOGGER = get_logger(name=__name__)
3032
MAAS_SUBSCRIPTION_NAMESPACE = "models-as-a-service"
@@ -282,6 +284,72 @@ def list_api_keys(
282284
return response, parsed_body
283285

284286

287+
def wait_for_auth_ready(auth: Auth, baseline_time: str, timeout: int = 60) -> None:
288+
"""Wait for Auth CR to reconcile after a patch."""
289+
for instance in TimeoutSampler(wait_timeout=timeout, sleep=2, func=lambda: auth.instance):
290+
auth_conditions = (instance.status or {}).get("conditions") or []
291+
ready_condition = next(
292+
(condition for condition in auth_conditions if condition.get("type") == "Ready"),
293+
None,
294+
)
295+
if (
296+
ready_condition
297+
and ready_condition.get("lastTransitionTime") != baseline_time
298+
and ready_condition.get("status") == "True"
299+
):
300+
return
301+
302+
303+
def resolve_api_key_username(
304+
request_session_http: requests.Session,
305+
base_url: str,
306+
key_id: str,
307+
ocp_user_token: str,
308+
) -> str:
309+
"""Fetch an API key by ID and return the owner's username."""
310+
get_resp, get_body = get_api_key(
311+
request_session_http=request_session_http,
312+
base_url=base_url,
313+
key_id=key_id,
314+
ocp_user_token=ocp_user_token,
315+
)
316+
assert get_resp.status_code == 200, (
317+
f"Expected 200 on GET /v1/api-keys/{key_id}, got {get_resp.status_code}: {get_resp.text[:200]}"
318+
)
319+
username = get_body.get("username") or get_body.get("owner")
320+
assert username, "Expected 'username' or 'owner' field in GET response"
321+
return username
322+
323+
324+
def create_and_yield_api_key_id(
325+
request_session_http: requests.Session,
326+
base_url: str,
327+
ocp_user_token: str,
328+
key_name_prefix: str,
329+
) -> Generator[str]:
330+
"""Create an API key, yield its ID, and revoke it on teardown."""
331+
key_name = f"{key_name_prefix}-{generate_random_name()}"
332+
_, body = create_api_key(
333+
base_url=base_url,
334+
ocp_user_token=ocp_user_token,
335+
request_session_http=request_session_http,
336+
api_key_name=key_name,
337+
)
338+
LOGGER.info(f"create_and_yield_api_key_id: created key id={body['id']} name={key_name}")
339+
yield body["id"]
340+
LOGGER.info(f"create_and_yield_api_key_id: teardown revoking key id={body['id']}")
341+
revoke_resp, _ = revoke_api_key(
342+
request_session_http=request_session_http,
343+
base_url=base_url,
344+
key_id=body["id"],
345+
ocp_user_token=ocp_user_token,
346+
)
347+
if revoke_resp.status_code not in (200, 404):
348+
raise AssertionError(
349+
f"Unexpected teardown status for key id={body['id']}: {revoke_resp.status_code} {revoke_resp.text[:200]}"
350+
)
351+
352+
285353
def revoke_api_key(
286354
request_session_http: requests.Session,
287355
base_url: str,

utilities/constants.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ class ApiGroups:
140140
KSERVE: str = "serving.kserve.io"
141141
KUADRANT_IO: str = "kuadrant.io"
142142
MAAS_IO: str = "maas.opendatahub.io"
143+
AUTH_IO: str = "SERVICES_PLATFORM_OPENDATAHUB_IO"
143144

144145

145146
class Annotations:

0 commit comments

Comments
 (0)