-
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
Changes from 4 commits
94eb3b0
f749d69
284fd2f
c4d4f0c
2ced4bc
34e7fc2
5fcfa88
9451f33
df0124d
294f488
9a29ea8
66d85e9
077da6a
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 |
|---|---|---|
| @@ -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. | ||
|
|
||
| ### 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__) | ||
|
|
||
|
|
||
|
|
@@ -28,8 +29,14 @@ def set_creds_for_org( | |
|
|
||
| for provider, credentials in creds_add.credential.items(): | ||
| # Validate provider and credentials | ||
| validate_provider(provider) | ||
| validate_provider_credentials(provider, credentials) | ||
| try: | ||
| validate_provider(provider) | ||
| validate_provider_credentials(provider, credentials) | ||
nishika26 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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 +151,13 @@ def get_provider_credential( | |
| Raises: | ||
| HTTPException: If credentials are not found | ||
| """ | ||
| validate_provider(provider) | ||
| try: | ||
| validate_provider(provider) | ||
| except ValueError as e: | ||
| logger.error( | ||
| f"[get_provider_credential] Validation error | organization_id: {org_id}, project_id: {project_id}, provider: {provider}, error: {str(e)}" | ||
| ) | ||
| raise HTTPException(status_code=400, detail=str(e)) | ||
|
|
||
| statement = select(Credential).where( | ||
| Credential.organization_id == org_id, | ||
|
|
@@ -176,8 +189,14 @@ 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(creds_in.provider) | ||
|
||
| 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,7 +235,13 @@ def remove_provider_credential( | |
| Raises: | ||
| HTTPException: If credentials not found or deletion fails | ||
| """ | ||
| validate_provider(provider) | ||
| try: | ||
| validate_provider(provider) | ||
| except ValueError as e: | ||
| logger.error( | ||
| f"[remove_provider_credential] Validation error | organization_id: {org_id}, project_id: {project_id}, provider: {provider}, error: {str(e)}" | ||
| ) | ||
| raise HTTPException(status_code=400, detail=str(e)) | ||
|
|
||
| # Verify credentials exist before attempting delete | ||
| creds = get_provider_credential( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -89,40 +89,33 @@ def _validate_credential_list(cls, v: list[dict[str, dict[str, str]]] | None): | |
| return v | ||
|
|
||
| if not isinstance(v, list): | ||
| raise TypeError( | ||
| "credential must be a list of single-key dicts (e.g., {'openai': {...}})." | ||
| raise ValueError( | ||
|
||
| "Credential must be a list of single-key dicts (e.g., {'openai': {...}})." | ||
| ) | ||
|
|
||
| errors: list[str] = [] | ||
|
|
||
| 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] | ||
|
|
||
| validate_provider(provider_key) | ||
|
|
||
| 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)) | ||
| 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
Collaborator
Author
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 ValueError( | ||
| "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': {...}}." | ||
| ) | ||
|
|
||
| (provider_key,) = item.keys() | ||
| values = item[provider_key] | ||
|
|
||
| # validate_provider will raise ValueError with clear message if invalid | ||
| 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.
Show resolved
Hide resolved
|
||
|
|
||
| # validate_provider_credentials will raise ValueError with clear message | ||
| validate_provider_credentials(provider_key, values) | ||
|
|
||
| return v | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.