From 1390b1c627b59c3b15aa6b57ff71021792b2bb65 Mon Sep 17 00:00:00 2001
From: "Chinmay D. Pai" <chinmaydpai@gmail.com>
Date: Thu, 2 Jul 2020 17:43:32 +0530
Subject: [PATCH 1/4] fix: limit commenting to logged in accounts

* do not allow users not logged in to comment
* add an hourly limit of 20 comments per user
* add better checks for urls and email

Signed-off-by: Chinmay D. Pai <chinmaydpai@gmail.com>
---
 .../templates/includes/comments/comments.html | 70 +++++++------------
 .../templates/includes/comments/comments.py   | 55 +++++++++------
 frappe/website/doctype/web_form/web_form.py   |  6 +-
 3 files changed, 61 insertions(+), 70 deletions(-)

diff --git a/frappe/templates/includes/comments/comments.html b/frappe/templates/includes/comments/comments.html
index ef53effe53..87241ed075 100644
--- a/frappe/templates/includes/comments/comments.html
+++ b/frappe/templates/includes/comments/comments.html
@@ -9,10 +9,10 @@
 	{% endif %}
 
 	<div itemscope itemtype="http://schema.org/UserComments" id="comment-list">
-        {% for comment in comment_list %}
-        <div class="my-3">
-            {% include "templates/includes/comments/comment.html" %}
-        </div>
+		{% for comment in comment_list %}
+		<div class="my-3">
+			{% include "templates/includes/comments/comment.html" %}
+		</div>
 		{% endfor %}
 	</div>
 </div>
@@ -25,26 +25,22 @@
 
 	<div class="comment-form-wrapper">
 		<a class="add-comment btn btn-light btn-sm">{{ _("Add Comment") }}</a>
-			<div style="display: none;" id="comment-form">
+		<div style="display: none;" id="comment-form">
 			<p>{{ _("Leave a Comment") }}</p>
 			<div class="alert" style="display:none;"></div>
 			<form>
 				<fieldset>
-					<div class="row {% if _login_required %} hidden {% endif %}"
-						style="margin-bottom: 15px;">
+					<div class="row" style="margin-bottom: 15px;">
 						<div class="col-sm-6">
-							<input class="form-control comment_by" name="comment_by"
-								placeholder="{{ _("Your Name") }}" type="text">
+							<input class="form-control comment_by" name="comment_by" placeholder="{{ _("Your Name") }}" type="text">
 						</div>
 						<div class="col-sm-6">
-							<input class="form-control comment_email" name="comment_email"
-								placeholder="{{ _("Your Email Address") }}" type="email">
+							<input class="form-control comment_email" name="comment_email" placeholder="{{ _("Your Email Address") }}" type="email">
 						</div>
 					</div>
 					<p><textarea class="form-control" name="comment" rows=10
-						placeholder="{{ _("Comment") }}"></textarea></p>
-                        <button class="btn btn-primary btn-sm" id="submit-comment" style="margin-top:10px">
-                            {{ _("Submit") }}</button>
+				       placeholder="{{ _("Comment") }}"></textarea></p>
+					<button class="btn btn-primary btn-sm" id="submit-comment" style="margin-top:10px">{{ _("Submit") }}</button>
 				</fieldset>
 			</form>
 		</div>
@@ -53,13 +49,9 @@
 {% endif %}
 <script>
 	frappe.ready(function() {
-		var login_required = {{ login_required and 1 or 0 }};
-
-		if (login_required && !frappe.is_user_logged_in()) {
+		if (!frappe.is_user_logged_in()) {
 			$(".login-required, .comment-form-wrapper").toggleClass("hidden");
-		}
-
-		if(frappe.is_user_logged_in()) {
+		} else {
 			$('input.comment_by').prop("disabled", true);
 			$('input.comment_email').prop("disabled", true);
 		}
@@ -75,18 +67,18 @@
 		}
 		$(".add-comment").click(function() {
 			$(this).toggle(false);
-		 	$("#comment-form").toggle();
-            var full_name = "", user_id = "";
-            if(frappe.is_user_logged_in()) {
-                full_name = frappe.get_cookie("full_name");
-                user_id = frappe.get_cookie("user_id");
-                if(user_id != "Guest") {
-        			$("[name='comment_email']").val(user_id);
-        			$("[name='comment_by']").val(full_name);
-                }
-            }
+			$("#comment-form").toggle();
+			var full_name = "", user_id = "";
+			if(frappe.is_user_logged_in()) {
+				full_name = frappe.get_cookie("full_name");
+				user_id = frappe.get_cookie("user_id");
+				if(user_id != "Guest") {
+					$("[name='comment_email']").val(user_id);
+					$("[name='comment_by']").val(full_name);
+				}
+			}
 			$("#comment-form textarea").val("");
-			})
+		})
 
 		$("#submit-comment").click(function() {
 			var args = {
@@ -99,13 +91,8 @@
 				route: "{{ pathname }}",
 			}
 
-			if(!args.comment_by || !args.comment_email || !args.comment) {
-				frappe.msgprint("{{ _("All fields are necessary to submit the comment.") }}");
-				return false;
-			}
-
-			if (args.comment_email!=='Administrator' && !validate_email(args.comment_email)) {
-				frappe.msgprint("{{ _("Please enter a valid email address.") }}");
+			if(!args.comment) {
+				frappe.msgprint("{{ _("Please add a valid comment.") }}");
 				return false;
 			}
 
@@ -121,15 +108,12 @@
 					} else {
 						if (r.message) {
 							$(r.message).appendTo("#comment-list");
-						} else {
-							// probably spam
-							frappe.msgprint('{{ _("Thank you for your comment. It will be published after approval") }}');
+							$(".add-comment").text(__("Add Another Comment"));
 						}
 						$(".no-comment, .add-comment").toggle(false);
 						$("#comment-form").toggle();
+						$(".add-comment").toggle();
 					}
-					$(".add-comment").text(__("Add Another Comment"));
-					$(".add-comment").toggle();
 				}
 			})
 
