mirror of
https://github.com/lfnovo/open-notebook.git
synced 2026-05-22 11:09:21 +00:00
fix: handle credential decryption errors gracefully (#740)
- Credential.get_all() now uses per-row error handling instead of failing on first bad row - Broken credentials include decryption_error field with descriptive message - DELETE endpoint falls back to direct DB delete when credential can't be decrypted - Frontend shows amber warning alert for broken credentials with disabled test/edit/discover - Added i18n translation keys for decryption error warning in all 9 locales
This commit is contained in:
parent
4ae459ca5e
commit
ba01f7df4e
15 changed files with 120 additions and 18 deletions
|
|
@ -223,6 +223,7 @@ def credential_to_response(cred: Credential, model_count: int = 0) -> Credential
|
|||
created=str(cred.created) if cred.created else "",
|
||||
updated=str(cred.updated) if cred.updated else "",
|
||||
model_count=model_count,
|
||||
decryption_error=cred.decryption_error,
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -617,6 +617,7 @@ class CredentialResponse(BaseModel):
|
|||
created: str
|
||||
updated: str
|
||||
model_count: int = 0
|
||||
decryption_error: Optional[str] = None
|
||||
|
||||
|
||||
class CredentialDeleteResponse(BaseModel):
|
||||
|
|
|
|||
|
|
@ -27,16 +27,22 @@ from pydantic import SecretStr
|
|||
from api.credentials_service import (
|
||||
credential_to_response,
|
||||
discover_with_config,
|
||||
migrate_from_env as svc_migrate_from_env,
|
||||
migrate_from_provider_config as svc_migrate_from_provider_config,
|
||||
get_provider_status,
|
||||
register_models,
|
||||
require_encryption_key,
|
||||
test_credential as svc_test_credential,
|
||||
validate_url,
|
||||
)
|
||||
from api.credentials_service import (
|
||||
get_env_status as svc_get_env_status,
|
||||
get_provider_status,
|
||||
)
|
||||
from api.credentials_service import (
|
||||
migrate_from_env as svc_migrate_from_env,
|
||||
)
|
||||
from api.credentials_service import (
|
||||
migrate_from_provider_config as svc_migrate_from_provider_config,
|
||||
)
|
||||
from api.credentials_service import (
|
||||
test_credential as svc_test_credential,
|
||||
)
|
||||
from api.models import (
|
||||
CreateCredentialRequest,
|
||||
|
|
@ -48,6 +54,7 @@ from api.models import (
|
|||
RegisterModelsResponse,
|
||||
UpdateCredentialRequest,
|
||||
)
|
||||
from open_notebook.database.repository import ensure_record_id, repo_delete, repo_query
|
||||
from open_notebook.domain.credential import Credential
|
||||
|
||||
router = APIRouter(prefix="/credentials", tags=["credentials"])
|
||||
|
|
@ -260,7 +267,36 @@ async def delete_credential(
|
|||
- Otherwise, linked models are cascade-deleted automatically
|
||||
"""
|
||||
try:
|
||||
cred = await Credential.get(credential_id)
|
||||
try:
|
||||
cred = await Credential.get(credential_id)
|
||||
except Exception as decrypt_err:
|
||||
# Credential exists but can't be decrypted (wrong encryption key).
|
||||
# Fall back to direct DB operations for deletion.
|
||||
logger.warning(
|
||||
f"Cannot decrypt credential {credential_id}, "
|
||||
f"falling back to direct delete: {decrypt_err}"
|
||||
)
|
||||
|
||||
# Delete linked models directly
|
||||
linked = await repo_query(
|
||||
"SELECT * FROM model WHERE credential = $cred_id",
|
||||
{"cred_id": ensure_record_id(credential_id)},
|
||||
)
|
||||
deleted_models = 0
|
||||
for model_row in linked:
|
||||
model_id = str(model_row.get("id", ""))
|
||||
if model_id:
|
||||
await repo_delete(model_id)
|
||||
deleted_models += 1
|
||||
|
||||
# Delete the credential itself
|
||||
await repo_delete(credential_id)
|
||||
|
||||
return CredentialDeleteResponse(
|
||||
message="Credential deleted successfully",
|
||||
deleted_models=deleted_models,
|
||||
)
|
||||
|
||||
linked_models = await cred.get_linked_models()
|
||||
|
||||
deleted_models = 0
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ import {
|
|||
RefreshCw,
|
||||
Key,
|
||||
ShieldAlert,
|
||||
AlertTriangle,
|
||||
Plus,
|
||||
Edit,
|
||||
Trash2,
|
||||
|
|
@ -822,7 +823,7 @@ function CredentialItem({
|
|||
<Button
|
||||
variant="ghost" size="sm"
|
||||
onClick={() => testCredential(credential.id)}
|
||||
disabled={isTestPending}
|
||||
disabled={isTestPending || !!credential.decryption_error}
|
||||
title={t.apiKeys.testConnection}
|
||||
>
|
||||
{isTestPending ? <Loader2 className="h-4 w-4 animate-spin" /> : <Plug className="h-4 w-4" />}
|
||||
|
|
@ -831,12 +832,13 @@ function CredentialItem({
|
|||
<Button
|
||||
variant="ghost" size="sm"
|
||||
onClick={() => setDiscoverOpen(true)}
|
||||
disabled={!!credential.decryption_error}
|
||||
title={t.apiKeys.syncModels}
|
||||
>
|
||||
<Bot className="h-4 w-4" />
|
||||
<span className="hidden sm:inline text-xs">Models</span>
|
||||
</Button>
|
||||
<Button variant="ghost" size="sm" onClick={() => setEditOpen(true)} title={t.common.edit}>
|
||||
<Button variant="ghost" size="sm" onClick={() => setEditOpen(true)} disabled={!!credential.decryption_error} title={t.common.edit}>
|
||||
<Edit className="h-4 w-4" />
|
||||
</Button>
|
||||
<Button
|
||||
|
|
@ -850,6 +852,17 @@ function CredentialItem({
|
|||
</div>
|
||||
</div>
|
||||
|
||||
{/* Decryption error warning */}
|
||||
{credential.decryption_error && (
|
||||
<Alert className="border-amber-500/50 bg-amber-50 dark:bg-amber-950/20">
|
||||
<AlertTriangle className="h-4 w-4 text-amber-600 dark:text-amber-400" />
|
||||
<AlertTitle className="text-amber-800 dark:text-amber-200">{t.apiKeys.decryptionError}</AlertTitle>
|
||||
<AlertDescription className="text-amber-700 dark:text-amber-300 text-sm">
|
||||
{t.apiKeys.decryptionErrorDescription}
|
||||
</AlertDescription>
|
||||
</Alert>
|
||||
)}
|
||||
|
||||
{/* Linked models grouped by type */}
|
||||
{linkedModels.length > 0 && (
|
||||
<div className="space-y-1.5 pt-1">
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ export interface Credential {
|
|||
created: string
|
||||
updated: string
|
||||
model_count: number
|
||||
decryption_error?: string | null
|
||||
}
|
||||
|
||||
export interface CreateCredentialRequest {
|
||||
|
|
|
|||
|
|
@ -916,6 +916,8 @@ export const bnIN = {
|
|||
configUpdateSuccess: "কনফিগারেশন সফলভাবে আপডেট",
|
||||
configDeleteSuccess: "কনফিগারেশন সফলভাবে মুছে ফেলা",
|
||||
apiKeyEditHint: "বিদ্যমান API কী রাখতে খালি রাখুন",
|
||||
decryptionError: "ডিক্রিপশন ত্রুটি",
|
||||
decryptionErrorDescription: "এই শংসাপত্রের API কী ডিক্রিপ্ট করা যায়নি। এনক্রিপশন কী পরিবর্তন হয়ে থাকতে পারে। এই শংসাপত্রটি মুছে সঠিক কী দিয়ে পুনরায় তৈরি করুন।",
|
||||
},
|
||||
setupBanner: {
|
||||
encryptionRequired: "এনক্রিপশন কী কনফিগার করা হয়নি",
|
||||
|
|
|
|||
|
|
@ -916,6 +916,8 @@ export const enUS = {
|
|||
configUpdateSuccess: "Configuration updated successfully",
|
||||
configDeleteSuccess: "Configuration deleted successfully",
|
||||
apiKeyEditHint: "Leave blank to keep the existing API key",
|
||||
decryptionError: "Decryption Error",
|
||||
decryptionErrorDescription: "This credential's API key could not be decrypted. The encryption key may have changed. Delete this credential and re-create it with the correct key.",
|
||||
},
|
||||
setupBanner: {
|
||||
encryptionRequired: "Encryption key not configured",
|
||||
|
|
|
|||
|
|
@ -915,6 +915,8 @@ export const frFR = {
|
|||
configUpdateSuccess: "Configuration mise à jour avec succès",
|
||||
configDeleteSuccess: "Configuration supprimée avec succès",
|
||||
apiKeyEditHint: "Laissez vide pour conserver la clé API existante",
|
||||
decryptionError: "Erreur de déchiffrement",
|
||||
decryptionErrorDescription: "La clé API de cette configuration n'a pas pu être déchiffrée. La clé de chiffrement a peut-être changé. Supprimez cette configuration et recréez-la avec la bonne clé.",
|
||||
},
|
||||
setupBanner: {
|
||||
encryptionRequired: "Clé de chiffrement non configurée",
|
||||
|
|
|
|||
|
|
@ -915,6 +915,8 @@ export const itIT = {
|
|||
configUpdateSuccess: "Configurazione aggiornata con successo",
|
||||
configDeleteSuccess: "Configurazione eliminata con successo",
|
||||
apiKeyEditHint: "Lascia vuoto per mantenere la chiave API esistente",
|
||||
decryptionError: "Errore di decrittazione",
|
||||
decryptionErrorDescription: "La chiave API di questa credenziale non può essere decrittata. La chiave di crittografia potrebbe essere cambiata. Elimina questa credenziale e ricreala con la chiave corretta.",
|
||||
},
|
||||
setupBanner: {
|
||||
encryptionRequired: "Chiave di crittografia non configurata",
|
||||
|
|
|
|||
|
|
@ -915,6 +915,8 @@ export const jaJP = {
|
|||
configUpdateSuccess: "設定が正常に変更されました",
|
||||
configDeleteSuccess: "設定が正常に削除されました",
|
||||
apiKeyEditHint: "既存のAPIキーを維持するには空白のままにしてください",
|
||||
decryptionError: "復号エラー",
|
||||
decryptionErrorDescription: "この認証情報のAPIキーを復号できませんでした。暗号化キーが変更された可能性があります。この認証情報を削除し、正しいキーで再作成してください。",
|
||||
},
|
||||
setupBanner: {
|
||||
encryptionRequired: "暗号化キーが設定されていません",
|
||||
|
|
|
|||
|
|
@ -915,6 +915,8 @@ export const ptBR = {
|
|||
configUpdateSuccess: "Configuração atualizada com sucesso",
|
||||
configDeleteSuccess: "Configuração excluída com sucesso",
|
||||
apiKeyEditHint: "Deixe em branco para manter a chave de API existente",
|
||||
decryptionError: "Erro de Descriptografia",
|
||||
decryptionErrorDescription: "A chave de API desta credencial não pôde ser descriptografada. A chave de criptografia pode ter sido alterada. Exclua esta credencial e recrie-a com a chave correta.",
|
||||
},
|
||||
setupBanner: {
|
||||
encryptionRequired: "Chave de criptografia não configurada",
|
||||
|
|
|
|||
|
|
@ -915,6 +915,8 @@ export const ruRU = {
|
|||
configUpdateSuccess: "Конфигурация успешно обновлена",
|
||||
configDeleteSuccess: "Конфигурация успешно удалена",
|
||||
apiKeyEditHint: "Оставьте пустым, чтобы сохранить текущий API-ключ",
|
||||
decryptionError: "Ошибка расшифровки",
|
||||
decryptionErrorDescription: "API-ключ этих учётных данных не удалось расшифровать. Возможно, ключ шифрования был изменён. Удалите эти учётные данные и создайте заново с правильным ключом.",
|
||||
},
|
||||
setupBanner: {
|
||||
encryptionRequired: "Ключ шифрования не настроен",
|
||||
|
|
|
|||
|
|
@ -915,6 +915,8 @@ export const zhCN = {
|
|||
configUpdateSuccess: "配置更新成功",
|
||||
configDeleteSuccess: "配置删除成功",
|
||||
apiKeyEditHint: "留空以保留现有 API 密钥",
|
||||
decryptionError: "解密错误",
|
||||
decryptionErrorDescription: "此凭证的 API 密钥无法解密。加密密钥可能已更改。请删除此凭证并使用正确的密钥重新创建。",
|
||||
},
|
||||
setupBanner: {
|
||||
encryptionRequired: "未配置加密密钥",
|
||||
|
|
|
|||
|
|
@ -915,6 +915,8 @@ export const zhTW = {
|
|||
configUpdateSuccess: "設定更新成功",
|
||||
configDeleteSuccess: "設定刪除成功",
|
||||
apiKeyEditHint: "留空以保留現有 API 金鑰",
|
||||
decryptionError: "解密錯誤",
|
||||
decryptionErrorDescription: "此憑證的 API 金鑰無法解密。加密金鑰可能已變更。請刪除此憑證並使用正確的金鑰重新建立。",
|
||||
},
|
||||
setupBanner: {
|
||||
encryptionRequired: "未設定加密金鑰",
|
||||
|
|
|
|||
|
|
@ -53,6 +53,7 @@ class Credential(ObjectModel):
|
|||
provider: str
|
||||
modalities: List[str] = []
|
||||
api_key: Optional[SecretStr] = None
|
||||
decryption_error: Optional[str] = None
|
||||
base_url: Optional[str] = None
|
||||
endpoint: Optional[str] = None
|
||||
api_version: Optional[str] = None
|
||||
|
|
@ -130,18 +131,47 @@ class Credential(ObjectModel):
|
|||
|
||||
@classmethod
|
||||
async def get_all(cls, order_by=None) -> List["Credential"]:
|
||||
"""Override get_all() to handle api_key decryption."""
|
||||
instances = await super().get_all(order_by=order_by)
|
||||
for instance in instances:
|
||||
if instance.api_key:
|
||||
raw = (
|
||||
instance.api_key.get_secret_value()
|
||||
if isinstance(instance.api_key, SecretStr)
|
||||
else instance.api_key
|
||||
"""Override get_all() to handle api_key decryption with per-row error handling."""
|
||||
order_clause = f" ORDER BY {order_by}" if order_by else ""
|
||||
results = await repo_query(
|
||||
f"SELECT * FROM {cls.table_name}{order_clause}",
|
||||
{},
|
||||
)
|
||||
credentials = []
|
||||
for row in results:
|
||||
try:
|
||||
cred = cls._from_db_row(row)
|
||||
credentials.append(cred)
|
||||
except Exception as e:
|
||||
logger.warning(
|
||||
f"Failed to decrypt credential {row.get('id', 'unknown')}: {e}"
|
||||
)
|
||||
decrypted = decrypt_value(raw)
|
||||
object.__setattr__(instance, "api_key", SecretStr(decrypted))
|
||||
return instances
|
||||
# Create a minimal credential with error info from raw DB fields
|
||||
try:
|
||||
error_cred = cls(
|
||||
name=row.get("name", "Unknown"),
|
||||
provider=row.get("provider", "unknown"),
|
||||
modalities=row.get("modalities", []),
|
||||
decryption_error=f"Failed to decrypt API key. The encryption key may have changed. Error: {e}",
|
||||
)
|
||||
# Preserve the DB id, created, updated from the raw row
|
||||
if row.get("id"):
|
||||
object.__setattr__(error_cred, "id", str(row["id"]))
|
||||
if row.get("created"):
|
||||
object.__setattr__(error_cred, "created", row["created"])
|
||||
if row.get("updated"):
|
||||
object.__setattr__(error_cred, "updated", row["updated"])
|
||||
# Mark that it had an api_key (even though we can't decrypt it)
|
||||
if row.get("api_key"):
|
||||
object.__setattr__(
|
||||
error_cred, "api_key", SecretStr("UNDECRYPTABLE")
|
||||
)
|
||||
credentials.append(error_cred)
|
||||
except Exception as inner_e:
|
||||
logger.error(
|
||||
f"Failed to create error credential for {row.get('id', 'unknown')}: {inner_e}"
|
||||
)
|
||||
return credentials
|
||||
|
||||
async def get_linked_models(self) -> list:
|
||||
"""Get all models linked to this credential."""
|
||||
|
|
@ -159,6 +189,8 @@ class Credential(ObjectModel):
|
|||
"""Override to encrypt api_key before storage."""
|
||||
data = {}
|
||||
for key, value in self.model_dump().items():
|
||||
if key == "decryption_error":
|
||||
continue
|
||||
if key == "api_key":
|
||||
# Handle SecretStr: extract, encrypt, store
|
||||
if self.api_key:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue