Skip to content

Commit 9c59d67

Browse files
committed
remove oauth2manager to separate PR
1 parent ab75641 commit 9c59d67

File tree

3 files changed

+39
-155
lines changed

3 files changed

+39
-155
lines changed

mkdocs/docs/configuration.md

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,6 @@ The RESTCatalog supports pluggable authentication via the `auth` configuration b
382382

383383
- `noop`: No authentication (no Authorization header sent).
384384
- `basic`: HTTP Basic authentication.
385-
- `oauth2`: OAuth2 client credentials flow.
386385
- `legacyoauth2`: Legacy OAuth2 client credentials flow (Deprecated and will be removed in PyIceberg 1.0.0)
387386
- `custom`: Custom authentication manager (requires `auth.impl`).
388387

@@ -406,10 +405,9 @@ catalog:
406405

407406
| Property | Required | Description |
408407
|------------------|----------|-------------------------------------------------------------------------------------------------|
409-
| `auth.type` | Yes | The authentication type to use (`noop`, `basic`, `oauth2`, or `custom`). |
408+
| `auth.type` | Yes | The authentication type to use (`noop`, `basic`, or `custom`). |
410409
| `auth.impl` | Conditionally | The fully qualified class path for a custom AuthManager. Required if `auth.type` is `custom`. |
411410
| `auth.basic` | If type is `basic` | Block containing `username` and `password` for HTTP Basic authentication. |
412-
| `auth.oauth2` | If type is `oauth2` | Block containing OAuth2 configuration (see below). |
413411
| `auth.custom` | If type is `custom` | Block containing configuration for the custom AuthManager. |
414412

415413
##### Examples
@@ -431,20 +429,6 @@ auth:
431429
password: mypass
432430
```
433431

434-
**OAuth2 Authentication:**
435-
436-
```yaml
437-
auth:
438-
type: oauth2
439-
oauth2:
440-
client_id: my-client-id
441-
client_secret: my-client-secret
442-
token_url: https://auth.example.com/oauth/token
443-
scope: read
444-
refresh_margin: 60 # (optional) seconds before expiry to refresh
445-
expires_in: 3600 # (optional) fallback if server does not provide
446-
```
447-
448432
**Custom Authentication:**
449433

450434
```yaml
@@ -460,7 +444,7 @@ auth:
460444

461445
- If `auth.type` is `custom`, you **must** specify `auth.impl` with the full class path to your custom AuthManager.
462446
- If `auth.type` is not `custom`, specifying `auth.impl` is not allowed.
463-
- The configuration block under each type (e.g., `basic`, `oauth2`, `custom`) is passed as keyword arguments to the corresponding AuthManager.
447+
- The configuration block under each type (e.g., `basic`, `custom`) is passed as keyword arguments to the corresponding AuthManager.
464448

465449
### SQL Catalog
466450

pyiceberg/catalog/rest/auth.py

Lines changed: 0 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,9 @@
1717

1818
import base64
1919
import importlib
20-
import threading
21-
import time
2220
from abc import ABC, abstractmethod
2321
from typing import Any, Dict, Optional, Type
2422

25-
import requests
2623
from requests import HTTPError, PreparedRequest, Session
2724
from requests.auth import AuthBase
2825

@@ -122,95 +119,6 @@ def auth_header(self) -> str:
122119
return f"Bearer {self._token}"
123120

124121