diff --git a/frappe/templates/includes/comments/comments.py b/frappe/templates/includes/comments/comments.py
index b5a366e63f..f58201ebdd 100644
--- a/frappe/templates/includes/comments/comments.py
+++ b/frappe/templates/includes/comments/comments.py
@@ -3,29 +3,45 @@
 from __future__ import unicode_literals
 
 import frappe
-import frappe.utils
+import re
 from frappe.website.render import clear_cache
+from frappe.utils import add_to_date, now
 
 from frappe import _
 
-@frappe.whitelist(allow_guest=True)
+@frappe.whitelist()
 def add_comment(comment, comment_email, comment_by, reference_doctype, reference_name, route):
 	doc = frappe.get_doc(reference_doctype, reference_name)
 
-	if len(comment) < 10:
-		frappe.msgprint(_('Comment Should be atleast 10 characters'))
-		return ''
+	if not comment.strip() or len(comment) < 10:
+		frappe.msgprint(_('Your comment should be atleast 10 characters long'))
+		return False
 
-	blacklist = ['http://', 'https://', '@gmail.com']
+	url_regex = re.compile(r"http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\(\),]|(?:%[0-9a-fA-F][0-9a-fA-F]))+", re.IGNORECASE)
+	email_regex = re.compile(r"(^[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+$)", re.IGNORECASE)
 
-	if any([b in comment for b in blacklist]):
+	if url_regex.match(comment) or email_regex.match(comment):
 		frappe.msgprint(_('Comments cannot have links or email addresses'))
-		return ''
+		return False
+
+	if not comment_email == frappe.session.user:
+		frappe.msgprint(_('Please comment as the logged in user'))
+		return False
+
+	comments_count = frappe.db.count("Comment", {
+		"comment_type": "Comment",
+		"comment_email": frappe.session.user,
+		"creation": (">", add_to_date(now(), hours=-1))
+	})
+
+	if comments_count > 20:
+		frappe.msgprint(_('Hourly comment limit reached for: {0}').format(frappe.bold(frappe.session.user)))
+		return False
 
 	comment = doc.add_comment(
-		text = comment,
-		comment_email = comment_email,
-		comment_by = comment_by)
+		text=comment,
+		comment_email=comment_email,
+		comment_by=comment_by)
 
 	comment.db_set('published', 1)
 
