-
Notifications
You must be signed in to change notification settings - Fork 10
Credentials: Add support for org credentials #179
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 28 commits
29d219a
b91b3fc
a7fb888
206fd9b
e935a15
dfd17b4
c1a54e8
6e841cc
df539d0
35329e2
d4fa0cc
ba4d3ad
7c49131
7533b1a
df8303e
668cb9b
1ab315e
db02b99
ac7b2a6
0e64dfd
2900865
6c34316
43080b7
4fae6db
1322ee7
636ba91
6258c81
2560882
d25aa89
083e72d
e2d20fa
dec44e0
92002f7
a4fd2ca
dd35f67
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 |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| """Added provider column to the credential table | ||
|
|
||
| Revision ID: 904ed70e7dab | ||
| Revises: 543f97951bd0 | ||
| Create Date: 2025-05-10 11:13:17.868238 | ||
|
|
||
| """ | ||
| from alembic import op | ||
| import sqlalchemy as sa | ||
| import sqlmodel.sql.sqltypes | ||
|
|
||
|
|
||
| revision = "904ed70e7dab" | ||
| down_revision = "f23675767ed2" | ||
| branch_labels = None | ||
| depends_on = None | ||
|
|
||
|
|
||
| def upgrade(): | ||
| op.add_column( | ||
| "credential", | ||
| sa.Column("provider", sqlmodel.sql.sqltypes.AutoString(), nullable=False), | ||
| ) | ||
| op.create_index( | ||
| op.f("ix_credential_provider"), "credential", ["provider"], unique=False | ||
| ) | ||
| op.drop_constraint( | ||
| "credential_organization_id_fkey", "credential", type_="foreignkey" | ||
| ) | ||
| op.create_foreign_key( | ||
| "credential_organization_id_fkey", | ||
| "credential", | ||
| "organization", | ||
| ["organization_id"], | ||
| ["id"], | ||
| ondelete="CASCADE", | ||
| ) | ||
| op.drop_constraint("project_organization_id_fkey", "project", type_="foreignkey") | ||
| op.create_foreign_key(None, "project", "organization", ["organization_id"], ["id"]) | ||
|
|
||
| # Add project_id column and foreign key | ||
| op.add_column("credential", sa.Column("project_id", sa.Integer(), nullable=True)) | ||
| op.create_foreign_key( | ||
| "credential_project_id_fkey", | ||
| "credential", | ||
| "project", | ||
| ["project_id"], | ||
| ["id"], | ||
| ondelete="SET NULL", | ||
| ) | ||
|
|
||
|
|
||
| def downgrade(): | ||
| # Drop project_id foreign key and column | ||
| op.drop_constraint("credential_project_id_fkey", "credential", type_="foreignkey") | ||
| op.drop_column("credential", "project_id") | ||
|
|
||
| op.drop_constraint(None, "project", type_="foreignkey") | ||
| op.create_foreign_key( | ||
| "project_organization_id_fkey", | ||
| "project", | ||
| "organization", | ||
| ["organization_id"], | ||
| ["id"], | ||
| ondelete="CASCADE", | ||
| ) | ||
| op.drop_constraint( | ||
| "credential_organization_id_fkey", "credential", type_="foreignkey" | ||
| ) | ||
| op.create_foreign_key( | ||
| "credential_organization_id_fkey", | ||
| "credential", | ||
| "organization", | ||
| ["organization_id"], | ||
| ["id"], | ||
| ) | ||
| op.drop_index(op.f("ix_credential_provider"), table_name="credential") | ||
| op.drop_column("credential", "provider") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,127 +1,238 @@ | ||
| from typing import List | ||
|
|
||
| from fastapi import APIRouter, Depends, HTTPException | ||
| from sqlalchemy.exc import IntegrityError | ||
|
|
||
| from app.api.deps import SessionDep, get_current_active_superuser | ||
| from app.crud.credentials import ( | ||
| get_creds_by_org, | ||
| get_key_by_org, | ||
| get_provider_credential, | ||
| remove_creds_for_org, | ||
| set_creds_for_org, | ||
| update_creds_for_org, | ||
| remove_provider_credential, | ||
| ) | ||
| from app.models import CredsCreate, CredsPublic, CredsUpdate | ||
| from app.models.organization import Organization | ||
| from app.models.project import Project | ||
| from app.utils import APIResponse | ||
| from datetime import datetime | ||
| from app.core.providers import validate_provider | ||
|
|
||
| router = APIRouter(prefix="/credentials", tags=["credentials"]) | ||
|
|
||
|
|
||
| @router.post( | ||
| "/", | ||
| dependencies=[Depends(get_current_active_superuser)], | ||
| response_model=APIResponse[CredsPublic], | ||
| response_model=APIResponse[List[CredsPublic]], | ||
| summary="Create new credentials for an organization and project", | ||
| description="Creates new credentials for a specific organization and project combination. This endpoint requires superuser privileges. Each organization can have different credentials for different providers and projects. Only one credential per provider is allowed per organization-project combination.", | ||
| ) | ||
| def create_new_credential(*, session: SessionDep, creds_in: CredsCreate): | ||
| new_creds = None | ||
| try: | ||
| existing_creds = get_creds_by_org( | ||
| session=session, org_id=creds_in.organization_id | ||
| ) | ||
| if not existing_creds: | ||
| new_creds = set_creds_for_org(session=session, creds_add=creds_in) | ||
| # Check if organization exists | ||
| organization = session.get(Organization, creds_in.organization_id) | ||
| if not organization: | ||
| raise HTTPException(status_code=404, detail="Organization not found") | ||
|
|
||
| # Check if project exists if project_id is provided | ||
| if creds_in.project_id: | ||
| project = session.get(Project, creds_in.project_id) | ||
| if not project: | ||
| raise HTTPException(status_code=404, detail="Project not found") | ||
| if project.organization_id != creds_in.organization_id: | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail="Project does not belong to the specified organization", | ||
| ) | ||
|
|
||
| # Check for existing credentials for each provider | ||
| for provider in creds_in.credential.keys(): | ||
| existing_cred = get_provider_credential( | ||
| session=session, | ||
| org_id=creds_in.organization_id, | ||
| provider=provider, | ||
| project_id=creds_in.project_id, | ||
| ) | ||
| if existing_cred: | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail=f"Credentials for provider '{provider}' already exist for this organization and project combination", | ||
| ) | ||
|
|
||
| # Create new credentials | ||
| new_creds = set_creds_for_org(session=session, creds_add=creds_in) | ||
| if not new_creds: | ||
| raise HTTPException(status_code=500, detail="Failed to create credentials") | ||
| return APIResponse.success_response([cred.to_public() for cred in new_creds]) | ||
| except ValueError as e: | ||
| if "Unsupported provider" in str(e): | ||
| raise HTTPException(status_code=400, detail=str(e)) | ||
| raise HTTPException(status_code=404, detail=str(e)) | ||
| except Exception as e: | ||
| raise HTTPException( | ||
| status_code=500, detail=f"An unexpected error occurred: {str(e)}" | ||
| ) | ||
|
|
||
| # Ensure inserted_at is set during creation | ||
| new_creds.inserted_at = datetime.utcnow() | ||
|
|
||
| return APIResponse.success_response(new_creds) | ||
|
|
||
|
|
||
| @router.get( | ||
| "/{org_id}", | ||
| dependencies=[Depends(get_current_active_superuser)], | ||
| response_model=APIResponse[CredsPublic], | ||
| response_model=APIResponse[List[CredsPublic]], | ||
| summary="Get all credentials for an organization and project", | ||
| description="Retrieves all provider credentials associated with a specific organization and project combination. If project_id is not provided, returns credentials for the organization level. This endpoint requires superuser privileges.", | ||
| ) | ||
| def read_credential(*, session: SessionDep, org_id: int): | ||
| def read_credential(*, session: SessionDep, org_id: int, project_id: int | None = None): | ||
| try: | ||
| creds = get_creds_by_org(session=session, org_id=org_id) | ||
| creds = get_creds_by_org(session=session, org_id=org_id, project_id=project_id) | ||
| if not creds: | ||
| raise HTTPException(status_code=404, detail="Credentials not found") | ||
| return APIResponse.success_response([cred.to_public() for cred in creds]) | ||
| except HTTPException: | ||
|
||
| raise | ||
| except Exception as e: | ||
| raise HTTPException( | ||
| status_code=500, detail=f"An unexpected error occurred: {str(e)}" | ||
| ) | ||
|
|
||
| if creds is None: | ||
| raise HTTPException(status_code=404, detail="Credentials not found") | ||
|
|
||
| return APIResponse.success_response(creds) | ||
|
|
||
|
|
||
| @router.get( | ||
| "/{org_id}/api-key", | ||
| "/{org_id}/{provider}", | ||
| dependencies=[Depends(get_current_active_superuser)], | ||
| response_model=APIResponse[dict], | ||
| summary="Get specific provider credentials for an organization and project", | ||
| description="Retrieves credentials for a specific provider (e.g., 'openai', 'anthropic') for a given organization and project combination. If project_id is not provided, returns organization-level credentials. This endpoint requires superuser privileges.", | ||
| ) | ||
| def read_api_key(*, session: SessionDep, org_id: int): | ||
| def read_provider_credential( | ||
| *, session: SessionDep, org_id: int, provider: str, project_id: int | None = None | ||
| ): | ||
| try: | ||
| api_key = get_key_by_org(session=session, org_id=org_id) | ||
| provider_enum = validate_provider(provider) | ||
| provider_creds = get_provider_credential( | ||
| session=session, | ||
| org_id=org_id, | ||
| provider=provider_enum, | ||
| project_id=project_id, | ||
| ) | ||
| if provider_creds is None: | ||
| raise HTTPException( | ||
| status_code=404, detail="Provider credentials not found" | ||
| ) | ||
| return APIResponse.success_response(provider_creds) | ||
|
||
| except ValueError as e: | ||
| raise HTTPException(status_code=400, detail=str(e)) | ||
| except HTTPException: | ||
| raise | ||
| except Exception as e: | ||
| raise HTTPException( | ||
| status_code=500, detail=f"An unexpected error occurred: {str(e)}" | ||
| ) | ||
|
|
||
| if api_key is None: | ||
| raise HTTPException(status_code=404, detail="API key not found") | ||
|
|
||
| return APIResponse.success_response({"api_key": api_key}) | ||
|
|
||
|
|
||
| @router.patch( | ||
| "/{org_id}", | ||
| dependencies=[Depends(get_current_active_superuser)], | ||
| response_model=APIResponse[CredsPublic], | ||
| response_model=APIResponse[List[CredsPublic]], | ||
| summary="Update organization and project credentials", | ||
| description="Updates credentials for a specific organization and project combination. Can update specific provider credentials or add new providers. If project_id is provided in the update, credentials will be moved to that project. This endpoint requires superuser privileges.", | ||
| ) | ||
| def update_credential(*, session: SessionDep, org_id: int, creds_in: CredsUpdate): | ||
| try: | ||
| if not creds_in or not creds_in.provider or not creds_in.credential: | ||
| raise HTTPException( | ||
| status_code=400, detail="Provider and credential must be provided" | ||
| ) | ||
|
Comment on lines
+121
to
+124
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. Does this need to be in a try-except? |
||
| organization = session.get(Organization, org_id) | ||
| if not organization: | ||
| raise HTTPException(status_code=404, detail="Organization not found") | ||
| updated_creds = update_creds_for_org( | ||
| session=session, org_id=org_id, creds_in=creds_in | ||
| ) | ||
|
|
||
| updated_creds.updated_at = datetime.utcnow() | ||
|
|
||
| return APIResponse.success_response(updated_creds) | ||
| if not updated_creds: | ||
| raise HTTPException(status_code=404, detail="Failed to update credentials") | ||
| return APIResponse.success_response( | ||
| [cred.to_public() for cred in updated_creds] | ||
| ) | ||
| except IntegrityError as e: | ||
| if "ForeignKeyViolation" in str(e): | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail="Invalid organization ID. Ensure the organization exists before updating credentials.", | ||
| ) | ||
| raise HTTPException( | ||
| status_code=500, detail=f"An unexpected database error occurred: {str(e)}" | ||
| ) | ||
| except ValueError as e: | ||
| if "Unsupported provider" in str(e): | ||
| raise HTTPException(status_code=400, detail=str(e)) | ||
| raise HTTPException(status_code=404, detail=str(e)) | ||
| except HTTPException: | ||
| raise | ||
| except Exception as e: | ||
| raise HTTPException( | ||
| status_code=500, detail=f"An unexpected error occurred: {str(e)}" | ||
| ) | ||
|
|
||
|
|
||
| from fastapi import HTTPException, Depends | ||
| from app.crud.credentials import remove_creds_for_org | ||
| from app.utils import APIResponse | ||
| from app.api.deps import SessionDep, get_current_active_superuser | ||
|
|
||
|
|
||
| @router.delete( | ||
| "/{org_id}/api-key", | ||
| "/{org_id}/{provider}", | ||
| dependencies=[Depends(get_current_active_superuser)], | ||
| response_model=APIResponse[dict], | ||
| summary="Delete specific provider credentials for an organization and project", | ||
| description="Removes credentials for a specific provider while keeping other provider credentials intact. If project_id is provided, only removes credentials for that project. This endpoint requires superuser privileges.", | ||
| ) | ||
| def delete_credential(*, session: SessionDep, org_id: int): | ||
| def delete_provider_credential( | ||
| *, session: SessionDep, org_id: int, provider: str, project_id: int | None = None | ||
| ): | ||
| try: | ||
| creds = remove_creds_for_org(session=session, org_id=org_id) | ||
| provider_enum = validate_provider(provider) | ||
| updated_creds = remove_provider_credential( | ||
| session=session, | ||
| org_id=org_id, | ||
| provider=provider_enum, | ||
| project_id=project_id, | ||
| ) | ||
| if not updated_creds: | ||
| raise HTTPException( | ||
| status_code=404, detail="Provider credentials not found" | ||
| ) | ||
| return APIResponse.success_response( | ||
| {"message": "Provider credentials removed successfully"} | ||
| ) | ||
| except ValueError: | ||
| raise HTTPException(status_code=404, detail="Provider credentials not found") | ||
| except HTTPException: | ||
| raise | ||
| except Exception as e: | ||
| raise HTTPException( | ||
| status_code=500, detail=f"An unexpected error occurred: {str(e)}" | ||
| ) | ||
|
|
||
| if creds is None: | ||
|
|
||
| @router.delete( | ||
| "/{org_id}", | ||
| dependencies=[Depends(get_current_active_superuser)], | ||
| response_model=APIResponse[dict], | ||
| summary="Delete all credentials for an organization and project", | ||
| description="Removes all credentials for a specific organization and project combination. If project_id is provided, only removes credentials for that project. This is a soft delete operation that marks credentials as inactive. This endpoint requires superuser privileges.", | ||
| ) | ||
| def delete_all_credentials( | ||
| *, session: SessionDep, org_id: int, project_id: int | None = None | ||
| ): | ||
| try: | ||
| creds = remove_creds_for_org( | ||
| session=session, org_id=org_id, project_id=project_id | ||
| ) | ||
| if not creds: | ||
| raise HTTPException( | ||
| status_code=404, detail="Credentials for organization not found" | ||
| ) | ||
| return APIResponse.success_response( | ||
| {"message": "Credentials deleted successfully"} | ||
| ) | ||
| except HTTPException: | ||
| raise | ||
| except Exception as e: | ||
| raise HTTPException( | ||
| status_code=404, detail="Credentials for organization not found" | ||
| status_code=500, detail=f"An unexpected error occurred: {str(e)}" | ||
| ) | ||
|
|
||
| # No need to manually set deleted_at and is_active if it's done in remove_creds_for_org | ||
| # Simply return the success response | ||
| return APIResponse.success_response({"message": "Credentials deleted successfully"}) | ||
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.
Because of these outer exception handlers, and most notably because of the base exception handling here, none of the exceptions in the function body will be raised on their own. Notably, all HTTPExceptions raised in the function body will be cause by this parent exception, which will return 500 to the user. Because the body-exceptions raise different status codes, I imagine this -- raising 500 always -- was not the intention.