-
Notifications
You must be signed in to change notification settings - Fork 1
fix: OAuth2 token encoding and /auth/info route #49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
4a882b5
2185149
f1547b1
526553d
343b88f
0831ab7
9e3d19c
3306569
5fd3be4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,20 @@ class SecretStoreError(Exception): | |
| pass | ||
|
|
||
|
|
||
| def _is_missing_path_error(exc: BaseException) -> bool: | ||
| """Heuristically decide whether an exception means 'key does not exist'. | ||
|
|
||
| Matches hvac's InvalidPath (checked by class name so hvac stays an optional | ||
| dependency) plus common HTTP-404 style messages. Anything else — auth | ||
| failures, connection errors, server errors — is NOT a missing key and must | ||
| surface to the caller instead of being silently swallowed. | ||
| """ | ||
| if type(exc).__name__ == "InvalidPath": | ||
| return True | ||
| message = str(exc).lower() | ||
| return "404" in message or "not found" in message or "path" in message and "missing" in message | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a real Vault failure contains Useful? React with 👍 / 👎. |
||
|
|
||
|
|
||
| class SecretStore(ABC): | ||
| """Abstract base class for secret store integrations.""" | ||
|
|
||
|
|
@@ -210,8 +224,10 @@ def get(self, key: str) -> str | None: | |
| ) | ||
| data = response.get("data", {}).get("data", {}) | ||
| return data.get("value") | ||
| except Exception: | ||
| return None | ||
| except Exception as exc: | ||
| if _is_missing_path_error(exc): | ||
| return None | ||
| raise SecretStoreError(f"Vault read failed for {key!r}: {exc}") from exc | ||
|
|
||
| def set(self, key: str, value: str) -> bool: | ||
| client = self._get_client() | ||
|
|
@@ -230,8 +246,10 @@ def delete(self, key: str) -> bool: | |
| mount_point=self.mount_point, | ||
| ) | ||
| return True | ||
| except Exception: | ||
| return False | ||
| except Exception as exc: | ||
| if _is_missing_path_error(exc): | ||
| return False | ||
| raise SecretStoreError(f"Vault delete failed for {key!r}: {exc}") from exc | ||
|
|
||
| def list_keys(self, prefix: str = "") -> list[str]: | ||
| client = self._get_client() | ||
|
|
@@ -242,8 +260,10 @@ def list_keys(self, prefix: str = "") -> list[str]: | |
| mount_point=self.mount_point, | ||
| ) | ||
| return response.get("data", {}).get("keys", []) | ||
| except Exception: | ||
| return [] | ||
| except Exception as exc: | ||
| if _is_missing_path_error(exc): | ||
| return [] | ||
| raise SecretStoreError(f"Vault list failed at {list_path!r}: {exc}") from exc | ||
|
|
||
|
|
||
| class DopplerStore(SecretStore): | ||
|
|
@@ -279,6 +299,26 @@ def get(self, key: str) -> str | None: | |
| return secrets[key].get("raw", "").strip() or secrets[key].get("computed", "").strip() | ||
| return None | ||
|
|
||
| def get_many(self, keys: list[str]) -> dict[str, str]: | ||
| """Batch fetch: Doppler returns the whole config's secrets per request, | ||
| so one request serves all keys instead of one request per key.""" | ||
| import requests | ||
|
|
||
| url = f"{self._base_url}/configs/config/secrets" | ||
| params = {"project": self.project, "config": self.config} | ||
| resp = requests.get(url, headers=self._headers(), params=params, timeout=10) | ||
| if resp.status_code != 200: | ||
| return {} | ||
| data = resp.json() | ||
| secrets = data.get("secrets", {}) | ||
| result: dict[str, str] = {} | ||
| for key in keys: | ||
| if key in secrets: | ||
| value = secrets[key].get("raw", "").strip() or secrets[key].get("computed", "").strip() | ||
| if value: | ||
| result[key] = value | ||
| return result | ||
|
|
||
| def set(self, key: str, value: str) -> bool: | ||
| import requests | ||
|
|
||
|
|
@@ -346,7 +386,10 @@ def _api_post(self, path: str, data: dict) -> bool: | |
| return resp.status_code in (200, 201) | ||
|
|
||
| def get(self, key: str) -> str | None: | ||
| items = self._api_get(f"/v1/vaults/{self.vault_id}/items?filter=title%20eq%20%22{key}%22") | ||
| from urllib.parse import quote | ||
|
|
||
| encoded_key = quote(key, safe="") | ||
| items = self._api_get(f"/v1/vaults/{self.vault_id}/items?filter=title%20eq%20%22{encoded_key}%22") | ||
|
Coding-Dev-Tools marked this conversation as resolved.
|
||
| if not items: | ||
| return None | ||
| item_list = items if isinstance(items, list) else items.get("items", []) | ||
|
|
@@ -370,9 +413,12 @@ def set(self, key: str, value: str) -> bool: | |
| return self._api_post(f"/v1/vaults/{self.vault_id}/items", payload) | ||
|
|
||
| def delete(self, key: str) -> bool: | ||
| from urllib.parse import quote | ||
|
|
||
| import requests | ||
|
|
||
| items = self._api_get(f"/v1/vaults/{self.vault_id}/items?filter=title%20eq%20%22{key}%22") | ||
| encoded_key = quote(key, safe="") | ||
| items = self._api_get(f"/v1/vaults/{self.vault_id}/items?filter=title%20eq%20%22{encoded_key}%22") | ||
| if not items: | ||
| return False | ||
| item_list = items if isinstance(items, list) else items.get("items", []) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import json | ||
|
|
||
| from envault.auth import OAuth2Auth | ||
|
|
||
|
|
||
| def test_oauth2_introspection_url_encodes_reserved_token_characters(monkeypatch): | ||
| captured: dict[str, object] = {} | ||
|
|
||
| class _Response: | ||
| status = 200 | ||
|
|
||
| def __enter__(self): | ||
| return self | ||
|
|
||
| def __exit__(self, exc_type, exc_value, traceback): | ||
| return False | ||
|
|
||
| def read(self): | ||
| return json.dumps({"active": True, "sub": "synthetic-user"}).encode() | ||
|
|
||
| def fake_urlopen(request, timeout): | ||
| captured["request"] = request | ||
| captured["timeout"] = timeout | ||
| return _Response() | ||
|
|
||
| monkeypatch.setattr("envault.auth.urlopen", fake_urlopen) | ||
|
|
||
| result = OAuth2Auth(provider_url="https://identity.example", strategy="introspect").check( | ||
| {"Authorization": "Bearer token+with&reserved=value"} | ||
| ) | ||
|
|
||
| assert result.success | ||
| request = captured["request"] | ||
| assert request.data == b"token=token%2Bwith%26reserved%3Dvalue" | ||
| assert captured["timeout"] == 10 |
Uh oh!
There was an error while loading. Please reload this page.