@@ -40,18 +56,13 @@ def add_comment(comment, comment_email, comment_by, reference_doctype, reference
 
 	# notify creator
 	frappe.sendmail(
-		recipients = frappe.db.get_value('User', doc.owner, 'email') or doc.owner,
-		subject = _('New Comment on {0}: {1}').format(doc.doctype, doc.name),
-		message = content,
+		recipients=frappe.db.get_value('User', doc.owner, 'email') or doc.owner,
+		subject=_('New Comment on {0}: {1}').format(doc.doctype, doc.name),
+		message=content,
 		reference_doctype=doc.doctype,
 		reference_name=doc.name
 	)
 
-	if comment.published:
-		# revert with template if all clear (no backlinks)
-		template = frappe.get_template("templates/includes/comments/comment.html")
-
-		return template.render({"comment": comment.as_dict()})
-
-	else:
-		return ''
+	# revert with template if all clear (no backlinks)
+	template = frappe.get_template("templates/includes/comments/comment.html")
+	return template.render({"comment": comment.as_dict()})
diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py
index 4fc3c99037..a3867f1396 100644
--- a/frappe/website/doctype/web_form/web_form.py
+++ b/frappe/website/doctype/web_form/web_form.py
@@ -122,10 +122,6 @@ def get_context(context):
 		'''Build context to render the `web_form.html` template'''
 		self.set_web_form_module()
 
-		context._login_required = False
-		if self.login_required and frappe.session.user == "Guest":
-			context._login_required = True
-
 		doc, delimeter = make_route_string(frappe.form_dict)
 		context.doc = doc
 		context.delimeter = delimeter
@@ -142,7 +138,7 @@ def get_context(context):
 		if self.is_standard:
 			self.use_meta_fields()
 
-		if not context._login_required:
+		if not frappe.session.user == "Guest":
 			if self.allow_edit:
 				if self.allow_multiple:
 					if not frappe.form_dict.name and not frappe.form_dict.new:

From 109c7a1ce8f763422d732a3a8d655b4719f2ef5e Mon Sep 17 00:00:00 2001
From: "Chinmay D. Pai" <chinmaydpai@gmail.com>
Date: Thu, 2 Jul 2020 19:18:13 +0530
Subject: [PATCH 2/4] fix: remove comment length restriction and explicitly set
 user

Signed-off-by: Chinmay D. Pai <chinmaydpai@gmail.com>
---
 frappe/templates/includes/comments/comments.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/frappe/templates/includes/comments/comments.py b/frappe/templates/includes/comments/comments.py
index f58201ebdd..abee98f9b4 100644
--- a/frappe/templates/includes/comments/comments.py
+++ b/frappe/templates/includes/comments/comments.py
@@ -13,8 +13,8 @@ from frappe import _
 def add_comment(comment, comment_email, comment_by, reference_doctype, reference_name, route):
 	doc = frappe.get_doc(reference_doctype, reference_name)
 
-	if not comment.strip() or len(comment) < 10:
-		frappe.msgprint(_('Your comment should be atleast 10 characters long'))
+	if not comment.strip():
+		frappe.msgprint(_('The comment cannot be empty'))
 		return False
 
 	url_regex = re.compile(r"http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\(\),]|(?:%[0-9a-fA-F][0-9a-fA-F]))+", re.IGNORECASE)
@@ -25,8 +25,7 @@ def add_comment(comment, comment_email, comment_by, reference_doctype, reference
 		return False
 
 	if not comment_email == frappe.session.user:
-		frappe.msgprint(_('Please comment as the logged in user'))
-		return False
+		comment_email = frappe.session.user
 
 	comments_count = frappe.db.count("Comment", {
 		"comment_type": "Comment",

From 491be746c8bc25ea03f18049dae23a9a1ecb70ef Mon Sep 17 00:00:00 2001
From: "Chinmay D. Pai" <chinmaydpai@gmail.com>
Date: Thu, 2 Jul 2020 19:28:08 +0530
Subject: [PATCH 3/4] chore: add client-side empty comment validation

Signed-off-by: Chinmay D. Pai <chinmaydpai@gmail.com>
---
 frappe/templates/includes/comments/comments.html | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/frappe/templates/includes/comments/comments.html b/frappe/templates/includes/comments/comments.html
index 87241ed075..ffd09523af 100644
--- a/frappe/templates/includes/comments/comments.html
+++ b/frappe/templates/includes/comments/comments.html
@@ -91,7 +91,7 @@
 				route: "{{ pathname }}",
 			}
 
-			if(!args.comment) {
+			if(!args.comment || !args.comment.trim()) {
 				frappe.msgprint("{{ _("Please add a valid comment.") }}");
 				return false;
 			}

From f1f8d0a7224709e4bbaf9eb9f99b5b427121f5b8 Mon Sep 17 00:00:00 2001
From: "Chinmay D. Pai" <chinmaydpai@gmail.com>
Date: Tue, 7 Jul 2020 14:59:28 +0530
Subject: [PATCH 4/4] fix: use search instead of match to find url and email

Signed-off-by: Chinmay D. Pai <chinmaydpai@gmail.com>
---
 frappe/templates/includes/comments/comments.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/frappe/templates/includes/comments/comments.py b/frappe/templates/includes/comments/comments.py
index abee98f9b4..5b1509aa60 100644
--- a/frappe/templates/includes/comments/comments.py
+++ b/frappe/templates/includes/comments/comments.py
@@ -20,7 +20,7 @@ def add_comment(comment, comment_email, comment_by, reference_doctype, reference
 	url_regex = re.compile(r"http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\(\),]|(?:%[0-9a-fA-F][0-9a-fA-F]))+", re.IGNORECASE)
 	email_regex = re.compile(r"(^[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+$)", re.IGNORECASE)
 
-	if url_regex.match(comment) or email_regex.match(comment):
+	if url_regex.search(comment) or email_regex.search(comment):
 		frappe.msgprint(_('Comments cannot have links or email addresses'))
 		return False