Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 10
Credentials: error handling for unsupported providers#707
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
94eb3b0f749d69284fd2fc4d4f0c2ced4bc34e7fc25fcfa889451f33df0124d294f4889a29ea866d85e9077da6aFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,46 @@ | ||
| Persist new credentials for the current organization and project. | ||
| Credentials are encrypted and stored securely for provider integrations (OpenAI, Langfuse, etc.). Only one credential per provider is allowed per organization-project combination. | ||
| Credentials are encrypted and stored securely for provider integrations (OpenAI, Langfuse, etc.). Only one credential per provider is allowed per organization-project combination. You can send credentials for a single provider or multiple providers in one request. Refer to the examples below for the required input parameters for each provider. | ||
| ### Supported Providers: | ||
| - **LLM:** openai, sarvamai, google(gemini) | ||
| - **Observability:** langfuse | ||
| - **Audio:** elevenlabs | ||
| ### Examples: | ||
| #### Single Provider | ||
| ```json | ||
| { | ||
| "credential": { | ||
| "openai": { | ||
| "api_key": "sk-proj-..." | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
| #### Multiple Providers | ||
| ```json | ||
| { | ||
| "credential": { | ||
| "openai": { | ||
| "api_key": "sk-proj-..." | ||
| }, | ||
| "google": { | ||
| "api_key": "AIzaSy..." | ||
| }, | ||
| "sarvamai": { | ||
| "api_key": "sarvam-..." | ||
| }, | ||
| "elevenlabs": { | ||
| "api_key": "sk_..." | ||
| }, | ||
| "langfuse": { | ||
| "public_key": "pk-lf-....", | ||
| "secret_key": "sk-lf-...", | ||
| "host": "https://cloud.langfuse.com" | ||
| } | ||
| } | ||
| } | ||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -11,6 +11,7 @@ | ||
| from app.core.util import now | ||
| from app.models import Credential, CredsCreate, CredsUpdate | ||
| logger = logging.getLogger(__name__) | ||
| @@ -27,9 +28,13 @@ def set_creds_for_org( | ||
| raise HTTPException(400, "No credentials provided") | ||
| for provider, credentials in creds_add.credential.items(): | ||
| # Validate provider and credentials | ||
| validate_provider(provider) | ||
| validate_provider_credentials(provider, credentials) | ||
| try: | ||
| validate_provider_credentials(provider, credentials) | ||
nishika26 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| except ValueError as e: | ||
| logger.error( | ||
| f"[set_creds_for_org] Validation error | project_id: {project_id}, provider: {provider}, error: {str(e)}" | ||
| ) | ||
| raise HTTPException(status_code=400, detail=str(e)) | ||
| # Encrypt entire credentials object | ||
| encrypted_credentials = encrypt_credentials(credentials) | ||
| @@ -144,7 +149,10 @@ def get_provider_credential( | ||
| Raises: | ||
| HTTPException: If credentials are not found | ||
| """ | ||
| validate_provider(provider) | ||
| try: | ||
| validate_provider(provider) | ||
| except ValueError as e: | ||
| raise HTTPException(status_code=400, detail=str(e)) | ||
| statement = select(Credential).where( | ||
| Credential.organization_id == org_id, | ||
| @@ -176,8 +184,13 @@ def update_creds_for_org( | ||
| if not creds_in.provider or not creds_in.credential: | ||
| raise ValueError("Provider and credential must be provided") | ||
| validate_provider(creds_in.provider) | ||
| validate_provider_credentials(creds_in.provider, creds_in.credential) | ||
| try: | ||
| validate_provider_credentials(creds_in.provider, creds_in.credential) | ||
| except ValueError as e: | ||
| logger.error( | ||
| f"[update_creds_for_org] Validation error | organization_id: {org_id}, project_id: {project_id}, provider: {creds_in.provider}, error: {str(e)}" | ||
| ) | ||
| raise HTTPException(status_code=400, detail=str(e)) | ||
| # Encrypt the entire credentials object | ||
| encrypted_credentials = encrypt_credentials(creds_in.credential) | ||
| @@ -216,8 +229,6 @@ def remove_provider_credential( | ||
| Raises: | ||
| HTTPException: If credentials not found or deletion fails | ||
| """ | ||
| validate_provider(provider) | ||
| # Verify credentials exist before attempting delete | ||
| creds = get_provider_credential( | ||
| session=session, | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -90,39 +90,30 @@ def _validate_credential_list(cls, v: list[dict[str, dict[str, str]]] | None): | ||
| if not isinstance(v, list): | ||
| raise TypeError( | ||
| "credential must be a list of single-key dicts (e.g., {'openai': {...}})." | ||
| "Credential must be a list of single-key dicts (e.g., {'openai': {...}})." | ||
| ) | ||
| errors: list[str] = [] | ||
| for item in v: | ||
| if not isinstance(item, dict): | ||
Collaborator 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. Is this post filter required inside ContributorAuthor 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. while it may be hard to maintain, this helps us catch any issue early on with the request body when it comes to credentials being given, otherwise we will only get to know about it after crud function is hit and we put the verification in that Collaborator 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. sure we can take it up later | ||
| raise TypeError( | ||
| "Credential must be a dict with a single provider key like {'openai': {...}}." | ||
| ) | ||
| if len(item) != 1: | ||
| raise ValueError( | ||
| "Credential must have exactly one provider key like {'openai': {...}}." | ||
| ) | ||
| for idx, item in enumerate(v): | ||
| try: | ||
| if not isinstance(item, dict): | ||
| raise TypeError( | ||
| "must be a dict with a single provider key like {'openai': {...}}." | ||
| ) | ||
| if len(item) != 1: | ||
| raise ValueError( | ||
| "must have exactly one provider key like {'openai': {...}}." | ||
| ) | ||
| (provider_key,) = item.keys() | ||
| values = item[provider_key] | ||
| (provider_key,) = item.keys() | ||
| values = item[provider_key] | ||
| validate_provider(provider_key) | ||
| validate_provider(provider_key) | ||
| if not isinstance(values, dict): | ||
| raise ValueError( | ||
| f"Value for provider '{provider_key}' must be an object/dict." | ||
| ) | ||
coderabbitai[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if not isinstance(values, dict): | ||
| raise TypeError( | ||
| f"value for provider '{provider_key}' must be an object/dict." | ||
| ) | ||
| validate_provider_credentials(provider_key, values) | ||
| except (TypeError, ValueError) as e: | ||
| errors.append(f"[{idx}] {e}") | ||
| if errors: | ||
| raise ValueError("credential validation failed:\n" + "\n".join(errors)) | ||
| validate_provider_credentials(provider_key, values) | ||
| return v | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: can we reuse
APIResponse.failure_response()instead of manually raising?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use failure response directly for situations where we are raising 200 status code with a failure payload, this manual raising is needed for the status code and anyway the global exception handler wraps HTTPException into APIResponse.failure_response() automatically, so the client still gets a consistent APIResponse shape, just with the right non-200 status code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the error code can be sent in the metadata. but okay we can take that up later