Conversation
…sting for invalid payload
…s to separate providers
…for updating provider credentials before sending it to the endpoint.
There was a problem hiding this comment.
Pull Request Overview
This PR implements a unified secrets verification endpoint that validates and stores cloud provider credentials. It replaces separate provider-specific endpoints with a single endpoint that verifies credentials against the provider account before storage.
- Unified single
/api/v1/users/me/secretsendpoint for all cloud providers - Added credential verification using provider SDKs (boto3 for AWS)
- Refactored frontend to use tabbed interface for different providers
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/routes/settings/+page.svelte | Converted from side-by-side layout to tabbed interface for cloud credentials |
| frontend/src/lib/types/api.ts | Updated API types to use unified secrets request/response schemas |
| frontend/src/lib/api.ts | Replaced provider-specific API calls with unified updateSecrets function |
| cli/internal/client/auth.go | Updated CLI client to use new unified secrets endpoint |
| api/src/app/schemas/secret_schema.py | Added validation for AWS credential format and length |
| api/src/app/schemas/creds_verify_schema.py | New schema for credential verification requests |
| api/src/app/crud/crud_secrets.py | New CRUD operations for user secrets management |
| api/src/app/cloud/ | New credential verification framework with AWS implementation |
| api/src/app/api/v1/users.py | Replaced separate endpoints with unified secrets verification endpoint |
| api/requirements.txt | Added boto3 dependency for AWS verification |
| api/tests/ | Comprehensive test coverage for new verification functionality |
Comments suppressed due to low confidence (1)
frontend/src/lib/api.ts:163
- The parameter name 'credentialprovider' should be renamed to 'provider' for consistency with the request schema and better readability.
updateSecrets: async (credentialprovider: string, payload: any): Promise<ApiResponse<SecretsResponse>> => {
|
Great work! This will really help with preventing a whole bunch of user errors, especially when they are onboarding for the first time |
Nareshp1
added
backend
… to reflect purpose.
…ts discriminated union.
alexchristy
left a comment
alexchristy
left a comment
There was a problem hiding this comment.
When testing the frontend, the error message breaks when the credentials aren't the correct length. When one or the other credential entry it gives just one [object Object] and when both are incorrect length you get two of them.
|
Fixed issue with error output (specifically with the pydantic errors that are returned with invalid payloads). When the schema validation passes, the regular error messages returned from the endpoint also work still. |
…rs when submitting credentials to properly extract and return the error messages to the user.
alexchristy
left a comment
alexchristy
left a comment
There was a problem hiding this comment.
Just tested and everything is working great!
|
I can't remember why I didn't approve this. Maybe I just got busy |
Checklist
Description
Created a single, unified endpoint for adding secrets to a user account. Based on the provider specified, the endpoint will attempt to authenticate to the provider account the secrets are linked to and check that the minimum permissions (if needed, ex. cloud providers) are present before storing the secrets in the database for the user.
Fixes: #69