caching of key fixed and email get sent when encrypted chat is started #3972
Conversation
…o direct-messagse
WalkthroughThis pull request streamlines the public key fetching method in the messaging template by directly updating the cached key. It adds a new HTML template for new encrypted chat email notifications and updates the thread creation view to send out these email notifications when a new messaging thread is initiated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Server
participant EmailService
User->>Server: Request to start new messaging thread
alt No existing thread
Server->>Server: Create new thread
end
alt Recipient has an email address
Server->>EmailService: Invoke send_mail with plain text & HTML templates (including new_chat.html)
EmailService-->>Server: Email sent confirmation
end
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
|
@coderabbitai does this meets the requirements of #3864 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 74249 🏁 Script executed: Length of output: 2904 @krrish-sehgal Yes, the changes in PR #3972 do meet the requirements of issue #3864. Looking at issue #3864 "Remove caching of recepient public key, or change it as soon as the chat is changed", I can see that your PR properly addresses this by modifying the messaging.html file to always fetch the recipient's public key when loading messages, rather than relying on a cached version. Specifically, you changed the code from:
This change ensures that the encryption/decryption process always uses the most up-to-date public key, which should prevent the "Error decrypting message: OperationError" mentioned in the issue. The implementation is clean and focused on the specific problem, making it a good solution for the reported issue. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
website/templates/messaging.html (1)
336-340: Refined Key Fetching Logic inopenChatThe updated code now unconditionally fetches and updates the recipient's public key from the server before loading messages. This change streamlines the process by eliminating the previous conditional check, ensuring that the most up-to-date key is always used in this context.
Please verify that this behavior is intended solely for the message-loading workflow in
openChat. Notably, the "send message" event handler (lines 378–384) still uses a conditional check to fetch the key. If the goal is to consistently avoid caching in favor of always retrieving a fresh key, you might consider aligning that logic as well or documenting why the approaches differ.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/templates/messaging.html(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run Tests
- GitHub Check: docker-test
- GitHub Check: Analyze (javascript-typescript)
|
@coderabbitai can you help me point out how can i fix #3860 in this PR itself, by pointing out the right pre exisiting email code |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1400 🏁 Script executed: Length of output: 6383 🏁 Script executed: Length of output: 74391 🏁 Script executed: Length of output: 22952 🏁 Script executed: Length of output: 6896 @krrish-sehgal Looking at issue #3860, I'll help you implement email notifications when someone starts an encrypted chat. Let me show you how to add this feature to the current PR: First, I need to see the current implementation of the messaging consumer where new messages are saved: 🏁 Script executed: Length of output: 341 🏁 Script executed: Length of output: 8998 🏁 Script executed: Length of output: 3980 🏁 Script executed: Length of output: 4963 @krrish-sehgal Based on my analysis, I can see that to implement issue #3860 (sending an email when someone starts an encrypted chat), we need to modify the Here's how to modify the function to send an email notification:
from django.core.mail import send_mail
from django.template.loader import render_to_string
from django.urls import reverse
from django.conf import settings
def start_thread(request, user_id):
if request.method == "POST":
other_user = get_object_or_404(User, id=user_id)
# Check if a thread already exists between the two users
thread = Thread.objects.filter(participants=request.user).filter(participants=other_user).first()
# Flag if this is a new thread (for sending email)
is_new_thread = not thread
if not thread:
# Create a new thread
thread = Thread.objects.create()
thread.participants.set([request.user, other_user]) # Use set() for ManyToManyField
# Send email notification to the recipient for new thread
if other_user.email:
subject = f"New encrypted chat from {request.user.username} on OWASP BLT"
chat_url = request.build_absolute_uri(reverse('messaging'))
# Create context for the email template
context = {
'sender_username': request.user.username,
'recipient_username': other_user.username,
'chat_url': chat_url,
}
# Render the email content
msg_plain = render_to_string('email/new_chat.txt', context)
msg_html = render_to_string('email/new_chat.html', context)
# Send the email
send_mail(
subject,
msg_plain,
settings.EMAIL_TO_STRING,
[other_user.email],
html_message=msg_html,
)
return JsonResponse({"success": True, "thread_id": thread.id})
return JsonResponse({"success": False, "error": "Invalid request"}, status=400)
Create a file Create a file <!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>New Encrypted Chat</title>
</head>
<body style="font-family: Arial, sans-serif; line-height: 1.6; color: #333; margin: 0; padding: 20px;">
<div style="max-width: 600px; margin: 0 auto;">
<div style="background-color: #e74c3c; color: white; padding: 10px 20px; text-align: center;">
<h1>New Encrypted Chat</h1>
</div>
<div style="padding: 20px; border: 1px solid #ddd; border-top: none;">
<p>Hello {{ recipient_username }},</p>
<p><strong>{{ sender_username }}</strong> has started a new encrypted chat with you on OWASP BLT.</p>
<p>Click the button below to view and respond to the conversation:</p>
<p style="text-align: center;">
<a href="{{ chat_url }}" style="display: inline-block; background-color: #e74c3c; color: white; text-decoration: none; padding: 10px 20px; border-radius: 5px;">Open Chat</a>
</p>
<p>This is a secure chat with end-to-end encryption. Only you and {{ sender_username }} can read the messages.</p>
<p>Regards,<br>The OWASP BLT Team</p>
</div>
<div style="margin-top: 20px; font-size: 12px; color: #777; text-align: center;">
<p>If you're having trouble with the button above, copy and paste this link into your browser: {{ chat_url }}</p>
</div>
</div>
</body>
</html>This implementation will send an email notification to the recipient when someone initiates a new encrypted chat with them, providing a link to open the conversation. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
website/views/user.py (1)
1068-1091: Email notification implementation is well structuredThe email notification logic is implemented clearly with appropriate context variables for the templates. However, I suggest adding error handling around the email sending process to prevent potential failures from affecting the thread creation.
Consider adding a try-except block around the email sending process:
# Send email notification to the recipient for new thread if other_user.email: + try: subject = f"New encrypted chat from {request.user.username} on OWASP BLT" chat_url = request.build_absolute_uri(reverse('messaging')) # Create context for the email template context = { 'sender_username': request.user.username, 'recipient_username': other_user.username, 'chat_url': chat_url, } # Render the email content msg_plain = render_to_string('email/new_chat.txt', context) msg_html = render_to_string('email/new_chat.html', context) # Send the email send_mail( subject, msg_plain, settings.EMAIL_TO_STRING, [other_user.email], html_message=msg_html, ) + except Exception as e: + logger.error(f"Failed to send email notification for new chat thread: {e}") + # Thread is still created even if email fails
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
website/templates/email/new_chat.html(1 hunks)website/templates/email/new_chat.txt(1 hunks)website/views/user.py(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- website/templates/email/new_chat.txt
- website/templates/email/new_chat.html
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run Tests
- GitHub Check: docker-test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
website/views/user.py (3)
1058-1067: Good implementation of thread creation logic and new thread trackingThe logic to check for an existing thread between users and create a new one if needed is implemented correctly. Using a flag to track whether this is a new thread is a clean approach for determining when to send notifications.
1072-1078: Well-designed context dictionary for email templatesThe context dictionary provides all necessary information for a helpful email notification, including both usernames and the chat URL.
1081-1091:✅ Verification successful
Proper use of template rendering for both plain text and HTML emails
Using both plain text and HTML templates follows best practices for email compatibility across different email clients.
Please verify that the email templates exist in the expected locations:
🏁 Script executed:
#!/bin/bash # Check if email templates exist echo "Checking for email templates..." fd "new_chat\.(txt|html)" --exec echo "Found template: {}"Length of output: 244
Templates verified – code approved
The email templates were confirmed to exist at:
./website/templates/email/new_chat.txt./website/templates/email/new_chat.htmlThe current implementation in
website/views/user.py(lines 1081-1091) properly utilizes both plain text and HTML template rendering for enhanced email compatibility. No further changes are required.
krrish-sehgal
changed the title
|
@krrish-sehgal please check the tests |
2 participants
Fixed #3864 and #3860
Summary by CodeRabbit
New Features
Refactor
Summary by CodeRabbit