Skip to content

Commit 5f79b47

Browse files
Gayathri Srividya RajavarapuGayathri Srividya Rajavarapu
authored andcommitted
fix: decode JSON string and support flat env-var properties for REST auth config
Fixes #3422. RestCatalog._create_session() expected auth to be a dict, but environment variables always produce string values. This caused auth initialization to fail for all pluggable auth types (basic, oauth2, google, entra, custom) when configured via PYICEBERG_CATALOG__<NAME>__AUTH. Two complementary fixes: 1. JSON string decode: if the 'auth' property is a string, JSON-parse it before processing. This supports: export PYICEBERG_CATALOG__REST__AUTH='{"type":"oauth2",...}' 2. Flat env-var property support: if 'auth' is absent but 'auth.type' is present, build the auth config from flat 'auth.*' properties. This is the canonical env-var style that avoids JSON-in-env quoting issues: export PYICEBERG_CATALOG__REST__AUTH__TYPE=oauth2 export PYICEBERG_CATALOG__REST__AUTH__OAUTH2__CLIENT_ID=id export PYICEBERG_CATALOG__REST__AUTH__OAUTH2__CLIENT_SECRET=secret export PYICEBERG_CATALOG__REST__AUTH__OAUTH2__TOKEN_URL=https://... Kebab-case keys (e.g. 'client-id') produced by the env-var parser are normalised to snake_case ('client_id') to match AuthManager constructor parameters. Regression tests added for both code paths (basic and oauth2) as well as invalid JSON detection.
1 parent d99e463 commit 5f79b47

2 files changed

Lines changed: 166 additions & 1 deletion

File tree

‎pyiceberg/catalog/rest/__init__.py‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
# under the License.
1717
from __future__ import annotations
1818

