Resolve PR review gaps across comments, security, feeds, and UX
This commit is contained in:
@@ -11,6 +11,10 @@ class AllArticlesFeed(Feed):
|
||||
link = "/articles/"
|
||||
description = "Honest AI coding tool reviews for developers."
|
||||
|
||||
def get_object(self, request):
|
||||
self.request = request
|
||||
return None
|
||||
|
||||
def items(self):
|
||||
return ArticlePage.objects.live().order_by("-first_published_at")[:20]
|
||||
|
||||
@@ -27,11 +31,16 @@ class AllArticlesFeed(Feed):
|
||||
return item.author.name
|
||||
|
||||
def item_link(self, item: ArticlePage):
|
||||
return f"{settings.WAGTAILADMIN_BASE_URL}{item.url}"
|
||||
if hasattr(self, "request") and self.request is not None:
|
||||
full_url = item.get_full_url(self.request)
|
||||
if full_url:
|
||||
return full_url
|
||||
return f"{settings.WAGTAILADMIN_BASE_URL.rstrip('/')}{item.url}"
|
||||
|
||||
|
||||
class TagArticlesFeed(AllArticlesFeed):
|
||||
def get_object(self, request, tag_slug: str):
|
||||
self.request = request
|
||||
return get_object_or_404(Tag, slug=tag_slug)
|
||||
|
||||
def title(self, obj):
|
||||
|
||||
@@ -6,10 +6,10 @@ from typing import Any
|
||||
|
||||
from django.core.paginator import EmptyPage, PageNotAnInteger, Paginator
|
||||
from django.db import models
|
||||
from django.db.models import CASCADE, PROTECT, SET_NULL
|
||||
from django.db.models import CASCADE, PROTECT, SET_NULL, Prefetch
|
||||
from modelcluster.contrib.taggit import ClusterTaggableManager
|
||||
from modelcluster.fields import ParentalKey
|
||||
from taggit.models import TaggedItemBase
|
||||
from taggit.models import Tag, TaggedItemBase
|
||||
from wagtail.admin.panels import FieldPanel, PageChooserPanel
|
||||
from wagtail.fields import RichTextField, StreamField
|
||||
from wagtail.models import Page
|
||||
@@ -62,6 +62,9 @@ class ArticleIndexPage(Page):
|
||||
ctx = super().get_context(request, *args, **kwargs)
|
||||
tag_slug = request.GET.get("tag")
|
||||
articles = self.get_articles()
|
||||
available_tags = (
|
||||
Tag.objects.filter(id__in=articles.values_list("tags__id", flat=True)).distinct().order_by("name")
|
||||
)
|
||||
if tag_slug:
|
||||
articles = articles.filter(tags__slug=tag_slug)
|
||||
paginator = Paginator(articles, self.ARTICLES_PER_PAGE)
|
||||
@@ -75,6 +78,7 @@ class ArticleIndexPage(Page):
|
||||
ctx["articles"] = page_obj
|
||||
ctx["paginator"] = paginator
|
||||
ctx["active_tag"] = tag_slug
|
||||
ctx["available_tags"] = available_tags
|
||||
return ctx
|
||||
|
||||
|
||||
@@ -168,8 +172,11 @@ class ArticlePage(SeoMixin, Page):
|
||||
def get_context(self, request, *args, **kwargs):
|
||||
ctx = super().get_context(request, *args, **kwargs)
|
||||
ctx["related_articles"] = self.get_related_articles()
|
||||
ctx["approved_comments"] = self.comments.filter(is_approved=True, parent__isnull=True).select_related(
|
||||
"parent"
|
||||
from apps.comments.models import Comment
|
||||
|
||||
approved_replies = Comment.objects.filter(is_approved=True).select_related("parent")
|
||||
ctx["approved_comments"] = self.comments.filter(is_approved=True, parent__isnull=True).prefetch_related(
|
||||
Prefetch("replies", queryset=approved_replies)
|
||||
)
|
||||
return ctx
|
||||
|
||||
|
||||
@@ -1,4 +1,8 @@
|
||||
import pytest
|
||||
from django.test import override_settings
|
||||
|
||||
from apps.blog.models import ArticleIndexPage, ArticlePage
|
||||
from apps.blog.tests.factories import AuthorFactory
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
@@ -6,3 +10,25 @@ def test_feed_endpoint(client):
|
||||
resp = client.get("/feed/")
|
||||
assert resp.status_code == 200
|
||||
assert resp["Content-Type"].startswith("application/rss+xml")
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
@override_settings(WAGTAILADMIN_BASE_URL="http://wrong-host.example")
|
||||
def test_feed_uses_request_host_for_item_links(client, home_page):
|
||||
index = ArticleIndexPage(title="Articles", slug="articles")
|
||||
home_page.add_child(instance=index)
|
||||
author = AuthorFactory()
|
||||
article = ArticlePage(
|
||||
title="Feed Article",
|
||||
slug="feed-article",
|
||||
author=author,
|
||||
summary="summary",
|
||||
body=[("rich_text", "<p>Body</p>")],
|
||||
)
|
||||
index.add_child(instance=article)
|
||||
article.save_revision().publish()
|
||||
|
||||
resp = client.get("/feed/")
|
||||
body = resp.content.decode()
|
||||
assert resp.status_code == 200
|
||||
assert "http://localhost/articles/feed-article/" in body
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
import pytest
|
||||
from taggit.models import Tag
|
||||
|
||||
from apps.blog.models import ArticleIndexPage, ArticlePage
|
||||
from apps.blog.tests.factories import AuthorFactory
|
||||
from apps.comments.models import Comment
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
@@ -29,6 +31,7 @@ def test_article_index_pagination_and_tag_filter(client, home_page):
|
||||
resp = client.get("/articles/?page=2")
|
||||
assert resp.status_code == 200
|
||||
assert resp.context["articles"].number == 2
|
||||
assert "Pagination" in resp.content.decode()
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
@@ -92,3 +95,65 @@ def test_article_page_renders_share_links_and_newsletter_form(client, home_page)
|
||||
assert "Share on LinkedIn" in html
|
||||
assert 'data-copy-link' in html
|
||||
assert 'name="source" value="article"' in html
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_article_page_renders_approved_comments_and_reply_form(client, home_page):
|
||||
index = ArticleIndexPage(title="Articles", slug="articles")
|
||||
home_page.add_child(instance=index)
|
||||
author = AuthorFactory()
|
||||
article = ArticlePage(
|
||||
title="Main",
|
||||
slug="main",
|
||||
author=author,
|
||||
summary="summary",
|
||||
body=[("rich_text", "<p>body</p>")],
|
||||
)
|
||||
index.add_child(instance=article)
|
||||
article.save_revision().publish()
|
||||
comment = Comment.objects.create(
|
||||
article=article,
|
||||
author_name="A",
|
||||
author_email="a@example.com",
|
||||
body="Top level",
|
||||
is_approved=True,
|
||||
)
|
||||
Comment.objects.create(
|
||||
article=article,
|
||||
parent=comment,
|
||||
author_name="B",
|
||||
author_email="b@example.com",
|
||||
body="Reply",
|
||||
is_approved=True,
|
||||
)
|
||||
|
||||
resp = client.get("/articles/main/")
|
||||
html = resp.content.decode()
|
||||
assert resp.status_code == 200
|
||||
assert "Top level" in html
|
||||
assert "Reply" in html
|
||||
assert f'name="parent_id" value="{comment.id}"' in html
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_article_index_renders_tag_filter_controls(client, home_page):
|
||||
index = ArticleIndexPage(title="Articles", slug="articles")
|
||||
home_page.add_child(instance=index)
|
||||
author = AuthorFactory()
|
||||
article = ArticlePage(
|
||||
title="Main",
|
||||
slug="main",
|
||||
author=author,
|
||||
summary="summary",
|
||||
body=[("rich_text", "<p>body</p>")],
|
||||
)
|
||||
index.add_child(instance=article)
|
||||
article.save_revision().publish()
|
||||
tag = Tag.objects.create(name="TagOne", slug="tag-one")
|
||||
article.tags.add(tag)
|
||||
article.save_revision().publish()
|
||||
|
||||
resp = client.get("/articles/")
|
||||
html = resp.content.decode()
|
||||
assert resp.status_code == 200
|
||||
assert "/articles/?tag=tag-one" in html
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import pytest
|
||||
from django.core.cache import cache
|
||||
from django.test import override_settings
|
||||
|
||||
from apps.blog.models import ArticleIndexPage, ArticlePage
|
||||
from apps.blog.tests.factories import AuthorFactory
|
||||
@@ -60,3 +61,71 @@ def test_comment_post_rejected_when_comments_disabled(client, home_page):
|
||||
)
|
||||
assert resp.status_code == 404
|
||||
assert Comment.objects.count() == 0
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_comment_reply_depth_is_enforced(client, home_page):
|
||||
cache.clear()
|
||||
index = ArticleIndexPage(title="Articles", slug="articles")
|
||||
home_page.add_child(instance=index)
|
||||
author = AuthorFactory()
|
||||
article = ArticlePage(title="A", slug="a", author=author, summary="s", body=[("rich_text", "<p>body</p>")])
|
||||
index.add_child(instance=article)
|
||||
article.save_revision().publish()
|
||||
|
||||
parent = Comment.objects.create(
|
||||
article=article,
|
||||
author_name="Parent",
|
||||
author_email="p@example.com",
|
||||
body="Parent",
|
||||
is_approved=True,
|
||||
)
|
||||
child = Comment.objects.create(
|
||||
article=article,
|
||||
parent=parent,
|
||||
author_name="Child",
|
||||
author_email="c@example.com",
|
||||
body="Child",
|
||||
is_approved=True,
|
||||
)
|
||||
|
||||
resp = client.post(
|
||||
"/comments/post/",
|
||||
{
|
||||
"article_id": article.id,
|
||||
"parent_id": child.id,
|
||||
"author_name": "TooDeep",
|
||||
"author_email": "deep@example.com",
|
||||
"body": "Nope",
|
||||
"honeypot": "",
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 302
|
||||
assert Comment.objects.count() == 2
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
@override_settings(TRUSTED_PROXY_IPS=[])
|
||||
def test_comment_uses_remote_addr_when_proxy_untrusted(client, home_page):
|
||||
cache.clear()
|
||||
index = ArticleIndexPage(title="Articles", slug="articles")
|
||||
home_page.add_child(instance=index)
|
||||
author = AuthorFactory()
|
||||
article = ArticlePage(title="A", slug="a", author=author, summary="s", body=[("rich_text", "<p>body</p>")])
|
||||
index.add_child(instance=article)
|
||||
article.save_revision().publish()
|
||||
|
||||
client.post(
|
||||
"/comments/post/",
|
||||
{
|
||||
"article_id": article.id,
|
||||
"author_name": "Test",
|
||||
"author_email": "test@example.com",
|
||||
"body": "Hello",
|
||||
"honeypot": "",
|
||||
},
|
||||
REMOTE_ADDR="10.0.0.1",
|
||||
HTTP_X_FORWARDED_FOR="203.0.113.7",
|
||||
)
|
||||
comment = Comment.objects.get()
|
||||
assert comment.ip_address == "10.0.0.1"
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from django.conf import settings
|
||||
from django.contrib import messages
|
||||
from django.core.cache import cache
|
||||
from django.core.exceptions import ValidationError
|
||||
from django.http import HttpResponse
|
||||
from django.shortcuts import get_object_or_404, redirect
|
||||
from django.views import View
|
||||
@@ -11,9 +13,18 @@ from apps.comments.forms import CommentForm
|
||||
from apps.comments.models import Comment
|
||||
|
||||
|
||||
def client_ip_from_request(request) -> str:
|
||||
remote_addr = request.META.get("REMOTE_ADDR", "").strip()
|
||||
trusted_proxies = getattr(settings, "TRUSTED_PROXY_IPS", [])
|
||||
x_forwarded_for = request.META.get("HTTP_X_FORWARDED_FOR", "")
|
||||
if remote_addr in trusted_proxies and x_forwarded_for:
|
||||
return x_forwarded_for.split(",")[0].strip()
|
||||
return remote_addr
|
||||
|
||||
|
||||
class CommentCreateView(View):
|
||||
def post(self, request):
|
||||
ip = (request.META.get("HTTP_X_FORWARDED_FOR") or request.META.get("REMOTE_ADDR", "")).split(",")[0].strip()
|
||||
ip = client_ip_from_request(request)
|
||||
key = f"comment-rate:{ip}"
|
||||
count = cache.get(key, 0)
|
||||
if count >= 3:
|
||||
@@ -34,6 +45,11 @@ class CommentCreateView(View):
|
||||
if parent_id:
|
||||
comment.parent = Comment.objects.filter(pk=parent_id, article=article).first()
|
||||
comment.ip_address = ip or None
|
||||
try:
|
||||
comment.full_clean()
|
||||
except ValidationError:
|
||||
messages.error(request, "Reply depth exceeds the allowed limit")
|
||||
return redirect(article.url)
|
||||
comment.save()
|
||||
messages.success(request, "Your comment is awaiting moderation")
|
||||
return redirect(f"{article.url}?commented=1")
|
||||
|
||||
@@ -37,6 +37,7 @@ def article_og_image_url(context, article) -> str:
|
||||
@register.simple_tag(takes_context=True)
|
||||
def article_json_ld(context, article):
|
||||
request = context["request"]
|
||||
nonce = getattr(request, "csp_nonce", "")
|
||||
data = {
|
||||
"@context": "https://schema.org",
|
||||
"@type": "Article",
|
||||
@@ -49,5 +50,9 @@ def article_json_ld(context, article):
|
||||
"image": _article_image_url(request, article),
|
||||
}
|
||||
return mark_safe(
|
||||
'<script type="application/ld+json">' + json.dumps(data, ensure_ascii=True) + "</script>"
|
||||
'<script type="application/ld+json" nonce="'
|
||||
+ nonce
|
||||
+ '">'
|
||||
+ json.dumps(data, ensure_ascii=True)
|
||||
+ "</script>"
|
||||
)
|
||||
|
||||
@@ -64,3 +64,38 @@ def test_article_comment_form_contains_csrf_token(client, home_page):
|
||||
html = resp.content.decode()
|
||||
assert resp.status_code == 200
|
||||
assert "csrfmiddlewaretoken" in html
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_consent_rejects_open_redirect(client, home_page):
|
||||
resp = client.post(
|
||||
"/consent/",
|
||||
{"reject_all": "1"},
|
||||
HTTP_REFERER="https://evil.example.com/phish",
|
||||
)
|
||||
assert resp.status_code == 302
|
||||
assert resp["Location"] == "/"
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_article_json_ld_script_has_csp_nonce(client, home_page):
|
||||
index = ArticleIndexPage(title="Articles", slug="articles")
|
||||
home_page.add_child(instance=index)
|
||||
author = AuthorFactory()
|
||||
article = ArticlePage(
|
||||
title="Nonce Article",
|
||||
slug="nonce-article",
|
||||
author=author,
|
||||
summary="summary",
|
||||
body=[("rich_text", "<p>Body</p>")],
|
||||
)
|
||||
index.add_child(instance=article)
|
||||
article.save_revision().publish()
|
||||
|
||||
resp = client.get("/articles/nonce-article/")
|
||||
csp = resp["Content-Security-Policy"]
|
||||
match = re.search(r"nonce-([^' ;]+)", csp)
|
||||
assert match
|
||||
nonce = match.group(1)
|
||||
html = resp.content.decode()
|
||||
assert f'type="application/ld+json" nonce="{nonce}"' in html
|
||||
|
||||
@@ -2,6 +2,7 @@ from __future__ import annotations
|
||||
|
||||
from django.http import HttpRequest, HttpResponse, HttpResponseNotAllowed
|
||||
from django.shortcuts import redirect, render
|
||||
from django.utils.http import url_has_allowed_host_and_scheme
|
||||
|
||||
from apps.core.consent import ConsentService
|
||||
|
||||
@@ -24,6 +25,12 @@ def consent_view(request: HttpRequest) -> HttpResponse:
|
||||
advertising = request.POST.get("advertising") in {"true", "1", "on"}
|
||||
|
||||
target = request.META.get("HTTP_REFERER", "/")
|
||||
if not url_has_allowed_host_and_scheme(
|
||||
url=target,
|
||||
allowed_hosts={request.get_host()},
|
||||
require_https=request.is_secure(),
|
||||
):
|
||||
target = "/"
|
||||
response = redirect(target)
|
||||
ConsentService.set_consent(response, analytics=analytics, advertising=advertising)
|
||||
return response
|
||||
|
||||
Reference in New Issue
Block a user