Add endpoint to update existing credential data (#SKY-7883) (#4693)
Co-authored-by: Suchintan Singh <suchintan@skyvern.com>
This commit is contained in:
@@ -5192,6 +5192,45 @@ class AgentDB(BaseAlchemyDB):
|
|||||||
await session.refresh(credential)
|
await session.refresh(credential)
|
||||||
return Credential.model_validate(credential)
|
return Credential.model_validate(credential)
|
||||||
|
|
||||||
|
async def update_credential_vault_data(
|
||||||
|
self,
|
||||||
|
credential_id: str,
|
||||||
|
organization_id: str,
|
||||||
|
item_id: str,
|
||||||
|
name: str,
|
||||||
|
credential_type: CredentialType,
|
||||||
|
username: str | None = None,
|
||||||
|
totp_type: str = "none",
|
||||||
|
totp_identifier: str | None = None,
|
||||||
|
card_last4: str | None = None,
|
||||||
|
card_brand: str | None = None,
|
||||||
|
secret_label: str | None = None,
|
||||||
|
) -> Credential:
|
||||||
|
async with self.Session() as session:
|
||||||
|
credential = (
|
||||||
|
await session.scalars(
|
||||||
|
select(CredentialModel)
|
||||||
|
.filter_by(credential_id=credential_id)
|
||||||
|
.filter_by(organization_id=organization_id)
|
||||||
|
.filter(CredentialModel.deleted_at.is_(None))
|
||||||
|
.with_for_update()
|
||||||
|
)
|
||||||
|
).first()
|
||||||
|
if not credential:
|
||||||
|
raise NotFoundError(f"Credential {credential_id} not found")
|
||||||
|
credential.item_id = item_id
|
||||||
|
credential.name = name
|
||||||
|
credential.credential_type = credential_type
|
||||||
|
credential.username = username
|
||||||
|
credential.totp_type = totp_type
|
||||||
|
credential.totp_identifier = totp_identifier
|
||||||
|
credential.card_last4 = card_last4
|
||||||
|
credential.card_brand = card_brand
|
||||||
|
credential.secret_label = secret_label
|
||||||
|
await session.commit()
|
||||||
|
await session.refresh(credential)
|
||||||
|
return Credential.model_validate(credential)
|
||||||
|
|
||||||
async def delete_credential(self, credential_id: str, organization_id: str) -> None:
|
async def delete_credential(self, credential_id: str, organization_id: str) -> None:
|
||||||
async with self.Session() as session:
|
async with self.Session() as session:
|
||||||
credential = (
|
credential = (
|
||||||
|
|||||||
@@ -280,6 +280,75 @@ async def create_credential(
|
|||||||
raise HTTPException(status_code=400, detail=f"Unsupported credential type: {data.credential_type}")
|
raise HTTPException(status_code=400, detail=f"Unsupported credential type: {data.credential_type}")
|
||||||
|
|
||||||
|
|
||||||
|
@legacy_base_router.put("/credentials/{credential_id}")
|
||||||
|
@legacy_base_router.put("/credentials/{credential_id}/", include_in_schema=False)
|
||||||
|
@base_router.post(
|
||||||
|
"/credentials/{credential_id}/update",
|
||||||
|
response_model=CredentialResponse,
|
||||||
|
summary="Update credential",
|
||||||
|
description="Overwrites the stored credential data (e.g. username/password) while keeping the same credential_id.",
|
||||||
|
tags=["Credentials"],
|
||||||
|
openapi_extra={
|
||||||
|
"x-fern-sdk-method-name": "update_credential",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
@base_router.post(
|
||||||
|
"/credentials/{credential_id}/update/",
|
||||||
|
response_model=CredentialResponse,
|
||||||
|
include_in_schema=False,
|
||||||
|
)
|
||||||
|
async def update_credential(
|
||||||
|
background_tasks: BackgroundTasks,
|
||||||
|
credential_id: str = Path(
|
||||||
|
...,
|
||||||
|
description="The unique identifier of the credential to update",
|
||||||
|
examples=["cred_1234567890"],
|
||||||
|
openapi_extra={"x-fern-sdk-parameter-name": "credential_id"},
|
||||||
|
),
|
||||||
|
data: CreateCredentialRequest = Body(
|
||||||
|
...,
|
||||||
|
description="The new credential data to store",
|
||||||
|
example={
|
||||||
|
"name": "My Credential",
|
||||||
|
"credential_type": "PASSWORD",
|
||||||
|
"credential": {"username": "user@example.com", "password": "newpassword123"},
|
||||||
|
},
|
||||||
|
openapi_extra={"x-fern-sdk-parameter-name": "data"},
|
||||||
|
),
|
||||||
|
current_org: Organization = Depends(org_auth_service.get_current_org),
|
||||||
|
) -> CredentialResponse:
|
||||||
|
existing_credential = await app.DATABASE.get_credential(
|
||||||
|
credential_id=credential_id, organization_id=current_org.organization_id
|
||||||
|
)
|
||||||
|
if not existing_credential:
|
||||||
|
raise HTTPException(status_code=404, detail=f"Credential not found, credential_id={credential_id}")
|
||||||
|
|
||||||
|
vault_type = existing_credential.vault_type or CredentialVaultType.BITWARDEN
|
||||||
|
credential_service = app.CREDENTIAL_VAULT_SERVICES.get(vault_type)
|
||||||
|
if not credential_service:
|
||||||
|
raise HTTPException(status_code=400, detail="Unsupported credential storage type")
|
||||||
|
|
||||||
|
old_item_id = existing_credential.item_id
|
||||||
|
|
||||||
|
updated_credential = await credential_service.update_credential(
|
||||||
|
credential=existing_credential,
|
||||||
|
data=data,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Schedule background cleanup of old vault item if the item_id changed
|
||||||
|
if old_item_id != updated_credential.item_id:
|
||||||
|
background_tasks.add_task(
|
||||||
|
credential_service.post_delete_credential_item,
|
||||||
|
old_item_id,
|
||||||
|
existing_credential.organization_id,
|
||||||
|
)
|
||||||
|
|
||||||
|
if updated_credential.vault_type == CredentialVaultType.BITWARDEN:
|
||||||
|
background_tasks.add_task(fetch_credential_item_background, updated_credential.item_id)
|
||||||
|
|
||||||
|
return _convert_to_response(updated_credential)
|
||||||
|
|
||||||
|
|
||||||
@legacy_base_router.delete("/credentials/{credential_id}")
|
@legacy_base_router.delete("/credentials/{credential_id}")
|
||||||
@legacy_base_router.delete("/credentials/{credential_id}/", include_in_schema=False)
|
@legacy_base_router.delete("/credentials/{credential_id}/", include_in_schema=False)
|
||||||
@base_router.post(
|
@base_router.post(
|
||||||
@@ -329,7 +398,12 @@ async def delete_credential(
|
|||||||
await credential_service.delete_credential(credential)
|
await credential_service.delete_credential(credential)
|
||||||
|
|
||||||
# Schedule background cleanup if the service implements it
|
# Schedule background cleanup if the service implements it
|
||||||
background_tasks.add_task(credential_service.post_delete_credential_item, credential.item_id)
|
if vault_type != CredentialVaultType.CUSTOM:
|
||||||
|
background_tasks.add_task(
|
||||||
|
credential_service.post_delete_credential_item,
|
||||||
|
credential.item_id,
|
||||||
|
credential.organization_id,
|
||||||
|
)
|
||||||
|
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|||||||
@@ -66,6 +66,35 @@ class AzureCredentialVaultService(CredentialVaultService):
|
|||||||
|
|
||||||
return credential
|
return credential
|
||||||
|
|
||||||
|
async def update_credential(self, credential: Credential, data: CreateCredentialRequest) -> Credential:
|
||||||
|
# Azure supports in-place secret updates, so we reuse the same item_id.
|
||||||
|
# NOTE: If the DB update below fails, the vault will contain the new data
|
||||||
|
# while DB metadata (name, type, username) remains stale. The actual credential
|
||||||
|
# data in the vault is still correct since it uses the same item_id. A retry
|
||||||
|
# of the update call will reconcile the DB metadata.
|
||||||
|
await self._update_azure_secret_item(
|
||||||
|
item_id=credential.item_id,
|
||||||
|
credential=data.credential,
|
||||||
|
)
|
||||||
|
|
||||||
|
try:
|
||||||
|
updated_credential = await self._update_db_credential(
|
||||||
|
credential=credential,
|
||||||
|
data=data,
|
||||||
|
item_id=credential.item_id,
|
||||||
|
)
|
||||||
|
except Exception:
|
||||||
|
LOG.error(
|
||||||
|
"DB update failed after Azure vault secret was already overwritten. "
|
||||||
|
"Vault data is updated but DB metadata may be stale.",
|
||||||
|
organization_id=credential.organization_id,
|
||||||
|
credential_id=credential.credential_id,
|
||||||
|
item_id=credential.item_id,
|
||||||
|
)
|
||||||
|
raise
|
||||||
|
|
||||||
|
return updated_credential
|
||||||
|
|
||||||
async def delete_credential(
|
async def delete_credential(
|
||||||
self,
|
self,
|
||||||
credential: Credential,
|
credential: Credential,
|
||||||
@@ -78,7 +107,7 @@ class AzureCredentialVaultService(CredentialVaultService):
|
|||||||
secret_value="",
|
secret_value="",
|
||||||
)
|
)
|
||||||
|
|
||||||
async def post_delete_credential_item(self, item_id: str) -> None:
|
async def post_delete_credential_item(self, item_id: str, _organization_id: str | None = None) -> None:
|
||||||
"""
|
"""
|
||||||
Background task to delete the credential item from Azure Key Vault.
|
Background task to delete the credential item from Azure Key Vault.
|
||||||
This allows the API to respond quickly while the deletion happens asynchronously.
|
This allows the API to respond quickly while the deletion happens asynchronously.
|
||||||
@@ -184,3 +213,42 @@ class AzureCredentialVaultService(CredentialVaultService):
|
|||||||
secret_name=secret_name,
|
secret_name=secret_name,
|
||||||
secret_value=secret_value,
|
secret_value=secret_value,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
async def _update_azure_secret_item(
|
||||||
|
self,
|
||||||
|
item_id: str,
|
||||||
|
credential: PasswordCredential | CreditCardCredential | SecretCredential,
|
||||||
|
) -> None:
|
||||||
|
if isinstance(credential, PasswordCredential):
|
||||||
|
data = AzureCredentialVaultService._PasswordCredentialDataImage(
|
||||||
|
type="password",
|
||||||
|
username=credential.username,
|
||||||
|
password=credential.password,
|
||||||
|
totp=credential.totp,
|
||||||
|
)
|
||||||
|
elif isinstance(credential, CreditCardCredential):
|
||||||
|
data = AzureCredentialVaultService._CreditCardCredentialDataImage(
|
||||||
|
type="credit_card",
|
||||||
|
card_number=credential.card_number,
|
||||||
|
card_cvv=credential.card_cvv,
|
||||||
|
card_exp_month=credential.card_exp_month,
|
||||||
|
card_exp_year=credential.card_exp_year,
|
||||||
|
card_brand=credential.card_brand,
|
||||||
|
card_holder_name=credential.card_holder_name,
|
||||||
|
)
|
||||||
|
elif isinstance(credential, SecretCredential):
|
||||||
|
data = AzureCredentialVaultService._SecretCredentialDataImage(
|
||||||
|
type="secret",
|
||||||
|
secret_value=credential.secret_value,
|
||||||
|
secret_label=credential.secret_label,
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
raise TypeError(f"Invalid credential type: {type(credential)}")
|
||||||
|
|
||||||
|
secret_value = data.model_dump_json(exclude_none=True)
|
||||||
|
|
||||||
|
await self._client.create_or_update_secret(
|
||||||
|
vault_name=self._vault_name,
|
||||||
|
secret_name=item_id,
|
||||||
|
secret_value=secret_value,
|
||||||
|
)
|
||||||
|
|||||||
@@ -46,6 +46,45 @@ class BitwardenCredentialVaultService(CredentialVaultService):
|
|||||||
|
|
||||||
return credential
|
return credential
|
||||||
|
|
||||||
|
async def update_credential(self, credential: Credential, data: CreateCredentialRequest) -> Credential:
|
||||||
|
org_collection = await app.DATABASE.get_organization_bitwarden_collection(credential.organization_id)
|
||||||
|
|
||||||
|
if not org_collection:
|
||||||
|
raise HTTPException(status_code=404, detail="Credential account not found. It might have been deleted.")
|
||||||
|
|
||||||
|
# Create new vault item with the updated data
|
||||||
|
new_item_id = await BitwardenService.create_credential_item(
|
||||||
|
collection_id=org_collection.collection_id,
|
||||||
|
name=data.name,
|
||||||
|
credential=data.credential,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Update DB record to point to the new vault item
|
||||||
|
try:
|
||||||
|
updated_credential = await self._update_db_credential(
|
||||||
|
credential=credential,
|
||||||
|
data=data,
|
||||||
|
item_id=new_item_id,
|
||||||
|
)
|
||||||
|
except Exception:
|
||||||
|
LOG.warning(
|
||||||
|
"DB update failed, attempting to clean up new Bitwarden vault item",
|
||||||
|
organization_id=credential.organization_id,
|
||||||
|
new_item_id=new_item_id,
|
||||||
|
)
|
||||||
|
try:
|
||||||
|
await BitwardenService.delete_credential_item(new_item_id)
|
||||||
|
except Exception as cleanup_error:
|
||||||
|
LOG.error(
|
||||||
|
"Failed to clean up orphaned Bitwarden vault item",
|
||||||
|
organization_id=credential.organization_id,
|
||||||
|
new_item_id=new_item_id,
|
||||||
|
error=str(cleanup_error),
|
||||||
|
)
|
||||||
|
raise
|
||||||
|
|
||||||
|
return updated_credential
|
||||||
|
|
||||||
async def delete_credential(
|
async def delete_credential(
|
||||||
self,
|
self,
|
||||||
credential: Credential,
|
credential: Credential,
|
||||||
@@ -59,5 +98,20 @@ class BitwardenCredentialVaultService(CredentialVaultService):
|
|||||||
await app.DATABASE.delete_credential(credential.credential_id, credential.organization_id)
|
await app.DATABASE.delete_credential(credential.credential_id, credential.organization_id)
|
||||||
await BitwardenService.delete_credential_item(credential.item_id)
|
await BitwardenService.delete_credential_item(credential.item_id)
|
||||||
|
|
||||||
|
async def post_delete_credential_item(self, item_id: str, organization_id: str | None = None) -> None:
|
||||||
|
try:
|
||||||
|
await BitwardenService.delete_credential_item(item_id)
|
||||||
|
LOG.info(
|
||||||
|
"Successfully deleted credential item from Bitwarden in background",
|
||||||
|
item_id=item_id,
|
||||||
|
)
|
||||||
|
except Exception as e:
|
||||||
|
LOG.warning(
|
||||||
|
"Failed to delete credential item from Bitwarden in background",
|
||||||
|
item_id=item_id,
|
||||||
|
error=str(e),
|
||||||
|
exc_info=True,
|
||||||
|
)
|
||||||
|
|
||||||
async def get_credential_item(self, db_credential: Credential) -> CredentialItem:
|
async def get_credential_item(self, db_credential: Credential) -> CredentialItem:
|
||||||
return await BitwardenService.get_credential_item(db_credential.item_id)
|
return await BitwardenService.get_credential_item(db_credential.item_id)
|
||||||
|
|||||||
@@ -21,11 +21,15 @@ class CredentialVaultService(ABC):
|
|||||||
async def create_credential(self, organization_id: str, data: CreateCredentialRequest) -> Credential:
|
async def create_credential(self, organization_id: str, data: CreateCredentialRequest) -> Credential:
|
||||||
"""Create a new credential in the vault and database."""
|
"""Create a new credential in the vault and database."""
|
||||||
|
|
||||||
|
@abstractmethod
|
||||||
|
async def update_credential(self, credential: Credential, data: CreateCredentialRequest) -> Credential:
|
||||||
|
"""Update an existing credential's vault data. Returns the updated credential."""
|
||||||
|
|
||||||
@abstractmethod
|
@abstractmethod
|
||||||
async def delete_credential(self, credential: Credential) -> None:
|
async def delete_credential(self, credential: Credential) -> None:
|
||||||
"""Delete a credential from the vault and database."""
|
"""Delete a credential from the vault and database."""
|
||||||
|
|
||||||
async def post_delete_credential_item(self, item_id: str) -> None:
|
async def post_delete_credential_item(self, item_id: str, organization_id: str | None = None) -> None:
|
||||||
"""
|
"""
|
||||||
Optional hook for scheduling background cleanup tasks after credential deletion.
|
Optional hook for scheduling background cleanup tasks after credential deletion.
|
||||||
Default implementation does nothing. Override in subclasses as needed.
|
Default implementation does nothing. Override in subclasses as needed.
|
||||||
@@ -84,3 +88,52 @@ class CredentialVaultService(ABC):
|
|||||||
)
|
)
|
||||||
else:
|
else:
|
||||||
raise Exception(f"Unsupported credential type: {data.credential_type}")
|
raise Exception(f"Unsupported credential type: {data.credential_type}")
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
async def _update_db_credential(
|
||||||
|
credential: Credential,
|
||||||
|
data: CreateCredentialRequest,
|
||||||
|
item_id: str,
|
||||||
|
) -> Credential:
|
||||||
|
if data.credential_type == CredentialType.PASSWORD:
|
||||||
|
return await app.DATABASE.update_credential_vault_data(
|
||||||
|
credential_id=credential.credential_id,
|
||||||
|
organization_id=credential.organization_id,
|
||||||
|
item_id=item_id,
|
||||||
|
name=data.name,
|
||||||
|
credential_type=data.credential_type,
|
||||||
|
username=data.credential.username,
|
||||||
|
totp_type=data.credential.totp_type,
|
||||||
|
totp_identifier=data.credential.totp_identifier,
|
||||||
|
card_last4=None,
|
||||||
|
card_brand=None,
|
||||||
|
)
|
||||||
|
elif data.credential_type == CredentialType.CREDIT_CARD:
|
||||||
|
return await app.DATABASE.update_credential_vault_data(
|
||||||
|
credential_id=credential.credential_id,
|
||||||
|
organization_id=credential.organization_id,
|
||||||
|
item_id=item_id,
|
||||||
|
name=data.name,
|
||||||
|
credential_type=data.credential_type,
|
||||||
|
username=None,
|
||||||
|
totp_type="none",
|
||||||
|
card_last4=data.credential.card_number[-4:],
|
||||||
|
card_brand=data.credential.card_brand,
|
||||||
|
totp_identifier=None,
|
||||||
|
)
|
||||||
|
elif data.credential_type == CredentialType.SECRET:
|
||||||
|
return await app.DATABASE.update_credential_vault_data(
|
||||||
|
credential_id=credential.credential_id,
|
||||||
|
organization_id=credential.organization_id,
|
||||||
|
item_id=item_id,
|
||||||
|
name=data.name,
|
||||||
|
credential_type=data.credential_type,
|
||||||
|
username=None,
|
||||||
|
totp_type="none",
|
||||||
|
card_last4=None,
|
||||||
|
card_brand=None,
|
||||||
|
totp_identifier=None,
|
||||||
|
secret_label=data.credential.secret_label,
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
raise Exception(f"Unsupported credential type: {data.credential_type}")
|
||||||
|
|||||||
@@ -160,6 +160,102 @@ class CustomCredentialVaultService(CredentialVaultService):
|
|||||||
)
|
)
|
||||||
raise
|
raise
|
||||||
|
|
||||||
|
async def update_credential(self, credential: Credential, data: CreateCredentialRequest) -> Credential:
|
||||||
|
LOG.info(
|
||||||
|
"Updating credential in custom vault",
|
||||||
|
organization_id=credential.organization_id,
|
||||||
|
credential_id=credential.credential_id,
|
||||||
|
name=data.name,
|
||||||
|
credential_type=data.credential_type,
|
||||||
|
)
|
||||||
|
|
||||||
|
try:
|
||||||
|
client = await self._get_client_for_organization(credential.organization_id)
|
||||||
|
|
||||||
|
# Create new credential in the external API
|
||||||
|
new_item_id = await client.create_credential(
|
||||||
|
name=data.name,
|
||||||
|
credential=data.credential,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Update DB record to point to the new vault item
|
||||||
|
try:
|
||||||
|
updated_credential = await self._update_db_credential(
|
||||||
|
credential=credential,
|
||||||
|
data=data,
|
||||||
|
item_id=new_item_id,
|
||||||
|
)
|
||||||
|
except Exception:
|
||||||
|
LOG.warning(
|
||||||
|
"DB update failed, attempting to clean up new external credential",
|
||||||
|
organization_id=credential.organization_id,
|
||||||
|
new_item_id=new_item_id,
|
||||||
|
)
|
||||||
|
try:
|
||||||
|
await client.delete_credential(new_item_id)
|
||||||
|
except Exception as cleanup_error:
|
||||||
|
LOG.error(
|
||||||
|
"Failed to clean up orphaned external credential",
|
||||||
|
organization_id=credential.organization_id,
|
||||||
|
new_item_id=new_item_id,
|
||||||
|
error=str(cleanup_error),
|
||||||
|
)
|
||||||
|
raise
|
||||||
|
|
||||||
|
LOG.info(
|
||||||
|
"Successfully updated credential in custom vault",
|
||||||
|
organization_id=credential.organization_id,
|
||||||
|
credential_id=credential.credential_id,
|
||||||
|
old_item_id=credential.item_id,
|
||||||
|
new_item_id=new_item_id,
|
||||||
|
)
|
||||||
|
|
||||||
|
return updated_credential
|
||||||
|
|
||||||
|
except Exception as e:
|
||||||
|
LOG.error(
|
||||||
|
"Failed to update credential in custom vault",
|
||||||
|
organization_id=credential.organization_id,
|
||||||
|
credential_id=credential.credential_id,
|
||||||
|
error=str(e),
|
||||||
|
exc_info=True,
|
||||||
|
)
|
||||||
|
raise
|
||||||
|
|
||||||
|
async def post_delete_credential_item(self, item_id: str, organization_id: str | None = None) -> None:
|
||||||
|
"""
|
||||||
|
Background task to delete the old credential item from the custom vault
|
||||||
|
after an update or delete operation.
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
if organization_id is None and self._client is None:
|
||||||
|
LOG.warning(
|
||||||
|
"Skipping custom vault cleanup; organization_id is required for per-organization configuration",
|
||||||
|
item_id=item_id,
|
||||||
|
organization_id=organization_id,
|
||||||
|
)
|
||||||
|
return
|
||||||
|
|
||||||
|
if self._client is not None:
|
||||||
|
client = self._client
|
||||||
|
else:
|
||||||
|
assert organization_id is not None
|
||||||
|
client = await self._get_client_for_organization(organization_id)
|
||||||
|
await client.delete_credential(item_id)
|
||||||
|
LOG.info(
|
||||||
|
"Successfully deleted credential item from custom vault in background",
|
||||||
|
organization_id=organization_id,
|
||||||
|
item_id=item_id,
|
||||||
|
)
|
||||||
|
except Exception as e:
|
||||||
|
LOG.warning(
|
||||||
|
"Failed to delete credential item from custom vault in background",
|
||||||
|
organization_id=organization_id,
|
||||||
|
item_id=item_id,
|
||||||
|
error=str(e),
|
||||||
|
exc_info=True,
|
||||||
|
)
|
||||||
|
|
||||||
async def delete_credential(self, credential: Credential) -> None:
|
async def delete_credential(self, credential: Credential) -> None:
|
||||||
"""
|
"""
|
||||||
Delete a credential from the custom vault and database.
|
Delete a credential from the custom vault and database.
|
||||||
|
|||||||
Reference in New Issue
Block a user