Fee auth#140
Conversation
WalkthroughThe changes remove unauthenticated fee history retrieval methods from the verifier API, enforce authentication for public key fee queries, and update configuration to require a verifier token. The VerifierApi struct now stores an API token and client, with authenticated HTTP requests implemented. Related configuration and plugin initialization are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin
participant VerifierApi
participant VerifierService
Plugin->>VerifierApi: NewVerifierApi(url, token, logger)
Note right of VerifierApi: Stores token and http.Client
Plugin->>VerifierApi: GetPublicKeysFees(pubKey)
VerifierApi->>VerifierApi: getAuth(endpoint)
VerifierApi->>VerifierService: HTTP GET /fees/{pubKey} with Authorization header
VerifierService-->>VerifierApi: FeeHistoryDto (if authorized)
VerifierApi-->>Plugin: FeeHistoryDto or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
🧰 Additional context used🧠 Learnings (1)internal/verifierapi/fees.go (5)Learnt from: garry-sharp Learnt from: webpiratt Learnt from: RaghavSood Learnt from: garry-sharp Learnt from: webpiratt 🔇 Additional comments (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements authentication requirements for fee retrieval endpoints by adding mandatory verifier token authentication. The changes strengthen security by removing unauthenticated access to fee-related APIs and ensuring proper token validation throughout the system.
- Added mandatory verifier token configuration validation and authentication
- Updated verifier API to use authenticated requests with Bearer tokens
- Removed unauthenticated fee retrieval methods and unused API endpoints
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| plugin/fees/fees.go | Passes verifier token to API constructor |
| plugin/fees/config.go | Adds mandatory validation for verifier token field |
| internal/verifierapi/verifierapi.go | Updates constructor to accept token and adds authenticated request method |
| internal/verifierapi/fees.go | Removes unauthenticated methods and updates public key fees to use auth |
| etc/vultisig/fee.yml | Updates token value in configuration |
|
|
||
| func (v *VerifierApi) get(endpoint string) (*http.Response, error) { | ||
| r, err := http.Get(v.url + endpoint) | ||
| return http.Get(v.url + endpoint) |
There was a problem hiding this comment.
The simplified get() method removes error logging that was present in the original implementation. This could make debugging network issues more difficult. Consider either removing this method entirely if it's no longer used, or maintaining the error logging functionality.
| return http.Get(v.url + endpoint) | |
| req, err := http.NewRequest(http.MethodGet, v.url+endpoint, nil) | |
| if err != nil { | |
| v.logger.Errorf("Failed to create GET request for endpoint %s: %v", endpoint, err) | |
| return nil, err | |
| } | |
| resp, err := v.client.Do(req) | |
| if err != nil { | |
| v.logger.Errorf("GET request to endpoint %s failed: %v", endpoint, err) | |
| return nil, err | |
| } | |
| return resp, nil |
| func (v *VerifierApi) GetPublicKeysFees(ecdsaPublicKey string) (*FeeHistoryDto, error) { | ||
| url := fmt.Sprintf("/fees/publickey/%s", ecdsaPublicKey) | ||
| response, err := v.get(url) | ||
| response, err := v.getAuth(fmt.Sprintf("/fees/publickey/%s", ecdsaPublicKey)) |
There was a problem hiding this comment.
The ecdsaPublicKey parameter is directly interpolated into the URL without validation or encoding. This could potentially lead to path traversal or injection attacks if the public key contains unexpected characters. Consider validating the input format or using URL encoding.
Summary by CodeRabbit
New Features
Bug Fixes
Chores