19+
import json
1920
from collections import deque
2021
from enum import Enum
2122
from typing import (
@@ -435,7 +436,16 @@ def _create_session(self) -> Session:
435436
elif ssl_client_cert := ssl_client.get(CERT):
436437
session.cert = ssl_client_cert
437438

438-
if auth_config := self.properties.get(AUTH):
439+
if raw_auth := self.properties.get(AUTH):
440+
# When auth is configured via an environment variable (e.g. PYICEBERG_CATALOG__<NAME>__AUTH),
441+
# the value arrives as a JSON string rather than a dict. Decode it before processing.
442+
if isinstance(raw_auth, str):
443+
try:
444+
auth_config: dict[str, Any] = json.loads(raw_auth)
445+
except json.JSONDecodeError as e:
446+
raise ValueError(f"Failed to parse auth configuration as JSON: {raw_auth!r}") from e
447+
else:
448+
auth_config = raw_auth
439449
auth_type = auth_config.get("type")
440450
if auth_type is None:
441451
raise ValueError("auth.type must be defined")
@@ -448,6 +458,29 @@ def _create_session(self) -> Session:
448458
if auth_type != CUSTOM and auth_impl:
449459
raise ValueError("auth.impl can only be specified when using custom auth.type")
450460

461+
self._auth_manager = AuthManagerFactory.create(auth_impl or auth_type, auth_type_config)
462+
session.auth = AuthManagerAdapter(self._auth_manager)
463+
elif auth_type := self.properties.get(f"{AUTH}.type"):
464+
# Support flattened env-var style configuration:
465+
# PYICEBERG_CATALOG__<NAME>__AUTH__TYPE=oauth2
466+
# PYICEBERG_CATALOG__<NAME>__AUTH__OAUTH2__CLIENT_ID=id
467+
# The env-var parser maps these to flat properties like "auth.type" and "auth.oauth2.client-id".
468+
# Key names are converted from kebab-case to snake_case to match AuthManager constructor parameters.
469+
auth_impl = self.properties.get(f"{AUTH}.impl")
470+
471+
if auth_type == CUSTOM and not auth_impl:
472+
raise ValueError("auth.impl must be specified when using custom auth.type")
473+
474+
if auth_type != CUSTOM and auth_impl:
475+
raise ValueError("auth.impl can only be specified when using custom auth.type")
476+
477+
type_prefix = f"{AUTH}.{auth_type}."
478+
auth_type_config = {
479+
k[len(type_prefix):].replace("-", "_"): v
480+
for k, v in self.properties.items()
481+
if k.startswith(type_prefix)
482+
}
483+
451484
self._auth_manager = AuthManagerFactory.create(auth_impl or auth_type, auth_type_config)
452485
session.auth = AuthManagerAdapter(self._auth_manager)
453486
else:

‎tests/catalog/test_rest.py‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3167,3 +3167,135 @@ def test_load_table_without_storage_credentials(
31673167
)
31683168
assert actual.metadata.model_dump() == expected.metadata.model_dump()
31693169
assert actual == expected
3170+
3171+
3172+
# Tests for issue #3422: REST catalog auth cannot be configured via environment
3173+
# variables unless auth JSON strings are decoded.
3174+
3175+
3176+
def test_rest_catalog_with_basic_auth_as_json_string(rest_mock: Mocker) -> None:
3177+
"""When auth arrives as a JSON string (e.g. from an environment variable), it should be decoded correctly."""
3178+
import json
3179+
3180+
rest_mock.get(
3181+
f"{TEST_URI}v1/config",
3182+
json={"defaults": {}, "overrides": {}},
3183+
status_code=200,
3184+
)
3185+
auth_dict = {
3186+
"type": "basic",
3187+
"basic": {
3188+
"username": "one",
3189+
"password": "two",
3190+
},
3191+
}
3192+
catalog_properties = {
3193+
"uri": TEST_URI,
3194+
"auth": json.dumps(auth_dict),
3195+
}
3196+
catalog = RestCatalog("rest", **catalog_properties) # type: ignore
3197+
assert catalog.uri == TEST_URI
3198+
3199+
encoded_user_pass = base64.b64encode(b"one:two").decode()
3200+
expected_auth_header = f"Basic {encoded_user_pass}"
3201+
assert rest_mock.last_request.headers["Authorization"] == expected_auth_header
3202+
3203+
3204+
def test_rest_catalog_with_oauth2_auth_as_json_string(requests_mock: Mocker) -> None:
3205+
"""OAuth2 auth configured as a JSON string (e.g. from an environment variable) should work correctly."""
3206+
import json
3207+
3208+
requests_mock.post(
3209+
f"{TEST_URI}oauth2/token",
3210+
json={
3211+
"access_token": "MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI3",
3212+
"token_type": "Bearer",
3213+
"expires_in": 3600,
3214+
},
3215+
status_code=200,
3216+
)
3217+
requests_mock.get(
3218+
f"{TEST_URI}v1/config",
3219+
json={"defaults": {}, "overrides": {}},
3220+
status_code=200,
3221+
)
3222+
auth_dict = {
3223+
"type": "oauth2",
3224+
"oauth2": {
3225+
"client_id": "some_client_id",
3226+
"client_secret": "some_client_secret",
3227+
"token_url": f"{TEST_URI}oauth2/token",
3228+
},
3229+
}
3230+
catalog_properties = {
3231+
"uri": TEST_URI,
3232+
"auth": json.dumps(auth_dict),
3233+
}
3234+
catalog = RestCatalog("rest", **catalog_properties) # type: ignore
3235+
assert catalog.uri == TEST_URI
3236+
3237+
3238+
def test_rest_catalog_with_invalid_json_auth_string() -> None:
3239+
"""An auth value that is a string but not valid JSON should raise a descriptive ValueError."""
3240+
with pytest.raises(ValueError, match="Failed to parse auth configuration as JSON"):
3241+
RestCatalog("rest", uri=TEST_URI, auth="not-valid-json") # type: ignore
3242+
3243+
3244+
def test_rest_catalog_with_basic_auth_flat_properties(rest_mock: Mocker) -> None:
3245+
"""Auth configured via flattened env-var properties (e.g. PYICEBERG_CATALOG__<NAME>__AUTH__TYPE=basic)
3246+
should initialise the correct AuthManager.
3247+
3248+
The env-var parser converts PYICEBERG_CATALOG__<NAME>__AUTH__TYPE=basic into the flat property
3249+
'auth.type' = 'basic' and PYICEBERG_CATALOG__<NAME>__AUTH__BASIC__USERNAME=one into
3250+
'auth.basic.username' = 'one'.
3251+
"""
3252+
rest_mock.get(
3253+
f"{TEST_URI}v1/config",
3254+
json={"defaults": {}, "overrides": {}},
3255+
status_code=200,
3256+
)
3257+
catalog_properties = {
3258+
"uri": TEST_URI,
3259+
# Flat properties as produced by the env-var config parser
3260+
"auth.type": "basic",
3261+
"auth.basic.username": "one",
3262+
"auth.basic.password": "two",
3263+
}
3264+
catalog = RestCatalog("rest", **catalog_properties) # type: ignore
3265+
assert catalog.uri == TEST_URI
3266+
3267+
encoded_user_pass = base64.b64encode(b"one:two").decode()
3268+
expected_auth_header = f"Basic {encoded_user_pass}"
3269+
assert rest_mock.last_request.headers["Authorization"] == expected_auth_header
3270+
3271+
3272+
def test_rest_catalog_with_oauth2_auth_flat_properties(requests_mock: Mocker) -> None:
3273+
"""OAuth2 auth configured via flattened env-var properties should work correctly.
3274+
3275+
PYICEBERG_CATALOG__<NAME>__AUTH__OAUTH2__CLIENT_ID maps to 'auth.oauth2.client-id'.
3276+
The dash is normalised to an underscore ('client_id') when forwarding to OAuth2AuthManager.
3277+
"""
3278+
requests_mock.post(
3279+
f"{TEST_URI}oauth2/token",
3280+
json={
3281+
"access_token": "MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI3",
3282+
"token_type": "Bearer",
3283+
"expires_in": 3600,
3284+
},
3285+
status_code=200,
3286+
)
3287+
requests_mock.get(
3288+
f"{TEST_URI}v1/config",
3289+
json={"defaults": {}, "overrides": {}},
3290+
status_code=200,
3291+
)
3292+
catalog_properties = {
3293+
"uri": TEST_URI,
3294+
# Flat properties as produced by the env-var config parser (note: kebab-case keys)
3295+
"auth.type": "oauth2",
3296+
"auth.oauth2.client-id": "some_client_id",
3297+
"auth.oauth2.client-secret": "some_client_secret",
3298+
"auth.oauth2.token-url": f"{TEST_URI}oauth2/token",
3299+
}
3300+
catalog = RestCatalog("rest", **catalog_properties) # type: ignore
3301+
assert catalog.uri == TEST_URI

0 commit comments

Comments
 (0)