diff --git a/src/openai/__init__.py b/src/openai/__init__.py index f59b47bb71..eb5ad4cf0f 100644 --- a/src/openai/__init__.py +++ b/src/openai/__init__.py @@ -188,6 +188,24 @@ def api_key(self, value: str | None) -> None: # type: ignore api_key = value + @property # type: ignore + @override + def _api_key_explicitly_empty(self) -> bool: + # Unlike the regular client, the module client's `api_key` can be reassigned + # at any time via the `openai.api_key = ...` module attribute, after the + # underlying client instance was already constructed. So rather than relying + # on the flag captured once at construction time, check the current value + # directly to decide whether auth was intentionally disabled. + return api_key == "" + + @_api_key_explicitly_empty.setter # type: ignore + def _api_key_explicitly_empty(self, value: bool) -> None: # type: ignore + # No-op: the getter above always derives this from the live `api_key`, so + # there is nothing to store. The setter only needs to exist so that + # `OpenAI.__init__` can assign to `self._api_key_explicitly_empty` without + # raising `AttributeError`. + pass + @property # type: ignore @override def admin_api_key(self) -> str | None: diff --git a/src/openai/_base_client.py b/src/openai/_base_client.py index 10d7b9f7ca..2008ce3c0a 100644 --- a/src/openai/_base_client.py +++ b/src/openai/_base_client.py @@ -468,7 +468,7 @@ def _custom_auth( def _build_headers(self, options: FinalRequestOptions, *, retries_taken: int = 0) -> httpx.Headers: custom_headers = options.headers or {} headers_dict = _merge_mappings({**self._auth_headers(options.security), **self.default_headers}, custom_headers) - self._validate_headers(headers_dict, custom_headers) + self._validate_headers(headers_dict, custom_headers, options.security) # headers are case-insensitive while dictionaries are not. headers = httpx.Headers(headers_dict) @@ -731,6 +731,7 @@ def _validate_headers( self, headers: Headers, # noqa: ARG002 custom_headers: Headers, # noqa: ARG002 + security: SecurityOptions | None = None, # noqa: ARG002 ) -> None: """Validate the given default headers and custom headers. diff --git a/src/openai/_client.py b/src/openai/_client.py index d7ca675b4d..187e3f4d43 100644 --- a/src/openai/_client.py +++ b/src/openai/_client.py @@ -197,6 +197,14 @@ def __init__( self.workload_identity = workload_identity if provider_runtime is None else None + _api_key_explicitly_set = api_key is not None + # Tracks whether the caller explicitly passed a literal `api_key=""`, as opposed + # to it defaulting to an empty string because no credentials were configured, or + # a key provider/workload identity being used (which resolve the real key later). + # This is needed so that requests don't fail header validation below when the + # caller intentionally disabled authentication (e.g. for local, auth-less + # OpenAI-compatible servers). + self._api_key_explicitly_empty = api_key == "" if provider_runtime is not None: self.api_key = "" self._api_key_provider = None @@ -224,6 +232,7 @@ def __init__( provider_runtime is None and _enforce_credentials and not self.api_key + and not _api_key_explicitly_set and self._api_key_provider is None and workload_identity is None and self.admin_api_key is None @@ -544,13 +553,25 @@ def default_headers(self) -> dict[str, str | Omit]: } @override - def _validate_headers(self, headers: Headers, custom_headers: Headers) -> None: + def _validate_headers( + self, headers: Headers, custom_headers: Headers, security: SecurityOptions | None = None + ) -> None: if self._provider_runtime is not None: return if _has_header(headers, "Authorization") or _has_omitted_header(custom_headers, "Authorization"): return + # An explicitly-passed `api_key=""` means the caller intentionally disabled + # authentication (e.g. for a local, auth-less OpenAI-compatible server), so + # don't fail requests just because no `Authorization` header could be built — + # as long as bearer auth (the auth method an empty `api_key` disables) is one + # of the accepted security methods for this request. Endpoints that *only* + # accept admin credentials (no `bearer_auth` alternative) still require them, + # since an empty `api_key` can never satisfy those. + if self._api_key_explicitly_empty and (security or {}).get("bearer_auth", False): + return + raise TypeError( '"Could not resolve authentication method. Expected either api_key or admin_api_key to be set. Or for one of the `Authorization` or `Authorization` headers to be explicitly omitted"' ) @@ -651,7 +672,12 @@ def copy( } else: auth_options = { - "api_key": api_key or self._api_key_provider or self.api_key, + # `api_key` defaults to `None`, meaning "not overridden, inherit from + # `self`" — but an explicitly-passed `api_key=""` (used to disable auth + # for local, auth-less servers) must still be honored rather than falling + # through to the inherited provider/key, so this checks `is not None` + # rather than truthiness. + "api_key": api_key if api_key is not None else (self._api_key_provider or self.api_key), "admin_api_key": admin_api_key or self.admin_api_key, "workload_identity": workload_identity or self.workload_identity, "base_url": base_url or self.base_url, @@ -803,6 +829,14 @@ def __init__( self.workload_identity = workload_identity if provider_runtime is None else None + _api_key_explicitly_set = api_key is not None + # Tracks whether the caller explicitly passed a literal `api_key=""`, as opposed + # to it defaulting to an empty string because no credentials were configured, or + # a key provider/workload identity being used (which resolve the real key later). + # This is needed so that requests don't fail header validation below when the + # caller intentionally disabled authentication (e.g. for local, auth-less + # OpenAI-compatible servers). + self._api_key_explicitly_empty = api_key == "" if provider_runtime is not None: self.api_key = "" self._api_key_provider = None @@ -830,6 +864,7 @@ def __init__( provider_runtime is None and _enforce_credentials and not self.api_key + and not _api_key_explicitly_set and self._api_key_provider is None and workload_identity is None and self.admin_api_key is None @@ -1153,13 +1188,25 @@ def default_headers(self) -> dict[str, str | Omit]: } @override - def _validate_headers(self, headers: Headers, custom_headers: Headers) -> None: + def _validate_headers( + self, headers: Headers, custom_headers: Headers, security: SecurityOptions | None = None + ) -> None: if self._provider_runtime is not None: return if _has_header(headers, "Authorization") or _has_omitted_header(custom_headers, "Authorization"): return + # An explicitly-passed `api_key=""` means the caller intentionally disabled + # authentication (e.g. for a local, auth-less OpenAI-compatible server), so + # don't fail requests just because no `Authorization` header could be built — + # as long as bearer auth (the auth method an empty `api_key` disables) is one + # of the accepted security methods for this request. Endpoints that *only* + # accept admin credentials (no `bearer_auth` alternative) still require them, + # since an empty `api_key` can never satisfy those. + if self._api_key_explicitly_empty and (security or {}).get("bearer_auth", False): + return + raise TypeError( '"Could not resolve authentication method. Expected either api_key or admin_api_key to be set. Or for one of the `Authorization` or `Authorization` headers to be explicitly omitted"' ) @@ -1267,7 +1314,12 @@ def copy( } else: auth_options = { - "api_key": api_key or self._api_key_provider or self.api_key, + # `api_key` defaults to `None`, meaning "not overridden, inherit from + # `self`" — but an explicitly-passed `api_key=""` (used to disable auth + # for local, auth-less servers) must still be honored rather than falling + # through to the inherited provider/key, so this checks `is not None` + # rather than truthiness. + "api_key": api_key if api_key is not None else (self._api_key_provider or self.api_key), "admin_api_key": admin_api_key or self.admin_api_key, "workload_identity": workload_identity or self.workload_identity, "base_url": base_url or self.base_url, diff --git a/src/openai/lib/azure.py b/src/openai/lib/azure.py index 4ebe0a98aa..e0cd9c1e54 100644 --- a/src/openai/lib/azure.py +++ b/src/openai/lib/azure.py @@ -363,7 +363,12 @@ def _auth_headers(self, security: SecurityOptions) -> dict[str, str]: # noqa: A return {} @override - def _validate_headers(self, headers: Headers, custom_headers: Headers) -> None: + def _validate_headers( + self, + headers: Headers, + custom_headers: Headers, + security: SecurityOptions | None = None, # noqa: ARG002 + ) -> None: if _has_auth_header(headers) or _has_auth_header(custom_headers): return @@ -689,7 +694,12 @@ def _auth_headers(self, security: SecurityOptions) -> dict[str, str]: # noqa: A return {} @override - def _validate_headers(self, headers: Headers, custom_headers: Headers) -> None: + def _validate_headers( + self, + headers: Headers, + custom_headers: Headers, + security: SecurityOptions | None = None, # noqa: ARG002 + ) -> None: if _has_auth_header(headers) or _has_auth_header(custom_headers): return diff --git a/tests/test_client.py b/tests/test_client.py index 33a5b1c224..0f4b1ad8e0 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -524,6 +524,84 @@ def test_validate_headers(self) -> None: with pytest.raises(OpenAIError, match="Missing credentials"): OpenAI(base_url=base_url, api_key=None, admin_api_key=None, _strict_response_validation=True) + # Explicitly passing api_key="" should not raise, even with _enforce_credentials=True. + # This is important for OpenAI-compatible local servers that don't require authentication. + with update_env( + **{ + "OPENAI_API_KEY": Omit(), + "OPENAI_ADMIN_KEY": Omit(), + } + ): + client = OpenAI( + base_url=base_url, + api_key="", + admin_api_key=None, + _strict_response_validation=True, + ) + assert client.api_key == "" + + # Requests should also succeed, not just client construction: no `Authorization` + # header should be required or added when api_key was explicitly set to "". + request = client._build_request( + FinalRequestOptions(method="get", url="/foo", security={"bearer_auth": True}) + ) + assert "Authorization" not in request.headers + + # An explicit empty api_key should not bypass validation for endpoints that + # require credentials the client doesn't have (e.g. admin-only endpoints). + with pytest.raises(TypeError, match="Could not resolve authentication method"): + client._build_request( + FinalRequestOptions( + method="get", + url="/organization/projects", + security={"admin_api_key_auth": True}, + ) + ) + + # Raw request helpers (`client.get(...)`, `.post(...)`, etc.) don't pass an + # explicit `security` override, so they fall back to `FinalRequestOptions`'s + # default of requiring *either* bearer or admin auth. Since bearer auth is + # one of the accepted methods here, the empty `api_key` should still bypass + # validation instead of being treated as admin-only. + default_security_request = client._build_request(FinalRequestOptions(method="get", url="/foo")) + assert "Authorization" not in default_security_request.headers + + # `copy()`/`with_options()` should also honor an explicitly-passed empty + # `api_key`, rather than silently falling back to the inherited key. + copied_client = client.copy(api_key="") + assert copied_client.api_key == "" + copied_request = copied_client._build_request( + FinalRequestOptions(method="get", url="/foo", security={"bearer_auth": True}) + ) + assert "Authorization" not in copied_request.headers + + # OPENAI_API_KEY="" in the environment (without explicit api_key arg) should still raise, + # as an empty env var likely indicates misconfiguration rather than intentional use. + with update_env( + **{ + "OPENAI_API_KEY": "", + "OPENAI_ADMIN_KEY": Omit(), + } + ): + with pytest.raises(OpenAIError, match="Missing credentials"): + OpenAI(base_url=base_url, admin_api_key=None, _strict_response_validation=True) + + def test_with_options_preserves_explicit_empty_api_key(self) -> None: + # `client.with_options(api_key="")` should disable auth on the copy, rather than + # inheriting the original client's non-empty `api_key` (a plain `or` fallback + # would treat the explicit `""` as "not provided" and keep the old key). + client = OpenAI(base_url=base_url, api_key=api_key, admin_api_key=None, _strict_response_validation=True) + + copied = client.with_options(api_key="") + assert copied.api_key == "" + + request = copied._build_request(FinalRequestOptions(method="get", url="/foo", security={"bearer_auth": True})) + assert "Authorization" not in request.headers + + # With no override, `with_options()` should still inherit the original api_key. + inherited = client.with_options(timeout=5) + assert inherited.api_key == api_key + @pytest.mark.respx(base_url=base_url) def test_api_key_provider_preserves_admin_auth(self, respx_mock: MockRouter) -> None: respx_mock.get("/organization/projects").mock(return_value=httpx.Response(200, json={"ok": True})) @@ -1837,6 +1915,84 @@ async def test_validate_headers(self) -> None: with pytest.raises(OpenAIError, match="Missing credentials"): AsyncOpenAI(base_url=base_url, api_key=None, admin_api_key=None, _strict_response_validation=True) + # Explicitly passing api_key="" should not raise, even with _enforce_credentials=True. + # This is important for OpenAI-compatible local servers that don't require authentication. + with update_env( + **{ + "OPENAI_API_KEY": Omit(), + "OPENAI_ADMIN_KEY": Omit(), + } + ): + client = AsyncOpenAI( + base_url=base_url, + api_key="", + admin_api_key=None, + _strict_response_validation=True, + ) + assert client.api_key == "" + + # Requests should also succeed, not just client construction: no `Authorization` + # header should be required or added when api_key was explicitly set to "". + request = client._build_request( + FinalRequestOptions(method="get", url="/foo", security={"bearer_auth": True}) + ) + assert "Authorization" not in request.headers + + # An explicit empty api_key should not bypass validation for endpoints that + # require credentials the client doesn't have (e.g. admin-only endpoints). + with pytest.raises(TypeError, match="Could not resolve authentication method"): + client._build_request( + FinalRequestOptions( + method="get", + url="/organization/projects", + security={"admin_api_key_auth": True}, + ) + ) + + # Raw request helpers (`client.get(...)`, `.post(...)`, etc.) don't pass an + # explicit `security` override, so they fall back to `FinalRequestOptions`'s + # default of requiring *either* bearer or admin auth. Since bearer auth is + # one of the accepted methods here, the empty `api_key` should still bypass + # validation instead of being treated as admin-only. + default_security_request = client._build_request(FinalRequestOptions(method="get", url="/foo")) + assert "Authorization" not in default_security_request.headers + + # `copy()`/`with_options()` should also honor an explicitly-passed empty + # `api_key`, rather than silently falling back to the inherited key. + copied_client = client.copy(api_key="") + assert copied_client.api_key == "" + copied_request = copied_client._build_request( + FinalRequestOptions(method="get", url="/foo", security={"bearer_auth": True}) + ) + assert "Authorization" not in copied_request.headers + + # OPENAI_API_KEY="" in the environment (without explicit api_key arg) should still raise, + # as an empty env var likely indicates misconfiguration rather than intentional use. + with update_env( + **{ + "OPENAI_API_KEY": "", + "OPENAI_ADMIN_KEY": Omit(), + } + ): + with pytest.raises(OpenAIError, match="Missing credentials"): + AsyncOpenAI(base_url=base_url, admin_api_key=None, _strict_response_validation=True) + + def test_with_options_preserves_explicit_empty_api_key(self) -> None: + # `client.with_options(api_key="")` should disable auth on the copy, rather than + # inheriting the original client's non-empty `api_key` (a plain `or` fallback + # would treat the explicit `""` as "not provided" and keep the old key). + client = AsyncOpenAI(base_url=base_url, api_key=api_key, admin_api_key=None, _strict_response_validation=True) + + copied = client.with_options(api_key="") + assert copied.api_key == "" + + request = copied._build_request(FinalRequestOptions(method="get", url="/foo", security={"bearer_auth": True})) + assert "Authorization" not in request.headers + + # With no override, `with_options()` should still inherit the original api_key. + inherited = client.with_options(timeout=5) + assert inherited.api_key == api_key + @pytest.mark.respx(base_url=base_url) async def test_api_key_provider_preserves_admin_auth(self, respx_mock: MockRouter) -> None: respx_mock.get("/organization/projects").mock(return_value=httpx.Response(200, json={"ok": True})) diff --git a/tests/test_module_client.py b/tests/test_module_client.py index cb509d3d19..b8d2fa0368 100644 --- a/tests/test_module_client.py +++ b/tests/test_module_client.py @@ -10,6 +10,7 @@ import openai from openai import DEFAULT_TIMEOUT, DEFAULT_MAX_RETRIES +from openai._models import FinalRequestOptions def reset_state() -> None: @@ -100,6 +101,24 @@ def test_http_client_option() -> None: assert openai.completions._client._client is new_client +def test_module_api_key_set_empty_after_load_bypasses_auth() -> None: + # The module client is constructed lazily on first access, capturing whatever + # `openai.api_key` was set to at that time. If the caller mutates `openai.api_key` + # afterwards, the already-constructed client must still pick up the new value — + # including switching to (or away from) the "explicitly disabled auth" state that + # an empty string represents, not just the plain `api_key` value. + openai.api_key = "real-key" + + client = openai.completions._client + assert client.api_key == "real-key" + + openai.api_key = "" + + assert client.api_key == "" + request = client._build_request(FinalRequestOptions(method="get", url="/foo", security={"bearer_auth": True})) + assert "Authorization" not in request.headers + + import contextlib from typing import Iterator