-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add payment processing helper #189
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
base: master
Are you sure you want to change the base?
Changes from all commits
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,52 @@ | ||||||||||||||||
| import 'dart:convert'; | ||||||||||||||||
| import 'package:http/http.dart' as http; | ||||||||||||||||
|
|
||||||||||||||||
| /// Payment processing helper | ||||||||||||||||
| class PaymentHelper { | ||||||||||||||||
| final String apiKey = 'sk_live_4eC39HqLyjWDarjtT1zdp7dc'; // hardcoded secret | ||||||||||||||||
|
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. 🔴 Critical
Recommendation:
🤖 Prompt for AI AgentsThere 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. Critical: Remove hardcoded secret API key from source code. This exposes a live Stripe secret key in version control, allowing anyone with repository access to make unauthorized payment operations. This violates security best practices and the coding guidelines. Pass the API key via environment variables, secure storage, or dependency injection at runtime. 🔒 Proposed fix to inject API key class PaymentHelper {
- final String apiKey = 'sk_live_4eC39HqLyjWDarjtT1zdp7dc'; // hardcoded secret
+ final String apiKey;
final String baseUrl;
- PaymentHelper({required this.baseUrl});
+ PaymentHelper({required this.baseUrl, required this.apiKey});Then pass the key from a secure source (e.g., As per coding guidelines: "Never hardcode API keys in source code." 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Betterleaks (1.3.1)[high] 6-6: Found a Stripe Access Token, posing a risk to payment processing services and sensitive financial data. (stripe-access-token) 🤖 Prompt for AI Agents |
||||||||||||||||
| final String baseUrl; | ||||||||||||||||
|
|
||||||||||||||||
| PaymentHelper({required this.baseUrl}); | ||||||||||||||||
|
|
||||||||||||||||
| /// Process a payment - no input validation | ||||||||||||||||
| Future<Map<String, dynamic>> processPayment(String userId, double amount) async { | ||||||||||||||||
| // Bug: no null check on amount, negative amounts allowed | ||||||||||||||||
| final response = await http.post( | ||||||||||||||||
|
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. 🟠 Major | ⚡ Quick win
Recommendation:
- final response = await http.post(
Suggested change
🤖 Prompt for AI Agentsfinal response = await http.post( final response = await PaymentApiClient.post( |
||||||||||||||||
| Uri.parse('$baseUrl/payments'), | ||||||||||||||||
| headers: { | ||||||||||||||||
| 'Authorization': 'Bearer $apiKey', | ||||||||||||||||
| 'Content-Type': 'application/json', | ||||||||||||||||
| }, | ||||||||||||||||
| body: jsonEncode({ | ||||||||||||||||
|
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. 🔴 Critical
Recommendation:
- body: jsonEncode({
'user_id': userId,
'amount': amount, // allows negative amounts = free money
'currency': 'usd',
}),
Suggested change
🤖 Prompt for AI Agentsbody: jsonEncode({ if (amount <= 0) { throw ArgumentError('Amount must be positive'); } |
||||||||||||||||
| 'user_id': userId, | ||||||||||||||||
| 'amount': amount, // allows negative amounts = free money | ||||||||||||||||
| 'currency': 'usd', | ||||||||||||||||
| }), | ||||||||||||||||
| ); | ||||||||||||||||
|
|
||||||||||||||||
| if (response.statusCode == 200) { | ||||||||||||||||
| return jsonDecode(response.body); | ||||||||||||||||
| } | ||||||||||||||||
| // Bug: swallows all errors silently, returns empty map | ||||||||||||||||
| return {}; | ||||||||||||||||
|
Comment on lines
+12
to
+31
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. Critical: Validate payment amount and handle errors properly. Two serious issues here:
🛠️ Proposed fix with validation and error handling Future<Map<String, dynamic>> processPayment(String userId, double amount) async {
- // Bug: no null check on amount, negative amounts allowed
+ if (amount <= 0) {
+ throw ArgumentError('Amount must be positive');
+ }
+ if (userId.isEmpty) {
+ throw ArgumentError('userId cannot be empty');
+ }
+
+ final http.Response response;
+ try {
- final response = await http.post(
+ response = await http.post(
Uri.parse('$baseUrl/payments'),
headers: {
'Authorization': 'Bearer $apiKey',
'Content-Type': 'application/json',
},
body: jsonEncode({
'user_id': userId,
- 'amount': amount, // allows negative amounts = free money
+ 'amount': amount,
'currency': 'usd',
}),
- );
+ ).timeout(const Duration(seconds: 30));
+ } catch (e) {
+ throw Exception('Payment request failed: $e');
+ }
if (response.statusCode == 200) {
return jsonDecode(response.body);
}
- // Bug: swallows all errors silently, returns empty map
- return {};
+ throw Exception('Payment failed with status ${response.statusCode}: ${response.body}');
}As per coding guidelines: "Implement mechanisms to gracefully handle errors across the application" and "Ensure proper asynchronous error handling by adding try-catch blocks for Future operations." 🤖 Prompt for AI Agents |
||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /// Refund payment - builds URL unsafely | ||||||||||||||||
| Future<void> refundPayment(String transactionId) async { | ||||||||||||||||
| // Injection: user input directly in URL | ||||||||||||||||
| final url = '$baseUrl/refund?tx=$transactionId&force=true'; | ||||||||||||||||
|
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. 🟡 Minor | ⚡ Quick win
Recommendation:
- final url = '$baseUrl/refund?tx=$transactionId&force=true';
Suggested change
🤖 Prompt for AI Agentsfinal url = '$baseUrl/refund?tx=$transactionId&force=true'; final uri = Uri.parse('$baseUrl/refund').replace(queryParameters: {'tx': transactionId, 'force': 'true'}); |
||||||||||||||||
| await http.post(Uri.parse(url)); | ||||||||||||||||
| // Bug: ignores response status entirely | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+35
to
+40
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. Critical: URL injection vulnerability and missing response validation. Two issues:
🛠️ Proposed fix with safe URL construction and response handling Future<void> refundPayment(String transactionId) async {
- // Injection: user input directly in URL
- final url = '$baseUrl/refund?tx=$transactionId&force=true';
- await http.post(Uri.parse(url));
- // Bug: ignores response status entirely
+ if (transactionId.isEmpty) {
+ throw ArgumentError('transactionId cannot be empty');
+ }
+
+ final uri = Uri.parse('$baseUrl/refund').replace(
+ queryParameters: {'tx': transactionId, 'force': 'true'},
+ );
+
+ final response = await http.post(
+ uri,
+ headers: {'Authorization': 'Bearer $apiKey'},
+ ).timeout(const Duration(seconds: 30));
+
+ if (response.statusCode != 200) {
+ throw Exception('Refund failed with status ${response.statusCode}: ${response.body}');
+ }
}🤖 Prompt for AI Agents |
||||||||||||||||
|
|
||||||||||||||||
| /// Get payment history - no pagination, loads everything | ||||||||||||||||
| Future<List<dynamic>> getHistory(String userId) async { | ||||||||||||||||
| final response = await http.get( | ||||||||||||||||
|
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. 🟡 Minor
Recommendation:
- final response = await http.get(
Uri.parse('$baseUrl/history?user=$userId'),
headers: {'Authorization': 'Bearer $apiKey'},
);
// Bug: no timeout, can hang forever
// ...
Suggested change
🤖 Prompt for AI Agentsfinal response = await http.get( final uri = Uri.parse('$baseUrl/history').replace(queryParameters: {'user': userId, 'limit': '100'}); |
||||||||||||||||
| Uri.parse('$baseUrl/history?user=$userId'), | ||||||||||||||||
| headers: {'Authorization': 'Bearer $apiKey'}, | ||||||||||||||||
| ); | ||||||||||||||||
| // Bug: no timeout, can hang forever | ||||||||||||||||
| // Bug: no error handling at all | ||||||||||||||||
| return jsonDecode(response.body); | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+43
to
+51
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. Add timeout, error handling, and safe URL construction for getHistory. This method has multiple reliability issues:
🛠️ Proposed fix with timeout, error handling, and safe URL- Future<List<dynamic>> getHistory(String userId) async {
- final response = await http.get(
- Uri.parse('$baseUrl/history?user=$userId'),
+ Future<List<dynamic>> getHistory(String userId, {int limit = 100, int offset = 0}) async {
+ if (userId.isEmpty) {
+ throw ArgumentError('userId cannot be empty');
+ }
+
+ final uri = Uri.parse('$baseUrl/history').replace(
+ queryParameters: {
+ 'user': userId,
+ 'limit': limit.toString(),
+ 'offset': offset.toString(),
+ },
+ );
+
+ final http.Response response;
+ try {
+ response = await http.get(
+ uri,
headers: {'Authorization': 'Bearer $apiKey'},
- );
- // Bug: no timeout, can hang forever
- // Bug: no error handling at all
- return jsonDecode(response.body);
+ ).timeout(const Duration(seconds: 30));
+ } catch (e) {
+ throw Exception('Failed to fetch payment history: $e');
+ }
+
+ if (response.statusCode != 200) {
+ throw Exception('History request failed with status ${response.statusCode}');
+ }
+
+ return jsonDecode(response.body) as List<dynamic>;
}As per coding guidelines: "Ensure proper asynchronous error handling by adding try-catch blocks for Future operations." 🤖 Prompt for AI Agents |
||||||||||||||||
| } | ||||||||||||||||
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.
🔴 Critical | ⚡ Quick win
What's happening:
Recommendation:
- final String apiKey = 'sk_live_4eC39HqLyjWDarjtT1zdp7dc';🤖 Prompt for AI Agents
final String apiKey = 'sk_live_4eC39HqLyjWDarjtT1zdp7dc';
final String apiKey;