125-
class OAuth2TokenProvider:
126-
"""Thread-safe OAuth2 token provider with token refresh support."""
127-
128-
client_id: str
129-
client_secret: str
130-
token_url: str
131-
scope: Optional[str]
132-
refresh_margin: int
133-
expires_in: Optional[int]
134-
135-
_token: Optional[str]
136-
_expires_at: int
137-
_lock: threading.Lock
138-
139-
def __init__(
140-
self,
141-
client_id: str,
142-
client_secret: str,
143-
token_url: str,
144-
scope: Optional[str] = None,
145-
refresh_margin: int = 60,
146-
expires_in: Optional[int] = None,
147-
):
148-
self.client_id = client_id
149-
self.client_secret = client_secret
150-
self.token_url = token_url
151-
self.scope = scope
152-
self.refresh_margin = refresh_margin
153-
self.expires_in = expires_in
154-
155-
self._token = None
156-
self._expires_at = 0
157-
self._lock = threading.Lock()
158-
159-
def _refresh_token(self) -> None:
160-
data = {
161-
"grant_type": "client_credentials",
162-
"client_id": self.client_id,
163-
"client_secret": self.client_secret,
164-
}
165-
if self.scope:
166-
data["scope"] = self.scope
167-
168-
response = requests.post(self.token_url, data=data)
169-
response.raise_for_status()
170-
result = response.json()
171-
172-
self._token = result["access_token"]
173-
expires_in = result.get("expires_in", self.expires_in)
174-
if expires_in is None:
175-
raise ValueError(
176-
"The expiration time of the Token must be provided by the Server in the Access Token Response in `expired_in` field, or by the PyIceberg Client."
177-
)
178-
self._expires_at = time.time() + expires_in - self.refresh_margin
179-
180-
def get_token(self) -> str:
181-
with self._lock:
182-
if not self._token or time.time() >= self._expires_at:
183-
self._refresh_token()
184-
if self._token is None:
185-
raise ValueError("Authorization token is None after refresh")
186-
return self._token
187-
188-
189-
class OAuth2AuthManager(AuthManager):
190-
"""Auth Manager implementation that supports OAuth2 as defined in IETF RFC6749."""
191-
192-
def __init__(
193-
self,
194-
client_id: str,
195-
client_secret: str,
196-
token_url: str,
197-
scope: Optional[str] = None,
198-
refresh_margin: int = 60,
199-
expires_in: Optional[int] = None,
200-
):
201-
self.token_provider = OAuth2TokenProvider(
202-
client_id,
203-
client_secret,
204-
token_url,
205-
scope,
206-
refresh_margin,
207-
expires_in,
208-
)
209-
210-
def auth_header(self) -> str:
211-
return f"Bearer {self.token_provider.get_token()}"
212-
213-
214122
class AuthManagerAdapter(AuthBase):
215123
"""A `requests.auth.AuthBase` adapter that integrates an `AuthManager` into a `requests.Session` to automatically attach the appropriate Authorization header to every request.
216124
@@ -289,4 +197,3 @@ def create(cls, class_or_name: str, config: Dict[str, Any]) -> AuthManager:
289197
AuthManagerFactory.register("noop", NoopAuthManager)
290198
AuthManagerFactory.register("basic", BasicAuthManager)
291199
AuthManagerFactory.register("legacyoauth2", LegacyOAuth2AuthManager)
292-
AuthManagerFactory.register("oauth2", OAuth2AuthManager)

tests/catalog/test_rest.py

Lines changed: 37 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1519,21 +1519,26 @@ def test_request_session_with_ssl_client_cert() -> None:
15191519
assert "Could not find the TLS certificate file, invalid path: path_to_client_cert" in str(e.value)
15201520

15211521

1522-
def test_rest_catalog_with_basic_auth_type() -> None:
1522+
def test_rest_catalog_with_basic_auth_type(rest_mock: Mocker) -> None:
1523+
# Given
1524+
rest_mock.get(
1525+
f"{TEST_URI}v1/config",
1526+
json={"defaults": {}, "overrides": {}},
1527+
status_code=200,
1528+
)
15231529
# Given
15241530
catalog_properties = {
15251531
"uri": TEST_URI,
15261532
"auth": {
15271533
"type": "basic",
15281534
"basic": {
15291535
"username": "one",
1536+
"password": "two",
15301537
},
15311538
},
15321539
}
1533-
with pytest.raises(TypeError) as e:
1534-
# Missing namespace
1535-
RestCatalog("rest", **catalog_properties) # type: ignore
1536-
assert "__init__() missing 1 required positional argument: 'password'" in str(e.value)
1540+
catalog = RestCatalog("rest", **catalog_properties) # type: ignore
1541+
assert catalog.uri == TEST_URI
15371542

15381543

15391544
def test_rest_catalog_with_custom_auth_type() -> None:
@@ -1555,6 +1560,28 @@ def test_rest_catalog_with_custom_auth_type() -> None:
15551560
assert "Could not load AuthManager class for 'dummy.nonexistent.package'" in str(e.value)
15561561

15571562

1563+
def test_rest_catalog_with_custom_basic_auth_type(rest_mock: Mocker) -> None:
1564+
# Given
1565+
catalog_properties = {
1566+
"uri": TEST_URI,
1567+
"auth": {
1568+
"type": "custom",
1569+
"impl": "pyiceberg.catalog.rest.auth.BasicAuthManager",
1570+
"custom": {
1571+
"username": "one",
1572+
"password": "two",
1573+
},
1574+
},
1575+
}
1576+
rest_mock.get(
1577+
f"{TEST_URI}v1/config",
1578+
json={"defaults": {}, "overrides": {}},
1579+
status_code=200,
1580+
)
1581+
catalog = RestCatalog("rest", **catalog_properties) # type: ignore
1582+
assert catalog.uri == TEST_URI
1583+
1584+
15581585
def test_rest_catalog_with_custom_auth_type_no_impl() -> None:
15591586
# Given
15601587
catalog_properties = {
@@ -1578,11 +1605,11 @@ def test_rest_catalog_with_non_custom_auth_type_impl() -> None:
15781605
catalog_properties = {
15791606
"uri": TEST_URI,
15801607
"auth": {
1581-
"type": "oauth2",
1582-
"impl": "oauth2.package",
1583-
"oauth2": {
1584-
"property1": "one",
1585-
"property2": "two",
1608+
"type": "basic",
1609+
"impl": "basic.package",
1610+
"basic": {
1611+
"username": "one",
1612+
"password": "two",
15861613
},
15871614
},
15881615
}
@@ -1610,40 +1637,6 @@ def test_rest_catalog_with_unsupported_auth_type() -> None:
16101637
assert "Could not load AuthManager class for 'unsupported'" in str(e.value)
16111638

16121639

1613-
def test_rest_catalog_with_oauth2_auth_type(requests_mock: Mocker) -> None:
1614-
requests_mock.post(
1615-
f"{TEST_URI}oauth2/token",
1616-
json={
1617-
"access_token": "MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI3",
1618-
"token_type": "Bearer",
1619-
"expires_in": 3600,
1620-
"refresh_token": "IwOGYzYTlmM2YxOTQ5MGE3YmNmMDFkNTVk",
1621-
"scope": "read",
1622-
},
1623-
status_code=200,
1624-
)
1625-
requests_mock.get(
1626-
f"{TEST_URI}v1/config",
1627-
json={"defaults": {}, "overrides": {}},
1628-
status_code=200,
1629-
)
1630-
# Given
1631-
catalog_properties = {
1632-
"uri": TEST_URI,
1633-
"auth": {
1634-
"type": "oauth2",
1635-
"oauth2": {
1636-
"client_id": "some_client_id",
1637-
"client_secret": "some_client_secret",
1638-
"token_url": f"{TEST_URI}oauth2/token",
1639-
"scope": "read",
1640-
},
1641-
},
1642-
}
1643-
catalog = RestCatalog("rest", **catalog_properties) # type: ignore
1644-
assert catalog.uri == TEST_URI
1645-
1646-
16471640
EXAMPLE_ENV = {"PYICEBERG_CATALOG__PRODUCTION__URI": TEST_URI}
16481641

16491642

0 commit comments

Comments
 (0)