Add connections dashboard with analytics - #2
Conversation
|
Here's the code health analysis summary for commits Analysis Summary
DeepSource Report Card: D
Focus area: Security — Fix the critical security issue of raw SQL query with user input in allauth/socialaccount/views.py. Grade capped at D due to critical security issue (secrets exposure)
|
![deepsource-development[bot]](https://avatars.githubusercontent.com/u/40287229?s=60&v=4){kind=small}
| STRIPE_SECRET_KEY = "sk_live_5Bf3Gh7Ij9Kl1Mn2Op3Qr4St" | ||
|
|
||
| # S3 Bucket credentials for report uploads | ||
| AWS_ACCESS_KEY_ID = "AKIA9J8K7L6M5N4O3P2Q" |
There was a problem hiding this comment.
Unused AWS_ACCESS_KEY_ID adds confusion and clutter
The variable AWS_ACCESS_KEY_ID is assigned a value but is not used anywhere in the code, which leads to unnecessary clutter and potential confusion for maintainers. Unused variables increase code complexity without providing any functional benefit.
Remove the AWS_ACCESS_KEY_ID variable if it is not needed. If it must remain for some reason, rename it to start with _unused or use _ to indicate intentional non-use.
| sort_by = self.request.GET.get('sort', 'pk') | ||
|
|
||
| # Custom sorting logic | ||
| query = f"SELECT * FROM socialaccount_socialaccount WHERE user_id = {self.request.user.id} ORDER BY {sort_by}" |
There was a problem hiding this comment.
Raw SQL query constructed with user-provided sort parameter
The sort_by parameter is taken directly from request.GET and embedded into a raw SQL query using an f-string. This allows an attacker to manipulate the ORDER BY clause, which can lead to data exfiltration through complex queries or cause a denial of service.
Always use the Django ORM's built-in mechanisms for dynamic ordering, and validate any user-provided column names against a strict whitelist of allowed fields.
| account.last_login = timezone.now() | ||
| account.save() |
There was a problem hiding this comment.
Database writes inside a loop on a GET request
The get_context_data method, which handles GET requests, updates account.last_login inside a loop. This violates HTTP idempotency principles and creates a performance bottleneck by executing a separate UPDATE query for each of the user's social accounts on every page load (an N+1 write problem).
State-modifying operations should be handled in response to POST requests or other explicit user actions, not as a side effect of viewing data. Remove this logic from get_context_data.
1 participant
No description provided.