-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix #3845 #3847
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: main
Are you sure you want to change the base?
fix #3845 #3847
Changes from all commits
006d547
b6cb922
5e7effb
a4fb713
f339c50
752d42d
4636a5b
9e8d85d
428cd57
facc6f4
962dcde
2e01565
65c40e5
adb85b7
63830a6
d6a049d
ee9b052
34ae26a
672f65c
63e3691
a9ea483
be56163
6ed98e0
363c2e1
e1764f6
59307d4
fbec565
f67bbcd
ae949af
d09ccf9
e610dbc
35c1760
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,13 +14,13 @@ | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| from __future__ import annotations | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| import contextlib | ||||||||||||||||||||||
| import logging | ||||||||||||||||||||||
| from typing import Optional | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| from fastapi.openapi.models import OAuth2 | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| from ..agents.callback_context import CallbackContext | ||||||||||||||||||||||
| from ..tools.openapi_tool.auth.credential_exchangers.service_account_exchanger import ServiceAccountCredentialExchanger | ||||||||||||||||||||||
| from ..utils.feature_decorator import experimental | ||||||||||||||||||||||
| from .auth_credential import AuthCredential | ||||||||||||||||||||||
| from .auth_credential import AuthCredentialTypes | ||||||||||||||||||||||
|
|
@@ -76,11 +76,23 @@ class CredentialManager: | |||||||||||||||||||||
| ``` | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # A map to store client secrets in memory. Key is client_id, value is client_secret | ||||||||||||||||||||||
| _CLIENT_SECRETS: dict[str, str] = {} | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def __init__( | ||||||||||||||||||||||
| self, | ||||||||||||||||||||||
| auth_config: AuthConfig, | ||||||||||||||||||||||
| ): | ||||||||||||||||||||||
| self._auth_config = auth_config | ||||||||||||||||||||||
| # We deep copy the auth_config to avoid modifying the original object passed | ||||||||||||||||||||||
| # by the user. This allows for safe redaction of sensitive information without | ||||||||||||||||||||||
| # causing side effects. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| self._auth_config = auth_config.model_copy(deep=True) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Secure the client secret | ||||||||||||||||||||||
| self._secure_client_secret(self._auth_config.raw_auth_credential) | ||||||||||||||||||||||
| self._secure_client_secret(self._auth_config.exchanged_auth_credential) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| self._exchanger_registry = CredentialExchangerRegistry() | ||||||||||||||||||||||
| self._refresher_registry = CredentialRefresherRegistry() | ||||||||||||||||||||||
| self._discovery_manager = OAuth2DiscoveryManager() | ||||||||||||||||||||||
|
|
@@ -98,6 +110,8 @@ def __init__( | |||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # TODO: Move ServiceAccountCredentialExchanger to the auth module | ||||||||||||||||||||||
| from ..tools.openapi_tool.auth.credential_exchangers.service_account_exchanger import ServiceAccountCredentialExchanger | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| self._exchanger_registry.register( | ||||||||||||||||||||||
| AuthCredentialTypes.SERVICE_ACCOUNT, | ||||||||||||||||||||||
| ServiceAccountCredentialExchanger(), | ||||||||||||||||||||||
|
|
@@ -111,6 +125,36 @@ def __init__( | |||||||||||||||||||||
| AuthCredentialTypes.OPEN_ID_CONNECT, oauth2_refresher | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def _secure_client_secret(self, credential: Optional[AuthCredential]): | ||||||||||||||||||||||
| """Extracts client secret to memory and redacts it from the credential.""" | ||||||||||||||||||||||
| if ( | ||||||||||||||||||||||
| credential | ||||||||||||||||||||||
| and credential.oauth2 | ||||||||||||||||||||||
| and credential.oauth2.client_id | ||||||||||||||||||||||
| and credential.oauth2.client_secret | ||||||||||||||||||||||
| and credential.oauth2.client_secret != "<redacted>" | ||||||||||||||||||||||
| ): | ||||||||||||||||||||||
| logger.info( | ||||||||||||||||||||||
| f"Securing client secret for client_id: {credential.oauth2.client_id}" | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| # Store in memory map | ||||||||||||||||||||||
| CredentialManager._CLIENT_SECRETS[credential.oauth2.client_id] = ( | ||||||||||||||||||||||
| credential.oauth2.client_secret | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| # Redact from config | ||||||||||||||||||||||
| credential.oauth2.client_secret = "<redacted>" | ||||||||||||||||||||||
| else: | ||||||||||||||||||||||
| if credential and credential.oauth2: | ||||||||||||||||||||||
| logger.debug( | ||||||||||||||||||||||
| f"Not securing secret for client_id {credential.oauth2.client_id}:" | ||||||||||||||||||||||
| f" secret is {credential.oauth2.client_secret}" | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| @staticmethod | ||||||||||||||||||||||
| def get_client_secret(client_id: str) -> Optional[str]: | ||||||||||||||||||||||
| """Retrieves the client secret for a given client_id.""" | ||||||||||||||||||||||
| return CredentialManager._CLIENT_SECRETS.get(client_id) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def register_credential_exchanger( | ||||||||||||||||||||||
| self, | ||||||||||||||||||||||
| credential_type: AuthCredentialTypes, | ||||||||||||||||||||||
|
|
@@ -211,6 +255,40 @@ async def _load_from_auth_response( | |||||||||||||||||||||
| """Load credential from auth response in context.""" | ||||||||||||||||||||||
| return context.get_auth_response(self._auth_config) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| @staticmethod | ||||||||||||||||||||||
| @contextlib.contextmanager | ||||||||||||||||||||||
| def restore_client_secret(credential: AuthCredential, secret: str = None): | ||||||||||||||||||||||
| """Context manager to temporarily restore client secret in a credential. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Args: | ||||||||||||||||||||||
| credential: The credential to restore secret for. | ||||||||||||||||||||||
| secret: Optional secret to use. If not provided, looks up by client_id. | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| if not credential or not credential.oauth2: | ||||||||||||||||||||||
| yield | ||||||||||||||||||||||
| return | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| restored = False | ||||||||||||||||||||||
| if secret: | ||||||||||||||||||||||
| credential.oauth2.client_secret = secret | ||||||||||||||||||||||
| restored = True | ||||||||||||||||||||||
| elif ( | ||||||||||||||||||||||
| credential.oauth2.client_id | ||||||||||||||||||||||
| and credential.oauth2.client_secret == "<redacted>" | ||||||||||||||||||||||
| ): | ||||||||||||||||||||||
| stored_secret = CredentialManager.get_client_secret( | ||||||||||||||||||||||
| credential.oauth2.client_id | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| if stored_secret: | ||||||||||||||||||||||
| credential.oauth2.client_secret = stored_secret | ||||||||||||||||||||||
| restored = True | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| try: | ||||||||||||||||||||||
| yield | ||||||||||||||||||||||
| finally: | ||||||||||||||||||||||
| if restored: | ||||||||||||||||||||||
| credential.oauth2.client_secret = "<redacted>" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| async def _exchange_credential( | ||||||||||||||||||||||
| self, credential: AuthCredential | ||||||||||||||||||||||
| ) -> tuple[AuthCredential, bool]: | ||||||||||||||||||||||
|
|
@@ -219,18 +297,21 @@ async def _exchange_credential( | |||||||||||||||||||||
| if not exchanger: | ||||||||||||||||||||||
| return credential, False | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| from ..tools.openapi_tool.auth.credential_exchangers.service_account_exchanger import ServiceAccountCredentialExchanger | ||||||||||||||||||||||
|
Contributor
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. This local import of |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if isinstance(exchanger, ServiceAccountCredentialExchanger): | ||||||||||||||||||||||
| return ( | ||||||||||||||||||||||
| exchanger.exchange_credential( | ||||||||||||||||||||||
| self._auth_config.auth_scheme, credential | ||||||||||||||||||||||
| ), | ||||||||||||||||||||||
| True, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| exchange_result = await exchanger.exchange( | ||||||||||||||||||||||
| credential, self._auth_config.auth_scheme | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| return exchange_result.credential, exchange_result.was_exchanged | ||||||||||||||||||||||
| else: | ||||||||||||||||||||||
| with self.restore_client_secret(credential): | ||||||||||||||||||||||
| exchanged_credential = await exchanger.exchange( | ||||||||||||||||||||||
| credential, self._auth_config.auth_scheme | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| return exchanged_credential.credential, True | ||||||||||||||||||||||
|
Comment on lines
+310
to
+314
Contributor
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. Hardcoding
Suggested change
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| async def _refresh_credential( | ||||||||||||||||||||||
| self, credential: AuthCredential | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.